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.

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

Prevent nowProvider from being passed to authorize endpoint #840

Merged
merged 3 commits into from
Nov 16, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
11 changes: 1 addition & 10 deletions __tests__/Auth0Client/constructor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import * as scope from '../../src/scope';

// @ts-ignore

import { loginWithRedirectFn, setupFn } from './helpers';
import { assertUrlEquals, loginWithRedirectFn, setupFn } from './helpers';

import { TEST_CLIENT_ID, TEST_CODE_CHALLENGE, TEST_DOMAIN } from '../constants';
import { ICache } from '../../src/cache';
Expand All @@ -33,15 +33,6 @@ jest

jest.spyOn(utils, 'runPopup');

const assertUrlEquals = (actualUrl, host, path, queryParams) => {
const url = new URL(actualUrl);
expect(url.host).toEqual(host);
expect(url.pathname).toEqual(path);
for (let [key, value] of Object.entries(queryParams)) {
expect(url.searchParams.get(key)).toEqual(value);
}
};
Comment on lines -36 to -43
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was duplicated from the one in helpers.ts, so I removed it.


const setup = setupFn(mockVerify);

describe('Auth0Client', () => {
Expand Down
35 changes: 30 additions & 5 deletions __tests__/Auth0Client/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const authorizationResponse: AuthenticationResult = {
state: TEST_STATE
};

export const assertPostFn = mockFetch => {
export const assertPostFn = (mockFetch: jest.Mock) => {
return (
url: string,
body: any,
Expand All @@ -51,12 +51,37 @@ export const assertPostFn = mockFetch => {
};
};

export const assertUrlEquals = (actualUrl, host, path, queryParams) => {
/**
* Extracts the keys and values from an IterableIterator and applies
* them to an object.
* @param itor The iterable
* @returns An object with the keys and values from the iterable
*/
const itorToObject = <K extends string | number | symbol, V>(
itor: IterableIterator<[K, V]>
): Record<K, V> =>
[...itor].reduce((m, [key, value]) => {
m[key] = value;
return m;
}, {} as Record<K, V>);

export const assertUrlEquals = (
actualUrl: URL | string,
host: string,
path: string,
queryParams: Record<string, string>,
strict: boolean = false
) => {
const url = new URL(actualUrl);
const searchParamsObj = itorToObject(url.searchParams.entries());

expect(url.host).toEqual(host);
expect(url.pathname).toEqual(path);
for (let [key, value] of Object.entries(queryParams)) {
expect(url.searchParams.get(key)).toEqual(value);

if (strict) {
expect(searchParamsObj).toStrictEqual(queryParams);
} else {
expect(searchParamsObj).toMatchObject(queryParams);
}
};

Expand All @@ -66,7 +91,7 @@ export const fetchResponse = (ok, json) =>
json: () => Promise.resolve(json)
});

export const setupFn = mockVerify => {
export const setupFn = (mockVerify: jest.Mock) => {
return (config?: Partial<Auth0ClientOptions>, claims?: Partial<IdToken>) => {
const auth0 = new Auth0Client(
Object.assign(
Expand Down
70 changes: 59 additions & 11 deletions __tests__/Auth0Client/loginWithRedirect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,17 +109,24 @@ describe('Auth0Client', () => {

const url = new URL(mockWindow.location.assign.mock.calls[0][0]);

assertUrlEquals(url, TEST_DOMAIN, '/authorize', {
client_id: TEST_CLIENT_ID,
redirect_uri: TEST_REDIRECT_URI,
scope: TEST_SCOPES,
response_type: 'code',
response_mode: 'query',
state: TEST_STATE,
nonce: TEST_NONCE,
code_challenge: TEST_CODE_CHALLENGE,
code_challenge_method: 'S256'
});
assertUrlEquals(
url,
TEST_DOMAIN,
'/authorize',
{
auth0Client: expect.any(String),
client_id: TEST_CLIENT_ID,
redirect_uri: TEST_REDIRECT_URI,
scope: TEST_SCOPES,
response_type: 'code',
response_mode: 'query',
state: TEST_STATE,
nonce: TEST_NONCE,
code_challenge: TEST_CODE_CHALLENGE,
code_challenge_method: 'S256'
},
true // strict mode
);

assertPost(
'https://auth0_domain/oauth/token',
Expand Down Expand Up @@ -453,5 +460,46 @@ describe('Auth0Client', () => {
}
);
});

it('should not include client options on the URL', async () => {
// ** IMPORTANT **: if adding a new client option, ensure it is added to the destructure
// list in Auth0Client._getParams so that it is not sent to the IdP
const auth0 = setup({
useRefreshTokens: true,
advancedOptions: {
defaultScope: 'openid profile email offline_access'
},
useCookiesForTransactions: true,
authorizeTimeoutInSeconds: 10,
cacheLocation: 'localstorage',
legacySameSiteCookie: true,
nowProvider: () => Date.now(),
sessionCheckExpiryDays: 1,
useFormData: true
});

await loginWithRedirect(auth0);

const url = new URL(mockWindow.location.assign.mock.calls[0][0]);

assertUrlEquals(
url,
TEST_DOMAIN,
'/authorize',
{
auth0Client: expect.any(String),
client_id: TEST_CLIENT_ID,
redirect_uri: TEST_REDIRECT_URI,
scope: 'openid profile email offline_access',
response_type: 'code',
response_mode: 'query',
state: TEST_STATE,
nonce: TEST_NONCE,
code_challenge: TEST_CODE_CHALLENGE,
code_challenge_method: 'S256'
},
true // strict mode
);
});
});
});
16 changes: 12 additions & 4 deletions src/Auth0Client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -309,21 +309,28 @@ export default class Auth0Client {
code_challenge: string,
redirect_uri: string
): AuthorizeOptions {
// These options should be excluded from the authorize URL,
// as they're options for the client and not for the IdP.
// ** IMPORTANT ** If adding a new client option, include it in this destructure list.
const {
domain,
leeway,
useRefreshTokens,
useCookiesForTransactions,
useFormData,
auth0Client,
cacheLocation,
advancedOptions,
detailedResponse,
...withoutClientOptions
nowProvider,
authorizeTimeoutInSeconds,
legacySameSiteCookie,
sessionCheckExpiryDays,
domain,
leeway,
...loginOptions
} = this.options;

return {
...withoutClientOptions,
...loginOptions,
...authorizeOptions,
scope: getUniqueScopes(
this.defaultScope,
Expand All @@ -339,6 +346,7 @@ export default class Auth0Client {
code_challenge_method: 'S256'
};
}

private _authorizeUrl(authorizeOptions: AuthorizeOptions) {
return this._url(`/authorize?${createQueryParams(authorizeOptions)}`);
}
Expand Down
3 changes: 2 additions & 1 deletion tsconfig.test.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"noImplicitAny": false
"noImplicitAny": false,
"target": "es6"
Comment on lines +4 to +5
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

es6 for testing so that I can get access to URLSearchParams.entries(), shouldn't cause any problems with the SDK at runtime?

}
}