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

Vercel deployment fails #691

Open
DavidDeSloovere opened this issue Feb 29, 2024 · 7 comments
Open

Vercel deployment fails #691

DavidDeSloovere opened this issue Feb 29, 2024 · 7 comments
Labels
bug A bug that needs to be resolved help-needed Action needed: The help of the community would be appreciated provider-authjs An issue with the authjs provider

Comments

@DavidDeSloovere
Copy link
Contributor

DavidDeSloovere commented Feb 29, 2024

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 DavidDeSloovere added bug A bug that needs to be resolved pending An issue waiting for triage labels Feb 29, 2024
@DavidDeSloovere
Copy link
Contributor Author

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
Copy link
Collaborator

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Member

zoey-kaiser commented Mar 2, 2024

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):

  • Test it properly ourselves
  • Write dedicated docs on how to use it
  • Maybe also add examples / find a way to add it to our CI (@phoenix-ru) to ensure it does not break!

@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 😄

@zoey-kaiser zoey-kaiser added help-needed Action needed: The help of the community would be appreciated provider-authjs An issue with the authjs provider and removed pending An issue waiting for triage labels Mar 2, 2024
@DavidDeSloovere
Copy link
Contributor Author

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

@phoenix-ru
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug that needs to be resolved help-needed Action needed: The help of the community would be appreciated provider-authjs An issue with the authjs provider
Projects
None yet
Development

No branches or pull requests

3 participants