Skip to content

Commit

Permalink
bugfix #5218: de/encode custom state using base16 (#6851)
Browse files Browse the repository at this point in the history
* bugfix #5218: de/encode custom state using base16

* use aws-sdk hex encoding util

unit tests for url safe encoding/decoding

* avoid using TextEncoder to support IE11

Co-authored-by: Sam Martinez <samlmar@amazon.com>
  • Loading branch information
div5yesh and sammartinez authored Sep 25, 2020
1 parent df83573 commit 8cc5587
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 4 deletions.
3 changes: 2 additions & 1 deletion packages/auth/src/Auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
Parser,
JS,
UniversalStorage,
urlSafeDecode,
} from '@aws-amplify/core';
import {
CookieStorage,
Expand Down Expand Up @@ -1974,7 +1975,7 @@ export class AuthClass {

dispatchAuthEvent(
'customOAuthState',
customState,
urlSafeDecode(customState),
`State for user ${currentUser.getUsername()}`
);
}
Expand Down
13 changes: 10 additions & 3 deletions packages/auth/src/OAuth/OAuth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
CognitoHostedUIIdentityProvider,
} from '../types/Auth';

import { ConsoleLogger as Logger, Hub } from '@aws-amplify/core';
import { ConsoleLogger as Logger, Hub, urlSafeEncode } from '@aws-amplify/core';

import sha256 from 'crypto-js/sha256';
import Base64 from 'crypto-js/enc-base64';
Expand Down Expand Up @@ -78,11 +78,18 @@ export default class OAuth {
customState?: string
) {
const generatedState = this._generateState(32);

/* encodeURIComponent is not URL safe, use urlSafeEncode instead. Cognito
single-encodes/decodes url on first sign in and double-encodes/decodes url
when user already signed in. Using encodeURIComponent, Base32, Base64 add
characters % or = which on further encoding becomes unsafe. '=' create issue
for parsing query params.
Refer: https://github.com/aws-amplify/amplify-js/issues/5218 */
const state = customState
? `${generatedState}-${customState}`
? `${generatedState}-${urlSafeEncode(customState)}`
: generatedState;

oAuthStorage.setState(encodeURIComponent(state));
oAuthStorage.setState(state);

const pkce_key = this._generateRandom(128);
oAuthStorage.setPKCE(pkce_key);
Expand Down
27 changes: 27 additions & 0 deletions packages/core/__tests__/StringUtils-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { urlSafeEncode, urlSafeDecode } from '../src/Util';
import { TextDecoder, TextEncoder } from 'util';
global.TextEncoder = TextEncoder;
global.TextDecoder = TextDecoder;

const complexCustomState = 'https://amplify-app:300/?empty=&list=1,a,%,@';

describe('StringUtils', () => {
test('urlSafe encoding', () => {
const urlSafeState = urlSafeEncode(complexCustomState);
expect(encodeURIComponent(urlSafeState)).toEqual(urlSafeState);
});
test('urlSafe decoding', () => {
const urlSafeState = urlSafeEncode(complexCustomState);
const encodedState = encodeURIComponent(urlSafeState);

expect(decodeURIComponent(encodedState)).toEqual(encodedState);
});
test('URI encode/decode with url safe state', () => {
const urlSafeState = urlSafeEncode(complexCustomState);
const encodedState = encodeURIComponent(urlSafeState);

expect(urlSafeDecode(decodeURIComponent(encodedState))).toEqual(
complexCustomState
);
});
});
18 changes: 18 additions & 0 deletions packages/core/src/Util/StringUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
export function urlSafeEncode(str: string) {
return str
.split('')
.map(char =>
char
.charCodeAt(0)
.toString(16)
.padStart(2, '0')
)
.join('');
}

export function urlSafeDecode(hex: string) {
return hex
.match(/.{2}/g)
.map(char => String.fromCharCode(parseInt(char, 16)))
.join('');
}
1 change: 1 addition & 0 deletions packages/core/src/Util/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ export * from './Retry';
export { default as Mutex } from './Mutex';
export { default as Reachability } from './Reachability';
export * from './DateUtils';
export * from './StringUtils';

0 comments on commit 8cc5587

Please sign in to comment.