Code Monkey home page Code Monkey logo

Comments (10)

cziegenberg avatar cziegenberg commented on May 23, 2024

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"

from oauth2-server.

alexbilbie avatar alexbilbie commented on May 23, 2024

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

from oauth2-server.

lapause avatar lapause commented on May 23, 2024

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.

from oauth2-server.

cziegenberg avatar cziegenberg commented on May 23, 2024

@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".

from oauth2-server.

alexbilbie avatar alexbilbie commented on May 23, 2024

@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?

from oauth2-server.

cziegenberg avatar cziegenberg commented on May 23, 2024

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).

from oauth2-server.

cziegenberg avatar cziegenberg commented on May 23, 2024

@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).

from oauth2-server.

alexbilbie avatar alexbilbie commented on May 23, 2024

@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

from oauth2-server.

alexbilbie avatar alexbilbie commented on May 23, 2024

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

from oauth2-server.

alexbilbie avatar alexbilbie commented on May 23, 2024

I've implemented all of the suggestions here in v2

from oauth2-server.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.