next-auth icon indicating copy to clipboard operation
next-auth copied to clipboard

Middleware unauthorized callback option

Open HXavS opened this issue 2 years ago โ€ข 9 comments

Discussions

#4791

โ˜•๏ธ Reasoning

What changes are being made? What feature/bug is being fixed here? Added callback option for middleware isAuthorized returning false.

###Previous behavior was to always go to get stuck in signIn redirect loop. Changing Pages.signIn option in middleware is inadequate because a user can be logged in but not authorized to a page. Changing the sign in page would cause user's not signed in to end up on the same page.

Current workaround is to Create a middleware function and pass as first arg of withAuth. This issue with this is that you would have to validate only user login with isAuthorized and handle actual permissions check in the function being fired "onSuccess". This is not intuitive and more complex then needed for most use cases. Callbacks stream line the handling of different authentication scenarios.

Default Behavior remains the same with noUser actions converted to configurable callback with default behavior working as current release.

๐Ÿงข Checklist

  • [ ] Documentation
  • [ ] Tests
  • [ ] Ready to be merged

๐ŸŽซ Affected issues

Fixes: #4787 #4234

๐Ÿ“Œ Resources

HXavS avatar Jun 29 '22 02:06 HXavS

The latest updates on your projects. Learn more about Vercel for Git โ†—๏ธŽ

1 Ignored Deployment
Name Status Preview Updated
next-auth โฌœ๏ธ Ignored (Inspect) Jun 29, 2022 at 2:27AM (UTC)

vercel[bot] avatar Jun 29 '22 02:06 vercel[bot]

Thanks. so there is pages.signIn that you can set: https://next-auth.js.org/configuration/nextjs#pages

I think we could rather add a error=Unauthorized query parameter if the user was not authorized and show that info in the signin page to inform the user correctly,instead of adding another callback method.

So here:

https://github.com/nextauthjs/next-auth/blob/f3233641d0ce4fb1b5c357e66e2d9cd70af27a15/packages/next-auth/src/next/middleware.ts#L131-L134

And here:

https://github.com/nextauthjs/next-auth/blob/f3233641d0ce4fb1b5c357e66e2d9cd70af27a15/packages/next-auth/src/core/pages/signin.tsx#L7

https://github.com/nextauthjs/next-auth/blob/f3233641d0ce4fb1b5c357e66e2d9cd70af27a15/packages/next-auth/src/core/pages/signin.tsx#L71

balazsorban44 avatar Jun 29 '22 08:06 balazsorban44

I originally went the route of using the pages.signIn. I thought a call back was more appropriate in some cases for a few reasons.

  • I do not want users who are not authorized to access admin only areas to have to log in again.
  • I would rather return them to where they were originally or default to their "homepage" with the error message.
  • If a user was not signed in and you redirected to a page without the sign in it creates a messy user experience. Let's say an admin goes to direct link, but is not logged in. Middleware Redirects to default user dashboard, which triggers another not authorized redirect to pages.signIn configured in api/auth/[...next-auth].ts. When the admin does sign in the callback will be to the default user dashboard and not original admin request.

Also the default login for next-auth does not included meaningful navigation for users to get out of the loop. If a user was to go to http://example.com/admin directly, then they would only be able to see the login screen with error message saying they are not authorized even if they did have an account.

HXavS avatar Jun 29 '22 15:06 HXavS

You can create your own signin page though and handle the error however you want, if the default would not cover your use case.

balazsorban44 avatar Jun 29 '22 23:06 balazsorban44

the reason is that I don't want to introduce another callback method for this

balazsorban44 avatar Jun 29 '22 23:06 balazsorban44

Okay then maybe something nuanced like this to prevent infinite sign in loops for users that exist but are not authorized for the route the middleware is evaluating.

  // the user is not logged in, redirect to the sign-in page
  const signInUrl = new URL(signInPage, req.nextUrl.origin)
  if(!token)
    signInUrl.searchParams.append(
      "callbackUrl",
      `${req.nextUrl.pathname}${req.nextUrl.search}`
    )

HXavS avatar Jul 01 '22 06:07 HXavS

How about allowing the user to return NextResponse or boolean from authorized?

If the return type is boolean then we would apply current logic (redirect to sign in if false, if true then allow the request to proceed). Otherwise, the middleware would just return the provided NextResponse allowing for custom redirects

import { withAuth } from "next-auth/middleware"

export default withAuth({
  callbacks: {
    authorized: ({ req, token }) => {
      const path = req.nextUrl.pathname;

      if (path.startsWith('/admin') && token !== null && token.role !== 'admin') {
        const url = req.nextUrl.clone();
        url.pathname = '/redirect-to'
        return NextResponse.redirect(url)
      }

      return token !== null;
    }
  }
})

export const config = {
  matcher: [
    "/profile/:path*",
    "/admin/:path*"
  ]
}

miikebar avatar Jul 03 '22 11:07 miikebar

@MrPumpking. I personally like this idea. In my opinion it simplifies user role handling logic. The examples in documentation show the admin role check. Yet, the current implementation seems more geared towards should user be allowed to log in. and not page specific permission handling.

Currently, the withAuth function from "next-auth/middleware" can be passed a function that is called onSuccess of Authorization. Custom role routing logic can be handled there.

The main thing I want to have fixed is that default behavior of infinitely looping logged in users constantly being brought to a login screen if authentication is ever returned false.

HXavS avatar Jul 05 '22 17:07 HXavS

๐Ÿ‘‹ Is there any news on this PR? I'm also interested in being able to inform the user the requested page is not authorized, instead of redirect them to the sign in page (that makes no sense if the user is correctly logged in)

Please tell me if I can help make this move forward ๐Ÿค—

pybuche avatar Aug 16 '22 09:08 pybuche

IMO we could also allow returning a URL instance so that we can pass in our NextResponse.redirect call, would be simpler for developers.

import { withAuth } from "next-auth/middleware"

export default withAuth({
  callbacks: {
    authorized: ({ req, token }) => {
      const path = req.nextUrl.pathname;

      if (path.startsWith('/admin') && token !== null && token.role !== 'admin') {
        const url = req.nextUrl.clone();
        url.pathname = '/redirect-to'
        return url
      }

      return token !== null;
    }
  }
})

export const config = {
  matcher: [
    "/profile/:path*",
    "/admin/:path*"
  ]
}

The internal would look something like this


  const isAuthorizedOrRedirectURL =
    (await options?.callbacks?.authorized?.({ req, token })) ?? !!token

  // the user is authorized, let the middleware handle the rest
  if (isAuthorizedOrRedirectURL === true) return await onSuccess?.(token)
  // the user is not authorized and a custom redirect URL is defined, use that
  if (isAuthorizedOrRedirectURL instanceof URL) {
    return NextResponse.redirect(isAuthorizedOrRedirectURL)
  }

Let me know what you think @balazsorban44 @MrPumpking ๐Ÿ‘€

ThangHuuVu avatar Nov 27 '22 06:11 ThangHuuVu

Middlware still doesn't work

nextjs 13.1.1 next-auth 4.18.3

pencilcheck avatar Jan 07 '23 13:01 pencilcheck

Closing for inactivity. In summary, we don't want to add more callback options to the withAuth method. Instead, we prefer the approach described at https://github.com/nextauthjs/next-auth/pull/4788#issuecomment-1169705857. We are happy to revisit if there are more community interests in the matter ๐Ÿซถ

ThangHuuVu avatar Jul 12 '23 15:07 ThangHuuVu