From fbd057b2582df435df172802d0a16197f6c61048 Mon Sep 17 00:00:00 2001 From: Matthew Jacobson Date: Wed, 20 Mar 2024 16:04:35 -0400 Subject: [PATCH] Alerting: Stop returning autogen routes for non-admin on api/v2/status (#84864) * Alerting: Stop returning autogen routes for non-admin on api/v2/status * Improve api/v2/status integration tests for user roles --- pkg/services/ngalert/api/api_alertmanager.go | 7 +- .../ngalert/api/api_alertmanager_test.go | 47 ++++++++++ .../api/alerting/api_alertmanager_test.go | 88 +++++++++++++++---- 3 files changed, 125 insertions(+), 17 deletions(-) diff --git a/pkg/services/ngalert/api/api_alertmanager.go b/pkg/services/ngalert/api/api_alertmanager.go index 51398e72951..3bc763a08df 100644 --- a/pkg/services/ngalert/api/api_alertmanager.go +++ b/pkg/services/ngalert/api/api_alertmanager.go @@ -50,7 +50,12 @@ func (srv AlertmanagerSrv) RouteGetAMStatus(c *contextmodel.ReqContext) response return errResp } - return response.JSON(http.StatusOK, am.GetStatus()) + status := am.GetStatus() + if !c.SignedInUser.HasRole(org.RoleAdmin) { + notifier.RemoveAutogenConfigIfExists(status.Config.Route) + } + + return response.JSON(http.StatusOK, status) } func (srv AlertmanagerSrv) RouteCreateSilence(c *contextmodel.ReqContext, postableSilence apimodels.PostableSilence) response.Response { diff --git a/pkg/services/ngalert/api/api_alertmanager_test.go b/pkg/services/ngalert/api/api_alertmanager_test.go index c6e8deadb8a..879ad9a39d7 100644 --- a/pkg/services/ngalert/api/api_alertmanager_test.go +++ b/pkg/services/ngalert/api/api_alertmanager_test.go @@ -405,6 +405,53 @@ func TestAlertmanagerAutogenConfig(t *testing.T) { compare(t, validConfigWithoutAutogen, string(response.Body())) }) }) + + t.Run("route GET status", func(t *testing.T) { + t.Run("when admin return autogen routes", func(t *testing.T) { + sut, _ := createSutForAutogen(t) + + rc := createRequestCtxInOrg(2) + rc.SignedInUser.OrgRole = org.RoleAdmin + + response := sut.RouteGetAMStatus(rc) + require.Equal(t, 200, response.Status()) + + var status struct { + Config apimodels.PostableApiAlertingConfig `json:"config"` + } + err := json.Unmarshal(response.Body(), &status) + require.NoError(t, err) + configBody, err := json.Marshal(apimodels.PostableUserConfig{ + TemplateFiles: map[string]string{"a": "template"}, + AlertmanagerConfig: status.Config, + }) + require.NoError(t, err) + + compare(t, validConfigWithAutogen, string(configBody)) + }) + + t.Run("when not admin return no autogen routes", func(t *testing.T) { + sut, _ := createSutForAutogen(t) + + rc := createRequestCtxInOrg(2) + + response := sut.RouteGetAMStatus(rc) + require.Equal(t, 200, response.Status()) + + var status struct { + Config apimodels.PostableApiAlertingConfig `json:"config"` + } + err := json.Unmarshal(response.Body(), &status) + require.NoError(t, err) + configBody, err := json.Marshal(apimodels.PostableUserConfig{ + TemplateFiles: map[string]string{"a": "template"}, + AlertmanagerConfig: status.Config, + }) + require.NoError(t, err) + + compare(t, validConfigWithoutAutogen, string(configBody)) + }) + }) } func TestRouteGetAlertingConfigHistory(t *testing.T) { diff --git a/pkg/tests/api/alerting/api_alertmanager_test.go b/pkg/tests/api/alerting/api_alertmanager_test.go index 94f928ccbf7..242efebf928 100644 --- a/pkg/tests/api/alerting/api_alertmanager_test.go +++ b/pkg/tests/api/alerting/api_alertmanager_test.go @@ -2000,25 +2000,37 @@ func TestIntegrationAlertmanagerStatus(t *testing.T) { dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{ DisableLegacyAlerting: true, EnableUnifiedAlerting: true, + DisableAnonymous: true, AppModeProduction: true, }) - grafanaListedAddr, _ := testinfra.StartGrafana(t, dir, path) + grafanaListedAddr, store := testinfra.StartGrafana(t, dir, path) - // Get the Alertmanager current status. - { - alertsURL := fmt.Sprintf("http://%s/api/alertmanager/grafana/api/v2/status", grafanaListedAddr) - // nolint:gosec - resp, err := http.Get(alertsURL) - require.NoError(t, err) - t.Cleanup(func() { - err := resp.Body.Close() - require.NoError(t, err) - }) - b, err := io.ReadAll(resp.Body) - require.NoError(t, err) - require.Equal(t, 200, resp.StatusCode) - require.JSONEq(t, ` + // Create a users to make authenticated requests + createUser(t, store, user.CreateUserCommand{ + DefaultOrgRole: string(org.RoleViewer), + Password: "viewer", + Login: "viewer", + }) + createUser(t, store, user.CreateUserCommand{ + DefaultOrgRole: string(org.RoleEditor), + Password: "editor", + Login: "editor", + }) + createUser(t, store, user.CreateUserCommand{ + DefaultOrgRole: string(org.RoleAdmin), + Password: "admin", + Login: "admin", + }) + + type testCase struct { + desc string + url string + expStatus int + expBody string + } + + cfgBody := ` { "cluster": { "peers": [], @@ -2054,7 +2066,51 @@ func TestIntegrationAlertmanagerStatus(t *testing.T) { "version": "N/A" } } -`, string(b)) +` + + testCases := []testCase{ + { + desc: "un-authenticated request should fail", + url: "http://%s/api/alertmanager/grafana/api/v2/status", + expStatus: http.StatusUnauthorized, + expBody: `{"extra":null,"message":"Unauthorized","messageId":"auth.unauthorized","statusCode":401,"traceID":""}`, + }, + { + desc: "viewer request should succeed", + url: "http://viewer:viewer@%s/api/alertmanager/grafana/api/v2/status", + expStatus: http.StatusOK, + expBody: cfgBody, + }, + { + desc: "editor request should succeed", + url: "http://editor:editor@%s/api/alertmanager/grafana/api/v2/status", + expStatus: http.StatusOK, + expBody: cfgBody, + }, + { + desc: "admin request should succeed", + url: "http://admin:admin@%s/api/alertmanager/grafana/api/v2/status", + expStatus: http.StatusOK, + expBody: cfgBody, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + resp, err := http.Get(fmt.Sprintf(tc.url, grafanaListedAddr)) + t.Cleanup(func() { + require.NoError(t, resp.Body.Close()) + }) + require.NoError(t, err) + require.Equal(t, tc.expStatus, resp.StatusCode) + b, err := io.ReadAll(resp.Body) + if tc.expStatus == http.StatusOK { + re := regexp.MustCompile(`"uid":"([\w|-]+)"`) + b = re.ReplaceAll(b, []byte(`"uid":""`)) + } + require.NoError(t, err) + require.JSONEq(t, tc.expBody, string(b)) + }) } }