8000 Fix compatibility of ldap 6.0 with security 5.x by jderusse · Pull Request #45804 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Fix compatibility of ldap 6.0 with security 5.x #45804

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
Merged
merged 1 commit into from
Mar 24, 2022
Merged

Fix compatibility of ldap 6.0 with security 5.x #45804

merged 1 commit into from
Mar 24, 2022

Conversation

jderusse
Copy link
Member
Q A
Branch? 6.0
Bug fix? yes
New feature? no
Deprecations? yes
Tickets -
License MIT
Doc PR -

In version 5.4, the LdapAuthenticator class implements the AuthenticatorInterface and therefor implemented the deprecated createAuthenticatedToken method.

In version 6.0, we removed that method from both the interface and the LdapAuthenticator class.

But the 2 class/interface are located in 2 differents packages, an our composer.json allows the combination symfony/ldap:6.0 + synfony/security-http:5.4. Leading to a fatal error Class Symfony\Component\Ldap\Security\LdapAuthenticator contains 1 abstract method and must therefore be declared abstract or implement the remaining methods

This PR re-add the method in the ldap component.

8000 @jderusse jderusse requested review from wouterj and chalasr as code owners March 21, 2022 19:14
@carsonbot carsonbot added this to the 6.0 milestone Mar 21, 2022
@carsonbot
Copy link

Hey!

I think @karlshea has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

nicolas-grekas
nicolas-grekas reviewed Mar 22, 2022
View reviewed changes
@wouterj
Copy link
Member
wouterj commented Mar 22, 2022

Hmm, I'm wondering how this slipped through the testsuite. Are we missing a compatibility test check here?

@chalasr
Copy link
Member
chalasr commented Mar 22, 2022

The LdapAuthenticator is not tested at all.

@jderusse jderusse force-pushed the ldap-comp branch 2 times, most recently from 6cd65d1 to ba3e8e4 Compare March 23, 2022 08:03
@jderusse jderusse mentioned this pull request Mar 23, 2022
Merged
@jderusse jderusse force-pushed the ldap-comp branch 2 times, most recently from 20cfff2 to 5db5381 Compare March 23, 2022 08:16
fabpot added a commit that referenced this pull request Mar 23, 2022
@fabpot
minor #45825 [Ldap] Add missing upgrade note for ldap (jderusse)
42bb472 
This PR was merged into the 5.4 branch.

Discussion
----------

[Ldap] Add missing upgrade note for ldap

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

As suggested in #45804 (comment)

Commits
-------

cd4d655 Add missing upgrade note for ldap
chalasr
chalasr reviewed Mar 23, 2022
View reviewed changes
@nicolas-grekas
Copy link
Member
nicolas-grekas commented Mar 23, 2022 •
edited
Loading

GH is having issues closing PRs 😬

@fabpot
Copy link
Member
fabpot commented Mar 24, 2022

Thank you @jderusse.

@fabpot fabpot merged commit 7d1aea4 into symfony:6.0 Mar 24, 2022
@fabpot fabpot mentioned this pull request Apr 2, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@fabpot fabpot fabpot approved these changes

@chalasr chalasr chalasr approved these changes

@wouterj wouterj Awaiting requested review from wouterj wouterj is a code owner

Assignees
No one assigned
Labels
Bug Deprecation Status: Reviewed
Projects
None yet
Milestone
6.0
Development

Successfully merging this pull request may close these issues.
6 participants
@jderusse @carsonbot @wouterj @chalasr @nicolas-grekas @fabpot
0