Code Monkey home page Code Monkey logo

Comments (8)

alexbilbie avatar alexbilbie commented on May 16, 2024
  1. This is easily fixed

  2. I think the refresh token grant is difficult to work with anyway because both the server and the client have to maintain session state, but having multiple sessions for each user/client ID for each client with different TTLs and different scopes is just asking for trouble.

The reason I created this library in the first place is because so many people have been implementing OAuth incorrectly and inconsistently and I think this is one example where perhaps deviating from the spec slightly is the best course of action

from oauth2-server.

cziegenberg avatar cziegenberg commented on May 16, 2024

The problem is, that point 1 depends on point 2 - you have a set of allowed scopes and can reduce them, but after that you won't have the chance to create a new access token with the original scopes. You need to keep it saved or the client will only have two options - always use full rights (insecure) or ask the user again for his credentials (bad idea).

I tried to understand point 2 and checked the Google implementation. They do it as expected: It's possible to create multiple access tokens using the refresh token (they say that the number of tokens is limited, which is a good idea). You only get a refresh token with the main access token request and this can theoretically stay in the database forever (independent of the expiration time).

I don't think it's too difficult:

  1. Create an "normal" access token and refresh token. -> One entry in the session table.
  2. Create a new access token using the refresh token. -> An additional entry in the session table, possible with the current table structure. You only have to call the "createSession" method and leave out the "deleteSession" method. The only problem is, that you create multiple sessions and - as far as I understand it - the access tokens generated with the refresh token should depend on the main access token - so they should share the same session id.
  3. Destroy all access tokens when a "normal" access token is generated. -> Automatically done with "deleteSession".

BUT I would propose a general correction of the table structure to avoid the mentioned problem and to make it a bit clearer, easier to implement/customize and optimize the performance:

  1. Use one main session table with all basic data (without fields that are only required by special grant types you perhaps never use).
  2. Add tables for the fields used by special grant types. So you only create an entry if you have data and avoid the massive use of NULLable columns (which isn't a good idea from the performance view). You can even delete the tables, which are not required for the own implementation.
  3. For the library the changes would be minimal - as far as I can see, only the call of "associateScope" would have to be changed, because it's not bound to the session id, but the generated access token.

I can work on that and provide optimized scripts for the database - MySQL, SqlServer, Oracle, and perhaps Postgres (learning at the moment) - and a Session class for your server example.

Attached an optimized structure, as I would propose it. I'm not sure about the roles of the fields "stage", "first_requested" and "last_updated". Are they really required? If so, only for specific scopes?

[Old structure replaced by new one, see next comment]

from oauth2-server.

cziegenberg avatar cziegenberg commented on May 16, 2024

I optimized the structure again and created a test implementation.

oauth

The Refresh Token Grant now works as expected and theoretically it's not necessary to change the scripts - I only had to rename some parameters (because the session id is now the session token id in some cases) and this renaming should be done also in the grant scripts.

I'm still not sure about the role of the "stage" field. Currently it seems to be really used with the Auth Code Grant only...

from oauth2-server.

alexbilbie avatar alexbilbie commented on May 16, 2024

Can you run me off a MySQL dump of this new structure please

from oauth2-server.

cziegenberg avatar cziegenberg commented on May 16, 2024

I just sent you the DB structure and a Session Storage example via mail.

from oauth2-server.

cziegenberg avatar cziegenberg commented on May 16, 2024

I just found another problem with the current implementation:

RFC, section 1.5:

  • "Refresh tokens are issued to the client by the authorization server and are used to obtain a new access token when the current access token becomes invalid or expires, or to obtain additional access tokens with identical or narrower scope"

RFC, section 6:

  • "The authorization server MAY issue a new refresh token, in which case the client MUST discard the old refresh token and replace it with the new refresh token."
  • "The authorization server MAY revoke the old refresh token after issuing a new refresh token to the client."
  • "If a new refresh token is issued, the refresh token scope MUST be identical to that of the refresh token included by the client in the request."

I modified the Session Storage script (sent to you via mail) to handle the creation of new refresh tokens in accordance with the RFC, but the flow of Refresh Token Grant class also needs to be modified.

The Refresh Token Grant must differ between two cases - issue a new refresh token when a new access token is created, or not. This defines the further steps in the flow and the possibilities on the client side. Therefore it must be possible to define the preferred behavior for this Grant:

RefreshToken::useRefreshTokenRotation(true|false)

If TRUE:

  • Return error, if scope parameter set (or if it's not identical to the prior scope).
  • Check credentials and refresh token.
  • Create a new access token.
  • Revoke the old refresh token (as the client MUST replace it anyway in this case).
  • Create a new refresh token.
  • Return the access token and the refresh token.

If FALSE:

  • The scope parameter is optional and can be used to limit the prior scope. If not set, the prior scope is used.
  • Check credentials and refresh token.
  • Create a new access token.
  • The old refresh token remains valid.
  • Return the access token only.

from oauth2-server.

cziegenberg avatar cziegenberg commented on May 16, 2024

I updated the DB structure and the Session Storage class once again, due to the following points...

Security problems:

  1. "A maximum authorization code lifetime of 10 minutes is RECOMMENDED." (RFC, section 4.1.2.) This is not implemented at the moment (could be perhaps handled using the current timestamps, but then it should be renamed and moved to the auth code table). I moved the timestamp and renamed it, so it's purpose is a bit clearer. And I also added the necessary checks to the Session Storage Example.

  2. "The client MUST NOT use the authorization code more than once. If an authorization code is used more than once, the authorization server MUST deny the request and SHOULD revoke (when possible) all tokens previously issued based on that authorization code." (RFC, section 4.1.2.) This is not implemented at the moment. To detect multiple usages of the auth code it must not be deleted. Also the DB structure was missing an assignment of issued tokens to the auth code - to invalidate them as described. (This assignment also replaces the previous "stage" field, because an existing assignment means "granted".)

Other problems:

  1. As a result of the Client Credentials Grant correction (no refresh token allowed), it wasn't possible anymore to create multiple valid tokens with this grant (which is required if you want to access different resource servers with only the required scope for security reasons). I modified the table structure (removed unique key in oauth_session) and the Session Storage script to allow this.

  2. There was an error in the Auth Code handling in the last version of my Session Storage class.

My changes in the Session Storage class:

createSession

  • removed stage parameter (only used for Auth Code Grant and solved more securely, see below); change of the SessionInterface required
  • removed saving of timestamps (not required, can be done in customized implementations if needed)
  • added expiration time for auth code (as recommended in RFC); change of the SessionInterface recommended

updateSession

  • removed unnecessary parameters ("authCode", "stage"); change of the SessionInterface required
  • removed the updates of the timestamps in the session table
  • corrected the handling of the auth code entry (error in my previous script)
  • added assignment of the generated token to the auth code entry (required for security reasons when validating the auth code)

validateAuthCode

  • check for codes added, that have been already used
  • added deletion of previously issued tokens, for the case that the auth code is used multiple times (important security feature)
  • check for expired auth codes added

deleteSession

  • differ between the owner type to delete only expired sessions when the Client Credentials Grant is used.

I sent you the updated files via mail...

oauth

from oauth2-server.

alexbilbie avatar alexbilbie commented on May 16, 2024

I've implemented all of this in v2.0 and significantly simplified the usage

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.