Skip to content

Commit

Permalink
feat(auth): HostedUI oidc signout (#13512)
Browse files Browse the repository at this point in the history
* chore(auth): add oauth metadata into token orchestrator (#13712) (#13736)

* chore: add oauth metadata into token orchestrator

* chore: add unit tests

* chore: address feedback

* 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: enable react native tests with localhost server

* chore: enable subdomain test

* chore: update some function calls in tests

* chore: minor reverts

* fix: unit tests fail on mehtod params

* chore: revert ppush branch

* chore: remove subdomain test rdundant

* chore: upadte step name

* chore: reflect API changes and clean up

* chore: revert unintented change glob

* chore: bundle size minor adjustments

* chore: move localhost page hosting to RN script in the app

* chore: revert unintended change

* chore: revert branch name for integ test

---------

Co-authored-by: israx <70438514+israx@users.noreply.github.com>
Co-authored-by: AllanZhengYP <zheallan@amazon.com>
  • Loading branch information
3 people committed Sep 3, 2024
1 parent a363362 commit e8fb997
Show file tree
Hide file tree
Showing 22 changed files with 463 additions and 199 deletions.
4 changes: 4 additions & 0 deletions .github/integ-config/detox-integ-all.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,7 @@
- test_name: 'integ_rn_ios_api_v6_rn_72_detox_cli'
working_directory: amplify-js-samples-staging/samples/react-native/api/v6/ApiGRAPHQL
timeout_minutes: 120
- test_name: 'integ_rn_ios_oidc_signout'
working_directory: amplify-js-samples-staging/samples/react-native/auth/HosteduiApp
timeout_minutes: 120
host_signout_page: true
7 changes: 7 additions & 0 deletions .github/integ-config/integ-all.yml
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,13 @@ tests:
sample_name: [sign-in-with-oauth]
spec: sign-in-with-oauth
browser: [chrome]
- test_name: integ_vue_sign_out_of_oidc_provider
desc: 'Sign-out of OIDC provider'
framework: vue
category: auth
sample_name: [sign-in-with-oauth]
spec: sign-out-oidc-provider
browser: [chrome]

# AUTH GEN2
- test_name: integ_react_javascript_authentication_gen2
Expand Down
9 changes: 9 additions & 0 deletions .github/workflows/callable-e2e-test-detox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ on:
timeout_minutes:
required: true
type: number
host_signout_page:
required: false
type: boolean
default: false

jobs:
e2e-test:
Expand Down Expand Up @@ -70,6 +74,11 @@ jobs:
JEST_JUNIT_OUTPUT_NAME: detox-test-results.xml
working-directory: ${{ inputs.working_directory }}
shell: bash
- name: Start the http-server and host the oidc signout page locally (background).
if: ${{ inputs.host_signout_page }}
run: yarn host:signout
working-directory: ${{ inputs.working_directory }}
shell: bash
- name: Detox run
run: |
$GITHUB_WORKSPACE/amplify-js/scripts/retry-yarn-script.sh -s 'detox test -c ios.sim.debug -u' -n 3
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/callable-e2e-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,4 @@ jobs:
test_name: ${{ matrix.integ-config.test_name }}
working_directory: ${{ matrix.integ-config.working_directory }}
timeout_minutes: ${{ matrix.integ-config.timeout_minutes || 45 }}
host_signout_page: ${{ matrix.integ-config.host_signout_page || false }}
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -137,5 +137,8 @@
"nx": "16.7.0",
"xml2js": "0.5.0",
"tar": "6.2.1"
},
"overrides": {
"tar": "6.2.1"
}
}
1 change: 1 addition & 0 deletions packages/auth/__tests__/providers/cognito/signOut.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ describe('signOut', () => {
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
@@ -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 redirectUrl is not provided', () => {
expect(getRedirectUrl(mockRedirectUrls)).toStrictEqual(mockRedirectUrls[0]);
});

it('should return redirectUrl 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 nor https as prefix irrespective of a redirectUrl 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 redirectUrl 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 redirectUrl 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
@@ -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/tokenProvider';
import { completeOAuthSignOut } from '../../../../../src/providers/cognito/utils/oauth/completeOAuthSignOut';
import { handleOAuthSignOut } from '../../../../../src/providers/cognito/utils/oauth/handleOAuthSignOut.native';
import { oAuthSignOutRedirect } from '../../../../../src/providers/cognito/utils/oauth/oAuthSignOutRedirect';
Expand All @@ -23,6 +24,9 @@ describe('handleOAuthSignOut (native)', () => {
// assert mocks
const mockCompleteOAuthSignOut = completeOAuthSignOut as jest.Mock;
const mockOAuthSignOutRedirect = oAuthSignOutRedirect as jest.Mock;
const mockTokenOrchestrator = tokenOrchestrator as jest.Mocked<
typeof tokenOrchestrator
>;
// create mocks
const mockStore = {
loadOAuthSignIn: jest.fn(),
Expand All @@ -43,33 +47,51 @@ describe('handleOAuthSignOut (native)', () => {
});
it('should complete OAuth sign out and redirect', async () => {
mockOAuthSignOutRedirect.mockResolvedValue({ type: 'success' });
await handleOAuthSignOut(cognitoConfig, mockStore);
await handleOAuthSignOut(
cognitoConfig,
mockStore,
mockTokenOrchestrator,
undefined,
);

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

it('should not complete OAuth sign out if redirect is canceled', async () => {
mockOAuthSignOutRedirect.mockResolvedValue({ type: 'canceled' });
await handleOAuthSignOut(cognitoConfig, mockStore);
await handleOAuthSignOut(
cognitoConfig,
mockStore,
mockTokenOrchestrator,
undefined,
);

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

it('should not complete OAuth sign out if redirect failed', async () => {
mockOAuthSignOutRedirect.mockResolvedValue({ type: 'error' });
await handleOAuthSignOut(cognitoConfig, mockStore);
await handleOAuthSignOut(
cognitoConfig,
mockStore,
mockTokenOrchestrator,
undefined,
);

expect(mockOAuthSignOutRedirect).toHaveBeenCalledWith(
cognitoConfig,
false,
undefined,
);
expect(mockCompleteOAuthSignOut).not.toHaveBeenCalled();
});
Expand All @@ -81,9 +103,18 @@ describe('handleOAuthSignOut (native)', () => {
preferPrivateSession: true,
});
mockOAuthSignOutRedirect.mockResolvedValue({ type: 'error' });
await handleOAuthSignOut(cognitoConfig, mockStore);
await handleOAuthSignOut(
cognitoConfig,
mockStore,
mockTokenOrchestrator,
undefined,
);

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

Expand All @@ -92,7 +123,12 @@ describe('handleOAuthSignOut (native)', () => {
isOAuthSignIn: false,
preferPrivateSession: false,
});
await handleOAuthSignOut(cognitoConfig, mockStore);
await handleOAuthSignOut(
cognitoConfig,
mockStore,
mockTokenOrchestrator,
undefined,
);

expect(mockOAuthSignOutRedirect).not.toHaveBeenCalled();
expect(mockCompleteOAuthSignOut).toHaveBeenCalledWith(mockStore);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,19 @@ describe('handleOAuthSignOut', () => {
isOAuthSignIn: true,
preferPrivateSession: false,
});
await handleOAuthSignOut(cognitoConfig, mockStore, mockTokenOrchestrator);
await handleOAuthSignOut(
cognitoConfig,
mockStore,
mockTokenOrchestrator,
undefined,
);

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 () => {
Expand All @@ -59,18 +68,32 @@ describe('handleOAuthSignOut', () => {
isOAuthSignIn: false,
preferPrivateSession: false,
});
await handleOAuthSignOut(cognitoConfig, mockStore, mockTokenOrchestrator);
await handleOAuthSignOut(
cognitoConfig,
mockStore,
mockTokenOrchestrator,
undefined,
);

expect(mockCompleteOAuthSignOut).toHaveBeenCalledWith(mockStore);
expect(mockOAuthSignOutRedirect).toHaveBeenCalledWith(cognitoConfig);
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, mockTokenOrchestrator);
await handleOAuthSignOut(
cognitoConfig,
mockStore,
mockTokenOrchestrator,
undefined,
);

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
5 changes: 2 additions & 3 deletions packages/auth/src/providers/cognito/apis/signOut.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ export async function signOut(input?: SignOutInput): Promise<void> {
} catch (err) {
hasOAuthConfig = false;
}

if (hasOAuthConfig) {
const oAuthStore = new DefaultOAuthStore(defaultStorage);
oAuthStore.setAuthConfig(cognitoConfig);
Expand All @@ -69,12 +68,12 @@ export async function signOut(input?: SignOutInput): Promise<void> {
cognitoConfig,
oAuthStore,
tokenOrchestrator,
input?.oauth?.redirectUrl,
)) ?? {};
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.`,
});
}
} else {
Expand Down
Loading

0 comments on commit e8fb997

Please sign in to comment.