Skip to content

feat: implement Google & GitHub OAuth and persist session stores via … #343

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.

Sign up for GitHub

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

Open
Soumadip-Eagle123 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: implement Google & GitHub OAuth and persist session stores via … #343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d482433
feat: implement Google & GitHub OAuth and persist session stores via …
Soumadip-Eagle123 May 20, 2026
5b1ded9
chore: address CodeRabbit security review recommendations and harden …
Soumadip-Eagle123 May 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion backend/.env.sample
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
PORT=5000
MONGO_URI=mongodb://localhost:27017/githubTracker
MONGO_URI=mongodb://mongo:27017/githubTracker
SESSION_SECRET=your-secret-key
GOOGLE_CLIENT_ID=
GOOGLE_CLIENT_SECRET=
GITHUB_CLIENT_ID=
GITHUB_CLIENT_SECRET=
74 changes: 74 additions & 0 deletions backend/config/passportConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
const passport = require("passport");
const GoogleStrategy = require('passport-google-oauth20').Strategy;
const GitHubStrategy = require('passport-github2').Strategy;
const LocalStrategy = require('passport-local').Strategy;
const User = require("../models/User");

Expand Down Expand Up @@ -29,6 +31,78 @@ passport.use(
)
);

passport.use(
new GoogleStrategy(
{
clientID: process.env.GOOGLE_CLIENT_ID,
clientSecret: process.env.GOOGLE_CLIENT_SECRET,
callbackURL: "/api/auth/google/callback",
state: true
},
async (accessToken, refreshToken, profile, done) => {
try {
// Check if user already exists by their email
let user = await User.findOne({ email: profile.emails[0].value });

if (!user) {
// Create a user placeholder if they are logging in for the first time
user = new User({
username: profile.displayName.replace(/\s+/g, "_").toLowerCase() + "_" + profile.id.substring(0, 5),
email: profile.emails[0].value,
Comment on lines +45 to +51
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

# First, let's find and read the relevant file
fd -t f "passportConfig.js" backend/config/

Repository: GitMetricsLab/github_tracker

Length of output: 105

🏁 Script executed:

# Get the line count first
wc -l backend/config/passportConfig.js

Repository: GitMetricsLab/github_tracker

Length of output: 109

🏁 Script executed:

# Read the relevant section (lines 34-103 for context)
sed -n '34,103p' backend/config/passportConfig.js

Repository: GitMetricsLab/github_tracker

Length of output: 2593

🏁 Script executed:

# Also check the User model to see if username has uniqueness constraint
fd -t f "User" -e "js" -e "ts" | xargs grep -l "username.*unique\|unique.*username" 2>/dev/null | head -5

Repository: GitMetricsLab/github_tracker

Length of output: 54

🏁 Script executed:

# Search for email validation patterns in the file
rg "email" backend/config/passportConfig.js -B 2 -A 2

Repository: GitMetricsLab/github_tracker

Length of output: 2157

🏁 Script executed:

# Find User model file
fd -t f "User" -e "js" -e "ts" models/ || fd -t f "User.js" || fd -t f "user.js" || fd -t f "*chema*" models/

Repository: GitMetricsLab/github_tracker

Length of output: 208

🏁 Script executed:

# Search for User model definition more broadly
rg "new Schema\|export.*User\|module.exports.*User" -A 10 --max-count=3

Repository: GitMetricsLab/github_tracker

Length of output: 54

🏁 Script executed:

# Look for username field definition in the codebase
rg "username.*:.*{" --type js --type ts -B 2 -A 2 | grep -A 5 -B 5 unique

Repository: GitMetricsLab/github_tracker

Length of output: 164

🏁 Script executed:

# Read the User model file
cat -n backend/models/User.js

Repository: GitMetricsLab/github_tracker

Length of output: 1309

🏁 Script executed:

# Search for unique constraints on username
rg "unique\|index" backend/models/User.js -B 2 -A 2

Repository: GitMetricsLab/github_tracker

Length of output: 54

Guard email extraction in Google OAuth strategy before dereferencing array.

Line 44 and line 50 directly access profile.emails[0].value without checking if the array exists. If Google returns a profile without the emails field, this throws a TypeError and crashes the callback. The GitHub strategy already handles this correctly with optional chaining. Add null-safe extraction and reject auth when email is missing.

Proposed fix 
- let user = await User.findOne({ email: profile.emails[0].value });
+ const googleEmail = profile.emails?.[0]?.value;
+ if (!googleEmail) {
+   return done(null, false, { message: "Google account email is required" });
+ }
+ let user = await User.findOne({ email: googleEmail });

 ...
-   email: profile.emails[0].value,
+   email: googleEmail,
📝 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.

Suggested change
let user = await User.findOne({ email: profile.emails[0].value });
if (!user) {
// Create a user placeholder if they are logging in for the first time
user = new User({
username: profile.displayName.replace(/\s+/g, "_").toLowerCase() + "_" + profile.id.substring(0, 5),
email: profile.emails[0].value,
const googleEmail = profile.emails?.[0]?.value;
if (!googleEmail) {
return done(null, false, { message: "Google account email is required" });
}
let user = await User.findOne({ email: googleEmail });
if (!user) {
// Create a user placeholder if they are logging in for the first time
user = new User({
username: profile.displayName.replace(/\s+/g, "_").toLowerCase() + "_" + profile.id.substring(0, 5),
email: googleEmail,
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/config/passportConfig.js` around lines 44 - 50, The Google OAuth
callback dereferences profile.emails[0].value in the User.findOne lookup and
when constructing a new User (username and email) which will throw if
profile.emails is missing; update the Google strategy callback to safely extract
the email (e.g., const email = profile.emails?.[0]?.value) and if email is falsy
call done(null, false, { message: "No email provided by Google" }) to reject
authentication, then use that guarded email value in User.findOne and in the new
User({ ... }) and keep the username generation using profile.displayName but
only if present.

Comment on lines +50 to +51
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle OAuth username collisions against unique index.

Line 49 and Line 85 can generate usernames that collide with existing local/OAuth users, causing duplicate-key failures on first OAuth login. Derive a deterministic unique base (provider + provider id suffix) or retry on duplicate key.

Also applies to: 85-87

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/config/passportConfig.js` around lines 49 - 50, The current OAuth
username construction using profile.displayName + "_" +
profile.id.substring(0,5) (see the username assignment in passportConfig.js) can
collide with existing users; change it to produce a deterministic unique base
that includes the provider to avoid collisions (e.g. use provider + "_" +
profile.id.substring(0,5) or provider + "_" + profile.provider + "_" + id
suffix) or implement a retry-on-duplicate-key routine around the user creation
logic (catch duplicate key errors when creating the User and append/increment a
suffix until creation succeeds); update both places where username is built (the
existing username assignment and the similar block later around lines creating
new OAuth users) and ensure any error handling for duplicate-key (E11000) is
covered so first OAuth login does not fail.

// Create a dummy password since the schema requires it
password: "OAUTH_USER_EXTERNAL_PROVIDER"
});
await user.save();
}

return done(null, {
id: user._id.toString(),
username: user.username,
email: user.email
});
} catch (err) {
return done(err);
}
}
)
);

// 2. GitHub OAuth Strategy
passport.use(
new GitHubStrategy(
{
clientID: process.env.GITHUB_CLIENT_ID,
clientSecret: process.env.GITHUB_CLIENT_SECRET,
callbackURL: "/api/auth/github/callback",
state: true
},
async (accessToken, refreshToken, profile, done) => {
try {
// If GitHub doesn't return a public email, fall back to a dummy identifier string
const userEmail = profile.emails?.[0]?.value || `${profile.username}@github.oauth`;
let user = await User.findOne({ email: userEmail });

if (!user) {
user = new User({
username: profile.username,
email: userEmail,
password: "OAUTH_USER_EXTERNAL_PROVIDER"
});
await user.save();
}

return done(null, {
id: user._id.toString(),
username: user.username,
email: user.email
});
} catch (err) {
return done(err);
}
}
)
);

// Serialize user (store user info in session)
passport.serializeUser((user, done) => {
done(null, user.id);
Expand Down
17 changes: 16 additions & 1 deletion backend/models/User.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,23 @@ const UserSchema = new mongoose.Schema({
},
password: {
type: String,
required: true,
required: false,
},
authProvider: {
type: String,
enum: ["local", "google", "github"],
default: "local",
},
providerId: {
type: String,
default: null,
},
});

UserSchema.pre('save', async function () {
if (this.password === "OAUTH_USER_EXTERNAL_PROVIDER") {
return;
}
Comment on lines +31 to +33
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Replace magic-password provider detection with explicit auth provider fields.

Line 22 and Line 34 key OAuth behavior off a literal password value. That makes auth mode user-input-dependent (a local user choosing this exact password gets a broken login path and plaintext persistence). Use explicit fields like authProvider and providerId instead of inspecting password content.

Suggested direction 
 const UserSchema = new mongoose.Schema({
   username: { type: String, required: true, unique: true },
   email: { type: String, required: true, unique: true },
   password: { type: String, required: true },
+  authProvider: { type: String, enum: ["local", "google", "github"], default: "local" },
+  providerId: { type: String, default: null },
 });
 
 UserSchema.pre('save', async function () {
-  if (this.password === "OAUTH_USER_EXTERNAL_PROVIDER") return;
+  if (this.authProvider !== "local") return;
   if (!this.isModified('password')) return;
   const salt = await bcrypt.genSalt(10);
   this.password = await bcrypt.hash(this.password, salt);
 });
 
 UserSchema.methods.comparePassword = async function (enteredPassword) {
-  if (this.password === "OAUTH_USER_EXTERNAL_PROVIDER") return false;
+  if (this.authProvider !== "local") return false;
   return await bcrypt.compare(enteredPassword, this.password);
 };

Also applies to: 34-36

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/models/User.js` around lines 22 - 24, Replace the magic-string check
this.password === "OAUTH_USER_EXTERNAL_PROVIDER" with explicit authProvider and
providerId fields on the User model: add authProvider (e.g. 'local' | 'oauth')
and providerId, update the model defaults/migration to include them, and change
all logic that currently branches on that password literal (e.g., the block in
User.js that returns early and the similar check at lines 34-36) to instead
check authProvider !== 'local' (or authProvider === 'oauth') to skip password
logic; also ensure user-creation and update paths set authProvider/providerId
for external users and that any password setters/validators (e.g.,
validatePassword, setPassword) only operate when authProvider === 'local'.

if (!this.isModified('password'))
return;

Expand All @@ -28,6 +40,9 @@ UserSchema.pre('save', async function () {

// Compare passwords during login
UserSchema.methods.comparePassword = async function (enteredPassword) {
if (this.password === "OAUTH_USER_EXTERNAL_PROVIDER") {
return false;
}
return await bcrypt.compare(enteredPassword, this.password);
};

Expand Down
8 changes: 5 additions & 3 deletions backend/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
"dev": "nodemon server.js",
"start": "node server.js",
"test": "jasmine spec/**/*.spec.cjs"

},
"keywords": [],
"author": "",
Expand All @@ -15,12 +14,15 @@
"dependencies": {
"bcryptjs": "^2.4.3",
"body-parser": "^1.20.3",
"connect-mongo": "^6.0.0",
"cors": "^2.8.5",
"dotenv": "^16.4.5",
"express": "^4.21.1",
"express": "^4.22.2",
"express-session": "^1.18.1",
"mongoose": "^8.8.2",
"mongoose": "^8.24.0",
"passport": "^0.7.0",
"passport-github2": "^0.1.12",
"passport-google-oauth20": "^2.0.0",
"passport-local": "^1.0.0",
"zod": "^4.4.3"
},
Expand Down
24 changes: 24 additions & 0 deletions backend/routes/auth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,28 @@ router.get("/logout", (req, res) => {
});
});

router.get("/google", passport.authenticate("google", { scope: ["profile", "email"] }));
Comment thread
coderabbitai[bot] marked this conversation as resolved.

router.get(
"/google/callback",
passport.authenticate("google", {
failureRedirect: `${process.env.FRONTEND_URL || "http://localhost:5173"}/login`
}),
(req, res) => {
res.redirect(process.env.FRONTEND_URL || "http://localhost:5173");
}
);

router.get("/github", passport.authenticate("github", { scope: ["user:email"] }));

router.get(
"/github/callback",
passport.authenticate("github", {
failureRedirect: `${process.env.FRONTEND_URL || "http://localhost:5173"}/login`
}),
(req, res) => {
res.redirect(process.env.FRONTEND_URL || "http://localhost:5173");
}
);

module.exports = router;
12 changes: 11 additions & 1 deletion backend/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const passport = require('passport');
const bodyParser = require('body-parser');
require('dotenv').config();
const cors = require('cors');

const MongoStore = require('connect-mongo');
// Passport configuration
require('./config/passportConfig');

Expand All @@ -20,6 +20,16 @@ app.use(session({
secret: process.env.SESSION_SECRET,
resave: false,
saveUninitialized: false,
store: MongoStore.create({
mongoUrl: process.env.MONGO_URI,
collectionName: 'sessions'
}),
cookie: {
maxAge: 1000 * 60 * 60 * 24,
httpOnly: true,
sameSite: 'lax',
secure: process.env.NODE_ENV === 'production'
}
Comment on lines +23 to +32
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

# First, let's locate and examine backend/server.js
find . -name "server.js" -type f | head -20

Repository: GitMetricsLab/github_tracker

Length of output: 92

🏁 Script executed:

# Let's check the directory structure to understand the project layout
git ls-files | grep -E "(backend|server)" | head -30

Repository: GitMetricsLab/github_tracker

Length of output: 355

🏁 Script executed:

# Search for session configuration in the codebase
rg "session\(" -A 10 --max-count 5

Repository: GitMetricsLab/github_tracker

Length of output: 1827

🏁 Script executed:

# Search for trust proxy configuration
rg "trust proxy" -i

Repository: GitMetricsLab/github_tracker

Length of output: 54

🏁 Script executed:

# Search for cookie configuration patterns
rg "cookie\s*:" -B 2 -A 5

Repository: GitMetricsLab/github_tracker

Length of output: 381

🏁 Script executed:

# Read the full backend/server.js file
cat -n backend/server.js

Repository: GitMetricsLab/github_tracker

Length of output: 1614

🏁 Script executed:

# Check Dockerfile configurations to understand deployment context
cat -n backend/Dockerfile.prod

Repository: GitMetricsLab/github_tracker

Length of output: 659

🏁 Script executed:

# Check environment setup in package.json
cat -n backend/package.json | head -30

Repository: GitMetricsLab/github_tracker

Length of output: 996

🏁 Script executed:

# Search for any proxy-related configuration in the codebase
rg "proxy" -i -B 2 -A 2

Repository: GitMetricsLab/github_tracker

Length of output: 54

🏁 Script executed:

# Check if NODE_ENV is used anywhere in the backend
rg "NODE_ENV" backend/

Repository: GitMetricsLab/github_tracker

Length of output: 54

Harden session cookie settings for production.

The session cookie configuration at lines 27-29 sets only maxAge, leaving critical security settings implicit. Add httpOnly, sameSite, secure, and trust proxy settings to prevent XSS session theft, CSRF attacks, and unencrypted transmission.

Proposed fix 
+app.set("trust proxy", 1);

 app.use(session({
   secret: process.env.SESSION_SECRET,
   resave: false,
   saveUninitialized: false,
   store: MongoStore.create({
     mongoUrl: process.env.MONGO_URI,
     collectionName: 'sessions'
   }),
   cookie: {
-    maxAge: 1000 * 60 * 60 * 24
+    maxAge: 1000 * 60 * 60 * 24,
+    httpOnly: true,
+    sameSite: "lax",
+    secure: process.env.NODE_ENV === "production",
   }
 }));
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/server.js` around lines 23 - 29, The session cookie currently only
sets maxAge; update the session middleware configuration (the MongoStore.create
+ cookie object) to add cookie.httpOnly: true, cookie.sameSite: 'lax' (or
'strict' if applicable) and cookie.secure set to true only when running in
production (e.g., process.env.NODE_ENV === 'production'), and ensure your
Express app enables trust proxy (app.set('trust proxy', 1)) before mounting the
session middleware so secure cookies work behind a proxy/load balancer; keep the
store: MongoStore.create(...) unchanged and apply these cookie and trust-proxy
changes around the session setup.

}));
app.use(passport.initialize());
app.use(passport.session());
Expand Down
14 changes: 14 additions & 0 deletions src/pages/Login/Login.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,20 @@ const Login: React.FC = () => {
>
{isLoading ? "Signing in..." : "Sign In"}
</button>
<div className="flex flex-col gap-3 mt-4">
<a
href={`${backendUrl}/api/auth/google`}
className="w-full text-center bg-red-600 hover:bg-red-700 text-white py-3 px-6 rounded-2xl font-semibold transition-all duration-300"
>
Continue with Google
</a>
<a
href={`${backendUrl}/api/auth/github`}
className="w-full text-center bg-zinc-800 hover:bg-zinc-900 text-white py-3 px-6 rounded-2xl font-semibold transition-all duration-300"
>
Continue with GitHub
</a>
</div>
</form>

{/* Message */}
Expand Down
14 changes: 14 additions & 0 deletions src/pages/Signup/Signup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,20 @@ const SignUp: React.FC = () => {
>
{isLoading ? "Creating account..." : "Create Account"}
</button>
<div className="flex flex-col gap-3 mt-4">
<a
href={`${backendUrl}/api/auth/google`}
className="w-full text-center bg-red-600 hover:bg-red-700 text-white py-3 px-6 rounded-2xl font-semibold transition-all duration-300"
>
Continue with Google
</a>
<a
href={`${backendUrl}/api/auth/github`}
className="w-full text-center bg-zinc-800 hover:bg-zinc-900 text-white py-3 px-6 rounded-2xl font-semibold transition-all duration-300"
>
Continue with GitHub
</a>
</div>
</form>

{message && (
Expand Down