From 1f742fcf93a05cf217610b5103c296b66eae3433 Mon Sep 17 00:00:00 2001 From: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com> Date: Tue, 25 Jul 2023 15:31:12 +0300 Subject: [PATCH] Guardian: Split dashboard and folder guardian implementation (#69722) * Split dashboard and folder guardian implementations * Replace guardian constructors * Simplify tests * Add tests * Apply suggestion from code review Differentiate errors for dashboard and folders * Remove tests for general folder * Add tests for general scope --- pkg/api/folder.go | 10 +- pkg/services/dashboards/models.go | 7 +- pkg/services/folder/folderimpl/folder.go | 4 +- .../guardian/accesscontrol_guardian.go | 416 +++++++--- .../guardian/accesscontrol_guardian_test.go | 733 ++++++++++++++---- pkg/services/guardian/guardian.go | 35 + pkg/services/guardian/provider.go | 9 + 7 files changed, 985 insertions(+), 229 deletions(-) diff --git a/pkg/api/folder.go b/pkg/api/folder.go index 211b1cb7e18..46fc794395b 100644 --- a/pkg/api/folder.go +++ b/pkg/api/folder.go @@ -86,7 +86,7 @@ func (hs *HTTPServer) GetFolderByUID(c *contextmodel.ReqContext) response.Respon return apierrors.ToFolderErrorResponse(err) } - g, err := guardian.NewByUID(c.Req.Context(), folder.UID, c.OrgID, c.SignedInUser) + g, err := guardian.NewByFolder(c.Req.Context(), folder, c.OrgID, c.SignedInUser) if err != nil { return response.Err(err) } @@ -119,7 +119,7 @@ func (hs *HTTPServer) GetFolderByID(c *contextmodel.ReqContext) response.Respons return apierrors.ToFolderErrorResponse(err) } - g, err := guardian.NewByUID(c.Req.Context(), folder.UID, c.OrgID, c.SignedInUser) + g, err := guardian.NewByFolder(c.Req.Context(), folder, c.OrgID, c.SignedInUser) if err != nil { return response.Err(err) } @@ -160,7 +160,7 @@ func (hs *HTTPServer) CreateFolder(c *contextmodel.ReqContext) response.Response // Required for cases when caller wants to immediately interact with the newly created object hs.accesscontrolService.ClearUserPermissionCache(c.SignedInUser) - g, err := guardian.NewByUID(c.Req.Context(), folder.UID, c.OrgID, c.SignedInUser) + g, err := guardian.NewByFolder(c.Req.Context(), folder, c.OrgID, c.SignedInUser) if err != nil { return response.Err(err) } @@ -215,7 +215,7 @@ func (hs *HTTPServer) MoveFolder(c *contextmodel.ReqContext) response.Response { return response.Error(http.StatusInternalServerError, "move folder failed", err) } - g, err := guardian.NewByUID(c.Req.Context(), cmd.UID, c.OrgID, c.SignedInUser) + g, err := guardian.NewByFolder(c.Req.Context(), theFolder, c.OrgID, c.SignedInUser) if err != nil { return response.Err(err) } @@ -251,7 +251,7 @@ func (hs *HTTPServer) UpdateFolder(c *contextmodel.ReqContext) response.Response if err != nil { return apierrors.ToFolderErrorResponse(err) } - g, err := guardian.NewByUID(c.Req.Context(), result.UID, c.OrgID, c.SignedInUser) + g, err := guardian.NewByFolder(c.Req.Context(), result, c.OrgID, c.SignedInUser) if err != nil { return response.Err(err) } diff --git a/pkg/services/dashboards/models.go b/pkg/services/dashboards/models.go index be670c8cdac..b8a10de177f 100644 --- a/pkg/services/dashboards/models.go +++ b/pkg/services/dashboards/models.go @@ -418,9 +418,10 @@ type DashboardACL struct { func (p DashboardACL) TableName() string { return "dashboard_acl" } type DashboardACLInfoDTO struct { - OrgID int64 `json:"-" xorm:"org_id"` - DashboardID int64 `json:"dashboardId,omitempty" xorm:"dashboard_id"` - FolderID int64 `json:"folderId,omitempty" xorm:"folder_id"` + OrgID int64 `json:"-" xorm:"org_id"` + DashboardID int64 `json:"dashboardId,omitempty" xorm:"dashboard_id"` + FolderID int64 `json:"folderId,omitempty" xorm:"folder_id"` + FolderUID string `json:"folderUid,omitempty" xorm:"folder_uid"` Created time.Time `json:"created"` Updated time.Time `json:"updated"` diff --git a/pkg/services/folder/folderimpl/folder.go b/pkg/services/folder/folderimpl/folder.go index 42f4e89b766..0204ec61cd9 100644 --- a/pkg/services/folder/folderimpl/folder.go +++ b/pkg/services/folder/folderimpl/folder.go @@ -127,7 +127,7 @@ func (s *Service) Get(ctx context.Context, cmd *folder.GetFolderQuery) (*folder. // do not get guardian by the folder ID because it differs from the nested folder ID // and the legacy folder ID has been associated with the permissions: // use the folde UID instead that is the same for both - g, err := guardian.NewByUID(ctx, dashFolder.UID, dashFolder.OrgID, cmd.SignedInUser) + g, err := guardian.NewByFolder(ctx, dashFolder, dashFolder.OrgID, cmd.SignedInUser) if err != nil { return nil, err } @@ -204,7 +204,7 @@ func (s *Service) GetChildren(ctx context.Context, cmd *folder.GetChildrenQuery) continue } - g, err := guardian.NewByUID(ctx, f.UID, f.OrgID, cmd.SignedInUser) + g, err := guardian.NewByFolder(ctx, dashFolder, dashFolder.OrgID, cmd.SignedInUser) if err != nil { return nil, err } diff --git a/pkg/services/guardian/accesscontrol_guardian.go b/pkg/services/guardian/accesscontrol_guardian.go index 2b3162d4546..612ea82289c 100644 --- a/pkg/services/guardian/accesscontrol_guardian.go +++ b/pkg/services/guardian/accesscontrol_guardian.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" @@ -19,7 +20,7 @@ var permissionMap = map[string]dashboards.PermissionType{ "Admin": dashboards.PERMISSION_ADMIN, } -var _ DashboardGuardian = new(AccessControlDashboardGuardian) +var _ DashboardGuardian = new(accessControlDashboardGuardian) // NewAccessControlDashboardGuardianByDashboard creates a dashboard guardian by the provided dashboardId. func NewAccessControlDashboardGuardian( @@ -28,7 +29,7 @@ func NewAccessControlDashboardGuardian( folderPermissionsService accesscontrol.FolderPermissionsService, dashboardPermissionsService accesscontrol.DashboardPermissionsService, dashboardService dashboards.DashboardService, -) (*AccessControlDashboardGuardian, error) { +) (DashboardGuardian, error) { var dashboard *dashboards.Dashboard if dashboardId != 0 { q := &dashboards.GetDashboardQuery{ @@ -46,17 +47,34 @@ func NewAccessControlDashboardGuardian( dashboard = qResult } - return &AccessControlDashboardGuardian{ - ctx: ctx, - cfg: cfg, - log: log.New("dashboard.permissions"), + if dashboard != nil && dashboard.IsFolder { + return &accessControlFolderGuardian{ + accessControlBaseGuardian: accessControlBaseGuardian{ + ctx: ctx, + cfg: cfg, + log: log.New("folder.permissions"), + user: user, + store: store, + ac: ac, + dashboardService: dashboardService, + }, + folder: dashboards.FromDashboard(dashboard), + folderPermissionsService: folderPermissionsService, + }, nil + } + + return &accessControlDashboardGuardian{ + accessControlBaseGuardian: accessControlBaseGuardian{ + ctx: ctx, + cfg: cfg, + log: log.New("dashboard.permissions"), + user: user, + store: store, + ac: ac, + dashboardService: dashboardService, + }, dashboard: dashboard, - user: user, - store: store, - ac: ac, - folderPermissionsService: folderPermissionsService, dashboardPermissionsService: dashboardPermissionsService, - dashboardService: dashboardService, }, nil } @@ -67,7 +85,7 @@ func NewAccessControlDashboardGuardianByUID( folderPermissionsService accesscontrol.FolderPermissionsService, dashboardPermissionsService accesscontrol.DashboardPermissionsService, dashboardService dashboards.DashboardService, -) (*AccessControlDashboardGuardian, error) { +) (DashboardGuardian, error) { var dashboard *dashboards.Dashboard if dashboardUID != "" { q := &dashboards.GetDashboardQuery{ @@ -85,17 +103,34 @@ func NewAccessControlDashboardGuardianByUID( dashboard = qResult } - return &AccessControlDashboardGuardian{ - cfg: cfg, - ctx: ctx, - log: log.New("dashboard.permissions"), + if dashboard != nil && dashboard.IsFolder { + return &accessControlFolderGuardian{ + accessControlBaseGuardian: accessControlBaseGuardian{ + ctx: ctx, + cfg: cfg, + log: log.New("folder.permissions"), + user: user, + store: store, + ac: ac, + dashboardService: dashboardService, + }, + folder: dashboards.FromDashboard(dashboard), + folderPermissionsService: folderPermissionsService, + }, nil + } + + return &accessControlDashboardGuardian{ + accessControlBaseGuardian: accessControlBaseGuardian{ + cfg: cfg, + ctx: ctx, + log: log.New("dashboard.permissions"), + user: user, + store: store, + ac: ac, + dashboardService: dashboardService, + }, dashboard: dashboard, - user: user, - store: store, - ac: ac, - folderPermissionsService: folderPermissionsService, dashboardPermissionsService: dashboardPermissionsService, - dashboardService: dashboardService, }, nil } @@ -108,41 +143,86 @@ func NewAccessControlDashboardGuardianByDashboard( folderPermissionsService accesscontrol.FolderPermissionsService, dashboardPermissionsService accesscontrol.DashboardPermissionsService, dashboardService dashboards.DashboardService, -) (*AccessControlDashboardGuardian, error) { - return &AccessControlDashboardGuardian{ - cfg: cfg, - ctx: ctx, - log: log.New("dashboard.permissions"), +) (DashboardGuardian, error) { + if dashboard != nil && dashboard.IsFolder { + return &accessControlFolderGuardian{ + accessControlBaseGuardian: accessControlBaseGuardian{ + ctx: ctx, + cfg: cfg, + log: log.New("folder.permissions"), + user: user, + store: store, + ac: ac, + dashboardService: dashboardService, + }, + folder: dashboards.FromDashboard(dashboard), + folderPermissionsService: folderPermissionsService, + }, nil + } + + return &accessControlDashboardGuardian{ + accessControlBaseGuardian: accessControlBaseGuardian{ + cfg: cfg, + ctx: ctx, + log: log.New("dashboard.permissions"), + user: user, + store: store, + ac: ac, + dashboardService: dashboardService, + }, dashboard: dashboard, - user: user, - store: store, - ac: ac, - folderPermissionsService: folderPermissionsService, dashboardPermissionsService: dashboardPermissionsService, - dashboardService: dashboardService, }, nil } -type AccessControlDashboardGuardian struct { - cfg *setting.Cfg - ctx context.Context - log log.Logger - dashboard *dashboards.Dashboard - user *user.SignedInUser - store db.DB - ac accesscontrol.AccessControl - folderPermissionsService accesscontrol.FolderPermissionsService - dashboardPermissionsService accesscontrol.DashboardPermissionsService - dashboardService dashboards.DashboardService +// NewAccessControlFolderGuardian creates a folder guardian by the provided folder. +func NewAccessControlFolderGuardian( + ctx context.Context, cfg *setting.Cfg, f *folder.Folder, user *user.SignedInUser, + store db.DB, ac accesscontrol.AccessControl, + folderPermissionsService accesscontrol.FolderPermissionsService, + dashboardPermissionsService accesscontrol.DashboardPermissionsService, + dashboardService dashboards.DashboardService, +) (DashboardGuardian, error) { + return &accessControlFolderGuardian{ + accessControlBaseGuardian: accessControlBaseGuardian{ + ctx: ctx, + cfg: cfg, + log: log.New("folder.permissions"), + user: user, + store: store, + ac: ac, + dashboardService: dashboardService, + }, + folder: f, + folderPermissionsService: folderPermissionsService, + }, nil } -func (a *AccessControlDashboardGuardian) CanSave() (bool, error) { - if a.dashboard == nil { - return false, ErrGuardianDashboardNotFound - } +type accessControlBaseGuardian struct { + cfg *setting.Cfg + ctx context.Context + log log.Logger + user *user.SignedInUser + ac accesscontrol.AccessControl + store db.DB + dashboardService dashboards.DashboardService +} - if a.dashboard.IsFolder { - return a.evaluate(accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(a.dashboard.UID))) +type accessControlDashboardGuardian struct { + accessControlBaseGuardian + dashboard *dashboards.Dashboard + dashboardPermissionsService accesscontrol.DashboardPermissionsService +} + +type accessControlFolderGuardian struct { + accessControlBaseGuardian + folder *folder.Folder + folderPermissionsService accesscontrol.FolderPermissionsService +} + +func (a *accessControlDashboardGuardian) CanSave() (bool, error) { + if a.dashboard == nil { + return false, ErrGuardianDashboardNotFound.Errorf("failed to check save permissions for dashboard") } return a.evaluate( @@ -150,31 +230,43 @@ func (a *AccessControlDashboardGuardian) CanSave() (bool, error) { ) } -func (a *AccessControlDashboardGuardian) CanEdit() (bool, error) { +func (a *accessControlFolderGuardian) CanSave() (bool, error) { + if a.folder == nil { + return false, ErrGuardianFolderNotFound.Errorf("failed to check save permissions for folder") + } + + return a.evaluate(accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(a.folder.UID))) +} + +func (a *accessControlDashboardGuardian) CanEdit() (bool, error) { if a.dashboard == nil { - return false, ErrGuardianDashboardNotFound + return false, ErrGuardianDashboardNotFound.Errorf("failed to check edit permissions for dashboard") } if a.cfg.ViewersCanEdit { return a.CanView() } - if a.dashboard.IsFolder { - return a.evaluate(accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(a.dashboard.UID))) - } - return a.evaluate( accesscontrol.EvalPermission(dashboards.ActionDashboardsWrite, dashboards.ScopeDashboardsProvider.GetResourceScopeUID(a.dashboard.UID)), ) } -func (a *AccessControlDashboardGuardian) CanView() (bool, error) { - if a.dashboard == nil { - return false, ErrGuardianDashboardNotFound +func (a *accessControlFolderGuardian) CanEdit() (bool, error) { + if a.folder == nil { + return false, ErrGuardianFolderNotFound.Errorf("failed to check edit permissions for folder") } - if a.dashboard.IsFolder { - return a.evaluate(accesscontrol.EvalPermission(dashboards.ActionFoldersRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(a.dashboard.UID))) + if a.cfg.ViewersCanEdit { + return a.CanView() + } + + return a.evaluate(accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(a.folder.UID))) +} + +func (a *accessControlDashboardGuardian) CanView() (bool, error) { + if a.dashboard == nil { + return false, ErrGuardianDashboardNotFound.Errorf("failed to check view permissions for dashboard") } return a.evaluate( @@ -182,16 +274,17 @@ func (a *AccessControlDashboardGuardian) CanView() (bool, error) { ) } -func (a *AccessControlDashboardGuardian) CanAdmin() (bool, error) { - if a.dashboard == nil { - return false, ErrGuardianDashboardNotFound +func (a *accessControlFolderGuardian) CanView() (bool, error) { + if a.folder == nil { + return false, ErrGuardianFolderNotFound.Errorf("failed to check view permissions for folder") } - if a.dashboard.IsFolder { - return a.evaluate(accesscontrol.EvalAll( - accesscontrol.EvalPermission(dashboards.ActionFoldersPermissionsRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(a.dashboard.UID)), - accesscontrol.EvalPermission(dashboards.ActionFoldersPermissionsWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(a.dashboard.UID)), - )) + return a.evaluate(accesscontrol.EvalPermission(dashboards.ActionFoldersRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(a.folder.UID))) +} + +func (a *accessControlDashboardGuardian) CanAdmin() (bool, error) { + if a.dashboard == nil { + return false, ErrGuardianDashboardNotFound.Errorf("failed to check admin permissions for dashboard") } return a.evaluate(accesscontrol.EvalAll( @@ -200,13 +293,20 @@ func (a *AccessControlDashboardGuardian) CanAdmin() (bool, error) { )) } -func (a *AccessControlDashboardGuardian) CanDelete() (bool, error) { - if a.dashboard == nil { - return false, ErrGuardianDashboardNotFound +func (a *accessControlFolderGuardian) CanAdmin() (bool, error) { + if a.folder == nil { + return false, ErrGuardianFolderNotFound.Errorf("failed to check admin permissions for folder") } - if a.dashboard.IsFolder { - return a.evaluate(accesscontrol.EvalPermission(dashboards.ActionFoldersDelete, dashboards.ScopeFoldersProvider.GetResourceScopeUID(a.dashboard.UID))) + return a.evaluate(accesscontrol.EvalAll( + accesscontrol.EvalPermission(dashboards.ActionFoldersPermissionsRead, dashboards.ScopeFoldersProvider.GetResourceScopeUID(a.folder.UID)), + accesscontrol.EvalPermission(dashboards.ActionFoldersPermissionsWrite, dashboards.ScopeFoldersProvider.GetResourceScopeUID(a.folder.UID)), + )) +} + +func (a *accessControlDashboardGuardian) CanDelete() (bool, error) { + if a.dashboard == nil { + return false, ErrGuardianDashboardNotFound.Errorf("failed to check delete permissions for dashboard") } return a.evaluate( @@ -214,7 +314,15 @@ func (a *AccessControlDashboardGuardian) CanDelete() (bool, error) { ) } -func (a *AccessControlDashboardGuardian) CanCreate(folderID int64, isFolder bool) (bool, error) { +func (a *accessControlFolderGuardian) CanDelete() (bool, error) { + if a.folder == nil { + return false, ErrGuardianFolderNotFound.Errorf("failed to check delete permissions for folder") + } + + return a.evaluate(accesscontrol.EvalPermission(dashboards.ActionFoldersDelete, dashboards.ScopeFoldersProvider.GetResourceScopeUID(a.folder.UID))) +} + +func (a *accessControlDashboardGuardian) CanCreate(folderID int64, isFolder bool) (bool, error) { if isFolder { return a.evaluate(accesscontrol.EvalPermission(dashboards.ActionFoldersCreate)) } @@ -225,14 +333,25 @@ func (a *AccessControlDashboardGuardian) CanCreate(folderID int64, isFolder bool return a.evaluate(accesscontrol.EvalPermission(dashboards.ActionDashboardsCreate, dashboards.ScopeFoldersProvider.GetResourceScopeUID(folder.UID))) } -func (a *AccessControlDashboardGuardian) evaluate(evaluator accesscontrol.Evaluator) (bool, error) { +func (a *accessControlFolderGuardian) CanCreate(folderID int64, isFolder bool) (bool, error) { + if isFolder { + return a.evaluate(accesscontrol.EvalPermission(dashboards.ActionFoldersCreate)) + } + folder, err := a.loadParentFolder(folderID) + if err != nil { + return false, err + } + return a.evaluate(accesscontrol.EvalPermission(dashboards.ActionDashboardsCreate, dashboards.ScopeFoldersProvider.GetResourceScopeUID(folder.UID))) +} + +func (a *accessControlDashboardGuardian) evaluate(evaluator accesscontrol.Evaluator) (bool, error) { ok, err := a.ac.Evaluate(a.ctx, a.user, evaluator) if err != nil { id := 0 if a.dashboard != nil { id = int(a.dashboard.ID) } - a.log.Debug("Failed to evaluate access control to folder or dashboard", "error", err, "userId", a.user.UserID, "id", id) + a.log.Debug("Failed to evaluate access control to dashboard", "error", err, "userId", a.user.UserID, "id", id) } if !ok && err == nil { @@ -240,29 +359,49 @@ func (a *AccessControlDashboardGuardian) evaluate(evaluator accesscontrol.Evalua if a.dashboard != nil { id = int(a.dashboard.ID) } - a.log.Debug("Access denied to folder or dashboard", "userId", a.user.UserID, "id", id, "permissions", evaluator.GoString()) + a.log.Debug("Access denied to dashboard", "userId", a.user.UserID, "id", id, "permissions", evaluator.GoString()) } return ok, err } -func (a *AccessControlDashboardGuardian) CheckPermissionBeforeUpdate(permission dashboards.PermissionType, updatePermissions []*dashboards.DashboardACL) (bool, error) { +func (a *accessControlFolderGuardian) evaluate(evaluator accesscontrol.Evaluator) (bool, error) { + ok, err := a.ac.Evaluate(a.ctx, a.user, evaluator) + if err != nil { + uid := "" + orgID := 0 + if a.folder != nil { + uid = a.folder.UID + orgID = int(a.folder.OrgID) + } + a.log.Debug("Failed to evaluate access control to folder", "error", err, "userId", a.user.UserID, "orgID", orgID, "uid", uid) + } + + if !ok && err == nil { + uid := "" + orgID := 0 + if a.folder != nil { + uid = a.folder.UID + orgID = int(a.folder.OrgID) + } + a.log.Debug("Access denied to folder", "userId", a.user.UserID, "orgID", orgID, "uid", uid, "permissions", evaluator.GoString()) + } + + return ok, err +} + +func (a *accessControlBaseGuardian) CheckPermissionBeforeUpdate(permission dashboards.PermissionType, updatePermissions []*dashboards.DashboardACL) (bool, error) { // always true for access control return true, nil } // GetACL translate access control permissions to dashboard acl info -func (a *AccessControlDashboardGuardian) GetACL() ([]*dashboards.DashboardACLInfoDTO, error) { +func (a *accessControlDashboardGuardian) GetACL() ([]*dashboards.DashboardACLInfoDTO, error) { if a.dashboard == nil { - return nil, ErrGuardianGetDashboardFailure + return nil, ErrGuardianGetDashboardFailure.Errorf("failed to translate access control permissions to dashboard acl info") } - var svc accesscontrol.PermissionsService - if a.dashboard.IsFolder { - svc = a.folderPermissionsService - } else { - svc = a.dashboardPermissionsService - } + svc := a.dashboardPermissionsService permissions, err := svc.GetPermissions(a.ctx, a.user, a.dashboard.UID) if err != nil { @@ -308,11 +447,67 @@ func (a *AccessControlDashboardGuardian) GetACL() ([]*dashboards.DashboardACLInf return acl, nil } -func (a *AccessControlDashboardGuardian) GetACLWithoutDuplicates() ([]*dashboards.DashboardACLInfoDTO, error) { +// GetACL translate access control permissions to dashboard acl info +func (a *accessControlFolderGuardian) GetACL() ([]*dashboards.DashboardACLInfoDTO, error) { + if a.folder == nil { + return nil, ErrGuardianGetFolderFailure.Errorf("failed to translate access control permissions to dashboard acl info") + } + + svc := a.folderPermissionsService + + permissions, err := svc.GetPermissions(a.ctx, a.user, a.folder.UID) + if err != nil { + return nil, err + } + + acl := make([]*dashboards.DashboardACLInfoDTO, 0, len(permissions)) + for _, p := range permissions { + if !p.IsManaged { + continue + } + + var role *org.RoleType + if p.BuiltInRole != "" { + tmp := org.RoleType(p.BuiltInRole) + role = &tmp + } + + acl = append(acl, &dashboards.DashboardACLInfoDTO{ + OrgID: a.folder.OrgID, + DashboardID: a.folder.ID, + FolderUID: a.folder.ParentUID, + Created: p.Created, + Updated: p.Updated, + UserID: p.UserId, + UserLogin: p.UserLogin, + UserEmail: p.UserEmail, + TeamID: p.TeamId, + TeamEmail: p.TeamEmail, + Team: p.Team, + Role: role, + Permission: permissionMap[svc.MapActions(p)], + PermissionName: permissionMap[svc.MapActions(p)].String(), + UID: a.folder.UID, + Title: a.folder.Title, + //Slug: a.folder.Slug, + IsFolder: true, + URL: a.folder.WithURL().URL, + Inherited: false, + }) + } + + return acl, nil +} + +func (a *accessControlDashboardGuardian) GetACLWithoutDuplicates() ([]*dashboards.DashboardACLInfoDTO, error) { return a.GetACL() } -func (a *AccessControlDashboardGuardian) GetHiddenACL(cfg *setting.Cfg) ([]*dashboards.DashboardACL, error) { +func (a *accessControlFolderGuardian) GetACLWithoutDuplicates() ([]*dashboards.DashboardACLInfoDTO, error) { + return a.GetACL() +} + +func (a *accessControlDashboardGuardian) GetHiddenACL(cfg *setting.Cfg) ([]*dashboards.DashboardACL, error) { var hiddenACL []*dashboards.DashboardACL if a.user.IsGrafanaAdmin { return hiddenACL, nil @@ -345,7 +540,52 @@ func (a *AccessControlDashboardGuardian) GetHiddenACL(cfg *setting.Cfg) ([]*dash return hiddenACL, nil } -func (a *AccessControlDashboardGuardian) loadParentFolder(folderID int64) (*dashboards.Dashboard, error) { +func (a *accessControlFolderGuardian) GetHiddenACL(cfg *setting.Cfg) ([]*dashboards.DashboardACL, error) { + var hiddenACL []*dashboards.DashboardACL + if a.user.IsGrafanaAdmin { + return hiddenACL, nil + } + + existingPermissions, err := a.GetACL() + if err != nil { + return hiddenACL, err + } + + for _, item := range existingPermissions { + if item.Inherited || item.UserLogin == a.user.Login { + continue + } + + if _, hidden := cfg.HiddenUsers[item.UserLogin]; hidden { + hiddenACL = append(hiddenACL, &dashboards.DashboardACL{ + OrgID: item.OrgID, + DashboardID: item.DashboardID, + UserID: item.UserID, + TeamID: item.TeamID, + Role: item.Role, + Permission: item.Permission, + Created: item.Created, + Updated: item.Updated, + }) + } + } + + return hiddenACL, nil +} + +func (a *accessControlDashboardGuardian) loadParentFolder(folderID int64) (*dashboards.Dashboard, error) { + if folderID == 0 { + return &dashboards.Dashboard{UID: accesscontrol.GeneralFolderUID}, nil + } + folderQuery := &dashboards.GetDashboardQuery{ID: folderID, OrgID: a.user.OrgID} + folderQueryResult, err := a.dashboardService.GetDashboard(a.ctx, folderQuery) + if err != nil { + return nil, err + } + return folderQueryResult, nil +} + +func (a *accessControlFolderGuardian) loadParentFolder(folderID int64) (*dashboards.Dashboard, error) { if folderID == 0 { return &dashboards.Dashboard{UID: accesscontrol.GeneralFolderUID}, nil } diff --git a/pkg/services/guardian/accesscontrol_guardian_test.go b/pkg/services/guardian/accesscontrol_guardian_test.go index 886180ec78d..3b79250b9d7 100644 --- a/pkg/services/guardian/accesscontrol_guardian_test.go +++ b/pkg/services/guardian/accesscontrol_guardian_test.go @@ -11,26 +11,42 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/infra/localcache" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" + acdb "github.com/grafana/grafana/pkg/services/accesscontrol/database" accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" - dashdb "github.com/grafana/grafana/pkg/services/dashboards/database" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/folder/foldertest" "github.com/grafana/grafana/pkg/services/licensing/licensingtest" "github.com/grafana/grafana/pkg/services/quota/quotatest" "github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest" - "github.com/grafana/grafana/pkg/services/tag/tagimpl" "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" ) +const ( + dashUID = "1" + folderID = 42 + folderUID = "42" + invalidFolderUID = "142" +) + +var ( + folderUIDScope = fmt.Sprintf("folders:uid:%s", folderUID) + invalidFolderUIDScope = fmt.Sprintf("folders:uid:%s", invalidFolderUID) + dashboard = &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: folderID} + fldr = &dashboards.Dashboard{OrgID: orgID, UID: folderUID, IsFolder: true} +) + type accessControlGuardianTestCase struct { desc string - dashUID string + dashboard *dashboards.Dashboard permissions []accesscontrol.Permission viewersCanEdit bool expected bool @@ -39,8 +55,8 @@ type accessControlGuardianTestCase struct { func TestAccessControlDashboardGuardian_CanSave(t *testing.T) { tests := []accessControlGuardianTestCase{ { - desc: "should be able to save with dashboard wildcard scope", - dashUID: "1", + desc: "should be able to save dashboard with dashboard wildcard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsWrite, @@ -50,8 +66,8 @@ func TestAccessControlDashboardGuardian_CanSave(t *testing.T) { expected: true, }, { - desc: "should be able to save with folder wildcard scope", - dashUID: "1", + desc: "should be able to save dashboard with folder wildcard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsWrite, @@ -61,8 +77,8 @@ func TestAccessControlDashboardGuardian_CanSave(t *testing.T) { expected: true, }, { - desc: "should be able to save with dashboard scope", - dashUID: "1", + desc: "should be able to save dashboard with dashboard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsWrite, @@ -72,8 +88,8 @@ func TestAccessControlDashboardGuardian_CanSave(t *testing.T) { expected: true, }, { - desc: "should be able to save with folder scope", - dashUID: "1", + desc: "should be able to save dashboard under root with general folder scope", + dashboard: &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: 0}, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsWrite, @@ -83,8 +99,19 @@ func TestAccessControlDashboardGuardian_CanSave(t *testing.T) { expected: true, }, { - desc: "should not be able to save with incorrect dashboard scope", - dashUID: "1", + desc: "should be able to save dashboard with folder scope", + dashboard: dashboard, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionDashboardsWrite, + Scope: folderUIDScope, + }, + }, + expected: true, + }, + { + desc: "should not be able to save dashboard with incorrect dashboard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsWrite, @@ -94,12 +121,78 @@ func TestAccessControlDashboardGuardian_CanSave(t *testing.T) { expected: false, }, { - desc: "should not be able to save with incorrect folder scope", - dashUID: "1", + desc: "should not be able to save dashboard with incorrect folder scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsWrite, - Scope: "folders:uid:100", + Scope: invalidFolderUIDScope, + }, + }, + expected: false, + }, + { + desc: "should not be able to save folder with folder write and dashboard wildcard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersWrite, + Scope: "dashboards:*", + }, + }, + expected: false, + }, + { + desc: "should be able to save folder with folder write and folder wildcard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersWrite, + Scope: "folders:*", + }, + }, + expected: true, + }, + { + desc: "should not be able to save folder with folder write and dashboard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersWrite, + Scope: "dashboards:uid:1", + }, + }, + expected: false, + }, + { + desc: "should be able to save folder with folder write and folder scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersWrite, + Scope: folderUIDScope, + }, + }, + expected: true, + }, + { + desc: "should not be able to save folder with folder write and incorrect dashboard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersWrite, + Scope: "dashboards:uid:10", + }, + }, + expected: false, + }, + { + desc: "should not be able to save folder with folder write and incorrect folder scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersWrite, + Scope: invalidFolderUID, }, }, expected: false, @@ -108,7 +201,7 @@ func TestAccessControlDashboardGuardian_CanSave(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - guardian, _ := setupAccessControlGuardianTest(t, tt.dashUID, tt.permissions, nil, testDashSvc(t)) + guardian := setupAccessControlGuardianTest(t, tt.dashboard, tt.permissions, nil, nil, nil) can, err := guardian.CanSave() require.NoError(t, err) assert.Equal(t, tt.expected, can) @@ -119,8 +212,8 @@ func TestAccessControlDashboardGuardian_CanSave(t *testing.T) { func TestAccessControlDashboardGuardian_CanEdit(t *testing.T) { tests := []accessControlGuardianTestCase{ { - desc: "should be able to edit with dashboard wildcard scope", - dashUID: "1", + desc: "should be able to edit dashboard with dashboard wildcard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsWrite, @@ -130,8 +223,8 @@ func TestAccessControlDashboardGuardian_CanEdit(t *testing.T) { expected: true, }, { - desc: "should be able to edit with folder wildcard scope", - dashUID: "1", + desc: "should be able to edit dashboard with folder wildcard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsWrite, @@ -141,8 +234,8 @@ func TestAccessControlDashboardGuardian_CanEdit(t *testing.T) { expected: true, }, { - desc: "should be able to edit with dashboard scope", - dashUID: "1", + desc: "should be able to edit dashboard with dashboard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsWrite, @@ -152,8 +245,8 @@ func TestAccessControlDashboardGuardian_CanEdit(t *testing.T) { expected: true, }, { - desc: "should be able to edit with folder scope", - dashUID: "1", + desc: "should be able to edit dashboard under root with general folder scope", + dashboard: &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: 0}, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsWrite, @@ -163,8 +256,19 @@ func TestAccessControlDashboardGuardian_CanEdit(t *testing.T) { expected: true, }, { - desc: "should not be able to edit with incorrect dashboard scope", - dashUID: "1", + desc: "should be able to edit dashboard with folder scope", + dashboard: dashboard, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionDashboardsWrite, + Scope: folderUIDScope, + }, + }, + expected: true, + }, + { + desc: "should not be able to edit dashboard with incorrect dashboard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsWrite, @@ -174,19 +278,19 @@ func TestAccessControlDashboardGuardian_CanEdit(t *testing.T) { expected: false, }, { - desc: "should not be able to edit with incorrect folder scope", - dashUID: "1", + desc: "should not be able to edit dashboard with incorrect folder scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsWrite, - Scope: "folders:uid:10", + Scope: invalidFolderUIDScope, }, }, expected: false, }, { - desc: "should be able to edit with read action when viewer_can_edit is true", - dashUID: "1", + desc: "should be able to edit dashboard with read action when viewer_can_edit is true", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsRead, @@ -196,14 +300,80 @@ func TestAccessControlDashboardGuardian_CanEdit(t *testing.T) { viewersCanEdit: true, expected: true, }, + { + desc: "should not be able to edit folder with folder write and dashboard wildcard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersWrite, + Scope: "dashboards:*", + }, + }, + expected: false, + }, + { + desc: "should be able to edit folder with folder write and folder wildcard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersWrite, + Scope: "folders:*", + }, + }, + expected: true, + }, + { + desc: "should not be able to edit folder with folder write and dashboard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersWrite, + Scope: "dashboards:uid:1", + }, + }, + expected: false, + }, + { + desc: "should be able to edit folder with folder write and folder scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersWrite, + Scope: folderUIDScope, + }, + }, + expected: true, + }, + { + desc: "should not be able to edit folder with folder write and incorrect folder scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersWrite, + Scope: invalidFolderUIDScope, + }, + }, + expected: false, + }, + { + desc: "should be able to edit folder with folder read action when viewer_can_edit is true", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersRead, + Scope: folderUIDScope, + }, + }, + viewersCanEdit: true, + expected: true, + }, } for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { cfg := setting.NewCfg() cfg.ViewersCanEdit = tt.viewersCanEdit - dashSvc := testDashSvc(t) - guardian, _ := setupAccessControlGuardianTest(t, tt.dashUID, tt.permissions, cfg, dashSvc) + guardian := setupAccessControlGuardianTest(t, tt.dashboard, tt.permissions, cfg, nil, nil) can, err := guardian.CanEdit() require.NoError(t, err) @@ -211,11 +381,12 @@ func TestAccessControlDashboardGuardian_CanEdit(t *testing.T) { }) } } + func TestAccessControlDashboardGuardian_CanView(t *testing.T) { tests := []accessControlGuardianTestCase{ { - desc: "should be able to view with dashboard wildcard scope", - dashUID: "1", + desc: "should be able to view dashboard with dashboard wildcard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsRead, @@ -225,8 +396,8 @@ func TestAccessControlDashboardGuardian_CanView(t *testing.T) { expected: true, }, { - desc: "should be able to view with folder wildcard scope", - dashUID: "1", + desc: "should be able to view dashboard with folder wildcard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsRead, @@ -236,8 +407,8 @@ func TestAccessControlDashboardGuardian_CanView(t *testing.T) { expected: true, }, { - desc: "should be able to view with dashboard scope", - dashUID: "1", + desc: "should be able to view dashboard with dashboard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsRead, @@ -247,8 +418,8 @@ func TestAccessControlDashboardGuardian_CanView(t *testing.T) { expected: true, }, { - desc: "should be able to view with folder scope", - dashUID: "1", + desc: "should be able to view dashboard under root with general folder scope", + dashboard: &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: 0}, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsRead, @@ -258,8 +429,19 @@ func TestAccessControlDashboardGuardian_CanView(t *testing.T) { expected: true, }, { - desc: "should not be able to view with incorrect dashboard scope", - dashUID: "1", + desc: "should be able to view dashboard with folder scope", + dashboard: dashboard, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionDashboardsRead, + Scope: folderUIDScope, + }, + }, + expected: true, + }, + { + desc: "should not be able to view dashboard with incorrect dashboard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsRead, @@ -269,12 +451,78 @@ func TestAccessControlDashboardGuardian_CanView(t *testing.T) { expected: false, }, { - desc: "should not be able to view with incorrect folder scope", - dashUID: "1", + desc: "should not be able to view dashboard with incorrect folder scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsRead, - Scope: "folders:uid:10", + Scope: invalidFolderUIDScope, + }, + }, + expected: false, + }, + { + desc: "should not be able to view folder with folders read and dashboard wildcard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersRead, + Scope: "dashboards:*", + }, + }, + expected: false, + }, + { + desc: "should be able to view folder with folders read and folder wildcard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersRead, + Scope: "folders:*", + }, + }, + expected: true, + }, + { + desc: "should not be able to folder view with folders read and dashboard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersRead, + Scope: "dashboards:uid:1", + }, + }, + expected: false, + }, + { + desc: "should be able to view folder with folders read and folder scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersRead, + Scope: folderUIDScope, + }, + }, + expected: true, + }, + { + desc: "should not be able to view folder with folders read incorrect dashboard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersRead, + Scope: "dashboards:uid:10", + }, + }, + expected: false, + }, + { + desc: "should not be able to view folder with folders read and incorrect folder scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersRead, + Scope: invalidFolderUIDScope, }, }, expected: false, @@ -283,7 +531,7 @@ func TestAccessControlDashboardGuardian_CanView(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - guardian, _ := setupAccessControlGuardianTest(t, tt.dashUID, tt.permissions, nil, testDashSvc(t)) + guardian := setupAccessControlGuardianTest(t, tt.dashboard, tt.permissions, nil, nil, nil) can, err := guardian.CanView() require.NoError(t, err) @@ -294,8 +542,8 @@ func TestAccessControlDashboardGuardian_CanView(t *testing.T) { func TestAccessControlDashboardGuardian_CanAdmin(t *testing.T) { tests := []accessControlGuardianTestCase{ { - desc: "should be able to admin with dashboard wildcard scope", - dashUID: "1", + desc: "should be able to admin dashboard with dashboard wildcard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsPermissionsRead, @@ -309,8 +557,8 @@ func TestAccessControlDashboardGuardian_CanAdmin(t *testing.T) { expected: true, }, { - desc: "should be able to admin with folder wildcard scope", - dashUID: "1", + desc: "should be able to admin dashboard with folder wildcard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsPermissionsRead, @@ -324,8 +572,8 @@ func TestAccessControlDashboardGuardian_CanAdmin(t *testing.T) { expected: true, }, { - desc: "should be able to admin with dashboard scope", - dashUID: "1", + desc: "should be able to admin dashboard with dashboard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsPermissionsRead, @@ -339,8 +587,8 @@ func TestAccessControlDashboardGuardian_CanAdmin(t *testing.T) { expected: true, }, { - desc: "should be able to admin with folder scope", - dashUID: "1", + desc: "should be able to admin dashboard under root with general folder scope", + dashboard: &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: 0}, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsPermissionsRead, @@ -354,8 +602,23 @@ func TestAccessControlDashboardGuardian_CanAdmin(t *testing.T) { expected: true, }, { - desc: "should not be able to admin with incorrect dashboard scope", - dashUID: "1", + desc: "should be able to admin dashboard with folder scope", + dashboard: dashboard, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionDashboardsPermissionsRead, + Scope: folderUIDScope, + }, + { + Action: dashboards.ActionDashboardsPermissionsWrite, + Scope: folderUIDScope, + }, + }, + expected: true, + }, + { + desc: "should not be able to admin dashboard with incorrect dashboard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsPermissionsRead, @@ -369,16 +632,150 @@ func TestAccessControlDashboardGuardian_CanAdmin(t *testing.T) { expected: false, }, { - desc: "should not be able to admin with incorrect folder scope", - dashUID: "1", + desc: "should not be able to admin dashboard with incorrect folder scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsPermissionsRead, - Scope: "folders:uid:10", + Scope: invalidFolderUIDScope, }, { Action: dashboards.ActionDashboardsPermissionsWrite, - Scope: "folders:uid:10", + Scope: invalidFolderUIDScope, + }, + }, + expected: false, + }, + { + desc: "should not be able to admin folder with folder read and write and dashboard wildcard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersPermissionsRead, + Scope: "dashboards:*", + }, + { + Action: dashboards.ActionFoldersPermissionsWrite, + Scope: "dashboards:*", + }, + }, + expected: false, + }, + { + desc: "should be able to admin folder with folder read and write and wildcard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersPermissionsRead, + Scope: "folders:*", + }, + { + Action: dashboards.ActionFoldersPermissionsWrite, + Scope: "folders:*", + }, + }, + expected: true, + }, + { + desc: "should not be able to admin folder with folder read and wildcard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersPermissionsRead, + Scope: "folders:*", + }, + }, + expected: false, + }, + { + desc: "should not be able to admin folder with folder write and wildcard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersPermissionsWrite, + Scope: "folders:*", + }, + }, + expected: false, + }, + { + desc: "should not be able to admin folder with folder read and write and dashboard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersPermissionsRead, + Scope: "dashboards:uid:1", + }, + { + Action: dashboards.ActionFoldersPermissionsWrite, + Scope: "dashboards:uid:1", + }, + }, + expected: false, + }, + { + desc: "should be able to admin folder with folder read and write and folder scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersPermissionsRead, + Scope: folderUIDScope, + }, + { + Action: dashboards.ActionFoldersPermissionsWrite, + Scope: folderUIDScope, + }, + }, + expected: true, + }, + { + desc: "should not be able to admin folder with folder read and folder scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersPermissionsRead, + Scope: folderUIDScope, + }, + }, + expected: false, + }, + { + desc: "should not be able to admin folder with folder write and folder scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersPermissionsWrite, + Scope: folderUIDScope, + }, + }, + expected: false, + }, + { + desc: "should not be able to admin folder with folder read and write and incorrect dashboard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersPermissionsRead, + Scope: "dashboards:uid:10", + }, + { + Action: dashboards.ActionFoldersPermissionsWrite, + Scope: "dashboards:uid:10", + }, + }, + expected: false, + }, + { + desc: "should not be able to admin folder with folder read and write and incorrect folder scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersPermissionsRead, + Scope: invalidFolderUIDScope, + }, + { + Action: dashboards.ActionFoldersPermissionsWrite, + Scope: invalidFolderUIDScope, }, }, expected: false, @@ -387,7 +784,7 @@ func TestAccessControlDashboardGuardian_CanAdmin(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - guardian, _ := setupAccessControlGuardianTest(t, tt.dashUID, tt.permissions, nil, testDashSvc(t)) + guardian := setupAccessControlGuardianTest(t, tt.dashboard, tt.permissions, nil, nil, nil) can, err := guardian.CanAdmin() require.NoError(t, err) @@ -395,11 +792,12 @@ func TestAccessControlDashboardGuardian_CanAdmin(t *testing.T) { }) } } + func TestAccessControlDashboardGuardian_CanDelete(t *testing.T) { tests := []accessControlGuardianTestCase{ { - desc: "should be able to delete with dashboard wildcard scope", - dashUID: "1", + desc: "should be able to delete dashboard with dashboard wildcard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsDelete, @@ -409,8 +807,8 @@ func TestAccessControlDashboardGuardian_CanDelete(t *testing.T) { expected: true, }, { - desc: "should be able to delete with folder wildcard scope", - dashUID: "1", + desc: "should be able to delete dashboard with folder wildcard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsDelete, @@ -420,8 +818,8 @@ func TestAccessControlDashboardGuardian_CanDelete(t *testing.T) { expected: true, }, { - desc: "should be able to delete with dashboard scope", - dashUID: "1", + desc: "should be able to delete dashboard with dashboard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsDelete, @@ -431,8 +829,8 @@ func TestAccessControlDashboardGuardian_CanDelete(t *testing.T) { expected: true, }, { - desc: "should be able to delete with folder scope", - dashUID: "1", + desc: "should be able to delete dashboard under root with general folder scope", + dashboard: &dashboards.Dashboard{OrgID: orgID, UID: dashUID, IsFolder: false, FolderID: 0}, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsDelete, @@ -442,8 +840,19 @@ func TestAccessControlDashboardGuardian_CanDelete(t *testing.T) { expected: true, }, { - desc: "should not be able to delete with incorrect dashboard scope", - dashUID: "1", + desc: "should be able to delete dashboard with folder scope", + dashboard: dashboard, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionDashboardsDelete, + Scope: folderUIDScope, + }, + }, + expected: true, + }, + { + desc: "should not be able to delete dashboard with incorrect dashboard scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsDelete, @@ -453,12 +862,78 @@ func TestAccessControlDashboardGuardian_CanDelete(t *testing.T) { expected: false, }, { - desc: "should not be able to delete with incorrect folder scope", - dashUID: "1", + desc: "should not be able to delete dashboard with incorrect folder scope", + dashboard: dashboard, permissions: []accesscontrol.Permission{ { Action: dashboards.ActionDashboardsDelete, - Scope: "folders:uid:10", + Scope: invalidFolderUIDScope, + }, + }, + expected: false, + }, + { + desc: "should not be able to delete folder with folder delete and dashboard wildcard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersDelete, + Scope: "dashboards:*", + }, + }, + expected: false, + }, + { + desc: "should be able to delete folder with folder deletea and folder wildcard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersDelete, + Scope: "folders:*", + }, + }, + expected: true, + }, + { + desc: "should not be able to delete folder with folder delete and dashboard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersDelete, + Scope: "dashboards:uid:1", + }, + }, + expected: false, + }, + { + desc: "should be able to delete folder with folder delete and folder scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersDelete, + Scope: folderUIDScope, + }, + }, + expected: true, + }, + { + desc: "should not be able to delete folder with folder delete and incorrect dashboard scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersDelete, + Scope: "dashboards:uid:10", + }, + }, + expected: false, + }, + { + desc: "should not be able to delete folder with folder delete and incorrect folder scope", + dashboard: fldr, + permissions: []accesscontrol.Permission{ + { + Action: dashboards.ActionFoldersDelete, + Scope: invalidFolderUIDScope, }, }, expected: false, @@ -467,7 +942,7 @@ func TestAccessControlDashboardGuardian_CanDelete(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - guardian, _ := setupAccessControlGuardianTest(t, tt.dashUID, tt.permissions, nil, testDashSvc(t)) + guardian := setupAccessControlGuardianTest(t, tt.dashboard, tt.permissions, nil, nil, nil) can, err := guardian.CanDelete() require.NoError(t, err) @@ -531,7 +1006,7 @@ func TestAccessControlDashboardGuardian_CanCreate(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - guardian, _ := setupAccessControlGuardianTest(t, "0", tt.permissions, nil, nil) + guardian := setupAccessControlGuardianTest(t, &dashboards.Dashboard{OrgID: orgID, UID: "0", IsFolder: tt.isFolder}, tt.permissions, nil, nil, nil) can, err := guardian.CanCreate(tt.folderID, tt.isFolder) require.NoError(t, err) @@ -544,6 +1019,7 @@ type accessControlGuardianGetHiddenACLTestCase struct { desc string permissions []accesscontrol.ResourcePermission hiddenUsers map[string]struct{} + isFolder bool } func TestAccessControlDashboardGuardian_GetHiddenACL(t *testing.T) { @@ -559,16 +1035,27 @@ func TestAccessControlDashboardGuardian_GetHiddenACL(t *testing.T) { }, hiddenUsers: map[string]struct{}{"user2": {}, "user3": {}}, }, + { + desc: "should only return permissions containing hidden users", + permissions: []accesscontrol.ResourcePermission{ + {RoleName: "managed:users:1:permissions", UserId: 1, UserLogin: "user1", IsManaged: true}, + {RoleName: "managed:teams:1:permissions", TeamId: 1, Team: "team1", IsManaged: true}, + {RoleName: "managed:users:2:permissions", UserId: 2, UserLogin: "user2", IsManaged: true}, + {RoleName: "managed:users:3:permissions", UserId: 3, UserLogin: "user3", IsManaged: true}, + {RoleName: "managed:users:4:permissions", UserId: 4, UserLogin: "user4", IsManaged: true}, + }, + hiddenUsers: map[string]struct{}{"user2": {}, "user3": {}}, + isFolder: true, + }, } for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - guardian, _ := setupAccessControlGuardianTest(t, "1", nil, nil, testDashSvc(t)) - mocked := accesscontrolmock.NewMockedPermissionsService() - guardian.dashboardPermissionsService = mocked mocked.On("MapActions", mock.Anything).Return("View") mocked.On("GetPermissions", mock.Anything, mock.Anything, mock.Anything).Return(tt.permissions, nil) + guardian := setupAccessControlGuardianTest(t, &dashboards.Dashboard{OrgID: orgID, UID: "1", IsFolder: tt.isFolder}, nil, nil, mocked, mocked) + cfg := setting.NewCfg() cfg.HiddenUsers = tt.hiddenUsers permissions, err := guardian.GetHiddenACL(cfg) @@ -585,69 +1072,53 @@ func TestAccessControlDashboardGuardian_GetHiddenACL(t *testing.T) { } } -func setupAccessControlGuardianTest(t *testing.T, uid string, +func setupAccessControlGuardianTest(t *testing.T, d *dashboards.Dashboard, permissions []accesscontrol.Permission, cfg *setting.Cfg, - dashboardSvc dashboards.DashboardService) (*AccessControlDashboardGuardian, *dashboards.Dashboard) { + dashboardPermissions accesscontrol.DashboardPermissionsService, folderPermissions accesscontrol.FolderPermissionsService) DashboardGuardian { t.Helper() store := db.InitTestDB(t) - toSave := dashboards.NewDashboard(uid) - toSave.SetUID(uid) + fakeDashboardService := dashboards.NewFakeDashboardService(t) + fakeDashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Maybe().Return(d, nil) - // seed dashboard - quotaService := quotatest.New(false, nil) - dashStore, err := dashdb.ProvideDashboardStore(store, store.Cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(store, store.Cfg), quotaService) - require.NoError(t, err) - dash, err := dashStore.SaveDashboard(context.Background(), dashboards.SaveDashboardCommand{ - Dashboard: toSave.Data, - UserID: 1, - OrgID: 1, - }) - require.NoError(t, err) - if dashboardSvc == nil { - fakeDashboardService := dashboards.NewFakeDashboardService(t) - qResult := &dashboards.Dashboard{} - fakeDashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Run(func(args mock.Arguments) { - q := args.Get(1).(*dashboards.GetDashboardQuery) - qResult = &dashboards.Dashboard{ - ID: q.ID, - UID: q.UID, - OrgID: q.OrgID, - } - }).Return(qResult, nil) - dashboardSvc = fakeDashboardService - } + ac := acimpl.ProvideAccessControl(cfg) + folderSvc := foldertest.NewFakeService() + + folderStore := foldertest.NewFakeFolderStore(t) + folderStore.On("GetFolderByID", mock.Anything, mock.Anything, mock.Anything).Maybe().Return(&folder.Folder{ID: folderID, UID: folderUID, OrgID: orgID}, nil) + + ac.RegisterScopeAttributeResolver(dashboards.NewDashboardUIDScopeResolver(folderStore, fakeDashboardService, folderSvc)) + ac.RegisterScopeAttributeResolver(dashboards.NewFolderUIDScopeResolver(folderSvc)) + ac.RegisterScopeAttributeResolver(dashboards.NewFolderIDScopeResolver(folderStore, folderSvc)) - ac := accesscontrolmock.New().WithPermissions(permissions) - // TODO replace with actual folder store implementation after resolving import cycles - ac.RegisterScopeAttributeResolver(dashboards.NewDashboardUIDScopeResolver(foldertest.NewFakeFolderStore(t), dashboardSvc, foldertest.NewFakeService())) license := licensingtest.NewFakeLicensing() license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe() teamSvc := teamimpl.ProvideService(store, store.Cfg) userSvc, err := userimpl.ProvideService(store, nil, store.Cfg, nil, nil, quotatest.New(false, nil), supportbundlestest.NewFakeBundleService()) require.NoError(t, err) - folderPermissions, err := ossaccesscontrol.ProvideFolderPermissions( - featuremgmt.WithFeatures(), routing.NewRouteRegister(), store, ac, license, &dashboards.FakeDashboardStore{}, foldertest.NewFakeService(), ac, teamSvc, userSvc) - require.NoError(t, err) - dashboardPermissions, err := ossaccesscontrol.ProvideDashboardPermissions( - featuremgmt.WithFeatures(), routing.NewRouteRegister(), store, ac, license, &dashboards.FakeDashboardStore{}, foldertest.NewFakeService(), ac, teamSvc, userSvc) - require.NoError(t, err) + acSvc := acimpl.ProvideOSSService(cfg, acdb.ProvideService(store), localcache.ProvideService(), featuremgmt.WithFeatures()) + if folderPermissions == nil { + folderPermissions, err = ossaccesscontrol.ProvideFolderPermissions( + featuremgmt.WithFeatures(), routing.NewRouteRegister(), store, ac, license, &dashboards.FakeDashboardStore{}, folderSvc, acSvc, teamSvc, userSvc) + require.NoError(t, err) + } + if dashboardPermissions == nil { + dashboardPermissions, err = ossaccesscontrol.ProvideDashboardPermissions( + featuremgmt.WithFeatures(), routing.NewRouteRegister(), store, ac, license, &dashboards.FakeDashboardStore{}, folderSvc, acSvc, teamSvc, userSvc) + require.NoError(t, err) + } - g, err := NewAccessControlDashboardGuardian(context.Background(), cfg, dash.ID, &user.SignedInUser{OrgID: 1}, store, ac, folderPermissions, dashboardPermissions, dashboardSvc) - require.NoError(t, err) - g.dashboard = dash - return g, dash -} + userPermissions := map[int64]map[string][]string{} + for _, p := range permissions { + if _, ok := userPermissions[orgID]; !ok { + userPermissions[orgID] = map[string][]string{} + } + userPermissions[orgID][p.Action] = append(userPermissions[orgID][p.Action], p.Scope) + } -func testDashSvc(t *testing.T) dashboards.DashboardService { - dashSvc := dashboards.NewFakeDashboardService(t) - d := &dashboards.Dashboard{} - dashSvc.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Run(func(args mock.Arguments) { - d := dashboards.NewDashboard("mocked") - d.ID = 1 - d.UID = "1" - }).Return(d, nil) - return dashSvc + g, err := NewAccessControlDashboardGuardianByDashboard(context.Background(), cfg, d, &user.SignedInUser{OrgID: orgID, Permissions: userPermissions}, store, ac, folderPermissions, dashboardPermissions, fakeDashboardService) + require.NoError(t, err) + return g } diff --git a/pkg/services/guardian/guardian.go b/pkg/services/guardian/guardian.go index 614702bebe4..9e810899f61 100644 --- a/pkg/services/guardian/guardian.go +++ b/pkg/services/guardian/guardian.go @@ -7,6 +7,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/team" "github.com/grafana/grafana/pkg/services/user" @@ -18,7 +19,9 @@ var ( ErrGuardianPermissionExists = errors.New("permission already exists") ErrGuardianOverride = errors.New("you can only override a permission to be higher") ErrGuardianGetDashboardFailure = errutil.NewBase(errutil.StatusInternal, "guardian.getDashboardFailure", errutil.WithPublicMessage("Failed to get dashboard")) + ErrGuardianGetFolderFailure = errutil.NewBase(errutil.StatusInternal, "guardian.getFolderFailure", errutil.WithPublicMessage("Failed to get folder")) ErrGuardianDashboardNotFound = errutil.NewBase(errutil.StatusNotFound, "guardian.dashboardNotFound") + ErrGuardianFolderNotFound = errutil.NewBase(errutil.StatusNotFound, "guardian.folderNotFound") ) // DashboardGuardian to be used for guard against operations without access on dashboard and acl @@ -73,6 +76,12 @@ var NewByDashboard = func(ctx context.Context, dash *dashboards.Dashboard, orgId panic("no guardian factory implementation provided") } +// NewByFolder factory for creating a new folder guardian instance +// When using access control this function is replaced on startup and the AccessControlDashboardGuardian is returned +var NewByFolder = func(ctx context.Context, f *folder.Folder, orgId int64, user *user.SignedInUser) (DashboardGuardian, error) { + panic("no guardian factory implementation provided") +} + // newDashboardGuardian creates a dashboard guardian by the provided dashId. func newDashboardGuardian(ctx context.Context, cfg *setting.Cfg, dashId int64, orgId int64, user *user.SignedInUser, store db.DB, dashSvc dashboards.DashboardService, teamSvc team.Service) (*dashboardGuardianImpl, error) { if dashId != 0 { @@ -151,6 +160,24 @@ func newDashboardGuardianByDashboard(ctx context.Context, cfg *setting.Cfg, dash }, nil } +// newDashboardGuardianByFolder creates a dashboard guardian by the provided folder. +// This constructor should be preferred over the other two if the dashboard in available +// since it avoids querying the database for fetching the dashboard. +// The folder.ID should be the sequence ID in the dashboard table. +func newDashboardGuardianByFolder(ctx context.Context, cfg *setting.Cfg, f *folder.Folder, orgId int64, user *user.SignedInUser, store db.DB, dashSvc dashboards.DashboardService, teamSvc team.Service) (*dashboardGuardianImpl, error) { + return &dashboardGuardianImpl{ + cfg: cfg, + user: user, + dashId: f.ID, + orgId: orgId, + log: log.New("dashboard.permissions"), + ctx: ctx, + store: store, + dashboardService: dashSvc, + teamService: teamSvc, + }, nil +} + func (g *dashboardGuardianImpl) CanSave() (bool, error) { return g.HasPermission(dashboards.PERMISSION_EDIT) } @@ -482,4 +509,12 @@ func MockDashboardGuardian(mock *FakeDashboardGuardian) { mock.User = user return mock, nil } + + NewByFolder = func(_ context.Context, f *folder.Folder, orgId int64, user *user.SignedInUser) (DashboardGuardian, error) { + mock.OrgID = orgId + mock.DashUID = f.UID + mock.DashID = f.ID + mock.User = user + return mock, nil + } } diff --git a/pkg/services/guardian/provider.go b/pkg/services/guardian/provider.go index 454db8d39ae..07b9be6e49d 100644 --- a/pkg/services/guardian/provider.go +++ b/pkg/services/guardian/provider.go @@ -6,6 +6,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" + "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/team" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" @@ -39,6 +40,10 @@ func InitLegacyGuardian(cfg *setting.Cfg, store db.DB, dashSvc dashboards.Dashbo NewByDashboard = func(ctx context.Context, dash *dashboards.Dashboard, orgId int64, user *user.SignedInUser) (DashboardGuardian, error) { return newDashboardGuardianByDashboard(ctx, cfg, dash, orgId, user, store, dashSvc, teamSvc) } + + NewByFolder = func(ctx context.Context, f *folder.Folder, orgId int64, user *user.SignedInUser) (DashboardGuardian, error) { + return newDashboardGuardianByFolder(ctx, cfg, f, orgId, user, store, dashSvc, teamSvc) + } } func InitAccessControlGuardian( @@ -56,4 +61,8 @@ func InitAccessControlGuardian( NewByDashboard = func(ctx context.Context, dash *dashboards.Dashboard, orgId int64, user *user.SignedInUser) (DashboardGuardian, error) { return NewAccessControlDashboardGuardianByDashboard(ctx, cfg, dash, user, store, ac, folderPermissionsService, dashboardPermissionsService, dashboardService) } + + NewByFolder = func(ctx context.Context, f *folder.Folder, orgId int64, user *user.SignedInUser) (DashboardGuardian, error) { + return NewAccessControlFolderGuardian(ctx, cfg, f, user, store, ac, folderPermissionsService, dashboardPermissionsService, dashboardService) + } }