From 56dd74123b35a9c3e72d0faa292e5e81d2eb6ef7 Mon Sep 17 00:00:00 2001 From: Hui Zhao Date: Fri, 20 Dec 2024 17:46:32 -0800 Subject: [PATCH] chore(adapter-nextjs): resolve comments --- .../handleSignInCallbackRequest.test.ts | 17 ++++--- ...ignInCallbackRequestForPagesRouter.test.ts | 12 ++--- ...CompleteRedirectIntermediate.test.ts.snap} | 0 ...ignInCompleteRedirectIntermediate.test.ts} | 4 +- .../resolveIdentityProviderFromUrl.test.ts | 1 + .../__tests__/auth/utils/tokenCookies.test.ts | 46 +++++++++---------- .../handlers/handleSignInCallbackRequest.ts | 6 +-- ...ndleSignInCallbackRequestForPagesRouter.ts | 6 +-- ...handleSignInSignUpRequestForPagesRouter.ts | 8 ++-- .../handleSignOutRequestForPagesRouter.ts | 2 +- ...teOnSignInCompleteRedirectIntermediate.ts} | 2 +- .../src/auth/utils/createUrlSearchParams.ts | 8 +--- .../getCookieValuesFromNextApiRequest.ts | 16 ++++--- .../auth/utils/getCookieValuesFromRequest.ts | 11 ++--- .../auth/utils/getSearchParamValueFromUrl.ts | 19 +++----- .../adapter-nextjs/src/auth/utils/index.ts | 2 +- .../utils/resolveIdentityProviderFromUrl.ts | 2 +- 17 files changed, 77 insertions(+), 85 deletions(-) rename packages/adapter-nextjs/__tests__/auth/utils/__snapshots__/{createOnSignInCompletedRedirectIntermediate.test.ts.snap => createOnSignInCompleteRedirectIntermediate.test.ts.snap} (100%) rename packages/adapter-nextjs/__tests__/auth/utils/{createOnSignInCompletedRedirectIntermediate.test.ts => createOnSignInCompleteRedirectIntermediate.test.ts} (58%) rename packages/adapter-nextjs/src/auth/utils/{createOnSignInCompletedRedirectIntermediate.ts => createOnSignInCompleteRedirectIntermediate.ts} (93%) diff --git a/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequest.test.ts b/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequest.test.ts index 8cd13a079cb..4a7dea1bd87 100644 --- a/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequest.test.ts +++ b/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequest.test.ts @@ -9,7 +9,7 @@ import { handleSignInCallbackRequest } from '../../../src/auth/handlers/handleSi import { appendSetCookieHeaders, createAuthFlowProofCookiesRemoveOptions, - createOnSignInCompletedRedirectIntermediate, + createOnSignInCompleteRedirectIntermediate, createSignInFlowProofCookies, createTokenCookies, createTokenCookiesSetOptions, @@ -30,8 +30,8 @@ const mockAppendSetCookieHeaders = jest.mocked(appendSetCookieHeaders); const mockCreateAuthFlowProofCookiesRemoveOptions = jest.mocked( createAuthFlowProofCookiesRemoveOptions, ); -const mockCreateOnSignInCompletedRedirectIntermediate = jest.mocked( - createOnSignInCompletedRedirectIntermediate, +const mockCreateOnSignInCompleteRedirectIntermediate = jest.mocked( + createOnSignInCompleteRedirectIntermediate, ); const mockCreateSignInFlowProofCookies = jest.mocked( createSignInFlowProofCookies, @@ -58,7 +58,7 @@ describe('handleSignInCallbackRequest', () => { afterEach(() => { mockAppendSetCookieHeaders.mockClear(); mockCreateAuthFlowProofCookiesRemoveOptions.mockClear(); - mockCreateOnSignInCompletedRedirectIntermediate.mockClear(); + mockCreateOnSignInCompleteRedirectIntermediate.mockClear(); mockCreateSignInFlowProofCookies.mockClear(); mockCreateTokenCookies.mockClear(); mockCreateTokenCookiesSetOptions.mockClear(); @@ -188,6 +188,11 @@ describe('handleSignInCallbackRequest', () => { '/', `redirect to /`, ], + [ + { ...mockHandlerInput, redirectOnSignInComplete: '' }, + '/', + `redirect to /`, + ], ] as [CreateAuthRoutesHandlersInput, string, string][])( 'returns a 200 response with expected redirect target: with handlerInput=%p, expectedFinalRedirect=%s, generates expected html=%s', async (handlerInput, expectedFinalRedirect, expectedHtml) => { @@ -245,7 +250,7 @@ describe('handleSignInCallbackRequest', () => { headers.append('Set-cookie', 'mock-cookie-1'); headers.append('Set-cookie', 'mock-cookie-2'); }); - mockCreateOnSignInCompletedRedirectIntermediate.mockImplementationOnce( + mockCreateOnSignInCompleteRedirectIntermediate.mockImplementationOnce( ({ redirectOnSignInComplete }) => `redirect to ${redirectOnSignInComplete}`, ); @@ -297,7 +302,7 @@ describe('handleSignInCallbackRequest', () => { mockCreateAuthFlowProofCookiesRemoveOptionsResult, ); expect( - mockCreateOnSignInCompletedRedirectIntermediate, + mockCreateOnSignInCompleteRedirectIntermediate, ).toHaveBeenCalledWith({ redirectOnSignInComplete: expectedFinalRedirect, }); diff --git a/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequestForPagesRouter.test.ts b/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequestForPagesRouter.test.ts index 992ceca7ae4..918e1c255ca 100644 --- a/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequestForPagesRouter.test.ts +++ b/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequestForPagesRouter.test.ts @@ -9,7 +9,7 @@ import { handleSignInCallbackRequestForPagesRouter } from '../../../src/auth/han import { appendSetCookieHeadersToNextApiResponse, createAuthFlowProofCookiesRemoveOptions, - createOnSignInCompletedRedirectIntermediate, + createOnSignInCompleteRedirectIntermediate, createSignInFlowProofCookies, createTokenCookies, createTokenCookiesSetOptions, @@ -33,8 +33,8 @@ const mockAppendSetCookieHeadersToNextApiResponse = jest.mocked( const mockCreateAuthFlowProofCookiesRemoveOptions = jest.mocked( createAuthFlowProofCookiesRemoveOptions, ); -const mockCreateOnSignInCompletedRedirectIntermediate = jest.mocked( - createOnSignInCompletedRedirectIntermediate, +const mockCreateOnSignInCompleteRedirectIntermediate = jest.mocked( + createOnSignInCompleteRedirectIntermediate, ); const mockCreateSignInFlowProofCookies = jest.mocked( createSignInFlowProofCookies, @@ -70,7 +70,7 @@ describe('handleSignInCallbackRequest', () => { afterEach(() => { mockAppendSetCookieHeadersToNextApiResponse.mockClear(); mockCreateAuthFlowProofCookiesRemoveOptions.mockClear(); - mockCreateOnSignInCompletedRedirectIntermediate.mockClear(); + mockCreateOnSignInCompleteRedirectIntermediate.mockClear(); mockCreateSignInFlowProofCookies.mockClear(); mockCreateTokenCookies.mockClear(); mockCreateTokenCookiesSetOptions.mockClear(); @@ -283,7 +283,7 @@ describe('handleSignInCallbackRequest', () => { response.appendHeader('Set-cookie', 'mock-cookie-2'); }, ); - mockCreateOnSignInCompletedRedirectIntermediate.mockImplementationOnce( + mockCreateOnSignInCompleteRedirectIntermediate.mockImplementationOnce( ({ redirectOnSignInComplete }) => `redirect to ${redirectOnSignInComplete}`, ); @@ -334,7 +334,7 @@ describe('handleSignInCallbackRequest', () => { ); expect( - mockCreateOnSignInCompletedRedirectIntermediate, + mockCreateOnSignInCompleteRedirectIntermediate, ).toHaveBeenCalledWith({ redirectOnSignInComplete: expectedFinalRedirect, }); diff --git a/packages/adapter-nextjs/__tests__/auth/utils/__snapshots__/createOnSignInCompletedRedirectIntermediate.test.ts.snap b/packages/adapter-nextjs/__tests__/auth/utils/__snapshots__/createOnSignInCompleteRedirectIntermediate.test.ts.snap similarity index 100% rename from packages/adapter-nextjs/__tests__/auth/utils/__snapshots__/createOnSignInCompletedRedirectIntermediate.test.ts.snap rename to packages/adapter-nextjs/__tests__/auth/utils/__snapshots__/createOnSignInCompleteRedirectIntermediate.test.ts.snap diff --git a/packages/adapter-nextjs/__tests__/auth/utils/createOnSignInCompletedRedirectIntermediate.test.ts b/packages/adapter-nextjs/__tests__/auth/utils/createOnSignInCompleteRedirectIntermediate.test.ts similarity index 58% rename from packages/adapter-nextjs/__tests__/auth/utils/createOnSignInCompletedRedirectIntermediate.test.ts rename to packages/adapter-nextjs/__tests__/auth/utils/createOnSignInCompleteRedirectIntermediate.test.ts index d70a4f47285..0dcb8292689 100644 --- a/packages/adapter-nextjs/__tests__/auth/utils/createOnSignInCompletedRedirectIntermediate.test.ts +++ b/packages/adapter-nextjs/__tests__/auth/utils/createOnSignInCompleteRedirectIntermediate.test.ts @@ -1,9 +1,9 @@ -import { createOnSignInCompletedRedirectIntermediate } from '../../../src/auth/utils/createOnSignInCompletedRedirectIntermediate'; +import { createOnSignInCompleteRedirectIntermediate } from '../../../src/auth/utils/createOnSignInCompleteRedirectIntermediate'; describe('createOnSignInCompletedRedirectIntermediate', () => { it('returns html with script that redirects to the redirectUrl', () => { const redirectUrl = 'https://example.com'; - const result = createOnSignInCompletedRedirectIntermediate({ + const result = createOnSignInCompleteRedirectIntermediate({ redirectOnSignInComplete: redirectUrl, }); diff --git a/packages/adapter-nextjs/__tests__/auth/utils/resolveIdentityProviderFromUrl.test.ts b/packages/adapter-nextjs/__tests__/auth/utils/resolveIdentityProviderFromUrl.test.ts index 0f64f537b4d..baab3c17d0e 100644 --- a/packages/adapter-nextjs/__tests__/auth/utils/resolveIdentityProviderFromUrl.test.ts +++ b/packages/adapter-nextjs/__tests__/auth/utils/resolveIdentityProviderFromUrl.test.ts @@ -3,6 +3,7 @@ import { resolveIdentityProviderFromUrl } from '../../../src/auth/utils/resolveI describe('resolveIdentityProviderFromUrl', () => { test.each([ ['https://example.com?provider=Google', 'Google'], + ['https://example.com?provider=GOogLe', 'Google'], ['https://example.com?provider=Facebook', 'Facebook'], ['https://example.com?provider=Amazon', 'LoginWithAmazon'], ['https://example.com?provider=Apple', 'SignInWithApple'], diff --git a/packages/adapter-nextjs/__tests__/auth/utils/tokenCookies.test.ts b/packages/adapter-nextjs/__tests__/auth/utils/tokenCookies.test.ts index 45052089cc8..926d10e798f 100644 --- a/packages/adapter-nextjs/__tests__/auth/utils/tokenCookies.test.ts +++ b/packages/adapter-nextjs/__tests__/auth/utils/tokenCookies.test.ts @@ -44,30 +44,28 @@ describe('createTokenCookies', () => { userPoolClientId: mockUserPoolClientId, }); - expect(result.sort()).toEqual( - [ - { - name: `${expectedCookieNamePrefix}.accessToken`, - value: 'access_token', - }, - { - name: `${expectedCookieNamePrefix}.idToken`, - value: 'id_token', - }, - { - name: `${expectedCookieNamePrefix}.refreshToken`, - value: 'refresh_token', - }, - { - name: `${expectedCookieNamePrefix}.clockDrift`, - value: '-42', - }, - { - name: `${AUTH_KEY_PREFIX}.${mockUserPoolClientId}.LastAuthUser`, - value: mockUserName, - }, - ].sort(), - ); + expect(result).toEqual([ + { + name: `${expectedCookieNamePrefix}.accessToken`, + value: 'access_token', + }, + { + name: `${expectedCookieNamePrefix}.idToken`, + value: 'id_token', + }, + { + name: `${expectedCookieNamePrefix}.refreshToken`, + value: 'refresh_token', + }, + { + name: `${expectedCookieNamePrefix}.clockDrift`, + value: '-42', + }, + { + name: `${AUTH_KEY_PREFIX}.${mockUserPoolClientId}.LastAuthUser`, + value: mockUserName, + }, + ]); }); }); diff --git a/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts b/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts index dbca5764b70..e1689c8f73d 100644 --- a/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts +++ b/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts @@ -5,7 +5,7 @@ import { PKCE_COOKIE_NAME, STATE_COOKIE_NAME } from '../constant'; import { appendSetCookieHeaders, createAuthFlowProofCookiesRemoveOptions, - createOnSignInCompletedRedirectIntermediate, + createOnSignInCompleteRedirectIntermediate, createSignInFlowProofCookies, createTokenCookies, createTokenCookiesSetOptions, @@ -73,8 +73,8 @@ export const handleSignInCallbackRequest: HandleSignInCallbackRequest = async ({ headers.set('Content-Type', 'text/html'); return new Response( - createOnSignInCompletedRedirectIntermediate({ - redirectOnSignInComplete: handlerInput.redirectOnSignInComplete ?? '/', + createOnSignInCompleteRedirectIntermediate({ + redirectOnSignInComplete: handlerInput.redirectOnSignInComplete || '/', }), { status: 200, diff --git a/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequestForPagesRouter.ts b/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequestForPagesRouter.ts index d9717168f34..e7210530759 100644 --- a/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequestForPagesRouter.ts +++ b/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequestForPagesRouter.ts @@ -5,7 +5,7 @@ import { PKCE_COOKIE_NAME, STATE_COOKIE_NAME } from '../constant'; import { appendSetCookieHeadersToNextApiResponse, createAuthFlowProofCookiesRemoveOptions, - createOnSignInCompletedRedirectIntermediate, + createOnSignInCompleteRedirectIntermediate, createSignInFlowProofCookies, createTokenCookies, createTokenCookiesSetOptions, @@ -85,9 +85,9 @@ export const handleSignInCallbackRequestForPagesRouter: HandleSignInCallbackRequ .appendHeader('Content-Type', 'text/html') .status(200) .send( - createOnSignInCompletedRedirectIntermediate({ + createOnSignInCompleteRedirectIntermediate({ redirectOnSignInComplete: - handlerInput.redirectOnSignInComplete ?? '/', + handlerInput.redirectOnSignInComplete || '/', }), ); }; diff --git a/packages/adapter-nextjs/src/auth/handlers/handleSignInSignUpRequestForPagesRouter.ts b/packages/adapter-nextjs/src/auth/handlers/handleSignInSignUpRequestForPagesRouter.ts index 065f28fbfa9..056ddceab0f 100644 --- a/packages/adapter-nextjs/src/auth/handlers/handleSignInSignUpRequestForPagesRouter.ts +++ b/packages/adapter-nextjs/src/auth/handlers/handleSignInSignUpRequestForPagesRouter.ts @@ -40,10 +40,10 @@ export const handleSignInSignUpRequestForPagesRouter: HandleSignInSignUpRequestF createAuthFlowProofCookiesSetOptions(setCookieOptions), ); - response.redirect( - 302, + const redirectUrl = type === 'signIn' ? createAuthorizeEndpoint(oAuthConfig.domain, redirectUrlSearchParams) - : createSignUpEndpoint(oAuthConfig.domain, redirectUrlSearchParams), - ); + : createSignUpEndpoint(oAuthConfig.domain, redirectUrlSearchParams); + + response.redirect(302, redirectUrl); }; diff --git a/packages/adapter-nextjs/src/auth/handlers/handleSignOutRequestForPagesRouter.ts b/packages/adapter-nextjs/src/auth/handlers/handleSignOutRequestForPagesRouter.ts index bf4d21f8c64..6b12bd84e2e 100644 --- a/packages/adapter-nextjs/src/auth/handlers/handleSignOutRequestForPagesRouter.ts +++ b/packages/adapter-nextjs/src/auth/handlers/handleSignOutRequestForPagesRouter.ts @@ -26,6 +26,6 @@ export const handleSignOutRequestForPagesRouter: HandleSignOutRequestForPagesRou response.redirect( 302, - createLogoutEndpoint(oAuthConfig.domain, urlSearchParams).toString(), + createLogoutEndpoint(oAuthConfig.domain, urlSearchParams), ); }; diff --git a/packages/adapter-nextjs/src/auth/utils/createOnSignInCompletedRedirectIntermediate.ts b/packages/adapter-nextjs/src/auth/utils/createOnSignInCompleteRedirectIntermediate.ts similarity index 93% rename from packages/adapter-nextjs/src/auth/utils/createOnSignInCompletedRedirectIntermediate.ts rename to packages/adapter-nextjs/src/auth/utils/createOnSignInCompleteRedirectIntermediate.ts index 320068edae0..dea9f18f3da 100644 --- a/packages/adapter-nextjs/src/auth/utils/createOnSignInCompletedRedirectIntermediate.ts +++ b/packages/adapter-nextjs/src/auth/utils/createOnSignInCompleteRedirectIntermediate.ts @@ -1,7 +1,7 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -export const createOnSignInCompletedRedirectIntermediate = ({ +export const createOnSignInCompleteRedirectIntermediate = ({ redirectOnSignInComplete, }: { redirectOnSignInComplete: string; diff --git a/packages/adapter-nextjs/src/auth/utils/createUrlSearchParams.ts b/packages/adapter-nextjs/src/auth/utils/createUrlSearchParams.ts index fc584dd60f4..f01b318b38c 100644 --- a/packages/adapter-nextjs/src/auth/utils/createUrlSearchParams.ts +++ b/packages/adapter-nextjs/src/auth/utils/createUrlSearchParams.ts @@ -47,13 +47,9 @@ export const createUrlSearchParamsForTokenExchange = (input: { redirect_uri: string; code_verifier: string; grant_type: string; -}): URLSearchParams => createUrlSearchParamsFromObject(input); +}): URLSearchParams => new URLSearchParams(input); export const createUrlSearchParamsForTokenRevocation = (input: { token: string; client_id: string; -}): URLSearchParams => createUrlSearchParamsFromObject(input); - -const createUrlSearchParamsFromObject = ( - input: Record, -): URLSearchParams => new URLSearchParams(input); +}): URLSearchParams => new URLSearchParams(input); diff --git a/packages/adapter-nextjs/src/auth/utils/getCookieValuesFromNextApiRequest.ts b/packages/adapter-nextjs/src/auth/utils/getCookieValuesFromNextApiRequest.ts index d770d1ca57d..c625e392797 100644 --- a/packages/adapter-nextjs/src/auth/utils/getCookieValuesFromNextApiRequest.ts +++ b/packages/adapter-nextjs/src/auth/utils/getCookieValuesFromNextApiRequest.ts @@ -3,18 +3,20 @@ import { NextApiRequest } from 'next'; -export const getCookieValuesFromNextApiRequest = ( +export const getCookieValuesFromNextApiRequest = < + CookieNames extends string[], + R = { + [key in CookieNames[number]]?: string | undefined; + }, +>( request: NextApiRequest, cookieNames: CookieNames, -): { - [key in CookieNames[number]]?: string | undefined; -} => { +): R => { const result: Record = {}; + for (const cookieName of cookieNames) { result[cookieName] = request.cookies[cookieName]; } - return result as { - [key in CookieNames[number]]?: string | undefined; - }; + return result as R; }; diff --git a/packages/adapter-nextjs/src/auth/utils/getCookieValuesFromRequest.ts b/packages/adapter-nextjs/src/auth/utils/getCookieValuesFromRequest.ts index 563d59773ff..44a69e6c97b 100644 --- a/packages/adapter-nextjs/src/auth/utils/getCookieValuesFromRequest.ts +++ b/packages/adapter-nextjs/src/auth/utils/getCookieValuesFromRequest.ts @@ -16,14 +16,11 @@ export const getCookieValuesFromRequest = ( const cookieValues: Record = cookieHeader .split(';') .map(cookie => cookie.trim().split('=')) - .reduce( - (result, [key, value]) => { - result[key] = value; + .reduce>((result, [key, value]) => { + result[key] = value; - return result; - }, - {} as Record, - ); + return result; + }, {}); const result: Record = {}; for (const cookieName of cookieNames) { diff --git a/packages/adapter-nextjs/src/auth/utils/getSearchParamValueFromUrl.ts b/packages/adapter-nextjs/src/auth/utils/getSearchParamValueFromUrl.ts index f3f5e78ca42..b368fa409f5 100644 --- a/packages/adapter-nextjs/src/auth/utils/getSearchParamValueFromUrl.ts +++ b/packages/adapter-nextjs/src/auth/utils/getSearchParamValueFromUrl.ts @@ -5,19 +5,12 @@ export const getSearchParamValueFromUrl = ( urlStr: string, paramName: string, ): string | null => { - try { - return new URL(urlStr).searchParams.get(paramName); - } catch (error) { - // In Next.js Pages Router the request object is an instance of IncomingMessage - // whose url property may contain only the path part of the URL + query params. - // In this case, we need to parse the URL manually - if (urlStr.includes('?')) { - const queryParams = urlStr.split('?')[1]; - if (queryParams) { - return new URLSearchParams(queryParams).get(paramName); - } + if (urlStr.includes('?')) { + const queryParams = urlStr.split('?')[1]; + if (queryParams) { + return new URLSearchParams(queryParams).get(paramName); } - - return null; } + + return null; }; diff --git a/packages/adapter-nextjs/src/auth/utils/index.ts b/packages/adapter-nextjs/src/auth/utils/index.ts index 184074a9896..a82d25f3dca 100644 --- a/packages/adapter-nextjs/src/auth/utils/index.ts +++ b/packages/adapter-nextjs/src/auth/utils/index.ts @@ -11,7 +11,7 @@ export { createAuthFlowProofCookiesRemoveOptions, } from './authFlowProofCookies'; export { createAuthFlowProofs } from './createAuthFlowProofs'; -export { createOnSignInCompletedRedirectIntermediate } from './createOnSignInCompletedRedirectIntermediate'; +export { createOnSignInCompleteRedirectIntermediate } from './createOnSignInCompleteRedirectIntermediate'; export { createUrlSearchParamsForSignInSignUp } from './createUrlSearchParams'; export { createAuthorizeEndpoint, diff --git a/packages/adapter-nextjs/src/auth/utils/resolveIdentityProviderFromUrl.ts b/packages/adapter-nextjs/src/auth/utils/resolveIdentityProviderFromUrl.ts index f3897b365db..ca9ca690ca3 100644 --- a/packages/adapter-nextjs/src/auth/utils/resolveIdentityProviderFromUrl.ts +++ b/packages/adapter-nextjs/src/auth/utils/resolveIdentityProviderFromUrl.ts @@ -17,4 +17,4 @@ const resolveProvider = (provider: string | null): string | null => { }; const capitalize = (value: string) => - `${value[0].toUpperCase()}${value.substring(1)}`; + `${value[0].toUpperCase()}${value.substring(1).toLowerCase()}`;