Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Ldap] Allow multiple values onextra_fields#49187

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.

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

Already on GitHub?Sign in to your account

Conversation

@mvhirsch
Copy link
Contributor

QA
Branch?5.4
Bug fix?yes
New feature?no
Deprecations?no
Tickets~
LicenseMIT
Doc PR

Loading users with theLdapUserProvider, usingextra_fields fails if I want to getmemberOf attribute.
This happens, if my user has multiple attributes set. After taking a look atLdap\Entry (which supports multiple values per attribute). Looking atLdapUserProvider::getAttributeValue() it was very clear, that every attribute must be unique.

The methodgetAttributeValue() has originated fromgetPassworddbf45e4, forcing the password attribute to be unique. Same goes foruidKeyc91689b. Loadingextra_fields should allow for multiple values per attribute (likememberOf).

Did I break backward compatibility? No, I didn't. Simply because it was never usable as array while loading users viaLdapUserProvider, instead anInvalidArgumentException was thrown.

@mvhirschmvhirschforce-pushed thebugfix-multiple-ldap-extra-fields branch from765d893 toe51b502CompareFebruary 1, 2023 13:36
@OskarStark
Copy link
Contributor

OskarStark commentedFeb 1, 2023•
edited
Loading

One could argue, that this is a new feature, but I am for handling this as a bugfix and merge it in5.4

@nicolas-grekas
Copy link
Member

Thank you@mvhirsch.

@nicolas-grekasnicolas-grekas merged commit69f113a intosymfony:5.4Feb 1, 2023
@mvhirschmvhirsch deleted the bugfix-multiple-ldap-extra-fields branchFebruary 1, 2023 20:36
This was referencedFeb 28, 2023
@DemigodCode
Copy link
Contributor

DemigodCode commentedApr 11, 2023•
edited
Loading

This "fix" breaks BC in my case.
We're loading the mail attribute of the user from ldap. That is a single-value.
Now I have to access all attributes like this: "$user->getExtraFields()['mail'][0]" before: "$user->getExtraFields()['mail']".

Had to update all occurences. An upgrade notice would be very nice for that.

That happens for all extraFields that are not "uidKey" or "passwordAttribute".
Each of those fields are now returned as array and not as string.

@OskarStark
Copy link
Contributor

OskarStark commentedApr 11, 2023•
edited
Loading

I think a BC break as not planned here.

Not sure how to fix this.

javiereguiluz added a commit to symfony/symfony-docs that referenced this pull requestMar 13, 2024
…ds()` (mvhirsch)This PR was merged into the 5.4 branch.Discussion----------[Ldap] Adds return value hint of `LdapUser::getExtraFields()`Hi,I'm not sure if this is the correct approach to document that. Please advice me where to document this. I'd love to get this done correctly.-Fixessymfony/symfony#49994- refssymfony/symfony#49187Commits-------f75e437 adds hint of return value
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@OskarStarkOskarStarkOskarStark approved these changes

@wouterjwouterjAwaiting requested review from wouterj

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

5 participants

@mvhirsch@OskarStark@nicolas-grekas@DemigodCode@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp