Skip to content

Commit

Permalink
refactor: clean up RequestUriCache
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Removed built in support for urn: request uris.
BREAKING CHANGE: Renamed RequestUriCache.prototype.resolveWebUri to
RequestUriCache.prototype.resolve
  • Loading branch information
panva committed Sep 2, 2020
1 parent 9b60266 commit 8c0b9c5
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 107 deletions.
6 changes: 1 addition & 5 deletions lib/actions/authorization/fetch_request_uri.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 1 addition & 5 deletions lib/helpers/request_uri_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
99 changes: 4 additions & 95 deletions test/request/uri_request.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,7 @@ declare class Session extends BaseModel {
}

declare class RequestUriCache {
resolveUrn(urn: string): Promise<string>;
resolveWebUri(requestUri: string): Promise<string>;
resolve(requestUri: string): Promise<string>;
}

declare class JWTStructured {
Expand Down

0 comments on commit 8c0b9c5

Please sign in to comment.