From b115e73edebbd998244e24f11e2cd73a46ebf7de Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Mon, 13 Jun 2022 14:26:17 +0100 Subject: [PATCH] LibraryPanels: Require only viewer permissions to use a Library Panel (#50241) * rename function to requireEditPermissionsOnFolder * Require only viewer permissions on a folder when connecting a library panel from it * update tests * require edit permissions on the dashboard * revert my change to the tests - these tests test something different * revert changes to a test file??? --- pkg/services/libraryelements/database.go | 16 +++++--- pkg/services/libraryelements/guard.go | 39 ++++++++++++++++++- .../libraryelements_permissions_test.go | 2 + 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/pkg/services/libraryelements/database.go b/pkg/services/libraryelements/database.go index 9b4470224c6..ed0f785b2ee 100644 --- a/pkg/services/libraryelements/database.go +++ b/pkg/services/libraryelements/database.go @@ -125,7 +125,7 @@ func (l *LibraryElementService) createLibraryElement(c context.Context, signedIn } err := l.SQLStore.WithTransactionalDbSession(c, func(session *sqlstore.DBSession) error { - if err := l.requirePermissionsOnFolder(c, signedInUser, cmd.FolderID); err != nil { + if err := l.requireEditPermissionsOnFolder(c, signedInUser, cmd.FolderID); err != nil { return err } if _, err := session.Insert(&element); err != nil { @@ -176,7 +176,7 @@ func (l *LibraryElementService) deleteLibraryElement(c context.Context, signedIn if err != nil { return err } - if err := l.requirePermissionsOnFolder(c, signedInUser, element.FolderID); err != nil { + if err := l.requireEditPermissionsOnFolder(c, signedInUser, element.FolderID); err != nil { return err } var connectionIDs []struct { @@ -422,13 +422,13 @@ func (l *LibraryElementService) handleFolderIDPatches(ctx context.Context, eleme // FolderID was provided in the PATCH request if toFolderID != -1 && toFolderID != fromFolderID { - if err := l.requirePermissionsOnFolder(ctx, user, toFolderID); err != nil { + if err := l.requireEditPermissionsOnFolder(ctx, user, toFolderID); err != nil { return err } } // Always check permissions for the folder where library element resides - if err := l.requirePermissionsOnFolder(ctx, user, fromFolderID); err != nil { + if err := l.requireEditPermissionsOnFolder(ctx, user, fromFolderID); err != nil { return err } @@ -638,6 +638,10 @@ func (l *LibraryElementService) getElementsForDashboardID(c context.Context, das // connectElementsToDashboardID adds connections for all elements Library Elements in a Dashboard. func (l *LibraryElementService) connectElementsToDashboardID(c context.Context, signedInUser *models.SignedInUser, elementUIDs []string, dashboardID int64) error { + if err := l.requireEditPermissionsOnDashboard(c, signedInUser, dashboardID); err != nil { + return err + } + err := l.SQLStore.WithTransactionalDbSession(c, func(session *sqlstore.DBSession) error { _, err := session.Exec("DELETE FROM "+models.LibraryElementConnectionTableName+" WHERE kind=1 AND connection_id=?", dashboardID) if err != nil { @@ -648,7 +652,7 @@ func (l *LibraryElementService) connectElementsToDashboardID(c context.Context, if err != nil { return err } - if err := l.requirePermissionsOnFolder(c, signedInUser, element.FolderID); err != nil { + if err := l.requireViewPermissionsOnFolder(c, signedInUser, element.FolderID); err != nil { return err } @@ -704,7 +708,7 @@ func (l *LibraryElementService) deleteLibraryElementsInFolderUID(c context.Conte folderID := folderUIDs[0].ID - if err := l.requirePermissionsOnFolder(c, signedInUser, folderID); err != nil { + if err := l.requireEditPermissionsOnFolder(c, signedInUser, folderID); err != nil { return err } var connectionIDs []struct { diff --git a/pkg/services/libraryelements/guard.go b/pkg/services/libraryelements/guard.go index 7265e3acc3d..3f7d776f6f7 100644 --- a/pkg/services/libraryelements/guard.go +++ b/pkg/services/libraryelements/guard.go @@ -23,7 +23,7 @@ func (l *LibraryElementService) requireSupportedElementKind(kindAsInt int64) err } } -func (l *LibraryElementService) requirePermissionsOnFolder(ctx context.Context, user *models.SignedInUser, folderID int64) error { +func (l *LibraryElementService) requireEditPermissionsOnFolder(ctx context.Context, user *models.SignedInUser, folderID int64) error { if isGeneralFolder(folderID) && user.HasRole(models.ROLE_EDITOR) { return nil } @@ -48,3 +48,40 @@ func (l *LibraryElementService) requirePermissionsOnFolder(ctx context.Context, return nil } + +func (l *LibraryElementService) requireViewPermissionsOnFolder(ctx context.Context, user *models.SignedInUser, folderID int64) error { + if isGeneralFolder(folderID) && user.HasRole(models.ROLE_VIEWER) { + return nil + } + + folder, err := l.folderService.GetFolderByID(ctx, user, folderID, user.OrgId) + if err != nil { + return err + } + + g := guardian.New(ctx, folder.Id, user.OrgId, user) + + canView, err := g.CanView() + if err != nil { + return err + } + if !canView { + return models.ErrFolderAccessDenied + } + + return nil +} + +func (l *LibraryElementService) requireEditPermissionsOnDashboard(ctx context.Context, user *models.SignedInUser, dashboardID int64) error { + g := guardian.New(ctx, dashboardID, user.OrgId, user) + + canEdit, err := g.CanEdit() + if err != nil { + return err + } + if !canEdit { + return models.ErrDashboardUpdateAccessDenied + } + + return nil +} diff --git a/pkg/services/libraryelements/libraryelements_permissions_test.go b/pkg/services/libraryelements/libraryelements_permissions_test.go index 362b98cd093..6375cf8a78f 100644 --- a/pkg/services/libraryelements/libraryelements_permissions_test.go +++ b/pkg/services/libraryelements/libraryelements_permissions_test.go @@ -48,6 +48,7 @@ func TestLibraryElementPermissions(t *testing.T) { {models.ROLE_ADMIN, viewerOnlyPermissions, viewerOnlyDesc, 200}, {models.ROLE_ADMIN, everyonePermissions, everyoneDesc, 200}, {models.ROLE_ADMIN, noPermissions, noDesc, 200}, + {models.ROLE_EDITOR, defaultPermissions, defaultDesc, 200}, {models.ROLE_EDITOR, adminOnlyPermissions, adminOnlyDesc, 403}, {models.ROLE_EDITOR, editorOnlyPermissions, editorOnlyDesc, 200}, @@ -55,6 +56,7 @@ func TestLibraryElementPermissions(t *testing.T) { {models.ROLE_EDITOR, viewerOnlyPermissions, viewerOnlyDesc, 403}, {models.ROLE_EDITOR, everyonePermissions, everyoneDesc, 200}, {models.ROLE_EDITOR, noPermissions, noDesc, 403}, + {models.ROLE_VIEWER, defaultPermissions, defaultDesc, 403}, {models.ROLE_VIEWER, adminOnlyPermissions, adminOnlyDesc, 403}, {models.ROLE_VIEWER, editorOnlyPermissions, editorOnlyDesc, 403},