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

fix(adapter-nextjs): wrong cookie attributes get set sometimes #14169

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
isNextRequest,
isValidOrigin,
} from '../../src/auth/utils';
import { globalSettings } from '../../src/utils';

jest.mock('aws-amplify/adapter-core/internals', () => ({
...jest.requireActual('aws-amplify/adapter-core/internals'),
Expand All @@ -29,6 +30,20 @@ jest.mock('aws-amplify/adapter-core/internals', () => ({
jest.mock('../../src/auth/handleAuthApiRouteRequestForAppRouter');
jest.mock('../../src/auth/handleAuthApiRouteRequestForPagesRouter');
jest.mock('../../src/auth/utils');
jest.mock('../../src/utils', () => ({
globalSettings: {
isServerSideAuthEnabled: jest.fn(() => true),
enableServerSideAuth: jest.fn(),
setRuntimeOptions: jest.fn(),
getRuntimeOptions: jest.fn(() => ({
cookies: {
sameSite: 'strict',
},
})),
isSSLOrigin: jest.fn(() => true),
setIsSSLOrigin: jest.fn(),
},
}));

const mockAmplifyConfig: ResourcesConfig = {
Auth: {
Expand All @@ -49,11 +64,6 @@ const mockAmplifyConfig: ResourcesConfig = {
},
};

const mockRuntimeOptions: NextServer.CreateServerRunnerRuntimeOptions = {
cookies: {
sameSite: 'strict',
},
};
const mockAssertTokenProviderConfig = jest.mocked(assertTokenProviderConfig);
const mockAssertOAuthConfig = jest.mocked(assertOAuthConfig);
const mockHandleAuthApiRouteRequestForAppRouter = jest.mocked(
Expand Down Expand Up @@ -83,9 +93,9 @@ describe('createAuthRoutesHandlersFactory', () => {
it('throws an error if the AMPLIFY_APP_ORIGIN environment variable is not defined', () => {
const throwingFunc = createAuthRouteHandlersFactory({
config: mockAmplifyConfig,
runtimeOptions: mockRuntimeOptions,
amplifyAppOrigin: undefined,
runWithAmplifyServerContext: mockRunWithAmplifyServerContext,
globalSettings,
});
expect(() => throwingFunc()).toThrow(
'Could not find the AMPLIFY_APP_ORIGIN environment variable.',
Expand All @@ -96,9 +106,9 @@ describe('createAuthRoutesHandlersFactory', () => {
mockIsValidOrigin.mockReturnValueOnce(false);
const throwingFunc = createAuthRouteHandlersFactory({
config: mockAmplifyConfig,
runtimeOptions: mockRuntimeOptions,
amplifyAppOrigin: 'domain-without-protocol.com',
runWithAmplifyServerContext: mockRunWithAmplifyServerContext,
globalSettings,
});
expect(() => throwingFunc()).toThrow(
'AMPLIFY_APP_ORIGIN environment variable contains an invalid origin string.',
Expand All @@ -108,9 +118,9 @@ describe('createAuthRoutesHandlersFactory', () => {
it('calls config assertion functions to validate the Auth configuration', () => {
const func = createAuthRouteHandlersFactory({
config: mockAmplifyConfig,
runtimeOptions: mockRuntimeOptions,
amplifyAppOrigin: AMPLIFY_APP_ORIGIN,
runWithAmplifyServerContext: mockRunWithAmplifyServerContext,
globalSettings,
});

func();
Expand All @@ -128,9 +138,9 @@ describe('createAuthRoutesHandlersFactory', () => {
const testCreateAuthRoutesHandlersFactoryInput: CreateAuthRouteHandlersFactoryInput =
{
config: mockAmplifyConfig,
runtimeOptions: mockRuntimeOptions,
amplifyAppOrigin: AMPLIFY_APP_ORIGIN,
runWithAmplifyServerContext: mockRunWithAmplifyServerContext,
globalSettings,
};
const testCreateAuthRoutesHandlersInput: CreateAuthRoutesHandlersInput = {
customState: 'random-state',
Expand Down Expand Up @@ -168,7 +178,9 @@ describe('createAuthRoutesHandlersFactory', () => {
response: param2,
handlerInput: testCreateAuthRoutesHandlersInput,
oAuthConfig: mockAmplifyConfig.Auth!.Cognito!.loginWith!.oauth,
setCookieOptions: mockRuntimeOptions.cookies,
setCookieOptions: {
sameSite: 'strict',
},
origin: 'https://example.com',
userPoolClientId: 'def',
runWithAmplifyServerContext: mockRunWithAmplifyServerContext,
Expand All @@ -190,7 +202,9 @@ describe('createAuthRoutesHandlersFactory', () => {
handlerContext: context,
handlerInput: testCreateAuthRoutesHandlersInput,
oAuthConfig: mockAmplifyConfig.Auth!.Cognito!.loginWith!.oauth,
setCookieOptions: mockRuntimeOptions.cookies,
setCookieOptions: {
sameSite: 'strict',
},
origin: 'https://example.com',
userPoolClientId: 'def',
runWithAmplifyServerContext: mockRunWithAmplifyServerContext,
Expand All @@ -211,11 +225,12 @@ describe('createAuthRoutesHandlersFactory', () => {
});

it('uses default values for parameters that have values as undefined', async () => {
(globalSettings.getRuntimeOptions as jest.Mock).mockReturnValueOnce({});
const createAuthRoutesHandlers = createAuthRouteHandlersFactory({
config: mockAmplifyConfig,
runtimeOptions: undefined,
amplifyAppOrigin: AMPLIFY_APP_ORIGIN,
runWithAmplifyServerContext: mockRunWithAmplifyServerContext,
globalSettings,
});
const handlerWithDefaultParamValues =
createAuthRoutesHandlers(/* undefined */);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
createTokenCookiesSetOptions,
createTokenRemoveCookies,
getAccessTokenUsername,
isServerSideAuthIgnoredCookie,
} from '../../../src/auth/utils';

jest.mock('../../../src/auth/utils/getAccessTokenUsername');
Expand Down Expand Up @@ -149,3 +150,20 @@ describe('createTokenCookiesRemoveOptions', () => {
});
});
});

describe('isServerSideAuthIgnoredCookie', () => {
HuiSF marked this conversation as resolved.
Show resolved Hide resolved
test.each([
['CognitoIdentityServiceProvider.1234.aaaa.clockDrift', true],
['CognitoIdentityServiceProvider.1234.aaaa.deviceKey', true],
['CognitoIdentityServiceProvider.1234.aaaa.clientMetadata', true],
['CognitoIdentityServiceProvider.1234.aaaa.oAuthMetadata', true],
['CognitoIdentityServiceProvider.1234.aaaa', true],
['CognitoIdentityServiceProvider.1234', true],
['CognitoIdentityServiceProvider.1234.aaaa.refreshToken', false],
['CognitoIdentityServiceProvider.1234.aaaa.accessToken', false],
['CognitoIdentityServiceProvider.1234.aaaa.idToken', false],
['CognitoIdentityServiceProvider.1234.aaaa.LastAuthUser', false],
])('returns %s for %s', (cookieName, expected) => {
expect(isServerSideAuthIgnoredCookie(cookieName)).toBe(expected);
});
});
101 changes: 85 additions & 16 deletions packages/adapter-nextjs/__tests__/createServerRunner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ jest.mock('../src/utils/createTokenValidator', () => ({
})),
}));

const mockGetRuntimeOptions = jest.fn(() => ({}));
const mockIsServerSideAuthEnabled = jest.fn(() => false);
const mockGlobalSettingsIsSSLOrigin = jest.fn(() => false);
const mockGlobalSettings: NextServer.GlobalSettings = {
isServerSideAuthEnabled: mockIsServerSideAuthEnabled,
enableServerSideAuth: jest.fn(),
setRuntimeOptions: jest.fn(),
getRuntimeOptions: mockGetRuntimeOptions,
isSSLOrigin: mockGlobalSettingsIsSSLOrigin,
setIsSSLOrigin: jest.fn(),
};

describe('createServerRunner', () => {
let createServerRunner: NextServer.CreateServerRunner;
let createRunWithAmplifyServerContextSpy: any;
Expand All @@ -56,6 +68,14 @@ describe('createServerRunner', () => {
const mockCreateUserPoolsTokenProvider = jest.fn();
const mockRunWithAmplifyServerContextCore = jest.fn();
const mockCreateAuthRouteHandlersFactory = jest.fn(() => jest.fn());
const mockIsSSLOriginUtil = jest.fn(() => true);
const mockIsValidOrigin = jest.fn(() => true);

beforeAll(() => {
jest.doMock('../src/utils/globalSettings', () => ({
globalSettings: mockGlobalSettings,
}));
});

beforeEach(() => {
process.env = modifiedProcessEnv;
Expand All @@ -69,6 +89,7 @@ describe('createServerRunner', () => {
createUserPoolsTokenProvider: mockCreateUserPoolsTokenProvider,
runWithAmplifyServerContext: mockRunWithAmplifyServerContextCore,
}));

jest.doMock('aws-amplify/utils', () => ({
...jest.requireActual('aws-amplify/utils'),
parseAmplifyConfig: mockParseAmplifyConfig,
Expand All @@ -81,20 +102,20 @@ describe('createServerRunner', () => {
createAuthRouteHandlersFactory: mockCreateAuthRouteHandlersFactory,
}));

jest.doMock('../src/auth/utils', () => ({
isSSLOrigin: mockIsSSLOriginUtil,
isValidOrigin: mockIsValidOrigin,
}));

({ createServerRunner } = require('../src'));

mockCreateAuthRouteHandlersFactory.mockReturnValue(jest.fn());
});

afterEach(() => {
process.env = originalProcessEnv;
createRunWithAmplifyServerContextSpy.mockClear();
mockParseAmplifyConfig.mockClear();
mockCreateAWSCredentialsAndIdentityIdProvider.mockClear();
mockCreateKeyValueStorageFromCookieStorageAdapter.mockClear();
mockCreateUserPoolsTokenProvider.mockClear();
mockRunWithAmplifyServerContextCore.mockClear();
mockCreateAuthRouteHandlersFactory.mockClear();

jest.clearAllMocks();
});

it('calls parseAmplifyConfig when the config object is imported from amplify configuration file', () => {
Expand All @@ -114,15 +135,34 @@ describe('createServerRunner', () => {

expect(mockCreateAuthRouteHandlersFactory).toHaveBeenCalledWith({
config: mockAmplifyConfig,
runtimeOptions: undefined,
amplifyAppOrigin: AMPLIFY_APP_ORIGIN,
runWithAmplifyServerContext: expect.any(Function),
globalSettings: mockGlobalSettings,
});
expect(result).toMatchObject({
createAuthRouteHandlers: expect.any(Function),
});
});

describe('when AMPLIFY_APP_ORIGIN is not set', () => {
it('it does NOT call globalSettings.setIsSSLOrigin() and isValidOrigin()', () => {
delete process.env.AMPLIFY_APP_ORIGIN;
createServerRunner({ config: mockAmplifyConfig });
expect(mockGlobalSettings.setIsSSLOrigin).not.toHaveBeenCalled();
expect(mockIsValidOrigin).not.toHaveBeenCalled();
process.env.AMPLIFY_APP_ORIGIN = AMPLIFY_APP_ORIGIN;
});
});

describe('when AMPLIFY_APP_ORIGIN is set with a https origin', () => {
it('it calls globalSettings.setIsSSLOrigin(), isValidOrigin() and globalSettings.enableServerSideAuth', () => {
createServerRunner({ config: mockAmplifyConfig });
expect(mockIsValidOrigin).toHaveBeenCalledWith(AMPLIFY_APP_ORIGIN);
expect(mockGlobalSettings.setIsSSLOrigin).toHaveBeenCalledWith(true);
expect(mockGlobalSettings.enableServerSideAuth).toHaveBeenCalled();
});
});

describe('runWithAmplifyServerContext', () => {
describe('when amplifyConfig.Auth is not defined', () => {
it('should call runWithAmplifyServerContextCore without Auth library options', () => {
Expand Down Expand Up @@ -150,6 +190,7 @@ describe('createServerRunner', () => {
expect(createRunWithAmplifyServerContextSpy).toHaveBeenCalledWith({
config: mockAmplifyConfigWithoutAuth,
tokenValidator: undefined,
globalSettings: mockGlobalSettings,
});
});
});
Expand Down Expand Up @@ -178,6 +219,7 @@ describe('createServerRunner', () => {
tokenValidator: expect.objectContaining({
getItem: expect.any(Function),
}),
globalSettings: mockGlobalSettings,
});
});
});
Expand Down Expand Up @@ -228,6 +270,7 @@ describe('createServerRunner', () => {
tokenValidator: expect.objectContaining({
getItem: expect.any(Function),
}),
globalSettings: mockGlobalSettings,
});
});

Expand All @@ -238,6 +281,9 @@ describe('createServerRunner', () => {
sameSite: 'lax',
expires: new Date('2024-09-05'),
};
mockGetRuntimeOptions.mockReturnValueOnce({
cookies: testCookiesOptions,
});
mockCreateKeyValueStorageFromCookieStorageAdapter.mockReturnValueOnce(
mockCookieStorageAdapter,
);
Expand All @@ -257,15 +303,36 @@ describe('createServerRunner', () => {

expect(
mockCreateKeyValueStorageFromCookieStorageAdapter,
).toHaveBeenCalledWith(
expect.any(Object),
expect.any(Object),
testCookiesOptions,
).toHaveBeenCalledWith(expect.any(Object), expect.any(Object), {
...testCookiesOptions,
path: '/',
});
});

it('should call createKeyValueStorageFromCookieStorageAdapter with specified runtimeOptions.cookies with enforced server auth cookie attributes', async () => {
const testCookiesOptions: NextServer.CreateServerRunnerRuntimeOptions['cookies'] =
{
domain: '.example.com',
sameSite: 'lax',
expires: new Date('2024-09-05'),
};
mockGetRuntimeOptions.mockReturnValueOnce({
cookies: testCookiesOptions,
});
mockIsServerSideAuthEnabled.mockReturnValueOnce(true);
mockGlobalSettingsIsSSLOrigin.mockReturnValueOnce(true);
mockCreateKeyValueStorageFromCookieStorageAdapter.mockReturnValueOnce(
mockCookieStorageAdapter,
);

// modify by reference should not affect the original configuration
testCookiesOptions.sameSite = 'strict';
runWithAmplifyServerContext({
const { runWithAmplifyServerContext } = createServerRunner({
config: mockAmplifyConfig,
runtimeOptions: {
cookies: testCookiesOptions,
},
});

await runWithAmplifyServerContext({
nextServerContext:
mockNextServerContext as unknown as NextServer.Context,
operation: jest.fn(),
Expand All @@ -275,7 +342,9 @@ describe('createServerRunner', () => {
mockCreateKeyValueStorageFromCookieStorageAdapter,
).toHaveBeenCalledWith(expect.any(Object), expect.any(Object), {
...testCookiesOptions,
sameSite: 'lax',
path: '/',
httpOnly: true,
secure: true,
});
});
});
Expand Down
Loading
Loading