feat(adapter-nextjs): server-side auth flows integrating cognito hosted UI #13827

HuiSF · 2024-09-19T01:36:43Z

Description of changes

Implemented the following flow integrating Cognito Hosted UI endpoints:

sign-in -> Hosted UI -> sign-in-callback (including Social Providers) -> httpOnly cookies
sign-up -> Hosted UI -> sign-in-callback -> httpOnly cookies
sign-out -> Hosted UI -> sign-out-callback -> httpOnly cookies

API Route handlers work with both Next.js App Router and Pages Router.

Issue #, if available

Description of how you validated changes

Unit tests
Manual testing with Next.js samples apps created with the App Router and Pages Router

Checklist

PR description included
yarn test passes
Unit Tests are changed or added
Relevant documentation is changed or added (and PR referenced)

Checklist for repo maintainers

Verify E2E tests for existing workflows are working as expected or add E2E tests for newly added workflows
New source file paths included in this PR have been added to CODEOWNERS, if appropriate

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

HuiSF · 2024-09-19T01:38:33Z

packages/adapter-nextjs/tsconfig.json

-		"alwaysStrict": true
+		"alwaysStrict": true,
+		"lib": [
+			"esnext"


The base tsconfig has jsdom as a lib, that makes origin variable a legit global variable which is not desired.

HuiSF · 2024-09-19T01:39:46Z

packages/adapter-nextjs/src/utils/cookie/serializeCookie.ts

Existing impl. moved out.

HuiSF · 2024-09-19T01:39:56Z

packages/adapter-nextjs/src/utils/cookie/ensureEncodedForJSCookie.ts

Existing impl. moved out.

HuiSF · 2024-09-19T17:35:03Z

packages/adapter-nextjs/src/auth/utils/tokenCookies.ts

+	path: '/',
+	httpOnly: true,
+	secure: true,


These three attributes are not configurable.

jjarvisp

Almost done with review, I should be able to wrap up soon.

packages/adapter-nextjs/src/auth/utils/getSearchParamValueFromUrl.ts

packages/adapter-nextjs/src/auth/utils/getCookieValuesFromRequest.ts

packages/adapter-nextjs/src/auth/utils/resolveRedirectUrl.ts

packages/adapter-nextjs/src/auth/utils/resolveIdentityProviderFromUrl.ts

packages/adapter-nextjs/src/auth/utils/getCookieValuesFromNextApiRequest.ts

packages/adapter-nextjs/src/auth/handlers/handleSignOutCallbackRequestForPagesRouter.ts

jjarvisp · 2024-11-29T19:52:16Z

packages/adapter-nextjs/src/auth/utils/getAccessTokenUsernameAndClockDrift.ts

+	const decoded = decodeJWT(accessToken);
+	const issuedAt = (decoded.payload.iat ?? 0) * 1000;
+	const clockDrift = issuedAt > 0 ? issuedAt - Date.now() : 0;
+	const username = (decoded.payload.username as string) ?? 'username';


Are there valid scenarios when username would be missing from payload or any particular reasoning around default username to "username"?

Not really. I just followed the original client-side impl.
This is a good question, let me dig and get back to you.

The username in the token payload corresponds the username in a Cognito user pool, and the username of a user in a Cognito user pool is always defined. From this I think we can safely remove the redundant default value fallback.

Agreed, thanks for looking into this.

packages/adapter-nextjs/src/auth/utils/createUrlSearchParams.ts

packages/adapter-nextjs/src/auth/handlers/handleSignOutRequestForPagesRouter.ts

packages/adapter-nextjs/src/auth/handlers/handleSignInSignUpRequestForPagesRouter.ts

packages/adapter-nextjs/__tests__/auth/utils/tokenCookies.test.ts

packages/adapter-nextjs/__tests__/auth/utils/getAccessTokenUsernameAndClockDrift.test.ts

packages/adapter-nextjs/src/auth/utils/createOnSignInCompletedRedirectIntermediate.ts

packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequestForPagesRouter.ts

…ide auth

… auth

…hosted ui

packages/adapter-nextjs/src/auth/handlers/handleSignOutCallbackRequest.ts

AllanZhengYP · 2024-12-23T18:20:15Z

packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts

+}) => {
+	const { code, state } = resolveCodeAndStateFromUrl(request.url);
+	if (!code || !state) {
+		return new Response(null, { status: 400 });


Do we see needs for customers to customize the error payloads and render different messages accordingly?

Good question - This particular spot is for unwrapping code and state to defined strings, and here it's redirecting back from Hosted UI endpoint, they should always be provided.

Though in the current implementation we are returning 400 response when state and pkce cookies are not presented (expired in 10 minutes without the end user completing the sign in) I will keep the implementation as is and looking into how to surface an error for this situation.

AllanZhengYP · 2024-12-23T18:33:00Z

packages/adapter-nextjs/src/auth/handlers/handleSignInCallbackRequest.ts

+	);
+	appendSetCookieHeaders(
+		headers,
+		createSignInFlowProofCookies({ state: '', pkce: '' }),


Super nit: prefer to re-name this locally to callout the cookie removal, similar to the next line. Something like

createSignInFlowProofRemoveCookies(),

Which is also a similar practice to the createTokenRemoveCookies below

This function sets only cookie name-value - which is not add or remove specific. The only difference is on the options.

packages/adapter-nextjs/src/auth/handlers/handleSignOutCallbackRequestForPagesRouter.ts

packages/adapter-nextjs/src/auth/handlers/handleSignInSignUpRequest.ts

packages/adapter-nextjs/src/auth/utils/authFlowProofCookies.ts

AllanZhengYP · 2024-12-23T20:01:16Z

packages/adapter-nextjs/src/auth/utils/resolveRedirectUrl.ts

+	oAuthConfig: OAuthConfig,
+) => {
+	const redirectUrl = oAuthConfig.redirectSignIn.find(url =>
+		url.startsWith(origin),


Should we use the URL util and parse the origin from the oAuthConfig more strictly?

I don't think it's necessary to construct a URL object from the config redirect URL strings - as a client-side validation this should be good enough, as eventually HostedUI will also match the URL on the service side.

Is startsWith sufficient to handle different domains but with the same subdomains, e.g. sub.abc.io v.s. sub.abc.io.com?

Well the origin remain unchanged (configured once by env var) as the predicate to look up a corresponding url from redirectURLs, and we are match origin starting from the protocol rather than a domain, I still think it's sufficient.

…ed UI (#13827) * chore(auth): export necessary utilities and types to support server-side auth * chore(aws-amplify): export necessary utilities to support server-side auth * feat(adapter-nextjs): server-side auth api route integrating cognito hosted ui * chore(adapter-nextjs): resolve comments * refactor(adapter-nextjs): remove redundant username fallback

HuiSF requested review from ukhan-amazon, haverchuck, cshfang, jimblanc and a team as code owners September 19, 2024 01:36

HuiSF commented Sep 19, 2024

View reviewed changes

packages/adapter-nextjs/src/utils/cookie/serializeCookie.ts Outdated

Copy link

Member Author

HuiSF Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Existing impl. moved out.

HuiSF commented Sep 19, 2024

View reviewed changes

packages/adapter-nextjs/src/utils/cookie/ensureEncodedForJSCookie.ts Outdated

Copy link

Member Author

HuiSF Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Existing impl. moved out.

ashika112 changed the title ~~eat(adapter-nextjs): server-side auth flows integrating cognio hosted UI~~ feat(adapter-nextjs): server-side auth flows integrating cognio hosted UI Sep 19, 2024

HuiSF force-pushed the hui/feat/adapter-nextjs/3-auth-api-handlers-impl branch from 6087817 to ae36b32 Compare September 19, 2024 17:23

HuiSF commented Sep 19, 2024

View reviewed changes

HuiSF force-pushed the hui/feat/adapter-nextjs/3-auth-api-handlers-impl branch from ae36b32 to 621d0cb Compare September 19, 2024 18:26

HuiSF force-pushed the hui/feat/adapter-nextjs/2-auth-api-handlers branch from 25c4eaa to 26ec9ed Compare October 1, 2024 23:02

HuiSF force-pushed the hui/feat/adapter-nextjs/3-auth-api-handlers-impl branch from 621d0cb to 284acbf Compare October 1, 2024 23:02

HuiSF force-pushed the hui/feat/adapter-nextjs/2-auth-api-handlers branch from 26ec9ed to 67d073f Compare November 21, 2024 17:05

HuiSF requested a review from pranavosu as a code owner November 21, 2024 17:05

HuiSF force-pushed the hui/feat/adapter-nextjs/3-auth-api-handlers-impl branch from 284acbf to 0ef2b81 Compare November 21, 2024 17:06

jjarvisp changed the title ~~feat(adapter-nextjs): server-side auth flows integrating cognio hosted UI~~ feat(adapter-nextjs): server-side auth flows integrating cognito hosted UI Nov 22, 2024

jjarvisp reviewed Dec 2, 2024

View reviewed changes

packages/adapter-nextjs/__tests__/auth/utils/tokenCookies.test.ts Outdated Show resolved Hide resolved

jjarvisp reviewed Dec 3, 2024

View reviewed changes

Base automatically changed from hui/feat/adapter-nextjs/2-auth-api-handlers to feat/server-auth/main December 20, 2024 18:56

HuiSF requested a review from sktimalsina as a code owner December 20, 2024 18:56

HuiSF added 3 commits December 20, 2024 16:21

chore(auth): export necessary utilities and types to support server-s…

acd2e41

…ide auth

chore(aws-amplify): export necessary utilities to support server-side…

fc1f867

… auth

feat(adapter-nextjs): server-side auth api route integrating cognito …

ff48e7a

…hosted ui

HuiSF force-pushed the hui/feat/adapter-nextjs/3-auth-api-handlers-impl branch from 0ef2b81 to ff48e7a Compare December 21, 2024 00:30

chore(adapter-nextjs): resolve comments

56dd741

jjarvisp approved these changes Dec 23, 2024

View reviewed changes

packages/adapter-nextjs/src/auth/handlers/handleSignOutCallbackRequest.ts Show resolved Hide resolved

AllanZhengYP reviewed Dec 23, 2024

View reviewed changes

AllanZhengYP approved these changes Jan 2, 2025

View reviewed changes

HuiSF force-pushed the hui/feat/adapter-nextjs/3-auth-api-handlers-impl branch from a585318 to 56dd741 Compare January 2, 2025 21:33

refactor(adapter-nextjs): remove redundant username fallback

97957f7

HuiSF merged commit 1162419 into feat/server-auth/main Jan 2, 2025
28 checks passed

HuiSF deleted the hui/feat/adapter-nextjs/3-auth-api-handlers-impl branch January 2, 2025 21:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(adapter-nextjs): server-side auth flows integrating cognito hosted UI #13827

feat(adapter-nextjs): server-side auth flows integrating cognito hosted UI #13827

HuiSF commented Sep 19, 2024 •

edited

Loading

HuiSF Sep 19, 2024

HuiSF Sep 19, 2024

HuiSF Sep 19, 2024

HuiSF Sep 19, 2024

jjarvisp left a comment

jjarvisp Nov 29, 2024

HuiSF Dec 21, 2024

HuiSF Dec 31, 2024

jjarvisp Dec 31, 2024

AllanZhengYP Dec 23, 2024

HuiSF Dec 31, 2024

AllanZhengYP Dec 23, 2024

HuiSF Dec 26, 2024

AllanZhengYP Dec 23, 2024

HuiSF Dec 26, 2024

AllanZhengYP Dec 31, 2024

HuiSF Dec 31, 2024

feat(adapter-nextjs): server-side auth flows integrating cognito hosted UI #13827

feat(adapter-nextjs): server-side auth flows integrating cognito hosted UI #13827

Conversation

HuiSF commented Sep 19, 2024 • edited Loading

Description of changes

Issue #, if available

Description of how you validated changes

Checklist

Checklist for repo maintainers

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jjarvisp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HuiSF commented Sep 19, 2024 •

edited

Loading