diff --git a/changelog/unreleased/delete-shared-resources.md b/changelog/unreleased/delete-shared-resources.md new file mode 100644 index 0000000000..793c82a98a --- /dev/null +++ b/changelog/unreleased/delete-shared-resources.md @@ -0,0 +1,5 @@ +Bugfix: Delete Shared Resources as Receiver + +It is now possible to delete a shared resource as a receiver and not having the data ending up in the receiver's trash bin, causing a possible leak. + +https://github.com/cs3org/reva/pull/1891 \ No newline at end of file diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index 33e5d7ff27..aa48f9fbca 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -27,6 +27,9 @@ import ( "sync" "time" + collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" + types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" @@ -850,6 +853,62 @@ func (s *svc) Delete(ctx context.Context, req *provider.DeleteRequest) (*provide if s.isShareName(ctx, p) { log.Debug().Msgf("path:%s points to share name", p) + sRes, err := s.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{}) + if err != nil { + return nil, err + } + + statRes, err := s.Stat(ctx, &provider.StatRequest{ + Ref: &provider.Reference{ + Path: p, + }, + }) + if err != nil { + return nil, err + } + + // the following will check that: + // - the resource to delete is a share the current user received + // - signal the storage the delete must not land in the trashbin + // - delete the resource and update the share status to pending + for _, share := range sRes.Shares { + if statRes != nil && (share.Share.ResourceId.OpaqueId == statRes.Info.Id.OpaqueId) && (share.Share.ResourceId.StorageId == statRes.Info.Id.StorageId) { + // this opaque needs explanation. It signals the storage the resource we're about to delete does not + // belong to the current user because it was share to her, thus delete the "node" and don't send it to + // the trash bin, since the share can be mounted as many times as desired. + req.Opaque = &types.Opaque{ + Map: map[string]*types.OpaqueEntry{ + "deleting_shared_resource": { + Value: []byte("true"), + Decoder: "plain", + }, + }, + } + + // the following block takes care of updating the state of the share to "pending". This will ensure the user + // can "Accept" the share once again. + r := &collaboration.UpdateReceivedShareRequest{ + Ref: &collaboration.ShareReference{ + Spec: &collaboration.ShareReference_Id{ + Id: &collaboration.ShareId{ + OpaqueId: share.Share.Id.OpaqueId, + }, + }, + }, + Field: &collaboration.UpdateReceivedShareRequest_UpdateField{ + Field: &collaboration.UpdateReceivedShareRequest_UpdateField_State{ + State: collaboration.ShareState_SHARE_STATE_REJECTED, + }, + }, + } + + _, err := s.UpdateReceivedShare(ctx, r) + if err != nil { + return nil, err + } + } + } + ref := &provider.Reference{Path: p} req.Ref = ref diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go index cdf813b73b..6de3f41a3a 100644 --- a/internal/grpc/services/storageprovider/storageprovider.go +++ b/internal/grpc/services/storageprovider/storageprovider.go @@ -530,6 +530,15 @@ func (s *service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro }, nil } + // check DeleteRequest for any known opaque properties. + if req.Opaque != nil { + _, ok := req.Opaque.Map["deleting_shared_resource"] + if ok { + // it is a binary key; its existence signals true. Although, do not assume. + ctx = context.WithValue(ctx, appctx.DeletingSharedResource, true) + } + } + if err := s.storage.Delete(ctx, newRef); err != nil { var st *rpc.Status switch err.(type) { diff --git a/pkg/appctx/appctx.go b/pkg/appctx/appctx.go index d28a7d604c..8c63c3ddae 100644 --- a/pkg/appctx/appctx.go +++ b/pkg/appctx/appctx.go @@ -24,6 +24,9 @@ import ( "github.com/rs/zerolog" ) +// DeletingSharedResource flags to a storage a shared resource is being deleted not by the owner. +var DeletingSharedResource struct{} + // WithLogger returns a context with an associated logger. func WithLogger(ctx context.Context, l *zerolog.Logger) context.Context { return l.WithContext(ctx) diff --git a/pkg/storage/utils/decomposedfs/tree/tree.go b/pkg/storage/utils/decomposedfs/tree/tree.go index 9e0c6d6ae0..934799f9e1 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree.go +++ b/pkg/storage/utils/decomposedfs/tree/tree.go @@ -339,7 +339,12 @@ func (t *Tree) ListFolder(ctx context.Context, n *node.Node) ([]*node.Node, erro // Delete deletes a node in the tree by moving it to the trash func (t *Tree) Delete(ctx context.Context, n *node.Node) (err error) { + deletingSharedResource := ctx.Value(appctx.DeletingSharedResource) + if deletingSharedResource != nil && deletingSharedResource.(bool) { + src := filepath.Join(t.lookup.InternalPath(n.ParentID), n.Name) + return os.Remove(src) + } // Prepare the trash // TODO use layout?, but it requires resolving the owners user if the username is used instead of the id. // the node knows the owner id so we use that for now diff --git a/tests/acceptance/expected-failures-on-OCIS-storage.md b/tests/acceptance/expected-failures-on-OCIS-storage.md index cd81b95df6..c3cc059ce8 100644 --- a/tests/acceptance/expected-failures-on-OCIS-storage.md +++ b/tests/acceptance/expected-failures-on-OCIS-storage.md @@ -779,14 +779,8 @@ _requires a [CS3 user provisioning api that can update the quota for a user](htt #### [invalid format of sharees response](https://github.com/owncloud/product/issues/292) #### [deleting a received share-folder moves it to trash-bin but does not unshare it](https://github.com/owncloud/ocis/issues/1123) -- [apiShareManagementToShares/acceptShares.feature:490](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/acceptShares.feature#L490) -- [apiShareManagementToShares/acceptShares.feature:491](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/acceptShares.feature#L491) - [apiShareManagementToShares/acceptShares.feature:494](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/acceptShares.feature#L494) -#### [Deleting a received folder moves it to trashbin](https://github.com/owncloud/ocis/issues/2217) - -- [apiTrashbin/trashbinSharingToShares.feature:23](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinSharingToShares.feature#L23) -- [apiTrashbin/trashbinSharingToShares.feature:24](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinSharingToShares.feature#L24) #### [deleting a file inside a received shared folder is moved to the trash-bin of the sharer not the receiver](https://github.com/owncloud/ocis/issues/1124) diff --git a/tests/acceptance/expected-failures-on-OWNCLOUD-storage.md b/tests/acceptance/expected-failures-on-OWNCLOUD-storage.md index 70b63193a1..25f769cce1 100644 --- a/tests/acceptance/expected-failures-on-OWNCLOUD-storage.md +++ b/tests/acceptance/expected-failures-on-OWNCLOUD-storage.md @@ -946,8 +946,6 @@ The following scenarios fail on OWNCLOUD storage but not on OCIS storage: - [apiShareManagementBasicToShares/createShareToSharesFolder.feature:290](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementBasicToShares/createShareToSharesFolder.feature#L290) - [apiShareManagementBasicToShares/createShareToSharesFolder.feature:291](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementBasicToShares/createShareToSharesFolder.feature#L291) -#### [invalid format of sharees response](https://github.com/owncloud/product/issues/292) - #### [deleting a received share-folder moves it to trash-bin but does not unshare it](https://github.com/owncloud/ocis/issues/1123) - [apiShareManagementToShares/acceptShares.feature:490](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/acceptShares.feature#L490) - [apiShareManagementToShares/acceptShares.feature:491](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/acceptShares.feature#L491) diff --git a/tests/acceptance/expected-failures-on-S3NG-storage.md b/tests/acceptance/expected-failures-on-S3NG-storage.md index c9c15b53d8..fc27dce558 100644 --- a/tests/acceptance/expected-failures-on-S3NG-storage.md +++ b/tests/acceptance/expected-failures-on-S3NG-storage.md @@ -772,17 +772,8 @@ _requires a [CS3 user provisioning api that can update the quota for a user](htt - [apiShareReshareToShares2/reShareSubfolder.feature:155](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareReshareToShares2/reShareSubfolder.feature#L155) - [apiShareReshareToShares2/reShareSubfolder.feature:156](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareReshareToShares2/reShareSubfolder.feature#L156) -#### [invalid format of sharees response](https://github.com/owncloud/product/issues/292) - -#### [deleting a received share-folder moves it to trash-bin but does not unshare it](https://github.com/owncloud/ocis/issues/1123) - -- [apiShareManagementToShares/acceptShares.feature:490](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/acceptShares.feature#L490) -- [apiShareManagementToShares/acceptShares.feature:491](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/acceptShares.feature#L491) - #### [deleting a file inside a received shared folder is moved to the trash-bin of the sharer not the receiver](https://github.com/owncloud/ocis/issues/1124) -- [apiTrashbin/trashbinSharingToShares.feature:23](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinSharingToShares.feature#L23) -- [apiTrashbin/trashbinSharingToShares.feature:24](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinSharingToShares.feature#L24) - [apiTrashbin/trashbinSharingToShares.feature:39](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinSharingToShares.feature#L39) - [apiTrashbin/trashbinSharingToShares.feature:40](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinSharingToShares.feature#L40) - [apiTrashbin/trashbinSharingToShares.feature:63](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiTrashbin/trashbinSharingToShares.feature#L63)