-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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: customizable authorize()
error
#9871
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Ignored Deployments
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9871 +/- ##
==========================================
- Coverage 42.56% 41.68% -0.88%
==========================================
Files 173 158 -15
Lines 27752 25406 -2346
Branches 1194 1040 -154
==========================================
- Hits 11813 10591 -1222
+ Misses 15939 14815 -1124 ☔ View full report in Codecov by Sentry. |
Any updates/plans on when this will be merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So in general LGTM. Curious though, why did you add skip
to a few of the core/index.test.ts
tests?
@ndom91 we will enable it again in my pr #100017 as I remember Balazs got some issues with the tests when he added the restart mock function |
Hi! Any updates/ETA on this? I am testing our migration to beta v5 and want to display custom error messages on signin flow from backend directly on the page without redirection. |
@balazsorban44 this has broken the redirection for me. Instead of My config: {
pages: {
signIn: '/<my-signin-page>',
error: '/<my-signin-page>', // Error code passed in query string as ?error=
},
...
basePath: "/api/auth",
} on build 13 of the beta this works as expected. I don't see any relevant changes in the regexp matching so am curious why this stopped working. |
Because of |
Both with |
@ccyen8358 hmm interesting, so I can't htink of any other way to solve it currently. Other than adding an additional custom error class designed to be extended from for this use-case 🤔 I've put up a quick prototype PR of that here to hopefully get some discussion going there 🙏 @sjoukedv hmm interesting, do you have an |
// path next.config.js
/** @type {import("next").NextConfig} */
module.exports = {
env: {
AUTH_URL: `${process.env.VERCEL_URL || 'http://localhost:3000'}/api/auth`,
NEXTAUTH_URL: `${process.env.VERCEL_URL || 'http://localhost:3000'}/api/auth`,
AUTH_SECRET: `${process.env.AUTH_SECRET || 'somestring'}`,
},
// ... // path auth.ts
export const config = {
// ...
basePath: "/api/auth",
trustHost: true
} satisfies NextAuthConfig
export const { handlers, auth, signIn, signOut } = NextAuth(config) With e.g. |
@sjoukedv hmm okay so we auto detect the basePath based on VERCEL_URL (among other things), so if that's available even in dev too, like you've set it yourself there, then you don't need a lot of this actually. Can you try removing auth_url and nextauth_url env vars and basePath config? |
@ndom91 That redirects me to Looks like the page becomes relative to
If I remove the leading slash from I have also verified the |
I think I'm running into a similar issue here. This is what I currently have for my pages setting pages: {
signIn: `/auth`,
error: `/auth/error`,
}, Both signIn and error are custom pages. When user gets redirected to error the resulting redirect redirects to I believe it would be a good idea to make pages route options an absolute url. |
I've tried adding a custom error but we're using the Is it possible to have it added to the response? I believe it should just be adding the following: const error = new URL(data.url).searchParams.get("error")
+ const code = new URL(data.url).searchParams.get("code")
if (res.ok) {
await __NEXTAUTH._getSession({ event: "storage" })
}
return {
error,
+ code,
status: res.status,
ok: res.ok,
url: error ? null : data.url,
} as any |
@CyanFlare @sjoukedv I think both of these are due to an issue we had with |
next-auth@beta-16 / next.js 14.1.4. A strange error occurs when imorting a classnext-auth@beta-16 / next.js 14.1.4. A strange error occurs when imorting a class Desktop.20240325.-.22471404_compressed.mp4 |
@MrOxMasTer Hmm so it's working "as expected" with |
yes |
With import { CredentialsSignin } from "@auth/core/errors"
class InvalidLoginError extends CredentialsSignin {
code = 'Invalid identifier or password'
}
export const config = {
providers: [
CredentialsProvider({
....
async authorize(credentials) {
throw new InvalidLoginError()
}
})
] |
Look where I'm importing the class from. It was moved to the main library to avoid downloading "@auth/core", but it doesn't work. But I can say that you have made a good point, that at least some implementation of the class works |
@MrOxMasTer Was just trying something with this again in I can't view your video anymore, but it cause anything else weird to happen with |
We implemented the login page based on the intercepting logic so we can not enable the redirection. As redirection will move out of the intercepted modal. I want to stick in the same modal. Is it possible to attach the code when redirection is disabled i also need the above change |
Es necesario poder obtener el codigo para diferenciar del error. |
but the problem I came across implementing your solution was I'm not able to use the package from "@auth/core/errors" import { CredentialsSignin } from "next-auth"; which is also not giving me expected results, thankyou in advance for the answer, I'd appreciate if you could tell me the complete way to do custom error |
This saved me! I was deep in a rabbit hole trying to get a custom error to show to a user if they had logged in with oAuth but then tried logging in with a password and the message had that extra bit on it. Thank you!!! |
@rmartin93 please can you shear a code snippet of your implementation of this |
I don't understand how the {
"url": "http://localhost:3000/login?error=CredentialsSignin&code=InvalidCredentials"
} Is there a way to just get the Edit: |
This introduces a way to throw custom errors in the
authorize()
callback of the Credentials provider.Any generic or sensitive server error will continue to return
error=Configuration
(full error is logged on the server), but for custom ones thrown fromauthorize()
that extendCredentialsSignin
, we will let the error propagate. Example:Then, based on your framework, one of the following will happen:
client-side (
fetch
to the sign-in endpoint):/signin?error=CredentialsSignin&code=custom
redirect: false
was set onsignIn
, it returns the error and code properties.server-side handled form actions:
CustomError
is thrown that you would need to handle in a catch block.Notes:
CredentialsSignin
was chosen to be backwards compatible with NextAuth.js v4next-auth/packages/next-auth/src/core/routes/callback.ts
Line 336 in 2072750
AuthorizedCallbackError
to AccessDenied for the same reasonnext-auth/packages/next-auth/src/core/routes/callback.ts
Line 99 in 2072750
next-auth
will be added in a separate PRRelated: #9099 (see #9099 (comment)). This PR won't close that issue yet, a
next-auth
PR will be opened after this gets merged