From c20682a242f3566ebb32796730cef2e175f3f072 Mon Sep 17 00:00:00 2001 From: Vijay Budhram Date: Tue, 2 Oct 2018 11:22:38 -0400 Subject: [PATCH] feat(2fa): check acr values during authorization flow --- lib/error.js | 9 +++++++++ lib/routes/authorization.js | 15 ++++++++++++++- test/api.js | 36 ++++++++++++++++++++++++++++++++++-- 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/lib/error.js b/lib/error.js index ec394f5..bee5434 100644 --- a/lib/error.js +++ b/lib/error.js @@ -276,4 +276,13 @@ AppError.staleAuthAt = function staleAuthAt(authAt) { }); }; +AppError.mismatchAcr = function mismatchAcr(foundValue) { + return new AppError({ + code: 400, + error: 'Bad Request', + errno: 120, + message: 'Mismatch acr value' + }, {foundValue}); +}; + module.exports = AppError; diff --git a/lib/routes/authorization.js b/lib/routes/authorization.js index 9bb72e0..1d3ff26 100644 --- a/lib/routes/authorization.js +++ b/lib/routes/authorization.js @@ -22,6 +22,8 @@ const TOKEN = 'token'; const ACCESS_TYPE_ONLINE = 'online'; const ACCESS_TYPE_OFFLINE = 'offline'; +const ACR_VALUE_AAL2 = 'AAL2'; + const PKCE_SHA256_CHALLENGE_METHOD = 'S256'; // This server only supports S256 PKCE, no 'plain' const PKCE_CODE_CHALLENGE_LENGTH = 43; @@ -189,7 +191,8 @@ module.exports = { is: CODE, then: Joi.optional(), otherwise: Joi.forbidden() - }) + }), + acr_values: Joi.string().max(256).optional() } }, response: { @@ -228,6 +231,16 @@ module.exports = { exitEarly = true; throw AppError.invalidAssertion(); } + + // Check to see if the acr value requested by oauth matches what is expected + const acrValues = req.payload.acr_values; + if (acrValues) { + const acrTokens = acrValues.split('\s+'); + if (acrTokens.includes(ACR_VALUE_AAL2) && ! (claims['fxa-aal'] >= 2)) { + throw AppError.mismatchAcr(claims['fxa-aal']); + } + } + // Any request for a key-bearing scope should be using a verified token. // Double-check that here as a defense-in-depth measure. if (! claims['fxa-tokenVerified']) { diff --git a/test/api.js b/test/api.js index 5911672..f910079 100644 --- a/test/api.js +++ b/test/api.js @@ -119,7 +119,8 @@ function authParams(params, options) { assertion: AN_ASSERTION, client_id: options.clientId || clientId, state: '1', - scope: 'a' + scope: 'a', + acr_values: options.acr_values || undefined }; params = params || {}; @@ -1001,7 +1002,6 @@ describe('/v1', function() { }); }); - describe('response', function() { describe('with a trusted client', function() { it('should redirect to the redirect_uri', function() { @@ -1025,6 +1025,38 @@ describe('/v1', function() { }); }); + describe('check acr payload', () => { + it('should throw error if mismatch with claims', () => { + const options = {aal: 1}; + const payload = {acr_values: 'AAL2'}; + mockAssertion().reply(200, mockVerifierResult(options)); + return Server.api.post({ + url: '/authorization', + payload: authParams(payload) + }).then(function (res) { + assert.equal(res.statusCode, 400); + assertSecurityHeaders(res); + assert.equal(res.result.message, 'Mismatch acr value'); + assert.equal(res.result.errno, 120, 'correct errno'); + }); + }); + + it('process request when correct acr_values in claims', () => { + const options = {aal: 2}; + const payload = {acr_values: 'AAL2'}; + mockAssertion().reply(200, mockVerifierResult(options)); + return Server.api.post({ + url: '/authorization', + payload: authParams(payload) + }).then(function (res) { + assert.equal(res.statusCode, 200); + assertSecurityHeaders(res); + assert.ok(res.result.code, 'code set'); + assert.ok(res.result.redirect, 'redirect set'); + assert.equal(res.result.state, 1, 'correct state'); + }); + }); + }); }); describe('/token', function() {