Skip to content

Commit

Permalink
fix: html-rendered response modes now honour 400 and 500 status codes
Browse files Browse the repository at this point in the history
  • Loading branch information
panva committed Apr 7, 2019
1 parent 26fadc8 commit 9771581
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 6 deletions.
7 changes: 6 additions & 1 deletion lib/response_modes/form_post.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
const htmlEscape = require('../helpers/html_escape');

const statusCodes = new Set([200, 400, 500]);

module.exports = function formPost(ctx, action, inputs) {
ctx.type = 'html';
ctx.status = 200;

if (!statusCodes.has(ctx.status)) {
ctx.status = 'error' in inputs ? 400 : 200;
}

const formInputs = Object.entries(inputs)
.map(([key, value]) => `<input type="hidden" name="${key}" value="${htmlEscape(value)}"/>`)
Expand Down
8 changes: 8 additions & 0 deletions lib/response_modes/jwt.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ const modes = {
web_message,
};

const RENDER_MODES = new Set(['form_post', 'web_message']);

module.exports = async function jwtResponseModes(ctx, redirectUri, payload) {
const { params, entities: { AccessToken } } = ctx.oidc;

Expand All @@ -35,5 +37,11 @@ module.exports = async function jwtResponseModes(ctx, redirectUri, payload) {

const response = await token.sign({ use: 'authorization' });

if (RENDER_MODES.has(mode)) {
if ('error' in payload && payload.error !== 'server_error') {
ctx.status = 400;
}
}

return modes[mode](ctx, redirectUri, { response });
};
7 changes: 6 additions & 1 deletion lib/response_modes/web_message.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
const jsesc = require('jsesc');

const statusCodes = new Set([200, 400, 500]);

module.exports = function webMessage(ctx, redirectUri, response) {
ctx.type = 'html';
ctx.status = 200;

if (!statusCodes.has(ctx.status)) {
ctx.status = 'error' in response ? 400 : 200;
}

ctx.response.remove('X-Frame-Options');
const csp = ctx.response.get('Content-Security-Policy');
Expand Down
4 changes: 2 additions & 2 deletions test/form_post/form_post.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('/auth', () => {
this.provider.once('authorization.error', spy);

return this.wrap({ route, verb, auth })
.expect(200)
.expect(400)
.expect(() => {
expect(spy.called).to.be.true;
})
Expand Down Expand Up @@ -72,7 +72,7 @@ describe('/auth', () => {
this.provider.once('server_error', spy);

return this.wrap({ route, verb, auth })
.expect(200)
.expect(500)
.expect(() => {
expect(spy.called).to.be.true;
})
Expand Down
55 changes: 55 additions & 0 deletions test/jwt_response_modes/jwt_response_modes.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,4 +180,59 @@ describe('configuration features.jwtResponseModes', () => {
});
});
});

Object.entries({
'query.jwt': [302, 302],
'fragment.jwt': [302, 302],
'form_post.jwt': [400, 500],
'web_message.jwt': [400, 500],
}).forEach(([mode, [errStatus, exceptionStatus]]) => {
context(`${mode} err handling`, () => {
it(`responds with a ${errStatus}`, function () {
const auth = new this.AuthorizationRequest({
response_type: 'code',
prompt: 'none',
response_mode: mode,
scope: 'openid',
});

const spy = sinon.spy();
this.provider.once('authorization.error', spy);

return this.wrap({ route, verb: 'get', auth })
.expect(errStatus)
.expect(() => {
expect(spy.called).to.be.true;
});
});

context('[exception]', () => {
before(async function () {
sinon.stub(this.provider.Session.prototype, 'accountId').throws();
});

after(async function () {
this.provider.Session.prototype.accountId.restore();
});

it(`responds with a ${exceptionStatus}`, function () {
const auth = new this.AuthorizationRequest({
response_type: 'code',
prompt: 'none',
response_mode: mode,
scope: 'openid',
});

const spy = sinon.spy();
this.provider.once('server_error', spy);

return this.wrap({ route, verb: 'get', auth })
.expect(exceptionStatus)
.expect(() => {
expect(spy.called).to.be.true;
});
});
});
});
});
});
4 changes: 2 additions & 2 deletions test/web_message/web_message.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ describe('configuration features.webMessageResponseMode', () => {
this.provider.once('authorization.error', spy);

await this.wrap({ route, auth, verb: 'get' })
.expect(200)
.expect(400)
.expect('pragma', 'no-cache')
.expect('cache-control', 'no-cache, no-store')
.expect('content-type', 'text/html; charset=utf-8')
Expand Down Expand Up @@ -248,7 +248,7 @@ describe('configuration features.webMessageResponseMode', () => {
this.provider.once('server_error', spy);

await this.wrap({ route, auth, verb: 'get' })
.expect(200)
.expect(500)
.expect('pragma', 'no-cache')
.expect('cache-control', 'no-cache, no-store')
.expect('content-type', 'text/html; charset=utf-8')
Expand Down

0 comments on commit 9771581

Please sign in to comment.