feat: request objects are now one-time use if they have iss, jti and exp

panva · Apr 7, 2019 · 1dc44dd · 1dc44dd
1 parent a22d6ce
commit 1dc44dd
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 27 deletions.
diff --git a/lib/actions/authorization/decode_request.js b/lib/actions/authorization/decode_request.js
@@ -90,6 +90,16 @@ module.exports = async function decodeRequest(PARAM_LIST, ctx, next) {
     } catch (err) {
       throw new InvalidRequestObject(`could not validate request object (${err.message})`);
     }
+
+    if (payload.jti && payload.exp && payload.iss) {
+      const unique = await ctx.oidc.provider.ReplayDetection.unique(
+        payload.iss, payload.jti, payload.exp,
+      );
+
+      if (!unique) {
+        throw new InvalidRequestObject(`request replay detected (jti: ${payload.jti})`);
+      }
+    }
   }
 
   const request = Object.entries(payload).reduce((acc, [key, value]) => {

diff --git a/lib/consts/jwa.js b/lib/consts/jwa.js
@@ -1,42 +1,43 @@
-const authSigningAlgValues = [
-  'HS256', 'HS384', 'HS512', 'RS256', 'RS384', 'RS512', 'PS256', 'PS384', 'PS512', 'ES256', 'ES384', 'ES512',
-];
-
 const signingAlgValues = [
-  'none', ...authSigningAlgValues,
+  'HS256', 'HS384', 'HS512',
+  'RS256', 'RS384', 'RS512',
+  'PS256', 'PS384', 'PS512',
+  'ES256', 'ES384', 'ES512',
 ];
 
 const encryptionAlgValues = [
   // asymmetric
-  'RSA-OAEP', 'RSA1_5', 'ECDH-ES', 'ECDH-ES+A128KW', 'ECDH-ES+A192KW', 'ECDH-ES+A256KW',
+  'RSA-OAEP', 'RSA1_5',
+  'ECDH-ES', 'ECDH-ES+A128KW', 'ECDH-ES+A192KW', 'ECDH-ES+A256KW',
   // symmetric
-  'A128GCMKW', 'A192GCMKW', 'A256GCMKW', 'A128KW', 'A192KW', 'A256KW', 'PBES2-HS256+A128KW', 'PBES2-HS384+A192KW', 'PBES2-HS512+A256KW',
+  'A128GCMKW', 'A192GCMKW', 'A256GCMKW', 'A128KW', 'A192KW', 'A256KW',
+  'PBES2-HS256+A128KW', 'PBES2-HS384+A192KW', 'PBES2-HS512+A256KW',
 ];
 
 const encryptionEncValues = [
   'A128CBC-HS256', 'A128GCM', 'A192CBC-HS384', 'A192GCM', 'A256CBC-HS512', 'A256GCM',
 ];
 
 module.exports = {
-  tokenEndpointAuthSigningAlgValues: authSigningAlgValues.slice(),
-  introspectionEndpointAuthSigningAlgValues: authSigningAlgValues.slice(),
-  revocationEndpointAuthSigningAlgValues: authSigningAlgValues.slice(),
+  tokenEndpointAuthSigningAlgValues: [...signingAlgValues],
+  introspectionEndpointAuthSigningAlgValues: [...signingAlgValues],
+  revocationEndpointAuthSigningAlgValues: [...signingAlgValues],
 
-  idTokenSigningAlgValues: signingAlgValues.slice(),
-  requestObjectSigningAlgValues: signingAlgValues.slice(),
-  userinfoSigningAlgValues: signingAlgValues.slice(),
-  introspectionSigningAlgValues: signingAlgValues.slice(),
-  authorizationSigningAlgValues: authSigningAlgValues.slice(), // intended, no none
+  idTokenSigningAlgValues: [...signingAlgValues, 'none'],
+  requestObjectSigningAlgValues: [...signingAlgValues, 'none'],
+  userinfoSigningAlgValues: [...signingAlgValues, 'none'],
+  introspectionSigningAlgValues: [...signingAlgValues, 'none'],
+  authorizationSigningAlgValues: [...signingAlgValues],
 
-  idTokenEncryptionAlgValues: encryptionAlgValues.slice(),
-  requestObjectEncryptionAlgValues: encryptionAlgValues.slice(),
-  userinfoEncryptionAlgValues: encryptionAlgValues.slice(),
-  introspectionEncryptionAlgValues: encryptionAlgValues.slice(),
-  authorizationEncryptionAlgValues: encryptionAlgValues.slice(),
+  idTokenEncryptionAlgValues: [...encryptionAlgValues],
+  requestObjectEncryptionAlgValues: [...encryptionAlgValues],
+  userinfoEncryptionAlgValues: [...encryptionAlgValues],
+  introspectionEncryptionAlgValues: [...encryptionAlgValues],
+  authorizationEncryptionAlgValues: [...encryptionAlgValues],
 
-  idTokenEncryptionEncValues: encryptionEncValues.slice(),
-  requestObjectEncryptionEncValues: encryptionEncValues.slice(),
-  userinfoEncryptionEncValues: encryptionEncValues.slice(),
-  introspectionEncryptionEncValues: encryptionEncValues.slice(),
-  authorizationEncryptionEncValues: encryptionEncValues.slice(),
+  idTokenEncryptionEncValues: [...encryptionEncValues],
+  requestObjectEncryptionEncValues: [...encryptionEncValues],
+  userinfoEncryptionEncValues: [...encryptionEncValues],
+  introspectionEncryptionEncValues: [...encryptionEncValues],
+  authorizationEncryptionEncValues: [...encryptionEncValues],
 };
diff --git a/test/request/jwt_request.test.js b/test/request/jwt_request.test.js
@@ -35,7 +35,7 @@ describe('request parameter features', () => {
     ['/auth', 'get', 'authorization.error', 302, 302, redirectSuccess, 'authorization.success'],
     ['/auth', 'post', 'authorization.error', 302, 302, redirectSuccess, 'authorization.success'],
     ['/device/auth', 'post', 'device_authorization.error', 200, 400, httpSuccess, 'device_authorization.success'],
-  ].forEach(([route, verb, errorEvt, successCode, errorCode, successFnCheck, successEvt]) => {
+  ].forEach(([route, verb, errorEvt, successCode, errorCode, successFnCheck, successEvt], index) => {
     describe(`${route} ${verb} passing request parameters as JWTs`, () => {
       before(function () {
         return this.login({
@@ -204,7 +204,7 @@ describe('request parameter features', () => {
           .expect(successFnCheck));
       });
 
-      it('works with signed by an actual HS', async function () {
+      it('works with signed by an actual DSA', async function () {
         const key = (await this.provider.Client.find('client-with-HS-sig')).keystore.get({
           alg: 'HS256',
         });
@@ -227,6 +227,53 @@ describe('request parameter features', () => {
           .expect(successFnCheck));
       });
 
+      it('supports optional replay prevention', async function () {
+        const key = (await this.provider.Client.find('client-with-HS-sig')).keystore.get({
+          alg: 'HS256',
+        });
+
+        const request = await JWT.sign({
+          response_type: 'code',
+          redirect_uri: 'https://client.example.com/cb',
+          jti: `very-random-and-collision-resistant-${index}`,
+        }, 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 replay detected/);
+          });
+      });
+
       it('doesnt allow request inception', function () {
         const spy = sinon.spy();
         this.provider.once(errorEvt, spy);