Skip to content

Commit

Permalink
fix(adapter-nextjs): wrong cookie attributes get set sometimes (#14169)
Browse files Browse the repository at this point in the history
  • Loading branch information
HuiSF authored Jan 30, 2025
1 parent 2170de5 commit 79cccbc
Show file tree
Hide file tree
Showing 31 changed files with 535 additions and 108 deletions.
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
@@ -1,6 +1,3 @@
/**
* @jest-environment node
*/
import { NextRequest } from 'next/server';
import { OAuthConfig } from 'aws-amplify/adapter-core/internals';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
/**
* @jest-environment node
*/
import { NextRequest } from 'next/server.js';
import { OAuthConfig } from 'aws-amplify/adapter-core/internals';
import { CookieStorage } from 'aws-amplify/adapter-core';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
/**
* @jest-environment node
*/
import { OAuthConfig } from 'aws-amplify/adapter-core/internals';
import { CookieStorage } from 'aws-amplify/adapter-core';
import { NextApiRequest } from 'next';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
/**
* @jest-environment node
*/
import { OAuthConfig } from 'aws-amplify/adapter-core/internals';
import { CookieStorage } from 'aws-amplify/adapter-core';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
/**
* @jest-environment node
*/
import { OAuthConfig } from 'aws-amplify/adapter-core/internals';
import { CookieStorage } from 'aws-amplify/adapter-core';
import { NextApiRequest } from 'next';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
/**
* @jest-environment node
*/
import { OAuthConfig } from 'aws-amplify/adapter-core/internals';
import {
AUTH_KEY_PREFIX,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
/**
* @jest-environment node
*/
import { OAuthConfig } from 'aws-amplify/adapter-core/internals';
import {
AUTH_KEY_PREFIX,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
/**
* @jest-environment node
*/
import { OAuthConfig } from 'aws-amplify/adapter-core/internals';

import { handleSignOutRequest } from '../../../src/auth/handlers/handleSignOutRequest';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
/**
* @jest-environment node
*/
import { OAuthConfig } from 'aws-amplify/adapter-core/internals';

import { handleSignOutRequestForPagesRouter } from '../../../src/auth/handlers/handleSignOutRequestForPagesRouter';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
/**
* @jest-environment node
*/
import { getCurrentUser } from 'aws-amplify/auth/server';
import { NextRequest } from 'next/server';
import { AuthUser } from 'aws-amplify/auth';
Expand Down
6 changes: 6 additions & 0 deletions packages/adapter-nextjs/__tests__/auth/utils/origin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ describe('isValidOrigin', () => {
['https://exam_ple.com', false],
['https://example.com?query=param', false],
['https://example.com:80/path#fragment', false],
['yea, I am not a origin, so?', false],
[undefined, false],
['', false],
] as [string, boolean][])('validates origin %s as %s', (origin, expected) => {
expect(isValidOrigin(origin)).toBe(expected);
});
Expand All @@ -53,6 +56,9 @@ describe('isSSLOrigin', () => {
['http://localhost', false],
['http://localhost:3000', false],
['https:// some-app.com', false],
['https://some-app.com:', false],
[undefined, false],
['', false],
])('check origin SSL %s status as %s', (origin, expected) => {
expect(isSSLOrigin(origin)).toBe(expected);
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
/**
* @jest-environment node
*/
import { NextRequest } from 'next/server.js';

import {
Expand Down
18 changes: 18 additions & 0 deletions packages/adapter-nextjs/__tests__/auth/utils/tokenCookies.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
createTokenCookiesSetOptions,
createTokenRemoveCookies,
getAccessTokenUsername,
isServerSideAuthAllowedCookie,
} from '../../../src/auth/utils';

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

describe('isServerSideAuthAllowedCookie', () => {
test.each([
['CognitoIdentityServiceProvider.1234.aaaa.clockDrift', false],
['CognitoIdentityServiceProvider.1234.aaaa.deviceKey', false],
['CognitoIdentityServiceProvider.1234.aaaa.clientMetadata', false],
['CognitoIdentityServiceProvider.1234.aaaa.oAuthMetadata', false],
['CognitoIdentityServiceProvider.1234.aaaa', false],
['CognitoIdentityServiceProvider.1234', false],
['CognitoIdentityServiceProvider.1234.aaaa.refreshToken', true],
['CognitoIdentityServiceProvider.1234.aaaa.accessToken', true],
['CognitoIdentityServiceProvider.1234.aaaa.idToken', true],
['CognitoIdentityServiceProvider.1234.aaaa.LastAuthUser', true],
])('returns %s for %s', (cookieName, expected) => {
expect(isServerSideAuthAllowedCookie(cookieName)).toBe(expected);
});
});
Loading

0 comments on commit 79cccbc

Please sign in to comment.