public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Access control: service account role check (#47710) (#47724)

Browse Source 
* forbid setting role higher than user's role

* change response code

* can assign API key permissions to non-admin users

* add: assign viewer role directly upon creation

* refactor: add AddSATcommand infavor of AddAPIkey

* refactor: frontend fixes for ServiceAccountToken

Co-authored-by: eleijonmarck <eric.leijonmarck@gmail.com>
(cherry picked from commit a245531f0c)

Co-authored-by: Ieva <ieva.vasiljeva@grafana.com>
This commit is contained in:
Grot (@grafanabot)
2022-04-13 17:27:23 +01:00
committed by GitHub
co-authored by Ieva
parent a8bfaafeb5
commit 67d42fc51c
15 changed files with 64 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/serviceaccounts/api/api.go
+3
View File
@@ -187,6 +187,9 @@ func (api *ServiceAccountsAPI) updateServiceAccount(c *models.ReqContext) respon
if cmd.Role != nil && !cmd.Role.IsValid() {
return response.Error(http.StatusBadRequest, "Invalid role specified", nil)
}
if cmd.Role != nil && !c.OrgRole.Includes(*cmd.Role) {
return response.Error(http.StatusForbidden, "Cannot assign a role higher than user's role", nil)
}
resp, err := api.store.UpdateServiceAccount(c.Req.Context(), c.OrgId, scopeID, &cmd)
if err != nil {
pkg/services/serviceaccounts/api/api_test.go
+19 -5
View File
@@ -227,7 +227,7 @@ func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock,
m := web.New()
signedUser := &models.SignedInUser{
OrgId: 1,
OrgRole: models.ROLE_ADMIN,
OrgRole: models.ROLE_VIEWER,
}
m.Use(func(c *web.Context) {
@@ -344,13 +344,14 @@ func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) {
Id int
}
role := models.ROLE_ADMIN
viewerRole := models.ROLE_VIEWER
editorRole := models.ROLE_EDITOR
var invalidRole models.RoleType = "InvalidRole"
testCases := []testUpdateSATestCase{
{
desc: "should be ok to update serviceaccount with permissions",
user: &tests.TestUser{Login: "servicetest1@admin", IsServiceAccount: true, Role: "Editor", Name: "Unaltered"},
body: &serviceaccounts.UpdateServiceAccountForm{Name: newString("New Name"), Role: &role},
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 *models.SignedInUser, _ accesscontrol.Options) ([]*accesscontrol.Permission, error) {
@@ -360,6 +361,19 @@ func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) {
),
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 *models.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",
user: &tests.TestUser{Login: "servicetest3@admin", IsServiceAccount: true, Role: "Invalid", Name: "Unaltered"},
@@ -375,7 +389,7 @@ func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) {
},
{
desc: "should be forbidden to update serviceaccount if no permissions",
user: &tests.TestUser{Login: "servicetest2@admin", IsServiceAccount: true},
user: &tests.TestUser{Login: "servicetest4@admin", IsServiceAccount: true},
body: nil,
acmock: tests.SetupMockAccesscontrol(
t,
pkg/services/serviceaccounts/api/token.go
+7 -13
View File
@@ -17,13 +17,12 @@ import (
const failedToDeleteMsg = "Failed to delete API key"
type TokenDTO struct {
Id int64 `json:"id"`
Name string `json:"name"`
Role models.RoleType `json:"role"`
Created *time.Time `json:"created"`
Expiration *time.Time `json:"expiration"`
SecondsUntilExpiration *float64 `json:"secondsUntilExpiration"`
HasExpired bool `json:"hasExpired"`
Id int64 `json:"id"`
Name string `json:"name"`
Created *time.Time `json:"created"`
Expiration *time.Time `json:"expiration"`
SecondsUntilExpiration *float64 `json:"secondsUntilExpiration"`
HasExpired bool `json:"hasExpired"`
}
func hasExpired(expiration *int64) bool {
@@ -60,7 +59,6 @@ func (api *ServiceAccountsAPI) ListTokens(ctx *models.ReqContext) response.Respo
result[i] = &TokenDTO{
Id: t.Id,
Name: t.Name,
Role: t.Role,
Created: &t.Created,
Expiration: expiration,
SecondsUntilExpiration: &secondsUntilExpiration,
@@ -91,7 +89,7 @@ func (api *ServiceAccountsAPI) CreateToken(c *models.ReqContext) response.Respon
}
}
cmd := models.AddApiKeyCommand{}
cmd := serviceaccounts.AddServiceAccountTokenCommand{}
if err := web.Bind(c.Req, &cmd); err != nil {
return response.Error(http.StatusBadRequest, "Bad request data", err)
}
@@ -99,10 +97,6 @@ func (api *ServiceAccountsAPI) CreateToken(c *models.ReqContext) response.Respon
// Force affected service account to be the one referenced in the URL
cmd.OrgId = c.OrgId
if !cmd.Role.IsValid() {
return response.Error(http.StatusBadRequest, "Invalid role specified", nil)
}
if api.cfg.ApiKeyMaxSecondsToLive != -1 {
if cmd.SecondsToLive == 0 {
return response.Error(http.StatusBadRequest, "Number of seconds before expiration should be set", nil)
pkg/services/serviceaccounts/api/token_test.go
+1 -2
View File
@@ -34,9 +34,8 @@ func createTokenforSA(t *testing.T, store serviceaccounts.Store, keyName string,
key, err := apikeygen.New(orgID, keyName)
require.NoError(t, err)
cmd := models.AddApiKeyCommand{
cmd := serviceaccounts.AddServiceAccountTokenCommand{
Name: keyName,
Role: "Viewer",
OrgId: orgID,
Key: key.HashedKey,
SecondsToLive: secondsToLive,
pkg/services/serviceaccounts/database/stats_test.go
+2 -2
View File
@@ -6,6 +6,7 @@ import (
"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/services/serviceaccounts/tests"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -20,9 +21,8 @@ func TestStore_UsageStats(t *testing.T) {
key, err := apikeygen.New(sa.OrgId, keyName)
require.NoError(t, err)
cmd := models.AddApiKeyCommand{
cmd := serviceaccounts.AddServiceAccountTokenCommand{
Name: keyName,
Role: "Viewer",
OrgId: sa.OrgId,
Key: key.HashedKey,
SecondsToLive: 0,
pkg/services/serviceaccounts/database/token_store.go
+3 -2
View File
@@ -5,10 +5,11 @@ import (
"time"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/serviceaccounts"
"github.com/grafana/grafana/pkg/services/sqlstore"
)
func (s *ServiceAccountsStoreImpl) AddServiceAccountToken(ctx context.Context, saID int64, cmd *models.AddApiKeyCommand) error {
func (s *ServiceAccountsStoreImpl) AddServiceAccountToken(ctx context.Context, saID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error {
return s.sqlStore.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
key := models.ApiKey{OrgId: cmd.OrgId, Name: cmd.Name}
exists, _ := sess.Get(&key)
@@ -28,7 +29,7 @@ func (s *ServiceAccountsStoreImpl) AddServiceAccountToken(ctx context.Context, s
t := models.ApiKey{
OrgId: cmd.OrgId,
Name: cmd.Name,
Role: cmd.Role,
Role: models.ROLE_VIEWER,
Key: cmd.Key,
Created: updated,
Updated: updated,
pkg/services/serviceaccounts/database/token_store_test.go
+3 -4
View File
@@ -6,6 +6,7 @@ import (
"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/services/serviceaccounts/tests"
"github.com/stretchr/testify/require"
)
@@ -28,9 +29,8 @@ func TestStore_AddServiceAccountToken(t *testing.T) {
key, err := apikeygen.New(user.OrgId, keyName)
require.NoError(t, err)
cmd := models.AddApiKeyCommand{
cmd := serviceaccounts.AddServiceAccountTokenCommand{
Name: keyName,
Role: "Viewer",
OrgId: user.OrgId,
Key: key.HashedKey,
SecondsToLive: tc.secondsToLive,
@@ -79,9 +79,8 @@ func TestStore_DeleteServiceAccountToken(t *testing.T) {
key, err := apikeygen.New(user.OrgId, keyName)
require.NoError(t, err)
cmd := models.AddApiKeyCommand{
cmd := serviceaccounts.AddServiceAccountTokenCommand{
Name: keyName,
Role: "Viewer",
OrgId: user.OrgId,
Key: key.HashedKey,
SecondsToLive: 0,
pkg/services/serviceaccounts/models.go
+9
View File
@@ -40,6 +40,15 @@ type ServiceAccountDTO struct {
AvatarUrl string `json:"avatarUrl"`
AccessControl map[string]bool `json:"accessControl,omitempty"`
}
type AddServiceAccountTokenCommand struct {
Name string `json:"name" binding:"Required"`
OrgId int64 `json:"-"`
Key string `json:"-"`
SecondsToLive int64 `json:"secondsToLive"`
Result *models.ApiKey `json:"-"`
}
type SearchServiceAccountsResult struct {
TotalCount int64 `json:"totalCount"`
ServiceAccounts []*ServiceAccountDTO `json:"serviceAccounts"`
pkg/services/serviceaccounts/serviceaccounts.go
+1 -1
View File
@@ -24,6 +24,6 @@ type Store interface {
ConvertToServiceAccounts(ctx context.Context, keys []int64) error
ListTokens(ctx context.Context, orgID int64, serviceAccount int64) ([]*models.ApiKey, error)
DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID, tokenID int64) error
AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *models.AddApiKeyCommand) error
AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *AddServiceAccountTokenCommand) error
GetUsageMetrics(ctx context.Context) (map[string]interface{}, error)
}
pkg/services/serviceaccounts/tests/common.go
+1 -1
View File
@@ -140,7 +140,7 @@ func (s *ServiceAccountsStoreMock) DeleteServiceAccountToken(ctx context.Context
return nil
}
func (s *ServiceAccountsStoreMock) AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *models.AddApiKeyCommand) error {
func (s *ServiceAccountsStoreMock) AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *serviceaccounts.AddServiceAccountTokenCommand) error {
s.Calls.AddServiceAccountToken = append(s.Calls.AddServiceAccountToken, []interface{}{ctx, cmd})
return nil
}