-
-
Notifications
You must be signed in to change notification settings - Fork 263
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] Support magic links in SignInPage
#4085
base: master
Are you sure you want to change the base?
Conversation
nodemailer
provider to SignInPage
SignInPage
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.
Cool, seems to work well, the demos look clean!
I wrote down some thoughts and ideas.
|
||
Find details on how to set up each provider in the [Auth.js documentation](https://authjs.dev/getting-started/authentication/oauth). | ||
::: | ||
|
||
## Magic Link |
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.
Should there be a default success alert that would already show in this demo?
|
||
{{"demo": "MagicLinkAlertSignInPage.js", "iframe": true, "height": 500}} | ||
|
||
To get the magic link working, you need to add the following code to your custom sign-in page: |
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.
I guess they don't really "need" to but it's a way that users can do it, right? So maybe we can say they "can" add that code instead, as this is an example of how they can do it with Auth.js?
if (provider.id === 'nodemailer' && | ||
(error as any).digest?.includes('verify-request')) { | ||
return { | ||
success: 'Check your email for a verification link.', |
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.
Would a more specific name like successAlert
or successMessage
be better?
// For the nodemailer provider, we want to return a success message | ||
// if the error is a 'verify-request' error, instead of redirecting | ||
// to a `verify-request` page | ||
if (provider.id === 'nodemailer' && (error as any).digest?.includes('verify-request')) { |
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.
I guess they don't provide types that we can use to type this error with the digest
?
redirectTo: callbackUrl ?? '/', | ||
}); | ||
} catch (error) { | ||
if (error instanceof Error && error.message === 'NEXT_REDIRECT') { |
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.
Not super clean that this is how "success" works (by catching an error) but if that's just how Auth.js does it I guess there's nothing we can do?
Ideally this code would be easier to follow.
Docs preview: https://deploy-preview-4085--mui-toolpad-docs.netlify.app/toolpad/core/react-sign-in-page/#magic-link