- standard user cannot delete another users api-keys (#6208) (#6217)

- added new method to get api key by ID
- added tests

api/apikey/apikey.go

@@ -11,6 +11,7 @@ import (
 type APIKeyService interface {
 	HashRaw(rawKey string) []byte
 	GenerateApiKey(user portainer.User, description string) (string, *portainer.APIKey, error)
+	GetAPIKey(apiKeyID portainer.APIKeyID) (*portainer.APIKey, error)
 	GetAPIKeys(userID portainer.UserID) ([]portainer.APIKey, error)
 	GetDigestUserAndKey(digest []byte) (portainer.User, portainer.APIKey, error)
 	UpdateAPIKey(apiKey *portainer.APIKey) error

api/apikey/service.go

@@ -63,6 +63,11 @@ func (a *apiKeyService) GenerateApiKey(user portainer.User, description string)
 	return prefixedAPIKey, apiKey, nil
 }
 
+// GetAPIKey returns an API key by its ID.
+func (a *apiKeyService) GetAPIKey(apiKeyID portainer.APIKeyID) (*portainer.APIKey, error) {
+	return a.apiKeyRepository.GetAPIKey(apiKeyID)
+}
+
 // GetAPIKeys returns all the API keys associated to a user.
 func (a *apiKeyService) GetAPIKeys(userID portainer.UserID) ([]portainer.APIKey, error) {
 	return a.apiKeyRepository.GetAPIKeysByUserID(userID)

api/apikey/service_test.go

@@ -71,6 +71,26 @@ func Test_GenerateApiKey(t *testing.T) {
 	})
 }
 
+func Test_GetAPIKey(t *testing.T) {
+	is := assert.New(t)
+
+	store, teardown := bolt.MustNewTestStore(true)
+	defer teardown()
+
+	service := NewAPIKeyService(store.APIKeyRepository(), store.User())
+
+	t.Run("Successfully returns all API keys", func(t *testing.T) {
+		user := portainer.User{ID: 1}
+		_, apiKey, err := service.GenerateApiKey(user, "test-1")
+		is.NoError(err)
+
+		apiKeyGot, err := service.GetAPIKey(apiKey.ID)
+		is.NoError(err)
+
+		is.Equal(apiKey, apiKeyGot)
+	})
+}
+
 func Test_GetAPIKeys(t *testing.T) {
 	is := assert.New(t)
 

api/http/handler/users/user_remove_access_token.go

@@ -56,6 +56,15 @@ func (handler *Handler) userRemoveAccessToken(w http.ResponseWriter, r *http.Req
 		return &httperror.HandlerError{http.StatusInternalServerError, "Unable to find a user with the specified identifier inside the database", err}
 	}
 
+	// check if the key exists and the key belongs to the user
+	apiKey, err := handler.apiKeyService.GetAPIKey(portainer.APIKeyID(apiKeyID))
+	if err != nil {
+		return &httperror.HandlerError{http.StatusInternalServerError, "API Key not found", err}
+	}
+	if apiKey.UserID != portainer.UserID(userID) {
+		return &httperror.HandlerError{http.StatusForbidden, "Permission denied to remove api-key", httperrors.ErrUnauthorized}
+	}
+
 	err = handler.apiKeyService.DeleteAPIKey(portainer.APIKeyID(apiKeyID))
 	if err != nil {
 		if err == apikey.ErrInvalidAPIKey {

api/http/handler/users/user_remove_access_token_test.go

@@ -97,4 +97,25 @@ func Test_userRemoveAccessToken(t *testing.T) {
 
 		is.Equal(0, len(keys))
 	})
+
+	t.Run("user cannot delete another users API Keys using api-key auth", func(t *testing.T) {
+		_, adminAPIKey, err := apiKeyService.GenerateApiKey(*adminUser, "admin-key")
+		is.NoError(err)
+
+		rawAPIKey, _, err := apiKeyService.GenerateApiKey(*user, "user-key")
+		is.NoError(err)
+
+		req := httptest.NewRequest(http.MethodDelete, fmt.Sprintf("/users/%d/tokens/%d", user.ID, adminAPIKey.ID), nil)
+		req.Header.Add("x-api-key", rawAPIKey)
+
+		rr := httptest.NewRecorder()
+		h.ServeHTTP(rr, req)
+
+		is.Equal(http.StatusForbidden, rr.Code)
+
+		adminKeyGot, err := apiKeyService.GetAPIKey(adminAPIKey.ID)
+		is.NoError(err)
+
+		is.Equal(adminAPIKey, adminKeyGot)
+	})
 }