From a7ed27a355c0265980fba6fc547d18f823b2296c Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Sat, 21 Sep 2019 14:09:01 +0200 Subject: [PATCH] feat: update FAPI RW behaviours --- .github/workflows/test.yml | 16 +++++ certification/fapi/api.js | 1 + certification/fapi/index.js | 34 +++++++--- certification/fapi/mtls.json | 6 +- certification/fapi/pkjwt.json | 6 +- .../authorization/check_response_mode.js | 12 +++- lib/actions/authorization/oidc_required.js | 19 +++++- .../authorization/process_request_object.js | 15 +++-- lib/helpers/features.js | 2 +- lib/models/id_token.js | 2 +- test/fapirw/fapirw.config.js | 5 +- test/fapirw/fapirw.test.js | 63 ++++++++++++++++--- 12 files changed, 146 insertions(+), 35 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4a7b475c2..ec30e3d1e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -127,6 +127,22 @@ jobs: CONFIGURATION: ./certification/fapi/mtls.json SUITE_BASE_URL: https://localhost.emobix.co.uk:8443 NODE_TLS_REJECT_UNAUTHORIZED: 0 + # - name: Run private_key_jwt-jarm variant + # run: node certification/fapi/runner + # env: + # PLAN_NAME: fapi-rw-id2-test-plan + # VARIANT: private_key_jwt-jarm + # CONFIGURATION: ./certification/fapi/pkjwt.json + # SUITE_BASE_URL: https://localhost.emobix.co.uk:8443 + # NODE_TLS_REJECT_UNAUTHORIZED: 0 + # - name: Run mtls-jarm variant + # run: node certification/fapi/runner + # env: + # PLAN_NAME: fapi-rw-id2-test-plan + # VARIANT: mtls-jarm + # CONFIGURATION: ./certification/fapi/mtls.json + # SUITE_BASE_URL: https://localhost.emobix.co.uk:8443 + # NODE_TLS_REJECT_UNAUTHORIZED: 0 deploy: runs-on: ubuntu-latest diff --git a/certification/fapi/api.js b/certification/fapi/api.js index b0c342875..84abdc356 100644 --- a/certification/fapi/api.js +++ b/certification/fapi/api.js @@ -98,6 +98,7 @@ class API { const { status, result } = await this.getModuleInfo({ moduleId }); debug('module id %s status is %s', moduleId, status); if (FINISHED.has(status)) { + if (!status || !result) continue; // eslint-disable-line no-continue if (!RESULTS.has(result)) { throw new Error(`module id ${moduleId} is ${status} but ${result}`); } diff --git a/certification/fapi/index.js b/certification/fapi/index.js index 146f431a6..ba40e1fcc 100644 --- a/certification/fapi/index.js +++ b/certification/fapi/index.js @@ -64,9 +64,13 @@ const fapi = new Provider(ISSUER, { clients: [ { client_id: 'pkjwt-one', + response_types: ['code', 'code id_token'], + grant_types: ['implicit', 'authorization_code', 'refresh_token'], redirect_uris: [ - `${SUITE_BASE_URL}/test/a/pkjwt/callback`, - `${SUITE_BASE_URL}/test/a/pkjwt/callback?dummy1=lorem&dummy2=ipsum`, + `${SUITE_BASE_URL}/test/a/oidc-provider-pkjwt/callback`, + 'https://staging.certification.openid.net/test/a/oidc-provider-pkjwt/callback', + `${SUITE_BASE_URL}/test/a/oidc-provider-pkjwt/callback?dummy1=lorem&dummy2=ipsum`, + 'https://staging.certification.openid.net/test/a/oidc-provider-pkjwt/callback?dummy1=lorem&dummy2=ipsum', ], token_endpoint_auth_method: 'private_key_jwt', jwks: { @@ -75,9 +79,13 @@ const fapi = new Provider(ISSUER, { }, { client_id: 'pkjwt-two', + response_types: ['code', 'code id_token'], + grant_types: ['implicit', 'authorization_code', 'refresh_token'], redirect_uris: [ - `${SUITE_BASE_URL}/test/a/pkjwt/callback`, - `${SUITE_BASE_URL}/test/a/pkjwt/callback?dummy1=lorem&dummy2=ipsum`, + `${SUITE_BASE_URL}/test/a/oidc-provider-pkjwt/callback`, + 'https://staging.certification.openid.net/test/a/oidc-provider-pkjwt/callback', + `${SUITE_BASE_URL}/test/a/oidc-provider-pkjwt/callback?dummy1=lorem&dummy2=ipsum`, + 'https://staging.certification.openid.net/test/a/oidc-provider-pkjwt/callback?dummy1=lorem&dummy2=ipsum', ], token_endpoint_auth_method: 'private_key_jwt', jwks: { @@ -86,9 +94,13 @@ const fapi = new Provider(ISSUER, { }, { client_id: 'mtls-one', + response_types: ['code', 'code id_token'], + grant_types: ['implicit', 'authorization_code', 'refresh_token'], redirect_uris: [ - `${SUITE_BASE_URL}/test/a/mtls/callback`, - `${SUITE_BASE_URL}/test/a/mtls/callback?dummy1=lorem&dummy2=ipsum`, + `${SUITE_BASE_URL}/test/a/oidc-provider-mtls/callback`, + 'https://staging.certification.openid.net/test/a/oidc-provider-mtls/callback', + `${SUITE_BASE_URL}/test/a/oidc-provider-mtls/callback?dummy1=lorem&dummy2=ipsum`, + 'https://staging.certification.openid.net/test/a/oidc-provider-mtls/callback?dummy1=lorem&dummy2=ipsum', ], token_endpoint_auth_method: 'self_signed_tls_client_auth', jwks: { @@ -97,9 +109,13 @@ const fapi = new Provider(ISSUER, { }, { client_id: 'mtls-two', + response_types: ['code', 'code id_token'], + grant_types: ['implicit', 'authorization_code', 'refresh_token'], redirect_uris: [ - `${SUITE_BASE_URL}/test/a/mtls/callback`, - `${SUITE_BASE_URL}/test/a/mtls/callback?dummy1=lorem&dummy2=ipsum`, + `${SUITE_BASE_URL}/test/a/oidc-provider-mtls/callback`, + 'https://staging.certification.openid.net/test/a/oidc-provider-mtls/callback', + `${SUITE_BASE_URL}/test/a/oidc-provider-mtls/callback?dummy1=lorem&dummy2=ipsum`, + 'https://staging.certification.openid.net/test/a/oidc-provider-mtls/callback?dummy1=lorem&dummy2=ipsum', ], token_endpoint_auth_method: 'self_signed_tls_client_auth', jwks: { @@ -152,7 +168,7 @@ const fapi = new Provider(ISSUER, { }, revocation: { enabled: true }, }, - responseTypes: ['code id_token'], + responseTypes: ['code id_token', 'code'], tokenEndpointAuthMethods, whitelistedJWA: { authorizationSigningAlgValues: ALGS, diff --git a/certification/fapi/mtls.json b/certification/fapi/mtls.json index 61b7c9997..b5efeda09 100644 --- a/certification/fapi/mtls.json +++ b/certification/fapi/mtls.json @@ -1,5 +1,5 @@ { - "alias": "mtls", + "alias": "oidc-provider-mtls", "server": { "discoveryUrl": "https://fapi.panva.cz/.well-known/openid-configuration" }, @@ -36,7 +36,7 @@ }, "client2": { "client_id": "mtls-two", - "scope": "openid", + "scope": "openid offline_access", "jwks": { "keys": [ { @@ -111,7 +111,7 @@ }, { "task": "Verify Complete", - "match": "https://*/test/a/mtls/callback*", + "match": "https://*/test/a/oidc-provider-mtls/callback*", "commands": [ [ "wait", diff --git a/certification/fapi/pkjwt.json b/certification/fapi/pkjwt.json index ebc5be5dc..cdc54f456 100644 --- a/certification/fapi/pkjwt.json +++ b/certification/fapi/pkjwt.json @@ -1,5 +1,5 @@ { - "alias": "pkjwt", + "alias": "oidc-provider-pkjwt", "server": { "discoveryUrl": "https://fapi.panva.cz/.well-known/openid-configuration" }, @@ -31,7 +31,7 @@ }, "client2": { "client_id": "pkjwt-two", - "scope": "openid", + "scope": "openid offline_access", "jwks": { "keys": [ { @@ -101,7 +101,7 @@ }, { "task": "Verify Complete", - "match": "https://*/test/a/pkjwt/callback*", + "match": "https://*/test/a/oidc-provider-pkjwt/callback*", "commands": [ [ "wait", diff --git a/lib/actions/authorization/check_response_mode.js b/lib/actions/authorization/check_response_mode.js index e71c2efff..5ee247ae4 100644 --- a/lib/actions/authorization/check_response_mode.js +++ b/lib/actions/authorization/check_response_mode.js @@ -8,7 +8,7 @@ const { isFrontChannel } = require('../../helpers/resolve_response_mode'); * * @throws: invalid_request */ -module.exports = function checkResponseMode(ctx, next) { +module.exports = function checkResponseMode(ctx, next, forceCheck) { const { params, client } = ctx.oidc; const frontChannel = isFrontChannel(params.response_type); @@ -23,8 +23,10 @@ module.exports = function checkResponseMode(ctx, next) { throw new UnsupportedResponseMode(); } + const JWT = /jwt/.test(mode); + if ( - mode !== undefined && mode.includes('jwt') + mode !== undefined && JWT && ( /^HS/.test(client.authorizationSignedResponseAlg) || /^(A|P|dir$)/.test(client.authorizationEncryptedResponseAlg) @@ -45,5 +47,11 @@ module.exports = function checkResponseMode(ctx, next) { throw new InvalidRequest('response_mode not allowed for this response_type unless encrypted'); } + if (params.response_type && instance(ctx.oidc.provider).configuration('features.fapiRW.enabled')) { + if ((!params.request || forceCheck) && !params.response_type.includes('id_token') && !JWT) { + throw new InvalidRequest('response_mode not allowed for this response_type in FAPI mode'); + } + } + return next(); }; diff --git a/lib/actions/authorization/oidc_required.js b/lib/actions/authorization/oidc_required.js index 12232b80e..2054750ff 100644 --- a/lib/actions/authorization/oidc_required.js +++ b/lib/actions/authorization/oidc_required.js @@ -1,3 +1,4 @@ +const instance = require('../../helpers/weak_cache'); const presence = require('../../helpers/validate_presence'); /* @@ -10,13 +11,25 @@ const presence = require('../../helpers/validate_presence'); module.exports = function oidcRequired(ctx, next) { const { params } = ctx.oidc; + const required = new Set(['redirect_uri']); + // Check for nonce if implicit or hybrid flow responding with id_token issued by the authorization // endpoint if (typeof params.response_type === 'string' && params.response_type.includes('id_token')) { - presence(ctx, 'redirect_uri', 'nonce'); - } else { - presence(ctx, 'redirect_uri'); + required.add('nonce'); } + // TODO: add the following once https://bitbucket.org/openid/fapi/issues/270/jarm-fapi-rw-openid-client-session-binding + // is resolved and the FAPI suite updated + // else if (instance(ctx.oidc.provider).configuration('features.fapiRW.enabled')) { + // required.add('state'); + // } + + if (instance(ctx.oidc.provider).configuration('features.fapiRW.enabled')) { + required.add(ctx.oidc.requestParamScopes.has('openid') ? 'nonce' : 'state'); + } + + presence(ctx, ...required); + return next(); }; diff --git a/lib/actions/authorization/process_request_object.js b/lib/actions/authorization/process_request_object.js index 75ed47280..926d9269a 100644 --- a/lib/actions/authorization/process_request_object.js +++ b/lib/actions/authorization/process_request_object.js @@ -92,9 +92,16 @@ module.exports = async function processRequestObject(PARAM_LIST, rejectDupesMidd params.state = request.state; } - if (request.response_mode !== undefined) { - params.response_mode = request.response_mode; - checkResponseMode(ctx, () => {}); + const fapiRW = conf('features.fapiRW.enabled'); + + if (request.response_mode !== undefined || fapiRW) { + if (request.response_mode !== undefined) { + params.response_mode = request.response_mode; + } + if (request.response_type !== undefined) { + params.response_type = request.response_type; + } + checkResponseMode(ctx, () => {}, fapiRW); } if (request.request !== undefined || request.request_uri !== undefined) { @@ -142,7 +149,7 @@ module.exports = async function processRequestObject(PARAM_LIST, rejectDupesMidd ignoreAzp: true, }; - if (conf('features.fapiRW.enabled')) { + if (fapiRW) { if (!('exp' in payload)) { throw new InvalidRequestObject('Request Object is missing the "exp" claim'); } diff --git a/lib/helpers/features.js b/lib/helpers/features.js index 361f5e007..c76e10f17 100644 --- a/lib/helpers/features.js +++ b/lib/helpers/features.js @@ -30,7 +30,7 @@ const DRAFTS = new Map(Object.entries({ name: 'Financial-grade API - Part 2: Read and Write API Security Profile', type: 'OIDF FAPI Working Group draft', url: 'https://openid.net/specs/openid-financial-api-part-2-ID2.html', - version: 'id02-rev.2', + version: 'id02-rev.3', }, mTLS: { name: 'OAuth 2.0 Mutual TLS Client Authentication and Certificate-Bound Access Tokens - draft 17', diff --git a/lib/models/id_token.js b/lib/models/id_token.js index efae3ba61..4cdd9e552 100644 --- a/lib/models/id_token.js +++ b/lib/models/id_token.js @@ -124,7 +124,7 @@ module.exports = function getIdToken(provider) { alg = client.authorizationSignedResponseAlg; signOptions = { audience: client.clientId, - expiresIn: 60, + expiresIn: 120, issuer: provider.issuer, noIat: true, }; diff --git a/test/fapirw/fapirw.config.js b/test/fapirw/fapirw.config.js index 4db1f7cce..627e7b50c 100644 --- a/test/fapirw/fapirw.config.js +++ b/test/fapirw/fapirw.config.js @@ -4,6 +4,7 @@ const config = clone(require('../default.config')); config.features = { fapiRW: { enabled: true }, + jwtResponseModes: { enabled: true }, requestObjects: { request: true, mergingStrategy: { name: 'strict' }, @@ -17,8 +18,8 @@ module.exports = { config, clients: [{ client_id: 'client', - response_types: ['id_token'], - grant_types: ['implicit'], + response_types: ['code id_token', 'code'], + grant_types: ['implicit', 'authorization_code'], redirect_uris: ['https://client.example.com/cb'], token_endpoint_auth_method: 'none', }], diff --git a/test/fapirw/fapirw.test.js b/test/fapirw/fapirw.test.js index 4b3d18774..0ef8dfac8 100644 --- a/test/fapirw/fapirw.test.js +++ b/test/fapirw/fapirw.test.js @@ -31,6 +31,57 @@ describe('Financial-grade API - Part 2: Read and Write API Security Profile beha }); }); + describe('FAPI Mode Authorization Request', () => { + beforeEach(function () { return this.login(); }); + afterEach(function () { return this.logout(); }); + + it('requires jwt response mode to be used when id token is not issued by authorization endpoint', function () { + const auth = new this.AuthorizationRequest({ + scope: 'openid', + client_id: 'client', + response_type: 'code', + nonce: 'foo', // TODO: see oidc_required.js + }); + + return this.wrap({ + agent: this.agent, + route: '/auth', + verb: 'get', + auth, + }) + .expect(302) + .expect(auth.validateClientLocation) + .expect(auth.validateError('invalid_request')) + .expect(auth.validateErrorDescription('response_mode not allowed for this response_type in FAPI mode')); + }); + + it('requires jwt response mode to be used when id token is not issued by authorization endpoint (JAR)', function () { + const request = `${base64url.encode(JSON.stringify({ alg: 'none' }))}.${base64url.encode(JSON.stringify({ + scope: 'openid', + client_id: 'client', + response_type: 'code', + nonce: 'foo', // TODO: see oidc_required.js + exp: epochTime() + 60, + }))}.`; + + const auth = new this.AuthorizationRequest({ + request, + state: undefined, + }); + + return this.wrap({ + agent: this.agent, + route: '/auth', + verb: 'get', + auth, + }) + .expect(302) + .expect(auth.validateClientLocation) + .expect(auth.validateError('invalid_request')) + .expect(auth.validateErrorDescription('response_mode not allowed for this response_type in FAPI mode')); + }); + }); + describe('Request Object', () => { beforeEach(function () { return this.login(); }); afterEach(function () { return this.logout(); }); @@ -39,7 +90,7 @@ describe('Financial-grade API - Part 2: Read and Write API Security Profile beha const request = `${base64url.encode(JSON.stringify({ alg: 'none' }))}.${base64url.encode(JSON.stringify({ client_id: 'client', scope: 'openid', - response_type: 'id_token', + response_type: 'code id_token', nonce: 'foo', redirect_uri: 'https://client.example.com/cb', state: 'foo', @@ -50,7 +101,7 @@ describe('Financial-grade API - Part 2: Read and Write API Security Profile beha request, scope: 'openid', client_id: 'client', - response_type: 'id_token', + response_type: 'code id_token', nonce: 'foo', state: 'foo', }); @@ -63,8 +114,7 @@ describe('Financial-grade API - Part 2: Read and Write API Security Profile beha }) .expect(302) .expect(auth.validateFragment) - .expect(auth.validatePresence(['id_token', 'state'])) - .expect(auth.validateState) + .expect(auth.validatePresence(['id_token', 'code', 'state'])) .expect(auth.validateClientLocation); }); @@ -72,7 +122,7 @@ describe('Financial-grade API - Part 2: Read and Write API Security Profile beha const request = `${base64url.encode(JSON.stringify({ alg: 'none' }))}.${base64url.encode(JSON.stringify({ client_id: 'client', scope: 'openid', - response_type: 'id_token', + response_type: 'code id_token', nonce: 'foo', }))}.`; @@ -80,7 +130,7 @@ describe('Financial-grade API - Part 2: Read and Write API Security Profile beha request, scope: 'openid', client_id: 'client', - response_type: 'id_token', + response_type: 'code id_token', nonce: 'foo', }); @@ -93,7 +143,6 @@ describe('Financial-grade API - Part 2: Read and Write API Security Profile beha .expect(302) .expect(auth.validateFragment) .expect(auth.validatePresence(['error', 'error_description', 'state'])) - .expect(auth.validateState) .expect(auth.validateClientLocation) .expect(auth.validateError('invalid_request_object')) .expect(auth.validateErrorDescription('Request Object is missing the "exp" claim'));