From f7f22530720cebd57f3ef802c1c5300583615d2d Mon Sep 17 00:00:00 2001 From: Yuriy Tseretyan Date: Thu, 19 May 2022 09:22:26 -0400 Subject: [PATCH] Alerting: Fix anonymous access to alerting (#49203) * introduce a fallback handler that checks that role is Viewer. * update UI nav links to allow alerting tabs for anonymous user * update rule api to check for Viewer role instead of SignedIn when RBAC is disabled --- pkg/api/index.go | 4 ++-- pkg/services/accesscontrol/accesscontrol.go | 5 +++++ pkg/services/ngalert/CHANGELOG.md | 1 + pkg/services/ngalert/api/api_prometheus.go | 2 +- pkg/services/ngalert/api/api_prometheus_test.go | 4 ++-- pkg/services/ngalert/api/api_ruler.go | 6 +++--- pkg/services/ngalert/api/api_ruler_test.go | 4 ++-- public/app/features/alerting/routes.tsx | 2 +- 8 files changed, 17 insertions(+), 11 deletions(-) diff --git a/pkg/api/index.go b/pkg/api/index.go index 50c93285805..9041df91674 100644 --- a/pkg/api/index.go +++ b/pkg/api/index.go @@ -513,7 +513,7 @@ func (hs *HTTPServer) buildAlertNavLinks(c *models.ReqContext) []*dtos.NavLink { hasAccess := ac.HasAccess(hs.AccessControl, c) var alertChildNavs []*dtos.NavLink - if hasAccess(ac.ReqSignedIn, ac.EvalAny(ac.EvalPermission(ac.ActionAlertingRuleRead), ac.EvalPermission(ac.ActionAlertingRuleExternalRead))) { + if hasAccess(ac.ReqViewer, ac.EvalAny(ac.EvalPermission(ac.ActionAlertingRuleRead), ac.EvalPermission(ac.ActionAlertingRuleExternalRead))) { alertChildNavs = append(alertChildNavs, &dtos.NavLink{ Text: "Alert rules", Id: "alert-list", Url: hs.Cfg.AppSubURL + "/alerting/list", Icon: "list-ul", }) @@ -527,7 +527,7 @@ func (hs *HTTPServer) buildAlertNavLinks(c *models.ReqContext) []*dtos.NavLink { alertChildNavs = append(alertChildNavs, &dtos.NavLink{Text: "Notification policies", Id: "am-routes", Url: hs.Cfg.AppSubURL + "/alerting/routes", Icon: "sitemap"}) } - if hasAccess(ac.ReqSignedIn, ac.EvalAny(ac.EvalPermission(ac.ActionAlertingInstanceRead), ac.EvalPermission(ac.ActionAlertingInstancesExternalRead))) { + if hasAccess(ac.ReqViewer, ac.EvalAny(ac.EvalPermission(ac.ActionAlertingInstanceRead), ac.EvalPermission(ac.ActionAlertingInstancesExternalRead))) { alertChildNavs = append(alertChildNavs, &dtos.NavLink{Text: "Silences", Id: "silences", Url: hs.Cfg.AppSubURL + "/alerting/silences", Icon: "bell-slash"}) alertChildNavs = append(alertChildNavs, &dtos.NavLink{Text: "Alert groups", Id: "groups", Url: hs.Cfg.AppSubURL + "/alerting/groups", Icon: "layer-group"}) } diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index 4a17b0d6c1d..7dd53540c33 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -129,6 +129,11 @@ var ReqGrafanaAdmin = func(c *models.ReqContext) bool { return c.IsGrafanaAdmin } +// ReqViewer returns true if the current user has models.ROLE_VIEWER. Note: this can be anonymous user as well +var ReqViewer = func(c *models.ReqContext) bool { + return c.OrgRole.Includes(models.ROLE_VIEWER) +} + var ReqOrgAdmin = func(c *models.ReqContext) bool { return c.OrgRole == models.ROLE_ADMIN } diff --git a/pkg/services/ngalert/CHANGELOG.md b/pkg/services/ngalert/CHANGELOG.md index 99a4536957b..c7d1f5cda2e 100644 --- a/pkg/services/ngalert/CHANGELOG.md +++ b/pkg/services/ngalert/CHANGELOG.md @@ -55,6 +55,7 @@ Scopes must have an order to ensure consistency and ease of search, this helps u - [FEATURE] Indicate whether contact point is provisioned when GETting Alertmanager configuration #48323 - [FEATURE] Indicate whether alert rule is provisioned when GETting the rule #48458 - [BUGFIX] Migration: ignore alerts that do not belong to any existing organization\dashboard #49192 +- [BUGFIX] Allow anonymous access to alerts #49203 ## 8.5.3 diff --git a/pkg/services/ngalert/api/api_prometheus.go b/pkg/services/ngalert/api/api_prometheus.go index a727a191caa..020268f65d7 100644 --- a/pkg/services/ngalert/api/api_prometheus.go +++ b/pkg/services/ngalert/api/api_prometheus.go @@ -154,7 +154,7 @@ func (srv PrometheusSrv) RouteGetRuleStatuses(c *models.ReqContext) response.Res return response.JSON(http.StatusInternalServerError, ruleResponse) } hasAccess := func(evaluator accesscontrol.Evaluator) bool { - return accesscontrol.HasAccess(srv.ac, c)(accesscontrol.ReqSignedIn, evaluator) + return accesscontrol.HasAccess(srv.ac, c)(accesscontrol.ReqViewer, evaluator) } groupedRules := make(map[ngmodels.AlertRuleGroupKey][]*ngmodels.AlertRule) diff --git a/pkg/services/ngalert/api/api_prometheus_test.go b/pkg/services/ngalert/api/api_prometheus_test.go index 5558ee35c30..b4b8b70d836 100644 --- a/pkg/services/ngalert/api/api_prometheus_test.go +++ b/pkg/services/ngalert/api/api_prometheus_test.go @@ -255,7 +255,7 @@ func TestRouteGetRuleStatuses(t *testing.T) { req, err := http.NewRequest("GET", "/api/v1/rules", nil) require.NoError(t, err) - c := &models.ReqContext{Context: &web.Context{Req: req}, SignedInUser: &models.SignedInUser{OrgId: orgID}, IsSignedIn: true} + c := &models.ReqContext{Context: &web.Context{Req: req}, SignedInUser: &models.SignedInUser{OrgId: orgID, OrgRole: models.ROLE_VIEWER}} t.Run("with no rules", func(t *testing.T) { _, _, _, api := setupAPI(t) @@ -325,7 +325,7 @@ func TestRouteGetRuleStatuses(t *testing.T) { req, err := http.NewRequest("GET", "/api/v1/rules?includeInternalLabels=true", nil) require.NoError(t, err) - c := &models.ReqContext{Context: &web.Context{Req: req}, SignedInUser: &models.SignedInUser{OrgId: orgID}, IsSignedIn: true} + c := &models.ReqContext{Context: &web.Context{Req: req}, SignedInUser: &models.SignedInUser{OrgId: orgID, OrgRole: models.ROLE_VIEWER}} r := api.RouteGetRuleStatuses(c) require.Equal(t, http.StatusOK, r.Status()) diff --git a/pkg/services/ngalert/api/api_ruler.go b/pkg/services/ngalert/api/api_ruler.go index 55c75e9f005..f3d3fe694a3 100644 --- a/pkg/services/ngalert/api/api_ruler.go +++ b/pkg/services/ngalert/api/api_ruler.go @@ -174,7 +174,7 @@ func (srv RulerSrv) RouteGetNamespaceRulesConfig(c *models.ReqContext) response. ruleGroupConfigs := make(map[string]apimodels.GettableRuleGroupConfig) hasAccess := func(evaluator accesscontrol.Evaluator) bool { - return accesscontrol.HasAccess(srv.ac, c)(accesscontrol.ReqSignedIn, evaluator) + return accesscontrol.HasAccess(srv.ac, c)(accesscontrol.ReqViewer, evaluator) } provenanceRecords, err := srv.provenanceStore.GetProvenances(c.Req.Context(), c.SignedInUser.OrgId, (&ngmodels.AlertRule{}).ResourceType()) @@ -227,7 +227,7 @@ func (srv RulerSrv) RouteGetRulesGroupConfig(c *models.ReqContext) response.Resp } hasAccess := func(evaluator accesscontrol.Evaluator) bool { - return accesscontrol.HasAccess(srv.ac, c)(accesscontrol.ReqSignedIn, evaluator) + return accesscontrol.HasAccess(srv.ac, c)(accesscontrol.ReqViewer, evaluator) } provenanceRecords, err := srv.provenanceStore.GetProvenances(c.Req.Context(), c.SignedInUser.OrgId, (&ngmodels.AlertRule{}).ResourceType()) @@ -287,7 +287,7 @@ func (srv RulerSrv) RouteGetRulesConfig(c *models.ReqContext) response.Response } hasAccess := func(evaluator accesscontrol.Evaluator) bool { - return accesscontrol.HasAccess(srv.ac, c)(accesscontrol.ReqSignedIn, evaluator) + return accesscontrol.HasAccess(srv.ac, c)(accesscontrol.ReqViewer, evaluator) } provenanceRecords, err := srv.provenanceStore.GetProvenances(c.Req.Context(), c.SignedInUser.OrgId, (&ngmodels.AlertRule{}).ResourceType()) diff --git a/pkg/services/ngalert/api/api_ruler_test.go b/pkg/services/ngalert/api/api_ruler_test.go index 5cce7f31d77..b29bf8dcfe3 100644 --- a/pkg/services/ngalert/api/api_ruler_test.go +++ b/pkg/services/ngalert/api/api_ruler_test.go @@ -575,7 +575,7 @@ func TestRouteGetNamespaceRulesConfig(t *testing.T) { ruleStore.PutRule(context.Background(), expectedRules...) ac := acMock.New().WithDisabled() - response := createService(ac, ruleStore, nil).RouteGetNamespaceRulesConfig(createRequestContext(orgID, "", map[string]string{ + response := createService(ac, ruleStore, nil).RouteGetNamespaceRulesConfig(createRequestContext(orgID, models2.ROLE_VIEWER, map[string]string{ ":Namespace": folder.Title, })) @@ -619,7 +619,7 @@ func TestRouteGetNamespaceRulesConfig(t *testing.T) { err := svc.provenanceStore.SetProvenance(context.Background(), rule, orgID, models.ProvenanceAPI) require.NoError(t, err) - response := svc.RouteGetNamespaceRulesConfig(createRequestContext(orgID, "", map[string]string{ + response := svc.RouteGetNamespaceRulesConfig(createRequestContext(orgID, models2.ROLE_VIEWER, map[string]string{ ":Namespace": folder.Title, })) diff --git a/public/app/features/alerting/routes.tsx b/public/app/features/alerting/routes.tsx index 1e7288788c9..9318f4ce812 100644 --- a/public/app/features/alerting/routes.tsx +++ b/public/app/features/alerting/routes.tsx @@ -134,7 +134,7 @@ const unifiedRoutes: RouteDescriptor[] = [ path: '/alerting/silences', roles: evaluateAccess( [AccessControlAction.AlertingInstanceRead, AccessControlAction.AlertingInstancesExternalRead], - ['Editor', 'Admin'] + ['Viewer', 'Editor', 'Admin'] ), component: SafeDynamicImport( () => import(/* webpackChunkName: "AlertSilences" */ 'app/features/alerting/unified/Silences')