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.

Sign up for GitHub

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

Jump to bottom

feat(adapter-nextjs): set cookie secure: false with non-SSL domain #13841

Open
wants to merge 1 commit into
base: hui/fix/adapter-nextjs/5-handler-type-def
Choose a base branch
from
Open

feat(adapter-nextjs): set cookie secure: false with non-SSL domain #13841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 19 additions & 0 deletions packages/adapter-nextjs/__tests__/auth/createAuthRouteHandlersFactory.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
isNextApiRequest,
isNextApiResponse,
isNextRequest,
isValidOrigin,
} from '../../src/auth/utils';

jest.mock('@aws-amplify/core/internals/utils');
Expand Down Expand Up @@ -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>;

Expand All @@ -72,6 +74,10 @@ describe('createAuthRoutesHandlersFactory', () => {
AMPLIFY_APP_ORIGIN: 'https://example.com',
};

beforeAll(() => {
mockIsValidOrigin.mockReturnValue(true);
});

beforeEach(() => {
process.env = modifiedProcessEnvVars;
});
Expand All @@ -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,
Expand Down
15 changes: 15 additions & 0 deletions packages/adapter-nextjs/__tests__/auth/handlers/handleSignInSignUpRequest.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
createSignInFlowProofCookies,
createSignUpEndpoint,
createUrlSearchParamsForSignInSignUp,
isNonSSLOrigin,
} from '../../../src/auth/utils';

jest.mock('../../../src/auth/utils');
Expand All @@ -30,6 +31,7 @@ const mockCreateSignUpEndpoint = jest.mocked(createSignUpEndpoint);
const mockCreateUrlSearchParamsForSignInSignUp = jest.mocked(
createUrlSearchParamsForSignInSignUp,
);
const mockIsNonSSLOrigin = jest.mocked(isNonSSLOrigin);

describe('handleSignInSignUpRequest', () => {
const mockCustomState = 'mockCustomState';
Expand All @@ -41,6 +43,10 @@ describe('handleSignInSignUpRequest', () => {
};
const mockToCodeChallenge = jest.fn(() => 'mockCodeChallenge');

beforeAll(() => {
mockIsNonSSLOrigin.mockReturnValue(true);
});

afterEach(() => {
mockAppendSetCookieHeaders.mockClear();
mockCreateAuthFlowProofCookiesSetOptions.mockClear();
Expand All @@ -50,6 +56,7 @@ describe('handleSignInSignUpRequest', () => {
mockCreateSignUpEndpoint.mockClear();
mockCreateUrlSearchParamsForSignInSignUp.mockClear();
mockToCodeChallenge.mockClear();
mockIsNonSSLOrigin.mockClear();
});

test.each(['signIn' as const, 'signUp' as const])(
Expand Down Expand Up @@ -145,11 +152,19 @@ describe('handleSignInSignUpRequest', () => {
);
}

expect(mockCreateAuthFlowProofCookiesSetOptions).toHaveBeenCalledWith(
mockSetCookieOptions,
{
secure: true,
},
);

expect(mockAppendSetCookieHeaders).toHaveBeenCalledWith(
expect.any(Headers),
mockCreateSignInFlowProofCookiesResult,
mockCreateAuthFlowProofCookiesSetOptionsResult,
);
expect(isNonSSLOrigin).toHaveBeenCalledWith(mockOrigin);
},
);
});
15 changes: 15 additions & 0 deletions ...es/adapter-nextjs/__tests__/auth/handlers/handleSignInSignUpRequestForPagesRouter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
createSignInFlowProofCookies,
createSignUpEndpoint,
createUrlSearchParamsForSignInSignUp,
isNonSSLOrigin,
} from '../../../src/auth/utils';
import { createMockNextApiResponse } from '../testUtils';

Expand All @@ -34,6 +35,7 @@ const mockCreateSignUpEndpoint = jest.mocked(createSignUpEndpoint);
const mockCreateUrlSearchParamsForSignInSignUp = jest.mocked(
createUrlSearchParamsForSignInSignUp,
);
const mockIsNonSSLOrigin = jest.mocked(isNonSSLOrigin);

describe('handleSignInSignUpRequest', () => {
const mockCustomState = 'mockCustomState';
Expand All @@ -54,6 +56,10 @@ describe('handleSignInSignUpRequest', () => {
mockResponse,
} = createMockNextApiResponse();

beforeAll(() => {
mockIsNonSSLOrigin.mockReturnValue(true);
});

afterEach(() => {
mockAppendSetCookieHeadersToNextApiResponse.mockClear();
mockCreateAuthFlowProofCookiesSetOptions.mockClear();
Expand All @@ -63,6 +69,7 @@ describe('handleSignInSignUpRequest', () => {
mockCreateSignUpEndpoint.mockClear();
mockCreateUrlSearchParamsForSignInSignUp.mockClear();
mockToCodeChallenge.mockClear();
mockIsNonSSLOrigin.mockClear();

mockResponseAppendHeader.mockClear();
mockResponseEnd.mockClear();
Expand Down Expand Up @@ -170,11 +177,19 @@ describe('handleSignInSignUpRequest', () => {
);
}

expect(mockCreateAuthFlowProofCookiesSetOptions).toHaveBeenCalledWith(
mockSetCookieOptions,
{
secure: true,
},
);

expect(mockAppendSetCookieHeadersToNextApiResponse).toHaveBeenCalledWith(
mockResponse,
mockCreateSignInFlowProofCookiesResult,
mockCreateAuthFlowProofCookiesSetOptionsResult,
);
expect(isNonSSLOrigin).toHaveBeenCalledWith(mockOrigin);
},
);
});
11 changes: 11 additions & 0 deletions packages/adapter-nextjs/__tests__/auth/handlers/handleSignOutRequest.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
createAuthFlowProofCookiesSetOptions,
createLogoutEndpoint,
createSignOutFlowProofCookies,
isNonSSLOrigin,
resolveRedirectSignOutUrl,
} from '../../../src/auth/utils';

Expand All @@ -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 () => {
Expand Down Expand Up @@ -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),
Expand Down
10 changes: 10 additions & 0 deletions packages/adapter-nextjs/__tests__/auth/handlers/handleSignOutRequestForPagesRouter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
createAuthFlowProofCookiesSetOptions,
createLogoutEndpoint,
createSignOutFlowProofCookies,
isNonSSLOrigin,
resolveRedirectSignOutUrl,
} from '../../../src/auth/utils';
import { createMockNextApiResponse } from '../testUtils';
Expand All @@ -26,6 +27,7 @@ const mockCreateSignOutFlowProofCookies = jest.mocked(
createSignOutFlowProofCookies,
);
const mockResolveRedirectSignOutUrl = jest.mocked(resolveRedirectSignOutUrl);
const mockIsNonSSLOrigin = jest.mocked(isNonSSLOrigin);

describe('handleSignOutRequest', () => {
const {
Expand All @@ -37,6 +39,10 @@ describe('handleSignOutRequest', () => {
mockResponse,
} = createMockNextApiResponse();

beforeAll(() => {
mockIsNonSSLOrigin.mockReturnValue(true);
});

afterEach(() => {
mockAppendSetCookieHeadersToNextApiResponse.mockClear();
mockCreateAuthFlowProofCookiesSetOptions.mockClear();
Expand Down Expand Up @@ -117,8 +123,12 @@ describe('handleSignOutRequest', () => {
expect.any(URLSearchParams),
);
expect(mockCreateSignOutFlowProofCookies).toHaveBeenCalled();
expect(mockIsNonSSLOrigin).toHaveBeenCalledWith(mockOrigin);
expect(mockCreateAuthFlowProofCookiesSetOptions).toHaveBeenCalledWith(
mockSetCookieOptions,
{
secure: true,
},
);
});
});
21 changes: 21 additions & 0 deletions packages/adapter-nextjs/__tests__/auth/utils/authFlowProofCookies.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
61 changes: 61 additions & 0 deletions packages/adapter-nextjs/__tests__/auth/utils/isValidOrigin.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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);
});
});
21 changes: 21 additions & 0 deletions packages/adapter-nextjs/__tests__/auth/utils/tokenCookies.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
10 changes: 10 additions & 0 deletions packages/adapter-nextjs/src/auth/createAuthRouteHandlersFactory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
isNextApiRequest,
isNextApiResponse,
isNextRequest,
isValidOrigin,
} from './utils';
import { handleAuthApiRouteRequestForAppRouter } from './handleAuthApiRouteRequestForAppRouter';
import { handleAuthApiRouteRequestForPagesRouter } from './handleAuthApiRouteRequestForPagesRouter';
Expand All @@ -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);

Expand Down
5 changes: 4 additions & 1 deletion packages/adapter-nextjs/src/auth/handlers/handleSignInSignUpRequest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
createSignInFlowProofCookies,
createSignUpEndpoint,
createUrlSearchParamsForSignInSignUp,
isNonSSLOrigin,
} from '../utils';

import { HandleSignInSignUpRequest } from './types';
Expand Down Expand Up @@ -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, {
Expand Down
Loading
Loading