diff --git a/pkg/services/accesscontrol/actest/fake.go b/pkg/services/accesscontrol/actest/fake.go index 50235d6ff1c..c394907ff07 100644 --- a/pkg/services/accesscontrol/actest/fake.go +++ b/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 +} diff --git a/pkg/services/serviceaccounts/api/api_test.go b/pkg/services/serviceaccounts/api/api_test.go index 2595c81594f..cdaad1e2696 100644 --- a/pkg/services/serviceaccounts/api/api_test.go +++ b/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" - "strconv" + "strings" "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) - 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 { + type TestCase struct { desc string - body map[string]interface{} + basicRole org.RoleType + permissions []accesscontrol.Permission + body string expectedCode int - wantID string - wantError string - acmock *accesscontrolmock.Mock + expectedSA *serviceaccounts.ServiceAccountDTO + expectedErr error } - testCases := []testCreateSATestCase{ + + tests := []TestCase{ { - desc: "should be ok to create service account with permissions", - body: map[string]interface{}{"name": "New SA", "role": "Viewer", "is_disabled": "false"}, - wantID: "sa-new-sa", - acmock: tests.SetupMockAccesscontrol( - t, - func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) { - return []accesscontrol.Permission{{Action: serviceaccounts.ActionCreate}}, nil - }, - false, - ), + desc: "should be able to create service account with correct permission", + basicRole: org.RoleViewer, + permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionCreate}}, + body: `{"name": "test", "isDisabled": false, "role": "Viewer"}`, + expectedSA: &serviceaccounts.ServiceAccountDTO{ + Name: "test", + OrgId: 1, + IsDisabled: false, + Role: string(org.RoleViewer), + }, expectedCode: http.StatusCreated, }, { - desc: "should fail to create a service account with higher privilege", - body: map[string]interface{}{"name": "New SA HP", "role": "Admin"}, - wantID: "sa-new-sa-hp", - acmock: tests.SetupMockAccesscontrol( - t, - func(c context.Context, siu *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) { - return []accesscontrol.Permission{{Action: serviceaccounts.ActionCreate}}, nil - }, - false, - ), + desc: "should not be able to create service account without permission", + basicRole: org.RoleViewer, + permissions: []accesscontrol.Permission{{}}, + body: `{"name": "test", "isDisabled": false, "role": "Viewer"}`, expectedCode: http.StatusForbidden, }, { - desc: "should fail to create a service account with invalid role", - body: map[string]interface{}{"name": "New SA", "role": "Random"}, - wantID: "sa-new-sa", - wantError: "invalid role value: Random", - 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 not be able to create service account with role that has higher privilege than caller", + basicRole: org.RoleViewer, + permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionCreate}}, + body: `{"name": "test", "isDisabled": false, "role": "Editor"}`, + expectedCode: http.StatusForbidden, }, { - 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, - ), + 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: "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, - ), + 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, }, - { - 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, - }, } - var requestResponse = func(server *web.Mux, httpMethod, requestpath string, body io.Reader) *httptest.ResponseRecorder { - req, err := http.NewRequest(httpMethod, requestpath, body) - req.Header.Add("Content-Type", "application/json") - require.NoError(t, err) - 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)) - } + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + server := setupTests(t, func(a *ServiceAccountsAPI) { + a.service = &fakeService{ExpectedServiceAccount: tt.expectedSA, ExpectedErr: tt.expectedErr} }) + 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) - services := setupTestServices(t, store) - - var requestResponse = func(server *web.Mux, 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 + type TestCase struct { + desc string + id int64 + permissions []accesscontrol.Permission + expectedCode int } - 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 *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) { - return []accesscontrol.Permission{{Action: serviceaccounts.ActionDelete, Scope: serviceaccounts.ScopeAll}}, nil - }, - false, - ), + tests := []TestCase{ + { + desc: "should be able to delete service account with correct permission", + id: 1, + permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionDelete, Scope: "serviceaccounts:id:1"}}, 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) - server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), testcase.acmock, store) - 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 *user.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) { - return []accesscontrol.Permission{}, nil - }, - false, - ), + }, + { + desc: "should not ba able to delete with wrong permission", + id: 2, + permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionDelete, Scope: "serviceaccounts:id:1"}}, 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 + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + server := setupTests(t) + req := server.NewRequest(http.MethodDelete, fmt.Sprintf("/api/serviceaccounts/%d", tt.id), nil) + 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) - m := web.New() - signedUser := &user.SignedInUser{ - OrgID: 1, - UserID: 1, - OrgRole: org.RoleViewer, + assert.Equal(t, tt.expectedCode, res.StatusCode) + require.NoError(t, res.Body.Close()) + }) } - - m.Use(func(c *web.Context) { - ctx := &models.ReqContext{ - Context: c, - IsSignedIn: true, - SignedInUser: signedUser, - Logger: log.New("serviceaccounts-test"), - } - c.Req = c.Req.WithContext(ctxkey.Set(c.Req.Context(), ctx)) - }) - a.RouterRegister.Register(m.Router) - return m, a } func TestServiceAccountsAPI_RetrieveServiceAccount(t *testing.T) { - store := db.InitTestDB(t) - services := setupTestServices(t, store) - - type testRetrieveSATestCase struct { + type TestCase struct { desc string - user *tests.TestUser + id int64 + permissions []accesscontrol.Permission expectedCode int - acmock *accesscontrolmock.Mock - Id int + expectedSA *serviceaccounts.ServiceAccountProfileDTO } - testCases := []testRetrieveSATestCase{ + + tests := []TestCase{ { - desc: "should be ok to retrieve serviceaccount with permissions", - user: &tests.TestUser{Login: "servicetest1@admin", IsServiceAccount: true}, - 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, - ), + desc: "should be able to get service account with correct permission", + id: 1, + permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionRead, Scope: "serviceaccounts:id:1"}}, + expectedSA: &serviceaccounts.ServiceAccountProfileDTO{}, expectedCode: http.StatusOK, }, { - desc: "should be forbidden to retrieve serviceaccount if no permissions", - 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, - ), + desc: "should not ba able to get service account with wrong permission", + id: 2, + permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionRead, Scope: "serviceaccounts:id:1"}}, 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 { - req, err := http.NewRequest(httpMethod, requestpath, nil) - require.NoError(t, err) - recorder := httptest.NewRecorder() - 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)) - } + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + server := setupTests(t, func(a *ServiceAccountsAPI) { + a.service = &fakeService{ExpectedServiceAccountProfile: tt.expectedSA} }) + 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) - services := setupTestServices(t, store) - - type testUpdateSATestCase struct { + type TestCase 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 - editorRole := org.RoleEditor - var invalidRole org.RoleType = "InvalidRole" - testCases := []testUpdateSATestCase{ + tests := []TestCase{ { - desc: "should be ok to update serviceaccount with permissions", - user: &tests.TestUser{Login: "servicetest1@admin", IsServiceAccount: true, Role: "Viewer", Name: "Unaltered"}, - body: &serviceaccounts.UpdateServiceAccountForm{Name: newString("New Name"), Role: &viewerRole}, - 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, - ), + desc: "should be able to update service account with correct permission", + id: 1, + body: `{"role": "Editor"}`, + basicRole: org.RoleAdmin, + permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}}, + expectedSA: &serviceaccounts.ServiceAccountProfileDTO{}, expectedCode: http.StatusOK, }, { - desc: "should be forbidden to set role higher than user's role", - user: &tests.TestUser{Login: "servicetest2@admin", IsServiceAccount: true, Role: "Viewer", Name: "Unaltered 2"}, - body: &serviceaccounts.UpdateServiceAccountForm{Name: newString("New Name 2"), Role: &editorRole}, - 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, - ), + desc: "should not be able to update service account with wrong permission", + id: 2, + body: `{}`, + basicRole: org.RoleAdmin, + permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}}, expectedCode: http.StatusForbidden, }, { - desc: "bad request when invalid role", - user: &tests.TestUser{Login: "servicetest3@admin", IsServiceAccount: true, Role: "Invalid", Name: "Unaltered"}, - body: &serviceaccounts.UpdateServiceAccountForm{Name: newString("NameB"), Role: &invalidRole}, - 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.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, - ), + desc: "should not be able to update service account with a role that has higher privilege then caller", + id: 1, + body: `{"role": "Admin"}`, + basicRole: org.RoleEditor, + permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}}, 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, + 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, }, } - var requestResponse = func(server *web.Mux, httpMethod, requestpath string, body io.Reader) *httptest.ResponseRecorder { - req, err := http.NewRequest(httpMethod, requestpath, body) - req.Header.Add("Content-Type", "application/json") - require.NoError(t, err) - recorder := httptest.NewRecorder() - server.ServeHTTP(recorder, req) - return recorder + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + server := setupTests(t, func(a *ServiceAccountsAPI) { + a.service = &fakeService{ExpectedServiceAccountProfile: tt.expectedSA} + }) + + req := server.NewRequest(http.MethodPatch, fmt.Sprintf("/api/serviceaccounts/%d", tt.id), 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()) + }) } +} - for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - server, saAPI := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store) - scopeID := tc.Id - if tc.user != nil { - createdUser := tests.SetupUserServiceAccount(t, store, *tc.user) - scopeID = int(createdUser.ID) - } +func setupTests(t *testing.T, opts ...func(a *ServiceAccountsAPI)) *webtest.Server { + t.Helper() + cfg := setting.NewCfg() + api := &ServiceAccountsAPI{ + cfg: cfg, + service: &fakeService{}, + accesscontrolService: &actest.FakeService{}, + accesscontrol: acimpl.ProvideAccessControl(cfg), + RouterRegister: routing.NewRouteRegister(), + log: log.NewNopLogger(), + permissionService: &actest.FakePermissionsService{}, + } - var rawBody io.Reader = http.NoBody - if tc.body != nil { - body, err := json.Marshal(tc.body) - require.NoError(t, err) - rawBody = bytes.NewReader(body) - } + for _, o := range opts { + o(api) + } + api.RegisterAPIEndpoints() + return webtest.NewServer(t, api.RouterRegister) +} - actual := requestResponse(server, http.MethodPatch, fmt.Sprintf(serviceAccountIDPath, scopeID), rawBody) +var _ service = new(fakeService) - actualCode := actual.Code - require.Equal(t, tc.expectedCode, actualCode) +type fakeService struct { + service + ExpectedErr error + ExpectedApiKey *apikey.APIKey + ExpectedServiceAccountTokens []apikey.APIKey + ExpectedServiceAccount *serviceaccounts.ServiceAccountDTO + ExpectedServiceAccountProfile *serviceaccounts.ServiceAccountProfileDTO +} - 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)) +func (f *fakeService) CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) { + return f.ExpectedServiceAccount, f.ExpectedErr +} - // 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) - } - }) - } +func (f *fakeService) DeleteServiceAccount(ctx context.Context, orgID, id int64) error { + return f.ExpectedErr } -type services struct { - OrgService org.Service - UserService user.Service - SAService tests.ServiceAccountMock - APIKeyService apikey.Service +func (f *fakeService) RetrieveServiceAccount(ctx context.Context, orgID, id int64) (*serviceaccounts.ServiceAccountProfileDTO, error) { + return f.ExpectedServiceAccountProfile, f.ExpectedErr } -func setupTestServices(t *testing.T, db *sqlstore.SQLStore) services { - kvStore := kvstore.ProvideService(db) - quotaService := quotatest.New(false, nil) - apiKeyService, err := apikeyimpl.ProvideService(db, db.Cfg, quotaService) - require.NoError(t, err) +func (f *fakeService) ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error) { + return f.ExpectedServiceAccountTokens, f.ExpectedErr +} - 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) +func (f *fakeService) UpdateServiceAccount(ctx context.Context, orgID, id int64, cmd *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) { + return f.ExpectedServiceAccountProfile, f.ExpectedErr +} - saStore := database.ProvideServiceAccountsStore(nil, db, apiKeyService, kvStore, userSvc, orgService) - svcmock := tests.ServiceAccountMock{Store: saStore, Calls: tests.Calls{}, Stats: nil, SecretScanEnabled: false} +func (f *fakeService) AddServiceAccountToken(ctx context.Context, id int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error { + cmd.Result = f.ExpectedApiKey + return f.ExpectedErr +} - return services{ - OrgService: orgService, - UserService: userSvc, - SAService: svcmock, - APIKeyService: apiKeyService, - } +func (f *fakeService) DeleteServiceAccountToken(ctx context.Context, orgID, id, tokenID int64) error { + return f.ExpectedErr } diff --git a/pkg/services/serviceaccounts/api/token_test.go b/pkg/services/serviceaccounts/api/token_test.go index 9e63c738856..0f91a88a1a0 100644 --- a/pkg/services/serviceaccounts/api/token_test.go +++ b/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 ( - serviceaccountIDTokensPath = "/api/serviceaccounts/%v/tokens" // #nosec G101 - 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 { +func TestServiceAccountsAPI_ListTokens(t *testing.T) { + type TestCase struct { desc string + id int64 + permissions []accesscontrol.Permission expectedCode int - body map[string]interface{} - acmock *accesscontrolmock.Mock } - testCases := []testCreateSAToken{ - { - desc: "should be ok to create serviceaccount token with scope all 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.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", - 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, - ), - body: map[string]interface{}{"name": "Test2", "role": "Viewer", "secondsToLive": 1, "orgId": 4, "serviceAccountId": 4}, - expectedCode: http.StatusOK, - }, + tests := []TestCase{ { - 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}, + desc: "should be able to list tokens with correct permission", + id: 1, + permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionRead, Scope: "serviceaccounts:id: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"}, + desc: "should not be able to list tokens with wrong permission", + id: 2, + permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionRead, Scope: "serviceaccounts:id:1"}}, expectedCode: http.StatusForbidden, }, } - 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(serviceaccountIDTokensPath, sa.ID) - bodyString := "" - if tc.body != nil { - b, err := json.Marshal(tc.body) - require.NoError(t, err) - bodyString = string(b) - } - - server, _ := setupTestServer(t, &services.SAService, routing.NewRouteRegister(), tc.acmock, store) - actual := requestResponse(server, http.MethodPost, endpoint, strings.NewReader(bodyString)) - - actualCode := actual.Code - actualBody := map[string]interface{}{} - - err := json.Unmarshal(actual.Body.Bytes(), &actualBody) + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + server := setupTests(t, func(a *ServiceAccountsAPI) { + a.service = &fakeService{} + }) + req := server.NewGetRequest(fmt.Sprintf("/api/serviceaccounts/%d/tokens", 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) - require.Equal(t, tc.expectedCode, actualCode, endpoint, actualBody) - if actualCode == http.StatusOK { - assert.Equal(t, tc.body["name"], actualBody["name"]) - - 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) - } + assert.Equal(t, tt.expectedCode, res.StatusCode) + require.NoError(t, res.Body.Close()) }) } } -func TestServiceAccountsAPI_DeleteToken(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 - keyName string - expectedCode int - acmock *accesscontrolmock.Mock +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 } - testCases := []testCreateSAToken{ + tests := []TestCase{ { - desc: "should be ok to delete serviceaccount token with scope id permissions", - keyName: "Test1", - 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, - ), - expectedCode: http.StatusOK, + desc: "should be able to create token for service account with correct permission", + id: 1, + body: `{"name": "test"}`, + tokenTTL: -1, + permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}}, + expectedApiKey: &apikey.APIKey{}, + expectedCode: http.StatusOK, }, { - desc: "should be ok to delete serviceaccount token with scope all permissions", - keyName: "Test2", - 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.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 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 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, }, } - 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) { - token := createTokenforSA(t, &services.SAService, tc.keyName, sa.OrgID, sa.ID, 1) - - endpoint := fmt.Sprintf(serviceaccountIDTokensDetailPath, sa.ID, token.Id) - bodyString := "" - 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) + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + server := setupTests(t, func(a *ServiceAccountsAPI) { + a.cfg.ApiKeyMaxSecondsToLive = tt.tokenTTL + a.service = &fakeService{ + ExpectedErr: tt.expectedErr, + 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) - 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) - } + assert.Equal(t, tt.expectedCode, res.StatusCode) + require.NoError(t, res.Body.Close()) }) } } -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 +func TestServiceAccountsAPI_DeleteToken(t *testing.T) { + type TestCase struct { + desc string + saID int64 + apikeyID int64 + permissions []accesscontrol.Permission + expectedErr error + expectedCode int } - var saId int64 = 1 - var timeInFuture = time.Now().Add(time.Second * 100).Unix() - var timeInPast = time.Now().Add(-time.Second * 100).Unix() - - testCases := []testCreateSAToken{ + tests := []TestCase{ { - 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 delete service account token with correct permission", + saID: 1, + apikeyID: 1, + permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}}, + 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 not be able to delete service account token with wrong permission", + saID: 2, + apikeyID: 1, + permissions: []accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:1"}}, + expectedCode: http.StatusForbidden, }, { - 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, + 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 { - 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{}{} + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + server := setupTests(t, func(a *ServiceAccountsAPI) { + a.service = &fakeService{ExpectedErr: tt.expectedErr} + }) - _ = json.Unmarshal(actual.Body.Bytes(), &actualBody) - require.Equal(t, tc.expectedCode, actualCode, endpoint, actualBody) + req := server.NewRequest(http.MethodDelete, fmt.Sprintf("/api/serviceaccounts/%d/tokens/%d", tt.saID, tt.apikeyID), nil) + 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) - require.Equal(t, tc.expectedHasExpired, actualBody[0]["hasExpired"]) - _, exists := actualBody[0][tc.expectedResponseBodyField] - require.Equal(t, exists, true) + assert.Equal(t, tt.expectedCode, res.StatusCode) + require.NoError(t, res.Body.Close()) }) } } diff --git a/pkg/services/serviceaccounts/tests/common.go b/pkg/services/serviceaccounts/tests/common.go index 1b5a16dd171..c02f8c91076 100644 --- a/pkg/services/serviceaccounts/tests/common.go +++ b/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 -}