refactor!: check request_uri_not_supported early

BREAKING CHANGE: `request_uri` parameter is no longer supported at the Device Authorization Endpoint.
panva · Dec 1, 2022 · 57b39a2 · 57b39a2
1 parent 90bf959
commit 57b39a2
Show file tree

Hide file tree

Showing 9 changed files with 55 additions and 17 deletions.
diff --git a/lib/actions/authorization/index.js b/lib/actions/authorization/index.js
@@ -144,15 +144,15 @@ export default function authorizationAction(provider, endpoint) {
   }
   use(() => authenticatedClientId,                             DA,                 BA);
   use(() => paramsMiddleware.bind(undefined, allowList),    A, DA,            PAR, BA);
-  use(() => stripOutsideJarParams,                                            PAR, BA);
   use(() => rejectDupesMiddleware,                          A, DA,            PAR, BA);
   use(() => rejectUnsupported,                              A, DA,            PAR, BA);
+  use(() => stripOutsideJarParams,                                            PAR, BA);
   use(() => checkClient,                                    A, DA, R, CV, DR         );
   use(() => checkClientGrantType,                              DA,                 BA);
   use(() => checkResponseMode,                              A,                PAR    );
   use(() => pushedAuthorizationRequestRemapErrors,                            PAR    );
   use(() => backchannelRequestRemapErrors,                                         BA);
-  use(() => fetchRequestUri,                                A, DA                    );
+  use(() => fetchRequestUri,                                A                        );
   use(() => processRequestObject.bind(
     undefined, allowList, rejectDupesMiddleware,
   ),                                                        A, DA,            PAR, BA);

diff --git a/lib/actions/authorization/reject_unsupported.js b/lib/actions/authorization/reject_unsupported.js
@@ -17,7 +17,7 @@ export default function rejectUnsupported(ctx, next) {
 
   if (
     params.request_uri !== undefined
-    && !(requestObjects.requestUri || pushedAuthorizationRequests.enabled)
+    && (ctx.oidc.route !== 'authorization' || !(requestObjects.requestUri || pushedAuthorizationRequests.enabled))
   ) {
     throw new RequestUriNotSupported();
   }

diff --git a/lib/actions/authorization/strip_outside_jar_params.js b/lib/actions/authorization/strip_outside_jar_params.js
@@ -1,5 +1,3 @@
-import { InvalidRequest } from '../../helpers/errors.js';
-
 /*
  * Makes sure that
  * - unauthenticated clients send the JAR Request Object
@@ -13,9 +11,6 @@ export default function stripOutsideJarParams(ctx, next) {
 
   for (const [param, value] of Object.entries(ctx.oidc.params)) { // eslint-disable-line no-restricted-syntax, max-len
     if (value !== undefined) {
-      if (param === 'request_uri') {
-        throw new InvalidRequest('`request_uri` parameter must not be used at the pushed_authorization_request_endpoint');
-      }
       if (JAR && (param !== 'client_id' && param !== 'request')) {
         ctx.oidc.params[param] = undefined;
       }

diff --git a/test/ciba/ciba.test.js b/test/ciba/ciba.test.js
@@ -321,6 +321,30 @@ describe('features.ciba', () => {
       });
 
       describe('param validation', () => {
+        ['request', 'request_uri', 'registration'].forEach((param) => {
+          it(`check for not supported parameter ${param}`, function () {
+            const spy = sinon.spy();
+            this.provider.once('backchannel_authentication.error', spy);
+
+            return this.agent.post(route)
+              .send({
+                client_id: 'client',
+                scope: 'openid',
+                [param]: 'some',
+              })
+              .type('form')
+              .expect(400)
+              .expect('content-type', /application\/json/)
+              .expect(() => {
+                expect(spy.calledOnce).to.be.true;
+              })
+              .expect({
+                error: `${param}_not_supported`,
+                error_description: `${param} parameter provided but not supported`,
+              });
+          });
+        });
+
         it('could not resolve Account', async function () {
           const spy = sinon.spy();
           this.provider.once('backchannel_authentication.error', spy);
@@ -536,6 +560,30 @@ describe('features.ciba', () => {
       const route = '/backchannel';
 
       describe('param validation', () => {
+        ['request_uri', 'registration'].forEach((param) => {
+          it(`check for not supported parameter ${param}`, function () {
+            const spy = sinon.spy();
+            this.provider.once('backchannel_authentication.error', spy);
+
+            return this.agent.post(route)
+              .send({
+                client_id: 'client',
+                scope: 'openid',
+                [param]: 'some',
+              })
+              .type('form')
+              .expect(400)
+              .expect('content-type', /application\/json/)
+              .expect(() => {
+                expect(spy.calledOnce).to.be.true;
+              })
+              .expect({
+                error: `${param}_not_supported`,
+                error_description: `${param} parameter provided but not supported`,
+              });
+          });
+        });
+
         it('validates request object is used', async function () {
           const spy = sinon.spy();
           this.provider.once('backchannel_authentication.error', spy);

diff --git a/test/ciba/ciba_jar.config.js b/test/ciba/ciba_jar.config.js
@@ -4,5 +4,5 @@ import merge from 'lodash/merge.js';
 import config from './ciba.config.js';
 
 export default merge(cloneDeep(config), {
-  config: { features: { requestObjects: { request: true } } },
+  config: { features: { requestObjects: { request: true, requestUri: true } } },
 });
diff --git a/test/device_code/device_authorization_endpoint.test.js b/test/device_code/device_authorization_endpoint.test.js
@@ -80,7 +80,6 @@ describe('device_authorization_endpoint', () => {
         return this.agent.post(route)
           .send({
             client_id: 'client',
-            scope: 'openid',
             [param]: 'some',
           })
           .type('form')

diff --git a/test/device_code/device_code.config.js b/test/device_code/device_code.config.js
@@ -8,7 +8,7 @@ merge(config.features, {
   deviceFlow: { enabled: true },
   backchannelLogout: { enabled: true },
   claimsParameter: { enabled: true },
-  requestObjects: { request: false, requestUri: false },
+  requestObjects: { request: false, requestUri: true },
   rpInitiatedLogout: { enabled: false },
   pushedAuthorizationRequests: { enabled: false },
 });

diff --git a/test/pushed_authorization_requests/pushed_authorization_requests.test.js b/test/pushed_authorization_requests/pushed_authorization_requests.test.js
@@ -120,8 +120,8 @@ describe('Pushed Request Object', () => {
                 })
                 .expect(400)
                 .expect({
-                  error: 'invalid_request',
-                  error_description: '`request_uri` parameter must not be used at the pushed_authorization_request_endpoint',
+                  error: 'request_uri_not_supported',
+                  error_description: 'request_uri parameter provided but not supported',
                 });
             });
 

diff --git a/test/request/uri_request.test.js b/test/request/uri_request.test.js
@@ -53,14 +53,10 @@ describe('request Uri features', () => {
     });
     expect(actual.query).to.have.property('code');
   }
-  function httpSuccess({ body }) {
-    expect(body).to.contain.key('device_code');
-  }
 
   [
     ['/auth', 'get', 'authorization.error', 303, 303, redirectSuccess],
     ['/auth', 'post', 'authorization.error', 303, 303, redirectSuccess],
-    ['/device/auth', 'post', 'device_authorization.error', 200, 400, httpSuccess],
   ].forEach(([route, verb, error, successCode, errorCode, successFnCheck]) => {
     describe(`${route} ${verb} passing request parameters in request_uri`, () => {
       before(function () { return this.login(); });