-
Notifications
You must be signed in to change notification settings - Fork 30
feat(auth): add email verification using OTP authentication #566
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
base: main
Are you sure you want to change the base?
Changes from all commits
3de7bda
2d9f3c1
363fcb8
d1d4daa
c28a760
93f5199
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ import { OAuth2Client } from "google-auth-library"; | |
| import User from "../models/user.model.js"; | ||
| import { generateTokenAndSetCookie, catchAsync } from "../lib/utils.js"; | ||
| import cloudinary from "../lib/cloudinary.js"; | ||
| import transporter from "../utils/nodemailer.js" | ||
|
|
||
| let googleClient; | ||
|
|
||
|
|
@@ -247,4 +248,95 @@ export const subscribeToPush = catchAsync(async (req, res) => { | |
| }).select("-password -__v"); | ||
|
|
||
| res.status(200).json({ message: "Push subscription saved" }); | ||
| }); | ||
| } catch (err) { | ||
| console.error("subscribeToPush:", err.message); | ||
| res.status(500).json({ message: "Could not save push subscription" }); | ||
| } | ||
| } | ||
|
|
||
| export const emailVerification = async (req, res) => { | ||
| try { | ||
| const user = await User.findById(req.userId); | ||
|
|
||
| if (!user) { | ||
| return res.status(404).json({ | ||
| message: "User not found", | ||
| }); | ||
| } | ||
|
|
||
| if (user.isVerified) { | ||
| return res.status(400).json({ | ||
| message: "Email already verified", | ||
| }); | ||
| } | ||
|
|
||
| const otp = Math.floor( | ||
| 100000 + Math.random() * 900000 | ||
| ).toString(); | ||
|
|
||
| user.otp = otp; | ||
| user.otpExpires = Date.now() + 5 * 60 * 1000; | ||
|
|
||
| await user.save(); | ||
|
|
||
| await transporter.sendMail({ | ||
| from: process.env.EMAIL, | ||
| to: user.email, | ||
| subject: "Email Verification OTP", | ||
| html: `<h2>Email Verification</h2> | ||
| <p>Your OTP is: <b>${otp}</b></p>`, | ||
| }); | ||
|
|
||
| res.status(200).json({ | ||
| message: "OTP sent successfully", | ||
| }); | ||
|
|
||
| } catch (error) { | ||
| console.log(error); | ||
| res.status(500).json({ | ||
| message: "Failed to send OTP", | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
| export const verifyOtp = async (req, res) => { | ||
| try { | ||
| const { otp } = req.body; | ||
|
|
||
| const user = await User.findById(req.userId); | ||
|
|
||
| if (!user) { | ||
| return res.status(404).json({ | ||
| message: "User not found", | ||
| }); | ||
| } | ||
|
|
||
| if (user.otp !== otp) { | ||
| return res.status(400).json({ | ||
| message: "Invalid OTP", | ||
| }); | ||
| } | ||
|
|
||
| if (user.otpExpires < Date.now()) { | ||
| return res.status(400).json({ | ||
| message: "OTP expired", | ||
| }); | ||
| } | ||
|
Comment on lines
+330
to
+352
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Current checks allow a bypass when OTP is missing/unset (e.g., omitted body OTP + unset stored OTP/expiry). Add strict validation for input format and stored OTP state before any compare. Proposed fix export const verifyOtp = async (req, res) => {
try {
- const { otp } = req.body;
+ const otp = String(req.body?.otp ?? "").trim();
+ if (!/^\d{6}$/.test(otp)) {
+ return res.status(400).json({ message: "Please provide a valid 6-digit OTP" });
+ }
const user = await User.findById(req.userId);
if (!user) {
return res.status(404).json({
message: "User not found",
});
}
+
+ if (!user.otp || !user.otpExpires) {
+ return res.status(400).json({ message: "No active OTP. Please request a new OTP." });
+ }
+
+ if (user.otpExpires < Date.now()) {
+ return res.status(400).json({ message: "OTP expired" });
+ }
if (user.otp !== otp) {
return res.status(400).json({
message: "Invalid OTP",
});
}
-
- if (user.otpExpires < Date.now()) {
- return res.status(400).json({
- message: "OTP expired",
- });
- }🤖 Prompt for AI Agents |
||
|
|
||
| user.isVerified = true; | ||
| user.otp = null; | ||
| user.otpExpires = null; | ||
|
|
||
| await user.save(); | ||
|
|
||
| res.status(200).json({ | ||
| message: "Email verified successfully", | ||
| }); | ||
|
|
||
| } catch (error) { | ||
| res.status(500).json({ | ||
| message: "Verification failed", | ||
| }); | ||
| } | ||
| }; | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,25 +33,16 @@ const userSchema = new mongoose.Schema({ | |
| type: mongoose.Schema.Types.Mixed, | ||
| default: null, | ||
| }, | ||
| isVerified: { | ||
| type: Boolean, | ||
| default: false | ||
| }, | ||
| lastSeen: { | ||
| type: Date, | ||
| default: Date.now, | ||
| }, | ||
| statusMood: { | ||
| type: String, | ||
| enum: [ | ||
| "coding", | ||
| "coffee_break", | ||
| "studying", | ||
| "gaming", | ||
| "working", | ||
| "sleeping", | ||
| "music", | ||
| "away", | ||
| ], | ||
| default: null, | ||
| trim: true, | ||
| }, | ||
| otp: String, | ||
| otpExpires: Date | ||
|
Comment on lines
+44
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OTP fields are modeled as normal readable fields, which leaks verification secrets. Because these fields are selectable by default, responses like Suggested direction- otp: String,
- otpExpires: Date
+ otpHash: {
+ type: String,
+ default: null,
+ select: false
+ },
+ otpExpires: {
+ type: Date,
+ default: null,
+ select: false
+ }🤖 Prompt for AI Agents |
||
| }, { timestamps: true }); | ||
|
|
||
| userSchema.index({ name: "text" }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| import nodemailer from 'nodemailer' | ||
|
|
||
| const transporter = nodemailer.createTransport({ | ||
| service: 'gmail', | ||
| auth: { | ||
| user: process.env.EMAIL, | ||
| pass: process.env.EMAIL_PASSWORD | ||
| } | ||
| }); | ||
|
|
||
| export default transporter |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,47 @@ | ||||||||||||||||||||||||||
| import { useState } from "react"; | ||||||||||||||||||||||||||
| import axios from "axios"; | ||||||||||||||||||||||||||
| import toast from "react-hot-toast"; | ||||||||||||||||||||||||||
| import { useNavigate } from "react-router-dom"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function SendVerifyEmailOtp() { | ||||||||||||||||||||||||||
| const [loading, setLoading] = useState(false); | ||||||||||||||||||||||||||
| const navigate = useNavigate(); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const handleSendOtp = async () => { | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| setLoading(true); | ||||||||||||||||||||||||||
| const res = await axios.post( | ||||||||||||||||||||||||||
| "http://localhost:5001/api/auth/send-verification-email-otp", | ||||||||||||||||||||||||||
| {}, | ||||||||||||||||||||||||||
| { withCredentials: true } | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
Comment on lines
+13
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoded localhost API origin will break non-local deployments. Line 14 pins requests to 🔧 Suggested fix+const API_BASE = import.meta.env.VITE_API_BASE_URL ?? "";
+
const res = await axios.post(
- "http://localhost:5001/api/auth/send-verification-email-otp",
+ `${API_BASE}/api/auth/send-verification-email-otp`,
{},
{ withCredentials: true }
);Apply the same pattern to 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| toast.success(res.data.message); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| navigate("/verify-email"); | ||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||
| toast.error( | ||||||||||||||||||||||||||
| error.response?.data?.message || | ||||||||||||||||||||||||||
| "Failed to send OTP" | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| } finally { | ||||||||||||||||||||||||||
| setLoading(false); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return ( <div className="min-h-screen flex items-center justify-center"> <div className="bg-base-100 shadow-xl p-8 rounded-xl w-full max-w-md"> <h1 className="text-3xl font-bold text-center mb-6"> | ||||||||||||||||||||||||||
| Verify Your Email </h1> | ||||||||||||||||||||||||||
| <button | ||||||||||||||||||||||||||
| onClick={handleSendOtp} | ||||||||||||||||||||||||||
| disabled={loading} | ||||||||||||||||||||||||||
| className="btn btn-primary w-full" | ||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||
| {loading ? "Sending..." : "Send OTP"} | ||||||||||||||||||||||||||
| </button> | ||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export default SendVerifyEmailOtp; | ||||||||||||||||||||||||||
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.
🧩 Analysis chain
🌐 Web query:
According to official Node.js documentation, is Math.random() cryptographically secure for OTP generation? What API is recommended instead for secure random integer generation?💡 Result:
According to official documentation and industry standards, Math.random is not cryptographically secure and must not be used for security-sensitive operations such as OTP generation [1][2]. It is designed as a pseudo-random number generator that is not intended for cryptographic purposes [1]. For secure random integer generation in Node.js, the
node:cryptomodule provides the recommended API, specificallycrypto.randomInt(min, max)[3][4]. This function generates a cryptographically secure random integer [3][4]. Alternatively, for broader cryptographic requirements,crypto.getRandomValues()(part of the Web Crypto API) is also available and recommended for generating cryptographically strong random values [5][6][7].Citations:
Replace
Math.random()OTP generation withcrypto.randomInt()Math.random()is not cryptographically secure and must not be used for security-sensitive OTPs. Usenode:crypto(e.g.,crypto.randomInt(100000, 1000000)) to generate the OTP.File:
backend/src/controllers/auth.controller.js(lines 301-303)🤖 Prompt for AI Agents