Skip to content

Commit a67424c

Browse files
mt-dfreymt-dfrey
authored
Merge pull request #147 from moneytree/only-support-pkce
remove PKCE option
2 parents 3099aee + 2234e0e commit a67424c

File tree

11 files changed

+92
-75
lines changed

11 files changed

+92
-75
lines changed

‎jest.config.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ module.exports = {
88
// https://github.com/facebook/jest/issues/2070#issuecomment-431706685
99
'<rootDir>/.*/__mocks__'
1010
],
11+
testPathIgnorePatterns: ['/__tests__/helper/'],
1112
globals: {
1213
__VERSION__: packageJSON.version
1314
}

‎sample/src/index.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,6 @@ elements.authorizeBtn.onclick = () => {
5555
authorizeOptions.scopes = scopesSelectedOptions.map((x) => x.value);
5656
}
5757

58-
authorizeOptions.pkce = true;
59-
6058
mtLinkSdk.authorize(authorizeOptions);
6159
};
6260

@@ -75,7 +73,6 @@ elements.doOnboardBtn.onclick = async () => {
7573
}
7674

7775
onBoardOptions.email = onboardOptionsElms.email.value;
78-
onBoardOptions.pkce = true;
7976

8077
mtLinkSdk.onboard(onBoardOptions);
8178
} catch (error) {

‎src/__tests__/helper/expect-url-to-match.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import qs from 'qs';
2+
3+
export interface UrlExpectation {
4+
baseUrl: string;
5+
path: string;
6+
query?: Record<string, string>;
7+
}
8+
9+
export default function expectUrlToMatchWithPKCE(actual: URL | string, expectation: UrlExpectation) {
10+
const url = typeof actual === 'string' ? new URL(actual) : actual;
11+
const actualQuery = qs.parse(new URLSearchParams(url.search).toString());
12+
13+
expect(actualQuery.code_challenge).toBeDefined();
14+
delete actualQuery.code_challenge; // ignore PKCE code challenge because it's randomly generated
15+
expect(actualQuery.code_challenge_method).toBe('S256');
16+
delete actualQuery.code_challenge_method;
17+
18+
expect(url.pathname).toBe(expectation.path);
19+
expect(`${url.protocol}//${url.hostname}`).toBe(expectation.baseUrl);
20+
expect(actualQuery).toEqual(expectation.query || {});
21+
}

‎src/__tests__/index.test.ts‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import tokenInfo from '../api/token-info';
1010
import mtLinkSdk, { Mode, MtLinkSdk } from '..';
1111

1212
import packageJson from '../../package.json';
13+
import expectUrlToMatchWithPKCE from './helper/expect-url-to-match';
1314

1415
jest.mock('../api/authorize');
1516
jest.mock('../api/onboard');
@@ -80,19 +81,30 @@ describe('index', () => {
8081

8182
const sdkVersion = packageJson.version;
8283

84+
const authQuery = {
85+
client_id: 'clientId',
86+
response_type: 'code',
87+
scope: 'scopes',
88+
redirect_uri: 'redirectUri',
89+
country: 'JP',
90+
configs: `authn_method=sso&sdk_platform=js&sdk_version=${sdkVersion}`
91+
}
8392
const result8 = instance.authorizeUrl({ scopes: 'scopes' });
84-
expect(result8).toBe(
85-
'https://myaccount.getmoneytree.com/oauth/authorize?client_id=clientId&response_type=code&' +
86-
'scope=scopes&redirect_uri=redirectUri&country=JP&saml_subject_id=samlSubjectId&' +
87-
`configs=authn_method%3Dsso%26sdk_platform%3Djs%26sdk_version%3D${sdkVersion}`
88-
);
93+
expectUrlToMatchWithPKCE(result8, {
94+
baseUrl: 'https://myaccount.getmoneytree.com',
95+
path: '/oauth/authorize',
96+
query: {
97+
...authQuery,
98+
saml_subject_id: 'samlSubjectId',
99+
}
100+
})
89101

90102
const result9 = instance.onboardUrl({ scopes: 'scopes' });
91-
expect(result9).toBe(
92-
'https://myaccount.getmoneytree.com/onboard?client_id=clientId&response_type=code&' +
93-
'scope=scopes&redirect_uri=redirectUri&country=JP&' +
94-
`configs=authn_method%3Dsso%26sdk_platform%3Djs%26sdk_version%3D${sdkVersion}`
95-
);
103+
expectUrlToMatchWithPKCE(result9, {
104+
baseUrl: 'https://myaccount.getmoneytree.com',
105+
path: '/onboard',
106+
query: authQuery
107+
})
96108

97109
const result10 = instance.logoutUrl({ backTo: 'backTo' });
98110
expect(result10).toBe(

‎src/api/__tests__/authorize-url.test.ts‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
import qs from 'qs';
21
import { mocked } from 'ts-jest/utils';
32

43
import { MY_ACCOUNT_DOMAINS } from '../../server-paths';
54
import { MtLinkSdk } from '../..';
65
import authorizeUrl from '../authorize-url';
76
import { generateConfigs } from '../../helper';
87
import storage from '../../storage';
8+
import expectUrlToMatchWithPKCE from '../../__tests__/helper/expect-url-to-match';
99

1010
jest.mock('../../storage');
1111

@@ -53,7 +53,7 @@ describe('api', () => {
5353

5454
const url = authorizeUrl(mtLinkSdk.storedOptions);
5555

56-
const query = qs.stringify({
56+
const query = {
5757
client_id: clientId,
5858
cobrand_client_id: cobrandClientId,
5959
response_type: 'code',
@@ -63,9 +63,8 @@ describe('api', () => {
6363
locale,
6464
saml_subject_id: samlSubjectId,
6565
configs: generateConfigs()
66-
});
67-
68-
expect(url).toBe(`${MY_ACCOUNT_DOMAINS.production}/oauth/authorize?${query}`);
66+
};
67+
expectUrlToMatchWithPKCE(url, {baseUrl: MY_ACCOUNT_DOMAINS.production, path: '/oauth/authorize', query})
6968
});
7069

7170
test('with options', () => {
@@ -85,7 +84,7 @@ describe('api', () => {
8584
scopes
8685
});
8786

88-
const query = qs.stringify({
87+
const query = {
8988
client_id: clientId,
9089
response_type: 'code',
9190
scope: scopes,
@@ -94,8 +93,8 @@ describe('api', () => {
9493
country,
9594
saml_subject_id: samlSubjectId,
9695
configs: generateConfigs()
97-
});
98-
expect(url).toBe(`${MY_ACCOUNT_DOMAINS.production}/oauth/authorize?${query}`);
96+
}
97+
expectUrlToMatchWithPKCE(url, {baseUrl: MY_ACCOUNT_DOMAINS.production, path: '/oauth/authorize', query})
9998
});
10099
});
101100
});

‎src/api/__tests__/authorize.test.ts‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
import qs from 'qs';
21
import { mocked } from 'ts-jest/utils';
32

43
import { MY_ACCOUNT_DOMAINS } from '../../server-paths';
54
import { MtLinkSdk } from '../..';
65
import authorize from '../authorize';
76
import { generateConfigs } from '../../helper';
87
import storage from '../../storage';
8+
import expectUrlToMatchWithPKCE from '../../__tests__/helper/expect-url-to-match';
99

1010
jest.mock('../../storage');
1111

@@ -57,8 +57,9 @@ describe('api', () => {
5757
authorize(mtLinkSdk.storedOptions);
5858

5959
expect(open).toBeCalledTimes(1);
60-
61-
const query = qs.stringify({
60+
expect(open).toBeCalledWith(expect.any(String), '_self', 'noreferrer');
61+
const url = open.mock.calls[0][0]
62+
const query = {
6263
client_id: clientId,
6364
cobrand_client_id: cobrandClientId,
6465
response_type: 'code',
@@ -68,9 +69,8 @@ describe('api', () => {
6869
locale,
6970
saml_subject_id: samlSubjectId,
7071
configs: generateConfigs()
71-
});
72-
const url = `${MY_ACCOUNT_DOMAINS.production}/oauth/authorize?${query}`;
73-
expect(open).toBeCalledWith(url, '_self', 'noreferrer');
72+
};
73+
expectUrlToMatchWithPKCE(url, {baseUrl: MY_ACCOUNT_DOMAINS.production, path: '/oauth/authorize', query })
7474
});
7575

7676
test('with options', () => {
@@ -92,8 +92,9 @@ describe('api', () => {
9292
});
9393

9494
expect(open).toBeCalledTimes(1);
95-
96-
const query = qs.stringify({
95+
expect(open).toBeCalledWith(expect.any(String), '_self', 'noreferrer');
96+
const url = open.mock.calls[0][0]
97+
const query = {
9798
client_id: clientId,
9899
response_type: 'code',
99100
scope: scopes,
@@ -102,9 +103,8 @@ describe('api', () => {
102103
country,
103104
saml_subject_id: samlSubjectId,
104105
configs: generateConfigs()
105-
});
106-
const url = `${MY_ACCOUNT_DOMAINS.production}/oauth/authorize?${query}`;
107-
expect(open).toBeCalledWith(url, '_self', 'noreferrer');
106+
};
107+
expectUrlToMatchWithPKCE(url, {baseUrl: MY_ACCOUNT_DOMAINS.production, path: '/oauth/authorize', query })
108108
});
109109

110110
test('without window', () => {

‎src/api/__tests__/onboard-url.test.ts‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { MtLinkSdk } from '../..';
66
import onboardUrl from '../onboard-url';
77
import { generateConfigs } from '../../helper';
88
import storage from '../../storage';
9+
import expectUrlToMatchWithPKCE from '../../__tests__/helper/expect-url-to-match';
910

1011
jest.mock('../../storage');
1112

@@ -53,7 +54,7 @@ describe('api', () => {
5354

5455
const url = onboardUrl(mtLinkSdk.storedOptions);
5556

56-
const query = qs.stringify({
57+
const query = {
5758
client_id: clientId,
5859
cobrand_client_id: cobrandClientId,
5960
response_type: 'code',
@@ -62,9 +63,9 @@ describe('api', () => {
6263
country,
6364
locale,
6465
configs: generateConfigs({ email })
65-
});
66+
};
6667

67-
expect(url).toBe(`${MY_ACCOUNT_DOMAINS.production}/onboard?${query}`);
68+
expectUrlToMatchWithPKCE(url, {baseUrl: MY_ACCOUNT_DOMAINS.production, path: '/onboard', query: query })
6869
});
6970

7071
test('with options', () => {
@@ -78,23 +79,24 @@ describe('api', () => {
7879
mtLinkSdk.init(clientId);
7980

8081
const url = onboardUrl(mtLinkSdk.storedOptions, {
81-
state,
82-
redirectUri,
83-
scopes,
84-
email
85-
});
82+
state,
83+
redirectUri,
84+
scopes,
85+
email
86+
}
87+
);
8688

87-
const query = qs.stringify({
89+
const query = {
8890
client_id: clientId,
8991
response_type: 'code',
9092
scope: scopes,
9193
redirect_uri: redirectUri,
9294
state,
9395
country,
9496
configs: generateConfigs({ email })
95-
});
97+
};
9698

97-
expect(url).toBe(`${MY_ACCOUNT_DOMAINS.production}/onboard?${query}`);
99+
expectUrlToMatchWithPKCE(url, {baseUrl: MY_ACCOUNT_DOMAINS.production, path: '/onboard', query})
98100
});
99101
});
100102
});

‎src/api/__tests__/onboard.test.ts‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
import qs from 'qs';
21
import { mocked } from 'ts-jest/utils';
32

43
import { MY_ACCOUNT_DOMAINS } from '../../server-paths';
54
import { MtLinkSdk } from '../..';
65
import onboard from '../onboard';
76
import { generateConfigs } from '../../helper';
87
import storage from '../../storage';
8+
import expectUrlToMatchWithPKCE from '../../__tests__/helper/expect-url-to-match';
99

1010
jest.mock('../../storage');
1111

@@ -57,8 +57,9 @@ describe('api', () => {
5757
onboard(mtLinkSdk.storedOptions);
5858

5959
expect(open).toBeCalledTimes(1);
60-
61-
const query = qs.stringify({
60+
expect(open).toBeCalledWith(expect.any(String), '_self', 'noreferrer');
61+
const url = open.mock.calls[0][0]
62+
const query = {
6263
client_id: clientId,
6364
cobrand_client_id: cobrandClientId,
6465
response_type: 'code',
@@ -67,9 +68,8 @@ describe('api', () => {
6768
country,
6869
locale,
6970
configs: generateConfigs({ email })
70-
});
71-
const url = `${MY_ACCOUNT_DOMAINS.production}/onboard?${query}`;
72-
expect(open).toBeCalledWith(url, '_self', 'noreferrer');
71+
};
72+
expectUrlToMatchWithPKCE(url, {baseUrl: MY_ACCOUNT_DOMAINS.production, path: '/onboard', query})
7373
});
7474

7575
test('with options', () => {
@@ -91,18 +91,19 @@ describe('api', () => {
9191
});
9292

9393
expect(open).toBeCalledTimes(1);
94-
95-
const query = qs.stringify({
94+
expect(open).toBeCalledWith(expect.any(String), '_self', 'noreferrer');
95+
const url = open.mock.calls[0][0]
96+
const query = {
9697
client_id: clientId,
9798
response_type: 'code',
9899
scope: scopes,
99100
redirect_uri: redirectUri,
100101
state,
101102
country,
102103
configs: generateConfigs({ email })
103-
});
104-
const url = `${MY_ACCOUNT_DOMAINS.production}/onboard?${query}`;
105-
expect(open).toBeCalledWith(url, '_self', 'noreferrer');
104+
};
105+
106+
expectUrlToMatchWithPKCE(url, {baseUrl: MY_ACCOUNT_DOMAINS.production, path: '/onboard', query})
106107
});
107108

108109
test('without window', () => {

‎src/api/authorize-url.ts‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,7 @@ export default function authorize(storedOptions: StoredOptions, options: Authori
2020
throw new Error('[mt-link-sdk] Make sure to call `init` before calling `authorizeUrl/authorize`.');
2121
}
2222

23-
const {
24-
scopes = defaultScopes,
25-
redirectUri = defaultRedirectUri,
26-
pkce = false,
27-
codeChallenge,
28-
state,
29-
...rest
30-
} = options;
23+
const { scopes = defaultScopes, redirectUri = defaultRedirectUri, codeChallenge, state, ...rest } = options;
3124

3225
if (!redirectUri) {
3326
throw new Error(
@@ -37,7 +30,7 @@ export default function authorize(storedOptions: StoredOptions, options: Authori
3730

3831
storage.del('cv');
3932

40-
const cc = codeChallenge || (pkce && generateCodeChallenge());
33+
const cc = codeChallenge || generateCodeChallenge();
4134

4235
const queryString = stringify({
4336
client_id: clientId,

‎src/api/onboard-url.ts‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,7 @@ export default function onboardUrl(storedOptions: StoredOptions, options: Onboar
1919
throw new Error('[mt-link-sdk] Make sure to call `init` before calling `onboardUrl/onboard`.');
2020
}
2121

22-
const {
23-
scopes = defaultScopes,
24-
redirectUri = defaultRedirectUri,
25-
pkce = false,
26-
codeChallenge,
27-
state,
28-
...rest
29-
} = options;
22+
const { scopes = defaultScopes, redirectUri = defaultRedirectUri, codeChallenge, state, ...rest } = options;
3023

3124
const configs = mergeConfigs(storedOptions, rest, ['authAction', 'showAuthToggle', 'showRememberMe', 'forceLogout']);
3225

@@ -38,7 +31,7 @@ export default function onboardUrl(storedOptions: StoredOptions, options: Onboar
3831

3932
storage.del('cv');
4033

41-
const cc = codeChallenge || (pkce && generateCodeChallenge());
34+
const cc = codeChallenge || generateCodeChallenge();
4235

4336
const queryString = stringify({
4437
client_id: clientId,

0 commit comments

Comments
 (0)