diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index 6919bad8576..b09ef3d7f8a 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -296,7 +296,6 @@ func SetupAPITestServer(t *testing.T, opts ...APITestServerOption) *webtest.Serv if hs.Cfg == nil { hs.Cfg = setting.NewCfg() - hs.Cfg.RBACEnabled = false } if hs.AccessControl == nil { diff --git a/pkg/api/metrics_test.go b/pkg/api/metrics_test.go index eb4859b1cfb..9a19570a044 100644 --- a/pkg/api/metrics_test.go +++ b/pkg/api/metrics_test.go @@ -19,9 +19,9 @@ import ( "github.com/grafana/grafana/pkg/plugins/config" pluginClient "github.com/grafana/grafana/pkg/plugins/manager/client" "github.com/grafana/grafana/pkg/plugins/manager/registry" + "github.com/grafana/grafana/pkg/services/datasources" fakeDatasources "github.com/grafana/grafana/pkg/services/datasources/fakes" "github.com/grafana/grafana/pkg/services/featuremgmt" - "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/query" "github.com/grafana/grafana/pkg/services/quota/quotatest" "github.com/grafana/grafana/pkg/services/user" @@ -75,7 +75,7 @@ func TestAPIEndpoint_Metrics_QueryMetricsV2(t *testing.T) { t.Run("Status code is 400 when data source response has an error and feature toggle is disabled", func(t *testing.T) { req := serverFeatureDisabled.NewPostRequest("/api/ds/query", strings.NewReader(reqValid)) - webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, OrgRole: org.RoleViewer}) + webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {datasources.ActionQuery: []string{datasources.ScopeAll}}}}) resp, err := serverFeatureDisabled.SendJSON(req) require.NoError(t, err) require.NoError(t, resp.Body.Close()) @@ -84,7 +84,7 @@ func TestAPIEndpoint_Metrics_QueryMetricsV2(t *testing.T) { t.Run("Status code is 207 when data source response has an error and feature toggle is enabled", func(t *testing.T) { req := serverFeatureEnabled.NewPostRequest("/api/ds/query", strings.NewReader(reqValid)) - webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, OrgRole: org.RoleViewer}) + webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {datasources.ActionQuery: []string{datasources.ScopeAll}}}}) resp, err := serverFeatureEnabled.SendJSON(req) require.NoError(t, err) require.NoError(t, resp.Body.Close()) @@ -117,7 +117,7 @@ func TestAPIEndpoint_Metrics_PluginDecryptionFailure(t *testing.T) { t.Run("Status code is 500 and a secrets plugin error is returned if there is a problem getting secrets from the remote plugin", func(t *testing.T) { req := httpServer.NewPostRequest("/api/ds/query", strings.NewReader(reqValid)) - webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, OrgRole: org.RoleViewer}) + webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {datasources.ActionQuery: []string{datasources.ScopeAll}}}}) resp, err := httpServer.SendJSON(req) require.NoError(t, err) require.Equal(t, http.StatusInternalServerError, resp.StatusCode) @@ -266,7 +266,7 @@ func TestDataSourceQueryError(t *testing.T) { hs.QuotaService = quotatest.New(false, nil) }) req := srv.NewPostRequest("/api/ds/query", strings.NewReader(tc.request)) - webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, OrgRole: org.RoleViewer}) + webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {datasources.ActionQuery: []string{datasources.ScopeAll}}}}) resp, err := srv.SendJSON(req) require.NoError(t, err) diff --git a/pkg/api/org_test.go b/pkg/api/org_test.go index 26ecdbfcea9..fa417c1aa13 100644 --- a/pkg/api/org_test.go +++ b/pkg/api/org_test.go @@ -18,45 +18,7 @@ import ( "github.com/grafana/grafana/pkg/web/webtest" ) -func TestAPIEndpoint_GetCurrentOrg_LegacyAccessControl(t *testing.T) { - type testCase struct { - desc string - user *user.SignedInUser - expectedCode int - } - - tests := []testCase{ - { - desc: "viewer can view current org", - user: &user.SignedInUser{OrgID: 1, OrgRole: org.RoleViewer}, - expectedCode: http.StatusOK, - }, - { - desc: "unauthenticated request cannot view current org", - expectedCode: http.StatusUnauthorized, - }, - } - - for _, tt := range tests { - t.Run(tt.desc, func(t *testing.T) { - server := SetupAPITestServer(t, func(hs *HTTPServer) { - hs.orgService = &orgtest.FakeOrgService{ExpectedOrg: &org.Org{}} - }) - - req := server.NewGetRequest("/api/org/") - if tt.user != nil { - req = webtest.RequestWithSignedInUser(req, tt.user) - } - - res, err := server.Send(req) - require.NoError(t, err) - assert.Equal(t, tt.expectedCode, res.StatusCode) - require.NoError(t, res.Body.Close()) - }) - } -} - -func TestAPIEndpoint_GetCurrentOrg_RBAC(t *testing.T) { +func TestAPIEndpoint_GetCurrentOrg(t *testing.T) { type testCase struct { desc string expectedCode int @@ -92,139 +54,7 @@ func TestAPIEndpoint_GetCurrentOrg_RBAC(t *testing.T) { } } -func TestAPIEndpoint_UpdateOrg_LegacyAccessControl(t *testing.T) { - type testCase struct { - desc string - path string - body string - role org.RoleType - isGrafanaAdmin bool - expectedCode int - } - - tests := []testCase{ - { - desc: "viewer cannot update current org", - path: "/api/org", - body: `{"name": "test"}`, - role: org.RoleViewer, - expectedCode: http.StatusForbidden, - }, - { - desc: "editor cannot update current org", - path: "/api/org", - body: `{"name": "test"}`, - role: org.RoleEditor, - expectedCode: http.StatusForbidden, - }, - { - desc: "admin can update current org", - path: "/api/org", - body: `{"name": "test"}`, - role: org.RoleAdmin, - expectedCode: http.StatusOK, - }, - { - desc: "viewer cannot update address of current org", - path: "/api/org/address", - body: `{}`, - role: org.RoleViewer, - expectedCode: http.StatusForbidden, - }, - { - desc: "editor cannot update address of current org", - path: "/api/org/address", - body: `{}`, - role: org.RoleEditor, - expectedCode: http.StatusForbidden, - }, - { - desc: "admin can update address of current org", - path: "/api/org/address", - body: `{}`, - role: org.RoleAdmin, - expectedCode: http.StatusOK, - }, - { - desc: "viewer cannot update target org", - path: "/api/orgs/1", - body: `{}`, - role: org.RoleViewer, - expectedCode: http.StatusForbidden, - }, - { - desc: "editor cannot update target org", - path: "/api/orgs/1", - body: `{}`, - role: org.RoleEditor, - expectedCode: http.StatusForbidden, - }, - { - desc: "admin cannot update target org", - path: "/api/orgs/1", - body: `{}`, - role: org.RoleAdmin, - expectedCode: http.StatusForbidden, - }, - { - desc: "grafana admin can update target org", - path: "/api/orgs/1", - body: `{"name": "test"}`, - role: org.RoleAdmin, - isGrafanaAdmin: true, - expectedCode: http.StatusOK, - }, - { - desc: "viewer cannot update address of target org", - path: "/api/orgs/1/address", - body: `{}`, - role: org.RoleViewer, - expectedCode: http.StatusForbidden, - }, - { - desc: "editor cannot update address of target org", - path: "/api/orgs/1/address", - body: `{}`, - role: org.RoleEditor, - expectedCode: http.StatusForbidden, - }, - { - desc: "admin cannot update address of target org", - path: "/api/orgs/1/address", - body: `{}`, - role: org.RoleAdmin, - expectedCode: http.StatusForbidden, - }, - { - desc: "grafana admin can update address of target org", - path: "/api/orgs/1/address", - body: `{}`, - role: org.RoleAdmin, - isGrafanaAdmin: true, - expectedCode: http.StatusOK, - }, - } - - for _, tt := range tests { - t.Run(tt.desc, func(t *testing.T) { - server := SetupAPITestServer(t, func(hs *HTTPServer) { - hs.orgService = &orgtest.FakeOrgService{ExpectedOrg: &org.Org{}} - }) - - req := webtest.RequestWithSignedInUser(server.NewRequest(http.MethodPut, tt.path, strings.NewReader(tt.body)), &user.SignedInUser{ - OrgID: 1, - OrgRole: tt.role, - IsGrafanaAdmin: tt.isGrafanaAdmin, - }) - res, err := server.SendJSON(req) - require.NoError(t, err) - assert.Equal(t, tt.expectedCode, res.StatusCode) - require.NoError(t, res.Body.Close()) - }) - } -} - -func TestAPIEndpoint_UpdateOrg_RBAC(t *testing.T) { +func TestAPIEndpoint_UpdateOrg(t *testing.T) { type testCase struct { desc string path string @@ -317,72 +147,7 @@ func TestAPIEndpoint_UpdateOrg_RBAC(t *testing.T) { } } -func TestAPIEndpoint_CreateOrgs_LegacyAccessControl(t *testing.T) { - type testCase struct { - desc string - role org.RoleType - isGrafanaAdmin bool - anyoneCanCreate bool - expectedCode int - } - - tests := []testCase{ - { - desc: "viewer cannot create org", - role: org.RoleViewer, - expectedCode: http.StatusForbidden, - }, - { - desc: "editor cannot create org", - role: org.RoleEditor, - expectedCode: http.StatusForbidden, - }, - { - desc: "admin cannot create org", - role: org.RoleAdmin, - expectedCode: http.StatusForbidden, - }, - { - desc: "grafana admin can create org", - role: org.RoleViewer, - isGrafanaAdmin: true, - expectedCode: http.StatusOK, - }, - { - desc: "viewer can create org when AllowUserOrgCreate is set to true", - role: org.RoleViewer, - isGrafanaAdmin: true, - anyoneCanCreate: true, - expectedCode: http.StatusOK, - }, - } - - for _, tt := range tests { - t.Run(tt.desc, func(t *testing.T) { - server := SetupAPITestServer(t, func(hs *HTTPServer) { - hs.orgService = &orgtest.FakeOrgService{ExpectedOrg: &org.Org{}} - }) - - prev := setting.AllowUserOrgCreate - defer func() { - setting.AllowUserOrgCreate = prev - }() - setting.AllowUserOrgCreate = tt.anyoneCanCreate - - req := webtest.RequestWithSignedInUser(server.NewPostRequest("/api/orgs", strings.NewReader(`{"name": "test"}`)), &user.SignedInUser{ - OrgID: 1, - OrgRole: tt.role, - IsGrafanaAdmin: tt.isGrafanaAdmin, - }) - res, err := server.SendJSON(req) - require.NoError(t, err) - assert.Equal(t, tt.expectedCode, res.StatusCode) - require.NoError(t, res.Body.Close()) - }) - } -} - -func TestAPIEndpoint_CreateOrgs_RBAC(t *testing.T) { +func TestAPIEndpoint_CreateOrgs(t *testing.T) { type testCase struct { desc string permission []accesscontrol.Permission @@ -422,58 +187,7 @@ func TestAPIEndpoint_CreateOrgs_RBAC(t *testing.T) { } } -func TestAPIEndpoint_DeleteOrgs_LegacyAccessControl2(t *testing.T) { - type testCase struct { - desc string - role org.RoleType - isGrafanaAdmin bool - expectedCode int - } - - tests := []testCase{ - { - desc: "viewer cannot delete org", - role: org.RoleViewer, - expectedCode: http.StatusForbidden, - }, - { - desc: "editor cannot delete org", - role: org.RoleEditor, - expectedCode: http.StatusForbidden, - }, - { - desc: "admin cannot delete org", - role: org.RoleAdmin, - expectedCode: http.StatusForbidden, - }, - { - desc: "grafana admin can delete org", - role: org.RoleViewer, - isGrafanaAdmin: true, - expectedCode: http.StatusOK, - }, - } - - for _, tt := range tests { - t.Run(tt.desc, func(t *testing.T) { - server := SetupAPITestServer(t, func(hs *HTTPServer) { - hs.orgService = &orgtest.FakeOrgService{ExpectedOrg: &org.Org{}} - }) - - req := webtest.RequestWithSignedInUser(server.NewRequest(http.MethodDelete, "/api/orgs/1", nil), &user.SignedInUser{ - OrgID: 2, - OrgRole: tt.role, - IsGrafanaAdmin: tt.isGrafanaAdmin, - }) - res, err := server.Send(req) - require.NoError(t, err) - assert.Equal(t, tt.expectedCode, res.StatusCode) - require.NoError(t, res.Body.Close()) - }) - } -} - -func TestAPIEndpoint_DeleteOrgs_RBAC(t *testing.T) { +func TestAPIEndpoint_DeleteOrgs(t *testing.T) { type testCase struct { desc string permission []accesscontrol.Permission @@ -511,69 +225,7 @@ func TestAPIEndpoint_DeleteOrgs_RBAC(t *testing.T) { } } -func TestAPIEndpoint_GetOrg_LegacyAccessControl(t *testing.T) { - type testCase struct { - desc string - role org.RoleType - isGrafanaAdmin bool - expectedCode int - } - - tests := []testCase{ - { - desc: "should not be able to fetch org as viewer", - role: org.RoleViewer, - isGrafanaAdmin: false, - expectedCode: http.StatusForbidden, - }, - { - desc: "should not be able to fetch org as editor", - role: org.RoleEditor, - isGrafanaAdmin: false, - expectedCode: http.StatusForbidden, - }, - { - desc: "should not be able to search org as amin", - role: org.RoleAdmin, - isGrafanaAdmin: false, - expectedCode: http.StatusForbidden, - }, - { - desc: "should be able to fetch org as grafana admin", - role: org.RoleViewer, - isGrafanaAdmin: true, - expectedCode: http.StatusOK, - }, - } - - for _, tt := range tests { - t.Run(tt.desc, func(t *testing.T) { - server := SetupAPITestServer(t, func(hs *HTTPServer) { - hs.orgService = &orgtest.FakeOrgService{ExpectedOrg: &org.Org{}} - }) - - verify := func(path string) { - req := webtest.RequestWithSignedInUser(server.NewGetRequest(path), &user.SignedInUser{ - OrgID: 1, - OrgRole: tt.role, - IsGrafanaAdmin: tt.isGrafanaAdmin, - }) - res, err := server.Send(req) - require.NoError(t, err) - assert.Equal(t, tt.expectedCode, res.StatusCode) - require.NoError(t, res.Body.Close()) - } - // search orgs - verify("/api/orgs") - // fetch by id - verify("/api/orgs/1") - // fetch by name - verify("/api/orgs/name/test") - }) - } -} - -func TestAPIEndpoint_GetOrg_RBAC(t *testing.T) { +func TestAPIEndpoint_GetOrg(t *testing.T) { type testCase struct { desc string permissions []accesscontrol.Permission diff --git a/pkg/api/org_users_test.go b/pkg/api/org_users_test.go index ce5c3b0d6be..26f44466428 100644 --- a/pkg/api/org_users_test.go +++ b/pkg/api/org_users_test.go @@ -10,7 +10,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/api/dtos" @@ -19,7 +18,6 @@ import ( "github.com/grafana/grafana/pkg/models/roletype" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/actest" - "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/login/logintest" @@ -29,7 +27,6 @@ import ( "github.com/grafana/grafana/pkg/services/quota/quotaimpl" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest" - "github.com/grafana/grafana/pkg/services/team/teamtest" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user/userimpl" "github.com/grafana/grafana/pkg/services/user/usertest" @@ -296,72 +293,7 @@ func TestOrgUsersAPIEndpoint_updateOrgRole(t *testing.T) { } } -func TestOrgUsersAPIEndpoint_LegacyAccessControl(t *testing.T) { - type testCase struct { - desc string - isTeamAdmin bool - isFolderAdmin bool - role org.RoleType - expectedCode int - } - - tests := []testCase{ - { - desc: "should be able to search org user when user is folder admin", - isFolderAdmin: true, - role: org.RoleViewer, - expectedCode: http.StatusOK, - }, - { - desc: "should be able to search org user when user is team admin", - isFolderAdmin: true, - role: org.RoleViewer, - expectedCode: http.StatusOK, - }, - { - desc: "should be able to search org user when user is admin", - role: org.RoleAdmin, - expectedCode: http.StatusOK, - }, - { - desc: "should not be able to search org user when user is viewer", - role: org.RoleViewer, - expectedCode: http.StatusForbidden, - }, - { - desc: "should not be able to search org user when user is editor", - role: org.RoleEditor, - expectedCode: http.StatusForbidden, - }, - } - - for _, tt := range tests { - t.Run(tt.desc, func(t *testing.T) { - server := SetupAPITestServer(t, func(hs *HTTPServer) { - cfg := setting.NewCfg() - cfg.RBACEnabled = false - hs.Cfg = cfg - - dashboardService := dashboards.NewFakeDashboardService(t) - dashboardService.On("HasAdminPermissionInDashboardsOrFolders", mock.Anything, mock.Anything).Return(tt.isFolderAdmin, nil).Maybe() - hs.DashboardService = dashboardService - - teamService := teamtest.NewFakeService() - teamService.ExpectedIsAdmin = tt.isTeamAdmin - hs.teamService = teamService - hs.orgService = &orgtest.FakeOrgService{ExpectedSearchOrgUsersResult: &org.SearchOrgUsersQueryResult{}} - hs.authInfoService = &logintest.AuthInfoServiceFake{} - }) - - res, err := server.Send(webtest.RequestWithSignedInUser(server.NewGetRequest("/api/org/users/lookup"), &user.SignedInUser{OrgID: 1, OrgRole: tt.role})) - require.NoError(t, err) - assert.Equal(t, tt.expectedCode, res.StatusCode) - require.NoError(t, res.Body.Close()) - }) - } -} - -func TestOrgUsersAPIEndpoint_AccessControl(t *testing.T) { +func TestOrgUsersAPIEndpoint(t *testing.T) { type testCase struct { desc string permissions []accesscontrol.Permission @@ -473,46 +405,15 @@ func TestGetOrgUsersAPIEndpoint_AccessControlMetadata(t *testing.T) { func TestGetOrgUsersAPIEndpoint_AccessControl(t *testing.T) { type testCase struct { - name string - enableAccessControl bool - role org.RoleType - isGrafanaAdmin bool - permissions []accesscontrol.Permission - expectedCode int - targetOrg int64 + name string + permissions []accesscontrol.Permission + expectedCode int + targetOrg int64 } tests := []testCase{ { - name: "server admin can get users in his org (legacy)", - enableAccessControl: false, - role: org.RoleViewer, - isGrafanaAdmin: true, - expectedCode: http.StatusOK, - targetOrg: 1, - }, - { - name: "server admin can get users in another org (legacy)", - enableAccessControl: false, - isGrafanaAdmin: true, - expectedCode: http.StatusOK, - targetOrg: 2, - }, - { - name: "org admin cannot get users in his org (legacy)", - enableAccessControl: false, - expectedCode: http.StatusForbidden, - targetOrg: 1, - }, - { - name: "org admin cannot get users in another org (legacy)", - enableAccessControl: false, - expectedCode: http.StatusForbidden, - targetOrg: 1, - }, - { - name: "user with permissions can get users in org", - enableAccessControl: true, + name: "user with permissions can get users in org", permissions: []accesscontrol.Permission{ {Action: accesscontrol.ActionOrgUsersRead, Scope: "users:*"}, }, @@ -520,17 +421,15 @@ func TestGetOrgUsersAPIEndpoint_AccessControl(t *testing.T) { targetOrg: 1, }, { - name: "user without permissions cannot get users in org", - enableAccessControl: true, - expectedCode: http.StatusForbidden, - targetOrg: 1, + name: "user without permissions cannot get users in org", + expectedCode: http.StatusForbidden, + targetOrg: 1, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { server := SetupAPITestServer(t, func(hs *HTTPServer) { hs.Cfg = setting.NewCfg() - hs.Cfg.RBACEnabled = tc.enableAccessControl hs.orgService = &orgtest.FakeOrgService{ ExpectedSearchOrgUsersResult: &org.SearchOrgUsersQueryResult{}, } @@ -539,8 +438,6 @@ func TestGetOrgUsersAPIEndpoint_AccessControl(t *testing.T) { }) u := userWithPermissions(1, tc.permissions) - u.OrgRole = tc.role - u.IsGrafanaAdmin = tc.isGrafanaAdmin res, err := server.Send(webtest.RequestWithSignedInUser(server.NewGetRequest(fmt.Sprintf("/api/orgs/%d/users/", tc.targetOrg)), u)) require.NoError(t, err) @@ -552,34 +449,15 @@ func TestGetOrgUsersAPIEndpoint_AccessControl(t *testing.T) { func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) { type testCase struct { - desc string - enableAccessControl bool - permissions []accesscontrol.Permission - isGrafanaAdmin bool - role org.RoleType - input string - expectedCode int + desc string + permissions []accesscontrol.Permission + input string + expectedCode int } tests := []testCase{ { - desc: "server admin can add users to his org (legacy)", - enableAccessControl: false, - isGrafanaAdmin: true, - input: `{"loginOrEmail": "user", "role": "Viewer"}`, - expectedCode: http.StatusOK, - }, - { - desc: "org admin cannot add users to his org (legacy)", - enableAccessControl: false, - role: org.RoleAdmin, - expectedCode: http.StatusForbidden, - input: `{"loginOrEmail": "user", "role": "Viewer"}`, - }, - { - desc: "user with permissions can add users to org", - enableAccessControl: true, - role: org.RoleViewer, + desc: "user with permissions can add users to org", permissions: []accesscontrol.Permission{ {Action: accesscontrol.ActionOrgUsersAdd, Scope: "users:*"}, }, @@ -587,17 +465,15 @@ func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) { expectedCode: http.StatusOK, }, { - desc: "user without permissions cannot add users to org", - enableAccessControl: true, - expectedCode: http.StatusForbidden, - input: `{"loginOrEmail": "user", "role": "Viewer"}`, + desc: "user without permissions cannot add users to org", + expectedCode: http.StatusForbidden, + input: `{"loginOrEmail": "user", "role": "Viewer"}`, }, } for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { server := SetupAPITestServer(t, func(hs *HTTPServer) { hs.Cfg = setting.NewCfg() - hs.Cfg.RBACEnabled = tt.enableAccessControl hs.orgService = &orgtest.FakeOrgService{} hs.authInfoService = &logintest.AuthInfoServiceFake{} hs.userService = &usertest.FakeUserService{ @@ -607,8 +483,6 @@ func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) { }) u := userWithPermissions(1, tt.permissions) - u.OrgRole = tt.role - u.IsGrafanaAdmin = tt.isGrafanaAdmin res, err := server.SendJSON(webtest.RequestWithSignedInUser(server.NewPostRequest("/api/orgs/1/users", strings.NewReader(tt.input)), u)) require.NoError(t, err) @@ -753,59 +627,39 @@ func TestOrgUsersAPIEndpointWithSetPerms_AccessControl(t *testing.T) { func TestPatchOrgUsersAPIEndpoint_AccessControl(t *testing.T) { type testCase struct { - name string - enableAccessControl bool - isGrafanaAdmin bool - role org.RoleType - permissions []accesscontrol.Permission - input string - expectedCode int + name string + role org.RoleType + permissions []accesscontrol.Permission + input string + expectedCode int } tests := []testCase{ { - name: "server admin can update users in his org (legacy)", - enableAccessControl: false, - isGrafanaAdmin: true, - input: `{"role": "Viewer"}`, - expectedCode: http.StatusOK, + name: "user with permissions can update org role", + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionOrgUsersWrite, Scope: "users:*"}}, + role: org.RoleAdmin, + input: `{"role": "Viewer"}`, + expectedCode: http.StatusOK, }, { - name: "org admin cannot update users in his org (legacy)", - enableAccessControl: false, - role: org.RoleAdmin, - input: `{"role": "Editor"}`, - expectedCode: http.StatusForbidden, + name: "user without permissions cannot update org role", + permissions: []accesscontrol.Permission{}, + input: `{"role": "Editor"}`, + expectedCode: http.StatusForbidden, }, { - name: "user with permissions can update org role", - enableAccessControl: true, - permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionOrgUsersWrite, Scope: "users:*"}}, - role: org.RoleAdmin, - input: `{"role": "Viewer"}`, - expectedCode: http.StatusOK, - }, - { - name: "user without permissions cannot update org role", - enableAccessControl: true, - permissions: []accesscontrol.Permission{}, - input: `{"role": "Editor"}`, - expectedCode: http.StatusForbidden, - }, - { - name: "user with permissions cannot update org role with more privileges", - enableAccessControl: true, - permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionOrgUsersWrite, Scope: "users:*"}}, - role: org.RoleViewer, - input: `{"role": "Admin"}`, - expectedCode: http.StatusForbidden, + name: "user with permissions cannot update org role with more privileges", + permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionOrgUsersWrite, Scope: "users:*"}}, + role: org.RoleViewer, + input: `{"role": "Admin"}`, + expectedCode: http.StatusForbidden, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { server := SetupAPITestServer(t, func(hs *HTTPServer) { hs.Cfg = setting.NewCfg() - hs.Cfg.RBACEnabled = tt.enableAccessControl hs.orgService = &orgtest.FakeOrgService{} hs.authInfoService = &logintest.AuthInfoServiceFake{ ExpectedUserAuth: &login.UserAuth{ @@ -820,59 +674,39 @@ func TestPatchOrgUsersAPIEndpoint_AccessControl(t *testing.T) { }) u := userWithPermissions(1, tt.permissions) - u.IsGrafanaAdmin = tt.isGrafanaAdmin res, err := server.SendJSON(webtest.RequestWithSignedInUser(server.NewRequest(http.MethodPatch, "/api/orgs/1/users/1", strings.NewReader(tt.input)), u)) require.NoError(t, err) assert.Equal(t, tt.expectedCode, res.StatusCode) require.NoError(t, res.Body.Close()) - - cfg := setting.NewCfg() - cfg.RBACEnabled = tt.enableAccessControl }) } } func TestDeleteOrgUsersAPIEndpoint_AccessControl(t *testing.T) { type testCase struct { - name string - enableAccessControl bool - permissions []accesscontrol.Permission - role org.RoleType - isGrafanaAdmin bool - expectedCode int + name string + permissions []accesscontrol.Permission + isGrafanaAdmin bool + expectedCode int } tests := []testCase{ { - name: "server admin can remove users from org (legacy)", - enableAccessControl: false, - isGrafanaAdmin: true, - expectedCode: http.StatusOK, - }, - { - name: "org admin can remove users from org (legacy)", - role: org.RoleAdmin, - expectedCode: http.StatusForbidden, - }, - { - name: "user with permissions can remove user from org", - enableAccessControl: true, + name: "user with permissions can remove user from org", permissions: []accesscontrol.Permission{ {Action: accesscontrol.ActionOrgUsersRemove, Scope: "users:*"}, }, expectedCode: http.StatusOK, }, { - name: "user without permissions cannot remove user from org", - enableAccessControl: true, - expectedCode: http.StatusForbidden, + name: "user without permissions cannot remove user from org", + expectedCode: http.StatusForbidden, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { server := SetupAPITestServer(t, func(hs *HTTPServer) { hs.Cfg = setting.NewCfg() - hs.Cfg.RBACEnabled = tt.enableAccessControl hs.accesscontrolService = actest.FakeService{} hs.orgService = &orgtest.FakeOrgService{ ExpectedOrgListResponse: orgtest.OrgListResponse{struct { @@ -893,9 +727,6 @@ func TestDeleteOrgUsersAPIEndpoint_AccessControl(t *testing.T) { require.NoError(t, err) assert.Equal(t, tt.expectedCode, res.StatusCode) require.NoError(t, res.Body.Close()) - - cfg := setting.NewCfg() - cfg.RBACEnabled = tt.enableAccessControl }) } } diff --git a/pkg/api/plugins_test.go b/pkg/api/plugins_test.go index 9cf95c431d2..69a3fffc5c6 100644 --- a/pkg/api/plugins_test.go +++ b/pkg/api/plugins_test.go @@ -5,7 +5,6 @@ import ( "encoding/json" "errors" "fmt" - "io" "net/http" "net/http/httptest" "os" @@ -35,82 +34,12 @@ import ( "github.com/grafana/grafana/pkg/services/org/orgtest" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginaccesscontrol" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginsettings" - "github.com/grafana/grafana/pkg/services/quota/quotatest" "github.com/grafana/grafana/pkg/services/updatechecker" - "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web/webtest" ) func Test_PluginsInstallAndUninstall(t *testing.T) { - type tc struct { - pluginAdminEnabled bool - pluginAdminExternalManageEnabled bool - expectedHTTPStatus int - expectedHTTPBody string - } - tcs := []tc{ - {pluginAdminEnabled: true, pluginAdminExternalManageEnabled: true, expectedHTTPStatus: 404, expectedHTTPBody: "404 page not found\n"}, - {pluginAdminEnabled: true, pluginAdminExternalManageEnabled: false, expectedHTTPStatus: 200, expectedHTTPBody: ""}, - {pluginAdminEnabled: false, pluginAdminExternalManageEnabled: true, expectedHTTPStatus: 404, expectedHTTPBody: "404 page not found\n"}, - {pluginAdminEnabled: false, pluginAdminExternalManageEnabled: false, expectedHTTPStatus: 404, expectedHTTPBody: "404 page not found\n"}, - } - - testName := func(action string, testCase tc) string { - return fmt.Sprintf("%s request returns %d when adminEnabled: %t and externalEnabled: %t", - action, testCase.expectedHTTPStatus, testCase.pluginAdminEnabled, testCase.pluginAdminExternalManageEnabled) - } - - inst := NewFakePluginInstaller() - for _, tc := range tcs { - srv := SetupAPITestServer(t, func(hs *HTTPServer) { - hs.Cfg = &setting.Cfg{ - PluginAdminEnabled: tc.pluginAdminEnabled, - PluginAdminExternalManageEnabled: tc.pluginAdminExternalManageEnabled, - } - hs.pluginInstaller = inst - hs.QuotaService = quotatest.New(false, nil) - }) - - t.Run(testName("Install", tc), func(t *testing.T) { - req := srv.NewPostRequest("/api/plugins/test/install", strings.NewReader("{ \"version\": \"1.0.2\" }")) - webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, OrgRole: org.RoleEditor, IsGrafanaAdmin: true}) - resp, err := srv.SendJSON(req) - require.NoError(t, err) - - body := new(strings.Builder) - _, err = io.Copy(body, resp.Body) - require.NoError(t, err) - require.Equal(t, tc.expectedHTTPBody, body.String()) - require.NoError(t, resp.Body.Close()) - require.Equal(t, tc.expectedHTTPStatus, resp.StatusCode) - - if tc.expectedHTTPStatus == 200 { - require.Equal(t, fakePlugin{pluginID: "test", version: "1.0.2"}, inst.plugins["test"]) - } - }) - - t.Run(testName("Uninstall", tc), func(t *testing.T) { - req := srv.NewPostRequest("/api/plugins/test/uninstall", strings.NewReader("{}")) - webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, OrgRole: org.RoleViewer, IsGrafanaAdmin: true}) - resp, err := srv.SendJSON(req) - require.NoError(t, err) - - body := new(strings.Builder) - _, err = io.Copy(body, resp.Body) - require.NoError(t, err) - require.Equal(t, tc.expectedHTTPBody, body.String()) - require.NoError(t, resp.Body.Close()) - require.Equal(t, tc.expectedHTTPStatus, resp.StatusCode) - - if tc.expectedHTTPStatus == 200 { - require.Empty(t, inst.plugins) - } - }) - } -} - -func Test_PluginsInstallAndUninstall_AccessControl(t *testing.T) { canInstall := []ac.Permission{{Action: pluginaccesscontrol.ActionInstall}} cannotInstall := []ac.Permission{{Action: "plugins:cannotinstall"}} diff --git a/pkg/api/preferences_test.go b/pkg/api/preferences_test.go index 4d8a1c9fc84..ffc706a3b07 100644 --- a/pkg/api/preferences_test.go +++ b/pkg/api/preferences_test.go @@ -1,8 +1,6 @@ package api import ( - "encoding/json" - "io" "net/http" "strings" "testing" @@ -35,45 +33,7 @@ var ( testUpdateOrgPreferencesWithHomeDashboardUIDCmd = `{ "theme": "light", "homeDashboardUID": "home"}` ) -func TestAPIEndpoint_GetCurrentOrgPreferences_LegacyAccessControl(t *testing.T) { - cfg := setting.NewCfg() - cfg.RBACEnabled = false - dashSvc := dashboards.NewFakeDashboardService(t) - qResult := &dashboards.Dashboard{UID: "home", ID: 1} - dashSvc.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(qResult, nil) - - prefService := preftest.NewPreferenceServiceFake() - prefService.ExpectedPreference = &pref.Preference{HomeDashboardID: 1, Theme: "dark"} - - server := SetupAPITestServer(t, func(hs *HTTPServer) { - hs.Cfg = cfg - hs.DashboardService = dashSvc - hs.preferenceService = prefService - }) - - t.Run("Viewer cannot get org preferences", func(t *testing.T) { - req := webtest.RequestWithSignedInUser(server.NewGetRequest(getOrgPreferencesURL), &user.SignedInUser{OrgID: 1, OrgRole: org.RoleViewer}) - res, err := server.Send(req) - require.NoError(t, err) - assert.Equal(t, http.StatusForbidden, res.StatusCode) - require.NoError(t, res.Body.Close()) - }) - - t.Run("Org Admin can get org preferences", func(t *testing.T) { - req := webtest.RequestWithSignedInUser(server.NewGetRequest(getOrgPreferencesURL), &user.SignedInUser{OrgID: 1, OrgRole: org.RoleAdmin}) - res, err := server.Send(req) - require.NoError(t, err) - assert.Equal(t, http.StatusOK, res.StatusCode) - var resp map[string]interface{} - b, err := io.ReadAll(res.Body) - assert.NoError(t, err) - assert.NoError(t, json.Unmarshal(b, &resp)) - assert.Equal(t, "home", resp["homeDashboardUID"]) - require.NoError(t, res.Body.Close()) - }) -} - -func TestAPIEndpoint_GetCurrentOrgPreferences_AccessControl(t *testing.T) { +func TestAPIEndpoint_GetCurrentOrgPreferences(t *testing.T) { prefService := preftest.NewPreferenceServiceFake() prefService.ExpectedPreference = &pref.Preference{HomeDashboardID: 1, Theme: "dark"} @@ -114,41 +74,7 @@ func TestAPIEndpoint_GetCurrentOrgPreferences_AccessControl(t *testing.T) { }) } -func TestAPIEndpoint_PutCurrentOrgPreferences_LegacyAccessControl(t *testing.T) { - cfg := setting.NewCfg() - cfg.RBACEnabled = false - - server := SetupAPITestServer(t, func(hs *HTTPServer) { - hs.Cfg = cfg - hs.preferenceService = preftest.NewPreferenceServiceFake() - }) - - input := strings.NewReader(testUpdateOrgPreferencesCmd) - t.Run("Viewer cannot update org preferences", func(t *testing.T) { - req := webtest.RequestWithSignedInUser(server.NewRequest(http.MethodPut, putOrgPreferencesURL, input), &user.SignedInUser{ - OrgID: 1, - OrgRole: org.RoleViewer, - }) - response, err := server.SendJSON(req) - require.NoError(t, err) - assert.Equal(t, http.StatusForbidden, response.StatusCode) - require.NoError(t, response.Body.Close()) - }) - - input = strings.NewReader(testUpdateOrgPreferencesCmd) - t.Run("Org Admin can update org preferences", func(t *testing.T) { - req := webtest.RequestWithSignedInUser(server.NewRequest(http.MethodPut, putOrgPreferencesURL, input), &user.SignedInUser{ - OrgID: 1, - OrgRole: org.RoleAdmin, - }) - response, err := server.SendJSON(req) - require.NoError(t, err) - assert.Equal(t, http.StatusOK, response.StatusCode) - require.NoError(t, response.Body.Close()) - }) -} - -func TestAPIEndpoint_PutCurrentOrgPreferences_AccessControl(t *testing.T) { +func TestAPIEndpoint_PutCurrentOrgPreferences(t *testing.T) { prefService := preftest.NewPreferenceServiceFake() prefService.ExpectedPreference = &pref.Preference{HomeDashboardID: 1, Theme: "dark"} diff --git a/pkg/api/quota_test.go b/pkg/api/quota_test.go index 230e45b29a7..18653fdbb21 100644 --- a/pkg/api/quota_test.go +++ b/pkg/api/quota_test.go @@ -10,7 +10,6 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/services/accesscontrol" - "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user/usertest" "github.com/grafana/grafana/pkg/setting" @@ -25,31 +24,7 @@ var ( testUpdateOrgQuotaCmd = `{ "limit": 20 }` ) -func TestAPIEndpoint_GetCurrentOrgQuotas_LegacyAccessControl(t *testing.T) { - cfg := setting.NewCfg() - cfg.Quota.Enabled = true - cfg.RBACEnabled = false - server := SetupAPITestServer(t, func(hs *HTTPServer) { - hs.Cfg = cfg - }) - - t.Run("Unsigned user cannot view CurrentOrgQuotas", func(t *testing.T) { - req := server.NewGetRequest(getCurrentOrgQuotasURL) - res, err := server.Send(req) - require.NoError(t, err) - assert.Equal(t, http.StatusUnauthorized, res.StatusCode) - require.NoError(t, res.Body.Close()) - }) - t.Run("Viewer can view CurrentOrgQuotas", func(t *testing.T) { - req := webtest.RequestWithSignedInUser(server.NewGetRequest(getCurrentOrgQuotasURL), &user.SignedInUser{OrgID: 1, OrgRole: org.RoleViewer}) - res, err := server.Send(req) - require.NoError(t, err) - assert.Equal(t, http.StatusOK, res.StatusCode) - require.NoError(t, res.Body.Close()) - }) -} - -func TestAPIEndpoint_GetCurrentOrgQuotas_AccessControl(t *testing.T) { +func TestAPIEndpoint_GetCurrentOrgQuotas(t *testing.T) { cfg := setting.NewCfg() cfg.Quota.Enabled = true server := SetupAPITestServer(t, func(hs *HTTPServer) { @@ -82,32 +57,7 @@ func TestAPIEndpoint_GetCurrentOrgQuotas_AccessControl(t *testing.T) { }) } -func TestAPIEndpoint_GetOrgQuotas_LegacyAccessControl(t *testing.T) { - cfg := setting.NewCfg() - cfg.Quota.Enabled = true - cfg.RBACEnabled = false - server := SetupAPITestServer(t, func(hs *HTTPServer) { - hs.Cfg = cfg - }) - - t.Run("Viewer cannot view another org quotas", func(t *testing.T) { - req := webtest.RequestWithSignedInUser(server.NewGetRequest(fmt.Sprintf(getOrgsQuotasURL, 2)), &user.SignedInUser{OrgID: 1, OrgRole: org.RoleViewer}) - res, err := server.Send(req) - require.NoError(t, err) - assert.Equal(t, http.StatusForbidden, res.StatusCode) - require.NoError(t, res.Body.Close()) - }) - - t.Run("Grafana admin viewer can view another org quotas", func(t *testing.T) { - req := webtest.RequestWithSignedInUser(server.NewGetRequest(fmt.Sprintf(getOrgsQuotasURL, 2)), &user.SignedInUser{OrgID: 1, OrgRole: org.RoleViewer, IsGrafanaAdmin: true}) - res, err := server.Send(req) - require.NoError(t, err) - assert.Equal(t, http.StatusOK, res.StatusCode) - require.NoError(t, res.Body.Close()) - }) -} - -func TestAPIEndpoint_GetOrgQuotas_AccessControl(t *testing.T) { +func TestAPIEndpoint_GetOrgQuotas(t *testing.T) { cfg := setting.NewCfg() cfg.Quota.Enabled = true server := SetupAPITestServer(t, func(hs *HTTPServer) { @@ -143,41 +93,7 @@ func TestAPIEndpoint_GetOrgQuotas_AccessControl(t *testing.T) { }) } -func TestAPIEndpoint_PutOrgQuotas_LegacyAccessControl(t *testing.T) { - cfg := setting.NewCfg() - cfg.Quota.Enabled = true - cfg.RBACEnabled = false - server := SetupAPITestServer(t, func(hs *HTTPServer) { - hs.Cfg = cfg - }) - - input := strings.NewReader(testUpdateOrgQuotaCmd) - t.Run("Viewer cannot update another org quotas", func(t *testing.T) { - req := webtest.RequestWithSignedInUser(server.NewRequest(http.MethodPut, fmt.Sprintf(putOrgsQuotasURL, 2, "org_user"), input), &user.SignedInUser{ - OrgID: 1, - OrgRole: org.RoleViewer, - }) - response, err := server.SendJSON(req) - require.NoError(t, err) - assert.Equal(t, http.StatusForbidden, response.StatusCode) - require.NoError(t, response.Body.Close()) - }) - - input = strings.NewReader(testUpdateOrgQuotaCmd) - t.Run("Grafana admin viewer can update another org quotas", func(t *testing.T) { - req := webtest.RequestWithSignedInUser(server.NewRequest(http.MethodPut, fmt.Sprintf(putOrgsQuotasURL, 2, "org_user"), input), &user.SignedInUser{ - OrgID: 1, - OrgRole: org.RoleViewer, - IsGrafanaAdmin: true, - }) - response, err := server.SendJSON(req) - require.NoError(t, err) - assert.Equal(t, http.StatusOK, response.StatusCode) - require.NoError(t, response.Body.Close()) - }) -} - -func TestAPIEndpoint_PutOrgQuotas_AccessControl(t *testing.T) { +func TestAPIEndpoint_PutOrgQuotas(t *testing.T) { cfg := setting.NewCfg() cfg.Quota = setting.QuotaSettings{ Enabled: true, diff --git a/pkg/infra/usagestats/service/api_test.go b/pkg/infra/usagestats/service/api_test.go index 4aa7ce51837..7f45b7ca97d 100644 --- a/pkg/infra/usagestats/service/api_test.go +++ b/pkg/infra/usagestats/service/api_test.go @@ -23,37 +23,37 @@ func TestApi_getUsageStats(t *testing.T) { type getUsageStatsTestCase struct { desc string expectedStatus int - IsGrafanaAdmin bool + permissions map[string][]string enabled bool } tests := []getUsageStatsTestCase{ { desc: "expect usage stats", enabled: true, - IsGrafanaAdmin: true, + permissions: map[string][]string{ActionRead: {}}, expectedStatus: 200, }, { desc: "expect usage stat preview still there after disabling", enabled: false, - IsGrafanaAdmin: true, + permissions: map[string][]string{ActionRead: {}}, expectedStatus: 200, }, { - desc: "expect http status 403 when not admin", + desc: "expect http status 403 when does not have the right permissions", enabled: false, - IsGrafanaAdmin: false, + permissions: map[string][]string{}, expectedStatus: 403, }, } sqlStore := dbtest.NewFakeDB() - uss := createService(t, setting.Cfg{}, sqlStore, false) + uss := createService(t, sqlStore, false) uss.registerAPIEndpoints() for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { uss.Cfg.ReportingEnabled = tt.enabled - server := setupTestServer(t, &user.SignedInUser{OrgID: 1, IsGrafanaAdmin: tt.IsGrafanaAdmin}, uss) + server := setupTestServer(t, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: tt.permissions}}, uss) usageStats, recorder := getUsageStats(t, server) require.Equal(t, tt.expectedStatus, recorder.Code) diff --git a/pkg/infra/usagestats/service/usage_stats_test.go b/pkg/infra/usagestats/service/usage_stats_test.go index a5d1e30f496..0f8d31a5366 100644 --- a/pkg/infra/usagestats/service/usage_stats_test.go +++ b/pkg/infra/usagestats/service/usage_stats_test.go @@ -21,6 +21,7 @@ import ( "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/infra/usagestats" + "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" "github.com/grafana/grafana/pkg/services/accesscontrol/actest" "github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest" "github.com/grafana/grafana/pkg/setting" @@ -41,7 +42,7 @@ func TestMetrics(t *testing.T) { const metricName = "stats.test_metric.count" sqlStore := dbtest.NewFakeDB() - uss := createService(t, setting.Cfg{}, sqlStore, false) + uss := createService(t, sqlStore, false) uss.RegisterMetricsFunc(func(context.Context) (map[string]interface{}, error) { return map[string]interface{}{metricName: 1}, nil @@ -150,7 +151,7 @@ func TestMetrics(t *testing.T) { func TestGetUsageReport_IncludesMetrics(t *testing.T) { sqlStore := dbtest.NewFakeDB() - uss := createService(t, setting.Cfg{}, sqlStore, true) + uss := createService(t, sqlStore, true) metricName := "stats.test_metric.count" uss.RegisterMetricsFunc(func(context.Context) (map[string]interface{}, error) { @@ -168,7 +169,7 @@ func TestRegisterMetrics(t *testing.T) { const goodMetricName = "stats.test_external_metric.count" sqlStore := dbtest.NewFakeDB() - uss := createService(t, setting.Cfg{}, sqlStore, false) + uss := createService(t, sqlStore, false) metrics := map[string]interface{}{"stats.test_metric.count": 1, "stats.test_metric_second.count": 2} uss.RegisterMetricsFunc(func(context.Context) (map[string]interface{}, error) { @@ -209,18 +210,19 @@ type httpResp struct { err error } -func createService(t *testing.T, cfg setting.Cfg, sqlStore db.DB, withDB bool) *UsageStats { +func createService(t *testing.T, sqlStore db.DB, withDB bool) *UsageStats { t.Helper() if withDB { sqlStore = db.InitTestDB(t) } + cfg := setting.NewCfg() service, _ := ProvideService( - &cfg, + cfg, kvstore.ProvideService(sqlStore), routing.NewRouteRegister(), tracing.InitializeTracerForTest(), - actest.FakeAccessControl{ExpectedDisabled: true}, + acimpl.ProvideAccessControl(cfg), actest.FakeService{}, supportbundlestest.NewFakeBundleService(), ) diff --git a/pkg/services/dashboardimport/api/api_test.go b/pkg/services/dashboardimport/api/api_test.go index 4752f4c27ff..84a5636575e 100644 --- a/pkg/services/dashboardimport/api/api_test.go +++ b/pkg/services/dashboardimport/api/api_test.go @@ -11,9 +11,10 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/components/simplejson" - acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" + "github.com/grafana/grafana/pkg/services/accesscontrol/actest" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/dashboardimport" + "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/quota" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/web/webtest" @@ -29,7 +30,7 @@ func TestImportDashboardAPI(t *testing.T) { }, } - importDashboardAPI := New(service, quotaServiceFunc(quotaNotReached), nil, acmock.New().WithDisabled()) + importDashboardAPI := New(service, quotaServiceFunc(quotaNotReached), nil, actest.FakeAccessControl{ExpectedEvaluate: true}) routeRegister := routing.NewRouteRegister() importDashboardAPI.RegisterAPIEndpoints(routeRegister) s := webtest.NewServer(t, routeRegister) @@ -106,7 +107,7 @@ func TestImportDashboardAPI(t *testing.T) { }, } - importDashboardAPI := New(service, quotaServiceFunc(quotaNotReached), nil, acmock.New().WithDisabled()) + importDashboardAPI := New(service, quotaServiceFunc(quotaNotReached), nil, actest.FakeAccessControl{ExpectedEvaluate: true}) routeRegister := routing.NewRouteRegister() importDashboardAPI.RegisterAPIEndpoints(routeRegister) s := webtest.NewServer(t, routeRegister) @@ -120,6 +121,9 @@ func TestImportDashboardAPI(t *testing.T) { req := s.NewPostRequest("/api/dashboards/import?trimdefaults=true", bytes.NewReader(jsonBytes)) webtest.RequestWithSignedInUser(req, &user.SignedInUser{ UserID: 1, + Permissions: map[int64]map[string][]string{ + 1: {dashboards.ActionDashboardsCreate: {}}, + }, }) resp, err := s.SendJSON(req) require.NoError(t, err) @@ -131,7 +135,7 @@ func TestImportDashboardAPI(t *testing.T) { t.Run("Quota reached", func(t *testing.T) { service := &serviceMock{} - importDashboardAPI := New(service, quotaServiceFunc(quotaReached), nil, acmock.New().WithDisabled()) + importDashboardAPI := New(service, quotaServiceFunc(quotaReached), nil, actest.FakeAccessControl{ExpectedEvaluate: true}) routeRegister := routing.NewRouteRegister() importDashboardAPI.RegisterAPIEndpoints(routeRegister) diff --git a/pkg/services/publicdashboards/api/api_test.go b/pkg/services/publicdashboards/api/api_test.go index 226274d7f1b..a377f8d4de8 100644 --- a/pkg/services/publicdashboards/api/api_test.go +++ b/pkg/services/publicdashboards/api/api_test.go @@ -304,27 +304,15 @@ func TestAPIGetPublicDashboard(t *testing.T) { PublicDashboardResult *PublicDashboard PublicDashboardErr error User *user.SignedInUser - AccessControlEnabled bool ShouldCallService bool }{ - { - Name: "retrieves public dashboard when dashboard is found", - DashboardUid: "1", - ExpectedHttpResponse: http.StatusOK, - PublicDashboardResult: pubdash, - PublicDashboardErr: nil, - User: userViewer, - AccessControlEnabled: false, - ShouldCallService: true, - }, { Name: "returns 404 when dashboard not found", DashboardUid: "77777", ExpectedHttpResponse: http.StatusNotFound, PublicDashboardResult: nil, PublicDashboardErr: ErrDashboardNotFound.Errorf(""), - User: userViewer, - AccessControlEnabled: false, + User: userViewerRBAC, ShouldCallService: true, }, { @@ -333,8 +321,7 @@ func TestAPIGetPublicDashboard(t *testing.T) { ExpectedHttpResponse: http.StatusInternalServerError, PublicDashboardResult: nil, PublicDashboardErr: errors.New("database broken"), - User: userViewer, - AccessControlEnabled: false, + User: userViewerRBAC, ShouldCallService: true, }, { @@ -344,7 +331,6 @@ func TestAPIGetPublicDashboard(t *testing.T) { PublicDashboardResult: pubdash, PublicDashboardErr: nil, User: userViewerRBAC, - AccessControlEnabled: true, ShouldCallService: true, }, { @@ -353,7 +339,6 @@ func TestAPIGetPublicDashboard(t *testing.T) { PublicDashboardResult: pubdash, PublicDashboardErr: nil, User: userViewer, - AccessControlEnabled: true, ShouldCallService: false, }, } @@ -368,7 +353,6 @@ func TestAPIGetPublicDashboard(t *testing.T) { } cfg := setting.NewCfg() - cfg.RBACEnabled = test.AccessControlEnabled testServer := setupTestServer( t, @@ -407,26 +391,14 @@ func TestApiCreatePublicDashboard(t *testing.T) { ExpectedHttpResponse int SaveDashboardErr error User *user.SignedInUser - AccessControlEnabled bool ShouldCallService bool }{ - { - Name: "returns 200 when update persists", - DashboardUid: "1", - publicDashboard: &PublicDashboard{IsEnabled: true}, - ExpectedHttpResponse: http.StatusOK, - SaveDashboardErr: nil, - User: userAdmin, - AccessControlEnabled: false, - ShouldCallService: true, - }, { Name: "returns 500 when not persisted", ExpectedHttpResponse: http.StatusInternalServerError, publicDashboard: &PublicDashboard{}, SaveDashboardErr: ErrInternalServerError.Errorf(""), - User: userAdmin, - AccessControlEnabled: false, + User: userAdminRBAC, ShouldCallService: true, }, { @@ -434,8 +406,7 @@ func TestApiCreatePublicDashboard(t *testing.T) { ExpectedHttpResponse: http.StatusNotFound, publicDashboard: &PublicDashboard{}, SaveDashboardErr: ErrDashboardNotFound.Errorf(""), - User: userAdmin, - AccessControlEnabled: false, + User: userAdminRBAC, ShouldCallService: true, }, { @@ -445,25 +416,14 @@ func TestApiCreatePublicDashboard(t *testing.T) { ExpectedHttpResponse: http.StatusOK, SaveDashboardErr: nil, User: userAdminRBAC, - AccessControlEnabled: true, ShouldCallService: true, }, - { - Name: "returns 403 when no permissions", - ExpectedHttpResponse: http.StatusForbidden, - publicDashboard: &PublicDashboard{IsEnabled: true}, - SaveDashboardErr: ErrInternalServerError.Errorf("default error"), - User: userViewer, - AccessControlEnabled: false, - ShouldCallService: false, - }, { Name: "returns 403 when no permissions RBAC on", ExpectedHttpResponse: http.StatusForbidden, publicDashboard: &PublicDashboard{IsEnabled: true}, SaveDashboardErr: nil, User: userAdmin, - AccessControlEnabled: true, ShouldCallService: false, }, } @@ -479,7 +439,6 @@ func TestApiCreatePublicDashboard(t *testing.T) { } cfg := setting.NewCfg() - cfg.RBACEnabled = test.AccessControlEnabled testServer := setupTestServer( t,