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(middleware): introduce withAuth Next.js method #3657

Merged
merged 27 commits into from
Feb 3, 2022

Conversation

balazsorban44
Copy link
Member

@balazsorban44 balazsorban44 commented Jan 18, 2022

This is currently an idea 💡 but it fixes #3037

To test it out, see #3657 (comment)

Useful resources:

Usage examples

Authentication only

You allow any logged-in user to access a page

// pages/_middleware.js

export { default } from "next-auth/middleware"

Authorization

Certain conditions must be met for the user to continue, for example, to restrict an admin dashboard, you can do:

// pages/admin/_middleware.js
import { withAuth } from "next-auth/middleware"

export default withAuth({
  callbacks: {
    authorized: ({ token }) => token?.user.isAdmin
  }
})

See the source code for the proposed API.

Suggestion: We could detect if NEXTAUTH_SECRET is present, sparing the user to fill an extra configuration field. (NOTE: This could be done in NextAuth in pages/api/auth/[...nextauth] as well)

Open for feedback.

@codecov-commenter
Copy link

codecov-commenter commented Jan 18, 2022

Codecov Report

Merging #3657 (817b2cf) into main (844c9b1) will decrease coverage by 0.25%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3657      +/-   ##
==========================================
- Coverage   12.94%   12.68%   -0.26%     
==========================================
  Files          93       94       +1     
  Lines        1468     1498      +30     
  Branches      390      399       +9     
==========================================
  Hits          190      190              
- Misses       1267     1297      +30     
  Partials       11       11              
Impacted Files Coverage Δ
src/jwt/index.ts 0.00% <ø> (ø)
src/lib/parse-url.ts 77.77% <ø> (ø)
src/next/middleware.ts 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 844c9b1...817b2cf. Read the comment docs.

@balazsorban44 balazsorban44 temporarily deployed to Preview January 18, 2022 00:57 Inactive
@github-actions
Copy link

github-actions bot commented Jan 18, 2022

🎉 Experimental release published on npm!

npm i next-auth@0.0.0-pr.3657.f1addc18
yarn add next-auth@0.0.0-pr.3657.f1addc18

@balazsorban44 balazsorban44 temporarily deployed to Preview January 18, 2022 01:45 Inactive
@balazsorban44 balazsorban44 temporarily deployed to Preview January 18, 2022 01:49 Inactive
@balazsorban44 balazsorban44 temporarily deployed to Preview January 18, 2022 01:54 Inactive
@pdevito3
Copy link

Love this idea and is what i'm used to doing on the backend side if I was making a BFF!

FWIW, something like the below is what I might use in a .NET app. The api is really easy and powerful. Might provide some inspiration here:

          services.AddAuthentication(options =>
          {
              options.DefaultScheme = "cookie";
              options.DefaultChallengeScheme = "oidc";
              options.DefaultSignOutScheme = "oidc";
          })
          .AddCookie("cookie", options =>
          {
              options.Cookie.Name = "__Host-bff";
              options.Cookie.SameSite = SameSiteMode.Strict;
          })
          .AddOpenIdConnect("oidc", options =>
          {
              options.Authority = "https://localhost:3385";
              options.ClientId = "react.bff";
              options.ClientSecret = "secret";
              options.ResponseType = "code";
              options.ResponseMode = "query";
              options.usePkce = true;

              options.GetClaimsFromUserInfoEndpoint = true;
              options.MapInboundClaims = false;
              options.SaveTokens = true;

              options.Scope.Clear();
              options.Scope.Add("openid");
              options.Scope.Add("profile");
              options.Scope.Add("role");
              options.Scope.Add("email");
              options.Scope.Add("recipe_management_access");
              options.Scope.Add("offline_access");

              options.TokenValidationParameters = new()
              {
                  NameClaimType = "name",
                  RoleClaimType = "role"
              };
          });

src/next/middleware.ts Outdated Show resolved Hide resolved
@zpg6
Copy link
Contributor

zpg6 commented Jan 18, 2022

Thanks for addressing this feature! Some feedback:

  • How would you combine this with other middleware in the same scope? My use case involves some unrelated items, like rate limiting, alongside the withAuth method for most of my API.

  • I'll often times be decoding the tokens again in the handler functions to get the userId as a part of the database query. Would it be more efficient (and would it be secure) to have the middleware store the token claims it already decoded in the request?

@balazsorban44
Copy link
Member Author

balazsorban44 commented Jan 18, 2022

@zpg6

  1. I haven't seen much middleware chaining yet, but It could work similarly to https://next-auth.js.org/configuration/initialization#advanced-initialization I guess. Any proposal? We could do like a wrapper I think:
withAuth(
  (req, ev) => {/*your middleware*/},
  {/*your options*/}
)

This would allow us to put it either as any element in the chain.

  1. Decoding a JWT is probably much more efficient than storing it somewhere and receiving it again. It also introduces complexity. We can deal with it later when/if we implement support for database sessions. For perf reasons, I don't think it's feasible yet, but in that case, caching would be an option probably.

@zpg6
Copy link
Contributor

zpg6 commented Jan 18, 2022

  1. Thumbs up on your suggested wrapper, and good point... still to be seen how the nextjs community handles chaining more generally. Having a lightweight escape hatch like this until then would be plenty.

  2. Thanks for the insights on performance of decoding twice. Just one of those things that feels redundant, but I suppose storing in the request does add extra complexity without any real code savings.

Planning to test our your pr soon, will follow up with any DX feedback.

@always-maap
Copy link

great work.
in the case of external authorization, what is the best practice in next auth middleware? for example, google handle IAM roles like so:

  • accesscontextmanager.accessPolicies.create
  • accesscontextmanager.accessPolicies.update
  • ...

each user(admin) has different roles indicated like above.
calling user role list in the token callback and attach to the token, then checking in _middleware? is it possible?

// pages/accessPolicies/_middleware.js
import { withAuth } from "next-auth/next/middleware"

export default withAuth({
  authorized: ({ token }) => token?.roles.includes('accesscontextmanager.accessPolicies')
})

@balazsorban44
Copy link
Member Author

balazsorban44 commented Jan 18, 2022

@always-maap next-auth has its own session, so it remains agnostic about the Provider you are using. In your case, as you say, the jwt callback can be used to save the user's roles at sign in, and then you can check that as you showed in your code above.

My example logic is simplified, but you can do much more complex checks since you get a function. The original req is also passed to authorized as a named parameter, in case you need some contextual information to decide on authorization.

Please see the changed files:

authorized: (options: {
token: JWT | null
req: NextRequest
}) => Awaitable<boolean>

@Rees1993
Copy link

Is the NextRequest req variable available inside the authorized function? I have a scenario where specific paths are accessible when the user isn't logged in and I need to check the current pathname.

@balazsorban44
Copy link
Member Author

balazsorban44 commented Jan 18, 2022

@Rees1993 literally the comment above yours :D

#3657 (comment)

There is also another way. You can include a middleware only for a subset of pages, if your URL structure allows it. (eg.: pages/admin/_middleware.js will only call the middleware for pages under /admin, like /admin/a /admin/b, /admin/a/b etc. A page like /home wouldn't even invoke the Middleware. )

Useful resource: https://nextjs.org/docs/middleware#execution-order

@Rees1993
Copy link

@balazsorban44 Well I think I might need to go to the opticians!

Unfortunately these pages are all at the base level (pages/contact.js, pages/login.js etc.) so I don't think it would work. Thanks for that link though as I forgot about that functionality.

Otherwise, I'm all for this middleware and look forward to testing it!

@balazsorban44 balazsorban44 temporarily deployed to Preview February 2, 2022 12:31 Inactive
@balazsorban44 balazsorban44 temporarily deployed to Preview February 2, 2022 15:31 Inactive
@balazsorban44
Copy link
Member Author

Moved authorized to callbacks.authorized to resemble the callbacks.signIn method as they have the same purpose. This aligns our APIs a bit better.

@balazsorban44 balazsorban44 temporarily deployed to Preview February 2, 2022 17:11 Inactive
@balazsorban44 balazsorban44 temporarily deployed to Preview February 3, 2022 16:59 Inactive
@balazsorban44 balazsorban44 merged commit f3be5e8 into main Feb 3, 2022
@balazsorban44 balazsorban44 deleted the feat/with-auth-middleware branch February 3, 2022 17:07
}
}

const token = await getToken({ req: req as any })

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@balazsorban44 Does this middleware will work with strategy: 'database'?

I have similar middleware, but with getSession request, because 'getToken' was null if i remember correct...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Only supports the "jwt" session strategy. We need to wait until databases at the Edge become mature enough to ensure a fast experience. (If you know of an Edge-compatible database, we would like if you proposed a new Adapter)"

caveats

mind sharing your sessions middleware function?  👀

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is pretty simple

export async function middleware(request: NextRequest) {

  const cookie = request.headers.get('cookie')

  const session = cookie
    ? await getSession({ req: { headers: { cookie } } as any })
    : null

  if (!session) {
    return NextResponse.redirect(new URL('/api/auth/signin', request.url))
  }
}

@zanzlender
Copy link

zanzlender commented Feb 25, 2022

I tried to do this but for some reason it doesn't work... I always get redirected to Next-auth 's default login page...
However looking at the middleware example I made this implementation without the next-auth/middleware which works for me.

// pages/_middleware.ts
import type { NextFetchEvent, NextRequest } from 'next/server'
import { getToken } from 'next-auth/jwt';

export function middleware(req: NextRequest, ev: NextFetchEvent) {
  let token = await getToken({
        req: request,
        secret: process.env.JWT_SECRET
      });

  if(token) return NextResponse.next();
  
  return NextResponse.redirect(`${process.env.HOST}/login`);
}

However like some mentioned in this and other issues, if you are using TypeScript, the req in getToken is compatible with NextApiRequest, but not NextRequest... I don't know if there is some hidden logic there, but for me it works just fine if you don't define the type and just use NextRequest.

However, since I wanted type safety, I just made this little change in @next-auth/jwt/index.d.ts.

I imported this
import type { NextRequest } from "next/server";

And changed the acceptable types in GetTokenParams --> req

export interface GetTokenParams<R extends boolean = false> {
    /** The request containing the JWT either in the cookies or in the `Authorization` header. */
    req: NextApiRequest | NextRequest | Pick<NextApiRequest, "cookies" | "headers">;   <-- added NextRequest
    ....
}

Correct me if this is wrong and if this could break the app, but for now it works for me, and I had no problems what so ever. 👍

@talDoFlemis
Copy link

On build time it complains about the req when I deploy on Vercel, one way that I've found that worked was this

import { getToken } from "next-auth/jwt"
import { NextResponse } from "next/server"
import { NextApiRequest } from "next"

export async function middleware(req: NextApiRequest) {
  const url = req.url
  const token = await getToken({
    req,
    secret: process.env.NEXTAUTH_SECRET,
    secureCookie: process.env.NODE_ENV === "production",
  })

  if (url?.includes("/admin") && !token) {
    const parsedUrl = new URL(url)
    parsedUrl.pathname = "/stop"
    return NextResponse.redirect(parsedUrl.href)
  }
}

In that way, we still can use the token for the middleware autentication.

@balazsorban44
Copy link
Member Author

Hi, please note that commenting on a merged PR is not going to be looked at. Please open a bug report with your reproduction if having issues. 🙏

@nextauthjs nextauthjs locked as resolved and limited conversation to collaborators Mar 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
core Refers to `@auth/core`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Next.js 12 Middleware