8000 Wrong example in authenticateToken for SimplePreAuthenticationListener · Issue #4148 · symfony/symfony-docs · GitHub
[go: up one dir, main page]
Skip to content

Wrong example in authenticateToken for SimplePreAuthenticationListener #4148

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
s7anley opened this issue Aug 19, 2014 · 22 comments
Closed

Wrong example in authenticateToken for SimplePreAuthenticationListener #4148

s7anley opened this issue Aug 19, 2014 · 22 comments

Comments

@s7anley
Copy link
s7anley commented Aug 19, 2014

In 2nd code example in storing authentication in the session section, it's not possible to get current User with $user = $token->getUser();. SimplePreAuthenticationListener is skipped when current user is not authenticate with AnonymousToken, which in this case is true.

Or maybe I'm missing something?
Thanks.
@weaverryan
Copy link
Member

@peterrehm what do you think here?

@peterrehm
Copy link
Contributor

@s7anley: It actually works in my case which I used when creating the docs. What do you mean with the Listener is skipped if you have an AnonymousToken?

@peterrehm
Copy link
Contributor

@s7anley I reconsidered and for now I cannot see any problem. If the token does not contain a valid user it is going through the normal authentication process. Is your authentication working on the first request when you provide credentials?

@peterrehm
Copy link
Contributor

@s7anley Can you provide further information? Or can we close this issue?

@s7anley
Copy link
Author
s7anley commented Aug 27, 2014

Sorry for my late response. I guess my question is not really well asked and mixed couple issues together. I wrote it during my first deeper investigation of Security component.

So my question is regrading part where instance of User is pulled from Token e.g. if ($user instanceof User) and then used to created PreAuthenticatedToken. In my option this case not occur, because you users is always set as anon.. Still, I just Maybe overlooked something or incorrectly configure Symfony.

@peterrehm
Copy link
Contributor

@s7anley This case occurs on the second request. If you get authenticated on the first request based on the provided credentials the user data will get stored in the token. On the next request the user is unserialized and you can get those data back from the token. (The user needs usually to be refreshed in this step).

It should work properly without any issues. This way was needed since actually the previous example was wrong. The previous example tried to re-authenticate based on credentials, but the credentials are not stored in the token/session due to security concerns.

@s7anley
Copy link
Author
s7anley commented Aug 28, 2014

@peterrehm And this 2nd request you mentioned is problem what I have described in first post. Once you are authenticated and token is restored from session in ContextListener, the SimplePreAuthenticationListener is skipped.

@peterrehm
Copy link
Contributor

And I cannot confirm this one. Just repoduced this with the following steps:

Firewall Configuration

    main:
        pattern:    ^/
        simple-preauth:
            provider: fos_userbundle
            authenticator: my_bundle.pre_auth_authenticator
        stateless: false
        anonymous: ~

And the Authenticator similar to what is described in the Article.

In the first request I provide the api key.

http://my_app/fancy/?apikey=12345

And I will get authenticated and the user will be stored to the session.

On the next request however the createToken() will not be called any more,
just the authenticateToken(). And the authenticateToken() will get a token of
the type Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken.
From this token you can fetch the user.

Everything is working as expected and working fine. The Listener is not being skipped.
I just tried this again in the actual app without any issues.

Relates to #4076

@peterrehm
Copy link
Contributor

@weaverryan If I am not proven to be wrong this can be closed.

@s7anley
Copy link
Author
s7anley commented Aug 29, 2014

@peterrehm authenticateToken is not called in 2nd request for me. Not sure how is possible, when ApiKeyAuthenticator is not used. For testing I use Symfony SE 2.5.3, built in server and sessions stored to filesystem. You may close this issue, but I'm not sure about this.

And also it's not necessary to inject userProvider to ApiKeyAuthenticator, because it's also 2nd argument of authenticateToken method.

@peterrehm
Copy link
Contributor

Can you upload a branch of the SE so that I can reproduce the bug? In my App based on 2.5.3 it is working as expected.

@peterrehm
Copy link
Contributor

@s7anley Any news?

@xabbuh
Copy link
Member
xabbuh commented Oct 11, 2014

Is this still an issue or is this also fixed with symfony/symfony#11414 being merged?

@PapyDanone
Copy link
Contributor

hey @peterrehm in your attempt to reproduce this issue does it still work if you remove anonymous: ~ from your firewall?

I'm actually getting an exception on 2nd request too and can somewhat relate to what @s7anley was saying.

I'm on SE 2.6.3.

Here's my security.yml:

secured_area:
     pattern: ^/
     stateless: false
     simple-preauth:
           authenticator: api_key_authenticator

The exception:

Uncaught PHP Exception Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException: "A Token was not found in the SecurityContext." at /Users/jeromeguilbot/Sites/tma-api/vendor/symfony/symfony/src/Symfony/Component/Security/Http/Firewall/AccessListener.php line 53

In my code, the only thing that differs from the example is, in ApiKeyAuthenticator.php:

public function createToken(Request $request, $providerKey)
{
        // look for an apikey query parameter
        $apiKey = $request->query->get('apikey');

        // or if you want to use an "apikey" header, then do something like this:
        // $apiKey = $request->headers->get('apikey');

        if (!$apiKey) {
            return null;
        }

        return new PreAuthenticatedToken(
            'anon.',
            $apiKey,
            $providerKey
        );
}

Basically I want the user to be either authenticated by an API key in the url or a token in the session if it is found, and that for all requests.

Am I missing something?

@peterrehm
Copy link
Contributor

You can omit the anonymous setting, I needed it since you can login either by a form or by the api key. The application supports both.

But if I recall correctly you should not return null, just return an empty apiKey in the PreAuthenticated Token or

public function createToken(Request $request, $providerKey)
{
        // look for an apikey query parameter
        $apiKey = $request->query->get('apikey');

        // or if you want to use an "apikey" header, then do something like this:
        // $apiKey = $request->headers->get('apikey');

        if (!$apiKey) {
            throw new BadCredentialsException('No Auth token found');
        }

        return new PreAuthenticatedToken(
            'anon.',
            $apiKey,
            $providerKey
        );
}

I assume based on the provided error message you somehow have no apiKey and therefore no token is returned.

@PapyDanone
Copy link
Contributor

Thanks for your quick response.

Indeed, I only pass the API key as a parameter in first request.

If, as you suggest, I return an empty apiKey in the PreAuthenticated token as follows:

if (!$apiKey) {
    return new PreAuthenticatedToken(
           'anon.',
           null,
           $providerKey
    );
}

I get the Authentication Failed. API Key "" does not exist. error thrown in the authenticateToken method.

As @s7anley was saying, $user = $token->getUser(); doesn't work on the 2nd pass (it contains the string anon. in my case, since it is what gets returned in createToken()). Also note that createToken is indeed called for every subsequent request, contrary to what you seemed to say earlier: "On the next request however the createToken() will not be called any more".

Does that clarify the situation a bit?

@peterrehm
Copy link
Contributor

Actually not really. IIRC createToken must not be called it there is the user/token in the session.
It looks like your token is not being stored to the session. It must be related to that. Have you found
out some more informations?

@PapyDanone
Copy link
Contributor

I think I went to the bottom of it.

In my case the refreshUser() method from ApiKeyUserProvider.php was never called on the second request. Instead, it was trying to call the refreshUser() of the default in_memory provider that you get with a fresh Symfony install. The user not being found, it would then completely invalidate the token with $this->context->setToken($token), $token being null;

Replacing the default provider with:

security:
    providers:
        simple_preauth_provider:
            id: api_user_provider

with api_user_provider referencing the service defined in the example, the whole thing finally works. I'm guessing it's been working for you @peterrehm all along because you had provider: fos_userbundle defined as the user provider, which would have reloaded the user from the database every time.

So I think we should simply update the docs with the snippet above... Am I missing something?

@peterrehm
Copy link
Contributor

I just had a look at it and must admit you are right. Registering the user provider has been abstracted in the article with just a short reference to the corresponding doc section about registering a custom user provider.

If you have no registered user provider it breaks obviously.

@peterrehm peterrehm mentioned this issue Jan 21, 2015
Closed
@peterrehm
Copy link
Contributor

Just added a PR.

weaverryan added a commit that referenced this issue Jan 30, 2015
@weaverryan
feature #4895 Added configuration of the user provider (peterrehm)
50c5a9e 
This PR was submitted for the master branch but it was merged into the 2.5 branch instead (closes #4895).

Discussion
----------

Added configuration of the user provider

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | all
| Fixed tickets | #4148

This should make it clearer that a user provider must be registered.

Commits
-------

075b652 Removed unneeded spaces
56dd365 Updated as per discussion
c4cbd84 Updated according to comment and changed to AppBundle
a6fb18c Added configuration of the your_api_key_user_provider as user provider
@PapyDanone
Copy link
Contributor

Thanks for updating this @peterrehm

@peterrehm
Copy link
Contributor

Thanks for investigating it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@weaverryan @s7anley @PapyDanone @xabbuh @peterrehm
0