Access control: use uid for dashboard and folder scopes (#46807)

* use uid:s for folder and dashboard permissions

* evaluate folder and dashboard permissions based on uids

* add dashboard.uid to accept list

* Check for exact suffix

* Check parent folder on create

* update test

* drop dashboard:create actions with dashboard scope

* fix typo

* AccessControl: test id 0 scope conversion

* AccessControl: store only parent folder UID

* AccessControl: extract general as a constant

* FolderServices: Prevent creation of a folder uid'd general

* FolderServices: Test folder creation prevention

* Update pkg/services/guardian/accesscontrol_guardian.go

* FolderServices: fix mock call expect

* FolderServices: remove uneeded mocks

Co-authored-by: jguer <joao.guerreiro@grafana.com>
This commit is contained in:
Karl Persson
2022-03-30 15:14:26 +02:00
committed by GitHub
parent 56e9c24f08
commit a5e4a533fa
21 changed files with 369 additions and 256 deletions

View File

@@ -9,6 +9,9 @@ import (
)
const (
ScopeFoldersRoot = "folders"
ScopeFoldersPrefix = "folders:uid:"
ActionFoldersCreate = "folders:create"
ActionFoldersRead = "folders:read"
ActionFoldersWrite = "folders:write"
@@ -16,7 +19,8 @@ const (
ActionFoldersPermissionsRead = "folders.permissions:read"
ActionFoldersPermissionsWrite = "folders.permissions:write"
ScopeFoldersRoot = "folders"
ScopeDashboardsRoot = "dashboards"
ScopeDashboardsPrefix = "dashboards:uid:"
)
var (
@@ -24,7 +28,7 @@ var (
ScopeFoldersProvider = ac.NewScopeProvider(ScopeFoldersRoot)
)
// NewNameScopeResolver provides an AttributeScopeResolver that is able to convert a scope prefixed with "folders:name:" into an id based scope.
// NewNameScopeResolver provides an AttributeScopeResolver that is able to convert a scope prefixed with "folders:name:" into an uid based scope.
func NewNameScopeResolver(db Store) (string, ac.AttributeScopeResolveFunc) {
prefix := ScopeFoldersProvider.GetResourceScopeName("")
resolver := func(ctx context.Context, orgID int64, scope string) (string, error) {
@@ -39,27 +43,34 @@ func NewNameScopeResolver(db Store) (string, ac.AttributeScopeResolveFunc) {
if err != nil {
return "", err
}
return ScopeFoldersProvider.GetResourceScope(strconv.FormatInt(folder.Id, 10)), nil
return ScopeFoldersProvider.GetResourceScopeUID(folder.Uid), nil
}
return prefix, resolver
}
// NewUidScopeResolver provides an AttributeScopeResolver that is able to convert a scope prefixed with "folders:uid:" into an id based scope.
func NewUidScopeResolver(db Store) (string, ac.AttributeScopeResolveFunc) {
prefix := ScopeFoldersProvider.GetResourceScopeUID("")
// NewIDScopeResolver provides an AttributeScopeResolver that is able to convert a scope prefixed with "folders:id:" into an uid based scope.
func NewIDScopeResolver(db Store) (string, ac.AttributeScopeResolveFunc) {
prefix := ScopeFoldersProvider.GetResourceScope("")
resolver := func(ctx context.Context, orgID int64, scope string) (string, error) {
if !strings.HasPrefix(scope, prefix) {
return "", ac.ErrInvalidScope
}
uid := scope[len(prefix):]
if len(uid) == 0 {
id, err := strconv.ParseInt(scope[len(prefix):], 10, 64)
if err != nil {
return "", ac.ErrInvalidScope
}
folder, err := db.GetFolderByUID(ctx, orgID, uid)
if id == 0 {
return ScopeFoldersProvider.GetResourceScopeUID(ac.GeneralFolderUID), nil
}
folder, err := db.GetFolderByID(ctx, orgID, id)
if err != nil {
return "", err
}
return ScopeFoldersProvider.GetResourceScope(strconv.FormatInt(folder.Id, 10)), nil
return ScopeFoldersProvider.GetResourceScopeUID(folder.Uid), nil
}
return prefix, resolver
}

View File

@@ -4,6 +4,7 @@ import (
"context"
"fmt"
"math/rand"
"strconv"
"testing"
"github.com/stretchr/testify/mock"
@@ -20,7 +21,7 @@ func TestNewNameScopeResolver(t *testing.T) {
require.Equal(t, "folders:name:", prefix)
})
t.Run("resolver should convert to id scope", func(t *testing.T) {
t.Run("resolver should convert to uid scope", func(t *testing.T) {
dashboardStore := &FakeDashboardStore{}
_, resolver := NewNameScopeResolver(dashboardStore)
@@ -30,6 +31,7 @@ func TestNewNameScopeResolver(t *testing.T) {
db := models.NewFolder(title)
db.Id = rand.Int63()
db.Uid = util.GenerateShortUID()
dashboardStore.On("GetFolderByTitle", mock.Anything, mock.Anything, mock.Anything).Return(db, nil).Once()
scope := "folders:name:" + title
@@ -37,7 +39,7 @@ func TestNewNameScopeResolver(t *testing.T) {
resolvedScope, err := resolver(context.Background(), orgId, scope)
require.NoError(t, err)
require.Equal(t, fmt.Sprintf("folders:id:%v", db.Id), resolvedScope)
require.Equal(t, fmt.Sprintf("folders:uid:%v", db.Uid), resolvedScope)
dashboardStore.AssertCalled(t, "GetFolderByTitle", mock.Anything, orgId, title)
})
@@ -71,56 +73,70 @@ func TestNewNameScopeResolver(t *testing.T) {
})
}
func TestNewUidScopeResolver(t *testing.T) {
func TestNewIDScopeResolver(t *testing.T) {
t.Run("prefix should be expected", func(t *testing.T) {
prefix, _ := NewUidScopeResolver(&FakeDashboardStore{})
require.Equal(t, "folders:uid:", prefix)
prefix, _ := NewIDScopeResolver(&FakeDashboardStore{})
require.Equal(t, "folders:id:", prefix)
})
t.Run("resolver should convert to id scope", func(t *testing.T) {
t.Run("resolver should convert to uid scope", func(t *testing.T) {
dashboardStore := &FakeDashboardStore{}
_, resolver := NewUidScopeResolver(dashboardStore)
_, resolver := NewIDScopeResolver(dashboardStore)
orgId := rand.Int63()
uid := util.GenerateShortUID()
db := &models.Folder{Id: rand.Int63()}
dashboardStore.On("GetFolderByUID", mock.Anything, mock.Anything, mock.Anything).Return(db, nil).Once()
db := &models.Folder{Id: rand.Int63(), Uid: uid}
dashboardStore.On("GetFolderByID", mock.Anything, mock.Anything, mock.Anything).Return(db, nil).Once()
scope := "folders:uid:" + uid
scope := "folders:id:" + strconv.FormatInt(db.Id, 10)
resolvedScope, err := resolver(context.Background(), orgId, scope)
require.NoError(t, err)
require.Equal(t, fmt.Sprintf("folders:id:%v", db.Id), resolvedScope)
require.Equal(t, fmt.Sprintf("folders:uid:%v", db.Uid), resolvedScope)
dashboardStore.AssertCalled(t, "GetFolderByUID", mock.Anything, orgId, uid)
dashboardStore.AssertCalled(t, "GetFolderByID", mock.Anything, orgId, db.Id)
})
t.Run("resolver should fail if input scope is not expected", func(t *testing.T) {
dashboardStore := &FakeDashboardStore{}
_, resolver := NewUidScopeResolver(dashboardStore)
_, resolver := NewIDScopeResolver(dashboardStore)
_, err := resolver(context.Background(), rand.Int63(), "folders:id:123")
_, err := resolver(context.Background(), rand.Int63(), "folders:uid:123")
require.ErrorIs(t, err, ac.ErrInvalidScope)
})
t.Run("resolver should convert id 0 to general uid scope", func(t *testing.T) {
var (
dashboardStore = &FakeDashboardStore{}
orgId = rand.Int63()
scope = "folders:id:0"
_, resolver = NewIDScopeResolver(dashboardStore)
)
resolvedScope, err := resolver(context.Background(), orgId, scope)
require.NoError(t, err)
require.Equal(t, "folders:uid:general", resolvedScope)
})
t.Run("resolver should fail if resource of input scope is empty", func(t *testing.T) {
dashboardStore := &FakeDashboardStore{}
_, resolver := NewUidScopeResolver(dashboardStore)
_, resolver := NewIDScopeResolver(dashboardStore)
_, err := resolver(context.Background(), rand.Int63(), "folders:uid:")
_, err := resolver(context.Background(), rand.Int63(), "folders:id:")
require.ErrorIs(t, err, ac.ErrInvalidScope)
})
t.Run("returns 'not found' if folder does not exist", func(t *testing.T) {
dashboardStore := &FakeDashboardStore{}
_, resolver := NewUidScopeResolver(dashboardStore)
_, resolver := NewIDScopeResolver(dashboardStore)
orgId := rand.Int63()
dashboardStore.On("GetFolderByUID", mock.Anything, mock.Anything, mock.Anything).Return(nil, models.ErrDashboardNotFound).Once()
scope := "folders:uid:" + util.GenerateShortUID()
dashboardStore.On("GetFolderByID", mock.Anything, mock.Anything, mock.Anything).Return(nil, models.ErrDashboardNotFound).Once()
scope := "folders:id:10"
resolvedScope, err := resolver(context.Background(), orgId, scope)
require.ErrorIs(t, err, models.ErrDashboardNotFound)
require.Empty(t, resolvedScope)

View File

@@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"
"strconv"
"time"
"github.com/grafana/grafana/pkg/infra/log"
@@ -488,8 +487,7 @@ func saveDashboard(sess *sqlstore.DBSession, cmd *models.SaveDashboardCommand) e
}
// delete existing tags
_, err = sess.Exec("DELETE FROM dashboard_tag WHERE dashboard_id=?", dash.Id)
if err != nil {
if _, err = sess.Exec("DELETE FROM dashboard_tag WHERE dashboard_id=?", dash.Id); err != nil {
return err
}
@@ -724,9 +722,10 @@ func (d *DashboardStore) deleteDashboard(cmd *models.DeleteDashboardCommand, ses
deletes = append(deletes, "DELETE FROM dashboard WHERE folder_id = ?")
var dashIds []struct {
Id int64
Id int64
Uid string
}
err := sess.SQL("SELECT id FROM dashboard WHERE folder_id = ?", dashboard.Id).Find(&dashIds)
err := sess.SQL("SELECT id, uid FROM dashboard WHERE folder_id = ?", dashboard.Id).Find(&dashIds)
if err != nil {
return err
}
@@ -738,14 +737,14 @@ func (d *DashboardStore) deleteDashboard(cmd *models.DeleteDashboardCommand, ses
}
// remove all access control permission with folder scope
_, err = sess.Exec("DELETE FROM permission WHERE scope = ?", dashboards.ScopeFoldersProvider.GetResourceScope(strconv.FormatInt(dashboard.Id, 10)))
_, err = sess.Exec("DELETE FROM permission WHERE scope = ?", dashboards.ScopeFoldersProvider.GetResourceScopeUID(dashboard.Uid))
if err != nil {
return err
}
for _, dash := range dashIds {
// remove all access control permission with child dashboard scopes
_, err = sess.Exec("DELETE FROM permission WHERE scope = ?", ac.Scope("dashboards", "id", strconv.FormatInt(dash.Id, 10)))
_, err = sess.Exec("DELETE FROM permission WHERE scope = ?", ac.GetResourceScopeUID("dashboards", dash.Uid))
if err != nil {
return err
}
@@ -792,7 +791,7 @@ func (d *DashboardStore) deleteDashboard(cmd *models.DeleteDashboardCommand, ses
}
}
} else {
_, err = sess.Exec("DELETE FROM permission WHERE scope = ?", ac.Scope("dashboards", "id", strconv.FormatInt(dashboard.Id, 10)))
_, err = sess.Exec("DELETE FROM permission WHERE scope = ?", ac.GetResourceScopeUID("dashboards", dashboard.Uid))
if err != nil {
return err
}

View File

@@ -3,7 +3,6 @@ package service
import (
"context"
"fmt"
"strconv"
"strings"
"time"
@@ -449,7 +448,6 @@ func (dr *DashboardServiceImpl) GetDashboardsByPluginID(ctx context.Context, que
func (dr *DashboardServiceImpl) setDefaultPermissions(ctx context.Context, dto *m.SaveDashboardDTO, dash *models.Dashboard, provisioned bool) error {
inFolder := dash.FolderId > 0
if dr.features.IsEnabled(featuremgmt.FlagAccesscontrol) {
resourceID := strconv.FormatInt(dash.Id, 10)
var permissions []accesscontrol.SetResourcePermissionCommand
if !provisioned {
permissions = append(permissions, accesscontrol.SetResourcePermissionCommand{
@@ -469,7 +467,7 @@ func (dr *DashboardServiceImpl) setDefaultPermissions(ctx context.Context, dto *
svc = dr.folderPermissions
}
_, err := svc.SetPermissions(ctx, dto.OrgId, resourceID, permissions...)
_, err := svc.SetPermissions(ctx, dto.OrgId, dash.Uid, permissions...)
if err != nil {
return err
}

View File

@@ -3,7 +3,6 @@ package service
import (
"context"
"errors"
"strconv"
"strings"
"github.com/grafana/grafana/pkg/infra/log"
@@ -34,7 +33,7 @@ func ProvideFolderService(
ac accesscontrol.AccessControl, sqlStore sqlstore.Store,
) *FolderServiceImpl {
ac.RegisterAttributeScopeResolver(dashboards.NewNameScopeResolver(dashboardStore))
ac.RegisterAttributeScopeResolver(dashboards.NewUidScopeResolver(dashboardStore))
ac.RegisterAttributeScopeResolver(dashboards.NewIDScopeResolver(dashboardStore))
return &FolderServiceImpl{
cfg: cfg,
@@ -134,7 +133,13 @@ func (f *FolderServiceImpl) GetFolderByTitle(ctx context.Context, user *models.S
func (f *FolderServiceImpl) CreateFolder(ctx context.Context, user *models.SignedInUser, orgID int64, title, uid string) (*models.Folder, error) {
dashFolder := models.NewDashboardFolder(title)
dashFolder.OrgId = orgID
dashFolder.SetUid(strings.TrimSpace(uid))
trimmedUID := strings.TrimSpace(uid)
if trimmedUID == accesscontrol.GeneralFolderUID {
return nil, models.ErrFolderInvalidUID
}
dashFolder.SetUid(trimmedUID)
userID := user.UserId
if userID == 0 {
userID = -1
@@ -167,8 +172,7 @@ func (f *FolderServiceImpl) CreateFolder(ctx context.Context, user *models.Signe
var permissionErr error
if f.features.IsEnabled(featuremgmt.FlagAccesscontrol) {
resourceID := strconv.FormatInt(folder.Id, 10)
_, permissionErr = f.permissions.SetPermissions(ctx, orgID, resourceID, []accesscontrol.SetResourcePermissionCommand{
_, permissionErr = f.permissions.SetPermissions(ctx, orgID, folder.Uid, []accesscontrol.SetResourcePermissionCommand{
{UserID: userID, Permission: models.PERMISSION_ADMIN.String()},
{BuiltinRole: string(models.ROLE_EDITOR), Permission: models.PERMISSION_EDIT.String()},
{BuiltinRole: string(models.ROLE_VIEWER), Permission: models.PERMISSION_VIEW.String()},

View File

@@ -138,6 +138,14 @@ func TestFolderService(t *testing.T) {
require.Equal(t, f, actualFolder)
})
t.Run("When creating folder should return error if uid is general", func(t *testing.T) {
dash := models.NewDashboardFolder("Test-Folder")
dash.Id = rand.Int63()
_, err := service.CreateFolder(context.Background(), user, orgID, dash.Title, "general")
require.ErrorIs(t, err, models.ErrFolderInvalidUID)
})
t.Run("When updating folder should not return access denied error", func(t *testing.T) {
dashboardFolder := models.NewDashboardFolder("Folder")
dashboardFolder.Id = rand.Int63()