sidebase / nuxt-auth

Authentication built for Nuxt 3! Easily add authentication via OAuth providers, credentials or Email Magic URLs!
https://auth.sidebase.io
MIT License
1.31k stars 164 forks source link

Vercel deployment fails #691

Open DavidDeSloovere opened 8 months ago

DavidDeSloovere commented 8 months ago

Environment

Deployment to VERCEL


Reproduction

minimal repro as a link is difficult, because it's a deployment issue

Describe the bug

On Vercel, with preview deployments, the origin/host is dynamic (branch names). There a even multiple hosts per deployment (1 for the branch, 1 for the commit, ...).

I notice the beta of authjs v5 doesn't even need a URL/origin in config, only the AUTH_SECRET: https://authjs.dev/getting-started/deployment#environment-variables

Even for nextauthjs v4 you don't have to set NEXTAUTH_URL https://next-auth.js.org/deployment#vercel

However nuxt-auth enforces the requirement of origin / AUTH_ORIGIN when not in production. https://github.com/sidebase/nuxt-auth/blob/aed60695ba543adcf7547f53dbf67f4114257af5/src/runtime/server/services/utils.ts#L11C14-L11C30

I understand that this is for better security. But having to define a (static) origin/url is limiting applications from deploying to Vercel.

Can this enforcement be removed for Vercel? I.e. maybe a setting to have host determined from the request even in production.

Additional context

I have tried many env variables. If I get it running, it is with a fixed origin that is incorrect for most deployments.

Logs

Error: AUTH_NO_ORIGIN: No `origin` - this is an error in production, see https://sidebase.io/nuxt-auth/resources/errors. You can ignore this during development
    at getServerOrigin (file:///var/task/chunks/nitro/vercel.mjs:5275:9)
    at file:///var/task/chunks/nitro/vercel.mjs:5298:5
    at createNitroApp (file:///var/task/chunks/nitro/vercel.mjs:5655:7)
    at file:///var/task/chunks/nitro/vercel.mjs:5663:18
    at ModuleJob.run (node:internal/modules/esm/module_job:218:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:329:24)
    at async importModuleDynamicallyWrapper (node:internal/vm/module:431:15)
    at async c (/opt/node-bridge/bridge-server-BGIDXK2J.js:2:3579)
DavidDeSloovere commented 8 months ago

I have pnpm patched getServerOrigin and this fixes deployments for Vercel. I can send a PR if you want.

  const isVercel = !!process.env.VERCEL;
  if (isVercel) {
    if (event) {
      return getURL(event.node.req, false);
    }

    const envVercelUrl = process.env.VERCEL_URL;
    if (envVercelUrl) {
      return envVercelUrl;
    }
  }
phoenix-ru commented 8 months ago

Thank you for your report. We are still internally discussing the security practices we want to enforce, especially in the light of #673 which will bring some compromises.

I would definitely not rely on using origin detection in production, but you brought a very valid point - preview branches. I believe we already solved such a usecase internally. Either I or @zoey-kaiser will check and come back to you.

DavidDeSloovere commented 8 months ago

I forgot this in my first comment: Thanks for all your hard work in making to library!

Looking forward to #673 😉

I don't know that much about (next-)authjs, but I think there are some change in v5 around this.

Also see the note at: https://authjs.dev/reference/core#trusthost

image

zoey-kaiser commented 8 months ago

I don't know that much about (next-)authjs, but I think there are some change in v5 around this.

Indeed, this trustHost value is also what has been blocking us for the longest time from migrating. (See comments here)

Thanks for pointing it out again!

zoey-kaiser commented 8 months ago

I would definitely not rely on using origin detection in production, but you brought a very valid point - preview branches. I believe we already solved such a usecase internally. Either I or @zoey-kaiser will check and come back to you.

I agree with @phoenix-ru here. One of my goals for the authjs migration is to focus on better-supporting platforms like Vercel. In reflection, I think this is something we neglected with the first version with NextAuth, primarily because we did not forsee us using Vercel when we developed this internally.

Therefore, I have begun outlining that I want to do the following for Vercel, Netlify, and NuxtHub (at minimum):

@DavidDeSloovere, as you already patched the issue locally with pnpm, ill transparently ask you, if this is a satisfactory solution for you during our transition. I am sure that we (or you) could integrate your logic into the current provider with more checks to verify that the extra code only runs on Vercel. I would be fine adding it as long as we are sure that Vercel is being used (a bit like with trustHost in authjs).

@phoenix-ru would you agree with this too?

// Edit I also wanted to thank you for being super cooperative and for providing a great outline of your investigation! It makes resolving these "trickier" deployment issues so much nicer 😄

DavidDeSloovere commented 8 months ago

I'm okay with my local patch, no worries.

phoenix-ru commented 8 months ago

@zoey-kaiser I would refrain from merging cloud provider-specific code at the current stage, before we have a solid concept around trustHost