From 3edc4edf09de27ed9cacec3f28143a148ef6bbac Mon Sep 17 00:00:00 2001 From: Luis Deschamps Rudge Date: Fri, 26 Jul 2019 17:19:30 -0300 Subject: [PATCH 1/8] wip --- src/Auth0Client.ts | 2 +- src/utils.ts | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/Auth0Client.ts b/src/Auth0Client.ts index 50ac03e6a..3f2bc9c2b 100644 --- a/src/Auth0Client.ts +++ b/src/Auth0Client.ts @@ -327,7 +327,7 @@ export default class Auth0Client { response_mode: 'web_message' }); - const codeResult = await runIframe(url, this.domainUrl); + const codeResult = await runIframe(url, stateIn, this.domainUrl); if (stateIn !== codeResult.state) { throw new Error('Invalid state'); } diff --git a/src/utils.ts b/src/utils.ts index 5a51f97c1..67752e1fb 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -19,12 +19,17 @@ export const parseQueryResult = (hash: string) => { }; }; -export const runIframe = (authorizeUrl: string, eventOrigin: string) => { +export const runIframe = ( + authorizeUrl: string, + state: string, + eventOrigin: string +) => { return new Promise((res, rej) => { var iframe = window.document.createElement('iframe'); iframe.setAttribute('width', '0'); iframe.setAttribute('height', '0'); iframe.style.display = 'none'; + iframe.id = state; const timeoutSetTimeoutId = setTimeout(() => { rej(TIMEOUT_ERROR); @@ -34,11 +39,15 @@ export const runIframe = (authorizeUrl: string, eventOrigin: string) => { const iframeEventHandler = function(e: MessageEvent) { if (e.origin != eventOrigin) return; if (!e.data || e.data.type !== 'authorization_response') return; - (e.source).close(); + if (e.data.response.state !== state) return; + e.data.response.error ? rej(e.data.response) : res(e.data.response); clearTimeout(timeoutSetTimeoutId); window.removeEventListener('message', iframeEventHandler, false); - window.document.body.removeChild(iframe); + + try { + window.document.body.removeChild(document.getElementById(state)); + } catch (error) {} }; window.addEventListener('message', iframeEventHandler, false); window.document.body.appendChild(iframe); From 52b0e99bad412ae774b70794df50d3593f08ac21 Mon Sep 17 00:00:00 2001 From: Luis Deschamps Rudge Date: Tue, 30 Jul 2019 18:35:14 -0300 Subject: [PATCH 2/8] Add unique ids to iframes --- __tests__/index.test.ts | 1 + __tests__/utils.test.ts | 61 +++++++++++++++++++++++++++++++++-------- src/utils.ts | 8 +++--- 3 files changed, 54 insertions(+), 16 deletions(-) diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts index c25e5029e..c04377f38 100644 --- a/__tests__/index.test.ts +++ b/__tests__/index.test.ts @@ -870,6 +870,7 @@ describe('Auth0', () => { await auth0.getTokenSilently(defaultOptionsIgnoreCacheTrue); expect(utils.runIframe).toHaveBeenCalledWith( `https://test.auth0.com/authorize?${TEST_QUERY_PARAMS}${TEST_TELEMETRY_QUERY_STRING}`, + TEST_ENCODED_STATE, 'https://test.auth0.com' ); }); diff --git a/__tests__/utils.test.ts b/__tests__/utils.test.ts index cbfde7e85..376314235 100644 --- a/__tests__/utils.test.ts +++ b/__tests__/utils.test.ts @@ -347,7 +347,8 @@ describe('utils', () => { const setup = customMessage => { const iframe = { setAttribute: jest.fn(), - style: { display: '' } + style: { display: '' }, + id: '' }; const url = 'https://authorize.com'; const origin = @@ -361,24 +362,27 @@ describe('utils', () => { expect(type).toBe('iframe'); return iframe; }); + window.document.getElementById = jest.fn(() => { + return iframe; + }); window.document.body.appendChild = jest.fn(); window.document.body.removeChild = jest.fn(); return { iframe, url, origin }; }; it('handles iframe correctly', async () => { + const state = 'state'; const origin = 'https://origin.com'; const message = { origin, source: { close: jest.fn() }, data: { type: 'authorization_response', - response: { id_token: 'id_token' } + response: { id_token: 'id_token', state } } }; const { iframe, url } = setup(message); - await runIframe(url, origin); + await runIframe(url, state, origin); - expect(message.source.close).toHaveBeenCalled(); expect(window.document.body.appendChild).toHaveBeenCalledWith(iframe); expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe); expect(iframe.setAttribute.mock.calls).toMatchObject([ @@ -387,6 +391,7 @@ describe('utils', () => { ['src', url] ]); expect(iframe.style.display).toBe('none'); + expect(iframe.id).toBe(state); }); describe('with invalid messages', () => { [ @@ -408,7 +413,7 @@ describe('utils', () => { jest.runAllTimers(); }, 10); jest.useFakeTimers(); - await expect(runIframe(url, origin)).rejects.toMatchObject( + await expect(runIframe(url, 'id', origin)).rejects.toMatchObject( TIMEOUT_ERROR ); jest.useRealTimers(); @@ -417,39 +422,69 @@ describe('utils', () => { }); }); it('returns authorization response message', async () => { + const state = 'state'; const origin = 'https://origin.com'; const message = { origin, source: { close: jest.fn() }, data: { type: 'authorization_response', - response: { id_token: 'id_token' } + response: { id_token: 'id_token', state } } }; const { iframe, url } = setup(message); - await expect(runIframe(url, origin)).resolves.toMatchObject( + await expect(runIframe(url, state, origin)).resolves.toMatchObject( message.data.response ); - expect(message.source.close).toHaveBeenCalled(); expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe); }); it('returns authorization error message', async () => { + const state = 'state'; const origin = 'https://origin.com'; const message = { origin, source: { close: jest.fn() }, data: { type: 'authorization_response', - response: { error: 'error' } + response: { error: 'error', state } } }; const { iframe, url } = setup(message); - await expect(runIframe(url, origin)).rejects.toMatchObject( + await expect(runIframe(url, state, origin)).rejects.toMatchObject( message.data.response ); - expect(message.source.close).toHaveBeenCalled(); expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe); }); + //in this test, it will timeout since we're not sending another message later + it('does nothing if the message.state is not the same as the input.state', async () => { + const inputState = 'state'; + const origin = 'https://origin.com'; + const message = { + origin, + source: { close: jest.fn() }, + data: { + type: 'authorization_response', + response: { id_token: 'id_token', state: 'message-state' } + } + }; + const { iframe, url } = setup(message); + + /** + * We need to run the timers after we start `runIframe` to simulate + * the window event listener, but we also need to use `jest.useFakeTimers` + * to trigger the timeout. That's why we're using a real `setTimeout`, + * then using fake timers then rolling back to real timers + */ + setTimeout(() => { + jest.runAllTimers(); + }, 10); + jest.useFakeTimers(); + await expect(runIframe(url, inputState, origin)).rejects.toMatchObject( + TIMEOUT_ERROR + ); + expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe); + jest.useRealTimers(); + }); it('times out after 60s', async () => { const { iframe, url, origin } = setup(''); /** @@ -462,7 +497,9 @@ describe('utils', () => { jest.runAllTimers(); }, 10); jest.useFakeTimers(); - await expect(runIframe(url, origin)).rejects.toMatchObject(TIMEOUT_ERROR); + await expect(runIframe(url, 'id', origin)).rejects.toMatchObject( + TIMEOUT_ERROR + ); expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe); jest.useRealTimers(); }); diff --git a/src/utils.ts b/src/utils.ts index 67752e1fb..7287b370c 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -21,7 +21,7 @@ export const parseQueryResult = (hash: string) => { export const runIframe = ( authorizeUrl: string, - state: string, + stateIn: string, eventOrigin: string ) => { return new Promise((res, rej) => { @@ -29,7 +29,7 @@ export const runIframe = ( iframe.setAttribute('width', '0'); iframe.setAttribute('height', '0'); iframe.style.display = 'none'; - iframe.id = state; + iframe.id = stateIn; const timeoutSetTimeoutId = setTimeout(() => { rej(TIMEOUT_ERROR); @@ -39,14 +39,14 @@ export const runIframe = ( const iframeEventHandler = function(e: MessageEvent) { if (e.origin != eventOrigin) return; if (!e.data || e.data.type !== 'authorization_response') return; - if (e.data.response.state !== state) return; + if (e.data.response.state !== stateIn) return; e.data.response.error ? rej(e.data.response) : res(e.data.response); clearTimeout(timeoutSetTimeoutId); window.removeEventListener('message', iframeEventHandler, false); try { - window.document.body.removeChild(document.getElementById(state)); + window.document.body.removeChild(document.getElementById(stateIn)); } catch (error) {} }; window.addEventListener('message', iframeEventHandler, false); From 8a3d4f47bcc9e3a89f4bbc8fb7d5412a0fcc757b Mon Sep 17 00:00:00 2001 From: Luis Deschamps Rudge Date: Tue, 30 Jul 2019 20:10:32 -0300 Subject: [PATCH 3/8] use only 1 iframe --- __tests__/utils.test.ts | 44 ++++++----------------------------------- src/utils.ts | 12 ++++------- 2 files changed, 10 insertions(+), 46 deletions(-) diff --git a/__tests__/utils.test.ts b/__tests__/utils.test.ts index 376314235..d20631281 100644 --- a/__tests__/utils.test.ts +++ b/__tests__/utils.test.ts @@ -381,7 +381,7 @@ describe('utils', () => { } }; const { iframe, url } = setup(message); - await runIframe(url, state, origin); + await runIframe(url, origin); expect(window.document.body.appendChild).toHaveBeenCalledWith(iframe); expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe); @@ -391,7 +391,7 @@ describe('utils', () => { ['src', url] ]); expect(iframe.style.display).toBe('none'); - expect(iframe.id).toBe(state); + expect(iframe.id).toBe('a0-spajs-iframe'); }); describe('with invalid messages', () => { [ @@ -413,7 +413,7 @@ describe('utils', () => { jest.runAllTimers(); }, 10); jest.useFakeTimers(); - await expect(runIframe(url, 'id', origin)).rejects.toMatchObject( + await expect(runIframe(url, origin)).rejects.toMatchObject( TIMEOUT_ERROR ); jest.useRealTimers(); @@ -433,7 +433,7 @@ describe('utils', () => { } }; const { iframe, url } = setup(message); - await expect(runIframe(url, state, origin)).resolves.toMatchObject( + await expect(runIframe(url, origin)).resolves.toMatchObject( message.data.response ); expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe); @@ -450,41 +450,11 @@ describe('utils', () => { } }; const { iframe, url } = setup(message); - await expect(runIframe(url, state, origin)).rejects.toMatchObject( + await expect(runIframe(url, origin)).rejects.toMatchObject( message.data.response ); expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe); }); - //in this test, it will timeout since we're not sending another message later - it('does nothing if the message.state is not the same as the input.state', async () => { - const inputState = 'state'; - const origin = 'https://origin.com'; - const message = { - origin, - source: { close: jest.fn() }, - data: { - type: 'authorization_response', - response: { id_token: 'id_token', state: 'message-state' } - } - }; - const { iframe, url } = setup(message); - - /** - * We need to run the timers after we start `runIframe` to simulate - * the window event listener, but we also need to use `jest.useFakeTimers` - * to trigger the timeout. That's why we're using a real `setTimeout`, - * then using fake timers then rolling back to real timers - */ - setTimeout(() => { - jest.runAllTimers(); - }, 10); - jest.useFakeTimers(); - await expect(runIframe(url, inputState, origin)).rejects.toMatchObject( - TIMEOUT_ERROR - ); - expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe); - jest.useRealTimers(); - }); it('times out after 60s', async () => { const { iframe, url, origin } = setup(''); /** @@ -497,9 +467,7 @@ describe('utils', () => { jest.runAllTimers(); }, 10); jest.useFakeTimers(); - await expect(runIframe(url, 'id', origin)).rejects.toMatchObject( - TIMEOUT_ERROR - ); + await expect(runIframe(url, origin)).rejects.toMatchObject(TIMEOUT_ERROR); expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe); jest.useRealTimers(); }); diff --git a/src/utils.ts b/src/utils.ts index 7287b370c..56c95a4a5 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -19,17 +19,14 @@ export const parseQueryResult = (hash: string) => { }; }; -export const runIframe = ( - authorizeUrl: string, - stateIn: string, - eventOrigin: string -) => { +export const runIframe = (authorizeUrl: string, eventOrigin: string) => { return new Promise((res, rej) => { + const IFRAME_ID = 'a0-spajs-iframe'; var iframe = window.document.createElement('iframe'); iframe.setAttribute('width', '0'); iframe.setAttribute('height', '0'); iframe.style.display = 'none'; - iframe.id = stateIn; + iframe.id = IFRAME_ID; const timeoutSetTimeoutId = setTimeout(() => { rej(TIMEOUT_ERROR); @@ -39,14 +36,13 @@ export const runIframe = ( const iframeEventHandler = function(e: MessageEvent) { if (e.origin != eventOrigin) return; if (!e.data || e.data.type !== 'authorization_response') return; - if (e.data.response.state !== stateIn) return; e.data.response.error ? rej(e.data.response) : res(e.data.response); clearTimeout(timeoutSetTimeoutId); window.removeEventListener('message', iframeEventHandler, false); try { - window.document.body.removeChild(document.getElementById(stateIn)); + window.document.body.removeChild(document.getElementById(IFRAME_ID)); } catch (error) {} }; window.addEventListener('message', iframeEventHandler, false); From 9d800ff207c3f83689dc51a7a50906056bd54b30 Mon Sep 17 00:00:00 2001 From: Luis Deschamps Rudge Date: Tue, 30 Jul 2019 20:11:30 -0300 Subject: [PATCH 4/8] remove state --- __tests__/index.test.ts | 1 - src/Auth0Client.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts index c04377f38..c25e5029e 100644 --- a/__tests__/index.test.ts +++ b/__tests__/index.test.ts @@ -870,7 +870,6 @@ describe('Auth0', () => { await auth0.getTokenSilently(defaultOptionsIgnoreCacheTrue); expect(utils.runIframe).toHaveBeenCalledWith( `https://test.auth0.com/authorize?${TEST_QUERY_PARAMS}${TEST_TELEMETRY_QUERY_STRING}`, - TEST_ENCODED_STATE, 'https://test.auth0.com' ); }); diff --git a/src/Auth0Client.ts b/src/Auth0Client.ts index 3f2bc9c2b..50ac03e6a 100644 --- a/src/Auth0Client.ts +++ b/src/Auth0Client.ts @@ -327,7 +327,7 @@ export default class Auth0Client { response_mode: 'web_message' }); - const codeResult = await runIframe(url, stateIn, this.domainUrl); + const codeResult = await runIframe(url, this.domainUrl); if (stateIn !== codeResult.state) { throw new Error('Invalid state'); } From c1fd3e093c782f509cb941d100b7e5518b63c262 Mon Sep 17 00:00:00 2001 From: Luis Deschamps Rudge Date: Tue, 30 Jul 2019 20:12:40 -0300 Subject: [PATCH 5/8] remove state --- __tests__/utils.test.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/__tests__/utils.test.ts b/__tests__/utils.test.ts index d20631281..3252f5c6f 100644 --- a/__tests__/utils.test.ts +++ b/__tests__/utils.test.ts @@ -370,14 +370,13 @@ describe('utils', () => { return { iframe, url, origin }; }; it('handles iframe correctly', async () => { - const state = 'state'; const origin = 'https://origin.com'; const message = { origin, source: { close: jest.fn() }, data: { type: 'authorization_response', - response: { id_token: 'id_token', state } + response: { id_token: 'id_token' } } }; const { iframe, url } = setup(message); @@ -422,14 +421,13 @@ describe('utils', () => { }); }); it('returns authorization response message', async () => { - const state = 'state'; const origin = 'https://origin.com'; const message = { origin, source: { close: jest.fn() }, data: { type: 'authorization_response', - response: { id_token: 'id_token', state } + response: { id_token: 'id_token' } } }; const { iframe, url } = setup(message); @@ -439,14 +437,13 @@ describe('utils', () => { expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe); }); it('returns authorization error message', async () => { - const state = 'state'; const origin = 'https://origin.com'; const message = { origin, source: { close: jest.fn() }, data: { type: 'authorization_response', - response: { error: 'error', state } + response: { error: 'error' } } }; const { iframe, url } = setup(message); From 70a18a12bae373e58281bf51eb0f318397e22760 Mon Sep 17 00:00:00 2001 From: Luis Deschamps Rudge Date: Tue, 30 Jul 2019 22:14:53 -0300 Subject: [PATCH 6/8] throw error when there's an iframe running --- __tests__/utils.test.ts | 36 +++++++++++++++++++++++++++++++----- src/errors.ts | 7 +++++++ src/utils.ts | 11 +++++++++-- 3 files changed, 47 insertions(+), 7 deletions(-) diff --git a/__tests__/utils.test.ts b/__tests__/utils.test.ts index 3252f5c6f..d4abb0c3e 100644 --- a/__tests__/utils.test.ts +++ b/__tests__/utils.test.ts @@ -15,8 +15,10 @@ import { urlDecodeB64 } from '../src/utils'; import { DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS } from '../src/constants'; +import { InternalError } from '../src/errors'; (global).TextEncoder = TextEncoder; +const TIMEOUT_ERROR = { error: 'timeout', error_description: 'Timeout' }; describe('utils', () => { describe('getUniqueScopes', () => { @@ -240,7 +242,6 @@ describe('utils', () => { }); }); describe('runPopup', () => { - const TIMEOUT_ERROR = { error: 'timeout', error_description: 'Timeout' }; const setup = customMessage => { const popup = { location: { href: '' }, @@ -343,7 +344,6 @@ describe('utils', () => { }); }); describe('runIframe', () => { - const TIMEOUT_ERROR = { error: 'timeout', error_description: 'Timeout' }; const setup = customMessage => { const iframe = { setAttribute: jest.fn(), @@ -362,9 +362,13 @@ describe('utils', () => { expect(type).toBe('iframe'); return iframe; }); - window.document.getElementById = jest.fn(() => { - return iframe; - }); + window.document.getElementById = jest + .fn() + .mockImplementationOnce(id => undefined) + .mockImplementationOnce(id => { + expect(id).toBe('a0-spajs-iframe'); + return iframe; + }); window.document.body.appendChild = jest.fn(); window.document.body.removeChild = jest.fn(); return { iframe, url, origin }; @@ -392,6 +396,28 @@ describe('utils', () => { expect(iframe.style.display).toBe('none'); expect(iframe.id).toBe('a0-spajs-iframe'); }); + it('throws an error when the iframe already exists', async () => { + const origin = 'https://origin.com'; + const { iframe, url } = setup(''); + + //the mock returns undefined in the first time + //and an object in the second time, so call it + //once here so when the method runs, it will + //return the object and throw + window.document.getElementById('foobar'); + + try { + await runIframe(url, origin); + } catch (error) { + expect(error).toBeInstanceOf(InternalError); + expect(error.error_description).toBe( + '`getTokenSilently` can only be called once at a time' + ); + expect(window.document.body.appendChild).not.toHaveBeenCalledWith( + iframe + ); + } + }); describe('with invalid messages', () => { [ '', diff --git a/src/errors.ts b/src/errors.ts index d169e2842..1b78deb1a 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -9,3 +9,10 @@ export class AuthenticationError extends Error { Object.setPrototypeOf(this, AuthenticationError.prototype); } } + +export class InternalError extends Error { + public error = 'internal_error'; + constructor(public error_description: string = 'Internal Error') { + super(error_description); + } +} diff --git a/src/utils.ts b/src/utils.ts index 56c95a4a5..25ab0d24f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,9 +1,11 @@ import * as qs from 'qss'; import { DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS } from './constants'; +import { InternalError } from './errors'; + +const TIMEOUT_ERROR = { error: 'timeout', error_description: 'Timeout' }; 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(',')) @@ -20,8 +22,13 @@ export const parseQueryResult = (hash: string) => { }; export const runIframe = (authorizeUrl: string, eventOrigin: string) => { + const IFRAME_ID = 'a0-spajs-iframe'; + if (document.getElementById(IFRAME_ID)) { + throw new InternalError( + '`getTokenSilently` can only be called once at a time' + ); + } return new Promise((res, rej) => { - const IFRAME_ID = 'a0-spajs-iframe'; var iframe = window.document.createElement('iframe'); iframe.setAttribute('width', '0'); iframe.setAttribute('height', '0'); From 5a03f7f3de97bc18bbc1953b18cc32c45387fb9e Mon Sep 17 00:00:00 2001 From: Luis Deschamps Rudge Date: Tue, 30 Jul 2019 22:18:54 -0300 Subject: [PATCH 7/8] error msg --- __tests__/utils.test.ts | 2 +- src/utils.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/__tests__/utils.test.ts b/__tests__/utils.test.ts index d4abb0c3e..0c7672f72 100644 --- a/__tests__/utils.test.ts +++ b/__tests__/utils.test.ts @@ -411,7 +411,7 @@ describe('utils', () => { } catch (error) { expect(error).toBeInstanceOf(InternalError); expect(error.error_description).toBe( - '`getTokenSilently` can only be called once at a time' + "`getTokenSilently` can only be called once at a time. Check your code to make sure you're not calling it multiple times." ); expect(window.document.body.appendChild).not.toHaveBeenCalledWith( iframe diff --git a/src/utils.ts b/src/utils.ts index 25ab0d24f..f8161bf90 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -25,7 +25,7 @@ export const runIframe = (authorizeUrl: string, eventOrigin: string) => { const IFRAME_ID = 'a0-spajs-iframe'; if (document.getElementById(IFRAME_ID)) { throw new InternalError( - '`getTokenSilently` can only be called once at a time' + "`getTokenSilently` can only be called once at a time. Check your code to make sure you're not calling it multiple times." ); } return new Promise((res, rej) => { From d744e480421a956994e0f4001abcf7ff0f5cfb0c Mon Sep 17 00:00:00 2001 From: Luis Deschamps Rudge Date: Fri, 23 Aug 2019 20:37:17 -0300 Subject: [PATCH 8/8] fix ts --- src/errors.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/errors.ts b/src/errors.ts index 1b78deb1a..35781c90f 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -14,5 +14,7 @@ export class InternalError extends Error { public error = 'internal_error'; constructor(public error_description: string = 'Internal Error') { super(error_description); + //https://github.com/Microsoft/TypeScript-wiki/blob/master/Breaking-Changes.md#extending-built-ins-like-error-array-and-map-may-no-longer-work + Object.setPrototypeOf(this, InternalError.prototype); } }