FrankHassanabad / Oauth2orizeRecipes

OAuth2 security recipes and examples based on OAuth2orize
MIT License
375 stars 97 forks source link

Wrong Issue Token Logic for Access+Refresh Token? #67

Open felixfrtz opened 6 years ago

felixfrtz commented 6 years ago

Hey,

I stumbled across the following issue, where the application does not issue a refresh Token where it should. Have a look at the following part in validate.js:

/**
 * Given an auth code this will generate a access and refresh token, save both of those and return
 * them if the auth code indicates to return both.  Otherwise only an access token will be returned.
 * @param   {Object}  authCode - The auth code
 * @throws  {Error}   If the auth code does not exist or is zero
 * @returns {Promise} The resolved refresh and access tokens as an array
 */
validate.generateTokens = (authCode) => {
  if (validate.isRefreshToken(authCode)) {
    return Promise.all([
      validate.generateToken(authCode),
      validate.generateRefreshToken(authCode),
    ]);
  }
  return Promise.all([validate.generateToken(authCode)]);
};

So, we only get a refresh token if the authCode provided is a refresh token. That doesn't make sense, right? A refresh token should make this issue only 1 (access) token, and an authorization code should make it issue 2 (access + refresh). It should be if (!validate.isRefreshToken(authCode)) if I understand the specification correctly.

dbeyda commented 5 years ago

@felixfrtz , I know I might be late, but I just stumbled on the same problem. And I think you are probably right.

MissakaI commented 4 years ago

From https://tools.ietf.org/html/rfc6749#section-1.5 it states

(H) The authorization server authenticates the client and validates the refresh token, and if valid, issues a new access token (and, optionally, a new refresh token).

So I think in both scenarios you should issue both tokens. Because if the refresh token isn't renewed during the refresh flow eventually the refresh token will expire prior to the expiration of access token.