Skip to content

[Ldap] Ldap username case fix #21291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[Ldap] Ldap username case fix #21291

wants to merge 2 commits into from

Conversation

quentinus95
Copy link
Contributor

@quentinus95 quentinus95 commented Jan 14, 2017 •
edited by nicolas-grekas
Loading

Q A
Branch? 3.1
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #20486
License MIT
Doc PR

@quentinus95
Copy link
Contributor Author

@csarrazi I have recreated a new PR and deleted the old one because I merged the whole master branch in it inadvertently (I forgot it was based on 3.1)

I have removed the null check in getAttributeValue method and set a default value for $uidKey if null given. I also set the password to null if the password attribute is null.

@quentinus95 quentinus95 changed the title Ldap case fix [Ldap] Ldap username case fix Jan 14, 2017
@nicolas-grekas
Copy link
Member

Which branch is this fix for? There's a mismatch between the description and the selected base branch (no need to change because we can while merging.)

@quentinus95
Copy link
Contributor Author

This fix is for the 3.1 branch, sorry!

@nicolas-grekas nicolas-grekas added this to the 3.1 milestone Jan 16, 2017
@nicolas-grekas
Copy link
Member

ping @csarrazi (3.1 is EOLed in two weeks)

@csarrazi
Copy link
Contributor

csarrazi commented Jan 16, 2017 •
edited
Loading

👍 , as I said in the previous PR. I would merge it against 3.1, as we still support it (even though EOL is in 2 weeks).

@quentinus95
Copy link
Contributor Author

Should I care about the failure on AppVeyor as it does not seems related to the changes of this PR ?

@fabpot
Copy link
Member

fabpot commented Jan 17, 2017

Thank you @quentinus95.

fabpot added a commit that referenced this pull request Jan 17, 2017
@fabpot
bug #21291 [Ldap] Ldap username case fix (quentinus95)
0dd6a90 
This PR was submitted for the master branch but it was merged into the 3.1 branch instead (closes #21291).

Discussion
----------

[Ldap] Ldap username case fix

| Q             | A
| ------------- | ---
| Branch?       | 3.1
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #20486
| License       | MIT
| Doc PR        |

Commits
-------

c91689b [Ldap] Using Ldap stored username instead of form submitted one
6641b79 [Ldap] load users with the good username case
@fabpot fabpot closed this Jan 17, 2017
@fabpot fabpot mentioned this pull request Jan 28, 2017
Merged
@fabpot fabpot mentioned this pull request Feb 6, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Bug Status: Needs Review
Projects
None yet
Milestone
3.1
Development

Successfully merging this pull request may close these issues.
5 participants
@quentinus95 @nicolas-grekas @csarrazi @fabpot @carsonbot