Skip to content

Commit 8a1d518

Browse files
committed
fix: address CodeRabbit review comments
1 parent 493a341 commit 8a1d518

1 file changed

Lines changed: 111 additions & 86 deletions

File tree

backend/config/passportConfig.js

Lines changed: 111 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -1,101 +1,126 @@
11
const passport = require("passport");
2-
const LocalStrategy = require('passport-local').Strategy;
3-
const GitHubStrategy = require('passport-github2').Strategy;
2+
const LocalStrategy = require("passport-local").Strategy;
3+
const GitHubStrategy = require("passport-github2").Strategy;
44
const User = require("../models/User");
55

66
passport.use(
7-
new LocalStrategy(
8-
{ usernameField: "email" },
9-
async (email, password, done) => {
10-
try {
11-
const user = await User.findOne({ email });
12-
if (!user) {
13-
return done(null, false, { message: 'Email is invalid ' });
14-
}
15-
16-
if (!user.password) {
17-
return done(null, false, { message: 'Use GitHub sign in for this account' });
18-
}
19-
20-
const isMatch = await user.comparePassword(password);
21-
if (!isMatch) {
22-
return done(null, false, { message: 'Invalid password' });
23-
}
24-
25-
return done(null, {
26-
id: user._id.toString(),
27-
username: user.username,
28-
email: user.email
29-
});
30-
} catch (err) {
31-
return done(err);
32-
}
7+
new LocalStrategy(
8+
{ usernameField: "email" },
9+
async (email, password, done) => {
10+
try {
11+
const user = await User.findOne({ email });
12+
13+
if (!user) {
14+
return done(null, false, {
15+
message: "Invalid email or password",
16+
});
3317
}
34-
)
18+
19+
if (!user.password) {
20+
return done(null, false, {
21+
message: "Use GitHub sign in for this account",
22+
});
23+
}
24+
25+
const isMatch = await user.comparePassword(password);
26+
27+
if (!isMatch) {
28+
return done(null, false, {
29+
message: "Invalid email or password",
30+
});
31+
}
32+
33+
return done(null, {
34+
id: user._id.toString(),
35+
username: user.username,
36+
email: user.email,
37+
});
38+
} catch (err) {
39+
return done(err);
40+
}
41+
}
42+
)
3543
);
3644

3745
if (process.env.GITHUB_CLIENT_ID && process.env.GITHUB_CLIENT_SECRET) {
38-
passport.use(
39-
new GitHubStrategy(
40-
{
41-
clientID: process.env.GITHUB_CLIENT_ID,
42-
clientSecret: process.env.GITHUB_CLIENT_SECRET,
43-
callbackURL: process.env.GITHUB_CALLBACK_URL,
44-
scope: ['user:email'],
45-
},
46-
async (accessToken, refreshToken, profile, done) => {
47-
try {
48-
const primaryEmail = profile.emails?.[0]?.value || null;
49-
const avatar = profile.photos?.[0]?.value || "";
50-
51-
let user = await User.findOne({ githubId: profile.id });
52-
if (!user && primaryEmail) {
53-
user = await User.findOne({ email: primaryEmail });
54-
}
55-
56-
if (!user) {
57-
const loginName = profile.username || `github_${profile.id}`;
58-
const uniqueSuffix = Math.random().toString(36).slice(2, 7);
59-
60-
user = new User({
61-
githubId: profile.id,
62-
username: `${loginName}_${uniqueSuffix}`,
63-
email: primaryEmail,
64-
avatar,
65-
});
66-
} else {
67-
user.githubId = user.githubId || profile.id;
68-
user.email = user.email || primaryEmail;
69-
user.avatar = user.avatar || avatar;
70-
}
71-
72-
await user.save();
73-
74-
return done(null, {
75-
id: user._id.toString(),
76-
username: user.username,
77-
email: user.email,
78-
});
79-
80-
} catch (err) {
81-
return done(err);
82-
}
46+
passport.use(
47+
new GitHubStrategy(
48+
{
49+
clientID: process.env.GITHUB_CLIENT_ID,
50+
clientSecret: process.env.GITHUB_CLIENT_SECRET,
51+
callbackURL: process.env.GITHUB_CALLBACK_URL,
52+
scope: ["user:email"],
53+
state: true,
54+
},
55+
56+
async (accessToken, refreshToken, profile, done) => {
57+
try {
58+
const primaryEmail = profile.emails?.[0]?.value;
59+
const avatar = profile.photos?.[0]?.value || "";
60+
61+
let user = await User.findOne({ githubId: profile.id });
62+
63+
if (!user && primaryEmail) {
64+
user = await User.findOne({ email: primaryEmail });
65+
}
66+
67+
if (!user) {
68+
const loginName =
69+
profile.username || `github_${profile.id}`;
70+
71+
const uniqueSuffix = Math.random()
72+
.toString(36)
73+
.slice(2, 7);
74+
75+
const userData = {
76+
githubId: profile.id,
77+
username: `${loginName}_${uniqueSuffix}`,
78+
avatar,
79+
};
80+
81+
if (primaryEmail) {
82+
userData.email = primaryEmail;
8383
}
84-
)
85-
);
84+
85+
user = new User(userData);
86+
87+
} else {
88+
user.githubId = user.githubId || profile.id;
89+
90+
if (primaryEmail) {
91+
user.email = user.email || primaryEmail;
92+
}
93+
94+
user.avatar = user.avatar || avatar;
95+
}
96+
97+
await user.save();
98+
99+
return done(null, {
100+
id: user._id.toString(),
101+
username: user.username,
102+
email: user.email,
103+
});
104+
105+
} catch (err) {
106+
return done(err);
107+
}
108+
}
109+
)
110+
);
86111
}
87112

88-
// Serialize user (store user info in session)
113+
// Serialize user
89114
passport.serializeUser((user, done) => {
90-
done(null, user.id);
115+
done(null, user.id);
91116
});
92117

93-
// Deserialize user (retrieve user from session)
118+
// Deserialize user
94119
passport.deserializeUser(async (id, done) => {
95-
try {
96-
const user = await User.findById(id);
97-
done(null, user);
98-
} catch (err) {
99-
done(err, null);
100-
}
101-
});
120+
try {
121+
const user = await User.findById(id);
122+
done(null, user);
123+
} catch (err) {
124+
done(err, null);
125+
}
126+
});

0 commit comments

Comments
 (0)