feat(adapter-nextjs): allow cookie secure: false with non-SSL domain

aws-amplify · Sep 23, 2024 · 6edd287 · 6edd287
1 parent 7a35370
commit 6edd287
Show file tree

Hide file tree

Showing 17 changed files with 216 additions and 6 deletions.
diff --git a/packages/adapter-nextjs/__tests__/auth/createAuthRouteHandlersFactory.test.ts b/packages/adapter-nextjs/__tests__/auth/createAuthRouteHandlersFactory.test.ts
@@ -17,6 +17,7 @@ import {
 	isNextApiRequest,
 	isNextApiResponse,
 	isNextRequest,
+	isValidOrigin,
 } from '../../src/auth/utils';
 
 jest.mock('@aws-amplify/core/internals/utils');
@@ -62,6 +63,7 @@ const mockIsNextRequest = jest.mocked(isNextRequest);
 const mockIsAuthRoutesHandlersContext = jest.mocked(
 	isAuthRoutesHandlersContext,
 );
+const mockIsValidOrigin = jest.mocked(isValidOrigin);
 const mockRunWithAmplifyServerContext =
 	jest.fn() as jest.MockedFunction<NextServer.RunOperationWithContext>;
 
@@ -72,6 +74,10 @@ describe('createAuthRoutesHandlersFactory', () => {
 		AMPLIFY_APP_ORIGIN: 'https://example.com',
 	};
 
+	beforeAll(() => {
+		mockIsValidOrigin.mockReturnValue(true);
+	});
+
 	beforeEach(() => {
 		process.env = modifiedProcessEnvVars;
 	});
@@ -91,6 +97,19 @@ describe('createAuthRoutesHandlersFactory', () => {
 		).toThrow('Could not find the AMPLIFY_APP_ORIGIN environment variable.');
 	});
 
+	it('throws an error if the AMPLIFY_APP_ORIGIN environment variable is invalid', () => {
+		mockIsValidOrigin.mockReturnValueOnce(false);
+		expect(() =>
+			createAuthRouteHandlersFactory({
+				config: mockAmplifyConfig,
+				runtimeOptions: mockRuntimeOptions,
+				runWithAmplifyServerContext: mockRunWithAmplifyServerContext,
+			}),
+		).toThrow(
+			'AMPLIFY_APP_ORIGIN environment variable contains an invalid origin string.',
+		);
+	});
+
 	it('calls config assertion functions to validate the Auth configuration', () => {
 		createAuthRouteHandlersFactory({
 			config: mockAmplifyConfig,

diff --git a/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInSignUpRequest.test.ts b/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInSignUpRequest.test.ts
@@ -13,6 +13,7 @@ import {
 	createSignInFlowProofCookies,
 	createSignUpEndpoint,
 	createUrlSearchParamsForSignInSignUp,
+	isNonSSLOrigin,
 } from '../../../src/auth/utils';
 
 jest.mock('../../../src/auth/utils');
@@ -30,6 +31,7 @@ const mockCreateSignUpEndpoint = jest.mocked(createSignUpEndpoint);
 const mockCreateUrlSearchParamsForSignInSignUp = jest.mocked(
 	createUrlSearchParamsForSignInSignUp,
 );
+const mockIsNonSSLOrigin = jest.mocked(isNonSSLOrigin);
 
 describe('handleSignInSignUpRequest', () => {
 	const mockCustomState = 'mockCustomState';
@@ -41,6 +43,10 @@ describe('handleSignInSignUpRequest', () => {
 	};
 	const mockToCodeChallenge = jest.fn(() => 'mockCodeChallenge');
 
+	beforeAll(() => {
+		mockIsNonSSLOrigin.mockReturnValue(true);
+	});
+
 	afterEach(() => {
 		mockAppendSetCookieHeaders.mockClear();
 		mockCreateAuthFlowProofCookiesSetOptions.mockClear();
@@ -50,6 +56,7 @@ describe('handleSignInSignUpRequest', () => {
 		mockCreateSignUpEndpoint.mockClear();
 		mockCreateUrlSearchParamsForSignInSignUp.mockClear();
 		mockToCodeChallenge.mockClear();
+		mockIsNonSSLOrigin.mockClear();
 	});
 
 	test.each(['signIn' as const, 'signUp' as const])(
@@ -145,11 +152,19 @@ describe('handleSignInSignUpRequest', () => {
 				);
 			}
 
+			expect(mockCreateAuthFlowProofCookiesSetOptions).toHaveBeenCalledWith(
+				mockSetCookieOptions,
+				{
+					secure: true,
+				},
+			);
+
 			expect(mockAppendSetCookieHeaders).toHaveBeenCalledWith(
 				expect.any(Headers),
 				mockCreateSignInFlowProofCookiesResult,
 				mockCreateAuthFlowProofCookiesSetOptionsResult,
 			);
+			expect(isNonSSLOrigin).toHaveBeenCalledWith(mockOrigin);
 		},
 	);
 });
diff --git a/...es/adapter-nextjs/__tests__/auth/handlers/handleSignInSignUpRequestForPagesRouter.test.ts b/...es/adapter-nextjs/__tests__/auth/handlers/handleSignInSignUpRequestForPagesRouter.test.ts
@@ -14,6 +14,7 @@ import {
 	createSignInFlowProofCookies,
 	createSignUpEndpoint,
 	createUrlSearchParamsForSignInSignUp,
+	isNonSSLOrigin,
 } from '../../../src/auth/utils';
 import { createMockNextApiResponse } from '../testUtils';
 
@@ -34,6 +35,7 @@ const mockCreateSignUpEndpoint = jest.mocked(createSignUpEndpoint);
 const mockCreateUrlSearchParamsForSignInSignUp = jest.mocked(
 	createUrlSearchParamsForSignInSignUp,
 );
+const mockIsNonSSLOrigin = jest.mocked(isNonSSLOrigin);
 
 describe('handleSignInSignUpRequest', () => {
 	const mockCustomState = 'mockCustomState';
@@ -54,6 +56,10 @@ describe('handleSignInSignUpRequest', () => {
 		mockResponse,
 	} = createMockNextApiResponse();
 
+	beforeAll(() => {
+		mockIsNonSSLOrigin.mockReturnValue(true);
+	});
+
 	afterEach(() => {
 		mockAppendSetCookieHeadersToNextApiResponse.mockClear();
 		mockCreateAuthFlowProofCookiesSetOptions.mockClear();
@@ -63,6 +69,7 @@ describe('handleSignInSignUpRequest', () => {
 		mockCreateSignUpEndpoint.mockClear();
 		mockCreateUrlSearchParamsForSignInSignUp.mockClear();
 		mockToCodeChallenge.mockClear();
+		mockIsNonSSLOrigin.mockClear();
 
 		mockResponseAppendHeader.mockClear();
 		mockResponseEnd.mockClear();
@@ -170,11 +177,19 @@ describe('handleSignInSignUpRequest', () => {
 				);
 			}
 
+			expect(mockCreateAuthFlowProofCookiesSetOptions).toHaveBeenCalledWith(
+				mockSetCookieOptions,
+				{
+					secure: true,
+				},
+			);
+
 			expect(mockAppendSetCookieHeadersToNextApiResponse).toHaveBeenCalledWith(
 				mockResponse,
 				mockCreateSignInFlowProofCookiesResult,
 				mockCreateAuthFlowProofCookiesSetOptionsResult,
 			);
+			expect(isNonSSLOrigin).toHaveBeenCalledWith(mockOrigin);
 		},
 	);
 });
diff --git a/packages/adapter-nextjs/__tests__/auth/handlers/handleSignOutRequest.test.ts b/packages/adapter-nextjs/__tests__/auth/handlers/handleSignOutRequest.test.ts
@@ -9,6 +9,7 @@ import {
 	createAuthFlowProofCookiesSetOptions,
 	createLogoutEndpoint,
 	createSignOutFlowProofCookies,
+	isNonSSLOrigin,
 	resolveRedirectSignOutUrl,
 } from '../../../src/auth/utils';
 
@@ -23,14 +24,20 @@ const mockCreateSignOutFlowProofCookies = jest.mocked(
 	createSignOutFlowProofCookies,
 );
 const mockResolveRedirectSignOutUrl = jest.mocked(resolveRedirectSignOutUrl);
+const mockIsNonSSLOrigin = jest.mocked(isNonSSLOrigin);
 
 describe('handleSignOutRequest', () => {
+	beforeAll(() => {
+		mockIsNonSSLOrigin.mockReturnValue(true);
+	});
+
 	afterEach(() => {
 		mockAppendSetCookieHeaders.mockClear();
 		mockCreateAuthFlowProofCookiesSetOptions.mockClear();
 		mockCreateLogoutEndpoint.mockClear();
 		mockCreateSignOutFlowProofCookies.mockClear();
 		mockResolveRedirectSignOutUrl.mockClear();
+		mockIsNonSSLOrigin.mockClear();
 	});
 
 	it('returns a 302 response with the correct headers and cookies', async () => {
@@ -93,8 +100,12 @@ describe('handleSignOutRequest', () => {
 			expect.any(URLSearchParams),
 		);
 		expect(mockCreateSignOutFlowProofCookies).toHaveBeenCalled();
+		expect(mockIsNonSSLOrigin).toHaveBeenCalledWith(mockOrigin);
 		expect(mockCreateAuthFlowProofCookiesSetOptions).toHaveBeenCalledWith(
 			mockSetCookieOptions,
+			{
+				secure: true,
+			},
 		);
 		expect(mockAppendSetCookieHeaders).toHaveBeenCalledWith(
 			expect.any(Headers),

diff --git a/packages/adapter-nextjs/__tests__/auth/handlers/handleSignOutRequestForPagesRouter.test.ts b/packages/adapter-nextjs/__tests__/auth/handlers/handleSignOutRequestForPagesRouter.test.ts
@@ -9,6 +9,7 @@ import {
 	createAuthFlowProofCookiesSetOptions,
 	createLogoutEndpoint,
 	createSignOutFlowProofCookies,
+	isNonSSLOrigin,
 	resolveRedirectSignOutUrl,
 } from '../../../src/auth/utils';
 import { createMockNextApiResponse } from '../testUtils';
@@ -26,6 +27,7 @@ const mockCreateSignOutFlowProofCookies = jest.mocked(
 	createSignOutFlowProofCookies,
 );
 const mockResolveRedirectSignOutUrl = jest.mocked(resolveRedirectSignOutUrl);
+const mockIsNonSSLOrigin = jest.mocked(isNonSSLOrigin);
 
 describe('handleSignOutRequest', () => {
 	const {
@@ -37,6 +39,10 @@ describe('handleSignOutRequest', () => {
 		mockResponse,
 	} = createMockNextApiResponse();
 
+	beforeAll(() => {
+		mockIsNonSSLOrigin.mockReturnValue(true);
+	});
+
 	afterEach(() => {
 		mockAppendSetCookieHeadersToNextApiResponse.mockClear();
 		mockCreateAuthFlowProofCookiesSetOptions.mockClear();
@@ -117,8 +123,12 @@ describe('handleSignOutRequest', () => {
 			expect.any(URLSearchParams),
 		);
 		expect(mockCreateSignOutFlowProofCookies).toHaveBeenCalled();
+		expect(mockIsNonSSLOrigin).toHaveBeenCalledWith(mockOrigin);
 		expect(mockCreateAuthFlowProofCookiesSetOptions).toHaveBeenCalledWith(
 			mockSetCookieOptions,
+			{
+				secure: true,
+			},
 		);
 	});
 });
diff --git a/packages/adapter-nextjs/__tests__/auth/utils/authFlowProofCookies.test.ts b/packages/adapter-nextjs/__tests__/auth/utils/authFlowProofCookies.test.ts
@@ -65,6 +65,27 @@ describe('createAuthFlowProofCookiesSetOptions', () => {
 			expires: new Date(0 + AUTH_FLOW_PROOF_COOKIE_EXPIRY),
 		});
 	});
+
+	it('returns expected cookie serialization options with specified parameters with overridden secure attribute', () => {
+		const setCookieOptions: CookieStorage.SetCookieOptions = {
+			domain: '.example.com',
+			sameSite: 'strict',
+		};
+
+		const options = createAuthFlowProofCookiesSetOptions(setCookieOptions, {
+			secure: false,
+		});
+
+		expect(nowSpy).toHaveBeenCalled();
+		expect(options).toEqual({
+			domain: setCookieOptions?.domain,
+			path: '/',
+			httpOnly: true,
+			secure: false,
+			sameSite: 'lax' as const,
+			expires: new Date(0 + AUTH_FLOW_PROOF_COOKIE_EXPIRY),
+		});
+	});
 });
 
 describe('createAuthFlowProofCookiesRemoveOptions', () => {

diff --git a/packages/adapter-nextjs/__tests__/auth/utils/isValidOrigin.test.ts b/packages/adapter-nextjs/__tests__/auth/utils/isValidOrigin.test.ts
@@ -0,0 +1,61 @@
+import {
+	isNonSSLOrigin,
+	isValidOrigin,
+} from '../../../src/auth/utils/isValidOrigin';
+
+describe('isValidOrigin', () => {
+	test.each([
+		// Valid origins
+		['http://example.com', true],
+		['https://example.com', true],
+		['http://www.example.com', true],
+		['https://subdomain.example.com', true],
+		['http://example.com:8080', true],
+		['https://example.com:443', true],
+		['http://localhost', true],
+		['http://localhost:3000', true],
+		['https://localhost:8080', true],
+		['http://127.0.0.1', true],
+		['http://127.0.0.1:8000', true],
+
+		// Invalid origins
+		['http://example.com/path', false],
+		['https://example.com/path/to/resource', false],
+		['http://example.com:8080/path', false],
+		['ftp://example.com', false],
+		['example.com', false],
+		['http:/example.com', false],
+		['https:example.com', false],
+		['http://', false],
+		['https://', false],
+		['localhost', false],
+		['http:localhost', false],
+		['https://localhost:', false],
+		['http://127.0.0.1:', false],
+		['https://.com', false],
+		['http://example.', false],
+		['https://example.com:abc', false],
+		['http:// example.com', false],
+		['https://exam ple.com', false],
+		['http://exa mple.com:8080', false],
+		['https://example.com:8080:8081', false],
+		['http://example.com:80:80', false],
+		['https://.example.com', false],
+		['http://example..com', false],
+		['https://exam_ple.com', false],
+		['https://example.com?query=param', false],
+		['https://example.com:80/path#fragment', false],
+	] as [string, boolean][])('validates origin %s as %s', (origin, expected) => {
+		expect(isValidOrigin(origin)).toBe(expected);
+	});
+});
+
+describe('isNonSSLLocalhostOrigin', () => {
+	test.each([
+		['http://localhost', true],
+		['http://localhost:3000', true],
+		['https://some-app.com', false],
+	])('check origin is non-SSL localhost %s as %s', (origin, expected) => {
+		expect(isNonSSLOrigin(origin)).toBe(expected);
+	});
+});
diff --git a/packages/adapter-nextjs/__tests__/auth/utils/tokenCookies.test.ts b/packages/adapter-nextjs/__tests__/auth/utils/tokenCookies.test.ts
@@ -120,6 +120,27 @@ describe('createTokenCookiesSetOptions', () => {
 
 		dateNowSpy.mockRestore();
 	});
+
+	it('returns an object with the correct cookie options with overridden secure attribute', () => {
+		const mockSetCookieOptions: CookieStorage.SetCookieOptions = {
+			domain: '.example.com',
+			sameSite: 'strict',
+			expires: new Date('2024-09-17'),
+		};
+
+		const result = createTokenCookiesSetOptions(mockSetCookieOptions, {
+			secure: false,
+		});
+
+		expect(result).toEqual({
+			domain: mockSetCookieOptions.domain,
+			path: '/',
+			httpOnly: true,
+			secure: false,
+			sameSite: 'strict',
+			expires: mockSetCookieOptions.expires,
+		});
+	});
 });
 
 describe('createTokenCookiesRemoveOptions', () => {

diff --git a/packages/adapter-nextjs/src/auth/createAuthRouteHandlersFactory.ts b/packages/adapter-nextjs/src/auth/createAuthRouteHandlersFactory.ts
@@ -20,6 +20,7 @@ import {
 	isNextApiRequest,
 	isNextApiResponse,
 	isNextRequest,
+	isValidOrigin,
 } from './utils';
 import { handleAuthApiRouteRequestForAppRouter } from './handleAuthApiRouteRequestForAppRouter';
 import { handleAuthApiRouteRequestForPagesRouter } from './handleAuthApiRouteRequestForPagesRouter';
@@ -37,6 +38,15 @@ export const createAuthRouteHandlersFactory = ({
 				'Add the AMPLIFY_APP_ORIGIN environment variable to the `.env` file of your Next.js project.',
 		});
 
+	if (!isValidOrigin(origin)) {
+		throw new AmplifyServerContextError({
+			message:
+				'AMPLIFY_APP_ORIGIN environment variable contains an invalid origin string.',
+			recoverySuggestion:
+				'Ensure the AMPLIFY_APP_ORIGIN environment variable is a valid origin string.',
+		});
+	}
+
 	assertTokenProviderConfig(resourcesConfig.Auth?.Cognito);
 	assertOAuthConfig(resourcesConfig.Auth.Cognito);
 

diff --git a/packages/adapter-nextjs/src/auth/handlers/handleSignInSignUpRequest.ts b/packages/adapter-nextjs/src/auth/handlers/handleSignInSignUpRequest.ts
@@ -9,6 +9,7 @@ import {
 	createSignInFlowProofCookies,
 	createSignUpEndpoint,
 	createUrlSearchParamsForSignInSignUp,
+	isNonSSLOrigin,
 } from '../utils';
 
 import { HandleSignInSignUpRequest } from './types';
@@ -43,7 +44,9 @@ export const handleSignInSignUpRequest: HandleSignInSignUpRequest = ({
 	appendSetCookieHeaders(
 		headers,
 		createSignInFlowProofCookies({ state, pkce: codeVerifier.value }),
-		createAuthFlowProofCookiesSetOptions(setCookieOptions),
+		createAuthFlowProofCookiesSetOptions(setCookieOptions, {
+			secure: isNonSSLOrigin(origin),
+		}),
 	);
 
 	return new Response(null, {