Skip to content

Commit 2edfbb7

Browse files
mildwonkeymildwonkey
authored
sqlstore split: dashboard permissions (grafana#49962)
* backend/sqlstore split: remove unused GetDashboardPermissionsForUser from sqlstore * remove debugging line * backend/sqlstore: move dashboard permission related functions to dashboard service
1 parent bb94681 commit 2edfbb7

30 files changed

+557
-481
lines changed

‎pkg/api/alerting_test.go

+11-4
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,17 @@ import (
77

88
"github.com/grafana/grafana/pkg/api/routing"
99

10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/mock"
12+
"github.com/stretchr/testify/require"
13+
1014
"github.com/grafana/grafana/pkg/api/dtos"
1115
"github.com/grafana/grafana/pkg/api/response"
1216
"github.com/grafana/grafana/pkg/models"
17+
"github.com/grafana/grafana/pkg/services/dashboards"
1318
"github.com/grafana/grafana/pkg/services/guardian"
1419
"github.com/grafana/grafana/pkg/services/search"
1520
"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
16-
"github.com/stretchr/testify/assert"
17-
"github.com/stretchr/testify/require"
1821
)
1922

2023
var (
@@ -46,9 +49,13 @@ func setUp(confs ...setUpConf) *HTTPServer {
4649
aclMockResp = c.aclMockResp
4750
}
4851
}
49-
store.ExpectedDashboardAclInfoList = aclMockResp
5052
store.ExpectedTeamsByUser = []*models.TeamDTO{}
51-
guardian.InitLegacyGuardian(store)
53+
dashSvc := &dashboards.FakeDashboardService{}
54+
dashSvc.On("GetDashboardAclInfoList", mock.Anything, mock.AnythingOfType("*models.GetDashboardAclInfoListQuery")).Run(func(args mock.Arguments) {
55+
q := args.Get(1).(*models.GetDashboardAclInfoListQuery)
56+
q.Result = aclMockResp
57+
}).Return(nil)
58+
guardian.InitLegacyGuardian(store, dashSvc)
5259
return hs
5360
}
5461

‎pkg/api/annotations_test.go

+10-7
Original file line numberDiff line numberDiff line change
@@ -1000,15 +1000,18 @@ func TestAPI_MassDeleteAnnotations_AccessControl(t *testing.T) {
10001000
func setUpACL() {
10011001
viewerRole := models.ROLE_VIEWER
10021002
editorRole := models.ROLE_EDITOR
1003-
1004-
aclMockResp := []*models.DashboardAclInfoDTO{
1005-
{Role: &viewerRole, Permission: models.PERMISSION_VIEW},
1006-
{Role: &editorRole, Permission: models.PERMISSION_EDIT},
1007-
}
10081003
store := mockstore.NewSQLStoreMock()
1009-
store.ExpectedDashboardAclInfoList = aclMockResp
10101004
store.ExpectedTeamsByUser = []*models.TeamDTO{}
1011-
guardian.InitLegacyGuardian(store)
1005+
dashSvc := &dashboards.FakeDashboardService{}
1006+
dashSvc.On("GetDashboardAclInfoList", mock.Anything, mock.AnythingOfType("*models.GetDashboardAclInfoListQuery")).Run(func(args mock.Arguments) {
1007+
q := args.Get(1).(*models.GetDashboardAclInfoListQuery)
1008+
q.Result = []*models.DashboardAclInfoDTO{
1009+
{Role: &viewerRole, Permission: models.PERMISSION_VIEW},
1010+
{Role: &editorRole, Permission: models.PERMISSION_EDIT},
1011+
}
1012+
}).Return(nil)
1013+
1014+
guardian.InitLegacyGuardian(store, dashSvc)
10121015
}
10131016

10141017
func setUpRBACGuardian(t *testing.T) {

‎pkg/api/api.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func (hs *HTTPServer) registerRoutes() {
2828
reqGrafanaAdmin := middleware.ReqGrafanaAdmin
2929
reqEditorRole := middleware.ReqEditorRole
3030
reqOrgAdmin := middleware.ReqOrgAdmin
31-
reqOrgAdminFolderAdminOrTeamAdmin := middleware.OrgAdminFolderAdminOrTeamAdmin(hs.SQLStore)
31+
reqOrgAdminFolderAdminOrTeamAdmin := middleware.OrgAdminFolderAdminOrTeamAdmin(hs.SQLStore, hs.dashboardService)
3232
reqCanAccessTeams := middleware.AdminOrEditorAndFeatureEnabled(hs.Cfg.EditorsCanAdmin)
3333
reqSnapshotPublicModeOrSignedIn := middleware.SnapshotPublicModeOrSignedIn(hs.Cfg)
3434
redirectFromLegacyPanelEditURL := middleware.RedirectFromLegacyPanelEditURL(hs.Cfg)

‎pkg/api/dashboard_public_config.go

-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package api
22

33
import (
44
"errors"
5-
"fmt"
65
"net/http"
76

87
"github.com/grafana/grafana/pkg/api/response"
@@ -42,8 +41,6 @@ func (hs *HTTPServer) SavePublicDashboard(c *models.ReqContext) response.Respons
4241

4342
pdc, err := hs.dashboardService.SavePublicDashboardConfig(c.Req.Context(), &dto)
4443

45-
fmt.Println("err:", err)
46-
4744
if errors.Is(err, models.ErrDashboardNotFound) {
4845
return response.Error(http.StatusNotFound, "dashboard not found", err)
4946
}

‎pkg/api/dashboard_snapshot_test.go

+17-15
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,16 @@ import (
88
"testing"
99
"time"
1010

11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/mock"
13+
"github.com/stretchr/testify/require"
14+
1115
"github.com/grafana/grafana/pkg/components/simplejson"
1216
"github.com/grafana/grafana/pkg/models"
17+
"github.com/grafana/grafana/pkg/services/dashboards"
1318
"github.com/grafana/grafana/pkg/services/dashboardsnapshots"
1419
"github.com/grafana/grafana/pkg/services/guardian"
1520
"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
16-
"github.com/stretchr/testify/assert"
17-
"github.com/stretchr/testify/require"
1821
)
1922

2023
func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) {
@@ -29,10 +32,9 @@ func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) {
2932
jsonModel, err := simplejson.NewJson([]byte(`{"id":100}`))
3033
require.NoError(t, err)
3134

32-
viewerRole := models.ROLE_VIEWER
33-
editorRole := models.ROLE_EDITOR
3435
sqlmock := mockstore.NewSQLStoreMock()
35-
aclMockResp := []*models.DashboardAclInfoDTO{}
36+
dashSvc := &dashboards.FakeDashboardService{}
37+
dashSvc.On("GetDashboardAclInfoList", mock.Anything, mock.AnythingOfType("*models.GetDashboardAclInfoListQuery")).Return(nil)
3638
hs := &HTTPServer{DashboardsnapshotsService: &dashboardsnapshots.Service{SQLStore: sqlmock}}
3739

3840
setUpSnapshotTest := func(t *testing.T) *models.DashboardSnapshot {
@@ -47,7 +49,6 @@ func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) {
4749
External: true,
4850
}
4951
sqlmock.ExpectedDashboardSnapshot = mockSnapshotResult
50-
sqlmock.ExpectedDashboardAclInfoList = aclMockResp
5152
sqlmock.ExpectedTeamsByUser = []*models.TeamDTO{}
5253

5354
return mockSnapshotResult
@@ -63,7 +64,7 @@ func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) {
6364
})
6465
mockSnapshotResult.ExternalDeleteUrl = ts.URL
6566
sc.handlerFunc = hs.DeleteDashboardSnapshot
66-
guardian.InitLegacyGuardian(sc.sqlStore)
67+
guardian.InitLegacyGuardian(sc.sqlStore, dashSvc)
6768
sc.fakeReqWithParams("DELETE", sc.url, map[string]string{"key": "12345"}).exec()
6869

6970
assert.Equal(t, 403, sc.resp.Code)
@@ -100,15 +101,19 @@ func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) {
100101
})
101102

102103
t.Run("When user is editor and dashboard has default ACL", func(t *testing.T) {
103-
aclMockResp = []*models.DashboardAclInfoDTO{
104-
{Role: &viewerRole, Permission: models.PERMISSION_VIEW},
105-
{Role: &editorRole, Permission: models.PERMISSION_EDIT},
106-
}
104+
dashSvc := &dashboards.FakeDashboardService{}
105+
dashSvc.On("GetDashboardAclInfoList", mock.Anything, mock.AnythingOfType("*models.GetDashboardAclInfoListQuery")).Run(func(args mock.Arguments) {
106+
q := args.Get(1).(*models.GetDashboardAclInfoListQuery)
107+
q.Result = []*models.DashboardAclInfoDTO{
108+
{Role: &viewerRole, Permission: models.PERMISSION_VIEW},
109+
{Role: &editorRole, Permission: models.PERMISSION_EDIT},
110+
}
111+
}).Return(nil)
107112

108113
loggedInUserScenarioWithRole(t, "Should be able to delete a snapshot when calling DELETE on", "DELETE",
109114
"/api/snapshots/12345", "/api/snapshots/:key", models.ROLE_EDITOR, func(sc *scenarioContext) {
110115
mockSnapshotResult := setUpSnapshotTest(t)
111-
116+
guardian.InitLegacyGuardian(sc.sqlStore, dashSvc)
112117
var externalRequest *http.Request
113118
ts := setupRemoteServer(func(rw http.ResponseWriter, req *http.Request) {
114119
rw.WriteHeader(200)
@@ -130,11 +135,9 @@ func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) {
130135
})
131136

132137
t.Run("When user is editor and creator of the snapshot", func(t *testing.T) {
133-
aclMockResp := []*models.DashboardAclInfoDTO{}
134138
loggedInUserScenarioWithRole(t, "Should be able to delete a snapshot when calling DELETE on",
135139
"DELETE", "/api/snapshots/12345", "/api/snapshots/:key", models.ROLE_EDITOR, func(sc *scenarioContext) {
136140
mockSnapshotResult := setUpSnapshotTest(t)
137-
sqlmock.ExpectedDashboardAclInfoList = aclMockResp
138141
mockSnapshotResult.UserId = testUserID
139142
mockSnapshotResult.External = false
140143

@@ -151,7 +154,6 @@ func TestDashboardSnapshotAPIEndpoint_singleSnapshot(t *testing.T) {
151154
})
152155

153156
t.Run("When deleting an external snapshot", func(t *testing.T) {
154-
aclMockResp = []*models.DashboardAclInfoDTO{}
155157
loggedInUserScenarioWithRole(t,
156158
"Should gracefully delete local snapshot when remote snapshot has already been removed when calling DELETE on",
157159
"DELETE", "/api/snapshots/12345", "/api/snapshots/:key", models.ROLE_EDITOR, func(sc *scenarioContext) {

0 commit comments

Comments
 (0)