spring-projects / spring-projects/spring-security

Think about changing how BindAuthenticator fetches attributes

Open
#8,727 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

in: ldap type: enhancement
Dominant language
Java
Stars
9.6k
Forks
6.3k
Avg merge
2d 11h
Merged PRs (30d)
52

Description

As requested, I am splitting the long and unclear #8560 into several issues. This issue is about (thinking about) changing how BindAuthenticator fetches attributes.

Current behaviour

I've documented the behaviour in #8725. I suspect you'll agree that it's confusing.

Better behaviour

One of:

  • with both bind strategies, we fetch the attributes with the same
    permissions
  • we let the user decide which permissions to fetch attributes with

Context

Once upon a time, BindAuthenticator did use the same permissions -- the user's permissions -- regardless of the bind strategy employed. (This was the case as of a790c7e192.) There was a suggestion (#1738) to change this, and later a pull request. Luke Taylor (who wrote the class) said, in the issue, that he would not change this behaviour because (I'm paraphrasing) it would complicate the class, and people who wanted something different could just write their own authenticators/authentication providers. Rob Winch denied the pull request with the same reasoning. This is, incidentally, entirely sensible.

Later, Cloudfoundry had an issue. They wanted to fetch attributes with the app user's permissions. At that point in time that was impossible without writing your own authenticator.

Cloudfoundry decided this was a Spring Security issue. They made a pull request which was accepted (6b436ff4099694f). The pull request changed what the search-and-bind strategy did, but did not change what the dnPatterns strategy did. That is why the present behaviour is inconsistent.

How to get better behaviour

First, note that it's not necessarily bad to do nothing. The behaviour is confusing, but it's not buggy. Also, while I'm not necessarily the only person to have run into it, I am (afaik) the only person to have posted an issue about it in 4 years, so it doesn't seem to bother your users much.

If you want to change it, I see several ways to do so:

  • always fetch with the user's permissions (i.e. revert 6b436ff4099694f)
  • always fetch with the app user's permissions (probably this entails doing a
    search with object scope when given dnPatterns)
  • give the user (of the class) more choice -- maybe let them configure
    multiple searches, where a search is a root and a scope and a filter and
    an attribute fetching strategy (this is a lot of breaking changes and
    complexity for not very much gain; see, once again, this)

The way I'd recommend is the first one. That pull request, I think, shouldn't have been accepted; after all, #1738, which makes essentially the same request, was declined.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the BindAuthenticator entry point and read the behavior documented in #8725, then compare how its two bind strategies fetch attributes. The issue presents several incompatible policy options rather than a defined change, so completion requires deciding which permissions model Spring Security should support and resolving the compatibility and complexity concerns described here.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, spring
Domain
authentication, backend, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.