Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix popup blocker showing for loginWithPopup in Firefox & Safari #732

Merged
merged 4 commits into from
Apr 12, 2021
Merged

Fix popup blocker showing for loginWithPopup in Firefox & Safari #732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eb2ec25
Move popup opening outside an async context
Apr 1, 2021
ee31116
Make loginWithPopup params optional instead of using defaults
Apr 8, 2021
8c86991
Fix tests
Apr 9, 2021
df62bae
Merge branch 'master' into fix/firefox-popup
frederikprijck Apr 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion __tests__/Auth0Client/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,10 @@ export const setupMessageEventLister = (
});

mockWindow.open.mockReturnValue({
close: () => {}
close: () => {},
location: {
href: ''
}
});
};

Expand Down
55 changes: 39 additions & 16 deletions __tests__/Auth0Client/loginWithPopup.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import {
DEFAULT_AUTH0_CLIENT,
DEFAULT_POPUP_CONFIG_OPTIONS
} from '../../src/constants';
import version from '../../src/version';

jest.mock('unfetch');
jest.mock('es-cookie');
Expand Down Expand Up @@ -76,6 +75,7 @@ describe('Auth0Client', () => {

mockWindow.open = jest.fn();
mockWindow.addEventListener = jest.fn();

mockWindow.crypto = {
subtle: {
digest: () => 'foo'
Expand All @@ -99,6 +99,9 @@ describe('Auth0Client', () => {
describe('loginWithPopup', () => {
it('should log the user in and get the user and claims', async () => {
const auth0 = setup({ scope: 'foo' });

mockWindow.open.mockReturnValue({ hello: 'world' });

await loginWithPopup(auth0);

const expectedUser = { sub: 'me' };
Expand Down Expand Up @@ -142,7 +145,9 @@ describe('Auth0Client', () => {

await loginWithPopup(auth0);

const [[url]] = (<jest.Mock>mockWindow.open).mock.calls;
// prettier-ignore
const url = (utils.runPopup as jest.Mock).mock.calls[0][0].popup.location.href;

assertUrlEquals(url, 'auth0_domain', '/authorize', {
state: TEST_STATE,
nonce: TEST_NONCE
Expand All @@ -154,7 +159,9 @@ describe('Auth0Client', () => {

await loginWithPopup(auth0);

const [[url]] = (<jest.Mock>mockWindow.open).mock.calls;
// prettier-ignore
const url = (utils.runPopup as jest.Mock).mock.calls[0][0].popup.location.href;

assertUrlEquals(url, 'auth0_domain', '/authorize', {
code_challenge: TEST_CODE_CHALLENGE,
code_challenge_method: 'S256'
Expand All @@ -170,7 +177,10 @@ describe('Auth0Client', () => {
});

expect(mockWindow.open).toHaveBeenCalled();
const [[url]] = (<jest.Mock>mockWindow.open).mock.calls;

// prettier-ignore
const url = (utils.runPopup as jest.Mock).mock.calls[0][0].popup.location.href;

assertUrlEquals(url, 'auth0_domain', '/authorize', {
redirect_uri: 'http://localhost',
client_id: TEST_CLIENT_ID,
Expand All @@ -195,7 +205,10 @@ describe('Auth0Client', () => {
});

expect(mockWindow.open).toHaveBeenCalled();
const [[url]] = (<jest.Mock>mockWindow.open).mock.calls;

// prettier-ignore
const url = (utils.runPopup as jest.Mock).mock.calls[0][0].popup.location.href;

assertUrlEquals(url, TEST_DOMAIN, '/authorize', {
redirect_uri: TEST_REDIRECT_URI,
client_id: TEST_CLIENT_ID,
Expand All @@ -215,8 +228,12 @@ describe('Auth0Client', () => {
const auth0 = setup({
useRefreshTokens: true
});

await loginWithPopup(auth0);
const [[url]] = (<jest.Mock>mockWindow.open).mock.calls;

// prettier-ignore
const url = (utils.runPopup as jest.Mock).mock.calls[0][0].popup.location.href;

assertUrlEquals(url, TEST_DOMAIN, '/authorize', {
scope: `${TEST_SCOPES} offline_access`
});
Expand All @@ -228,18 +245,20 @@ describe('Auth0Client', () => {
redirect_uri
});
await loginWithPopup(auth0);
const [[url]] = (<jest.Mock>mockWindow.open).mock.calls;

// prettier-ignore
const url = (utils.runPopup as jest.Mock).mock.calls[0][0].popup.location.href;

assertUrlEquals(url, TEST_DOMAIN, '/authorize', {
redirect_uri
});
});

it('should log the user in with a popup and get the token', async () => {
const auth0 = setup();

await loginWithPopup(auth0);

expect(mockWindow.open).toHaveBeenCalled();

assertPost(
'https://auth0_domain/oauth/token',
{
Expand All @@ -260,19 +279,20 @@ describe('Auth0Client', () => {

await loginWithPopup(auth0);

expect(utils.runPopup).toHaveBeenCalledWith(
expect.any(String),
DEFAULT_POPUP_CONFIG_OPTIONS
);
expect(utils.runPopup).toHaveBeenCalledWith({
...DEFAULT_POPUP_CONFIG_OPTIONS,
popup: expect.anything()
});
});

it('should be able to provide custom config', async () => {
const auth0 = setup({ leeway: 10 });

await loginWithPopup(auth0, {}, { timeoutInSeconds: 3 });

expect(utils.runPopup).toHaveBeenCalledWith(expect.any(String), {
timeoutInSeconds: 3
expect(utils.runPopup).toHaveBeenCalledWith({
timeoutInSeconds: 3,
popup: expect.anything()
});
});

Expand Down Expand Up @@ -345,7 +365,10 @@ describe('Auth0Client', () => {
await loginWithPopup(auth0);

expect(mockWindow.open).toHaveBeenCalled();
const [[url]] = (<jest.Mock>mockWindow.open).mock.calls;

// prettier-ignore
const url = (utils.runPopup as jest.Mock).mock.calls[0][0].popup.location.href;

assertUrlEquals(url, TEST_DOMAIN, '/authorize', {
auth0Client: btoa(JSON.stringify(auth0Client))
});
Expand Down
35 changes: 5 additions & 30 deletions __tests__/utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ describe('utils', () => {
jest.runAllTimers();
}, 10);
jest.useFakeTimers();
await expect(runPopup(url, { popup })).rejects.toMatchObject(
await expect(runPopup({ popup })).rejects.toMatchObject(
TIMEOUT_ERROR
);
jest.useRealTimers();
Expand All @@ -287,7 +287,7 @@ describe('utils', () => {

const { popup, url } = setup(message);

await expect(runPopup(url, { popup })).resolves.toMatchObject(
await expect(runPopup({ popup })).resolves.toMatchObject(
message.data.response
);

Expand All @@ -308,7 +308,7 @@ describe('utils', () => {

const { popup, url } = setup(message);

await expect(runPopup(url, { popup })).rejects.toMatchObject({
await expect(runPopup({ popup })).rejects.toMatchObject({
...message.data.response,
message: 'error_description'
});
Expand All @@ -334,7 +334,7 @@ describe('utils', () => {
jest.useFakeTimers();

await expect(
runPopup(url, {
runPopup({
timeoutInSeconds: seconds,
popup
})
Expand All @@ -357,35 +357,10 @@ describe('utils', () => {

jest.useFakeTimers();

await expect(runPopup(url, { popup })).rejects.toMatchObject(
TIMEOUT_ERROR
);
await expect(runPopup({ popup })).rejects.toMatchObject(TIMEOUT_ERROR);

jest.useRealTimers();
});

it('creates and uses a popup window if none was given', async () => {
const message = {
data: {
type: 'authorization_response',
response: { id_token: 'id_token' }
}
};

const { popup, url } = setup(message);
const oldOpenFn = window.open;

window.open = <any>jest.fn(() => popup);

await expect(runPopup(url, {})).resolves.toMatchObject(
message.data.response
);

expect(popup.location.href).toBe(url);
expect(popup.close).toHaveBeenCalled();

window.open = oldOpenFn;
});
});
describe('runIframe', () => {
const TIMEOUT_ERROR = {
Expand Down
28 changes: 20 additions & 8 deletions src/Auth0Client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import {
runIframe,
sha256,
bufferToBase64UrlEncoded,
validateCrypto
validateCrypto,
openPopup
} from './utils';

import { oauthToken, TokenEndpointResponse } from './api';
Expand Down Expand Up @@ -335,9 +336,16 @@ export default class Auth0Client {
* @param config
*/
public async loginWithPopup(
options: PopupLoginOptions = {},
config: PopupConfigOptions = {}
options?: PopupLoginOptions,
config?: PopupConfigOptions
) {
options = options || {};
config = config || {};

if (!config.popup) {
config.popup = openPopup('');
}

const { ...authorizeOptions } = options;
const stateIn = encode(createRandomString());
const nonceIn = encode(createRandomString());
Expand All @@ -358,7 +366,9 @@ export default class Auth0Client {
response_mode: 'web_message'
});

const codeResult = await runPopup(url, {
config.popup.location.href = url;

const codeResult = await runPopup({
...config,
timeoutInSeconds:
config.timeoutInSeconds ||
Expand Down Expand Up @@ -420,7 +430,7 @@ export default class Auth0Client {
* (the SDK stores a corresponding ID Token with every Access Token, and uses the
* scope and audience to look up the ID Token)
*
* @typeparam TUser The type to return, has to extend {@link User}.
* @typeparam TUser The type to return, has to extend {@link User}.
* @param options
*/
public async getUser<TUser extends User>(
Expand Down Expand Up @@ -453,7 +463,9 @@ export default class Auth0Client {
*
* @param options
*/
public async getIdTokenClaims(options: GetIdTokenClaimsOptions = {}): Promise<IdToken> {
public async getIdTokenClaims(
options: GetIdTokenClaimsOptions = {}
): Promise<IdToken> {
const audience = options.audience || this.options.audience || 'default';
const scope = getUniqueScopes(this.defaultScope, this.scope, options.scope);

Expand Down Expand Up @@ -829,8 +841,8 @@ export default class Auth0Client {
nonceIn,
code_challenge,
options.redirect_uri ||
this.options.redirect_uri ||
window.location.origin
this.options.redirect_uri ||
window.location.origin
);

const url = this._authorizeUrl({
Expand Down
20 changes: 4 additions & 16 deletions src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export const runIframe = (
});
};

const openPopup = (url: string) => {
export const openPopup = (url: string) => {
const width = 400;
const height = 600;
const left = window.screenX + (window.innerWidth - width) / 2;
Expand All @@ -93,24 +93,12 @@ const openPopup = (url: string) => {
);
};

export const runPopup = (authorizeUrl: string, config: PopupConfigOptions) => {
let popup = config.popup;

if (popup) {
popup.location.href = authorizeUrl;
} else {
popup = openPopup(authorizeUrl);
}

if (!popup) {
throw new Error('Could not open popup');
}

export const runPopup = (config: PopupConfigOptions) => {
return new Promise<AuthenticationResult>((resolve, reject) => {
let popupEventListener: EventListenerOrEventListenerObject;

const timeoutId = setTimeout(() => {
reject(new PopupTimeoutError(popup));
reject(new PopupTimeoutError(config.popup));
window.removeEventListener('message', popupEventListener, false);
}, (config.timeoutInSeconds || DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS) * 1000);

Expand All @@ -121,7 +109,7 @@ export const runPopup = (authorizeUrl: string, config: PopupConfigOptions) => {

clearTimeout(timeoutId);
window.removeEventListener('message', popupEventListener, false);
popup.close();
config.popup.close();

if (e.data.response.error) {
return reject(GenericError.fromPayload(e.data.response));
Expand Down