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

feat: forward auth params from signin to provider #823

Merged
merged 5 commits into from Dec 5, 2020

Conversation

balazsorban44
Copy link
Member

@balazsorban44 balazsorban44 commented Nov 2, 2020

According to the spec, additional parameters can be sent to the authorization url, please see the spec:

https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest

I would like to send the prompt: "login" param, so my users will be forced to log in on the Provider, whenever they click on a button with the handler

signIn("identity-server4", {}, {
  prompt: "login"
})

NOTE: These params should not be sent in the body as args here:

// If is any other provider type, POST to provider URL with CSRF Token,
// callback URL and any other parameters supplied.
const fetchOptions = {
method: 'post',
headers: {
'Content-Type': 'application/x-www-form-urlencoded'
},
body: _encodedForm({
...args,
csrfToken: await getCsrfToken(),
callbackUrl: callbackUrl,
json: true
})
}

This is why I decided to add a third argument to the signIn function instead. This also keeps this change backward compatible.

Tested this locally, and it works as intended

This fixes #737, as far as I can tell.

@vercel
Copy link

vercel bot commented Nov 2, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/nextauthjs/next-auth/95h0lbbzs
✅ Preview: https://next-auth-git-feature-forward-auth-params.nextauthjs.vercel.app

@vercel vercel bot temporarily deployed to Preview November 2, 2020 12:21 Inactive
@vercel vercel bot temporarily deployed to Preview November 2, 2020 12:28 Inactive
@vercel vercel bot temporarily deployed to Preview November 2, 2020 12:48 Inactive
@iaincollins iaincollins added the enhancement New feature or request label Nov 2, 2020
@iaincollins iaincollins self-requested a review November 2, 2020 14:13
@makezi
Copy link

makezi commented Nov 13, 2020

This would greatly benefit the situation in which I would like to append form data (along with register=true) to the URL for my custom Auth0 lock screen to default to the signup tab and process said form data.

@vercel vercel bot temporarily deployed to Preview December 4, 2020 00:30 Inactive
@vercel vercel bot temporarily deployed to Preview December 5, 2020 08:50 Inactive
@balazsorban44 balazsorban44 changed the title Forward auth params from signin to provider feat: forward auth params from signin to provider Dec 5, 2020
@balazsorban44 balazsorban44 merged commit 545a7e7 into nextauthjs:main Dec 5, 2020
@balazsorban44 balazsorban44 deleted the feature/forward-auth-params branch December 5, 2020 08:54
@jmfury
Copy link
Contributor

jmfury commented Jan 13, 2021

This would greatly benefit the situation in which I would like to append form data (along with register=true) to the URL for my custom Auth0 lock screen to default to the signup tab and process said form data.

@makezi or @balazsorban44 Did either of you happen to get this option working with Auth0? (in tests or the like?)

I'm using the canary and I don't see a difference when passing args in.

Adding mode (statically) as a param in the authorizationUrl within the Auth0 provider options works but for some reason I can't use the incoming req to set that dynamically (provider options are returned from a function in this case, maybe that's not necessary?).

I'd like to use a more dynamic option like the one this PR adds.. i.e. Support both login or signUp simply from the signin() method

@balazsorban44
Copy link
Member Author

Hi there! I removed this option after #1022 in canary, but if there is still a need for it, maybe I could add it back. Maybe open a feautr request issue on it?

@jmfury
Copy link
Contributor

jmfury commented Jan 13, 2021

Hi there! I removed this option after #1022 in canary, but if there is still a need for it, maybe I could add it back. Maybe open a feautr request issue on it?

@balazsorban44 I think there would be a need to accept the args on the /signin server route as well.. otherwise these aren't passed thru to the URL that's built there 🔗

I'll plan to PR something in addition since that'll help me describe the interface & useful options for Auth0 specifically 👍

@makezi
Copy link

makezi commented Jan 13, 2021

@balazsorban44 I know that having authParams is still necessary for Auth0's case for customisation/passing params to Auth0 Lock. This is blocking for our situation hence have to remain on < version 3.2.0-canary.15

@jmfury My case was a bit more complicated hence had to customise the lock sign in page to read some other user registration data from the app client. Either way, with authParams you could still pass params to lock, mainly initialScreen to open lock on sign up. We made it easier to just provide an auth interface across our app, an example:

// utils/auth.js
import { signIn } from 'next-auth/client';

function handleSignIn(event, params) {
  event.preventDefault();
  signIn('auth0', { callbackUrl: '/' }, params);
}

function handleRegister(event, params) {
  event.preventDefault();
  // This is what we did but in your case you would do { initialScreen: 'signUp', ...params }
  signIn('auth0', { callbackUrl: '/' }, { register: true, ...params} );
}

...

<a href="/register" onClick={handleRegister}>Register</button>

Hope that helps! I'll upvote that PR 👍!

@jmfury
Copy link
Contributor

jmfury commented Jan 14, 2021

@makezi This was hugely helpful..

After digging thru the client code I realized I was wrong about the above (re: server route for signin).

The following is a customized version of the original signIn() helper for my use case, but perhaps it is useful for you as well?

async function signUp(provider, args) {
  const csrfToken = await getCsrfToken()
  const signInUrl = `${process.env.NEXT_PUBLIC_NEXTAUTH_URL}/api/auth/signin/${provider}` 
  const fetchOptions = {
    method: 'POST',
    headers: {
      'Content-Type': 'application/x-www-form-urlencoded'
    },
    body: encodeForm({ // Taken from the `next-auth` helper
      csrfToken,
      callbackUrl: args?.callbackUrl || process.env.NEXT_PUBLIC_NEXTAUTH_URL,
      json: true
    })
  }
  const res = await fetch(signInUrl, fetchOptions)
  const data = await res.json()
  return (window.location = data?.url ? data.url + `&mode=signUp`  : '/my-signin-error-page') // add custom params to signInUrl before navigating, or go to error page if the call fails
}

This isn't quite as robust as the signIn() helper just yet, definitely not firm on this.

Given the above, another option to consider might be some sort of getSignInUrl() helper. This could simply return the URL to the client and it would be the consumer's responsibility to add custom params and navigate to the URL itself...

import { getSignInUrl } from 'next-auth/client';

async function handleCustomSignIn() {
  const baseSignInUrl = await getSignInUrl('auth0', { callbackUrl: '/' }) 
  return window.location = baseSignInUrl + `&foo=bar` // Consumer chooses what to do with the URL
}
......

<Button type="button" onClick={handleCustomSignIn}>Register</Button>

Maybe there's too much overlap with signIn() here and that's totally fine, just a thought.

@balazsorban44
Copy link
Member Author

So this is available in next-auth@canary.26 again! 🎉

@github-actions
Copy link

github-actions bot commented Feb 1, 2021

🎉 This PR is included in version 3.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Send ui_locales to external provider trough login
4 participants