Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(auth): HostedUI oidc signout #13512

Merged
merged 38 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
c45bdfd
chore(auth): add oauth metadata into token orchestrator (#13712) (#13…
israx Aug 20, 2024
40d0b21
wip: hardcode signout uri for poc
Samaritan1011001 May 9, 2024
a6c710c
chore: expose the prefferedRedirectSignOutUrl
Samaritan1011001 May 13, 2024
6207d8d
chore: add prefered url change to native file
Samaritan1011001 May 13, 2024
1f043fd
chore: correct param name
Samaritan1011001 May 13, 2024
49eec47
chore: update getRedirectUrl function to consider preferred url
Samaritan1011001 Jun 11, 2024
4c47c15
chore: add unit test for the feature
Samaritan1011001 Jun 17, 2024
cbec22f
chore: update input type to use the accepted format
Samaritan1011001 Jun 18, 2024
932cfbb
chore: review comments
Samaritan1011001 Jun 25, 2024
569da99
fix: address npm audit issues
Samaritan1011001 Jul 11, 2024
1d1167a
chore: update comments, bundle size and rn version
Samaritan1011001 Jul 18, 2024
328b2d2
chore: update bundle size limit
Samaritan1011001 Jul 18, 2024
e08b285
chore: update bundle size limit
Samaritan1011001 Jul 18, 2024
b0983f7
chore: address coments and rename a param to getRedirecturl funciton
Samaritan1011001 Jul 23, 2024
2aaabf1
chore: make preid release ready
Samaritan1011001 Jul 24, 2024
567cccf
chore: update yarn.lock
Samaritan1011001 Jul 24, 2024
17fa0fc
chore: add test and update push-integ branch
Samaritan1011001 Jul 29, 2024
3caf098
chore: revert preid release updates
Samaritan1011001 Jul 29, 2024
8be0c43
chore: update sample name
Samaritan1011001 Jul 29, 2024
6237797
chore: enable react native tests with localhost server
Samaritan1011001 Aug 21, 2024
e29b60c
Merge branch 'feat/hosted-ui-sign-out' into fix/redirect-rn
Samaritan1011001 Aug 21, 2024
01fbba6
chore: enable subdomain test
Samaritan1011001 Aug 21, 2024
a0a6644
chore: update some function calls in tests
Samaritan1011001 Aug 21, 2024
1238421
chore: minor reverts
Samaritan1011001 Aug 21, 2024
05e4536
fix: unit tests fail on mehtod params
Samaritan1011001 Aug 22, 2024
64dc9d9
chore: revert ppush branch
Samaritan1011001 Aug 22, 2024
31328b7
Merge branch 'main' into fix/redirect-rn
Samaritan1011001 Aug 22, 2024
8b0c176
chore: remove subdomain test rdundant
Samaritan1011001 Aug 22, 2024
2ba5466
chore: upadte step name
Samaritan1011001 Aug 23, 2024
e2966dc
Merge branch 'main' into fix/redirect-rn
Samaritan1011001 Aug 29, 2024
e41ec5a
chore: reflect API changes and clean up
Samaritan1011001 Aug 29, 2024
ea55b87
chore: revert unintented change glob
Samaritan1011001 Aug 29, 2024
4cf5652
chore: bundle size minor adjustments
Samaritan1011001 Aug 29, 2024
a0b55d3
chore: move localhost page hosting to RN script in the app
Samaritan1011001 Aug 29, 2024
611aa61
chore: revert unintended change
Samaritan1011001 Aug 30, 2024
4ed8f7d
Merge branch 'main' into fix/redirect-rn
AllanZhengYP Sep 3, 2024
26a4920
chore: revert branch name for integ test
Samaritan1011001 Sep 3, 2024
f728aa4
Merge branch 'main' into fix/redirect-rn
Samaritan1011001 Sep 3, 2024
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
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
14 changes: 14 additions & 0 deletions .github/integ-config/integ-all.yml
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,20 @@ 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]
- test_name: subdomain_authentication
desc: 'Sign-in with the OAuth flow and subdomains'
framework: next
category: auth
sample_name: [subdomains]
spec: subdomains
browser: [chrome]

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

jobs:
e2e-test:
name: E2E-Detox ${{ inputs.test_name }}
runs-on: macos-latest
runs-on: macos-latest-large
Samaritan1011001 marked this conversation as resolved.
Show resolved Hide resolved
timeout-minutes: ${{ inputs.timeout_minutes }}

steps:
Expand Down Expand Up @@ -70,6 +74,14 @@ 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)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Step to boot the localhost server.

if: ${{ inputs.host_signout_page }}
run: |
cd oidc-signout-hosted-page/ &&
npx http-server -p 8000 &
Samaritan1011001 marked this conversation as resolved.
Show resolved Hide resolved
sleep 60
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 }}
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
- fix/redirect-rn
Samaritan1011001 marked this conversation as resolved.
Show resolved Hide resolved

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 }}
Samaritan1011001 marked this conversation as resolved.
Show resolved Hide resolved
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"
Samaritan1011001 marked this conversation as resolved.
Show resolved Hide resolved
}
}
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,
undefined,
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
@@ -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', () => {
Samaritan1011001 marked this conversation as resolved.
Show resolved Hide resolved
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,36 @@ 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,
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
Loading
Loading