From 3282afc648edc6cd5793056753b2ffc397c859aa Mon Sep 17 00:00:00 2001 From: Ieva Date: Tue, 20 Sep 2022 16:40:52 +0100 Subject: [PATCH] V8.5.12 security patch (#486) (#55495) * Data source: prevent from using auth proxy header as custom data source header (#477) * apply security changes for auth proxy permission escalation * add links to CVE * remove duplicate check * apply security fix for admin only folder migration (#484) Co-authored-by: Karl Persson Co-authored-by: Karl Persson --- pkg/api/datasources.go | 30 ++++++ pkg/api/datasources_test.go | 65 ++++++++++++ .../datasources/service/datasource_service.go | 17 ++- .../service/datasource_service_test.go | 79 +++++++++++++- pkg/services/ngalert/eval/eval.go | 44 ++------ pkg/services/query/query.go | 29 +---- pkg/services/query/query_test.go | 3 +- .../migrations/accesscontrol/admin_only.go | 100 ++++++++++++++++++ .../accesscontrol/dashboard_permissions.go | 7 +- .../sqlstore/migrations/migrations.go | 8 +- 10 files changed, 309 insertions(+), 73 deletions(-) create mode 100644 pkg/services/sqlstore/migrations/accesscontrol/admin_only.go diff --git a/pkg/api/datasources.go b/pkg/api/datasources.go index 55fea4d9696..b4301dcd4ec 100644 --- a/pkg/api/datasources.go +++ b/pkg/api/datasources.go @@ -4,19 +4,23 @@ import ( "context" "encoding/json" "errors" + "fmt" "net/http" "sort" "strconv" + "strings" "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana/pkg/api/datasource" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" + "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/plugins/adapters" "github.com/grafana/grafana/pkg/services/datasources" "github.com/grafana/grafana/pkg/services/datasources/permissions" + "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/web" ) @@ -241,6 +245,26 @@ func validateURL(cmdType string, url string) response.Response { return nil } +// validateJSONData prevents the user from adding a custom header with name that matches the auth proxy header name. +// This is done to prevent data source proxy from being used to circumvent auth proxy. +// For more context take a look at CVE-2022-35957 +func validateJSONData(jsonData *simplejson.Json, cfg *setting.Cfg) error { + if jsonData == nil || !cfg.AuthProxyEnabled { + return nil + } + + for key, value := range jsonData.MustMap() { + if strings.HasPrefix(key, "httpHeaderName") { + header := fmt.Sprint(value) + if http.CanonicalHeaderKey(header) == http.CanonicalHeaderKey(cfg.AuthProxyHeaderName) { + datasourcesLogger.Error("Forbidden to add a data source header with a name equal to auth proxy header name", "headerName", key) + return errors.New("validation error, invalid header name specified") + } + } + } + return nil +} + // POST /api/datasources/ func (hs *HTTPServer) AddDataSource(c *models.ReqContext) response.Response { cmd := models.AddDataSourceCommand{} @@ -256,6 +280,9 @@ func (hs *HTTPServer) AddDataSource(c *models.ReqContext) response.Response { return resp } } + if err := validateJSONData(cmd.JsonData, hs.Cfg); err != nil { + return response.Error(http.StatusBadRequest, "Failed to add datasource", err) + } if err := hs.DataSourcesService.AddDataSource(c.Req.Context(), &cmd); err != nil { if errors.Is(err, models.ErrDataSourceNameExists) || errors.Is(err, models.ErrDataSourceUidExists) { @@ -289,6 +316,9 @@ func (hs *HTTPServer) UpdateDataSource(c *models.ReqContext) response.Response { if resp := validateURL(cmd.Type, cmd.Url); resp != nil { return resp } + if err := validateJSONData(cmd.JsonData, hs.Cfg); err != nil { + return response.Error(http.StatusBadRequest, "Failed to update datasource", err) + } ds, err := hs.getRawDataSourceById(c.Req.Context(), cmd.Id, cmd.OrgId) if err != nil { diff --git a/pkg/api/datasources_test.go b/pkg/api/datasources_test.go index c490e2a0142..21805196f01 100644 --- a/pkg/api/datasources_test.go +++ b/pkg/api/datasources_test.go @@ -15,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/routing" + "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/models" ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/datasources" @@ -82,6 +83,7 @@ func TestAddDataSource_InvalidURL(t *testing.T) { sc := setupScenarioContext(t, "/api/datasources") hs := &HTTPServer{ DataSourcesService: &dataSourcesServiceMock{}, + Cfg: setting.NewCfg(), } sc.m.Post(sc.url, routing.Wrap(func(c *models.ReqContext) response.Response { @@ -108,6 +110,7 @@ func TestAddDataSource_URLWithoutProtocol(t *testing.T) { DataSourcesService: &dataSourcesServiceMock{ expectedDatasource: &models.DataSource{}, }, + Cfg: setting.NewCfg(), } sc := setupScenarioContext(t, "/api/datasources") @@ -127,10 +130,42 @@ func TestAddDataSource_URLWithoutProtocol(t *testing.T) { assert.Equal(t, 200, sc.resp.Code) } +// Using a custom header whose name matches the name specified for auth proxy header should fail +func TestAddDataSource_InvalidJSONData(t *testing.T) { + hs := &HTTPServer{ + DataSourcesService: &dataSourcesServiceMock{}, + Cfg: setting.NewCfg(), + } + + sc := setupScenarioContext(t, "/api/datasources") + + hs.Cfg = setting.NewCfg() + hs.Cfg.AuthProxyEnabled = true + hs.Cfg.AuthProxyHeaderName = "X-AUTH-PROXY-HEADER" + jsonData := simplejson.New() + jsonData.Set("httpHeaderName1", hs.Cfg.AuthProxyHeaderName) + + sc.m.Post(sc.url, routing.Wrap(func(c *models.ReqContext) response.Response { + c.Req.Body = mockRequestBody(models.AddDataSourceCommand{ + Name: "Test", + Url: "localhost:5432", + Access: "direct", + Type: "test", + JsonData: jsonData, + }) + return hs.AddDataSource(c) + })) + + sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() + + assert.Equal(t, 400, sc.resp.Code) +} + // Updating data sources with invalid URLs should lead to an error. func TestUpdateDataSource_InvalidURL(t *testing.T) { hs := &HTTPServer{ DataSourcesService: &dataSourcesServiceMock{}, + Cfg: setting.NewCfg(), } sc := setupScenarioContext(t, "/api/datasources/1234") @@ -149,6 +184,35 @@ func TestUpdateDataSource_InvalidURL(t *testing.T) { assert.Equal(t, 400, sc.resp.Code) } +// Using a custom header whose name matches the name specified for auth proxy header should fail +func TestUpdateDataSource_InvalidJSONData(t *testing.T) { + hs := &HTTPServer{ + DataSourcesService: &dataSourcesServiceMock{}, + Cfg: setting.NewCfg(), + } + sc := setupScenarioContext(t, "/api/datasources/1234") + + hs.Cfg.AuthProxyEnabled = true + hs.Cfg.AuthProxyHeaderName = "X-AUTH-PROXY-HEADER" + jsonData := simplejson.New() + jsonData.Set("httpHeaderName1", hs.Cfg.AuthProxyHeaderName) + + sc.m.Put(sc.url, routing.Wrap(func(c *models.ReqContext) response.Response { + c.Req.Body = mockRequestBody(models.AddDataSourceCommand{ + Name: "Test", + Url: "localhost:5432", + Access: "direct", + Type: "test", + JsonData: jsonData, + }) + return hs.AddDataSource(c) + })) + + sc.fakeReqWithParams("PUT", sc.url, map[string]string{}).exec() + + assert.Equal(t, 400, sc.resp.Code) +} + // Updating data sources with URLs not specifying protocol should work. func TestUpdateDataSource_URLWithoutProtocol(t *testing.T) { const name = "Test" @@ -158,6 +222,7 @@ func TestUpdateDataSource_URLWithoutProtocol(t *testing.T) { DataSourcesService: &dataSourcesServiceMock{ expectedDatasource: &models.DataSource{}, }, + Cfg: setting.NewCfg(), } sc := setupScenarioContext(t, "/api/datasources/1234") diff --git a/pkg/services/datasources/service/datasource_service.go b/pkg/services/datasources/service/datasource_service.go index dbc22e6e261..eccace7c04b 100644 --- a/pkg/services/datasources/service/datasource_service.go +++ b/pkg/services/datasources/service/datasource_service.go @@ -309,7 +309,7 @@ func (s *Service) httpClientOptions(ds *models.DataSource) (*sdkhttpclient.Optio } opts := &sdkhttpclient.Options{ Timeouts: timeouts, - Headers: s.getCustomHeaders(ds.JsonData, s.DecryptedValues(ds)), + Headers: GetCustomHeaders(ds.JsonData, s.DecryptedValues(ds), s.cfg), Labels: map[string]string{ "datasource_name": ds.Name, "datasource_uid": ds.Uid, @@ -438,16 +438,17 @@ func (s *Service) getTimeout(ds *models.DataSource) time.Duration { return time.Duration(timeout) * time.Second } -// getCustomHeaders returns a map with all the to be set headers +// GetCustomHeaders returns a map with all the to be set headers // The map key represents the HeaderName and the value represents this header's value -func (s *Service) getCustomHeaders(jsonData *simplejson.Json, decryptedValues map[string]string) map[string]string { +func GetCustomHeaders(jsonData *simplejson.Json, decryptedValues map[string]string, cfg *setting.Cfg) map[string]string { headers := make(map[string]string) if jsonData == nil { return headers } - index := 1 + index := 0 for { + index++ headerNameSuffix := fmt.Sprintf("httpHeaderName%d", index) headerValueSuffix := fmt.Sprintf("httpHeaderValue%d", index) @@ -457,10 +458,16 @@ func (s *Service) getCustomHeaders(jsonData *simplejson.Json, decryptedValues ma break } + // skip a header with name that corresponds to auth proxy header's name + // to make sure that data source proxy isn't used to circumvent auth proxy. + // For more context take a look at CVE-2022-35957 + if cfg.AuthProxyEnabled && http.CanonicalHeaderKey(key) == http.CanonicalHeaderKey(cfg.AuthProxyHeaderName) { + continue + } + if val, ok := decryptedValues[headerValueSuffix]; ok { headers[key] = val } - index++ } return headers diff --git a/pkg/services/datasources/service/datasource_service_test.go b/pkg/services/datasources/service/datasource_service_test.go index 2df5cb86f50..db16411dee4 100644 --- a/pkg/services/datasources/service/datasource_service_test.go +++ b/pkg/services/datasources/service/datasource_service_test.go @@ -445,7 +445,7 @@ func TestService_GetHttpTransport(t *testing.T) { SecureJsonData: map[string][]byte{"httpHeaderValue1": encryptedData}, } - headers := dsService.getCustomHeaders(json, map[string]string{"httpHeaderValue1": "Bearer xf5yhfkpsnmgo"}) + headers := GetCustomHeaders(json, map[string]string{"httpHeaderValue1": "Bearer xf5yhfkpsnmgo"}, dsService.cfg) require.Equal(t, "Bearer xf5yhfkpsnmgo", headers["Authorization"]) // 1. Start HTTP test server which checks the request headers @@ -537,6 +537,83 @@ func TestService_GetHttpTransport(t *testing.T) { require.NotNil(t, configuredOpts.SigV4) require.Equal(t, "es", configuredOpts.SigV4.Service) }) + + t.Run("Should not add a header with key that corresponds to auth proxies key name", func(t *testing.T) { + provider := httpclient.NewProvider() + + cfg.AuthProxyEnabled = true + cfg.AuthProxyHeaderName = "X-AUTH-PROXY-HEADER" + + allowedHeaderName := "X-ALLOWED-HEADER" + allowedHeaderValue := "X-ALLOWED-HEADER_value" + jsonData := simplejson.New() + jsonData.Set("httpHeaderName1", allowedHeaderName) + jsonData.Set("httpHeaderName2", cfg.AuthProxyHeaderName) + + secretsService := secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore()) + dsService := ProvideService(nil, secretsService, cfg, featuremgmt.WithFeatures(), acmock.New(), acmock.NewPermissionsServicesMock()) + + encryptedJsonData, err := secretsService.EncryptJsonData( + context.Background(), + map[string]string{ + "httpHeaderValue1": allowedHeaderValue, + "httpHeaderValue2": "admin", + }, secrets.WithoutScope()) + require.NoError(t, err) + + ds := models.DataSource{ + Id: 1, + Url: "http://k8s:8001", + Type: "Kubernetes", + JsonData: jsonData, + SecureJsonData: encryptedJsonData, + } + + headers := GetCustomHeaders(jsonData, map[string]string{"httpHeaderValue2": "admin", "httpHeaderValue1": "X-ALLOWED-HEADER_value"}, dsService.cfg) + require.Equal(t, "", headers[cfg.AuthProxyHeaderName], "header with auth proxy header name should have been stripped") + require.Equal(t, allowedHeaderValue, headers[allowedHeaderName], "other headers should stay intact") + + // 1. Start HTTP test server which checks the request headers + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get(cfg.AuthProxyHeaderName) != "" { + w.WriteHeader(http.StatusInternalServerError) + _, err := w.Write([]byte("Did not expect the header to be set")) + require.NoError(t, err) + return + } + + if r.Header.Get(allowedHeaderName) != allowedHeaderValue { + w.WriteHeader(http.StatusInternalServerError) + _, err := w.Write([]byte("Expected the allowed header to be set")) + require.NoError(t, err) + return + } + + w.WriteHeader(200) + _, err := w.Write([]byte("Ok")) + require.NoError(t, err) + })) + defer backend.Close() + + // 2. Get HTTP transport from datasource which uses the test server as backend + ds.Url = backend.URL + rt, err := dsService.GetHTTPTransport(&ds, provider) + require.NoError(t, err) + require.NotNil(t, rt) + + // 3. Send test request which should not have a header with name that matches auth proxy header + req := httptest.NewRequest("GET", backend.URL+"/test-headers", nil) + res, err := rt.RoundTrip(req) + require.NoError(t, err) + t.Cleanup(func() { + err := res.Body.Close() + require.NoError(t, err) + }) + body, err := ioutil.ReadAll(res.Body) + require.NoError(t, err) + bodyStr := string(body) + require.Equal(t, "Ok", bodyStr) + }) } func TestService_getTimeout(t *testing.T) { diff --git a/pkg/services/ngalert/eval/eval.go b/pkg/services/ngalert/eval/eval.go index 2667c661b0d..31f3edf1314 100644 --- a/pkg/services/ngalert/eval/eval.go +++ b/pkg/services/ngalert/eval/eval.go @@ -11,12 +11,12 @@ import ( "strings" "time" - "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/expr" "github.com/grafana/grafana/pkg/expr/classic" "github.com/grafana/grafana/pkg/infra/log" m "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/datasources" + "github.com/grafana/grafana/pkg/services/datasources/service" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/secrets" "github.com/grafana/grafana/pkg/setting" @@ -147,7 +147,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, secretsService secrets.Service) (*expr.Request, error) { +func GetExprRequest(ctx AlertExecCtx, data []models.AlertQuery, now time.Time, dsCacheService datasources.CacheService, secretsService secrets.Service, cfg *setting.Cfg) (*expr.Request, error) { req := &expr.Request{ OrgId: ctx.OrgID, Headers: map[string]string{ @@ -197,7 +197,7 @@ func GetExprRequest(ctx AlertExecCtx, data []models.AlertQuery, now time.Time, d if err != nil { return nil, err } - customHeaders := getCustomHeaders(ds.JsonData, decryptedData) + customHeaders := service.GetCustomHeaders(ds.JsonData, decryptedData, cfg) for k, v := range customHeaders { if _, ok := req.Headers[k]; !ok { req.Headers[k] = v @@ -220,40 +220,14 @@ func GetExprRequest(ctx AlertExecCtx, data []models.AlertQuery, now time.Time, d return req, nil } -func getCustomHeaders(jsonData *simplejson.Json, decryptedValues map[string]string) map[string]string { - headers := make(map[string]string) - if jsonData == nil { - return headers - } - - index := 1 - for { - headerNameSuffix := fmt.Sprintf("httpHeaderName%d", index) - headerValueSuffix := fmt.Sprintf("httpHeaderValue%d", index) - - key := jsonData.Get(headerNameSuffix).MustString() - if key == "" { - // No (more) header values are available - break - } - - if val, ok := decryptedValues[headerValueSuffix]; ok { - headers[key] = val - } - index++ - } - - return headers -} - type NumberValueCapture struct { Var string // RefID Labels data.Labels Value *float64 } -func executeCondition(ctx AlertExecCtx, c *models.Condition, now time.Time, exprService *expr.Service, dsCacheService datasources.CacheService, secretsService secrets.Service) ExecutionResults { - execResp, err := executeQueriesAndExpressions(ctx, c.Data, now, exprService, dsCacheService, secretsService) +func executeCondition(ctx AlertExecCtx, c *models.Condition, now time.Time, exprService *expr.Service, dsCacheService datasources.CacheService, secretsService secrets.Service, cfg *setting.Cfg) ExecutionResults { + execResp, err := executeQueriesAndExpressions(ctx, c.Data, now, exprService, dsCacheService, secretsService, cfg) if err != nil { return ExecutionResults{Error: err} } @@ -336,7 +310,7 @@ func executeCondition(ctx AlertExecCtx, c *models.Condition, now time.Time, expr return result } -func executeQueriesAndExpressions(ctx AlertExecCtx, data []models.AlertQuery, now time.Time, exprService *expr.Service, dsCacheService datasources.CacheService, secretsService secrets.Service) (resp *backend.QueryDataResponse, err error) { +func executeQueriesAndExpressions(ctx AlertExecCtx, data []models.AlertQuery, now time.Time, exprService *expr.Service, dsCacheService datasources.CacheService, secretsService secrets.Service, cfg *setting.Cfg) (resp *backend.QueryDataResponse, err error) { defer func() { if e := recover(); e != nil { ctx.Log.Error("alert rule panic", "error", e, "stack", string(debug.Stack())) @@ -349,7 +323,7 @@ func executeQueriesAndExpressions(ctx AlertExecCtx, data []models.AlertQuery, no } }() - queryDataReq, err := GetExprRequest(ctx, data, now, dsCacheService, secretsService) + queryDataReq, err := GetExprRequest(ctx, data, now, dsCacheService, secretsService, cfg) if err != nil { return nil, err } @@ -590,7 +564,7 @@ func (e *evaluatorImpl) ConditionEval(condition *models.Condition, now time.Time alertExecCtx := AlertExecCtx{OrgID: condition.OrgID, Ctx: alertCtx, ExpressionsEnabled: e.cfg.ExpressionsEnabled, Log: e.log} - execResult := executeCondition(alertExecCtx, condition, now, expressionService, e.dataSourceCache, e.secretsService) + execResult := executeCondition(alertExecCtx, condition, now, expressionService, e.dataSourceCache, e.secretsService, e.cfg) evalResults := evaluateExecutionResult(execResult, now) return evalResults, nil @@ -603,7 +577,7 @@ func (e *evaluatorImpl) QueriesAndExpressionsEval(orgID int64, data []models.Ale alertExecCtx := AlertExecCtx{OrgID: orgID, Ctx: alertCtx, ExpressionsEnabled: e.cfg.ExpressionsEnabled, Log: e.log} - execResult, err := executeQueriesAndExpressions(alertExecCtx, data, now, expressionService, e.dataSourceCache, e.secretsService) + execResult, err := executeQueriesAndExpressions(alertExecCtx, data, now, expressionService, e.dataSourceCache, e.secretsService, e.cfg) if err != nil { return nil, fmt.Errorf("failed to execute conditions: %w", err) } diff --git a/pkg/services/query/query.go b/pkg/services/query/query.go index 95d3e7437c3..ac32ca99c65 100644 --- a/pkg/services/query/query.go +++ b/pkg/services/query/query.go @@ -3,7 +3,6 @@ package query import ( "context" "fmt" - "strings" "time" "github.com/grafana/grafana/pkg/api/dtos" @@ -14,6 +13,7 @@ import ( "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/adapters" "github.com/grafana/grafana/pkg/services/datasources" + "github.com/grafana/grafana/pkg/services/datasources/service" "github.com/grafana/grafana/pkg/services/oauthtoken" "github.com/grafana/grafana/pkg/services/secrets" "github.com/grafana/grafana/pkg/setting" @@ -23,11 +23,6 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/backend" ) -const ( - headerName = "httpHeaderName" - headerValue = "httpHeaderValue" -) - func ProvideService( cfg *setting.Cfg, dataSourceCache datasources.CacheService, @@ -146,7 +141,7 @@ func (s *Service) handleQueryData(ctx context.Context, user *models.SignedInUser } } - for k, v := range customHeaders(ds.JsonData, instanceSettings.DecryptedSecureJSONData) { + for k, v := range service.GetCustomHeaders(ds.JsonData, instanceSettings.DecryptedSecureJSONData, s.cfg) { req.Headers[k] = v } @@ -167,26 +162,6 @@ type parsedRequest struct { parsedQueries []parsedQuery } -func customHeaders(jsonData *simplejson.Json, decryptedJsonData map[string]string) map[string]string { - if jsonData == nil { - return nil - } - - data := jsonData.MustMap() - - headers := map[string]string{} - for k := range data { - if strings.HasPrefix(k, headerName) { - if header, ok := data[k].(string); ok { - valueKey := strings.ReplaceAll(k, headerName, headerValue) - headers[header] = decryptedJsonData[valueKey] - } - } - } - - return headers -} - func (s *Service) parseMetricRequest(ctx context.Context, user *models.SignedInUser, skipCache bool, reqDTO dtos.MetricRequest) (*parsedRequest, error) { if len(reqDTO.Queries) == 0 { return nil, NewErrBadQuery("no queries found") diff --git a/pkg/services/query/query_test.go b/pkg/services/query/query_test.go index 3b0193302f6..480e0337ea6 100644 --- a/pkg/services/query/query_test.go +++ b/pkg/services/query/query_test.go @@ -14,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/query" "github.com/grafana/grafana/pkg/services/secrets" + "github.com/grafana/grafana/pkg/setting" "github.com/stretchr/testify/require" ) @@ -64,7 +65,7 @@ func setup() *testContext { dataSourceCache: dc, oauthTokenService: tc, pluginRequestValidator: rv, - queryService: query.ProvideService(nil, dc, nil, rv, sc, pc, tc), + queryService: query.ProvideService(setting.NewCfg(), dc, nil, rv, sc, pc, tc), } } diff --git a/pkg/services/sqlstore/migrations/accesscontrol/admin_only.go b/pkg/services/sqlstore/migrations/accesscontrol/admin_only.go new file mode 100644 index 00000000000..41a30a529f3 --- /dev/null +++ b/pkg/services/sqlstore/migrations/accesscontrol/admin_only.go @@ -0,0 +1,100 @@ +package accesscontrol + +import ( + "strings" + + "xorm.io/xorm" + + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/sqlstore/migrator" +) + +func AddAdminOnlyMigration(mg *migrator.Migrator) { + mg.AddMigration("admin only folder/dashboard permission", &adminOnlyMigrator{}) +} + +type adminOnlyMigrator struct { + migrator.MigrationBase +} + +func (m *adminOnlyMigrator) SQL(dialect migrator.Dialect) string { + return CodeMigrationSQL +} + +func (m *adminOnlyMigrator) Exec(sess *xorm.Session, mg *migrator.Migrator) error { + logger := log.New("admin-permissions-only-migrator") + type model struct { + UID string `xorm:"uid"` + OrgID int64 `xorm:"org_id"` + IsFolder bool `xorm:"is_folder"` + } + var models []model + + // Find all dashboards and folders that should have only admin permission in acl + // When a dashboard or folder only has admin permission the acl table should be empty and the has_acl set to true + sql := ` + SELECT res.uid, res.is_folder, res.org_id + FROM (SELECT dashboard.id, dashboard.uid, dashboard.is_folder, dashboard.org_id, count(dashboard_acl.id) as count + FROM dashboard + LEFT JOIN dashboard_acl ON dashboard.id = dashboard_acl.dashboard_id + WHERE dashboard.has_acl IS TRUE + GROUP BY dashboard.id) as res + WHERE res.count = 0 + ` + + if err := sess.SQL(sql).Find(&models); err != nil { + return err + } + + for _, model := range models { + var scope string + + // set scope based on type + if model.IsFolder { + scope = "folders:uid:" + model.UID + } else { + scope = "dashboards:uid:" + model.UID + } + + // Find all managed editor and viewer permissions with scopes to folder or dashboard + sql = ` + SELECT r.id + FROM role r + LEFT JOIN permission p on r.id = p.role_id + WHERE p.scope = ? + AND r.org_id = ? + AND r.name IN ('managed:builtins:editor:permissions', 'managed:builtins:viewer:permissions') + GROUP BY r.id + ` + + var roleIDS []int64 + if err := sess.SQL(sql, scope, model.OrgID).Find(&roleIDS); err != nil { + return err + } + + if len(roleIDS) == 0 { + continue + } + + msg := "removing viewer and editor permissions on " + if model.IsFolder { + msg += "folder" + } else { + msg += "dashboard" + } + + logger.Info(msg, "uid", model.UID) + + // Remove managed permission for editors and viewers if there was any + removeSQL := `DELETE FROM permission WHERE scope = ? AND role_id IN(?` + strings.Repeat(", ?", len(roleIDS)-1) + `) ` + params := []interface{}{removeSQL, scope} + for _, id := range roleIDS { + params = append(params, id) + } + if _, err := sess.Exec(params...); err != nil { + return err + } + } + + return nil +} diff --git a/pkg/services/sqlstore/migrations/accesscontrol/dashboard_permissions.go b/pkg/services/sqlstore/migrations/accesscontrol/dashboard_permissions.go index 7ff19d4e614..d42897302a1 100644 --- a/pkg/services/sqlstore/migrations/accesscontrol/dashboard_permissions.go +++ b/pkg/services/sqlstore/migrations/accesscontrol/dashboard_permissions.go @@ -69,6 +69,7 @@ type dashboard struct { FolderID int64 `xorm:"folder_id"` OrgID int64 `xorm:"org_id"` IsFolder bool + HasAcl bool `xorm:"has_acl"` } func (m dashboardPermissionsMigrator) Exec(sess *xorm.Session, migrator *migrator.Migrator) error { @@ -76,8 +77,8 @@ func (m dashboardPermissionsMigrator) Exec(sess *xorm.Session, migrator *migrato m.dialect = migrator.Dialect var dashboards []dashboard - if err := m.sess.SQL("SELECT id, is_folder, folder_id, org_id FROM dashboard").Find(&dashboards); err != nil { - return err + if err := m.sess.SQL("SELECT id, is_folder, folder_id, org_id, has_acl FROM dashboard").Find(&dashboards); err != nil { + return fmt.Errorf("failed to list dashboards: %w", err) } var acl []models.DashboardAcl @@ -108,7 +109,7 @@ func (m dashboardPermissionsMigrator) migratePermissions(dashboards []dashboard, permissionMap[d.OrgID] = map[string][]*ac.Permission{} } - if (d.IsFolder || d.FolderID == 0) && len(acls) == 0 { + if (d.IsFolder || d.FolderID == 0) && len(acls) == 0 && !d.HasAcl { permissionMap[d.OrgID]["managed:builtins:editor:permissions"] = append( permissionMap[d.OrgID]["managed:builtins:editor:permissions"], m.mapPermission(d.ID, models.PERMISSION_EDIT, d.IsFolder)..., diff --git a/pkg/services/sqlstore/migrations/migrations.go b/pkg/services/sqlstore/migrations/migrations.go index 0c43889d878..0ad34293525 100644 --- a/pkg/services/sqlstore/migrations/migrations.go +++ b/pkg/services/sqlstore/migrations/migrations.go @@ -91,7 +91,13 @@ func (*OSSMigrations) AddMigration(mg *Migrator) { addCommentMigrations(mg) } } - accesscontrol.AddManagedFolderAlertActionsMigration(mg) + + if mg.Cfg != nil && mg.Cfg.IsFeatureToggleEnabled != nil { + if mg.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { + accesscontrol.AddManagedFolderAlertActionsMigration(mg) + accesscontrol.AddAdminOnlyMigration(mg) + } + } } func addMigrationLogMigrations(mg *Migrator) {