Authz: fix snapshot tests legacy guardian (#73823)

* Guardian: remove unused dependencies

* API: rewrite tests to use access control guardian
This commit is contained in:
Karl Persson 2023-08-28 09:49:10 +02:00 committed by GitHub
parent 372f32963d
commit 01d98114b9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 163 additions and 261 deletions

View File

@ -285,11 +285,6 @@ func SetupAPITestServer(t *testing.T, opts ...APITestServerOption) *webtest.Serv
return s return s
} }
var (
viewerRole = org.RoleViewer
editorRole = org.RoleEditor
)
type mockSearchService struct{ ExpectedResult model.HitList } type mockSearchService struct{ ExpectedResult model.HitList }
func (mss *mockSearchService) SearchHandler(_ context.Context, q *search.Query) (model.HitList, error) { func (mss *mockSearchService) SearchHandler(_ context.Context, q *search.Query) (model.HitList, error) {

View File

@ -9,6 +9,10 @@ import (
"testing" "testing"
"time" "time"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/web/webtest"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock" "github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
@ -19,11 +23,106 @@ import (
"github.com/grafana/grafana/pkg/services/dashboardsnapshots" "github.com/grafana/grafana/pkg/services/dashboardsnapshots"
"github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/guardian"
"github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/team/teamtest"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util/errutil"
) )
func TestHTTPServer_DeleteDashboardSnapshot(t *testing.T) {
setup := func(t *testing.T, svc dashboards.DashboardService, userID int64, deleteURL string) *webtest.Server {
t.Helper()
return SetupAPITestServer(t, func(hs *HTTPServer) {
cfg := setting.NewCfg()
cfg.SnapshotEnabled = true
hs.Cfg = cfg
hs.dashboardsnapshotsService = setUpSnapshotTest(t, userID, deleteURL)
hs.DashboardService = svc
hs.AccessControl = acimpl.ProvideAccessControl(hs.Cfg)
guardian.InitAccessControlGuardian(hs.Cfg, hs.AccessControl, hs.DashboardService)
})
}
allowedUser := userWithPermissions(1, []accesscontrol.Permission{
{Action: dashboards.ActionDashboardsWrite, Scope: "dashboards:uid:1"},
})
t.Run("User should not be able to delete snapshot without permissions", func(t *testing.T) {
svc := dashboards.NewFakeDashboardService(t)
svc.On("GetDashboard", mock.Anything, mock.Anything).Return(&dashboards.Dashboard{UID: "1"}, nil)
server := setup(t, svc, 0, "")
res, err := server.Send(webtest.RequestWithSignedInUser(
server.NewRequest(http.MethodDelete, "/api/snapshots/12345", nil),
&user.SignedInUser{UserID: 1, OrgID: 1},
))
require.NoError(t, err)
assert.Equal(t, http.StatusForbidden, res.StatusCode)
require.NoError(t, res.Body.Close())
})
t.Run("User should be able to delete snapshot with correct permissions", func(t *testing.T) {
svc := dashboards.NewFakeDashboardService(t)
svc.On("GetDashboard", mock.Anything, mock.Anything).Return(&dashboards.Dashboard{UID: "1"}, nil)
server := setup(t, svc, 0, "")
res, err := server.Send(webtest.RequestWithSignedInUser(
server.NewRequest(http.MethodDelete, "/api/snapshots/12345", nil),
allowedUser,
))
require.NoError(t, err)
assert.Equal(t, http.StatusOK, res.StatusCode)
require.NoError(t, res.Body.Close())
})
t.Run("User should not be able to delete snapshot if fetching dashboard fails", func(t *testing.T) {
svc := dashboards.NewFakeDashboardService(t)
svc.On("GetDashboard", mock.Anything, mock.Anything).Return(nil, errors.New("some-error"))
server := setup(t, svc, 0, "")
res, err := server.Send(webtest.RequestWithSignedInUser(
server.NewRequest(http.MethodDelete, "/api/snapshots/12345", nil),
allowedUser,
))
require.NoError(t, err)
assert.Equal(t, http.StatusInternalServerError, res.StatusCode)
require.NoError(t, res.Body.Close())
})
t.Run("User should be able to delete snapshot if connected dashboard no longer exists", func(t *testing.T) {
svc := dashboards.NewFakeDashboardService(t)
svc.On("GetDashboard", mock.Anything, mock.Anything).Return(nil, dashboards.ErrDashboardNotFound)
server := setup(t, svc, 0, "")
res, err := server.Send(webtest.RequestWithSignedInUser(
server.NewRequest(http.MethodDelete, "/api/snapshots/12345", nil),
allowedUser,
))
require.NoError(t, err)
assert.Equal(t, http.StatusOK, res.StatusCode)
require.NoError(t, res.Body.Close())
})
t.Run("User should be able to delete when user is creator but does not have permissions to edit dashboard", func(t *testing.T) {
svc := dashboards.NewFakeDashboardService(t)
svc.On("GetDashboard", mock.Anything, mock.Anything).Return(&dashboards.Dashboard{UID: "1"}, nil)
server := setup(t, svc, 1, "")
res, err := server.Send(webtest.RequestWithSignedInUser(
server.NewRequest(http.MethodDelete, "/api/snapshots/12345", nil),
&user.SignedInUser{UserID: 1, OrgID: 1},
))
require.NoError(t, err)
assert.Equal(t, http.StatusOK, res.StatusCode)
require.NoError(t, res.Body.Close())
})
}
func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) { func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) {
setupRemoteServer := func(fn func(http.ResponseWriter, *http.Request)) *httptest.Server { setupRemoteServer := func(fn func(http.ResponseWriter, *http.Request)) *httptest.Server {
s := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { s := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
@ -34,60 +133,6 @@ func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) {
} }
sqlmock := dbtest.NewFakeDB() sqlmock := dbtest.NewFakeDB()
jsonModel, err := simplejson.NewJson([]byte(`{"id":100}`))
require.NoError(t, err)
setUpSnapshotTest := func(t *testing.T, userId int64, deleteUrl string) dashboardsnapshots.Service {
t.Helper()
dashSnapSvc := dashboardsnapshots.NewMockService(t)
dashSnapSvc.On("DeleteDashboardSnapshot", mock.Anything, mock.AnythingOfType("*dashboardsnapshots.DeleteDashboardSnapshotCommand")).Return(nil).Maybe()
res := &dashboardsnapshots.DashboardSnapshot{
ID: 1,
Key: "12345",
DeleteKey: "54321",
Dashboard: jsonModel,
Expires: time.Now().Add(time.Duration(1000) * time.Second),
UserID: 999999,
}
if userId != 0 {
res.UserID = userId
}
if deleteUrl != "" {
res.External = true
res.ExternalDeleteURL = deleteUrl
}
dashSnapSvc.On("GetDashboardSnapshot", mock.Anything, mock.AnythingOfType("*dashboardsnapshots.GetDashboardSnapshotQuery")).Return(res, nil)
dashSnapSvc.On("DeleteDashboardSnapshot", mock.Anything, mock.AnythingOfType("*dashboardsnapshots.DeleteDashboardSnapshotCommand")).Return(nil).Maybe()
return dashSnapSvc
}
t.Run("When user has editor role and is not in the ACL", func(t *testing.T) {
loggedInUserScenarioWithRole(t, "Should not be able to delete snapshot when calling DELETE on",
"DELETE", "/api/snapshots/12345", "/api/snapshots/:key", org.RoleEditor, func(sc *scenarioContext) {
d := setUpSnapshotTest(t, 0, "")
hs := buildHttpServer(d, true)
sc.handlerFunc = hs.DeleteDashboardSnapshot
teamSvc := &teamtest.FakeService{}
dashSvc := dashboards.NewFakeDashboardService(t)
var qResult *dashboards.Dashboard
dashSvc.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Run(func(args mock.Arguments) {
q := args.Get(1).(*dashboards.GetDashboardQuery)
qResult = &dashboards.Dashboard{
ID: q.ID,
UID: q.UID,
}
}).Return(qResult, nil).Maybe()
dashSvc.On("GetDashboardACLInfoList", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardACLInfoListQuery")).Return(nil, nil).Maybe()
hs.DashboardService = dashSvc
guardian.InitLegacyGuardian(setting.NewCfg(), sc.sqlStore, dashSvc, teamSvc)
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()
assert.Equal(t, 403, sc.resp.Code)
}, sqlmock)
})
t.Run("When user is anonymous", func(t *testing.T) { t.Run("When user is anonymous", func(t *testing.T) {
anonymousUserScenario(t, "Should be able to delete a snapshot when calling GET on", "GET", anonymousUserScenario(t, "Should be able to delete a snapshot when calling GET on", "GET",
@ -116,113 +161,6 @@ func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) {
}) })
}) })
t.Run("When user is editor and dashboard has default ACL", func(t *testing.T) {
teamSvc := &teamtest.FakeService{}
dashSvc := &dashboards.FakeDashboardService{}
qResult := []*dashboards.DashboardACLInfoDTO{
{Role: &viewerRole, Permission: dashboards.PERMISSION_VIEW},
{Role: &editorRole, Permission: dashboards.PERMISSION_EDIT},
}
dashSvc.On("GetDashboardACLInfoList", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardACLInfoListQuery")).Return(qResult, nil)
loggedInUserScenarioWithRole(t, "Should not be able to delete a snapshot when fetching guardian fails during calling DELETE on", "DELETE",
"/api/snapshots/12345", "/api/snapshots/:key", org.RoleEditor, func(sc *scenarioContext) {
ts := setupRemoteServer(func(rw http.ResponseWriter, req *http.Request) {
rw.WriteHeader(200)
})
dashSvc := dashboards.NewFakeDashboardService(t)
dashSvc.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(nil, errutil.Error{PublicMessage: "some error"}).Maybe()
guardian.InitLegacyGuardian(sc.cfg, sc.sqlStore, dashSvc, teamSvc)
d := setUpSnapshotTest(t, 0, ts.URL)
hs := buildHttpServer(d, true)
hs.DashboardService = dashSvc
sc.handlerFunc = hs.DeleteDashboardSnapshot
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()
assert.Equal(t, http.StatusInternalServerError, sc.resp.Code)
}, sqlmock)
loggedInUserScenarioWithRole(t, "Should be able to delete a snapshot from a deleted dashboard when calling DELETE on", "DELETE",
"/api/snapshots/12345", "/api/snapshots/:key", org.RoleEditor, func(sc *scenarioContext) {
var externalRequest *http.Request
ts := setupRemoteServer(func(rw http.ResponseWriter, req *http.Request) {
rw.WriteHeader(200)
externalRequest = req
})
dashSvc := dashboards.NewFakeDashboardService(t)
dashSvc.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(nil, dashboards.ErrDashboardNotFound).Maybe()
guardian.InitLegacyGuardian(sc.cfg, sc.sqlStore, dashSvc, teamSvc)
d := setUpSnapshotTest(t, 0, ts.URL)
hs := buildHttpServer(d, true)
hs.DashboardService = dashSvc
sc.handlerFunc = hs.DeleteDashboardSnapshot
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()
assert.Equal(t, 200, sc.resp.Code)
respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes())
require.NoError(t, err)
assert.True(t, strings.HasPrefix(respJSON.Get("message").MustString(), "Snapshot deleted"))
assert.Equal(t, 1, respJSON.Get("id").MustInt())
assert.Equal(t, ts.URL, fmt.Sprintf("http://%s", externalRequest.Host))
assert.Equal(t, "/", externalRequest.URL.EscapedPath())
}, sqlmock)
loggedInUserScenarioWithRole(t, "Should be able to delete a snapshot when calling DELETE on", "DELETE",
"/api/snapshots/12345", "/api/snapshots/:key", org.RoleEditor, func(sc *scenarioContext) {
var externalRequest *http.Request
ts := setupRemoteServer(func(rw http.ResponseWriter, req *http.Request) {
rw.WriteHeader(200)
externalRequest = req
})
dashSvc := dashboards.NewFakeDashboardService(t)
qResult := &dashboards.Dashboard{}
dashSvc.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(qResult, nil).Maybe()
qResultACL := []*dashboards.DashboardACLInfoDTO{
{Role: &viewerRole, Permission: dashboards.PERMISSION_VIEW},
{Role: &editorRole, Permission: dashboards.PERMISSION_EDIT},
}
dashSvc.On("GetDashboardACLInfoList", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardACLInfoListQuery")).Return(qResultACL, nil)
guardian.InitLegacyGuardian(sc.cfg, sc.sqlStore, dashSvc, teamSvc)
d := setUpSnapshotTest(t, 0, ts.URL)
hs := buildHttpServer(d, true)
hs.DashboardService = dashSvc
sc.handlerFunc = hs.DeleteDashboardSnapshot
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()
assert.Equal(t, 200, sc.resp.Code)
respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes())
require.NoError(t, err)
assert.True(t, strings.HasPrefix(respJSON.Get("message").MustString(), "Snapshot deleted"))
assert.Equal(t, 1, respJSON.Get("id").MustInt())
assert.Equal(t, ts.URL, fmt.Sprintf("http://%s", externalRequest.Host))
assert.Equal(t, "/", externalRequest.URL.EscapedPath())
}, sqlmock)
})
t.Run("When user is editor and creator of the snapshot", func(t *testing.T) {
loggedInUserScenarioWithRole(t, "Should be able to delete a snapshot when calling DELETE on",
"DELETE", "/api/snapshots/12345", "/api/snapshots/:key", org.RoleEditor, func(sc *scenarioContext) {
d := setUpSnapshotTest(t, testUserID, "")
dashSvc := dashboards.NewFakeDashboardService(t)
hs := buildHttpServer(d, true)
hs.DashboardService = dashSvc
sc.handlerFunc = hs.DeleteDashboardSnapshot
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()
assert.Equal(t, 200, sc.resp.Code)
respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes())
require.NoError(t, err)
assert.True(t, strings.HasPrefix(respJSON.Get("message").MustString(), "Snapshot deleted"))
assert.Equal(t, 1, respJSON.Get("id").MustInt())
}, sqlmock)
})
t.Run("When deleting an external snapshot", func(t *testing.T) { t.Run("When deleting an external snapshot", func(t *testing.T) {
loggedInUserScenarioWithRole(t, loggedInUserScenarioWithRole(t,
"Should gracefully delete local snapshot when remote snapshot has already been removed when calling DELETE on", "Should gracefully delete local snapshot when remote snapshot has already been removed when calling DELETE on",
@ -442,3 +380,31 @@ func buildHttpServer(d dashboardsnapshots.Service, snapshotEnabled bool) *HTTPSe
} }
return hs return hs
} }
func setUpSnapshotTest(t *testing.T, userId int64, deleteUrl string) dashboardsnapshots.Service {
t.Helper()
dashSnapSvc := dashboardsnapshots.NewMockService(t)
dashSnapSvc.On("DeleteDashboardSnapshot", mock.Anything, mock.AnythingOfType("*dashboardsnapshots.DeleteDashboardSnapshotCommand")).Return(nil).Maybe()
jsonModel, err := simplejson.NewJson([]byte(`{"id":100}`))
require.NoError(t, err)
res := &dashboardsnapshots.DashboardSnapshot{
ID: 1,
Key: "12345",
DeleteKey: "54321",
Dashboard: jsonModel,
Expires: time.Now().Add(time.Duration(1000) * time.Second),
UserID: 999999,
}
if userId != 0 {
res.UserID = userId
}
if deleteUrl != "" {
res.External = true
res.ExternalDeleteURL = deleteUrl
}
dashSnapSvc.On("GetDashboardSnapshot", mock.Anything, mock.AnythingOfType("*dashboardsnapshots.GetDashboardSnapshotQuery")).Return(res, nil)
dashSnapSvc.On("DeleteDashboardSnapshot", mock.Anything, mock.AnythingOfType("*dashboardsnapshots.DeleteDashboardSnapshotCommand")).Return(nil).Maybe()
return dashSnapSvc
}

View File

@ -842,7 +842,7 @@ func permissionScenario(t *testing.T, desc string, canSave bool, fn permissionSc
foldertest.NewFakeService(), foldertest.NewFakeService(),
) )
require.NoError(t, err) require.NoError(t, err)
guardian.InitAccessControlGuardian(cfg, sqlStore, ac, folderPermissions, dashboardPermissions, dashboardService) guardian.InitAccessControlGuardian(cfg, ac, dashboardService)
savedFolder := saveTestFolder(t, "Saved folder", testOrgID, sqlStore) savedFolder := saveTestFolder(t, "Saved folder", testOrgID, sqlStore)
savedDashInFolder := saveTestDashboard(t, "Saved dash in folder", testOrgID, savedFolder.ID, sqlStore) savedDashInFolder := saveTestDashboard(t, "Saved dash in folder", testOrgID, savedFolder.ID, sqlStore)

View File

@ -4,7 +4,6 @@ import (
"context" "context"
"errors" "errors"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards"
@ -18,10 +17,7 @@ var _ DashboardGuardian = new(accessControlDashboardGuardian)
// NewAccessControlDashboardGuardianByDashboard creates a dashboard guardian by the provided dashboardId. // NewAccessControlDashboardGuardianByDashboard creates a dashboard guardian by the provided dashboardId.
func NewAccessControlDashboardGuardian( func NewAccessControlDashboardGuardian(
ctx context.Context, cfg *setting.Cfg, dashboardId int64, user *user.SignedInUser, ctx context.Context, cfg *setting.Cfg, dashboardId int64, user *user.SignedInUser,
store db.DB, ac accesscontrol.AccessControl, ac accesscontrol.AccessControl, dashboardService dashboards.DashboardService,
folderPermissionsService accesscontrol.FolderPermissionsService,
dashboardPermissionsService accesscontrol.DashboardPermissionsService,
dashboardService dashboards.DashboardService,
) (DashboardGuardian, error) { ) (DashboardGuardian, error) {
var dashboard *dashboards.Dashboard var dashboard *dashboards.Dashboard
if dashboardId != 0 { if dashboardId != 0 {
@ -47,12 +43,10 @@ func NewAccessControlDashboardGuardian(
cfg: cfg, cfg: cfg,
log: log.New("folder.permissions"), log: log.New("folder.permissions"),
user: user, user: user,
store: store,
ac: ac, ac: ac,
dashboardService: dashboardService, dashboardService: dashboardService,
}, },
folder: dashboards.FromDashboard(dashboard), folder: dashboards.FromDashboard(dashboard),
folderPermissionsService: folderPermissionsService,
}, nil }, nil
} }
@ -62,22 +56,17 @@ func NewAccessControlDashboardGuardian(
cfg: cfg, cfg: cfg,
log: log.New("dashboard.permissions"), log: log.New("dashboard.permissions"),
user: user, user: user,
store: store,
ac: ac, ac: ac,
dashboardService: dashboardService, dashboardService: dashboardService,
}, },
dashboard: dashboard, dashboard: dashboard,
dashboardPermissionsService: dashboardPermissionsService,
}, nil }, nil
} }
// NewAccessControlDashboardGuardianByDashboard creates a dashboard guardian by the provided dashboardUID. // NewAccessControlDashboardGuardianByDashboard creates a dashboard guardian by the provided dashboardUID.
func NewAccessControlDashboardGuardianByUID( func NewAccessControlDashboardGuardianByUID(
ctx context.Context, cfg *setting.Cfg, dashboardUID string, user *user.SignedInUser, ctx context.Context, cfg *setting.Cfg, dashboardUID string, user *user.SignedInUser,
store db.DB, ac accesscontrol.AccessControl, ac accesscontrol.AccessControl, dashboardService dashboards.DashboardService,
folderPermissionsService accesscontrol.FolderPermissionsService,
dashboardPermissionsService accesscontrol.DashboardPermissionsService,
dashboardService dashboards.DashboardService,
) (DashboardGuardian, error) { ) (DashboardGuardian, error) {
var dashboard *dashboards.Dashboard var dashboard *dashboards.Dashboard
if dashboardUID != "" { if dashboardUID != "" {
@ -103,12 +92,10 @@ func NewAccessControlDashboardGuardianByUID(
cfg: cfg, cfg: cfg,
log: log.New("folder.permissions"), log: log.New("folder.permissions"),
user: user, user: user,
store: store,
ac: ac, ac: ac,
dashboardService: dashboardService, dashboardService: dashboardService,
}, },
folder: dashboards.FromDashboard(dashboard), folder: dashboards.FromDashboard(dashboard),
folderPermissionsService: folderPermissionsService,
}, nil }, nil
} }
@ -118,12 +105,10 @@ func NewAccessControlDashboardGuardianByUID(
ctx: ctx, ctx: ctx,
log: log.New("dashboard.permissions"), log: log.New("dashboard.permissions"),
user: user, user: user,
store: store,
ac: ac, ac: ac,
dashboardService: dashboardService, dashboardService: dashboardService,
}, },
dashboard: dashboard, dashboard: dashboard,
dashboardPermissionsService: dashboardPermissionsService,
}, nil }, nil
} }
@ -132,10 +117,7 @@ func NewAccessControlDashboardGuardianByUID(
// since it avoids querying the database for fetching the dashboard. // since it avoids querying the database for fetching the dashboard.
func NewAccessControlDashboardGuardianByDashboard( func NewAccessControlDashboardGuardianByDashboard(
ctx context.Context, cfg *setting.Cfg, dashboard *dashboards.Dashboard, user *user.SignedInUser, ctx context.Context, cfg *setting.Cfg, dashboard *dashboards.Dashboard, user *user.SignedInUser,
store db.DB, ac accesscontrol.AccessControl, ac accesscontrol.AccessControl, dashboardService dashboards.DashboardService,
folderPermissionsService accesscontrol.FolderPermissionsService,
dashboardPermissionsService accesscontrol.DashboardPermissionsService,
dashboardService dashboards.DashboardService,
) (DashboardGuardian, error) { ) (DashboardGuardian, error) {
if dashboard != nil && dashboard.IsFolder { if dashboard != nil && dashboard.IsFolder {
return &accessControlFolderGuardian{ return &accessControlFolderGuardian{
@ -144,12 +126,10 @@ func NewAccessControlDashboardGuardianByDashboard(
cfg: cfg, cfg: cfg,
log: log.New("folder.permissions"), log: log.New("folder.permissions"),
user: user, user: user,
store: store,
ac: ac, ac: ac,
dashboardService: dashboardService, dashboardService: dashboardService,
}, },
folder: dashboards.FromDashboard(dashboard), folder: dashboards.FromDashboard(dashboard),
folderPermissionsService: folderPermissionsService,
}, nil }, nil
} }
@ -159,22 +139,17 @@ func NewAccessControlDashboardGuardianByDashboard(
ctx: ctx, ctx: ctx,
log: log.New("dashboard.permissions"), log: log.New("dashboard.permissions"),
user: user, user: user,
store: store,
ac: ac, ac: ac,
dashboardService: dashboardService, dashboardService: dashboardService,
}, },
dashboard: dashboard, dashboard: dashboard,
dashboardPermissionsService: dashboardPermissionsService,
}, nil }, nil
} }
// NewAccessControlFolderGuardian creates a folder guardian by the provided folder. // NewAccessControlFolderGuardian creates a folder guardian by the provided folder.
func NewAccessControlFolderGuardian( func NewAccessControlFolderGuardian(
ctx context.Context, cfg *setting.Cfg, f *folder.Folder, user *user.SignedInUser, ctx context.Context, cfg *setting.Cfg, f *folder.Folder, user *user.SignedInUser,
store db.DB, ac accesscontrol.AccessControl, ac accesscontrol.AccessControl, dashboardService dashboards.DashboardService,
folderPermissionsService accesscontrol.FolderPermissionsService,
dashboardPermissionsService accesscontrol.DashboardPermissionsService,
dashboardService dashboards.DashboardService,
) (DashboardGuardian, error) { ) (DashboardGuardian, error) {
return &accessControlFolderGuardian{ return &accessControlFolderGuardian{
accessControlBaseGuardian: accessControlBaseGuardian{ accessControlBaseGuardian: accessControlBaseGuardian{
@ -182,12 +157,10 @@ func NewAccessControlFolderGuardian(
cfg: cfg, cfg: cfg,
log: log.New("folder.permissions"), log: log.New("folder.permissions"),
user: user, user: user,
store: store,
ac: ac, ac: ac,
dashboardService: dashboardService, dashboardService: dashboardService,
}, },
folder: f, folder: f,
folderPermissionsService: folderPermissionsService,
}, nil }, nil
} }
@ -197,20 +170,17 @@ type accessControlBaseGuardian struct {
log log.Logger log log.Logger
user *user.SignedInUser user *user.SignedInUser
ac accesscontrol.AccessControl ac accesscontrol.AccessControl
store db.DB
dashboardService dashboards.DashboardService dashboardService dashboards.DashboardService
} }
type accessControlDashboardGuardian struct { type accessControlDashboardGuardian struct {
accessControlBaseGuardian accessControlBaseGuardian
dashboard *dashboards.Dashboard dashboard *dashboards.Dashboard
dashboardPermissionsService accesscontrol.DashboardPermissionsService
} }
type accessControlFolderGuardian struct { type accessControlFolderGuardian struct {
accessControlBaseGuardian accessControlBaseGuardian
folder *folder.Folder folder *folder.Folder
folderPermissionsService accesscontrol.FolderPermissionsService
} }
func (a *accessControlDashboardGuardian) CanSave() (bool, error) { func (a *accessControlDashboardGuardian) CanSave() (bool, error) {

View File

@ -9,23 +9,13 @@ import (
"github.com/stretchr/testify/mock" "github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/api/routing"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
acdb "github.com/grafana/grafana/pkg/services/accesscontrol/database"
"github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol"
"github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/folder/foldertest" "github.com/grafana/grafana/pkg/services/folder/foldertest"
"github.com/grafana/grafana/pkg/services/licensing/licensingtest" "github.com/grafana/grafana/pkg/services/licensing/licensingtest"
"github.com/grafana/grafana/pkg/services/quota/quotatest"
"github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest"
"github.com/grafana/grafana/pkg/services/team/teamimpl"
"github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/userimpl"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
) )
@ -201,7 +191,7 @@ func TestAccessControlDashboardGuardian_CanSave(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) { t.Run(tt.desc, func(t *testing.T) {
guardian := setupAccessControlGuardianTest(t, tt.dashboard, tt.permissions, nil, nil, nil) guardian := setupAccessControlGuardianTest(t, tt.dashboard, tt.permissions, nil)
can, err := guardian.CanSave() can, err := guardian.CanSave()
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, tt.expected, can) assert.Equal(t, tt.expected, can)
@ -373,7 +363,7 @@ func TestAccessControlDashboardGuardian_CanEdit(t *testing.T) {
t.Run(tt.desc, func(t *testing.T) { t.Run(tt.desc, func(t *testing.T) {
cfg := setting.NewCfg() cfg := setting.NewCfg()
cfg.ViewersCanEdit = tt.viewersCanEdit cfg.ViewersCanEdit = tt.viewersCanEdit
guardian := setupAccessControlGuardianTest(t, tt.dashboard, tt.permissions, cfg, nil, nil) guardian := setupAccessControlGuardianTest(t, tt.dashboard, tt.permissions, cfg)
can, err := guardian.CanEdit() can, err := guardian.CanEdit()
require.NoError(t, err) require.NoError(t, err)
@ -531,7 +521,7 @@ func TestAccessControlDashboardGuardian_CanView(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) { t.Run(tt.desc, func(t *testing.T) {
guardian := setupAccessControlGuardianTest(t, tt.dashboard, tt.permissions, nil, nil, nil) guardian := setupAccessControlGuardianTest(t, tt.dashboard, tt.permissions, nil)
can, err := guardian.CanView() can, err := guardian.CanView()
require.NoError(t, err) require.NoError(t, err)
@ -784,7 +774,7 @@ func TestAccessControlDashboardGuardian_CanAdmin(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) { t.Run(tt.desc, func(t *testing.T) {
guardian := setupAccessControlGuardianTest(t, tt.dashboard, tt.permissions, nil, nil, nil) guardian := setupAccessControlGuardianTest(t, tt.dashboard, tt.permissions, nil)
can, err := guardian.CanAdmin() can, err := guardian.CanAdmin()
require.NoError(t, err) require.NoError(t, err)
@ -942,7 +932,7 @@ func TestAccessControlDashboardGuardian_CanDelete(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) { t.Run(tt.desc, func(t *testing.T) {
guardian := setupAccessControlGuardianTest(t, tt.dashboard, tt.permissions, nil, nil, nil) guardian := setupAccessControlGuardianTest(t, tt.dashboard, tt.permissions, nil)
can, err := guardian.CanDelete() can, err := guardian.CanDelete()
require.NoError(t, err) require.NoError(t, err)
@ -1006,7 +996,7 @@ func TestAccessControlDashboardGuardian_CanCreate(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) { t.Run(tt.desc, func(t *testing.T) {
guardian := setupAccessControlGuardianTest(t, &dashboards.Dashboard{OrgID: orgID, UID: "0", IsFolder: tt.isFolder}, tt.permissions, nil, nil, nil) guardian := setupAccessControlGuardianTest(t, &dashboards.Dashboard{OrgID: orgID, UID: "0", IsFolder: tt.isFolder}, tt.permissions, nil)
can, err := guardian.CanCreate(tt.folderID, tt.isFolder) can, err := guardian.CanCreate(tt.folderID, tt.isFolder)
require.NoError(t, err) require.NoError(t, err)
@ -1015,12 +1005,11 @@ func TestAccessControlDashboardGuardian_CanCreate(t *testing.T) {
} }
} }
func setupAccessControlGuardianTest(t *testing.T, d *dashboards.Dashboard, func setupAccessControlGuardianTest(
permissions []accesscontrol.Permission, t *testing.T, d *dashboards.Dashboard,
cfg *setting.Cfg, permissions []accesscontrol.Permission, cfg *setting.Cfg,
dashboardPermissions accesscontrol.DashboardPermissionsService, folderPermissions accesscontrol.FolderPermissionsService) DashboardGuardian { ) DashboardGuardian {
t.Helper() t.Helper()
store := db.InitTestDB(t)
fakeDashboardService := dashboards.NewFakeDashboardService(t) fakeDashboardService := dashboards.NewFakeDashboardService(t)
fakeDashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Maybe().Return(d, nil) fakeDashboardService.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Maybe().Return(d, nil)
@ -1037,21 +1026,6 @@ func setupAccessControlGuardianTest(t *testing.T, d *dashboards.Dashboard,
license := licensingtest.NewFakeLicensing() license := licensingtest.NewFakeLicensing()
license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe() license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe()
teamSvc := teamimpl.ProvideService(store, store.Cfg)
userSvc, err := userimpl.ProvideService(store, nil, store.Cfg, nil, nil, quotatest.New(false, nil), supportbundlestest.NewFakeBundleService())
require.NoError(t, err)
acSvc := acimpl.ProvideOSSService(cfg, acdb.ProvideService(store), localcache.ProvideService(), featuremgmt.WithFeatures())
if folderPermissions == nil {
folderPermissions, err = ossaccesscontrol.ProvideFolderPermissions(
featuremgmt.WithFeatures(), routing.NewRouteRegister(), store, ac, license, &dashboards.FakeDashboardStore{}, folderSvc, acSvc, teamSvc, userSvc)
require.NoError(t, err)
}
if dashboardPermissions == nil {
dashboardPermissions, err = ossaccesscontrol.ProvideDashboardPermissions(
featuremgmt.WithFeatures(), routing.NewRouteRegister(), store, ac, license, &dashboards.FakeDashboardStore{}, folderSvc, acSvc, teamSvc, userSvc)
require.NoError(t, err)
}
userPermissions := map[int64]map[string][]string{} userPermissions := map[int64]map[string][]string{}
for _, p := range permissions { for _, p := range permissions {
@ -1061,7 +1035,7 @@ func setupAccessControlGuardianTest(t *testing.T, d *dashboards.Dashboard,
userPermissions[orgID][p.Action] = append(userPermissions[orgID][p.Action], p.Scope) userPermissions[orgID][p.Action] = append(userPermissions[orgID][p.Action], p.Scope)
} }
g, err := NewAccessControlDashboardGuardianByDashboard(context.Background(), cfg, d, &user.SignedInUser{OrgID: orgID, Permissions: userPermissions}, store, ac, folderPermissions, dashboardPermissions, fakeDashboardService) g, err := NewAccessControlDashboardGuardianByDashboard(context.Background(), cfg, d, &user.SignedInUser{OrgID: orgID, Permissions: userPermissions}, ac, fakeDashboardService)
require.NoError(t, err) require.NoError(t, err)
return g return g
} }

View File

@ -19,7 +19,6 @@ var (
ErrGuardianPermissionExists = errors.New("permission already exists") ErrGuardianPermissionExists = errors.New("permission already exists")
ErrGuardianOverride = errors.New("you can only override a permission to be higher") ErrGuardianOverride = errors.New("you can only override a permission to be higher")
ErrGuardianGetDashboardFailure = errutil.Internal("guardian.getDashboardFailure", errutil.WithPublicMessage("Failed to get dashboard")) ErrGuardianGetDashboardFailure = errutil.Internal("guardian.getDashboardFailure", errutil.WithPublicMessage("Failed to get dashboard"))
ErrGuardianGetFolderFailure = errutil.Internal("guardian.getFolderFailure", errutil.WithPublicMessage("Failed to get folder"))
ErrGuardianDashboardNotFound = errutil.NotFound("guardian.dashboardNotFound") ErrGuardianDashboardNotFound = errutil.NotFound("guardian.dashboardNotFound")
ErrGuardianFolderNotFound = errutil.NotFound("guardian.folderNotFound") ErrGuardianFolderNotFound = errutil.NotFound("guardian.folderNotFound")
) )

View File

@ -16,12 +16,11 @@ type Provider struct{}
func ProvideService( func ProvideService(
cfg *setting.Cfg, store db.DB, ac accesscontrol.AccessControl, cfg *setting.Cfg, store db.DB, ac accesscontrol.AccessControl,
folderPermissionsService accesscontrol.FolderPermissionsService, dashboardPermissionsService accesscontrol.DashboardPermissionsService,
dashboardService dashboards.DashboardService, teamService team.Service, dashboardService dashboards.DashboardService, teamService team.Service,
) *Provider { ) *Provider {
if !ac.IsDisabled() { if !ac.IsDisabled() {
// TODO: Fix this hack, see https://github.com/grafana/grafana-enterprise/issues/2935 // TODO: Fix this hack, see https://github.com/grafana/grafana-enterprise/issues/2935
InitAccessControlGuardian(cfg, store, ac, folderPermissionsService, dashboardPermissionsService, dashboardService) InitAccessControlGuardian(cfg, ac, dashboardService)
} else { } else {
InitLegacyGuardian(cfg, store, dashboardService, teamService) InitLegacyGuardian(cfg, store, dashboardService, teamService)
} }
@ -47,22 +46,21 @@ func InitLegacyGuardian(cfg *setting.Cfg, store db.DB, dashSvc dashboards.Dashbo
} }
func InitAccessControlGuardian( func InitAccessControlGuardian(
cfg *setting.Cfg, store db.DB, ac accesscontrol.AccessControl, folderPermissionsService accesscontrol.FolderPermissionsService, cfg *setting.Cfg, ac accesscontrol.AccessControl, dashboardService dashboards.DashboardService,
dashboardPermissionsService accesscontrol.DashboardPermissionsService, dashboardService dashboards.DashboardService,
) { ) {
New = func(ctx context.Context, dashId int64, orgId int64, user *user.SignedInUser) (DashboardGuardian, error) { New = func(ctx context.Context, dashId int64, orgId int64, user *user.SignedInUser) (DashboardGuardian, error) {
return NewAccessControlDashboardGuardian(ctx, cfg, dashId, user, store, ac, folderPermissionsService, dashboardPermissionsService, dashboardService) return NewAccessControlDashboardGuardian(ctx, cfg, dashId, user, ac, dashboardService)
} }
NewByUID = func(ctx context.Context, dashUID string, orgId int64, user *user.SignedInUser) (DashboardGuardian, error) { NewByUID = func(ctx context.Context, dashUID string, orgId int64, user *user.SignedInUser) (DashboardGuardian, error) {
return NewAccessControlDashboardGuardianByUID(ctx, cfg, dashUID, user, store, ac, folderPermissionsService, dashboardPermissionsService, dashboardService) return NewAccessControlDashboardGuardianByUID(ctx, cfg, dashUID, user, ac, dashboardService)
} }
NewByDashboard = func(ctx context.Context, dash *dashboards.Dashboard, orgId int64, user *user.SignedInUser) (DashboardGuardian, error) { NewByDashboard = func(ctx context.Context, dash *dashboards.Dashboard, orgId int64, user *user.SignedInUser) (DashboardGuardian, error) {
return NewAccessControlDashboardGuardianByDashboard(ctx, cfg, dash, user, store, ac, folderPermissionsService, dashboardPermissionsService, dashboardService) return NewAccessControlDashboardGuardianByDashboard(ctx, cfg, dash, user, ac, dashboardService)
} }
NewByFolder = func(ctx context.Context, f *folder.Folder, orgId int64, user *user.SignedInUser) (DashboardGuardian, error) { NewByFolder = func(ctx context.Context, f *folder.Folder, orgId int64, user *user.SignedInUser) (DashboardGuardian, error) {
return NewAccessControlFolderGuardian(ctx, cfg, f, user, store, ac, folderPermissionsService, dashboardPermissionsService, dashboardService) return NewAccessControlFolderGuardian(ctx, cfg, f, user, ac, dashboardService)
} }
} }

View File

@ -382,7 +382,7 @@ func scenarioWithPanel(t *testing.T, desc string, fn func(t *testing.T, sc scena
foldertest.NewFakeService(), foldertest.NewFakeService(),
) )
require.NoError(t, svcErr) require.NoError(t, svcErr)
guardian.InitAccessControlGuardian(sqlStore.Cfg, sqlStore, ac, folderPermissions, dashboardPermissions, dashboardService) guardian.InitAccessControlGuardian(sqlStore.Cfg, ac, dashboardService)
testScenario(t, desc, func(t *testing.T, sc scenarioContext) { testScenario(t, desc, func(t *testing.T, sc scenarioContext) {
command := getCreatePanelCommand(sc.folder.ID, "Text - Library Panel") command := getCreatePanelCommand(sc.folder.ID, "Text - Library Panel")
@ -440,7 +440,7 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo
foldertest.NewFakeService(), foldertest.NewFakeService(),
) )
require.NoError(t, dashSvcErr) require.NoError(t, dashSvcErr)
guardian.InitAccessControlGuardian(sqlStore.Cfg, sqlStore, ac, folderPermissions, dashboardPermissions, dashService) guardian.InitAccessControlGuardian(sqlStore.Cfg, ac, dashService)
service := LibraryElementService{ service := LibraryElementService{
Cfg: sqlStore.Cfg, Cfg: sqlStore.Cfg,
features: featuremgmt.WithFeatures(), features: featuremgmt.WithFeatures(),

View File

@ -823,7 +823,7 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo
foldertest.NewFakeService(), foldertest.NewFakeService(),
) )
require.NoError(t, err) require.NoError(t, err)
guardian.InitAccessControlGuardian(setting.NewCfg(), sqlStore, ac, acmock.NewMockedPermissionsService(), acmock.NewMockedPermissionsService(), dashService) guardian.InitAccessControlGuardian(setting.NewCfg(), ac, dashService)
dashboardStore, err := database.ProvideDashboardStore(sqlStore, cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore, sqlStore.Cfg), quotaService) dashboardStore, err := database.ProvideDashboardStore(sqlStore, cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore, sqlStore.Cfg), quotaService)
require.NoError(t, err) require.NoError(t, err)