public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Chore: clean up access control for data sources (#73010)

Browse Source 
* move DS guardian interfaces to OSS, move allow guardian to OSS

* update codeowner file
This commit is contained in:
Ieva
2023-08-21 14:26:49 +01:00
committed by GitHub
parent f7355668e7
commit ec9c35fae5
16 changed files with 109 additions and 180 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/alerting/extractor.go
+13 -20
View File
@@ -10,7 +10,7 @@ import (
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/alerting/models"
"github.com/grafana/grafana/pkg/services/datasources"
"github.com/grafana/grafana/pkg/services/datasources/permissions"
"github.com/grafana/grafana/pkg/services/datasources/guardian"
)
type DashAlertExtractor interface {
@@ -20,18 +20,18 @@ type DashAlertExtractor interface {
// DashAlertExtractorService extracts alerts from the dashboard json.
type DashAlertExtractorService struct {
datasourcePermissionsService permissions.DatasourcePermissionsService
datasourceService datasources.DataSourceService
alertStore AlertStore
log log.Logger
dsGuardian guardian.DatasourceGuardianProvider
datasourceService datasources.DataSourceService
alertStore AlertStore
log log.Logger
}
func ProvideDashAlertExtractorService(datasourcePermissionsService permissions.DatasourcePermissionsService, datasourceService datasources.DataSourceService, store AlertStore) *DashAlertExtractorService {
func ProvideDashAlertExtractorService(dsGuardian guardian.DatasourceGuardianProvider, datasourceService datasources.DataSourceService, store AlertStore) *DashAlertExtractorService {
return &DashAlertExtractorService{
datasourcePermissionsService: datasourcePermissionsService,
datasourceService: datasourceService,
alertStore: store,
log: log.New("alerting.extractor"),
dsGuardian: dsGuardian,
datasourceService: datasourceService,
alertStore: store,
log: log.New("alerting.extractor"),
}
}
@@ -210,17 +210,10 @@ func (e *DashAlertExtractorService) getAlertFromPanels(ctx context.Context, json
return nil, err
}
dsFilterQuery := datasources.DatasourcesPermissionFilterQuery{
User: dashAlertInfo.User,
Datasources: []*datasources.DataSource{datasource},
}
dataSources, err := e.datasourcePermissionsService.FilterDatasourcesBasedOnQueryPermissions(ctx, &dsFilterQuery)
canQuery, err := e.dsGuardian.New(dashAlertInfo.OrgID, dashAlertInfo.User, *datasource).CanQuery(datasource.ID)
if err != nil {
if !errors.Is(err, permissions.ErrNotImplemented) {
return nil, err
}
} else if len(dataSources) == 0 {
return nil, err
} else if !canQuery {
return nil, datasources.ErrDataSourceAccessDenied
}
pkg/services/alerting/extractor_test.go
+3 -73
View File
@@ -2,12 +2,10 @@ package alerting
import (
"context"
"errors"
"os"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/components/simplejson"
@@ -16,7 +14,7 @@ import (
"github.com/grafana/grafana/pkg/services/alerting/models"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/datasources"
"github.com/grafana/grafana/pkg/services/datasources/permissions"
"github.com/grafana/grafana/pkg/services/datasources/guardian"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/setting"
@@ -34,18 +32,13 @@ func TestAlertRuleExtraction(t *testing.T) {
json, err := os.ReadFile("./testdata/graphite-alert.json")
require.Nil(t, err)
dsPermissions := permissions.NewMockDatasourcePermissionService()
dsPermissions.DsResult = []*datasources.DataSource{
{
ID: 1,
},
}
dsGuardian := guardian.ProvideGuardian()
dsService := &fakeDatasourceService{ExpectedDatasource: defaultDs}
db := dbtest.NewFakeDB()
cfg := &setting.Cfg{}
store := ProvideAlertStore(db, localcache.ProvideService(), cfg, nil, featuremgmt.WithFeatures())
extractor := ProvideDashAlertExtractorService(dsPermissions, dsService, store)
extractor := ProvideDashAlertExtractorService(dsGuardian, dsService, store)
t.Run("Parsing alert rules from dashboard json", func(t *testing.T) {
dashJSON, err := simplejson.NewJson(json)
@@ -306,69 +299,6 @@ func TestAlertRuleExtraction(t *testing.T) {
})
}
func TestFilterPermissionsErrors(t *testing.T) {
RegisterCondition("query", func(model *simplejson.Json, index int) (Condition, error) {
return &FakeCondition{}, nil
})
// mock data
defaultDs := &datasources.DataSource{ID: 12, OrgID: 1, Name: "I am default", IsDefault: true, UID: "def-uid"}
json, err := os.ReadFile("./testdata/graphite-alert.json")
require.Nil(t, err)
dashJSON, err := simplejson.NewJson(json)
require.Nil(t, err)
dsPermissions := permissions.NewMockDatasourcePermissionService()
dsService := &fakeDatasourceService{ExpectedDatasource: defaultDs}
extractor := ProvideDashAlertExtractorService(dsPermissions, dsService, nil)
tc := []struct {
name string
result []*datasources.DataSource
err error
expectedErr error
}{
{
"Data sources are filtered and return results don't return an error",
[]*datasources.DataSource{defaultDs},
nil,
nil,
},
{
"Data sources are filtered but return empty results should return error",
nil,
nil,
datasources.ErrDataSourceAccessDenied,
},
{
"Using default OSS implementation doesn't return an error",
nil,
permissions.ErrNotImplemented,
nil,
},
{
"Returning an error different from ErrNotImplemented should fails",
nil,
errors.New("random error"),
errors.New("random error"),
},
}
for _, test := range tc {
t.Run(test.name, func(t *testing.T) {
dsPermissions.DsResult = test.result
dsPermissions.ErrResult = test.err
_, err = extractor.GetAlerts(WithUAEnabled(context.Background(), true), DashAlertInfo{
User: nil,
Dash: dashboards.NewDashboardFromJson(dashJSON),
OrgID: 1,
})
assert.Equal(t, err, test.expectedErr)
})
}
}
type fakeDatasourceService struct {
ExpectedDatasource *datasources.DataSource
datasources.DataSourceService
pkg/services/datasources/guardian/allow_guardian.go
+19
View File
@@ -0,0 +1,19 @@
package guardian
import (
"github.com/grafana/grafana/pkg/services/datasources"
)
var _ DatasourceGuardian = new(AllowGuardian)
// AllowGuardian is used whenever an enterprise build is running without a license.
// It allows every one to Query all data sources and will not filter out any of them
type AllowGuardian struct{}
func (n AllowGuardian) CanQuery(datasourceID int64) (bool, error) {
return true, nil
}
func (n AllowGuardian) FilterDatasourcesByQueryPermissions(ds []*datasources.DataSource) ([]*datasources.DataSource, error) {
return ds, nil
}
pkg/services/datasources/guardian/provider.go
+25
View File
@@ -0,0 +1,25 @@
package guardian
import (
"github.com/grafana/grafana/pkg/services/datasources"
"github.com/grafana/grafana/pkg/services/user"
)
type DatasourceGuardianProvider interface {
New(orgID int64, user *user.SignedInUser, dataSources ...datasources.DataSource) DatasourceGuardian
}
type DatasourceGuardian interface {
CanQuery(datasourceID int64) (bool, error)
FilterDatasourcesByQueryPermissions([]*datasources.DataSource) ([]*datasources.DataSource, error)
}
func ProvideGuardian() *OSSProvider {
return &OSSProvider{}
}
type OSSProvider struct{}
func (p *OSSProvider) New(orgID int64, user *user.SignedInUser, dataSources ...datasources.DataSource) DatasourceGuardian {
return &AllowGuardian{}
}
pkg/services/datasources/permissions/datasource_permissions.go
-25
View File
@@ -1,25 +0,0 @@
package permissions
import (
"context"
"errors"
"github.com/grafana/grafana/pkg/services/datasources"
)
var ErrNotImplemented = errors.New("not implemented")
type DatasourcePermissionsService interface {
FilterDatasourcesBasedOnQueryPermissions(ctx context.Context, cmd *datasources.DatasourcesPermissionFilterQuery) ([]*datasources.DataSource, error)
}
// dummy method
func (hs *OSSDatasourcePermissionsService) FilterDatasourcesBasedOnQueryPermissions(ctx context.Context, cmd *datasources.DatasourcesPermissionFilterQuery) ([]*datasources.DataSource, error) {
return nil, ErrNotImplemented
}
type OSSDatasourcePermissionsService struct{}
func ProvideDatasourcePermissionsService() *OSSDatasourcePermissionsService {
return &OSSDatasourcePermissionsService{}
}
pkg/services/datasources/permissions/datasource_permissions_mocks.go
-21
View File
@@ -1,21 +0,0 @@
package permissions
import (
"context"
"github.com/grafana/grafana/pkg/services/datasources"
)
type mockDatasourcePermissionService struct {
DsResult []*datasources.DataSource
DsUidResult []string
ErrResult error
}
func (m *mockDatasourcePermissionService) FilterDatasourcesBasedOnQueryPermissions(ctx context.Context, cmd *datasources.DatasourcesPermissionFilterQuery) ([]*datasources.DataSource, error) {
return m.DsResult, m.ErrResult
}
func NewMockDatasourcePermissionService() *mockDatasourcePermissionService {
return &mockDatasourcePermissionService{}
}
pkg/services/datasources/service/cache.go
+31 -1
View File
@@ -9,6 +9,7 @@ import (
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/datasources"
"github.com/grafana/grafana/pkg/services/datasources/guardian"
"github.com/grafana/grafana/pkg/services/user"
)
@@ -16,12 +17,13 @@ const (
DefaultCacheTTL = 5 * time.Second
)
func ProvideCacheService(cacheService *localcache.CacheService, sqlStore db.DB) *CacheServiceImpl {
func ProvideCacheService(cacheService *localcache.CacheService, sqlStore db.DB, dsGuardian guardian.DatasourceGuardianProvider) *CacheServiceImpl {
return &CacheServiceImpl{
logger: log.New("datasources"),
cacheTTL: DefaultCacheTTL,
CacheService: cacheService,
SQLStore: sqlStore,
dsGuardian: dsGuardian,
}
}
@@ -30,6 +32,7 @@ type CacheServiceImpl struct {
cacheTTL time.Duration
CacheService *localcache.CacheService
SQLStore db.DB
dsGuardian guardian.DatasourceGuardianProvider
}
func (dc *CacheServiceImpl) GetDatasource(
@@ -44,6 +47,9 @@ func (dc *CacheServiceImpl) GetDatasource(
if cached, found := dc.CacheService.Get(cacheKey); found {
ds := cached.(*datasources.DataSource)
if ds.OrgID == user.OrgID {
if err := dc.canQuery(user, ds); err != nil {
return nil, err
}
return ds, nil
}
}
@@ -62,6 +68,11 @@ func (dc *CacheServiceImpl) GetDatasource(
dc.CacheService.Set(uidKey(ds.OrgID, ds.UID), ds, time.Second*5)
}
dc.CacheService.Set(cacheKey, ds, dc.cacheTTL)
if err = dc.canQuery(user, ds); err != nil {
return nil, err
}
return ds, nil
}
@@ -83,6 +94,9 @@ func (dc *CacheServiceImpl) GetDatasourceByUID(
if cached, found := dc.CacheService.Get(uidCacheKey); found {
ds := cached.(*datasources.DataSource)
if ds.OrgID == user.OrgID {
if err := dc.canQuery(user, ds); err != nil {
return nil, err
}
return ds, nil
}
}
@@ -98,6 +112,11 @@ func (dc *CacheServiceImpl) GetDatasourceByUID(
dc.CacheService.Set(uidCacheKey, ds, dc.cacheTTL)
dc.CacheService.Set(idKey(ds.ID), ds, dc.cacheTTL)
if err = dc.canQuery(user, ds); err != nil {
return nil, err
}
return ds, nil
}
@@ -108,3 +127,14 @@ func idKey(id int64) string {
func uidKey(orgID int64, uid string) string {
return fmt.Sprintf("ds-orgid-uid-%d-%s", orgID, uid)
}
func (dc *CacheServiceImpl) canQuery(user *user.SignedInUser, ds *datasources.DataSource) error {
guardian := dc.dsGuardian.New(user.OrgID, user, *ds)
if canQuery, err := guardian.CanQuery(ds.ID); err != nil || !canQuery {
if err != nil {
return err
}
return datasources.ErrDataSourceAccessDenied
}
return nil
}
pkg/services/publicdashboards/api/common_test.go
+2 -1
View File
@@ -22,6 +22,7 @@ import (
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/datasources"
fakeDatasources "github.com/grafana/grafana/pkg/services/datasources/fakes"
"github.com/grafana/grafana/pkg/services/datasources/guardian"
datasourceService "github.com/grafana/grafana/pkg/services/datasources/service"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/pluginsintegration/plugincontext"
@@ -100,7 +101,7 @@ func buildQueryDataService(t *testing.T, cs datasources.CacheService, fpc *fakeP
// default cache service
if cs == nil {
cs = datasourceService.ProvideCacheService(localcache.ProvideService(), store)
cs = datasourceService.ProvideCacheService(localcache.ProvideService(), store, guardian.ProvideGuardian())
}
// default fakePluginClient
pkg/services/publicdashboards/api/query_test.go
+2 -1
View File
@@ -22,6 +22,7 @@ import (
"github.com/grafana/grafana/pkg/services/dashboards"
dashboardStore "github.com/grafana/grafana/pkg/services/dashboards/database"
"github.com/grafana/grafana/pkg/services/datasources"
"github.com/grafana/grafana/pkg/services/datasources/guardian"
datasourcesService "github.com/grafana/grafana/pkg/services/datasources/service"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/publicdashboards"
@@ -263,7 +264,7 @@ func TestIntegrationUnauthenticatedUserCanGetPubdashPanelQueryData(t *testing.T)
}
db := db.InitTestDB(t)
cacheService := datasourcesService.ProvideCacheService(localcache.ProvideService(), db)
cacheService := datasourcesService.ProvideCacheService(localcache.ProvideService(), db, guardian.ProvideGuardian())
qds := buildQueryDataService(t, cacheService, nil, db)
dsStore := datasourcesService.CreateStore(db, log.New("publicdashboards.test"))
_, _ = dsStore.AddDataSource(context.Background(), &datasources.AddDataSourceCommand{