Limiting grants, clients and scopes

[alexbilbie]

Following on from discussion in #20 and #21 the library needs to support the following features:

• Limiting clients to certain scopes
• Limiting clients to certain grants
• Limiting scopes to certain grants (e.g. read only scopes over implicit grant)
• Associating sessions with grant types

Should limiting clients to grants/scopes be opt-in (i.e. a client has to be whitelisted to use a particular grant/scope) or opt-out (i.e. blacklisted from using a particular grant/scope)?

I think it makes sense for the scopes to be opt-out of grants.

@philsturgeon @jacksonj04 @lapause @ziege any thoughts about this?

[cziegenberg]

I will answer to the mentioned points later, but first want to add a fifth one:

• Access token expiration time dependent of used grant type

as mentioned in http://tools.ietf.org/html/rfc6749#section-1.5, Refresh Tokens:
"access tokens may have a shorter lifetime and fewer permissions than authorized by the resource owner"

[alexbilbie]

Unless you have separate endpoints for different grants (i.e. different instances of the auth server code) then you can already do this.

e.g.

/access_token has auth_code and refresh_token grants enabled with token TTL of 3600 seconds
/access_token_normal has client_credentials grant enabled with token TTL of 86400 seconds

If you just have the one endpoint and have the refresh_token grant enabled then all access tokens will have whatever TTL is set

[lapause]

I think the library should be kept light and those cases handled by developers.
IMO, what library should do is provide to the storage interfaces methods the maximum useful information so that security limitations could be implemented there. Sample usages can be provided to developer in the interfaces inline doc.

Here is what I would do:

• In the Grant classes completeFlow() methods, grab the scopes early (by the way, maybe use a getScopes( array $scopes) method in the ScopeInterface rather than a getScope() one, to allow developer to grab multiple scopes in one request and eventually make cross checks (e.g forbidding the usage of a scope with another). Pass an optional grant type to this method to allow scopes/grants limitations
• Pass the grant type used and the detail of requested scopes in the ClientInterface::getClient() method, to allow clients/grant and clients/scopes limitations
• Pass the client details in the verifyCredentialsCallback function of the password grant type, to allow users/clients limitations

When I say 'details' in the points above, it's about passing the complete array received from getClient() or getScope(), not just the ID. Without that, it would force the dev to grab complementary information again, multiplying DB requests.

[cziegenberg]

@alexbilbie Yes, I thought so too, but that's not secure (at the moment). You need to add the Refresh Token Grant to the allowed grant types to get the "refresh_token" returned in the response. This allows the client to generate a new token using "/access_token_normal" and bypass the limitation in "/access_token".

This could be fixed if one could differ between "allow creating a token using the Refresh Token Grant" and "return a refresh token".

[alexbilbie]

@lapause I agree with keeping it lightweight - in the implementation I run at work I've not edited the library and I've made it support limiting scopes to certain clients and also limiting grants to certain clients.

@ziege why would you allow returning a refresh token but not allow a client to use it?

[cziegenberg]

I see a need for all the listed features, and I think that the implementation should be as easy to use as possible, but it should also be "secure by design". For me this always means using a whitelist, not a blacklist.

In my opinion the developer is responsible for this. The library should not directly handled these extensions, it only needs to offer an easy way to implement them, e.g. see my proposal for the extension of the getScope method - all you need is some additional information. This would keep the implementation as easy as it is now, but would allow to extend it.

The first three points could be solved by extending the ClientInterface and the ScopeInterface by the following information (or allow another way to access it):

ScopeInterface::getScope:

• client_id
• owner_id (user_id, client_id,...)
• grant_type

ClientInterface::getClient:

• grant_type

Generally it would be good to give (read) access to as much relevant information as possible, to allow the implementation of other checks.

For point 4 I would add a column to the session table (or think about the general session table structure, regarding #25 and the problem with multiple valid tokens and the invalidation/deletion of old ones).

[cziegenberg]

@alexbilbie I will try to explain it with an example:

I want to use the Client Credentials Grant and I also want to use refresh tokens. I request my access token using the URI "/access_token". To get the access token and the refresh token, I need to activate the Client Credentials Grant - but also the Refresh Token Grant, because otherwise no refresh token will be returned. The expiration time of the access token is set to 86400 seconds.

Now I can create a second script "/refresh_token" to get a new access token with the refresh token I got before. There only the Refresh Token Grant is active, and its expiration time is set to 3600 seconds.

Now the problem:
I can bypass the expiration time limitation by requesting a new token with the refresh token at "/access_token" instead of "/refresh_token", because the Refresh Token Grant is required to be active in both cases.

Solutions:

1. Bind the expiration time to the grant type (would avoid insecure implementations like the one mentioned in the example, and avoid the need of two scripts where one could do the job). The setExpiresIn() method could be moved from the AuthServer to the GrantTypeInterface.
2. Or offer an option to add the refresh token to the response, although is not added to the allowed grant types (more difficult to understand, could lead to errors and security problems in the implementation).

[alexbilbie]

@ziege thanks, I understand what you mean now. I agree with what you're saying and I agree that option 1 is a better option

[alexbilbie]

In the dev branch grants are now injected into getClient(), clients and grants are now injected into getScope()

[alexbilbie]

I've implemented all of the suggestions here in v2