From 62208726332903900464c97fb4de03fee0919853 Mon Sep 17 00:00:00 2001 From: gotjosh Date: Mon, 8 Nov 2021 13:26:08 +0000 Subject: [PATCH] Alerting: fix bug where user is able to access rules from namespaces user is not part of (#41403) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add fix * Add tests Co-authored-by: Yuriy Tseretyan Co-authored-by: Armand Grillet <2117580+armandgrillet@users.noreply.github.com> Co-authored-by: Jean-Philippe Quéméner Co-authored-by: George Robinson --- pkg/services/ngalert/api/api_prometheus.go | 5 ++++ pkg/services/ngalert/api/api_ruler.go | 7 ++++- pkg/tests/api/alerting/api_prometheus_test.go | 26 +++++++++++++++++++ pkg/tests/api/alerting/api_ruler_test.go | 18 +++++++++++++ 4 files changed, 55 insertions(+), 1 deletion(-) diff --git a/pkg/services/ngalert/api/api_prometheus.go b/pkg/services/ngalert/api/api_prometheus.go index 5521c94f463..36d8dc1d5b3 100644 --- a/pkg/services/ngalert/api/api_prometheus.go +++ b/pkg/services/ngalert/api/api_prometheus.go @@ -76,6 +76,11 @@ func (srv PrometheusSrv) RouteGetRuleStatuses(c *models.ReqContext) response.Res return ErrResp(http.StatusInternalServerError, err, "failed to get namespaces visible to the user") } + if len(namespaceMap) == 0 { + srv.log.Debug("User does not have access to any namespaces") + return response.JSON(http.StatusOK, ruleResponse) + } + namespaceUIDs := make([]string, len(namespaceMap)) for k := range namespaceMap { namespaceUIDs = append(namespaceUIDs, k) diff --git a/pkg/services/ngalert/api/api_ruler.go b/pkg/services/ngalert/api/api_ruler.go index af1819236de..96406bc804c 100644 --- a/pkg/services/ngalert/api/api_ruler.go +++ b/pkg/services/ngalert/api/api_ruler.go @@ -151,6 +151,12 @@ func (srv RulerSrv) RouteGetRulesConfig(c *models.ReqContext) response.Response if err != nil { return ErrResp(http.StatusInternalServerError, err, "failed to get namespaces visible to the user") } + result := apimodels.NamespaceConfigResponse{} + + if len(namespaceMap) == 0 { + srv.log.Debug("User has no access to any namespaces") + return response.JSON(http.StatusOK, result) + } namespaceUIDs := make([]string, len(namespaceMap)) for k := range namespaceMap { @@ -214,7 +220,6 @@ func (srv RulerSrv) RouteGetRulesConfig(c *models.ReqContext) response.Response } } - result := apimodels.NamespaceConfigResponse{} for namespace, m := range configs { for _, ruleGroupConfig := range m { result[namespace] = append(result[namespace], ruleGroupConfig) diff --git a/pkg/tests/api/alerting/api_prometheus_test.go b/pkg/tests/api/alerting/api_prometheus_test.go index 969af2b56ba..bcf6ae5d926 100644 --- a/pkg/tests/api/alerting/api_prometheus_test.go +++ b/pkg/tests/api/alerting/api_prometheus_test.go @@ -761,6 +761,32 @@ func TestPrometheusRulesPermissions(t *testing.T) { "evaluationTime": 0 }] } +}`, string(b)) + } + + // remove permissions from _ALL_ folders + require.NoError(t, store.UpdateDashboardACL(1, nil)) + + // make sure that no folders are included in the response + { + promRulesURL := fmt.Sprintf("http://grafana:password@%s/api/prometheus/grafana/api/v1/rules", grafanaListedAddr) + // nolint:gosec + resp, err := http.Get(promRulesURL) + require.NoError(t, err) + t.Cleanup(func() { + err := resp.Body.Close() + require.NoError(t, err) + }) + b, err := ioutil.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, 200, resp.StatusCode) + + require.JSONEq(t, ` +{ + "status": "success", + "data": { + "groups": [] + } }`, string(b)) } } diff --git a/pkg/tests/api/alerting/api_ruler_test.go b/pkg/tests/api/alerting/api_ruler_test.go index e4298e8b228..3ab3ceb4bb7 100644 --- a/pkg/tests/api/alerting/api_ruler_test.go +++ b/pkg/tests/api/alerting/api_ruler_test.go @@ -245,6 +245,24 @@ func TestAlertRulePermissions(t *testing.T) { }` assert.JSONEq(t, expectedGetNamespaceResponseBody, body) } + + // Remove permissions from ALL folders. + require.NoError(t, store.UpdateDashboardACL(1, nil)) + { + u := fmt.Sprintf("http://grafana:password@%s/api/ruler/grafana/api/v1/rules", grafanaListedAddr) + // nolint:gosec + resp, err := http.Get(u) + require.NoError(t, err) + t.Cleanup(func() { + err := resp.Body.Close() + require.NoError(t, err) + }) + b, err := ioutil.ReadAll(resp.Body) + require.NoError(t, err) + + assert.Equal(t, resp.StatusCode, 200) + require.JSONEq(t, `{}`, string(b)) + } } func createRule(t *testing.T, grafanaListedAddr string, folder string, user, password string) {