nextauthjs / next-auth

Authentication for the Web.
https://authjs.dev
ISC License
24.78k stars 3.49k forks source link

Allow customizing state #5727

Open jasonkuhrt opened 2 years ago

jasonkuhrt commented 2 years ago

Description 📓

Allow customizing the state query string parameter of authorization URL sent by OAuth providers like GitHub.

E.g.:

CleanShot 2022-11-04 at 08 23 23@2x

Motivation:

The reason we control it is that GH App only allows specifying ONE callback URL. This is a big problem for dynamic preview deployments, like per PR ones.

The solution we managed and have been using for 1.5 years now is to, in preview, have the gh app redirect to PDP CP router microservice which will in turn look at the state (base 64 encoded json data) and if it finds a redirect url, honour it.

This solves the problem because each pdp cp preview deployment controls the state it creates when trying to login via github.

We do not apply this pattern in production.

How to reproduce ☕️

N/A

Contributing 🙌🏽

Yes, I am willing to help implement this feature in a PR

balazsorban44 commented 1 year ago

The use case seems to be the same as https://github.com/nextauthjs/next-auth/pull/5375 but sounds a bit more versatile and could potentially be used for other purposes as well, so I think we are open to a change like this. :+1:

The only change that this might need is to honor a user value here: https://github.com/nextauthjs/next-auth/blob/c676e93d8a4ff5ba6288ed5ae8587c59ddd55267/packages/next-auth/src/core/lib/oauth/state-handler.ts#L19

I think passing through params.state here would be sufficient: https://github.com/nextauthjs/next-auth/blob/c676e93d8a4ff5ba6288ed5ae8587c59ddd55267/packages/next-auth/src/core/lib/oauth/authorization-url.ts#L57

What do you think?

jasonkuhrt commented 1 year ago

Are you open to making state become base64 JSON with a "secret" field. Then users can add whatever additional fields they want.

Alternatively, or in addition, a way to create the state on a per-request basis would be useful too, by e.g., passing a function that receives the request.

shirish87 commented 1 year ago

Changes suggested by @balazsorban44 seem sufficient to support checking of user-supplied state.

@jasonkuhrt users can somewhat do request-based state creation and read, but I suppose it comes at a performance cost.

image

image

NextAuthOptions params.state with a base64'd string that gets jwt signed and stored in the state cookie later: image

One additional change I was thinking of is making the state value available to the signIn callback. The state cookie gets cleared after the state check is complete, so it would be helpful to pass this for post successful login decision-making. image

Full patch:

diff --git a/packages/next-auth/src/core/lib/oauth/authorization-url.ts b/packages/next-auth/src/core/lib/oauth/authorization-url.ts
index de392db1..0227b65d 100644
--- a/packages/next-auth/src/core/lib/oauth/authorization-url.ts
+++ b/packages/next-auth/src/core/lib/oauth/authorization-url.ts
@@ -54,7 +54,7 @@ export default async function getAuthorizationUrl({
   const authorizationParams: AuthorizationParameters = params
   const cookies: Cookie[] = []

-  const state = await createState(options)
+  const state = await createState(options, params.state)
   if (state) {
     authorizationParams.state = state.value
     cookies.push(state.cookie)
diff --git a/packages/next-auth/src/core/lib/oauth/callback.ts b/packages/next-auth/src/core/lib/oauth/callback.ts
index 3185bf4c..79d4c6a2 100644
--- a/packages/next-auth/src/core/lib/oauth/callback.ts
+++ b/packages/next-auth/src/core/lib/oauth/callback.ts
@@ -142,7 +142,7 @@ export default async function oAuthCallback(params: {
       tokens,
       logger,
     })
-    return { ...profileResult, cookies: resCookies }
+    return { ...profileResult, state: state?.value, cookies: resCookies }
   } catch (error) {
     throw new OAuthCallbackError(error as Error)
   }
diff --git a/packages/next-auth/src/core/lib/oauth/state-handler.ts b/packages/next-auth/src/core/lib/oauth/state-handler.ts
index 5325b199..d594f700 100644
--- a/packages/next-auth/src/core/lib/oauth/state-handler.ts
+++ b/packages/next-auth/src/core/lib/oauth/state-handler.ts
@@ -7,7 +7,8 @@ const STATE_MAX_AGE = 60 * 15 // 15 minutes in seconds

 /** Returns state if the provider supports it */
 export async function createState(
-  options: InternalOptions<"oauth">
+  options: InternalOptions<"oauth">,
+  paramsState?: string | null,
 ): Promise<{ cookie: Cookie; value: string } | undefined> {
   const { logger, provider, jwt, cookies } = options

@@ -16,7 +17,7 @@ export async function createState(
     return
   }

-  const state = generators.state()
+  const state = paramsState ?? generators.state()
   const maxAge = cookies.state.options.maxAge ?? STATE_MAX_AGE

   const encodedState = await jwt.encode({
diff --git a/packages/next-auth/src/core/routes/callback.ts b/packages/next-auth/src/core/routes/callback.ts
index b7008d64..4fc8d1c0 100644
--- a/packages/next-auth/src/core/routes/callback.ts
+++ b/packages/next-auth/src/core/routes/callback.ts
@@ -43,6 +43,8 @@ export default async function callback(params: {
         profile,
         account,
         OAuthProfile,
+        // @ts-expect-error
+        state,
         cookies: oauthCookies,
       } = await oAuthCallback({
         query,
@@ -94,6 +96,8 @@ export default async function callback(params: {
             user: userOrProfile,
             account,
             profile: OAuthProfile,
+            // @ts-expect-error
+            state,
           })
           if (!isAllowed) {
             return { redirect: `${url}/error?error=AccessDenied`, cookies }
septatrix commented 1 year ago

Motivation:

The reason we control it is that GH App only allows specifying ONE callback URL. This is a big problem for dynamic preview deployments, like per PR ones.

This seems to be possible using redirectProxyUrl if I read its documentation correctly, no?

My use case is similar albeit a bit different. We want to redirect a user back to the page where specific page where they entered the website instead of back to the landing page. Our users for example can register devices by scanning QR codes which lead to our website with some payload set. When they have no session they are then redirected to our SSO but after the login they are at our landing page and all the custom payload (path, query params) are lost and the user has to scan the QR code again.