AC: Remove legacy AC from folders permissions API (#71526)

* Remove legacy AC from folder permissions API

* Update pkg/api/folder_permission.go

---------

Co-authored-by: Ieva <ieva.vasiljeva@grafana.com>
This commit is contained in:
Jo 2023-07-17 18:21:01 +02:00 committed by GitHub
parent 128990dbb7
commit 4b2ddaed44
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 16 additions and 46 deletions

View File

@ -12,7 +12,6 @@ import (
"github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/guardian"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/web" "github.com/grafana/grafana/pkg/web"
) )
@ -128,7 +127,7 @@ func (hs *HTTPServer) UpdateFolderPermissions(c *contextmodel.ReqContext) respon
hiddenACL, err := g.GetHiddenACL(hs.Cfg) hiddenACL, err := g.GetHiddenACL(hs.Cfg)
if err != nil { if err != nil {
return response.Error(500, "Error while retrieving hidden permissions", err) return response.Error(http.StatusInternalServerError, "Error while retrieving hidden permissions", err)
} }
items = append(items, hiddenACL...) items = append(items, hiddenACL...)
@ -136,46 +135,23 @@ func (hs *HTTPServer) UpdateFolderPermissions(c *contextmodel.ReqContext) respon
if err != nil { if err != nil {
if errors.Is(err, guardian.ErrGuardianPermissionExists) || if errors.Is(err, guardian.ErrGuardianPermissionExists) ||
errors.Is(err, guardian.ErrGuardianOverride) { errors.Is(err, guardian.ErrGuardianOverride) {
return response.Error(400, err.Error(), err) return response.Error(http.StatusBadRequest, err.Error(), err)
} }
return response.Error(500, "Error while checking folder permissions", err) return response.Error(http.StatusInternalServerError, "Error while checking folder permissions", err)
} }
return response.Error(403, "Cannot remove own admin permission for a folder", nil) return response.Error(http.StatusForbidden, "Cannot remove own admin permission for a folder", nil)
} }
if !hs.AccessControl.IsDisabled() {
old, err := g.GetACL() old, err := g.GetACL()
if err != nil { if err != nil {
return response.Error(500, "Error while checking dashboard permissions", err) return response.Error(http.StatusInternalServerError, "Error while checking folder permissions", err)
} }
if err := hs.updateDashboardAccessControl(c.Req.Context(), c.OrgID, folder.UID, true, items, old); err != nil { if err := hs.updateDashboardAccessControl(c.Req.Context(), c.OrgID, folder.UID, true, items, old); err != nil {
return response.Error(500, "Failed to create permission", err) return response.Error(http.StatusInternalServerError, "Failed to create permission", err)
} }
return response.Success("Dashboard permissions updated") return response.Success("Folder permissions updated")
}
if err := hs.DashboardService.UpdateDashboardACL(c.Req.Context(), folder.ID, items); err != nil {
if errors.Is(err, dashboards.ErrDashboardACLInfoMissing) {
err = dashboards.ErrFolderACLInfoMissing
}
if errors.Is(err, dashboards.ErrDashboardPermissionDashboardEmpty) {
err = dashboards.ErrFolderPermissionFolderEmpty
}
if errors.Is(err, dashboards.ErrFolderACLInfoMissing) || errors.Is(err, dashboards.ErrFolderPermissionFolderEmpty) {
return response.Error(409, err.Error(), err)
}
return response.Error(500, "Failed to create permission", err)
}
return response.JSON(http.StatusOK, util.DynMap{
"message": "Folder permissions updated",
"id": folder.ID,
"title": folder.Title,
})
} }
// swagger:parameters getFolderPermissionList // swagger:parameters getFolderPermissionList

View File

@ -13,6 +13,7 @@ import (
"github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/api/routing"
"github.com/grafana/grafana/pkg/infra/db/dbtest" "github.com/grafana/grafana/pkg/infra/db/dbtest"
"github.com/grafana/grafana/pkg/services/accesscontrol"
accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards"
@ -50,7 +51,7 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) {
folderPermissionsService: folderPermissions, folderPermissionsService: folderPermissions,
dashboardPermissionsService: dashboardPermissions, dashboardPermissionsService: dashboardPermissions,
DashboardService: dashboardService, DashboardService: dashboardService,
AccessControl: accesscontrolmock.New().WithDisabled(), AccessControl: ac,
} }
t.Run("Given folder not exists", func(t *testing.T) { t.Run("Given folder not exists", func(t *testing.T) {
@ -135,7 +136,6 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) {
}) })
folderService.ExpectedFolder = &folder.Folder{ID: 1, UID: "uid", Title: "Folder"} folderService.ExpectedFolder = &folder.Folder{ID: 1, UID: "uid", Title: "Folder"}
dashboardStore.On("UpdateDashboardACL", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
mockSQLStore := dbtest.NewFakeDB() mockSQLStore := dbtest.NewFakeDB()
loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", org.RoleAdmin, func(sc *scenarioContext) { loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", org.RoleAdmin, func(sc *scenarioContext) {
@ -157,6 +157,7 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) {
}, },
} }
folderPermissions.On("SetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]accesscontrol.ResourcePermission{}, nil).Once()
updateFolderPermissionScenario(t, updatePermissionContext{ updateFolderPermissionScenario(t, updatePermissionContext{
desc: "When calling POST on", desc: "When calling POST on",
url: "/api/folders/uid/permissions", url: "/api/folders/uid/permissions",
@ -167,14 +168,12 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) {
assert.Equal(t, 200, sc.resp.Code) assert.Equal(t, 200, sc.resp.Code)
var resp struct { var resp struct {
ID int64 Message string
Title string
} }
err := json.Unmarshal(sc.resp.Body.Bytes(), &resp) err := json.Unmarshal(sc.resp.Body.Bytes(), &resp)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, int64(1), resp.ID) assert.Equal(t, "Folder permissions updated", resp.Message)
assert.Equal(t, "Folder", resp.Title)
}, },
}, hs) }, hs)
}) })
@ -299,12 +298,7 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) {
}, },
}) })
var gotItems []*dashboards.DashboardACL
folderService.ExpectedFolder = &folder.Folder{ID: 1, UID: "uid", Title: "Folder"} folderService.ExpectedFolder = &folder.Folder{ID: 1, UID: "uid", Title: "Folder"}
dashboardStore.On("UpdateDashboardACL", mock.Anything, mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
gotItems = args.Get(2).([]*dashboards.DashboardACL)
}).Return(nil).Once()
var resp []*dashboards.DashboardACLInfoDTO var resp []*dashboards.DashboardACLInfoDTO
mockSQLStore := dbtest.NewFakeDB() mockSQLStore := dbtest.NewFakeDB()
@ -335,6 +329,7 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) {
} }
assert.Len(t, cmd.Items, 3) assert.Len(t, cmd.Items, 3)
folderPermissions.On("SetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]accesscontrol.ResourcePermission{}, nil).Once()
updateFolderPermissionScenario(t, updatePermissionContext{ updateFolderPermissionScenario(t, updatePermissionContext{
desc: "When calling POST on", desc: "When calling POST on",
url: "/api/folders/uid/permissions", url: "/api/folders/uid/permissions",
@ -343,7 +338,6 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) {
fn: func(sc *scenarioContext) { fn: func(sc *scenarioContext) {
sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
assert.Equal(t, 200, sc.resp.Code) assert.Equal(t, 200, sc.resp.Code)
assert.Len(t, gotItems, 4)
}, },
}, hs) }, hs)
}) })