From a0a60e07da4a277602f757f095fe487db28c2c34 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Sun, 19 May 2019 16:58:44 +0200 Subject: [PATCH] refactor: less bound body parser functions, test coverage --- README.md | 4 +-- docs/README.md | 4 +-- example/README.md | 4 +-- lib/actions/check_session.js | 4 +-- lib/actions/interaction.js | 8 ++--- lib/actions/introspection.js | 13 ++++--- lib/actions/registration.js | 4 +-- lib/actions/revocation.js | 13 ++++--- lib/actions/token.js | 3 +- lib/helpers/configuration.js | 2 +- lib/helpers/defaults.js | 2 +- lib/helpers/initialize_adapter.js | 4 ++- lib/helpers/initialize_app.js | 4 +-- lib/helpers/interaction/check.js | 1 + lib/helpers/interaction/index.js | 2 ++ lib/helpers/interaction/prompt.js | 1 + lib/models/id_token.js | 5 ++- lib/models/session.js | 1 + lib/provider.js | 2 ++ lib/shared/selective_body.js | 11 ++++-- .../backchannel_logout.test.js | 35 +++++++++++++++++++ 21 files changed, 90 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 4645b3ab7..d3f99b39f 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,7 @@ The following drafts/experimental specifications are implemented by oidc-provide Updates to draft and experimental specification versions are released as MINOR library versions, if you utilize these specification implementations consider using the tilde `~` operator in your package.json since breaking changes may be introduced as part of these version updates. Alternatively -[acknowledge](https://github.com/panva/node-oidc-provider/tree/master/docs#features) the version and +[acknowledge](https://github.com/panva/node-oidc-provider/tree/master/docs/README.md#features) the version and be notified of breaking changes as part of your CI. Missing a feature? - If it wasn't already discussed before, [ask for it][suggest-feature]. @@ -115,7 +115,7 @@ Also be sure to check the available configuration docs section. ## Configuration oidc-provider allows to be extended and configured in various ways to fit a variety of uses. See -the [available configuration](/docs). +the [available configuration](/docs/README.md). ```js const Provider = require('oidc-provider'); diff --git a/docs/README.md b/docs/README.md index b11008b7d..fa489da6f 100644 --- a/docs/README.md +++ b/docs/README.md @@ -180,7 +180,7 @@ interaction session object. The Provider instance comes with helpers that aid with getting interaction details as well as packing the results. See them used in the [step-by-step](https://github.com/panva/node-oidc-provider-example) -or [in-repo](/example/index.js) examples. +or [in-repo](/example) examples. **`#provider.interactionDetails(req)`** @@ -653,7 +653,7 @@ new Provider('http://localhost:3000', { // NOTICE: The following draft features are enabled and their implemented version not acknowledged // NOTICE: - OAuth 2.0 Web Message Response Mode - draft 00 (This is an Individual draft. URL: https://tools.ietf.org/html/draft-sakimura-oauth-wmrm-00) // NOTICE: Breaking changes between draft version updates may occur and these will be published as MINOR semver oidc-provider updates. -// NOTICE: You may disable this notice and these potentially breaking updates by acknowledging the current draft version. See https://github.com/panva/node-oidc-provider/tree/master/docs#features +// NOTICE: You may disable this notice and these potentially breaking updates by acknowledging the current draft version. See https://github.com/panva/node-oidc-provider/tree/master/docs/README.md#features new Provider('http://localhost:3000', { features: { webMessageResponseMode: { diff --git a/example/README.md b/example/README.md index f86ae2e52..886199b4e 100644 --- a/example/README.md +++ b/example/README.md @@ -11,8 +11,8 @@ See the following examples Further resources -- [Configuration](/docs) -- [Mounting to a path](/docs#mounting-oidc-provider) +- [Configuration](/docs/README.md) +- [Mounting to a path](/docs/README.md#mounting-oidc-provider) Useful to know oidc-provider dependencies - [Koa](https://koajs.com/) - web framework oidc-provider uses internally diff --git a/lib/actions/check_session.js b/lib/actions/check_session.js index cb5aef84e..e8ebf0a02 100644 --- a/lib/actions/check_session.js +++ b/lib/actions/check_session.js @@ -1,5 +1,5 @@ const instance = require('../helpers/weak_cache'); -const bodyParser = require('../shared/selective_body'); +const { json: parseBody } = require('../shared/selective_body'); const noCache = require('../shared/no_cache'); const paramsMiddleware = require('../shared/assemble_params'); const presence = require('../helpers/validate_presence'); @@ -153,7 +153,7 @@ window.addEventListener('message', receiveMessage, false); }, post: [ noCache, - bodyParser.bind(undefined, 'application/json'), + parseBody, paramsMiddleware.bind(undefined, PARAM_LIST), async function checkClientOrigin(ctx, next) { presence(ctx, 'origin', 'client_id'); diff --git a/lib/actions/interaction.js b/lib/actions/interaction.js index 8a124f429..b7d9d9d56 100644 --- a/lib/actions/interaction.js +++ b/lib/actions/interaction.js @@ -2,21 +2,16 @@ const url = require('url'); const querystring = require('querystring'); const { inspect } = require('util'); -const _ = require('lodash'); - const attention = require('../helpers/attention'); -const bodyParser = require('../shared/selective_body'); +const { urlencoded: parseBody } = require('../shared/selective_body'); const views = require('../views'); const instance = require('../helpers/weak_cache'); const epochTime = require('../helpers/epoch_time'); const noCache = require('../shared/no_cache'); -const parseBody = bodyParser.bind(undefined, 'application/x-www-form-urlencoded'); - const keys = new Set(); const dbg = obj => querystring.stringify(Object.entries(obj).reduce((acc, [key, value]) => { keys.add(key); - if (_.isEmpty(value)) return acc; acc[key] = inspect(value, { depth: null }); return acc; }, {}), '
', ': ', { @@ -117,6 +112,7 @@ you are expected to disable these interactions and provide your own'); }); break; } + /* istanbul ignore next */ default: ctx.throw(501, 'not implemented'); } diff --git a/lib/actions/introspection.js b/lib/actions/introspection.js index 75f68cb97..a57f404e7 100644 --- a/lib/actions/introspection.js +++ b/lib/actions/introspection.js @@ -7,7 +7,7 @@ const presence = require('../helpers/validate_presence'); const getTokenAuth = require('../shared/token_auth'); const noCache = require('../shared/no_cache'); const instance = require('../helpers/weak_cache'); -const bodyParser = require('../shared/selective_body'); +const { urlencoded: parseBody } = require('../shared/selective_body'); const rejectDupes = require('../shared/reject_dupes'); const paramsMiddleware = require('../shared/assemble_params'); const { InvalidRequest } = require('../helpers/errors'); @@ -20,7 +20,6 @@ module.exports = function introspectionAction(provider) { const PARAM_LIST = new Set(['token', 'token_type_hint', ...authParams]); const configuration = instance(provider).configuration(); const { pairwiseIdentifier, features: { jwtIntrospection } } = configuration; - const parseBody = bodyParser.bind(undefined, 'application/x-www-form-urlencoded'); const { grantTypeHandlers } = instance(provider); const { IdToken, AccessToken, ClientCredentials, RefreshToken, Client, @@ -31,12 +30,18 @@ module.exports = function introspectionAction(provider) { } function getClientCredentials(token) { - if (!grantTypeHandlers.has('client_credentials')) return undefined; + /* istanbul ignore if */ + if (!grantTypeHandlers.has('client_credentials')) { + return undefined; + } return ClientCredentials.find(token); } function getRefreshToken(token) { - if (!grantTypeHandlers.has('refresh_token')) return undefined; + /* istanbul ignore if */ + if (!grantTypeHandlers.has('refresh_token')) { + return undefined; + } return RefreshToken.find(token); } diff --git a/lib/actions/registration.js b/lib/actions/registration.js index bc875b22f..5e62cefb3 100644 --- a/lib/actions/registration.js +++ b/lib/actions/registration.js @@ -2,14 +2,12 @@ const { has, findKey, chain } = require('lodash'); const constantEquals = require('../helpers/constant_equals'); const noCache = require('../shared/no_cache'); -const bodyParser = require('../shared/selective_body'); +const { json: parseBody } = require('../shared/selective_body'); const epochTime = require('../helpers/epoch_time'); const { InvalidToken, InvalidRequest } = require('../helpers/errors'); const instance = require('../helpers/weak_cache'); const setWWWAuthenticate = require('../helpers/set_www_authenticate'); -const parseBody = bodyParser.bind(undefined, 'application/json'); - const FORBIDDEN = [ 'registration_access_token', 'registration_client_uri', diff --git a/lib/actions/revocation.js b/lib/actions/revocation.js index d008a3b48..718c2a7d2 100644 --- a/lib/actions/revocation.js +++ b/lib/actions/revocation.js @@ -7,7 +7,7 @@ const { InvalidRequest } = require('../helpers/errors'); const presence = require('../helpers/validate_presence'); const instance = require('../helpers/weak_cache'); const getTokenAuth = require('../shared/token_auth'); -const bodyParser = require('../shared/selective_body'); +const { urlencoded: parseBody } = require('../shared/selective_body'); const rejectDupes = require('../shared/reject_dupes'); const paramsMiddleware = require('../shared/assemble_params'); const revokeGrant = require('../helpers/revoke_grant'); @@ -15,7 +15,6 @@ const revokeGrant = require('../helpers/revoke_grant'); const revokeable = new Set(['AccessToken', 'ClientCredentials', 'RefreshToken']); module.exports = function revocationAction(provider) { - const parseBody = bodyParser.bind(undefined, 'application/x-www-form-urlencoded'); const { params: authParams, middleware: tokenAuth } = getTokenAuth(provider, 'revocation'); const PARAM_LIST = new Set(['token', 'token_type_hint', ...authParams]); const { grantTypeHandlers } = instance(provider); @@ -25,12 +24,18 @@ module.exports = function revocationAction(provider) { } function getClientCredentials(token) { - if (!grantTypeHandlers.has('client_credentials')) return undefined; + /* istanbul ignore if */ + if (!grantTypeHandlers.has('client_credentials')) { + return undefined; + } return provider.ClientCredentials.find(token); } function getRefreshToken(token) { - if (!grantTypeHandlers.has('refresh_token')) return undefined; + /* istanbul ignore if */ + if (!grantTypeHandlers.has('refresh_token')) { + return undefined; + } return provider.RefreshToken.find(token); } diff --git a/lib/actions/token.js b/lib/actions/token.js index db6f238e6..22754b95b 100644 --- a/lib/actions/token.js +++ b/lib/actions/token.js @@ -7,7 +7,7 @@ const { } = require('../helpers/errors'); const noCache = require('../shared/no_cache'); const getTokenAuth = require('../shared/token_auth'); -const bodyParser = require('../shared/selective_body'); +const { urlencoded: parseBody } = require('../shared/selective_body'); const rejectDupes = require('../shared/reject_dupes'); const paramsMiddleware = require('../shared/assemble_params'); const checkResourceFormat = require('../shared/check_resource_format'); @@ -15,7 +15,6 @@ const checkResourceFormat = require('../shared/check_resource_format'); const grantTypeSet = new Set(['grant_type']); module.exports = function tokenAction(provider) { - const parseBody = bodyParser.bind(undefined, 'application/x-www-form-urlencoded'); const { params: authParams, middleware: tokenAuth } = getTokenAuth(provider, 'token'); const { grantTypeParams } = instance(provider); diff --git a/lib/helpers/configuration.js b/lib/helpers/configuration.js index c37080ae5..aad9ff495 100644 --- a/lib/helpers/configuration.js +++ b/lib/helpers/configuration.js @@ -483,7 +483,7 @@ module.exports = class Configuration { attention.info(` - ${name} (This is an ${type}. URL: ${url})`); }); attention.info('Breaking changes between draft version updates may occur and these will be published as MINOR semver oidc-provider updates.'); - attention.info('You may disable this notice and these potentially breaking updates by acknowledging the current draft version. See https://github.com/panva/node-oidc-provider/tree/master/docs#features'); + attention.info('You may disable this notice and these potentially breaking updates by acknowledging the current draft version. See https://github.com/panva/node-oidc-provider/tree/master/docs/README.md#features'); if (throwDraft) { throw new Error('An unacknowledged version of a draft feature is included in this oidc-provider version.'); diff --git a/lib/helpers/defaults.js b/lib/helpers/defaults.js index 862cc7c73..e1c4cd796 100644 --- a/lib/helpers/defaults.js +++ b/lib/helpers/defaults.js @@ -308,7 +308,7 @@ const DEFAULTS = { * // NOTICE: The following draft features are enabled and their implemented version not acknowledged * // NOTICE: - OAuth 2.0 Web Message Response Mode - draft 00 (This is an Individual draft. URL: https://tools.ietf.org/html/draft-sakimura-oauth-wmrm-00) * // NOTICE: Breaking changes between draft version updates may occur and these will be published as MINOR semver oidc-provider updates. - * // NOTICE: You may disable this notice and these potentially breaking updates by acknowledging the current draft version. See https://github.com/panva/node-oidc-provider/tree/master/docs#features + * // NOTICE: You may disable this notice and these potentially breaking updates by acknowledging the current draft version. See https://github.com/panva/node-oidc-provider/tree/master/docs/README.md#features * * new Provider('http://localhost:3000', { * features: { diff --git a/lib/helpers/initialize_adapter.js b/lib/helpers/initialize_adapter.js index 46ce01ad7..9c4eb62af 100644 --- a/lib/helpers/initialize_adapter.js +++ b/lib/helpers/initialize_adapter.js @@ -5,7 +5,9 @@ const attention = require('./attention'); module.exports = function initializeAdapter(adapter = MemoryAdapter) { if (adapter === MemoryAdapter) { - attention.warn('a quick start development-only in-memory adapter is used'); + attention.warn('a quick start development-only in-memory adapter is used, you MUST change it in' + + ' order to not lose all stateful provider data upon restart and to be able to share these' + + ' between processes'); } if (!adapter.prototype || !adapter.prototype.constructor.name) { diff --git a/lib/helpers/initialize_app.js b/lib/helpers/initialize_app.js index e5ed1932b..fcdf79238 100644 --- a/lib/helpers/initialize_app.js +++ b/lib/helpers/initialize_app.js @@ -216,10 +216,10 @@ module.exports = function initializeApp() { if (proxyWarning.pass) return next(); if (issuer.startsWith('https:') && !ctx.secure && ctx.get('x-forwarded-proto') === 'https') { - attention.warn(`x-forwarded-proto header detected but not trusted, you must set proxy=true on the provider, see documentation for more details (${homepage}/tree/${version}/docs#trusting-tls-offloading-proxies)`); + attention.warn(`x-forwarded-proto header detected but not trusted, you must set proxy=true on the provider, see documentation for more details (${homepage}/tree/${version}/docs/README.md#trusting-tls-offloading-proxies)`); proxyWarning.pass = true; } else if (issuer.startsWith('https:') && !ctx.secure && !ctx.get('x-forwarded-proto')) { - attention.warn(`x-forwarded-proto header not detected for an https issuer, you must configure your ssl offloading proxy and the provider, see documentation for more details (${homepage}/tree/${version}/docs#trusting-tls-offloading-proxies)`); + attention.warn(`x-forwarded-proto header not detected for an https issuer, you must configure your ssl offloading proxy and the provider, see documentation for more details (${homepage}/tree/${version}/docs/README.md#trusting-tls-offloading-proxies)`); proxyWarning.pass = true; } diff --git a/lib/helpers/interaction/check.js b/lib/helpers/interaction/check.js index 77019e6e9..9e2c37518 100644 --- a/lib/helpers/interaction/check.js +++ b/lib/helpers/interaction/check.js @@ -1,3 +1,4 @@ +/* istanbul ignore file */ /* eslint-disable no-param-reassign */ class Check { diff --git a/lib/helpers/interaction/index.js b/lib/helpers/interaction/index.js index 930a80359..c51ff373a 100644 --- a/lib/helpers/interaction/index.js +++ b/lib/helpers/interaction/index.js @@ -1,3 +1,5 @@ +/* istanbul ignore file */ + const Check = require('./check'); const Prompt = require('./prompt'); const login = require('./prompts/login'); diff --git a/lib/helpers/interaction/prompt.js b/lib/helpers/interaction/prompt.js index 1e41e7cbd..3dce34aba 100644 --- a/lib/helpers/interaction/prompt.js +++ b/lib/helpers/interaction/prompt.js @@ -1,3 +1,4 @@ +/* istanbul ignore file */ /* eslint-disable no-param-reassign */ const Check = require('./check'); diff --git a/lib/models/id_token.js b/lib/models/id_token.js index 159bf4b13..e68603e7d 100644 --- a/lib/models/id_token.js +++ b/lib/models/id_token.js @@ -67,6 +67,7 @@ module.exports = function getIdToken(provider) { case 'authorization': alg = client.authorizationSignedResponseAlg; break; + /* istanbul ignore next */ default: throw new Error('invalid IdToken use'); } @@ -104,7 +105,7 @@ module.exports = function getIdToken(provider) { })(); let encryption; - switch (use) { + switch (use) { // eslint-disable-line default-case case 'idtoken': encryption = { alg: client.idTokenEncryptedResponseAlg, @@ -129,8 +130,6 @@ module.exports = function getIdToken(provider) { enc: client.authorizationEncryptedResponseEnc, }; break; - default: - encryption = {}; } if (!encryption.enc) return signed; diff --git a/lib/models/session.js b/lib/models/session.js index 3c7e552a5..bce944d8c 100644 --- a/lib/models/session.js +++ b/lib/models/session.js @@ -42,6 +42,7 @@ module.exports = function getSession(provider) { return new Set(); } + /* istanbul ignore next */ throw new Error('expected Array to be stored'); } diff --git a/lib/provider.js b/lib/provider.js index fc873c011..dcacd973a 100644 --- a/lib/provider.js +++ b/lib/provider.js @@ -270,6 +270,7 @@ class Provider extends events.EventEmitter { return this.app.callback(); } + /* istanbul ignore next */ listen(...args) { return this.app.listen(...args); } @@ -307,6 +308,7 @@ class Provider extends events.EventEmitter { get requestUriCache() { return instance(this).requestUriCache; } } +/* istanbul ignore next */ ['env', 'proxy', 'subdomainOffset', 'keys'].forEach((method) => { Object.defineProperty(Provider.prototype, method, { get() { diff --git a/lib/shared/selective_body.js b/lib/shared/selective_body.js index bc432f274..4ecaa0c42 100644 --- a/lib/shared/selective_body.js +++ b/lib/shared/selective_body.js @@ -7,7 +7,7 @@ const { InvalidRequest } = require('../helpers/errors'); let warned; -module.exports = async function selectiveBody(cty, ctx, next) { +async function selectiveBody(cty, ctx, next) { if (ctx.is(cty)) { try { let usedFallback; @@ -50,4 +50,11 @@ is not recommended, resolving to use req.body or request.body instead'); } else { throw new InvalidRequest(`only ${cty} content-type bodies are supported on ${ctx.method} ${ctx.path}`); } -}; +} + +Object.assign(selectiveBody, { + json: selectiveBody.bind(undefined, 'application/json'), + urlencoded: selectiveBody.bind(undefined, 'application/x-www-form-urlencoded'), +}); + +module.exports = selectiveBody; diff --git a/test/backchannel_logout/backchannel_logout.test.js b/test/backchannel_logout/backchannel_logout.test.js index 918b94b4f..7f28d7a95 100644 --- a/test/backchannel_logout/backchannel_logout.test.js +++ b/test/backchannel_logout/backchannel_logout.test.js @@ -7,6 +7,8 @@ const nock = require('nock'); const bootstrap = require('../test_helper'); +const fail = () => { throw new Error('expected promise to be rejected'); }; + describe('Back-Channel Logout 1.0', () => { before(bootstrap(__dirname)); @@ -36,6 +38,27 @@ describe('Back-Channel Logout 1.0', () => { return client.backchannelLogout('subject', 'foo'); }); + it('uses custom http options when provided', async function () { + const client = await this.provider.Client.find('client'); + + const pre = i(this.provider).configuration('httpOptions'); + i(this.provider).configuration().httpOptions = (opts) => { + Object.assign(opts.body, { foo: 'bar' }); + return opts; + }; + + nock('https://client.example.com/') + .filteringRequestBody((body) => { + expect(body).to.match(/^logout_token=(([\w-]+\.?){3})&foo=bar$/); + }) + .post('/backchannel_logout') + .reply(200); + + return client.backchannelLogout('subject', 'foo').then(() => { + i(this.provider).configuration().httpOptions = pre; + }); + }); + it('omits sid when its not required', async function () { const client = await this.provider.Client.find('no-sid'); @@ -54,6 +77,18 @@ describe('Back-Channel Logout 1.0', () => { return client.backchannelLogout('subject', 'foo'); }); + + it('handles non-200 OK responses', async function () { + const client = await this.provider.Client.find('no-sid'); + + nock('https://no-sid.example.com/') + .post('/backchannel_logout') + .reply(500); + + return client.backchannelLogout('subject', 'foo').then(fail, (err) => { + expect(err.message).to.eql('expected 200 OK from https://no-sid.example.com/backchannel_logout, got: 500 Internal Server Error'); + }); + }); }); describe('discovery', () => {