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

fix: Fix next-auth middleware not working on Lambda #38

Merged
merged 3 commits into from
Feb 23, 2023

Conversation

khuezy
Copy link
Contributor

@khuezy khuezy commented Feb 2, 2023

This PR allows next-auth/middleware to properly run in Lambda environment.

When using this w/ NextjsSitev2 you will probably get infinite redirects due to the way the CloudFront Distribution is set up. To get around this, you can make changes to NextjsSite, eg:
NOTE: there's a limit of 25 cache behaviors, so if you need more than that just request a quota upgrade and you should easily get several hundred.

    createCloudFrontDistributionForRegional() {
       ...
        const baseServerBehavior = {
            viewerProtocolPolicy: cloudfront.ViewerProtocolPolicy.REDIRECT_TO_HTTPS,
            origin: new origins.HttpOrigin(Fn.parseDomainName(fnUrl.url)),
            allowedMethods: cloudfront.AllowedMethods.ALLOW_ALL,
            cachedMethods: cloudfront.CachedMethods.CACHE_GET_HEAD_OPTIONS,
            compress: true,

            cachePolicy: cdk?.cachePolicies?.serverRequests ??
                this.createCloudFrontServerCachePolicy(),
        };
        const middlewareBehavior = {
            ...baseServerBehavior,
            edgeLambdas: isMiddlewareEnabled ? [{
                eventType: cloudfront.LambdaEdgeEventType.VIEWER_REQUEST,
                functionVersion: middlewareFn.currentVersion,
                includeBody: true
            }] : undefined
            }
        const serverBehavior = {
            ...baseServerBehavior
        }
        const middlewareRoutes = this.getMiddlewareRoutes(middlewareBehavior)
       ...
       additionalBehaviors: {
                ...middlewareRoutes,
                "_next/data/*": serverBehavior,
                "_next/image*": imageBehavior,
                "_next/*": staticFileBehaviour,
                ...(cfDistributionProps.additionalBehaviors || {}),
            },
    }
    // create middleware behavior only for the matcher defined in middleware.ts
    getMiddlewareRoutes(behavior) {
        const middlewareManifest = path.join(this.props.path, '.next/server/middleware-manifest.json')
        const content = fs.readFileSync(middlewareManifest).toString('utf8')
        const obj = JSON.parse(content)
        const matcher = obj.middleware['/'].matchers
        const routes = {}
    
        matcher.forEach((reg) => {
          let path = reg.regexp
            .replace('^(?:\\/(_next\\/data\\/[^/]{1,}))?', '')
            .replace('(.json)?[\\/#\\?]?$', '')
            .replace('(?:\\/((?:[^\\/#\\?]+?)(?:\\/(?:[^\\/#\\?]+?))*))?', '/*')
            .replaceAll('\\', '')
            .replace(/^\//, '')
          routes[path] = behavior
        })
    
        return routes
    }

Or you can not use the default middleware and read the token manually:
NOTE: the current NextjsSite distribution will route all requests to middleware... this should be fixed in the future per logic above.

This will guard all your /api endpoints:

import { NextRequest, NextResponse } from 'next/server'
import { getToken } from 'next-auth/jwt'

export async function middleware(req: NextRequest) {
     const pathname = req.nextUrl.pathname
     // /api/auth shouldn't be restricted or else user will never be able to login
     if (!pathname.startsWith('/api/auth/') && pathname.startsWith('/api/')) { // add your own path checks
       const token = await getToken({ req })
       if (!token) {
         return new NextResponse('Unauthorized', {
           status: 401,
           headers: { 'content-type': 'text/plain' }
         })
      }
    }
}

@changeset-bot
Copy link

changeset-bot bot commented Feb 2, 2023

⚠️ No Changeset found

Latest commit: cb8de25

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Feb 2, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
open-next ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 22, 2023 at 11:05PM (UTC)

@@ -306,6 +310,7 @@ function esbuildSync(options: BuildOptions) {
format: "esm",
platform: "node",
bundle: true,
minify: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This saves up to 60% of size

@@ -57,8 +57,14 @@ export async function handler(event: CloudFrontRequestEvent): Promise<CloudFront
return request;
}

// In the case where `middleware.ts` response w/ a body, send that back to the client
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This may be useful to have if your middleware.ts returns data immediately to the client.

@revmischa
Copy link
Contributor

Just my opinion but having to request quota increases on every AWS account you want to deploy a small-medium sized nextjs project is not a great experience

@khuezy
Copy link
Contributor Author

khuezy commented Feb 2, 2023

Yes I agree, but we have to eat the 💩 sandwich due to the default limitation. 25 should be enough for a small project though. If you're using a lot more behaviors then expecting the user to request for quota increase seems reasonable.

@timothymiller
Copy link

timothymiller commented Feb 15, 2023

It's great to see the progress on this PR. Next-auth support is a blocker to using SST for my team. 🚀🚀🚀🚀 Let's Ship it!!

@@ -57,8 +57,14 @@ export async function handler(event: CloudFrontRequestEvent): Promise<CloudFront
return request;
}

// In the case where `middleware.ts` response w/ a body, send that back to the client
let responseBody
const state = Object.getOwnPropertySymbols(response).find(s => s.toString() === 'Symbol(state)')
Copy link
Contributor

Choose a reason for hiding this comment

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

is this symbol not exported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason, the symbol wasn't create via Symbol.for('state') so I couldn't access it by key.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

turns out it was much simpler to use response.text() to get the body.

return {
status: response.status,
body: responseBody,
body: response.body ? await response.text() : undefined,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for cleaning up! Can you also remove all the console.log please? CW $ adds up pretty quick from the extra bytes.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah.. I will make a separate PR for that today. I want to add a DEBUG flag, and when u do DEBUG=true open-next build, it will console.log.

@fwang fwang merged commit 61d9b91 into opennextjs:main Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants