Return 404 when deleting non-existing API key (#33346)

The server needs to return a HTTP 404 (Not Found) when an API key that does not exist is deleted.

pkg/api/apikey.go

@@ -43,7 +43,13 @@ func DeleteAPIKey(c *models.ReqContext) response.Response {
 
 	err := bus.Dispatch(cmd)
 	if err != nil {
-		return response.Error(500, "Failed to delete API key", err)
+		var status int
+		if errors.Is(err, models.ErrApiKeyNotFound) {
+			status = 404
+		} else {
+			status = 500
+		}
+		return response.Error(status, "Failed to delete API key", err)
 	}
 
 	return response.Success("API key deleted")

pkg/models/apikey.go

@@ -5,9 +5,12 @@ import (
 	"time"
 )
 
-var ErrInvalidApiKey = errors.New("invalid API key")
-var ErrInvalidApiKeyExpiration = errors.New("negative value for SecondsToLive")
-var ErrDuplicateApiKey = errors.New("API key, organization ID and name must be unique")
+var (
+	ErrApiKeyNotFound          = errors.New("API key not found")
+	ErrInvalidApiKey           = errors.New("invalid API key")
+	ErrInvalidApiKeyExpiration = errors.New("negative value for SecondsToLive")
+	ErrDuplicateApiKey         = errors.New("API key, organization ID and name must be unique")
+)
 
 type ApiKey struct {
 	Id      int64

pkg/services/sqlstore/apikey.go

@@ -35,8 +35,17 @@ func DeleteApiKeyCtx(ctx context.Context, cmd *models.DeleteApiKeyCommand) error
 
 func deleteAPIKey(sess *DBSession, id, orgID int64) error {
 	rawSQL := "DELETE FROM api_key WHERE id=? and org_id=?"
-	_, err := sess.Exec(rawSQL, id, orgID)
-	return err
+	result, err := sess.Exec(rawSQL, id, orgID)
+	if err != nil {
+		return err
+	}
+	n, err := result.RowsAffected()
+	if err != nil {
+		return err
+	} else if n == 0 {
+		return models.ErrApiKeyNotFound
+	}
+	return nil
 }
 
 func AddApiKey(cmd *models.AddApiKeyCommand) error {

pkg/services/sqlstore/apikey_test.go

@@ -3,6 +3,7 @@
 package sqlstore
 
 import (
+	"context"
 	"testing"
 	"time"
 
@@ -122,17 +123,27 @@ func TestApiKeyErrors(t *testing.T) {
 	mockTimeNow()
 	defer resetTimeNow()
 
-	t.Run("Testing API Duplicate Key Errors", func(t *testing.T) {
+	t.Run("Testing API Key errors", func(t *testing.T) {
 		InitTestDB(t)
-		t.Run("Given saved api key", func(t *testing.T) {
-			cmd := models.AddApiKeyCommand{OrgId: 0, Name: "duplicate", Key: "asd"}
-			err := AddApiKey(&cmd)
-			assert.Nil(t, err)
 
-			t.Run("Add API Key with existing Org ID and Name", func(t *testing.T) {
+		t.Run("Delete non-existing key should return error", func(t *testing.T) {
+			cmd := models.DeleteApiKeyCommand{Id: 1}
+			err := DeleteApiKeyCtx(context.Background(), &cmd)
+
+			assert.EqualError(t, err, models.ErrApiKeyNotFound.Error())
+		})
+
+		t.Run("Testing API Duplicate Key Errors", func(t *testing.T) {
+			t.Run("Given saved api key", func(t *testing.T) {
 				cmd := models.AddApiKeyCommand{OrgId: 0, Name: "duplicate", Key: "asd"}
-				err = AddApiKey(&cmd)
-				assert.EqualError(t, err, models.ErrDuplicateApiKey.Error())
+				err := AddApiKey(&cmd)
+				assert.Nil(t, err)
+
+				t.Run("Add API Key with existing Org ID and Name", func(t *testing.T) {
+					cmd := models.AddApiKeyCommand{OrgId: 0, Name: "duplicate", Key: "asd"}
+					err = AddApiKey(&cmd)
+					assert.EqualError(t, err, models.ErrDuplicateApiKey.Error())
+				})
 			})
 		})
 	})