From 2bca1272062748970d69add4c80b0ac895973212 Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Fri, 17 Jan 2020 15:36:12 +0000 Subject: [PATCH 01/10] Added retry logic to getJSON --- __tests__/utils.test.ts | 55 +++++++++++++++++++++++++++++++++++++++++ package-lock.json | 2 +- src/utils.ts | 30 +++++++++++++++++++--- 3 files changed, 83 insertions(+), 4 deletions(-) diff --git a/__tests__/utils.test.ts b/__tests__/utils.test.ts index dd1f88ef6..cad287c45 100644 --- a/__tests__/utils.test.ts +++ b/__tests__/utils.test.ts @@ -254,15 +254,18 @@ describe('utils', () => { expect(openPopup).toThrowError('Could not open popup'); }); }); + describe('oauthToken', () => { let oauthToken; let mockUnfetch; + beforeEach(() => { jest.resetModules(); jest.mock('unfetch'); mockUnfetch = require('unfetch'); oauthToken = require('../src/utils').oauthToken; }); + it('calls oauth/token with the correct url', async () => { mockUnfetch.mockReturnValue( new Promise(res => @@ -284,11 +287,13 @@ describe('utils', () => { method: 'POST' }); }); + it('handles error with error response', async () => { const theError = { error: 'the-error', error_description: 'the-error-description' }; + mockUnfetch.mockReturnValue( new Promise(res => res({ @@ -297,6 +302,7 @@ describe('utils', () => { }) ) ); + try { await oauthToken({ baseUrl: 'https://test.com', @@ -310,6 +316,7 @@ describe('utils', () => { expect(error.error_description).toBe(theError.error_description); } }); + it('handles error without error response', async () => { mockUnfetch.mockReturnValue( new Promise(res => @@ -319,6 +326,7 @@ describe('utils', () => { }) ) ); + try { await oauthToken({ baseUrl: 'https://test.com', @@ -336,7 +344,54 @@ describe('utils', () => { ); } }); + + it('retries the request in the event of a network failure', async () => { + // Fetch only fails in the case of a network issue, so should be + // retried here. Failure status (4xx, 5xx, etc) return a resolved Promise + // with the failure in the body. + // https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API + mockUnfetch.mockReturnValue(Promise.reject(new Error('Network failure'))); + + try { + await oauthToken({ + baseUrl: 'https://test.com', + client_id: 'client_idIn', + code: 'codeIn', + code_verifier: 'code_verifierIn' + }); + } catch (error) { + expect(error.message).toBe('Network failure'); + expect(mockUnfetch).toHaveBeenCalledTimes(5); + } + }); + + it('continues the program after failing a couple of times then succeeding', async () => { + // Fetch only fails in the case of a network issue, so should be + // retried here. Failure status (4xx, 5xx, etc) return a resolved Promise + // with the failure in the body. + // https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API + mockUnfetch + .mockReturnValueOnce(Promise.reject(new Error('Network failure'))) + .mockReturnValueOnce(Promise.reject(new Error('Network failure'))) + .mockReturnValue( + Promise.resolve({ + ok: true, + json: () => Promise.resolve({ access_token: 'access-token' }) + }) + ); + + const result = await oauthToken({ + baseUrl: 'https://test.com', + client_id: 'client_idIn', + code: 'codeIn', + code_verifier: 'code_verifierIn' + }); + + expect(result.access_token).toBe('access-token'); + expect(mockUnfetch).toHaveBeenCalledTimes(3); + }); }); + describe('runPopup', () => { const TIMEOUT_ERROR = { error: 'timeout', error_description: 'Timeout' }; const setup = customMessage => { diff --git a/package-lock.json b/package-lock.json index 17c3fa54b..9a3033409 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@auth0/auth0-spa-js", - "version": "1.6.0", + "version": "1.7.0-beta.2", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/src/utils.ts b/src/utils.ts index c655bd7d7..1ab7d841a 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -179,24 +179,47 @@ export const bufferToBase64UrlEncoded = input => { }; const getJSON = async (url, options) => { - const response = await fetch(url, options); + let fetchError, response; + + for (let i = 0; i < 5; i++) { + try { + response = await fetch(url, options); + fetchError = null; + break; + } catch (e) { + // Fetch only fails in the case of a network issue, so should be + // retried here. Failure status (4xx, 5xx, etc) return a resolved Promise + // with the failure in the body. + // https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API + fetchError = e; + } + } + + if (fetchError) { + throw fetchError; + } + const { error, error_description, ...success } = await response.json(); + if (!response.ok) { const errorMessage = error_description || `HTTP error. Unable to fetch ${url}`; const e = new Error(errorMessage); + e.error = error || 'request_error'; e.error_description = errorMessage; + throw e; } + return success; }; export const oauthToken = async ({ baseUrl, ...options -}: TokenEndpointOptions) => - await getJSON(`${baseUrl}/oauth/token`, { +}: TokenEndpointOptions) => { + return await getJSON(`${baseUrl}/oauth/token`, { method: 'POST', body: JSON.stringify({ redirect_uri: window.location.origin, @@ -206,6 +229,7 @@ export const oauthToken = async ({ 'Content-type': 'application/json' } }); +}; export const getCrypto = () => { //ie 11.x uses msCrypto From 3e31e26ef26187f0c5f701236ba720411f49b6ec Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Fri, 17 Jan 2020 17:00:10 +0000 Subject: [PATCH 02/10] Moved retry count to a constant --- __tests__/utils.test.ts | 11 +++++++++-- src/constants.ts | 5 +++++ src/utils.ts | 8 ++++++-- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/__tests__/utils.test.ts b/__tests__/utils.test.ts index cad287c45..658263ea8 100644 --- a/__tests__/utils.test.ts +++ b/__tests__/utils.test.ts @@ -16,7 +16,11 @@ import { getCryptoSubtle, validateCrypto } from '../src/utils'; -import { DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS } from '../src/constants'; + +import { + DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS, + DEFAULT_SILENT_TOKEN_RETRY_COUNT +} from '../src/constants'; (global).TextEncoder = TextEncoder; @@ -361,7 +365,10 @@ describe('utils', () => { }); } catch (error) { expect(error.message).toBe('Network failure'); - expect(mockUnfetch).toHaveBeenCalledTimes(5); + + expect(mockUnfetch).toHaveBeenCalledTimes( + DEFAULT_SILENT_TOKEN_RETRY_COUNT + ); } }); diff --git a/src/constants.ts b/src/constants.ts index 7d590b1e5..1f6d3019f 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -11,3 +11,8 @@ export const DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS = 60; export const DEFAULT_POPUP_CONFIG_OPTIONS: PopupConfigOptions = { timeoutInSeconds: DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS }; + +/** + * @ignore + */ +export const DEFAULT_SILENT_TOKEN_RETRY_COUNT = 5; diff --git a/src/utils.ts b/src/utils.ts index 1ab7d841a..265f80db6 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,12 +1,16 @@ import fetch from 'unfetch'; -import { DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS } from './constants'; import { AuthenticationResult, PopupConfigOptions, TokenEndpointOptions } from './global'; +import { + DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS, + DEFAULT_SILENT_TOKEN_RETRY_COUNT +} from './constants'; + const dedupe = arr => arr.filter((x, i) => arr.indexOf(x) === i); const TIMEOUT_ERROR = { error: 'timeout', error_description: 'Timeout' }; @@ -181,7 +185,7 @@ export const bufferToBase64UrlEncoded = input => { const getJSON = async (url, options) => { let fetchError, response; - for (let i = 0; i < 5; i++) { + for (let i = 0; i < DEFAULT_SILENT_TOKEN_RETRY_COUNT; i++) { try { response = await fetch(url, options); fetchError = null; From e196a80e97076ed6601e1e4091bea71e8185c754 Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Fri, 17 Jan 2020 17:05:22 +0000 Subject: [PATCH 03/10] Reverted changes to oauthToken --- src/utils.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index 265f80db6..35288313e 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -222,8 +222,8 @@ const getJSON = async (url, options) => { export const oauthToken = async ({ baseUrl, ...options -}: TokenEndpointOptions) => { - return await getJSON(`${baseUrl}/oauth/token`, { +}: TokenEndpointOptions) => + await getJSON(`${baseUrl}/oauth/token`, { method: 'POST', body: JSON.stringify({ redirect_uri: window.location.origin, @@ -233,7 +233,6 @@ export const oauthToken = async ({ 'Content-type': 'application/json' } }); -}; export const getCrypto = () => { //ie 11.x uses msCrypto From 7f3b67d800a6eca1a008e0018460edc966e230e6 Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Fri, 7 Feb 2020 12:36:52 +0000 Subject: [PATCH 04/10] Reduced retry count to 3 --- src/constants.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/constants.ts b/src/constants.ts index 1f6d3019f..f4cdf3dd4 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -15,4 +15,4 @@ export const DEFAULT_POPUP_CONFIG_OPTIONS: PopupConfigOptions = { /** * @ignore */ -export const DEFAULT_SILENT_TOKEN_RETRY_COUNT = 5; +export const DEFAULT_SILENT_TOKEN_RETRY_COUNT = 3; From 6cc0f57b7bfd307a28de4d7e3f4400f7b9f7c5b3 Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Fri, 7 Feb 2020 15:03:05 +0000 Subject: [PATCH 05/10] Implemented a timeout around the fetch call --- __tests__/index.test.ts | 11 +++++--- __tests__/utils.test.ts | 56 +++++++++++++++++++++++++++++++++++++++++ src/Auth0Client.ts | 11 +++++--- src/constants.ts | 5 ++++ src/global.ts | 1 + src/utils.ts | 21 +++++++++++++--- 6 files changed, 96 insertions(+), 9 deletions(-) diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts index d426d8d4f..13109fcd7 100644 --- a/__tests__/index.test.ts +++ b/__tests__/index.test.ts @@ -11,7 +11,10 @@ import createAuth0Client, { } from '../src/index'; import { AuthenticationError } from '../src/errors'; import version from '../src/version'; -import { DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS } from '../src/constants'; +import { + DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS, + DEFAULT_FETCH_TIMEOUT_MS +} from '../src/constants'; const GET_TOKEN_SILENTLY_LOCK_KEY = 'auth0.lock.getTokenSilently'; const TEST_DOMAIN = 'test.auth0.com'; @@ -1409,7 +1412,8 @@ describe('Auth0', () => { refresh_token: TEST_REFRESH_TOKEN, client_id: TEST_CLIENT_ID, grant_type: 'refresh_token', - redirect_uri: 'http://localhost' + redirect_uri: 'http://localhost', + timeout: DEFAULT_FETCH_TIMEOUT_MS }); expect(cache.save).toHaveBeenCalledWith({ @@ -1638,7 +1642,8 @@ describe('Auth0', () => { code: TEST_CODE, code_verifier: TEST_RANDOM_STRING, grant_type: 'authorization_code', - redirect_uri: 'http://localhost' + redirect_uri: 'http://localhost', + timeout: DEFAULT_FETCH_TIMEOUT_MS }); }); diff --git a/__tests__/utils.test.ts b/__tests__/utils.test.ts index 658263ea8..a65aeeea5 100644 --- a/__tests__/utils.test.ts +++ b/__tests__/utils.test.ts @@ -21,6 +21,7 @@ import { DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS, DEFAULT_SILENT_TOKEN_RETRY_COUNT } from '../src/constants'; +import { ExportConverter } from 'typedoc/dist/lib/converter/nodes'; (global).TextEncoder = TextEncoder; @@ -397,6 +398,61 @@ describe('utils', () => { expect(result.access_token).toBe('access-token'); expect(mockUnfetch).toHaveBeenCalledTimes(3); }); + + it('surfaces a timeout error when the fetch continuously times out', async () => { + const createPromise = () => + new Promise((resolve, _) => { + setTimeout( + () => + resolve({ + ok: true, + json: () => Promise.resolve({ access_token: 'access-token' }) + }), + 500 + ); + }); + + mockUnfetch.mockReturnValue(createPromise()); + + try { + await oauthToken({ + baseUrl: 'https://test.com', + client_id: 'client_idIn', + code: 'codeIn', + code_verifier: 'code_verifierIn', + timeout: 100 + }); + } catch (e) { + expect(e.message).toBe("Timeout when executing 'fetch'"); + expect(mockUnfetch).toHaveBeenCalledTimes(3); + } + }); + + it('retries the request in the event of a timeout', async () => { + const fetchResult = { + ok: true, + json: () => Promise.resolve({ access_token: 'access-token' }) + }; + + mockUnfetch.mockReturnValueOnce( + new Promise((resolve, _) => { + setTimeout(() => resolve(fetchResult), 1000); + }) + ); + + mockUnfetch.mockReturnValue(Promise.resolve(fetchResult)); + + const result = await oauthToken({ + baseUrl: 'https://test.com', + client_id: 'client_idIn', + code: 'codeIn', + code_verifier: 'code_verifierIn', + timeout: 500 + }); + + expect(result.access_token).toBe('access-token'); + expect(mockUnfetch).toHaveBeenCalledTimes(2); + }); }); describe('runPopup', () => { diff --git a/src/Auth0Client.ts b/src/Auth0Client.ts index 9c427fe96..0459cf554 100644 --- a/src/Auth0Client.ts +++ b/src/Auth0Client.ts @@ -19,7 +19,10 @@ import TransactionManager from './transaction-manager'; import { verify as verifyIdToken } from './jwt'; import { AuthenticationError, GenericError } from './errors'; import * as ClientStorage from './storage'; -import { DEFAULT_POPUP_CONFIG_OPTIONS } from './constants'; +import { + DEFAULT_POPUP_CONFIG_OPTIONS, + DEFAULT_FETCH_TIMEOUT_MS +} from './constants'; import version from './version'; import { Auth0ClientOptions, @@ -581,7 +584,8 @@ export default class Auth0Client { code_verifier, code: codeResult.code, grant_type: 'authorization_code', - redirect_uri: params.redirect_uri + redirect_uri: params.redirect_uri, + timeout: DEFAULT_FETCH_TIMEOUT_MS } as OAuthTokenOptions); const decodedToken = this._verifyIdToken(tokenResult.id_token, nonceIn); @@ -626,7 +630,8 @@ export default class Auth0Client { client_id: this.options.client_id, grant_type: 'refresh_token', refresh_token: cache.refresh_token, - redirect_uri + redirect_uri, + timeout: DEFAULT_FETCH_TIMEOUT_MS } as RefreshTokenOptions); const decodedToken = this._verifyIdToken(tokenResult.id_token); diff --git a/src/constants.ts b/src/constants.ts index f4cdf3dd4..1aeb4ba52 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -16,3 +16,8 @@ export const DEFAULT_POPUP_CONFIG_OPTIONS: PopupConfigOptions = { * @ignore */ export const DEFAULT_SILENT_TOKEN_RETRY_COUNT = 3; + +/** + * @ignore + */ +export const DEFAULT_FETCH_TIMEOUT_MS = 10000; diff --git a/src/global.ts b/src/global.ts index 53a90c2b0..3930adc22 100644 --- a/src/global.ts +++ b/src/global.ts @@ -261,6 +261,7 @@ export interface TokenEndpointOptions { baseUrl: string; client_id: string; grant_type: string; + timeout: number; } /** diff --git a/src/utils.ts b/src/utils.ts index 35288313e..805211545 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -14,6 +14,7 @@ import { const dedupe = arr => arr.filter((x, i) => arr.indexOf(x) === i); const TIMEOUT_ERROR = { error: 'timeout', error_description: 'Timeout' }; + export const getUniqueScopes = (...scopes: string[]) => { const scopeString = scopes.filter(Boolean).join(); return dedupe(scopeString.replace(/\s/g, ',').split(',')) @@ -182,12 +183,25 @@ export const bufferToBase64UrlEncoded = input => { ); }; -const getJSON = async (url, options) => { +const fetchWithTimeout = (url, options, timeout) => { + // The promise will resolve with one of these two promises (the fetch and the timeout), whichever completes first. + return Promise.race([ + fetch(url, options), + new Promise((_, reject) => { + setTimeout( + () => reject(new Error("Timeout when executing 'fetch'")), + timeout + ); + }) + ]); +}; + +const getJSON = async (url, timeout, options) => { let fetchError, response; for (let i = 0; i < DEFAULT_SILENT_TOKEN_RETRY_COUNT; i++) { try { - response = await fetch(url, options); + response = await fetchWithTimeout(url, options, timeout); fetchError = null; break; } catch (e) { @@ -221,9 +235,10 @@ const getJSON = async (url, options) => { export const oauthToken = async ({ baseUrl, + timeout, ...options }: TokenEndpointOptions) => - await getJSON(`${baseUrl}/oauth/token`, { + await getJSON(`${baseUrl}/oauth/token`, timeout, { method: 'POST', body: JSON.stringify({ redirect_uri: window.location.origin, From 021445fbe5c86296e7fcc4bc555e6859591b9820 Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Fri, 7 Feb 2020 19:24:36 +0000 Subject: [PATCH 06/10] Made the fetch timeout a default value and adjusted tests --- __tests__/index.test.ts | 6 ++---- src/Auth0Client.ts | 6 ++---- src/global.ts | 2 +- src/utils.ts | 5 +++-- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts index 13109fcd7..88df338fd 100644 --- a/__tests__/index.test.ts +++ b/__tests__/index.test.ts @@ -1412,8 +1412,7 @@ describe('Auth0', () => { refresh_token: TEST_REFRESH_TOKEN, client_id: TEST_CLIENT_ID, grant_type: 'refresh_token', - redirect_uri: 'http://localhost', - timeout: DEFAULT_FETCH_TIMEOUT_MS + redirect_uri: 'http://localhost' }); expect(cache.save).toHaveBeenCalledWith({ @@ -1642,8 +1641,7 @@ describe('Auth0', () => { code: TEST_CODE, code_verifier: TEST_RANDOM_STRING, grant_type: 'authorization_code', - redirect_uri: 'http://localhost', - timeout: DEFAULT_FETCH_TIMEOUT_MS + redirect_uri: 'http://localhost' }); }); diff --git a/src/Auth0Client.ts b/src/Auth0Client.ts index 0459cf554..8cb0cd826 100644 --- a/src/Auth0Client.ts +++ b/src/Auth0Client.ts @@ -584,8 +584,7 @@ export default class Auth0Client { code_verifier, code: codeResult.code, grant_type: 'authorization_code', - redirect_uri: params.redirect_uri, - timeout: DEFAULT_FETCH_TIMEOUT_MS + redirect_uri: params.redirect_uri } as OAuthTokenOptions); const decodedToken = this._verifyIdToken(tokenResult.id_token, nonceIn); @@ -630,8 +629,7 @@ export default class Auth0Client { client_id: this.options.client_id, grant_type: 'refresh_token', refresh_token: cache.refresh_token, - redirect_uri, - timeout: DEFAULT_FETCH_TIMEOUT_MS + redirect_uri } as RefreshTokenOptions); const decodedToken = this._verifyIdToken(tokenResult.id_token); diff --git a/src/global.ts b/src/global.ts index 3930adc22..88cf4ed1f 100644 --- a/src/global.ts +++ b/src/global.ts @@ -261,7 +261,7 @@ export interface TokenEndpointOptions { baseUrl: string; client_id: string; grant_type: string; - timeout: number; + timeout?: number; } /** diff --git a/src/utils.ts b/src/utils.ts index 805211545..8cc40b99b 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -8,7 +8,8 @@ import { import { DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS, - DEFAULT_SILENT_TOKEN_RETRY_COUNT + DEFAULT_SILENT_TOKEN_RETRY_COUNT, + DEFAULT_FETCH_TIMEOUT_MS } from './constants'; const dedupe = arr => arr.filter((x, i) => arr.indexOf(x) === i); @@ -183,7 +184,7 @@ export const bufferToBase64UrlEncoded = input => { ); }; -const fetchWithTimeout = (url, options, timeout) => { +const fetchWithTimeout = (url, options, timeout = DEFAULT_FETCH_TIMEOUT_MS) => { // The promise will resolve with one of these two promises (the fetch and the timeout), whichever completes first. return Promise.race([ fetch(url, options), From 178850becebeaf997b2f746d4ce00b69f611212a Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Tue, 11 Feb 2020 11:50:19 +0000 Subject: [PATCH 07/10] Fixed broken test after merge --- __tests__/index.test.ts | 5 +++-- src/Auth0Client.ts | 8 +++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts index c4f880ca9..6ab19da26 100644 --- a/__tests__/index.test.ts +++ b/__tests__/index.test.ts @@ -9,6 +9,7 @@ import createAuth0Client, { PopupConfigOptions, GetTokenSilentlyOptions } from '../src/index'; + import { AuthenticationError } from '../src/errors'; import version from '../src/version'; @@ -43,7 +44,7 @@ const TEST_TELEMETRY_QUERY_STRING = `&auth0Client=${encodeURIComponent( ) )}`; -const DEFAULT_POPUP_CONFIG_OPTIONS: PopupConfigOptions = {}; +import { DEFAULT_POPUP_CONFIG_OPTIONS } from '../src/constants'; const mockEnclosedCache = { get: jest.fn(), @@ -286,7 +287,7 @@ describe('Auth0', () => { const { auth0, utils } = await setup({ authorizeTimeoutInSeconds: 1 }); const popup = {}; utils.openPopup.mockReturnValue(popup); - await auth0.loginWithPopup({}, DEFAULT_POPUP_CONFIG_OPTIONS); + await auth0.loginWithPopup({}); expect(utils.runPopup).toHaveBeenCalledWith( popup, `https://test.auth0.com/authorize?${TEST_QUERY_PARAMS}${TEST_TELEMETRY_QUERY_STRING}`, diff --git a/src/Auth0Client.ts b/src/Auth0Client.ts index d1c1d83ef..acf97789b 100644 --- a/src/Auth0Client.ts +++ b/src/Auth0Client.ts @@ -21,7 +21,7 @@ import { AuthenticationError, GenericError } from './errors'; import * as ClientStorage from './storage'; import { DEFAULT_POPUP_CONFIG_OPTIONS, - DEFAULT_FETCH_TIMEOUT_MS + DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS } from './constants'; import version from './version'; import { @@ -224,7 +224,7 @@ export default class Auth0Client { */ public async loginWithPopup( options: PopupLoginOptions = {}, - config: PopupConfigOptions = DEFAULT_POPUP_CONFIG_OPTIONS + config: PopupConfigOptions = {} ) { const popup = await openPopup(); const { ...authorizeOptions } = options; @@ -250,7 +250,9 @@ export default class Auth0Client { const codeResult = await runPopup(popup, url, { ...config, timeoutInSeconds: - config.timeoutInSeconds || this.options.authorizeTimeoutInSeconds + config.timeoutInSeconds || + this.options.authorizeTimeoutInSeconds || + DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS }); if (stateIn !== codeResult.state) { From df1ba301e95be7e00956b652a8dbcc94a15c7283 Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Thu, 13 Feb 2020 18:36:33 +0000 Subject: [PATCH 08/10] Implemented AbortController to abort fetch on timeout --- __tests__/utils.test.ts | 19 ++++++++++++------- src/utils.ts | 18 +++++++++++++----- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/__tests__/utils.test.ts b/__tests__/utils.test.ts index 395c71525..91587e608 100644 --- a/__tests__/utils.test.ts +++ b/__tests__/utils.test.ts @@ -21,7 +21,6 @@ import { DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS, DEFAULT_SILENT_TOKEN_RETRY_COUNT } from '../src/constants'; -import { ExportConverter } from 'typedoc/dist/lib/converter/nodes'; (global).TextEncoder = TextEncoder; @@ -285,12 +284,18 @@ describe('utils', () => { code_verifier: 'code_verifierIn' }); - expect(mockUnfetch).toHaveBeenCalledWith('https://test.com/oauth/token', { - body: - '{"redirect_uri":"http://localhost","grant_type":"authorization_code","client_id":"client_idIn","code":"codeIn","code_verifier":"code_verifierIn"}', - headers: { 'Content-type': 'application/json' }, - method: 'POST' - }); + expect(mockUnfetch.mock.calls[0][0]).toBe('https://test.com/oauth/token'); + + expect(mockUnfetch.mock.calls[0][1]).toEqual( + expect.objectContaining({ + body: + '{"redirect_uri":"http://localhost","grant_type":"authorization_code","client_id":"client_idIn","code":"codeIn","code_verifier":"code_verifierIn"}', + headers: { 'Content-type': 'application/json' }, + method: 'POST' + }) + ); + + expect(mockUnfetch.mock.calls[0][1].signal).not.toBeUndefined(); }); it('handles error with error response', async () => { diff --git a/src/utils.ts b/src/utils.ts index 97e132c00..74a5be126 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -189,14 +189,22 @@ export const bufferToBase64UrlEncoded = input => { }; const fetchWithTimeout = (url, options, timeout = DEFAULT_FETCH_TIMEOUT_MS) => { + const controller = new AbortController(); + const signal = controller.signal; + + const fetchOptions = { + ...options, + signal + }; + // The promise will resolve with one of these two promises (the fetch and the timeout), whichever completes first. return Promise.race([ - fetch(url, options), + fetch(url, fetchOptions), new Promise((_, reject) => { - setTimeout( - () => reject(new Error("Timeout when executing 'fetch'")), - timeout - ); + setTimeout(() => { + controller.abort(); + reject(new Error("Timeout when executing 'fetch'")); + }, timeout); }) ]); }; From 6cc044e42501f3fc7af385bfa7157b71292d1e3f Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Thu, 13 Feb 2020 18:42:06 +0000 Subject: [PATCH 09/10] Added abortcontroller polyfill --- package-lock.json | 5 +++++ package.json | 1 + src/index.ts | 1 + 3 files changed, 7 insertions(+) diff --git a/package-lock.json b/package-lock.json index 9a3033409..778d5cf97 100644 --- a/package-lock.json +++ b/package-lock.json @@ -936,6 +936,11 @@ "integrity": "sha512-sY5AXXVZv4Y1VACTtR11UJCPHHudgY5i26Qj5TypE6DKlIApbwb5uqhXcJ5UUGbvZNRh7EeIoW+LrJumBsKp7w==", "dev": true }, + "abortcontroller-polyfill": { + "version": "1.4.0", + "resolved": "https://registry.npmjs.org/abortcontroller-polyfill/-/abortcontroller-polyfill-1.4.0.tgz", + "integrity": "sha512-3ZFfCRfDzx3GFjO6RAkYx81lPGpUS20ISxux9gLxuKnqafNcFQo59+IoZqpO2WvQlyc287B62HDnDdNYRmlvWA==" + }, "accepts": { "version": "1.3.7", "resolved": "https://registry.npmjs.org/accepts/-/accepts-1.3.7.tgz", diff --git a/package.json b/package.json index 704eddb7e..f14f7f12f 100644 --- a/package.json +++ b/package.json @@ -71,6 +71,7 @@ "wait-on": "^3.3.0" }, "dependencies": { + "abortcontroller-polyfill": "^1.4.0", "browser-tabs-lock": "^1.2.1", "core-js": "^3.2.1", "es-cookie": "^1.2.0", diff --git a/src/index.ts b/src/index.ts index c424b7eda..0d82cf68d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -5,6 +5,7 @@ import 'core-js/es/array/includes'; import 'core-js/es/string/includes'; import 'promise-polyfill/src/polyfill'; import 'fast-text-encoding'; +import 'abortcontroller-polyfill/dist/abortcontroller-polyfill-only'; // @ts-ignore import Auth0Client from './Auth0Client'; From db1ff54a64d0a178ea37d1be04fbe6d55d92753c Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Fri, 14 Feb 2020 16:17:35 +0000 Subject: [PATCH 10/10] Created factory function for AbortController to be mocked and tested --- __tests__/utils.test.ts | 31 ++++++++++++++++++++----------- src/utils.ts | 4 +++- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/__tests__/utils.test.ts b/__tests__/utils.test.ts index 91587e608..e1a0ea001 100644 --- a/__tests__/utils.test.ts +++ b/__tests__/utils.test.ts @@ -262,12 +262,20 @@ describe('utils', () => { describe('oauthToken', () => { let oauthToken; let mockUnfetch; + let abortController; beforeEach(() => { jest.resetModules(); jest.mock('unfetch'); mockUnfetch = require('unfetch'); - oauthToken = require('../src/utils').oauthToken; + + const utils = require('../src/utils'); + oauthToken = utils.oauthToken; + + // Set up an AbortController that we can test has been called in the event of a timeout + abortController = new AbortController(); + jest.spyOn(abortController, 'abort'); + utils.createAbortController = jest.fn(() => abortController); }); it('calls oauth/token with the correct url', async () => { @@ -276,6 +284,7 @@ describe('utils', () => { res({ ok: true, json: () => new Promise(ress => ress(true)) }) ) ); + await oauthToken({ grant_type: 'authorization_code', baseUrl: 'https://test.com', @@ -284,16 +293,13 @@ describe('utils', () => { code_verifier: 'code_verifierIn' }); - expect(mockUnfetch.mock.calls[0][0]).toBe('https://test.com/oauth/token'); - - expect(mockUnfetch.mock.calls[0][1]).toEqual( - expect.objectContaining({ - body: - '{"redirect_uri":"http://localhost","grant_type":"authorization_code","client_id":"client_idIn","code":"codeIn","code_verifier":"code_verifierIn"}', - headers: { 'Content-type': 'application/json' }, - method: 'POST' - }) - ); + expect(mockUnfetch).toBeCalledWith('https://test.com/oauth/token', { + body: + '{"redirect_uri":"http://localhost","grant_type":"authorization_code","client_id":"client_idIn","code":"codeIn","code_verifier":"code_verifierIn"}', + headers: { 'Content-type': 'application/json' }, + method: 'POST', + signal: abortController.signal + }); expect(mockUnfetch.mock.calls[0][1].signal).not.toBeUndefined(); }); @@ -402,6 +408,7 @@ describe('utils', () => { expect(result.access_token).toBe('access-token'); expect(mockUnfetch).toHaveBeenCalledTimes(3); + expect(abortController.abort).not.toHaveBeenCalled(); }); it('surfaces a timeout error when the fetch continuously times out', async () => { @@ -430,6 +437,7 @@ describe('utils', () => { } catch (e) { expect(e.message).toBe("Timeout when executing 'fetch'"); expect(mockUnfetch).toHaveBeenCalledTimes(3); + expect(abortController.abort).toHaveBeenCalledTimes(3); } }); @@ -457,6 +465,7 @@ describe('utils', () => { expect(result.access_token).toBe('access-token'); expect(mockUnfetch).toHaveBeenCalledTimes(2); + expect(abortController.abort).toHaveBeenCalled(); }); }); diff --git a/src/utils.ts b/src/utils.ts index 74a5be126..d44094c2a 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -16,6 +16,8 @@ const dedupe = arr => arr.filter((x, i) => arr.indexOf(x) === i); const TIMEOUT_ERROR = { error: 'timeout', error_description: 'Timeout' }; +export const createAbortController = () => new AbortController(); + export const getUniqueScopes = (...scopes: string[]) => { const scopeString = scopes.filter(Boolean).join(); return dedupe(scopeString.replace(/\s/g, ',').split(',')) @@ -189,7 +191,7 @@ export const bufferToBase64UrlEncoded = input => { }; const fetchWithTimeout = (url, options, timeout = DEFAULT_FETCH_TIMEOUT_MS) => { - const controller = new AbortController(); + const controller = createAbortController(); const signal = controller.signal; const fetchOptions = {