mirror of
https://github.com/grafana/grafana.git
synced 2024-11-25 18:30:41 -06:00
Alerting: Fix alerting evaluation to use proper permissions (#55127)
* access control to log user name if it does not have permissions * update ngalert Evaluator to accept user instead of creating a pseudo one * update alerting eval (rule\query testing) API to provide the real user to the Evaluator * update scheduler to create a pseudo user with proper permissions
This commit is contained in:
parent
d896db6d30
commit
896eeb65a9
@ -34,7 +34,7 @@ func (a *AccessControl) Evaluate(ctx context.Context, user *user.SignedInUser, e
|
||||
metrics.MAccessEvaluationCount.Inc()
|
||||
|
||||
if !verifyPermissions(user) {
|
||||
a.log.Warn("no permissions set for user", "userID", user.UserID, "orgID", user.OrgID)
|
||||
a.log.Warn("no permissions set for user", "userID", user.UserID, "orgID", user.OrgID, "login", user.Login)
|
||||
return false, nil
|
||||
}
|
||||
// Test evaluation without scope resolver first, this will prevent 403 for wildcard scopes when resource does not exist
|
||||
|
@ -53,7 +53,7 @@ func (srv TestingApiSrv) RouteTestGrafanaRuleConfig(c *models.ReqContext, body a
|
||||
now = timeNow()
|
||||
}
|
||||
|
||||
evalResults := srv.evaluator.ConditionEval(c.Req.Context(), evalCond, now)
|
||||
evalResults := srv.evaluator.ConditionEval(c.Req.Context(), c.SignedInUser, evalCond, now)
|
||||
|
||||
frame := evalResults.AsDataFrame()
|
||||
return response.JSONStreaming(http.StatusOK, util.DynMap{
|
||||
@ -115,7 +115,7 @@ func (srv TestingApiSrv) RouteEvalQueries(c *models.ReqContext, cmd apimodels.Ev
|
||||
return ErrResp(http.StatusBadRequest, err, "invalid queries or expressions")
|
||||
}
|
||||
|
||||
evalResults, err := srv.evaluator.QueriesAndExpressionsEval(c.Req.Context(), c.SignedInUser.OrgID, cmd.Data, now)
|
||||
evalResults, err := srv.evaluator.QueriesAndExpressionsEval(c.Req.Context(), c.SignedInUser, cmd.Data, now)
|
||||
if err != nil {
|
||||
return ErrResp(http.StatusBadRequest, err, "Failed to evaluate queries and expressions")
|
||||
}
|
||||
|
@ -70,7 +70,7 @@ func TestRouteTestGrafanaRuleConfig(t *testing.T) {
|
||||
|
||||
evaluator := &eval.FakeEvaluator{}
|
||||
var result []eval.Result
|
||||
evaluator.EXPECT().ConditionEval(mock.Anything, mock.Anything).Return(result)
|
||||
evaluator.EXPECT().ConditionEval(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(result)
|
||||
|
||||
srv := createTestingApiSrv(ds, ac, evaluator)
|
||||
|
||||
@ -85,7 +85,7 @@ func TestRouteTestGrafanaRuleConfig(t *testing.T) {
|
||||
|
||||
require.Equal(t, http.StatusOK, response.Status())
|
||||
|
||||
evaluator.AssertCalled(t, "ConditionEval", mock.Anything, mock.Anything, mock.Anything)
|
||||
evaluator.AssertCalled(t, "ConditionEval", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
|
||||
})
|
||||
})
|
||||
|
||||
@ -110,7 +110,7 @@ func TestRouteTestGrafanaRuleConfig(t *testing.T) {
|
||||
|
||||
evaluator := &eval.FakeEvaluator{}
|
||||
var result []eval.Result
|
||||
evaluator.EXPECT().ConditionEval(mock.Anything, mock.Anything).Return(result)
|
||||
evaluator.EXPECT().ConditionEval(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(result)
|
||||
|
||||
srv := createTestingApiSrv(ds, ac, evaluator)
|
||||
|
||||
@ -124,7 +124,7 @@ func TestRouteTestGrafanaRuleConfig(t *testing.T) {
|
||||
})
|
||||
|
||||
require.Equal(t, http.StatusUnauthorized, response.Status())
|
||||
evaluator.AssertNotCalled(t, "ConditionEval", mock.Anything, mock.Anything, mock.Anything)
|
||||
evaluator.AssertNotCalled(t, "ConditionEval", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
|
||||
|
||||
rc.IsSignedIn = true
|
||||
|
||||
@ -139,7 +139,7 @@ func TestRouteTestGrafanaRuleConfig(t *testing.T) {
|
||||
|
||||
require.Equal(t, http.StatusOK, response.Status())
|
||||
|
||||
evaluator.AssertCalled(t, "ConditionEval", mock.Anything, mock.Anything, mock.Anything)
|
||||
evaluator.AssertCalled(t, "ConditionEval", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
|
||||
})
|
||||
})
|
||||
}
|
||||
@ -198,7 +198,7 @@ func TestRouteEvalQueries(t *testing.T) {
|
||||
},
|
||||
},
|
||||
}
|
||||
evaluator.EXPECT().QueriesAndExpressionsEval(mock.Anything, mock.Anything, mock.Anything).Return(result, nil)
|
||||
evaluator.EXPECT().QueriesAndExpressionsEval(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(result, nil)
|
||||
|
||||
srv := createTestingApiSrv(ds, ac, evaluator)
|
||||
|
||||
@ -241,7 +241,7 @@ func TestRouteEvalQueries(t *testing.T) {
|
||||
},
|
||||
},
|
||||
}
|
||||
evaluator.EXPECT().QueriesAndExpressionsEval(mock.Anything, mock.Anything, mock.Anything).Return(result, nil)
|
||||
evaluator.EXPECT().QueriesAndExpressionsEval(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(result, nil)
|
||||
|
||||
srv := createTestingApiSrv(ds, ac, evaluator)
|
||||
|
||||
|
@ -16,7 +16,6 @@ import (
|
||||
"github.com/grafana/grafana/pkg/infra/log"
|
||||
"github.com/grafana/grafana/pkg/services/datasources"
|
||||
"github.com/grafana/grafana/pkg/services/ngalert/models"
|
||||
"github.com/grafana/grafana/pkg/services/org"
|
||||
"github.com/grafana/grafana/pkg/services/user"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
|
||||
@ -27,9 +26,9 @@ import (
|
||||
//go:generate mockery --name Evaluator --structname FakeEvaluator --inpackage --filename evaluator_mock.go --with-expecter
|
||||
type Evaluator interface {
|
||||
// ConditionEval executes conditions and evaluates the result.
|
||||
ConditionEval(ctx context.Context, condition models.Condition, now time.Time) Results
|
||||
ConditionEval(ctx context.Context, user *user.SignedInUser, condition models.Condition, now time.Time) Results
|
||||
// QueriesAndExpressionsEval executes queries and expressions and returns the result.
|
||||
QueriesAndExpressionsEval(ctx context.Context, orgID int64, data []models.AlertQuery, now time.Time) (*backend.QueryDataResponse, error)
|
||||
QueriesAndExpressionsEval(ctx context.Context, user *user.SignedInUser, data []models.AlertQuery, now time.Time) (*backend.QueryDataResponse, error)
|
||||
}
|
||||
|
||||
type evaluatorImpl struct {
|
||||
@ -151,7 +150,7 @@ func (s State) String() string {
|
||||
|
||||
// AlertExecCtx is the context provided for executing an alert condition.
|
||||
type AlertExecCtx struct {
|
||||
OrgID int64
|
||||
User *user.SignedInUser
|
||||
ExpressionsEnabled bool
|
||||
Log log.Logger
|
||||
|
||||
@ -161,7 +160,7 @@ type AlertExecCtx struct {
|
||||
// getExprRequest validates the condition, gets the datasource information and creates an expr.Request from it.
|
||||
func getExprRequest(ctx AlertExecCtx, data []models.AlertQuery, now time.Time, dsCacheService datasources.CacheService) (*expr.Request, error) {
|
||||
req := &expr.Request{
|
||||
OrgId: ctx.OrgID,
|
||||
OrgId: ctx.User.OrgID,
|
||||
Headers: map[string]string{
|
||||
// Some data sources check this in query method as sometimes alerting needs special considerations.
|
||||
"FromAlert": "true",
|
||||
@ -191,10 +190,7 @@ func getExprRequest(ctx AlertExecCtx, data []models.AlertQuery, now time.Time, d
|
||||
if expr.IsDataSource(q.DatasourceUID) {
|
||||
ds = expr.DataSourceModel()
|
||||
} else {
|
||||
ds, err = dsCacheService.GetDatasourceByUID(ctx.Ctx, q.DatasourceUID, &user.SignedInUser{
|
||||
OrgID: ctx.OrgID,
|
||||
OrgRole: org.RoleAdmin, // Get DS as admin for service, API calls (test/post) must check permissions based on user.
|
||||
}, true)
|
||||
ds, err = dsCacheService.GetDatasourceByUID(ctx.Ctx, q.DatasourceUID, ctx.User, true)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@ -552,8 +548,8 @@ func (evalResults Results) AsDataFrame() data.Frame {
|
||||
}
|
||||
|
||||
// ConditionEval executes conditions and evaluates the result.
|
||||
func (e *evaluatorImpl) ConditionEval(ctx context.Context, condition models.Condition, now time.Time) Results {
|
||||
execResp, err := e.QueriesAndExpressionsEval(ctx, condition.OrgID, condition.Data, now)
|
||||
func (e *evaluatorImpl) ConditionEval(ctx context.Context, user *user.SignedInUser, condition models.Condition, now time.Time) Results {
|
||||
execResp, err := e.QueriesAndExpressionsEval(ctx, user, condition.Data, now)
|
||||
var execResults ExecutionResults
|
||||
if err != nil {
|
||||
execResults = ExecutionResults{Error: err}
|
||||
@ -564,11 +560,11 @@ func (e *evaluatorImpl) ConditionEval(ctx context.Context, condition models.Cond
|
||||
}
|
||||
|
||||
// QueriesAndExpressionsEval executes queries and expressions and returns the result.
|
||||
func (e *evaluatorImpl) QueriesAndExpressionsEval(ctx context.Context, orgID int64, data []models.AlertQuery, now time.Time) (*backend.QueryDataResponse, error) {
|
||||
func (e *evaluatorImpl) QueriesAndExpressionsEval(ctx context.Context, user *user.SignedInUser, data []models.AlertQuery, now time.Time) (*backend.QueryDataResponse, error) {
|
||||
alertCtx, cancelFn := context.WithTimeout(ctx, e.cfg.UnifiedAlerting.EvaluationTimeout)
|
||||
defer cancelFn()
|
||||
|
||||
alertExecCtx := AlertExecCtx{OrgID: orgID, Ctx: alertCtx, ExpressionsEnabled: e.cfg.ExpressionsEnabled, Log: e.log}
|
||||
alertExecCtx := AlertExecCtx{User: user, Ctx: alertCtx, ExpressionsEnabled: e.cfg.ExpressionsEnabled, Log: e.log}
|
||||
|
||||
execResult, err := executeQueriesAndExpressions(alertExecCtx, data, now, e.expressionService, e.dataSourceCache)
|
||||
if err != nil {
|
||||
|
@ -1,16 +1,19 @@
|
||||
// Code generated by mockery v2.10.0. DO NOT EDIT.
|
||||
// Code generated by mockery v2.14.0. DO NOT EDIT.
|
||||
|
||||
package eval
|
||||
|
||||
import (
|
||||
"context"
|
||||
context "context"
|
||||
|
||||
backend "github.com/grafana/grafana-plugin-sdk-go/backend"
|
||||
|
||||
mock "github.com/stretchr/testify/mock"
|
||||
|
||||
models "github.com/grafana/grafana/pkg/services/ngalert/models"
|
||||
|
||||
time "time"
|
||||
|
||||
user "github.com/grafana/grafana/pkg/services/user"
|
||||
)
|
||||
|
||||
// FakeEvaluator is an autogenerated mock type for the Evaluator type
|
||||
@ -26,13 +29,13 @@ func (_m *FakeEvaluator) EXPECT() *FakeEvaluator_Expecter {
|
||||
return &FakeEvaluator_Expecter{mock: &_m.Mock}
|
||||
}
|
||||
|
||||
// ConditionEval provides a mock function with given fields: condition, now
|
||||
func (_m *FakeEvaluator) ConditionEval(ctx context.Context, condition models.Condition, now time.Time) Results {
|
||||
ret := _m.Called(condition, now)
|
||||
// ConditionEval provides a mock function with given fields: ctx, _a1, condition, now
|
||||
func (_m *FakeEvaluator) ConditionEval(ctx context.Context, _a1 *user.SignedInUser, condition models.Condition, now time.Time) Results {
|
||||
ret := _m.Called(ctx, _a1, condition, now)
|
||||
|
||||
var r0 Results
|
||||
if rf, ok := ret.Get(0).(func(models.Condition, time.Time) Results); ok {
|
||||
r0 = rf(condition, now)
|
||||
if rf, ok := ret.Get(0).(func(context.Context, *user.SignedInUser, models.Condition, time.Time) Results); ok {
|
||||
r0 = rf(ctx, _a1, condition, now)
|
||||
} else {
|
||||
if ret.Get(0) != nil {
|
||||
r0 = ret.Get(0).(Results)
|
||||
@ -48,15 +51,17 @@ type FakeEvaluator_ConditionEval_Call struct {
|
||||
}
|
||||
|
||||
// ConditionEval is a helper method to define mock.On call
|
||||
// - ctx context.Context
|
||||
// - _a1 *user.SignedInUser
|
||||
// - condition models.Condition
|
||||
// - now time.Time
|
||||
func (_e *FakeEvaluator_Expecter) ConditionEval(condition interface{}, now interface{}) *FakeEvaluator_ConditionEval_Call {
|
||||
return &FakeEvaluator_ConditionEval_Call{Call: _e.mock.On("ConditionEval", condition, now)}
|
||||
func (_e *FakeEvaluator_Expecter) ConditionEval(ctx interface{}, _a1 interface{}, condition interface{}, now interface{}) *FakeEvaluator_ConditionEval_Call {
|
||||
return &FakeEvaluator_ConditionEval_Call{Call: _e.mock.On("ConditionEval", ctx, _a1, condition, now)}
|
||||
}
|
||||
|
||||
func (_c *FakeEvaluator_ConditionEval_Call) Run(run func(condition models.Condition, now time.Time)) *FakeEvaluator_ConditionEval_Call {
|
||||
func (_c *FakeEvaluator_ConditionEval_Call) Run(run func(ctx context.Context, _a1 *user.SignedInUser, condition models.Condition, now time.Time)) *FakeEvaluator_ConditionEval_Call {
|
||||
_c.Call.Run(func(args mock.Arguments) {
|
||||
run(args[0].(models.Condition), args[1].(time.Time))
|
||||
run(args[0].(context.Context), args[1].(*user.SignedInUser), args[2].(models.Condition), args[3].(time.Time))
|
||||
})
|
||||
return _c
|
||||
}
|
||||
@ -66,13 +71,13 @@ func (_c *FakeEvaluator_ConditionEval_Call) Return(_a0 Results) *FakeEvaluator_C
|
||||
return _c
|
||||
}
|
||||
|
||||
// QueriesAndExpressionsEval provides a mock function with given fields: orgID, data, now
|
||||
func (_m *FakeEvaluator) QueriesAndExpressionsEval(ctx context.Context, orgID int64, data []models.AlertQuery, now time.Time) (*backend.QueryDataResponse, error) {
|
||||
ret := _m.Called(orgID, data, now)
|
||||
// QueriesAndExpressionsEval provides a mock function with given fields: ctx, _a1, data, now
|
||||
func (_m *FakeEvaluator) QueriesAndExpressionsEval(ctx context.Context, _a1 *user.SignedInUser, data []models.AlertQuery, now time.Time) (*backend.QueryDataResponse, error) {
|
||||
ret := _m.Called(ctx, _a1, data, now)
|
||||
|
||||
var r0 *backend.QueryDataResponse
|
||||
if rf, ok := ret.Get(0).(func(int64, []models.AlertQuery, time.Time) *backend.QueryDataResponse); ok {
|
||||
r0 = rf(orgID, data, now)
|
||||
if rf, ok := ret.Get(0).(func(context.Context, *user.SignedInUser, []models.AlertQuery, time.Time) *backend.QueryDataResponse); ok {
|
||||
r0 = rf(ctx, _a1, data, now)
|
||||
} else {
|
||||
if ret.Get(0) != nil {
|
||||
r0 = ret.Get(0).(*backend.QueryDataResponse)
|
||||
@ -80,8 +85,8 @@ func (_m *FakeEvaluator) QueriesAndExpressionsEval(ctx context.Context, orgID in
|
||||
}
|
||||
|
||||
var r1 error
|
||||
if rf, ok := ret.Get(1).(func(int64, []models.AlertQuery, time.Time) error); ok {
|
||||
r1 = rf(orgID, data, now)
|
||||
if rf, ok := ret.Get(1).(func(context.Context, *user.SignedInUser, []models.AlertQuery, time.Time) error); ok {
|
||||
r1 = rf(ctx, _a1, data, now)
|
||||
} else {
|
||||
r1 = ret.Error(1)
|
||||
}
|
||||
@ -95,16 +100,17 @@ type FakeEvaluator_QueriesAndExpressionsEval_Call struct {
|
||||
}
|
||||
|
||||
// QueriesAndExpressionsEval is a helper method to define mock.On call
|
||||
// - orgID int64
|
||||
// - ctx context.Context
|
||||
// - _a1 *user.SignedInUser
|
||||
// - data []models.AlertQuery
|
||||
// - now time.Time
|
||||
func (_e *FakeEvaluator_Expecter) QueriesAndExpressionsEval(orgID interface{}, data interface{}, now interface{}) *FakeEvaluator_QueriesAndExpressionsEval_Call {
|
||||
return &FakeEvaluator_QueriesAndExpressionsEval_Call{Call: _e.mock.On("QueriesAndExpressionsEval", orgID, data, now)}
|
||||
func (_e *FakeEvaluator_Expecter) QueriesAndExpressionsEval(ctx interface{}, _a1 interface{}, data interface{}, now interface{}) *FakeEvaluator_QueriesAndExpressionsEval_Call {
|
||||
return &FakeEvaluator_QueriesAndExpressionsEval_Call{Call: _e.mock.On("QueriesAndExpressionsEval", ctx, _a1, data, now)}
|
||||
}
|
||||
|
||||
func (_c *FakeEvaluator_QueriesAndExpressionsEval_Call) Run(run func(orgID int64, data []models.AlertQuery, now time.Time)) *FakeEvaluator_QueriesAndExpressionsEval_Call {
|
||||
func (_c *FakeEvaluator_QueriesAndExpressionsEval_Call) Run(run func(ctx context.Context, _a1 *user.SignedInUser, data []models.AlertQuery, now time.Time)) *FakeEvaluator_QueriesAndExpressionsEval_Call {
|
||||
_c.Call.Run(func(args mock.Arguments) {
|
||||
run(args[0].(int64), args[1].([]models.AlertQuery), args[2].(time.Time))
|
||||
run(args[0].(context.Context), args[1].(*user.SignedInUser), args[2].([]models.AlertQuery), args[3].(time.Time))
|
||||
})
|
||||
return _c
|
||||
}
|
||||
@ -113,3 +119,18 @@ func (_c *FakeEvaluator_QueriesAndExpressionsEval_Call) Return(_a0 *backend.Quer
|
||||
_c.Call.Return(_a0, _a1)
|
||||
return _c
|
||||
}
|
||||
|
||||
type mockConstructorTestingTNewFakeEvaluator interface {
|
||||
mock.TestingT
|
||||
Cleanup(func())
|
||||
}
|
||||
|
||||
// NewFakeEvaluator creates a new instance of FakeEvaluator. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
|
||||
func NewFakeEvaluator(t mockConstructorTestingTNewFakeEvaluator) *FakeEvaluator {
|
||||
mock := &FakeEvaluator{}
|
||||
mock.Mock.Test(t)
|
||||
|
||||
t.Cleanup(func() { mock.AssertExpectations(t) })
|
||||
|
||||
return mock
|
||||
}
|
||||
|
@ -11,12 +11,15 @@ import (
|
||||
|
||||
"github.com/grafana/grafana/pkg/infra/log"
|
||||
"github.com/grafana/grafana/pkg/services/alerting"
|
||||
"github.com/grafana/grafana/pkg/services/datasources"
|
||||
"github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
|
||||
"github.com/grafana/grafana/pkg/services/ngalert/eval"
|
||||
"github.com/grafana/grafana/pkg/services/ngalert/metrics"
|
||||
ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
|
||||
"github.com/grafana/grafana/pkg/services/ngalert/state"
|
||||
"github.com/grafana/grafana/pkg/services/ngalert/store"
|
||||
"github.com/grafana/grafana/pkg/services/org"
|
||||
"github.com/grafana/grafana/pkg/services/user"
|
||||
"github.com/grafana/grafana/pkg/setting"
|
||||
|
||||
"github.com/benbjohnson/clock"
|
||||
@ -331,7 +334,21 @@ func (sch *schedule) ruleRoutine(grafanaCtx context.Context, key ngmodels.AlertR
|
||||
logger := logger.New("version", e.rule.Version, "attempt", attempt, "now", e.scheduledAt)
|
||||
start := sch.clock.Now()
|
||||
|
||||
results := sch.evaluator.ConditionEval(ctx, e.rule.GetEvalCondition(), e.scheduledAt)
|
||||
schedulerUser := &user.SignedInUser{
|
||||
UserID: -1,
|
||||
Login: "grafana_scheduler",
|
||||
OrgID: e.rule.OrgID,
|
||||
OrgRole: org.RoleAdmin,
|
||||
Permissions: map[int64]map[string][]string{
|
||||
e.rule.OrgID: {
|
||||
datasources.ActionQuery: []string{
|
||||
datasources.ScopeAll,
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
results := sch.evaluator.ConditionEval(ctx, schedulerUser, e.rule.GetEvalCondition(), e.scheduledAt)
|
||||
dur := sch.clock.Now().Sub(start)
|
||||
evalTotal.Inc()
|
||||
evalDuration.Observe(dur.Seconds())
|
||||
|
Loading…
Reference in New Issue
Block a user