Skip to content

Commit 8b53aae

Browse files
authored
Merge pull request #2505 from Real-Dev-Squad/develop
Dev to Main Sync
2 parents 3393e9f + d7627c0 commit 8b53aae

File tree

13 files changed

+472
-278
lines changed

13 files changed

+472
-278
lines changed

constants/roles.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ const ROLES = {
55
MEMBER: "member",
66
ARCHIVED: "archived",
77
INDISCORD: "in_discord",
8+
DEVELOPER: "developer",
9+
DESIGNER:"designer",
10+
QA:"qa",
11+
PRODUCT_MANAGER:"product_manager",
12+
PROJECT_MANAGER:"project_manager",
13+
SOCIAL_MEDIA:"social_media",
14+
MAVEN:"maven"
815
};
916

1017
module.exports = ROLES;

constants/users.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
const ROLES = require("./roles");
2+
13
const profileStatus = {
24
PENDING: "PENDING",
35
APPROVED: "APPROVED",
@@ -32,6 +34,16 @@ const USERS_PATCH_HANDLER_ERROR_MESSAGES = {
3234
},
3335
};
3436

37+
export const ALL_USER_ROLES = [
38+
ROLES.DEVELOPER,
39+
ROLES.DESIGNER,
40+
ROLES.PRODUCT_MANAGER,
41+
ROLES.PROJECT_MANAGER,
42+
ROLES.QA,
43+
ROLES.SOCIAL_MEDIA,
44+
ROLES.MAVEN,
45+
];
46+
3547
const USERS_PATCH_HANDLER_SUCCESS_MESSAGES = {
3648
ARCHIVE_USERS: {
3749
SUCCESSFULLY_UPDATED_DATA: "Successfully updated users archived role to true if in_discord role is false",
@@ -64,4 +76,5 @@ module.exports = {
6476
discordNicknameLength,
6577
SIMULTANEOUS_WORKER_CALLS,
6678
MAX_USERNAME_LENGTH,
79+
ALL_USER_ROLES
6780
};

controllers/auth.js

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
const passport = require("passport");
2+
const config = require("config");
23
const users = require("../models/users");
34
const QrCodeAuthModel = require("../models/qrCodeAuth");
45
const authService = require("../services/authService");
56
const dataAccess = require("../services/dataAccessLayer");
7+
const logger = require("../utils/logger");
68
const {
79
SOMETHING_WENT_WRONG,
810
DATA_ADDED_SUCCESSFULLY,
911
USER_DOES_NOT_EXIST_ERROR,
1012
} = require("../constants/errorMessages");
13+
const ROLES = require("../constants/roles");
1114

1215
const googleAuthLogin = (req, res, next) => {
1316
const { redirectURL } = req.query;
@@ -84,13 +87,10 @@ async function handleGoogleLogin(req, res, user, authRedirectionUrl) {
8487
};
8588

8689
const userDataFromDB = await users.fetchUser({ email: userData.email });
87-
88-
if (userDataFromDB.userExists) {
89-
if (userDataFromDB.user.roles?.developer) {
90-
const errorMessage = encodeURIComponent("Google login is restricted for developer role.");
91-
const separator = authRedirectionUrl.search ? "&" : "?";
92-
return res.redirect(`${authRedirectionUrl}${separator}error=${errorMessage}`);
93-
}
90+
if (userDataFromDB.userExists && userDataFromDB.user?.role === ROLES.DEVELOPER) {
91+
return res.status(403).json({
92+
message: "Google Login is restricted for developers,please use github Login",
93+
});
9494
}
9595

9696
const { userId, incompleteUserDetails } = await users.addOrUpdate(userData);
@@ -187,6 +187,14 @@ const githubAuthCallback = (req, res, next) => {
187187
}
188188
}
189189

190+
const userDataFromDB = await users.fetchUser({ email: userData.email });
191+
const userRole = userDataFromDB.user?.role;
192+
if (userDataFromDB.userExists && userRole && userRole !== ROLES.DEVELOPER) {
193+
return res.status(403).json({
194+
message: "Github Login is restricted for non-developers,please use Google Login",
195+
});
196+
}
197+
190198
const { userId, incompleteUserDetails, role } = await users.addOrUpdate(userData);
191199

192200
const token = authService.generateAuthToken({ userId });

controllers/users.js

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -470,23 +470,38 @@ const getSelfDetails = async (req, res) => {
470470
* @param res {Object} - Express response object
471471
*/
472472

473-
const updateSelf = async (req, res) => {
473+
const updateSelf = async (req, res, next) => {
474474
try {
475-
const { id: userId, roles: userRoles, discordId } = req.userData;
475+
const { id: userId, roles: userRoles, discordId, incompleteUserDetails, role: existingRole } = req.userData;
476476
const devFeatureFlag = req.query.dev === "true";
477477
const { user } = await dataAccess.retrieveUsers({ id: userId });
478+
const { first_name: firstName, last_name: lastName, role } = req.body;
478479
let rolesToDisable = [];
479480

480-
if (req.body.username) {
481-
if (!user.incompleteUserDetails) {
482-
return res.boom.forbidden("Cannot update username again");
481+
if (devFeatureFlag) {
482+
if (req.body.username) {
483+
return res.boom.forbidden("You are not authorized to perform this operation");
483484
}
484-
await userQuery.setIncompleteUserDetails(userId);
485-
}
486-
487-
if (req.body.roles) {
488-
if (user && (user.roles.in_discord || user.roles.developer)) {
489-
return res.boom.forbidden("Cannot update roles");
485+
const username = await userService.validateUserSignup(
486+
userId,
487+
incompleteUserDetails,
488+
firstName,
489+
lastName,
490+
role,
491+
existingRole
492+
);
493+
if (username) {
494+
req.body.username = username;
495+
}
496+
} else {
497+
if (req.body?.username) {
498+
if (!user.incompleteUserDetails) {
499+
return res.boom.forbidden("Cannot update username again");
500+
}
501+
await userQuery.setIncompleteUserDetails(userId);
502+
}
503+
if (role) {
504+
return res.boom.forbidden("You are not authorized to perform this operation");
490505
}
491506
}
492507

@@ -542,7 +557,7 @@ const updateSelf = async (req, res) => {
542557
return res.boom.notFound("User not found");
543558
} catch (error) {
544559
logger.error(`Error while updating user: ${error}`);
545-
return res.boom.serverUnavailable(SOMETHING_WENT_WRONG);
560+
return next(error);
546561
}
547562
};
548563

@@ -1118,15 +1133,15 @@ const updateUsernames = async (req, res) => {
11181133
}
11191134
};
11201135

1121-
const updateProfile = async (req, res) => {
1136+
const updateProfile = async (req, res, next) => {
11221137
try {
11231138
const { id: currentUserId, roles = {} } = req.userData;
11241139
const isSelf = req.params.userId === currentUserId;
11251140
const isSuperUser = roles[ROLES.SUPERUSER];
11261141
const profile = req.query.profile === "true";
11271142

1128-
if (isSelf && profile && req.query.dev === "true") {
1129-
return await updateSelf(req, res);
1143+
if (isSelf && profile) {
1144+
return await updateSelf(req, res, next);
11301145
} else if (isSuperUser) {
11311146
return await updateUser(req, res);
11321147
}

middlewares/validators/user.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const {
55
USER_STATUS,
66
USERS_PATCH_HANDLER_ACTIONS,
77
USERS_PATCH_HANDLER_ERROR_MESSAGES,
8+
ALL_USER_ROLES,
89
} = require("../../constants/users");
910
const ROLES = require("../../constants/roles");
1011
const { IMAGE_VERIFICATION_TYPES } = require("../../constants/imageVerificationTypes");
@@ -52,11 +53,10 @@ const updateUser = async (req, res, next) => {
5253
.optional(),
5354
discordId: joi.string().optional(),
5455
disabledRoles: joi.array().items(joi.string().valid("super_user", "member")).optional(),
55-
roles: joi.object().keys({
56-
designer: joi.boolean().optional(),
57-
maven: joi.boolean().optional(),
58-
product_manager: joi.boolean().optional(),
59-
}),
56+
role: joi
57+
.string()
58+
.valid(...Object.values(ALL_USER_ROLES))
59+
.optional(),
6060
});
6161

6262
try {

services/users.js

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ const firestore = require("../utils/firestore");
22
const { formatUsername } = require("../utils/username");
33
const userModel = firestore.collection("users");
44
const tasksModel = require("../models/tasks");
5+
const userQuery = require("../models/users");
6+
const { ALL_USER_ROLES } = require("../constants/users");
7+
const Forbidden = require("http-errors").Forbidden;
58

69
const getUsersWithIncompleteTasks = async (users) => {
710
if (users.length === 0) return [];
@@ -46,7 +49,43 @@ const generateUniqueUsername = async (firstName, lastName) => {
4649
}
4750
};
4851

52+
/**
53+
* Validates user signup details and handles incomplete user details flow.
54+
*
55+
* @async
56+
* @function validateUserSignup
57+
* @param {string} userId - The id for the user.
58+
* @param {boolean} incompleteUserDetails - Indicates if the user has incomplete details.
59+
* @param {string|null} firstName - The user's first name.
60+
* @param {string|null} lastName - The user's last name.
61+
* @param {string|null} role - The role to assign to the user.
62+
* @param {string|null} existingRole - The user's existing role, if any.
63+
* @returns {Promise<string|null>} Returns a generated username if incompleteUserDetails is true and all required fields are present, otherwise undefined.
64+
*/
65+
66+
const validateUserSignup = async (userId, incompleteUserDetails, firstName, lastName, role, existingRole) => {
67+
try {
68+
if (!incompleteUserDetails) {
69+
const alreadyHasRole = existingRole && ALL_USER_ROLES.includes(existingRole);
70+
if (role && alreadyHasRole) {
71+
throw new Forbidden("Cannot update role again");
72+
}
73+
return null;
74+
}
75+
if (!firstName || !lastName || !role) {
76+
throw new Forbidden("You are not authorized to perform this operation");
77+
}
78+
const username = await generateUniqueUsername(firstName, lastName);
79+
await userQuery.setIncompleteUserDetails(userId);
80+
return username;
81+
} catch (err) {
82+
logger.error(`Error while validating user signup: ${err.message}`);
83+
throw err;
84+
}
85+
};
86+
4987
module.exports = {
5088
generateUniqueUsername,
5189
getUsersWithIncompleteTasks,
90+
validateUserSignup,
5291
};

test/config/test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ module.exports = {
5959
},
6060
rdsUi: {
6161
baseUrl: "https://realdevsquad.com",
62+
newSignupUrl: "https://my.realdevsquad.com/new-signup",
6263
routes: {
6364
authRedirection: "/goto",
6465
},

test/fixtures/auth/googleUserInfo.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ module.exports = () => {
3535
in_discord: true,
3636
archived: false,
3737
},
38+
role: "designer",
3839
incompleteUserDetails: false,
3940
updated_at: Date.now(),
4041
created_at: Date.now(),
@@ -65,10 +66,10 @@ module.exports = () => {
6566
{
6667
6768
roles: {
68-
developer: true,
6969
in_discord: true,
7070
archived: false,
7171
},
72+
role: "developer",
7273
incompleteUserDetails: false,
7374
updated_at: Date.now(),
7475
created_at: Date.now(),

test/fixtures/user/user.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ module.exports = () => {
2525
2626
discordJoinedAt: "2023-04-06T01:47:34.488000+00:00",
2727
joined_discord: "2023-01-13T18:21:09.278000+00:00",
28+
role: "developer",
2829
roles: {
2930
member: true,
3031
in_discord: true,
@@ -60,6 +61,7 @@ module.exports = () => {
6061
},
6162
nickname_synced: false,
6263
profileStatus: "BLOCKED",
64+
incompleteUserDetails: false,
6365
},
6466
{
6567
username: "pranavg",
@@ -95,10 +97,12 @@ module.exports = () => {
9597
github_display_name: "Sagar Bajpai",
9698
phone: "1234567890",
9799
100+
incompleteUserDetails: false,
98101
status: "active",
99102
tokens: {
100103
githubAccessToken: "githubAccessToken",
101104
},
105+
role: "designer",
102106
roles: {
103107
restricted: false,
104108
app_owner: true,
@@ -175,6 +179,8 @@ module.exports = () => {
175179
githubAccessToken: "githubAccessToken",
176180
},
177181
status: "active",
182+
incompleteUserDetails: true,
183+
role: "developer",
178184
roles: {
179185
member: true,
180186
archived: false,

test/integration/auth.test.js

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const chaiHttp = require("chai-http");
55
const passport = require("passport");
66
const app = require("../../server");
77
const cleanDb = require("../utils/cleanDb");
8+
const config = require("config");
89
const { generateGithubAuthRedirectUrl } = require("..//utils/github");
910
const { generateGoogleAuthRedirectUrl, stubPassportAuthenticate } = require("..//utils/googleauth");
1011
const { addUserToDBForTest } = require("../../utils/users");
@@ -404,7 +405,7 @@ describe("auth", function () {
404405
});
405406
});
406407

407-
it("should redirect the google user to login page if the user is a developer", async function () {
408+
it("should return 403 Forbidden if a developer tries to log in using google", async function () {
408409
await addUserToDBForTest(googleUserInfo[3]);
409410
const rdsUiUrl = new URL(config.get("services.rdsUi.baseUrl")).href;
410411
stubPassportAuthenticate(googleUserInfo[2]);
@@ -414,20 +415,19 @@ describe("auth", function () {
414415
.get("/auth/google/callback")
415416
.query({ code: "codeReturnedByGoogle", state: rdsUiUrl })
416417
.redirects(0);
417-
expect(res).to.have.status(302);
418-
const errorMessage = "Google login is restricted for developer role.";
419-
const expectedUrl = `https://realdevsquad.com/?error=${encodeURIComponent(errorMessage)}`;
420-
expect(res.headers.location).to.equal(expectedUrl);
418+
expect(res).to.have.status(403);
419+
const errorMessage = "Google Login is restricted for developers,please use github Login";
420+
expect(res.body.message).to.equal(errorMessage);
421421
});
422422

423-
it("should log in existing google user with same email via github OAuth", async function () {
424-
await addUserToDBForTest(googleUserInfo[1]);
423+
it("should return 403 Forbidden if a non-developer tries to login using github", async function () {
424+
await addUserToDBForTest(userData[3]);
425425
const rdsUiUrl = new URL(config.get("services.rdsUi.baseUrl")).href;
426426
const userInfoFromGitHub = {
427427
...githubUserInfo[0],
428428
_json: {
429429
...githubUserInfo[0]._json,
430-
email: "test12@gmail.com",
430+
email: "abc1@gmail.com",
431431
},
432432
};
433433
stubPassportAuthenticate(userInfoFromGitHub);
@@ -437,13 +437,14 @@ describe("auth", function () {
437437
.get("/auth/github/callback")
438438
.query({ code: "codeReturnedByGithub", state: rdsUiUrl })
439439
.redirects(0);
440-
expect(res).to.have.status(302);
441-
expect(res.headers.location).to.equal(rdsUiUrl);
440+
expect(res).to.have.status(403);
441+
const errorMessage = "Github Login is restricted for non-developers,please use Google Login";
442+
expect(res.body.message).to.equal(errorMessage);
442443
});
443444

444-
it("should log in existing github user with same email via google OAuth", async function () {
445-
await addUserToDBForTest(userData[0]);
446-
const rdsUiUrl = new URL(config.get("services.rdsUi.baseUrl")).href;
445+
it("should log in existing github user with no role and same email via google OAuth", async function () {
446+
await addUserToDBForTest(userData[1]);
447+
const newSignupUrl = new URL(config.get("services.rdsUi.newSignupUrl")).href;
447448
const userInfoFromGoogle = {
448449
...googleUserInfo[0],
449450
emails: [{ value: "[email protected]", verified: true }],
@@ -453,10 +454,10 @@ describe("auth", function () {
453454
const res = await chai
454455
.request(app)
455456
.get("/auth/google/callback")
456-
.query({ code: "codeReturnedByGoogle", state: rdsUiUrl })
457+
.query({ code: "codeReturnedByGoogle", state: newSignupUrl })
457458
.redirects(0);
458459
expect(res).to.have.status(302);
459-
expect(res.headers.location).to.equal(rdsUiUrl);
460+
expect(res.headers.location).to.equal(newSignupUrl);
460461
});
461462

462463
it("should get the verified email and redirect the google user to the goto page on successful login", async function () {

0 commit comments

Comments
 (0)