From 18efa7083fb4abde4e48f2930f728f20e94a258e Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Tue, 16 Jan 2024 22:00:31 +0100 Subject: [PATCH] refactor(JAR): Request Objects are no longer checked for one time use --- lib/actions/authorization/check_dpop_jkt.js | 18 +++---- .../authorization/process_request_object.js | 27 ---------- .../pushed_authorization_requests.test.js | 31 ----------- test/request/jwt_request.test.js | 51 ------------------- 4 files changed, 8 insertions(+), 119 deletions(-) diff --git a/lib/actions/authorization/check_dpop_jkt.js b/lib/actions/authorization/check_dpop_jkt.js index 01282d388..f53bd6ab4 100644 --- a/lib/actions/authorization/check_dpop_jkt.js +++ b/lib/actions/authorization/check_dpop_jkt.js @@ -7,20 +7,18 @@ import epochTime from '../../helpers/epoch_time.js'; * when provided, otherwise defaults dpop_jkt to it. */ export default async function checkDpopJkt(ctx, next) { - const { params, route } = ctx.oidc; + const { params } = ctx.oidc; const dPoP = await dpopValidate(ctx); if (dPoP) { - if (route !== 'pushed_authorization_request') { - const { ReplayDetection } = ctx.oidc.provider; - const unique = await ReplayDetection.unique( - ctx.oidc.client.clientId, - dPoP.jti, - epochTime() + DPOP_OK_WINDOW, - ); + const { ReplayDetection } = ctx.oidc.provider; + const unique = await ReplayDetection.unique( + ctx.oidc.client.clientId, + dPoP.jti, + epochTime() + DPOP_OK_WINDOW, + ); - ctx.assert(unique, new InvalidRequest('DPoP proof JWT Replay detected')); - } + ctx.assert(unique, new InvalidRequest('DPoP proof JWT Replay detected')); if (params.dpop_jkt && params.dpop_jkt !== dPoP.thumbprint) { throw new InvalidRequest('DPoP proof key thumbprint does not match dpop_jkt'); diff --git a/lib/actions/authorization/process_request_object.js b/lib/actions/authorization/process_request_object.js index 018ab5484..db12ad912 100644 --- a/lib/actions/authorization/process_request_object.js +++ b/lib/actions/authorization/process_request_object.js @@ -2,8 +2,6 @@ import * as JWT from '../../helpers/jwt.js'; import instance from '../../helpers/weak_cache.js'; import { InvalidRequest, InvalidRequestObject, OIDCProviderError } from '../../helpers/errors.js'; import isPlainObject from '../../helpers/_/is_plain_object.js'; -import dpopValidate, { DPOP_OK_WINDOW } from '../../helpers/validate_dpop.js'; -import epochTime from '../../helpers/epoch_time.js'; /* * Decrypts and validates the content of provided request parameter and replaces the parameters @@ -190,31 +188,6 @@ export default async function processRequestObject(PARAM_LIST, rejectDupesMiddle } } - if (!pushedRequestObject && payload.jti && payload.exp && payload.iss) { - if (route === 'pushed_authorization_request') { - const dPoP = await dpopValidate(ctx); - if (dPoP) { - const { ReplayDetection } = ctx.oidc.provider; - const unique = await ReplayDetection.unique( - ctx.oidc.client.clientId, - dPoP.jti, - epochTime() + DPOP_OK_WINDOW, - ); - - ctx.assert(unique, new InvalidRequest('DPoP proof JWT Replay detected')); - } - } - const unique = await ctx.oidc.provider.ReplayDetection.unique( - payload.iss, - payload.jti, - payload.exp + conf.clockTolerance, - ); - - if (!unique) { - throw new InvalidRequestObject('request object replay detected'); - } - } - if (trusted) { ctx.oidc.trusted = Object.keys(request); } else if (ctx.oidc.insecureRequestUri) { diff --git a/test/pushed_authorization_requests/pushed_authorization_requests.test.js b/test/pushed_authorization_requests/pushed_authorization_requests.test.js index 5463a96e5..5c263e9b9 100644 --- a/test/pushed_authorization_requests/pushed_authorization_requests.test.js +++ b/test/pushed_authorization_requests/pushed_authorization_requests.test.js @@ -329,37 +329,6 @@ describe('Pushed Request Object', () => { expect(spy).to.have.property('calledOnce', true); }); - it('checks for request object replay', async function () { - const request = await JWT.sign({ - jti: randomBytes(16).toString('base64url'), - response_type: 'code', - client_id: clientId, - iss: clientId, - aud: this.provider.issuer, - }, this.key, 'HS256', { - expiresIn: 30, - }); - - await this.agent.post('/request') - .auth(clientId, 'secret') - .type('form') - .send({ request }) - .expect(201) - .expect(({ body }) => { - expect(body).to.have.keys('expires_in', 'request_uri'); - }); - - await this.agent.post('/request') - .auth(clientId, 'secret') - .type('form') - .send({ request }) - .expect(400) - .expect(({ body }) => { - expect(body).to.have.property('error', 'invalid_request_object'); - expect(body).to.have.property('error_description').and.matches(/^request object replay detected/); - }); - }); - it('defaults to MAX_TTL when no expires_in is present', async function () { const spy = sinon.spy(); this.provider.once('pushed_authorization_request.success', spy); diff --git a/test/request/jwt_request.test.js b/test/request/jwt_request.test.js index d144cf74a..8ae6873f6 100644 --- a/test/request/jwt_request.test.js +++ b/test/request/jwt_request.test.js @@ -395,57 +395,6 @@ describe('request parameter features', () => { })); }); - it('supports optional replay prevention', async function () { - const client = await this.provider.Client.find('client-with-HS-sig'); - let [key] = client.symmetricKeyStore.selectForSign({ alg: 'HS256' }); - key = await importJWK(key); - - const request = await JWT.sign({ - jti: randomBytes(16).toString('base64url'), - client_id: 'client-with-HS-sig', - response_type: 'code', - redirect_uri: 'https://client.example.com/cb', - scope: 'openid', - }, key, 'HS256', { - issuer: 'client-with-HS-sig', audience: this.provider.issuer, expiresIn: 30, - }); - - await this.wrap({ - agent: this.agent, - route, - verb, - auth: { - request, - scope: 'openid', - client_id: 'client-with-HS-sig', - response_type: 'code', - }, - }) - .expect(successCode) - .expect(successFnCheck); - - const spy = sinon.spy(); - this.provider.once(errorEvt, spy); - - await this.wrap({ - agent: this.agent, - route, - verb, - auth: { - request, - scope: 'openid', - client_id: 'client-with-HS-sig', - response_type: 'code', - }, - }) - .expect(errorCode) - .expect(() => { - expect(spy.calledOnce).to.be.true; - expect(spy.args[0][1]).to.have.property('message', 'invalid_request_object'); - expect(spy.args[0][1]).to.have.property('error_description').that.matches(/^request object replay detected/); - }); - }); - it('doesnt allow request inception', function () { const spy = sinon.spy(); this.provider.once(errorEvt, spy);