Skip to content

Commit

Permalink
fix(adapter-nextjs): wrong use of nullish coalescing (#14112)
Browse files Browse the repository at this point in the history
  • Loading branch information
HuiSF authored Jan 6, 2025
1 parent 075d886 commit f8d978b
Show file tree
Hide file tree
Showing 16 changed files with 131 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from '../../src/auth/handlers';
import { NextServer } from '../../src';
import {
getRedirectOrDefault,
hasActiveUserSessionWithAppRouter,
isSupportedAuthApiRoutePath,
} from '../../src/auth/utils';
Expand All @@ -37,6 +38,7 @@ const mockIsSupportedAuthApiRoutePath = jest.mocked(
);
const mockRunWithAmplifyServerContext =
jest.fn() as jest.MockedFunction<NextServer.RunOperationWithContext>;
const mockGetRedirectOrDefault = jest.mocked(getRedirectOrDefault);

describe('handleAuthApiRouteRequestForAppRouter', () => {
const testOrigin = 'https://example.com';
Expand All @@ -57,6 +59,13 @@ describe('handleAuthApiRouteRequestForAppRouter', () => {
beforeAll(() => {
mockHasUserSignedInWithAppRouter.mockResolvedValue(false);
mockIsSupportedAuthApiRoutePath.mockReturnValue(true);
mockGetRedirectOrDefault.mockImplementation(
(redirect: string | undefined) => redirect || '/',
);
});

afterEach(() => {
mockGetRedirectOrDefault.mockClear();
});

it('returns a 405 response when input.request has an unsupported method', async () => {
Expand Down Expand Up @@ -192,6 +201,7 @@ describe('handleAuthApiRouteRequestForAppRouter', () => {

expect(response.status).toBe(302);
expect(response.headers.get('Location')).toBe('/');
expect(mockGetRedirectOrDefault).toHaveBeenCalledWith(undefined);
},
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from '../../src/auth/handlers';
import { NextServer } from '../../src';
import {
getRedirectOrDefault,
hasActiveUserSessionWithPagesRouter,
isSupportedAuthApiRoutePath,
} from '../../src/auth/utils';
Expand Down Expand Up @@ -40,6 +41,7 @@ const mockHasUserSignedInWithPagesRouter = jest.mocked(
);
const mockRunWithAmplifyServerContext =
jest.fn() as jest.MockedFunction<NextServer.RunOperationWithContext>;
const mockGetRedirectOrDefault = jest.mocked(getRedirectOrDefault);

describe('handleAuthApiRouteRequestForPagesRouter', () => {
const testOrigin = 'https://example.com';
Expand Down Expand Up @@ -67,6 +69,9 @@ describe('handleAuthApiRouteRequestForPagesRouter', () => {
beforeAll(() => {
mockHasUserSignedInWithPagesRouter.mockResolvedValue(false);
mockIsSupportedAuthApiRoutePath.mockReturnValue(true);
mockGetRedirectOrDefault.mockImplementation(
(redirect: string | undefined) => redirect || '/',
);
});

afterEach(() => {
Expand All @@ -75,6 +80,7 @@ describe('handleAuthApiRouteRequestForPagesRouter', () => {
mockResponseStatus.mockClear();
mockResponseSend.mockClear();
mockResponseRedirect.mockClear();
mockGetRedirectOrDefault.mockClear();
});

it('sets response.status(405) when request has an unsupported method', () => {
Expand Down Expand Up @@ -197,6 +203,7 @@ describe('handleAuthApiRouteRequestForPagesRouter', () => {
});

expect(mockResponseRedirect).toHaveBeenCalledWith(302, '/');
expect(mockGetRedirectOrDefault).toHaveBeenCalledWith(undefined);
},
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
createTokenCookiesSetOptions,
exchangeAuthNTokens,
getCookieValuesFromRequest,
getRedirectOrDefault,
resolveCodeAndStateFromUrl,
resolveRedirectSignInUrl,
} from '../../../src/auth/utils';
Expand Down Expand Up @@ -44,6 +45,7 @@ const mockExchangeAuthNTokens = jest.mocked(exchangeAuthNTokens);
const mockGetCookieValuesFromRequest = jest.mocked(getCookieValuesFromRequest);
const mockResolveCodeAndStateFromUrl = jest.mocked(resolveCodeAndStateFromUrl);
const mockResolveRedirectSignInUrl = jest.mocked(resolveRedirectSignInUrl);
const mockGetRedirectOrDefault = jest.mocked(getRedirectOrDefault);

describe('handleSignInCallbackRequest', () => {
const mockHandlerInput: CreateAuthRoutesHandlersInput = {
Expand All @@ -55,6 +57,12 @@ describe('handleSignInCallbackRequest', () => {
const mockSetCookieOptions = {} as CookieStorage.SetCookieOptions;
const mockOrigin = 'https://example.com';

beforeAll(() => {
mockGetRedirectOrDefault.mockImplementation(
(redirect: string | undefined) => redirect || '/',
);
});

afterEach(() => {
mockAppendSetCookieHeaders.mockClear();
mockCreateAuthFlowProofCookiesRemoveOptions.mockClear();
Expand Down Expand Up @@ -307,6 +315,9 @@ describe('handleSignInCallbackRequest', () => {
).toHaveBeenCalledWith({
redirectOnSignInComplete: expectedFinalRedirect,
});
expect(mockGetRedirectOrDefault).toHaveBeenCalledWith(
handlerInput.redirectOnSignInComplete,
);
},
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
createTokenCookiesSetOptions,
exchangeAuthNTokens,
getCookieValuesFromNextApiRequest,
getRedirectOrDefault,
resolveCodeAndStateFromUrl,
resolveRedirectSignInUrl,
} from '../../../src/auth/utils';
Expand Down Expand Up @@ -49,6 +50,7 @@ const mockGetCookieValuesFromNextApiRequest = jest.mocked(
);
const mockResolveCodeAndStateFromUrl = jest.mocked(resolveCodeAndStateFromUrl);
const mockResolveRedirectSignInUrl = jest.mocked(resolveRedirectSignInUrl);
const mockGetRedirectOrDefault = jest.mocked(getRedirectOrDefault);

describe('handleSignInCallbackRequest', () => {
const mockHandlerInput: CreateAuthRoutesHandlersInput = {
Expand All @@ -67,6 +69,12 @@ describe('handleSignInCallbackRequest', () => {
mockResponse,
} = createMockNextApiResponse();

beforeAll(() => {
mockGetRedirectOrDefault.mockImplementation(
(redirect: string | undefined) => redirect || '/',
);
});

afterEach(() => {
mockAppendSetCookieHeadersToNextApiResponse.mockClear();
mockCreateAuthFlowProofCookiesRemoveOptions.mockClear();
Expand All @@ -78,6 +86,7 @@ describe('handleSignInCallbackRequest', () => {
mockGetCookieValuesFromNextApiRequest.mockClear();
mockResolveCodeAndStateFromUrl.mockClear();
mockResolveRedirectSignInUrl.mockClear();
mockGetRedirectOrDefault.mockClear();

mockResponseAppendHeader.mockClear();
mockResponseEnd.mockClear();
Expand Down Expand Up @@ -339,6 +348,9 @@ describe('handleSignInCallbackRequest', () => {
).toHaveBeenCalledWith({
redirectOnSignInComplete: expectedFinalRedirect,
});
expect(getRedirectOrDefault).toHaveBeenCalledWith(
handlerInput.redirectOnSignInComplete,
);
},
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
createTokenCookiesRemoveOptions,
createTokenRemoveCookies,
getCookieValuesFromRequest,
getRedirectOrDefault,
revokeAuthNTokens,
} from '../../../src/auth/utils';

Expand All @@ -33,6 +34,7 @@ const mockCreateTokenRemoveCookies = jest.mocked(createTokenRemoveCookies);
const mockGetCookieValuesFromRequest = jest.mocked(getCookieValuesFromRequest);
const mockRevokeAuthNTokens = jest.mocked(revokeAuthNTokens);
const mockCreateKeysForAuthStorage = jest.mocked(createKeysForAuthStorage);
const mockGetRedirectOrDefault = jest.mocked(getRedirectOrDefault);

describe('handleSignOutCallbackRequest', () => {
const mockRequest = new Request(
Expand All @@ -45,12 +47,19 @@ describe('handleSignOutCallbackRequest', () => {
domain: '.example.com',
};

beforeAll(() => {
mockGetRedirectOrDefault.mockImplementation(
(redirect: string | undefined) => redirect || '/',
);
});

afterEach(() => {
mockAppendSetCookieHeaders.mockClear();
mockCreateTokenCookiesRemoveOptions.mockClear();
mockCreateTokenRemoveCookies.mockClear();
mockGetCookieValuesFromRequest.mockClear();
mockRevokeAuthNTokens.mockClear();
mockGetRedirectOrDefault.mockClear();
});

it(`returns a 400 response when the request does not have the "${IS_SIGNING_OUT_COOKIE_NAME}" cookie`, async () => {
Expand Down Expand Up @@ -125,6 +134,7 @@ describe('handleSignOutCallbackRequest', () => {
// verify the response
expect(response.status).toBe(302);
expect(response.headers.get('Location')).toBe('/');
expect(mockGetRedirectOrDefault).toHaveBeenCalledWith(undefined);

// verify the calls to dependencies
expect(mockGetCookieValuesFromRequest).toHaveBeenCalledWith(mockRequest, [
Expand Down Expand Up @@ -284,6 +294,9 @@ describe('handleSignOutCallbackRequest', () => {
mockCreateTokenRemoveCookiesResult,
mockCreateTokenCookiesRemoveOptionsResult,
);
expect(mockGetRedirectOrDefault).toHaveBeenCalledWith(
handlerInput.redirectOnSignOutComplete,
);
},
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
createTokenCookiesRemoveOptions,
createTokenRemoveCookies,
getCookieValuesFromNextApiRequest,
getRedirectOrDefault,
revokeAuthNTokens,
} from '../../../src/auth/utils';
import { createMockNextApiResponse } from '../testUtils';
Expand All @@ -39,6 +40,7 @@ const mockGetCookieValuesFromNextApiRequest = jest.mocked(
);
const mockRevokeAuthNTokens = jest.mocked(revokeAuthNTokens);
const mockCreateKeysForAuthStorage = jest.mocked(createKeysForAuthStorage);
const mockGetRedirectOrDefault = jest.mocked(getRedirectOrDefault);

describe('handleSignOutCallbackRequest', () => {
const mockRequest = {
Expand All @@ -59,12 +61,19 @@ describe('handleSignOutCallbackRequest', () => {
mockResponse,
} = createMockNextApiResponse();

beforeAll(() => {
mockGetRedirectOrDefault.mockImplementation(
(redirect: string | undefined) => redirect || '/',
);
});

afterEach(() => {
mockAppendSetCookieHeadersToNextApiResponse.mockClear();
mockCreateTokenCookiesRemoveOptions.mockClear();
mockCreateTokenRemoveCookies.mockClear();
mockGetCookieValuesFromNextApiRequest.mockClear();
mockRevokeAuthNTokens.mockClear();
mockGetRedirectOrDefault.mockClear();

mockResponseAppendHeader.mockClear();
mockResponseEnd.mockClear();
Expand Down Expand Up @@ -114,6 +123,7 @@ describe('handleSignOutCallbackRequest', () => {

// verify the response
expect(mockResponseRedirect).toHaveBeenCalledWith(302, '/');
expect(mockGetRedirectOrDefault).toHaveBeenCalledWith(undefined);

// verify the calls to dependencies
expect(mockGetCookieValuesFromNextApiRequest).toHaveBeenCalledWith(
Expand Down Expand Up @@ -332,6 +342,9 @@ describe('handleSignOutCallbackRequest', () => {
mockCreateTokenRemoveCookiesResult,
mockCreateTokenCookiesRemoveOptionsResult,
);
expect(mockGetRedirectOrDefault).toHaveBeenCalledWith(
handlerInput.redirectOnSignOutComplete,
);
},
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ describe('cognitoHostedUIEndpoints', () => {
test.each(testCase)(
'factory %s returns expected url: %s',
(fn, expected, args) => {
// eslint-disable-next-line import/namespace
expect(cognitoHostedUIEndpoints[fn].apply(null, args)).toBe(expected);
},
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { getRedirectOrDefault } from '../../../src/auth/utils/getRedirectOrDefault';

describe('getRedirectOrDefault', () => {
test.each([
[undefined, '/'],
['', '/'],
['/home', '/home'],
])('when input redirect is `%s` returns `%s`', (input, output) => {
expect(getRedirectOrDefault(input)).toBe(output);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import { HandleAuthApiRouteRequestForAppRouter } from './types';
import {
getRedirectOrDefault,
hasActiveUserSessionWithAppRouter,
isSupportedAuthApiRoutePath,
} from './utils';
Expand Down Expand Up @@ -49,7 +50,9 @@ export const handleAuthApiRouteRequestForAppRouter: HandleAuthApiRouteRequestFor
return new Response(null, {
status: 302,
headers: new Headers({
Location: handlerInput.redirectOnSignInComplete ?? '/',
Location: getRedirectOrDefault(
handlerInput.redirectOnSignInComplete,
),
}),
});
}
Expand All @@ -74,7 +77,9 @@ export const handleAuthApiRouteRequestForAppRouter: HandleAuthApiRouteRequestFor
return new Response(null, {
status: 302,
headers: new Headers({
Location: handlerInput.redirectOnSignInComplete ?? '/',
Location: getRedirectOrDefault(
handlerInput.redirectOnSignInComplete,
),
}),
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import { HandleAuthApiRouteRequestForPagesRouter } from './types';
import {
getRedirectOrDefault,
hasActiveUserSessionWithPagesRouter,
isSupportedAuthApiRoutePath,
} from './utils';
Expand Down Expand Up @@ -53,7 +54,10 @@ export const handleAuthApiRouteRequestForPagesRouter: HandleAuthApiRouteRequestF
});

if (hasActiveUserSession) {
response.redirect(302, handlerInput.redirectOnSignInComplete ?? '/');
response.redirect(
302,
getRedirectOrDefault(handlerInput.redirectOnSignInComplete),
);

return;
}
Expand All @@ -78,7 +82,10 @@ export const handleAuthApiRouteRequestForPagesRouter: HandleAuthApiRouteRequestF
});

if (hasActiveUserSession) {
response.redirect(302, handlerInput.redirectOnSignInComplete ?? '/');
response.redirect(
302,
getRedirectOrDefault(handlerInput.redirectOnSignInComplete),
);

return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
createTokenCookiesSetOptions,
exchangeAuthNTokens,
getCookieValuesFromRequest,
getRedirectOrDefault,
resolveCodeAndStateFromUrl,
resolveRedirectSignInUrl,
} from '../utils';
Expand Down Expand Up @@ -74,7 +75,9 @@ export const handleSignInCallbackRequest: HandleSignInCallbackRequest = async ({

return new Response(
createOnSignInCompleteRedirectIntermediate({
redirectOnSignInComplete: handlerInput.redirectOnSignInComplete || '/',
redirectOnSignInComplete: getRedirectOrDefault(
handlerInput.redirectOnSignInComplete,
),
}),
{
status: 200,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
createTokenCookiesSetOptions,
exchangeAuthNTokens,
getCookieValuesFromNextApiRequest,
getRedirectOrDefault,
resolveCodeAndStateFromUrl,
resolveRedirectSignInUrl,
} from '../utils';
Expand Down Expand Up @@ -86,8 +87,9 @@ export const handleSignInCallbackRequestForPagesRouter: HandleSignInCallbackRequ
.status(200)
.send(
createOnSignInCompleteRedirectIntermediate({
redirectOnSignInComplete:
handlerInput.redirectOnSignInComplete || '/',
redirectOnSignInComplete: getRedirectOrDefault(
handlerInput.redirectOnSignInComplete,
),
}),
);
};
Loading

0 comments on commit f8d978b

Please sign in to comment.