Adding PasswordLess login to sign in options #785
Conversation
|
@sofeel is attempting to deploy a commit to the Codú Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe project has been updated to include passwordless authentication via email, enhancing user experience by simplifying the login process. This involves adding a new email input field in the UI, configuring an email provider in the authentication server, and setting up an AWS SNS-based email server for delivery. Additionally, minor UI adjustments and the exportation of constants for theme toggling have been made to improve usability and customization. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 6
Configuration used: CodeRabbit UI
Files selected for processing (7)
- app/(app)/auth/error/page.tsx (1 hunks)
- app/(app)/auth/page.tsx (1 hunks)
- app/(app)/get-started/_client.tsx (2 hunks)
- components/Theme/ThemeToggle/ThemeToggle.tsx (1 hunks)
- sample.env (1 hunks)
- server/auth.ts (2 hunks)
- utils/createPasswordLessEmailTemplate.ts (1 hunks)
Additional comments: 6
components/Theme/ThemeToggle/ThemeToggle.tsx (1)
- 7-7: Exporting
THEME_MODESas a constant is a good practice for ensuring consistency and reusability of theme mode values across the application. Well done on implementing this change.app/(app)/auth/page.tsx (1)
- 10-58: The implementation of the
PostAuthPagecomponent effectively utilizes theuseThemehook and theTHEME_MODESconstant for theme management. The conditional rendering of the Codú logo based on the current theme is a nice touch. Good job on these changes.utils/createPasswordLessEmailTemplate.ts (4)
- 1-1: The function
createPasswordLessEmailTemplateis well-named, clearly indicating its purpose. However, it's important to ensure that the URL parameter is always validated before being passed to this function to prevent injection attacks.- 409-486: The conditional comments and the alternative rendering for Microsoft Outlook (
<!--[if mso]>) and other clients (<!--[if !mso]><!-- -->) are correctly implemented. However, ensure thorough testing across different email clients to verify that the button renders as expected. Email client rendering can be notoriously inconsistent, and what works in one client may not work in another.- 1-683: The template makes good use of semantic HTML and accessibility considerations, such as alt text for images (not shown but assumed to be part of the template's images if any are used). However, ensure that any dynamic content, such as URLs or user-generated content, is properly sanitized to prevent XSS attacks. This is particularly important for email templates, which can be a vector for phishing attacks.
- 1-683: The responsive design considerations using media queries (
@media only screen and (max-width: 600px)) are a good practice for ensuring the email looks good on mobile devices. However, testing on actual devices and using email testing services like Litmus or Email on Acid can help catch any unforeseen issues.
8e6fc49d-d378-4905-b5b1-ec4755d7d9cb.mp4New user signing up with only email |
7d4b4c68-cc8a-43a5-bfdb-84e7199e50a2.mp4Existing user signing in with passwordless |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
| @@ -4,7 +4,7 @@ import { MoonIcon, SunIcon } from "@heroicons/react/20/solid"; | |||
| import { useTheme } from "next-themes"; | |||
| import { useEffect, useState } from "react"; | |||
|
|
|||
| const THEME_MODES = { | |||
| export const THEME_MODES = { | |||
There was a problem hiding this comment.
Needed this exported so I could use it to determine the coco log that should be displayed. ie black or white
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (4)
- app/(app)/auth/error/page.tsx (1 hunks)
- app/(app)/auth/page.tsx (1 hunks)
- app/(app)/get-started/_client.tsx (2 hunks)
- server/auth.ts (2 hunks)
Files skipped from review as they are similar to previous changes (4)
- app/(app)/auth/error/page.tsx
- app/(app)/auth/page.tsx
- app/(app)/get-started/_client.tsx
- server/auth.ts
app/(app)/get-started/_client.tsx
Outdated
|
|
||
| useEffect(() => { | ||
| // toLowerCase can maybe be removed. Just want to make sure its not TRUE in Prod | ||
| setIsAlpha(process.env.ALPHA?.toLowerCase() === "true"); |
There was a problem hiding this comment.
Not sure if this is the correct way to go about alpha stuff. Further guidance needed
There was a problem hiding this comment.
I'll take a look later today and make a little guide so you can see how I was handling it.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- app/(app)/get-started/_client.tsx (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/(app)/get-started/_client.tsx
Made that change. Let me know if anything else is needed. You will have to set up the email creds to get this to work also. |
|
I haven't forgot about this, just wanted to see if there was an alternative to all the SMTP credentials. What about using key/secret key with AWS? Does that work? Adapted from the nodmailer docs: |
I had tried this initially but ill try again tonight to be sure. |
…ed README to explain how to set up passwordless
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (4)
- README.md (1 hunks)
- app/(app)/get-started/_client.tsx (2 hunks)
- server/auth.ts (2 hunks)
- utils/sendEmail.ts (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- app/(app)/get-started/_client.tsx
- server/auth.ts
Additional comments: 1
utils/sendEmail.ts (1)
- 21-21: The export of
nodemailerSesTransporteris correctly implemented, ensuring that the SES transporter is available for use elsewhere in the application. This change aligns with the PR's objective of enhancing email delivery capabilities.
| @@ -36,7 +36,7 @@ const sendEmail = async (config: MailConfig) => { | |||
| const to = emailSchema.parse(recipient); | |||
| // send some mail | |||
| return new Promise((resolve, reject) => { | |||
| transporter.sendMail( | |||
| nodemailerSesTransporter.sendMail( | |||
There was a problem hiding this comment.
The sendEmail function utilizes the nodemailerSesTransporter to send emails. This function is well-structured and includes error handling and logging. However, consider abstracting the logging mechanism to a separate utility for better maintainability and to facilitate centralized logging in the future.
- console.log("Error sending mail:", err);
+ // Consider using a centralized logging utility here
- console.log(info.envelope);
- console.log(info.messageId);
+ // Consider using a centralized logging utility hereCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| nodemailerSesTransporter.sendMail( | |
| nodemailerSesTransporter.sendMail( |
There was a problem hiding this comment.
Create an issue for this comment and resolve the comment
There was a problem hiding this comment.
@JohnAllenTech, I attempted to create the GitHub issue as requested, but encountered an issue with the GitHub CLI not recognizing @coderabbitai[bot] as a valid user for assignment. This is likely because bots cannot be directly assigned to issues in the same way that human users can.
However, the issue regarding abstracting the logging mechanism in the sendEmail function has been acknowledged. For tracking and implementation purposes, it would be beneficial for a member of the project team to manually create this issue on GitHub, ensuring it includes the context of the PR and the specific comment for clarity.
If there's anything more I can assist with or any other way you'd like to proceed, please let me know!
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Create a GitHub issue for abstracting the logging mechanism in the sendEmail function
gh issue create --title "Abstract logging mechanism in sendEmail function" --body "The `sendEmail` function in `utils/sendEmail.ts` currently uses direct calls to `console.log` for logging errors and information. It's suggested to abstract these logging calls to a separate utility for better maintainability and to facilitate centralized logging in the future.
This improvement was suggested in the context of PR https://github.com/codu-code/codu/pull/785, specifically in this comment: https://github.com/codu-code/codu/pull/785#discussion_r1516922759
@JohnAllenTech requested the creation of this issue to track the suggested improvement." --assignee @coderabbitai[bot]
Length of output: 1013
| ### Setting up Passwordless auth locally | ||
|
|
||
| In order to use Passwordless login locally you need to have a `ACCESS_KEY` and `SECRET_KEY` value. | ||
|
|
||
| Niall has written a [tutorial](https://www.codu.co/articles/sending-emails-with-aws-ses-and-nodemailer-in-node-js-xfuucrri) on how to send emails with AWS SES and shows how to get these values. | ||
|
|
||
| Check out the example .env file [here](./sample.env) to see how to populate these values | ||
|
|
||
| **Note: Currenly the AWS region of the SNS service is hardcoded to "eu-west-1" it may be necessary to change this if your SNS service is in a different region** |
There was a problem hiding this comment.
The instructions for setting up Passwordless authentication locally are clear and helpful. Including a tutorial link for obtaining ACCESS_KEY and SECRET_KEY values is a thoughtful addition. However, the note about the hardcoded AWS region ("eu-west-1") is crucial. It's recommended to make this configuration more flexible by allowing it to be set via environment variables, enhancing the application's adaptability to different deployment environments.
- **Note: Currently the AWS region of the SNS service is hardcoded to "eu-west-1" it may be necessary to change this if your SNS service is in a different region**
+ **Note: Ensure to set the AWS region in your environment variables to match the region of your SNS service.**Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| ### Setting up Passwordless auth locally | |
| In order to use Passwordless login locally you need to have a `ACCESS_KEY` and `SECRET_KEY` value. | |
| Niall has written a [tutorial](https://www.codu.co/articles/sending-emails-with-aws-ses-and-nodemailer-in-node-js-xfuucrri) on how to send emails with AWS SES and shows how to get these values. | |
| Check out the example .env file [here](./sample.env) to see how to populate these values | |
| **Note: Currenly the AWS region of the SNS service is hardcoded to "eu-west-1" it may be necessary to change this if your SNS service is in a different region** | |
| ### Setting up Passwordless auth locally | |
| In order to use Passwordless login locally you need to have a `ACCESS_KEY` and `SECRET_KEY` value. | |
| Niall has written a [tutorial](https://www.codu.co/articles/sending-emails-with-aws-ses-and-nodemailer-in-node-js-xfuucrri) on how to send emails with AWS SES and shows how to get these values. | |
| Check out the example .env file [here](./sample.env) to see how to populate these values | |
| **Note: Ensure to set the AWS region in your environment variables to match the region of your SNS service.** |
There was a problem hiding this comment.
No because we dont set the AWS region in env variables. Dont want to add it as an env var as that would make this PR a breaking change
There was a problem hiding this comment.
@JohnAllenTech, understood. To ensure clarity for future contributors and users, it might be beneficial to document the hardcoded AWS region limitation in the README or relevant documentation. This can serve as a temporary measure until a more flexible solution is implemented. Appreciate your attention to maintaining the stability of the PR.
|
@NiallJoeMaher should be good to go. No longer using the email creds. Now piggy backing on the current sendEmail function |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
server/auth.ts
Outdated
| EmailProvider({ | ||
| server: {}, | ||
| sendVerificationRequest: sendPasswordLessEmail, | ||
| from: process.env.EMAIL_FROM, |
There was a problem hiding this comment.
Is this a new variable? Could we use ADMIN_EMAIL. I think this is the very last thing (I kinda promise 😂).
There was a problem hiding this comment.
Ah good point. Lemme change that
There was a problem hiding this comment.
Actually even better I think its not needed. Will confirm
There was a problem hiding this comment.
@NiallJoeMaher sorted in #19379a8
Best tasting taco so far alright |
✨ Codu Pull Request 💻
Pull Request details
Any Breaking changes
Associated Screenshots
Sign in page dark mode
Sign in page light mode
Email callback page dark mode
Email callback page light mode
PasswordLess email
Email expired or already used page
Summary by CodeRabbit
sample.envfor passwordless email delivery.