BCDA-7212: Make 500 response errors properly reflect the error they e…

…ncounter (#174)

## 🎫 Ticket

https://jira.cms.gov/browse/BCDA-7212

## 🛠 Changes

Updated error status on certain endpoint failures to properly describe
what went wrong.

## ℹ️ Context for reviewers

Some 500 errors were being sent incorrectly for a Bad Requests.

## ✅ Acceptance Validation

(How were the changes verified? Did you fully test the acceptance
criteria in the ticket? Provide reproducible testing instructions and
screenshots if applicable.)

## 🔒 Security Implications

- [ ] This PR adds a new software dependency or dependencies.
- [ ] This PR modifies or invalidates one or more of our security
controls.
- [ ] This PR stores or transmits data that was not stored or
transmitted before.
- [ ] This PR requires additional review of its security implications
for other reasons.

If any security implications apply, add Jason Ashbaugh (GitHub username:
StewGoin) as a reviewer and do not merge this PR without his approval.

ssas/service/admin/api.go

@@ -149,7 +149,7 @@ func updateGroup(w http.ResponseWriter, r *http.Request) {
 	gd := ssas.GroupData{}
 	err := json.Unmarshal(body, &gd)
 	if err != nil {
-		logger.Error("failed to marshal JSON: ", err)
+		logger.Error("failed to unmarshal JSON: ", err)
 		service.JSONError(w, http.StatusBadRequest, "invalid request body", "")
 		return
 	}
@@ -272,7 +272,7 @@ func updateSystem(w http.ResponseWriter, r *http.Request) {
 	_, err = ssas.UpdateSystem(r.Context(), id, v)
 	if err != nil {
 		logger.Errorf("failed to update system; %s", err)
-		service.JSONError(w, http.StatusBadRequest, http.StatusText(http.StatusBadRequest), "failed to update system")
+		service.JSONError(w, http.StatusNotFound, http.StatusText(http.StatusNotFound), "failed to update system")
 		return
 	}
 
@@ -308,7 +308,7 @@ func deleteGroup(w http.ResponseWriter, r *http.Request) {
 	err := ssas.DeleteGroup(r.Context(), id)
 	if err != nil {
 		logger.Errorf("failed to delete group; %s", err)
-		service.JSONError(w, http.StatusBadRequest, http.StatusText(http.StatusBadRequest), "failed to delete group")
+		service.JSONError(w, http.StatusNotFound, http.StatusText(http.StatusNotFound), "failed to delete group")
 		return
 	}
 
@@ -565,6 +565,12 @@ func deactivateSystemCredentials(w http.ResponseWriter, r *http.Request) {
 */
 func revokeToken(w http.ResponseWriter, r *http.Request) {
 	tokenID := chi.URLParam(r, "tokenID")
+
+	if tokenID == "" {
+		service.JSONError(w, http.StatusBadRequest, "Missing tokenID", "")
+		return
+	}
+
 	ssas.SetCtxEntry(r, "Op", "TokenBlacklist")
 	logger := ssas.GetCtxLogger(r.Context())
 	logger.Infof("Operation Called: admin.revokeToken()")
@@ -592,7 +598,7 @@ func registerIP(w http.ResponseWriter, r *http.Request) {
 	system, err := ssas.GetSystemByID(r.Context(), systemID)
 	if err != nil {
 		logger.Errorf("failed to retrieve system; %s", err)
-		service.JSONError(w, http.StatusNotFound, http.StatusText(http.StatusBadRequest), "Invalid system ID")
+		service.JSONError(w, http.StatusNotFound, http.StatusText(http.StatusNotFound), "Invalid system ID")
 		return
 	}
 
@@ -653,7 +659,7 @@ func getSystemIPs(w http.ResponseWriter, r *http.Request) {
 	ips, err := system.GetIps(r.Context())
 	if err != nil {
 		logger.Error("Could not retrieve system ips", err)
-		service.JSONError(w, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError), "")
+		service.JSONError(w, http.StatusNotFound, http.StatusText(http.StatusNotFound), "")
 		return
 	}
 
@@ -711,7 +717,7 @@ func deleteSystemIP(w http.ResponseWriter, r *http.Request) {
 	err = system.DeleteIP(r.Context(), ipID)
 	if err != nil {
 		logger.Errorf("failed to delete IP: %s", err)
-		service.JSONError(w, http.StatusBadRequest, http.StatusText(http.StatusBadRequest), "failed to delete IP")
+		service.JSONError(w, http.StatusNotFound, http.StatusText(http.StatusNotFound), "failed to delete IP")
 		return
 	}
 
@@ -742,13 +748,13 @@ func createToken(w http.ResponseWriter, r *http.Request) {
 	b, err := io.ReadAll(r.Body)
 	if err != nil {
 		logger.Error(err)
-		service.JSONError(w, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError), "")
+		service.JSONError(w, http.StatusBadRequest, http.StatusText(http.StatusBadRequest), "")
 		return
 	}
 
 	if err := json.Unmarshal(b, &body); err != nil {
 		logger.Error("unable to marshal JSON: ", err)
-		service.JSONError(w, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError), "")
+		service.JSONError(w, http.StatusBadRequest, http.StatusText(http.StatusBadRequest), "")
 		return
 	}
 
@@ -828,6 +834,12 @@ func createKey(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if pk.PublicKey == "" || pk.Signature == "" {
+		logger.Error("failed to receive PublicKey and/or Signature")
+		service.JSONError(w, http.StatusBadRequest, http.StatusText(http.StatusBadRequest), "")
+		return
+	}
+
 	key, err := system.AddAdditionalPublicKey(strings.NewReader(pk.PublicKey), pk.Signature)
 	if err != nil {
 		logger.Error("failed to add additional public key: ", err)
@@ -849,7 +861,7 @@ func deleteKey(w http.ResponseWriter, r *http.Request) {
 	system, err := ssas.GetSystemByID(r.Context(), systemID)
 	if err != nil {
 		logger.Error("failed to get system: ", err)
-		service.JSONError(w, http.StatusNotFound, http.StatusText(http.StatusInternalServerError), "Invalid system ID")
+		service.JSONError(w, http.StatusNotFound, http.StatusText(http.StatusNotFound), "Invalid system ID")
 		return
 	}
 

ssas/service/admin/api_test.go

@@ -266,7 +266,7 @@ func (s *APITestSuite) TestRevokeTokenNoToken() {
 	handler.ServeHTTP(rr, req)
 
 	// TODO(BCDA-7212): Handle gracefully
-	assert.Equal(s.T(), http.StatusInternalServerError, rr.Result().StatusCode)
+	assert.Equal(s.T(), http.StatusBadRequest, rr.Result().StatusCode)
 }
 
 func (s *APITestSuite) TestDeleteGroup() {
@@ -949,7 +949,7 @@ func (s *APITestSuite) TestDeleteIPIPNotFound() {
 
 	rr := httptest.NewRecorder()
 	handler.ServeHTTP(rr, req)
-	assert.Equal(s.T(), http.StatusBadRequest, rr.Result().StatusCode)
+	assert.Equal(s.T(), http.StatusNotFound, rr.Result().StatusCode)
 	_ = ssas.CleanDatabase(group)
 }
 
@@ -1064,7 +1064,7 @@ func (s *APITestSuite) TestUpdateNonExistingSystem() {
 	handler := http.HandlerFunc(updateSystem)
 	rr := httptest.NewRecorder()
 	handler.ServeHTTP(rr, req)
-	assert.Equal(s.T(), http.StatusBadRequest, rr.Result().StatusCode)
+	assert.Equal(s.T(), http.StatusNotFound, rr.Result().StatusCode)
 	assert.Contains(s.T(), rr.Body.String(), "failed to update system")
 }
 
@@ -1363,7 +1363,7 @@ func (s *APITestSuite) TestCreateV2SystemTokenNonJson() {
 	handler.ServeHTTP(rr, req)
 
 	// TODO(BCDA-7212): Handle gracefully
-	assert.Equal(s.T(), http.StatusInternalServerError, rr.Result().StatusCode)
+	assert.Equal(s.T(), http.StatusBadRequest, rr.Result().StatusCode)
 }
 
 func (s *APITestSuite) TestCreateV2SystemTokenMissingLabel() {
@@ -1504,7 +1504,7 @@ func (s *APITestSuite) TestCreatePublicKeyMissingFields() {
 	handler.ServeHTTP(rr, req)
 
 	// TODO(BCDA-7212): Handle gracefully
-	assert.Equal(s.T(), http.StatusInternalServerError, rr.Result().StatusCode)
+	assert.Equal(s.T(), http.StatusBadRequest, rr.Result().StatusCode)
 }
 
 func (s *APITestSuite) TestDeletePublicKeySystemNotFound() {

ssas/service/admin/router_test.go

@@ -109,7 +109,7 @@ func (s *RouterTestSuite) TestDeleteGroup() {
 	rr := httptest.NewRecorder()
 	s.router.ServeHTTP(rr, req)
 	res := rr.Result()
-	assert.Equal(s.T(), http.StatusBadRequest, res.StatusCode)
+	assert.Equal(s.T(), http.StatusNotFound, res.StatusCode)
 }
 
 func (s *RouterTestSuite) TestPostSystem() {

test/postman_test/SSAS.postman_collection.json

@@ -293,16 +293,16 @@
 			"response": []
 		},
 		{
-			"name": "admin delete group, 400",
+			"name": "admin delete group, 404",
 			"event": [
 				{
 					"listen": "test",
 					"script": {
 						"id": "95ce3470-8202-4e9f-b4f6-4fe36a09cb0d",
 						"exec": [
-							"pm.test(\"response is 400 and record not found\", function () {",
-							"    pm.response.to.have.status(400);",
-							"    pm.response.to.have.body('{\"error\":\"Bad Request\",\"error_description\":\"failed to delete group\"}',)",
+							"pm.test(\"response is 404 and not found\", function () {",
+							"    pm.response.to.have.status(404);",
+							"    pm.response.to.have.body('{\"error\":\"Not Found\",\"error_description\":\"failed to delete group\"}',)",
 							"});"
 						],
 						"type": "text/javascript"