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

[SDK-2172] Add SDK metrics to all API calls #659

Merged
merged 5 commits into from
Dec 2, 2020
Merged

[SDK-2172] Add SDK metrics to all API calls #659

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6675b61
[SDK-2172] Add SDK metrics to all API calls
frederikprijck Dec 1, 2020
78b64db
Rework to limit duplication
frederikprijck Dec 1, 2020
cc681a0
Use DEFAULT_AUTH0_CLIENT
frederikprijck Dec 1, 2020
7a2a1d1
Fix build
frederikprijck Dec 1, 2020
f5d1b02
Update tests to include DEFAULT_AUTH0_CLIENT
frederikprijck Dec 2, 2020
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
114 changes: 93 additions & 21 deletions __tests__/Auth0Client/getTokenSilently.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
} from '../constants';

import { releaseLockSpy } from '../../__mocks__/browser-tabs-lock';
import version from '../../src/version';

jest.mock('unfetch');
jest.mock('es-cookie');
Expand Down Expand Up @@ -179,13 +180,24 @@ describe('Auth0Client', () => {

await getTokenSilently(auth0);

assertPost('https://auth0_domain/oauth/token', {
redirect_uri: TEST_REDIRECT_URI,
client_id: TEST_CLIENT_ID,
code_verifier: TEST_CODE_VERIFIER,
grant_type: 'authorization_code',
code: TEST_CODE
});
assertPost(
'https://auth0_domain/oauth/token',
{
redirect_uri: TEST_REDIRECT_URI,
client_id: TEST_CLIENT_ID,
code_verifier: TEST_CODE_VERIFIER,
grant_type: 'authorization_code',
code: TEST_CODE
},
{
'Auth0-Client': btoa(
JSON.stringify({
name: 'auth0-spa-js',
version: version
})
)
Copy link
Contributor

Choose a reason for hiding this comment

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

As we're adding coverage here, it would be worth adding the same assertions on the headers for the test that follows ("..when using refresh tokens"). Looks like an omission testing for one but not the other, even though in reality we know both are covered.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ye I thought about it, wasn't sure and decided to not pollute all tests with it. But I think I can add it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated the PR to address this. Also added it on a few more places and used DEFAUKT_AUTH0_CLIENT where appropriate

}
);
});

it('calls the token endpoint with the correct params when using refresh tokens', async () => {
Expand Down Expand Up @@ -351,6 +363,7 @@ describe('Auth0Client', () => {

jest.spyOn(<any>utils, 'runIframe').mockResolvedValue({
access_token: TEST_ACCESS_TOKEN,
code: TEST_CODE,
state: TEST_STATE
});

Expand All @@ -359,6 +372,19 @@ describe('Auth0Client', () => {
await getTokenSilently(auth0);

expectToHaveBeenCalledWithAuth0ClientParam(utils.runIframe, auth0Client);
assertPost(
'https://auth0_domain/oauth/token',
{
redirect_uri: TEST_REDIRECT_URI,
client_id: TEST_CLIENT_ID,
code_verifier: TEST_CODE_VERIFIER,
grant_type: 'authorization_code',
code: TEST_CODE
},
{
'Auth0-Client': btoa(JSON.stringify(auth0Client))
}
);
});

it('refreshes the token when cache available without access token', async () => {
Expand Down Expand Up @@ -458,6 +484,14 @@ describe('Auth0Client', () => {
redirect_uri: TEST_REDIRECT_URI,
refresh_token: TEST_REFRESH_TOKEN
},
{
'Auth0-Client': btoa(
JSON.stringify({
name: 'auth0-spa-js',
version: version
})
)
},
1
);

Expand All @@ -474,13 +508,24 @@ describe('Auth0Client', () => {

await loginWithRedirect(auth0);

assertPost('https://auth0_domain/oauth/token', {
redirect_uri: TEST_REDIRECT_URI,
client_id: TEST_CLIENT_ID,
code_verifier: TEST_CODE_VERIFIER,
grant_type: 'authorization_code',
code: TEST_CODE
});
assertPost(
'https://auth0_domain/oauth/token',
{
redirect_uri: TEST_REDIRECT_URI,
client_id: TEST_CLIENT_ID,
code_verifier: TEST_CODE_VERIFIER,
grant_type: 'authorization_code',
code: TEST_CODE
},
{
'Auth0-Client': btoa(
JSON.stringify({
name: 'auth0-spa-js',
version: version
})
)
}
);

mockFetch.mockResolvedValueOnce(
fetchResponse(true, {
Expand All @@ -501,6 +546,14 @@ describe('Auth0Client', () => {
redirect_uri: TEST_REDIRECT_URI,
refresh_token: TEST_REFRESH_TOKEN
},
{
'Auth0-Client': btoa(
JSON.stringify({
name: 'auth0-spa-js',
version: version
})
)
},
1
);

Expand All @@ -519,13 +572,24 @@ describe('Auth0Client', () => {

await loginWithRedirect(auth0);

assertPost('https://auth0_domain/oauth/token', {
redirect_uri: TEST_REDIRECT_URI,
client_id: TEST_CLIENT_ID,
code_verifier: TEST_CODE_VERIFIER,
grant_type: 'authorization_code',
code: TEST_CODE
});
assertPost(
'https://auth0_domain/oauth/token',
{
redirect_uri: TEST_REDIRECT_URI,
client_id: TEST_CLIENT_ID,
code_verifier: TEST_CODE_VERIFIER,
grant_type: 'authorization_code',
code: TEST_CODE
},
{
'Auth0-Client': btoa(
JSON.stringify({
name: 'auth0-spa-js',
version: version
})
)
}
);

const access_token = await getTokenSilently(auth0, { ignoreCache: true });

Expand All @@ -537,6 +601,14 @@ describe('Auth0Client', () => {
redirect_uri: TEST_REDIRECT_URI,
refresh_token: TEST_REFRESH_TOKEN
},
{
'Auth0-Client': btoa(
JSON.stringify({
name: 'auth0-spa-js',
version: version
})
)
},
1
);

Expand Down
7 changes: 6 additions & 1 deletion __tests__/Auth0Client/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,15 @@ const authorizationResponse: AuthenticationResult = {
};

export const assertPostFn = mockFetch => {
return (url, body, callNum = 0) => {
return (url, body, headers = null, callNum = 0) => {
const [actualUrl, opts] = mockFetch.mock.calls[callNum];
expect(url).toEqual(actualUrl);
expect(body).toEqual(JSON.parse(opts.body));
if (headers) {
Object.keys(headers).forEach(header =>
expect(headers[header]).toEqual(opts.headers[header])
);
}
};
};

Expand Down
28 changes: 23 additions & 5 deletions __tests__/utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { MessageChannel } from 'worker_threads';
import unfetch from 'unfetch';
// @ts-ignore
import Worker from '../src/token.worker';
import version from '../src/version';

jest.mock('../src/token.worker');
jest.mock('unfetch');
Expand Down Expand Up @@ -274,19 +275,27 @@ describe('utils', () => {
res({ ok: true, json: () => new Promise(ress => ress(true)) })
)
);
const auth0Client = {
name: 'auth0-spa-js',
version: version
};

await oauthToken({
grant_type: 'authorization_code',
baseUrl: 'https://test.com',
client_id: 'client_idIn',
code: 'codeIn',
code_verifier: 'code_verifierIn'
code_verifier: 'code_verifierIn',
auth0Client
});

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' },
headers: {
'Content-type': 'application/json',
'Auth0-Client': btoa(JSON.stringify(auth0Client))
},
method: 'POST',
signal: abortController.signal
});
Expand All @@ -309,6 +318,10 @@ describe('utils', () => {
code: 'codeIn',
code_verifier: 'code_verifierIn'
};
const auth0Client = {
name: 'auth0-spa-js',
version: version
};

await oauthToken(
{
Expand All @@ -318,14 +331,18 @@ describe('utils', () => {
code: 'codeIn',
code_verifier: 'code_verifierIn',
audience: '__test_audience__',
scope: '__test_scope__'
scope: '__test_scope__',
auth0Client
},
worker
);

expect(mockUnfetch).toBeCalledWith('https://test.com/oauth/token', {
body: JSON.stringify(body),
headers: { 'Content-type': 'application/json' },
headers: {
'Content-type': 'application/json',
'Auth0-Client': btoa(JSON.stringify(auth0Client))
},
method: 'POST',
signal: abortController.signal
});
Expand All @@ -337,7 +354,8 @@ describe('utils', () => {
audience: '__test_audience__',
scope: '__test_scope__',
headers: {
'Content-type': 'application/json'
'Content-type': 'application/json',
'Auth0-Client': btoa(JSON.stringify(auth0Client))
},
method: 'POST',
timeout: 10000,
Expand Down
26 changes: 11 additions & 15 deletions src/Auth0Client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,10 @@ import {
MISSING_REFRESH_TOKEN_ERROR_MESSAGE,
DEFAULT_SCOPE,
RECOVERABLE_ERRORS,
DEFAULT_SESSION_CHECK_EXPIRY_DAYS
DEFAULT_SESSION_CHECK_EXPIRY_DAYS,
DEFAULT_AUTH0_CLIENT
} from './constants';

import version from './version';

import {
Auth0ClientOptions,
BaseLoginOptions,
Expand Down Expand Up @@ -200,14 +199,7 @@ export default class Auth0Client {

private _url(path) {
const auth0Client = encodeURIComponent(
btoa(
JSON.stringify(
this.options.auth0Client || {
name: 'auth0-spa-js',
version: version
}
)
)
btoa(JSON.stringify(this.options.auth0Client || DEFAULT_AUTH0_CLIENT))
);
return `${this.domainUrl}${path}&auth0Client=${auth0Client}`;
}
Expand Down Expand Up @@ -381,7 +373,8 @@ export default class Auth0Client {
code_verifier,
code: codeResult.code,
grant_type: 'authorization_code',
redirect_uri: params.redirect_uri
redirect_uri: params.redirect_uri,
auth0Client: this.options.auth0Client
} as OAuthTokenOptions,
this.worker
);
Expand Down Expand Up @@ -517,7 +510,8 @@ export default class Auth0Client {
client_id: this.options.client_id,
code_verifier: transaction.code_verifier,
grant_type: 'authorization_code',
code
code,
auth0Client: this.options.auth0Client
} as OAuthTokenOptions;

// some old versions of the SDK might not have added redirect_uri to the
Expand Down Expand Up @@ -830,7 +824,8 @@ export default class Auth0Client {
code_verifier,
code: codeResult.code,
grant_type: 'authorization_code',
redirect_uri: params.redirect_uri
redirect_uri: params.redirect_uri,
auth0Client: this.options.auth0Client
} as OAuthTokenOptions,
this.worker
);
Expand Down Expand Up @@ -899,7 +894,8 @@ export default class Auth0Client {
grant_type: 'refresh_token',
refresh_token: cache && cache.refresh_token,
redirect_uri,
...(timeout && { timeout })
...(timeout && { timeout }),
auth0Client: this.options.auth0Client
} as RefreshTokenOptions,
this.worker
);
Expand Down
9 changes: 9 additions & 0 deletions src/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { PopupConfigOptions } from './global';
import version from './version';

/**
* @ignore
Expand Down Expand Up @@ -58,3 +59,11 @@ export const RECOVERABLE_ERRORS = [
* @ignore
*/
export const DEFAULT_SESSION_CHECK_EXPIRY_DAYS = 1;

/**
* @ignore
*/
export const DEFAULT_AUTH0_CLIENT = {
name: 'auth0-spa-js',
version: version
};
1 change: 1 addition & 0 deletions src/global.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ export interface TokenEndpointOptions {
client_id: string;
grant_type: string;
timeout?: number;
auth0Client: any;
[key: string]: any;
}

Expand Down
Loading