Storage: use static access rules (#52334)

* Storage: use static access rules

* Storage: use static access rules

* Storage: add tests

pkg/services/store/file_guardian.go

@@ -0,0 +1,121 @@
+package store
+
+import (
+	"context"
+	"strings"
+
+	"github.com/grafana/grafana/pkg/infra/filestorage"
+	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/models"
+)
+
+const (
+	ActionFilesRead   = "files:read"
+	ActionFilesWrite  = "files:write"
+	ActionFilesDelete = "files:delete"
+)
+
+var (
+	denyAllPathFilter  = filestorage.NewDenyAllPathFilter()
+	allowAllPathFilter = filestorage.NewAllowAllPathFilter()
+)
+
+func isValidAction(action string) bool {
+	return action == ActionFilesRead || action == ActionFilesWrite || action == ActionFilesDelete
+}
+
+type storageAuthService interface {
+	newGuardian(ctx context.Context, user *models.SignedInUser, prefix string) fileGuardian
+}
+
+type fileGuardian interface {
+	canView(path string) bool
+	canWrite(path string) bool
+	canDelete(path string) bool
+	can(action string, path string) bool
+
+	getPathFilter(action string) filestorage.PathFilter
+}
+
+type pathFilterFileGuardian struct {
+	ctx                context.Context
+	user               *models.SignedInUser
+	prefix             string
+	pathFilterByAction map[string]filestorage.PathFilter
+	log                log.Logger
+}
+
+func (a *pathFilterFileGuardian) getPathFilter(action string) filestorage.PathFilter {
+	if !isValidAction(action) {
+		a.log.Warn("Unsupported action", "action", action)
+		return denyAllPathFilter
+	}
+
+	if filter, ok := a.pathFilterByAction[action]; ok {
+		return filter
+	}
+
+	return denyAllPathFilter
+}
+
+func (a *pathFilterFileGuardian) canWrite(path string) bool {
+	return a.can(ActionFilesWrite, path)
+}
+
+func (a *pathFilterFileGuardian) canView(path string) bool {
+	return a.can(ActionFilesRead, path)
+}
+
+func (a *pathFilterFileGuardian) canDelete(path string) bool {
+	return a.can(ActionFilesDelete, path)
+}
+
+func (a *pathFilterFileGuardian) can(action string, path string) bool {
+	if path == a.prefix {
+		path = filestorage.Delimiter
+	} else {
+		path = strings.TrimPrefix(path, a.prefix)
+	}
+	allow := false
+
+	if !isValidAction(action) {
+		a.log.Warn("Unsupported action", "action", action, "path", path)
+		return false
+	}
+
+	pathFilter, ok := a.pathFilterByAction[action]
+
+	if !ok {
+		a.log.Warn("Missing path filter", "action", action, "path", path)
+		return false
+	}
+
+	allow = pathFilter.IsAllowed(path)
+	if !allow {
+		a.log.Warn("denying", "action", action, "path", path)
+	}
+	return allow
+}
+
+type denyAllFileGuardian struct {
+}
+
+func (d denyAllFileGuardian) canView(path string) bool {
+	return d.can(ActionFilesRead, path)
+}
+
+func (d denyAllFileGuardian) canWrite(path string) bool {
+	return d.can(ActionFilesWrite, path)
+}
+
+func (d denyAllFileGuardian) canDelete(path string) bool {
+	return d.can(ActionFilesDelete, path)
+}
+
+func (d denyAllFileGuardian) can(action string, path string) bool {
+	return false
+}
+
+func (d denyAllFileGuardian) getPathFilter(action string) filestorage.PathFilter {
+	return denyAllPathFilter
+}

pkg/services/store/http.go

@@ -37,7 +37,7 @@ func ProvideHTTPService(store StorageService) HTTPStorageService {
 
 func UploadErrorToStatusCode(err error) int {
 	switch {
-	case errors.Is(err, ErrUploadFeatureDisabled):
+	case errors.Is(err, ErrStorageNotFound):
 		return 404
 
 	case errors.Is(err, ErrUnsupportedStorage):
@@ -49,6 +49,9 @@ func UploadErrorToStatusCode(err error) int {
 	case errors.Is(err, ErrFileAlreadyExists):
 		return 400
 
+	case errors.Is(err, ErrAccessDenied):
+		return 403
+
 	default:
 		return 500
 	}

pkg/services/store/service.go

@@ -3,7 +3,6 @@ package store
 import (
 	"context"
 	"errors"
-	"fmt"
 	"os"
 	"path/filepath"
 
@@ -19,13 +18,16 @@ import (
 
 var grafanaStorageLogger = log.New("grafanaStorageLogger")
 
-var ErrUploadFeatureDisabled = errors.New("upload feature is disabled")
 var ErrUnsupportedStorage = errors.New("storage does not support this operation")
 var ErrUploadInternalError = errors.New("upload internal error")
 var ErrValidationFailed = errors.New("request validation failed")
 var ErrFileAlreadyExists = errors.New("file exists")
+var ErrStorageNotFound = errors.New("storage not found")
+var ErrAccessDenied = errors.New("access denied")
 
 const RootPublicStatic = "public-static"
+const RootResources = "resources"
+const RootDevenv = "devenv"
 
 const MAX_UPLOAD_SIZE = 1 * 1024 * 1024 // 3MB
 
@@ -69,6 +71,7 @@ type standardStorageService struct {
 	sql         *sqlstore.SQLStore
 	tree        *nestedTree
 	cfg         storageServiceConfig
+	authService storageAuthService
 }
 
 func ProvideService(sql *sqlstore.SQLStore, features featuremgmt.FeatureToggles, cfg *setting.Cfg) StorageService {
@@ -90,7 +93,7 @@ func ProvideService(sql *sqlstore.SQLStore, features featuremgmt.FeatureToggles,
 		devenv := filepath.Join(cfg.StaticRootPath, "..", "devenv")
 		if _, err := os.Stat(devenv); !os.IsNotExist(err) {
 			// path/to/whatever exists
-			s := newDiskStorage("devenv", "Development Environment", &StorageLocalDiskConfig{
+			s := newDiskStorage(RootDevenv, "Development Environment", &StorageLocalDiskConfig{
 				Path: devenv,
 				Roots: []string{
 					"/dev-dashboards/",
@@ -104,7 +107,7 @@ func ProvideService(sql *sqlstore.SQLStore, features featuremgmt.FeatureToggles,
 		storages := make([]storageRuntime, 0)
 		if features.IsEnabled(featuremgmt.FlagStorageLocalUpload) {
 			storages = append(storages,
-				newSQLStorage("resources",
+				newSQLStorage(RootResources,
 					"Resources",
 					&StorageSQLConfig{orgId: orgId}, sql).
 					setBuiltin(true).
@@ -114,10 +117,39 @@ func ProvideService(sql *sqlstore.SQLStore, features featuremgmt.FeatureToggles,
 		return storages
 	}
 
-	return newStandardStorageService(sql, globalRoots, initializeOrgStorages)
+	authService := newStaticStorageAuthService(func(ctx context.Context, user *models.SignedInUser, storageName string) map[string]filestorage.PathFilter {
+		if user == nil || !user.IsGrafanaAdmin {
+			return nil
 		}
 
-func newStandardStorageService(sql *sqlstore.SQLStore, globalRoots []storageRuntime, initializeOrgStorages func(orgId int64) []storageRuntime) *standardStorageService {
+		switch storageName {
+		case RootPublicStatic:
+			return map[string]filestorage.PathFilter{
+				ActionFilesRead:   allowAllPathFilter,
+				ActionFilesWrite:  denyAllPathFilter,
+				ActionFilesDelete: denyAllPathFilter,
+			}
+		case RootDevenv:
+			return map[string]filestorage.PathFilter{
+				ActionFilesRead:   allowAllPathFilter,
+				ActionFilesWrite:  denyAllPathFilter,
+				ActionFilesDelete: denyAllPathFilter,
+			}
+		case RootResources:
+			return map[string]filestorage.PathFilter{
+				ActionFilesRead:   allowAllPathFilter,
+				ActionFilesWrite:  allowAllPathFilter,
+				ActionFilesDelete: allowAllPathFilter,
+			}
+		default:
+			return nil
+		}
+	})
+
+	return newStandardStorageService(sql, globalRoots, initializeOrgStorages, authService)
+}
+
+func newStandardStorageService(sql *sqlstore.SQLStore, globalRoots []storageRuntime, initializeOrgStorages func(orgId int64) []storageRuntime, authService storageAuthService) *standardStorageService {
 	rootsByOrgId := make(map[int64][]storageRuntime)
 	rootsByOrgId[ac.GlobalOrgID] = globalRoots
 
@@ -129,6 +161,7 @@ func newStandardStorageService(sql *sqlstore.SQLStore, globalRoots []storageRunt
 	return &standardStorageService{
 		sql:         sql,
 		tree:        res,
+		authService: authService,
 		cfg: storageServiceConfig{
 			allowUnsanitizedSvgUpload: false,
 		},
@@ -149,12 +182,15 @@ func getOrgId(user *models.SignedInUser) int64 {
 }
 
 func (s *standardStorageService) List(ctx context.Context, user *models.SignedInUser, path string) (*StorageListFrame, error) {
-	// apply access control here
+	guardian := s.authService.newGuardian(ctx, user, getFirstSegment(path))
-	return s.tree.ListFolder(ctx, getOrgId(user), path)
+	return s.tree.ListFolder(ctx, getOrgId(user), path, guardian.getPathFilter(ActionFilesRead))
 }
 
 func (s *standardStorageService) Read(ctx context.Context, user *models.SignedInUser, path string) (*filestorage.File, error) {
-	// TODO: permission check!
+	guardian := s.authService.newGuardian(ctx, user, getFirstSegment(path))
+	if !guardian.canView(path) {
+		return nil, ErrAccessDenied
+	}
 	return s.tree.GetFile(ctx, getOrgId(user), path)
 }
 
@@ -171,12 +207,17 @@ type UploadRequest struct {
 }
 
 func (s *standardStorageService) Upload(ctx context.Context, user *models.SignedInUser, req *UploadRequest) error {
-	upload, storagePath := s.tree.getRoot(getOrgId(user), req.Path)
+	guardian := s.authService.newGuardian(ctx, user, getFirstSegment(req.Path))
-	if upload == nil {
+	if !guardian.canWrite(req.Path) {
-		return ErrUploadFeatureDisabled
+		return ErrAccessDenied
 	}
 
-	if upload.Meta().ReadOnly {
+	root, storagePath := s.tree.getRoot(getOrgId(user), req.Path)
+	if root == nil {
+		return ErrStorageNotFound
+	}
+
+	if root.Meta().ReadOnly {
 		return ErrUnsupportedStorage
 	}
 
@@ -195,7 +236,7 @@ func (s *standardStorageService) Upload(ctx context.Context, user *models.Signed
 	grafanaStorageLogger.Info("uploading a file", "filetype", req.MimeType, "path", req.Path)
 
 	if !req.OverwriteExistingFile {
-		file, err := upload.Store().Get(ctx, storagePath)
+		file, err := root.Store().Get(ctx, storagePath)
 		if err != nil {
 			grafanaStorageLogger.Error("failed while checking file existence", "err", err, "path", req.Path)
 			return ErrUploadInternalError
@@ -206,7 +247,7 @@ func (s *standardStorageService) Upload(ctx context.Context, user *models.Signed
 		}
 	}
 
-	if err := upload.Store().Upsert(ctx, upsertCommand); err != nil {
+	if err := root.Store().Upsert(ctx, upsertCommand); err != nil {
 		grafanaStorageLogger.Error("failed while uploading the file", "err", err, "path", req.Path)
 		return ErrUploadInternalError
 	}
@@ -215,9 +256,14 @@ func (s *standardStorageService) Upload(ctx context.Context, user *models.Signed
 }
 
 func (s *standardStorageService) DeleteFolder(ctx context.Context, user *models.SignedInUser, cmd *DeleteFolderCmd) error {
+	guardian := s.authService.newGuardian(ctx, user, getFirstSegment(cmd.Path))
+	if !guardian.canDelete(cmd.Path) {
+		return ErrAccessDenied
+	}
+
 	root, storagePath := s.tree.getRoot(getOrgId(user), cmd.Path)
 	if root == nil {
-		return fmt.Errorf("resources storage is not enabled")
+		return ErrStorageNotFound
 	}
 
 	if root.Meta().ReadOnly {
@@ -227,13 +273,18 @@ func (s *standardStorageService) DeleteFolder(ctx context.Context, user *models.
 	if storagePath == "" {
 		storagePath = filestorage.Delimiter
 	}
-	return root.Store().DeleteFolder(ctx, storagePath, &filestorage.DeleteFolderOptions{Force: true})
+	return root.Store().DeleteFolder(ctx, storagePath, &filestorage.DeleteFolderOptions{Force: true, AccessFilter: guardian.getPathFilter(ActionFilesDelete)})
 }
 
 func (s *standardStorageService) CreateFolder(ctx context.Context, user *models.SignedInUser, cmd *CreateFolderCmd) error {
+	guardian := s.authService.newGuardian(ctx, user, getFirstSegment(cmd.Path))
+	if !guardian.canWrite(cmd.Path) {
+		return ErrAccessDenied
+	}
+
 	root, storagePath := s.tree.getRoot(getOrgId(user), cmd.Path)
 	if root == nil {
-		return fmt.Errorf("resources storage is not enabled")
+		return ErrStorageNotFound
 	}
 
 	if root.Meta().ReadOnly {
@@ -248,9 +299,14 @@ func (s *standardStorageService) CreateFolder(ctx context.Context, user *models.
 }
 
 func (s *standardStorageService) Delete(ctx context.Context, user *models.SignedInUser, path string) error {
+	guardian := s.authService.newGuardian(ctx, user, getFirstSegment(path))
+	if !guardian.canDelete(path) {
+		return ErrAccessDenied
+	}
+
 	root, storagePath := s.tree.getRoot(getOrgId(user), path)
 	if root == nil {
-		return fmt.Errorf("resources storage is not enabled")
+		return ErrStorageNotFound
 	}
 
 	if root.Meta().ReadOnly {

pkg/services/store/service_test.go

@@ -17,13 +17,22 @@ import (
 
 var (
 	dummyUser           = &models.SignedInUser{OrgId: 1}
-)
+	allowAllAuthService = newStaticStorageAuthService(func(ctx context.Context, user *models.SignedInUser, storageName string) map[string]filestorage.PathFilter {
-
+		return map[string]filestorage.PathFilter{
-func TestListFiles(t *testing.T) {
+			ActionFilesDelete: allowAllPathFilter,
-	publicRoot, err := filepath.Abs("../../../public")
+			ActionFilesWrite:  allowAllPathFilter,
-	require.NoError(t, err)
+			ActionFilesRead:   allowAllPathFilter,
-	roots := []storageRuntime{
+		}
-		newDiskStorage("public", "Public static files", &StorageLocalDiskConfig{
+	})
+	denyAllAuthService = newStaticStorageAuthService(func(ctx context.Context, user *models.SignedInUser, storageName string) map[string]filestorage.PathFilter {
+		return map[string]filestorage.PathFilter{
+			ActionFilesDelete: denyAllPathFilter,
+			ActionFilesWrite:  denyAllPathFilter,
+			ActionFilesRead:   denyAllPathFilter,
+		}
+	})
+	publicRoot, _            = filepath.Abs("../../../public")
+	publicStaticFilesStorage = newDiskStorage("public", "Public static files", &StorageLocalDiskConfig{
 		Path: publicRoot,
 		Roots: []string{
 			"/testdata/",
@@ -33,12 +42,15 @@ func TestListFiles(t *testing.T) {
 			"/maps/",
 			"/upload/",
 		},
-		}).setReadOnly(true).setBuiltin(true),
+	}).setReadOnly(true).setBuiltin(true)
-	}
+)
+
+func TestListFiles(t *testing.T) {
+	roots := []storageRuntime{publicStaticFilesStorage}
 
 	store := newStandardStorageService(sqlstore.InitTestDB(t), roots, func(orgId int64) []storageRuntime {
 		return make([]storageRuntime, 0)
-	})
+	}, allowAllAuthService)
 	frame, err := store.List(context.Background(), dummyUser, "public/testdata")
 	require.NoError(t, err)
 
@@ -53,22 +65,38 @@ func TestListFiles(t *testing.T) {
 	experimental.CheckGoldenJSONFrame(t, "testdata", "public_testdata_js_libraries.golden", testDsFrame, true)
 }
 
-func setupUploadStore(t *testing.T) (StorageService, *filestorage.MockFileStorage, string) {
+func TestListFilesWithoutPermissions(t *testing.T) {
+	roots := []storageRuntime{publicStaticFilesStorage}
+
+	store := newStandardStorageService(sqlstore.InitTestDB(t), roots, func(orgId int64) []storageRuntime {
+		return make([]storageRuntime, 0)
+	}, denyAllAuthService)
+	frame, err := store.List(context.Background(), dummyUser, "public/testdata")
+	require.NoError(t, err)
+	rowLen, err := frame.RowLen()
+	require.NoError(t, err)
+	require.Equal(t, 0, rowLen)
+}
+
+func setupUploadStore(t *testing.T, authService storageAuthService) (StorageService, *filestorage.MockFileStorage, string) {
 	t.Helper()
 	storageName := "resources"
 	mockStorage := &filestorage.MockFileStorage{}
 	sqlStorage := newSQLStorage(storageName, "Testing upload", &StorageSQLConfig{orgId: 1}, sqlstore.InitTestDB(t))
 	sqlStorage.store = mockStorage
 
+	if authService == nil {
+		authService = allowAllAuthService
+	}
 	store := newStandardStorageService(sqlstore.InitTestDB(t), []storageRuntime{sqlStorage}, func(orgId int64) []storageRuntime {
 		return make([]storageRuntime, 0)
-	})
+	}, authService)
 
 	return store, mockStorage, storageName
 }
 
 func TestShouldUploadWhenNoFileAlreadyExists(t *testing.T) {
-	service, mockStorage, storageName := setupUploadStore(t)
+	service, mockStorage, storageName := setupUploadStore(t, nil)
 
 	mockStorage.On("Get", mock.Anything, "/myFile.jpg").Return(nil, nil)
 	mockStorage.On("Upsert", mock.Anything, mock.Anything).Return(nil)
@@ -82,8 +110,20 @@ func TestShouldUploadWhenNoFileAlreadyExists(t *testing.T) {
 	require.NoError(t, err)
 }
 
+func TestShouldFailUploadWithoutAccess(t *testing.T) {
+	service, _, storageName := setupUploadStore(t, denyAllAuthService)
+
+	err := service.Upload(context.Background(), dummyUser, &UploadRequest{
+		EntityType: EntityTypeImage,
+		Contents:   make([]byte, 0),
+		Path:       storageName + "/myFile.jpg",
+		MimeType:   "image/jpg",
+	})
+	require.ErrorIs(t, err, ErrAccessDenied)
+}
+
 func TestShouldFailUploadWhenFileAlreadyExists(t *testing.T) {
-	service, mockStorage, storageName := setupUploadStore(t)
+	service, mockStorage, storageName := setupUploadStore(t, nil)
 
 	mockStorage.On("Get", mock.Anything, "/myFile.jpg").Return(&filestorage.File{Contents: make([]byte, 0)}, nil)
 
@@ -97,7 +137,7 @@ func TestShouldFailUploadWhenFileAlreadyExists(t *testing.T) {
 }
 
 func TestShouldDelegateFileDeletion(t *testing.T) {
-	service, mockStorage, storageName := setupUploadStore(t)
+	service, mockStorage, storageName := setupUploadStore(t, nil)
 
 	mockStorage.On("Delete", mock.Anything, "/myFile.jpg").Return(nil)
 
@@ -106,7 +146,7 @@ func TestShouldDelegateFileDeletion(t *testing.T) {
 }
 
 func TestShouldDelegateFolderCreation(t *testing.T) {
-	service, mockStorage, storageName := setupUploadStore(t)
+	service, mockStorage, storageName := setupUploadStore(t, nil)
 
 	mockStorage.On("CreateFolder", mock.Anything, "/nestedFolder/mostNestedFolder").Return(nil)
 
@@ -115,9 +155,9 @@ func TestShouldDelegateFolderCreation(t *testing.T) {
 }
 
 func TestShouldDelegateFolderDeletion(t *testing.T) {
-	service, mockStorage, storageName := setupUploadStore(t)
+	service, mockStorage, storageName := setupUploadStore(t, nil)
 
-	mockStorage.On("DeleteFolder", mock.Anything, "/", &filestorage.DeleteFolderOptions{Force: true}).Return(nil)
+	mockStorage.On("DeleteFolder", mock.Anything, "/", mock.Anything).Return(nil)
 
 	err := service.DeleteFolder(context.Background(), dummyUser, &DeleteFolderCmd{
 		Path:  storageName,

pkg/services/store/static_auth.go

@@ -0,0 +1,41 @@
+package store
+
+import (
+	"context"
+
+	"github.com/grafana/grafana/pkg/infra/filestorage"
+	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/models"
+)
+
+type createPathFilterByAction func(ctx context.Context, user *models.SignedInUser, storageName string) map[string]filestorage.PathFilter
+
+func newStaticStorageAuthService(createPathFilterByAction createPathFilterByAction) storageAuthService {
+	return &staticStorageAuth{
+		denyAllFileGuardian:      &denyAllFileGuardian{},
+		createPathFilterByAction: createPathFilterByAction,
+		log:                      log.New("staticStorageAuthService"),
+	}
+}
+
+type staticStorageAuth struct {
+	log                      log.Logger
+	denyAllFileGuardian      fileGuardian
+	createPathFilterByAction createPathFilterByAction
+}
+
+func (a *staticStorageAuth) newGuardian(ctx context.Context, user *models.SignedInUser, storageName string) fileGuardian {
+	pathFilter := a.createPathFilterByAction(ctx, user, storageName)
+
+	if pathFilter == nil {
+		return a.denyAllFileGuardian
+	}
+
+	return &pathFilterFileGuardian{
+		ctx:                ctx,
+		user:               user,
+		log:                a.log,
+		prefix:             storageName,
+		pathFilterByAction: pathFilter,
+	}
+}

pkg/services/store/tree.go

@@ -85,7 +85,7 @@ func (t *nestedTree) GetFile(ctx context.Context, orgId int64, path string) (*fi
 	return root.Store().Get(ctx, path)
 }
 
-func (t *nestedTree) ListFolder(ctx context.Context, orgId int64, path string) (*StorageListFrame, error) {
+func (t *nestedTree) ListFolder(ctx context.Context, orgId int64, path string, accessFilter filestorage.PathFilter) (*StorageListFrame, error) {
 	if path == "" || path == "/" {
 		t.assureOrgIsInitialized(orgId)
 
@@ -150,6 +150,7 @@ func (t *nestedTree) ListFolder(ctx context.Context, orgId int64, path string) (
 		Recursive:   false,
 		WithFolders: true,
 		WithFiles:   true,
+		Filter:      accessFilter,
 	})
 
 	if err != nil {

pkg/services/store/types.go

@@ -30,7 +30,7 @@ type WriteValueResponse struct {
 
 type storageTree interface {
 	GetFile(ctx context.Context, orgId int64, path string) (*filestorage.File, error)
-	ListFolder(ctx context.Context, orgId int64, path string) (*StorageListFrame, error)
+	ListFolder(ctx context.Context, orgId int64, path string, accessFilter filestorage.PathFilter) (*StorageListFrame, error)
 }
 
 //-------------------------------------------

pkg/services/store/utils.go

@@ -28,3 +28,8 @@ func getPathAndScope(c *models.ReqContext) (string, string) {
 	}
 	return splitFirstSegment(path)
 }
+
+func getFirstSegment(path string) string {
+	firstSegment, _ := splitFirstSegment(path)
+	return firstSegment
+}