8000 [Security] SimpleAuthenticationProvider is lacking a user-checker · Issue #26314 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Security] SimpleAuthenticationProvider is lacking a user-checker #26314

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
linaori opened this issue Feb 26, 2018 · 0 comments
Closed

[Security] SimpleAuthenticationProvider is lacking a user-checker #26314

linaori opened this issue Feb 26, 2018 · 0 comments
Labels
Bug Security Status: Needs Review

Comments

@linaori
Copy link
Contributor
linaori commented Feb 26, 2018
Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 2.7 or 2.8

2.8 introduced custom user-checkers, but 2.7 already misses the injection. Would have to check which version to add it to.

This issue is coming from symfony/symfony-docs#9338. The SimpleAuthenticationProvider does not have a user-checker, unlike all other providers (except for anonymous). This means that the developer has to manually inject a user-checker in their SimpleAuthenticatorInterface implementation. Additionally this means that the configured user-checker is not passed, which causes different behavior.

Fixing this is quite simple, make sure the factory adds it as argument in the provider and call the methods at the right now. I'd consider this a bug fix (while it's also a "new" feature). In theory it should be 100% backwards compatible, code and behavioral wise. There are 2 scenarios where I foresee an issue:

  • A custom user-checker was defined for this firewall, but never used (or a second custom was injected as shown by the PR in docs).
  • No user-checker was defined, but the user implements the AdvancedUserInterface and was never checked before

In both scenarios, making the default (or configured) user-checker function properly, could add extra checks to the authentication process, which would narrow down whether or not a user can successfully authenticate. However, I consider this a bug despite being a behavioral change.

If the developer has added the user-checker manually in the implementation, it will still be executed, this will keep working. If the same user-checker is configured in the firewall, it will still be executed, albeit at a different location, changing the flow slightly.
@i3or1s i3or1s mentioned this issue Mar 2, 2018
Closed
nicolas-grekas added a commit that referenced this issue Mar 19, 2018
@nicolas-grekas
bug #26370 [Security] added userChecker to SimpleAuthenticationProvid…
7753282 
…er (i3or1s)

This PR was submitted for the 2.8 branch but it was squashed and merged into the 2.7 branch instead (closes #26370).

Discussion
----------

[Security] added userChecker to SimpleAuthenticationProvider

[Security] added userChecker to SimpleAuthenticationProvider
[SecurityBundle] [DependencyInjection] updated SimpleFormFactory

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

Introduces user checker to the simple authentication provider.

Commits
-------

cb9c92d [Security] added userChecker to SimpleAuthenticationProvider
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Security Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@nicolas-grekas @linaori @xabbuh @carsonbot
0