From 91a0248419aea92528a9fb29c780785ef7a3a07f Mon Sep 17 00:00:00 2001 From: Hui Zhao Date: Mon, 6 Jan 2025 17:26:26 -0800 Subject: [PATCH 1/3] feat(adapter-nextjs): surface redirect error and sign-in timeout error --- .../handleSignInCallbackRequest.test.ts | 85 +++++++++------ ...ignInCallbackRequestForPagesRouter.test.ts | 103 ++++++++++++------ .../signInCallbackErrorCombinations.ts | 60 ++++++++++ .../auth/utils/parseSignInCallbackUrl.test.ts | 16 +++ .../utils/resolveCodeAndStateFromUrl.test.ts | 13 --- packages/adapter-nextjs/src/auth/constant.ts | 14 ++- .../handlers/handleSignInCallbackRequest.ts | 36 +++++- ...ndleSignInCallbackRequestForPagesRouter.ts | 35 +++++- .../adapter-nextjs/src/auth/utils/index.ts | 2 +- ...teFromUrl.ts => parseSignInCallbackUrl.ts} | 6 +- 10 files changed, 276 insertions(+), 94 deletions(-) create mode 100644 packages/adapter-nextjs/__tests__/auth/handlers/signInCallbackErrorCombinations.ts create mode 100644 packages/adapter-nextjs/__tests__/auth/utils/parseSignInCallbackUrl.test.ts delete mode 100644 packages/adapter-nextjs/__tests__/auth/utils/resolveCodeAndStateFromUrl.test.ts rename packages/adapter-nextjs/src/auth/utils/{resolveCodeAndStateFromUrl.ts => parseSignInCallbackUrl.ts} (62%) diff --git a/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequest.test.ts b/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequest.test.ts index ef3ee357a0a..3f54a94a33d 100644 --- a/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequest.test.ts +++ b/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequest.test.ts @@ -16,7 +16,7 @@ import { exchangeAuthNTokens, getCookieValuesFromRequest, getRedirectOrDefault, - resolveCodeAndStateFromUrl, + parseSignInCallbackUrl, resolveRedirectSignInUrl, } from '../../../src/auth/utils'; import { CreateAuthRoutesHandlersInput } from '../../../src/auth/types'; @@ -25,6 +25,11 @@ import { STATE_COOKIE_NAME, } from '../../../src/auth/constant'; +import { + ERROR_CLIENT_COOKIE_COMBINATIONS, + ERROR_URL_PARAMS_COMBINATIONS, +} from './signInCallbackErrorCombinations'; + jest.mock('../../../src/auth/utils'); const mockAppendSetCookieHeaders = jest.mocked(appendSetCookieHeaders); @@ -43,7 +48,7 @@ const mockCreateTokenCookiesSetOptions = jest.mocked( ); const mockExchangeAuthNTokens = jest.mocked(exchangeAuthNTokens); const mockGetCookieValuesFromRequest = jest.mocked(getCookieValuesFromRequest); -const mockResolveCodeAndStateFromUrl = jest.mocked(resolveCodeAndStateFromUrl); +const mockParseSignInCallbackUrl = jest.mocked(parseSignInCallbackUrl); const mockResolveRedirectSignInUrl = jest.mocked(resolveRedirectSignInUrl); const mockGetRedirectOrDefault = jest.mocked(getRedirectOrDefault); @@ -72,19 +77,25 @@ describe('handleSignInCallbackRequest', () => { mockCreateTokenCookiesSetOptions.mockClear(); mockExchangeAuthNTokens.mockClear(); mockGetCookieValuesFromRequest.mockClear(); - mockResolveCodeAndStateFromUrl.mockClear(); + mockParseSignInCallbackUrl.mockClear(); mockResolveRedirectSignInUrl.mockClear(); }); - test.each([ - [null, 'state'], - ['state', null], - ])( - 'returns a 400 response when request.url contains query params: code=%s, state=%s', - async (code, state) => { - mockResolveCodeAndStateFromUrl.mockReturnValueOnce({ + test.each(ERROR_URL_PARAMS_COMBINATIONS)( + 'returns a $expectedStatus response when request.url contains query params: code=$code, state=$state, error=$error, error_description=$errorDescription', + async ({ + code, + state, + error, + errorDescription, + expectedStatus, + expectedRedirect, + }) => { + mockParseSignInCallbackUrl.mockReturnValueOnce({ code, state, + error, + errorDescription, }); const url = 'https://example.com/api/auth/sign-in-callback'; const request = new NextRequest(new URL(url)); @@ -98,33 +109,30 @@ describe('handleSignInCallbackRequest', () => { origin: mockOrigin, }); - expect(response.status).toBe(400); - expect(mockResolveCodeAndStateFromUrl).toHaveBeenCalledWith(url); + expect(response.status).toBe(expectedStatus); + expect(mockParseSignInCallbackUrl).toHaveBeenCalledWith(url); + + if (expectedStatus === 302) { + expect(response.headers.get('Location')).toBe(expectedRedirect); + } }, ); - test.each([ - ['client state cookie is missing', undefined, 'state', 'pkce'], - [ - 'client cookie state a different value from the state query parameter', - 'state_different', - 'state', - 'pkce', - ], - ['client pkce cookie is missing', 'state', 'state', undefined], - ])( - `returns a 400 response when %s`, - async (_, clientState, state, clientPkce) => { - mockResolveCodeAndStateFromUrl.mockReturnValueOnce({ + test.each(ERROR_CLIENT_COOKIE_COMBINATIONS)( + `returns a $expectedStatus response when client cookies are: state=$state, pkce=$pkce and expected state value is 'state_b'`, + async ({ state, pkce, expectedStatus, expectedRedirect }) => { + mockParseSignInCallbackUrl.mockReturnValueOnce({ code: 'not_important_for_this_test', - state, + state: 'not_important_for_this_test', + error: null, + errorDescription: null, }); mockGetCookieValuesFromRequest.mockReturnValueOnce({ - [STATE_COOKIE_NAME]: clientState, - [PKCE_COOKIE_NAME]: clientPkce, + [STATE_COOKIE_NAME]: state, + [PKCE_COOKIE_NAME]: pkce, }); - const url = `https://example.com/api/auth/sign-in-callback?state=${state}&code=not_important_for_this_test`; + const url = `https://example.com/api/auth/sign-in-callback?state=state_b&code=not_important_for_this_test`; const request = new NextRequest(new URL(url)); const response = await handleSignInCallbackRequest({ @@ -136,12 +144,19 @@ describe('handleSignInCallbackRequest', () => { origin: mockOrigin, }); - expect(response.status).toBe(400); - expect(mockResolveCodeAndStateFromUrl).toHaveBeenCalledWith(url); + expect(response.status).toBe(expectedStatus); + expect(mockParseSignInCallbackUrl).toHaveBeenCalledWith(url); expect(mockGetCookieValuesFromRequest).toHaveBeenCalledWith(request, [ PKCE_COOKIE_NAME, STATE_COOKIE_NAME, ]); + + if (expectedStatus === 302) { + expect(mockGetRedirectOrDefault).toHaveBeenCalledWith( + mockHandlerInput.redirectOnSignOutComplete, + ); + expect(response.headers.get('Location')).toBe(expectedRedirect); + } }, ); @@ -151,9 +166,11 @@ describe('handleSignInCallbackRequest', () => { const mockSignInCallbackUrl = 'https://example.com/api/auth/sign-in-callback'; const mockError = 'invalid_grant'; - mockResolveCodeAndStateFromUrl.mockReturnValueOnce({ + mockParseSignInCallbackUrl.mockReturnValueOnce({ code: mockCode, state: 'not_important_for_this_test', + error: null, + errorDescription: null, }); mockGetCookieValuesFromRequest.mockReturnValueOnce({ [STATE_COOKIE_NAME]: 'not_important_for_this_test', @@ -245,9 +262,11 @@ describe('handleSignInCallbackRequest', () => { mockCreateAuthFlowProofCookiesRemoveOptions.mockReturnValueOnce( mockCreateAuthFlowProofCookiesRemoveOptionsResult, ); - mockResolveCodeAndStateFromUrl.mockReturnValueOnce({ + mockParseSignInCallbackUrl.mockReturnValueOnce({ code: mockCode, state: 'not_important_for_this_test', + error: null, + errorDescription: null, }); mockGetCookieValuesFromRequest.mockReturnValueOnce({ [STATE_COOKIE_NAME]: 'not_important_for_this_test', diff --git a/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequestForPagesRouter.test.ts b/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequestForPagesRouter.test.ts index 5fd5612ae0c..c59c580c474 100644 --- a/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequestForPagesRouter.test.ts +++ b/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequestForPagesRouter.test.ts @@ -16,7 +16,7 @@ import { exchangeAuthNTokens, getCookieValuesFromNextApiRequest, getRedirectOrDefault, - resolveCodeAndStateFromUrl, + parseSignInCallbackUrl, resolveRedirectSignInUrl, } from '../../../src/auth/utils'; import { CreateAuthRoutesHandlersInput } from '../../../src/auth/types'; @@ -26,6 +26,11 @@ import { } from '../../../src/auth/constant'; import { createMockNextApiResponse } from '../testUtils'; +import { + ERROR_CLIENT_COOKIE_COMBINATIONS, + ERROR_URL_PARAMS_COMBINATIONS, +} from './signInCallbackErrorCombinations'; + jest.mock('../../../src/auth/utils'); const mockAppendSetCookieHeadersToNextApiResponse = jest.mocked( @@ -48,7 +53,7 @@ const mockExchangeAuthNTokens = jest.mocked(exchangeAuthNTokens); const mockGetCookieValuesFromNextApiRequest = jest.mocked( getCookieValuesFromNextApiRequest, ); -const mockResolveCodeAndStateFromUrl = jest.mocked(resolveCodeAndStateFromUrl); +const mockParseSignInCallbackUrl = jest.mocked(parseSignInCallbackUrl); const mockResolveRedirectSignInUrl = jest.mocked(resolveRedirectSignInUrl); const mockGetRedirectOrDefault = jest.mocked(getRedirectOrDefault); @@ -66,6 +71,7 @@ describe('handleSignInCallbackRequest', () => { mockResponseEnd, mockResponseStatus, mockResponseSend, + mockResponseRedirect, mockResponse, } = createMockNextApiResponse(); @@ -84,7 +90,7 @@ describe('handleSignInCallbackRequest', () => { mockCreateTokenCookiesSetOptions.mockClear(); mockExchangeAuthNTokens.mockClear(); mockGetCookieValuesFromNextApiRequest.mockClear(); - mockResolveCodeAndStateFromUrl.mockClear(); + mockParseSignInCallbackUrl.mockClear(); mockResolveRedirectSignInUrl.mockClear(); mockGetRedirectOrDefault.mockClear(); @@ -92,17 +98,24 @@ describe('handleSignInCallbackRequest', () => { mockResponseEnd.mockClear(); mockResponseStatus.mockClear(); mockResponseSend.mockClear(); + mockResponseRedirect.mockClear(); }); - test.each([ - [null, 'state'], - ['state', null], - ])( - 'returns a 400 response when request.url contains query params: code=%s, state=%s', - async (code, state) => { - mockResolveCodeAndStateFromUrl.mockReturnValueOnce({ + test.each(ERROR_URL_PARAMS_COMBINATIONS)( + 'returns a $expectedStatus response when request.url contains query params: code=$code, state=$state, error=$error, error_description=$errorDescription', + async ({ + code, + state, + error, + errorDescription, + expectedStatus, + expectedRedirect, + }) => { + mockParseSignInCallbackUrl.mockReturnValueOnce({ code, state, + error, + errorDescription, }); const url = '/api/auth/sign-in-callback'; const mockRequest = { @@ -120,36 +133,40 @@ describe('handleSignInCallbackRequest', () => { origin: mockOrigin, }); - expect(mockResponseStatus).toHaveBeenCalledWith(400); - expect(mockResponseEnd).toHaveBeenCalled(); - expect(mockResolveCodeAndStateFromUrl).toHaveBeenCalledWith(url); + if (expectedStatus === 400) { + expect(mockResponseStatus).toHaveBeenCalledWith(expectedStatus); + expect(mockResponseEnd).toHaveBeenCalled(); + } else { + expect(mockGetRedirectOrDefault).toHaveBeenCalledWith( + mockHandlerInput.redirectOnSignOutComplete, + ); + expect(mockResponseRedirect).toHaveBeenCalledWith( + expectedStatus, + expectedRedirect, + ); + } + expect(mockParseSignInCallbackUrl).toHaveBeenCalledWith(url); }, ); - test.each([ - ['client state cookie is missing', undefined, 'state', 'pkce'], - [ - 'client cookie state a different value from the state query parameter', - 'state_different', - 'state', - 'pkce', - ], - ['client pkce cookie is missing', 'state', 'state', undefined], - ])( - `returns a 400 response when %s`, - async (_, clientState, state, clientPkce) => { - mockResolveCodeAndStateFromUrl.mockReturnValueOnce({ + test.each(ERROR_CLIENT_COOKIE_COMBINATIONS)( + `returns a $expectedStatus response when client cookies are: state=$state, pkce=$pkce and expected state value is 'state_b'`, + async ({ state, pkce, expectedStatus, expectedRedirect }) => { + mockParseSignInCallbackUrl.mockReturnValueOnce({ code: 'not_important_for_this_test', - state, + state: 'not_important_for_this_test', + error: null, + errorDescription: null, }); mockGetCookieValuesFromNextApiRequest.mockReturnValueOnce({ - [STATE_COOKIE_NAME]: clientState, - [PKCE_COOKIE_NAME]: clientPkce, + [STATE_COOKIE_NAME]: state, + [PKCE_COOKIE_NAME]: pkce, }); + const expectedState = 'state_b'; - const url = `/api/auth/sign-in-callback?state=${state}&code=not_important_for_this_test`; + const url = `/api/auth/sign-in-callback?state=${expectedState}&code=not_important_for_this_test`; const mockRequest = { - query: { state }, + query: { state: expectedState }, url, } as unknown as NextApiRequest; @@ -163,9 +180,19 @@ describe('handleSignInCallbackRequest', () => { origin: mockOrigin, }); - expect(mockResponseStatus).toHaveBeenCalledWith(400); - expect(mockResponseEnd).toHaveBeenCalled(); - expect(mockResolveCodeAndStateFromUrl).toHaveBeenCalledWith(url); + if (expectedStatus === 400) { + expect(mockResponseStatus).toHaveBeenCalledWith(expectedStatus); + expect(mockResponseEnd).toHaveBeenCalled(); + } else { + expect(mockGetRedirectOrDefault).toHaveBeenCalledWith( + mockHandlerInput.redirectOnSignOutComplete, + ); + expect(mockResponseRedirect).toHaveBeenCalledWith( + expectedStatus, + expectedRedirect, + ); + } + expect(mockParseSignInCallbackUrl).toHaveBeenCalledWith(url); expect(mockGetCookieValuesFromNextApiRequest).toHaveBeenCalledWith( mockRequest, [PKCE_COOKIE_NAME, STATE_COOKIE_NAME], @@ -183,9 +210,11 @@ describe('handleSignInCallbackRequest', () => { query: {}, url: '/api/auth/sign-in-callback', } as unknown as NextApiRequest; - mockResolveCodeAndStateFromUrl.mockReturnValueOnce({ + mockParseSignInCallbackUrl.mockReturnValueOnce({ code: mockCode, state: 'not_important_for_this_test', + error: null, + errorDescription: null, }); mockGetCookieValuesFromNextApiRequest.mockReturnValueOnce({ [STATE_COOKIE_NAME]: 'not_important_for_this_test', @@ -277,9 +306,11 @@ describe('handleSignInCallbackRequest', () => { mockCreateAuthFlowProofCookiesRemoveOptions.mockReturnValueOnce( mockCreateAuthFlowProofCookiesRemoveOptionsResult, ); - mockResolveCodeAndStateFromUrl.mockReturnValueOnce({ + mockParseSignInCallbackUrl.mockReturnValueOnce({ code: mockCode, state: 'not_important_for_this_test', + error: null, + errorDescription: null, }); mockGetCookieValuesFromNextApiRequest.mockReturnValueOnce({ [STATE_COOKIE_NAME]: 'not_important_for_this_test', diff --git a/packages/adapter-nextjs/__tests__/auth/handlers/signInCallbackErrorCombinations.ts b/packages/adapter-nextjs/__tests__/auth/handlers/signInCallbackErrorCombinations.ts new file mode 100644 index 00000000000..d8e545c38da --- /dev/null +++ b/packages/adapter-nextjs/__tests__/auth/handlers/signInCallbackErrorCombinations.ts @@ -0,0 +1,60 @@ +import { SIGN_IN_TIMEOUT_ERROR } from '../../../src/auth/constant'; + +export const ERROR_URL_PARAMS_COMBINATIONS = [ + { + code: null, + state: 'state', + error: null, + errorDescription: null, + expectedStatus: 400, + }, + { + code: 'code', + state: null, + error: null, + errorDescription: null, + expectedStatus: 400, + }, + { + code: null, + state: null, + error: null, + errorDescription: 'errorDescription', + expectedStatus: 302, + expectedRedirect: '/sign-in?error=errorDescription', + }, + { + code: null, + state: null, + error: 'error', + errorDescription: null, + expectedStatus: 302, + expectedRedirect: '/sign-in?error=error', + }, +]; + +export const ERROR_CLIENT_COOKIE_COMBINATIONS = [ + { + state: 'state_a', + pkce: 'pkce', + expectedStatus: 400, + }, + { + state: undefined, + pkce: undefined, + expectedStatus: 302, + expectedRedirect: `/sign-in?error=${SIGN_IN_TIMEOUT_ERROR}`, + }, + { + state: undefined, + pkce: 'pkce', + expectedStatus: 302, + expectedRedirect: `/sign-in?error=${SIGN_IN_TIMEOUT_ERROR}`, + }, + { + state: 'state', + pkce: undefined, + expectedStatus: 302, + expectedRedirect: `/sign-in?error=${SIGN_IN_TIMEOUT_ERROR}`, + }, +]; diff --git a/packages/adapter-nextjs/__tests__/auth/utils/parseSignInCallbackUrl.test.ts b/packages/adapter-nextjs/__tests__/auth/utils/parseSignInCallbackUrl.test.ts new file mode 100644 index 00000000000..157b9667f03 --- /dev/null +++ b/packages/adapter-nextjs/__tests__/auth/utils/parseSignInCallbackUrl.test.ts @@ -0,0 +1,16 @@ +import { parseSignInCallbackUrl } from '../../../src/auth/utils/parseSignInCallbackUrl'; + +describe('parseSignInCallbackUrl', () => { + it('returns the code and state from the url', () => { + const url = + 'https://example.com?code=123&state=456&error=789&error_description=abc'; + const result = parseSignInCallbackUrl(url); + + expect(result).toEqual({ + code: '123', + state: '456', + error: '789', + errorDescription: 'abc', + }); + }); +}); diff --git a/packages/adapter-nextjs/__tests__/auth/utils/resolveCodeAndStateFromUrl.test.ts b/packages/adapter-nextjs/__tests__/auth/utils/resolveCodeAndStateFromUrl.test.ts deleted file mode 100644 index 6b3194107ae..00000000000 --- a/packages/adapter-nextjs/__tests__/auth/utils/resolveCodeAndStateFromUrl.test.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { resolveCodeAndStateFromUrl } from '../../../src/auth/utils/resolveCodeAndStateFromUrl'; - -describe('resolveCodeAndStateFromUrl', () => { - it('returns the code and state from the url', () => { - const url = 'https://example.com?code=123&state=456'; - const result = resolveCodeAndStateFromUrl(url); - - expect(result).toEqual({ - code: '123', - state: '456', - }); - }); -}); diff --git a/packages/adapter-nextjs/src/auth/constant.ts b/packages/adapter-nextjs/src/auth/constant.ts index d269b06034b..500ee331e90 100644 --- a/packages/adapter-nextjs/src/auth/constant.ts +++ b/packages/adapter-nextjs/src/auth/constant.ts @@ -22,6 +22,16 @@ export const PKCE_COOKIE_NAME = 'com.amplify.server_auth.pkce'; export const STATE_COOKIE_NAME = 'com.amplify.server_auth.state'; export const IS_SIGNING_OUT_COOKIE_NAME = 'com.amplify.server_auth.isSigningOut'; -export const AUTH_FLOW_PROOF_MAX_AGE = 10 * 60; // 10 mins in seconds -export const REMOVE_COOKIE_MAX_AGE = -1; // -1 to remove the cookie immediately (0 ==> session cookie) + +// The 5 minutes is from the Cognito Social Identity Provider settings, see: +// https://docs.aws.amazon.com/cognito/latest/developerguide/cognito-user-pools-social-idp.html +export const AUTH_FLOW_PROOF_MAX_AGE = 5 * 60; + +// -1 to remove the cookie immediately (0 ==> session cookie as observed) +export const REMOVE_COOKIE_MAX_AGE = -1; + +// With server-side auth flow, we don't support the less secure implicit flow export const OAUTH_GRANT_TYPE = 'authorization_code'; + +export const SIGN_IN_TIMEOUT_ERROR = + 'Sign in has to be completed within 5 minutes.'; diff --git a/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts b/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts index 69533724ed6..4804506ad0c 100644 --- a/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts +++ b/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts @@ -1,7 +1,11 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { PKCE_COOKIE_NAME, STATE_COOKIE_NAME } from '../constant'; +import { + PKCE_COOKIE_NAME, + SIGN_IN_TIMEOUT_ERROR, + STATE_COOKIE_NAME, +} from '../constant'; import { appendSetCookieHeaders, createAuthFlowProofCookiesRemoveOptions, @@ -12,7 +16,7 @@ import { exchangeAuthNTokens, getCookieValuesFromRequest, getRedirectOrDefault, - resolveCodeAndStateFromUrl, + parseSignInCallbackUrl, resolveRedirectSignInUrl, } from '../utils'; @@ -26,14 +30,38 @@ export const handleSignInCallbackRequest: HandleSignInCallbackRequest = async ({ setCookieOptions, origin, }) => { - const { code, state } = resolveCodeAndStateFromUrl(request.url); + const { code, state, error, errorDescription } = parseSignInCallbackUrl( + request.url, + ); + + if (errorDescription || error) { + return new Response(null, { + status: 302, + headers: new Headers({ + location: `${getRedirectOrDefault(handlerInput.redirectOnSignOutComplete)}?error=${errorDescription || error}`, + }), + }); + } + if (!code || !state) { return new Response(null, { status: 400 }); } const { [PKCE_COOKIE_NAME]: clientPkce, [STATE_COOKIE_NAME]: clientState } = getCookieValuesFromRequest(request, [PKCE_COOKIE_NAME, STATE_COOKIE_NAME]); - if (!clientState || clientState !== state || !clientPkce) { + + // The state and pkce cookies are removed from cookie store after 5 minutes + if (!clientState || !clientPkce) { + return new Response(null, { + status: 302, + headers: new Headers({ + location: `${getRedirectOrDefault(handlerInput.redirectOnSignOutComplete)}?error=${SIGN_IN_TIMEOUT_ERROR}`, + }), + }); + } + + // Most likely the cookie has been tampered + if (clientState !== state) { return new Response(null, { status: 400 }); } diff --git a/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequestForPagesRouter.ts b/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequestForPagesRouter.ts index 843f931be21..376fa78ec23 100644 --- a/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequestForPagesRouter.ts +++ b/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequestForPagesRouter.ts @@ -1,7 +1,11 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { PKCE_COOKIE_NAME, STATE_COOKIE_NAME } from '../constant'; +import { + PKCE_COOKIE_NAME, + SIGN_IN_TIMEOUT_ERROR, + STATE_COOKIE_NAME, +} from '../constant'; import { appendSetCookieHeadersToNextApiResponse, createAuthFlowProofCookiesRemoveOptions, @@ -12,7 +16,7 @@ import { exchangeAuthNTokens, getCookieValuesFromNextApiRequest, getRedirectOrDefault, - resolveCodeAndStateFromUrl, + parseSignInCallbackUrl, resolveRedirectSignInUrl, } from '../utils'; @@ -28,7 +32,19 @@ export const handleSignInCallbackRequestForPagesRouter: HandleSignInCallbackRequ setCookieOptions, origin, }) => { - const { code, state } = resolveCodeAndStateFromUrl(request.url!); + const { code, state, error, errorDescription } = parseSignInCallbackUrl( + request.url!, + ); + + if (errorDescription || error) { + response.redirect( + 302, + `${getRedirectOrDefault(handlerInput.redirectOnSignOutComplete)}?error=${errorDescription || error}`, + ); + + return; + } + if (!code || !state) { response.status(400).end(); @@ -41,7 +57,18 @@ export const handleSignInCallbackRequestForPagesRouter: HandleSignInCallbackRequ STATE_COOKIE_NAME, ]); - if (!clientState || clientState !== state || !clientPkce) { + // The state and pkce cookies are removed from cookie store after 5 minutes + if (!clientState || !clientPkce) { + response.redirect( + 302, + `${getRedirectOrDefault(handlerInput.redirectOnSignOutComplete)}?error=${SIGN_IN_TIMEOUT_ERROR}`, + ); + + return; + } + + // Most likely the cookie has been tampered + if (clientState !== state) { response.status(400).end(); return; diff --git a/packages/adapter-nextjs/src/auth/utils/index.ts b/packages/adapter-nextjs/src/auth/utils/index.ts index 31a57a12f5b..72c0f19f074 100644 --- a/packages/adapter-nextjs/src/auth/utils/index.ts +++ b/packages/adapter-nextjs/src/auth/utils/index.ts @@ -36,7 +36,7 @@ export { } from './hasActiveUserSession'; export { isSupportedAuthApiRoutePath } from './isSupportedAuthApiRoutePath'; export { isValidOrigin, isSSLOrigin } from './origin'; -export { resolveCodeAndStateFromUrl } from './resolveCodeAndStateFromUrl'; +export { parseSignInCallbackUrl } from './parseSignInCallbackUrl'; export { resolveIdentityProviderFromUrl } from './resolveIdentityProviderFromUrl'; export { resolveRedirectSignInUrl, diff --git a/packages/adapter-nextjs/src/auth/utils/resolveCodeAndStateFromUrl.ts b/packages/adapter-nextjs/src/auth/utils/parseSignInCallbackUrl.ts similarity index 62% rename from packages/adapter-nextjs/src/auth/utils/resolveCodeAndStateFromUrl.ts rename to packages/adapter-nextjs/src/auth/utils/parseSignInCallbackUrl.ts index 3f6f7f20916..f3918f22157 100644 --- a/packages/adapter-nextjs/src/auth/utils/resolveCodeAndStateFromUrl.ts +++ b/packages/adapter-nextjs/src/auth/utils/parseSignInCallbackUrl.ts @@ -3,12 +3,16 @@ import { getSearchParamValueFromUrl } from './getSearchParamValueFromUrl'; -export const resolveCodeAndStateFromUrl = ( +export const parseSignInCallbackUrl = ( urlStr: string, ): { code: string | null; state: string | null; + error: string | null; + errorDescription: string | null; } => ({ state: getSearchParamValueFromUrl(urlStr, 'state'), code: getSearchParamValueFromUrl(urlStr, 'code'), + error: getSearchParamValueFromUrl(urlStr, 'error'), + errorDescription: getSearchParamValueFromUrl(urlStr, 'error_description'), }); From 3f8307f88f9c57108428b707ccbba5fcfd37074d Mon Sep 17 00:00:00 2001 From: Hui Zhao Date: Fri, 10 Jan 2025 10:48:24 -0800 Subject: [PATCH 2/3] feat(adapter-nextjs): expose both error and errorDescription --- .../handleSignInCallbackRequest.test.ts | 25 ++++++++++++++ ...ignInCallbackRequestForPagesRouter.test.ts | 24 ++++++++++++++ .../auth/handlers/mockImplementation.ts | 20 +++++++++++ .../signInCallbackErrorCombinations.ts | 20 +++++++---- .../createErrorSearchParamsString.test.ts | 33 +++++++++++++++++++ packages/adapter-nextjs/src/auth/constant.ts | 3 +- .../handlers/handleSignInCallbackRequest.ts | 18 ++++++++-- ...ndleSignInCallbackRequestForPagesRouter.ts | 16 +++++++-- .../utils/createErrorSearchParamsString.ts | 26 +++++++++++++++ .../adapter-nextjs/src/auth/utils/index.ts | 1 + 10 files changed, 172 insertions(+), 14 deletions(-) create mode 100644 packages/adapter-nextjs/__tests__/auth/handlers/mockImplementation.ts create mode 100644 packages/adapter-nextjs/__tests__/auth/utils/createErrorSearchParamsString.test.ts create mode 100644 packages/adapter-nextjs/src/auth/utils/createErrorSearchParamsString.ts diff --git a/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequest.test.ts b/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequest.test.ts index 3f54a94a33d..5c5f4f8d172 100644 --- a/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequest.test.ts +++ b/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequest.test.ts @@ -9,6 +9,7 @@ import { handleSignInCallbackRequest } from '../../../src/auth/handlers/handleSi import { appendSetCookieHeaders, createAuthFlowProofCookiesRemoveOptions, + createErrorSearchParamsString, createOnSignInCompleteRedirectIntermediate, createSignInFlowProofCookies, createTokenCookies, @@ -22,6 +23,8 @@ import { import { CreateAuthRoutesHandlersInput } from '../../../src/auth/types'; import { PKCE_COOKIE_NAME, + SIGN_IN_TIMEOUT_ERROR_CODE, + SIGN_IN_TIMEOUT_ERROR_MESSAGE, STATE_COOKIE_NAME, } from '../../../src/auth/constant'; @@ -29,6 +32,7 @@ import { ERROR_CLIENT_COOKIE_COMBINATIONS, ERROR_URL_PARAMS_COMBINATIONS, } from './signInCallbackErrorCombinations'; +import { mockCreateErrorSearchParamsStringImplementation } from './mockImplementation'; jest.mock('../../../src/auth/utils'); @@ -51,6 +55,9 @@ const mockGetCookieValuesFromRequest = jest.mocked(getCookieValuesFromRequest); const mockParseSignInCallbackUrl = jest.mocked(parseSignInCallbackUrl); const mockResolveRedirectSignInUrl = jest.mocked(resolveRedirectSignInUrl); const mockGetRedirectOrDefault = jest.mocked(getRedirectOrDefault); +const mockCreateErrorSearchParamsString = jest.mocked( + createErrorSearchParamsString, +); describe('handleSignInCallbackRequest', () => { const mockHandlerInput: CreateAuthRoutesHandlersInput = { @@ -66,6 +73,9 @@ describe('handleSignInCallbackRequest', () => { mockGetRedirectOrDefault.mockImplementation( (redirect: string | undefined) => redirect || '/', ); + mockCreateErrorSearchParamsString.mockImplementation( + mockCreateErrorSearchParamsStringImplementation, + ); }); afterEach(() => { @@ -79,6 +89,7 @@ describe('handleSignInCallbackRequest', () => { mockGetCookieValuesFromRequest.mockClear(); mockParseSignInCallbackUrl.mockClear(); mockResolveRedirectSignInUrl.mockClear(); + mockCreateErrorSearchParamsString.mockClear(); }); test.each(ERROR_URL_PARAMS_COMBINATIONS)( @@ -115,6 +126,13 @@ describe('handleSignInCallbackRequest', () => { if (expectedStatus === 302) { expect(response.headers.get('Location')).toBe(expectedRedirect); } + + if (error || errorDescription) { + expect(mockCreateErrorSearchParamsString).toHaveBeenCalledWith({ + error, + errorDescription, + }); + } }, ); @@ -157,6 +175,13 @@ describe('handleSignInCallbackRequest', () => { ); expect(response.headers.get('Location')).toBe(expectedRedirect); } + + if (!state || !pkce) { + expect(mockCreateErrorSearchParamsString).toHaveBeenCalledWith({ + error: SIGN_IN_TIMEOUT_ERROR_CODE, + errorDescription: SIGN_IN_TIMEOUT_ERROR_MESSAGE, + }); + } }, ); diff --git a/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequestForPagesRouter.test.ts b/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequestForPagesRouter.test.ts index c59c580c474..38ae6ff3b7d 100644 --- a/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequestForPagesRouter.test.ts +++ b/packages/adapter-nextjs/__tests__/auth/handlers/handleSignInCallbackRequestForPagesRouter.test.ts @@ -9,6 +9,7 @@ import { handleSignInCallbackRequestForPagesRouter } from '../../../src/auth/han import { appendSetCookieHeadersToNextApiResponse, createAuthFlowProofCookiesRemoveOptions, + createErrorSearchParamsString, createOnSignInCompleteRedirectIntermediate, createSignInFlowProofCookies, createTokenCookies, @@ -22,6 +23,8 @@ import { import { CreateAuthRoutesHandlersInput } from '../../../src/auth/types'; import { PKCE_COOKIE_NAME, + SIGN_IN_TIMEOUT_ERROR_CODE, + SIGN_IN_TIMEOUT_ERROR_MESSAGE, STATE_COOKIE_NAME, } from '../../../src/auth/constant'; import { createMockNextApiResponse } from '../testUtils'; @@ -30,6 +33,7 @@ import { ERROR_CLIENT_COOKIE_COMBINATIONS, ERROR_URL_PARAMS_COMBINATIONS, } from './signInCallbackErrorCombinations'; +import { mockCreateErrorSearchParamsStringImplementation } from './mockImplementation'; jest.mock('../../../src/auth/utils'); @@ -56,6 +60,9 @@ const mockGetCookieValuesFromNextApiRequest = jest.mocked( const mockParseSignInCallbackUrl = jest.mocked(parseSignInCallbackUrl); const mockResolveRedirectSignInUrl = jest.mocked(resolveRedirectSignInUrl); const mockGetRedirectOrDefault = jest.mocked(getRedirectOrDefault); +const mockCreateErrorSearchParamsString = jest.mocked( + createErrorSearchParamsString, +); describe('handleSignInCallbackRequest', () => { const mockHandlerInput: CreateAuthRoutesHandlersInput = { @@ -79,6 +86,9 @@ describe('handleSignInCallbackRequest', () => { mockGetRedirectOrDefault.mockImplementation( (redirect: string | undefined) => redirect || '/', ); + mockCreateErrorSearchParamsString.mockImplementation( + mockCreateErrorSearchParamsStringImplementation, + ); }); afterEach(() => { @@ -146,6 +156,13 @@ describe('handleSignInCallbackRequest', () => { ); } expect(mockParseSignInCallbackUrl).toHaveBeenCalledWith(url); + + if (error || errorDescription) { + expect(mockCreateErrorSearchParamsString).toHaveBeenCalledWith({ + error, + errorDescription, + }); + } }, ); @@ -197,6 +214,13 @@ describe('handleSignInCallbackRequest', () => { mockRequest, [PKCE_COOKIE_NAME, STATE_COOKIE_NAME], ); + + if (!state || !pkce) { + expect(mockCreateErrorSearchParamsString).toHaveBeenCalledWith({ + error: SIGN_IN_TIMEOUT_ERROR_CODE, + errorDescription: SIGN_IN_TIMEOUT_ERROR_MESSAGE, + }); + } }, ); diff --git a/packages/adapter-nextjs/__tests__/auth/handlers/mockImplementation.ts b/packages/adapter-nextjs/__tests__/auth/handlers/mockImplementation.ts new file mode 100644 index 00000000000..2ecf546c120 --- /dev/null +++ b/packages/adapter-nextjs/__tests__/auth/handlers/mockImplementation.ts @@ -0,0 +1,20 @@ +import { createErrorSearchParamsString } from '../../../src/auth/utils'; + +export const mockCreateErrorSearchParamsStringImplementation: typeof createErrorSearchParamsString< + string | null, + string | null +> = ({ error, errorDescription }) => { + if (error && errorDescription) { + return 'hasErrorAndErrorDescription'; + } + + if (error) { + return 'hasError'; + } + + if (errorDescription) { + return 'hasErrorDescription'; + } + + return undefined; +}; diff --git a/packages/adapter-nextjs/__tests__/auth/handlers/signInCallbackErrorCombinations.ts b/packages/adapter-nextjs/__tests__/auth/handlers/signInCallbackErrorCombinations.ts index d8e545c38da..af018e18ecb 100644 --- a/packages/adapter-nextjs/__tests__/auth/handlers/signInCallbackErrorCombinations.ts +++ b/packages/adapter-nextjs/__tests__/auth/handlers/signInCallbackErrorCombinations.ts @@ -1,5 +1,3 @@ -import { SIGN_IN_TIMEOUT_ERROR } from '../../../src/auth/constant'; - export const ERROR_URL_PARAMS_COMBINATIONS = [ { code: null, @@ -21,7 +19,7 @@ export const ERROR_URL_PARAMS_COMBINATIONS = [ error: null, errorDescription: 'errorDescription', expectedStatus: 302, - expectedRedirect: '/sign-in?error=errorDescription', + expectedRedirect: '/sign-in?hasErrorDescription', }, { code: null, @@ -29,7 +27,15 @@ export const ERROR_URL_PARAMS_COMBINATIONS = [ error: 'error', errorDescription: null, expectedStatus: 302, - expectedRedirect: '/sign-in?error=error', + expectedRedirect: '/sign-in?hasError', + }, + { + code: null, + state: null, + error: 'error', + errorDescription: 'errorDescription', + expectedStatus: 302, + expectedRedirect: '/sign-in?hasErrorAndErrorDescription', }, ]; @@ -43,18 +49,18 @@ export const ERROR_CLIENT_COOKIE_COMBINATIONS = [ state: undefined, pkce: undefined, expectedStatus: 302, - expectedRedirect: `/sign-in?error=${SIGN_IN_TIMEOUT_ERROR}`, + expectedRedirect: '/sign-in?hasErrorAndErrorDescription', }, { state: undefined, pkce: 'pkce', expectedStatus: 302, - expectedRedirect: `/sign-in?error=${SIGN_IN_TIMEOUT_ERROR}`, + expectedRedirect: '/sign-in?hasErrorAndErrorDescription', }, { state: 'state', pkce: undefined, expectedStatus: 302, - expectedRedirect: `/sign-in?error=${SIGN_IN_TIMEOUT_ERROR}`, + expectedRedirect: '/sign-in?hasErrorAndErrorDescription', }, ]; diff --git a/packages/adapter-nextjs/__tests__/auth/utils/createErrorSearchParamsString.test.ts b/packages/adapter-nextjs/__tests__/auth/utils/createErrorSearchParamsString.test.ts new file mode 100644 index 00000000000..dd7b390f9a3 --- /dev/null +++ b/packages/adapter-nextjs/__tests__/auth/utils/createErrorSearchParamsString.test.ts @@ -0,0 +1,33 @@ +import { createErrorSearchParamsString } from '../../../src/auth/utils/createErrorSearchParamsString'; + +describe('createErrorSearchParamsString', () => { + test.each([ + { + error: null, + errorDescription: null, + expected: undefined, + }, + { + error: 'error', + errorDescription: null, + expected: 'error=error', + }, + { + error: null, + errorDescription: 'errorDescription', + expected: 'error_description=errorDescription', + }, + { + error: 'error', + errorDescription: 'errorDescription', + expected: 'error=error&error_description=errorDescription', + }, + ])( + `returns $expected when called with error: $error and errorDescription: $errorDescription`, + ({ error, errorDescription, expected }) => { + const result = createErrorSearchParamsString({ error, errorDescription }); + + expect(result).toEqual(expected); + }, + ); +}); diff --git a/packages/adapter-nextjs/src/auth/constant.ts b/packages/adapter-nextjs/src/auth/constant.ts index 500ee331e90..9fadab6c81f 100644 --- a/packages/adapter-nextjs/src/auth/constant.ts +++ b/packages/adapter-nextjs/src/auth/constant.ts @@ -33,5 +33,6 @@ export const REMOVE_COOKIE_MAX_AGE = -1; // With server-side auth flow, we don't support the less secure implicit flow export const OAUTH_GRANT_TYPE = 'authorization_code'; -export const SIGN_IN_TIMEOUT_ERROR = +export const SIGN_IN_TIMEOUT_ERROR_CODE = 'timeout'; +export const SIGN_IN_TIMEOUT_ERROR_MESSAGE = 'Sign in has to be completed within 5 minutes.'; diff --git a/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts b/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts index 4804506ad0c..b35ee1d4d15 100644 --- a/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts +++ b/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts @@ -3,12 +3,14 @@ import { PKCE_COOKIE_NAME, - SIGN_IN_TIMEOUT_ERROR, + SIGN_IN_TIMEOUT_ERROR_CODE, + SIGN_IN_TIMEOUT_ERROR_MESSAGE, STATE_COOKIE_NAME, } from '../constant'; import { appendSetCookieHeaders, createAuthFlowProofCookiesRemoveOptions, + createErrorSearchParamsString, createOnSignInCompleteRedirectIntermediate, createSignInFlowProofCookies, createTokenCookies, @@ -35,10 +37,15 @@ export const handleSignInCallbackRequest: HandleSignInCallbackRequest = async ({ ); if (errorDescription || error) { + const searchParamsString = createErrorSearchParamsString({ + error, + errorDescription, + })!; // safe unwrap as errorDescription or error is not null + return new Response(null, { status: 302, headers: new Headers({ - location: `${getRedirectOrDefault(handlerInput.redirectOnSignOutComplete)}?error=${errorDescription || error}`, + location: `${getRedirectOrDefault(handlerInput.redirectOnSignOutComplete)}?${searchParamsString}`, }), }); } @@ -52,10 +59,15 @@ export const handleSignInCallbackRequest: HandleSignInCallbackRequest = async ({ // The state and pkce cookies are removed from cookie store after 5 minutes if (!clientState || !clientPkce) { + const searchParamsString = createErrorSearchParamsString({ + error: SIGN_IN_TIMEOUT_ERROR_CODE, + errorDescription: SIGN_IN_TIMEOUT_ERROR_MESSAGE, + }); + return new Response(null, { status: 302, headers: new Headers({ - location: `${getRedirectOrDefault(handlerInput.redirectOnSignOutComplete)}?error=${SIGN_IN_TIMEOUT_ERROR}`, + location: `${getRedirectOrDefault(handlerInput.redirectOnSignOutComplete)}?${searchParamsString}`, }), }); } diff --git a/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequestForPagesRouter.ts b/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequestForPagesRouter.ts index 376fa78ec23..ab1c1897490 100644 --- a/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequestForPagesRouter.ts +++ b/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequestForPagesRouter.ts @@ -3,12 +3,14 @@ import { PKCE_COOKIE_NAME, - SIGN_IN_TIMEOUT_ERROR, + SIGN_IN_TIMEOUT_ERROR_CODE, + SIGN_IN_TIMEOUT_ERROR_MESSAGE, STATE_COOKIE_NAME, } from '../constant'; import { appendSetCookieHeadersToNextApiResponse, createAuthFlowProofCookiesRemoveOptions, + createErrorSearchParamsString, createOnSignInCompleteRedirectIntermediate, createSignInFlowProofCookies, createTokenCookies, @@ -37,9 +39,13 @@ export const handleSignInCallbackRequestForPagesRouter: HandleSignInCallbackRequ ); if (errorDescription || error) { + const searchParamsString = createErrorSearchParamsString({ + error, + errorDescription, + })!; // safe unwrap as errorDescription or error is not null response.redirect( 302, - `${getRedirectOrDefault(handlerInput.redirectOnSignOutComplete)}?error=${errorDescription || error}`, + `${getRedirectOrDefault(handlerInput.redirectOnSignOutComplete)}?${searchParamsString}`, ); return; @@ -59,9 +65,13 @@ export const handleSignInCallbackRequestForPagesRouter: HandleSignInCallbackRequ // The state and pkce cookies are removed from cookie store after 5 minutes if (!clientState || !clientPkce) { + const searchParamsString = createErrorSearchParamsString({ + error: SIGN_IN_TIMEOUT_ERROR_CODE, + errorDescription: SIGN_IN_TIMEOUT_ERROR_MESSAGE, + }); response.redirect( 302, - `${getRedirectOrDefault(handlerInput.redirectOnSignOutComplete)}?error=${SIGN_IN_TIMEOUT_ERROR}`, + `${getRedirectOrDefault(handlerInput.redirectOnSignOutComplete)}?${searchParamsString}`, ); return; diff --git a/packages/adapter-nextjs/src/auth/utils/createErrorSearchParamsString.ts b/packages/adapter-nextjs/src/auth/utils/createErrorSearchParamsString.ts new file mode 100644 index 00000000000..f3da9ada6ab --- /dev/null +++ b/packages/adapter-nextjs/src/auth/utils/createErrorSearchParamsString.ts @@ -0,0 +1,26 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +export const createErrorSearchParamsString = < + T1 extends string | null, + T2 extends string | null, + R = T1 extends string ? string : T2 extends string ? string : undefined, +>({ + error, + errorDescription, +}: { + error: T1; + errorDescription: T2; +}): R => { + const errorParams = new URLSearchParams(); + + if (error) { + errorParams.set('error', error); + } + + if (errorDescription) { + errorParams.set('error_description', errorDescription); + } + + return (errorParams.toString() || undefined) as R; +}; diff --git a/packages/adapter-nextjs/src/auth/utils/index.ts b/packages/adapter-nextjs/src/auth/utils/index.ts index 72c0f19f074..572a18d5184 100644 --- a/packages/adapter-nextjs/src/auth/utils/index.ts +++ b/packages/adapter-nextjs/src/auth/utils/index.ts @@ -11,6 +11,7 @@ export { createAuthFlowProofCookiesRemoveOptions, } from './authFlowProofCookies'; export { createAuthFlowProofs } from './createAuthFlowProofs'; +export { createErrorSearchParamsString } from './createErrorSearchParamsString'; export { createOnSignInCompleteRedirectIntermediate } from './createOnSignInCompleteRedirectIntermediate'; export { createUrlSearchParamsForSignInSignUp } from './createUrlSearchParams'; export { From c88a487f8c0d53340f85da007777ae50dcfc20e8 Mon Sep 17 00:00:00 2001 From: Hui Zhao Date: Fri, 10 Jan 2025 13:52:38 -0800 Subject: [PATCH 3/3] chore(adapter-nextjs): remove unnecessary undefined fallback --- .../auth/handlers/mockImplementation.ts | 28 +++++++++---------- .../createErrorSearchParamsString.test.ts | 2 +- .../handlers/handleSignInCallbackRequest.ts | 2 +- ...ndleSignInCallbackRequestForPagesRouter.ts | 2 +- .../utils/createErrorSearchParamsString.ts | 14 ++++------ 5 files changed, 21 insertions(+), 27 deletions(-) diff --git a/packages/adapter-nextjs/__tests__/auth/handlers/mockImplementation.ts b/packages/adapter-nextjs/__tests__/auth/handlers/mockImplementation.ts index 2ecf546c120..dbe6960dcbb 100644 --- a/packages/adapter-nextjs/__tests__/auth/handlers/mockImplementation.ts +++ b/packages/adapter-nextjs/__tests__/auth/handlers/mockImplementation.ts @@ -1,20 +1,18 @@ import { createErrorSearchParamsString } from '../../../src/auth/utils'; -export const mockCreateErrorSearchParamsStringImplementation: typeof createErrorSearchParamsString< - string | null, - string | null -> = ({ error, errorDescription }) => { - if (error && errorDescription) { - return 'hasErrorAndErrorDescription'; - } +export const mockCreateErrorSearchParamsStringImplementation: typeof createErrorSearchParamsString = + ({ error, errorDescription }) => { + if (error && errorDescription) { + return 'hasErrorAndErrorDescription'; + } - if (error) { - return 'hasError'; - } + if (error) { + return 'hasError'; + } - if (errorDescription) { - return 'hasErrorDescription'; - } + if (errorDescription) { + return 'hasErrorDescription'; + } - return undefined; -}; + return ''; + }; diff --git a/packages/adapter-nextjs/__tests__/auth/utils/createErrorSearchParamsString.test.ts b/packages/adapter-nextjs/__tests__/auth/utils/createErrorSearchParamsString.test.ts index dd7b390f9a3..a806bf4a9be 100644 --- a/packages/adapter-nextjs/__tests__/auth/utils/createErrorSearchParamsString.test.ts +++ b/packages/adapter-nextjs/__tests__/auth/utils/createErrorSearchParamsString.test.ts @@ -5,7 +5,7 @@ describe('createErrorSearchParamsString', () => { { error: null, errorDescription: null, - expected: undefined, + expected: '', }, { error: 'error', diff --git a/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts b/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts index b35ee1d4d15..91c1ab27af0 100644 --- a/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts +++ b/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts @@ -40,7 +40,7 @@ export const handleSignInCallbackRequest: HandleSignInCallbackRequest = async ({ const searchParamsString = createErrorSearchParamsString({ error, errorDescription, - })!; // safe unwrap as errorDescription or error is not null + }); return new Response(null, { status: 302, diff --git a/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequestForPagesRouter.ts b/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequestForPagesRouter.ts index ab1c1897490..15685032500 100644 --- a/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequestForPagesRouter.ts +++ b/packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequestForPagesRouter.ts @@ -42,7 +42,7 @@ export const handleSignInCallbackRequestForPagesRouter: HandleSignInCallbackRequ const searchParamsString = createErrorSearchParamsString({ error, errorDescription, - })!; // safe unwrap as errorDescription or error is not null + }); response.redirect( 302, `${getRedirectOrDefault(handlerInput.redirectOnSignOutComplete)}?${searchParamsString}`, diff --git a/packages/adapter-nextjs/src/auth/utils/createErrorSearchParamsString.ts b/packages/adapter-nextjs/src/auth/utils/createErrorSearchParamsString.ts index f3da9ada6ab..746acc2adc4 100644 --- a/packages/adapter-nextjs/src/auth/utils/createErrorSearchParamsString.ts +++ b/packages/adapter-nextjs/src/auth/utils/createErrorSearchParamsString.ts @@ -1,17 +1,13 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -export const createErrorSearchParamsString = < - T1 extends string | null, - T2 extends string | null, - R = T1 extends string ? string : T2 extends string ? string : undefined, ->({ +export const createErrorSearchParamsString = ({ error, errorDescription, }: { - error: T1; - errorDescription: T2; -}): R => { + error: string | null; + errorDescription: string | null; +}): string => { const errorParams = new URLSearchParams(); if (error) { @@ -22,5 +18,5 @@ export const createErrorSearchParamsString = < errorParams.set('error_description', errorDescription); } - return (errorParams.toString() || undefined) as R; + return errorParams.toString(); };