Skip to content

Commit 51090af

Browse files
Merge pull request #1945 from Real-Dev-Squad/develop
Dev to Main Sync
2 parents 3b627bc + 592ab43 commit 51090af

File tree

12 files changed

+151
-29
lines changed

12 files changed

+151
-29
lines changed

controllers/discordactions.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,14 @@ const addGroupRoleToMember = async (req, res) => {
115115
const [{ roleExists, existingRoles }, userData] = await Promise.all([roleExistsPromise, userDataPromise]);
116116

117117
if (!roleExists || req.userData.id !== userData.user.id) {
118-
res.boom.forbidden("Permission denied. Cannot add the role.");
118+
return res.boom.forbidden("Permission denied. Cannot add the role.");
119+
}
120+
121+
if (existingRoles.docs.length > 0) {
122+
const roleDetails = existingRoles.docs[0].data();
123+
if (roleDetails.rolename && !roleDetails.rolename.startsWith("group-")) {
124+
return res.boom.forbidden("Cannot use rolename that is not a group role");
125+
}
119126
}
120127

121128
const { roleData, wasSuccess } = await discordRolesModel.addGroupRoleToMember(memberGroupRole);
@@ -163,7 +170,7 @@ const deleteRole = async (req, res) => {
163170
const [{ roleExists }, userData] = await Promise.all([roleExistsPromise, userDataPromise]);
164171

165172
if (!roleExists || req.userData.id !== userData.user.id) {
166-
res.boom.forbidden("Permission denied. Cannot delete the role.");
173+
return res.boom.forbidden("Permission denied. Cannot delete the role.");
167174
}
168175
await discordServices.removeRoleFromUser(roleid, userid, req.userData);
169176

middlewares/authorizeUsersAndService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const { Services } = require("../constants/bot");
88
const ROLES = require("../constants/roles");
99
const { INTERNAL_SERVER_ERROR_MESSAGE } = require("../constants/progresses");
1010

11-
export const authorization = (allowedRoles: string[], allowedServices: string[]) => {
11+
export const authorizeAndAuthenticate = (allowedRoles: string[], allowedServices: string[]) => {
1212
const isRolesValid = allowedRoles.every((role) => Object.values(ROLES).includes(role));
1313
if (!isRolesValid) {
1414
throw new Error("Invalid role");

routes/discordactions.js

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ const checkIsVerifiedDiscord = require("../middlewares/verifydiscord");
2525
const checkCanGenerateDiscordLink = require("../middlewares/checkCanGenerateDiscordLink");
2626
const { SUPERUSER } = require("../constants/roles");
2727
const authorizeRoles = require("../middlewares/authorizeRoles");
28+
const ROLES = require("../constants/roles");
29+
const { Services } = require("../constants/bot");
2830
const { verifyCronJob } = require("../middlewares/authorizeBot");
31+
const { authorizeAndAuthenticate } = require("../middlewares/authorizeUsersAndService");
2932

3033
const router = express.Router();
3134

@@ -43,21 +46,27 @@ router.patch(
4346
checkIsVerifiedDiscord,
4447
updateDiscordImageForVerification
4548
);
46-
router.put("/group-idle", authenticate, authorizeRoles([SUPERUSER]), setRoleIdleToIdleUsers);
47-
router.put("/group-idle-7d", authenticate, authorizeRoles([SUPERUSER]), setRoleIdle7DToIdleUsers);
49+
router.put(
50+
"/group-idle",
51+
authorizeAndAuthenticate([ROLES.SUPERUSER], [Services.CRON_JOB_HANDLER]),
52+
setRoleIdleToIdleUsers
53+
);
54+
router.put(
55+
"/group-idle-7d",
56+
authorizeAndAuthenticate([ROLES.SUPERUSER], [Services.CRON_JOB_HANDLER]),
57+
setRoleIdle7DToIdleUsers
58+
);
4859
router.post(
4960
"/nicknames/sync",
50-
authenticate,
51-
authorizeRoles([SUPERUSER]),
61+
authorizeAndAuthenticate([ROLES.SUPERUSER], [Services.CRON_JOB_HANDLER]),
5262
checkIsVerifiedDiscord,
5363
updateDiscordNicknames
5464
);
5565
router.post("/nickname/status", verifyCronJob, validateUpdateUsersNicknameStatusBody, updateUsersNicknameStatus);
5666
router.post("/discord-roles", authenticate, authorizeRoles([SUPERUSER]), syncDiscordGroupRolesInFirestore);
5767
router.put(
5868
"/group-onboarding-31d-plus",
59-
authenticate,
60-
authorizeRoles([SUPERUSER]),
69+
authorizeAndAuthenticate([ROLES.SUPERUSER], [Services.CRON_JOB_HANDLER]),
6170
setRoleToUsersWith31DaysPlusOnboarding
6271
);
6372
module.exports = router;

routes/external-accounts.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,16 @@ const externalAccount = require("../controllers/external-accounts");
66
const authenticate = require("../middlewares/authenticate");
77
const authorizeRoles = require("../middlewares/authorizeRoles");
88
const { SUPERUSER } = require("../constants/roles");
9+
const ROLES = require("../constants/roles");
10+
const { Services } = require("../constants/bot");
11+
const { authorizeAndAuthenticate } = require("../middlewares/authorizeUsersAndService");
12+
913
router.post("/", validator.externalAccountData, authorizeBot.verifyDiscordBot, externalAccount.addExternalAccountData);
1014
router.get("/:token", authenticate, externalAccount.getExternalAccountData);
1115
router.patch("/discord-sync", authenticate, authorizeRoles([SUPERUSER]), externalAccount.syncExternalAccountData);
1216
router.post(
1317
"/users",
14-
authenticate,
15-
authorizeRoles([SUPERUSER]),
18+
authorizeAndAuthenticate([ROLES.SUPERUSER], [Services.CRON_JOB_HANDLER]),
1619
validator.postExternalAccountsUsers,
1720
externalAccount.externalAccountsUsersPostHandler
1821
);

routes/tasks.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ const {
1010
getUsersValidator,
1111
} = require("../middlewares/validators/tasks");
1212
const authorizeRoles = require("../middlewares/authorizeRoles");
13-
const { authorization } = require("../middlewares/authorizeUsersAndService");
13+
const { authorizeAndAuthenticate } = require("../middlewares/authorizeUsersAndService");
1414
const { APPOWNER, SUPERUSER } = require("../constants/roles");
1515
const assignTask = require("../middlewares/assignTask");
1616
const { cacheResponse, invalidateCache } = require("../utils/cache");
@@ -19,7 +19,10 @@ const { verifyCronJob } = require("../middlewares/authorizeBot");
1919
const { CLOUDFLARE_WORKER, CRON_JOB_HANDLER } = require("../constants/bot");
2020

2121
const oldAuthorizationMiddleware = authorizeRoles([APPOWNER, SUPERUSER]);
22-
const newAuthorizationMiddleware = authorization([APPOWNER, SUPERUSER], [CLOUDFLARE_WORKER, CRON_JOB_HANDLER]);
22+
const newAuthorizationMiddleware = authorizeAndAuthenticate(
23+
[APPOWNER, SUPERUSER],
24+
[CLOUDFLARE_WORKER, CRON_JOB_HANDLER]
25+
);
2326

2427
// Middleware to check if 'dev' query parameter is set to true
2528
const enableDevModeMiddleware = (req, res, next) => {

routes/userStatus.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,21 @@ const {
1717
validateMassUpdate,
1818
validateGetQueryParams,
1919
} = require("../middlewares/validators/userStatus");
20+
const { authorizeAndAuthenticate } = require("../middlewares/authorizeUsersAndService");
21+
const ROLES = require("../constants/roles");
22+
const { Services } = require("../constants/bot");
2023

2124
router.get("/", validateGetQueryParams, getUserStatusControllers);
2225
router.get("/self", authenticate, getUserStatus);
2326
router.get("/:userId", getUserStatus);
2427
router.patch("/self", authenticate, validateUserStatus, updateUserStatusController);
25-
router.patch("/update", authenticate, authorizeRoles([SUPERUSER]), updateAllUserStatus);
26-
router.patch("/batch", authenticate, authorizeRoles([SUPERUSER]), validateMassUpdate, batchUpdateUsersStatus);
28+
router.patch("/update", authorizeAndAuthenticate([ROLES.SUPERUSER], [Services.CRON_JOB_HANDLER]), updateAllUserStatus);
29+
router.patch(
30+
"/batch",
31+
authorizeAndAuthenticate([ROLES.SUPERUSER], [Services.CRON_JOB_HANDLER]),
32+
validateMassUpdate,
33+
batchUpdateUsersStatus
34+
);
2735
router.patch("/:userId", authenticate, authorizeRoles([SUPERUSER]), validateUserStatus, updateUserStatus);
2836
router.delete("/:userId", authenticate, authorizeRoles([SUPERUSER]), deleteUserStatus);
2937

routes/users.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@ const userValidator = require("../middlewares/validators/user");
88
const { upload } = require("../utils/multer");
99
const { getUserBadges } = require("../controllers/badges");
1010
const checkIsVerifiedDiscord = require("../middlewares/verifydiscord");
11+
const { authorizeAndAuthenticate } = require("../middlewares/authorizeUsersAndService");
12+
const ROLES = require("../constants/roles");
13+
const { Services } = require("../constants/bot");
1114

12-
router.post("/", authenticate, authorizeRoles([SUPERUSER]), users.markUnverified);
15+
router.post("/", authorizeAndAuthenticate([ROLES.SUPERUSER], [Services.CRON_JOB_HANDLER]), users.markUnverified);
1316
router.post("/update-in-discord", authenticate, authorizeRoles([SUPERUSER]), users.setInDiscordScript);
1417
router.post("/verify", authenticate, users.verifyUser);
1518
router.get("/userId/:userId", users.getUserById);

test/fixtures/discordactions/discordactions.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ const groupData = [
22
{ rolename: "Group 1", roleid: "1" },
33
{ rolename: "Group 2", roleid: "2" },
44
{ rolename: "Group 3", roleid: "3" },
5+
{ rolename: "admin", roleid: "4" },
6+
{ rolename: "group-test", roleid: "5" },
57
];
68

79
const groupIdle7d = { rolename: "group-idle-7d+", roleid: 4, createdBy: "1dad23q23j131j" };

test/integration/authorizeUsersAndService.test.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect } from "chai";
2-
import { authorization } from "../../middlewares/authorizeUsersAndService";
2+
import { authorizeAndAuthenticate } from "../../middlewares/authorizeUsersAndService";
33
import bot from "../utils/generateBotToken";
44
const userData = require("../fixtures/user/user")();
55
import authService from "../../services/authService";
@@ -41,46 +41,46 @@ describe("Middleware | Authorization", function () {
4141
});
4242
describe("Input validations", function () {
4343
it("should throw an error for invalid roles", function () {
44-
expect(() => authorization(["invalid_role"], [Services.CRON_JOB_HANDLER])).to.throw("Invalid role");
44+
expect(() => authorizeAndAuthenticate (["invalid_role"], [Services.CRON_JOB_HANDLER])).to.throw("Invalid role");
4545
});
4646

4747
it("should throw an error for invalid services", function () {
48-
expect(() => authorization([ROLES.APPOWNER], ["invalid_service"])).to.throw("Invalid service name");
48+
expect(() => authorizeAndAuthenticate ([ROLES.APPOWNER], ["invalid_service"])).to.throw("Invalid service name");
4949
});
5050
});
5151

5252
describe("Service Authorization", function () {
5353
it("should return unauthorized for invalid authorization header format", async function () {
5454
req.headers.authorization = "InvalidHeader";
5555

56-
await authorization([ROLES.APPOWNER], [Services.CRON_JOB_HANDLER])(req, res, next);
56+
await authorizeAndAuthenticate ([ROLES.APPOWNER], [Services.CRON_JOB_HANDLER])(req, res, next);
5757
expect(res.boom.unauthorized.calledOnce).to.be.equal(true);
5858
});
5959

6060
it("should return unauthorized for invalid JWT token", async function () {
6161
req.headers.authorization = "Bearer invalid_token";
62-
await authorization([ROLES.APPOWNER], [Services.CRON_JOB_HANDLER])(req, res, next);
62+
await authorizeAndAuthenticate ([ROLES.APPOWNER], [Services.CRON_JOB_HANDLER])(req, res, next);
6363
expect(res.boom.unauthorized.calledOnce).to.be.equal(true);
6464
});
6565

6666
it("should call verifyCronJob for valid cron job token", async function () {
6767
const jwtToken = bot.generateCronJobToken({ name: CRON_JOB_HANDLER });
6868
req.headers.authorization = `Bearer ${jwtToken}`;
69-
await authorization([ROLES.APPOWNER], [Services.CRON_JOB_HANDLER])(req, res, next);
69+
await authorizeAndAuthenticate ([ROLES.APPOWNER], [Services.CRON_JOB_HANDLER])(req, res, next);
7070
expect(next.calledOnce).to.be.equal(true);
7171
});
7272

7373
it("should call verifyDiscordBot for valid Discord bot token", async function () {
7474
const jwtToken = bot.generateToken({ name: CLOUDFLARE_WORKER });
7575
req.headers.authorization = `Bearer ${jwtToken}`;
76-
await authorization([ROLES.APPOWNER], [Services.CLOUDFLARE_WORKER])(req, res, next);
76+
await authorizeAndAuthenticate ([ROLES.APPOWNER], [Services.CLOUDFLARE_WORKER])(req, res, next);
7777
expect(next.calledOnce).to.be.equal(true);
7878
});
7979

8080
it("should return unauthorized for unknown service names", async function () {
8181
const jwtToken = bot.generateToken({ name: "Invalid name" });
8282
req.headers.authorization = `Bearer ${jwtToken}`;
83-
await authorization([ROLES.APPOWNER], [Services.CLOUDFLARE_WORKER])(req, res, next);
83+
await authorizeAndAuthenticate ([ROLES.APPOWNER], [Services.CLOUDFLARE_WORKER])(req, res, next);
8484
expect(res.boom.unauthorized.calledOnce).to.be.equal(true);
8585
});
8686
});
@@ -89,7 +89,7 @@ describe("Middleware | Authorization", function () {
8989
return res.json({ message: "pong" });
9090
};
9191

92-
router.get("/for-super-user", authorization([ROLES.SUPERUSER], [Services.CRON_JOB_HANDLER]), pongHandler);
92+
router.get("/for-super-user", authorizeAndAuthenticate ([ROLES.SUPERUSER], [Services.CRON_JOB_HANDLER]), pongHandler);
9393

9494
const app = express();
9595
AppMiddlewares(app);

test/integration/discordactions.test.js

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,12 @@ describe("Discord actions", function () {
210210
describe("POST /discord-actions/roles", function () {
211211
let roleid;
212212
beforeEach(async function () {
213-
const discordRoleModelPromise = [discordRoleModel.add(groupData[0]), discordRoleModel.add(groupData[1])];
213+
const discordRoleModelPromise = [
214+
discordRoleModel.add(groupData[0]),
215+
discordRoleModel.add(groupData[1]),
216+
discordRoleModel.add(groupData[3]),
217+
discordRoleModel.add(groupData[4]),
218+
];
214219
roleid = groupData[0].roleid;
215220
await Promise.all(discordRoleModelPromise);
216221
});
@@ -219,19 +224,52 @@ describe("Discord actions", function () {
219224
sinon.restore();
220225
await cleanDb();
221226
});
227+
it("should not be able to add role if it is not a group type role", async function () {
228+
const fetchStub = sinon.stub(discordRolesModel, "isGroupRoleExists");
229+
230+
fetchStub.returns(
231+
{
232+
roleExists: true,
233+
existingRoles: {
234+
docs: [
235+
{
236+
data: () => ({
237+
date: new Date().toISOString(),
238+
createdBy: "CzI06Da1zPwciLcyIwU4",
239+
roleid: "1214641424516124823",
240+
rolename: "admin",
241+
}),
242+
},
243+
],
244+
},
245+
},
246+
{ user: userData[0] }
247+
);
248+
249+
const res = await chai
250+
.request(app)
251+
.post("/discord-actions/roles")
252+
.set("cookie", `${cookieName}=${jwt}`)
253+
.send({ roleid, userid: userData[0].discordId });
254+
expect(res).to.have.status(403);
255+
expect(res.body).to.be.an("object");
256+
expect(res.body.message).to.equal("Cannot use rolename that is not a group role");
222257

258+
fetchStub.restore();
259+
});
223260
it("should allow role to be added", async function () {
224261
fetchStub.returns(
225262
Promise.resolve({
226263
status: 200,
227264
json: () => Promise.resolve({}),
228265
})
229266
);
267+
const roleId = groupData[4].roleid;
230268
const res = await chai
231269
.request(app)
232270
.post("/discord-actions/roles")
233271
.set("cookie", `${cookieName}=${jwt}`)
234-
.send({ roleid, userid: userData[0].discordId });
272+
.send({ roleid: roleId, userid: userData[0].discordId });
235273

236274
expect(res).to.have.status(201);
237275
expect(res.body).to.be.an("object");
@@ -245,7 +283,8 @@ describe("Discord actions", function () {
245283
})
246284
);
247285

248-
const body = { roleid, userid: userData[0].discordId };
286+
const roleId = groupData[4].roleid;
287+
const body = { roleid: roleId, userid: userData[0].discordId };
249288
const res = await chai
250289
.request(app)
251290
.post("/discord-actions/roles")

0 commit comments

Comments
 (0)