Repository: CVEProject/cve-services (js)

Issue:
Correct use of error messages orgDne and orgDneParam
The orgDne message in /src/utils/error.js includes the phrase 'designated by the shortname query parameter' but is sometime used for parameters with other names. In /src/controller/org.controller/error.js, the orgDneParam message is identical to the message in orgDne. 

Notes on current state of code:
org controller error orgDneParam
- GET /api/org/{identifier}, getOrg calls it for query param _identifier_
- Other uses are for path params named _shortname_; should be replaced with orgDne

utils error.js orgDne

- POST /api/cve-id, reserveCveId calls it for _short_name_ query param
- PUT /api/cve-id, modifyCveId calls it for _org_ query param
- PUT /org/{shortname}/user/{username}, updateUser calls error.orgDne  for path param _shortName_  and for query param _org_short_name_ 

Steps todo are:

- [ ] Update orgDne to take another argument called paramName
- [ ] Change the message in orgDne to `The organization '${shortname}' designated by the '${paramName}' query parameter does not exist.`
- [ ] Update calls to orgDne to pass in the correct paramName
- [ ] Change GET /api/org/{identifier}, getOrg to call orgDne instead of orgDneParam


Pull request:
Resolves #792 improve error messages related to org does not exist
Closes Issue #792

# Summary
Splits org DNE error messaging into two functions. One static one saying that the org specified by the shortname path parameter doesn't exist, the other takes arguments for the name and type of parameter specifying the org.

# Important Changes
cve-id.controller.js and org.controller.js
- modified endpoints to use correct messaging function

org.controller/error.js and /utils/error.js
- implement the two error message functions

# Testing

Steps to manually test updated functionality, if possible
1) Test each of these endpoints with a shortname for an org that does not exist. All should give an error message that the org specified by the shortname path parameter does not exist
- [x] GET /org/{shortname}/users  
- [x] GET /org/{shortname}/user/{username}
- [x] GET /org/{shortname}/id_quota
- [x] PUT /org/{shortname}/user/{username}
- [x] PUT /org/{shortname}/user/{username}/reset_secret
- [x] POST /org/{shortname}/user
- [x] PUT /org/{shortname}
2) Test the following endpoints as specified
- [x] POST /cve-id use an org that doesn't exist for the query param short_name
- [x] PUT /cve-id/:id use an org that doesn't exist for the query param org
- [x] GET /org/{identifier}  use an org that doesn't exist for the path param identifier
- [x] PUT /org/{shortname}/user/{username}  use an org that doesn't exist for the query param org_short_name




Patch:
diff --git a/src/controller/cve-id.controller/cve-id.controller.js b/src/controller/cve-id.controller/cve-id.controller.js
index 111ba84a..943b417a 100644
--- a/src/controller/cve-id.controller/cve-id.controller.js
+++ b/src/controller/cve-id.controller/cve-id.controller.js
@@ -173,7 +173,7 @@ async function reserveCveId (req, res, next) {
     const result = await orgRepo.findOneByShortName(shortName)
     if (!result) {
       logger.info({ uuid: req.ctx.uuid, message: shortName + ' organization does not exist.' })
-      return res.status(403).json(error.orgDne(shortName))
+      return res.status(403).json(error.orgDne(shortName, 'short_name', 'query'))
     }
 
     const payload = await getPayload(req, result)
@@ -330,7 +330,7 @@ async function modifyCveId (req, res, next) {
 
       if (!orgUUID) {
         logger.info({ uuid: req.ctx.uuid, message: id + ' could not be reassigned to ' + newOrgShortName + ' in MongoDB because it does not exist.' })
-        return res.status(404).json(error.orgDne(newOrgShortName))
+        return res.status(404).json(error.orgDne(newOrgShortName, 'org', 'query'))
       }
 
       cveId.owning_cna = orgUUID
diff --git a/src/controller/org.controller/error.js b/src/controller/org.controller/error.js
index 795f8f70..66f329ef 100644
--- a/src/controller/org.controller/error.js
+++ b/src/controller/org.controller/error.js
@@ -1,10 +1,10 @@
 const idrErr = require('../../utils/error')
 
 class OrgControllerError extends idrErr.IDRError {
-  orgDneParam (shortname) { // org
+  orgDnePathParam (shortname) { // org
     const err = {}
     err.error = 'ORG_DNE_PARAM'
-    err.message = `The '${shortname}' organization designated by the shortname parameter does not exist.`
+    err.message = `The '${shortname}' organization designated by the shortname path parameter does not exist.`
     return err
   }
 
diff --git a/src/controller/org.controller/org.controller.js b/src/controller/org.controller/org.controller.js
index 20652bb2..67f2a77d 100644
--- a/src/controller/org.controller/org.controller.js
+++ b/src/controller/org.controller/org.controller.js
@@ -81,7 +81,7 @@ async function getOrg (req, res, next) {
 
     if (!result) { // an empty result can only happen if the requestor is the Secretariat
       logger.info({ uuid: req.ctx.uuid, message: identifier + ' organization does not exist.' })
-      return res.status(404).json(error.orgDneParam(identifier))
+      return res.status(404).json(error.orgDne(identifier, 'identifier', 'path'))
     }
 
     logger.info({ uuid: req.ctx.uuid, message: identifier + ' organization was sent to the user.', org: result })
@@ -117,7 +117,7 @@ async function getUsers (req, res, next) {
 
     if (!orgUUID) {
       logger.info({ uuid: req.ctx.uuid, message: orgShortName + ' organization does not exist.' })
-      return res.status(404).json(error.orgDneParam(orgShortName))
+      return res.status(404).json(error.orgDnePathParam(orgShortName))
     }
 
     if (orgShortName !== shortName && !isSecretariat) {
@@ -165,7 +165,7 @@ async function getUser (req, res, next) {
     const orgUUID = await orgRepo.getOrgUUID(orgShortName)
     if (!orgUUID) { // the org can only be non-existent if the requestor is the Secretariat
       logger.info({ uuid: req.ctx.uuid, message: orgShortName + ' organization does not exist.' })
-      return res.status(404).json(error.orgDneParam(orgShortName))
+      return res.status(404).json(error.orgDnePathParam(orgShortName))
     }
 
     const userRepo = req.ctx.repositories.getUserRepository()
@@ -204,7 +204,7 @@ async function getOrgIdQuota (req, res, next) {
     let result = await repo.findOneByShortName(shortName)
     if (!result) { // a null result can only happen if the requestor is the Secretariat
       logger.info({ uuid: req.ctx.uuid, message: shortName + ' organization does not exist.' })
-      return res.status(404).json(error.orgDneParam(shortName))
+      return res.status(404).json(error.orgDnePathParam(shortName))
     }
 
     const returnPayload = {
@@ -331,7 +331,7 @@ async function updateOrg (req, res, next) {
     // org doesn't exist
     if (!org) {
       logger.info({ uuid: req.ctx.uuid, message: shortName + ' organization could not be updated in MongoDB because it does not exist.' })
-      return res.status(404).json(error.orgDneParam(shortName))
+      return res.status(404).json(error.orgDnePathParam(shortName))
     }
 
     Object.keys(req.ctx.query).forEach(k => {
@@ -394,7 +394,7 @@ async function updateOrg (req, res, next) {
     let result = await orgRepo.updateByOrgUUID(org.UUID, newOrg)
     if (result.n === 0) {
       logger.info({ uuid: req.ctx.uuid, message: shortName + ' organization could not be updated in MongoDB because it does not exist.' })
-      return res.status(404).json(error.orgDneParam(shortName))
+      return res.status(404).json(error.orgDnePathParam(shortName))
     }
 
     result = await orgRepo.aggregate(agt)
@@ -438,7 +438,7 @@ async function createUser (req, res, next) {
     const orgUUID = await orgRepo.getOrgUUID(orgShortName)
     if (!orgUUID) { // the org can only be non-existent if the requestor is the Secretariat
       logger.info({ uuid: req.ctx.uuid, message: 'The user could not be created because ' + orgShortName + ' organization does not exist.' })
-      return res.status(404).json(error.orgDneParam(orgShortName))
+      return res.status(404).json(error.orgDnePathParam(orgShortName))
     }
 
     Object.keys(req.ctx.body).forEach(k => {
@@ -546,7 +546,7 @@ async function updateUser (req, res, next) {
 
     if (!orgUUID) {
       logger.info({ uuid: req.ctx.uuid, message: 'The user could not be updated because ' + shortName + ' organization does not exist.' })
-      return res.status(404).json(error.orgDneParam(shortName))
+      return res.status(404).json(error.orgDnePathParam(shortName))
     }
 
     if (shortName !== requesterShortName && !isSecretariat) {
@@ -628,7 +628,7 @@ async function updateUser (req, res, next) {
 
       if (!newUser.org_UUID) {
         logger.info({ uuid: req.ctx.uuid, message: 'The user could not be updated because ' + newOrgShortName + ' organization does not exist.' })
-        return res.status(404).json(error.orgDne(newOrgShortName))
+        return res.status(404).json(error.orgDne(newOrgShortName, 'org_short_name', 'query'))
       }
     }
 
@@ -736,7 +736,7 @@ async function resetSecret (req, res, next) {
     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))
+      return res.status(404).json(error.orgDnePathParam(orgShortName))
     }
 
     if (orgShortName !== requesterShortName && !isSecretariat) {
diff --git a/src/utils/error.js b/src/utils/error.js
index bf38db9e..983af32c 100644
--- a/src/utils/error.js
+++ b/src/utils/error.js
@@ -7,10 +7,10 @@ class IDRError {
     return err
   }
 
-  orgDne (shortname) { // super
+  orgDne (shortname, paramName, paramType) { // super
     const err = {}
     err.error = 'ORG_DNE'
-    err.message = `The organization '${shortname}' designated by the shortname query parameter does not exist.`
+    err.message = `The organization '${shortname}' designated by the ${paramName} ${paramType} parameter does not exist.`
     return err
   }
 


Test patch:
diff --git a/test-http/src/test/org_user_tests/org.py b/test-http/src/test/org_user_tests/org.py
index 5547297a..59d5fa4c 100644
--- a/test-http/src/test/org_user_tests/org.py
+++ b/test-http/src/test/org_user_tests/org.py
@@ -71,7 +71,7 @@ def test_get_mitre_by_nonexistent_org_uuid():
         headers=utils.BASE_HEADERS,
     )
     assert res.status_code == 404
-    response_contains_json(res, 'error', 'ORG_DNE_PARAM')
+    response_contains_json(res, 'error', 'ORG_DNE')
 
 
 #### GET /org/:shortname/id_quota ####
@@ -299,7 +299,7 @@ def test_put_update_org_that_does_not_exist():
     )
     assert res.status_code == 404
     response_contains_json(res, 'error', 'ORG_DNE_PARAM')
-    assert_contains(res, 'by the shortname parameter does not exist')
+    assert_contains(res, 'by the shortname path parameter does not exist')
 
 
 def test_put_update_mitre_org_nonexistent_user():
diff --git a/test/unit-tests/cve-id/cveIdUpdateTest.js b/test/unit-tests/cve-id/cveIdUpdateTest.js
index 5ac4cbfe..0330bf31 100644
--- a/test/unit-tests/cve-id/cveIdUpdateTest.js
+++ b/test/unit-tests/cve-id/cveIdUpdateTest.js
@@ -166,7 +166,7 @@ describe('Testing the PUT /cve-id/:id endpoint in CveId Controller', () => {
 
           expect(res).to.have.status(404)
           expect(res).to.have.property('body').and.to.be.a('object')
-          const errObj = error.orgDne(cveIdFixtures.nonExistentOrg.short_name)
+          const errObj = error.orgDne(cveIdFixtures.nonExistentOrg.short_name, 'org', 'query')
           expect(res.body.error).to.equal(errObj.error)
           expect(res.body.message).to.equal(errObj.message)
           done()
diff --git a/test/unit-tests/cve-id/reserveCveId/cveIdReserveGeneralLogicTest.js b/test/unit-tests/cve-id/reserveCveId/cveIdReserveGeneralLogicTest.js
index ba156fa7..bbc1d27f 100644
--- a/test/unit-tests/cve-id/reserveCveId/cveIdReserveGeneralLogicTest.js
+++ b/test/unit-tests/cve-id/reserveCveId/cveIdReserveGeneralLogicTest.js
@@ -279,7 +279,7 @@ describe('Testing the general logic of POST /cve-id endpoint in CveId Controller
 
           expect(res).to.have.status(403)
           expect(res).to.have.property('body').and.to.be.a('object')
-          const errObj = error.orgDne(cveIdFixtures.nonExistentOrg.short_name)
+          const errObj = error.orgDne(cveIdFixtures.nonExistentOrg.short_name, 'short_name', 'query')
           expect(res.body.error).to.equal(errObj.error)
           expect(res.body.message).to.equal(errObj.message)
           done()
diff --git a/test/unit-tests/org/orgGetIdQuotaTest.js b/test/unit-tests/org/orgGetIdQuotaTest.js
index cd3f8432..a2630b34 100644
--- a/test/unit-tests/org/orgGetIdQuotaTest.js
+++ b/test/unit-tests/org/orgGetIdQuotaTest.js
@@ -106,7 +106,7 @@ describe('Testing the GET /org/:shortname/id_quota endpoint in Org Controller',
 
           expect(res).to.have.status(404)
           expect(res).to.have.property('body').and.to.be.a('object')
-          const errObj = error.orgDneParam(orgFixtures.nonExistentOrg.short_name)
+          const errObj = error.orgDnePathParam(orgFixtures.nonExistentOrg.short_name)
           expect(res.body.error).to.equal(errObj.error)
           expect(res.body.message).to.equal(errObj.message)
           done()
diff --git a/test/unit-tests/org/orgGetSingleTest.js b/test/unit-tests/org/orgGetSingleTest.js
index 96885739..0a6b77d6 100644
--- a/test/unit-tests/org/orgGetSingleTest.js
+++ b/test/unit-tests/org/orgGetSingleTest.js
@@ -61,7 +61,7 @@ describe('Testing the GET /org/:identifier endpoint in Org Controller', () => {
 
       expect(res).to.have.status(404)
       expect(res).to.have.property('body').and.to.be.a('object')
-      const errObj = error.orgDneParam(orgFixtures.nonExistentOrg.short_name)
+      const errObj = error.orgDne(orgFixtures.nonExistentOrg.short_name, 'identifier', 'path')
       expect(res.body.error).to.equal(errObj.error)
       expect(res.body.message).to.equal(errObj.message)
     })
diff --git a/test/unit-tests/org/orgUpdateTest.js b/test/unit-tests/org/orgUpdateTest.js
index f723e542..f978d6f9 100644
--- a/test/unit-tests/org/orgUpdateTest.js
+++ b/test/unit-tests/org/orgUpdateTest.js
@@ -92,7 +92,7 @@ describe('Testing the PUT /org/:shortname endpoint in Org Controller', () => {
 
       expect(res).to.have.status(404)
       expect(res).to.have.property('body').and.to.be.a('object')
-      const errObj = error.orgDneParam(orgFixtures.nonExistentOrg.short_name)
+      const errObj = error.orgDnePathParam(orgFixtures.nonExistentOrg.short_name)
       expect(res.body.error).to.equal(errObj.error)
       expect(res.body.message).to.equal(errObj.message)
     })
diff --git a/test/unit-tests/user/userCreateTest.js b/test/unit-tests/user/userCreateTest.js
index ec493ae0..7c527104 100644
--- a/test/unit-tests/user/userCreateTest.js
+++ b/test/unit-tests/user/userCreateTest.js
@@ -143,7 +143,7 @@ describe('Testing the POST /org/:shortname/user endpoint in Org Controller', ()
 
           expect(res).to.have.status(404)
           expect(res).to.have.property('body').and.to.be.a('object')
-          const errObj = error.orgDneParam(userFixtures.nonExistentOrg.short_name)
+          const errObj = error.orgDnePathParam(userFixtures.nonExistentOrg.short_name)
           expect(res.body.error).to.equal(errObj.error)
           expect(res.body.message).to.equal(errObj.message)
           done()
diff --git a/test/unit-tests/user/userGetAllTest.js b/test/unit-tests/user/userGetAllTest.js
index a8d02cb2..7cabcabc 100644
--- a/test/unit-tests/user/userGetAllTest.js
+++ b/test/unit-tests/user/userGetAllTest.js
@@ -56,7 +56,7 @@ describe('Testing the GET /org/:shortname/users endpoint in Org Controller', ()
 
           expect(res).to.have.status(404)
           expect(res).to.have.property('body').and.to.be.a('object')
-          const errObj = error.orgDneParam(orgFixtures.nonExistentOrg.short_name)
+          const errObj = error.orgDnePathParam(orgFixtures.nonExistentOrg.short_name)
           expect(res.body.error).to.equal(errObj.error)
           expect(res.body.message).to.equal(errObj.message)
           done()
diff --git a/test/unit-tests/user/userGetSingleTest.js b/test/unit-tests/user/userGetSingleTest.js
index 4b2e5546..47a7cc77 100644
--- a/test/unit-tests/user/userGetSingleTest.js
+++ b/test/unit-tests/user/userGetSingleTest.js
@@ -94,7 +94,7 @@ describe('Testing the GET /org/:shortname/user/:username endpoint in Org Control
 
           expect(res).to.have.status(404)
           expect(res).to.have.property('body').and.to.be.a('object')
-          const errObj = error.orgDneParam(userFixtures.nonExistentOrg.short_name)
+          const errObj = error.orgDnePathParam(userFixtures.nonExistentOrg.short_name)
           expect(res.body.error).to.equal(errObj.error)
           expect(res.body.message).to.equal(errObj.message)
           done()
diff --git a/test/unit-tests/user/userResetSecretTest.js b/test/unit-tests/user/userResetSecretTest.js
index 27a48ef7..b2023873 100644
--- a/test/unit-tests/user/userResetSecretTest.js
+++ b/test/unit-tests/user/userResetSecretTest.js
@@ -163,7 +163,7 @@ describe('Testing the PUT /org/:shortname/user/:username/reset_secret endpoint i
 
           expect(res).to.have.status(404)
           expect(res).to.have.property('body').and.to.be.a('object')
-          const errObj = error.orgDneParam(userFixtures.nonExistentOrg.short_name)
+          const errObj = error.orgDnePathParam(userFixtures.nonExistentOrg.short_name)
           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 6002a4cc..54842f9e 100644
--- a/test/unit-tests/user/userUpdateTest.js
+++ b/test/unit-tests/user/userUpdateTest.js
@@ -138,7 +138,7 @@ describe('Testing the PUT /org/:shortname/user/:username endpoint in Org Control
 
           expect(res).to.have.status(404)
           expect(res).to.have.property('body').and.to.be.a('object')
-          const errObj = error.orgDneParam(userFixtures.nonExistentOrg.short_name)
+          const errObj = error.orgDnePathParam(userFixtures.nonExistentOrg.short_name)
           expect(res.body.error).to.equal(errObj.error)
           expect(res.body.message).to.equal(errObj.message)
           done()
@@ -226,7 +226,7 @@ describe('Testing the PUT /org/:shortname/user/:username endpoint in Org Control
 
           expect(res).to.have.status(404)
           expect(res).to.have.property('body').and.to.be.a('object')
-          const errObj = error.orgDne(userFixtures.nonExistentOrg.short_name)
+          const errObj = error.orgDne(userFixtures.nonExistentOrg.short_name, 'org_short_name', 'query')
           expect(res.body.error).to.equal(errObj.error)
           expect(res.body.message).to.equal(errObj.message)
           done()