Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow customizing state #5727

Open
jasonkuhrt opened this issue Nov 4, 2022 · 4 comments
Open

Allow customizing state #5727

jasonkuhrt opened this issue Nov 4, 2022 · 4 comments
Labels
accepted The feature request is considered. If nobody works actively on it, feel free to. enhancement New feature or request

Comments

@jasonkuhrt
Copy link

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

@jasonkuhrt jasonkuhrt added enhancement New feature or request triage Unseen or unconfirmed by a maintainer yet. Provide extra information in the meantime. labels Nov 4, 2022
@balazsorban44
Copy link
Member

The use case seems to be the same as #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. 👍

The only change that this might need is to honor a user value here:

I think passing through params.state here would be sufficient:

const state = await createState(options)

What do you think?

@balazsorban44 balazsorban44 added accepted The feature request is considered. If nobody works actively on it, feel free to. and removed triage Unseen or unconfirmed by a maintainer yet. Provide extra information in the meantime. labels Nov 6, 2022
@jasonkuhrt
Copy link
Author

jasonkuhrt commented Nov 7, 2022

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
Copy link

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted The feature request is considered. If nobody works actively on it, feel free to. enhancement New feature or request
Projects
None yet
4 participants