diff --git a/pkg/api/api.go b/pkg/api/api.go index b6385a34ecc..167a94fda63 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -228,8 +228,10 @@ func (hs *HTTPServer) registerRoutes() { orgRoute.Get("/read/*", routing.Wrap(hs.StorageService.Read)) if hs.Features.IsEnabled(featuremgmt.FlagStorageLocalUpload) { - orgRoute.Delete("/delete/*", reqSignedIn, routing.Wrap(hs.StorageService.Delete)) - orgRoute.Post("/upload", reqSignedIn, routing.Wrap(hs.StorageService.Upload)) + orgRoute.Post("/delete/*", reqGrafanaAdmin, routing.Wrap(hs.StorageService.Delete)) + orgRoute.Post("/upload", reqGrafanaAdmin, routing.Wrap(hs.StorageService.Upload)) + orgRoute.Post("/createFolder", reqGrafanaAdmin, routing.Wrap(hs.StorageService.CreateFolder)) + orgRoute.Post("/deleteFolder", reqGrafanaAdmin, routing.Wrap(hs.StorageService.DeleteFolder)) } }) } diff --git a/pkg/infra/filestorage/api.go b/pkg/infra/filestorage/api.go index e298d6ce768..e9c3c1f344c 100644 --- a/pkg/infra/filestorage/api.go +++ b/pkg/infra/filestorage/api.go @@ -157,6 +157,16 @@ type ListOptions struct { Filter PathFilter } +type DeleteFolderOptions struct { + // Force if set to true, the `deleteFolder` operation will delete the selected folder together with all the nested files & folders + Force bool + + // AccessFilter must match all the nested files & folders in order for the `deleteFolder` operation to succeed + // The access check is not performed if `AccessFilter` is nil + AccessFilter PathFilter +} + +//go:generate mockery --name FileStorage --structname MockFileStorage --inpackage --filename file_storage_mock.go type FileStorage interface { Get(ctx context.Context, path string) (*File, error) Delete(ctx context.Context, path string) error @@ -166,7 +176,7 @@ type FileStorage interface { List(ctx context.Context, folderPath string, paging *Paging, options *ListOptions) (*ListResponse, error) CreateFolder(ctx context.Context, path string) error - DeleteFolder(ctx context.Context, path string) error + DeleteFolder(ctx context.Context, path string, options *DeleteFolderOptions) error close() error } diff --git a/pkg/infra/filestorage/cdk_blob_filestorage.go b/pkg/infra/filestorage/cdk_blob_filestorage.go index 988b8749497..566aa90ad57 100644 --- a/pkg/infra/filestorage/cdk_blob_filestorage.go +++ b/pkg/infra/filestorage/cdk_blob_filestorage.go @@ -9,10 +9,9 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "gocloud.dev/blob" - "gocloud.dev/gcerrors" - _ "gocloud.dev/blob/fileblob" _ "gocloud.dev/blob/memblob" + "gocloud.dev/gcerrors" ) const ( @@ -215,20 +214,62 @@ func (c cdkBlobStorage) CreateFolder(ctx context.Context, path string) error { return nil } -func (c cdkBlobStorage) DeleteFolder(ctx context.Context, folderPath string) error { - directoryMarkerPath := fmt.Sprintf("%s%s%s", folderPath, Delimiter, directoryMarker) - exists, err := c.bucket.Exists(ctx, strings.ToLower(directoryMarkerPath)) - - if err != nil { - return err +func (c cdkBlobStorage) DeleteFolder(ctx context.Context, folderPath string, options *DeleteFolderOptions) error { + folderPrefix := strings.ToLower(c.convertFolderPathToPrefix(folderPath)) + directoryMarkerPath := folderPrefix + directoryMarker + if !options.Force { + return c.bucket.Delete(ctx, directoryMarkerPath) } - if !exists { - return nil + iterators := []*blob.ListIterator{c.bucket.List(&blob.ListOptions{ + Prefix: folderPrefix, + Delimiter: Delimiter, + })} + + var pathsToDelete []string + + for len(iterators) > 0 { + obj, err := iterators[0].Next(ctx) + if errors.Is(err, io.EOF) { + iterators = iterators[1:] + continue + } + + if err != nil { + c.log.Error("force folder delete: failed to retrieve next object", "err", err) + return err + } + + path := obj.Key + lowerPath := strings.ToLower(path) + if obj.IsDir { + iterators = append([]*blob.ListIterator{c.bucket.List(&blob.ListOptions{ + Prefix: lowerPath, + Delimiter: Delimiter, + })}, iterators...) + continue + } + + pathsToDelete = append(pathsToDelete, lowerPath) } - err = c.bucket.Delete(ctx, strings.ToLower(directoryMarkerPath)) - return err + for _, path := range pathsToDelete { + if !options.AccessFilter.IsAllowed(path) { + c.log.Error("force folder delete: unauthorized access", "path", path) + return fmt.Errorf("force folder delete error, unauthorized access to %s", path) + } + } + + var lastErr error + for _, path := range pathsToDelete { + if err := c.bucket.Delete(ctx, path); err != nil { + c.log.Error("force folder delete: failed while deleting a file", "err", err, "path", path) + lastErr = err + // keep going and delete remaining files + } + } + + return lastErr } //nolint: gocyclo diff --git a/pkg/infra/filestorage/db_filestorage.go b/pkg/infra/filestorage/db_filestorage.go index 43e12f62446..f84f146c08a 100644 --- a/pkg/infra/filestorage/db_filestorage.go +++ b/pkg/infra/filestorage/db_filestorage.go @@ -4,6 +4,7 @@ import ( "context" "crypto/md5" "encoding/hex" + "reflect" // can ignore because we don't need a cryptographically secure hash function // sha1 low chance of collisions and better performance than sha256 @@ -135,30 +136,23 @@ func (s dbFileStorage) Delete(ctx context.Context, filePath string) error { if err != nil { return err } - err = s.db.WithDbSession(ctx, func(sess *sqlstore.DBSession) error { - table := &file{} - exists, innerErr := sess.Table("file").Where("path_hash = ?", pathHash).Get(table) - if innerErr != nil { - return innerErr + err = s.db.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error { + deletedFilesCount, err := sess.Table("file").Where("path_hash = ?", pathHash).Delete(&file{}) + if err != nil { + return err } - if !exists { - return nil + deletedMetaCount, err := sess.Table("file_meta").Where("path_hash = ?", pathHash).Delete(&fileMeta{}) + if err != nil { + if rollErr := sess.Rollback(); rollErr != nil { + return fmt.Errorf("failed to roll back transaction due to error: %s: %w", rollErr, err) + } + + return err } - number, innerErr := sess.Table("file").Where("path_hash = ?", pathHash).Delete(table) - if innerErr != nil { - return innerErr - } - s.log.Info("Deleted file", "path", filePath, "affectedRecords", number) - - metaTable := &fileMeta{} - number, innerErr = sess.Table("file_meta").Where("path_hash = ?", pathHash).Delete(metaTable) - if innerErr != nil { - return innerErr - } - s.log.Info("Deleted metadata", "path", filePath, "affectedRecords", number) - return innerErr + s.log.Info("Deleted file", "path", filePath, "deletedMetaCount", deletedMetaCount, "deletedFilesCount", deletedFilesCount) + return err }) return err @@ -490,24 +484,87 @@ func (s dbFileStorage) CreateFolder(ctx context.Context, path string) error { return err } -func (s dbFileStorage) DeleteFolder(ctx context.Context, folderPath string) error { - err := s.db.WithDbSession(ctx, func(sess *sqlstore.DBSession) error { - existing := &file{} - internalFolderPathHash, err := createPathHash(folderPath + Delimiter) - if err != nil { - return err - } - exists, err := sess.Table("file").Where("path_hash = ?", internalFolderPathHash).Get(existing) +func (s dbFileStorage) DeleteFolder(ctx context.Context, folderPath string, options *DeleteFolderOptions) error { + lowerFolderPath := strings.ToLower(folderPath) + if lowerFolderPath == "" || lowerFolderPath == Delimiter { + lowerFolderPath = Delimiter + } else if !strings.HasSuffix(lowerFolderPath, Delimiter) { + lowerFolderPath = lowerFolderPath + Delimiter + } + + if !options.Force { + return s.Delete(ctx, lowerFolderPath) + } + + err := s.db.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error { + var rawHashes []interface{} + + // xorm does not support `.Delete()` with `.Join()`, so we first have to retrieve all path_hashes and then use them to filter `file_meta` table + err := sess.Table("file"). + Cols("path_hash"). + Where("LOWER(path) LIKE ?", lowerFolderPath+"%"). + Find(&rawHashes) if err != nil { return err } - if !exists { + if len(rawHashes) == 0 { + s.log.Info("Force deleted folder", "path", lowerFolderPath, "deletedFilesCount", 0, "deletedMetaCount", 0) return nil } - _, err = sess.Table("file").Where("path_hash = ?", internalFolderPathHash).Delete(existing) - return err + accessFilter := options.AccessFilter.asSQLFilter() + accessibleFilesCount, err := sess.Table("file"). + Cols("path_hash"). + Where("LOWER(path) LIKE ?", lowerFolderPath+"%"). + Where(accessFilter.Where, accessFilter.Args...). + Count(&file{}) + if err != nil { + return err + } + + if int64(len(rawHashes)) != accessibleFilesCount { + s.log.Error("force folder delete: unauthorized access", "path", lowerFolderPath, "expectedAccessibleFilesCount", int64(len(rawHashes)), "actualAccessibleFilesCount", accessibleFilesCount) + return fmt.Errorf("force folder delete: unauthorized access for path %s", lowerFolderPath) + } + + var hashes []interface{} + for _, hash := range rawHashes { + if hashString, ok := hash.(string); ok { + hashes = append(hashes, hashString) + + // MySQL returns the `path_hash` field as []uint8 + } else if hashUint, ok := hash.([]uint8); ok { + hashes = append(hashes, string(hashUint)) + } else { + return fmt.Errorf("invalid hash type: %s", reflect.TypeOf(hash)) + } + } + + deletedFilesCount, err := sess. + Table("file"). + In("path_hash", hashes...). + Delete(&file{}) + + if err != nil { + return err + } + + deletedMetaCount, err := sess. + Table("file_meta"). + In("path_hash", hashes...). + Delete(&fileMeta{}) + + if err != nil { + if rollErr := sess.Rollback(); rollErr != nil { + return fmt.Errorf("failed to roll back transaction due to error: %s: %w", rollErr, err) + } + + return err + } + + s.log.Info("Force deleted folder", "path", folderPath, "deletedFilesCount", deletedFilesCount, "deletedMetaCount", deletedMetaCount) + return nil }) return err diff --git a/pkg/infra/filestorage/file_storage_mock.go b/pkg/infra/filestorage/file_storage_mock.go new file mode 100644 index 00000000000..c4b6bf70488 --- /dev/null +++ b/pkg/infra/filestorage/file_storage_mock.go @@ -0,0 +1,130 @@ +// Code generated by mockery v2.10.6. DO NOT EDIT. + +package filestorage + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" +) + +// MockFileStorage is an autogenerated mock type for the FileStorage type +type MockFileStorage struct { + mock.Mock +} + +// CreateFolder provides a mock function with given fields: ctx, path +func (_m *MockFileStorage) CreateFolder(ctx context.Context, path string) error { + ret := _m.Called(ctx, path) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { + r0 = rf(ctx, path) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// Delete provides a mock function with given fields: ctx, path +func (_m *MockFileStorage) Delete(ctx context.Context, path string) error { + ret := _m.Called(ctx, path) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { + r0 = rf(ctx, path) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// DeleteFolder provides a mock function with given fields: ctx, path, options +func (_m *MockFileStorage) DeleteFolder(ctx context.Context, path string, options *DeleteFolderOptions) error { + ret := _m.Called(ctx, path, options) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, *DeleteFolderOptions) error); ok { + r0 = rf(ctx, path, options) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// Get provides a mock function with given fields: ctx, path +func (_m *MockFileStorage) Get(ctx context.Context, path string) (*File, error) { + ret := _m.Called(ctx, path) + + var r0 *File + if rf, ok := ret.Get(0).(func(context.Context, string) *File); ok { + r0 = rf(ctx, path) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*File) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, string) error); ok { + r1 = rf(ctx, path) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// List provides a mock function with given fields: ctx, folderPath, paging, options +func (_m *MockFileStorage) List(ctx context.Context, folderPath string, paging *Paging, options *ListOptions) (*ListResponse, error) { + ret := _m.Called(ctx, folderPath, paging, options) + + var r0 *ListResponse + if rf, ok := ret.Get(0).(func(context.Context, string, *Paging, *ListOptions) *ListResponse); ok { + r0 = rf(ctx, folderPath, paging, options) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*ListResponse) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, string, *Paging, *ListOptions) error); ok { + r1 = rf(ctx, folderPath, paging, options) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Upsert provides a mock function with given fields: ctx, command +func (_m *MockFileStorage) Upsert(ctx context.Context, command *UpsertFileCommand) error { + ret := _m.Called(ctx, command) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, *UpsertFileCommand) error); ok { + r0 = rf(ctx, command) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// close provides a mock function with given fields: +func (_m *MockFileStorage) close() error { + ret := _m.Called() + + var r0 error + if rf, ok := ret.Get(0).(func() error); ok { + r0 = rf() + } else { + r0 = ret.Error(0) + } + + return r0 +} diff --git a/pkg/infra/filestorage/fs_integration_test.go b/pkg/infra/filestorage/fs_integration_test.go index f2ea34e0479..89a152a962a 100644 --- a/pkg/infra/filestorage/fs_integration_test.go +++ b/pkg/infra/filestorage/fs_integration_test.go @@ -1183,6 +1183,123 @@ func TestIntegrationFsStorage(t *testing.T) { }, }, }, + { + name: "should be able to delete folders with files if using force", + steps: []interface{}{ + cmdCreateFolder{ + path: "/folder/dashboards/myNewFolder", + }, + cmdUpsert{ + cmd: UpsertFileCommand{ + Path: "/folder/dashboards/myNewFolder/file.jpg", + Contents: emptyContents, + }, + }, + cmdDeleteFolder{ + path: "/folder/dashboards/myNewFolder", + options: &DeleteFolderOptions{ + Force: true, + }, + }, + queryListFolders{ + input: queryListFoldersInput{path: "/", options: &ListOptions{Recursive: true}}, + checks: [][]interface{}{ + checks(fPath("/folder")), + checks(fPath("/folder/dashboards")), + }, + }, + queryGet{ + input: queryGetInput{ + path: "/folder/dashboards/myNewFolder/file.jpg", + }, + }, + }, + }, + { + name: "should be able to delete root folder with force", + steps: []interface{}{ + cmdCreateFolder{ + path: "/folder/dashboards/myNewFolder", + }, + cmdUpsert{ + cmd: UpsertFileCommand{ + Path: "/folder/dashboards/myNewFolder/file.jpg", + Contents: emptyContents, + }, + }, + cmdDeleteFolder{ + path: "/", + options: &DeleteFolderOptions{ + Force: true, + }, + }, + queryListFolders{ + input: queryListFoldersInput{path: "/", options: &ListOptions{Recursive: true}}, + checks: [][]interface{}{}, + }, + queryGet{ + input: queryGetInput{ + path: "/folder/dashboards/myNewFolder/file.jpg", + }, + }, + }, + }, + { + name: "should not be able to delete a folder unless have access to all nested files", + steps: []interface{}{ + cmdCreateFolder{ + path: "/folder/dashboards/myNewFolder", + }, + cmdUpsert{ + cmd: UpsertFileCommand{ + Path: "/folder/dashboards/myNewFolder/file.jpg", + Contents: emptyContents, + }, + }, + cmdUpsert{ + cmd: UpsertFileCommand{ + Path: "/folder/dashboards/abc/file.jpg", + Contents: emptyContents, + }, + }, + cmdDeleteFolder{ + path: "/", + options: &DeleteFolderOptions{ + Force: true, + AccessFilter: NewPathFilter([]string{"/"}, nil, nil, []string{"/folder/dashboards/abc/file.jpg"}), + }, + error: &cmdErrorOutput{ + message: "force folder delete: unauthorized access for path %s", + args: []interface{}{"/"}, + }, + }, + queryListFolders{ + input: queryListFoldersInput{path: "/", options: &ListOptions{Recursive: true}}, + checks: [][]interface{}{ + checks(fPath("/folder")), + checks(fPath("/folder/dashboards")), + checks(fPath("/folder/dashboards/abc")), + checks(fPath("/folder/dashboards/myNewFolder")), + }, + }, + queryGet{ + input: queryGetInput{ + path: "/folder/dashboards/myNewFolder/file.jpg", + }, + checks: checks( + fName("file.jpg"), + ), + }, + queryGet{ + input: queryGetInput{ + path: "/folder/dashboards/myNewFolder/file.jpg", + }, + checks: checks( + fName("file.jpg"), + ), + }, + }, + }, } } diff --git a/pkg/infra/filestorage/test_utils.go b/pkg/infra/filestorage/test_utils.go index 657e3cb9e0e..7ca19b6a631 100644 --- a/pkg/infra/filestorage/test_utils.go +++ b/pkg/infra/filestorage/test_utils.go @@ -32,8 +32,9 @@ type cmdCreateFolder struct { } type cmdDeleteFolder struct { - path string - error *cmdErrorOutput + path string + error *cmdErrorOutput + options *DeleteFolderOptions } type queryGetInput struct { @@ -175,7 +176,7 @@ func handleCommand(t *testing.T, ctx context.Context, cmd interface{}, cmdName s } expectedErr = c.error case cmdDeleteFolder: - err = fs.DeleteFolder(ctx, c.path) + err = fs.DeleteFolder(ctx, c.path, c.options) if c.error == nil { require.NoError(t, err, "%s: should be able to delete %s", cmdName, c.path) } diff --git a/pkg/infra/filestorage/wrapper.go b/pkg/infra/filestorage/wrapper.go index 4165053dc74..cfdb344091e 100644 --- a/pkg/infra/filestorage/wrapper.go +++ b/pkg/infra/filestorage/wrapper.go @@ -256,26 +256,58 @@ func (b wrapper) CreateFolder(ctx context.Context, path string) error { return b.wrapped.CreateFolder(ctx, rootedPath) } -func (b wrapper) DeleteFolder(ctx context.Context, path string) error { +func (b wrapper) deleteFolderOptionsWithDefaults(options *DeleteFolderOptions) *DeleteFolderOptions { + if options == nil { + return &DeleteFolderOptions{ + Force: false, + AccessFilter: b.filter, + } + } + + if options.AccessFilter == nil { + return &DeleteFolderOptions{ + Force: options.Force, + AccessFilter: b.filter, + } + } + + var filter PathFilter + if options.AccessFilter != nil { + filter = newAndPathFilter(b.filter, wrapPathFilter(options.AccessFilter, b.rootFolder)) + } else { + filter = b.filter + } + + return &DeleteFolderOptions{ + Force: options.Force, + AccessFilter: filter, + } +} + +func (b wrapper) DeleteFolder(ctx context.Context, path string, options *DeleteFolderOptions) error { if err := b.validatePath(path); err != nil { return err } rootedPath := b.addRoot(path) - if !b.filter.IsAllowed(rootedPath) { - return nil + + optionsWithDefaults := b.deleteFolderOptionsWithDefaults(options) + if !optionsWithDefaults.AccessFilter.IsAllowed(rootedPath) { + return fmt.Errorf("delete folder unauthorized - no access to %s", rootedPath) } - isEmpty, err := b.isFolderEmpty(ctx, path) - if err != nil { - return err + if !optionsWithDefaults.Force { + isEmpty, err := b.isFolderEmpty(ctx, path) + if err != nil { + return err + } + + if !isEmpty { + return fmt.Errorf("folder %s is not empty - cant remove it", path) + } } - if !isEmpty { - return fmt.Errorf("folder %s is not empty - cant remove it", path) - } - - return b.wrapped.DeleteFolder(ctx, rootedPath) + return b.wrapped.DeleteFolder(ctx, rootedPath, optionsWithDefaults) } func (b wrapper) List(ctx context.Context, folderPath string, paging *Paging, options *ListOptions) (*ListResponse, error) { diff --git a/pkg/services/store/http.go b/pkg/services/store/http.go index d177759a256..a2fcb6db191 100644 --- a/pkg/services/store/http.go +++ b/pkg/services/store/http.go @@ -1,8 +1,10 @@ package store import ( + "encoding/json" "errors" "fmt" + "io" "io/ioutil" "net/http" "strings" @@ -20,6 +22,8 @@ type HTTPStorageService interface { List(c *models.ReqContext) response.Response Read(c *models.ReqContext) response.Response Delete(c *models.ReqContext) response.Response + DeleteFolder(c *models.ReqContext) response.Response + CreateFolder(c *models.ReqContext) response.Response Upload(c *models.ReqContext) response.Response } @@ -71,6 +75,14 @@ func (s *httpStorage) Upload(c *models.ReqContext) response.Response { }) } + folder, ok := c.Req.MultipartForm.Value["folder"] + if !ok || len(folder) != 1 { + return response.JSON(400, map[string]interface{}{ + "message": "please specify the upload folder", + "err": true, + }) + } + fileHeader := files[0] if fileHeader.Size > MAX_UPLOAD_SIZE { return errFileTooBig @@ -95,7 +107,7 @@ func (s *httpStorage) Upload(c *models.ReqContext) response.Response { return errFileTooBig } - path := RootResources + "/" + fileHeader.Filename + path := folder[0] + "/" + fileHeader.Filename mimeType := http.DetectContentType(data) @@ -140,17 +152,75 @@ func (s *httpStorage) Read(c *models.ReqContext) response.Response { func (s *httpStorage) Delete(c *models.ReqContext) response.Response { // full path is api/storage/delete/upload/example.jpg, but we only want the part after upload - _, path := getPathAndScope(c) - err := s.store.Delete(c.Req.Context(), c.SignedInUser, "/"+path) + scope, path := getPathAndScope(c) + + err := s.store.Delete(c.Req.Context(), c.SignedInUser, scope+"/"+path) if err != nil { - return response.Error(400, "cannot call delete", err) + return response.Error(400, "failed to delete the file: "+err.Error(), err) } - return response.JSON(200, map[string]string{ + return response.JSON(200, map[string]interface{}{ "message": "Removed file from storage", + "success": true, "path": path, }) } +func (s *httpStorage) DeleteFolder(c *models.ReqContext) response.Response { + body, err := io.ReadAll(c.Req.Body) + if err != nil { + return response.Error(500, "error reading bytes", err) + } + + cmd := &DeleteFolderCmd{} + err = json.Unmarshal(body, cmd) + if err != nil { + return response.Error(400, "error parsing body", err) + } + + if cmd.Path == "" { + return response.Error(400, "empty path", err) + } + + // full path is api/storage/delete/upload/example.jpg, but we only want the part after upload + _, path := getPathAndScope(c) + if err := s.store.DeleteFolder(c.Req.Context(), c.SignedInUser, cmd); err != nil { + return response.Error(400, "failed to delete the folder: "+err.Error(), err) + } + + return response.JSON(200, map[string]interface{}{ + "message": "Removed folder from storage", + "success": true, + "path": path, + }) +} + +func (s *httpStorage) CreateFolder(c *models.ReqContext) response.Response { + body, err := io.ReadAll(c.Req.Body) + if err != nil { + return response.Error(500, "error reading bytes", err) + } + + cmd := &CreateFolderCmd{} + err = json.Unmarshal(body, cmd) + if err != nil { + return response.Error(400, "error parsing body", err) + } + + if cmd.Path == "" { + return response.Error(400, "empty path", err) + } + + if err := s.store.CreateFolder(c.Req.Context(), c.SignedInUser, cmd); err != nil { + return response.Error(400, "failed to create the folder: "+err.Error(), err) + } + + return response.JSON(200, map[string]interface{}{ + "message": "Folder created", + "success": true, + "path": cmd.Path, + }) +} + func (s *httpStorage) List(c *models.ReqContext) response.Response { params := web.Params(c.Req) path := params["*"] diff --git a/pkg/services/store/service.go b/pkg/services/store/service.go index 7089494a7c4..e096e8c89af 100644 --- a/pkg/services/store/service.go +++ b/pkg/services/store/service.go @@ -19,7 +19,7 @@ import ( var grafanaStorageLogger = log.New("grafanaStorageLogger") var ErrUploadFeatureDisabled = errors.New("upload feature is disabled") -var ErrUnsupportedStorage = errors.New("storage does not support upload operation") +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") @@ -29,6 +29,15 @@ const RootResources = "resources" const MAX_UPLOAD_SIZE = 3 * 1024 * 1024 // 3MB +type DeleteFolderCmd struct { + Path string `json:"path"` + Force bool `json:"force"` +} + +type CreateFolderCmd struct { + Path string `json:"path"` +} + type StorageService interface { registry.BackgroundService @@ -42,6 +51,10 @@ type StorageService interface { Delete(ctx context.Context, user *models.SignedInUser, path string) error + DeleteFolder(ctx context.Context, user *models.SignedInUser, cmd *DeleteFolderCmd) error + + CreateFolder(ctx context.Context, user *models.SignedInUser, cmd *CreateFolderCmd) error + validateUploadRequest(ctx context.Context, user *models.SignedInUser, req *UploadRequest, storagePath string) validationResult // sanitizeUploadRequest sanitizes the upload request and converts it into a command accepted by the FileStorage API @@ -86,15 +99,10 @@ func ProvideService(sql *sqlstore.SQLStore, features featuremgmt.FeatureToggles, return storages } - s := newStandardStorageService(globalRoots, initializeOrgStorages) - s.sql = sql - s.cfg = storageServiceConfig{ - allowUnsanitizedSvgUpload: false, - } - return s + return newStandardStorageService(sql, globalRoots, initializeOrgStorages) } -func newStandardStorageService(globalRoots []storageRuntime, initializeOrgStorages func(orgId int64) []storageRuntime) *standardStorageService { +func newStandardStorageService(sql *sqlstore.SQLStore, globalRoots []storageRuntime, initializeOrgStorages func(orgId int64) []storageRuntime) *standardStorageService { rootsByOrgId := make(map[int64][]storageRuntime) rootsByOrgId[ac.GlobalOrgID] = globalRoots @@ -104,7 +112,11 @@ func newStandardStorageService(globalRoots []storageRuntime, initializeOrgStorag } res.init() return &standardStorageService{ + sql: sql, tree: res, + cfg: storageServiceConfig{ + allowUnsanitizedSvgUpload: false, + }, } } @@ -143,13 +155,18 @@ type UploadRequest struct { OverwriteExistingFile bool } +func storageSupportsMutatingOperations(path string) bool { + // TODO: this is temporary - make it rbac-driven + return strings.HasPrefix(path, RootResources+"/") || path == RootResources +} + func (s *standardStorageService) Upload(ctx context.Context, user *models.SignedInUser, req *UploadRequest) error { upload, _ := s.tree.getRoot(getOrgId(user), RootResources) if upload == nil { return ErrUploadFeatureDisabled } - if !strings.HasPrefix(req.Path, RootResources+"/") { + if !storageSupportsMutatingOperations(req.Path) { return ErrUnsupportedStorage } @@ -188,12 +205,53 @@ func (s *standardStorageService) Upload(ctx context.Context, user *models.Signed return nil } -func (s *standardStorageService) Delete(ctx context.Context, user *models.SignedInUser, path string) error { - upload, _ := s.tree.getRoot(getOrgId(user), RootResources) - if upload == nil { - return fmt.Errorf("upload feature is not enabled") +func (s *standardStorageService) DeleteFolder(ctx context.Context, user *models.SignedInUser, cmd *DeleteFolderCmd) error { + resources, _ := s.tree.getRoot(getOrgId(user), RootResources) + if resources == nil { + return fmt.Errorf("resources storage is not enabled") } - err := upload.Delete(ctx, path) + + if !storageSupportsMutatingOperations(cmd.Path) { + return ErrUnsupportedStorage + } + + storagePath := strings.TrimPrefix(cmd.Path, RootResources) + if storagePath == "" { + storagePath = filestorage.Delimiter + } + return resources.DeleteFolder(ctx, storagePath, &filestorage.DeleteFolderOptions{Force: true}) +} + +func (s *standardStorageService) CreateFolder(ctx context.Context, user *models.SignedInUser, cmd *CreateFolderCmd) error { + if !storageSupportsMutatingOperations(cmd.Path) { + return ErrUnsupportedStorage + } + + resources, _ := s.tree.getRoot(getOrgId(user), RootResources) + if resources == nil { + return fmt.Errorf("resources storage is not enabled") + } + + storagePath := strings.TrimPrefix(cmd.Path, RootResources) + err := resources.CreateFolder(ctx, storagePath) + if err != nil { + return err + } + return nil +} + +func (s *standardStorageService) Delete(ctx context.Context, user *models.SignedInUser, path string) error { + if !storageSupportsMutatingOperations(path) { + return ErrUnsupportedStorage + } + + resources, _ := s.tree.getRoot(getOrgId(user), RootResources) + if resources == nil { + return fmt.Errorf("resources storage is not enabled") + } + + storagePath := strings.TrimPrefix(path, RootResources) + err := resources.Delete(ctx, storagePath) if err != nil { return err } diff --git a/pkg/services/store/service_test.go b/pkg/services/store/service_test.go index 9dba7282639..9c705621512 100644 --- a/pkg/services/store/service_test.go +++ b/pkg/services/store/service_test.go @@ -3,16 +3,15 @@ package store import ( "bytes" "context" - "os" "path/filepath" "testing" "github.com/grafana/grafana-plugin-sdk-go/experimental" + "github.com/grafana/grafana/pkg/infra/filestorage" "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/sqlstore" - "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/tsdb/testdatasource" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -37,7 +36,7 @@ func TestListFiles(t *testing.T) { }).setReadOnly(true).setBuiltin(true), } - store := newStandardStorageService(roots, func(orgId int64) []storageRuntime { + store := newStandardStorageService(sqlstore.InitTestDB(t), roots, func(orgId int64) []storageRuntime { return make([]storageRuntime, 0) }) frame, err := store.List(context.Background(), dummyUser, "public/testdata") @@ -54,18 +53,75 @@ func TestListFiles(t *testing.T) { experimental.CheckGoldenJSONFrame(t, "testdata", "public_testdata_js_libraries.golden", testDsFrame, true) } -func TestUpload(t *testing.T) { - features := featuremgmt.WithFeatures(featuremgmt.FlagStorageLocalUpload) - path, err := os.Getwd() - require.NoError(t, err) - cfg := &setting.Cfg{AppURL: "http://localhost:3000/", DataPath: path} - s := ProvideService(sqlstore.InitTestDB(t), features, cfg) - request := UploadRequest{ +func setupUploadStore(t *testing.T) (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 + + store := newStandardStorageService(sqlstore.InitTestDB(t), []storageRuntime{sqlStorage}, func(orgId int64) []storageRuntime { + return make([]storageRuntime, 0) + }) + + return store, mockStorage, storageName +} + +func TestShouldUploadWhenNoFileAlreadyExists(t *testing.T) { + service, mockStorage, storageName := setupUploadStore(t) + + mockStorage.On("Get", mock.Anything, "/myFile.jpg").Return(nil, nil) + mockStorage.On("Upsert", mock.Anything, mock.Anything).Return(nil) + + err := service.Upload(context.Background(), dummyUser, &UploadRequest{ EntityType: EntityTypeImage, Contents: make([]byte, 0), - Path: "resources/myFile.jpg", + Path: storageName + "/myFile.jpg", MimeType: "image/jpg", - } - err = s.Upload(context.Background(), dummyUser, &request) + }) + require.NoError(t, err) +} + +func TestShouldFailUploadWhenFileAlreadyExists(t *testing.T) { + service, mockStorage, storageName := setupUploadStore(t) + + mockStorage.On("Get", mock.Anything, "/myFile.jpg").Return(&filestorage.File{Contents: make([]byte, 0)}, nil) + + err := service.Upload(context.Background(), dummyUser, &UploadRequest{ + EntityType: EntityTypeImage, + Contents: make([]byte, 0), + Path: storageName + "/myFile.jpg", + MimeType: "image/jpg", + }) + require.ErrorIs(t, err, ErrFileAlreadyExists) +} + +func TestShouldDelegateFileDeletion(t *testing.T) { + service, mockStorage, storageName := setupUploadStore(t) + + mockStorage.On("Delete", mock.Anything, "/myFile.jpg").Return(nil) + + err := service.Delete(context.Background(), dummyUser, storageName+"/myFile.jpg") + require.NoError(t, err) +} + +func TestShouldDelegateFolderCreation(t *testing.T) { + service, mockStorage, storageName := setupUploadStore(t) + + mockStorage.On("CreateFolder", mock.Anything, "/nestedFolder/mostNestedFolder").Return(nil) + + err := service.CreateFolder(context.Background(), dummyUser, &CreateFolderCmd{Path: storageName + "/nestedFolder/mostNestedFolder"}) + require.NoError(t, err) +} + +func TestShouldDelegateFolderDeletion(t *testing.T) { + service, mockStorage, storageName := setupUploadStore(t) + + mockStorage.On("DeleteFolder", mock.Anything, "/", &filestorage.DeleteFolderOptions{Force: true}).Return(nil) + + err := service.DeleteFolder(context.Background(), dummyUser, &DeleteFolderCmd{ + Path: storageName, + Force: true, + }) require.NoError(t, err) } diff --git a/public/app/features/storage/CreateNewFolderModal.tsx b/public/app/features/storage/CreateNewFolderModal.tsx new file mode 100644 index 00000000000..152e7cf55ca --- /dev/null +++ b/public/app/features/storage/CreateNewFolderModal.tsx @@ -0,0 +1,44 @@ +import React from 'react'; +import { SubmitHandler, Validate } from 'react-hook-form'; + +import { Button, Field, Form, Input, Modal } from '@grafana/ui'; + +type FormModel = { folderName: string }; + +interface Props { + onSubmit: SubmitHandler; + onDismiss: () => void; + validate: Validate; +} + +const initialFormModel = { folderName: '' }; + +export function CreateNewFolderModal({ validate, onDismiss, onSubmit }: Props) { + return ( + +
+ {({ register, errors }) => ( + <> + + + + + + + + + )} +
+
+ ); +} diff --git a/public/app/features/storage/StoragePage.tsx b/public/app/features/storage/StoragePage.tsx index 4d53c6f1b2c..64639923997 100644 --- a/public/app/features/storage/StoragePage.tsx +++ b/public/app/features/storage/StoragePage.tsx @@ -1,16 +1,19 @@ import { css } from '@emotion/css'; -import React, { useMemo } from 'react'; +import React, { useMemo, useState } from 'react'; import { useAsync } from 'react-use'; import { DataFrame, GrafanaTheme2, isDataFrame, ValueLinkConfig } from '@grafana/data'; import { locationService } from '@grafana/runtime'; import { useStyles2, IconName, Spinner, TabsBar, Tab, Button, HorizontalGroup } from '@grafana/ui'; +import appEvents from 'app/core/app_events'; import { Page } from 'app/core/components/Page/Page'; import { useNavModel } from 'app/core/hooks/useNavModel'; import { GrafanaRouteComponentProps } from 'app/core/navigation/types'; +import { ShowConfirmModalEvent } from 'app/types/events'; import { AddRootView } from './AddRootView'; import { Breadcrumb } from './Breadcrumb'; +import { CreateNewFolderModal } from './CreateNewFolderModal'; import { ExportView } from './ExportView'; import { FileView } from './FileView'; import { FolderView } from './FolderView'; @@ -26,8 +29,20 @@ interface QueryParams { view: StorageView; } +const folderNameRegex = /^[a-z\d!\-_.*'() ]+$/; +const folderNameMaxLength = 256; + interface Props extends GrafanaRouteComponentProps {} +const getParentPath = (path: string) => { + const lastSlashIdx = path.lastIndexOf('/'); + if (lastSlashIdx < 1) { + return ''; + } + + return path.substring(0, lastSlashIdx); +}; + export default function StoragePage(props: Props) { const styles = useStyles2(getStyles); const navModel = useNavModel('storage'); @@ -41,6 +56,8 @@ export default function StoragePage(props: Props) { locationService.push(url); }; + const [isAddingNewFolder, setIsAddingNewFolder] = useState(false); + const listing = useAsync((): Promise => { return getGrafanaStorage() .list(path) @@ -74,17 +91,26 @@ export default function StoragePage(props: Props) { let isFolder = path?.indexOf('/') < 0; if (listing.value) { const length = listing.value.length; - if (length > 1) { - isFolder = true; - } if (length === 1) { const first = listing.value.fields[0].values.get(0) as string; isFolder = !path.endsWith(first); + } else { + // TODO: handle files/folders which do not exist + isFolder = true; } } return isFolder; }, [path, listing]); + const fileNames = useMemo(() => { + return ( + listing.value?.fields + ?.find((f) => f.name === 'name') + ?.values?.toArray() + ?.filter((v) => typeof v === 'string') ?? [] + ); + }, [listing]); + const renderView = () => { const isRoot = !path?.length || path === '/'; switch (view) { @@ -135,20 +161,43 @@ export default function StoragePage(props: Props) { }); } const canAddFolder = isFolder && path.startsWith('resources'); - const canDelete = !isFolder && path.startsWith('resources/'); + const canDelete = path.startsWith('resources/'); return (
- + -
- {canAddFolder && } + + {canAddFolder && } {canDelete && ( - )} -
+
@@ -166,6 +215,41 @@ export default function StoragePage(props: Props) { ) : ( )} + + {isAddingNewFolder && ( + { + const folderPath = `${path}/${folderName}`; + const res = await getGrafanaStorage().createFolder(folderPath); + if (typeof res?.error !== 'string') { + setPath(folderPath); + setIsAddingNewFolder(false); + } + }} + onDismiss={() => { + setIsAddingNewFolder(false); + }} + validate={(folderName) => { + const lowerCase = folderName.toLowerCase(); + const trimmedLowerCase = lowerCase.trim(); + const existingTrimmedLowerCaseNames = fileNames.map((f) => f.trim().toLowerCase()); + + if (existingTrimmedLowerCaseNames.includes(trimmedLowerCase)) { + return 'A file or a folder with the same name already exists'; + } + + if (!folderNameRegex.test(lowerCase)) { + return 'Name contains illegal characters'; + } + + if (folderName.length > folderNameMaxLength) { + return `Name is too long, maximum length: ${folderNameMaxLength} characters`; + } + + return true; + }} + /> + )}
); }; diff --git a/public/app/features/storage/helper.ts b/public/app/features/storage/helper.ts index f93c15d841c..366fe02867a 100644 --- a/public/app/features/storage/helper.ts +++ b/public/app/features/storage/helper.ts @@ -8,6 +8,8 @@ export interface GrafanaStorage { get: (path: string) => Promise; list: (path: string) => Promise; upload: (folder: string, file: File) => Promise; + createFolder: (path: string) => Promise<{ error?: string }>; + delete: (path: { isFolder: boolean; path: string }) => Promise<{ error?: string }>; } class SimpleStorage implements GrafanaStorage { @@ -34,6 +36,52 @@ class SimpleStorage implements GrafanaStorage { return undefined; } + async createFolder(path: string): Promise<{ error?: string }> { + const res = await getBackendSrv().post<{ success: boolean; message: string }>( + '/api/storage/createFolder', + JSON.stringify({ path }) + ); + + if (!res.success) { + return { + error: res.message ?? 'unknown error', + }; + } + + return {}; + } + + async deleteFolder(req: { path: string; force: boolean }): Promise<{ error?: string }> { + const res = await getBackendSrv().post<{ success: boolean; message: string }>( + `/api/storage/deleteFolder`, + JSON.stringify(req) + ); + + if (!res.success) { + return { + error: res.message ?? 'unknown error', + }; + } + + return {}; + } + + async deleteFile(req: { path: string }): Promise<{ error?: string }> { + const res = await getBackendSrv().post<{ success: boolean; message: string }>(`/api/storage/delete/${req.path}`); + + if (!res.success) { + return { + error: res.message ?? 'unknown error', + }; + } + + return {}; + } + + async delete(req: { isFolder: boolean; path: string }): Promise<{ error?: string }> { + return req.isFolder ? this.deleteFolder({ path: req.path, force: true }) : this.deleteFile({ path: req.path }); + } + async upload(folder: string, file: File): Promise { const formData = new FormData(); formData.append('folder', folder);