-
Notifications
You must be signed in to change notification settings - Fork 63
Adding Google Login using Firebase Authentication #190
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
53ede56
1e6a956
8d0d583
ea13422
dd28dc3
28b1998
06466ad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -27,6 +27,31 @@ router.post("/login", passport.authenticate('local'), (req, res) => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| res.status(200).json( { message: 'Login successful', user: req.user } ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| router.post("/google-login", async (req, res) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { email, name, googleId } = req.body; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let user = await User.findOne({ email }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!user) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user = new User({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| username: name, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| email, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| googleId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await user.save(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log("New Google user saved:", user); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log("Google user found:", user); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return res.json({ message: "Login successful", user }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.error("Google login error:", error); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return res.status(500).json({ message: "Server error" }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+30
to
+53
Contributor
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. Critical security and account management issues need to be addressed. The Google login endpoint has several critical issues:
Apply this diff to fix the security and account management issues: router.post("/google-login", async (req, res) => {
try {
const { email, name, googleId } = req.body;
let user = await User.findOne({ email });
if (!user) {
user = new User({
username: name,
email,
googleId,
});
await user.save();
console.log("New Google user saved:", user);
} else {
- console.log("Google user found:", user);
+ // Link Google account if not already linked
+ if (!user.googleId) {
+ user.googleId = googleId;
+ await user.save();
+ console.log("Google account linked to existing user:", user);
+ } else if (user.googleId !== googleId) {
+ return res.status(400).json({ message: "Email is associated with a different Google account" });
+ } else {
+ console.log("Google user found:", user);
+ }
}
+ // Establish session like the regular login route
+ req.login(user, (err) => {
+ if (err) {
+ return res.status(500).json({ message: "Session creation failed" });
+ }
return res.json({ message: "Login successful", user });
+ });
} catch (error) {
console.error("Google login error:", error);
return res.status(500).json({ message: "Server error" });
}
});📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Logout route | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| router.get("/logout", (req, res) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -40,3 +65,4 @@ router.get("/logout", (req, res) => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| module.exports = router; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,12 +19,15 @@ | |||||||||||||||||||||||||||||||||||||||||
| "@primer/octicons-react": "^19.15.5", | ||||||||||||||||||||||||||||||||||||||||||
| "@vitejs/plugin-react": "^4.3.3", | ||||||||||||||||||||||||||||||||||||||||||
| "axios": "^1.7.7", | ||||||||||||||||||||||||||||||||||||||||||
| "firebase": "^12.1.0", | ||||||||||||||||||||||||||||||||||||||||||
| "framer-motion": "^12.23.12", | ||||||||||||||||||||||||||||||||||||||||||
| "i": "^0.3.7", | ||||||||||||||||||||||||||||||||||||||||||
| "lucide-react": "^0.525.0", | ||||||||||||||||||||||||||||||||||||||||||
| "octokit": "^4.0.2", | ||||||||||||||||||||||||||||||||||||||||||
| "postcss": "^8.4.47", | ||||||||||||||||||||||||||||||||||||||||||
| "react": "^18.3.1", | ||||||||||||||||||||||||||||||||||||||||||
| "react-dom": "^18.3.1", | ||||||||||||||||||||||||||||||||||||||||||
| "react-firebase-hooks": "^5.1.1", | ||||||||||||||||||||||||||||||||||||||||||
| "react-hot-toast": "^2.4.1", | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+22
to
31
Contributor
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. 💡 Verification agent 🧩 Analysis chainVerify and remove potentially unused or unintended dependencies ("i", "react-firebase-hooks") The newly added "i" and "react-firebase-hooks" packages don’t appear in the provided code paths. If unused, they increase install size and attack surface. Run this to confirm usage: If unused, remove them: "dependencies": {
...
- "firebase": "^12.1.0",
+ "firebase": "^12.1.0",
...
- "i": "^0.3.7",
+ // "i" removed if unused
...
- "react-firebase-hooks": "^5.1.1",
+ // "react-firebase-hooks" removed if unused
...
},🏁 Script executed: #!/bin/bash
# Find direct imports/usages of 'i' and 'react-firebase-hooks'
rg -n -S $'from [\'"]i[\'"]|require\\([\'"]i[\'"]\\)' || true
rg -n -S 'react-firebase-hooks' || trueLength of output: 153 Remove unused dependencies “i” and “react-firebase-hooks” • package.json – lines for Suggested diff: "dependencies": {
"firebase": "^12.1.0",
"framer-motion": "^12.23.12",
- "i": "^0.3.7",
- "react-firebase-hooks": "^5.1.1",
"lucide-react": "^0.525.0",
"octokit": "^4.0.2",
…
},📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
| "react-icons": "^5.3.0", | ||||||||||||||||||||||||||||||||||||||||||
| "react-router-dom": "^6.28.0", | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,60 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { signInWithGoogle } from "../hooks/firebase"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import React, { useState } from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import axios from "axios"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useNavigate } from "react-router-dom"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const backendUrl = import.meta.env.VITE_BACKEND_URL; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const GoogleSignIn = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [isLoading, setIsLoading] = useState(false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const navigate = useNavigate(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const handleSignIn = async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setIsLoading(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const userCredential = await signInWithGoogle(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log("Google userCredential:", userCredential); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (userCredential && userCredential.user) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const idToken = await userCredential.user.getIdToken(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { email, displayName, uid } = userCredential.user; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log("Google user info:", { email, displayName, uid }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const response = await axios.post( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `${backendUrl}/api/auth/google-login`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| email, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: displayName, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| googleId: uid, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log("Backend response:", response.data); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (response.data.message === "Login successful") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| navigate("/"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| alert(response.data.message || "Google login failed"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| alert("No user returned from Google sign-in"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.error("Google sign-in error:", error); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| alert("Google sign-in failed"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } finally { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setIsLoading(false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+12
to
+47
Contributor
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. 🛠️ Refactor suggestion Several improvements needed for production readiness. The core authentication flow is functional but has areas for improvement:
Consider these improvements for better UX and security: const handleSignIn = async () => {
setIsLoading(true);
try {
const userCredential = await signInWithGoogle();
console.log("Google userCredential:", userCredential);
if (userCredential && userCredential.user) {
- const idToken = await userCredential.user.getIdToken();
const { email, displayName, uid } = userCredential.user;
console.log("Google user info:", { email, displayName, uid });
const response = await axios.post(
`${backendUrl}/api/auth/google-login`,
{
email,
name: displayName,
googleId: uid,
}
);
console.log("Backend response:", response.data);
if (response.data.message === "Login successful") {
navigate("/");
} else {
- alert(response.data.message || "Google login failed");
+ // TODO: Replace with proper error state or toast notification
+ console.error("Login failed:", response.data.message);
}
} else {
- alert("No user returned from Google sign-in");
+ console.error("No user returned from Google sign-in");
}
} catch (error) {
console.error("Google sign-in error:", error);
- alert("Google sign-in failed");
+ // TODO: Handle specific error types (user cancelled, network error, etc.)
} finally {
setIsLoading(false);
}
};📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <button | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onClick={handleSignIn} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| disabled={isLoading} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| className="w-full bg-gradient-to-r from-purple-600 via-pink-600 to-indigo-600 text-white py-4 px-6 rounded-2xl font-semibold focus:ring-4 focus:ring-purple-500/50 transition-all duration-300 disabled:opacity-50 disabled:cursor-not-allowed transform hover:scale-105 shadow-lg" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {isLoading ? "Signing in..." : "Sign in with Google"} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </button> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export default GoogleSignIn; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,23 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||
| import { initializeApp } from 'firebase/app'; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import { getAuth, GoogleAuthProvider, signInWithPopup, setPersistence, browserLocalPersistence } from 'firebase/auth'; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const firebaseConfig = { | ||||||||||||||||||||||||||||||||||||||||||||||||
| apiKey: import.meta.env.VITE_FIREBASE_API_KEY, | ||||||||||||||||||||||||||||||||||||||||||||||||
| authDomain: import.meta.env.VITE_FIREBASE_AUTH_DOMAIN, | ||||||||||||||||||||||||||||||||||||||||||||||||
| projectId: import.meta.env.VITE_FIREBASE_PROJECT_ID, | ||||||||||||||||||||||||||||||||||||||||||||||||
| storageBucket: import.meta.env.VITE_FIREBASE_STORAGE_BUCKET, | ||||||||||||||||||||||||||||||||||||||||||||||||
| messagingSenderId: import.meta.env.VITE_FIREBASE_MESSAGING_SENDER_ID, | ||||||||||||||||||||||||||||||||||||||||||||||||
| appId: import.meta.env.VITE_FIREBASE_APP_ID | ||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+4
to
+11
Contributor
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. 🛠️ Refactor suggestion Fail fast if required Firebase env vars are missing InitializeApp will throw opaque errors later if any of these are undefined. Guard early with a clear message. Apply this check: const firebaseConfig = {
apiKey: import.meta.env.VITE_FIREBASE_API_KEY,
authDomain: import.meta.env.VITE_FIREBASE_AUTH_DOMAIN,
projectId: import.meta.env.VITE_FIREBASE_PROJECT_ID,
storageBucket: import.meta.env.VITE_FIREBASE_STORAGE_BUCKET,
messagingSenderId: import.meta.env.VITE_FIREBASE_MESSAGING_SENDER_ID,
appId: import.meta.env.VITE_FIREBASE_APP_ID
};
+
+const missing = Object.entries(firebaseConfig)
+ .filter(([, v]) => !v)
+ .map(([k]) => k);
+if (missing.length) {
+ throw new Error(`Missing Firebase env vars: ${missing.join(", ")}`);
+}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const app = initializeApp(firebaseConfig); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const auth = getAuth(app); | ||||||||||||||||||||||||||||||||||||||||||||||||
| const googleAuthProvider = new GoogleAuthProvider(); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const signInWithGoogle = async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||
| await setPersistence(auth, browserLocalPersistence); | ||||||||||||||||||||||||||||||||||||||||||||||||
| return await signInWithPopup(auth, googleAuthProvider); | ||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+18
to
+21
Contributor
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. 💡 Verification agent 🧩 Analysis chainCritical: Send and verify Firebase ID token on the backend; don’t trust client-supplied UID/email The current flow returns a UserCredential to the caller, which then posts uid/email to your backend (per PR description). Without verifying an ID token server-side, an attacker can spoof uid/email. The backend must verify a Firebase ID token using the Admin SDK (or Google’s token verification) before creating/authenticating a user. Refactor to return the ID token alongside the credential so the frontend can send it to the backend: -const signInWithGoogle = async () => {
- await setPersistence(auth, browserLocalPersistence);
- return await signInWithPopup(auth, googleAuthProvider);
-};
+const signInWithGoogle = async () => {
+ await setPersistence(auth, browserLocalPersistence);
+ const cred = await signInWithPopup(auth, googleAuthProvider);
+ const idToken = await cred.user.getIdToken();
+ return { cred, idToken };
+};Then, POST the idToken to the backend (e.g., in GoogleSignIn.tsx) and verify it there with Firebase Admin before trusting any user claims. Optionally add a popup fallback for Safari/mobile: -import { getAuth, GoogleAuthProvider, signInWithPopup, setPersistence, browserLocalPersistence } from 'firebase/auth';
+import { getAuth, GoogleAuthProvider, signInWithPopup, signInWithRedirect, setPersistence, browserLocalPersistence } from 'firebase/auth';
...
- const cred = await signInWithPopup(auth, googleAuthProvider);
+ let cred;
+ try {
+ cred = await signInWithPopup(auth, googleAuthProvider);
+ } catch (e: any) {
+ if (e?.code === 'auth/popup-blocked' || e?.code === 'auth/cancelled-popup-request') {
+ await signInWithRedirect(auth, googleAuthProvider);
+ return { cred: undefined, idToken: undefined }; // handle redirect result in caller if you add it
+ }
+ throw e;
+ }Confirm the backend’s /google-login endpoint verifies a Firebase ID token (not raw uid/email) using the Firebase Admin SDK before creating/authenticating users. If not, this is a blocker to merge. 🏁 Script executed: #!/bin/bash
# Search for the /google-login handler in the server code
rg -n "google-login"
# Search for Firebase Admin SDK token verification calls
rg -n "verifyIdToken"
rg -n "auth().verifyIdToken"
# Search for any use of firebase-admin in server dependencies
grep -R "firebase-admin" -n .Length of output: 264 🏁 Script executed: #!/bin/bash
# Display the /google-login handler implementation to verify ID token verification
sed -n '1,200p' backend/routes/auth.jsLength of output: 1851 Critical: Implement Firebase ID Token Verification in The backend currently trusts client-supplied • Frontend (src/hooks/firebase.ts)
• Backend (backend/routes/auth.js)
Please implement these changes before merging. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| export { auth, signInWithGoogle }; | ||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,6 +3,9 @@ import axios from "axios"; | |||||||||||||||||||||||||||||||||||||||||||||||
| import { useNavigate, Link } from "react-router-dom"; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import { ThemeContext } from "../../context/ThemeContext"; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import type { ThemeContextType } from "../../context/ThemeContext"; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import GoogleSignIn from "../../components/GoogleSignIn"; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| const backendUrl = import.meta.env.VITE_BACKEND_URL; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -122,12 +125,23 @@ const Login: React.FC = () => { | |||||||||||||||||||||||||||||||||||||||||||||||
| <button | ||||||||||||||||||||||||||||||||||||||||||||||||
| type="submit" | ||||||||||||||||||||||||||||||||||||||||||||||||
| disabled={isLoading} | ||||||||||||||||||||||||||||||||||||||||||||||||
| className="w-full bg-gradient-to-r from-purple-600 via-pink-600 to-indigo-600 text-white py-4 px-6 rounded-2xl font-semibold focus:ring-4 focus:ring-purple-500/50 transition-all duration-300 disabled:opacity-50 disabled:cursor-not-allowed" | ||||||||||||||||||||||||||||||||||||||||||||||||
| className="w-full bg-gradient-to-r from-purple-600 via-pink-600 to-indigo-600 text-white py-4 px-6 rounded-2xl font-semibold focus:ring-4 focus:ring-purple-500/50 transition-all duration-300 disabled:opacity-50 disabled:cursor-not-allowed transform hover:scale-105 shadow-lg" | ||||||||||||||||||||||||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||||||||||||||||||||||||
| {isLoading ? "Signing in..." : "Sign In"} | ||||||||||||||||||||||||||||||||||||||||||||||||
| </button> | ||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="flex items-center my-4"> | ||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="flex-grow h-px bg-gray-300" /> | ||||||||||||||||||||||||||||||||||||||||||||||||
| <span className="mx-4 text-gray-500 font-semibold">or</span> | ||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="flex-grow h-px bg-gray-300" /> | ||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| {/* Google Sign-In Button */} | ||||||||||||||||||||||||||||||||||||||||||||||||
| <div> | ||||||||||||||||||||||||||||||||||||||||||||||||
| <GoogleSignIn /> | ||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+132
to
+141
Contributor
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. 💡 Verification agent 🧩 Analysis chainUnify post-login navigation and error handling across password and Google flows Password login navigates to "/home" and surfaces messages in-form; GoogleSignIn (per summary) navigates to "/" and uses alerts. For consistency and better UX, pass callbacks so GoogleSignIn uses the same messaging and destination. Apply: - <div>
- <GoogleSignIn />
- </div>
+ <div>
+ <GoogleSignIn
+ onSuccess={() => navigate("/home")}
+ onError={(msg: string) => setMessage(msg || "Something went wrong")}
+ />
+ </div>Follow-up: update GoogleSignIn to accept optional onSuccess/onError props and to surface backend responses via these instead of alert(). Ensure CORS is configured on the backend to allow the frontend origin (e.g., http://localhost:5173) for the /api/auth/google-login endpoint. 🏁 Script executed: #!/bin/bash
# Find all occurrences of GoogleSignIn in the codebase
rg "GoogleSignIn" -n src
# Locate the import in Login.tsx
rg "import .*GoogleSignIn" -n src/pages/Login/Login.tsx
# Search for the component definition
rg -n "export (default )?function GoogleSignIn" -n srcLength of output: 624 Unify post-login navigation and error handling across password and Google flows
📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
| </form> | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| {/* Message */} | ||||||||||||||||||||||||||||||||||||||||||||||||
| {message && ( | ||||||||||||||||||||||||||||||||||||||||||||||||
| <div className={`mt-6 p-4 rounded-2xl text-center text-sm font-medium ${ | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,4 +4,8 @@ import react from '@vitejs/plugin-react' | |
| // https://vite.dev/config/ | ||
| export default defineConfig({ | ||
| plugins: [react()], | ||
| server:{ | ||
| host:'0.0.0.0', | ||
| port:5173 | ||
| } | ||
|
Comment on lines
+7
to
+10
Contributor
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. 💡 Verification agent ❓ Verification inconclusiveParameterize dev server host/port and avoid binding to all interfaces by default Binding to 0.0.0.0 exposes your dev server to the LAN. Prefer localhost by default and allow overrides via env for multi-device testing. Also avoids duplicating host config with the "vite --host" script. Apply this minimal change to make host/port configurable: server:{
- host:'0.0.0.0',
- port:5173
+ host: process.env.VITE_DEV_HOST ?? 'localhost',
+ port: Number(process.env.VITE_DEV_PORT ?? 5173)
}Ensure Firebase Authentication’s Authorized domains include the dev URL you’ll actually visit in the browser (e.g., http://localhost:5173). If you plan to use a LAN IP, add that domain too. Parameterize Vite Dev Server Host/Port // vite.config.ts
server: {
- host: '0.0.0.0',
- port: 5173
+ host: process.env.VITE_DEV_HOST ?? 'localhost',
+ port: Number(process.env.VITE_DEV_PORT ?? 5173),
}• This lets you override via VITE_DEV_HOST/VITE_DEV_PORT when you need LAN access, without duplicating the 🤖 Prompt for AI Agents |
||
| }) | ||
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.
Potential runtime errors in password hashing for Google users.
Making the password field optional could cause issues in the pre-save hook (lines 25-37) and
comparePasswordmethod (lines 40-41), as these assume a password exists and attempt to hash/compare undefined values.Apply this diff to handle optional passwords properly:
UserSchema.pre('save', async function (next) { - if (!this.isModified('password')) + if (!this.isModified('password') || !this.password) return next(); try { const salt = await bcrypt.genSalt(10); this.password = await bcrypt.hash(this.password, salt); next(); } catch (err) { return next(err); } }); // Compare passwords during login UserSchema.methods.comparePassword = async function (enteredPassword) { + if (!this.password) { + return false; + } return await bcrypt.compare(enteredPassword, this.password); };📝 Committable suggestion
🤖 Prompt for AI Agents