diff --git a/pkg/api/accesscontrol.go b/pkg/api/accesscontrol.go index b5de00c49f7..07a7bb0fd83 100644 --- a/pkg/api/accesscontrol.go +++ b/pkg/api/accesscontrol.go @@ -444,7 +444,7 @@ func (hs *HTTPServer) getAccessControlMetadata(c *contextmodel.ReqContext, // Context must contain permissions in the given org (see LoadPermissionsMiddleware or AuthorizeInOrgMiddleware) func (hs *HTTPServer) getMultiAccessControlMetadata(c *contextmodel.ReqContext, orgID int64, prefix string, resourceIDs map[string]bool) map[string]ac.Metadata { - if hs.AccessControl.IsDisabled() || !c.QueryBool("accesscontrol") { + if !c.QueryBool("accesscontrol") { return map[string]ac.Metadata{} } diff --git a/pkg/api/admin.go b/pkg/api/admin.go index 38dc042ac6d..ae3354ffa98 100644 --- a/pkg/api/admin.go +++ b/pkg/api/admin.go @@ -68,10 +68,6 @@ func (hs *HTTPServer) AdminGetStats(c *contextmodel.ReqContext) response.Respons } func (hs *HTTPServer) getAuthorizedSettings(ctx context.Context, user *user.SignedInUser, bag setting.SettingsBag) (setting.SettingsBag, error) { - if hs.AccessControl.IsDisabled() { - return bag, nil - } - eval := func(scope string) (bool, error) { return hs.AccessControl.Evaluate(ctx, user, ac.EvalPermission(ac.ActionSettingsRead, scope)) } @@ -113,10 +109,6 @@ func (hs *HTTPServer) getAuthorizedSettings(ctx context.Context, user *user.Sign } func (hs *HTTPServer) getAuthorizedVerboseSettings(ctx context.Context, user *user.SignedInUser, bag setting.VerboseSettingsBag) (setting.VerboseSettingsBag, error) { - if hs.AccessControl.IsDisabled() { - return bag, nil - } - eval := func(scope string) (bool, error) { return hs.AccessControl.Evaluate(ctx, user, ac.EvalPermission(ac.ActionSettingsRead, scope)) } diff --git a/pkg/api/annotations.go b/pkg/api/annotations.go index 92a8856e33a..661dcd5b78f 100644 --- a/pkg/api/annotations.go +++ b/pkg/api/annotations.go @@ -14,7 +14,6 @@ import ( contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/guardian" - "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/web" @@ -390,41 +389,32 @@ func (hs *HTTPServer) MassDeleteAnnotations(c *contextmodel.ReqContext) response // validations only for RBAC. A user can mass delete all annotations in a (dashboard + panel) or a specific annotation // if has access to that dashboard. - if !hs.AccessControl.IsDisabled() { - var dashboardId int64 + var dashboardId int64 - if cmd.AnnotationId != 0 { - annotation, respErr := findAnnotationByID(c.Req.Context(), hs.annotationsRepo, cmd.AnnotationId, c.SignedInUser) - if respErr != nil { - return respErr - } - dashboardId = annotation.DashboardID - deleteParams = &annotations.DeleteParams{ - OrgID: c.OrgID, - ID: cmd.AnnotationId, - } - } else { - dashboardId = cmd.DashboardId - deleteParams = &annotations.DeleteParams{ - OrgID: c.OrgID, - DashboardID: cmd.DashboardId, - PanelID: cmd.PanelId, - } + if cmd.AnnotationId != 0 { + annotation, respErr := findAnnotationByID(c.Req.Context(), hs.annotationsRepo, cmd.AnnotationId, c.SignedInUser) + if respErr != nil { + return respErr } - - canSave, err := hs.canMassDeleteAnnotations(c, dashboardId) - if err != nil || !canSave { - return dashboardGuardianResponse(err) + dashboardId = annotation.DashboardID + deleteParams = &annotations.DeleteParams{ + OrgID: c.OrgID, + ID: cmd.AnnotationId, } - } else { // legacy permissions + } else { + dashboardId = cmd.DashboardId deleteParams = &annotations.DeleteParams{ OrgID: c.OrgID, - ID: cmd.AnnotationId, DashboardID: cmd.DashboardId, PanelID: cmd.PanelId, } } + canSave, err := hs.canMassDeleteAnnotations(c, dashboardId) + if err != nil || !canSave { + return dashboardGuardianResponse(err) + } + err = hs.annotationsRepo.Delete(c.Req.Context(), deleteParams) if err != nil { @@ -501,9 +491,6 @@ func (hs *HTTPServer) canSaveAnnotation(c *contextmodel.ReqContext, annotation * if annotation.GetType() == annotations.Dashboard { return canEditDashboard(c, annotation.DashboardID) } else { - if hs.AccessControl.IsDisabled() { - return c.SignedInUser.HasRole(org.RoleEditor), nil - } return true, nil } } @@ -609,21 +596,15 @@ func AnnotationTypeScopeResolver(annotationsRepo annotations.Repository) (string func (hs *HTTPServer) canCreateAnnotation(c *contextmodel.ReqContext, dashboardId int64) (bool, error) { if dashboardId != 0 { - if !hs.AccessControl.IsDisabled() { - evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsCreate, accesscontrol.ScopeAnnotationsTypeDashboard) - if canSave, err := hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator); err != nil || !canSave { - return canSave, err - } + evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsCreate, accesscontrol.ScopeAnnotationsTypeDashboard) + if canSave, err := hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator); err != nil || !canSave { + return canSave, err } return canEditDashboard(c, dashboardId) } else { // organization annotations - if !hs.AccessControl.IsDisabled() { - evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsCreate, accesscontrol.ScopeAnnotationsTypeOrganization) - return hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator) - } else { - return c.SignedInUser.HasRole(org.RoleEditor), nil - } + evaluator := accesscontrol.EvalPermission(accesscontrol.ActionAnnotationsCreate, accesscontrol.ScopeAnnotationsTypeOrganization) + return hs.AccessControl.Evaluate(c.Req.Context(), c.SignedInUser, evaluator) } } diff --git a/pkg/api/annotations_test.go b/pkg/api/annotations_test.go index 0ea84fd4eba..b8836ac22d6 100644 --- a/pkg/api/annotations_test.go +++ b/pkg/api/annotations_test.go @@ -2,392 +2,24 @@ package api import ( "context" - "fmt" "io" "net/http" "strings" "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/api/dtos" - "github.com/grafana/grafana/pkg/api/response" - "github.com/grafana/grafana/pkg/api/routing" - "github.com/grafana/grafana/pkg/infra/db" - "github.com/grafana/grafana/pkg/infra/db/dbtest" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" "github.com/grafana/grafana/pkg/services/annotations" "github.com/grafana/grafana/pkg/services/annotations/annotationstest" - contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" - "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/guardian" - "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/web/webtest" ) -func TestAnnotationsAPIEndpoint(t *testing.T) { - hs := setupSimpleHTTPServer(nil) - store := db.InitTestDB(t) - store.Cfg = hs.Cfg - hs.SQLStore = store - - t.Run("Given an annotation without a dashboard ID", func(t *testing.T) { - cmd := dtos.PostAnnotationsCmd{ - Time: 1000, - Text: "annotation text", - Tags: []string{"tag1", "tag2"}, - } - - updateCmd := dtos.UpdateAnnotationsCmd{ - Time: 1000, - Text: "annotation text", - Tags: []string{"tag1", "tag2"}, - } - - patchCmd := dtos.PatchAnnotationsCmd{ - Time: 1000, - Text: "annotation text", - Tags: []string{"tag1", "tag2"}, - } - - t.Run("When user is an Org Viewer", func(t *testing.T) { - role := org.RoleViewer - t.Run("Should not be allowed to save an annotation", func(t *testing.T) { - postAnnotationScenario(t, "When calling POST on", "/api/annotations", "/api/annotations", role, - cmd, store, nil, func(sc *scenarioContext) { - sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() - assert.Equal(t, 403, sc.resp.Code) - }) - - putAnnotationScenario(t, "When calling PUT on", "/api/annotations/1", "/api/annotations/:annotationId", - role, updateCmd, func(sc *scenarioContext) { - sc.fakeReqWithParams("PUT", sc.url, map[string]string{}).exec() - assert.Equal(t, 403, sc.resp.Code) - }) - - patchAnnotationScenario(t, "When calling PATCH on", "/api/annotations/1", - "/api/annotations/:annotationId", role, patchCmd, func(sc *scenarioContext) { - sc.fakeReqWithParams("PATCH", sc.url, map[string]string{}).exec() - assert.Equal(t, 403, sc.resp.Code) - }) - - mock := dbtest.NewFakeDB() - loggedInUserScenarioWithRole(t, "When calling DELETE on", "DELETE", "/api/annotations/1", - "/api/annotations/:annotationId", role, func(sc *scenarioContext) { - sc.handlerFunc = hs.DeleteAnnotationByID - sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() - assert.Equal(t, 403, sc.resp.Code) - }, mock) - }) - }) - - t.Run("When user is an Org Editor", func(t *testing.T) { - role := org.RoleEditor - t.Run("Should be able to save an annotation", func(t *testing.T) { - postAnnotationScenario(t, "When calling POST on", "/api/annotations", "/api/annotations", role, - cmd, store, nil, func(sc *scenarioContext) { - sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() - assert.Equal(t, 200, sc.resp.Code) - }) - - putAnnotationScenario(t, "When calling PUT on", "/api/annotations/1", "/api/annotations/:annotationId", role, updateCmd, func(sc *scenarioContext) { - sc.fakeReqWithParams("PUT", sc.url, map[string]string{}).exec() - assert.Equal(t, 200, sc.resp.Code) - }) - - patchAnnotationScenario(t, "When calling PATCH on", "/api/annotations/1", "/api/annotations/:annotationId", role, patchCmd, func(sc *scenarioContext) { - sc.fakeReqWithParams("PATCH", sc.url, map[string]string{}).exec() - assert.Equal(t, 200, sc.resp.Code) - }) - mock := dbtest.NewFakeDB() - loggedInUserScenarioWithRole(t, "When calling DELETE on", "DELETE", "/api/annotations/1", - "/api/annotations/:annotationId", role, func(sc *scenarioContext) { - sc.handlerFunc = hs.DeleteAnnotationByID - sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() - assert.Equal(t, 200, sc.resp.Code) - }, mock) - }) - }) - }) - - t.Run("Given an annotation with a dashboard ID and the dashboard does not have an ACL", func(t *testing.T) { - cmd := dtos.PostAnnotationsCmd{ - Time: 1000, - Text: "annotation text", - Tags: []string{"tag1", "tag2"}, - DashboardId: 1, - PanelId: 1, - } - - dashboardUIDCmd := dtos.PostAnnotationsCmd{ - Time: 1000, - Text: "annotation text", - Tags: []string{"tag1", "tag2"}, - DashboardUID: "home", - PanelId: 1, - } - - updateCmd := dtos.UpdateAnnotationsCmd{ - Time: 1000, - Text: "annotation text", - Tags: []string{"tag1", "tag2"}, - Id: 1, - } - - patchCmd := dtos.PatchAnnotationsCmd{ - Time: 8000, - Text: "annotation text 50", - Tags: []string{"foo", "bar"}, - Id: 1, - } - - deleteCmd := dtos.MassDeleteAnnotationsCmd{ - DashboardId: 1, - PanelId: 1, - } - - deleteWithDashboardUIDCmd := dtos.MassDeleteAnnotationsCmd{ - DashboardUID: "home", - PanelId: 1, - } - - t.Run("When user is an Org Viewer", func(t *testing.T) { - role := org.RoleViewer - dashSvc := dashboards.NewFakeDashboardService(t) - t.Run("Should not be allowed to save an annotation", func(t *testing.T) { - postAnnotationScenario(t, "When calling POST on", "/api/annotations", "/api/annotations", role, cmd, store, dashSvc, func(sc *scenarioContext) { - setUpACL() - sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() - assert.Equal(t, 403, sc.resp.Code) - }) - - putAnnotationScenario(t, "When calling PUT on", "/api/annotations/1", "/api/annotations/:annotationId", role, updateCmd, func(sc *scenarioContext) { - setUpACL() - sc.fakeReqWithParams("PUT", sc.url, map[string]string{}).exec() - assert.Equal(t, 403, sc.resp.Code) - }) - - patchAnnotationScenario(t, "When calling PATCH on", "/api/annotations/1", "/api/annotations/:annotationId", role, patchCmd, func(sc *scenarioContext) { - setUpACL() - sc.fakeReqWithParams("PATCH", sc.url, map[string]string{}).exec() - assert.Equal(t, 403, sc.resp.Code) - }) - mock := dbtest.NewFakeDB() - loggedInUserScenarioWithRole(t, "When calling DELETE on", "DELETE", "/api/annotations/1", - "/api/annotations/:annotationId", role, func(sc *scenarioContext) { - setUpACL() - sc.handlerFunc = hs.DeleteAnnotationByID - sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() - assert.Equal(t, 403, sc.resp.Code) - }, mock) - }) - }) - - t.Run("When user is an Org Editor", func(t *testing.T) { - role := org.RoleEditor - t.Run("Should be able to save an annotation", func(t *testing.T) { - dashSvc := dashboards.NewFakeDashboardService(t) - postAnnotationScenario(t, "When calling POST on", "/api/annotations", "/api/annotations", role, cmd, store, dashSvc, func(sc *scenarioContext) { - setUpACL() - sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() - assert.Equal(t, 200, sc.resp.Code) - }) - - putAnnotationScenario(t, "When calling PUT on", "/api/annotations/1", "/api/annotations/:annotationId", role, updateCmd, func(sc *scenarioContext) { - setUpACL() - sc.fakeReqWithParams("PUT", sc.url, map[string]string{}).exec() - assert.Equal(t, 200, sc.resp.Code) - }) - - patchAnnotationScenario(t, "When calling PATCH on", "/api/annotations/1", "/api/annotations/:annotationId", role, patchCmd, func(sc *scenarioContext) { - setUpACL() - sc.fakeReqWithParams("PATCH", sc.url, map[string]string{}).exec() - assert.Equal(t, 200, sc.resp.Code) - }) - mock := dbtest.NewFakeDB() - loggedInUserScenarioWithRole(t, "When calling DELETE on", "DELETE", "/api/annotations/1", - "/api/annotations/:annotationId", role, func(sc *scenarioContext) { - setUpACL() - sc.handlerFunc = hs.DeleteAnnotationByID - sc.fakeReqWithParams("DELETE", sc.url, map[string]string{}).exec() - assert.Equal(t, 200, sc.resp.Code) - }, mock) - }) - }) - - t.Run("When user is an Admin", func(t *testing.T) { - role := org.RoleAdmin - - mockStore := dbtest.NewFakeDB() - - t.Run("Should be able to do anything", func(t *testing.T) { - dashSvc := dashboards.NewFakeDashboardService(t) - result := &dashboards.Dashboard{} - dashSvc.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Return(result, nil) - postAnnotationScenario(t, "When calling POST on", "/api/annotations", "/api/annotations", role, cmd, store, dashSvc, func(sc *scenarioContext) { - setUpACL() - sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() - assert.Equal(t, 200, sc.resp.Code) - }) - - postAnnotationScenario(t, "When calling POST on", "/api/annotations", "/api/annotations", role, dashboardUIDCmd, mockStore, dashSvc, func(sc *scenarioContext) { - setUpACL() - sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() - assert.Equal(t, 200, sc.resp.Code) - - dashSvc.AssertCalled(t, "GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")) - }) - - putAnnotationScenario(t, "When calling PUT on", "/api/annotations/1", "/api/annotations/:annotationId", role, updateCmd, func(sc *scenarioContext) { - setUpACL() - sc.fakeReqWithParams("PUT", sc.url, map[string]string{}).exec() - assert.Equal(t, 200, sc.resp.Code) - }) - - patchAnnotationScenario(t, "When calling PATCH on", "/api/annotations/1", "/api/annotations/:annotationId", role, patchCmd, func(sc *scenarioContext) { - setUpACL() - sc.fakeReqWithParams("PATCH", sc.url, map[string]string{}).exec() - assert.Equal(t, 200, sc.resp.Code) - }) - - deleteAnnotationsScenario(t, "When calling POST on", "/api/annotations/mass-delete", - "/api/annotations/mass-delete", role, deleteCmd, store, nil, func(sc *scenarioContext) { - setUpACL() - sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() - assert.Equal(t, 200, sc.resp.Code) - }) - - dashSvc = dashboards.NewFakeDashboardService(t) - result = &dashboards.Dashboard{ - ID: 1, - UID: deleteWithDashboardUIDCmd.DashboardUID, - } - dashSvc.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Run(func(args mock.Arguments) { - q := args.Get(1).(*dashboards.GetDashboardQuery) - result = &dashboards.Dashboard{ - ID: q.ID, - UID: deleteWithDashboardUIDCmd.DashboardUID, - } - }).Return(result, nil) - deleteAnnotationsScenario(t, "When calling POST with dashboardUID on", "/api/annotations/mass-delete", - "/api/annotations/mass-delete", role, deleteWithDashboardUIDCmd, mockStore, dashSvc, func(sc *scenarioContext) { - setUpACL() - sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() - assert.Equal(t, 200, sc.resp.Code) - dashSvc.AssertCalled(t, "GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")) - }) - }) - }) - }) -} - -func postAnnotationScenario(t *testing.T, desc string, url string, routePattern string, role org.RoleType, - cmd dtos.PostAnnotationsCmd, store db.DB, dashSvc *dashboards.FakeDashboardService, fn scenarioFunc) { - t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) { - hs := setupSimpleHTTPServer(nil) - hs.SQLStore = store - hs.DashboardService = dashSvc - - sc := setupScenarioContext(t, url) - sc.dashboardService = dashSvc - - sc.defaultHandler = routing.Wrap(func(c *contextmodel.ReqContext) response.Response { - c.Req.Body = mockRequestBody(cmd) - c.Req.Header.Add("Content-Type", "application/json") - sc.context = c - sc.context.UserID = testUserID - sc.context.OrgID = testOrgID - sc.context.OrgRole = role - return hs.PostAnnotation(c) - }) - - sc.m.Post(routePattern, sc.defaultHandler) - fn(sc) - }) -} - -func putAnnotationScenario(t *testing.T, desc string, url string, routePattern string, role org.RoleType, - cmd dtos.UpdateAnnotationsCmd, fn scenarioFunc) { - t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) { - hs := setupSimpleHTTPServer(nil) - store := db.InitTestDB(t) - store.Cfg = hs.Cfg - hs.SQLStore = store - - sc := setupScenarioContext(t, url) - sc.defaultHandler = routing.Wrap(func(c *contextmodel.ReqContext) response.Response { - c.Req.Body = mockRequestBody(cmd) - c.Req.Header.Add("Content-Type", "application/json") - sc.context = c - sc.context.UserID = testUserID - sc.context.OrgID = testOrgID - sc.context.OrgRole = role - - return hs.UpdateAnnotation(c) - }) - - sc.m.Put(routePattern, sc.defaultHandler) - - fn(sc) - }) -} - -func patchAnnotationScenario(t *testing.T, desc string, url string, routePattern string, role org.RoleType, cmd dtos.PatchAnnotationsCmd, fn scenarioFunc) { - t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) { - hs := setupSimpleHTTPServer(nil) - store := db.InitTestDB(t) - store.Cfg = hs.Cfg - hs.SQLStore = store - - sc := setupScenarioContext(t, url) - sc.defaultHandler = routing.Wrap(func(c *contextmodel.ReqContext) response.Response { - c.Req.Body = mockRequestBody(cmd) - c.Req.Header.Add("Content-Type", "application/json") - sc.context = c - sc.context.UserID = testUserID - sc.context.OrgID = testOrgID - sc.context.OrgRole = role - - return hs.PatchAnnotation(c) - }) - - sc.m.Patch(routePattern, sc.defaultHandler) - - fn(sc) - }) -} - -func deleteAnnotationsScenario(t *testing.T, desc string, url string, routePattern string, role org.RoleType, - cmd dtos.MassDeleteAnnotationsCmd, store db.DB, dashSvc dashboards.DashboardService, fn scenarioFunc) { - t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) { - hs := setupSimpleHTTPServer(nil) - hs.SQLStore = store - hs.DashboardService = dashSvc - - sc := setupScenarioContext(t, url) - sc.defaultHandler = routing.Wrap(func(c *contextmodel.ReqContext) response.Response { - c.Req.Body = mockRequestBody(cmd) - c.Req.Header.Add("Content-Type", "application/json") - sc.context = c - sc.context.UserID = testUserID - sc.context.OrgID = testOrgID - sc.context.OrgRole = role - - return hs.MassDeleteAnnotations(c) - }) - - sc.m.Post(routePattern, sc.defaultHandler) - - fn(sc) - }) -} - -func TestAPI_Annotations_AccessControl(t *testing.T) { +func TestAPI_Annotations(t *testing.T) { type testCase struct { desc string path string @@ -674,32 +306,6 @@ func TestService_AnnotationTypeScopeResolver(t *testing.T) { } } -func setUpACL() { - viewerRole := org.RoleViewer - editorRole := org.RoleEditor - store := dbtest.NewFakeDB() - 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")).Run(func(args mock.Arguments) { - // q := args.Get(1).(*dashboards.GetDashboardACLInfoListQuery) - - }).Return(qResult, nil) - var result *dashboards.Dashboard - dashSvc.On("GetDashboard", mock.Anything, mock.AnythingOfType("*dashboards.GetDashboardQuery")).Run(func(args mock.Arguments) { - q := args.Get(1).(*dashboards.GetDashboardQuery) - result = &dashboards.Dashboard{ - ID: q.ID, - UID: q.UID, - } - }).Return(result, nil) - - guardian.InitLegacyGuardian(setting.NewCfg(), store, dashSvc, teamSvc) -} - func setUpRBACGuardian(t *testing.T) { origNewGuardian := guardian.New t.Cleanup(func() { diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index b09ef3d7f8a..264d084089b 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -177,7 +177,6 @@ type scenarioContext struct { authInfoService *logintest.AuthInfoServiceFake dashboardVersionService dashver.Service userService user.Service - dashboardService dashboards.DashboardService } func (sc *scenarioContext) exec() { diff --git a/pkg/api/index.go b/pkg/api/index.go index aa859f21456..f61bcde5b42 100644 --- a/pkg/api/index.go +++ b/pkg/api/index.go @@ -135,15 +135,13 @@ func (hs *HTTPServer) setIndexViewData(c *contextmodel.ReqContext) (*dtos.IndexV data.CSPContent = middleware.ReplacePolicyVariables(hs.Cfg.CSPTemplate, appURL, c.RequestNonce) } - if !hs.AccessControl.IsDisabled() { - userPermissions, err := hs.accesscontrolService.GetUserPermissions(c.Req.Context(), c.SignedInUser, ac.Options{ReloadCache: false}) - if err != nil { - return nil, err - } - - data.User.Permissions = ac.BuildPermissionsMap(userPermissions) + userPermissions, err := hs.accesscontrolService.GetUserPermissions(c.Req.Context(), c.SignedInUser, ac.Options{ReloadCache: false}) + if err != nil { + return nil, err } + data.User.Permissions = ac.BuildPermissionsMap(userPermissions) + if setting.DisableGravatar { data.User.GravatarUrl = hs.Cfg.AppSubURL + "/public/img/user_profile.png" } diff --git a/pkg/infra/usagestats/service/service.go b/pkg/infra/usagestats/service/service.go index 9633d6001cc..04ef707e274 100644 --- a/pkg/infra/usagestats/service/service.go +++ b/pkg/infra/usagestats/service/service.go @@ -45,10 +45,8 @@ func ProvideService(cfg *setting.Cfg, accesscontrol: accesscontrol, } - if !accesscontrol.IsDisabled() { - if err := declareFixedRoles(accesscontrolService); err != nil { - return nil, err - } + if err := declareFixedRoles(accesscontrolService); err != nil { + return nil, err } s.registerAPIEndpoints() diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index cade656531f..754066ae707 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -104,12 +104,8 @@ type User struct { } // HasGlobalAccess checks user access with globally assigned permissions only -func HasGlobalAccess(ac AccessControl, service Service, c *contextmodel.ReqContext) func(fallback func(*contextmodel.ReqContext) bool, evaluator Evaluator) bool { - return func(fallback func(*contextmodel.ReqContext) bool, evaluator Evaluator) bool { - if ac.IsDisabled() { - return fallback(c) - } - +func HasGlobalAccess(ac AccessControl, service Service, c *contextmodel.ReqContext) func(evaluator Evaluator) bool { + return func(evaluator Evaluator) bool { userCopy := *c.SignedInUser userCopy.OrgID = GlobalOrgID userCopy.OrgRole = "" diff --git a/pkg/services/navtree/navtreeimpl/admin.go b/pkg/services/navtree/navtreeimpl/admin.go index 8726077a4bc..dbb4f191e1c 100644 --- a/pkg/services/navtree/navtreeimpl/admin.go +++ b/pkg/services/navtree/navtreeimpl/admin.go @@ -105,7 +105,7 @@ func (s *ServiceImpl) getAdminNode(c *contextmodel.ReqContext) (*navtree.NavLink }) } - if hasGlobalAccess(ac.ReqGrafanaAdmin, orgsAccessEvaluator) { + if hasGlobalAccess(orgsAccessEvaluator) { configNodes = append(configNodes, &navtree.NavLink{ Text: "Organizations", SubTitle: "Isolated instances of Grafana running on the same server", Id: "global-orgs", Url: s.cfg.AppSubURL + "/admin/orgs", Icon: "building", }) diff --git a/pkg/services/navtree/navtreeimpl/applinks.go b/pkg/services/navtree/navtreeimpl/applinks.go index 0f022dc9be4..6db754a1aba 100644 --- a/pkg/services/navtree/navtreeimpl/applinks.go +++ b/pkg/services/navtree/navtreeimpl/applinks.go @@ -237,8 +237,7 @@ func (s *ServiceImpl) addPluginToSection(c *contextmodel.ReqContext, treeRoot *n func (s *ServiceImpl) hasAccessToInclude(c *contextmodel.ReqContext, pluginID string) func(include *plugins.Includes) bool { hasAccess := ac.HasAccess(s.accessControl, c) return func(include *plugins.Includes) bool { - useRBAC := s.features.IsEnabled(featuremgmt.FlagAccessControlOnCall) && - !s.accessControl.IsDisabled() && include.RequiresRBACAction() + useRBAC := s.features.IsEnabled(featuremgmt.FlagAccessControlOnCall) && include.RequiresRBACAction() if useRBAC && !hasAccess(ac.EvalPermission(include.Action)) { s.log.Debug("plugin include is covered by RBAC, user doesn't have access", "plugin", pluginID, diff --git a/pkg/services/ngalert/api/api_ruler.go b/pkg/services/ngalert/api/api_ruler.go index 867fa0445dd..df2897208d7 100644 --- a/pkg/services/ngalert/api/api_ruler.go +++ b/pkg/services/ngalert/api/api_ruler.go @@ -51,7 +51,7 @@ var ( // Returns http.StatusUnauthorized if user does not have access to any of the rules that match the filter. // Returns http.StatusBadRequest if all rules that match the filter and the user is authorized to delete are provisioned. func (srv RulerSrv) RouteDeleteAlertRules(c *contextmodel.ReqContext, namespaceTitle string, group string) response.Response { - namespace, err := srv.store.GetNamespaceByTitle(c.Req.Context(), namespaceTitle, c.SignedInUser.OrgID, c.SignedInUser, true) + namespace, err := srv.store.GetNamespaceByTitle(c.Req.Context(), namespaceTitle, c.SignedInUser.OrgID, c.SignedInUser) if err != nil { return toNamespaceErrorResponse(err) } @@ -148,7 +148,7 @@ func (srv RulerSrv) RouteDeleteAlertRules(c *contextmodel.ReqContext, namespaceT // RouteGetNamespaceRulesConfig returns all rules in a specific folder that user has access to func (srv RulerSrv) RouteGetNamespaceRulesConfig(c *contextmodel.ReqContext, namespaceTitle string) response.Response { - namespace, err := srv.store.GetNamespaceByTitle(c.Req.Context(), namespaceTitle, c.SignedInUser.OrgID, c.SignedInUser, false) + namespace, err := srv.store.GetNamespaceByTitle(c.Req.Context(), namespaceTitle, c.SignedInUser.OrgID, c.SignedInUser) if err != nil { return toNamespaceErrorResponse(err) } @@ -191,7 +191,7 @@ func (srv RulerSrv) RouteGetNamespaceRulesConfig(c *contextmodel.ReqContext, nam // RouteGetRulesGroupConfig returns rules that belong to a specific group in a specific namespace (folder). // If user does not have access to at least one of the rule in the group, returns status 401 Unauthorized func (srv RulerSrv) RouteGetRulesGroupConfig(c *contextmodel.ReqContext, namespaceTitle string, ruleGroup string) response.Response { - namespace, err := srv.store.GetNamespaceByTitle(c.Req.Context(), namespaceTitle, c.SignedInUser.OrgID, c.SignedInUser, false) + namespace, err := srv.store.GetNamespaceByTitle(c.Req.Context(), namespaceTitle, c.SignedInUser.OrgID, c.SignedInUser) if err != nil { return toNamespaceErrorResponse(err) } @@ -297,7 +297,7 @@ func (srv RulerSrv) RouteGetRulesConfig(c *contextmodel.ReqContext) response.Res } func (srv RulerSrv) RoutePostNameRulesConfig(c *contextmodel.ReqContext, ruleGroupConfig apimodels.PostableRuleGroupConfig, namespaceTitle string) response.Response { - namespace, err := srv.store.GetNamespaceByTitle(c.Req.Context(), namespaceTitle, c.SignedInUser.OrgID, c.SignedInUser, true) + namespace, err := srv.store.GetNamespaceByTitle(c.Req.Context(), namespaceTitle, c.SignedInUser.OrgID, c.SignedInUser) if err != nil { return toNamespaceErrorResponse(err) } @@ -336,14 +336,11 @@ func (srv RulerSrv) updateAlertRulesInGroup(c *contextmodel.ReqContext, groupKey return nil } - // if RBAC is disabled the permission are limited to folder access that is done upstream - if !srv.ac.IsDisabled() { - err = authorizeRuleChanges(groupChanges, func(evaluator accesscontrol.Evaluator) bool { - return hasAccess(evaluator) - }) - if err != nil { - return err - } + err = authorizeRuleChanges(groupChanges, func(evaluator accesscontrol.Evaluator) bool { + return hasAccess(evaluator) + }) + if err != nil { + return err } if err := verifyProvisionedRulesNotAffected(c.Req.Context(), srv.provenanceStore, c.OrgID, groupChanges); err != nil { diff --git a/pkg/services/ngalert/api/persist.go b/pkg/services/ngalert/api/persist.go index c07fdc7d422..52d5ccdf8bb 100644 --- a/pkg/services/ngalert/api/persist.go +++ b/pkg/services/ngalert/api/persist.go @@ -11,7 +11,7 @@ import ( // RuleStore is the interface for persisting alert rules and instances type RuleStore interface { GetUserVisibleNamespaces(context.Context, int64, *user.SignedInUser) (map[string]*folder.Folder, error) - GetNamespaceByTitle(context.Context, string, int64, *user.SignedInUser, bool) (*folder.Folder, error) + GetNamespaceByTitle(context.Context, string, int64, *user.SignedInUser) (*folder.Folder, error) GetAlertRulesGroupByRuleUID(ctx context.Context, query *ngmodels.GetAlertRulesGroupByRuleUIDQuery) ([]*ngmodels.AlertRule, error) ListAlertRules(ctx context.Context, query *ngmodels.ListAlertRulesQuery) (ngmodels.RulesGroup, error) diff --git a/pkg/services/ngalert/api/util.go b/pkg/services/ngalert/api/util.go index 48da342039f..edb7b5f9059 100644 --- a/pkg/services/ngalert/api/util.go +++ b/pkg/services/ngalert/api/util.go @@ -85,7 +85,7 @@ func (p *AlertingProxy) createProxyContext(ctx *contextmodel.ReqContext, request // Some data sources require legacy Editor role in order to perform mutating operations. In this case, we elevate permissions for the context that we // will provide downstream. // TODO (yuri) remove this after RBAC for plugins is implemented - if !p.ac.IsDisabled() && !ctx.SignedInUser.HasRole(org.RoleEditor) { + if !ctx.SignedInUser.HasRole(org.RoleEditor) { newUser := *ctx.SignedInUser newUser.OrgRole = org.RoleEditor cpy.SignedInUser = &newUser diff --git a/pkg/services/ngalert/api/util_test.go b/pkg/services/ngalert/api/util_test.go index 7f7c34a03b5..7fea2c987f4 100644 --- a/pkg/services/ngalert/api/util_test.go +++ b/pkg/services/ngalert/api/util_test.go @@ -139,26 +139,6 @@ func TestAlertingProxy_createProxyContext(t *testing.T) { } }) }) - t.Run("if access control is disabled", func(t *testing.T) { - t.Run("should not alter user", func(t *testing.T) { - proxy := AlertingProxy{ - DataProxy: nil, - ac: accesscontrolmock.New().WithDisabled(), - } - - req := &http.Request{} - resp := &response.NormalResponse{} - - for _, roleType := range []org.RoleType{org.RoleViewer, org.RoleEditor, org.RoleAdmin} { - roleCtx := *ctx - roleCtx.SignedInUser = &user.SignedInUser{ - OrgRole: roleType, - } - newCtx := proxy.createProxyContext(&roleCtx, req, resp) - require.Equalf(t, roleCtx.SignedInUser, newCtx.SignedInUser, "user should not be altered if access control is disabled and role is %s", roleType) - } - }) - }) } func Test_containsProvisionedAlerts(t *testing.T) { diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index 04b8c5ac404..287f2046501 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -9,7 +9,6 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/folder" - "github.com/grafana/grafana/pkg/services/guardian" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/search/model" "github.com/grafana/grafana/pkg/services/sqlstore" @@ -387,27 +386,12 @@ func (st DBstore) GetUserVisibleNamespaces(ctx context.Context, orgID int64, use } // GetNamespaceByTitle is a handler for retrieving a namespace by its title. Alerting rules follow a Grafana folder-like structure which we call namespaces. -func (st DBstore) GetNamespaceByTitle(ctx context.Context, namespace string, orgID int64, user *user.SignedInUser, withCanSave bool) (*folder.Folder, error) { +func (st DBstore) GetNamespaceByTitle(ctx context.Context, namespace string, orgID int64, user *user.SignedInUser) (*folder.Folder, error) { folder, err := st.FolderService.Get(ctx, &folder.GetFolderQuery{OrgID: orgID, Title: &namespace, SignedInUser: user}) if err != nil { return nil, err } - // if access control is disabled, check that the user is allowed to save in the folder. - if withCanSave && st.AccessControl.IsDisabled() { - g, err := guardian.NewByUID(ctx, folder.UID, orgID, user) - if err != nil { - return folder, err - } - - if canSave, err := g.CanSave(); err != nil || !canSave { - if err != nil { - st.Logger.Error("checking can save permission has failed", "userId", user.UserID, "username", user.Login, "namespace", namespace, "orgId", orgID, "error", err) - } - return nil, ngmodels.ErrCannotEditNamespace - } - } - return folder, nil } diff --git a/pkg/services/ngalert/tests/fakes/rules.go b/pkg/services/ngalert/tests/fakes/rules.go index 84da789735b..00a60380c92 100644 --- a/pkg/services/ngalert/tests/fakes/rules.go +++ b/pkg/services/ngalert/tests/fakes/rules.go @@ -245,7 +245,7 @@ func (f *RuleStore) GetUserVisibleNamespaces(_ context.Context, orgID int64, _ * return namespacesMap, nil } -func (f *RuleStore) GetNamespaceByTitle(_ context.Context, title string, orgID int64, _ *user.SignedInUser, _ bool) (*folder.Folder, error) { +func (f *RuleStore) GetNamespaceByTitle(_ context.Context, title string, orgID int64, _ *user.SignedInUser) (*folder.Folder, error) { folders := f.Folders[orgID] for _, folder := range folders { if folder.Title == title { diff --git a/pkg/services/serviceaccounts/api/api.go b/pkg/services/serviceaccounts/api/api.go index fb7c85687af..9a585938a41 100644 --- a/pkg/services/serviceaccounts/api/api.go +++ b/pkg/services/serviceaccounts/api/api.go @@ -110,18 +110,16 @@ func (api *ServiceAccountsAPI) CreateServiceAccount(c *contextmodel.ReqContext) return response.ErrOrFallback(http.StatusInternalServerError, "Failed to create service account", err) } - if !api.accesscontrol.IsDisabled() { - if c.SignedInUser.IsRealUser() { - if _, err := api.permissionService.SetUserPermission(c.Req.Context(), c.OrgID, accesscontrol.User{ID: c.SignedInUser.UserID}, strconv.FormatInt(serviceAccount.Id, 10), "Admin"); err != nil { - return response.Error(http.StatusInternalServerError, "Failed to set permissions for service account creator", err) - } + if c.SignedInUser.IsRealUser() { + if _, err := api.permissionService.SetUserPermission(c.Req.Context(), c.OrgID, accesscontrol.User{ID: c.SignedInUser.UserID}, strconv.FormatInt(serviceAccount.Id, 10), "Admin"); err != nil { + return response.Error(http.StatusInternalServerError, "Failed to set permissions for service account creator", err) } - - // Clear permission cache for the user who's created the service account, so that new permissions are fetched for their next call - // Required for cases when caller wants to immediately interact with the newly created object - api.accesscontrolService.ClearUserPermissionCache(c.SignedInUser) } + // Clear permission cache for the user who's created the service account, so that new permissions are fetched for their next call + // Required for cases when caller wants to immediately interact with the newly created object + api.accesscontrolService.ClearUserPermissionCache(c.SignedInUser) + return response.JSON(http.StatusCreated, serviceAccount) } @@ -339,7 +337,7 @@ func (api *ServiceAccountsAPI) ConvertToServiceAccount(ctx *contextmodel.ReqCont } func (api *ServiceAccountsAPI) getAccessControlMetadata(c *contextmodel.ReqContext, saIDs map[string]bool) map[string]accesscontrol.Metadata { - if api.accesscontrol.IsDisabled() || !c.QueryBool("accesscontrol") { + if !c.QueryBool("accesscontrol") { return map[string]accesscontrol.Metadata{} } diff --git a/pkg/services/supportbundles/supportbundlesimpl/service.go b/pkg/services/supportbundles/supportbundlesimpl/service.go index 1bb71575fd3..97459bc6aae 100644 --- a/pkg/services/supportbundles/supportbundlesimpl/service.go +++ b/pkg/services/supportbundles/supportbundlesimpl/service.go @@ -77,10 +77,8 @@ func ProvideService( return s, nil } - if !accessControl.IsDisabled() { - if err := s.declareFixedRoles(accesscontrolService); err != nil { - return nil, err - } + if err := s.declareFixedRoles(accesscontrolService); err != nil { + return nil, err } s.registerAPIEndpoints(httpServer, routeRegister)