[Security][Guard] GuardAuthenticationProvider::authenticate cannot return null #27016
Conversation
|
I can confirm that this fix is working |
| return $this->authenticateViaGuard($guardAuthenticator, $token); | ||
| } | ||
| if (null === $guardAuthenticator) { | ||
| throw new AuthenticationException('Cannot find guard authenticator this token originated from.'); |
There was a problem hiding this comment.
Would it be an idea to add some token info? Perhaps the GuardProviderKey
|
I've added the key to the exception and also added some tests. |
There was a problem hiding this comment.
Thanks, looks great!
As it's a bug, I think it can be merged in 2.8. However, the core team can change this for you while merging.
There is a small BC break in here, in case someone overwrote supports() to make guard authenticator support non-GuardTokenInterface tokens. But that's just weird and an edge case, so I think we can ignore it.
| // no matching authenticator found - but there will be multiple GuardAuthenticationProvider | ||
| // instances that will be checked if you have multiple firewalls. | ||
|
|
||
| return null; |
There was a problem hiding this comment.
does Symfony use explicit null returns at the end of function bodies? (I'm not sure, so don't immediately change this)
There was a problem hiding this comment.
yes it does :) this method could have a ?AuthenticatorInterface return type starting from 4.0, explicit null would be mandatory
There was a problem hiding this comment.
Those two are mutually exclusive, a nullable return type cannot have an implicit return value of null. Should I fix something?
There was a problem hiding this comment.
Sorry, read your comment wrong, thought it read 'implicit null' for some reason. Shall I add the return type?
There was a problem hiding this comment.
@biomedia-thomas nope. If this patch gets merged, it must be on 2.8 (the lowest maintained branch where it applies) where return types cannot be added due to support of old PHP versions. We'll take care of adding it when merging it up to master
dd9769c to
575fe1a
Compare
| 'Token with provider key "%s" did not originate from any of the guard authenticators of provider "%s".', | ||
| $token->getGuardProviderKey(), | ||
| $this->providerKey | ||
| )); |
575fe1a to
aef5e16
Compare
…ll according to interface specification
aef5e16 to
9dff22c
Compare
|
Thank you @biomedia-thomas. |
…e cannot return null (biomedia-thomas) This PR was merged into the 2.8 branch. Discussion ---------- [Security][Guard] GuardAuthenticationProvider::authenticate cannot return null | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #26942 | License | MIT Authenticate method in GuardAuthenticationProvider returned null when the token does not originate from any of the guard authenticators. This check was not done in the supports method. According to the interface authenticate cannot return null. This patch copies theguard authenticator checks to the supports method. Commits ------- 9dff22c [Security] guardAuthenticationProvider::authenticate cannot return null according to interface specification
|
Congratz for your first contrib to Symfony @biomedia-thomas, thanks |
Authenticate method in GuardAuthenticationProvider returned null when the token does not originate from any of the guard authenticators. This check was not done in the supports method. According to the interface authenticate cannot return null. This patch copies theguard authenticator checks to the supports method.