From 4bc6a7c4073331e4b363efc538f5b9463bbbf8d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Tue, 2 Mar 2021 10:34:01 +0100 Subject: [PATCH] LibraryPanels: Deletes library panels during folder deletion (#31572) * Refactor: adds permissions for library panel creation * Refactor: checks folder permissions for patch requests * Chore: changes after PR comments * Refactor: adds permissions to delete * Refactor: moves get all permission tests out of get all tests * Chore: move out get all tests to a separate file * Refactor: adds permissions to get handler * Refactor: fixes a bug with getting library panels in General folder * Refactor: adds permissions for connect/disconnect * Refactor: adds permissions and tests for get connected dashboards * Tests: adds tests for connected dashboards in General Folder * LibraryPanels: Deletes library panels during folder deletion * LibraryPanels: Deletes library panels during folder deletion * Update pkg/api/folder.go Co-authored-by: Arve Knudsen * Update pkg/services/librarypanels/librarypanels_permissions_test.go Co-authored-by: Arve Knudsen * Chore: updates after PR comments * Chore: forgot to change some function signatures Co-authored-by: Arve Knudsen --- pkg/api/api.go | 2 +- pkg/api/folder.go | 14 ++++- pkg/services/librarypanels/database.go | 53 +++++++++++++++++++ pkg/services/librarypanels/librarypanels.go | 7 +++ .../librarypanels_permissions_test.go | 19 +++++++ .../librarypanels/librarypanels_test.go | 32 +++++++++++ pkg/services/librarypanels/models.go | 2 + 7 files changed, 127 insertions(+), 2 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index 62fd774e848..f15a50701f7 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -294,7 +294,7 @@ func (hs *HTTPServer) registerRoutes() { folderRoute.Group("/:uid", func(folderUidRoute routing.RouteRegister) { folderUidRoute.Get("/", routing.Wrap(GetFolderByUID)) folderUidRoute.Put("/", bind(models.UpdateFolderCommand{}), routing.Wrap(UpdateFolder)) - folderUidRoute.Delete("/", routing.Wrap(DeleteFolder)) + folderUidRoute.Delete("/", routing.Wrap(hs.DeleteFolder)) folderUidRoute.Group("/permissions", func(folderPermissionRoute routing.RouteRegister) { folderPermissionRoute.Get("/", routing.Wrap(hs.GetFolderPermissionList)) diff --git a/pkg/api/folder.go b/pkg/api/folder.go index bef99ea7a0a..644ba2ef3c8 100644 --- a/pkg/api/folder.go +++ b/pkg/api/folder.go @@ -4,6 +4,8 @@ import ( "errors" "fmt" + "github.com/grafana/grafana/pkg/services/librarypanels" + "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/models" @@ -84,8 +86,18 @@ func UpdateFolder(c *models.ReqContext, cmd models.UpdateFolderCommand) response return response.JSON(200, toFolderDto(g, cmd.Result)) } -func DeleteFolder(c *models.ReqContext) response.Response { +func (hs *HTTPServer) DeleteFolder(c *models.ReqContext) response.Response { // temporarily adding this function to HTTPServer, will be removed from HTTPServer when librarypanels featuretoggle is removed s := dashboards.NewFolderService(c.OrgId, c.SignedInUser) + if hs.Cfg.IsPanelLibraryEnabled() { + err := hs.LibraryPanelService.DeleteLibraryPanelsInFolder(c, c.Params(":uid")) + if err != nil { + if errors.Is(err, librarypanels.ErrFolderHasConnectedLibraryPanels) { + return response.Error(403, "Folder could not be deleted because it contains linked library panels", err) + } + return toFolderError(err) + } + } + f, err := s.DeleteFolder(c.Params(":uid")) if err != nil { return toFolderError(err) diff --git a/pkg/services/librarypanels/database.go b/pkg/services/librarypanels/database.go index 4448fe45181..a461ae605dc 100644 --- a/pkg/services/librarypanels/database.go +++ b/pkg/services/librarypanels/database.go @@ -231,6 +231,59 @@ func (lps *LibraryPanelService) disconnectLibraryPanelsForDashboard(c *models.Re }) } +// deleteLibraryPanelsInFolder deletes all Library Panels for a folder. +func (lps *LibraryPanelService) deleteLibraryPanelsInFolder(c *models.ReqContext, folderUID string) error { + return lps.SQLStore.WithTransactionalDbSession(c.Context.Req.Context(), func(session *sqlstore.DBSession) error { + var folderUIDs []struct { + ID int64 `xorm:"id"` + } + err := session.SQL("SELECT id from dashboard WHERE uid=? AND org_id=? AND is_folder=1", folderUID, c.SignedInUser.OrgId).Find(&folderUIDs) + if err != nil { + return err + } + if len(folderUIDs) != 1 { + return fmt.Errorf("found %d folders, while expecting at most one", len(folderUIDs)) + } + folderID := folderUIDs[0].ID + + if err := requirePermissionsOnFolder(c.SignedInUser, folderID); err != nil { + return err + } + var dashIDs []struct { + DashboardID int64 `xorm:"dashboard_id"` + } + sql := "SELECT lpd.dashboard_id FROM library_panel AS lp" + sql += " INNER JOIN library_panel_dashboard lpd on lp.id = lpd.librarypanel_id" + sql += " WHERE lp.folder_id=? AND lp.org_id=?" + err = session.SQL(sql, folderID, c.SignedInUser.OrgId).Find(&dashIDs) + if err != nil { + return err + } + if len(dashIDs) > 0 { + return ErrFolderHasConnectedLibraryPanels + } + + var panelIDs []struct { + ID int64 `xorm:"id"` + } + err = session.SQL("SELECT id from library_panel WHERE folder_id=? AND org_id=?", folderID, c.SignedInUser.OrgId).Find(&panelIDs) + if err != nil { + return err + } + for _, panelID := range panelIDs { + _, err := session.Exec("DELETE FROM library_panel_dashboard WHERE librarypanel_id=?", panelID.ID) + if err != nil { + return err + } + } + if _, err := session.Exec("DELETE FROM library_panel WHERE folder_id=? AND org_id=?", folderID, c.SignedInUser.OrgId); err != nil { + return err + } + + return nil + }) +} + func getLibraryPanel(session *sqlstore.DBSession, uid string, orgID int64) (LibraryPanelWithMeta, error) { libraryPanels := make([]LibraryPanelWithMeta, 0) sql := sqlStatmentLibrayPanelDTOWithMeta + "WHERE lp.uid=? AND lp.org_id=?" diff --git a/pkg/services/librarypanels/librarypanels.go b/pkg/services/librarypanels/librarypanels.go index 62bcf6e7655..105bda9dc5f 100644 --- a/pkg/services/librarypanels/librarypanels.go +++ b/pkg/services/librarypanels/librarypanels.go @@ -219,6 +219,13 @@ func (lps *LibraryPanelService) DisconnectLibraryPanelsForDashboard(c *models.Re return lps.disconnectLibraryPanelsForDashboard(c, dash.Id, panelCount) } +func (lps *LibraryPanelService) DeleteLibraryPanelsInFolder(c *models.ReqContext, folderUID string) error { + if !lps.IsEnabled() { + return nil + } + return lps.deleteLibraryPanelsInFolder(c, folderUID) +} + // AddMigration defines database migrations. // If Panel Library is not enabled does nothing. func (lps *LibraryPanelService) AddMigration(mg *migrator.Migrator) { diff --git a/pkg/services/librarypanels/librarypanels_permissions_test.go b/pkg/services/librarypanels/librarypanels_permissions_test.go index 4906c4cb0e2..3eaa8184bad 100644 --- a/pkg/services/librarypanels/librarypanels_permissions_test.go +++ b/pkg/services/librarypanels/librarypanels_permissions_test.go @@ -149,6 +149,25 @@ func TestLibraryPanelPermissions(t *testing.T) { resp = sc.service.disconnectHandler(sc.reqContext) require.Equal(t, testCase.status, resp.Status()) }) + + testScenario(t, fmt.Sprintf("When %s tries to delete all library panels in a folder with %s, it should return correct status", testCase.role, testCase.desc), + func(t *testing.T, sc scenarioContext) { + folder := createFolderWithACL(t, "Folder", sc.user, testCase.items) + cmd := getCreateCommand(folder.Id, "Library Panel Name") + resp := sc.service.createHandler(sc.reqContext, cmd) + validateAndUnMarshalResponse(t, resp) + sc.reqContext.SignedInUser.OrgRole = testCase.role + + err := sc.service.DeleteLibraryPanelsInFolder(sc.reqContext, folder.Uid) + switch testCase.status { + case 200: + require.NoError(t, err) + case 403: + require.EqualError(t, err, models.ErrFolderAccessDenied.Error()) + default: + t.Fatalf("Unrecognized test case status %d", testCase.status) + } + }) } var generalFolderCases = []struct { diff --git a/pkg/services/librarypanels/librarypanels_test.go b/pkg/services/librarypanels/librarypanels_test.go index 67fd78067ad..5b806c5215b 100644 --- a/pkg/services/librarypanels/librarypanels_test.go +++ b/pkg/services/librarypanels/librarypanels_test.go @@ -630,6 +630,38 @@ func TestDisconnectLibraryPanelsForDashboard(t *testing.T) { }) } +func TestDeleteLibraryPanelsInFolder(t *testing.T) { + scenarioWithLibraryPanel(t, "When an admin tries to delete a folder that contains connected library panels, it should fail", + func(t *testing.T, sc scenarioContext) { + sc.reqContext.ReplaceAllParams(map[string]string{":uid": sc.initialResult.Result.UID, ":dashboardId": "1"}) + resp := sc.service.connectHandler(sc.reqContext) + require.Equal(t, 200, resp.Status()) + + err := sc.service.DeleteLibraryPanelsInFolder(sc.reqContext, sc.folder.Uid) + require.EqualError(t, err, ErrFolderHasConnectedLibraryPanels.Error()) + }) + + scenarioWithLibraryPanel(t, "When an admin tries to delete a folder that contains disconnected library panels, it should delete all disconnected library panels too", + func(t *testing.T, sc scenarioContext) { + resp := sc.service.getAllHandler(sc.reqContext) + require.Equal(t, 200, resp.Status()) + var result libraryPanelsResult + err := json.Unmarshal(resp.Body(), &result) + require.NoError(t, err) + require.NotNil(t, result.Result) + require.Equal(t, 1, len(result.Result)) + + err = sc.service.DeleteLibraryPanelsInFolder(sc.reqContext, sc.folder.Uid) + require.NoError(t, err) + resp = sc.service.getAllHandler(sc.reqContext) + require.Equal(t, 200, resp.Status()) + err = json.Unmarshal(resp.Body(), &result) + require.NoError(t, err) + require.NotNil(t, result.Result) + require.Equal(t, 0, len(result.Result)) + }) +} + type libraryPanel struct { ID int64 `json:"id"` OrgID int64 `json:"orgId"` diff --git a/pkg/services/librarypanels/models.go b/pkg/services/librarypanels/models.go index 6e782e6b4fc..681a37f23af 100644 --- a/pkg/services/librarypanels/models.go +++ b/pkg/services/librarypanels/models.go @@ -96,6 +96,8 @@ var ( errLibraryPanelHeaderUIDMissing = errors.New("library panel header is missing required property uid") // errLibraryPanelHeaderNameMissing is an error for when a library panel header is missing the name property. errLibraryPanelHeaderNameMissing = errors.New("library panel header is missing required property name") + // ErrFolderHasConnectedLibraryPanels is an error for when an user deletes a folder that contains connected library panels. + ErrFolderHasConnectedLibraryPanels = errors.New("folder contains library panels that are linked to dashboards") ) // Commands