Repository: CVEProject/cve-services (js)

Issue:
user enumeration via reset_secret
/org/{shortname}/user/{username}/reset_secret can be used, without Secretariat privileges, to check whether a specific pair of CNA name and username exists in CVE Services. This seems contrary to the spirit of https://github.com/CVEProject/cve-services/issues/413 Scenario 2 (although it arguably has a lower risk level).

Suppose I know that Ashley Ruiz had worked for the door_2 CNA, but left. I want to discover non-public information about where she is currently working, for one of two reasons:
* I'm a manager at door_2 and want to know what company my departing employees are going to
* I suspect that she's now at window_1, and want to know her email address so that I can back-channel a CVE ID request for a window_1 product

reset_secret gives me useful responses such as:
```
{"error":"USER_DNE","message":"The user 'ashley.ruiz@win_5.com' designated by the username parameter does not exist."}
{"error":"USER_DNE","message":"The user 'ashleyruiz@win_5.com' designated by the username parameter does not exist."}

==> she is probably not working at win_5

{"error":"USER_DNE","message":"The user 'ashley.ruiz@window_1.com' designated by the username parameter does not exist."}

==> not her correct email address
```
Now try:
```
/api/org/window_1/user/ashleyruiz@window_1.com/reset_secret
==>
{"error":"NOT_SAME_USER_OR_SECRETARIAT","message":"This information can only be viewed or modified by the Secretariat, an Org Admin or if the requester is the user."}
```
Here, I have successfully reached line 651 and thus ashleyruiz@window_1.com must be valid. This gives me reasonable confidence about an employer and email address.
https://github.com/CVEProject/cve-services/blob/facb0f8f532ee16540c0eb09e469283bb50581d4/src/controller/org.controller/org.controller.js#L645-L653
https://github.com/CVEProject/cve-services/blob/facb0f8f532ee16540c0eb09e469283bb50581d4/datadump/pre-population/users.json#L48

Pull request description:
#564, #574 Update INT from DEV
Fixes #564, #574

Patch:
diff --git a/src/controller/org.controller/org.controller.js b/src/controller/org.controller/org.controller.js
index b025cda0..1c982c26 100644
--- a/src/controller/org.controller/org.controller.js
+++ b/src/controller/org.controller/org.controller.js
@@ -485,6 +485,11 @@ async function updateUser (req, res, next) {
       return res.status(404).json(error.orgDneParam(shortName))
     }
 
+    if (shortName !== requesterShortName && !isSecretariat) {
+      logger.info({ uuid: req.ctx.uuid, message: shortName + ' organization can only be viewed by the users of the same organization or the Secretariat.' })
+      return res.status(403).json(error.notSameOrgOrSecretariat())
+    }
+
     const user = await userRepo.findOneByUserNameAndOrgUUID(username, orgUUID)
     if (!user) {
       logger.info({ uuid: req.ctx.uuid, message: 'The user could not be updated because ' + username + ' does not exist for ' + shortName + ' organization.' })
@@ -636,19 +641,24 @@ async function resetSecret (req, res, next) {
     const orgShortName = req.ctx.params.shortname
     const userRepo = req.ctx.repositories.getUserRepository()
     const orgRepo = req.ctx.repositories.getOrgRepository()
+    const isSecretariat = await orgRepo.isSecretariat(requesterShortName)
     const orgUUID = await orgRepo.getOrgUUID(orgShortName) // userUUID may be null if user does not exist
     if (!orgUUID) {
       logger.info({ uuid: req.ctx.uuid, messsage: orgShortName + ' organization does not exist.' })
       return res.status(404).json(error.orgDneParam(orgShortName))
     }
 
+    if (orgShortName !== requesterShortName && !isSecretariat) {
+      logger.info({ uuid: req.ctx.uuid, message: orgShortName + ' organization can only be viewed by the users of the same organization or the Secretariat.' })
+      return res.status(403).json(error.notSameOrgOrSecretariat())
+    }
+
     const oldUser = await userRepo.findOneByUserNameAndOrgUUID(username, orgUUID)
     if (!oldUser) {
       logger.info({ uuid: req.ctx.uuid, messsage: username + ' user does not exist.' })
       return res.status(404).json(error.userDne(username))
     }
 
-    const isSecretariat = await orgRepo.isSecretariat(requesterShortName)
     const isAdmin = await userRepo.isAdmin(requesterUsername, requesterShortName)
     // check if the user is not the requester or if the requester is not a secretariat
     if ((orgShortName !== requesterShortName || username !== requesterUsername) && !isSecretariat) {


Test patch:
diff --git a/test-http/src/test/org_user_tests/org_as_org_admin.py b/test-http/src/test/org_user_tests/org_as_org_admin.py
index 32279584..ac2d43a0 100644
--- a/test-http/src/test/org_user_tests/org_as_org_admin.py
+++ b/test-http/src/test/org_user_tests/org_as_org_admin.py
@@ -266,7 +266,7 @@ def test_org_admin_cannot_update_user_for_another_org(org_admin_headers):
         headers=org_admin_headers
     )
     assert res.status_code == 403
-    response_contains_json(res, 'error', 'NOT_SAME_USER_OR_SECRETARIAT')
+    response_contains_json(res, 'error', 'NOT_SAME_ORG_OR_SECRETARIAT')
 
 
 def test_org_admin_cannot_update_user_new_shortname_dne(org_admin_headers):
@@ -455,7 +455,7 @@ def test_org_admin_reset_diff_org_secret(org_admin_headers):
         headers=org_admin_headers
     )
     assert res.status_code == 403
-    response_contains_json(res, 'error', 'NOT_SAME_USER_OR_SECRETARIAT')
+    response_contains_json(res, 'error', 'NOT_SAME_ORG_OR_SECRETARIAT')
 
 
 def test_org_admin_reset_same_org_secret(org_admin_headers):
diff --git a/test/unit-tests/user/userResetSecretTest.js b/test/unit-tests/user/userResetSecretTest.js
index cc660b95..54883d93 100644
--- a/test/unit-tests/user/userResetSecretTest.js
+++ b/test/unit-tests/user/userResetSecretTest.js
@@ -273,7 +273,7 @@ describe('Testing the PUT /org/:shortname/user/:username/reset_secret endpoint i
 
           expect(res).to.have.status(403)
           expect(res).to.have.property('body').and.to.be.a('object')
-          const errObj = error.notSameUserOrSecretariat()
+          const errObj = error.notSameOrgOrSecretariat()
           expect(res.body.error).to.equal(errObj.error)
           expect(res.body.message).to.equal(errObj.message)
           done()
@@ -302,7 +302,7 @@ describe('Testing the PUT /org/:shortname/user/:username/reset_secret endpoint i
 
           expect(res).to.have.status(403)
           expect(res).to.have.property('body').and.to.be.a('object')
-          const errObj = error.notSameUserOrSecretariat()
+          const errObj = error.notSameOrgOrSecretariat()
           expect(res.body.error).to.equal(errObj.error)
           expect(res.body.message).to.equal(errObj.message)
           done()
@@ -331,7 +331,7 @@ describe('Testing the PUT /org/:shortname/user/:username/reset_secret endpoint i
 
           expect(res).to.have.status(403)
           expect(res).to.have.property('body').and.to.be.a('object')
-          const errObj = error.notSameUserOrSecretariat()
+          const errObj = error.notSameOrgOrSecretariat()
           expect(res.body.error).to.equal(errObj.error)
           expect(res.body.message).to.equal(errObj.message)
           done()
diff --git a/test/unit-tests/user/userUpdateTest.js b/test/unit-tests/user/userUpdateTest.js
index 38bdf1ec..8718b162 100644
--- a/test/unit-tests/user/userUpdateTest.js
+++ b/test/unit-tests/user/userUpdateTest.js
@@ -270,7 +270,7 @@ describe('Testing the PUT /org/:shortname/user/:username endpoint in Org Control
 
           expect(res).to.have.status(403)
           expect(res).to.have.property('body').and.to.be.a('object')
-          const errObj = error.notSameUserOrSecretariat()
+          const errObj = error.notSameOrgOrSecretariat()
           expect(res.body.error).to.equal(errObj.error)
           expect(res.body.message).to.equal(errObj.message)
           done()
@@ -320,7 +320,7 @@ describe('Testing the PUT /org/:shortname/user/:username endpoint in Org Control
 
           expect(res).to.have.status(403)
           expect(res).to.have.property('body').and.to.be.a('object')
-          const errObj = error.notSameUserOrSecretariat()
+          const errObj = error.notSameOrgOrSecretariat()
           expect(res.body.error).to.equal(errObj.error)
           expect(res.body.message).to.equal(errObj.message)
           done()