fix(sharestorageprovider): Stat resource when resolving share by path

When resolving a received shared by mountpath, make sure that the
shared resource actually exists. This avoids issues with dangling shares
for already delete resources using the same mountpoint.

Fixes: owncloud/ocis#7895

changelog/unreleased/fix-stat-recreated-share.md

@@ -0,0 +1,8 @@
+Bugfix: Fix Stat() by Path on re-created resource
+
+We fixed bug that caused Stat Requests using a Path reference to a mount point
+in the sharejail to not resolve correctly, when a share using the same
+mount point to an already deleted resource was still existing.
+
+https://github.com/cs3org/reva/pull/4561
+https://github.com/owncloud/ocis/issues/7895

internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go

@@ -1057,9 +1057,11 @@ func (s *service) resolveAcceptedShare(ctx context.Context, ref *provider.Refere
  return lsRes.Share, lsRes.Status, nil
  }
 
- // we currently need to list all shares and match the path if the request is relative to the share jail root
+ // we currently need to list all accepted shares and match the path if the
+ // request is relative to the share jail root. Also we need to Stat() the
+ // shared resource's id to check whether that still exist. There might be
+ // old shares using the same path but for an already vanished resource id.
  if ref.ResourceId.OpaqueId == utils.ShareStorageProviderID && ref.Path != "." {
- // we need to list accepted shares and match the path
  lsRes, err := sharingCollaborationClient.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{
  Filters: []*collaboration.Filter{
  {
@@ -1079,10 +1081,31 @@ func (s *service) resolveAcceptedShare(ctx context.Context, ref *provider.Refere
  }
  for _, receivedShare := range lsRes.Shares {
  if isMountPointForPath(receivedShare.MountPoint.Path, ref.Path) {
+ // Only return this share if the resource still exists.
+ gatewayClient, err := s.gatewaySelector.Next()
+ if err != nil {
+ return nil, nil, err
+ }
+ sRes, err := gatewayClient.Stat(ctx, &provider.StatRequest{
+ Ref: &provider.Reference{ResourceId: receivedShare.GetShare().GetResourceId()},
+ })
+ if err != nil {
+ appctx.GetLogger(ctx).Debug().
+ Err(err).
+ Interface("resourceID", receivedShare.GetShare().GetResourceId()).
+ Msg("resolveAcceptedShare: failed to stat shared resource")
+ continue
+ }
+ if sRes.Status.Code != rpc.Code_CODE_OK {
+ appctx.GetLogger(ctx).Debug().
+ Interface("resourceID", receivedShare.GetShare().GetResourceId()).
+ Interface("status", sRes.Status).
+ Msg("resolveAcceptedShare: failed to stat shared resource")
+ continue
+ }
  return receivedShare, lsRes.Status, nil
  }
  }
-
  }
 
  return nil, status.NewNotFound(ctx, "sharesstorageprovider: not found "+ref.String()), nil

internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go

@@ -261,7 +261,23 @@ var _ = Describe("Sharesstorageprovider", func() {
  },
  }
  default:
- if req.Ref.ResourceId.OpaqueId == "shareddir-merged" {
+ switch req.Ref.ResourceId.OpaqueId {
+ case "shareddir", "shareddir2":
+ permissionSet := &sprovider.ResourcePermissions{
+ Stat: true,
+ ListContainer: true,
+ }
+ return &sprovider.StatResponse{
+ Status: status.NewOK(context.Background()),
+ Info: &sprovider.ResourceInfo{
+ Type: sprovider.ResourceType_RESOURCE_TYPE_CONTAINER,
+ Path: "share1-shareddir",
+ Id: req.Ref.ResourceId,
+ PermissionSet: permissionSet,
+ Size: 100,
+ },
+ }
+ case "shareddir-merged":
  permissionSet := &sprovider.ResourcePermissions{
  Stat: true,
  ListContainer: true,
@@ -280,9 +296,10 @@ var _ = Describe("Sharesstorageprovider", func() {
  Size: 100,
  },
  }
- }
- return &sprovider.StatResponse{
- Status: status.NewNotFound(context.Background(), "not found"),
+ default:
+ return &sprovider.StatResponse{
+ Status: status.NewNotFound(context.Background(), "not found"),
+ }
  }
  }
  },