Skip to content

Commit

Permalink
fix: proper createRemoteJWKSet timeoutDuration handling
Browse files Browse the repository at this point in the history
fixes #277
  • Loading branch information
panva committed Oct 6, 2021
1 parent a13eb04 commit ca710ea
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 23 deletions.
40 changes: 26 additions & 14 deletions src/runtime/browser/fetch_jwks.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,38 @@
import type { FetchFunction } from '../interfaces.d'
import { JOSEError } from '../../util/errors.js'
import { JOSEError, JWKSTimeout } from '../../util/errors.js'
import globalThis, { isCloudflareWorkers } from './global.js'

const fetchJwks: FetchFunction = async (url: URL, timeout: number) => {
let controller!: AbortController
let id!: ReturnType<typeof setTimeout>
let timedOut = false
if (typeof AbortController === 'function') {
controller = new AbortController()
setTimeout(() => controller.abort(), timeout)
id = setTimeout(() => {
timedOut = true
controller.abort()
}, timeout)
}

const response = await globalThis.fetch(url.href, {
signal: controller ? controller.signal : undefined,
redirect: 'manual',
method: 'GET',
...(!isCloudflareWorkers()
? {
referrerPolicy: 'no-referrer',
credentials: 'omit',
mode: 'cors',
}
: undefined),
})
const response = await globalThis
.fetch(url.href, {
signal: controller ? controller.signal : undefined,
redirect: 'manual',
method: 'GET',
...(!isCloudflareWorkers()
? {
referrerPolicy: 'no-referrer',
credentials: 'omit',
mode: 'cors',
}
: undefined),
})
.catch((err) => {
if (timedOut) throw new JWKSTimeout()
throw err
})

if (id !== undefined) clearTimeout(id)

if (response.status !== 200) {
throw new JOSEError('Expected 200 OK from the JSON Web Key Set HTTP response')
Expand Down
12 changes: 10 additions & 2 deletions src/runtime/node/fetch_jwks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { ClientRequest, IncomingMessage } from 'http'
import type { RequestOptions } from 'https'

import type { FetchFunction } from '../interfaces.d'
import { JOSEError } from '../../util/errors.js'
import { JOSEError, JWKSTimeout } from '../../util/errors.js'
import { concat, decoder } from '../../lib/buffer_utils.js'

const protocols: { [protocol: string]: (...args: Parameters<typeof https>) => ClientRequest } = {
Expand All @@ -30,7 +30,15 @@ const fetchJwks: FetchFunction = async (
timeout,
})

const [response] = <[IncomingMessage]>await once(req, 'response')
const [response] = <[IncomingMessage]>(
await Promise.race([once(req, 'response'), once(req, 'timeout')])
)

// timeout reached
if (!response) {
req.destroy()
throw new JWKSTimeout()
}

if (response.statusCode !== 200) {
throw new JOSEError('Expected 200 OK from the JSON Web Key Set HTTP response')
Expand Down
11 changes: 11 additions & 0 deletions src/util/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,17 @@ export class JWKSMultipleMatchingKeys extends JOSEError {
message = 'multiple matching keys found in the JSON Web Key Set'
}

/**
* Timeout was reached when retrieving the JWKS response.
*/
export class JWKSTimeout extends JOSEError {
static code = 'ERR_JWKS_TIMEOUT'

code = JWKSTimeout.code

message = 'request timed out'
}

/**
* An error subclass thrown when JWS signature verification fails.
*/
Expand Down
6 changes: 6 additions & 0 deletions test-browser/jwks.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,9 @@ QUnit.test('fetches the JWKSet', async (assert) => {
);
assert.ok(await jwks({ alg, kid }));
});

const conditional = typeof AbortController === 'function' ? QUnit.test : QUnit.skip;
conditional('timeout', async (assert) => {
const jwks = createRemoteJWKSet(new URL(jwksUri));
await assert.rejects(jwks({ alg: 'RS256' }, { timeoutDuration: 0 }), 'request timed out');
});
15 changes: 15 additions & 0 deletions test-cloudflare-workers/cloudflare.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,21 @@ test('createRemoteJWKSet', macro, async () => {
await jwks({ alg, kid });
});

test('remote jwk set timeout', macro, async () => {
const jwksUri = 'https://www.googleapis.com/oauth2/v3/certs';
const jwks = jwksRemote(new URL(jwksUri), { timeoutDuration: 0 });
await jwks({ alg: 'RS256' }).then(
() => {
throw new Error('should fail');
},
(err) => {
if (err.code !== 'ERR_JWKS_TIMEOUT') {
throw err;
}
},
);
});

test('ECDH-ES', macro, async () => {
const keypair = await utilGenerateKeyPair('ECDH-ES');
await jweAsymmetricTest(keypair, 'ECDH-ES');
Expand Down
11 changes: 10 additions & 1 deletion test-deno/jwks.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { assertThrowsAsync } from 'https://deno.land/[email protected]/testing/asserts.ts';

import createRemoteJWKSet from '../dist/deno/jwks/remote.ts';
import { JWKSNoMatchingKey, JWKSMultipleMatchingKeys } from '../dist/deno/util/errors.ts';
import {
JWKSTimeout,
JWKSNoMatchingKey,
JWKSMultipleMatchingKeys,
} from '../dist/deno/util/errors.ts';

const jwksUri = 'https://www.googleapis.com/oauth2/v3/certs';

Expand All @@ -21,3 +25,8 @@ Deno.test('fetches the JWKSet', async () => {
);
await jwks({ alg, kid }, <any>null);
});

Deno.test('timeout', async () => {
const jwks = createRemoteJWKSet(new URL(jwksUri), { timeoutDuration: 0 });
await assertThrowsAsync(() => jwks(<any>{}, <any>null), JWKSTimeout, 'request timed out');
});
29 changes: 23 additions & 6 deletions test/jwks/remote.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Promise.all([

test.before(async (t) => {
nock.disableNetConnect();
t.context.server = createServer().listen(3000);
t.context.server = createServer().unref().listen(3000);
t.context.server.removeAllListeners('request');
await once(t.context.server, 'listening');
});
Expand All @@ -42,6 +42,10 @@ Promise.all([
await new Promise((resolve) => t.context.server.close(resolve));
});

test.afterEach(() => {
nock.disableNetConnect();
});

test.afterEach((t) => {
t.context.server.removeAllListeners('request');
t.true(nock.isDone());
Expand Down Expand Up @@ -261,7 +265,7 @@ Promise.all([
});
});

test.serial('handles ENOTFOUND', async (t) => {
test('handles ENOTFOUND', async (t) => {
nock.enableNetConnect();
const url = new URL('https://op.example.com/jwks');
const JWKS = createRemoteJWKSet(url);
Expand All @@ -270,15 +274,17 @@ Promise.all([
});
});

test.serial('handles ECONNREFUSED', async (t) => {
test('handles ECONNREFUSED', async (t) => {
nock.enableNetConnect();
const url = new URL('http://localhost:3001/jwks');
const JWKS = createRemoteJWKSet(url);
await t.throwsAsync(JWKS({ alg: 'RS256' }), {
code: 'ECONNREFUSED',
});
});

test.serial('handles ECONNRESET', async (t) => {
test('handles ECONNRESET', async (t) => {
nock.enableNetConnect();
const url = new URL('http://localhost:3000/jwks');
t.context.server.once('connection', (socket) => {
socket.destroy();
Expand All @@ -288,10 +294,21 @@ Promise.all([
code: 'ECONNRESET',
});
});

test('handles a timeout', async (t) => {
t.timeout(1000);
nock.enableNetConnect();
const url = new URL('http://localhost:3000/jwks');
const JWKS = createRemoteJWKSet(url, {
timeoutDuration: 500,
});
await t.throwsAsync(JWKS({ alg: 'RS256' }), {
code: 'ERR_JWKS_TIMEOUT',
});
});
},
(err) => {
test.serial('failed to import', (t) => {
console.error(err);
test('failed to import', (t) => {
t.fail();
});
},
Expand Down

0 comments on commit ca710ea

Please sign in to comment.