NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[Security] fixed pre/post authentication checks#9902

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

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:2.3fromfabpot:security-checks

Dec 31, 2013

Merged

[Security] fixed pre/post authentication checks#9902

fabpot merged 1 commit intosymfony:2.3fromfabpot:security-checks

Dec 31, 2013

Conversation

Copy link

Member

fabpot commentedDec 30, 2013

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#8510,#9622
License	MIT
Doc PR	N/A

After further investigation of#8510, I found that all checks in thecheckPreAuth actually belongs tocheckPostAuth and the same goes for checks inCheckPostAuth (I checked the original source from Spring and indeed, that's how it is implemented there: seehttps://github.com/spring-projects/spring-security/blob/master/core/src/main/java/org/springframework/security/authentication/dao/AbstractUserDetailsAuthenticationProvider.java#L305
).

So, this PR fixes that issue. I think that we can do this change safely in 2.3 as the error message is the same for all causes by default ($hideUserNotFoundExceptions istrue by default inUserAuthenticationProvider).

The only "real" change is whether the authentication is checked or not.

fabpot mentioned this pull request

Dec 30, 2013

[Security] Check account isn't locked before user authentication#9622

Closed

[Security] fixed pre/post authentication checks

ada82a2

fabpot added a commit that referenced this pull request

Dec 31, 2013

bug#9902[Security] fixed pre/post authentication checks (fabpot)

3704e08

This PR was merged into the 2.3 branch.Discussion----------[Security] fixed pre/post authentication checks| Q             | A| ------------- | ---| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#8510,#9622| License       | MIT| Doc PR        | N/AAfter further investigation of#8510, I found that all checks in the `checkPreAuth` actually belongs to `checkPostAuth` and the same goes for checks in `CheckPostAuth` (I checked the original source from Spring and indeed, that's how it is implemented there: seehttps://github.com/spring-projects/spring-security/blob/master/core/src/main/java/org/springframework/security/authentication/dao/AbstractUserDetailsAuthenticationProvider.java#L305).So, this PR fixes that issue. I think that we can do this change safely in 2.3 as the error message is the same for all causes by default (`$hideUserNotFoundExceptions` is `true` by default in `UserAuthenticationProvider`).The only "real" change is whether the authentication is checked or not.Commits-------ada82a2 [Security] fixed pre/post authentication checks