nextauthjs / next-auth

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

Callback url not set correctly if using custom login page #5409

Open voinik opened 2 years ago

voinik commented 2 years ago

Environment

System: OS: macOS 12.6 CPU: (10) arm64 Apple M1 Pro Binaries: Node: 16.17.0 - ~/.nvm/versions/node/v16.17.0/bin/node Yarn: 1.22.15 - ~/.nvm/versions/node/v16.17.0/bin/yarn npm: 8.15.0 - ~/.nvm/versions/node/v16.17.0/bin/npm Browsers: Firefox: 104.0.2 npmPackages: next: 12.3.0 => 12.3.0 next-auth: 4.10.3 => 4.10.3 react: 18.2.0 => 18.2.0

Reproduction URL

https://github.com/voinik/next-auth-wrong-callbackurl

Describe the issue

If you have a custom login page and try to access a page that requires you to be authenticated, then you get redirected to that login page, which is correct. You also get a redirectUrl=... added to the url during the redirect to your login page, which is also correct.

But when you subsequently login through email (I haven't tried other Providers), the url that gets sent to the email address contains an incorrect callbackUrl.

Let's say we tried to go to the /client route but were redirected to the login page because the /client page requires authentication. And let's say we login through email. The url we expect to get in the email is something like localhost:3000/api/auth/callback/email?callbackUrl=http%3A%2F%2Flocalhost%3A3000%2Fclient&token=..... or similar. But what we actually get is: http://localhost:3000/api/auth/callback/email?callbackUrl=http%3A%2F%2Flocalhost%3A3000%2Flogin%3FcallbackUrl%3D%252Fclient&token=......

Notice how the callbackUrl contains another callbackUrl.

Edit: If we go to the homepage and click on the "Log in" button I created on the far right of the header, then the callbackUrl we eventually get is: http://localhost:3000/api/auth/callback/email?callbackUrl=http%3A%2F%2Flocalhost%3A3000%2Flogin%3FcallbackUrl%3Dhttp%253A%252F%252Flocalhost%253A3000%252F&token=...... Notice how the complete baseUrl is inside the nested callbackUrl. To fix this I added the ternary check in the first return in the circumvention code below.

I managed to circumvent the issue by adding this to my callbacks object in my options object:

async redirect({ baseUrl, url }) {
    const redirectUrl = decodeURIComponent(url);
    const callbackIndex = redirectUrl.indexOf('callbackUrl=');
    if (callbackIndex > -1) {
        const callbackPath = redirectUrl.slice(callbackIndex + 12);
        // If I try to login from my homepage, the nested callbackUrl contains the full baseUrl.
        // This behavior seems to be triggerd if you call `signIn()` from a page.
        return callbackPath.includes(baseUrl) ? callbackPath : baseUrl + callbackPath;
    }
    return url;
},

But this obviously not a great long term solution.

How to reproduce

Here's a copy of the README of my repo:

How to reproduce the bug

  1. Add an .env file with NEXTAUTH_SECRET=heheheh
  2. Run npm install
  3. Start the project
  4. Go to http://localhost:3000/
  5. I've setup the middleware to require the user to be authenticated when going to the /client route, so click on the Client anchor tag
  6. You'll end up on the custom login page I created. Click on Do it! to attempt to log in.
  7. First of all, notice the url bar. It should say http://localhost:3000/login?callbackUrl=http%3A%2F%2Flocalhost%3A3000%2Flogin%3FcallbackUrl%3D%252Fclient&error=EmailSignin. Notice how the callbackUrl refers to the url of the custom login page that itself has a callbackUrl.
  8. Now check your terminal. I've logged the url that gets sent to the sendVerificationRequest function, which is the url that gets sent in the login email. It should look like this: url: http://localhost:3000/api/auth/callback/email?callbackUrl=http%3A%2F%2Flocalhost%3A3000%2Flogin%3FcallbackUrl%3D%252Fclient&token=.......

Notice how the callbackUrl that ends up in the email is the base url with a callback url that contains the url of the login page that itself again has a callbackUrl.

This means that when a user gets redirected to the login page (because they're attempting to access a page that requires them to be logged in), they get redirected to... the login page itself, rather than where they came from. The user will arrive on the login page and there will be a callbackUrl in the url bar to the page they actually want to go to.

Expected behavior

When using custom login pages we expect the url in the email to be something like localhost:3000/api/auth/callback/email?callbackUrl=http%3A%2F%2Flocalhost%3A3000%2Fclient&token=..... or similar. Instead we're getting a callbackUrl inside another callbackUrl.

lholiveiraa commented 2 years ago

did you do something to solve temporarily this issue in your code?

voinik commented 2 years ago

@lholiveiraa Yep, read the code underneath "I managed to circumvent the issue by adding this to my callbacks object in my options object:" in my post

niicojs commented 2 years ago

You can fix it passing the callback url passed in your login page to the signin function.


function MyCompoenent() {
    const route = useRouter();
    return (
    <Button
      leftIcon={<FaSalesforce />}
      colorScheme="teal"
      variant="solid"
      onClick={() =>
        signIn(
          'salesforce',
          { redirect: true, callbackUrl: route.query.callbackUrl } 
        )
      }
    >
      Login
    </Button>
  );
}
altsst commented 1 year ago

i still experience this issue

y-nk commented 1 year ago

same here. this is not solved at all, but maybe nobody cares?

y-nk commented 1 year ago

@voinik i found this a bit more safe. for some reason i got multiple imbrication of the callbackUrl params...

async redirect({ url, baseUrl }) {
  const redirectUrl = decodeURIComponent(url)
  const callbackIndex = redirectUrl.lastIndexOf('?callbackUrl=');

  if (callbackIndex > -1) {
    const callbackPath = redirectUrl.slice(callbackIndex);
    return callbackPath.includes(baseUrl) ? callbackPath : baseUrl + callbackPath;
  }

  return baseUrl
},
itsjavi commented 1 year ago

This also happens when you don't send the "email" form field. In my case I forgot to add the name="email" property.

The error code is not very useful... https://github.com/nextauthjs/next-auth/blob/0e6c51adecc54d3102ef3cda57f909400bc99c29/packages/next-auth/src/core/routes/signin.ts#L38

Because according to the docs EmailSignin means that the email failed to be sent https://next-auth.js.org/configuration/pages#sign-in-page\

An error code like EmailRequired would be more helpful

aeksco commented 1 year ago

I just encountered this issue when setting up login using the AzureADB2C adapter - took quite a bit of time to narrow the problem down to a side effect of setting a custom value for AuthOptions.pages.signIn. The solution provided by @voinik and @y-nk resolved the problem I was facing (thank you 🙏).

A similar issue with the same underlying redirect problem was encountered in #5192 when using the signIn() function with a custom sign-in page. Passing SignInOptions options into the signIn() function may or may not solve the problem depending on how your codebase is structured.

I suggest an addition to the options.pages documentation to inform readers that a custom signIn page will require one of the following additions to ensure correct handling of redirects:

aeksco commented 1 year ago

I just encountered this issue when setting up login using the AzureADB2C adapter - took quite a bit of time to narrow the problem down to a side effect of setting a custom value for AuthOptions.pages.signIn. The solution provided by @voinik and @y-nk resolved the problem I was facing (thank you 🙏).

A similar issue with the same underlying redirect problem was encountered in #5192 when using the signIn() function with a custom sign-in page. Passing SignInOptions options into the signIn() function may or may not solve the problem depending on how your codebase is structured.

I suggest an addition to the options.pages documentation to inform readers that a custom signIn page will require one of the following additions to ensure correct handling of redirects:

  • Pass SignInOptions into signIn() function
  • Override the default behavior of AuthOptions.callbacks.redirect

Follow up to the above:

Within a few hours of implementing AuthOptions.callbacks.redirect solution above I began noticing an INVALID_CALLBACK_URL_ERROR would occur when restarting the authentication flow after the previous attempt was interrupted or cancelled. I've removed AuthOptions.callbacks.redirect from my implementation, and updated my login page to pass SignInOptions into the signIng() function like so:

import { signIn } from "next-auth/react";
import { useRouter } from "next/router";

export function LoginPage() {
  const router = useRouter();

  const callbackUrl = Array.isArray(router.query.callbackUrl)
    ? router.query.callbackUrl[0]
    : router.query.callbackUrl;

  return (
    <div>
      Login Page

      <button
        onClick={() => {
          signIn("azure-ad-b2c", {
            redirect: true,
            callbackUrl,
          });
        }}
      >
        Sign In with AzureAD B2C
      </button>
    </div>
  );
}

Since making the above change I haven't had any other issues materialize - my instinct at the moment is to avoid modification of the redirect behavior unless absolutely necessary, and instead always pass a SignInOptions object into signIn() to support the expected redirect behavior on custom sign-in pages.

olayway commented 1 year ago

Same issue 👩‍💻🔫

http://localhost:3000/auth/signin?callbackUrl=http%3A%2F%2Flocalhost%3A3000%2Fauth%2Fsignin%3FcallbackUrl%3Dhttp%253A%252F%252Flocalhost%253A3000%252Fauth%252Fsignin%253FcallbackUrl%253Dhttp%25253A%25252F%25252Flocalhost%25253A3000
harsimran-d commented 2 months ago

as mentioned by @aeksco the approach that works best is described above https://github.com/nextauthjs/next-auth/issues/5409#issuecomment-1561873908

but needed some changes to be used in the app router as the api have changed

what worked for me was as the following

import { signIn } from "next-auth/react";
import { useSearchParams } from "next/navigation";

export function LoginPage() {
  const searchParams = useSearchParams();
  const callbackUrl = searchParams.get("callbackUrl") || "/";

  return (
    <div>
      Login Page

      <button
        onClick={() => {
          signIn("azure-ad-b2c", {
            redirect: true,
            callbackUrl,
          });
        }}
      >
        Sign In with AzureAD B2C
      </button>
    </div>
  );
}