oauth2-server icon indicating copy to clipboard operation
oauth2-server copied to clipboard

Password grant for SPA's

Open boydcl opened this issue 6 years ago • 29 comments

The documentation advises to use the password grant type for first party frontend apps such as single page applications. (link)

This confuses me because the password grant requires the client to store the client_secret. Considering your first party SPA has no backend component I wouldn't know how to securely store that secret.

Can someone elaborate on that?

boydcl avatar Apr 17 '18 15:04 boydcl

Password grant does not require client secret https://tools.ietf.org/html/rfc6749#section-1.3.3

mtangoo avatar Apr 17 '18 16:04 mtangoo

The Password grant referred to in the docs does require to pass the client_secret. See http://oauth2.thephpleague.com/authorization-server/resource-owner-password-credentials-grant/

boydcl avatar Apr 17 '18 17:04 boydcl

https://tools.ietf.org/html/rfc6749#section-4.3.2

Client/Secret should be sent if the client is capable of keeping secret.

Did you omit the id and secret and it didn't work? AFAIK it should work fine

mtangoo avatar Apr 17 '18 17:04 mtangoo

@boyd91 has a point, the current implementation requires client_secret as part of password grant. Not sure if this is completely under the specs or we are missing something

bitgandtter avatar Apr 17 '18 17:04 bitgandtter

The password grant in this implementation indeed requires the client_secret to be passed. https://github.com/thephpleague/oauth2-server/blob/master/src/Grant/PasswordGrant.php#L51

boydcl avatar Apr 17 '18 17:04 boydcl

I cannot currently verify that myself but I would expect that line since some cases require client/secret.

The question am asking is, is this ennforced? If it is then it should be logged as a bug

mtangoo avatar Apr 17 '18 17:04 mtangoo

At leas on my side its enforced, i workaround using implicit grant for my SPA but as far as password grant client_secret its mandatory

bitgandtter avatar Apr 17 '18 17:04 bitgandtter

By the way most of the "engine and tires" of this library require your own implementation. I guess the same is the case with password grant

mtangoo avatar Apr 17 '18 17:04 mtangoo

By reading the contents of validateClient it certainly seems like it's unconditionally enforced. https://github.com/thephpleague/oauth2-server/blob/master/src/Grant/AbstractGrant.php#L172

boydcl avatar Apr 17 '18 17:04 boydcl

https://github.com/thephpleague/oauth2-server/blob/master/src/Repositories/ClientRepositoryInterface.php

mtangoo avatar Apr 17 '18 18:04 mtangoo

It is wrong to use Password Grant for SPA. The only right solution (if you really need oauth 2.0 authorisation) is Implicit Grant flow. Otherwise you misunderstand at the basic level the purpose of oauth 2.0.

alex-filat avatar Apr 17 '18 22:04 alex-filat

Actually the level of trust password grant requires should warn anyone against using it anywhere. I would not recommend it even for mobile apps (Native or otherwise)

That being said, the question about enforcement of secret for the grant if indeed it does require it all the times should be addressed

mtangoo avatar Apr 18 '18 06:04 mtangoo

As @alex-filat said, your SPA (if it is calling your OAuth service directly from the front-end) should be using an Implicit Grant.

There are plenty of valid reasons to use the Password grant, but you will definitely need to send Client ID & Client Secret, so it's impractical to use this for an SPA without some kind of back-end proxy to store your secrets and make requests on behalf of the app.

simonhamp avatar Apr 18 '18 09:04 simonhamp

@simonhamp Thanks for your clarification.

In that case I would suggest the doc needs an update. It now states

If the client is a web application that has runs entirely on the front end (e.g. a single page web application) you should implement the password grant for a first party clients and the implicit grant for a third party clients.

http://oauth2.thephpleague.com/authorization-server/which-grant/

Since the client_secret is always required, the password grant can never be a viable option for a frontend only SPA (regardless wether it is first or third party) because such clients can never really keep the client_secret a secret.

boydcl avatar Apr 18 '18 09:04 boydcl

Thanks @boyd. So I've reopened this for the issue around documentation.

simonhamp avatar Apr 18 '18 09:04 simonhamp

@simonhamp

There are plenty of valid reasons to use the Password grant, but you will definitely need to send Client ID & Client Secret, so it's impractical to use this for an SPA without some kind of back-end proxy to store your secrets and make requests on behalf of the app.

Is this according to RFC or an opinion of this specific library? As per RFC only grant_type, username and password are required. Scope is optional and there is no mention of client id nor of secret. Since The library tries as much as it can to stick to standard, I would expect at least the client ID/Secret to be optional.

mtangoo avatar Apr 18 '18 09:04 mtangoo

It needs further investigation. If the client is confidential or has been issued auth credentials, it must authenticate with the OAuth2 server.

The package first checks for Basic HTTP auth and if not present, will look for the client ID and password to try and authenticate the client.

Even the implicit grant is not recommended now for SPAs so we will need to update the documentation to reflect this. There might be an implementation issue with the password grant here where it should allow no client auth which seems to be the case given the spec but we will need to look at this more closely in due course

Sephster avatar Apr 18 '18 10:04 Sephster

Oops, I didn't mean to close this. Clicked the wrong button sorry :)

Sephster avatar Apr 18 '18 10:04 Sephster

@mtangoo relevant part is in the docs for the section you're referencing:

If the client type is confidential or the client was issued client credentials (or assigned other authentication requirements), the client MUST authenticate with the authorization server as described in Section 3.2.1.

Following this through the spec leads us to Section 2.3, which indicates that client_id and client_secret are required.

But it's not a requirement of this library, it's an implementation detail. As you said, "engine and tires" :) It all comes down to how you implement the ClientRepositoryInterface.

simonhamp avatar Apr 18 '18 10:04 simonhamp

Documentation has been updated to not advocate the password grant for SPAs. At the moment it states that you should use the implicit grant but I believe current best practices are actually to use an Auth Code grant without passing the client secret. This can't be achieved with the library at present so I've left it recommending the implicit grant for now. Will need to do some more investigation into this.

Also, the library can accept two client auth mechanisms in its current format which does not conform to the RFC.

The client MUST NOT use more than one authentication method in each request.

At the moment I believe we accept basic auth and client_secret etc but we don't enforce that only one of these should be specified. This should probably be changed to conform to the specs better but I will need to do a more thorough investigation before making any changes.

I believe we will also need to update the diagram

Sephster avatar Apr 18 '18 12:04 Sephster

@simonhamp thanks for clarification.

But it's not a requirement of this library, it's an implementation detail. As you said, "engine and tires" :) It all comes down to how you implement the ClientRepositoryInterface.

I was under impression that the library as quoted by OP forces that and hence bypassing ClientRepositoryInterface. Thanks for clarifying.

I love how this library gives us power, to even shoot our own legs, especially the "engine and the tires";)

mtangoo avatar Apr 18 '18 17:04 mtangoo

@Sephster @simonhamp and the team, your work in this library is highly appreciated. Its such a Wondeful library!

mtangoo avatar Apr 18 '18 18:04 mtangoo

@Sephster

I believe current best practices are actually to use an Auth Code grant without passing the client secret. This can't be achieved with the library at present

In this other issue https://github.com/thephpleague/oauth2-server/issues/881 you confirm that client secret is actually not being used for the Auth Code grant, so which of the two statements is correct?

Looking at the code seems like the secret is required at the moment: https://github.com/thephpleague/oauth2-server/blob/master/src/Grant/AbstractGrant.php#L181..L189

What's the current status to support this? I could do a PR if you want, but I'm not familiar with the library so I'm not sure of the actual side-effects of changing the code.

NoelDeMartin avatar Aug 21 '18 17:08 NoelDeMartin

@NoelDeMartin good question and apologies for any confusion I've caused. As I had stated in #881, the Auth Code Grant can be used without a secret according to the OAuth2 specs. However, I've made a mistake with my assertions in #881.

The initial auth code request doesn't require a secret but you are right in stating that at the moment, the _access token request _does indeed require a secret because it uses the default validateClient function which mandates that a secret must be provided.

I'm working on a fix for this at the moment but it is taking some time to get out the door. It will be a breaking change so won't be released until version 8. The main challenge is that the client retrieval function both validates and retrieves client information which is problematic at the moment.

Sephster avatar Aug 26 '18 13:08 Sephster

@Sephster, Will v8 come with fix for https://github.com/thephpleague/oauth2-server/pull/817?

mtangoo avatar Aug 27 '18 13:08 mtangoo

Yes it will. I've made some progress on this but keep hitting gotchas. The new PKCE handling should:

  • support PKCE if a code challenge is sent
  • Force PKCE support for public clients if this option is specified

This requires a couple of modifications to the code because the auth code grant doesn't currently support public clients. In addition, we have challenges around the way clients are authenticated because we also use this method to get client information.

For my changes to work, we need to know if the client is public or private in isolation. I've started to build this out but there is still a bit of work to do. You can see my progress here: https://github.com/thephpleague/oauth2-server/compare/8.0.0...Sephster:force-pkce-for-public-clients

Sephster avatar Aug 27 '18 14:08 Sephster

That sounds really great! When you think you are ready please open PR so that I can help testing or if you need any input. Good work as usual!

mtangoo avatar Aug 27 '18 18:08 mtangoo

If anyone else reads this thread wondering about the status of the original issue, the client_secret was no longer required for the password grant in 5.0.0-RC1.

Password grant updated Allow support for public clients

https://github.com/thephpleague/oauth2-server/blob/0227f14b7bbeaa2ec1fec378ba0c1afd927e95b0/src/Grant/AbstractGrant.php#L187

matt-allan avatar Feb 15 '19 18:02 matt-allan

~~Apparently this was changed back at some point, as Version 8.0 requires the client_secret for the password grant.~~

Nevermind, I misunderstood the 8.0 changes. It should still work on 8.0 assuming your validateClient method allows it.

matt-allan avatar Jul 17 '19 20:07 matt-allan