Skip to content

Commit

Permalink
Merge pull request #172 from node-oauth/release-4.3.0
Browse files Browse the repository at this point in the history
merge release 4.3.0 into master
  • Loading branch information
jankapunkt committed Nov 28, 2022
2 parents e01e841 + cf7b701 commit c993eb5
Show file tree
Hide file tree
Showing 16 changed files with 748 additions and 81 deletions.
49 changes: 17 additions & 32 deletions .github/workflows/tests-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down Expand Up @@ -410,6 +410,8 @@ declare namespace OAuth2Server {
scope?: string | string[] | undefined;
client: Client;
user: User;
codeChallenge?: string;
codeChallengeMethod?: string;
[key: string]: any;
}

Expand Down
31 changes: 31 additions & 0 deletions lib/grant-types/authorization-code-grant-type.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
});
};
Expand Down
43 changes: 35 additions & 8 deletions lib/handlers/authorize-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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),
Expand All @@ -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');
}
})
Expand All @@ -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);
Expand Down Expand Up @@ -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);
};

Expand Down Expand Up @@ -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.
*/
Expand Down
12 changes: 11 additions & 1 deletion lib/handlers/token-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

/**
Expand Down Expand Up @@ -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`');
}

Expand Down Expand Up @@ -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 };
Expand All @@ -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 };
Expand Down
77 changes: 77 additions & 0 deletions lib/pkce/pkce.js
Original file line number Diff line number Diff line change
@@ -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;
Loading

0 comments on commit c993eb5

Please sign in to comment.