Adding Google Login using Firebase Authentication #190
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughAdded Google Sign-In using Firebase on the frontend and a new backend endpoint to handle Google-based login/signup. Updated the user model to support googleId and optional password, integrated a GoogleSignIn component into Login/Signup pages, added Firebase config/hook, adjusted dev server config, and updated dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Frontend (GoogleSignIn)
participant Firebase as Firebase Auth
participant API as Backend /api/auth/google-login
participant DB as DB (Users)
User->>UI: Click "Sign in with Google"
UI->>Firebase: signInWithPopup(Google)
Firebase-->>UI: UserCredential (email, displayName, uid)
UI->>API: POST /google-login { email, name, googleId }
API->>DB: Find user by email
alt User exists
DB-->>API: Existing user
API-->>UI: { message: "Login successful", user }
else User not found
API->>DB: Create user with email, username=name, googleId
DB-->>API: New user
API-->>UI: { message: "Login successful", user }
end
UI-->>User: Navigate to home
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
src/hooks/firebase.ts (1)
15-17: Ensure account selection prompt for better UXTrigger Google account chooser on every login to avoid sticky sessions picking a prior account.
Apply:
const auth = getAuth(app); const googleAuthProvider = new GoogleAuthProvider(); +googleAuthProvider.setCustomParameters({ prompt: 'select_account' });src/App.tsx (1)
1-1: Remove leading blank line to satisfy common lintersNo functional change, but many configs flag leading blank lines at file start.
- import Navbar from "./components/Navbar";src/pages/Signup/Signup.tsx (1)
158-158: Minor: Remove unnecessary empty line.The extra blank line after the message div is unnecessary and inconsistent with the surrounding code formatting.
-
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
backend/models/User.js(1 hunks)backend/routes/auth.js(2 hunks)package.json(1 hunks)src/App.tsx(1 hunks)src/components/GoogleSignIn.tsx(1 hunks)src/hooks/firebase.ts(1 hunks)src/pages/Login/Login.tsx(2 hunks)src/pages/Signup/Signup.tsx(3 hunks)vite.config.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/components/GoogleSignIn.tsx (1)
src/hooks/firebase.ts (1)
signInWithGoogle(23-23)
backend/routes/auth.js (1)
backend/config/passportConfig.js (3)
user(10-10)user(40-40)User(3-3)
🔇 Additional comments (8)
src/pages/Login/Login.tsx (2)
6-7: GoogleSignIn wiring looks goodImport placement and usage align with the new auth flow.
128-128: Button styling change is safeUI-only enhancement; no impact on logic.
backend/models/User.js (2)
17-21: LGTM! Google ID field properly configured.The
googleIdfield is well-configured with appropriate constraints:
unique: trueensures no duplicate Google accountssparse: trueallows multiple users without Google IDs (null values)- Optional field supports both Google and email/password users
5-8: Username uniqueness requirements verified—no dependencies found.
Our search across JS/TS files and AST patterns didn’t uncover any code that assumes or enforcesusernameuniqueness. Removing theunique: trueconstraint is safe in the current codebase.src/pages/Signup/Signup.tsx (2)
5-5: LGTM! Clean integration of Google Sign-In component.The import is properly structured and the component integration follows React best practices.
137-147: LGTM! Well-designed UI integration with proper visual separation.The OR separator and Google Sign-In button integration maintains good UX with:
- Clear visual separation between authentication methods
- Consistent styling with the existing form
- Proper component encapsulation
src/components/GoogleSignIn.tsx (2)
1-6: LGTM! Clean imports and environment configuration.The imports are well-organized and the environment variable usage for backend URL is appropriate for configuration management.
49-57: LGTM! Well-designed button component with proper accessibility.The button implementation includes:
- Proper loading state management
- Accessibility features (disabled state)
- Consistent styling with the application theme
- Good user feedback with loading text
| password: { | ||
| type: String, | ||
| }, |
There was a problem hiding this comment.
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 comparePassword method (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
‼️ 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| password: { | |
| type: String, | |
| }, | |
| UserSchema.pre('save', async function (next) { | |
| // Only hash if the password was modified and actually exists | |
| 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 there's no stored password (e.g. OAuth user), always fail comparison | |
| if (!this.password) { | |
| return false; | |
| } | |
| return await bcrypt.compare(enteredPassword, this.password); | |
| }; |
🤖 Prompt for AI Agents
In backend/models/User.js around lines 14 to 16 (and affecting the pre-save hook
at ~25-37 and comparePassword at ~40-41), the schema makes password optional but
the pre-save hook and comparePassword assume a defined password; update them so
the pre-save hook only runs hashing when this.isModified('password') &&
this.password is truthy, and skip hashing for users without a password (e.g.,
Google OAuth). Also change comparePassword to immediately return false (or a
rejected Promise) if the stored password is falsy, and only call bcrypt.compare
when both the candidate password and stored hash exist; keep the same
async/callback pattern used elsewhere. This prevents attempts to hash/compare
undefined values for OAuth users.
| 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" }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Critical security and account management issues need to be addressed.
The Google login endpoint has several critical issues:
-
Account security vulnerability: When finding an existing user by email, the endpoint doesn't verify the
googleIdmatches, potentially allowing account takeover if someone knows another user's email. -
Missing account linking: If a user exists with email/password but no
googleId, the endpoint doesn't update their record to link the Google account. -
No authentication state management: Unlike the Passport-based
/loginroute, this endpoint doesn't establish a session or provide authentication context.
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
‼️ 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 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" }); | |
| } | |
| }); | |
| 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 { | |
| // 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" }); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In backend/routes/auth.js around lines 30 to 53, the Google login handler
currently trusts email alone and neither verifies the googleId nor links
accounts or establishes auth state; update it to first validate incoming payload
(email and googleId), then lookup User by email and: if found and user.googleId
exists, ensure it matches the provided googleId and reject with 401 on mismatch;
if found and user.googleId is absent, set user.googleId to the provided value
and save to link the Google account; if not found, create a new user with
googleId and save; after any successful lookup or creation, establish an
authenticated session identical to your /login flow (e.g., call req.login or
issue the same JWT/session cookie you use elsewhere) and return the user without
sensitive fields, while returning appropriate 4xx errors for invalid payloads
and mismatched googleId to prevent account takeover.
| "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", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify 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”
A project-wide search found no imports or requires for these two packages beyond their entries in package.json. Removing them will slim install size and reduce attack surface.
• package.json – lines for
– “i” (^0.3.7)
– “react-firebase-hooks” (^5.1.1)
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
‼️ 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "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", | |
| "dependencies": { | |
| "firebase": "^12.1.0", | |
| "framer-motion": "^12.23.12", | |
| "lucide-react": "^0.525.0", | |
| "octokit": "^4.0.2", | |
| "postcss": "^8.4.47", | |
| "react": "^18.3.1", | |
| "react-dom": "^18.3.1", | |
| "react-hot-toast": "^2.4.1", | |
| }, |
🤖 Prompt for AI Agents
In package.json around lines 22 to 31, remove the unused dependency entries for
"i" (^0.3.7) and "react-firebase-hooks" (^5.1.1) from the dependencies section,
then run your package manager to update the lockfile (npm install or yarn
install) and commit the updated package.json and lockfile; after that run the
test/build steps to ensure nothing breaks and update any CI cache if necessary.
| 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); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Several improvements needed for production readiness.
The core authentication flow is functional but has areas for improvement:
-
Unused ID token: The
idTokenis retrieved but never used, which may indicate incomplete Firebase authentication verification. -
User experience: Using
alert()for error messages is not ideal for modern web applications - consider using toast notifications or inline error states. -
Error handling: Generic error messages don't help users understand specific issues (network failures, auth cancellation, etc.).
-
Security consideration: The backend endpoint lacks proper session management (see backend review comments).
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
‼️ 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 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); | |
| } | |
| }; | |
| const handleSignIn = async () => { | |
| setIsLoading(true); | |
| try { | |
| const userCredential = await signInWithGoogle(); | |
| console.log("Google userCredential:", userCredential); | |
| if (userCredential && userCredential.user) { | |
| 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 { | |
| // TODO: Replace with proper error state or toast notification | |
| console.error("Login failed:", response.data.message); | |
| } | |
| } else { | |
| console.error("No user returned from Google sign-in"); | |
| } | |
| } catch (error) { | |
| console.error("Google sign-in error:", error); | |
| // TODO: Handle specific error types (user cancelled, network error, etc.) | |
| } finally { | |
| setIsLoading(false); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In src/components/GoogleSignIn.tsx around lines 12 to 47, the handler retrieves
an idToken but never uses it and shows raw alerts on errors; update the flow to
send the idToken to the backend (either in the POST body or Authorization
header) so the server can verify the Firebase token and establish a session,
replace alert() calls with a UI-friendly error mechanism (toast or component
state like setError and render inline messages), and improve error handling by
distinguishing Axios/network errors, Firebase auth cancellations, and unexpected
exceptions to display meaningful messages and log full error details; keep
setIsLoading(false) in finally.
| 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 | ||
| }; |
There was a problem hiding this comment.
🛠️ 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
‼️ 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 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 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(", ")}`); | |
| } |
🤖 Prompt for AI Agents
In src/hooks/firebase.ts around lines 4 to 11, the firebaseConfig is built
directly from import.meta.env so missing env vars cause opaque errors later; add
a guard that enumerates the required variable names, checks import.meta.env for
each, collects any missing keys, and if any are missing throw (or console.error
then throw) a clear Error listing which env vars are absent (and stop
initialization). Run this check before using firebaseConfig or calling
initializeApp so the failure is immediate and the error message is actionable.
| const signInWithGoogle = async () => { | ||
| await setPersistence(auth, browserLocalPersistence); | ||
| return await signInWithPopup(auth, googleAuthProvider); | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Critical: 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 google-login Endpoint
The backend currently trusts client-supplied email/googleId without verifying the Firebase ID token. This allows attackers to spoof identities. You must:
• Frontend (src/hooks/firebase.ts)
- Return the user’s ID token alongside the credential:
const signInWithGoogle = async () => { await setPersistence(auth, browserLocalPersistence); - return await signInWithPopup(auth, googleAuthProvider); + const cred = await signInWithPopup(auth, googleAuthProvider); + const idToken = await cred.user.getIdToken(); + return { cred, idToken }; }; - Send
idToken(not raw UID/email) in your POST to/api/auth/google-login.
• Backend (backend/routes/auth.js)
- Install & initialize the Firebase Admin SDK:
// at top of file const admin = require('firebase-admin'); admin.initializeApp({ credential: admin.credential.cert(require('../path/to/serviceAccountKey.json')), });
- Verify the ID token in
/google-loginbefore trusting any claims:router.post("/google-login", async (req, res) => { try {
- const { email, name, googleId } = req.body;
+ const { idToken } = req.body;
+ const decoded = await admin.auth().verifyIdToken(idToken);
+ const { uid, email, name } = decoded;
let user = await User.findOne({ email });
if (!user) {
user = new User({ username: name, email, googleId: uid });
await user.save();
}
return res.json({ message: "Login successful", user });
} catch (error) {
console.error("Google login error:", error);
return res.status(401).json({ message: "Invalid or expired token" });
}
});
```
Please implement these changes before merging.
🤖 Prompt for AI Agents
In src/hooks/firebase.ts around lines 18-21, you need to return the Firebase ID
token from signInWithGoogle and ensure the frontend sends that idToken (not raw
uid/email) in the POST to /api/auth/google-login; modify signInWithGoogle to
obtain and return user.getIdToken() (or equivalent) alongside the credential so
the caller can include idToken in the request body. On the backend
(backend/routes/auth.js) add and initialize the Firebase Admin SDK (require
firebase-admin, initializeApp with service account credential) near the top of
the file, then in the /google-login handler verify the received idToken with
admin.auth().verifyIdToken(idToken) and only trust claims (uid/email) from the
verified token result before creating or logging in the user; reject requests
with invalid or missing tokens with a 401.
| <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> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Unify 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
- In src/pages/Login/Login.tsx, update the GoogleSignIn usage to pass the same callbacks you use for the password flow:
<div>
- <GoogleSignIn
-
onSuccess={() => navigate("/home")} -
onError={(msg: string) => setMessage(msg || "Something went wrong")} - />
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <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> | |
| <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 | |
| onSuccess={() => navigate("/home")} | |
| onError={(msg: string) => setMessage(msg || "Something went wrong")} | |
| /> | |
| </div> |
🤖 Prompt for AI Agents
In src/pages/Login/Login.tsx around lines 132-141, the GoogleSignIn usage needs
to be updated to accept the same post-login navigation and error handling
callbacks as the password flow: change the <GoogleSignIn /> invocation to pass
the existing onSuccess and onError handlers (or the functions used by the
password flow that call navigate("/") and show errors) so both flows behave
identically; in src/components/GoogleSignIn.tsx add optional props onSuccess?:
() => void and onError?: (msg: string) => void, remove any direct navigate("/")
or alert() calls, and instead call onSuccess() on successful sign-in or
onError(errorMessage) on failure; optionally apply the same change to
src/pages/Signup/Signup.tsx for parity; also verify backend CORS permits the
frontend origin (e.g., http://localhost:5173) to call /api/auth/google-login.
| server:{ | ||
| host:'0.0.0.0', | ||
| port:5173 | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Parameterize 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
Avoid binding to 0.0.0.0 by default—use localhost and fall back to env vars for multi-device testing:
// 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 --host CLI flag.
• Don’t forget to add your actual dev URL(s) (e.g. http://localhost:5173 or any LAN IP) to Firebase Authentication’s Authorized Domains.
🤖 Prompt for AI Agents
In vite.config.ts around lines 7 to 10, the dev server is hardcoded to host
'0.0.0.0' and port 5173; change it to use localhost by default and read fallback
values from VITE_DEV_HOST and VITE_DEV_PORT environment variables so developers
only opt into LAN binding when they set the envs. Update the server config to
set host = process.env.VITE_DEV_HOST || 'localhost' and port =
Number(process.env.VITE_DEV_PORT) || 5173 (ensure numeric conversion and a safe
default), and ensure any dev URLs you use are added to Firebase Auth Authorized
Domains when enabling LAN access.
2 participants
Related Issue
Description
Currently, the website’s authentication supports only email login. This update requests the addition of Google Login using Firebase Authentication, allowing users to sign in easily and securely with their Google accounts, alongside the existing email login.
How Has This Been Tested?
Manual Testing:
QA Checklist:
Screenshots
Type of Change
Summary by CodeRabbit