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

[feat] Support magic links in SignInPage #4085

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

@bharatkashyap bharatkashyap added the enhancement This is not a bug, nor a new feature label Sep 13, 2024
@bharatkashyap bharatkashyap changed the title [feat] Add nodemailer provider to SignInPage [feat] Support magic links in SignInPage Sep 13, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 15, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 17, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 22, 2024
@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Sep 30, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 10, 2024
Copy link
Member

@apedroferreira apedroferreira left a 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
Copy link
Member

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

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.',
Copy link
Member

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')) {
Copy link
Member

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') {
Copy link
Member

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.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is not a bug, nor a new feature PR: out-of-date The pull request has merge conflicts and can't be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[auth] Add magic link/one-time-password support to the credentials provider
2 participants