AC: Remove legacy AC from dashboard permissions API (#71524)

* remove IsDisabled from org API

* remove IsDisabled from dashboard_permissions API
This commit is contained in:
Jo 2023-07-17 17:54:39 +02:00 committed by GitHub
parent 43f4e55a76
commit a7d905c03b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 18 additions and 38 deletions

View File

@ -172,41 +172,29 @@ func (hs *HTTPServer) UpdateDashboardPermissions(c *contextmodel.ReqContext) res
hiddenACL, err := g.GetHiddenACL(hs.Cfg)
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...)
if okToUpdate, err := g.CheckPermissionBeforeUpdate(dashboards.PERMISSION_ADMIN, items); err != nil || !okToUpdate {
if err != nil {
if errors.Is(err, guardian.ErrGuardianPermissionExists) || 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 dashboard permissions", err)
return response.Error(http.StatusInternalServerError, "Error while checking dashboard 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()
if err != nil {
return response.Error(500, "Error while checking dashboard permissions", err)
}
if err := hs.updateDashboardAccessControl(c.Req.Context(), dash.OrgID, dash.UID, false, items, old); err != nil {
return response.Error(500, "Failed to update permissions", err)
}
return response.Success("Dashboard permissions updated")
old, err := g.GetACL()
if err != nil {
return response.Error(http.StatusInternalServerError, "Error while checking dashboard permissions", err)
}
if err := hs.DashboardService.UpdateDashboardACL(c.Req.Context(), dashID, items); err != nil {
if errors.Is(err, dashboards.ErrDashboardACLInfoMissing) ||
errors.Is(err, dashboards.ErrDashboardPermissionDashboardEmpty) {
return response.Error(409, err.Error(), err)
}
return response.Error(500, "Failed to create permission", err)
if err := hs.updateDashboardAccessControl(c.Req.Context(), dash.OrgID, dash.UID, false, items, old); err != nil {
return response.Error(http.StatusInternalServerError, "Failed to update permissions", err)
}
return response.Success("Dashboard permissions updated")
}

View File

@ -15,6 +15,7 @@ import (
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/infra/db/dbtest"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/services/accesscontrol"
accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/dashboards"
@ -48,11 +49,12 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) {
)
require.NoError(t, err)
hs := &HTTPServer{
Cfg: settings,
SQLStore: mockSQLStore,
Features: features,
DashboardService: dashboardService,
AccessControl: accesscontrolmock.New().WithDisabled(),
Cfg: settings,
SQLStore: mockSQLStore,
Features: features,
DashboardService: dashboardService,
AccessControl: accesscontrolmock.New(),
dashboardPermissionsService: dashboardPermissions,
}
t.Run("Given user has no admin permissions", func(t *testing.T) {
@ -74,7 +76,6 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) {
},
}
dashboardStore.On("UpdateDashboardACL", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
updateDashboardPermissionScenario(t, updatePermissionContext{
desc: "When calling POST on",
url: "/api/dashboards/id/1/permissions",
@ -125,6 +126,7 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) {
},
}
dashboardPermissions.On("SetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]accesscontrol.ResourcePermission{}, nil).Once()
updateDashboardPermissionScenario(t, updatePermissionContext{
desc: "When calling POST on",
url: "/api/dashboards/id/1/permissions",
@ -315,11 +317,7 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) {
}
assert.Len(t, cmd.Items, 3)
var numOfItems []*dashboards.DashboardACL
dashboardStore.On("UpdateDashboardACL", mock.Anything, mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
items := args.Get(2).([]*dashboards.DashboardACL)
numOfItems = items
}).Return(nil).Once()
dashboardPermissions.On("SetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]accesscontrol.ResourcePermission{}, nil).Once()
updateDashboardPermissionScenario(t, updatePermissionContext{
desc: "When calling POST on",
url: "/api/dashboards/id/1/permissions",
@ -328,7 +326,6 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) {
fn: func(sc *scenarioContext) {
sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec()
assert.Equal(t, 200, sc.resp.Code)
assert.Len(t, numOfItems, 4)
},
}, hs)
})

View File

@ -11,7 +11,6 @@ import (
"github.com/grafana/grafana/pkg/infra/metrics"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/web"
)
@ -131,10 +130,6 @@ func (hs *HTTPServer) CreateOrg(c *contextmodel.ReqContext) response.Response {
if err := web.Bind(c.Req, &cmd); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
acEnabled := !hs.AccessControl.IsDisabled()
if !acEnabled && !(setting.AllowUserOrgCreate || c.IsGrafanaAdmin) {
return response.Error(http.StatusForbidden, "Access denied", nil)
}
cmd.UserID = c.UserID
result, err := hs.orgService.CreateWithMember(c.Req.Context(), &cmd)