Skip to content

Commit

Permalink
Support for Deleting Shared Folders as a Share Receiver (#1891)
Browse files Browse the repository at this point in the history
  • Loading branch information
refs authored Jul 22, 2021
1 parent 1fb0bd6 commit 1cbc542
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 17 deletions.
5 changes: 5 additions & 0 deletions changelog/unreleased/delete-shared-resources.md
Original file line number Diff line number Diff line change
@@ -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
59 changes: 59 additions & 0 deletions internal/grpc/services/gateway/storageprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions internal/grpc/services/storageprovider/storageprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions pkg/appctx/appctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions pkg/storage/utils/decomposedfs/tree/tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 0 additions & 6 deletions tests/acceptance/expected-failures-on-OCIS-storage.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 0 additions & 2 deletions tests/acceptance/expected-failures-on-OWNCLOUD-storage.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 0 additions & 9 deletions tests/acceptance/expected-failures-on-S3NG-storage.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 1cbc542

Please sign in to comment.