Merge pull request #172 from node-oauth/release-4.3.0

merge release 4.3.0 into master

Author: jankapunkt

.github/workflows/tests-release.yml

@@ -23,10 +23,10 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-    - uses: actions/checkout@v2
-    - uses: actions/setup-node@v2
+    - uses: actions/checkout@v3
+    - uses: actions/setup-node@v3
       with:
-        node-version: '12'
+        node-version: '14'
     # install to create local package-lock.json but don't cache the files
     # also: no audit for dev dependencies
     - run: npm i --package-lock-only  && npm audit --production
@@ -40,24 +40,16 @@ jobs:
     needs: [audit]
     strategy:
       matrix:
-        node: [12, 14, 16]
+        node: [14, 16, 18]
     steps:
     - name: Checkout ${{ matrix.node }}
-      uses: actions/checkout@v2
+      uses: actions/checkout@v3
 
     - name: Setup node ${{ matrix.node }}
-      uses: actions/setup-node@v2
+      uses: actions/setup-node@v3
       with:
         node-version: ${{ matrix.node }}
 
-    - name: Cache dependencies ${{ matrix.node }}
-      uses: actions/cache@v1
-      with:
-        path: ~/.npm
-        key: ${{ runner.os }}-node-${{ matrix.node }}-${{ hashFiles('**/package-lock.json') }}
-        restore-keys: |
-          ${{ runner.os }}-node-${{ matrix.node }}
-
     # for this workflow we also require npm audit to pass
     - run: npm i
     - run: npm run test:coverage
@@ -81,35 +73,28 @@ jobs:
     needs: [unittest]
     strategy:
       matrix:
-        node: [12, 14] # TODO get running for node 16
+        node: [14] # TODO get running for node 16 once we removed bluebird dependency
     steps:
     # checkout this repo
     - name: Checkout ${{ matrix.node }}
-      uses: actions/checkout@v2
+      uses: actions/checkout@v3
 
     # checkout express-adapter repo
     - name: Checkout express-adapter ${{ matrix.node }}
-      uses: actions/checkout@v2
+      uses: actions/checkout@v3
       with:
         repository:  node-oauth/express-oauth-server
         path: github/testing/express
 
     - name: Setup node ${{ matrix.node }}
-      uses: actions/setup-node@v2
+      uses: actions/setup-node@v3
       with:
         node-version: ${{ matrix.node }}
 
-    - name: Cache dependencies ${{ matrix.node }}
-      uses: actions/cache@v1
-      with:
-        path: ~/.npm
-        key: ${{ runner.os }}-node-${{ matrix.node }}-node-oauth/express-oauth-server-${{ hashFiles('github/testing/express/**/package-lock.json') }}
-        restore-keys: |
-          ${{ runner.os }}-node-${{ matrix.node }}-node-oauth/express-oauth-server
-
     # in order to test the adapter we need to use the current checkout
     # and install it as local dependency
     # we just cloned and install it as local dependency
+    # xxx: added bluebird as explicit dependency
     - run: |
         cd github/testing/express
         npm i
@@ -122,10 +107,10 @@ jobs:
     runs-on: ubuntu-latest
     needs: [integrationtests]
     steps:
-      - uses: actions/checkout@v2
-      - uses: actions/setup-node@v2
+      - uses: actions/checkout@v3
+      - uses: actions/setup-node@v3
         with:
-          node-version: 12
+          node-version: 14
           registry-url: https://registry.npmjs.org/
       - run: npm i
       - run: npm publish --dry-run
@@ -139,11 +124,11 @@ jobs:
       contents: read
       packages: write
     steps:
-    - uses: actions/checkout@v2
-    - uses: actions/setup-node@v2
+    - uses: actions/checkout@v3
+    - uses: actions/setup-node@v3
       with:
         # we always publish targeting the lowest supported node version
-        node-version: 12
+        node-version: 14
         registry-url: $registry-url(npm)
     - run: npm i
     - run: npm publish --dry-run

.github/workflows/tests.yml

@@ -18,7 +18,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        node: [12, 14, 16]
+        node: [14, 16, 18]
     steps:
     - name: Checkout ${{ matrix.node }}
       uses: actions/checkout@v2

index.d.ts

@@ -306,7 +306,7 @@ declare namespace OAuth2Server {
          *
          */
         saveAuthorizationCode(
-          code: Pick<AuthorizationCode, 'authorizationCode' | 'expiresAt' | 'redirectUri' | 'scope'>,
+          code: Pick<AuthorizationCode, 'authorizationCode' | 'expiresAt' | 'redirectUri' | 'scope' | 'codeChallenge' | 'codeChallengeMethod'>,
           client: Client,
           user: User,
           callback?: Callback<AuthorizationCode>): Promise<AuthorizationCode | Falsey>;
@@ -410,6 +410,8 @@ declare namespace OAuth2Server {
         scope?: string | string[] | undefined;
         client: Client;
         user: User;
+        codeChallenge?: string;
+        codeChallengeMethod?: string;
         [key: string]: any;
     }
 

lib/grant-types/authorization-code-grant-type.js

@@ -13,6 +13,7 @@ const promisify = require('promisify-any').use(Promise);
 const ServerError = require('../errors/server-error');
 const isFormat = require('@node-oauth/formats');
 const util = require('util');
+const pkce = require('../pkce/pkce');
 
 /**
  * Constructor.
@@ -118,6 +119,36 @@ AuthorizationCodeGrantType.prototype.getAuthorizationCode = function(request, cl
         throw new InvalidGrantError('Invalid grant: `redirect_uri` is not a valid URI');
       }
 
+      // optional: PKCE code challenge
+
+      if (code.codeChallenge) {
+        if (!request.body.code_verifier) {
+          throw new InvalidGrantError('Missing parameter: `code_verifier`');
+        }
+
+        const hash = pkce.getHashForCodeChallenge({
+          method: code.codeChallengeMethod,
+          verifier: request.body.code_verifier
+        });
+
+        if (!hash) {
+          // notice that we assume that codeChallengeMethod is already
+          // checked at an earlier stage when being read from
+          // request.body.code_challenge_method
+          throw new ServerError('Server error: `getAuthorizationCode()` did not return a valid `codeChallengeMethod` property');
+        }
+
+        if (code.codeChallenge !== hash) {
+          throw new InvalidGrantError('Invalid grant: code verifier is invalid');
+        }
+      }
+      else {
+        if (request.body.code_verifier) {
+          // No code challenge but code_verifier was passed in.
+          throw new InvalidGrantError('Invalid grant: code verifier is invalid');
+        }
+      }
+
       return code;
     });
 };

lib/handlers/authorize-handler.js

@@ -21,6 +21,7 @@ const UnauthorizedClientError = require('../errors/unauthorized-client-error');
 const isFormat = require('@node-oauth/formats');
 const tokenUtil = require('../utils/token-util');
 const url = require('url');
+const pkce = require('../pkce/pkce');
 
 /**
  * Response types.
@@ -77,10 +78,6 @@ AuthorizeHandler.prototype.handle = function(request, response) {
     throw new InvalidArgumentError('Invalid argument: `response` must be an instance of Response');
   }
 
-  if (request.query.allowed === 'false' || request.body.allowed === 'false') {
-    return Promise.reject(new AccessDeniedError('Access denied: user denied access to application'));
-  }
-
   const fns = [
     this.getAuthorizationCodeLifetime(),
     this.getClient(request),
@@ -98,7 +95,7 @@ AuthorizeHandler.prototype.handle = function(request, response) {
       return Promise.bind(this)
         .then(function() {
           state = this.getState(request);
-          if(request.query.allowed === 'false') {
+          if (request.query.allowed === 'false' || request.body.allowed === 'false') {
             throw new AccessDeniedError('Access denied: user denied access to application');
           }
         })
@@ -114,8 +111,10 @@ AuthorizeHandler.prototype.handle = function(request, response) {
         })
         .then(function(authorizationCode) {
           ResponseType = this.getResponseType(request);
+          const codeChallenge = this.getCodeChallenge(request);
+          const codeChallengeMethod = this.getCodeChallengeMethod(request);
 
-          return this.saveAuthorizationCode(authorizationCode, expiresAt, scope, client, uri, user);
+          return this.saveAuthorizationCode(authorizationCode, expiresAt, scope, client, uri, user, codeChallenge, codeChallengeMethod);
         })
         .then(function(code) {
           const responseType = new ResponseType(code.authorizationCode);
@@ -293,13 +292,20 @@ AuthorizeHandler.prototype.getRedirectUri = function(request, client) {
  * Save authorization code.
  */
 
-AuthorizeHandler.prototype.saveAuthorizationCode = function(authorizationCode, expiresAt, scope, client, redirectUri, user) {
-  const code = {
+AuthorizeHandler.prototype.saveAuthorizationCode = function(authorizationCode, expiresAt, scope, client, redirectUri, user, codeChallenge, codeChallengeMethod) {
+  let code = {
     authorizationCode: authorizationCode,
     expiresAt: expiresAt,
     redirectUri: redirectUri,
     scope: scope
   };
+
+  if(codeChallenge && codeChallengeMethod){
+    code = Object.assign({
+      codeChallenge: codeChallenge,
+      codeChallengeMethod: codeChallengeMethod
+    }, code);
+  }
   return promisify(this.model.saveAuthorizationCode, 3).call(this.model, code, client, user);
 };
 
@@ -369,6 +375,27 @@ AuthorizeHandler.prototype.updateResponse = function(response, redirectUri, stat
   response.redirect(url.format(redirectUri));
 };
 
+AuthorizeHandler.prototype.getCodeChallenge = function(request) {
+  return request.body.code_challenge;
+};
+
+/**
+ * Get code challenge method from request or defaults to plain.
+ * https://www.rfc-editor.org/rfc/rfc7636#section-4.3
+ *
+ * @throws {InvalidRequestError} if request contains unsupported code_challenge_method
+ *  (see https://www.rfc-editor.org/rfc/rfc7636#section-4.4)
+ */
+AuthorizeHandler.prototype.getCodeChallengeMethod = function(request) {
+  const algorithm = request.body.code_challenge_method;
+
+  if (algorithm && !pkce.isValidMethod(algorithm)) {
+    throw new InvalidRequestError(`Invalid request: transform algorithm '${algorithm}' not supported`);
+  }
+
+  return algorithm || 'plain';
+};
+
 /**
  * Export constructor.
  */

lib/handlers/token-handler.js

@@ -18,6 +18,7 @@ const TokenModel = require('../models/token-model');
 const UnauthorizedClientError = require('../errors/unauthorized-client-error');
 const UnsupportedGrantTypeError = require('../errors/unsupported-grant-type-error');
 const auth = require('basic-auth');
+const pkce = require('../pkce/pkce');
 const isFormat = require('@node-oauth/formats');
 
 /**
@@ -114,12 +115,14 @@ TokenHandler.prototype.handle = function(request, response) {
 TokenHandler.prototype.getClient = function(request, response) {
   const credentials = this.getClientCredentials(request);
   const grantType = request.body.grant_type;
+  const codeVerifier = request.body.code_verifier;
+  const isPkce = pkce.isPKCERequest({ grantType, codeVerifier });
 
   if (!credentials.clientId) {
     throw new InvalidRequestError('Missing parameter: `client_id`');
   }
 
-  if (this.isClientAuthenticationRequired(grantType) && !credentials.clientSecret) {
+  if (this.isClientAuthenticationRequired(grantType) && !credentials.clientSecret && !isPkce) {
     throw new InvalidRequestError('Missing parameter: `client_secret`');
   }
 
@@ -174,6 +177,7 @@ TokenHandler.prototype.getClient = function(request, response) {
 TokenHandler.prototype.getClientCredentials = function(request) {
   const credentials = auth(request);
   const grantType = request.body.grant_type;
+  const codeVerifier = request.body.code_verifier;
 
   if (credentials) {
     return { clientId: credentials.name, clientSecret: credentials.pass };
@@ -183,6 +187,12 @@ TokenHandler.prototype.getClientCredentials = function(request) {
     return { clientId: request.body.client_id, clientSecret: request.body.client_secret };
   }
 
+  if (pkce.isPKCERequest({ grantType, codeVerifier })) {
+    if(request.body.client_id) {
+      return { clientId: request.body.client_id };
+    }
+  }
+
   if (!this.isClientAuthenticationRequired(grantType)) {
     if(request.body.client_id) {
       return { clientId: request.body.client_id };

lib/pkce/pkce.js

@@ -0,0 +1,77 @@
+'use strict';
+
+/**
+ * Module dependencies.
+ */
+const { base64URLEncode } = require('../utils/string-util');
+const { createHash } = require('../utils/crypto-util');
+const codeChallengeRegexp = /^([a-zA-Z0-9.\-_~]){43,128}$/;
+/**
+ * Export `TokenUtil`.
+ */
+
+const pkce = {
+  /**
+   * Return hash for code-challenge method-type.
+   *
+   * @param method {String} the code challenge method
+   * @param verifier {String} the code_verifier
+   * @return {String|undefined}
+   */
+  getHashForCodeChallenge: function({ method, verifier }) {
+    // to prevent undesired side-effects when passing some wird values
+    // to createHash or base64URLEncode we first check if the values are right
+    if (pkce.isValidMethod(method) && typeof verifier === 'string' && verifier.length > 0) {
+      if (method === 'plain') {
+        return verifier;
+      }
+
+      if (method === 'S256') {
+        const hash = createHash({ data: verifier });
+        return base64URLEncode(hash);
+      }
+    }
+  },
+
+  /**
+   * Check if the request is a PCKE request. We assume PKCE if grant type is
+   * 'authorization_code' and code verifier is present.
+   *
+   * @param grantType {String}
+   * @param codeVerifier {String}
+   * @return {boolean}
+   */
+  isPKCERequest: function ({ grantType, codeVerifier }) {
+    return grantType === 'authorization_code' && !!codeVerifier;
+  },
+
+  /**
+   * Matches a code verifier (or code challenge) against the following criteria:
+   *
+   * code-verifier = 43*128unreserved
+   * unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
+   * ALPHA = %x41-5A / %x61-7A
+   * DIGIT = %x30-39
+   *
+   * @see: https://datatracker.ietf.org/doc/html/rfc7636#section-4.1
+   * @param codeChallenge {String}
+   * @return {Boolean}
+   */
+  codeChallengeMatchesABNF: function (codeChallenge) {
+    return typeof codeChallenge === 'string' &&
+      !!codeChallenge.match(codeChallengeRegexp);
+  },
+
+  /**
+   * Checks if the code challenge method is one of the supported methods
+   * 'sha256' or 'plain'
+   *
+   * @param method {String}
+   * @return {boolean}
+   */
+  isValidMethod: function (method) {
+    return method === 'S256' || method === 'plain';
+  }
+};
+
+module.exports = pkce;