diff --git a/controllers/users.js b/controllers/users.js index 88abb1eda..6cbebe7b4 100644 --- a/controllers/users.js +++ b/controllers/users.js @@ -36,6 +36,7 @@ const config = require("config"); const { generateUniqueUsername } = require("../services/users"); const userService = require("../services/users"); const discordDeveloperRoleId = config.get("discordDeveloperRoleId"); +const usersCollection = firestore.collection("users"); const verifyUser = async (req, res) => { const userId = req.userData.id; @@ -714,6 +715,12 @@ const updateUser = async (req, res) => { const { id: profileDiffId, message } = req.body; const devFeatureFlag = req.query.dev === "true"; let profileDiffData; + + const userDoc = await usersCollection.doc(req.params.userId).get(); + if (!userDoc.exists) { + return res.boom.notFound("The User doesn't exist."); + } + if (devFeatureFlag) { profileDiffData = await profileDiffsQuery.fetchProfileDiffUnobfuscated(profileDiffId); } else { @@ -1096,6 +1103,26 @@ const updateUsernames = async (req, res) => { } }; +const updateProfile = async (req, res) => { + try { + const { id: currentUserId, roles = {} } = req.userData; + const isSelf = req.params.userId === currentUserId; + const isSuperUser = roles[ROLES.SUPERUSER]; + const profile = req.query.profile === "true"; + + if (isSelf && profile && req.query.dev === "true") { + return await updateSelf(req, res); + } else if (isSuperUser) { + return await updateUser(req, res); + } + + return res.boom.badRequest("Invalid Request."); + } catch (err) { + logger.error(`Error in updateUserStatusController: ${err}`); + return res.boom.badImplementation("An unexpected error occurred."); + } +}; + module.exports = { verifyUser, generateChaincode, @@ -1128,4 +1155,5 @@ module.exports = { isDeveloper, getIdentityStats, updateUsernames, + updateProfile, }; diff --git a/middlewares/conditionalMiddleware.ts b/middlewares/conditionalMiddleware.ts new file mode 100644 index 000000000..3d1249e80 --- /dev/null +++ b/middlewares/conditionalMiddleware.ts @@ -0,0 +1,10 @@ +const conditionalMiddleware = (validator) => { + return async (req, res, next) => { + if (req.params.userId === req.userData.id && req.query.profile === "true") { + return validator(req, res, next); + } + next(); + }; +}; + +module.exports = conditionalMiddleware; diff --git a/routes/users.js b/routes/users.js index e38c2ca31..921b139f6 100644 --- a/routes/users.js +++ b/routes/users.js @@ -14,12 +14,13 @@ const { Services } = require("../constants/bot"); const authenticateProfile = require("../middlewares/authenticateProfile"); const { devFlagMiddleware } = require("../middlewares/devFlag"); const { userAuthorization } = require("../middlewares/userAuthorization"); +const conditionalMiddleware = require("../middlewares/conditionalMiddleware"); router.post("/", authorizeAndAuthenticate([ROLES.SUPERUSER], [Services.CRON_JOB_HANDLER]), users.markUnverified); router.post("/update-in-discord", authenticate, authorizeRoles([SUPERUSER]), users.setInDiscordScript); router.post("/verify", authenticate, users.verifyUser); router.get("/userId/:userId", users.getUserById); -router.patch("/self", authenticate, userValidator.updateUser, users.updateSelf); +router.patch("/self", authenticate, userValidator.updateUser, users.updateSelf); // this route is being deprecated soon, please use alternate available `/users/:userId?profile=true` PATCH endpoint. router.get("/", authenticateProfile(authenticate), userValidator.getUsers, users.getUsers); router.get("/self", authenticate, users.getSelfDetails); router.get("/isDeveloper", authenticate, users.isDeveloper); @@ -75,7 +76,7 @@ router.patch( router.get("/picture/:id", authenticate, authorizeRoles([SUPERUSER]), users.getUserImageForVerification); router.patch("/profileURL", authenticate, userValidator.updateProfileURL, users.profileURL); router.patch("/rejectDiff", authenticate, authorizeRoles([SUPERUSER]), users.rejectProfileDiff); -router.patch("/:userId", authenticate, authorizeRoles([SUPERUSER]), users.updateUser); +router.patch("/:userId", authenticate, conditionalMiddleware(userValidator.updateUser), users.updateProfile); router.get("/suggestedUsers/:skillId", authenticate, authorizeRoles([SUPERUSER]), users.getSuggestedUsers); module.exports = router; router.post("/batch-username-update", authenticate, authorizeRoles([SUPERUSER]), users.updateUsernames); diff --git a/test/integration/users.test.js b/test/integration/users.test.js index f607e0d61..8ba547d3f 100644 --- a/test/integration/users.test.js +++ b/test/integration/users.test.js @@ -1759,16 +1759,6 @@ describe("Users", function () { .set("cookie", `${cookieName}=${superUserAuthToken}`) .send({ id: `${profileDiffsId}`, - first_name: "Ankur", - last_name: "Narkhede", - yoe: 0, - company: "", - designation: "AO", - github_id: "ankur1337", - linkedin_id: "ankurnarkhede", - twitter_id: "ankur909", - instagram_id: "", - website: "", message: "", }) .end((err, res) => { @@ -1792,9 +1782,9 @@ describe("Users", function () { return done(err); } - expect(res).to.have.status(401); - expect(res.body.error).to.be.equal("Unauthorized"); - expect(res.body.message).to.be.equal("You are not authorized for this action."); + expect(res).to.have.status(400); + expect(res.body.error).to.be.equal("Bad Request"); + expect(res.body.message).to.be.equal("Invalid Request."); return done(); }); }); @@ -1819,6 +1809,372 @@ describe("Users", function () { }); }); + describe("PATCH /users/:userId?profile=true", function () { + beforeEach(function () { + fetchStub = Sinon.stub(global, "fetch"); + fetchStub.returns( + Promise.resolve({ + status: 200, + json: () => Promise.resolve(getDiscordMembers), + }) + ); + }); + + afterEach(function () { + Sinon.restore(); + }); + + it("Should update the user", function (done) { + chai + .request(app) + .patch(`/users/${userId}?profile=true&dev=true`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ + first_name: "Test first_name", + }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(204); + + return done(); + }); + }); + + it("Should update the user status", function (done) { + chai + .request(app) + .patch(`/users/${userId}?profile=true&dev=true`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ + status: "ooo", + }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(204); + + return done(); + }); + }); + + it("Should update the username with valid username", function (done) { + chai + .request(app) + .patch(`/users/${userId}?profile=true&dev=true`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ + username: "validUsername123", + }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(204); + + return done(); + }); + }); + + it("Should allow updating user role when in_discord is not present and is not developer", function (done) { + addUser(newUser).then((newUserId) => { + const newUserJwt = authService.generateAuthToken({ userId: newUserId }); + chai + .request(app) + .patch(`/users/${newUserId}?profile=true&dev=true`) + .set("cookie", `${cookieName}=${newUserJwt}`) + .send({ + roles: { + maven: true, + }, + }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(204); + return done(); + }); + }); + }); + + it("Should not remove old roles when updating user roles", async function () { + const newUserId = await addUser(newUser); + const newUserJwt = authService.generateAuthToken({ userId: newUserId }); + + const getUserResponseBeforeUpdate = await chai + .request(app) + .get(`/users?profile=true`) + .set("cookie", `${cookieName}=${newUserJwt}`); + + expect(getUserResponseBeforeUpdate).to.have.status(200); + expect(getUserResponseBeforeUpdate.body.roles).to.not.have.property("maven"); + expect(getUserResponseBeforeUpdate.body.roles.in_discord).to.equal(false); + + const updateRolesResponse = await chai + .request(app) + .patch(`/users/${newUserId}?profile=true&dev=true`) + .set("cookie", `${cookieName}=${newUserJwt}`) + .send({ + roles: { + maven: true, + }, + }); + + expect(updateRolesResponse).to.have.status(204); + + const getUserResponseAfterUpdate = await chai + .request(app) + .get(`/users?profile=true`) + .set("cookie", `${cookieName}=${newUserJwt}`); + + expect(getUserResponseAfterUpdate).to.have.status(200); + expect(getUserResponseAfterUpdate.body.roles).to.have.property("maven"); + expect(getUserResponseAfterUpdate.body.roles.maven).to.equal(true); + expect(getUserResponseAfterUpdate.body.roles.in_discord).to.equal(false); + }); + + it("Should not update the user roles when user has in_discord and developer true", function (done) { + chai + .request(app) + .patch(`/users/${userId}?profile=true&dev=true`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ + roles: { + maven: true, + }, + }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(403); + + return done(); + }); + }); + + it("Should return 400 for invalid status value", function (done) { + chai + .request(app) + .patch(`/users/${userId}?profile=true&dev=true`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ + status: "blah", + }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(400); + expect(res.body).to.be.an("object"); + expect(res.body).to.eql({ + statusCode: 400, + error: "Bad Request", + message: '"status" must be one of [ooo, idle, active, onboarding]', + }); + + return done(); + }); + }); + + it("Should return 400 if required roles is missing", function (done) { + chai + .request(app) + .patch(`/users/${userId}?profile=true&dev=true`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ + roles: { + in_discord: false, + developer: true, + }, + }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(400); + + return done(); + }); + }); + + it("Should return 400 if invalid roles", function (done) { + chai + .request(app) + .patch(`/users/${userId}?profile=true&dev=true`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ + roles: { + archived: "false", + in_discord: false, + developer: true, + }, + }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(400); + + return done(); + }); + }); + + it("Should return 400 for invalid username", function (done) { + chai + .request(app) + .patch(`/users/${userId}?profile=true&dev=true`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ + username: "@invalidUser-name", + }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(400); + expect(res.body).to.be.an("object"); + expect(res.body).to.eql({ + statusCode: 400, + error: "Bad Request", + message: "Username must be between 4 and 32 characters long and contain only letters or numbers.", + }); + + return done(); + }); + }); + + it("Should update the social id with valid social id", function (done) { + chai + .request(app) + .patch(`/users/${userId}?profile=true&dev=true`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ + twitter_id: "Valid_twitterId", + }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(204); + return done(); + }); + }); + + it("Should return 400 for invalid Twitter ID", function (done) { + chai + .request(app) + .patch(`/users/${userId}?profile=true&dev=true`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ + twitter_id: "invalid@twitter_id", + }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(400); + expect(res.body).to.be.an("object"); + expect(res.body).to.eql({ + statusCode: 400, + error: "Bad Request", + message: "Invalid Twitter ID. ID should not contain special character @ or spaces", + }); + + return done(); + }); + }); + + it("Should return 400 for invalid Linkedin ID", function (done) { + chai + .request(app) + .patch(`/users/${userId}?profile=true&dev=true`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ + linkedin_id: "invalid@linkedin_id", + }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(400); + expect(res.body).to.be.an("object"); + expect(res.body).to.eql({ + statusCode: 400, + error: "Bad Request", + message: "Invalid Linkedin ID. ID should not contain special character @ or spaces", + }); + + return done(); + }); + }); + + it("Should return 400 for invalid instagram ID", function (done) { + chai + .request(app) + .patch(`/users/${userId}?profile=true&dev=true`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ + instagram_id: "invalid@instagram_id", + }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(400); + expect(res.body).to.be.an("object"); + expect(res.body).to.eql({ + statusCode: 400, + error: "Bad Request", + message: "Invalid Instagram ID. ID should not contain special character @ or spaces", + }); + + return done(); + }); + }); + + it("Should return 400 is space is included in the social ID", function (done) { + chai + .request(app) + .patch(`/users/${userId}?profile=true&dev=true`) + .set("cookie", `${cookieName}=${jwt}`) + .send({ + linkedin_id: "Linkedin 123", + }) + .end((err, res) => { + if (err) { + return done(err); + } + + expect(res).to.have.status(400); + expect(res.body).to.be.an("object"); + expect(res.body).to.eql({ + statusCode: 400, + error: "Bad Request", + message: "Invalid Linkedin ID. ID should not contain special character @ or spaces", + }); + + return done(); + }); + }); + }); + describe("GET /users/chaincode", function () { it("Should save the userId and timestamp in firestore collection and return the document ID as chaincode in response", function (done) { chai diff --git a/test/unit/middlewares/conditionalMiddleware.test.ts b/test/unit/middlewares/conditionalMiddleware.test.ts new file mode 100644 index 000000000..442cfe1c5 --- /dev/null +++ b/test/unit/middlewares/conditionalMiddleware.test.ts @@ -0,0 +1,64 @@ +import chai from "chai"; +import sinon from "sinon"; +const { expect } = chai; +const conditionalMiddleware = require("../../../middlewares/conditionalMiddleware"); +const authService = require("../../../services/authService"); +const addUser = require("../../utils/addUser"); + +describe("conditional Middleware", function () { + let req, res, next, validatorStub, middleware; + + beforeEach(async function () { + const userId = await addUser(); + validatorStub = sinon.spy(); + middleware = conditionalMiddleware(validatorStub); + + req = { + params: { userId }, + query: {}, + userData: { id: userId }, + }; + res = { + boom: { + unauthorized: sinon.spy(), + forbidden: sinon.spy(), + badRequest: sinon.spy(), + }, + }; + next = sinon.spy(); + }); + + it("should call the validator when profile query is true", async function () { + req.query.profile = "true"; + await middleware(req, res, next); + + expect(validatorStub.calledOnceWith(req, res, next)).to.equal(true); + expect(next.calledOnce).to.equal(false); + }); + + it("should call next when profile query is not true", async function () { + req.query.profile = "false"; + + await middleware(req, res, next); + + expect(validatorStub.called).to.equal(false); + expect(next.calledOnce).to.equal(true); + }); + + it("should call next when profile query is missing", async function () { + await middleware(req, res, next); + + expect(validatorStub.called).to.equal(false); + expect(next.calledOnce).to.equal(true); + }); + + it("should call next when userData.id does not match params.userId", async function () { + req.params.userId = "anotherUserId"; + req.query.profile = "true"; + + await middleware(req, res, next); + + expect(validatorStub.called).to.equal(false); + expect(next.calledOnce).to.equal(true); + }); +});