From 8c0b9c509863b1e2d1882575a8c41b0187e2f269 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Wed, 2 Sep 2020 12:52:50 +0200 Subject: [PATCH] refactor: clean up RequestUriCache BREAKING CHANGE: Removed built in support for urn: request uris. BREAKING CHANGE: Renamed RequestUriCache.prototype.resolveWebUri to RequestUriCache.prototype.resolve --- .../authorization/fetch_request_uri.js | 6 +- lib/helpers/request_uri_cache.js | 6 +- test/request/uri_request.test.js | 99 +------------------ types/index.d.ts | 3 +- 4 files changed, 7 insertions(+), 107 deletions(-) diff --git a/lib/actions/authorization/fetch_request_uri.js b/lib/actions/authorization/fetch_request_uri.js index 7f02d66e7..b3bf1b4a5 100644 --- a/lib/actions/authorization/fetch_request_uri.js +++ b/lib/actions/authorization/fetch_request_uri.js @@ -59,11 +59,7 @@ module.exports = async function fetchRequestUri(ctx, next) { params.request = loadedRequestObject.request; } else { const cache = instance(ctx.oidc.provider).requestUriCache; - if (protocol === 'urn:') { - params.request = await cache.resolveUrn(params.request_uri); - } else { - params.request = await cache.resolveWebUri(params.request_uri); - } + params.request = await cache.resolve(params.request_uri); } assert(params.request); params.request_uri = undefined; diff --git a/lib/helpers/request_uri_cache.js b/lib/helpers/request_uri_cache.js index b9eb5dde8..b58626018 100644 --- a/lib/helpers/request_uri_cache.js +++ b/lib/helpers/request_uri_cache.js @@ -12,11 +12,7 @@ class RequestUriCache { this.provider = provider; } - async resolveUrn(requestUri) { // eslint-disable-line - throw new Error('resolving request_uri by URN is not implemented'); - } - - async resolveWebUri(requestUri) { + async resolve(requestUri) { const { cache } = this; const cacheKey = crypto.createHash('sha256').update(requestUri).digest('hex'); const cached = cache.get(cacheKey); diff --git a/test/request/uri_request.test.js b/test/request/uri_request.test.js index 3d8ad7e3b..8500f70e1 100644 --- a/test/request/uri_request.test.js +++ b/test/request/uri_request.test.js @@ -206,97 +206,6 @@ describe('request Uri features', () => { }); }); - describe('urn support', () => { - afterEach(sinon.restore); - - it('urn cannot be registered and will fail on client#requestUriAllowed()', function () { - const spy = sinon.spy(); - this.provider.once(error, spy); - sinon.spy(this.provider.Client.prototype, 'requestUriAllowed'); - - return this.wrap({ - agent: this.agent, - route, - verb, - auth: { - request_uri: 'urn:example', - scope: 'openid', - client_id: 'client', - response_type: 'code', - }, - }) - .expect(errorCode) - .expect(() => { - expect(this.provider.Client.prototype.requestUriAllowed.calledOnce).to.be.true; - expect(spy.calledOnce).to.be.true; - expect(spy.args[0][1]).to.have.property('message', 'invalid_request_uri'); - expect(spy.args[0][1]).to.have.property( - 'error_description', - 'provided request_uri is not whitelisted', - ); - }); - }); - - it('but this is overloadable public API', function () { - const spy = sinon.spy(); - this.provider.once(error, spy); - sinon.stub(this.provider.Client.prototype, 'requestUriAllowed').returns(true); - - return this.wrap({ - agent: this.agent, - route, - verb, - auth: { - request_uri: 'urn:example', - scope: 'openid', - client_id: 'client', - response_type: 'code', - }, - }) - .expect(errorCode) - .expect(() => { - expect(this.provider.Client.prototype.requestUriAllowed.calledOnce).to.be.true; - expect(spy.calledOnce).to.be.true; - expect(spy.args[0][1]).to.have.property('message', 'invalid_request_uri'); - expect(spy.args[0][1]).to.have.property( - 'error_description', - 'could not load or parse request_uri (resolving request_uri by URN is not implemented)', - ); - }); - }); - - it('so is the request uri cache', async function () { - const spy = sinon.spy(); - this.provider.once(error, spy); - sinon.stub(this.provider.Client.prototype, 'requestUriAllowed').returns(true); - - const request = await JWT.sign({ - client_id: 'client', - response_type: 'code', - redirect_uri: 'https://client.example.com/cb', - }, null, 'none', { issuer: 'client', audience: this.provider.issuer }); - - sinon.stub(this.provider.requestUriCache, 'resolveUrn').returns(request); - - return this.wrap({ - agent: this.agent, - route, - verb, - auth: { - request_uri: 'urn:example', - scope: 'openid', - client_id: 'client', - response_type: 'code', - }, - }) - .expect(successCode) - .expect(successFnCheck) - .expect(() => { - expect(this.provider.requestUriCache.resolveUrn.calledOnce).to.be.true; - }); - }); - }); - context('when client has requestUris set', () => { before(async function () { (await this.provider.Client.find('client')).requestUris = ['https://thisoneisallowed.com']; @@ -728,10 +637,10 @@ describe('request Uri features', () => { .get('/cachedRequest') .reply(200, 'content2'); - const first = await cache.resolveWebUri('https://client.example.com/cachedRequest#1'); - const second = await cache.resolveWebUri('https://client.example.com/cachedRequest#1'); - const third = await cache.resolveWebUri('https://client.example.com/cachedRequest#2'); - const fourth = await cache.resolveWebUri('https://client.example.com/cachedRequest#2'); + const first = await cache.resolve('https://client.example.com/cachedRequest#1'); + const second = await cache.resolve('https://client.example.com/cachedRequest#1'); + const third = await cache.resolve('https://client.example.com/cachedRequest#2'); + const fourth = await cache.resolve('https://client.example.com/cachedRequest#2'); expect(first).to.equal(second); expect(first).not.to.equal(third); diff --git a/types/index.d.ts b/types/index.d.ts index b49a57b40..75a7e532b 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -218,8 +218,7 @@ declare class Session extends BaseModel { } declare class RequestUriCache { - resolveUrn(urn: string): Promise; - resolveWebUri(requestUri: string): Promise; + resolve(requestUri: string): Promise; } declare class JWTStructured {