Skip to content

Commit 21d306b

Browse files
authored
Merge pull request #1363 from Real-Dev-Squad/feat/issue1314
2 parents efbd6a9 + 03437e6 commit 21d306b

File tree

11 files changed

+159
-104
lines changed

11 files changed

+159
-104
lines changed

constants/userDataLevels.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
const ACCESS_LEVEL = {
2+
PUBLIC: "public",
3+
INTERNAL: "internal",
4+
PRIVATE: "private",
5+
CONFIDENTIAL: "confidential",
6+
};
7+
8+
const ROLE_LEVEL = {
9+
private: ["super_user"],
10+
internal: ["super_user"],
11+
confidential: ["super_user"],
12+
};
13+
14+
const KEYS_NOT_ALLOWED = {
15+
public: ["email", "phone", "chaincode"],
16+
internal: ["phone", "chaincode"],
17+
private: ["chaincode"],
18+
confidential: [],
19+
};
20+
21+
module.exports = { ACCESS_LEVEL, KEYS_NOT_ALLOWED, ROLE_LEVEL };

constants/users.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ const profileStatus = {
44
NOT_APPROVED: "NOT APPROVED",
55
};
66

7-
const USER_SENSITIVE_DATA = ["phone", "email", "chaincode", "tokens"];
8-
97
const USER_STATUS = {
108
OOO: "ooo",
119
IDLE: "idle",
@@ -23,5 +21,4 @@ module.exports = {
2321
profileStatus,
2422
USER_STATUS,
2523
ALLOWED_FILTER_PARAMS,
26-
USER_SENSITIVE_DATA,
2724
};

controllers/users.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,10 @@ const getUsers = async (req, res) => {
101101
}
102102

103103
const data = await dataAccess.retrieveUsers({ query: req.query });
104+
104105
return res.json({
105106
message: "Users returned successfully!",
106-
users: data.allUsers,
107+
users: data.users,
107108
links: {
108109
next: data.nextId ? getPaginationLink(req.query, "next", data.nextId) : "",
109110
prev: data.prevId ? getPaginationLink(req.query, "prev", data.prevId) : "",
@@ -205,10 +206,9 @@ const getUsernameAvailabilty = async (req, res) => {
205206
const getSelfDetails = async (req, res) => {
206207
try {
207208
if (req.userData) {
208-
if (req.query.private) {
209-
return res.send(req.userData);
210-
}
211-
const user = await dataAccess.retrieveUsers({ userdata: req.userData });
209+
const user = await dataAccess.retrieveUsers({
210+
userdata: req.userData,
211+
});
212212
return res.send(user);
213213
}
214214
return res.boom.notFound("User doesn't exist");
@@ -407,6 +407,7 @@ const updateUser = async (req, res) => {
407407
const generateChaincode = async (req, res) => {
408408
try {
409409
const { id } = req.userData;
410+
410411
const chaincode = await chaincodeQuery.storeChaincode(id);
411412
await userQuery.addOrUpdate({ chaincode }, id);
412413
return res.json({

middlewares/authenticate.js

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
const authService = require("../services/authService");
2-
const users = require("../models/users");
2+
const dataAccess = require("../services/dataAccessLayer");
33

44
/**
55
* Middleware to check if the user has been restricted. If user is restricted,
@@ -54,7 +54,7 @@ module.exports = async (req, res, next) => {
5454
const { userId } = authService.verifyAuthToken(token);
5555

5656
// add user data to `req.userData` for further use
57-
const userData = await users.fetchUser({ userId });
57+
const userData = await dataAccess.retrieveUsers({ id: userId });
5858
req.userData = userData.user;
5959

6060
return checkRestricted(req, res, next);
@@ -79,8 +79,7 @@ module.exports = async (req, res, next) => {
7979
});
8080

8181
// add user data to `req.userData` for further use
82-
req.userData = await users.fetchUser({ userId });
83-
82+
req.userData = await dataAccess.retrieveUsers({ id: userId });
8483
return checkRestricted(req, res, next);
8584
} else {
8685
return res.boom.unauthorized("Unauthenticated User");

services/dataAccessLayer.js

Lines changed: 42 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
const userQuery = require("../models/users");
22
const members = require("../models/members");
3-
const { USER_SENSITIVE_DATA } = require("../constants/users");
3+
const { ROLE_LEVEL, KEYS_NOT_ALLOWED, ACCESS_LEVEL } = require("../constants/userDataLevels");
44

55
const retrieveUsers = async ({
66
id = null,
77
username = null,
88
usernames = null,
99
query = null,
1010
userdata,
11+
level = ACCESS_LEVEL.PUBLIC,
12+
role = null,
1113
userIds = [],
1214
}) => {
1315
if (id || username) {
@@ -17,79 +19,93 @@ const retrieveUsers = async ({
1719
} else {
1820
result = await userQuery.fetchUser({ username: username });
1921
}
20-
removeSensitiveInfo(result.user);
22+
const user = levelSpecificAccess(result.user, level, role);
23+
result.user = user;
2124
return result;
2225
} else if (usernames) {
2326
const { users } = await userQuery.fetchUsers(usernames);
24-
users.forEach((element) => {
25-
removeSensitiveInfo(element);
27+
const result = [];
28+
users.forEach((userdata) => {
29+
const user = levelSpecificAccess(userdata, level, role);
30+
result.push(user);
2631
});
27-
return users;
32+
return result;
2833
} else if (userIds.length > 0) {
2934
const userDetails = await userQuery.fetchUserByIds(userIds);
30-
3135
Object.keys(userDetails).forEach((userId) => {
3236
removeSensitiveInfo(userDetails[userId]);
3337
});
34-
3538
return userDetails;
3639
} else if (query) {
3740
const { allUsers, nextId, prevId } = await userQuery.fetchPaginatedUsers(query);
38-
allUsers.forEach((element) => {
39-
removeSensitiveInfo(element);
41+
const users = [];
42+
allUsers.forEach((userdata) => {
43+
const user = levelSpecificAccess(userdata, level, role);
44+
users.push(user);
4045
});
41-
return { allUsers, nextId, prevId };
46+
return { users, nextId, prevId };
4247
} else {
43-
removeSensitiveInfo(userdata);
44-
return userdata;
48+
const result = await userQuery.fetchUser({ userId: userdata.id });
49+
return levelSpecificAccess(result.user, level, role);
4550
}
4651
};
4752

48-
const retrieveDiscordUsers = async () => {
53+
const retrieveDiscordUsers = async (level = ACCESS_LEVEL.PUBLIC, role = null) => {
4954
const users = await userQuery.getDiscordUsers();
50-
users.forEach((element) => {
51-
removeSensitiveInfo(element);
55+
const usersData = [];
56+
users.forEach((userdata) => {
57+
const user = levelSpecificAccess(userdata, level, role);
58+
usersData.push(user);
5259
});
53-
return users;
60+
return usersData;
5461
};
5562

5663
const retreiveFilteredUsers = async (query) => {
5764
const users = await userQuery.getUsersBasedOnFilter(query);
58-
users.forEach((element) => {
59-
removeSensitiveInfo(element);
65+
users.forEach((userdata) => {
66+
removeSensitiveInfo(userdata);
6067
});
6168
return users;
6269
};
6370

6471
const retrieveMembers = async (query) => {
6572
const allUsers = await members.fetchUsers(query);
66-
allUsers.forEach((element) => {
67-
removeSensitiveInfo(element);
73+
allUsers.forEach((userdata) => {
74+
removeSensitiveInfo(userdata);
6875
});
6976
return allUsers;
7077
};
7178

7279
const retrieveUsersWithRole = async (role) => {
7380
const users = await members.fetchUsersWithRole(role);
74-
users.forEach((element) => {
75-
removeSensitiveInfo(element);
81+
users.forEach((userdata) => {
82+
removeSensitiveInfo(userdata);
7683
});
7784
return users;
7885
};
7986

80-
const removeSensitiveInfo = function (obj) {
81-
for (let i = 0; i < USER_SENSITIVE_DATA.length; i++) {
82-
if (Object.prototype.hasOwnProperty.call(obj, USER_SENSITIVE_DATA[i])) {
83-
delete obj[USER_SENSITIVE_DATA[i]];
87+
const removeSensitiveInfo = function (obj, level = ACCESS_LEVEL.PUBLIC) {
88+
for (let i = 0; i < KEYS_NOT_ALLOWED[level].length; i++) {
89+
if (Object.prototype.hasOwnProperty.call(obj, KEYS_NOT_ALLOWED[level][i])) {
90+
delete obj[KEYS_NOT_ALLOWED[level][i]];
8491
}
8592
}
8693
};
8794

95+
const levelSpecificAccess = (user, level = ACCESS_LEVEL.PUBLIC, role = null) => {
96+
if (level === ACCESS_LEVEL.PUBLIC || ROLE_LEVEL[level].includes(role)) {
97+
removeSensitiveInfo(user, level);
98+
return user;
99+
}
100+
return "unauthorized";
101+
};
102+
88103
module.exports = {
89104
retrieveUsers,
90105
removeSensitiveInfo,
91106
retrieveDiscordUsers,
92107
retrieveMembers,
93108
retrieveUsersWithRole,
94109
retreiveFilteredUsers,
110+
levelSpecificAccess,
95111
};

test/fixtures/user/user.js

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -267,12 +267,6 @@ module.exports = () => {
267267
linkedin_id: "testuser1",
268268
github_id: "testuser1",
269269
github_display_name: "Test User",
270-
phone: "1234567890",
271-
272-
chaincode: "1234",
273-
tokens: {
274-
githubAccessToken: "githubAccessToken",
275-
},
276270
roles: {
277271
member: true,
278272
},
@@ -329,6 +323,29 @@ module.exports = () => {
329323
twitter_id: "ramsingh123",
330324
linkedin_id: "ramsingh123",
331325
},
326+
{
327+
username: "testuser3",
328+
first_name: "test3",
329+
last_name: "user3",
330+
yoe: 1,
331+
img: "./img.png",
332+
linkedin_id: "testuser1",
333+
github_id: "testuser",
334+
github_display_name: "Test User 3",
335+
phone: "1234567890",
336+
337+
chaincode: "12345",
338+
tokens: {
339+
githubAccessToken: "githubAccessToken",
340+
},
341+
roles: {
342+
member: true,
343+
},
344+
picture: {
345+
publicId: "profile/mtS4DhUvNYsKqI7oCWVB/aenklfhtjldc5ytei3ar",
346+
url: "https://res.cloudinary.com/realdevsquad/image/upload/v1667685133/profile/mtS4DhUvNYsKqI7oCWVB/aenklfhtjldc5ytei3ar.jpg",
347+
},
348+
},
332349
{
333350
username: "sahsisunny",
334351
first_name: "sunny",

test/integration/tasks.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -767,7 +767,7 @@ describe("Tasks", function () {
767767
});
768768

769769
it("Should return Forbidden error if task is not assigned to self", async function () {
770-
const { userId } = await addUser(userData[0]);
770+
const userId = await addUser(userData[0]);
771771
const jwt = authService.generateAuthToken({ userId });
772772

773773
const res = await chai.request(app).patch(`/tasks/self/${taskId1}`).set("cookie", `${cookieName}=${jwt}`);

test/integration/users.test.js

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,6 @@ describe("Users", function () {
290290
expect(res.body.users).to.be.a("array");
291291
expect(res.body.users[0]).to.not.have.property("phone");
292292
expect(res.body.users[0]).to.not.have.property("email");
293-
expect(res.body.users[0]).to.not.have.property("tokens");
294293
expect(res.body.users[0]).to.not.have.property("chaincode");
295294

296295
return done();
@@ -315,7 +314,6 @@ describe("Users", function () {
315314
});
316315
expect(res.body.users[0]).to.not.have.property("phone");
317316
expect(res.body.users[0]).to.not.have.property("email");
318-
expect(res.body.users[0]).to.not.have.property("tokens");
319317
expect(res.body.users[0]).to.not.have.property("chaincode");
320318
return done();
321319
});
@@ -341,7 +339,6 @@ describe("Users", function () {
341339
expect(res.body.users.length).to.equal(1);
342340
expect(res.body.users[0]).to.not.have.property("phone");
343341
expect(res.body.users[0]).to.not.have.property("email");
344-
expect(res.body.users[0]).to.not.have.property("tokens");
345342
expect(res.body.users[0]).to.not.have.property("chaincode");
346343
return done();
347344
});
@@ -552,31 +549,11 @@ describe("Users", function () {
552549
expect(res.body).to.be.a("object");
553550
expect(res.body).to.not.have.property("phone");
554551
expect(res.body).to.not.have.property("email");
555-
expect(res.body).to.not.have.property("tokens");
556552
expect(res.body).to.not.have.property("chaincode");
557553
return done();
558554
});
559555
});
560556

561-
it("Should return details with phone and email when query 'private' is true", function (done) {
562-
chai
563-
.request(app)
564-
.get("/users/self")
565-
.query({ private: true })
566-
.set("cookie", `${cookieName}=${jwt}`)
567-
.end((err, res) => {
568-
if (err) {
569-
return done();
570-
}
571-
572-
expect(res).to.have.status(200);
573-
expect(res.body).to.be.a("object");
574-
expect(res.body).to.have.property("phone");
575-
expect(res.body).to.have.property("email");
576-
return done();
577-
});
578-
});
579-
580557
it("Should return 401 if not logged in", function (done) {
581558
chai
582559
.request(app)
@@ -616,7 +593,6 @@ describe("Users", function () {
616593
expect(res.body.user).to.be.a("object");
617594
expect(res.body.user).to.not.have.property("phone");
618595
expect(res.body.user).to.not.have.property("email");
619-
expect(res.body.user).to.not.have.property("tokens");
620596
expect(res.body.user).to.not.have.property("chaincode");
621597
return done();
622598
});
@@ -658,7 +634,6 @@ describe("Users", function () {
658634
expect(res.body.user).to.be.a("object");
659635
expect(res.body.user).to.not.have.property("phone");
660636
expect(res.body.user).to.not.have.property("email");
661-
expect(res.body.user).to.not.have.property("tokens");
662637
expect(res.body.user).to.not.have.property("chaincode");
663638
return done();
664639
});

test/integration/usersFilter.test.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,6 @@ describe("Filter Users", function () {
365365
res.body.users.forEach((user) => {
366366
expect(user).to.not.have.property("phone");
367367
expect(user).to.not.have.property("email");
368-
expect(user).to.not.have.property("tokens");
369368
});
370369
return done();
371370
});

test/unit/models/users.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ describe("users", function () {
9494
});
9595

9696
it("It should have created_At and updated_At fields", async function () {
97-
const userData = userDataArray[14];
97+
const userData = userDataArray[15];
9898
await users.addOrUpdate(userData);
9999
const githubUsername = "sahsisunny";
100100
const { user, userExists } = await users.fetchUser({ githubUsername });
@@ -279,7 +279,7 @@ describe("users", function () {
279279
});
280280
it("returns users with member role", async function () {
281281
const members = await users.getUsersByRole("member");
282-
expect(members.length).to.be.equal(6);
282+
expect(members.length).to.be.equal(7);
283283
members.forEach((member) => {
284284
expect(member.roles.member).to.be.equal(true);
285285
});

0 commit comments

Comments
 (0)