Skip to content

Commit

Permalink
feat: allow authorization requests with only a Request Object
Browse files Browse the repository at this point in the history
  • Loading branch information
panva committed Aug 27, 2019
1 parent 878ea24 commit e3fa143
Show file tree
Hide file tree
Showing 13 changed files with 320 additions and 64 deletions.
6 changes: 3 additions & 3 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -927,8 +927,8 @@ _**default value**_:
[Financial-grade API - Part 2: Read and Write API Security Profile](https://openid.net/specs/openid-financial-api-part-2-ID2.html)

Enables extra behaviours defined in FAPI Part 1 & 2 that cannot be achieved by other configuration options, namely:
- request object "exp" claim is required
- request object must contain all parameters that are also sent as regular parameters
- Request Object "exp" claim is required
- Request Object must contain all parameters that are also sent as regular parameters
- userinfo endpoint becomes a FAPI resource, echoing back the x-fapi-interaction-id header and disabling query string as a mechanism for providing access tokens


Expand Down Expand Up @@ -1772,7 +1772,7 @@ To change the default client response_types configure `clientDefaults` to be an

### clockTolerance

A `Number` value (in seconds) describing the allowed system clock skew for validating client-provided JWTs, e.g. Request objects, DPoP Proofs and otherwise comparing timestamps
A `Number` value (in seconds) describing the allowed system clock skew for validating client-provided JWTs, e.g. Request Objects, DPoP Proofs and otherwise comparing timestamps

_**recommendation**_: Only set this to a reasonable value when needed to cover server-side client and oidc-provider server clock skew. More than 5 minutes (if needed) is probably a sign something else is wrong.

Expand Down
43 changes: 41 additions & 2 deletions lib/actions/authorization/check_client.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
const { InvalidClient } = require('../../helpers/errors');
const { strict: assert } = require('assert');

const { InvalidClient, InvalidRequestObject } = require('../../helpers/errors');
const presence = require('../../helpers/validate_presence');
const base64url = require('../../helpers/base64url');

/*
* Checks client_id
Expand All @@ -10,7 +13,43 @@ const presence = require('../../helpers/validate_presence');
* @throws: invalid_client
*/
module.exports = async function checkClient(ctx, next) {
presence(ctx, 'client_id');
const { oidc: { params } } = ctx;

try {
presence(ctx, 'client_id');
} catch (err) {
const { request } = params;
if (request === undefined) {
throw err;
}

const parts = request.split('.');
let decoded;
let clientId;

try {
assert(parts.length === 3 || parts.length === 5);
parts.forEach((part, i, { length }) => {
if (length === 3 && i === 1) { // JWT Payload
decoded = JSON.parse(base64url.decodeToBuffer(part));
} else if (length === 5 && i === 0) { // JWE Header
decoded = JSON.parse(base64url.decodeToBuffer(part));
}
});
} catch (error) {
throw new InvalidRequestObject(`Request Object is not a valid ${parts.length === 5 ? 'JWE' : 'JWT'}`);
}

if (decoded) {
clientId = decoded.client_id || decoded.iss;
}

if (typeof clientId !== 'string' || !clientId) {
throw err;
}

params.client_id = clientId;
}

const client = await ctx.oidc.provider.Client.find(ctx.oidc.params.client_id);

Expand Down
13 changes: 1 addition & 12 deletions lib/actions/authorization/fetch_request_uri.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
const { URL } = require('url');
const { strict: assert } = require('assert');

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

const allowedSchemes = new Set(['http:', 'https:', 'urn:']);
Expand All @@ -21,17 +19,8 @@ const allowedSchemes = new Set(['http:', 'https:', 'urn:']);
* @throws: request_uri_not_supported
*/
module.exports = async function fetchRequestUri(ctx, next) {
const { request, requestUri } = instance(ctx.oidc.provider).configuration('features');
const { 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');
}
Expand Down
14 changes: 8 additions & 6 deletions lib/actions/authorization/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const getTokenAuth = require('../../shared/token_auth');

const checkClient = require('./check_client');
const checkResponseMode = require('./check_response_mode');
const rejectUnsupported = require('./reject_unsupported');
const rejectRegistration = require('./reject_registration');
const oauthRequired = require('./oauth_required');
const oneRedirectUriClients = require('./one_redirect_uri_clients');
Expand Down Expand Up @@ -50,7 +51,6 @@ const DR = 'device_resume';
const ALL = [A, DA, R, CV, DR];

const parseBody = bodyParser.bind(_, 'application/x-www-form-urlencoded');
const rejectDupeClientId = rejectDupes.bind(_, { only: new Set(['client_id']) });

module.exports = function authorizationAction(provider, endpoint) {
const {
Expand Down Expand Up @@ -115,15 +115,17 @@ module.exports = function authorizationAction(provider, endpoint) {
}
use(() => deviceAuthorizationClientId, DA );
use(() => paramsMiddleware.bind(_, whitelist), A, DA );
use(() => rejectDupeClientId, ...ALL);
use(() => checkClient, ...ALL);
use(() => oneRedirectUriClients, A );
use(() => rejectDupesMiddleware, A, DA );
use(() => rejectUnsupported, A, DA );
use(() => checkClient, ...ALL);
use(() => checkClientGrantType, DA );
use(() => checkResponseMode, A );
use(() => oauthRequired, A );
use(() => fetchRequestUri, A, DA );
use(() => processRequestObject.bind(_, whitelist), A, DA );
use(() => processRequestObject.bind(
_, whitelist, rejectDupesMiddleware,
), A, DA );
use(() => oneRedirectUriClients, A );
use(() => oauthRequired, A );
use(() => rejectRegistration, A, DA );
use(() => oidcRequired, A );
use(() => assignDefaults, A, DA );
Expand Down
32 changes: 21 additions & 11 deletions lib/actions/authorization/process_request_object.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ const checkResponseMode = require('./check_response_mode');
*
* @throws: invalid_request_object
*/
module.exports = async function processRequestObject(PARAM_LIST, ctx, next) {
module.exports = async function processRequestObject(PARAM_LIST, rejectDupesMiddleware, ctx, next) {
const { params, client } = ctx.oidc;

if (
client.requestObjectSigningAlg
&& params.request === undefined
) {
throw new InvalidRequest('request object must be used by this client');
throw new InvalidRequest('Request Object must be used by this client');
}

if (params.request === undefined) {
Expand Down Expand Up @@ -59,7 +59,7 @@ module.exports = async function processRequestObject(PARAM_LIST, ctx, next) {
try {
decoded = JWT.decode(params.request);
} catch (err) {
throw new InvalidRequestObject(`could not parse request object (${err.message})`);
throw new InvalidRequestObject(`could not parse Request Object (${err.message})`);
}

const { payload, header: { alg } } = decoded;
Expand All @@ -68,7 +68,7 @@ module.exports = async function processRequestObject(PARAM_LIST, ctx, next) {
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')) {
} else if (Array.isArray(value)) {
acc[key] = value;
} else if (typeof value !== 'string') {
acc[key] = String(value);
Expand All @@ -80,6 +80,8 @@ module.exports = async function processRequestObject(PARAM_LIST, ctx, next) {
return acc;
}, {});

rejectDupesMiddleware({ oidc: { params: request } }, () => {});

if (request.state !== undefined) {
params.state = request.state;
}
Expand All @@ -90,14 +92,22 @@ module.exports = async function processRequestObject(PARAM_LIST, ctx, next) {
}

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

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

if (request.client_id !== undefined && request.client_id !== params.client_id) {
if (
params.client_id
&& 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 @@ -118,7 +128,7 @@ module.exports = async function processRequestObject(PARAM_LIST, ctx, next) {

if (conf('features.fapiRW.enabled')) {
if (!('exp' in payload)) {
throw new InvalidRequestObject('request object is missing the "exp" claim');
throw new InvalidRequestObject('Request Object is missing the "exp" claim');
}

const missing = Object.entries(ctx.oidc.params)
Expand All @@ -137,7 +147,7 @@ module.exports = async function processRequestObject(PARAM_LIST, ctx, next) {
}).filter(Boolean);

if (missing.length) {
throw new InvalidRequestObject(`all parameters shall be present inside the signed request object (missing: ${missing.join(', ')})`);
throw new InvalidRequestObject(`all parameters shall be present inside the signed Request Object (missing: ${missing.join(', ')})`);
}
}

Expand All @@ -152,7 +162,7 @@ module.exports = async function processRequestObject(PARAM_LIST, ctx, next) {
await JWT.verify(params.request, client.keystore, opts);
trusted = true;
} catch (err) {
throw new InvalidRequestObject(`could not validate request object (${err.message})`);
throw new InvalidRequestObject(`could not validate Request Object (${err.message})`);
}

if (payload.jti && payload.exp && payload.iss) {
Expand All @@ -169,7 +179,7 @@ module.exports = async function processRequestObject(PARAM_LIST, ctx, next) {
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');
throw new InvalidRequestObject('Request Object from unsecure request_uri must be signed and/or symmetrically encrypted');
}

Object.assign(params, request);
Expand Down
22 changes: 22 additions & 0 deletions lib/actions/authorization/reject_unsupported.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
const { RequestNotSupported, RequestUriNotSupported } = require('../../helpers/errors');
const instance = require('../../helpers/weak_cache');

/*
* Rejects registration parameter as not supported.
*
* @throws: registration_not_supported
*/
module.exports = function rejectUnsupported(ctx, next) {
const { request, requestUri } = instance(ctx.oidc.provider).configuration('features');
const { params } = ctx.oidc;

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

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

return next();
};
6 changes: 3 additions & 3 deletions lib/helpers/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ const DEFAULTS = {
* clockTolerance
*
* description: A `Number` value (in seconds) describing the allowed system clock skew for
* validating client-provided JWTs, e.g. request objects, DPoP Proofs and otherwise comparing
* validating client-provided JWTs, e.g. Request Objects, DPoP Proofs and otherwise comparing
* timestamps
* recommendation: Only set this to a reasonable value when needed to cover server-side client and
* oidc-provider server clock skew. More than 5 minutes (if needed) is probably a sign something
Expand Down Expand Up @@ -727,8 +727,8 @@ const DEFAULTS = {
* description: Enables extra behaviours defined in FAPI Part 1 & 2 that cannot be achieved by
* other configuration options, namely:
*
* - request object "exp" claim is required
* - request object must contain all parameters that are also sent as regular parameters
* - Request Object "exp" claim is required
* - Request Object must contain all parameters that are also sent as regular parameters
* - userinfo endpoint becomes a FAPI resource, echoing back the x-fapi-interaction-id header
* and disabling query string as a mechanism for providing access tokens
*/
Expand Down
12 changes: 9 additions & 3 deletions lib/shared/authorization_error_handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const instance = require('../helpers/weak_cache');
const errOut = require('../helpers/err_out');
const processSessionState = require('../helpers/process_session_state');
const resolveResponseMode = require('../helpers/resolve_response_mode');
const oneRedirectUriClients = require('../actions/authorization/one_redirect_uri_clients');

const debug = new Debug('oidc-provider:authentication:error');
const serverError = new Debug('oidc-provider:server_error');
Expand All @@ -17,6 +18,7 @@ module.exports = (provider) => {
Err: RedirectUriMismatch,
method: 'redirectUriAllowed',
check: 'redirectUriCheckPerformed',
recovery: oneRedirectUriClients,
},
web_message_uri: {
Err: WebMessageUriMismatch,
Expand Down Expand Up @@ -65,14 +67,18 @@ module.exports = (provider) => {
}

for (const [param, { // eslint-disable-line no-restricted-syntax
Err, check, flag, method,
Err, check, flag, method, recovery,
}] of AD_ACTA_CHECKS) {
if (
(!flag || instance(provider).configuration(flag))
&& !(err instanceof Err) && oidc.client
&& safe(params[param]) && !oidc[check]
&& !oidc[check]
) {
if (!oidc.client[method](safe(params[param]))) {
if (recovery && !safe(params[param])) {
recovery(ctx, () => {});
}

if (safe(params[param]) && !oidc.client[method](params[param])) {
getOutAndEmit(ctx, caught, safe(params.state));
err = new Err();
ctx.status = err.statusCode;
Expand Down
8 changes: 3 additions & 5 deletions lib/shared/reject_dupes.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ function defaultMap([key, value]) {
return Array.isArray(value) ? key : undefined;
}

async function rejectDupes({ except, only }, ctx, next) {
module.exports = function rejectDupes({ except, only } = {}, ctx, next) {
let mapFn;

if (except) {
Expand All @@ -39,7 +39,5 @@ async function rejectDupes({ except, only }, ctx, next) {
throw new InvalidRequest(`parameters must not be provided twice. (${params.join(',')})`);
}

await next();
}

module.exports = rejectDupes;
return next();
};
Loading

0 comments on commit e3fa143

Please sign in to comment.