diff --git a/changelog/unreleased/fix-stat-recreated-share.md b/changelog/unreleased/fix-stat-recreated-share.md new file mode 100644 index 0000000000..480cd4cf88 --- /dev/null +++ b/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 diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index 8d5316c6d9..5fd8db144c 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -1057,11 +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 - - // look up share for this resourceid lsRes, err := sharingCollaborationClient.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{ Filters: []*collaboration.Filter{ { @@ -1080,15 +1080,32 @@ func (s *service) resolveAcceptedShare(ctx context.Context, ref *provider.Refere return nil, lsRes.Status, nil } for _, receivedShare := range lsRes.Shares { - // make sure to skip unaccepted shares - if receivedShare.State != collaboration.ShareState_SHARE_STATE_ACCEPTED { - continue - } 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 diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go index 9ff1f8592a..d2921a53e6 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider_test.go +++ b/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"), + } } } },