Stop collecting github access token and remove it from existing user's data (#1227)

* wrote scripts and tests

* forgot to add this file

* fixing test errors

* implemented batch instead of promise.all to send update request to firestore

* fixed test and resolved comments

* fixed tests

* forgot to fix this

* forgot to fix this

* preventing object sink

* preventing object sink

* fixing function call object sink warning

* ignore object sink

* resolved comments by randhir

* resolved comments

* fixing tests

Author: fakhruddinkw

controllers/auth.js

@@ -26,9 +26,6 @@ const githubAuth = (req, res, next) => {
       userData = {
         github_id: user.username,
         github_display_name: user.displayName,
-        tokens: {
-          githubAccessToken: accessToken,
-        },
       };
 
       const { userId, incompleteUserDetails } = await users.addOrUpdate(userData);

controllers/users.js

@@ -577,7 +577,26 @@ const setInDiscordScript = async (req, res) => {
     await setInDiscordFalseScript();
     return res.json({ message: "Successfully added the in_discord field to false for all users" });
   } catch (err) {
-    return res.status(500).json({ message: INTERNAL_SERVER_ERROR });
+    return res.boom.badImplementation({ message: INTERNAL_SERVER_ERROR });
+  }
+};
+
+const removeTokens = async (req, res) => {
+  try {
+    const users = await userQuery.fetchUsersWithToken();
+
+    if (!users.length) {
+      return res.status(404).json({ message: "No users found with github Token!" });
+    }
+
+    await userQuery.removeGitHubToken(users);
+
+    return res.status(200).json({
+      message: "Github Token removed from all users!",
+      usersFound: users.length,
+    });
+  } catch (err) {
+    return res.boom.badImplementation({ message: INTERNAL_SERVER_ERROR });
   }
 };
 
@@ -628,5 +647,6 @@ module.exports = {
   nonVerifiedDiscordUsers,
   setInDiscordScript,
   markUnverified,
+  removeTokens,
   updateRoles,
 };

models/users.js

@@ -572,6 +572,56 @@ const fetchAllUsers = async () => {
   return users;
 };
 
+const fetchUsersWithToken = async () => {
+  try {
+    const users = [];
+    const usersRef = await userModel.where("tokens", "!=", false).get();
+    usersRef.forEach((user) => {
+      users.push(userModel.doc(user.id));
+    });
+    return users;
+  } catch (err) {
+    logger.error(`Error while fetching all users with tokens field: ${err}`);
+    throw err;
+  }
+};
+
+const removeGitHubToken = async (users) => {
+  try {
+    const length = users.length;
+
+    let numberOfBatches = length / 500;
+    const remainder = length % 500;
+
+    if (remainder) {
+      numberOfBatches = numberOfBatches + 1;
+    }
+
+    const batchArray = [];
+    for (let i = 0; i < numberOfBatches; i++) {
+      const batch = firestore.batch();
+      batchArray.push(batch);
+    }
+
+    let batchIndex = 0;
+    let operations = 0;
+
+    for (let i = 0; i < length; i++) {
+      batchArray[batchIndex].update(users[i], { tokens: admin.firestore.FieldValue.delete() });
+      operations++;
+
+      if (operations === 500) {
+        batchIndex++;
+        operations = 0;
+      }
+    }
+
+    await Promise.all(batchArray.map(async (batch) => await batch.commit()));
+  } catch (err) {
+    logger.error(`Error while deleting tokens field: ${err}`);
+    throw err;
+  }
+};
 module.exports = {
   addOrUpdate,
   fetchPaginatedUsers,
@@ -592,4 +642,6 @@ module.exports = {
   getUserImageForVerification,
   getDiscordUsers,
   fetchAllUsers,
+  fetchUsersWithToken,
+  removeGitHubToken,
 };

routes/users.js

@@ -19,6 +19,8 @@ router.get("/self", authenticate, users.getSelfDetails);
 router.get("/isUsernameAvailable/:username", authenticate, users.getUsernameAvailabilty);
 router.get("/chaincode", authenticate, users.generateChaincode);
 router.get("/search", userValidator.validateUserQueryParams, users.filterUsers);
+router.patch("/remove-tokens", authenticate, authorizeRoles([SUPERUSER]), users.removeTokens);
+
 router.get("/:username", users.getUser);
 router.get("/:userId/intro", authenticate, authorizeRoles([SUPERUSER]), users.getUserIntro);
 router.put("/self/intro", authenticate, userValidator.validateJoinData, users.addUserIntro);

test/integration/users.test.js

@@ -37,6 +37,7 @@ const { userPhotoVerificationData } = require("../fixtures/user/photo-verificati
 const Sinon = require("sinon");
 const { INTERNAL_SERVER_ERROR } = require("../../constants/errorMessages");
 const photoVerificationModel = firestore.collection("photo-verification");
+
 chai.use(chaiHttp);
 
 describe("Users", function () {
@@ -1502,4 +1503,31 @@ describe("Users", function () {
       });
     });
   });
+
+  describe("PATCH /users/remove-tokens", function () {
+    before(async function () {
+      await addOrUpdate(userData[0]);
+      await addOrUpdate(userData[1]);
+      await addOrUpdate(userData[2]);
+      await addOrUpdate(userData[3]);
+    });
+    after(async function () {
+      await cleanDb();
+    });
+    it("should remove all the users with token field", function (done) {
+      chai
+        .request(app)
+        .patch("/users/remove-tokens")
+        .set("Cookie", `${cookieName}=${superUserAuthToken}`)
+        .end((err, res) => {
+          if (err) {
+            return done(err);
+          }
+          expect(res).to.have.status(200);
+          expect(res.body.message).to.be.equal("Github Token removed from all users!");
+          expect(res.body.usersFound).to.be.equal(3);
+          return done();
+        });
+    });
+  });
 });

test/unit/models/users.test.js

@@ -220,4 +220,41 @@ describe("users", function () {
       expect(data[0]).to.have.all.keys([...Object.keys(joinData[0]), "id"]);
     });
   });
+
+  describe("remove github token from users", function () {
+    beforeEach(async function () {
+      const addUsersPromises = [];
+      userDataArray.forEach((user) => {
+        addUsersPromises.push(userModel.add(user));
+      });
+      await Promise.all(addUsersPromises);
+    });
+
+    afterEach(async function () {
+      await cleanDb();
+    });
+
+    it("return array of users", async function () {
+      const data = await users.fetchUsersWithToken();
+      expect(data).to.be.not.equal(null);
+    });
+    it('removes token field from user"s data', async function () {
+      const userRef = await users.fetchUsersWithToken();
+      const dataBefore = await userRef[1].get();
+      const beforeRemoval = Object.keys(dataBefore.data()).includes("tokens");
+      expect(beforeRemoval).to.be.equal(true);
+      await users.removeGitHubToken(userRef);
+      const dataAfter = await userRef[1].get();
+      const afterRemoval = Object.keys(dataAfter.data()).includes("tokens");
+      expect(afterRemoval).to.be.equal(false);
+    });
+
+    it("throws error if id is not found in db", async function () {
+      try {
+        await users.removeGitHubToken("1223");
+      } catch (error) {
+        expect(error).to.be.instanceOf(Error);
+      }
+    });
+  });
 });