diff --git a/.github/workflows/tests-release.yml b/.github/workflows/tests-release.yml index 2615910..3d72283 100644 --- a/.github/workflows/tests-release.yml +++ b/.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 diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 4726c73..08a6f94 100644 --- a/.github/workflows/tests.yml +++ b/.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 diff --git a/index.d.ts b/index.d.ts index 260e34a..3d780b3 100644 --- a/index.d.ts +++ b/index.d.ts @@ -306,7 +306,7 @@ declare namespace OAuth2Server { * */ saveAuthorizationCode( - code: Pick, + code: Pick, client: Client, user: User, callback?: Callback): Promise; @@ -410,6 +410,8 @@ declare namespace OAuth2Server { scope?: string | string[] | undefined; client: Client; user: User; + codeChallenge?: string; + codeChallengeMethod?: string; [key: string]: any; } diff --git a/lib/grant-types/authorization-code-grant-type.js b/lib/grant-types/authorization-code-grant-type.js index ed66eea..92193d3 100644 --- a/lib/grant-types/authorization-code-grant-type.js +++ b/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; }); }; diff --git a/lib/handlers/authorize-handler.js b/lib/handlers/authorize-handler.js index 830bfa7..57413e9 100644 --- a/lib/handlers/authorize-handler.js +++ b/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. */ diff --git a/lib/handlers/token-handler.js b/lib/handlers/token-handler.js index 285843e..0f0c57a 100644 --- a/lib/handlers/token-handler.js +++ b/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 }; diff --git a/lib/pkce/pkce.js b/lib/pkce/pkce.js new file mode 100644 index 0000000..e7603d9 --- /dev/null +++ b/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; diff --git a/lib/utils/crypto-util.js b/lib/utils/crypto-util.js new file mode 100644 index 0000000..3e2158f --- /dev/null +++ b/lib/utils/crypto-util.js @@ -0,0 +1,24 @@ +'use strict'; + +const crypto = require('crypto'); + +/** + * Export `StringUtil`. + */ + +module.exports = { + /** + * + * @param algorithm {String} the hash algorithm, default is 'sha256' + * @param data {Buffer|String|TypedArray|DataView} the data to hash + * @param encoding {String|undefined} optional, the encoding to calculate the + * digest + * @return {Buffer|String} if {encoding} undefined a {Buffer} is returned, otherwise a {String} + */ + createHash: function({ algorithm = 'sha256', data = undefined, encoding = undefined }) { + return crypto + .createHash(algorithm) + .update(data) + .digest(encoding); + } +}; diff --git a/lib/utils/string-util.js b/lib/utils/string-util.js new file mode 100644 index 0000000..464bd07 --- /dev/null +++ b/lib/utils/string-util.js @@ -0,0 +1,19 @@ +'use strict'; + +/** + * Export `StringUtil`. + */ + +module.exports = { + /** + * + * @param str + * @return {string} + */ + base64URLEncode: function(str) { + return str.toString('base64') + .replace(/\+/g, '-') + .replace(/\//g, '_') + .replace(/=/g, ''); + } +}; diff --git a/lib/utils/token-util.js b/lib/utils/token-util.js index a66e252..8626dac 100644 --- a/lib/utils/token-util.js +++ b/lib/utils/token-util.js @@ -4,8 +4,8 @@ * Module dependencies. */ -const crypto = require('crypto'); const randomBytes = require('bluebird').promisify(require('crypto').randomBytes); +const { createHash } = require('../utils/crypto-util'); /** * Export `TokenUtil`. @@ -19,10 +19,7 @@ module.exports = { generateRandomToken: function() { return randomBytes(256).then(function(buffer) { - return crypto - .createHash('sha256') - .update(buffer) - .digest('hex'); + return createHash({ data: buffer, encoding: 'hex' }); }); } diff --git a/package.json b/package.json index cf4b2ba..ae54d62 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@node-oauth/oauth2-server", "description": "Complete, framework-agnostic, compliant and well tested module for implementing an OAuth2 Server in node.js", - "version": "4.2.0", + "version": "4.3.0", "keywords": [ "oauth", "oauth2" @@ -33,13 +33,13 @@ "devDependencies": { "chai": "^4.3.4", "eslint": "^8.0.0", - "mocha": "^9.2.2", + "mocha": "^10.0.0", "nyc": "^15.1.0", - "sinon": "^13.0.1" + "sinon": "^14.0.0" }, "license": "MIT", "engines": { - "node": ">=12.0.0" + "node": ">=14.0.0" }, "scripts": { "pretest": "./node_modules/.bin/eslint lib test index.js", diff --git a/test/integration/handlers/authorize-handler_test.js b/test/integration/handlers/authorize-handler_test.js index 054e2cc..b91408a 100644 --- a/test/integration/handlers/authorize-handler_test.js +++ b/test/integration/handlers/authorize-handler_test.js @@ -159,7 +159,7 @@ describe('AuthorizeHandler integration', function() { } }); - it('should throw an error if `allowed` is `false`', function() { + it('should redirect to an error response if user denied access', function() { const model = { getAccessToken: function() { return { @@ -170,49 +170,29 @@ describe('AuthorizeHandler integration', function() { getClient: function() { return { grants: ['authorization_code'], redirectUris: ['http://example.com/cb'] }; }, - saveAuthorizationCode: function() { - throw new Error('Unhandled exception'); - } + saveAuthorizationCode: function() {} }; const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model }); const request = new Request({ body: { - client_id: 'test' + client_id: 12345, + response_type: 'code' }, + method: {}, headers: { 'Authorization': 'Bearer foo' }, - method: {}, query: { - allowed: 'false', - state: 'foobar' + state: 'foobar', + allowed: 'false' } }); const response = new Response({ body: {}, headers: {} }); return handler.handle(request, response) .then(should.fail) - .catch(function(e) { - e.should.be.an.instanceOf(AccessDeniedError); - e.message.should.equal('Access denied: user denied access to application'); - }); - }); - - it('should throw an error if `allowed` is `false` body', function() { - const model = { - getAccessToken: function() {}, - getClient: function() {}, - saveAuthorizationCode: function() {} - }; - const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model }); - const request = new Request({ body: { allowed: 'false' }, headers: {}, method: {}, query: {} }); - const response = new Response({ body: {}, headers: {} }); - - return handler.handle(request, response) - .then(should.fail) - .catch(function(e) { - e.should.be.an.instanceOf(AccessDeniedError); - e.message.should.equal('Access denied: user denied access to application'); + .catch(function() { + response.get('location').should.equal('http://example.com/cb?error=access_denied&error_description=Access%20denied%3A%20user%20denied%20access%20to%20application&state=foobar'); }); }); @@ -1321,4 +1301,67 @@ describe('AuthorizeHandler integration', function() { response.get('location').should.equal('http://example.com/cb?state=foobar'); }); }); + + describe('getCodeChallengeMethod()', function() { + it('should get code challenge method', function() { + const model = { + getAccessToken: function() {}, + getClient: function() {}, + saveAuthorizationCode: function() {} + }; + const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model }); + const request = new Request({ body: {code_challenge_method: 'S256'}, headers: {}, method: {}, query: {} }); + + const codeChallengeMethod = handler.getCodeChallengeMethod(request); + codeChallengeMethod.should.equal('S256'); + }); + + it('should throw if the code challenge method is not supported', async function () { + const model = { + getAccessToken: function() {}, + getClient: function() {}, + saveAuthorizationCode: function() {} + }; + const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model }); + const request = new Request({ body: {code_challenge_method: 'foo'}, headers: {}, method: {}, query: {} }); + + try { + handler.getCodeChallengeMethod(request); + + should.fail(); + } catch (e) { + // defined in RFC 7636 - 4.4 + e.should.be.an.instanceOf(InvalidRequestError); + e.message.should.equal('Invalid request: transform algorithm \'foo\' not supported'); + } + }); + + it('should get default code challenge method plain if missing', function() { + const model = { + getAccessToken: function() {}, + getClient: function() {}, + saveAuthorizationCode: function() {} + }; + const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model }); + const request = new Request({ body: {}, headers: {}, method: {}, query: {} }); + + const codeChallengeMethod = handler.getCodeChallengeMethod(request); + codeChallengeMethod.should.equal('plain'); + }); + }); + + describe('getCodeChallenge()', function() { + it('should get code challenge', function() { + const model = { + getAccessToken: function() {}, + getClient: function() {}, + saveAuthorizationCode: function() {} + }; + const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model }); + const request = new Request({ body: {code_challenge: 'challenge'}, headers: {}, method: {}, query: {} }); + + const codeChallengeMethod = handler.getCodeChallenge(request); + codeChallengeMethod.should.equal('challenge'); + }); + }); }); diff --git a/test/integration/handlers/token-handler_test.js b/test/integration/handlers/token-handler_test.js index 41ec524..0cb60c3 100644 --- a/test/integration/handlers/token-handler_test.js +++ b/test/integration/handlers/token-handler_test.js @@ -20,6 +20,8 @@ const UnauthorizedClientError = require('../../../lib/errors/unauthorized-client const UnsupportedGrantTypeError = require('../../../lib/errors/unsupported-grant-type-error'); const should = require('chai').should(); const util = require('util'); +const crypto = require('crypto'); +const stringUtil = require('../../../lib/utils/string-util'); /** * Test `TokenHandler` integration. @@ -827,6 +829,197 @@ describe('TokenHandler integration', function() { }); }); + describe('with PKCE', function() { + it('should return a token when code verifier is valid using S256 code challenge method', function() { + const codeVerifier = stringUtil.base64URLEncode(crypto.randomBytes(32)); + const authorizationCode = { + authorizationCode: 12345, + client: { id: 'foobar' }, + expiresAt: new Date(new Date().getTime() * 2), + user: {}, + codeChallengeMethod: 'S256', + codeChallenge: stringUtil.base64URLEncode(crypto.createHash('sha256').update(codeVerifier).digest()) + }; + const client = { id: 'foobar', grants: ['authorization_code'] }; + const token = {}; + const model = { + getAuthorizationCode: function() { return authorizationCode; }, + getClient: function() {}, + saveToken: function() { return token; }, + validateScope: function() { return 'foo'; }, + revokeAuthorizationCode: function() { return authorizationCode; } + }; + const handler = new TokenHandler({ accessTokenLifetime: 120, model: model, refreshTokenLifetime: 120 }); + const request = new Request({ + body: { + code: 12345, + grant_type: 'authorization_code', + code_verifier: codeVerifier + }, + headers: {}, + method: {}, + query: {} + }); + + return handler.handleGrantType(request, client) + .then(function(data) { + data.should.equal(token); + }) + .catch(should.fail); + }); + + it('should return a token when code verifier is valid using plain code challenge method', function() { + const codeVerifier = stringUtil.base64URLEncode(crypto.randomBytes(32)); + const authorizationCode = { + authorizationCode: 12345, + client: { id: 'foobar' }, + expiresAt: new Date(new Date().getTime() * 2), + user: {}, + codeChallengeMethod: 'plain', + codeChallenge: codeVerifier + }; + const client = { id: 'foobar', grants: ['authorization_code'] }; + const token = {}; + const model = { + getAuthorizationCode: function() { return authorizationCode; }, + getClient: function() {}, + saveToken: function() { return token; }, + validateScope: function() { return 'foo'; }, + revokeAuthorizationCode: function() { return authorizationCode; } + }; + const handler = new TokenHandler({ accessTokenLifetime: 120, model: model, refreshTokenLifetime: 120 }); + const request = new Request({ + body: { + code: 12345, + grant_type: 'authorization_code', + code_verifier: codeVerifier + }, + headers: {}, + method: {}, + query: {} + }); + + return handler.handleGrantType(request, client) + .then(function(data) { + data.should.equal(token); + }) + .catch(should.fail); + }); + + it('should throw an invalid grant error when code verifier is invalid', function() { + const codeVerifier = stringUtil.base64URLEncode(crypto.randomBytes(32)); + const authorizationCode = { + authorizationCode: 12345, + client: { id: 'foobar' }, + expiresAt: new Date(new Date().getTime() * 2), + user: {}, + codeChallengeMethod: 'S256', + codeChallenge: stringUtil.base64URLEncode(crypto.createHash('sha256').update(codeVerifier).digest()) + }; + const client = { id: 'foobar', grants: ['authorization_code'] }; + const token = {}; + const model = { + getAuthorizationCode: function() { return authorizationCode; }, + getClient: function() {}, + saveToken: function() { return token; }, + validateScope: function() { return 'foo'; }, + revokeAuthorizationCode: function() { return authorizationCode; } + }; + const handler = new TokenHandler({ accessTokenLifetime: 120, model: model, refreshTokenLifetime: 120 }); + const request = new Request({ + body: { + code: 12345, + grant_type: 'authorization_code', + code_verifier: '123123123123123123123123123123123123123123123' + }, + headers: {}, + method: {}, + query: {} + }); + + return handler.handleGrantType(request, client) + .then(should.fail) + .catch(function(e) { + e.should.be.an.instanceOf(InvalidGrantError); + e.message.should.equal('Invalid grant: code verifier is invalid'); + }); + }); + + it('should throw an invalid grant error when code verifier is missing', function() { + const codeVerifier = stringUtil.base64URLEncode(crypto.randomBytes(32)); + const authorizationCode = { + authorizationCode: 12345, + client: { id: 'foobar' }, + expiresAt: new Date(new Date().getTime() * 2), + user: {}, + codeChallengeMethod: 'S256', + codeChallenge: stringUtil.base64URLEncode(crypto.createHash('sha256').update(codeVerifier).digest()) + }; + const client = { id: 'foobar', grants: ['authorization_code'] }; + const token = {}; + const model = { + getAuthorizationCode: function() { return authorizationCode; }, + getClient: function() {}, + saveToken: function() { return token; }, + validateScope: function() { return 'foo'; }, + revokeAuthorizationCode: function() { return authorizationCode; } + }; + const handler = new TokenHandler({ accessTokenLifetime: 120, model: model, refreshTokenLifetime: 120 }); + const request = new Request({ + body: { + code: 12345, + grant_type: 'authorization_code' + }, + headers: {}, + method: {}, + query: {} + }); + + return handler.handleGrantType(request, client) + .then(should.fail) + .catch(function(e) { + e.should.be.an.instanceOf(InvalidGrantError); + e.message.should.equal('Missing parameter: `code_verifier`'); + }); + }); + + it('should throw an invalid grant error when code verifier is present but code challenge is missing', function() { + const authorizationCode = { + authorizationCode: 12345, + client: { id: 'foobar' }, + expiresAt: new Date(new Date().getTime() * 2), + user: {} + }; + const client = { id: 'foobar', grants: ['authorization_code'] }; + const token = {}; + const model = { + getAuthorizationCode: function() { return authorizationCode; }, + getClient: function() {}, + saveToken: function() { return token; }, + validateScope: function() { return 'foo'; }, + revokeAuthorizationCode: function() { return authorizationCode; } + }; + const handler = new TokenHandler({ accessTokenLifetime: 120, model: model, refreshTokenLifetime: 120 }); + const request = new Request({ + body: { + code: 12345, + grant_type: 'authorization_code', + code_verifier: '123123123123123123123123123123123123123123123' + }, + headers: {}, + method: {}, + query: {} + }); + + return handler.handleGrantType(request, client) + .then(should.fail) + .catch(function(e) { + e.should.be.an.instanceOf(InvalidGrantError); + e.message.should.equal('Invalid grant: code verifier is invalid'); + }); + }); + }); + describe('with grant_type `client_credentials`', function() { it('should return a token', function() { const client = { grants: ['client_credentials'] }; diff --git a/test/unit/grant-types/authorization-code-grant-type_test.js b/test/unit/grant-types/authorization-code-grant-type_test.js index 83cc854..7672ed4 100644 --- a/test/unit/grant-types/authorization-code-grant-type_test.js +++ b/test/unit/grant-types/authorization-code-grant-type_test.js @@ -5,10 +5,14 @@ */ const AuthorizationCodeGrantType = require('../../../lib/grant-types/authorization-code-grant-type'); +const InvalidGrantError = require('../../../lib/errors/invalid-grant-error'); +const ServerError = require('../../../lib/errors/server-error'); const Promise = require('bluebird'); const Request = require('../../../lib/request'); const sinon = require('sinon'); const should = require('chai').should(); +const stringUtil = require('../../../lib/utils/string-util'); +const crypto = require('crypto'); /** * Test `AuthorizationCodeGrantType`. @@ -87,4 +91,140 @@ describe('AuthorizationCodeGrantType', function() { .catch(should.fail); }); }); + + describe('with PKCE', function() { + it('should throw an error if the `code_verifier` is invalid with S256 code challenge method', function() { + const codeVerifier = stringUtil.base64URLEncode(crypto.randomBytes(32)); + const authorizationCode = { + authorizationCode: 12345, + client: { id: 'foobar' }, + expiresAt: new Date(new Date().getTime() * 2), + user: {}, + codeChallengeMethod: 'S256', + codeChallenge: stringUtil.base64URLEncode(crypto.createHash('sha256').update(codeVerifier).digest()) + }; + const client = { id: 'foobar', isPublic: true }; + const model = { + getAuthorizationCode: function() { return authorizationCode; }, + revokeAuthorizationCode: function() {}, + saveToken: function() {} + }; + const grantType = new AuthorizationCodeGrantType({ accessTokenLifetime: 123, model: model }); + const request = new Request({ body: { code: 12345, code_verifier: 'foo' }, headers: {}, method: {}, query: {} }); + + return grantType.getAuthorizationCode(request, client) + .then(should.fail) + .catch(function(e) { + e.should.be.an.instanceOf(InvalidGrantError); + e.message.should.equal('Invalid grant: code verifier is invalid'); + }); + }); + + it('should throw an error in getAuthorizationCode if an invalid code challenge method has been saved', function () { + const codeVerifier = stringUtil.base64URLEncode(crypto.randomBytes(32)); + const authorizationCode = { + authorizationCode: 12345, + client: { id: 'foobar', isPublic: true }, + expiresAt: new Date(new Date().getTime() * 2), + user: {}, + codeChallengeMethod: 'foobar', // assume this bypassed validation + codeChallenge: stringUtil.base64URLEncode(crypto.createHash('sha256').update(codeVerifier).digest()) + }; + const client = { id: 'foobar', isPublic: true }; + const model = { + getAuthorizationCode: function() { return authorizationCode; }, + revokeAuthorizationCode: function() {}, + saveToken: function() {} + }; + const grantType = new AuthorizationCodeGrantType({ accessTokenLifetime: 123, model: model }); + const request = new Request({ body: { code: 12345, code_verifier: codeVerifier }, headers: {}, method: {}, query: {} }); + + return grantType.getAuthorizationCode(request, client) + .then(should.fail) + .catch(function(e) { + e.should.be.an.instanceOf(ServerError); + e.message.should.equal('Server error: `getAuthorizationCode()` did not return a valid `codeChallengeMethod` property'); + }); + }); + + it('should throw an error if the `code_verifier` is invalid with plain code challenge method', function() { + const codeVerifier = stringUtil.base64URLEncode(crypto.randomBytes(32)); + const authorizationCode = { + authorizationCode: 12345, + client: { id: 'foobar' }, + expiresAt: new Date(new Date().getTime() * 2), + user: {}, + codeChallengeMethod: 'plain', + codeChallenge: codeVerifier + }; + // fixme: The isPublic option is not used, as a result any client which allows authorization_code grant also accepts PKCE requests. + const client = { id: 'foobar', isPublic: true }; + const model = { + getAuthorizationCode: function() { return authorizationCode; }, + revokeAuthorizationCode: function() {}, + saveToken: function() {} + }; + const grantType = new AuthorizationCodeGrantType({ accessTokenLifetime: 123, model: model }); + const request = new Request({ body: { code: 12345, code_verifier: 'foo' }, headers: {}, method: {}, query: {} }); + + return grantType.getAuthorizationCode(request, client) + .then(should.fail) + .catch(function(e) { + e.should.be.an.instanceOf(InvalidGrantError); + e.message.should.equal('Invalid grant: code verifier is invalid'); + }); + }); + + it('should return an auth code when `code_verifier` is valid with S256 code challenge method', function() { + const codeVerifier = stringUtil.base64URLEncode(crypto.randomBytes(32)); + const authorizationCode = { + authorizationCode: 12345, + client: { id: 'foobar', isPublic: true }, + expiresAt: new Date(new Date().getTime() * 2), + user: {}, + codeChallengeMethod: 'S256', + codeChallenge: stringUtil.base64URLEncode(crypto.createHash('sha256').update(codeVerifier).digest()) + }; + const client = { id: 'foobar', isPublic: true }; + const model = { + getAuthorizationCode: function() { return authorizationCode; }, + revokeAuthorizationCode: function() {}, + saveToken: function() {} + }; + const grantType = new AuthorizationCodeGrantType({ accessTokenLifetime: 123, model: model }); + const request = new Request({ body: { code: 12345, code_verifier: codeVerifier }, headers: {}, method: {}, query: {} }); + + return grantType.getAuthorizationCode(request, client) + .then(function(data) { + data.should.equal(authorizationCode); + }) + .catch(should.fail); + }); + + it('should return an auth code when `code_verifier` is valid with plain code challenge method', function() { + const codeVerifier = stringUtil.base64URLEncode(crypto.randomBytes(32)); + const authorizationCode = { + authorizationCode: 12345, + client: { id: 'foobar' }, + expiresAt: new Date(new Date().getTime() * 2), + user: {}, + codeChallengeMethod: 'plain', + codeChallenge: codeVerifier + }; + const client = { id: 'foobar', isPublic: true }; + const model = { + getAuthorizationCode: function() { return authorizationCode; }, + revokeAuthorizationCode: function() {}, + saveToken: function() {} + }; + const grantType = new AuthorizationCodeGrantType({ accessTokenLifetime: 123, model: model }); + const request = new Request({ body: { code: 12345, code_verifier: codeVerifier }, headers: {}, method: {}, query: {} }); + + return grantType.getAuthorizationCode(request, client) + .then(function(data) { + data.should.equal(authorizationCode); + }) + .catch(should.fail); + }); + }); }); diff --git a/test/unit/handlers/authorize-handler_test.js b/test/unit/handlers/authorize-handler_test.js index 376bc1e..0038c7c 100644 --- a/test/unit/handlers/authorize-handler_test.js +++ b/test/unit/handlers/authorize-handler_test.js @@ -98,6 +98,26 @@ describe('AuthorizeHandler', function() { }) .catch(should.fail); }); + + it('should call `model.saveAuthorizationCode()` with code challenge', function() { + const model = { + getAccessToken: function() {}, + getClient: function() {}, + saveAuthorizationCode: sinon.stub().returns({}) + }; + const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model }); + + return handler.saveAuthorizationCode('foo', 'bar', 'qux', 'biz', 'baz', 'boz', 'codeChallenge', 'codeChallengeMethod') + .then(function() { + model.saveAuthorizationCode.callCount.should.equal(1); + model.saveAuthorizationCode.firstCall.args.should.have.length(3); + model.saveAuthorizationCode.firstCall.args[0].should.eql({ authorizationCode: 'foo', expiresAt: 'bar', redirectUri: 'baz', scope: 'qux', codeChallenge: 'codeChallenge', codeChallengeMethod: 'codeChallengeMethod' }); + model.saveAuthorizationCode.firstCall.args[1].should.equal('biz'); + model.saveAuthorizationCode.firstCall.args[2].should.equal('boz'); + model.saveAuthorizationCode.firstCall.thisValue.should.equal(model); + }) + .catch(should.fail); + }); }); describe('validateRedirectUri()', function() { diff --git a/test/unit/pkce/pkce_test.js b/test/unit/pkce/pkce_test.js new file mode 100644 index 0000000..363eb8d --- /dev/null +++ b/test/unit/pkce/pkce_test.js @@ -0,0 +1,99 @@ +'use strict'; + +/** + * Module dependencies. + */ + +const pkce = require('../../../lib/pkce/pkce'); +const should = require('chai').should(); +const { base64URLEncode } = require('../../../lib/utils/string-util'); +const { createHash } = require('../../../lib/utils/crypto-util'); + +describe('PKCE', function() { + describe(pkce.isPKCERequest.name, function () { + it('returns, whether parameters define a PKCE request', function () { + [ + [true, 'authorization_code', 'foo'], + [true, 'authorization_code', '123123123123123123123123123123123123123123123'], + [false, 'authorization_code', ''], + [false, 'authorization_code', undefined], + [false, 'foo_code', '123123123123123123123123123123123123123123123'], + [false, '', '123123123123123123123123123123123123123123123'], + [false, undefined, '123123123123123123123123123123123123123123123'], + [false, 'foo_code', 'bar'] + ].forEach(triple => { + should.equal(triple[0], pkce.isPKCERequest({ + grantType: triple[1], + codeVerifier: triple[2] + })); + }); + }); + }); + describe(pkce.codeChallengeMatchesABNF.name, function () { + it('returns whether a string matches the criteria for codeChallenge', function () { + [ + [false, undefined], + [false, null], + [false, ''], + [false, '123123123112312312311231231231123123123112'], // too short + [false, '123123123112312312311231231231123123123112+'], // invalid chars + [false, '123123123112312312311231231231123123123112312312311231231231123123123112312312311231231231123123123112312312311231231231123123123'], // too long + // invalid chars + [true, '-_.~abcdefghijklmnopqrstuvwxyz0123456789ABCDEFHIJKLMNOPQRSTUVWXYZ'], + ].forEach(pair => { + should.equal(pair[0], pkce.codeChallengeMatchesABNF(pair[1])); + }); + }); + }); + describe(pkce.getHashForCodeChallenge.name, function () { + it('returns nothing if method is not valid', function () { + const verifier = '-_.~abcdefghijklmnopqrstuvwxyz0123456789ABCDEFHIJKLMNOPQRSTUVWXYZ'; + + [ + [undefined, undefined, verifier], + [undefined, null, verifier], + [undefined, '', verifier], + [undefined, 'foo', verifier], + ].forEach(triple => { + should.equal(triple[0], pkce.getHashForCodeChallenge({ + method: triple[1], + verifier: triple[2], + })); + }); + }); + it('return the verifier on plain and undefined on S256 if verifier is falsy', function () { + [ + [undefined, 'plain', undefined], + [undefined, 'S256', undefined], + [undefined, 'plain', ''], + [undefined, 'S256', ''], + [undefined, 'plain', null], + [undefined, 'S256', null], + ].forEach(triple => { + should.equal(triple[0], pkce.getHashForCodeChallenge({ + method: triple[1], + verifier: triple[2], + })); + }); + }); + it('returns the unhashed verifier when method is plain', function () { + const verifier = '-_.~abcdefghijklmnopqrstuvwxyz0123456789ABCDEFHIJKLMNOPQRSTUVWXYZ'; + const hash = pkce.getHashForCodeChallenge({ method: 'plain', verifier }); + should.equal(hash, verifier); + }); + it('returns the hash verifier when method is S256', function () { + const verifier = '-_.~abcdefghijklmnopqrstuvwxyz0123456789ABCDEFHIJKLMNOPQRSTUVWXYZ'; + const hash = pkce.getHashForCodeChallenge({ method: 'S256', verifier }); + const expectedHash = base64URLEncode(createHash({ data: verifier })); + should.equal(hash, expectedHash); + }); + }); + describe(pkce.isValidMethod.name, function () { + it('returns if a method is plain or S256', function () { + should.equal(pkce.isValidMethod('plain'), true); + should.equal(pkce.isValidMethod('S256'), true); + should.equal(pkce.isValidMethod('foo'), false); + should.equal(pkce.isValidMethod(), false); + }); + }); +});