Skip to content
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

[Security] API Key Authentication with Session #4060

Closed
peterrehm opened this issue Jul 25, 2014 · 6 comments
Closed

[Security] API Key Authentication with Session #4060

peterrehm opened this issue Jul 25, 2014 · 6 comments

Comments

@peterrehm
Copy link
Contributor

I assume there is an issue with storing the authentication within the session in the following doc entry.

http://symfony.com/doc/current/cookbook/security/api_key_authentication.html#storing-authentication-in-the-session

The (in the article) above mentioned authenticateToken() method relies only on the provided token / the credentials. But after the authentication the credentials are being erased by default. To adjust the situation we could add

        if (($user = $token->getUser()) instanceof User) {
            return new PreAuthenticatedToken(
                $user,
                $authToken,
                $providerKey,
                $user->getRoles()
            );
        }

to return the user stored in the session (which is at this step after the refresh of the provider). If it is as I describe here I will create a doc PR.

/cc @Seldaek

@Seldaek
Copy link
Member

Seldaek commented Jul 28, 2014

I am not sure I got what you mean correctly, nor what you are trying to achieve exactly, but if you have time please start a doc PR and then we can discuss on that.

@peterrehm
Copy link
Contributor Author

On the first request you are calling the following route:

https://my-app/api_login?token=12345

In createToken() a token with the credentials 12345 is created. In the next step autenticateToken() you get authenticated.

On the next request however the credentials will be empty since you are just calling

https://my-app/fancy_action

expecting to be already authenticated. Since there are no credentials provided any more the authenticateToken() will fail to authenticate. If I check if the token contains a valid user I can
directly return the PreAuthenticatedToken as described above.

If you just follow the docs it was not working for me since authenticateToken() was failing on every second request. Shall I update the docs or am I missing something?

@Seldaek
Copy link
Member

Seldaek commented Jul 29, 2014

OK I see what you're getting at now. I think it's a misunderstanding though. The first line of the page says:

Nowadays, it's quite usual to authenticate the user via an API key (when developing a web service for instance). The API key is provided for every request and is passed as a query string parameter or via an HTTP header.

Rather than having to first login and then query again using a session cookie or something you got in the login response, it makes more sense to just run the request you want to do in the first place with the auth token, and be authenticated on the fly on every request.

Your approach might be valuable as an alternative approach though if you want to submit it but I would add that as another sub-section in the page rather than tweak the existing one.

@peterrehm
Copy link
Contributor Author

I am actually adressing the section about storing the token in the session below which I think is wrong.

http://symfony.com/doc/current/cookbook/security/api_key_authentication.html#storing-authentication-in-the-session

I was not able to get it to work this way due to the above mentioned issue. So then I think it should be fixed in this section?

@Seldaek
Copy link
Member

Seldaek commented Jul 29, 2014

Oh sorry I didn't read that carefully enough. Then yes perhaps you're right. I haven't touched this code in a while though so I am not 100% and I'd appreciate if anyone else reviews this, but it sounds like the credentials being erased is indeed a problem.

@peterrehm
Copy link
Contributor Author

No problem :) I just submitted the PR #4076. Would be great if you could have a quick look.

weaverryan added a commit that referenced this issue Aug 13, 2014
…nticator (peterrehm)

This PR was submitted for the master branch but it was merged into the 2.4 branch instead (closes #4076).

Discussion
----------

Fixed description of session storage of the ApiKeyAuthenticator

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

I assume the authentication is needed for each request (even if token is stored in the session)
since you can add custom logic in the authenticator.

Commits
-------

f3c02dd Fixed description for session storage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants