ServiceAccounts: Delete ServiceAccount (#40470)

* Add extra fields to OSS types to support enterprise * WIP service accounts * Update public/app/features/api-keys/ApiKeysForm.tsx Co-authored-by: Hugo Häggmark <hugo.haggmark@grafana.com> * Create a service account at the same time as the API key * Use service account credentials when accessing API with APIkey * Throw better error * Use Boolean for "create service account button" * Add GetRole to service, merge RoleDTO and Role structs This patch merges the identical OSS and Enterprise data structures, which improves the code for two reasons: 1. Makes switching between OSS and Enterprise easier 2. Reduces the chance of incompatibilities developing between the same functions in OSS and Enterprise * Start work cloning permissions onto service account * If API key is not linked to a service account, continue login as usual * Fallback to old auth if no service account linked to key * Commented * Add CloneUserToServiceAccount * Update mock.go * Put graphical bits behind a feature toggle * Start adding LinkAPIKeyToServiceAccount * Update pkg/models/user.go Co-authored-by: Eric Leijonmarck <eric.leijonmarck@gmail.com> * Update pkg/api/apikey.go Co-authored-by: Eric Leijonmarck <eric.leijonmarck@gmail.com> * Update pkg/api/apikey.go Co-authored-by: Eric Leijonmarck <eric.leijonmarck@gmail.com> * Finish LinkAPIKeyToServiceAccount * Update comment * Handle api key link error * Update pkg/services/sqlstore/apikey.go Co-authored-by: Emil Tullstedt <emil.tullstedt@grafana.com> * Feature toggle * Update pkg/services/accesscontrol/accesscontrol.go Co-authored-by: Ieva <ieva.vasiljeva@grafana.com> * Not needed (yet) * Better error messages for OSS accesscontrol * Set an invalid user id as default * ServiceAccountId should be string * Re-arrange field names * ServiceAccountId is integer * Update ossaccesscontrol.go * Linter * Remove fronend edits * Remove console log * Update ApiKeysForm.tsx * feat: add serviceaccount deletion * feat: make sure we do not accidently delete serviceaccount * feat: ServiceAccount Type * refactor: userDeletions function * refactor: serviceaccount deletions\ * refactor: error name and removed attribute for userDeletecommand * refactor:: remove serviceaccount type for now * WIP * add mocked function * Remove unnecessary db query, move to right place * Update pkg/services/accesscontrol/mock/mock.go Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com> * Update pkg/services/accesscontrol/mock/mock.go Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com> * Update pkg/services/accesscontrol/mock/mock.go Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com> * Better error messages * Better and correcter error messages * add mocked function * refactor: move function call, add error msg * add IsServiceAccount and fix table * add service accounts package * WIP * WIP * working serviceaccountsapi registration * WIP tests * test * test working * test running for service * moved the error out of the models package * fixed own review * linting errors * Update pkg/services/serviceaccounts/database/database.go Co-authored-by: Jeremy Price <Jeremy.price@grafana.com> * tests running for api * WIP * WIP * removed unused secrets background svc * removed background svc for serviceaccount infavor or wire.go * serviceaccounts manager tests * registering as backend service Co-authored-by: Jeremy Price <jeremy.price@grafana.com> Co-authored-by: Hugo Häggmark <hugo.haggmark@grafana.com> Co-authored-by: Emil Tullstedt <emil.tullstedt@grafana.com> Co-authored-by: Ieva <ieva.vasiljeva@grafana.com> Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>
2021-11-11 15:10:24 +00:00
parent cd01384d3a
commit 4fd3dd41bc
16 changed files with 509 additions and 21 deletions
@@ -0,0 +1,120 @@
+package api
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/grafana/grafana/pkg/api/routing"
+	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/accesscontrol"
+	accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
+	"github.com/grafana/grafana/pkg/services/serviceaccounts"
+	"github.com/grafana/grafana/pkg/services/serviceaccounts/tests"
+	"github.com/grafana/grafana/pkg/services/sqlstore"
+	"github.com/grafana/grafana/pkg/setting"
+	"github.com/stretchr/testify/require"
+	"gopkg.in/macaron.v1"
+)
+
+var (
+	serviceaccountIDPath = "/api/serviceaccounts/%s"
+)
+
+// test the accesscontrol endpoints
+// with permissions and without permissions
+func TestServiceAccountsAPI_DeleteServiceAccount(t *testing.T) {
+	store := sqlstore.InitTestDB(t)
+	svcmock := tests.ServiceAccountMock{}
+
+	var requestResponse = func(server *macaron.Macaron, httpMethod, requestpath string) *httptest.ResponseRecorder {
+		req, err := http.NewRequest(httpMethod, requestpath, nil)
+		require.NoError(t, err)
+		recorder := httptest.NewRecorder()
+		server.ServeHTTP(recorder, req)
+		return recorder
+	}
+	t.Run("should be able to delete serviceaccount for with permissions", func(t *testing.T) {
+		testcase := struct {
+			user         tests.TestUser
+			acmock       *accesscontrolmock.Mock
+			expectedCode int
+		}{
+
+			user: tests.TestUser{Login: "servicetest1@admin", IsServiceAccount: true},
+			acmock: tests.SetupMockAccesscontrol(
+				t,
+				func(c context.Context, siu *models.SignedInUser) ([]*accesscontrol.Permission, error) {
+					return []*accesscontrol.Permission{{Action: serviceaccounts.ActionDelete, Scope: serviceaccounts.ScopeAll}}, nil
+				},
+				false,
+			),
+			expectedCode: http.StatusOK,
+		}
+		serviceAccountDeletionScenario(t, http.MethodDelete, serviceaccountIDPath, &testcase.user, func(httpmethod string, endpoint string, user *tests.TestUser) {
+			createduser := tests.SetupUserServiceAccount(t, store, testcase.user)
+			server := setupTestServer(t, &svcmock, routing.NewRouteRegister(), testcase.acmock)
+			actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, fmt.Sprint(createduser.Id))).Code
+			require.Equal(t, testcase.expectedCode, actual)
+		})
+	})
+
+	t.Run("should be forbidden to delete serviceaccount via accesscontrol on endpoint", func(t *testing.T) {
+		testcase := struct {
+			user         tests.TestUser
+			acmock       *accesscontrolmock.Mock
+			expectedCode int
+		}{
+			user: tests.TestUser{Login: "servicetest2@admin", IsServiceAccount: true},
+			acmock: tests.SetupMockAccesscontrol(
+				t,
+				func(c context.Context, siu *models.SignedInUser) ([]*accesscontrol.Permission, error) {
+					return []*accesscontrol.Permission{}, nil
+				},
+				false,
+			),
+			expectedCode: http.StatusForbidden,
+		}
+		serviceAccountDeletionScenario(t, http.MethodDelete, serviceaccountIDPath, &testcase.user, func(httpmethod string, endpoint string, user *tests.TestUser) {
+			createduser := tests.SetupUserServiceAccount(t, store, testcase.user)
+			server := setupTestServer(t, &svcmock, routing.NewRouteRegister(), testcase.acmock)
+			actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, fmt.Sprint(createduser.Id))).Code
+			require.Equal(t, testcase.expectedCode, actual)
+		})
+	})
+}
+
+func serviceAccountDeletionScenario(t *testing.T, httpMethod string, endpoint string, user *tests.TestUser, fn func(httpmethod string, endpoint string, user *tests.TestUser)) {
+	t.Helper()
+	fn(httpMethod, endpoint, user)
+}
+
+func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock, routerRegister routing.RouteRegister, acmock *accesscontrolmock.Mock) *macaron.Macaron {
+	a := NewServiceAccountsAPI(
+		svc,
+		acmock,
+		routerRegister,
+	)
+	a.RegisterAPIEndpoints(&setting.Cfg{FeatureToggles: map[string]bool{"service-accounts": true}})
+
+	m := macaron.New()
+	signedUser := &models.SignedInUser{
+		OrgId:   1,
+		OrgRole: models.ROLE_ADMIN,
+	}
+
+	m.Use(func(c *macaron.Context) {
+		ctx := &models.ReqContext{
+			Context:      c,
+			IsSignedIn:   true,
+			SignedInUser: signedUser,
+			Logger:       log.New("serviceaccounts-test"),
+		}
+		c.Map(ctx)
+	})
+	a.RouterRegister.Register(m.Router)
+	return m
+}