Skip to content

Commit

Permalink
fix: request object processing order related and general fixes
Browse files Browse the repository at this point in the history
- `state` from a parsed request object is now always used even if
subsequent processing of the request object fails
- `response_mode` from a parsed request object is now additionally
validated
- validated `response_mode` from a parsed request object is now used
even if subsequent processing of the request object fails
- unsigned request objects now also have their payload asserted
  • Loading branch information
panva committed Jun 18, 2019
1 parent e7bcfd2 commit 9fd3fba
Show file tree
Hide file tree
Showing 7 changed files with 219 additions and 91 deletions.
33 changes: 29 additions & 4 deletions lib/actions/authorization/fetch_request_uri.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
const { URL } = require('url');
const assert = require('assert');

const { InvalidRequestUri } = require('../../helpers/errors');
const {
InvalidRequest, InvalidRequestUri, RequestNotSupported, RequestUriNotSupported,
} = require('../../helpers/errors');
const instance = require('../../helpers/weak_cache');

const allowedSchemes = new Set(['http:', 'https:', 'urn:']);
Expand All @@ -12,12 +14,35 @@ const allowedSchemes = new Set(['http:', 'https:', 'urn:']);
* uses the response body as a value for the request parameter to be validated by a downstream
* middleware
*
*
* @throws: invalid_request
* @throws: invalid_request_uri
* @see: RequestUriCache
* @see: decodeRequest
* @throws: request_not_supported
* @throws: request_uri_not_supported
*/
module.exports = async function fetchRequestUri(ctx, next) {
const { params } = ctx.oidc;
const { request, requestUri } = instance(ctx.oidc.provider).configuration('features');
const { client, params } = ctx.oidc;

if (!request.enabled && params.request !== undefined) {
throw new RequestNotSupported();
}

if (!requestUri.enabled && params.request_uri !== undefined) {
throw new RequestUriNotSupported();
}

if (params.request !== undefined && params.request_uri !== undefined) {
throw new InvalidRequest('request and request_uri parameters MUST NOT be used together');
}

if (
client.requestObjectSigningAlg
&& params.request === undefined
&& params.request_uri === undefined
) {
throw new InvalidRequest('request or request_uri must be provided for this client');
}

if (params.request_uri !== undefined) {
let protocol;
Expand Down
8 changes: 4 additions & 4 deletions lib/actions/authorization/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ const getTokenAuth = require('../../shared/token_auth');

const checkClient = require('./check_client');
const checkResponseMode = require('./check_response_mode');
const throwNotSupported = require('./throw_not_supported');
const rejectRegistration = require('./reject_registration');
const oauthRequired = require('./oauth_required');
const oneRedirectUriClients = require('./one_redirect_uri_clients');
const fetchRequestUri = require('./fetch_request_uri');
const decodeRequest = require('./decode_request');
const processRequestObject = require('./process_request_object');
const oidcRequired = require('./oidc_required');
const checkPrompt = require('./check_prompt');
const checkMaxAge = require('./check_max_age');
Expand Down Expand Up @@ -119,10 +119,10 @@ module.exports = function authorizationAction(provider, endpoint) {
use(() => rejectDupesMiddleware, A, DA );
use(() => checkClientGrantType, DA );
use(() => checkResponseMode, A );
use(() => throwNotSupported, A, DA );
use(() => oauthRequired, A );
use(() => fetchRequestUri, A, DA );
use(() => decodeRequest.bind(_, whitelist), A, DA );
use(() => processRequestObject.bind(_, whitelist), A, DA );
use(() => rejectRegistration, A, DA );
use(() => oidcRequired, A );
use(() => assignDefaults, A, DA );
use(() => checkOpenidScope.bind(_, whitelist), A, DA );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,25 @@ const { isPlainObject } = require('lodash');

const JWT = require('../../helpers/jwt');
const instance = require('../../helpers/weak_cache');
const { InvalidRequest, InvalidRequestObject } = require('../../helpers/errors');
const { InvalidRequestObject } = require('../../helpers/errors');

const checkResponseMode = require('./check_response_mode');

/*
* Decrypts and validates the content of provided request parameter and replaces the parameters
* provided via OAuth2.0 authorization request with these
*
* @throws: invalid_request_object
*/
module.exports = async function decodeRequest(PARAM_LIST, ctx, next) {
const { keystore, configuration: conf } = instance(ctx.oidc.provider);
module.exports = async function processRequestObject(PARAM_LIST, ctx, next) {
const { params, client } = ctx.oidc;
let trusted = false; // signed or encrypted by client confidential material

if (params.request === undefined) {
if (client.requestObjectSigningAlg) {
throw new InvalidRequest('request or request_uri must be provided for this client');
}
return next();
}

const { keystore, configuration: conf } = instance(ctx.oidc.provider);
let trusted = false; // signed or encrypted by client confidential material

if (conf('features.encryption.enabled') && params.request.split('.').length === 5) {
try {
const header = JWT.header(params.request);
Expand Down Expand Up @@ -57,15 +56,40 @@ module.exports = async function decodeRequest(PARAM_LIST, ctx, next) {

const { payload, header: { alg } } = decoded;

if (payload.request !== undefined || payload.request_uri !== undefined) {
const request = Object.entries(payload).reduce((acc, [key, value]) => {
if (PARAM_LIST.has(key)) {
if (key === 'claims' && isPlainObject(value)) {
acc[key] = JSON.stringify(value);
} else if (key === 'resource' && Array.isArray(value) && conf('features.resourceIndicators.enabled')) {
acc[key] = value;
} else if (typeof value !== 'string') {
acc[key] = String(value);
} else {
acc[key] = value;
}
}

return acc;
}, {});

if (request.state !== undefined) {
params.state = request.state;
}

if (request.response_mode !== undefined) {
params.response_mode = request.response_mode;
checkResponseMode(ctx, () => {});
}

if (request.request !== undefined || request.request_uri !== undefined) {
throw new InvalidRequestObject('request object must not contain request or request_uri properties');
}

if (PARAM_LIST.has('response_type') && payload.response_type !== undefined && payload.response_type !== params.response_type) {
if (request.response_type !== undefined && request.response_type !== params.response_type) {
throw new InvalidRequestObject('request response_type must equal the one in request parameters');
}

if (payload.client_id !== undefined && payload.client_id !== params.client_id) {
if (request.client_id !== undefined && request.client_id !== params.client_id) {
throw new InvalidRequestObject('request client_id must equal the one in request parameters');
}

Expand All @@ -77,14 +101,17 @@ module.exports = async function decodeRequest(PARAM_LIST, ctx, next) {
throw new InvalidRequestObject('unsupported signed request alg');
}

if (alg !== 'none') {
const opts = {
issuer: payload.iss ? client.clientId : undefined,
audience: payload.aud ? ctx.oidc.issuer : undefined,
clockTolerance: conf('clockTolerance'),
ignoreAzp: true,
};

if (alg === 'none') {
JWT.assertPayload(payload, opts);
} else {
try {
const opts = {
issuer: payload.iss ? client.clientId : undefined,
audience: payload.aud ? ctx.oidc.issuer : undefined,
clockTolerance: conf('clockTolerance'),
ignoreAzp: true,
};
await JWT.verify(params.request, client.keystore, opts);
trusted = true;
} catch (err) {
Expand All @@ -102,27 +129,12 @@ module.exports = async function decodeRequest(PARAM_LIST, ctx, next) {
}
}

const request = Object.entries(payload).reduce((acc, [key, value]) => {
if (PARAM_LIST.has(key)) {
if (key === 'claims' && isPlainObject(value)) {
acc[key] = JSON.stringify(value);
} else if (key === 'resource' && Array.isArray(value) && conf('features.resourceIndicators')) {
acc[key] = value;
} else if (typeof value !== 'string') {
acc[key] = String(value);
} else {
acc[key] = value;
}
}

return acc;
}, {});

if (trusted) {
ctx.oidc.signed = Object.keys(request);
} else if (ctx.oidc.insecureRequestUri) {
throw new InvalidRequestObject('request object from unsecure request_uri must be signed and/or symmetrically encrypted');
}

Object.assign(params, request);

params.request = undefined;
Expand Down
14 changes: 14 additions & 0 deletions lib/actions/authorization/reject_registration.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
const { RegistrationNotSupported } = require('../../helpers/errors');

/*
* Rejects registration parameter as not supported.
*
* @throws: registration_not_supported
*/
module.exports = function rejectRegistration(ctx, next) {
if (ctx.oidc.params.registration !== undefined) {
throw new RegistrationNotSupported();
}

return next();
};
39 changes: 0 additions & 39 deletions lib/actions/authorization/throw_not_supported.js

This file was deleted.

2 changes: 1 addition & 1 deletion test/encryption/encryption.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ const route = '/auth';
response_type: 'id_token',
nonce: 'foobar',
redirect_uri: 'https://client.example.com/cb',
}, null, 'none', { issuer: 'client', audience: this.provider.issuer }).then(signed => JWT.encrypt(signed, client.keystore.get({ alg: 'A128KW' }), { enc: 'A128CBC-HS256', alg: 'A128KW' })).then(encrypted => this.wrap({
}, null, 'none', { issuer: 'clientSymmetric', audience: this.provider.issuer }).then(signed => JWT.encrypt(signed, client.keystore.get({ alg: 'A128KW' }), { enc: 'A128CBC-HS256', alg: 'A128KW' })).then(encrypted => this.wrap({
route,
verb,
auth: {
Expand Down
Loading

0 comments on commit 9fd3fba

Please sign in to comment.