Skip to content

Commit

Permalink
chore: enable subdomain authentication test in ci (#13733)
Browse files Browse the repository at this point in the history
* wip: hardcode signout uri for poc

* chore: expose the prefferedRedirectSignOutUrl

* chore: add prefered url change to native file

* chore: correct param name

* chore: update getRedirectUrl function to consider preferred url

* chore: add unit test for the feature

* chore: update input type to use the accepted format

* chore: review comments

* fix: address npm audit issues

* chore: update comments, bundle size and rn version

* chore: update bundle size limit

* chore: update bundle size limit

* chore: address coments and rename a param to getRedirecturl funciton

* chore: make preid release ready

* chore: update yarn.lock

* chore: add test and update push-integ branch

* chore: revert preid release updates

* chore: update sample name

* chore: update yarn.lock

* chore(auth): add oauth metadata into token orchestrator (#13712)

* chore: add oauth metadata into token orchestrator

* chore: add unit tests

* chore: address feedback

* chore: fix unit tests

* chore: enable tests in ci

---------

Co-authored-by: ManojNB <[email protected]>
  • Loading branch information
israx and Samaritan1011001 authored Aug 20, 2024
1 parent 5224dc2 commit 3ab9863
Show file tree
Hide file tree
Showing 28 changed files with 425 additions and 1,314 deletions.
849 changes: 4 additions & 845 deletions .github/integ-config/integ-all.yml

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion .github/workflows/push-integ-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ concurrency:
on:
push:
branches:
- replace-with-your-branch
- test/subdomain-authentication

jobs:
e2e:
Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/push-preid-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,4 @@ jobs:
uses: ./.github/workflows/callable-npm-publish-preid.yml
# The preid should be detected from the branch name recommending feat/{PREID}/whatever as branch naming pattern
# if your branch doesn't follow this pattern, you can override it here for your branch.
with:
preid: ${{ needs.parse-preid.outputs.preid }}
with: preid:${{ needs.parse-preid.outputs.preid }}
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -138,5 +138,8 @@
"xml2js": "0.5.0",
"tar": "6.2.1",
"glob@^10.0.0": "10.3.10"
},
"overrides": {
"tar": "6.2.1"
}
}
2 changes: 2 additions & 0 deletions packages/auth/__tests__/providers/cognito/signOut.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ describe('signOut', () => {
expect(mockHandleOAuthSignOut).toHaveBeenCalledWith(
cognitoConfigWithOauth,
mockDefaultOAuthStoreInstance,
mockTokenOrchestrator,
undefined,
);
// 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
@@ -0,0 +1,35 @@
import { invalidAppSchemeException } from '../../../../../src/errors/constants';
import { getRedirectUrl } from '../../../../../src/providers/cognito/utils/oauth/getRedirectUrl.native';

describe('getRedirectUrl (native)', () => {
const mockRedirectUrls = [
'myDevApp://',
'myProdApp://',
'https://intermidiateSite.com',
];

it('should return the first non http/s url from the array when preferredRedirectUrl is not provided', () => {
expect(getRedirectUrl(mockRedirectUrls)).toStrictEqual(mockRedirectUrls[0]);
});

it('should return preferredRedirectUrl if it matches at least one of the redirect urls from config', () => {
const configRedirectUrl = mockRedirectUrls[2];

expect(getRedirectUrl(mockRedirectUrls, configRedirectUrl)).toStrictEqual(
configRedirectUrl,
);
});

it('should throw an exception when there is no url with no http or https as prefix irrespective if a preferredSignOutUrl is given or not', () => {
const mockRedirectUrlsWithNoAppScheme = ['https://intermidiateSite.com'];
expect(() =>
getRedirectUrl(
mockRedirectUrlsWithNoAppScheme,
mockRedirectUrlsWithNoAppScheme[0],
),
).toThrow(invalidAppSchemeException);
expect(() => getRedirectUrl(mockRedirectUrlsWithNoAppScheme)).toThrow(
invalidAppSchemeException,
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import { getRedirectUrl } from '../../../../../src/providers/cognito/utils/oauth';
import {
invalidOriginException,
invalidPreferredRedirectUrlException,
invalidRedirectException,
} from '../../../../../src/errors/constants';

describe('getRedirectUrl', () => {
const mockRedirectUrls = ['https://example.com/app'];
let windowSpy: jest.SpyInstance;

beforeEach(() => {
windowSpy = jest.spyOn(window, 'window', 'get');
});

afterEach(() => {
windowSpy.mockRestore();
});

it('should return the redirect url that has the same origin and same pathName', () => {
windowSpy.mockReturnValue({
location: {
origin: 'https://example.com/',
pathname: 'app',
},
});
expect(getRedirectUrl(mockRedirectUrls)).toStrictEqual(mockRedirectUrls[0]);
});

it('should throw an invalid origin exception if there is no url that is the same origin and pathname', () => {
windowSpy.mockReturnValue({
location: {
origin: 'https://differentOrigin.com/',
pathname: 'differentApp',
},
});
expect(() => getRedirectUrl(mockRedirectUrls)).toThrow(
invalidOriginException,
);
});

it('should throw an invalid redirect exception if there is no url that is the same origin/pathname and is also not http or https', () => {
const mockNonHttpRedirectUrls = ['test-non-http-string'];
windowSpy.mockReturnValue({
location: {
origin: 'https://differentOrigin.com/',
pathname: 'differentApp',
},
});
expect(() => getRedirectUrl(mockNonHttpRedirectUrls)).toThrow(
invalidRedirectException,
);
});

it('should return the preferredRedirectUrl if it is provided and matches one of the redirectUrls from config', () => {
expect(getRedirectUrl(mockRedirectUrls, mockRedirectUrls[0])).toStrictEqual(
mockRedirectUrls[0],
);
});

it('should throw an exception if preferredRedirectUrl is given but does not match any of the redirectUrls from config', () => {
expect(() =>
getRedirectUrl(mockRedirectUrls, 'https://unknownOrigin.com'),
).toThrow(invalidPreferredRedirectUrlException);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ describe('handleOAuthSignOut (native)', () => {
expect(mockOAuthSignOutRedirect).toHaveBeenCalledWith(
cognitoConfig,
false,
undefined,
);
expect(mockCompleteOAuthSignOut).toHaveBeenCalledWith(mockStore);
});
Expand All @@ -59,6 +60,7 @@ describe('handleOAuthSignOut (native)', () => {
expect(mockOAuthSignOutRedirect).toHaveBeenCalledWith(
cognitoConfig,
false,
undefined,
);
expect(mockCompleteOAuthSignOut).not.toHaveBeenCalled();
});
Expand All @@ -70,6 +72,7 @@ describe('handleOAuthSignOut (native)', () => {
expect(mockOAuthSignOutRedirect).toHaveBeenCalledWith(
cognitoConfig,
false,
undefined,
);
expect(mockCompleteOAuthSignOut).not.toHaveBeenCalled();
});
Expand All @@ -81,9 +84,13 @@ describe('handleOAuthSignOut (native)', () => {
preferPrivateSession: true,
});
mockOAuthSignOutRedirect.mockResolvedValue({ type: 'error' });
await handleOAuthSignOut(cognitoConfig, mockStore);
await handleOAuthSignOut(cognitoConfig, mockStore, undefined);

expect(mockOAuthSignOutRedirect).toHaveBeenCalledWith(cognitoConfig, true);
expect(mockOAuthSignOutRedirect).toHaveBeenCalledWith(
cognitoConfig,
true,
undefined,
);
expect(mockCompleteOAuthSignOut).toHaveBeenCalledWith(mockStore);
});

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,18 +45,40 @@ describe('handleOAuthSignOut', () => {
isOAuthSignIn: true,
preferPrivateSession: false,
});
await handleOAuthSignOut(cognitoConfig, mockStore);
await handleOAuthSignOut(cognitoConfig, mockStore, mockTokenOrchestrator);

expect(mockCompleteOAuthSignOut).toHaveBeenCalledWith(mockStore);
expect(mockOAuthSignOutRedirect).toHaveBeenCalledWith(cognitoConfig);
expect(mockOAuthSignOutRedirect).toHaveBeenCalledWith(
cognitoConfig,
false,
undefined,
);
});

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,
false,
undefined,
);
});

it('should complete OAuth sign out but not redirect', async () => {
mockStore.loadOAuthSignIn.mockResolvedValue({
isOAuthSignIn: false,
preferPrivateSession: false,
});
await handleOAuthSignOut(cognitoConfig, mockStore);
await handleOAuthSignOut(cognitoConfig, mockStore, mockTokenOrchestrator);

expect(mockCompleteOAuthSignOut).toHaveBeenCalledWith(mockStore);
expect(mockOAuthSignOutRedirect).not.toHaveBeenCalled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ describe('oAuthSignOutRedirect', () => {

expect(mockGetRedirectUrl).toHaveBeenCalledWith(
authConfig.loginWith.oauth.redirectSignOut,
undefined,
);
expect(mockOpenAuthSession).toHaveBeenCalledWith(
`https://${domain}/logout?client_id=${userPoolClientId}&logout_uri=${encodedSignOutRedirectUrl}`,
Expand Down
17 changes: 17 additions & 0 deletions packages/auth/src/errors/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,30 @@ export const DEVICE_METADATA_NOT_FOUND_EXCEPTION =
'DeviceMetadataNotFoundException';
export const AUTO_SIGN_IN_EXCEPTION = 'AutoSignInException';
export const INVALID_REDIRECT_EXCEPTION = 'InvalidRedirectException';
export const INVALID_APP_SCHEME_EXCEPTION = 'InvalidAppSchemeException';
export const INVALID_PREFERRED_REDIRECT_EXCEPTION =
'InvalidPreferredRedirectUrlException';

export const invalidRedirectException = new AuthError({
name: INVALID_REDIRECT_EXCEPTION,
message:
'signInRedirect or signOutRedirect had an invalid format or was not found.',
recoverySuggestion:
'Please make sure the signIn/Out redirect in your oauth config is valid.',
});
export const invalidAppSchemeException = new AuthError({
name: INVALID_APP_SCHEME_EXCEPTION,
message: 'A valid non-http app scheme was not found in the config.',
recoverySuggestion:
'Please make sure a valid custom app scheme is present in the config.',
});
export const invalidPreferredRedirectUrlException = new AuthError({
name: INVALID_PREFERRED_REDIRECT_EXCEPTION,
message:
'The given preferredRedirectUrl does not match any items in the redirectSignOutUrls array from the config.',
recoverySuggestion:
'Please make sure a matching preferredRedirectUrl is provided.',
});
export const INVALID_ORIGIN_EXCEPTION = 'InvalidOriginException';
export const invalidOriginException = new AuthError({
name: INVALID_ORIGIN_EXCEPTION,
Expand Down
13 changes: 8 additions & 5 deletions packages/auth/src/providers/cognito/apis/signOut.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,20 @@ export async function signOut(input?: SignOutInput): Promise<void> {
} catch (err) {
hasOAuthConfig = false;
}

if (hasOAuthConfig) {
const oAuthStore = new DefaultOAuthStore(defaultStorage);
oAuthStore.setAuthConfig(cognitoConfig);
const { type } =
(await handleOAuthSignOut(cognitoConfig, oAuthStore)) ?? {};
const { type, error } =
(await handleOAuthSignOut(
cognitoConfig,
oAuthStore,
tokenOrchestrator,
input?.oauth?.preferredSignOutUrl,
)) ?? {};
if (type === 'error') {
throw new AuthError({
name: OAUTH_SIGNOUT_EXCEPTION,
message:
'An error occurred when attempting to log out from OAuth provider.',
message: `An error occurred when attempting to log out from OAuth provider. ${error}`,
});
}
} else {
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();
}
}
Loading

0 comments on commit 3ab9863

Please sign in to comment.