From 7810180b5c04e7c4cd2d452cdd224aed51093b4e Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Tue, 29 Mar 2022 08:18:10 +0200 Subject: [PATCH] Grant id in mountpoint (#2664) * receive also the grantID in mountpoints * fix shares * add changelog * linter * change the earliest share implementation * we could have shares with no metadata * add spaceAlias to mountpoints * move Metadata to the share pkg --- .../unreleased/sharing-fix-add-grantID.md | 5 + internal/grpc/services/gateway/gateway.go | 1 - .../services/gateway/usershareprovider.go | 9 +- .../sharesstorageprovider.go | 144 ++++++++++++------ pkg/share/share.go | 7 + 5 files changed, 113 insertions(+), 53 deletions(-) create mode 100644 changelog/unreleased/sharing-fix-add-grantID.md diff --git a/changelog/unreleased/sharing-fix-add-grantID.md b/changelog/unreleased/sharing-fix-add-grantID.md new file mode 100644 index 0000000000..77553a5026 --- /dev/null +++ b/changelog/unreleased/sharing-fix-add-grantID.md @@ -0,0 +1,5 @@ +Enhancement: Add grantID to mountpoint + +We distinguish between the mountpoint of a share and the grant where the original file is located on the storage. + +https://github.com/cs3org/reva/pull/2664 diff --git a/internal/grpc/services/gateway/gateway.go b/internal/grpc/services/gateway/gateway.go index f2829ebea8..66315918e1 100644 --- a/internal/grpc/services/gateway/gateway.go +++ b/internal/grpc/services/gateway/gateway.go @@ -24,7 +24,6 @@ import ( "strings" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" - "github.com/cs3org/reva/v2/pkg/errtypes" "github.com/cs3org/reva/v2/pkg/rgrpc" "github.com/cs3org/reva/v2/pkg/sharedconf" diff --git a/internal/grpc/services/gateway/usershareprovider.go b/internal/grpc/services/gateway/usershareprovider.go index 13f38a96b1..cd18b3fe4a 100644 --- a/internal/grpc/services/gateway/usershareprovider.go +++ b/internal/grpc/services/gateway/usershareprovider.go @@ -32,6 +32,7 @@ import ( "github.com/cs3org/reva/v2/pkg/errtypes" "github.com/cs3org/reva/v2/pkg/rgrpc/status" "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" + "github.com/cs3org/reva/v2/pkg/share" "github.com/cs3org/reva/v2/pkg/storage/utils/grants" rtrace "github.com/cs3org/reva/v2/pkg/trace" "github.com/pkg/errors" @@ -166,7 +167,7 @@ func (s *svc) ListReceivedShares(ctx context.Context, req *collaboration.ListRec // TODO: This is a hack for now. // Can we do that cleaner somehow? // The `ListStorageSpaces` method in sharesstorageprovider/sharesstorageprovider.go needs the etags. - shareEtags := make(map[string]string, len(res.Shares)) + shareMetaData := make(map[string]share.Metadata, len(res.Shares)) for _, rs := range res.Shares { sRes, err := s.Stat(ctx, &provider.StatRequest{Ref: &provider.Reference{ResourceId: rs.Share.ResourceId}}) if err != nil { @@ -182,10 +183,10 @@ func (s *svc) ListReceivedShares(ctx context.Context, req *collaboration.ListRec Msg("ListRecievedShares: failed to stat the resource") continue } - shareEtags[rs.Share.Id.OpaqueId] = sRes.Info.Etag + shareMetaData[rs.Share.Id.OpaqueId] = share.Metadata{ETag: sRes.Info.Etag, Mtime: sRes.Info.Mtime} } - marshalled, err := json.Marshal(shareEtags) + marshalled, err := json.Marshal(shareMetaData) if err != nil { logger.Error(). Err(err). @@ -197,7 +198,7 @@ func (s *svc) ListReceivedShares(ctx context.Context, req *collaboration.ListRec Map: map[string]*typesv1beta1.OpaqueEntry{}, } } - opaque.Map["etags"] = &typesv1beta1.OpaqueEntry{ + opaque.Map["shareMetadata"] = &typesv1beta1.OpaqueEntry{ Decoder: "json", Value: marshalled, } diff --git a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go index 4164784c3f..1d67f6cd31 100644 --- a/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go +++ b/internal/grpc/services/sharesstorageprovider/sharesstorageprovider.go @@ -25,6 +25,7 @@ import ( "path/filepath" "strings" + "github.com/cs3org/reva/v2/pkg/share" "google.golang.org/grpc" codes "google.golang.org/grpc/codes" gstatus "google.golang.org/grpc/status" @@ -359,24 +360,13 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora } var receivedShares []*collaboration.ReceivedShare - var shareEtags map[string]string + var shareMd map[string]share.Metadata + var err error if fetchShares { - lsRes, err := s.gateway.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{ - // FIXME filter by received shares for resource id - listing all shares is tooo expensive! - }) + receivedShares, shareMd, err = s.fetchShares(ctx) if err != nil { return nil, errors.Wrap(err, "sharesstorageprovider: error calling ListReceivedSharesRequest") } - if lsRes.Status.Code != rpc.Code_CODE_OK { - return nil, fmt.Errorf("sharesstorageprovider: error calling ListReceivedSharesRequest") - } - receivedShares = lsRes.Shares - if lsRes.Opaque != nil { - if entry, ok := lsRes.Opaque.Map["etags"]; ok { - // If we can't get the etags thats fine, just continue. - _ = json.Unmarshal(entry.Value, &shareEtags) - } - } } res := &provider.ListStorageSpacesResponse{ @@ -390,39 +380,29 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora OpaqueId: utils.ShareStorageProviderID, } if spaceID == nil || utils.ResourceIDEqual(virtualRootID, spaceID) { - var earliestShare *collaboration.Share - // Lookup the last changed received share and use its etag for the share jail. - for _, rs := range receivedShares { - current := rs.Share - switch { - case earliestShare == nil: - earliestShare = current - case current.Mtime.Seconds > earliestShare.Mtime.Seconds: - earliestShare = current - case current.Mtime.Seconds == earliestShare.Mtime.Seconds && - current.Mtime.Nanos > earliestShare.Mtime.Nanos: - earliestShare = current - } - } - + earliestShare, atLeastOneAccepted := findEarliestShare(receivedShares, shareMd) var opaque *typesv1beta1.Opaque if earliestShare != nil { - if etag, ok := shareEtags[earliestShare.Id.OpaqueId]; ok { - opaque = utils.AppendPlainToOpaque(opaque, "etag", etag) + if md, ok := shareMd[earliestShare.Id.OpaqueId]; ok { + opaque = utils.AppendPlainToOpaque(opaque, "etag", md.ETag) } } - space := &provider.StorageSpace{ - Opaque: opaque, - Id: &provider.StorageSpaceId{ - OpaqueId: virtualRootID.StorageId + "!" + virtualRootID.OpaqueId, - }, - SpaceType: "virtual", - //Owner: &userv1beta1.User{Id: receivedShare.Share.Owner}, // FIXME actually, the mount point belongs to the recipient - // the sharesstorageprovider keeps track of mount points - Root: virtualRootID, - Name: "Shares Jail", + // only display the shares jail if we have accepted shares + if atLeastOneAccepted { + opaque = utils.AppendPlainToOpaque(opaque, "spaceAlias", "virtual/shares") + space := &provider.StorageSpace{ + Opaque: opaque, + Id: &provider.StorageSpaceId{ + OpaqueId: virtualRootID.StorageId + "!" + virtualRootID.OpaqueId, + }, + SpaceType: "virtual", + //Owner: &userv1beta1.User{Id: receivedShare.Share.Owner}, // FIXME actually, the mount point belongs to the recipient + // the sharesstorageprovider keeps track of mount points + Root: virtualRootID, + Name: "Shares Jail", + } + res.StorageSpaces = append(res.StorageSpaces, space) } - res.StorageSpaces = append(res.StorageSpaces, space) } case "grant": for _, receivedShare := range receivedShares { @@ -433,8 +413,8 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora continue } var opaque *typesv1beta1.Opaque - if etag, ok := shareEtags[receivedShare.Share.Id.OpaqueId]; ok { - opaque = utils.AppendPlainToOpaque(opaque, "etag", etag) + if md, ok := shareMd[receivedShare.Share.Id.OpaqueId]; ok { + opaque = utils.AppendPlainToOpaque(opaque, "etag", md.ETag) } // we know a grant for this resource space := &provider.StorageSpace{ @@ -474,9 +454,15 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora } } var opaque *typesv1beta1.Opaque - if etag, ok := shareEtags[receivedShare.Share.Id.OpaqueId]; ok { - opaque = utils.AppendPlainToOpaque(opaque, "etag", etag) + if md, ok := shareMd[receivedShare.Share.Id.OpaqueId]; ok { + opaque = utils.AppendPlainToOpaque(opaque, "etag", md.ETag) + } + // add the resourceID for the grant + if receivedShare.Share.ResourceId != nil { + opaque = utils.AppendPlainToOpaque(opaque, "grantStorageID", receivedShare.Share.ResourceId.StorageId) + opaque = utils.AppendPlainToOpaque(opaque, "grantOpaqueID", receivedShare.Share.ResourceId.OpaqueId) } + space := &provider.StorageSpace{ Opaque: opaque, Id: &provider.StorageSpaceId{ @@ -492,6 +478,7 @@ func (s *service) ListStorageSpaces(ctx context.Context, req *provider.ListStora // for now use the name from the share to override the name determined by stat if receivedShare.MountPoint != nil { space.Name = receivedShare.MountPoint.Path + space.Opaque = utils.AppendPlainToOpaque(space.Opaque, "spaceAlias", space.SpaceType+"/"+strings.ReplaceAll(strings.ToLower(space.Name), " ", "-")) } // what if we don't have a name? @@ -670,7 +657,11 @@ func (s *service) Unlock(ctx context.Context, req *provider.UnlockRequest) (*pro func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provider.StatResponse, error) { if isVirtualRoot(req.Ref.ResourceId) && (req.Ref.Path == "" || req.Ref.Path == ".") { - // The root is empty, it is filled by mountpoints + receivedShares, shareMd, err := s.fetchShares(ctx) + if err != nil { + return nil, err + } + earliestShare, _ := findEarliestShare(receivedShares, shareMd) return &provider.StatResponse{ Status: status.NewOK(ctx), Info: &provider.ResourceInfo{ @@ -687,13 +678,14 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide OpaqueId: utils.ShareStorageProviderID, }, Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER, - Mtime: &typesv1beta1.Timestamp{}, + Mtime: shareMd[earliestShare.Id.OpaqueId].Mtime, Path: "/", MimeType: "httpd/unix-directory", Size: 0, PermissionSet: &provider.ResourcePermissions{ // TODO }, + Etag: shareMd[earliestShare.Id.OpaqueId].ETag, }, }, nil } @@ -939,3 +931,59 @@ func (s *service) rejectReceivedShare(ctx context.Context, receivedShare *collab return errtypes.NewErrtypeFromStatus(res.Status) } + +func (s *service) fetchShares(ctx context.Context) ([]*collaboration.ReceivedShare, map[string]share.Metadata, error) { + lsRes, err := s.gateway.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{ + // FIXME filter by received shares for resource id - listing all shares is tooo expensive! + }) + if err != nil { + return nil, nil, errors.Wrap(err, "sharesstorageprovider: error calling ListReceivedSharesRequest") + } + if lsRes.Status.Code != rpc.Code_CODE_OK { + return nil, nil, fmt.Errorf("sharesstorageprovider: error calling ListReceivedSharesRequest") + } + receivedShares := lsRes.Shares + + var shareMd map[string]share.Metadata + if lsRes.Opaque != nil { + if entry, ok := lsRes.Opaque.Map["shareMetadata"]; ok { + // If we can't get the etags thats fine, just continue. + _ = json.Unmarshal(entry.Value, &shareMd) + } + } + return receivedShares, shareMd, nil +} + +func findEarliestShare(receivedShares []*collaboration.ReceivedShare, shareMd map[string]share.Metadata) (earliestShare *collaboration.Share, atLeastOneAccepted bool) { + for _, rs := range receivedShares { + var hasCurrentMd bool + var hasEarliestMd bool + + current := rs.Share + if rs.State == collaboration.ShareState_SHARE_STATE_ACCEPTED { + atLeastOneAccepted = true + } + + // We cannot assume that every share has metadata + if current.Id != nil { + _, hasCurrentMd = shareMd[current.Id.OpaqueId] + } + if earliestShare != nil && earliestShare.Id != nil { + _, hasEarliestMd = shareMd[earliestShare.Id.OpaqueId] + } + + switch { + case earliestShare == nil: + earliestShare = current + // ignore if one of the shares has no metadata + case !hasEarliestMd || !hasCurrentMd: + continue + case shareMd[current.Id.OpaqueId].Mtime.Seconds > shareMd[earliestShare.Id.OpaqueId].Mtime.Seconds: + earliestShare = current + case shareMd[current.Id.OpaqueId].Mtime.Seconds == shareMd[earliestShare.Id.OpaqueId].Mtime.Seconds && + shareMd[current.Id.OpaqueId].Mtime.Nanos > shareMd[earliestShare.Id.OpaqueId].Mtime.Nanos: + earliestShare = current + } + } + return earliestShare, atLeastOneAccepted +} diff --git a/pkg/share/share.go b/pkg/share/share.go index 2c4755dd58..5b6f1a5556 100644 --- a/pkg/share/share.go +++ b/pkg/share/share.go @@ -24,6 +24,7 @@ import ( userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocs/conversions" "github.com/cs3org/reva/v2/pkg/utils" "google.golang.org/genproto/protobuf/field_mask" @@ -37,6 +38,12 @@ const ( //go:generate mockery -name Manager +// Metadata contains Metadata for a share +type Metadata struct { + ETag string + Mtime *types.Timestamp +} + // Manager is the interface that manipulates shares. type Manager interface { // Create a new share in fn with the given acl.