From 6e3298ad3d012d2874f897d344cbc7b3da8656bd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20K=C3=BCster?= <jkuester@uni-bremen.de>
Date: Thu, 25 Nov 2021 11:29:07 +0100
Subject: [PATCH 1/7] feature(pkce): added pkce support
---
.../authorization-code-grant-type.js | 31 +++
lib/handlers/token-handler.js | 12 +-
lib/pkce/pkce.js | 77 +++++++
lib/utils/crypto-util.js | 24 +++
lib/utils/string-util.js | 19 ++
lib/utils/token-util.js | 7 +-
.../handlers/token-handler_test.js | 193 ++++++++++++++++++
.../authorization-code-grant-type_test.js | 112 ++++++++++
test/unit/pkce/pkce_test.js | 99 +++++++++
9 files changed, 568 insertions(+), 6 deletions(-)
create mode 100644 lib/pkce/pkce.js
create mode 100644 lib/utils/crypto-util.js
create mode 100644 lib/utils/string-util.js
create mode 100644 test/unit/pkce/pkce_test.js
diff --git a/lib/grant-types/authorization-code-grant-type.js b/lib/grant-types/authorization-code-grant-type.js
index 8f21aef..fd7e19d 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 is = require('../validator/is');
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/token-handler.js b/lib/handlers/token-handler.js
index 8195969..5aa312c 100644
--- a/lib/handlers/token-handler.js
+++ b/lib/handlers/token-handler.js
@@ -19,6 +19,7 @@ const UnauthorizedClientError = require('../errors/unauthorized-client-error');
const UnsupportedGrantTypeError = require('../errors/unsupported-grant-type-error');
const auth = require('basic-auth');
const is = require('../validator/is');
+const pkce = require('../pkce/pkce');
/**
* Grant types.
@@ -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/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..80e5bb2 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,13 @@
*/
const AuthorizationCodeGrantType = require('../../../lib/grant-types/authorization-code-grant-type');
+const InvalidGrantError = require('../../../lib/errors/invalid-grant-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 +90,113 @@ 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 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/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);
+ });
+ });
+});
From 3242fc23d9fd90ee8304793f932510cb3d4ccaff Mon Sep 17 00:00:00 2001
From: Jonathan Martinsson <jonathan.martinsson@gmail.com>
Date: Mon, 26 Sep 2022 09:41:37 +0200
Subject: [PATCH 2/7] save code challenge with authorization code
---
index.d.ts | 4 +++-
lib/handlers/authorize-handler.js | 18 +++++++++++++++---
test/unit/handlers/authorize-handler_test.js | 4 ++--
3 files changed, 20 insertions(+), 6 deletions(-)
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<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;
}
diff --git a/lib/handlers/authorize-handler.js b/lib/handlers/authorize-handler.js
index 830bfa7..22bb442 100644
--- a/lib/handlers/authorize-handler.js
+++ b/lib/handlers/authorize-handler.js
@@ -114,8 +114,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,12 +295,14 @@ AuthorizeHandler.prototype.getRedirectUri = function(request, client) {
* Save authorization code.
*/
-AuthorizeHandler.prototype.saveAuthorizationCode = function(authorizationCode, expiresAt, scope, client, redirectUri, user) {
+AuthorizeHandler.prototype.saveAuthorizationCode = function(authorizationCode, expiresAt, scope, client, redirectUri, user, codeChallenge, codeChallengeMethod) {
const code = {
authorizationCode: authorizationCode,
expiresAt: expiresAt,
redirectUri: redirectUri,
- scope: scope
+ scope: scope,
+ codeChallenge: codeChallenge,
+ codeChallengeMethod: codeChallengeMethod
};
return promisify(this.model.saveAuthorizationCode, 3).call(this.model, code, client, user);
};
@@ -369,6 +373,14 @@ AuthorizeHandler.prototype.updateResponse = function(response, redirectUri, stat
response.redirect(url.format(redirectUri));
};
+AuthorizeHandler.prototype.getCodeChallenge = function(request) {
+ return request.body.code_challenge || request.query.code_challenge;
+};
+
+AuthorizeHandler.prototype.getCodeChallengeMethod = function(request) {
+ return request.body.code_challenge_method || request.query.code_challenge_method;
+};
+
/**
* Export constructor.
*/
diff --git a/test/unit/handlers/authorize-handler_test.js b/test/unit/handlers/authorize-handler_test.js
index 376bc1e..0c7a3db 100644
--- a/test/unit/handlers/authorize-handler_test.js
+++ b/test/unit/handlers/authorize-handler_test.js
@@ -87,11 +87,11 @@ describe('AuthorizeHandler', function() {
};
const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model });
- return handler.saveAuthorizationCode('foo', 'bar', 'qux', 'biz', 'baz', 'boz')
+ 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' });
+ 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);
From c599cb49371ed4d34c68b112bb77000fee68b943 Mon Sep 17 00:00:00 2001
From: Jonathan Martinsson <jonathan.martinsson@gmail.com>
Date: Mon, 10 Oct 2022 12:12:41 +0200
Subject: [PATCH 3/7] Use default code challenge method plain if missing
* only allow parameters in body
---
lib/handlers/authorize-handler.js | 8 +++-
.../handlers/authorize-handler_test.js | 40 +++++++++++++++++++
2 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/lib/handlers/authorize-handler.js b/lib/handlers/authorize-handler.js
index 22bb442..136d00e 100644
--- a/lib/handlers/authorize-handler.js
+++ b/lib/handlers/authorize-handler.js
@@ -374,11 +374,15 @@ AuthorizeHandler.prototype.updateResponse = function(response, redirectUri, stat
};
AuthorizeHandler.prototype.getCodeChallenge = function(request) {
- return request.body.code_challenge || request.query.code_challenge;
+ 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
+ */
AuthorizeHandler.prototype.getCodeChallengeMethod = function(request) {
- return request.body.code_challenge_method || request.query.code_challenge_method;
+ return request.body.code_challenge_method || 'plain';
};
/**
diff --git a/test/integration/handlers/authorize-handler_test.js b/test/integration/handlers/authorize-handler_test.js
index 054e2cc..e14b12c 100644
--- a/test/integration/handlers/authorize-handler_test.js
+++ b/test/integration/handlers/authorize-handler_test.js
@@ -1321,4 +1321,44 @@ 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 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');
+ });
+ });
});
From c597a2406045d4b1fe958a4d57abaf15c1fa5924 Mon Sep 17 00:00:00 2001
From: Jonathan Martinsson <jonathan.martinsson@gmail.com>
Date: Mon, 31 Oct 2022 10:54:18 +0100
Subject: [PATCH 4/7] only add code challenge properties to code when
codeChallenge and codeChallengeMethod ar set
---
lib/handlers/authorize-handler.js | 13 +++++++++----
test/unit/handlers/authorize-handler_test.js | 20 ++++++++++++++++++++
2 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/lib/handlers/authorize-handler.js b/lib/handlers/authorize-handler.js
index 136d00e..9a586d2 100644
--- a/lib/handlers/authorize-handler.js
+++ b/lib/handlers/authorize-handler.js
@@ -296,14 +296,19 @@ AuthorizeHandler.prototype.getRedirectUri = function(request, client) {
*/
AuthorizeHandler.prototype.saveAuthorizationCode = function(authorizationCode, expiresAt, scope, client, redirectUri, user, codeChallenge, codeChallengeMethod) {
- const code = {
+ let code = {
authorizationCode: authorizationCode,
expiresAt: expiresAt,
redirectUri: redirectUri,
- scope: scope,
- codeChallenge: codeChallenge,
- codeChallengeMethod: codeChallengeMethod
+ 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);
};
diff --git a/test/unit/handlers/authorize-handler_test.js b/test/unit/handlers/authorize-handler_test.js
index 0c7a3db..0038c7c 100644
--- a/test/unit/handlers/authorize-handler_test.js
+++ b/test/unit/handlers/authorize-handler_test.js
@@ -87,6 +87,26 @@ describe('AuthorizeHandler', function() {
};
const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model });
+ return handler.saveAuthorizationCode('foo', 'bar', 'qux', 'biz', 'baz', 'boz')
+ .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' });
+ 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);
+ });
+
+ 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);
From 6e6edcb21146080188cfa2f3cbebed9d0ca132df Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20K=C3=BCster?= <jkuester@uni-bremen.de>
Date: Sun, 27 Nov 2022 11:32:20 +0100
Subject: [PATCH 5/7] fix(tests): move same line statements in seperate lines
Co-authored-by: Jonathan Martinsson <jonathan.martinsson@gmail.com>
---
test/integration/handlers/authorize-handler_test.js | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/test/integration/handlers/authorize-handler_test.js b/test/integration/handlers/authorize-handler_test.js
index 80905dc..c242269 100644
--- a/test/integration/handlers/authorize-handler_test.js
+++ b/test/integration/handlers/authorize-handler_test.js
@@ -1309,7 +1309,8 @@ describe('AuthorizeHandler integration', 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 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');
@@ -1321,7 +1322,8 @@ describe('AuthorizeHandler integration', function() {
getClient: function() {},
saveAuthorizationCode: function() {}
};
- const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model }); const request = new Request({ body: {}, headers: {}, method: {}, query: {} });
+ 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');
@@ -1335,7 +1337,8 @@ describe('AuthorizeHandler integration', 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 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');
From 2411f92993e9236e31adaae47f9c0a5723fd90b1 Mon Sep 17 00:00:00 2001
From: jankapunkt <jkuester@uni-bremen.de>
Date: Sun, 27 Nov 2022 11:54:35 +0100
Subject: [PATCH 6/7] compliance(pkce): throw InvalidRequestError if code
request contains unsupported code challenge method
---
lib/handlers/authorize-handler.js | 12 ++++++++++-
.../handlers/authorize-handler_test.js | 20 +++++++++++++++++++
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/lib/handlers/authorize-handler.js b/lib/handlers/authorize-handler.js
index d5c2cf5..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.
@@ -381,9 +382,18 @@ AuthorizeHandler.prototype.getCodeChallenge = function(request) {
/**
* 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) {
- return request.body.code_challenge_method || 'plain';
+ 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';
};
/**
diff --git a/test/integration/handlers/authorize-handler_test.js b/test/integration/handlers/authorize-handler_test.js
index c242269..b91408a 100644
--- a/test/integration/handlers/authorize-handler_test.js
+++ b/test/integration/handlers/authorize-handler_test.js
@@ -1316,6 +1316,26 @@ describe('AuthorizeHandler integration', function() {
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() {},
From b7999859eb3290d4149b34d599bae1a09c9f1198 Mon Sep 17 00:00:00 2001
From: jankapunkt <jkuester@uni-bremen.de>
Date: Mon, 28 Nov 2022 09:58:30 +0100
Subject: [PATCH 7/7] test(pkce): added test for bypassed saving unsupported
code challenge method
---
.../authorization-code-grant-type_test.js | 28 +++++++++++++++++++
1 file changed, 28 insertions(+)
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 80e5bb2..7672ed4 100644
--- a/test/unit/grant-types/authorization-code-grant-type_test.js
+++ b/test/unit/grant-types/authorization-code-grant-type_test.js
@@ -6,6 +6,7 @@
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');
@@ -119,6 +120,33 @@ describe('AuthorizationCodeGrantType', function() {
});
});
+ 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 = {