chore: address coments and rename a param to getRedirecturl funciton

aws-amplify · Jul 24, 2024 · 3fefc50 · 3fefc50
1 parent afe817d
commit 3fefc50
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 21 deletions.
diff --git a/packages/auth/__tests__/providers/cognito/utils/oauth/getRedirectUrl.test.ts b/packages/auth/__tests__/providers/cognito/utils/oauth/getRedirectUrl.test.ts
@@ -18,35 +18,35 @@ describe('getRedirectUrl', () => {
 	});
 
 	it('should return the redirect url that has the same origin and same pathName', () => {
-		windowSpy.mockImplementation(() => ({
+		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.mockImplementation(() => ({
+		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.mockImplementation(() => ({
+		windowSpy.mockReturnValue({
 			location: {
 				origin: 'https://differentOrigin.com/',
 				pathname: 'differentApp',
 			},
-		}));
+		});
 		expect(() => getRedirectUrl(mockNonHttpRedirectUrls)).toThrow(
 			invalidRedirectException,
 		);

diff --git a/packages/auth/src/providers/cognito/utils/oauth/getRedirectUrl.native.ts b/packages/auth/src/providers/cognito/utils/oauth/getRedirectUrl.native.ts
@@ -7,14 +7,14 @@ import {
 } from '../../../../errors/constants';
 
 /**
- * An appScheme (non http/s url) is always required to proceed further.
- * If a preferredSignOutUrl is given, then we use that after validating the existence of appScheme.
+* - Validate there is always an appScheme (required), if not throw invalidAppSchemeException.
+* - If a preferredRedirectUrl is given, validate it's in the configured list, if not throw invalidPreferredRedirectUrlException.
+* - If preferredRedirectUrl is not given, use the appScheme which is present in the configured list.
 @internal */
 export function getRedirectUrl(
 	redirects: string[],
-	preferredSignOutUrl?: string,
+	preferredRedirectUrl?: string,
 ): string {
-	let preferredRedirectUrl;
 	// iOS always requires a non http/s url (appScheme) to be registered so we validate it's existence here.
 	const appSchemeRedirectUrl = redirects?.find(
 		redirect =>
@@ -23,15 +23,11 @@ export function getRedirectUrl(
 	if (!appSchemeRedirectUrl) {
 		throw invalidAppSchemeException;
 	}
-	if (preferredSignOutUrl) {
-		preferredRedirectUrl = redirects?.find(
-			redirect => redirect === preferredSignOutUrl,
-		);
-		if (!preferredRedirectUrl) {
-			throw invalidPreferredRedirectUrlException;
+	if (preferredRedirectUrl) {
+		if (redirects?.includes(preferredRedirectUrl)) {
+			return preferredRedirectUrl;
 		}
-
-		return preferredRedirectUrl;
+		throw invalidPreferredRedirectUrlException;
 	}
 
 	return appSchemeRedirectUrl;

diff --git a/packages/auth/src/providers/cognito/utils/oauth/getRedirectUrl.ts b/packages/auth/src/providers/cognito/utils/oauth/getRedirectUrl.ts
@@ -9,11 +9,11 @@ import {
 /** @internal */
 export function getRedirectUrl(
 	redirects: string[],
-	preferredSignOutUrl?: string,
+	preferredRedirectUrl?: string,
 ): string {
-	if (preferredSignOutUrl) {
+	if (preferredRedirectUrl) {
 		const redirectUrl = redirects?.find(
-			redirect => redirect === preferredSignOutUrl,
+			redirect => redirect === preferredRedirectUrl,
 		);
 		if (!redirectUrl) {
 			throw invalidPreferredRedirectUrlException;