[Ldap] Use discovered DNs for the authentication bind#16827
[Ldap] Use discovered DNs for the authentication bind#16827junowilderness wants to merge 1 commit intosymfony:masterfrom junowilderness:master
Conversation
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | yes - makes the authentication more flexible |
| BC breaks? | yes - when finished, would invalidate the need for the dn_string configuration setting, which has not yet been removed. |
| Deprecations? | no |
| Tests pass? | no - still need fixing |
| Fixed tickets | #16823 |
| License | MIT |
| Doc PR | TBD |
|
@csarrazi Can you have a look at this one? |
|
I have nothing against the principle, but instead of adding a new interface, and a This will be made much easier when #15994 will have been merged. |
|
The DN is available from the entry's data, so we should rather set the username to be the user's DN in the UserProvider. |
|
Thus, no need to create a new user class, or change the authentication provider, a simple modification of the user provider should be enough. |
|
If time permits, today. |
|
This is not complete. We may wish to remove dn_string. But I think this is close to what you intend. |
|
The problem is that this will break compatibility with any user provider which does not fetch users from an Ldap server. This is the actual goal of dn_string. By default, it is equal to The code for the authentication provider needs to work with both:
This means that the dn_string should be needed, and that we actually need to do things in two steps:
Regardless, the user provider part is correct. You just need to adapt the authentication provider, and we should be okay. |
|
@cilefen Will you have time to finish this one in the coming days/weeks? I would love to be able to merge this one before 3.1 code freeze. Thanks. |
|
@fabpot I regret out of my ignorance I have difficulty with the development process of the project, and although I appreciate the detailed reviews, I am confused about the direction to take, and I have little time to commit to giving this the attention it deserves. |
|
@cilefen @fabpot Want me to take over the rest of the PR? |
|
@csarrazi Do you have time to take over this one? |
|
@fabpot Can do, but not until next week, as I am quite busy these days. |
|
@csarrazi Thanks! |
|
Closing for now, waiting for @csarrazi to fix the original issue. |