Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ describe('signOut', () => {
expect(mockHandleOAuthSignOut).toHaveBeenCalledWith(
cognitoConfigWithOauth,
mockDefaultOAuthStoreInstance,
mockTokenOrchestrator,
);
// In cases of OAuth, token removal and Hub dispatch should be performed by the OAuth handling since
// these actions can be deferred or canceled out of altogether.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ const mockAuthTokenStore = {
setKeyValueStorage: jest.fn(),
getDeviceMetadata: jest.fn(),
clearDeviceMetadata: jest.fn(),
setOAuthMetadata: jest.fn(),
getOAuthMetadata: jest.fn(),
};
const mockTokenRefresher = jest.fn();
const validAuthConfig: ResourcesConfig = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ describe('tokenOrchestrator', () => {
setKeyValueStorage: jest.fn(),
getDeviceMetadata: jest.fn(),
clearDeviceMetadata: jest.fn(),
getOAuthMetadata: jest.fn(),
setOAuthMetadata: jest.fn(),
};

beforeAll(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { AuthErrorTypes } from '../../../../../src/types/Auth';
import { OAuthStore } from '../../../../../src/providers/cognito/utils/types';
import { completeOAuthFlow } from '../../../../../src/providers/cognito/utils/oauth/completeOAuthFlow';

jest.mock('../../../../../src/providers/cognito/tokenProvider');
jest.mock('@aws-amplify/core', () => ({
Hub: {
dispatch: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

import { TokenOrchestrator } from '../../../../../src/providers/cognito';
import { completeOAuthSignOut } from '../../../../../src/providers/cognito/utils/oauth/completeOAuthSignOut';
import { handleOAuthSignOut } from '../../../../../src/providers/cognito/utils/oauth/handleOAuthSignOut';
import { oAuthSignOutRedirect } from '../../../../../src/providers/cognito/utils/oauth/oAuthSignOutRedirect';
Expand All @@ -12,6 +13,7 @@ jest.mock(
jest.mock(
'../../../../../src/providers/cognito/utils/oauth/oAuthSignOutRedirect',
);
jest.mock('../../../../../src/providers/cognito/tokenProvider');

describe('handleOAuthSignOut', () => {
const region = 'us-west-2';
Expand All @@ -27,9 +29,13 @@ describe('handleOAuthSignOut', () => {
const mockStore = {
loadOAuthSignIn: jest.fn(),
} as unknown as jest.Mocked<DefaultOAuthStore>;
const mockTokenOrchestrator = {
getOAuthMetadata: jest.fn(),
} as unknown as jest.Mocked<TokenOrchestrator>;

afterEach(() => {
mockStore.loadOAuthSignIn.mockReset();
mockTokenOrchestrator.getOAuthMetadata.mockReset();
mockCompleteOAuthSignOut.mockClear();
mockOAuthSignOutRedirect.mockClear();
});
Expand All @@ -39,7 +45,21 @@ describe('handleOAuthSignOut', () => {
isOAuthSignIn: true,
preferPrivateSession: false,
});
await handleOAuthSignOut(cognitoConfig, mockStore);
await handleOAuthSignOut(cognitoConfig, mockStore, mockTokenOrchestrator);

expect(mockCompleteOAuthSignOut).toHaveBeenCalledWith(mockStore);
expect(mockOAuthSignOutRedirect).toHaveBeenCalledWith(cognitoConfig);
});

it('should complete OAuth sign out and redirect when there oauth metadata in tokenOrchestrator', async () => {
mockTokenOrchestrator.getOAuthMetadata.mockResolvedValue({
oauthSignIn: true,
});
mockStore.loadOAuthSignIn.mockResolvedValue({
isOAuthSignIn: false,
preferPrivateSession: false,
});
await handleOAuthSignOut(cognitoConfig, mockStore, mockTokenOrchestrator);

expect(mockCompleteOAuthSignOut).toHaveBeenCalledWith(mockStore);
expect(mockOAuthSignOutRedirect).toHaveBeenCalledWith(cognitoConfig);
Expand All @@ -50,7 +70,7 @@ describe('handleOAuthSignOut', () => {
isOAuthSignIn: false,
preferPrivateSession: false,
});
await handleOAuthSignOut(cognitoConfig, mockStore);
await handleOAuthSignOut(cognitoConfig, mockStore, mockTokenOrchestrator);

expect(mockCompleteOAuthSignOut).toHaveBeenCalledWith(mockStore);
expect(mockOAuthSignOutRedirect).not.toHaveBeenCalled();
Expand Down
6 changes: 5 additions & 1 deletion packages/auth/src/providers/cognito/apis/signOut.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@ export async function signOut(input?: SignOutInput): Promise<void> {
const oAuthStore = new DefaultOAuthStore(defaultStorage);
oAuthStore.setAuthConfig(cognitoConfig);
const { type } =
(await handleOAuthSignOut(cognitoConfig, oAuthStore)) ?? {};
(await handleOAuthSignOut(
cognitoConfig,
oAuthStore,
tokenOrchestrator,
)) ?? {};
if (type === 'error') {
throw new AuthError({
name: OAUTH_SIGNOUT_EXCEPTION,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
AuthTokenStore,
CognitoAuthTokens,
DeviceMetadata,
OAuthMetadata,
TokenRefresher,
} from './types';

Expand Down Expand Up @@ -203,4 +204,12 @@ export class TokenOrchestrator implements AuthTokenOrchestrator {
clearDeviceMetadata(username?: string): Promise<void> {
return this.getTokenStore().clearDeviceMetadata(username);
}

setOAuthMetadata(metadata: OAuthMetadata): Promise<void> {
return this.getTokenStore().setOAuthMetadata(metadata);
}

getOAuthMetadata(): Promise<OAuthMetadata | null> {
return this.getTokenStore().getOAuthMetadata();
}
}
18 changes: 18 additions & 0 deletions packages/auth/src/providers/cognito/tokenProvider/TokenStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
AuthTokenStore,
CognitoAuthTokens,
DeviceMetadata,
OAuthMetadata,
} from './types';
import { TokenProviderErrorCode, assert } from './errorHelpers';

Expand Down Expand Up @@ -163,6 +164,7 @@ export class DefaultTokenStore implements AuthTokenStore {
this.getKeyValueStorage().removeItem(authKeys.refreshToken),
this.getKeyValueStorage().removeItem(authKeys.signInDetails),
this.getKeyValueStorage().removeItem(this.getLastAuthUserKey()),
this.getKeyValueStorage().removeItem(authKeys.oauthMetadata),
]);
}

Expand Down Expand Up @@ -222,6 +224,22 @@ export class DefaultTokenStore implements AuthTokenStore {

return lastAuthUser;
}

async setOAuthMetadata(metadata: OAuthMetadata): Promise<void> {
const { oauthMetadata: oauthMetadataKey } = await this.getAuthKeys();
await this.getKeyValueStorage().setItem(
oauthMetadataKey,
JSON.stringify(metadata),
);
}

async getOAuthMetadata(): Promise<OAuthMetadata | null> {
const { oauthMetadata: oauthMetadataKey } = await this.getAuthKeys();
const oauthMetadata =
await this.getKeyValueStorage().getItem(oauthMetadataKey);

return oauthMetadata && JSON.parse(oauthMetadata);
}
}

export const createKeysForAuthStorage = (
Expand Down
9 changes: 9 additions & 0 deletions packages/auth/src/providers/cognito/tokenProvider/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export const AuthTokenStorageKeys = {
randomPasswordKey: 'randomPasswordKey',
deviceGroupKey: 'deviceGroupKey',
signInDetails: 'signInDetails',
oauthMetadata: 'oauthMetadata',
};

export interface AuthTokenStore {
Expand All @@ -44,6 +45,8 @@ export interface AuthTokenStore {
setKeyValueStorage(keyValueStorage: KeyValueStorageInterface): void;
getDeviceMetadata(username?: string): Promise<DeviceMetadata | null>;
clearDeviceMetadata(username?: string): Promise<void>;
setOAuthMetadata(metadata: OAuthMetadata): Promise<void>;
getOAuthMetadata(): Promise<OAuthMetadata | null>;
}

export interface AuthTokenOrchestrator {
Expand All @@ -58,6 +61,8 @@ export interface AuthTokenOrchestrator {
clearTokens(): Promise<void>;
getDeviceMetadata(username?: string): Promise<DeviceMetadata | null>;
clearDeviceMetadata(username?: string): Promise<void>;
setOAuthMetadata(metadata: OAuthMetadata): Promise<void>;
getOAuthMetadata(): Promise<OAuthMetadata | null>;
}

export interface CognitoUserPoolTokenProviderType extends TokenProvider {
Expand All @@ -78,3 +83,7 @@ export interface DeviceMetadata {
deviceGroupKey?: string;
randomPassword: string;
}

export interface OAuthMetadata {
oauthSignIn: boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { Hub, decodeJWT } from '@aws-amplify/core';

import { cacheCognitoTokens } from '../../tokenProvider/cacheTokens';
import { dispatchSignedInHubEvent } from '../dispatchSignedInHubEvent';
import { tokenOrchestrator } from '../../tokenProvider';

import { createOAuthError } from './createOAuthError';
import { resolveAndClearInflightPromises } from './inflightPromise';
Expand Down Expand Up @@ -227,6 +228,9 @@ const completeFlow = async ({
redirectUri: string;
state: string;
}) => {
await tokenOrchestrator.setOAuthMetadata({
oauthSignIn: true,
});
await oAuthStore.clearOAuthData();
await oAuthStore.storeOAuthSignIn(true, preferPrivateSession);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,30 @@ import { CognitoUserPoolConfig } from '@aws-amplify/core';

import { OpenAuthSessionResult } from '../../../../utils/types';
import { DefaultOAuthStore } from '../../utils/signInWithRedirectStore';
import { TokenOrchestrator } from '../../tokenProvider';

import { completeOAuthSignOut } from './completeOAuthSignOut';
import { oAuthSignOutRedirect } from './oAuthSignOutRedirect';

export const handleOAuthSignOut = async (
cognitoConfig: CognitoUserPoolConfig,
store: DefaultOAuthStore,
tokenOrchestrator: TokenOrchestrator,
): Promise<void | OpenAuthSessionResult> => {
const { isOAuthSignIn } = await store.loadOAuthSignIn();
const oauthMetadata = await tokenOrchestrator.getOAuthMetadata();

// Clear everything before attempting to visted logout endpoint since the current application
// state could be wiped away on redirect
await completeOAuthSignOut(store);

if (isOAuthSignIn) {
// The isOAuthSignIn flag is propagated by the oAuthToken store which manages oauth keys in local storage only.
// These keys are used to determine if a user is in an inflight or signedIn oauth states.
// However, this behavior represents an issue when 2 apps share the same set of tokens in Cookie storage because the app that didn't
// start the OAuth will not have access to the oauth keys.
// A heuristic solution is to add oauth metadata to the tokenOrchestrator which will have access to the underlying
// storage mechanism that is used by Amplify.
if (isOAuthSignIn || oauthMetadata?.oauthSignIn) {
// On web, this will always end up being a void action
return oAuthSignOutRedirect(cognitoConfig);
}
Expand Down