Repository: CVEProject/cve-services (js)

Issue:
Bad Error Messages for POST /cve/:id/cna and POST /org/:shortname/user Endpoints
These endpoints have error messages that should be improved:

- POST /cve/:id/cna when the CNA does not own the id throws the error "It appears the user does not own the CVE". 

- POST /org/:shortname/user when an admin CNA user is attempting to create a user for a different org throws the error "This information can only be viewed or modified by the Secretariat or Org Admin". This is misleading because the user is an Admin but trying to create a user in a different org, not view or modify something.

**Definition of Done**
- [ ] Improved error message for POST /cve/:id/cna
- [ ] Improved error message for POST /org/:shortname/user

Pull request:
Resolves #749 Updated error messages for creating and updating users and creating CVE records
Closes Issue #749

# Summary
Improved error messages for POST  `/org/:shortname/user`, PUT `/org/:shortname/user/:username` and POST `/cve/:id/cna`.

# Important Changes
- Changed `It appears It appears the Org does not own the CVE` to `Org does not own the CVE`.
- Changed `This information can only be viewed or modified by the Secretariat or Org Admin.` to `Users can only be created by the Secretariat or Org Admin.`
- Added new error message for updating users, `Contact your org Admin to update fields other than your name.`

# Testing

Steps to manually test updated functionality, if possible
- [ ] 1) Attempt to POST a CVE record as a CNA in an org that doesn't own the corresponding Cve-Id.
- [ ] 2) Attempt to create a user as a non Secretariat or Admin
- [ ] 3) Attempt to update non name fields as a non admin user for your own account



Patch:
diff --git a/src/controller/org.controller/org.controller.js b/src/controller/org.controller/org.controller.js
index 41b6596a..7c454233 100644
--- a/src/controller/org.controller/org.controller.js
+++ b/src/controller/org.controller/org.controller.js
@@ -610,7 +610,7 @@ async function updateUser (req, res, next) {
     // Check for correct privileges if the requested changes require them
     if (changesRequirePrivilegedRole && !(isAdmin || isSecretariat)) {
       logger.info({ uuid: req.ctx.uuid, message: 'The user could not be updated because ' + requesterUsername + ' user is not Org Admin or Secretariat to modify these fields.' })
-      return res.status(403).json(error.notOrgAdminOrSecretariat())
+      return res.status(403).json(error.notOrgAdminOrSecretariatUpdate())
     }
 
     // check if the new org exist
diff --git a/src/middleware/error.js b/src/middleware/error.js
index a666f2ab..76a4ecbd 100644
--- a/src/middleware/error.js
+++ b/src/middleware/error.js
@@ -52,7 +52,7 @@ class MiddlewareError extends idrErr.IDRError {
   orgDoesNotOwnId (org, id) {
     const err = {
       error: 'ORG_DOES_NOT_OWN_ID',
-      message: `It appears that ${org} does not own ${id}`
+      message: `${org} does not own ${id}`
     }
     return err
   }
diff --git a/src/utils/error.js b/src/utils/error.js
index 42ddc6f1..bf38db9e 100644
--- a/src/utils/error.js
+++ b/src/utils/error.js
@@ -82,7 +82,14 @@ class IDRError {
   notOrgAdminOrSecretariat () { // super
     const err = {}
     err.error = 'NOT_ORG_ADMIN_OR_SECRETARIAT'
-    err.message = 'This information can only be viewed or modified by the Secretariat or Org Admin.'
+    err.message = 'Users can only be created by the Secretariat or Org Admin.'
+    return err
+  }
+
+  notOrgAdminOrSecretariatUpdate () {
+    const err = {}
+    err.error = 'NOT_ORG_ADMIN_OR_SECRETARIAT_UPDATE'
+    err.message = 'Contact your org Admin to update fields other than your name.'
     return err
   }
 


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 aef9230a..4a0b8473 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
@@ -7,13 +7,15 @@ import requests
 import uuid
 from src import env, utils
 from src.test.org_user_tests.org import (ORG_URL, create_new_user_with_new_org_by_uuid,
-                            create_new_user_with_new_org_by_shortname,
-                            post_new_org_user, post_new_org)
+                                         create_new_user_with_new_org_by_shortname,
+                                         post_new_org_user, post_new_org)
 from src.utils import response_contains, response_contains_json
 
 MAX_SHORTNAME_LENGTH = 32
 
 ### GET /org ####
+
+
 def test_org_admin_get_all_orgs(org_admin_headers):
     """ services api rejects requests for all orgs by non-secretariat users """
     res = requests.get(
@@ -28,7 +30,7 @@ def test_org_admin_get_all_orgs(org_admin_headers):
 def test_org_admin_get_mitre_org(org_admin_headers):
     """ services api rejects requests for secretariat by non-secretariat users """
     res = requests.get(
-        f'{env.AWG_BASE_URL}{ORG_URL}/mitre', # the secretariat's org
+        f'{env.AWG_BASE_URL}{ORG_URL}/mitre',  # the secretariat's org
         headers=org_admin_headers
     )
     assert res.status_code == 403
@@ -37,8 +39,8 @@ def test_org_admin_get_mitre_org(org_admin_headers):
 
 def test_org_admin_get_another_org(org_admin_headers):
     """ services api rejects requests for any org by another org user """
-    different_org = str(uuid.uuid4())[:MAX_SHORTNAME_LENGTH] # name of an org
-    res = post_new_org(different_org, different_org) # create an org
+    different_org = str(uuid.uuid4())[:MAX_SHORTNAME_LENGTH]  # name of an org
+    res = post_new_org(different_org, different_org)  # create an org
     assert res.status_code == 200
     res = requests.get(
         f'{env.AWG_BASE_URL}{ORG_URL}/{different_org}',
@@ -63,7 +65,7 @@ def test_org_admin_get_own_org(org_admin_headers):
 def test_org_admin_get_secretariat_id_quota_info(org_admin_headers):
     """ services api rejects requests for secretariat by non-secretariat users """
     res = requests.get(
-        f'{env.AWG_BASE_URL}{ORG_URL}/mitre/id_quota', # the secretariat's org
+        f'{env.AWG_BASE_URL}{ORG_URL}/mitre/id_quota',  # the secretariat's org
         headers=org_admin_headers
     )
     assert res.status_code == 403
@@ -72,8 +74,8 @@ def test_org_admin_get_secretariat_id_quota_info(org_admin_headers):
 
 def test_org_admin_get_another_org_id_quota_info(org_admin_headers):
     """ services api rejects requests for any org by another org user """
-    different_org = str(uuid.uuid4())[:MAX_SHORTNAME_LENGTH] # name of an org
-    res = post_new_org(different_org, different_org) # create an org
+    different_org = str(uuid.uuid4())[:MAX_SHORTNAME_LENGTH]  # name of an org
+    res = post_new_org(different_org, different_org)  # create an org
     assert res.status_code == 200
     res = requests.get(
         f'{env.AWG_BASE_URL}{ORG_URL}/{different_org}/id_quota',
@@ -209,6 +211,8 @@ def test_org_get_bad_user_page(org_admin_headers):
     response_contains_json(res, 'details', utils.BAD_PAGE_ERROR_DETAILS)
 
 #### POST /org ####
+
+
 def test_org_admin_cannot_create_another_org(org_admin_headers):
     """ services api does not allow org admins to create other orgs """
     res = requests.post(
@@ -240,7 +244,7 @@ def test_org_admin_cannot_create_user_for_another_org(org_admin_headers):
     res = requests.post(
         f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user',
         headers=org_admin_headers,
-        json={'username':'BLARG', 'org_UUID': 'test'}
+        json={'username': 'BLARG', 'org_UUID': 'test'}
     )
     assert res.status_code == 400
     response_contains_json(res, 'error', 'SHORTNAME_MISMATCH')
@@ -254,7 +258,7 @@ def test_org_admin_cannot_create_user_for_another_org(org_admin_headers):
     res = requests.post(
         f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user',
         headers=org_admin_headers,
-        json={'username':'BLARG'}
+        json={'username': 'BLARG'}
     )
     assert res.status_code == 403
     response_contains_json(res, 'error', 'NOT_ORG_ADMIN_OR_SECRETARIAT')
@@ -269,7 +273,7 @@ def test_org_admin_cannot_create_existen_user(org_admin_headers):
     res = requests.post(
         f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user',
         headers=org_admin_headers,
-        json={'username':user}
+        json={'username': user}
     )
     assert res.status_code == 400
     response_contains_json(res, 'error', 'USER_EXISTS')
@@ -342,7 +346,8 @@ def test_org_admin_cannot_update_duplicate_user_with_new_username(org_admin_head
     org = org_admin_headers["CVE-API-ORG"][:MAX_SHORTNAME_LENGTH]
     user1 = org_admin_headers['CVE-API-USER']
     user2 = str(uuid.uuid4())
-    res = post_new_org_user(org, user2) # creating a user with same org as admin org user
+    # creating a user with same org as admin org user
+    res = post_new_org_user(org, user2)
     assert res.status_code == 200
     res = requests.put(
         f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user1}?new_username={user2}',
@@ -370,8 +375,9 @@ def test_org_admin_update_same_org_user_state_sn_un(org_admin_headers):
     """  allows admin users to update a user's active state and user username """
     org = org_admin_headers["CVE-API-ORG"][:MAX_SHORTNAME_LENGTH]
     user = str(uuid.uuid4())
-    res = post_new_org_user(org, user) # creating a user with same org as admin org user
-    assert res.status_code == 200 
+    # creating a user with same org as admin org user
+    res = post_new_org_user(org, user)
+    assert res.status_code == 200
     new_shortname = str(uuid.uuid4())[:MAX_SHORTNAME_LENGTH]   # used in query
     new_username = str(uuid.uuid4())    # used in query
     res = post_new_org(new_shortname, new_shortname)  # create new org
@@ -382,7 +388,8 @@ def test_org_admin_update_same_org_user_state_sn_un(org_admin_headers):
     )
     assert res.status_code == 200
     assert json.loads(res.content.decode())['updated']['active'] == False
-    assert json.loads(res.content.decode())['updated']['username'] == new_username
+    assert json.loads(res.content.decode())[
+        'updated']['username'] == new_username
     assert json.loads(res.content.decode())['updated']['username'] is not None
 
 
@@ -390,22 +397,28 @@ def test_org_admin_update_same_org_user_roles_name(org_admin_headers):
     """  allows admin users to update a user's name, add role, and remove role """
     org = org_admin_headers["CVE-API-ORG"][:MAX_SHORTNAME_LENGTH]
     user = str(uuid.uuid4())
-    res = post_new_org_user(org, user) # creating a user with same org as admin org user
+    # creating a user with same org as admin org user
+    res = post_new_org_user(org, user)
     assert res.status_code == 200
     res = requests.put(
-        f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?active_roles.add=admin', # adding role
+        # adding role
+        f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?active_roles.add=admin',
         headers=org_admin_headers
     )
     assert res.status_code == 200
-    assert json.loads(res.content.decode())['updated']['authority']['active_roles'] == ["ADMIN"]
+    assert json.loads(res.content.decode())[
+        'updated']['authority']['active_roles'] == ["ADMIN"]
     res = requests.put(
-        f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?active_roles.remove=admin', # removing role
+        # removing role
+        f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?active_roles.remove=admin',
         headers=org_admin_headers
     )
     assert res.status_code == 200
-    assert json.loads(res.content.decode())['updated']['authority']['active_roles'] == []
+    assert json.loads(res.content.decode())[
+        'updated']['authority']['active_roles'] == []
     res = requests.put(
-        f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?name.first=t&name.last=e&name.middle=s&name.suffix=t', # updating name
+        # updating name
+        f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?name.first=t&name.last=e&name.middle=s&name.suffix=t',
         headers=org_admin_headers
     )
     assert res.status_code == 200
@@ -434,29 +447,36 @@ def test_org_admin_update_same_org_user_roles_name(org_admin_headers):
 
 
 def test_org_admin_update_own_user_roles_name(org_admin_headers):
-    """  allows admin users to update its own name and remove role """ 
+    """  allows admin users to update its own name and remove role """
     org = org_admin_headers["CVE-API-ORG"][:MAX_SHORTNAME_LENGTH]
     user = org_admin_headers['CVE-API-USER']
     res = requests.put(
-        f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?active_roles.remove=admin', # removing role
+        # removing role
+        f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?active_roles.remove=admin',
         headers=org_admin_headers
     )
     assert res.status_code == 200
-    assert json.loads(res.content.decode())['updated']['authority']['active_roles'] == []
+    assert json.loads(res.content.decode())[
+        'updated']['authority']['active_roles'] == []
     res = requests.put(
-        f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?active_roles.add=admin', # adding role
+        # adding role
+        f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?active_roles.add=admin',
         headers=org_admin_headers
     )
     assert res.status_code == 403
-    response_contains_json(res, 'error', 'NOT_ORG_ADMIN_OR_SECRETARIAT') # cannot add role because org admin doesn't have "ADMIN" role anymore
+    # cannot add role because org admin doesn't have "ADMIN" role anymore
+    response_contains_json(res, 'error', 'NOT_ORG_ADMIN_OR_SECRETARIAT_UPDATE')
     res = requests.put(
-        f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?active_roles.add=admin', # adding "ADMIN" role back to org admin user
+        # adding "ADMIN" role back to org admin user
+        f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?active_roles.add=admin',
         headers=utils.BASE_HEADERS
     )
     assert res.status_code == 200
-    assert json.loads(res.content.decode())['updated']['authority']['active_roles'] == ["ADMIN"]
+    assert json.loads(res.content.decode())[
+        'updated']['authority']['active_roles'] == ["ADMIN"]
     res = requests.put(
-        f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?name.first=t&name.last=e&name.middle=s&name.suffix=t', # updating name
+        # updating name
+        f'{env.AWG_BASE_URL}{ORG_URL}/{org}/user/{user}?name.first=t&name.last=e&name.middle=s&name.suffix=t',
         headers=org_admin_headers
     )
     assert res.status_code == 200
@@ -545,4 +565,6 @@ def test_admin_role_preserved_after_resetting_own_secret(org_admin_headers):
         headers=headers2
     )
     assert res2.status_code == 200
-    assert json.loads(res2.content.decode())["authority"]["active_roles"][0] == "ADMIN" # admin role still remains after changing secret
+    # admin role still remains after changing secret
+    assert json.loads(res2.content.decode())[
+        "authority"]["active_roles"][0] == "ADMIN"
diff --git a/test-http/src/test/org_user_tests/regular_users.py b/test-http/src/test/org_user_tests/regular_users.py
index 3677716e..3d015c1e 100644
--- a/test-http/src/test/org_user_tests/regular_users.py
+++ b/test-http/src/test/org_user_tests/regular_users.py
@@ -39,7 +39,7 @@ def test_regular_user_update_name_and_username(reg_user_headers):
         headers=reg_user_headers
     )
     assert res.status_code == 403
-    response_contains_json(res, 'error', 'NOT_ORG_ADMIN_OR_SECRETARIAT')
+    response_contains_json(res, 'error', 'NOT_ORG_ADMIN_OR_SECRETARIAT_UPDATE')
 
 
 def test_regular_user_cannot_update_for_another_user(reg_user_headers):
@@ -72,7 +72,7 @@ def test_regular_user_cannot_update_duplicate_user_with_new_username(reg_user_he
         headers=reg_user_headers
     )
     assert res.status_code == 403
-    response_contains_json(res, 'error', 'NOT_ORG_ADMIN_OR_SECRETARIAT')
+    response_contains_json(res, 'error', 'NOT_ORG_ADMIN_OR_SECRETARIAT_UPDATE')
 
 
 def test_regular_user_cannot_update_organization_with_new_shortname(reg_user_headers):
@@ -97,7 +97,7 @@ def test_regular_user_cannot_update_active_state(reg_user_headers):
         headers=reg_user_headers
     )
     assert res.status_code == 403
-    response_contains_json(res, 'error', 'NOT_ORG_ADMIN_OR_SECRETARIAT')
+    response_contains_json(res, 'error', 'NOT_ORG_ADMIN_OR_SECRETARIAT_UPDATE')
 
 
 def test_regular_user_cannot_add_role(reg_user_headers):
@@ -110,7 +110,7 @@ def test_regular_user_cannot_add_role(reg_user_headers):
         headers=reg_user_headers
     )
     assert res.status_code == 403
-    response_contains_json(res, 'error', 'NOT_ORG_ADMIN_OR_SECRETARIAT')
+    response_contains_json(res, 'error', 'NOT_ORG_ADMIN_OR_SECRETARIAT_UPDATE')
 
 
 def test_regular_user_cannot_remove_role(reg_user_headers):
@@ -123,7 +123,7 @@ def test_regular_user_cannot_remove_role(reg_user_headers):
         headers=reg_user_headers
     )
     assert res.status_code == 403
-    response_contains_json(res, 'error', 'NOT_ORG_ADMIN_OR_SECRETARIAT')
+    response_contains_json(res, 'error', 'NOT_ORG_ADMIN_OR_SECRETARIAT_UPDATE')
 
 
 def test_regular_user_cannot_update_user_org_dne(reg_user_headers):
diff --git a/test/unit-tests/user/userUpdateTest.js b/test/unit-tests/user/userUpdateTest.js
index 80b5d400..6002a4cc 100644
--- a/test/unit-tests/user/userUpdateTest.js
+++ b/test/unit-tests/user/userUpdateTest.js
@@ -422,7 +422,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.notOrgAdminOrSecretariat()
+          const errObj = error.notOrgAdminOrSecretariatUpdate()
           expect(res.body.error).to.equal(errObj.error)
           expect(res.body.message).to.equal(errObj.message)
           done()