Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Deleting Shared Folders as a Share Receiver #1891

Merged
merged 16 commits into from
Jul 22, 2021
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 @@ -801,14 +801,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 @@ -959,8 +959,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 @@ -794,17 +794,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