Refactor controller functions to use service layer for retrieving user data (/users/userId/:userId) route (/users/self) route (#1249)

* removal of sensitive info(phone,email,tokens,chaincode) on /users route

* update in removal of sensitive info(phone,email,tokens,chaincode) on /users route

* Update dataAccessLayer.js

* updated query id and qualifiers params in /users in removal of sensitive info(phone,email,tokens,chaincode) on /users route

* updated dataAccessLayer for lint issues

* Update users.js

* updated delete statement to hasOwnProperty

* updated default arguments to function removeSensitiveInfo

* made updates in the dataAccessLayer

* Update users.js

* typo fixes

* Added unit tests

* updated to using in operator

* updated to query as argument in place of req

* updates to /user/userId

* added code changes for /self route

* remove extra lines

* Update users.js

* removed commented lines

* lint error fix

* Update users.js

---------

Co-authored-by: Sen, Sriza <[email protected]>

Author: mailmesriza98

controllers/users.js

@@ -7,7 +7,6 @@ const { profileDiffStatus } = require("../constants/profileDiff");
 const { logType } = require("../constants/logs");
 const dataAccess = require("../services/dataAccessLayer");
 const logger = require("../utils/logger");
-const obfuscate = require("../utils/obfuscate");
 const { SOMETHING_WENT_WRONG, INTERNAL_SERVER_ERROR } = require("../constants/errorMessages");
 const { getPaginationLink, getUsernamesFromPRs, getRoleToUpdate } = require("../utils/users");
 const { setInDiscordFalseScript } = require("../services/discordService");
@@ -39,9 +38,10 @@ const verifyUser = async (req, res) => {
 };
 
 const getUserById = async (req, res) => {
-  let result;
+  let result, user;
   try {
-    result = await userQuery.fetchUser({ userId: req.params.userId });
+    result = await dataAccess.retrieveUsers({ id: req.params.userId });
+    user = result.user;
   } catch (error) {
     logger.error(`Error while fetching user: ${error}`);
     return res.boom.serverUnavailable(SOMETHING_WENT_WRONG);
@@ -51,15 +51,6 @@ const getUserById = async (req, res) => {
     return res.boom.notFound("User doesn't exist");
   }
 
-  const { phone = "", email = "", ...user } = result.user;
-  try {
-    user.phone = obfuscate.obfuscatePhone(phone);
-    user.email = obfuscate.obfuscateMail(email);
-  } catch (error) {
-    logger.error(`Error while formatting phone and email: ${error}`);
-    return res.boom.badImplementation("Error while formatting phone and email");
-  }
-
   return res.json({
     message: "User returned successfully!",
     user,
@@ -82,17 +73,17 @@ const getUsers = async (req, res) => {
     // getting user details by id if present.
     if (req.query.id) {
       const id = req.query.id;
-      let result;
+      let result, user;
       try {
         result = await dataAccess.retrieveUsers({ id: id });
+        user = result.user;
       } catch (error) {
         logger.error(`Error while fetching user: ${error}`);
         return res.boom.serverUnavailable(SOMETHING_WENT_WRONG);
       }
       if (!result.userExists) {
         return res.boom.notFound("User doesn't exist");
       }
-      const user = result.user;
       return res.json({
         message: "User returned successfully!",
         user,
@@ -212,14 +203,14 @@ const getUsernameAvailabilty = async (req, res) => {
  * @param res {Object} - Express response object
  */
 
-const getSelfDetails = (req, res) => {
+const getSelfDetails = async (req, res) => {
   try {
     if (req.userData) {
       if (req.query.private) {
         return res.send(req.userData);
       }
-      const { phone, email, ...userData } = req.userData;
-      return res.send(userData);
+      const user = await dataAccess.retrieveUsers({ userdata: req.userData });
+      return res.send(user);
     }
     return res.boom.notFound("User doesn't exist");
   } catch (error) {
@@ -563,8 +554,8 @@ const filterUsers = async (req, res) => {
     if (!Object.keys(req.query).length) {
       return res.boom.badRequest("filter for item not provided");
     }
-    const users = await dataAccess.retreiveFilteredUsers(req.query);
 
+    const users = await dataAccess.retreiveFilteredUsers(req.query);
     return res.json({
       message: users.length ? "Users found successfully!" : "No users found",
       users: users,

services/dataAccessLayer.js

@@ -1,7 +1,7 @@
 const userQuery = require("../models/users");
 const { USER_SENSITIVE_DATA } = require("../constants/users");
 
-const retrieveUsers = async ({ id = null, username = null, usernames = null, query = null }) => {
+const retrieveUsers = async ({ id = null, username = null, usernames = null, query = null, userdata }) => {
   if (id || username) {
     let result;
     if (id != null) {
@@ -17,12 +17,15 @@ const retrieveUsers = async ({ id = null, username = null, usernames = null, que
       removeSensitiveInfo(element);
     });
     return users;
-  } else {
+  } else if (query) {
     const { allUsers, nextId, prevId } = await userQuery.fetchPaginatedUsers(query);
     allUsers.forEach((element) => {
       removeSensitiveInfo(element);
     });
     return { allUsers, nextId, prevId };
+  } else {
+    removeSensitiveInfo(userdata);
+    return userdata;
   }
 };
 

test/fixtures/user/removalData.js

@@ -0,0 +1,3 @@
+module.exports = () => {
+  return ["phone", "emails", "tokens", "chaincode"];
+};

test/integration/users.test.js

@@ -175,6 +175,10 @@ describe("Users", function () {
           userData.forEach((user) => {
             expect(user.roles.archived).to.equal(false);
           });
+          expect(res.body.users[0]).to.not.have.property("phone");
+          expect(res.body.users[0]).to.not.have.property("email");
+          expect(res.body.users[0]).to.not.have.property("tokens");
+          expect(res.body.users[0]).to.not.have.property("chaincode");
           return done();
         });
     });
@@ -410,7 +414,8 @@ describe("Users", function () {
           expect(res.body).to.be.a("object");
           expect(res.body).to.not.have.property("phone");
           expect(res.body).to.not.have.property("email");
-
+          expect(res.body).to.not.have.property("tokens");
+          expect(res.body).to.not.have.property("chaincode");
           return done();
         });
     });
@@ -513,6 +518,11 @@ describe("Users", function () {
           expect(res.body).to.be.a("object");
           expect(res.body.message).to.equal("User returned successfully!");
           expect(res.body.user).to.be.a("object");
+          expect(res.body.user).to.not.have.property("phone");
+          expect(res.body.user).to.not.have.property("email");
+          expect(res.body.user).to.not.have.property("tokens");
+          expect(res.body.user).to.not.have.property("chaincode");
+
           return done();
         });
     });

test/unit/services/dataAccessLayer.test.js

@@ -6,6 +6,7 @@ const sinon = require("sinon");
 const { retrieveUsers, removeSensitiveInfo, retreiveFilteredUsers } = require("../../../services/dataAccessLayer");
 const userData = require("../../fixtures/user/user")();
 const { USER_SENSITIVE_DATA } = require("../../../constants/users");
+
 chai.use(chaiHttp);
 const expect = chai.expect;
 let fetchUserStub;
@@ -58,6 +59,15 @@ describe("Data Access Layer", function () {
         });
       });
     });
+
+    it("should return /users/self data and remove sensitive info", async function () {
+      const userdata = userData[12];
+      await retrieveUsers({ userdata });
+      removeSensitiveInfo(userData[12]);
+      USER_SENSITIVE_DATA.forEach((key) => {
+        expect(userData[12]).to.not.have.property(key);
+      });
+    });
   });
 
   describe("retrieveFilteredUsers", function () {