diff --git a/pkg/api/api.go b/pkg/api/api.go index 45f2a4fede8..7e4835ddab2 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -258,7 +258,6 @@ func (hs *HTTPServer) registerRoutes() { apiRoute.Group("/auth/keys", func(keysRoute routing.RouteRegister) { keysRoute.Get("/", routing.Wrap(hs.GetAPIKeys)) keysRoute.Post("/", quota("api_key"), routing.Wrap(hs.AddAPIKey)) - keysRoute.Post("/additional", quota("api_key"), routing.Wrap(hs.AdditionalAPIKey)) keysRoute.Delete("/:id", routing.Wrap(hs.DeleteAPIKey)) }, reqOrgAdmin) diff --git a/pkg/api/apikey.go b/pkg/api/apikey.go index a4fca6abebb..4f32a349d26 100644 --- a/pkg/api/apikey.go +++ b/pkg/api/apikey.go @@ -10,7 +10,6 @@ import ( "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/components/apikeygen" "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/web" ) @@ -80,12 +79,9 @@ func (hs *HTTPServer) AddAPIKey(c *models.ReqContext) response.Response { return response.Error(400, "Number of seconds before expiration is greater than the global limit", nil) } } + + cmd.ServiceAccountId = nil // Security: API keys can't be added to SAs through this endpoint since we do not implement access checks here cmd.OrgId = c.OrgId - var err error - if hs.Features.IsEnabled(featuremgmt.FlagServiceAccounts) { - // Api keys should now be created with addadditionalapikey endpoint - return response.Error(400, "API keys should now be added via the AdditionalAPIKey endpoint.", err) - } newKeyInfo, err := apikeygen.New(cmd.OrgId, cmd.Name) if err != nil { @@ -111,16 +107,3 @@ func (hs *HTTPServer) AddAPIKey(c *models.ReqContext) response.Response { return response.JSON(200, result) } - -// AddAPIKey adds an additional API key to a service account -func (hs *HTTPServer) AdditionalAPIKey(c *models.ReqContext) response.Response { - cmd := models.AddApiKeyCommand{} - if err := web.Bind(c.Req, &cmd); err != nil { - return response.Error(http.StatusBadRequest, "bad request data", err) - } - if !hs.Features.IsEnabled(featuremgmt.FlagServiceAccounts) { - return response.Error(500, "Requires services-accounts feature", errors.New("feature missing")) - } - - return hs.AddAPIKey(c) -} diff --git a/pkg/models/apikey.go b/pkg/models/apikey.go index 9a19cc3ddd5..29f0f14b2bd 100644 --- a/pkg/models/apikey.go +++ b/pkg/models/apikey.go @@ -21,7 +21,7 @@ type ApiKey struct { Created time.Time Updated time.Time Expires *int64 - ServiceAccountId int64 + ServiceAccountId *int64 } // --------------------- @@ -32,7 +32,7 @@ type AddApiKeyCommand struct { OrgId int64 `json:"-"` Key string `json:"-"` SecondsToLive int64 `json:"secondsToLive"` - ServiceAccountId int64 `json:"-"` + ServiceAccountId *int64 `json:"-"` Result *ApiKey `json:"-"` } diff --git a/pkg/services/contexthandler/contexthandler.go b/pkg/services/contexthandler/contexthandler.go index 71757ab77d8..d3f21c95a1a 100644 --- a/pkg/services/contexthandler/contexthandler.go +++ b/pkg/services/contexthandler/contexthandler.go @@ -235,7 +235,7 @@ func (h *ContextHandler) initContextWithAPIKey(reqContext *models.ReqContext) bo return true } - if apikey.ServiceAccountId < 1 { //There is no service account attached to the apikey + if apikey.ServiceAccountId == nil || *apikey.ServiceAccountId < 1 { //There is no service account attached to the apikey //Use the old APIkey method. This provides backwards compatibility. reqContext.SignedInUser = &models.SignedInUser{} reqContext.OrgRole = apikey.Role @@ -248,7 +248,7 @@ func (h *ContextHandler) initContextWithAPIKey(reqContext *models.ReqContext) bo //There is a service account attached to the API key //Use service account linked to API key as the signed in user - query := models.GetSignedInUserQuery{UserId: apikey.ServiceAccountId, OrgId: apikey.OrgId} + query := models.GetSignedInUserQuery{UserId: *apikey.ServiceAccountId, OrgId: apikey.OrgId} if err := bus.Dispatch(reqContext.Req.Context(), &query); err != nil { reqContext.Logger.Error( "Failed to link API key to service account in", diff --git a/pkg/services/serviceaccounts/api/api.go b/pkg/services/serviceaccounts/api/api.go index 06b190c1f12..aeeaa69acac 100644 --- a/pkg/services/serviceaccounts/api/api.go +++ b/pkg/services/serviceaccounts/api/api.go @@ -1,12 +1,11 @@ package api import ( + "context" "errors" "net/http" "strconv" - "time" - "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/middleware" @@ -15,27 +14,40 @@ import ( acmiddleware "github.com/grafana/grafana/pkg/services/accesscontrol/middleware" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/serviceaccounts" + "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web" ) +type APIKeyStore interface { + AddAPIKey(ctx context.Context, cmd *models.AddApiKeyCommand) error + GetApiKeyById(ctx context.Context, query *models.GetApiKeyByIdQuery) error + DeleteApiKey(ctx context.Context, cmd *models.DeleteApiKeyCommand) error +} + type ServiceAccountsAPI struct { + cfg *setting.Cfg service serviceaccounts.Service accesscontrol accesscontrol.AccessControl RouterRegister routing.RouteRegister store serviceaccounts.Store + apiKeyStore APIKeyStore } func NewServiceAccountsAPI( + cfg *setting.Cfg, service serviceaccounts.Service, accesscontrol accesscontrol.AccessControl, routerRegister routing.RouteRegister, store serviceaccounts.Store, + apiKeyStore APIKeyStore, ) *ServiceAccountsAPI { return &ServiceAccountsAPI{ + cfg: cfg, service: service, accesscontrol: accesscontrol, RouterRegister: routerRegister, store: store, + apiKeyStore: apiKeyStore, } } @@ -54,7 +66,12 @@ func (api *ServiceAccountsAPI) RegisterAPIEndpoints( serviceAccountsRoute.Get("/upgrade", auth(middleware.ReqOrgAdmin, accesscontrol.EvalPermission(serviceaccounts.ActionCreate, serviceaccounts.ScopeID)), routing.Wrap(api.UpgradeServiceAccounts)) serviceAccountsRoute.Post("/convert/:keyId", auth(middleware.ReqOrgAdmin, accesscontrol.EvalPermission(serviceaccounts.ActionCreate, serviceaccounts.ScopeID)), routing.Wrap(api.ConvertToServiceAccount)) serviceAccountsRoute.Post("/", auth(middleware.ReqOrgAdmin, accesscontrol.EvalPermission(serviceaccounts.ActionCreate, serviceaccounts.ScopeID)), routing.Wrap(api.CreateServiceAccount)) - serviceAccountsRoute.Get("/:serviceAccountId/tokens", auth(middleware.ReqOrgAdmin, accesscontrol.EvalPermission(serviceaccounts.ActionRead, serviceaccounts.ScopeID)), routing.Wrap(api.ListTokens)) + serviceAccountsRoute.Get("/:serviceAccountId/tokens", auth(middleware.ReqOrgAdmin, + accesscontrol.EvalPermission(serviceaccounts.ActionRead, serviceaccounts.ScopeID)), routing.Wrap(api.ListTokens)) + serviceAccountsRoute.Post("/:serviceAccountId/tokens", auth(middleware.ReqOrgAdmin, + accesscontrol.EvalPermission(serviceaccounts.ActionWrite, serviceaccounts.ScopeID)), routing.Wrap(api.CreateToken)) + serviceAccountsRoute.Delete("/:serviceAccountId/tokens/:tokenId", auth(middleware.ReqOrgAdmin, + accesscontrol.EvalPermission(serviceaccounts.ActionWrite, serviceaccounts.ScopeID)), routing.Wrap(api.DeleteToken)) }) } @@ -110,33 +127,6 @@ func (api *ServiceAccountsAPI) ConvertToServiceAccount(ctx *models.ReqContext) r } } -func (api *ServiceAccountsAPI) ListTokens(ctx *models.ReqContext) response.Response { - saID, err := strconv.ParseInt(web.Params(ctx.Req)[":serviceAccountId"], 10, 64) - if err != nil { - return response.Error(http.StatusBadRequest, "serviceAccountId is invalid", err) - } - if saTokens, err := api.store.ListTokens(ctx.Req.Context(), ctx.OrgId, saID); err == nil { - result := make([]*models.ApiKeyDTO, len(saTokens)) - for i, t := range saTokens { - var expiration *time.Time = nil - if t.Expires != nil { - v := time.Unix(*t.Expires, 0) - expiration = &v - } - result[i] = &models.ApiKeyDTO{ - Id: t.Id, - Name: t.Name, - Role: t.Role, - Expiration: expiration, - } - } - - return response.JSON(200, result) - } else { - return response.Error(500, "Internal server error", err) - } -} - func (api *ServiceAccountsAPI) ListServiceAccounts(ctx *models.ReqContext) response.Response { serviceAccounts, err := api.store.ListServiceAccounts(ctx.Req.Context(), ctx.OrgId, -1) if err != nil { diff --git a/pkg/services/serviceaccounts/api/api_test.go b/pkg/services/serviceaccounts/api/api_test.go index 4749974901a..2338c3dd159 100644 --- a/pkg/services/serviceaccounts/api/api_test.go +++ b/pkg/services/serviceaccounts/api/api_test.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/serviceaccounts/tests" "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web" "github.com/stretchr/testify/require" @@ -96,9 +97,11 @@ func serviceAccountRequestScenario(t *testing.T, httpMethod string, endpoint str } func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock, routerRegister routing.RouteRegister, acmock *accesscontrolmock.Mock, sqlStore *sqlstore.SQLStore) *web.Mux { - a := NewServiceAccountsAPI(svc, acmock, routerRegister, database.NewServiceAccountsStore(sqlStore)) + a := NewServiceAccountsAPI(setting.NewCfg(), svc, acmock, routerRegister, database.NewServiceAccountsStore(sqlStore), sqlStore) a.RegisterAPIEndpoints(featuremgmt.WithFeatures(featuremgmt.FlagServiceAccounts)) + a.cfg.ApiKeyMaxSecondsToLive = -1 // disable api key expiration + m := web.New() signedUser := &models.SignedInUser{ OrgId: 1, diff --git a/pkg/services/serviceaccounts/api/token.go b/pkg/services/serviceaccounts/api/token.go new file mode 100644 index 00000000000..bfeef7b4185 --- /dev/null +++ b/pkg/services/serviceaccounts/api/token.go @@ -0,0 +1,165 @@ +package api + +import ( + "errors" + "net/http" + "strconv" + "time" + + "github.com/grafana/grafana/pkg/api/dtos" + "github.com/grafana/grafana/pkg/api/response" + "github.com/grafana/grafana/pkg/components/apikeygen" + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/serviceaccounts" + "github.com/grafana/grafana/pkg/web" +) + +const failedToDeleteMsg = "Failed to delete API key" + +func (api *ServiceAccountsAPI) ListTokens(ctx *models.ReqContext) response.Response { + saID, err := strconv.ParseInt(web.Params(ctx.Req)[":serviceAccountId"], 10, 64) + if err != nil { + return response.Error(http.StatusBadRequest, "serviceAccountId is invalid", err) + } + + if saTokens, err := api.store.ListTokens(ctx.Req.Context(), ctx.OrgId, saID); err == nil { + result := make([]*models.ApiKeyDTO, len(saTokens)) + for i, t := range saTokens { + var expiration *time.Time = nil + if t.Expires != nil { + v := time.Unix(*t.Expires, 0) + expiration = &v + } + result[i] = &models.ApiKeyDTO{ + Id: t.Id, + Name: t.Name, + Role: t.Role, + Expiration: expiration, + } + } + + return response.JSON(200, result) + } else { + return response.Error(500, "Internal server error", err) + } +} + +// CreateNewToken adds an API key to a service account +func (api *ServiceAccountsAPI) CreateToken(c *models.ReqContext) response.Response { + saID, err := strconv.ParseInt(web.Params(c.Req)[":serviceAccountId"], 10, 64) + if err != nil { + return response.Error(http.StatusBadRequest, "serviceAccountId is invalid", err) + } + + // confirm service account exists + if _, err := api.store.RetrieveServiceAccount(c.Req.Context(), c.OrgId, saID); err != nil { + switch { + case errors.Is(err, serviceaccounts.ErrServiceAccountNotFound): + return response.Error(http.StatusNotFound, "Failed to retrieve service account", err) + default: + return response.Error(http.StatusInternalServerError, "Failed to retrieve service account", err) + } + } + + cmd := models.AddApiKeyCommand{} + if err := web.Bind(c.Req, &cmd); err != nil { + return response.Error(http.StatusBadRequest, "bad request data", err) + } + + // Force affected service account to be the one referenced in the URL + cmd.ServiceAccountId = &saID + cmd.OrgId = c.OrgId + + if !cmd.Role.IsValid() { + return response.Error(400, "Invalid role specified", nil) + } + + if api.cfg.ApiKeyMaxSecondsToLive != -1 { + if cmd.SecondsToLive == 0 { + return response.Error(400, "Number of seconds before expiration should be set", nil) + } + if cmd.SecondsToLive > api.cfg.ApiKeyMaxSecondsToLive { + return response.Error(400, "Number of seconds before expiration is greater than the global limit", nil) + } + } + + newKeyInfo, err := apikeygen.New(cmd.OrgId, cmd.Name) + if err != nil { + return response.Error(500, "Generating API key failed", err) + } + + cmd.Key = newKeyInfo.HashedKey + + if err := api.apiKeyStore.AddAPIKey(c.Req.Context(), &cmd); err != nil { + if errors.Is(err, models.ErrInvalidApiKeyExpiration) { + return response.Error(400, err.Error(), nil) + } + if errors.Is(err, models.ErrDuplicateApiKey) { + return response.Error(409, err.Error(), nil) + } + return response.Error(500, "Failed to add API Key", err) + } + + result := &dtos.NewApiKeyResult{ + ID: cmd.Result.Id, + Name: cmd.Result.Name, + Key: newKeyInfo.ClientSecret, + } + + return response.JSON(200, result) +} + +// DeleteToken deletes service account tokens +func (api *ServiceAccountsAPI) DeleteToken(c *models.ReqContext) response.Response { + saID, err := strconv.ParseInt(web.Params(c.Req)[":serviceAccountId"], 10, 64) + if err != nil { + return response.Error(http.StatusBadRequest, "serviceAccountId is invalid", err) + } + + // confirm service account exists + if _, err := api.store.RetrieveServiceAccount(c.Req.Context(), c.OrgId, saID); err != nil { + switch { + case errors.Is(err, serviceaccounts.ErrServiceAccountNotFound): + return response.Error(http.StatusNotFound, "Failed to retrieve service account", err) + default: + return response.Error(http.StatusInternalServerError, "Failed to retrieve service account", err) + } + } + + tokenID, err := strconv.ParseInt(web.Params(c.Req)[":tokenId"], 10, 64) + if err != nil { + return response.Error(http.StatusBadRequest, "serviceAccountId is invalid", err) + } + + // confirm API key belongs to service account. TODO: refactor get & delete to single call + cmdGet := &models.GetApiKeyByIdQuery{ApiKeyId: tokenID} + if err = api.apiKeyStore.GetApiKeyById(c.Req.Context(), cmdGet); err != nil { + status := 404 + if err != nil && !errors.Is(err, models.ErrApiKeyNotFound) { + status = 500 + } else { + err = models.ErrApiKeyNotFound + } + + return response.Error(status, failedToDeleteMsg, err) + } + + // verify service account ID matches the URL + if *cmdGet.Result.ServiceAccountId != saID { + return response.Error(404, failedToDeleteMsg, err) + } + + cmdDel := &models.DeleteApiKeyCommand{Id: tokenID, OrgId: c.OrgId} + if err = api.apiKeyStore.DeleteApiKey(c.Req.Context(), cmdDel); err != nil { + status := 404 + if err != nil && !errors.Is(err, models.ErrApiKeyNotFound) { + status = 500 + } else { + err = models.ErrApiKeyNotFound + } + + return response.Error(status, failedToDeleteMsg, err) + } + + return response.Success("API key deleted") +} diff --git a/pkg/services/serviceaccounts/api/token_test.go b/pkg/services/serviceaccounts/api/token_test.go new file mode 100644 index 00000000000..ebb176816c1 --- /dev/null +++ b/pkg/services/serviceaccounts/api/token_test.go @@ -0,0 +1,238 @@ +package api + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/grafana/grafana/pkg/api/routing" + "github.com/grafana/grafana/pkg/bus" + "github.com/grafana/grafana/pkg/components/apikeygen" + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/accesscontrol" + accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" + "github.com/grafana/grafana/pkg/services/serviceaccounts" + "github.com/grafana/grafana/pkg/services/serviceaccounts/tests" + "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/web" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + serviceaccountIDTokensPath = "/api/serviceaccounts/%v/tokens" // #nosec G101 + serviceaccountIDTokensDetailPath = "/api/serviceaccounts/%v/tokens/%v" // #nosec G101 +) + +func createTokenforSA(t *testing.T, keyName string, orgID int64, saID int64) *models.ApiKey { + key, err := apikeygen.New(orgID, keyName) + require.NoError(t, err) + cmd := models.AddApiKeyCommand{ + Name: keyName, + Role: "Viewer", + OrgId: orgID, + Key: key.HashedKey, + SecondsToLive: 0, + ServiceAccountId: &saID, + Result: &models.ApiKey{}, + } + err = bus.Dispatch(context.Background(), &cmd) + require.NoError(t, err) + + return cmd.Result +} + +func TestServiceAccountsAPI_CreateToken(t *testing.T) { + store := sqlstore.InitTestDB(t) + svcmock := tests.ServiceAccountMock{} + sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true}) + + type testCreateSAToken struct { + desc string + 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 *models.SignedInUser) ([]*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 *models.SignedInUser) ([]*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 *models.SignedInUser) ([]*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 *models.SignedInUser) ([]*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 { + req, err := http.NewRequest(httpMethod, requestpath, requestBody) + 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) { + 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, &svcmock, 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) + 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 := models.GetApiKeyByNameQuery{KeyName: tc.body["name"].(string), OrgId: sa.OrgId} + err = store.GetApiKeyByName(context.Background(), &query) + require.NoError(t, err) + + assert.Equal(t, sa.Id, *query.Result.ServiceAccountId) + assert.Equal(t, sa.OrgId, query.Result.OrgId) + } + }) + } +} + +func TestServiceAccountsAPI_DeleteToken(t *testing.T) { + store := sqlstore.InitTestDB(t) + svcmock := tests.ServiceAccountMock{} + sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true}) + + type testCreateSAToken struct { + desc string + keyName string + expectedCode int + acmock *accesscontrolmock.Mock + } + + testCases := []testCreateSAToken{ + { + desc: "should be ok to delete serviceaccount token with scope id permissions", + keyName: "Test1", + acmock: tests.SetupMockAccesscontrol( + t, + func(c context.Context, siu *models.SignedInUser) ([]*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", + keyName: "Test2", + acmock: tests.SetupMockAccesscontrol( + t, + func(c context.Context, siu *models.SignedInUser) ([]*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 *models.SignedInUser) ([]*accesscontrol.Permission, error) { + return []*accesscontrol.Permission{{Action: serviceaccounts.ActionWrite, Scope: "serviceaccounts:id:10"}}, nil + }, + false, + ), + 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) + recorder := httptest.NewRecorder() + server.ServeHTTP(recorder, req) + return recorder + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + token := createTokenforSA(t, tc.keyName, sa.OrgId, sa.Id) + + endpoint := fmt.Sprintf(serviceaccountIDTokensDetailPath, sa.Id, token.Id) + bodyString := "" + server := setupTestServer(t, &svcmock, 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 := models.GetApiKeyByNameQuery{KeyName: tc.keyName, OrgId: sa.OrgId} + err := store.GetApiKeyByName(context.Background(), &query) + if actualCode == http.StatusOK { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/pkg/services/serviceaccounts/database/database.go b/pkg/services/serviceaccounts/database/database.go index 212f761d0cf..4f4b9c2ced8 100644 --- a/pkg/services/serviceaccounts/database/database.go +++ b/pkg/services/serviceaccounts/database/database.go @@ -131,6 +131,7 @@ func (s *ServiceAccountsStoreImpl) ListTokens(ctx context.Context, orgID int64, }) return result, err } + func (s *ServiceAccountsStoreImpl) ListServiceAccounts(ctx context.Context, orgID, serviceAccountID int64) ([]*models.OrgUserDTO, error) { query := models.GetOrgUsersQuery{OrgId: orgID, IsServiceAccount: true} if serviceAccountID > 0 { diff --git a/pkg/services/serviceaccounts/manager/service.go b/pkg/services/serviceaccounts/manager/service.go index 447093b3266..04cc111cb03 100644 --- a/pkg/services/serviceaccounts/manager/service.go +++ b/pkg/services/serviceaccounts/manager/service.go @@ -12,6 +12,7 @@ import ( "github.com/grafana/grafana/pkg/services/serviceaccounts/api" "github.com/grafana/grafana/pkg/services/serviceaccounts/database" "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/setting" ) var ( @@ -25,6 +26,7 @@ type ServiceAccountsService struct { } func ProvideServiceAccountsService( + cfg *setting.Cfg, features featuremgmt.FeatureToggles, store *sqlstore.SQLStore, ac accesscontrol.AccessControl, @@ -42,7 +44,7 @@ func ProvideServiceAccountsService( } } - serviceaccountsAPI := api.NewServiceAccountsAPI(s, ac, routeRegister, s.store) + serviceaccountsAPI := api.NewServiceAccountsAPI(cfg, s, ac, routeRegister, s.store, store) serviceaccountsAPI.RegisterAPIEndpoints(features) return s, nil diff --git a/pkg/services/serviceaccounts/models.go b/pkg/services/serviceaccounts/models.go index 1108cc46d88..e3b8cfd48c7 100644 --- a/pkg/services/serviceaccounts/models.go +++ b/pkg/services/serviceaccounts/models.go @@ -4,7 +4,7 @@ import "github.com/grafana/grafana/pkg/services/accesscontrol" var ( ScopeAll = "serviceaccounts:*" - ScopeID = accesscontrol.Scope("serviceaccounts", "id", accesscontrol.Parameter(":serviceaccountId")) + ScopeID = accesscontrol.Scope("serviceaccounts", "id", accesscontrol.Parameter(":serviceAccountId")) ) const ( diff --git a/pkg/services/sqlstore/apikey.go b/pkg/services/sqlstore/apikey.go index c80f146590d..72e4474b7fc 100644 --- a/pkg/services/sqlstore/apikey.go +++ b/pkg/services/sqlstore/apikey.go @@ -125,7 +125,7 @@ func (ss *SQLStore) UpdateApikeyServiceAccount(ctx context.Context, apikeyId int ss.log.Warn("API key not found", "err", err) return models.ErrApiKeyNotFound } - key.ServiceAccountId = saccountId + key.ServiceAccountId = &saccountId if _, err := sess.ID(key.Id).Update(&key); err != nil { ss.log.Warn("Could not update api key", "err", err) diff --git a/pkg/services/sqlstore/apikey_test.go b/pkg/services/sqlstore/apikey_test.go index 06a17378f2c..3e69a3c6498 100644 --- a/pkg/services/sqlstore/apikey_test.go +++ b/pkg/services/sqlstore/apikey_test.go @@ -34,7 +34,20 @@ func TestApiKeyDataAccess(t *testing.T) { }) t.Run("Add non expiring key", func(t *testing.T) { - cmd := models.AddApiKeyCommand{OrgId: 1, Name: "non-expiring", Key: "asd1", SecondsToLive: 0} + cmd := models.AddApiKeyCommand{OrgId: 1, Name: "non-expiring", Key: "asd1", SecondsToLive: 0, ServiceAccountId: nil} + err := ss.AddAPIKey(context.Background(), &cmd) + assert.Nil(t, err) + + query := models.GetApiKeyByNameQuery{KeyName: "non-expiring", OrgId: 1} + err = ss.GetApiKeyByName(context.Background(), &query) + assert.Nil(t, err) + + assert.Nil(t, query.Result.Expires) + }) + + t.Run("Add key for service account", func(t *testing.T) { + var one int64 = 1 + cmd := models.AddApiKeyCommand{OrgId: 1, Name: "non-expiring-SA", Key: "sa1-key", ServiceAccountId: &one} err := ss.AddAPIKey(context.Background(), &cmd) assert.Nil(t, err)