From 4df1a0c27a5d532d9f1d54d13e85bd2858425f58 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Wed, 11 Mar 2020 18:58:09 +0100 Subject: [PATCH] refactor: preparation for removal of structured tokens from db --- docs/README.md | 14 +++++++------- example/my_adapter.js | 9 --------- lib/helpers/defaults.js | 27 +++++++++++---------------- lib/helpers/jwt.js | 6 +----- lib/helpers/symbols.js | 1 + lib/models/base_model.js | 13 +++++++++++-- lib/models/device_code.js | 6 ++---- lib/models/formats/dynamic.js | 14 ++++---------- lib/models/formats/jwt.js | 19 +++++++++---------- lib/models/formats/jwt_ietf.js | 18 ++++++------------ lib/models/formats/opaque.js | 7 +++++-- lib/models/mixins/has_format.js | 8 -------- lib/models/replay_detection.js | 2 +- lib/models/session.js | 8 ++++---- test/formats/jwt.test.js | 7 ++++--- test/formats/jwt_ietf.test.js | 7 ++++--- test/formats/opaque.test.js | 8 ++++++++ 17 files changed, 78 insertions(+), 96 deletions(-) diff --git a/docs/README.md b/docs/README.md index ab00371da..831a01957 100644 --- a/docs/README.md +++ b/docs/README.md @@ -2271,11 +2271,11 @@ _**default value**_: ### formats -This option allows to configure the token serialization format. The different values change how a client-facing token value is generated as well as what properties get sent to the adapter for storage. - - `opaque` (default) formatted tokens store every property as a root property in your adapter - - `jwt` formatted tokens are issued as JWTs and stored the same as `opaque` only with additional property `jwt`. See `formats.jwtAccessTokenSigningAlg` for resolving the JWT Access Token signing algorithm. Note this is a proprietary format that will eventually get deprecated in favour of the 'jwt-ietf' value (once it gets stable and close to being an RFC) - - `jwt-ietf` formatted tokens are issued as JWTs and stored the same as `opaque` only with additional property `jwt-ietf`. See `formats.jwtAccessTokenSigningAlg` for resolving the JWT Access Token signing algorithm. This is an implementation of [JSON Web Token (JWT) Profile for OAuth 2.0 Access Tokens](https://tools.ietf.org/html/draft-ietf-oauth-access-token-jwt-05) draft and to enable it you need to enable `features.ietfJWTAccessTokenProfile`. 'jwt-ietf' value (once it gets stable and close to being an RFC) - - the value may also be a function dynamically determining the format (returning either `jwt`, `jwt-ietf`, or `opaque` depending on the token itself) +This option allows to configure the token value format. The different values change how a client-facing token value is generated. + Supported formats are: + - `opaque` (default) tokens are PRNG generated random strings using url safe base64 alphabet. See `formats.bitsOfOpaqueRandomness` for influencing the token length. + - `jwt` tokens are issued as JWTs. See `formats.tokenSigningAlg` for resolving the JWT Access Token signing algorithm. Note this is a proprietary format that will eventually get deprecated in favour of the 'jwt-ietf' value (once it gets stable and close to being an RFC). + - `jwt-ietf` tokens are issued as JWTs. See `formats.tokenSigningAlg` for resolving the JWT Access Token signing algorithm. This is an implementation of [JSON Web Token (JWT) Profile for OAuth 2.0 Access Tokens](https://tools.ietf.org/html/draft-ietf-oauth-access-token-jwt-05) draft and to enable it you need to enable `features.ietfJWTAccessTokenProfile`. @@ -2301,10 +2301,10 @@ Configure `formats`: { AccessToken: 'jwt' } ``` -
(Click to expand) To dynamically decide on the format used, e.g. only if it is intended for a resource
+
(Click to expand) To dynamically decide on the format used
-server Configure `formats`: +Configure `formats`: ```js diff --git a/example/my_adapter.js b/example/my_adapter.js index 9985f42e9..9974dae87 100644 --- a/example/my_adapter.js +++ b/example/my_adapter.js @@ -87,15 +87,6 @@ class MyAdapter { * - policies {string[]} - [InitialAccessToken, RegistrationAccessToken only] array of policies * - request {string} - [PushedAuthorizationRequest only] Pushed Request Object value * - * - * when `jwt` - * - same as `opaque` with the addition of - * - jwt {string} - the JWT value returned to the client - * - * when `jwt-ietf` - * - same as `opaque` with the addition of - * - 'jwt-ietf' {string} - the JWT value returned to the client - * * Client model will only use this when registered through Dynamic Registration features and * will contain all client properties. * diff --git a/lib/helpers/defaults.js b/lib/helpers/defaults.js index bdac67fe6..e706c8de1 100644 --- a/lib/helpers/defaults.js +++ b/lib/helpers/defaults.js @@ -1635,24 +1635,20 @@ function getDefaults() { /* * formats * - * description: This option allows to configure the token serialization format. The different - * values change how a client-facing token value is generated as well as what properties get - * sent to the adapter for storage. - * - `opaque` (default) formatted tokens store every property as a root property in your adapter - * - `jwt` formatted tokens are issued as JWTs and stored the same as `opaque` only with - * additional property `jwt`. See `formats.jwtAccessTokenSigningAlg` for resolving the JWT + * description: This option allows to configure the token value format. The different + * values change how a client-facing token value is generated. + * + * Supported formats are: + * - `opaque` (default) tokens are PRNG generated random strings using url safe base64 alphabet. + * See `formats.bitsOfOpaqueRandomness` for influencing the token length. + * - `jwt` tokens are issued as JWTs. See `formats.tokenSigningAlg` for resolving the JWT * Access Token signing algorithm. * Note this is a proprietary format that will eventually get deprecated in favour of the - * 'jwt-ietf' value (once it gets stable and close to being an RFC) - * - `jwt-ietf` formatted tokens are issued as JWTs and stored the same as `opaque` only with - * additional property `jwt-ietf`. See `formats.jwtAccessTokenSigningAlg` for resolving the - * JWT Access Token signing algorithm. - * This is an implementation of + * 'jwt-ietf' value (once it gets stable and close to being an RFC). + * - `jwt-ietf` tokens are issued as JWTs. See `formats.tokenSigningAlg` for resolving the JWT + * Access Token signing algorithm. This is an implementation of * [JSON Web Token (JWT) Profile for OAuth 2.0 Access Tokens](https://tools.ietf.org/html/draft-ietf-oauth-access-token-jwt-05) * draft and to enable it you need to enable `features.ietfJWTAccessTokenProfile`. - * 'jwt-ietf' value (once it gets stable and close to being an RFC) - * - the value may also be a function dynamically determining the format (returning either - * `jwt`, `jwt-ietf`, or `opaque` depending on the token itself) * * example: To enable JWT Access Tokens * @@ -1661,8 +1657,7 @@ function getDefaults() { * { AccessToken: 'jwt' } * ``` * - * example: To dynamically decide on the format used, e.g. only if it is intended for a resource - * server + * example: To dynamically decide on the format used * * Configure `formats`: * ```js diff --git a/lib/helpers/jwt.js b/lib/helpers/jwt.js index 741380f07..7587ebf8f 100644 --- a/lib/helpers/jwt.js +++ b/lib/helpers/jwt.js @@ -83,7 +83,7 @@ class JWT { static assertPayload(payload, { clockTolerance = 0, audience, ignoreExpiration, - ignoreAzp, ignoreIssued, ignoreNotBefore, issuer, jti, + ignoreAzp, ignoreIssued, ignoreNotBefore, issuer, } = {}) { const timestamp = epochTime(); @@ -114,10 +114,6 @@ class JWT { assert.equal(typeof payload.iss, 'string', 'invalid iss value'); } - if (jti) { - assert.equal(payload.jti, jti, 'jwt jti invalid'); - } - if (audience) { verifyAudience( payload, diff --git a/lib/helpers/symbols.js b/lib/helpers/symbols.js index 29d198dc7..39696c7b5 100644 --- a/lib/helpers/symbols.js +++ b/lib/helpers/symbols.js @@ -1,3 +1,4 @@ module.exports = { ROUTER_URL_METHOD: Symbol('ROUTER_URL_METHOD'), + LOOKUP_VALUE: Symbol('LOOKUP_VALUE'), }; diff --git a/lib/models/base_model.js b/lib/models/base_model.js index 0da12fc4d..ddff9d62c 100644 --- a/lib/models/base_model.js +++ b/lib/models/base_model.js @@ -13,6 +13,7 @@ const snakeCase = require('../helpers/_/snake_case'); const epochTime = require('../helpers/epoch_time'); const pickBy = require('../helpers/_/pick_by'); const instance = require('../helpers/weak_cache'); +const { LOOKUP_VALUE } = require('../helpers/symbols'); const hasFormat = require('./mixins/has_format'); @@ -42,6 +43,14 @@ module.exports = function getBaseToken(provider) { this.jti = jti; } + static instantiate(payload, lookupValue) { + const model = new this(payload); + if (lookupValue) { + model[LOOKUP_VALUE] = lookupValue; + } + return model; + } + async save(ttl) { if (!this.jti) { this.jti = this.generateTokenId(); @@ -92,10 +101,10 @@ module.exports = function getBaseToken(provider) { try { assert(stored); - const payload = await this.verify(value, stored, { ignoreExpiration }); + const payload = await this.verify(stored, { ignoreExpiration }); assert.equal(jti, payload.jti); - return new this(payload); + return this.instantiate(payload, value); } catch (err) { return undefined; } diff --git a/lib/models/device_code.js b/lib/models/device_code.js index 1a17a4cf1..537302fae 100644 --- a/lib/models/device_code.js +++ b/lib/models/device_code.js @@ -23,10 +23,8 @@ module.exports = (provider) => class DeviceCode extends apply([ try { assert(stored); assert.equal(userCode, stored.userCode); - const payload = await this.verify(undefined, stored, { - ignoreExpiration, foundByReference: true, - }); - return new this(payload); + const payload = await this.verify(stored, { ignoreExpiration }); + return this.instantiate(payload); } catch (err) { return undefined; } diff --git a/lib/models/formats/dynamic.js b/lib/models/formats/dynamic.js index 40d4537e7..43581abd5 100644 --- a/lib/models/formats/dynamic.js +++ b/lib/models/formats/dynamic.js @@ -1,6 +1,5 @@ const { strict: assert } = require('assert'); -const JWT = require('../../helpers/jwt'); const instance = require('../../helpers/weak_cache'); const ctxRef = require('../ctx_ref'); @@ -22,14 +21,9 @@ module.exports = (provider, formats) => ({ getTokenId(...args) { let format; const [value] = args; - if (value && (value.length === 27 || value.length === 43)) { - format = 'opaque'; - } else if (JWT_REGEX.test(value)) { - if (JWT.header(value).typ === 'at+jwt') { - format = 'jwt-ietf'; - } else { - format = 'jwt'; - } + if (JWT_REGEX.test(value)) { + // get tokenId is the same for jwt and jwt-ietf + format = 'jwt'; } else { format = 'opaque'; } @@ -37,7 +31,7 @@ module.exports = (provider, formats) => ({ return formats[format].getTokenId.apply(this, args); }, async verify(...args) { - const format = args[1].format || (args[1].jwt ? 'jwt' : args[1]['jwt-ietf'] ? 'jwt-ietf' : 'opaque'); // eslint-disable-line no-nested-ternary + const { format } = args[0]; assert(formats[format] && format !== 'dynamic', 'invalid format resolved'); return formats[format].verify.apply(this, args); }, diff --git a/lib/models/formats/jwt.js b/lib/models/formats/jwt.js index 6f97cc800..aa00e91dc 100644 --- a/lib/models/formats/jwt.js +++ b/lib/models/formats/jwt.js @@ -1,9 +1,12 @@ const { strict: assert } = require('assert'); +const { JWS } = require('jose'); + const JWT = require('../../helpers/jwt'); const instance = require('../../helpers/weak_cache'); const nanoid = require('../../helpers/nanoid'); const base64url = require('../../helpers/base64url'); +const { LOOKUP_VALUE } = require('../../helpers/symbols'); const ctxRef = require('../ctx_ref'); function getClaim(token, claim) { @@ -50,8 +53,8 @@ module.exports = (provider, { opaque }) => { let { accountId: sub } = payload; let value; - if (this.jwt) { - value = this.jwt; + if (this[LOOKUP_VALUE]) { + value = this[LOOKUP_VALUE]; } else { const ctx = ctxRef.get(this); const { key, alg } = await getSigningAlgAndKey(ctx, this, azp); @@ -98,20 +101,16 @@ module.exports = (provider, { opaque }) => { fields: structuredToken.header, }); } - payload.jwt = value; + payload.format = 'jwt'; return [value, payload]; }, getTokenId(token) { + JWS.verify(token, instance(provider).keystore, { parse: false }); return getClaim(token, 'jti'); }, - async verify(token, stored, { ignoreExpiration, foundByReference }) { - let jti; - if (!foundByReference) { - assert.equal(token, stored.jwt); - jti = this.getTokenId(token); - } - return opaque.verify.call(this, jti, stored, { ignoreExpiration, foundByReference }); + async verify(stored, { ignoreExpiration } = {}) { + return opaque.verify.call(this, stored, { ignoreExpiration, format: 'jwt' }); }, }; }; diff --git a/lib/models/formats/jwt_ietf.js b/lib/models/formats/jwt_ietf.js index 46fd10e4b..577e9e1c6 100644 --- a/lib/models/formats/jwt_ietf.js +++ b/lib/models/formats/jwt_ietf.js @@ -2,10 +2,9 @@ const { strict: assert } = require('assert'); const instance = require('../../helpers/weak_cache'); const JWT = require('../../helpers/jwt'); +const { LOOKUP_VALUE } = require('../../helpers/symbols'); const ctxRef = require('../ctx_ref'); -const PROPERTY = 'jwt-ietf'; - module.exports = (provider, { opaque, jwt }) => ({ generateTokenId: jwt.generateTokenId, getTokenId: jwt.getTokenId, @@ -25,8 +24,8 @@ module.exports = (provider, { opaque, jwt }) => ({ } let value; - if (this[PROPERTY]) { - value = this[PROPERTY]; + if (this[LOOKUP_VALUE]) { + value = this[LOOKUP_VALUE]; } else { const ctx = ctxRef.get(this); const { key, alg } = await jwt.getSigningAlgAndKey(ctx, this, clientId); @@ -75,16 +74,11 @@ module.exports = (provider, { opaque, jwt }) => ({ typ: 'at+jwt', fields: structuredToken.header, }); } - payload[PROPERTY] = value; + payload.format = 'jwt-ietf'; return [value, payload]; }, - async verify(token, stored, { ignoreExpiration, foundByReference }) { - let jti; - if (!foundByReference) { - assert.equal(token, stored[PROPERTY]); - jti = this.getTokenId(token); - } - return opaque.verify.call(this, jti, stored, { ignoreExpiration, foundByReference }); + async verify(stored, { ignoreExpiration } = {}) { + return opaque.verify.call(this, stored, { ignoreExpiration, format: 'jwt-ietf' }); }, }); diff --git a/lib/models/formats/opaque.js b/lib/models/formats/opaque.js index 64937c10f..1d6b282d8 100644 --- a/lib/models/formats/opaque.js +++ b/lib/models/formats/opaque.js @@ -1,3 +1,5 @@ +const { strict: assert } = require('assert'); + const pickBy = require('../../helpers/_/pick_by'); const { assertPayload } = require('../../helpers/jwt'); const epochTime = require('../../helpers/epoch_time'); @@ -18,6 +20,7 @@ module.exports = (provider) => ({ const exp = this.exp || now + this.expiration; const value = this.jti; const payload = { + format: 'opaque', iat: this.iat || epochTime(), ...(exp ? { exp } : undefined), ...pickBy( @@ -35,11 +38,11 @@ module.exports = (provider) => ({ getTokenId(token) { return token; }, - async verify(token, stored, { ignoreExpiration, foundByReference }) { + async verify(stored, { ignoreExpiration, format = 'opaque' } = {}) { + assert.equal(stored.format, format); assertPayload(stored, { ignoreExpiration, clockTolerance: instance(provider).configuration('clockTolerance'), - ...(foundByReference ? undefined : { jti: token }), }); return stored; diff --git a/lib/models/mixins/has_format.js b/lib/models/mixins/has_format.js index 21859d39a..4a55c5394 100644 --- a/lib/models/mixins/has_format.js +++ b/lib/models/mixins/has_format.js @@ -42,14 +42,6 @@ module.exports = (provider, type, superclass) => { instance(provider).dynamic[type] = FORMAT; } - const { IN_PAYLOAD } = klass.prototype.constructor; - Object.defineProperties(klass.prototype.constructor, { - format: { value: dynamic ? 'dynamic' : FORMAT }, - ...(FORMAT === 'jwt-ietf' ? { IN_PAYLOAD: { value: [...IN_PAYLOAD, 'jwt-ietf'] } } : undefined), - ...(FORMAT === 'jwt' ? { IN_PAYLOAD: { value: [...IN_PAYLOAD, 'jwt'] } } : undefined), - ...(dynamic ? { IN_PAYLOAD: { value: [...IN_PAYLOAD, 'format', 'jwt', 'jwt-ietf'] } } : undefined), - }); - return klass; } diff --git a/lib/models/replay_detection.js b/lib/models/replay_detection.js index eadee74ce..dab05df90 100644 --- a/lib/models/replay_detection.js +++ b/lib/models/replay_detection.js @@ -30,7 +30,7 @@ module.exports = (provider) => class ReplayDetection extends hasFormat(provider, return false; } - const inst = new this({ + const inst = this.instantiate({ jti: id, iss, }); diff --git a/lib/models/session.js b/lib/models/session.js index 617d0ec87..58f80cb04 100644 --- a/lib/models/session.js +++ b/lib/models/session.js @@ -83,8 +83,8 @@ module.exports = function getSession(provider) { const stored = await this.adapter.findByUid(uid); try { assert(stored); - const payload = await this.verify(undefined, stored, { foundByReference: true }); - return new this(payload); + const payload = await this.verify(stored); + return this.instantiate(payload); } catch (err) { return undefined; } @@ -113,9 +113,9 @@ module.exports = function getSession(provider) { // underlying session was removed since we have a session id in cookie, let's assign an // empty data so that session.new is not true and cookie will get written even if nothing // gets written to it - session = new this({}); + session = this.instantiate({}); } else { - session = new this(); + session = this.instantiate(); } } diff --git a/test/formats/jwt.test.js b/test/formats/jwt.test.js index 8e338544b..c057f598c 100644 --- a/test/formats/jwt.test.js +++ b/test/formats/jwt.test.js @@ -17,6 +17,7 @@ function decode(b64urljson) { if (FORMAT === 'jwt') { describe('jwt storage', () => { before(bootstrap(__dirname)); + const format = 'jwt'; const accountId = 'account'; const claims = {}; const clientId = 'client'; @@ -69,6 +70,7 @@ if (FORMAT === 'jwt') { const jwt = await token.save(); assert.calledWith(upsert, string, { + format, accountId, aud, claims, @@ -78,7 +80,6 @@ if (FORMAT === 'jwt') { gty, iat: number, jti: upsert.getCall(0).args[0], - jwt: string, kind, scope, sid, @@ -118,6 +119,7 @@ if (FORMAT === 'jwt') { const jwt = await token.save(); assert.calledWith(upsert, string, { + format, accountId, aud, claims, @@ -127,7 +129,6 @@ if (FORMAT === 'jwt') { gty, iat: number, jti: upsert.getCall(0).args[0], - jwt: string, kind, scope, sid, @@ -167,12 +168,12 @@ if (FORMAT === 'jwt') { const jwt = await token.save(); assert.calledWith(upsert, string, { + format, aud, clientId, exp: number, iat: number, jti: upsert.getCall(0).args[0], - jwt: string, kind, scope, 'x5t#S256': s256, diff --git a/test/formats/jwt_ietf.test.js b/test/formats/jwt_ietf.test.js index a445ed978..ab21f7110 100644 --- a/test/formats/jwt_ietf.test.js +++ b/test/formats/jwt_ietf.test.js @@ -17,6 +17,7 @@ function decode(b64urljson) { if (FORMAT === 'jwt-ietf') { describe('jwt-ietf storage', () => { before(bootstrap(__dirname)); + const format = 'jwt-ietf'; const accountId = 'account'; const claims = {}; const clientId = 'client'; @@ -69,6 +70,7 @@ if (FORMAT === 'jwt-ietf') { const jwt = await token.save(); assert.calledWith(upsert, string, { + format, accountId, aud, claims, @@ -78,7 +80,6 @@ if (FORMAT === 'jwt-ietf') { gty, iat: number, jti: upsert.getCall(0).args[0], - 'jwt-ietf': string, kind, scope, sid, @@ -118,6 +119,7 @@ if (FORMAT === 'jwt-ietf') { const jwt = await token.save(); assert.calledWith(upsert, string, { + format, accountId, aud, claims, @@ -127,7 +129,6 @@ if (FORMAT === 'jwt-ietf') { gty, iat: number, jti: upsert.getCall(0).args[0], - 'jwt-ietf': string, kind, scope, sid, @@ -167,12 +168,12 @@ if (FORMAT === 'jwt-ietf') { const jwt = await token.save(); assert.calledWith(upsert, string, { + format, aud, clientId, exp: number, iat: number, jti: upsert.getCall(0).args[0], - 'jwt-ietf': string, kind, scope, 'x5t#S256': s256, diff --git a/test/formats/opaque.test.js b/test/formats/opaque.test.js index 9cf4af16e..a5cd4b999 100644 --- a/test/formats/opaque.test.js +++ b/test/formats/opaque.test.js @@ -10,6 +10,7 @@ const { spy, match: { string, number }, assert } = sinon; if (FORMAT === 'opaque') { describe('opaque storage', () => { before(bootstrap(__dirname)); + const format = 'opaque'; const accountId = 'account'; const claims = {}; const clientId = 'client'; @@ -63,6 +64,7 @@ if (FORMAT === 'opaque') { expect(upsert.getCall(0).args[0]).to.have.lengthOf(43); assert.calledWith(upsert, string, { + format, accountId, aud, claims, @@ -92,6 +94,7 @@ if (FORMAT === 'opaque') { expect(upsert.getCall(0).args[0]).to.have.lengthOf(43); assert.calledWith(upsert, string, { + format, accountId, acr, amr, @@ -125,6 +128,7 @@ if (FORMAT === 'opaque') { expect(upsert.getCall(0).args[0]).to.have.lengthOf(43); assert.calledWith(upsert, string, { + format, accountId, acr, amr, @@ -164,6 +168,7 @@ if (FORMAT === 'opaque') { expect(upsert.getCall(0).args[0]).to.have.lengthOf(43); assert.calledWith(upsert, string, { + format, accountId, acr, amr, @@ -199,6 +204,7 @@ if (FORMAT === 'opaque') { expect(upsert.getCall(0).args[0]).to.have.lengthOf(43); assert.calledWith(upsert, string, { + format, aud, clientId, exp: number, @@ -223,6 +229,7 @@ if (FORMAT === 'opaque') { expect(upsert.getCall(0).args[0]).to.have.lengthOf(43); assert.calledWith(upsert, string, { + format, exp: number, iat: number, jti: upsert.getCall(0).args[0], @@ -244,6 +251,7 @@ if (FORMAT === 'opaque') { expect(upsert.getCall(0).args[0]).to.have.lengthOf(43); assert.calledWith(upsert, string, { + format, clientId, kind, policies,