ServiceAccount: Rewrite the api test to use fakes (#60441)

* RBAC: Add fake for permissions service

* ServiceAccount: Rewrite create api tests

* ServiceAccount: Rewrite api delete tests

* ServiceAccount: Rewrite api test for RetriveServiceAccount

* ServiceAccount: Refactor UpdateServiceAccount api test

* ServiceAccount: Refactor CreateToken api test

* ServiceAccount: refactor delete token api tests

* ServiceAccount: rewrite list tokens api test

* Remove test helper that is not used any more

* ServiceAccount: remove unused test helpers

pkg/services/accesscontrol/actest/fake.go

@@ -94,3 +94,36 @@ func (f FakeStore) GetUsersBasicRoles(ctx context.Context, userFilter []int64, o
 func (f FakeStore) DeleteUserPermissions(ctx context.Context, orgID, userID int64) error {
 	return f.ExpectedErr
 }
+
+var _ accesscontrol.PermissionsService = new(FakePermissionsService)
+
+type FakePermissionsService struct {
+	ExpectedErr          error
+	ExpectedPermission   *accesscontrol.ResourcePermission
+	ExpectedPermissions  []accesscontrol.ResourcePermission
+	ExpectedMappedAction string
+}
+
+func (f *FakePermissionsService) GetPermissions(ctx context.Context, user *user.SignedInUser, resourceID string) ([]accesscontrol.ResourcePermission, error) {
+	return f.ExpectedPermissions, f.ExpectedErr
+}
+
+func (f *FakePermissionsService) SetUserPermission(ctx context.Context, orgID int64, user accesscontrol.User, resourceID, permission string) (*accesscontrol.ResourcePermission, error) {
+	return f.ExpectedPermission, f.ExpectedErr
+}
+
+func (f *FakePermissionsService) SetTeamPermission(ctx context.Context, orgID, teamID int64, resourceID, permission string) (*accesscontrol.ResourcePermission, error) {
+	return f.ExpectedPermission, f.ExpectedErr
+}
+
+func (f *FakePermissionsService) SetBuiltInRolePermission(ctx context.Context, orgID int64, builtInRole string, resourceID string, permission string) (*accesscontrol.ResourcePermission, error) {
+	return f.ExpectedPermission, f.ExpectedErr
+}
+
+func (f *FakePermissionsService) SetPermissions(ctx context.Context, orgID int64, resourceID string, commands ...accesscontrol.SetResourcePermissionCommand) ([]accesscontrol.ResourcePermission, error) {
+	return f.ExpectedPermissions, f.ExpectedErr
+}
+
+func (f *FakePermissionsService) MapActions(permission accesscontrol.ResourcePermission) string {
+	return f.ExpectedMappedAction
+}

pkg/services/serviceaccounts/api/api_test.go

@@ -1,585 +1,296 @@
 package api
 
 import (
-	"bytes"
 	"context"
-	"encoding/json"
 	"fmt"
-	"io"
 	"net/http"
-	"net/http/httptest"
+	"strings"
-	"strconv"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/grafana/pkg/api/routing"
-	"github.com/grafana/grafana/pkg/infra/db"
-	"github.com/grafana/grafana/pkg/infra/kvstore"
 	"github.com/grafana/grafana/pkg/infra/log"
-	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
+	"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
-	accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
-	"github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol"
 	"github.com/grafana/grafana/pkg/services/apikey"
-	"github.com/grafana/grafana/pkg/services/apikey/apikeyimpl"
-	"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
-	"github.com/grafana/grafana/pkg/services/licensing"
 	"github.com/grafana/grafana/pkg/services/org"
-	"github.com/grafana/grafana/pkg/services/org/orgimpl"
-	"github.com/grafana/grafana/pkg/services/quota/quotatest"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
-	"github.com/grafana/grafana/pkg/services/serviceaccounts/database"
-	"github.com/grafana/grafana/pkg/services/serviceaccounts/retriever"
-	"github.com/grafana/grafana/pkg/services/serviceaccounts/tests"
-	"github.com/grafana/grafana/pkg/services/sqlstore"
-	"github.com/grafana/grafana/pkg/services/team/teamimpl"
 	"github.com/grafana/grafana/pkg/services/user"
-	"github.com/grafana/grafana/pkg/services/user/userimpl"
 	"github.com/grafana/grafana/pkg/setting"
-	"github.com/grafana/grafana/pkg/web"
+	"github.com/grafana/grafana/pkg/web/webtest"
 )
 
-var (
-	serviceAccountPath   = "/api/serviceaccounts/"
-	serviceAccountIDPath = serviceAccountPath + "%v"
-)
-
-// TODO:
-// refactor this set of tests to make use of fakes for the ServiceAccountService
-// all of the API tests are calling with all of the db store injections
-// which is not ideal
-// this is a bit of a hack to get the tests to pass until we refactor the tests
-// to use fakes as in the user service tests
-
 func TestServiceAccountsAPI_CreateServiceAccount(t *testing.T) {
-	store := db.InitTestDB(t)
+	type TestCase struct {
-	services := setupTestServices(t, store)
-
-	autoAssignOrg := store.Cfg.AutoAssignOrg
-	store.Cfg.AutoAssignOrg = true
-	defer func() {
-		store.Cfg.AutoAssignOrg = autoAssignOrg
-	}()
-
-	orgCmd := &org.CreateOrgCommand{Name: "Some Test Org"}
-	_, err := services.OrgService.CreateWithMember(context.Background(), orgCmd)
-	require.Nil(t, err)
-
-	type testCreateSATestCase struct {
 		desc         string
-		body         map[string]interface{}
+		basicRole    org.RoleType
+		permissions  []accesscontrol.Permission
+		body         string
 		expectedCode int
-		wantID       string
+		expectedSA   *serviceaccounts.ServiceAccountDTO
-		wantError    string
+		expectedErr  error
-		acmock       *accesscontrolmock.Mock
 	}
-	testCases := []testCreateSATestCase{
+
+	tests := []TestCase{
 		{
-			desc:   "should be ok to create service account with permissions",
+			desc:        "should be able to create service account with correct permission",
-			body:   map[string]interface{}{"name": "New SA", "role": "Viewer", "is_disabled": "false"},
+			basicRole:   org.RoleViewer,
-			wantID: "sa-new-sa",
+			permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionCreate}},
-			acmock: tests.SetupMockAccesscontrol(
+			body:        `{"name": "test", "isDisabled": false, "role": "Viewer"}`,
-				t,
+			expectedSA: &serviceaccounts.ServiceAccountDTO{
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
+				Name:       "test",
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionCreate}}, nil
+				OrgId:      1,
-				},
+				IsDisabled: false,
-				false,
+				Role:       string(org.RoleViewer),
-			),
+			},
 			expectedCode: http.StatusCreated,
 		},
 		{
-			desc:   "should fail to create a service account with higher privilege",
+			desc:         "should not be able to create service account without permission",
-			body:   map[string]interface{}{"name": "New SA HP", "role": "Admin"},
+			basicRole:    org.RoleViewer,
-			wantID: "sa-new-sa-hp",
+			permissions:  []accesscontrol.Permission{{}},
-			acmock: tests.SetupMockAccesscontrol(
+			body:         `{"name": "test", "isDisabled": false, "role": "Viewer"}`,
-				t,
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionCreate}}, nil
-				},
-				false,
-			),
 			expectedCode: http.StatusForbidden,
 		},
 		{
-			desc:      "should fail to create a service account with invalid role",
+			desc:         "should not be able to create service account with role that has higher privilege than caller",
-			body:      map[string]interface{}{"name": "New SA", "role": "Random"},
+			basicRole:    org.RoleViewer,
-			wantID:    "sa-new-sa",
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionCreate}},
-			wantError: "invalid role value: Random",
+			body:         `{"name": "test", "isDisabled": false, "role": "Editor"}`,
-			acmock: tests.SetupMockAccesscontrol(
-				t,
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionCreate}}, nil
-				},
-				false,
-			),
-			expectedCode: http.StatusBadRequest,
-		},
-		{
-			desc:      "not ok - duplicate name",
-			body:      map[string]interface{}{"name": "New SA"},
-			wantError: "service account already exists",
-			acmock: tests.SetupMockAccesscontrol(
-				t,
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionCreate}}, nil
-				},
-				false,
-			),
-			expectedCode: http.StatusBadRequest,
-		},
-		{
-			desc:      "not ok - missing name",
-			body:      map[string]interface{}{},
-			wantError: "required value Name must not be empty",
-			acmock: tests.SetupMockAccesscontrol(
-				t,
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionCreate}}, nil
-				},
-				false,
-			),
-			expectedCode: http.StatusBadRequest,
-		},
-		{
-			desc: "should be forbidden to create service account if no permissions",
-			body: map[string]interface{}{},
-			acmock: tests.SetupMockAccesscontrol(
-				t,
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{}, nil
-				},
-				false,
-			),
 			expectedCode: http.StatusForbidden,
 		},
+		{
+			desc:         "should not be able to create service account with invalid role",
+			basicRole:    org.RoleViewer,
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionCreate}},
+			body:         `{"name": "test", "isDisabled": false, "role": "random"}`,
+			expectedCode: http.StatusBadRequest,
+		},
+		{
+			desc:         "should not be able to create service account with missing name",
+			basicRole:    org.RoleViewer,
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionCreate}},
+			body:         `{"name": "", "isDisabled": false, "role": "Viewer"}`,
+			expectedCode: http.StatusBadRequest,
+		},
 	}
 
-	var requestResponse = func(server *web.Mux, httpMethod, requestpath string, body io.Reader) *httptest.ResponseRecorder {
+	for _, tt := range tests {
-		req, err := http.NewRequest(httpMethod, requestpath, body)
+		t.Run(tt.desc, func(t *testing.T) {
-		req.Header.Add("Content-Type", "application/json")
+			server := setupTests(t, func(a *ServiceAccountsAPI) {
-		require.NoError(t, err)
+				a.service = &fakeService{ExpectedServiceAccount: tt.expectedSA, ExpectedErr: tt.expectedErr}
-		recorder := httptest.NewRecorder()
-		server.ServeHTTP(recorder, req)
-		return recorder
-	}
-
-	testUser := &tests.TestUser{}
-	for _, tc := range testCases {
-		t.Run(tc.desc, func(t *testing.T) {
-			serviceAccountRequestScenario(t, http.MethodPost, serviceAccountPath, testUser, func(httpmethod string, endpoint string, usr *tests.TestUser) {
-				server, api := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store)
-				marshalled, err := json.Marshal(tc.body)
-				require.NoError(t, err)
-
-				ioReader := bytes.NewReader(marshalled)
-
-				actual := requestResponse(server, httpmethod, endpoint, ioReader)
-
-				actualCode := actual.Code
-				actualBody := map[string]interface{}{}
-
-				err = json.Unmarshal(actual.Body.Bytes(), &actualBody)
-				require.NoError(t, err)
-				require.Equal(t, tc.expectedCode, actualCode, actualBody)
-
-				if actualCode == http.StatusCreated {
-					sa := serviceaccounts.ServiceAccountDTO{}
-					err = json.Unmarshal(actual.Body.Bytes(), &sa)
-					require.NoError(t, err)
-					assert.NotZero(t, sa.Id)
-					assert.Equal(t, tc.body["name"], sa.Name)
-					assert.Equal(t, tc.wantID, sa.Login)
-					tempUser := &user.SignedInUser{
-						OrgID:  1,
-						UserID: 1,
-						Permissions: map[int64]map[string][]string{
-							1: {
-								serviceaccounts.ActionRead:       []string{serviceaccounts.ScopeAll},
-								accesscontrol.ActionOrgUsersRead: []string{accesscontrol.ScopeUsersAll},
-							},
-						},
-					}
-					perms, err := api.permissionService.GetPermissions(context.Background(), tempUser, strconv.FormatInt(sa.Id, 10))
-					assert.NoError(t, err)
-					assert.Equal(t, 1, len(perms), "should have added managed permissions for SA creator")
-					assert.Equal(t, int64(1), perms[0].ID)
-					assert.Equal(t, int64(1), perms[0].UserId)
-				} else if actualCode == http.StatusBadRequest {
-					assert.Contains(t, tc.wantError, actualBody["error"].(string))
-				}
 			})
+			req := server.NewRequest(http.MethodPost, "/api/serviceaccounts/", strings.NewReader(tt.body))
+			webtest.RequestWithSignedInUser(req, &user.SignedInUser{OrgRole: tt.basicRole, OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}})
+			res, err := server.SendJSON(req)
+			require.NoError(t, err)
+
+			assert.Equal(t, tt.expectedCode, res.StatusCode)
+			require.NoError(t, res.Body.Close())
 		})
 	}
 }
 
-// test the accesscontrol endpoints
-// with permissions and without permissions
 func TestServiceAccountsAPI_DeleteServiceAccount(t *testing.T) {
-	store := db.InitTestDB(t)
+	type TestCase struct {
-	services := setupTestServices(t, store)
+		desc         string
-
+		id           int64
-	var requestResponse = func(server *web.Mux, httpMethod, requestpath string) *httptest.ResponseRecorder {
+		permissions  []accesscontrol.Permission
-		req, err := http.NewRequest(httpMethod, requestpath, nil)
+		expectedCode int
-		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},
+	tests := []TestCase{
-			acmock: tests.SetupMockAccesscontrol(
+		{
-				t,
+			desc:         "should be able to delete service account with correct permission",
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
+			id:           1,
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionDelete, Scope: serviceaccounts.ScopeAll}}, nil
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionDelete, Scope: "serviceaccounts:id:1"}},
-				},
-				false,
-			),
 			expectedCode: http.StatusOK,
-		}
+		},
-		serviceAccountRequestScenario(t, http.MethodDelete, serviceAccountIDPath, &testcase.user, func(httpmethod string, endpoint string, user *tests.TestUser) {
+		{
-			createduser := tests.SetupUserServiceAccount(t, store, testcase.user)
+			desc:         "should not ba able to delete with wrong permission",
-			server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), testcase.acmock, store)
+			id:           2,
-			actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, fmt.Sprint(createduser.ID))).Code
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionDelete, Scope: "serviceaccounts:id:1"}},
-			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 *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{}, nil
-				},
-				false,
-			),
 			expectedCode: http.StatusForbidden,
-		}
+		},
-		serviceAccountRequestScenario(t, http.MethodDelete, serviceAccountIDPath, &testcase.user, func(httpmethod string, endpoint string, user *tests.TestUser) {
-			createduser := tests.SetupUserServiceAccount(t, store, testcase.user)
-			server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), testcase.acmock, store)
-			actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, createduser.ID)).Code
-			require.Equal(t, testcase.expectedCode, actual)
-		})
-	})
-}
-
-func serviceAccountRequestScenario(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,
-	sqlStore db.DB,
-) (*web.Mux, *ServiceAccountsAPI) {
-	cfg := setting.NewCfg()
-	teamSvc := teamimpl.ProvideService(sqlStore, cfg)
-	orgSvc, err := orgimpl.ProvideService(sqlStore, cfg, quotatest.New(false, nil))
-	require.NoError(t, err)
-
-	userSvc, err := userimpl.ProvideService(sqlStore, orgSvc, cfg, teamimpl.ProvideService(sqlStore, cfg), nil, quotatest.New(false, nil))
-	require.NoError(t, err)
-
-	// TODO: create fake for retriever to pass into the permissionservice
-	retrieverSvc := retriever.ProvideService(sqlStore, nil, nil, nil, nil)
-	saPermissionService, err := ossaccesscontrol.ProvideServiceAccountPermissions(
-		cfg, routing.NewRouteRegister(), sqlStore, acmock, &licensing.OSSLicensingService{}, retrieverSvc, acmock, teamSvc, userSvc)
-	require.NoError(t, err)
-	acService := actest.FakeService{}
-
-	a := NewServiceAccountsAPI(cfg, svc, acmock, acService, routerRegister, saPermissionService)
-	a.RegisterAPIEndpoints()
-
-	a.cfg.ApiKeyMaxSecondsToLive = -1 // disable api key expiration
-
-	m := web.New()
-	signedUser := &user.SignedInUser{
-		OrgID:   1,
-		UserID:  1,
-		OrgRole: org.RoleViewer,
 	}
 
-	m.Use(func(c *web.Context) {
+	for _, tt := range tests {
-		ctx := &models.ReqContext{
+		t.Run(tt.desc, func(t *testing.T) {
-			Context:      c,
+			server := setupTests(t)
-			IsSignedIn:   true,
+			req := server.NewRequest(http.MethodDelete, fmt.Sprintf("/api/serviceaccounts/%d", tt.id), nil)
-			SignedInUser: signedUser,
+			webtest.RequestWithSignedInUser(req, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}})
-			Logger:       log.New("serviceaccounts-test"),
+			res, err := server.Send(req)
-		}
+			require.NoError(t, err)
-		c.Req = c.Req.WithContext(ctxkey.Set(c.Req.Context(), ctx))
+
-	})
+			assert.Equal(t, tt.expectedCode, res.StatusCode)
-	a.RouterRegister.Register(m.Router)
+			require.NoError(t, res.Body.Close())
-	return m, a
+		})
+	}
 }
 
 func TestServiceAccountsAPI_RetrieveServiceAccount(t *testing.T) {
-	store := db.InitTestDB(t)
+	type TestCase struct {
-	services := setupTestServices(t, store)
-
-	type testRetrieveSATestCase struct {
 		desc         string
-		user         *tests.TestUser
+		id           int64
+		permissions  []accesscontrol.Permission
 		expectedCode int
-		acmock       *accesscontrolmock.Mock
+		expectedSA   *serviceaccounts.ServiceAccountProfileDTO
-		Id           int
 	}
-	testCases := []testRetrieveSATestCase{
+
+	tests := []TestCase{
 		{
-			desc: "should be ok to retrieve serviceaccount with permissions",
+			desc:         "should be able to get service account with correct permission",
-			user: &tests.TestUser{Login: "servicetest1@admin", IsServiceAccount: true},
+			id:           1,
-			acmock: tests.SetupMockAccesscontrol(
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionRead, Scope: "serviceaccounts:id:1"}},
-				t,
+			expectedSA:   &serviceaccounts.ServiceAccountProfileDTO{},
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionRead, Scope: serviceaccounts.ScopeAll}}, nil
-				},
-				false,
-			),
 			expectedCode: http.StatusOK,
 		},
 		{
-			desc: "should be forbidden to retrieve serviceaccount if no permissions",
+			desc:         "should not ba able to get service account with wrong permission",
-			user: &tests.TestUser{Login: "servicetest2@admin", IsServiceAccount: true},
+			id:           2,
-			acmock: tests.SetupMockAccesscontrol(
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionRead, Scope: "serviceaccounts:id:1"}},
-				t,
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{}, nil
-				},
-				false,
-			),
 			expectedCode: http.StatusForbidden,
 		},
-		{
-			desc: "should be not found when the user doesnt exist",
-			user: nil,
-			Id:   12,
-			acmock: tests.SetupMockAccesscontrol(
-				t,
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionRead, Scope: serviceaccounts.ScopeAll}}, nil
-				},
-				false,
-			),
-			expectedCode: http.StatusNotFound,
-		},
 	}
 
-	var requestResponse = func(server *web.Mux, httpMethod, requestpath string) *httptest.ResponseRecorder {
+	for _, tt := range tests {
-		req, err := http.NewRequest(httpMethod, requestpath, nil)
+		t.Run(tt.desc, func(t *testing.T) {
-		require.NoError(t, err)
+			server := setupTests(t, func(a *ServiceAccountsAPI) {
-		recorder := httptest.NewRecorder()
+				a.service = &fakeService{ExpectedServiceAccountProfile: tt.expectedSA}
-		server.ServeHTTP(recorder, req)
-		return recorder
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.desc, func(t *testing.T) {
-			serviceAccountRequestScenario(t, http.MethodGet, serviceAccountIDPath, tc.user, func(httpmethod string, endpoint string, user *tests.TestUser) {
-				scopeID := tc.Id
-				if tc.user != nil {
-					createdUser := tests.SetupUserServiceAccount(t, store, *tc.user)
-					scopeID = int(createdUser.ID)
-				}
-				server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store)
-
-				actual := requestResponse(server, httpmethod, fmt.Sprintf(endpoint, scopeID))
-
-				actualCode := actual.Code
-				require.Equal(t, tc.expectedCode, actualCode)
-
-				if actualCode == http.StatusOK {
-					actualBody := map[string]interface{}{}
-					err := json.Unmarshal(actual.Body.Bytes(), &actualBody)
-					require.NoError(t, err)
-					require.Equal(t, scopeID, int(actualBody["id"].(float64)))
-					require.Equal(t, tc.user.Login, actualBody["login"].(string))
-				}
 			})
+			req := server.NewGetRequest(fmt.Sprintf("/api/serviceaccounts/%d", tt.id))
+			webtest.RequestWithSignedInUser(req, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}})
+			res, err := server.Send(req)
+			require.NoError(t, err)
+			assert.Equal(t, tt.expectedCode, res.StatusCode)
+			require.NoError(t, res.Body.Close())
 		})
 	}
 }
 
-func newString(s string) *string {
-	return &s
-}
-
 func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) {
-	store := db.InitTestDB(t)
+	type TestCase struct {
-	services := setupTestServices(t, store)
-
-	type testUpdateSATestCase struct {
 		desc         string
-		user         *tests.TestUser
+		id           int64
+		body         string
+		basicRole    org.RoleType
+		permissions  []accesscontrol.Permission
+		expectedSA   *serviceaccounts.ServiceAccountProfileDTO
 		expectedCode int
-		acmock       *accesscontrolmock.Mock
-		body         *serviceaccounts.UpdateServiceAccountForm
-		Id           int
 	}
 
-	viewerRole := org.RoleViewer
+	tests := []TestCase{
-	editorRole := org.RoleEditor
-	var invalidRole org.RoleType = "InvalidRole"
-	testCases := []testUpdateSATestCase{
 		{
-			desc: "should be ok to update serviceaccount with permissions",
+			desc:         "should be able to update service account with correct permission",
-			user: &tests.TestUser{Login: "servicetest1@admin", IsServiceAccount: true, Role: "Viewer", Name: "Unaltered"},
+			id:           1,
-			body: &serviceaccounts.UpdateServiceAccountForm{Name: newString("New Name"), Role: &viewerRole},
+			body:         `{"role": "Editor"}`,
-			acmock: tests.SetupMockAccesscontrol(
+			basicRole:    org.RoleAdmin,
-				t,
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}},
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
+			expectedSA:   &serviceaccounts.ServiceAccountProfileDTO{},
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: serviceaccounts.ScopeAll}}, nil
-				},
-				false,
-			),
 			expectedCode: http.StatusOK,
 		},
 		{
-			desc: "should be forbidden to set role higher than user's role",
+			desc:         "should not be able to update service account with wrong permission",
-			user: &tests.TestUser{Login: "servicetest2@admin", IsServiceAccount: true, Role: "Viewer", Name: "Unaltered 2"},
+			id:           2,
-			body: &serviceaccounts.UpdateServiceAccountForm{Name: newString("New Name 2"), Role: &editorRole},
+			body:         `{}`,
-			acmock: tests.SetupMockAccesscontrol(
+			basicRole:    org.RoleAdmin,
-				t,
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}},
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: serviceaccounts.ScopeAll}}, nil
-				},
-				false,
-			),
 			expectedCode: http.StatusForbidden,
 		},
 		{
-			desc: "bad request when invalid role",
+			desc:         "should not be able to update service account with a role that has higher privilege then caller",
-			user: &tests.TestUser{Login: "servicetest3@admin", IsServiceAccount: true, Role: "Invalid", Name: "Unaltered"},
+			id:           1,
-			body: &serviceaccounts.UpdateServiceAccountForm{Name: newString("NameB"), Role: &invalidRole},
+			body:         `{"role": "Admin"}`,
-			acmock: tests.SetupMockAccesscontrol(
+			basicRole:    org.RoleEditor,
-				t,
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}},
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
+			expectedCode: http.StatusForbidden,
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: serviceaccounts.ScopeAll}}, nil
+		},
-				},
+		{
-				false,
+			desc:         "should not be able to update service account with invalid role",
-			),
+			id:           1,
+			body:         `{"role": "fake"}`,
+			basicRole:    org.RoleEditor,
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}},
 			expectedCode: http.StatusBadRequest,
 		},
-		{
-			desc: "should be forbidden to update serviceaccount if no permissions",
-			user: &tests.TestUser{Login: "servicetest4@admin", IsServiceAccount: true},
-			body: nil,
-			acmock: tests.SetupMockAccesscontrol(
-				t,
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{}, nil
-				},
-				false,
-			),
-			expectedCode: http.StatusForbidden,
-		},
-		{
-			desc: "should be not found when the user doesnt exist",
-			user: nil,
-			body: nil,
-			Id:   12,
-			acmock: tests.SetupMockAccesscontrol(
-				t,
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: serviceaccounts.ScopeAll}}, nil
-				},
-				false,
-			),
-			expectedCode: http.StatusNotFound,
-		},
 	}
 
-	var requestResponse = func(server *web.Mux, httpMethod, requestpath string, body io.Reader) *httptest.ResponseRecorder {
+	for _, tt := range tests {
-		req, err := http.NewRequest(httpMethod, requestpath, body)
+		t.Run(tt.desc, func(t *testing.T) {
-		req.Header.Add("Content-Type", "application/json")
+			server := setupTests(t, func(a *ServiceAccountsAPI) {
-		require.NoError(t, err)
+				a.service = &fakeService{ExpectedServiceAccountProfile: tt.expectedSA}
-		recorder := httptest.NewRecorder()
+			})
-		server.ServeHTTP(recorder, req)
-		return recorder
-	}
 
-	for _, tc := range testCases {
+			req := server.NewRequest(http.MethodPatch, fmt.Sprintf("/api/serviceaccounts/%d", tt.id), strings.NewReader(tt.body))
-		t.Run(tc.desc, func(t *testing.T) {
+			webtest.RequestWithSignedInUser(req, &user.SignedInUser{OrgRole: tt.basicRole, OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}})
-			server, saAPI := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store)
+			res, err := server.SendJSON(req)
-			scopeID := tc.Id
+			require.NoError(t, err)
-			if tc.user != nil {
-				createdUser := tests.SetupUserServiceAccount(t, store, *tc.user)
-				scopeID = int(createdUser.ID)
-			}
 
-			var rawBody io.Reader = http.NoBody
+			assert.Equal(t, tt.expectedCode, res.StatusCode)
-			if tc.body != nil {
+			require.NoError(t, res.Body.Close())
-				body, err := json.Marshal(tc.body)
-				require.NoError(t, err)
-				rawBody = bytes.NewReader(body)
-			}
-
-			actual := requestResponse(server, http.MethodPatch, fmt.Sprintf(serviceAccountIDPath, scopeID), rawBody)
-
-			actualCode := actual.Code
-			require.Equal(t, tc.expectedCode, actualCode)
-
-			if actualCode == http.StatusOK {
-				actualBody := map[string]interface{}{}
-				err := json.Unmarshal(actual.Body.Bytes(), &actualBody)
-				require.NoError(t, err)
-				assert.Equal(t, scopeID, int(actualBody["id"].(float64)))
-				assert.Equal(t, *tc.body.Name, actualBody["name"].(string))
-				serviceAccountData := actualBody["serviceaccount"].(map[string]interface{})
-				assert.Equal(t, string(*tc.body.Role), serviceAccountData["role"].(string))
-				assert.Equal(t, tc.user.Login, serviceAccountData["login"].(string))
-
-				// Ensure the user was updated in DB
-				sa, err := saAPI.service.RetrieveServiceAccount(context.Background(), 1, int64(scopeID))
-				require.NoError(t, err)
-				require.Equal(t, *tc.body.Name, sa.Name)
-				require.Equal(t, string(*tc.body.Role), sa.Role)
-			}
 		})
 	}
 }
 
-type services struct {
+func setupTests(t *testing.T, opts ...func(a *ServiceAccountsAPI)) *webtest.Server {
-	OrgService    org.Service
+	t.Helper()
-	UserService   user.Service
+	cfg := setting.NewCfg()
-	SAService     tests.ServiceAccountMock
+	api := &ServiceAccountsAPI{
-	APIKeyService apikey.Service
+		cfg:                  cfg,
-}
+		service:              &fakeService{},
-
+		accesscontrolService: &actest.FakeService{},
-func setupTestServices(t *testing.T, db *sqlstore.SQLStore) services {
+		accesscontrol:        acimpl.ProvideAccessControl(cfg),
-	kvStore := kvstore.ProvideService(db)
+		RouterRegister:       routing.NewRouteRegister(),
-	quotaService := quotatest.New(false, nil)
+		log:                  log.NewNopLogger(),
-	apiKeyService, err := apikeyimpl.ProvideService(db, db.Cfg, quotaService)
+		permissionService:    &actest.FakePermissionsService{},
-	require.NoError(t, err)
-
-	orgService, err := orgimpl.ProvideService(db, setting.NewCfg(), quotaService)
-	require.NoError(t, err)
-	userSvc, err := userimpl.ProvideService(db, orgService, db.Cfg, nil, nil, quotaService)
-	require.NoError(t, err)
-
-	saStore := database.ProvideServiceAccountsStore(nil, db, apiKeyService, kvStore, userSvc, orgService)
-	svcmock := tests.ServiceAccountMock{Store: saStore, Calls: tests.Calls{}, Stats: nil, SecretScanEnabled: false}
-
-	return services{
-		OrgService:    orgService,
-		UserService:   userSvc,
-		SAService:     svcmock,
-		APIKeyService: apiKeyService,
 	}
+
+	for _, o := range opts {
+		o(api)
+	}
+	api.RegisterAPIEndpoints()
+	return webtest.NewServer(t, api.RouterRegister)
+}
+
+var _ service = new(fakeService)
+
+type fakeService struct {
+	service
+	ExpectedErr                   error
+	ExpectedApiKey                *apikey.APIKey
+	ExpectedServiceAccountTokens  []apikey.APIKey
+	ExpectedServiceAccount        *serviceaccounts.ServiceAccountDTO
+	ExpectedServiceAccountProfile *serviceaccounts.ServiceAccountProfileDTO
+}
+
+func (f *fakeService) CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) {
+	return f.ExpectedServiceAccount, f.ExpectedErr
+}
+
+func (f *fakeService) DeleteServiceAccount(ctx context.Context, orgID, id int64) error {
+	return f.ExpectedErr
+}
+
+func (f *fakeService) RetrieveServiceAccount(ctx context.Context, orgID, id int64) (*serviceaccounts.ServiceAccountProfileDTO, error) {
+	return f.ExpectedServiceAccountProfile, f.ExpectedErr
+}
+
+func (f *fakeService) ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error) {
+	return f.ExpectedServiceAccountTokens, f.ExpectedErr
+}
+
+func (f *fakeService) UpdateServiceAccount(ctx context.Context, orgID, id int64, cmd *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) {
+	return f.ExpectedServiceAccountProfile, f.ExpectedErr
+}
+
+func (f *fakeService) AddServiceAccountToken(ctx context.Context, id int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error {
+	cmd.Result = f.ExpectedApiKey
+	return f.ExpectedErr
+}
+
+func (f *fakeService) DeleteServiceAccountToken(ctx context.Context, orgID, id, tokenID int64) error {
+	return f.ExpectedErr
 }

pkg/services/serviceaccounts/api/token_test.go

@@ -1,12 +1,8 @@
 package api
 
 import (
-	"context"
-	"encoding/json"
 	"fmt"
-	"io"
 	"net/http"
-	"net/http/httptest"
 	"strings"
 	"testing"
 	"time"
@@ -14,349 +10,169 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"github.com/grafana/grafana/pkg/api/routing"
-	"github.com/grafana/grafana/pkg/components/apikeygen"
-	apikeygenprefix "github.com/grafana/grafana/pkg/components/apikeygenprefixed"
-	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
-	accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
 	"github.com/grafana/grafana/pkg/services/apikey"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
-	"github.com/grafana/grafana/pkg/services/serviceaccounts/tests"
 	"github.com/grafana/grafana/pkg/services/user"
-	"github.com/grafana/grafana/pkg/web"
+	"github.com/grafana/grafana/pkg/web/webtest"
 )
 
-const (
+func TestServiceAccountsAPI_ListTokens(t *testing.T) {
-	serviceaccountIDTokensPath       = "/api/serviceaccounts/%v/tokens"    // #nosec G101
+	type TestCase struct {
-	serviceaccountIDTokensDetailPath = "/api/serviceaccounts/%v/tokens/%v" // #nosec G101
-)
-
-func createTokenforSA(t *testing.T, service serviceaccounts.Service, keyName string, orgID int64, saID int64, secondsToLive int64) *apikey.APIKey {
-	key, err := apikeygen.New(orgID, keyName)
-	require.NoError(t, err)
-
-	cmd := serviceaccounts.AddServiceAccountTokenCommand{
-		Name:          keyName,
-		OrgId:         orgID,
-		Key:           key.HashedKey,
-		SecondsToLive: secondsToLive,
-		Result:        &apikey.APIKey{},
-	}
-
-	err = service.AddServiceAccountToken(context.Background(), saID, &cmd)
-	require.NoError(t, err)
-	return cmd.Result
-}
-
-func TestServiceAccountsAPI_CreateToken(t *testing.T) {
-	store := db.InitTestDB(t)
-	services := setupTestServices(t, store)
-	sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true})
-
-	type testCreateSAToken struct {
 		desc         string
+		id           int64
+		permissions  []accesscontrol.Permission
 		expectedCode int
-		body         map[string]interface{}
-		acmock       *accesscontrolmock.Mock
 	}
 
-	testCases := []testCreateSAToken{
+	tests := []TestCase{
 		{
-			desc: "should be ok to create serviceaccount token with scope all permissions",
+			desc:         "should be able to list tokens with correct permission",
-			acmock: tests.SetupMockAccesscontrol(
+			id:           1,
-				t,
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionRead, Scope: "serviceaccounts:id:1"}},
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: serviceaccounts.ScopeAll}}, nil
-				},
-				false,
-			),
-			body:         map[string]interface{}{"name": "Test1", "role": "Viewer", "secondsToLive": 1},
 			expectedCode: http.StatusOK,
 		},
 		{
-			desc: "serviceaccount token should match SA orgID and SA provided in parameters even if specified in body",
+			desc:         "should not be able to list tokens with wrong permission",
-			acmock: tests.SetupMockAccesscontrol(
+			id:           2,
-				t,
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionRead, Scope: "serviceaccounts:id:1"}},
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: serviceaccounts.ScopeAll}}, nil
-				},
-				false,
-			),
-			body:         map[string]interface{}{"name": "Test2", "role": "Viewer", "secondsToLive": 1, "orgId": 4, "serviceAccountId": 4},
-			expectedCode: http.StatusOK,
-		},
-		{
-			desc: "should be ok to create serviceaccount token with scope id permissions",
-			acmock: tests.SetupMockAccesscontrol(
-				t,
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}}, nil
-				},
-				false,
-			),
-			body:         map[string]interface{}{"name": "Test3", "role": "Viewer", "secondsToLive": 1},
-			expectedCode: http.StatusOK,
-		},
-		{
-			desc: "should be forbidden to create serviceaccount token if wrong scoped",
-			acmock: tests.SetupMockAccesscontrol(
-				t,
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:2"}}, nil
-				},
-				false,
-			),
-			body:         map[string]interface{}{"name": "Test4", "role": "Viewer"},
 			expectedCode: http.StatusForbidden,
 		},
 	}
 
-	var requestResponse = func(server *web.Mux, httpMethod, requestpath string, requestBody io.Reader) *httptest.ResponseRecorder {
+	for _, tt := range tests {
-		req, err := http.NewRequest(httpMethod, requestpath, requestBody)
+		t.Run(tt.desc, func(t *testing.T) {
-		require.NoError(t, err)
+			server := setupTests(t, func(a *ServiceAccountsAPI) {
-		req.Header.Add("Content-Type", "application/json")
+				a.service = &fakeService{}
-		recorder := httptest.NewRecorder()
+			})
-		server.ServeHTTP(recorder, req)
+			req := server.NewGetRequest(fmt.Sprintf("/api/serviceaccounts/%d/tokens", tt.id))
-		return recorder
+			webtest.RequestWithSignedInUser(req, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}})
+			res, err := server.Send(req)
+			require.NoError(t, err)
+
+			assert.Equal(t, tt.expectedCode, res.StatusCode)
+			require.NoError(t, res.Body.Close())
+		})
+	}
+}
+
+func TestServiceAccountsAPI_CreateToken(t *testing.T) {
+	type TestCase struct {
+		desc           string
+		id             int64
+		body           string
+		permissions    []accesscontrol.Permission
+		tokenTTL       int64
+		expectedErr    error
+		expectedApiKey *apikey.APIKey
+		expectedCode   int
 	}
 
-	for _, tc := range testCases {
+	tests := []TestCase{
-		t.Run(tc.desc, func(t *testing.T) {
+		{
-			endpoint := fmt.Sprintf(serviceaccountIDTokensPath, sa.ID)
+			desc:           "should be able to create token for service account with correct permission",
-			bodyString := ""
+			id:             1,
-			if tc.body != nil {
+			body:           `{"name": "test"}`,
-				b, err := json.Marshal(tc.body)
+			tokenTTL:       -1,
-				require.NoError(t, err)
+			permissions:    []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}},
-				bodyString = string(b)
+			expectedApiKey: &apikey.APIKey{},
-			}
+			expectedCode:   http.StatusOK,
+		},
+		{
+			desc:         "should not be able to create token for service account with wrong permission",
+			id:           2,
+			body:         `{"name": "test"}`,
+			tokenTTL:     -1,
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}},
+			expectedCode: http.StatusForbidden,
+		},
+		{
+			desc:         "should not be able to create token for service account that dont exists",
+			id:           1,
+			body:         `{"name": "test"}`,
+			tokenTTL:     -1,
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}},
+			expectedErr:  serviceaccounts.ErrServiceAccountNotFound,
+			expectedCode: http.StatusNotFound,
+		},
+		{
+			desc:         "should not be able to create token for service account if max ttl is configured but not set in body",
+			id:           1,
+			body:         `{"name": "test"}`,
+			tokenTTL:     10 * int64(time.Hour),
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}},
+			expectedCode: http.StatusBadRequest,
+		},
+	}
 
-			server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store)
+	for _, tt := range tests {
-			actual := requestResponse(server, http.MethodPost, endpoint, strings.NewReader(bodyString))
+		t.Run(tt.desc, func(t *testing.T) {
-
+			server := setupTests(t, func(a *ServiceAccountsAPI) {
-			actualCode := actual.Code
+				a.cfg.ApiKeyMaxSecondsToLive = tt.tokenTTL
-			actualBody := map[string]interface{}{}
+				a.service = &fakeService{
-
+					ExpectedErr:    tt.expectedErr,
-			err := json.Unmarshal(actual.Body.Bytes(), &actualBody)
+					ExpectedApiKey: tt.expectedApiKey,
+				}
+			})
+			req := server.NewRequest(http.MethodPost, fmt.Sprintf("/api/serviceaccounts/%d/tokens", tt.id), strings.NewReader(tt.body))
+			webtest.RequestWithSignedInUser(req, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}})
+			res, err := server.SendJSON(req)
 			require.NoError(t, err)
-			require.Equal(t, tc.expectedCode, actualCode, endpoint, actualBody)
 
-			if actualCode == http.StatusOK {
+			assert.Equal(t, tt.expectedCode, res.StatusCode)
-				assert.Equal(t, tc.body["name"], actualBody["name"])
+			require.NoError(t, res.Body.Close())
-
-				query := apikey.GetByNameQuery{KeyName: tc.body["name"].(string), OrgId: sa.OrgID}
-				err = services.APIKeyService.GetApiKeyByName(context.Background(), &query)
-				require.NoError(t, err)
-
-				assert.Equal(t, sa.ID, *query.Result.ServiceAccountId)
-				assert.Equal(t, sa.OrgID, query.Result.OrgId)
-				assert.True(t, strings.HasPrefix(actualBody["key"].(string), "glsa"))
-
-				keyInfo, err := apikeygenprefix.Decode(actualBody["key"].(string))
-				assert.NoError(t, err)
-
-				hash, err := keyInfo.Hash()
-				require.NoError(t, err)
-				require.Equal(t, query.Result.Key, hash)
-			}
 		})
 	}
 }
 
 func TestServiceAccountsAPI_DeleteToken(t *testing.T) {
-	store := db.InitTestDB(t)
+	type TestCase struct {
-	services := setupTestServices(t, store)
-
-	sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true})
-
-	type testCreateSAToken struct {
 		desc         string
-		keyName      string
+		saID         int64
+		apikeyID     int64
+		permissions  []accesscontrol.Permission
+		expectedErr  error
 		expectedCode int
-		acmock       *accesscontrolmock.Mock
 	}
 
-	testCases := []testCreateSAToken{
+	tests := []TestCase{
 		{
-			desc:    "should be ok to delete serviceaccount token with scope id permissions",
+			desc:         "should be able to delete service account token with correct permission",
-			keyName: "Test1",
+			saID:         1,
-			acmock: tests.SetupMockAccesscontrol(
+			apikeyID:     1,
-				t,
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}},
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}}, nil
-				},
-				false,
-			),
 			expectedCode: http.StatusOK,
 		},
 		{
-			desc:    "should be ok to delete serviceaccount token with scope all permissions",
+			desc:         "should not be able to delete service account token with wrong permission",
-			keyName: "Test2",
+			saID:         2,
-			acmock: tests.SetupMockAccesscontrol(
+			apikeyID:     1,
-				t,
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}},
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: serviceaccounts.ScopeAll}}, nil
-				},
-				false,
-			),
-			expectedCode: http.StatusOK,
-		},
-		{
-			desc:    "should be forbidden to delete serviceaccount token if wrong scoped",
-			keyName: "Test3",
-			acmock: tests.SetupMockAccesscontrol(
-				t,
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:10"}}, nil
-				},
-				false,
-			),
 			expectedCode: http.StatusForbidden,
 		},
+		{
+			desc:         "should not be able to delete service account token when service account don't exist",
+			saID:         1,
+			apikeyID:     1,
+			permissions:  []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}},
+			expectedErr:  serviceaccounts.ErrServiceAccountNotFound,
+			expectedCode: http.StatusNotFound,
+		},
 	}
 
-	var requestResponse = func(server *web.Mux, httpMethod, requestpath string, requestBody io.Reader) *httptest.ResponseRecorder {
+	for _, tt := range tests {
-		req, err := http.NewRequest(httpMethod, requestpath, requestBody)
+		t.Run(tt.desc, func(t *testing.T) {
-		require.NoError(t, err)
+			server := setupTests(t, func(a *ServiceAccountsAPI) {
-		req.Header.Add("Content-Type", "application/json")
+				a.service = &fakeService{ExpectedErr: tt.expectedErr}
-		recorder := httptest.NewRecorder()
+			})
-		server.ServeHTTP(recorder, req)
-		return recorder
-	}
 
-	for _, tc := range testCases {
+			req := server.NewRequest(http.MethodDelete, fmt.Sprintf("/api/serviceaccounts/%d/tokens/%d", tt.saID, tt.apikeyID), nil)
-		t.Run(tc.desc, func(t *testing.T) {
+			webtest.RequestWithSignedInUser(req, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}})
-			token := createTokenforSA(t, &services.SAService, tc.keyName, sa.OrgID, sa.ID, 1)
+			res, err := server.SendJSON(req)
+			require.NoError(t, err)
 
-			endpoint := fmt.Sprintf(serviceaccountIDTokensDetailPath, sa.ID, token.Id)
+			assert.Equal(t, tt.expectedCode, res.StatusCode)
-			bodyString := ""
+			require.NoError(t, res.Body.Close())
-			server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store)
-			actual := requestResponse(server, http.MethodDelete, endpoint, strings.NewReader(bodyString))
-
-			actualCode := actual.Code
-			actualBody := map[string]interface{}{}
-
-			_ = json.Unmarshal(actual.Body.Bytes(), &actualBody)
-			require.Equal(t, tc.expectedCode, actualCode, endpoint, actualBody)
-
-			query := apikey.GetByNameQuery{KeyName: tc.keyName, OrgId: sa.OrgID}
-			err := services.APIKeyService.GetApiKeyByName(context.Background(), &query)
-			if actualCode == http.StatusOK {
-				require.Error(t, err)
-			} else {
-				require.NoError(t, err)
-			}
-		})
-	}
-}
-
-func TestServiceAccountsAPI_ListTokens(t *testing.T) {
-	store := db.InitTestDB(t)
-	services := setupTestServices(t, store)
-
-	sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true})
-	type testCreateSAToken struct {
-		desc                      string
-		tokens                    []apikey.APIKey
-		expectedHasExpired        bool
-		expectedResponseBodyField string
-		expectedCode              int
-		acmock                    *accesscontrolmock.Mock
-	}
-
-	var saId int64 = 1
-	var timeInFuture = time.Now().Add(time.Second * 100).Unix()
-	var timeInPast = time.Now().Add(-time.Second * 100).Unix()
-
-	testCases := []testCreateSAToken{
-		{
-			desc: "should be able to list serviceaccount with no expiration date",
-			tokens: []apikey.APIKey{{
-				Id:               1,
-				OrgId:            1,
-				ServiceAccountId: &saId,
-				Expires:          nil,
-				Name:             "Test1",
-			}},
-			acmock: tests.SetupMockAccesscontrol(
-				t,
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionRead, Scope: "serviceaccounts:id:1"}}, nil
-				},
-				false,
-			),
-			expectedHasExpired:        false,
-			expectedResponseBodyField: "hasExpired",
-			expectedCode:              http.StatusOK,
-		},
-		{
-			desc: "should be able to list serviceaccount with secondsUntilExpiration",
-			tokens: []apikey.APIKey{{
-				Id:               1,
-				OrgId:            1,
-				ServiceAccountId: &saId,
-				Expires:          &timeInFuture,
-				Name:             "Test2",
-			}},
-			acmock: tests.SetupMockAccesscontrol(
-				t,
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionRead, Scope: "serviceaccounts:id:1"}}, nil
-				},
-				false,
-			),
-			expectedHasExpired:        false,
-			expectedResponseBodyField: "secondsUntilExpiration",
-			expectedCode:              http.StatusOK,
-		},
-		{
-			desc: "should be able to list serviceaccount with expired token",
-			tokens: []apikey.APIKey{{
-				Id:               1,
-				OrgId:            1,
-				ServiceAccountId: &saId,
-				Expires:          &timeInPast,
-				Name:             "Test3",
-			}},
-			acmock: tests.SetupMockAccesscontrol(
-				t,
-				func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
-					return []accesscontrol.Permission{{Action: serviceaccounts.ActionRead, Scope: "serviceaccounts:id:1"}}, nil
-				},
-				false,
-			),
-			expectedHasExpired:        true,
-			expectedResponseBodyField: "secondsUntilExpiration",
-			expectedCode:              http.StatusOK,
-		},
-	}
-
-	var requestResponse = func(server *web.Mux, httpMethod, requestpath string, requestBody io.Reader) *httptest.ResponseRecorder {
-		req, err := http.NewRequest(httpMethod, requestpath, requestBody)
-		require.NoError(t, err)
-		req.Header.Add("Content-Type", "application/json")
-		recorder := httptest.NewRecorder()
-		server.ServeHTTP(recorder, req)
-		return recorder
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.desc, func(t *testing.T) {
-			endpoint := fmt.Sprintf(serviceAccountIDPath+"/tokens", sa.ID)
-			services.SAService.ExpectedTokens = tc.tokens
-			server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store)
-			actual := requestResponse(server, http.MethodGet, endpoint, http.NoBody)
-
-			actualCode := actual.Code
-			actualBody := []map[string]interface{}{}
-
-			_ = json.Unmarshal(actual.Body.Bytes(), &actualBody)
-			require.Equal(t, tc.expectedCode, actualCode, endpoint, actualBody)
-
-			require.Equal(t, tc.expectedCode, actualCode)
-			require.Equal(t, tc.expectedHasExpired, actualBody[0]["hasExpired"])
-			_, exists := actualBody[0][tc.expectedResponseBodyField]
-			require.Equal(t, exists, true)
 		})
 	}
 }

pkg/services/serviceaccounts/tests/common.go

@@ -15,7 +15,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/org/orgimpl"
 	"github.com/grafana/grafana/pkg/services/quota/quotaimpl"
 	"github.com/grafana/grafana/pkg/services/quota/quotatest"
-	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/services/user/userimpl"
@@ -101,63 +100,6 @@ func SetupApiKey(t *testing.T, sqlStore *sqlstore.SQLStore, testKey TestApiKey)
 	return addKeyCmd.Result
 }
 
-// Service implements the API exposed methods for service accounts.
-type serviceAccountStore interface {
-	CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error)
-	RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error)
-	UpdateServiceAccount(ctx context.Context, orgID, serviceAccountID int64,
-		saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error)
-	SearchOrgServiceAccounts(ctx context.Context, query *serviceaccounts.SearchOrgServiceAccountsQuery) (*serviceaccounts.SearchOrgServiceAccountsResult, error)
-	ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error)
-	DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error
-	GetAPIKeysMigrationStatus(ctx context.Context, orgID int64) (*serviceaccounts.APIKeysMigrationStatus, error)
-	HideApiKeysTab(ctx context.Context, orgID int64) error
-	MigrateApiKeysToServiceAccounts(ctx context.Context, orgID int64) error
-	MigrateApiKey(ctx context.Context, orgID int64, keyId int64) error
-	RevertApiKey(ctx context.Context, saId int64, keyId int64) error
-	// Service account tokens
-	AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error
-	DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID, tokenID int64) error
-}
-
-// create mock for serviceaccountservice
-type ServiceAccountMock struct {
-	Store             serviceAccountStore
-	Calls             Calls
-	Stats             *serviceaccounts.Stats
-	SecretScanEnabled bool
-	ExpectedTokens    []apikey.APIKey
-	ExpectedError     error
-}
-
-func (s *ServiceAccountMock) CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) {
-	s.Calls.CreateServiceAccount = append(s.Calls.CreateServiceAccount, []interface{}{ctx, orgID, saForm})
-	return s.Store.CreateServiceAccount(ctx, orgID, saForm)
-}
-func (s *ServiceAccountMock) DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error {
-	s.Calls.DeleteServiceAccount = append(s.Calls.DeleteServiceAccount, []interface{}{ctx, orgID, serviceAccountID})
-	return s.Store.DeleteServiceAccount(ctx, orgID, serviceAccountID)
-}
-
-func (s *ServiceAccountMock) RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error) {
-	s.Calls.RetrieveServiceAccount = append(s.Calls.RetrieveServiceAccount, []interface{}{ctx, orgID, serviceAccountID})
-	return s.Store.RetrieveServiceAccount(ctx, orgID, serviceAccountID)
-}
-
-func (s *ServiceAccountMock) UpdateServiceAccount(ctx context.Context,
-	orgID, serviceAccountID int64,
-	saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) {
-	s.Calls.UpdateServiceAccount = append(s.Calls.UpdateServiceAccount, []interface{}{ctx, orgID, serviceAccountID, saForm})
-	return s.Store.UpdateServiceAccount(ctx, orgID, serviceAccountID, saForm)
-}
-
-func (s *ServiceAccountMock) RetrieveServiceAccountIdByName(ctx context.Context, orgID int64, name string) (int64, error) {
-	return 0, nil
-}
-func (s *ServiceAccountMock) Migrated(ctx context.Context, orgID int64) bool {
-	return false
-}
-
 func SetupMockAccesscontrol(t *testing.T,
 	userpermissionsfunc func(c context.Context, siu *user.SignedInUser, opt accesscontrol.Options) ([]accesscontrol.Permission, error),
 	disableAccessControl bool) *accesscontrolmock.Mock {
@@ -169,75 +111,3 @@ func SetupMockAccesscontrol(t *testing.T,
 	acmock.GetUserPermissionsFunc = userpermissionsfunc
 	return acmock
 }
-
-var _ serviceaccounts.Service = new(ServiceAccountMock)
-
-type Calls struct {
-	CreateServiceAccount            []interface{}
-	RetrieveServiceAccount          []interface{}
-	DeleteServiceAccount            []interface{}
-	GetAPIKeysMigrationStatus       []interface{}
-	HideApiKeysTab                  []interface{}
-	MigrateApiKeysToServiceAccounts []interface{}
-	MigrateApiKey                   []interface{}
-	RevertApiKey                    []interface{}
-	ListTokens                      []interface{}
-	DeleteServiceAccountToken       []interface{}
-	UpdateServiceAccount            []interface{}
-	AddServiceAccountToken          []interface{}
-	SearchOrgServiceAccounts        []interface{}
-	RetrieveServiceAccountIdByName  []interface{}
-}
-
-func (s *ServiceAccountMock) HideApiKeysTab(ctx context.Context, orgID int64) error {
-	s.Calls.HideApiKeysTab = append(s.Calls.HideApiKeysTab, []interface{}{ctx})
-	return nil
-}
-
-func (s *ServiceAccountMock) GetAPIKeysMigrationStatus(ctx context.Context, orgID int64) (*serviceaccounts.APIKeysMigrationStatus, error) {
-	s.Calls.GetAPIKeysMigrationStatus = append(s.Calls.GetAPIKeysMigrationStatus, []interface{}{ctx})
-	return nil, nil
-}
-
-func (s *ServiceAccountMock) MigrateApiKeysToServiceAccounts(ctx context.Context, orgID int64) error {
-	s.Calls.MigrateApiKeysToServiceAccounts = append(s.Calls.MigrateApiKeysToServiceAccounts, []interface{}{ctx})
-	return nil
-}
-
-func (s *ServiceAccountMock) MigrateApiKey(ctx context.Context, orgID int64, keyId int64) error {
-	s.Calls.MigrateApiKey = append(s.Calls.MigrateApiKey, []interface{}{ctx})
-	return nil
-}
-
-func (s *ServiceAccountMock) RevertApiKey(ctx context.Context, saId int64, keyId int64) error {
-	s.Calls.RevertApiKey = append(s.Calls.RevertApiKey, []interface{}{ctx})
-	return nil
-}
-
-func (s *ServiceAccountMock) ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error) {
-	s.Calls.ListTokens = append(s.Calls.ListTokens, []interface{}{ctx, query.OrgID, query.ServiceAccountID})
-	return s.ExpectedTokens, s.ExpectedError
-}
-
-func (s *ServiceAccountMock) SearchOrgServiceAccounts(ctx context.Context, query *serviceaccounts.SearchOrgServiceAccountsQuery) (*serviceaccounts.SearchOrgServiceAccountsResult, error) {
-	s.Calls.SearchOrgServiceAccounts = append(s.Calls.SearchOrgServiceAccounts, []interface{}{ctx, query})
-	return nil, nil
-}
-
-func (s *ServiceAccountMock) AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error {
-	s.Calls.AddServiceAccountToken = append(s.Calls.AddServiceAccountToken, []interface{}{ctx, cmd})
-	return s.Store.AddServiceAccountToken(ctx, serviceAccountID, cmd)
-}
-
-func (s *ServiceAccountMock) DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID, tokenID int64) error {
-	s.Calls.DeleteServiceAccountToken = append(s.Calls.DeleteServiceAccountToken, []interface{}{ctx, orgID, serviceAccountID, tokenID})
-	return s.Store.DeleteServiceAccountToken(ctx, orgID, serviceAccountID, tokenID)
-}
-
-func (s *ServiceAccountMock) GetUsageMetrics(ctx context.Context) (*serviceaccounts.Stats, error) {
-	if s.Stats == nil {
-		return &serviceaccounts.Stats{}, nil
-	}
-
-	return s.Stats, nil
-}