Skip to content

Commit

Permalink
refactor: removed dynamicScopes configuration option
Browse files Browse the repository at this point in the history
BREAKING CHANGE: removed `dynamicScopes` configuration option, scope
configuration using pre-configured values is gone in favour of
Resource Indicators refactor.
  • Loading branch information
panva committed Mar 6, 2020
1 parent b395a0d commit 285fc7a
Show file tree
Hide file tree
Showing 18 changed files with 11 additions and 314 deletions.
33 changes: 3 additions & 30 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,6 @@ location / {
- [conformIdTokenClaims ❗](#conformidtokenclaims)
- [cookies](#cookies)
- [discovery](#discovery)
- [dynamicScopes](#dynamicscopes)
- [expiresWithSession](#expireswithsession)
- [extraAccessTokenClaims](#extraaccesstokenclaims)
- [extraClientMetadata](#extraclientmetadata)
Expand Down Expand Up @@ -2141,30 +2140,6 @@ _**default value**_:
}
```

### dynamicScopes

Array of the dynamic scope values that the OP supports. These must be regular expressions that the OP will check string scope values, that aren't in the static list, against.



_**default value**_:
```js
[]
```
<a id="dynamic-scopes-to-enable-a-dynamic-scope-values-like-api-write-hex-id-and-api-read-hex-id"></a><details><summary>(Click to expand) To enable a dynamic scope values like `api:write:{hex id}` and `api:read:{hex id}`</summary><br>


Configure `dynamicScopes` like so:


```js
[
/^api:write:[a-fA-F0-9]{2,}$/,
/^api:read:[a-fA-F0-9]{2,}$/,
]
```
</details>

### expiresWithSession

Function used to decide whether the given authorization code/ device code or implicit returned access token be bound to the user session. This will be applied to all tokens issued from the authorization / device code in the future. When tokens are session-bound the session will be loaded by its `uid` every time the token is encountered. Session bound tokens will effectively get revoked if the end-user logs out.
Expand Down Expand Up @@ -2226,16 +2201,14 @@ validator function that will be executed in order once for every property define

_**default value**_:
```js
<<<<<<< HEAD
function extraClientMetadataValidator(key, value, metadata, ctx) {
=======
function validator(ctx, key, value, metadata) {
function extraClientMetadataValidator(ctx, key, value, metadata) {
// @param ctx - koa request context (only provided when a client is being constructed during
// Client Registration Request or Client Update Request
>>>>>>> eed8a207... refactor: extraClientMetadata.validator arguments reordered
// @param key - the client metadata property name
// @param value - the property value
// @param metadata - the current accumulated client metadata
// @param ctx - koa request context (only provided when a client is being constructed during
// Client Registration Request or Client Update Request
// validations for key, value, other related metadata
// throw new Provider.errors.InvalidClientMetadata() to reject the client metadata
// metadata[key] = value; to (re)assign metadata values
Expand Down
12 changes: 1 addition & 11 deletions lib/actions/authorization/check_scope.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
const instance = require('../../helpers/weak_cache');
const { InvalidScope } = require('../../helpers/errors');
const { DYNAMIC_SCOPE_LABEL } = require('../../consts');

/*
* Validates that all requested scopes are supported by the provider, and that offline_access prompt
Expand All @@ -9,7 +8,7 @@ const { DYNAMIC_SCOPE_LABEL } = require('../../consts');
* @throws: invalid_request
*/
module.exports = function checkScope(PARAM_LIST, ctx, next) {
const { scopes: statics, dynamicScopes: dynamics } = instance(ctx.oidc.provider).configuration();
const { scopes: statics } = instance(ctx.oidc.provider).configuration();
const { prompts, client } = ctx.oidc;

let whitelist;
Expand All @@ -25,15 +24,6 @@ module.exports = function checkScope(PARAM_LIST, ctx, next) {
return true;
}

for (const dynamic of dynamics) { // eslint-disable-line no-restricted-syntax
if (dynamic.test(scope)) {
if (whitelist && !whitelist.has(dynamic[DYNAMIC_SCOPE_LABEL])) {
throw new InvalidScope('requested scope is not whitelisted', scope);
}
return true;
}
}

return false;
});

Expand Down
3 changes: 1 addition & 2 deletions lib/actions/discovery.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

const defaults = require('../helpers/_/defaults');
const instance = require('../helpers/weak_cache');
const { DYNAMIC_SCOPE_LABEL } = require('../consts');

module.exports = function discovery(ctx, next) {
const config = instance(ctx.oidc.provider).configuration();
Expand All @@ -25,7 +24,7 @@ module.exports = function discovery(ctx, next) {
authorization_response_iss_parameter_supported: features.issAuthResp.enabled ? true : undefined,
response_modes_supported: ['form_post', 'fragment', 'query'],
response_types_supported: config.responseTypes,
scopes_supported: [...config.scopes].concat([...config.dynamicScopes].map((s) => s[DYNAMIC_SCOPE_LABEL]).filter(Boolean)),
scopes_supported: [...config.scopes],
subject_types_supported: [...config.subjectTypes],
token_endpoint_auth_methods_supported: [...config.tokenEndpointAuthMethods],
token_endpoint_auth_signing_alg_values_supported: config.tokenEndpointAuthSigningAlgValues,
Expand Down
11 changes: 0 additions & 11 deletions lib/actions/grants/client_credentials.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
const instance = require('../../helpers/weak_cache');
const { InvalidGrant } = require('../../helpers/errors');
const { InvalidScope } = require('../../helpers/errors');
const { DYNAMIC_SCOPE_LABEL } = require('../../consts');

module.exports.handler = async function clientCredentialsHandler(ctx, next) {
const { ClientCredentials, ReplayDetection } = ctx.oidc.provider;
const {
audiences,
scopes: statics,
dynamicScopes: dynamics,
features: { dPoP: { iatTolerance }, mTLS: { getCertificate } },
} = instance(ctx.oidc.provider).configuration();

Expand All @@ -25,15 +23,6 @@ module.exports.handler = async function clientCredentialsHandler(ctx, next) {
return true;
}

for (const dynamic of dynamics) { // eslint-disable-line no-restricted-syntax
if (dynamic.test(scope)) {
if (whitelist && !whitelist.has(dynamic[DYNAMIC_SCOPE_LABEL])) {
throw new InvalidScope('requested scope is not whitelisted', scope);
}
return true;
}
}

return false;
}) : [];

Expand Down
1 change: 0 additions & 1 deletion lib/consts/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ const PUSHED_REQUEST_URN = 'urn:ietf:params:oauth:request_uri:';
module.exports = {
CLIENT_ATTRIBUTES,
DEV_KEYSTORE,
DYNAMIC_SCOPE_LABEL: Symbol('dynamic_scope_label'),
JWA,
PARAM_LIST,
PUSHED_REQUEST_URN,
Expand Down
11 changes: 1 addition & 10 deletions lib/helpers/claims.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const isPlainObject = require('./_/is_plain_object');

module.exports = function getClaims(provider) {
const {
claims: claimConfig, claimsSupported, scopes, dynamicScopes, pairwiseIdentifier,
claims: claimConfig, claimsSupported, pairwiseIdentifier,
} = instance(provider).configuration();

return class Claims {
Expand All @@ -26,15 +26,6 @@ module.exports = function getClaims(provider) {
scope(value = '') {
assert(!Object.keys(this.filter).length, 'scope cannot be assigned after mask has been set');
value.split(' ').forEach((scope) => {
if (!scopes.has(scope)) {
for (const dynamic of dynamicScopes) { // eslint-disable-line no-restricted-syntax
if (dynamic.test(scope)) {
scope = dynamic; // eslint-disable-line no-param-reassign
break;
}
}
}

this.mask(claimConfig.get(scope));
});
return this;
Expand Down
6 changes: 1 addition & 5 deletions lib/helpers/client_schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const {
WEB_URI,
WHEN,
},
DYNAMIC_SCOPE_LABEL,
} = require('../consts');

const validUrl = require('./valid_url');
Expand Down Expand Up @@ -46,9 +45,6 @@ module.exports = function getSchema(provider) {
const { features } = configuration;

const { scopes } = configuration;
const dynamicScopes = new Set(
[...configuration.dynamicScopes].map((s) => s[DYNAMIC_SCOPE_LABEL]).filter(Boolean),
);

const RECOGNIZED_METADATA = [...RECOGNIZED];
const DEFAULT = JSON.parse(JSON.stringify(DEFAULTS));
Expand Down Expand Up @@ -640,7 +636,7 @@ module.exports = function getSchema(provider) {
if (this.scope) {
const parsed = new Set(this.scope.split(' '));
parsed.forEach((scope) => {
if (!scopes.has(scope) && !dynamicScopes.has(scope)) {
if (!scopes.has(scope)) {
this.invalidate('scope must only contain supported scopes');
}
});
Expand Down
11 changes: 2 additions & 9 deletions lib/helpers/configuration.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class Configuration {

ensureSets() {
[
'scopes', 'subjectTypes', 'dynamicScopes', 'extraParams', 'acrValues',
'scopes', 'subjectTypes', 'extraParams', 'acrValues',
'tokenEndpointAuthMethods', 'introspectionEndpointAuthMethods', 'revocationEndpointAuthMethods',
'features.requestObjects.mergingStrategy.whitelist',
].forEach((prop) => {
Expand Down Expand Up @@ -201,9 +201,6 @@ class Configuration {
if (typeof scope === 'string' && !this.scopes.has(scope)) {
this.scopes.add(scope);
}
if (scope instanceof RegExp && !this.dynamicScopes.has(scope)) {
this.dynamicScopes.add(scope);
}
});
}

Expand Down Expand Up @@ -247,11 +244,7 @@ class Configuration {
Object.keys(this.claims.get(scope)).forEach(Set.prototype.add.bind(claims));
}
});
this.dynamicScopes.forEach((scope) => {
if (this.claims.has(scope)) {
Object.keys(this.claims.get(scope)).forEach(Set.prototype.add.bind(claims));
}
});

this.claims.forEach((value, key) => {
if (value === null) claims.add(key);
});
Expand Down
19 changes: 0 additions & 19 deletions lib/helpers/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -1937,25 +1937,6 @@ function getDefaults() {
*/
scopes: ['openid', 'offline_access'],

/*
* dynamicScopes
*
* description: Array of the dynamic scope values that the OP supports. These must be regular
* expressions that the OP will check string scope values, that aren't in the static list,
* against.
*
* example: To enable a dynamic scope values like `api:write:{hex id}` and `api:read:{hex id}`
* Configure `dynamicScopes` like so:
*
* ```js
* [
* /^api:write:[a-fA-F0-9]{2,}$/,
* /^api:read:[a-fA-F0-9]{2,}$/,
* ]
* ```
*/
dynamicScopes: [],

/*
* subjectTypes
*
Expand Down
9 changes: 1 addition & 8 deletions lib/helpers/oidc_context.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,17 +197,10 @@ module.exports = function getContext(provider) {
const requestParamScopes = new Set();
if (this.params.scope) {
const scopes = this.params.scope.split(' ');
const { scopes: statics, dynamicScopes: dynamics } = instance(provider).configuration();
const { scopes: statics } = instance(provider).configuration();
scopes.forEach((scope) => {
if (statics.has(scope)) {
requestParamScopes.add(scope);
return;
}
for (const dynamic of dynamics) { // eslint-disable-line no-restricted-syntax
if (dynamic.test(scope)) {
requestParamScopes.add(scope);
return;
}
}
});
}
Expand Down
2 changes: 0 additions & 2 deletions lib/index.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
const Provider = require('./provider');
const errors = require('./helpers/errors');
const { DYNAMIC_SCOPE_LABEL } = require('./consts');

module.exports = Provider;
module.exports.Provider = Provider;
module.exports.errors = errors;
module.exports.DYNAMIC_SCOPE_LABEL = DYNAMIC_SCOPE_LABEL;
module.exports.interactionPolicy = require('./helpers/interaction_policy');
9 changes: 1 addition & 8 deletions test/configuration/client_metadata.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const mtlsKeys = require('../jwks/jwks.json');

const sigKey = global.keystore.get().toJWK();
const privateKey = global.keystore.get().toJWK(true);
const { DYNAMIC_SCOPE_LABEL, errors: { InvalidClientMetadata } } = Provider;
const { errors: { InvalidClientMetadata } } = Provider;

describe('Client metadata validation', () => {
let DefaultProvider;
Expand Down Expand Up @@ -310,9 +310,6 @@ describe('Client metadata validation', () => {
});

context('scope', function () {
const SIGN = /^sign:[a-fA-F0-9]{2,}$/;
SIGN[DYNAMIC_SCOPE_LABEL] = 'sign:{hex}';

mustBeString(this.title);
allows(this.title, undefined);
allows(this.title, 'openid');
Expand All @@ -322,10 +319,6 @@ describe('Client metadata validation', () => {
allows(this.title, 'openid profile', undefined, { claims: { profile: ['given_name'] } });
allows(this.title, 'profile', undefined, { scopes: ['profile'] });
allows(this.title, 'profile', undefined, { claims: { profile: ['given_name'] } });
allows(this.title, 'openid sign:{hex}', undefined, { dynamicScopes: [SIGN] });
allows(this.title, 'openid sign:{hex}', undefined, { claims: new Map([[SIGN, ['given_name']]]) });
allows(this.title, 'sign:{hex}', undefined, { dynamicScopes: [SIGN] });
allows(this.title, 'sign:{hex}', undefined, { claims: new Map([[SIGN, ['given_name']]]) });
rejects(this.title, 'foo', /must only contain supported scopes/);
});

Expand Down
10 changes: 0 additions & 10 deletions test/configuration/constructor.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,6 @@ describe('Provider configuration', () => {
});
});

describe('dynamicScopes', () => {
it('only accepts arrays and sets', () => {
new Provider('http://localhost:3000', { dynamicScopes: [/foo/] });
new Provider('http://localhost:3000', { dynamicScopes: new Set([/foo/]) });
expect(() => {
new Provider('http://localhost:3000', { dynamicScopes: { foo: true } });
}).to.throw('dynamicScopes must be an Array or Set');
});
});

describe('claims', () => {
it('only accepts maps and objects', () => {
new Provider('http://localhost:3000', { claims: { foo: null } });
Expand Down
5 changes: 0 additions & 5 deletions test/configuration/scopes_claims.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,18 @@ describe('custom claims', () => {
});

it('detects new scopes from claims definition', () => {
const foo = /^foo:\d+$/;
const bar = /^bar:\d+$/;
const claims = new Map(Object.entries({
insurance: ['company_name', 'coverage'],
payment: {
preferred_method: null,
},
}));
claims.set(foo, ['foo']);
claims.set(foo, { bar: null });

const provider = new Provider('https://op.example.com', {
claims,
});

expect(i(provider).configuration('scopes')).to.contain('insurance', 'payment');
expect(i(provider).configuration('dynamicScopes')).to.contain(foo, bar);
});

it('removes the acr claim if no acrs are configured', () => {
Expand Down
41 changes: 0 additions & 41 deletions test/dynamic_scopes/dynamic_scopes.config.js

This file was deleted.

Loading

0 comments on commit 285fc7a

Please sign in to comment.