[Alerting]: namespace fixes (#34470)

* [Alerting]: forbid viewers for updating rules if viewers can edit

check for CanSave instead of CanEdit

* Clear ngalert tables when deleting the folder

* Apply suggestions from code review

* Log failure to check save permission

Co-authored-by: gotjosh <josue@grafana.com>
This commit is contained in:
Sofia Papagiannaki 2021-05-20 15:49:33 +03:00 committed by GitHub
parent fd6e338651
commit 23939eab10
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 315 additions and 21 deletions

View File

@ -69,6 +69,7 @@ func (ng *AlertNG) Init() error {
BaseInterval: baseInterval,
DefaultIntervalSeconds: defaultIntervalSeconds,
SQLStore: ng.SQLStore,
Logger: ng.Log,
}
var err error

View File

@ -9,6 +9,8 @@ import (
"testing"
"time"
"github.com/grafana/grafana/pkg/infra/log"
gokit_log "github.com/go-kit/kit/log"
"github.com/go-openapi/strfmt"
"github.com/prometheus/alertmanager/api/v2/models"
@ -42,6 +44,7 @@ func setupAMTest(t *testing.T) *Alertmanager {
BaseInterval: 10 * time.Second,
DefaultIntervalSeconds: 60,
SQLStore: sqlStore,
Logger: log.New("alertmanager-test"),
}
am, err := New(cfg, store, m)

View File

@ -368,16 +368,19 @@ func (st DBstore) GetRuleGroupAlertRules(query *ngmodels.ListRuleGroupAlertRules
}
// 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(namespace string, orgID int64, user *models.SignedInUser, withEdit bool) (*models.Folder, error) {
func (st DBstore) GetNamespaceByTitle(namespace string, orgID int64, user *models.SignedInUser, withCanSave bool) (*models.Folder, error) {
s := dashboards.NewFolderService(orgID, user, st.SQLStore)
folder, err := s.GetFolderByTitle(namespace)
if err != nil {
return nil, err
}
if withEdit {
if withCanSave {
g := guardian.New(folder.Id, orgID, user)
if canAdmin, err := g.CanEdit(); err != nil || !canAdmin {
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
}
}

View File

@ -3,8 +3,8 @@ package store
import (
"time"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/ngalert/models"
"github.com/grafana/grafana/pkg/services/sqlstore"
)
@ -28,4 +28,5 @@ type DBstore struct {
// default alert definiiton interval
DefaultIntervalSeconds int64
SQLStore *sqlstore.SQLStore
Logger log.Logger
}

View File

@ -38,7 +38,11 @@ func SetupTestEnv(t *testing.T, baseIntervalSeconds int64) *store.DBstore {
err := ng.Init()
require.NoError(t, err)
return &store.DBstore{SQLStore: ng.SQLStore, BaseInterval: time.Duration(baseIntervalSeconds) * time.Second}
return &store.DBstore{
SQLStore: ng.SQLStore,
BaseInterval: time.Duration(baseIntervalSeconds) * time.Second,
Logger: log.New("ngalert-test"),
}
}
func overrideAlertNGInRegistry(t *testing.T, cfg *setting.Cfg) ngalert.AlertNG {

View File

@ -465,6 +465,19 @@ func deleteDashboard(cmd *models.DeleteDashboardCommand, sess *DBSession) error
}
}
}
// clean ngalert tables
ngalertDeletes := []string{
"DELETE FROM alert_rule WHERE namespace_uid = (SELECT uid FROM dashboard WHERE id = ?)",
"DELETE FROM alert_rule_version WHERE rule_namespace_uid = (SELECT uid FROM dashboard WHERE id = ?)",
}
for _, sql := range ngalertDeletes {
_, err := sess.Exec(sql, dashboard.Id)
if err != nil {
return err
}
}
}
if err := deleteAlertDefinition(dashboard.Id, sess); err != nil {

View File

@ -285,7 +285,6 @@ func TestAMConfigAccess(t *testing.T) {
// Fetch Request
resp, err := client.Do(req)
if err != nil {
fmt.Println(err)
return
}
t.Cleanup(func() {
@ -386,7 +385,8 @@ func TestAlertAndGroupsQuery(t *testing.T) {
// Now, let's test the endpoint with some alerts.
{
// Create the namespace we'll save our alerts to.
require.NoError(t, createFolder(t, store, 0, "default"))
_, err := createFolder(t, store, 0, "default")
require.NoError(t, err)
}
// Create an alert that will fire as quickly as possible
@ -464,6 +464,255 @@ func TestAlertAndGroupsQuery(t *testing.T) {
}
}
func TestRulerAccess(t *testing.T) {
// Setup Grafana and its Database
dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
EnableFeatureToggles: []string{"ngalert"},
EnableQuota: true,
DisableAnonymous: true,
ViewersCanEdit: true,
})
store := testinfra.SetUpDatabase(t, dir)
// override bus to get the GetSignedInUserQuery handler
store.Bus = bus.GetBus()
grafanaListedAddr := testinfra.StartGrafana(t, dir, path, store)
// Create the namespace we'll save our alerts to.
_, err := createFolder(t, store, 0, "default")
require.NoError(t, err)
// Create a users to make authenticated requests
require.NoError(t, createUser(t, store, models.ROLE_VIEWER, "viewer", "viewer"))
require.NoError(t, createUser(t, store, models.ROLE_EDITOR, "editor", "editor"))
require.NoError(t, createUser(t, store, models.ROLE_ADMIN, "admin", "admin"))
// Now, let's test the access policies.
testCases := []struct {
desc string
url string
expStatus int
expectedResponse string
}{
{
desc: "un-authenticated request should fail",
url: "http://%s/api/ruler/grafana/api/v1/rules/default",
expStatus: http.StatusUnauthorized,
expectedResponse: `{"message": "Unauthorized"}`,
},
{
desc: "viewer request should fail",
url: "http://viewer:viewer@%s/api/ruler/grafana/api/v1/rules/default",
expStatus: http.StatusForbidden,
expectedResponse: `{"error":"user does not have permissions to edit the namespace", "message":"user does not have permissions to edit the namespace"}`,
},
{
desc: "editor request should succeed",
url: "http://editor:editor@%s/api/ruler/grafana/api/v1/rules/default",
expStatus: http.StatusAccepted,
expectedResponse: `{"message":"rule group updated successfully"}`,
},
{
desc: "admin request should succeed",
url: "http://admin:admin@%s/api/ruler/grafana/api/v1/rules/default",
expStatus: http.StatusAccepted,
expectedResponse: `{"message":"rule group updated successfully"}`,
},
}
for i, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
interval, err := model.ParseDuration("1m")
require.NoError(t, err)
rules := apimodels.PostableRuleGroupConfig{
Name: "arulegroup",
Rules: []apimodels.PostableExtendedRuleNode{
{
ApiRuleNode: &apimodels.ApiRuleNode{
For: interval,
Labels: map[string]string{"label1": "val1"},
Annotations: map[string]string{"annotation1": "val1"},
},
// this rule does not explicitly set no data and error states
// therefore it should get the default values
GrafanaManagedAlert: &apimodels.PostableGrafanaRule{
Title: fmt.Sprintf("AlwaysFiring %d", i),
Condition: "A",
Data: []ngmodels.AlertQuery{
{
RefID: "A",
RelativeTimeRange: ngmodels.RelativeTimeRange{
From: ngmodels.Duration(time.Duration(5) * time.Hour),
To: ngmodels.Duration(time.Duration(3) * time.Hour),
},
DatasourceUID: "-100",
Model: json.RawMessage(`{
"type": "math",
"expression": "2 + 3 > 1"
}`),
},
},
},
},
},
}
buf := bytes.Buffer{}
enc := json.NewEncoder(&buf)
err = enc.Encode(&rules)
require.NoError(t, err)
u := fmt.Sprintf(tc.url, grafanaListedAddr)
// nolint:gosec
resp, err := http.Post(u, "application/json", &buf)
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, tc.expStatus, resp.StatusCode)
require.JSONEq(t, tc.expectedResponse, string(b))
})
}
}
func TestDeleteFolderWithRules(t *testing.T) {
// Setup Grafana and its Database
dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
EnableFeatureToggles: []string{"ngalert"},
EnableQuota: true,
DisableAnonymous: true,
ViewersCanEdit: true,
})
store := testinfra.SetUpDatabase(t, dir)
// override bus to get the GetSignedInUserQuery handler
store.Bus = bus.GetBus()
grafanaListedAddr := testinfra.StartGrafana(t, dir, path, store)
// Create the namespace we'll save our alerts to.
namespaceUID, err := createFolder(t, store, 0, "default")
require.NoError(t, err)
require.NoError(t, createUser(t, store, models.ROLE_VIEWER, "viewer", "viewer"))
require.NoError(t, createUser(t, store, models.ROLE_EDITOR, "editor", "editor"))
createRule(t, grafanaListedAddr, "default", "editor", "editor")
// First, let's have an editor create a rule within the folder/namespace.
{
u := fmt.Sprintf("http://editor:editor@%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, 202, resp.StatusCode)
re := regexp.MustCompile(`"uid":"([\w|-]+)"`)
b = re.ReplaceAll(b, []byte(`"uid":""`))
re = regexp.MustCompile(`"updated":"(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z)"`)
b = re.ReplaceAll(b, []byte(`"updated":"2021-05-19T19:47:55Z"`))
expectedGetRulesResponseBody := fmt.Sprintf(`{
"default": [
{
"name": "arulegroup",
"interval": "1m",
"rules": [
{
"expr": "",
"for": "2m",
"labels": {
"label1": "val1"
},
"annotations": {
"annotation1": "val1"
},
"grafana_alert": {
"id": 1,
"orgId": 1,
"title": "rule under folder default",
"condition": "A",
"data": [
{
"refId": "A",
"queryType": "",
"relativeTimeRange": {
"from": 18000,
"to": 10800
},
"datasourceUid": "-100",
"model": {
"expression": "2 + 3 > 1",
"intervalMs": 1000,
"maxDataPoints": 43200,
"type": "math"
}
}
],
"updated": "2021-05-19T19:47:55Z",
"intervalSeconds": 60,
"version": 1,
"uid": "",
"namespace_uid": %q,
"namespace_id": 1,
"rule_group": "arulegroup",
"no_data_state": "NoData",
"exec_err_state": "Alerting"
}
}
]
}
]
}`, namespaceUID)
assert.JSONEq(t, expectedGetRulesResponseBody, string(b))
}
// Next, the editor can delete the folder.
{
u := fmt.Sprintf("http://editor:editor@%s/api/folders/%s", grafanaListedAddr, namespaceUID)
req, err := http.NewRequest(http.MethodDelete, u, nil)
require.NoError(t, err)
client := &http.Client{}
resp, err := client.Do(req)
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, `{"id":1,"message":"Folder default deleted","title":"default"}`, string(b))
}
// Finally, we ensure the rules were deleted.
{
u := fmt.Sprintf("http://editor:editor@%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, 202, resp.StatusCode)
assert.JSONEq(t, "{}", string(b))
}
}
func TestAlertRuleCRUD(t *testing.T) {
// Setup Grafana and its Database
dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
@ -478,10 +727,12 @@ func TestAlertRuleCRUD(t *testing.T) {
grafanaListedAddr := testinfra.StartGrafana(t, dir, path, store)
err := createUser(t, store, models.ROLE_EDITOR, "grafana", "password")
require.NoError(t, err)
// Create the namespace we'll save our alerts to.
require.NoError(t, createFolder(t, store, 0, "default"))
_, err = createFolder(t, store, 0, "default")
require.NoError(t, err)
interval, err := model.ParseDuration("1m")
require.NoError(t, err)
@ -1197,7 +1448,8 @@ func TestQuota(t *testing.T) {
grafanaListedAddr := testinfra.StartGrafana(t, dir, path, store)
// Create the namespace we'll save our alerts to.
require.NoError(t, createFolder(t, store, 0, "default"))
_, err := createFolder(t, store, 0, "default")
require.NoError(t, err)
// Create a user to make authenticated requests
require.NoError(t, createUser(t, store, models.ROLE_EDITOR, "grafana", "password"))
@ -1298,7 +1550,8 @@ func TestEval(t *testing.T) {
require.NoError(t, createUser(t, store, models.ROLE_EDITOR, "grafana", "password"))
// Create the namespace we'll save our alerts to.
require.NoError(t, createFolder(t, store, 0, "default"))
_, err := createFolder(t, store, 0, "default")
require.NoError(t, err)
// test eval conditions
testCases := []struct {
@ -1641,7 +1894,7 @@ func TestEval(t *testing.T) {
// createFolder creates a folder for storing our alerts under. Grafana uses folders as a replacement for alert namespaces to match its permission model.
// We use the dashboard command using IsFolder = true to tell it's a folder, it takes the dashboard as the name of the folder.
func createFolder(t *testing.T, store *sqlstore.SQLStore, folderID int64, folderName string) error {
func createFolder(t *testing.T, store *sqlstore.SQLStore, folderID int64, folderName string) (string, error) {
t.Helper()
cmd := models.SaveDashboardCommand{
@ -1652,9 +1905,13 @@ func createFolder(t *testing.T, store *sqlstore.SQLStore, folderID int64, folder
"title": folderName,
}),
}
_, err := store.SaveDashboard(cmd)
f, err := store.SaveDashboard(cmd)
return err
if err != nil {
return "", err
}
return f.Uid, nil
}
// rulesNamespaceWithoutVariableValues takes a apimodels.NamespaceConfigResponse JSON-based input and makes the dynamic fields static e.g. uid, dates, etc.

View File

@ -57,7 +57,8 @@ func TestNotificationChannels(t *testing.T) {
{
// Create the namespace we'll save our alerts to.
require.NoError(t, createFolder(t, s, 0, "default"))
_, err := createFolder(t, s, 0, "default")
require.NoError(t, err)
// Post the alertmanager config.
u := fmt.Sprintf("http://grafana:password@%s/api/alertmanager/grafana/config/api/v1/alerts", grafanaListedAddr)

View File

@ -30,7 +30,8 @@ func TestPrometheusRules(t *testing.T) {
grafanaListedAddr := testinfra.StartGrafana(t, dir, path, store)
// Create the namespace under default organisation (orgID = 1) where we'll save our alerts to.
require.NoError(t, createFolder(t, store, 0, "default"))
_, err := createFolder(t, store, 0, "default")
require.NoError(t, err)
// Create a user to make authenticated requests
require.NoError(t, createUser(t, store, models.ROLE_EDITOR, "grafana", "password"))

View File

@ -34,16 +34,18 @@ func TestAlertRulePermissions(t *testing.T) {
require.NoError(t, createUser(t, store, models.ROLE_EDITOR, "grafana", "password"))
// Create the namespace we'll save our alerts to.
require.NoError(t, createFolder(t, store, 0, "folder1"))
_, err := createFolder(t, store, 0, "folder1")
require.NoError(t, err)
_, err = createFolder(t, store, 0, "folder2")
// Create the namespace we'll save our alerts to.
require.NoError(t, createFolder(t, store, 0, "folder2"))
require.NoError(t, err)
// Create rule under folder1
createRule(t, grafanaListedAddr, "folder1")
createRule(t, grafanaListedAddr, "folder1", "grafana", "password")
// Create rule under folder2
createRule(t, grafanaListedAddr, "folder2")
createRule(t, grafanaListedAddr, "folder2", "grafana", "password")
// With the rules created, let's make sure that rule definitions are stored.
{
@ -240,7 +242,7 @@ func TestAlertRulePermissions(t *testing.T) {
}
}
func createRule(t *testing.T, grafanaListedAddr string, folder string) {
func createRule(t *testing.T, grafanaListedAddr string, folder string, user, password string) {
t.Helper()
interval, err := model.ParseDuration("1m")
@ -282,7 +284,7 @@ func createRule(t *testing.T, grafanaListedAddr string, folder string) {
err = enc.Encode(&rules)
require.NoError(t, err)
u := fmt.Sprintf("http://grafana:password@%s/api/ruler/grafana/api/v1/rules/%s", grafanaListedAddr, folder)
u := fmt.Sprintf("http://%s:%s@%s/api/ruler/grafana/api/v1/rules/%s", user, password, grafanaListedAddr, folder)
// nolint:gosec
resp, err := http.Post(u, "application/json", &buf)
require.NoError(t, err)

View File

@ -239,6 +239,12 @@ func CreateGrafDir(t *testing.T, opts ...GrafanaOpts) (string, string) {
_, err = anonSect.NewKey("plugin_admin_enabled", "true")
require.NoError(t, err)
}
if o.ViewersCanEdit {
usersSection, err := cfg.NewSection("users")
require.NoError(t, err)
_, err = usersSection.NewKey("viewers_can_edit", "true")
require.NoError(t, err)
}
}
cfgPath := filepath.Join(cfgDir, "test.ini")
@ -257,5 +263,7 @@ type GrafanaOpts struct {
AnonymousUserRole models.RoleType
EnableQuota bool
DisableAnonymous bool
CatalogAppEnabled bool
ViewersCanEdit bool
PluginAdminEnabled bool
}