feat: use 303 See Other HTTP response status code for built in redirects

panva · Jun 23, 2021 · c243bf6 · c243bf6
1 parent f3c1d5f
commit c243bf6
Show file tree

Hide file tree

Showing 42 changed files with 235 additions and 226 deletions.
diff --git a/certification/fapi/index.js b/certification/fapi/index.js
@@ -252,6 +252,7 @@ if (process.env.NODE_ENV === 'production') {
         default:
       }
     } else if (ctx.method === 'GET' || ctx.method === 'HEAD') {
+      ctx.status = 303;
       ctx.redirect(ctx.href.replace(/^http:\/\//i, 'https://'));
     } else {
       ctx.body = {

diff --git a/certification/oidc/index.js b/certification/oidc/index.js
@@ -114,6 +114,7 @@ let server;
           default:
         }
       } else if (ctx.method === 'GET' || ctx.method === 'HEAD') {
+        ctx.status = 303;
         ctx.redirect(ctx.href.replace(/^http:\/\//i, 'https://'));
       } else {
         ctx.body = {

diff --git a/example/koa.js b/example/koa.js
@@ -32,6 +32,7 @@ if (process.env.NODE_ENV === 'production') {
     if (ctx.secure) {
       await next();
     } else if (ctx.method === 'GET' || ctx.method === 'HEAD') {
+      ctx.status = 303;
       ctx.redirect(ctx.href.replace(/^http:\/\//i, 'https://'));
     } else {
       ctx.body = {

diff --git a/example/routes/koa.js b/example/routes/koa.js
@@ -126,6 +126,7 @@ module.exports = (provider) => {
           ctx.cookies.set('google.state', state, { path, sameSite: 'strict' });
           ctx.cookies.set('google.nonce', nonce, { path, sameSite: 'strict' });
 
+          ctx.status = 303;
           return ctx.redirect(ctx.google.authorizationUrl({
             state, nonce, scope: 'openid email profile',
           }));

diff --git a/example/standalone.js b/example/standalone.js
@@ -35,6 +35,7 @@ let server;
       if (ctx.secure) {
         await next();
       } else if (ctx.method === 'GET' || ctx.method === 'HEAD') {
+        ctx.status = 303;
         ctx.redirect(ctx.href.replace(/^http:\/\//i, 'https://'));
       } else {
         ctx.body = {

diff --git a/lib/actions/authorization/interactions.js b/lib/actions/authorization/interactions.js
@@ -86,7 +86,7 @@ module.exports = async function interactions(resumeRouteName, ctx, next) {
       throw new errors.CustomOIDCProviderError(failedCheck.error, failedCheck.error_description);
     }
   } catch (err) {
-    const code = /^(code|device)_/.test(oidc.route) ? 400 : 302;
+    const code = /^(code|device)_/.test(oidc.route) ? 400 : 303;
     err.status = code;
     err.statusCode = code;
     err.expose = true;
@@ -148,5 +148,6 @@ module.exports = async function interactions(resumeRouteName, ctx, next) {
   );
 
   oidc.provider.emit('interaction.started', ctx, prompt);
+  ctx.status = 303;
   ctx.redirect(destination);
 };
diff --git a/lib/actions/end_session.js b/lib/actions/end_session.js
@@ -210,6 +210,7 @@ module.exports = {
 
       ctx.oidc.provider.emit('end_session.success', ctx);
 
+      ctx.status = 303;
       ctx.redirect(uri);
 
       await next();

diff --git a/lib/provider.js b/lib/provider.js
@@ -260,7 +260,7 @@ class Provider extends events.EventEmitter {
   async interactionFinished(req, res, result, { mergeWithLastSubmission = true } = {}) {
     const returnTo = await this.interactionResult(req, res, result, { mergeWithLastSubmission });
 
-    res.statusCode = 302; // eslint-disable-line no-param-reassign
+    res.statusCode = 303; // eslint-disable-line no-param-reassign
     res.setHeader('Location', returnTo);
     res.setHeader('Content-Length', '0');
     res.end();

diff --git a/lib/response_modes/fragment.js b/lib/response_modes/fragment.js
@@ -2,5 +2,6 @@ const formatUri = require('../helpers/redirect_uri');
 
 module.exports = (ctx, redirectUri, payload) => {
   const uri = formatUri(redirectUri, payload, 'fragment');
+  ctx.status = 303;
   ctx.redirect(uri);
 };
diff --git a/lib/response_modes/query.js b/lib/response_modes/query.js
@@ -2,5 +2,6 @@ const formatUri = require('../helpers/redirect_uri');
 
 module.exports = (ctx, redirectUri, payload) => {
   const uri = formatUri(redirectUri, payload, 'query');
+  ctx.status = 303;
   ctx.redirect(uri);
 };
diff --git a/test/auth_time/auth_time.test.js b/test/auth_time/auth_time.test.js
@@ -23,7 +23,7 @@ describe('responds with a id_token containing auth_time', () => {
     let id_token;
 
     await this.wrap({ route: '/auth', verb: 'get', auth })
-      .expect(302)
+      .expect(303)
       .expect(auth.validateFragment)
       .expect(auth.validatePresence(['id_token', 'state']))
       .expect(auth.validateState)
@@ -54,7 +54,7 @@ describe('responds with a id_token containing auth_time', () => {
       let id_token;
 
       await this.wrap({ route: '/auth', verb: 'get', auth })
-        .expect(302)
+        .expect(303)
         .expect(auth.validateFragment)
         .expect(auth.validatePresence(['id_token', 'state']))
         .expect(auth.validateState)
@@ -76,7 +76,7 @@ describe('responds with a id_token containing auth_time', () => {
       let id_token;
 
       await this.wrap({ route: '/auth', verb: 'get', auth })
-        .expect(302)
+        .expect(303)
         .expect(auth.validateFragment)
         .expect(auth.validatePresence(['id_token', 'state']))
         .expect(auth.validateState)
@@ -99,7 +99,7 @@ describe('responds with a id_token containing auth_time', () => {
     let id_token;
 
     await this.wrap({ route: '/auth', verb: 'get', auth })
-      .expect(302)
+      .expect(303)
       .expect(auth.validateFragment)
       .expect(auth.validatePresence(['id_token', 'state']))
       .expect(auth.validateState)
@@ -121,7 +121,7 @@ describe('responds with a id_token containing auth_time', () => {
     let id_token;
 
     await this.wrap({ route: '/auth', verb: 'get', auth })
-      .expect(302)
+      .expect(303)
       .expect(auth.validateFragment)
       .expect(auth.validatePresence(['id_token', 'state']))
       .expect(auth.validateState)

diff --git a/test/authorization_code/code.grant.test.js b/test/authorization_code/code.grant.test.js
@@ -36,7 +36,7 @@ describe('grant_type=authorization_code', () => {
           response_type: 'code',
           redirect_uri: 'https://client.example.com/cb',
         })
-        .expect(302)
+        .expect(303)
         .expect((response) => {
           const { query: { code } } = parseUrl(response.headers.location, true);
           const jti = this.getTokenJti(code);
@@ -310,7 +310,7 @@ describe('grant_type=authorization_code', () => {
           response_type: 'code',
           redirect_uri: 'https://client.example.com/cb3',
         })
-        .expect(302)
+        .expect(303)
         .expect((response) => {
           const { query: { code } } = parseUrl(response.headers.location, true);
           this.ac = code;
@@ -354,7 +354,7 @@ describe('grant_type=authorization_code', () => {
           scope: 'openid',
           response_type: 'code',
         })
-        .expect(302)
+        .expect(303)
         .expect((response) => {
           const { query: { code } } = parseUrl(response.headers.location, true);
           const jti = this.getTokenJti(code);

diff --git a/test/backchannel_logout/backchannel_logout.test.js b/test/backchannel_logout/backchannel_logout.test.js
@@ -93,7 +93,7 @@ describe('Back-Channel Logout 1.0', () => {
           response_type: 'code id_token',
           redirect_uri: 'https://client.example.com/cb',
         })
-        .expect(302)
+        .expect(303)
         .expect((response) => {
           const { query } = parseUrl(response.headers.location.replace('#', '?'), true);
           expect(query).to.have.property('code');
@@ -176,7 +176,7 @@ describe('Back-Channel Logout 1.0', () => {
       return this.agent.post('/session/end/confirm')
         .send(params)
         .type('form')
-        .expect(302)
+        .expect(303)
         .expect(() => {
           (() => {
             const { sid } = session.authorizations.client;
@@ -211,7 +211,7 @@ describe('Back-Channel Logout 1.0', () => {
       return this.agent.post('/session/end/confirm')
         .send(params)
         .type('form')
-        .expect(302)
+        .expect(303)
         .expect(() => {
           expect(client.backchannelLogout.called).to.be.true;
           expect(client.backchannelLogout.calledWith(accountId, sid)).to.be.true;
@@ -234,7 +234,7 @@ describe('Back-Channel Logout 1.0', () => {
       return this.agent.post('/session/end/confirm')
         .send(params)
         .type('form')
-        .expect(302)
+        .expect(303)
         .expect(() => {
           expect(client.backchannelLogout.called).to.be.false;
           client.backchannelLogout.restore();

diff --git a/test/certificate_bound_access_tokens/certificate_bound_access_tokens.test.js b/test/certificate_bound_access_tokens/certificate_bound_access_tokens.test.js
@@ -257,7 +257,7 @@ describe('features.mTLS.certificateBoundAccessTokens', () => {
       });
 
       await this.wrap({ route: '/auth', verb: 'get', auth })
-        .expect(302)
+        .expect(303)
         .expect(auth.validateClientLocation)
         .expect(({ headers: { location } }) => {
           const { query: { code } } = url.parse(location, true);
@@ -376,7 +376,7 @@ describe('features.mTLS.certificateBoundAccessTokens', () => {
       });
 
       await this.wrap({ route: '/auth', verb: 'get', auth })
-        .expect(302)
+        .expect(303)
         .expect(auth.validateClientLocation)
         .expect(({ headers: { location } }) => {
           const { query: { code } } = url.parse(location, true);