Access control: refactor RBAC checks (#48107)

* refactor RBAC checks

* fix a test

* another test fix

* and another
This commit is contained in:
Ieva 2022-04-25 10:42:09 +02:00 committed by GitHub
parent 2e599643f6
commit 68ca5b2e05
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 27 additions and 29 deletions

View File

@ -12,7 +12,6 @@ import (
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/annotations"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/guardian"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/web"
@ -275,7 +274,7 @@ func (hs *HTTPServer) MassDeleteAnnotations(c *models.ReqContext) response.Respo
// validations only for RBAC. A user can mass delete all annotations in a (dashboard + panel) or a specific annotation
// if has access to that dashboard.
if hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if !hs.AccessControl.IsDisabled() {
var dashboardId int64
if cmd.AnnotationId != 0 {
@ -351,7 +350,7 @@ func (hs *HTTPServer) canSaveAnnotation(c *models.ReqContext, annotation *annota
if annotation.GetType() == annotations.Dashboard {
return canEditDashboard(c, annotation.DashboardId)
} else {
if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if hs.AccessControl.IsDisabled() {
return c.SignedInUser.HasRole(models.ROLE_EDITOR), nil
}
return true, nil
@ -446,7 +445,7 @@ func AnnotationTypeScopeResolver() (string, accesscontrol.AttributeScopeResolveF
func (hs *HTTPServer) canCreateAnnotation(c *models.ReqContext, dashboardId int64) (bool, error) {
if dashboardId != 0 {
if hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if !hs.AccessControl.IsDisabled() {
evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsCreate, accesscontrol.ScopeAnnotationsTypeDashboard)
if canSave, err := hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator); err != nil || !canSave {
return canSave, err
@ -454,7 +453,7 @@ func (hs *HTTPServer) canCreateAnnotation(c *models.ReqContext, dashboardId int6
}
return canEditDashboard(c, dashboardId)
} else { // organization annotations
if hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if !hs.AccessControl.IsDisabled() {
evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsCreate, accesscontrol.ScopeAnnotationsTypeOrganization)
return hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator)
} else {

View File

@ -11,7 +11,6 @@ import (
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/guardian"
"github.com/grafana/grafana/pkg/web"
)
@ -133,7 +132,7 @@ func (hs *HTTPServer) UpdateDashboardPermissions(c *models.ReqContext) response.
return response.Error(403, "Cannot remove own admin permission for a folder", nil)
}
if hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if !hs.AccessControl.IsDisabled() {
old, err := g.GetAcl()
if err != nil {
return response.Error(500, "Error while checking dashboard permissions", err)

View File

@ -38,6 +38,7 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) {
dashboardService: dashboardservice.ProvideDashboardService(
settings, dashboardStore, nil, features, accesscontrolmock.NewPermissionsServicesMock(),
),
AccessControl: accesscontrolmock.New().WithDisabled(),
}
t.Run("Given user has no admin permissions", func(t *testing.T) {

View File

@ -9,7 +9,6 @@ import (
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/guardian"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/web"
@ -115,7 +114,7 @@ func (hs *HTTPServer) UpdateFolderPermissions(c *models.ReqContext) response.Res
return response.Error(403, "Cannot remove own admin permission for a folder", nil)
}
if hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if !hs.AccessControl.IsDisabled() {
old, err := g.GetAcl()
if err != nil {
return response.Error(500, "Error while checking dashboard permissions", err)

View File

@ -44,6 +44,7 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) {
dashboardService: service.ProvideDashboardService(
settings, dashboardStore, nil, features, permissionsServices,
),
AccessControl: accesscontrolmock.New().WithDisabled(),
}
t.Run("Given folder not exists", func(t *testing.T) {

View File

@ -733,7 +733,7 @@ func (hs *HTTPServer) setIndexViewData(c *models.ReqContext) (*dtos.IndexViewDat
LoadingLogo: "public/img/grafana_icon.svg",
}
if hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if !hs.AccessControl.IsDisabled() {
userPermissions, err := hs.AccessControl.GetUserPermissions(c.Req.Context(), c.SignedInUser, ac.Options{ReloadCache: false})
if err != nil {
return nil, err

View File

@ -10,7 +10,6 @@ import (
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/infra/metrics"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util"
@ -90,7 +89,7 @@ func (hs *HTTPServer) CreateOrg(c *models.ReqContext) response.Response {
if err := web.Bind(c.Req, &cmd); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
acEnabled := hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol)
acEnabled := !hs.AccessControl.IsDisabled()
if !acEnabled && !(setting.AllowUserOrgCreate || c.IsGrafanaAdmin) {
return response.Error(403, "Access denied", nil)
}

View File

@ -8,7 +8,6 @@ import (
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/web"
)
@ -19,7 +18,7 @@ func (hs *HTTPServer) CreateTeam(c *models.ReqContext) response.Response {
if err := web.Bind(c.Req, &cmd); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
accessControlEnabled := hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol)
accessControlEnabled := !hs.AccessControl.IsDisabled()
if !accessControlEnabled && c.OrgRole == models.ROLE_VIEWER {
return response.Error(403, "Not allowed to create team.", nil)
}
@ -63,7 +62,7 @@ func (hs *HTTPServer) UpdateTeam(c *models.ReqContext) response.Response {
return response.Error(http.StatusBadRequest, "teamId is invalid", err)
}
if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if hs.AccessControl.IsDisabled() {
if err := hs.teamGuardian.CanAdmin(c.Req.Context(), cmd.OrgId, cmd.Id, c.SignedInUser); err != nil {
return response.Error(403, "Not allowed to update team", err)
}
@ -88,7 +87,7 @@ func (hs *HTTPServer) DeleteTeamByID(c *models.ReqContext) response.Response {
}
user := c.SignedInUser
if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if hs.AccessControl.IsDisabled() {
if err := hs.teamGuardian.CanAdmin(c.Req.Context(), orgId, teamId, user); err != nil {
return response.Error(403, "Not allowed to delete team", err)
}
@ -116,7 +115,7 @@ func (hs *HTTPServer) SearchTeams(c *models.ReqContext) response.Response {
// Using accesscontrol the filtering is done based on user permissions
userIdFilter := models.FilterIgnoreUser
if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if hs.AccessControl.IsDisabled() {
userIdFilter = userFilter(c)
}
@ -174,7 +173,7 @@ func (hs *HTTPServer) GetTeamByID(c *models.ReqContext) response.Response {
// Using accesscontrol the filtering has already been performed at middleware layer
userIdFilter := models.FilterIgnoreUser
if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if hs.AccessControl.IsDisabled() {
userIdFilter = userFilter(c)
}
@ -210,7 +209,7 @@ func (hs *HTTPServer) GetTeamPreferences(c *models.ReqContext) response.Response
orgId := c.OrgId
if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if hs.AccessControl.IsDisabled() {
if err := hs.teamGuardian.CanAdmin(c.Req.Context(), orgId, teamId, c.SignedInUser); err != nil {
return response.Error(403, "Not allowed to view team preferences.", err)
}
@ -233,7 +232,7 @@ func (hs *HTTPServer) UpdateTeamPreferences(c *models.ReqContext) response.Respo
orgId := c.OrgId
if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if hs.AccessControl.IsDisabled() {
if err := hs.teamGuardian.CanAdmin(c.Req.Context(), orgId, teamId, c.SignedInUser); err != nil {
return response.Error(403, "Not allowed to update team preferences.", err)
}

View File

@ -11,7 +11,6 @@ import (
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/web"
)
@ -27,7 +26,7 @@ func (hs *HTTPServer) GetTeamMembers(c *models.ReqContext) response.Response {
// With accesscontrol the permission check has been done at middleware layer
// and the membership filtering will be done at DB layer based on user permissions
if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if hs.AccessControl.IsDisabled() {
if err := hs.teamGuardian.CanAdmin(c.Req.Context(), query.OrgId, query.TeamId, c.SignedInUser); err != nil {
return response.Error(403, "Not allowed to list team members", err)
}
@ -70,7 +69,7 @@ func (hs *HTTPServer) AddTeamMember(c *models.ReqContext) response.Response {
return response.Error(http.StatusBadRequest, "teamId is invalid", err)
}
if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if hs.AccessControl.IsDisabled() {
if err := hs.teamGuardian.CanAdmin(c.Req.Context(), cmd.OrgId, cmd.TeamId, c.SignedInUser); err != nil {
return response.Error(403, "Not allowed to add team member", err)
}
@ -110,7 +109,7 @@ func (hs *HTTPServer) UpdateTeamMember(c *models.ReqContext) response.Response {
}
orgId := c.OrgId
if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if hs.AccessControl.IsDisabled() {
if err := hs.teamGuardian.CanAdmin(c.Req.Context(), orgId, teamId, c.SignedInUser); err != nil {
return response.Error(403, "Not allowed to update team member", err)
}
@ -153,7 +152,7 @@ func (hs *HTTPServer) RemoveTeamMember(c *models.ReqContext) response.Response {
return response.Error(http.StatusBadRequest, "userId is invalid", err)
}
if !hs.Features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if hs.AccessControl.IsDisabled() {
if err := hs.teamGuardian.CanAdmin(c.Req.Context(), orgId, teamId, c.SignedInUser); err != nil {
return response.Error(403, "Not allowed to remove team member", err)
}

View File

@ -105,7 +105,7 @@ func (c *PermissionChecker) CheckWritePermissions(ctx context.Context, orgId int
if !c.features.IsEnabled(featuremgmt.FlagAnnotationComments) {
return false, nil
}
if c.features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if !c.accessControl.IsDisabled() {
evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsWrite, accesscontrol.ScopeAnnotationsTypeDashboard)
if canEdit, err := c.accessControl.Evaluate(ctx, signedInUser, evaluator); err != nil || !canEdit {
return canEdit, err

View File

@ -33,6 +33,7 @@ type Service struct {
cfg *setting.Cfg
features featuremgmt.FeatureToggles
permissionsService accesscontrol.PermissionsService
ac accesscontrol.AccessControl
ptc proxyTransportCache
dsDecryptionCache secureJSONDecryptionCache
@ -74,6 +75,7 @@ func ProvideService(
cfg: cfg,
features: features,
permissionsService: permissionsServices.GetDataSourceService(),
ac: ac,
}
ac.RegisterAttributeScopeResolver(NewNameScopeResolver(store))
@ -162,7 +164,7 @@ func (s *Service) AddDataSource(ctx context.Context, cmd *models.AddDataSourceCo
return err
}
if s.features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if !s.ac.IsDisabled() {
// This belongs in Data source permissions, and we probably want
// to do this with a hook in the store and rollback on fail.
// We can't use events, because there's no way to communicate

View File

@ -38,7 +38,7 @@ func TestService(t *testing.T) {
})
secretsService := secretsManager.SetupTestService(t, database.ProvideSecretsStore(sqlStore))
s := ProvideService(sqlStore, secretsService, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewPermissionsServicesMock())
s := ProvideService(sqlStore, secretsService, cfg, featuremgmt.WithFeatures(), acmock.New().WithDisabled(), acmock.NewPermissionsServicesMock())
var ds *models.DataSource

View File

@ -12,7 +12,7 @@ import (
type Provider struct{}
func ProvideService(store *sqlstore.SQLStore, ac accesscontrol.AccessControl, permissionsServices accesscontrol.PermissionsServices, features featuremgmt.FeatureToggles) *Provider {
if features.IsEnabled(featuremgmt.FlagAccesscontrol) {
if !ac.IsDisabled() {
// TODO: Fix this hack, see https://github.com/grafana/grafana-enterprise/issues/2935
InitAcessControlGuardian(store, ac, permissionsServices)
} else {