Skip to content

SimpleAutheticationProvider anonymous user handle #26871

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
berislavsapina opened this issue Apr 9, 2018 · 11 comments
Closed

SimpleAutheticationProvider anonymous user handle #26871

berislavsapina opened this issue Apr 9, 2018 · 11 comments

Comments

@berislavsapina
Copy link

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 3.4.8

Two commits that are added in class SimpleAuthenticationProvider break our application.

Commit #1: c318306
Commit #2: cb9c92d

Problem is when I have a token class that have anonymous user. So in getUser() i get an empty string. ('')

Also in last version it wasn't necessary to pass $user object, now it is.

It is needed to find way how to handle public token when we have anonymous user.

Corresponding PR #26370

Thanks for your time :D

@linaori
Copy link
Contributor

linaori commented Apr 10, 2018

So in your case this returns an empty string? $user = $authToken->getUser();

It is needed to find way how to handle public token when we have anonymous user.

There's a different authenticator for anonymous users: anonymous: ~ in your firewall. You should most likely not be creating an anonymous user manually.

@berislavsapina
Copy link
Author

berislavsapina commented Apr 10, 2018

Hi, even if I add anonymous: ~ in my firewall, still SimpleAuthenticationProvider is called and there it breaks.

@chalasr
Copy link
Member

chalasr commented Apr 10, 2018

What about an early return if $token instanceof AnonymousToken to restaure the previous behavior?

@berislavsapina
Copy link
Author

This one if (empty($user)) { return $authToken; }
will resolve my problem. I do not know is the best solution ??

@chalasr
Copy link
Member

chalasr commented Apr 10, 2018

Are you creating the AnonymousToken object in your code? Unless I'm mistaken $user should be anon. here. Anyway I don't think we want to return for any empty user, only anonymous ones.

See also

if (null !== $this->tokenStorage->getToken() && !$this->tokenStorage->getToken() instanceof AnonymousToken) {
return;
}

@berislavsapina
Copy link
Author

You are right. I agree with you :D

@aschempp
Copy link
Contributor

What about an early return if $token instanceof AnonymousToken to restaure the previous behavior?

Be aware that this is not necessarily correct. At least for the AuthenticationTrustResolver, the token class that is supposed to be considered anonymous can be configured: https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml#L8

@linaori
Copy link
Contributor

linaori commented Apr 15, 2018

Parameters are not an extension point you should rely on. Previously all parameter based class names for services were removed. I'm not sure why this one still exists, because it's extremely fragile to make a token like this. I didn't find any documentation on this either.

@aschempp
Copy link
Contributor

I totally agree, just saying you can configure the ToBeConsideredAnonymousToken in the trust resolver, so just assuming AnonymousToken is the one to look for is not necessarily true 😉

@NaxYo
Copy link

NaxYo commented Apr 25, 2018

Same issue here, breaking on version 2.8.38. Similar scenario with 'anon.' user token from a custom authenticator.

I'm trying to understand the AnonymousToken + UsernameNotFoundException thing, but can't really get it working just with changes on my Authenticator and Provider... am I missing something?

@chalasr
Copy link
Member

chalasr commented Apr 25, 2018

See #27044

fabpot added a commit that referenced this issue Apr 25, 2018
…ace (chalasr)

This PR was merged into the 2.7 branch.

Discussion
----------

[Security] Skip user checks if not implementing UserInterface

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #26871
| License       | MIT
| Doc PR        | n/a

Commits
-------

384acf9 [Security] Skip user checks if not implementing UserInterface
@fabpot fabpot closed this as completed Apr 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants