diff --git a/internal/grpc/services/usershareprovider/usershareprovider.go b/internal/grpc/services/usershareprovider/usershareprovider.go index c2f3f9db6cb..466a500973b 100644 --- a/internal/grpc/services/usershareprovider/usershareprovider.go +++ b/internal/grpc/services/usershareprovider/usershareprovider.go @@ -20,6 +20,7 @@ package usershareprovider import ( "context" + "net/mail" "path/filepath" "regexp" "slices" @@ -180,8 +181,18 @@ func (s *service) CreateShare(ctx context.Context, req *collaboration.CreateShar Status: status.NewPermissionDenied(ctx, nil, "user tenantId does not match the target user tenantId"), }, nil } - } + if req.GetGrant().GetGrantee().GetUserId().GetType() == userpb.UserType_USER_TYPE_GUEST { + // guests are identified by their mail address, lets be strict here and only accept bare mail addresses + // and reject the "Mailbox"-Format that also contains a Display name + addr, err := mail.ParseAddress(req.GetGrant().GetGrantee().GetUserId().GetOpaqueId()) + if err != nil || addr.Name != "" { + return &collaboration.CreateShareResponse{ + Status: status.NewInvalidArg(ctx, "invalid mail address for guest grantee"), + }, nil + } + } + } gatewayClient, err := s.gatewaySelector.Next() if err != nil { return nil, err @@ -206,7 +217,7 @@ func (s *service) CreateShare(ctx context.Context, req *collaboration.CreateShar UserId: &userpb.UserId{ OpaqueId: req.GetGrant().GetGrantee().GetUserId().GetOpaqueId(), Idp: user.GetId().GetIdp(), - Type: userpb.UserType_USER_TYPE_PRIMARY}, + Type: req.GetGrant().GetGrantee().GetUserId().GetType()}, } } // some for group grantees diff --git a/internal/grpc/services/usershareprovider/usershareprovider_test.go b/internal/grpc/services/usershareprovider/usershareprovider_test.go index 6dd5b75af75..e4974cd256a 100644 --- a/internal/grpc/services/usershareprovider/usershareprovider_test.go +++ b/internal/grpc/services/usershareprovider/usershareprovider_test.go @@ -436,6 +436,7 @@ var _ = Describe("user share provider service", func() { Entry("ListGrants", conversions.RoleFromName("manager").CS3ResourcePermissions(), &providerpb.ResourcePermissions{ListGrants: true}, rpcpb.Code_CODE_OK, 1), ) }) + Context("create share with tenant awareness", func() { JustBeforeEach(func() { rgrpcService := usershareprovider.New(gatewaySelector, manager, []*regexp.Regexp{}) @@ -491,6 +492,94 @@ var _ = Describe("user share provider service", func() { manager.AssertNumberOfCalls(GinkgoT(), "Share", 1) }) + + It("succeeds when sharing with a guest using an email address", func() { + createShareResponse, err := provider.CreateShare(ctx, &collaborationpb.CreateShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + PermissionSet: conversions.RoleFromName("manager").CS3ResourcePermissions(), + }, + Grant: &collaborationpb.ShareGrant{ + Grantee: &providerpb.Grantee{ + Type: providerpb.GranteeType_GRANTEE_TYPE_USER, + Id: &providerpb.Grantee_UserId{UserId: &userpb.UserId{OpaqueId: "GUEST@example.com", TenantId: "tenant1", Type: userpb.UserType_USER_TYPE_GUEST}}, + }, + Permissions: &collaborationpb.SharePermissions{ + Permissions: conversions.RoleFromName("viewer").CS3ResourcePermissions(), + }, + }, + }) + + Expect(err).ToNot(HaveOccurred()) + Expect(createShareResponse.Status.Code).To(Equal(rpcpb.Code_CODE_OK)) + + manager.AssertNumberOfCalls(GinkgoT(), "Share", 1) + }) + + It("succeeds when sharing with a guest using a mail address with a quoted localpart including path separators", func() { + createShareResponse, err := provider.CreateShare(ctx, &collaborationpb.CreateShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + PermissionSet: conversions.RoleFromName("manager").CS3ResourcePermissions(), + }, + Grant: &collaborationpb.ShareGrant{ + Grantee: &providerpb.Grantee{ + Type: providerpb.GranteeType_GRANTEE_TYPE_USER, + Id: &providerpb.Grantee_UserId{UserId: &userpb.UserId{OpaqueId: "\"my/../email\"@email.com", TenantId: "tenant1", Type: userpb.UserType_USER_TYPE_GUEST}}, + }, + Permissions: &collaborationpb.SharePermissions{ + Permissions: conversions.RoleFromName("viewer").CS3ResourcePermissions(), + }, + }, + }) + + Expect(err).ToNot(HaveOccurred()) + Expect(createShareResponse.Status.Code).To(Equal(rpcpb.Code_CODE_OK)) + + manager.AssertNumberOfCalls(GinkgoT(), "Share", 1) + }) + + It("fails when sharing with a guest using an invalid mail address", func() { + createShareResponse, err := provider.CreateShare(ctx, &collaborationpb.CreateShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + PermissionSet: conversions.RoleFromName("manager").CS3ResourcePermissions(), + }, + Grant: &collaborationpb.ShareGrant{ + Grantee: &providerpb.Grantee{ + Type: providerpb.GranteeType_GRANTEE_TYPE_USER, + Id: &providerpb.Grantee_UserId{UserId: &userpb.UserId{OpaqueId: "not-a-mail", TenantId: "tenant1", Type: userpb.UserType_USER_TYPE_GUEST}}, + }, + Permissions: &collaborationpb.SharePermissions{ + Permissions: conversions.RoleFromName("viewer").CS3ResourcePermissions(), + }, + }, + }) + + Expect(err).ToNot(HaveOccurred()) + Expect(createShareResponse.Status.Code).To(Equal(rpcpb.Code_CODE_INVALID_ARGUMENT)) + + manager.AssertNumberOfCalls(GinkgoT(), "Share", 0) + }) + + It("fails when sharing with a guest using a mail address including a Mailbox name", func() { + createShareResponse, err := provider.CreateShare(ctx, &collaborationpb.CreateShareRequest{ + ResourceInfo: &providerpb.ResourceInfo{ + PermissionSet: conversions.RoleFromName("manager").CS3ResourcePermissions(), + }, + Grant: &collaborationpb.ShareGrant{ + Grantee: &providerpb.Grantee{ + Type: providerpb.GranteeType_GRANTEE_TYPE_USER, + Id: &providerpb.Grantee_UserId{UserId: &userpb.UserId{OpaqueId: "Test Useer ", TenantId: "tenant1", Type: userpb.UserType_USER_TYPE_GUEST}}, + }, + Permissions: &collaborationpb.SharePermissions{ + Permissions: conversions.RoleFromName("viewer").CS3ResourcePermissions(), + }, + }, + }) + + Expect(err).ToNot(HaveOccurred()) + Expect(createShareResponse.Status.Code).To(Equal(rpcpb.Code_CODE_INVALID_ARGUMENT)) + + manager.AssertNumberOfCalls(GinkgoT(), "Share", 0) + }) }) }) diff --git a/pkg/share/manager/jsoncs3/jsoncs3.go b/pkg/share/manager/jsoncs3/jsoncs3.go index 2e90b062ea2..492ec38009e 100644 --- a/pkg/share/manager/jsoncs3/jsoncs3.go +++ b/pkg/share/manager/jsoncs3/jsoncs3.go @@ -26,6 +26,7 @@ import ( "time" gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" rpcv1beta1 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" @@ -482,7 +483,7 @@ func (m *Manager) Share(ctx context.Context, md *provider.ResourceInfo, g *colla }) eg.Go(func() error { - err := m.CreatedCache.Add(ctx, s.GetCreator().GetOpaqueId(), shareID) + err := m.CreatedCache.Add(ctx, utils.NewFSSafeUserID(s.GetCreator()), shareID) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) @@ -496,13 +497,12 @@ func (m *Manager) Share(ctx context.Context, md *provider.ResourceInfo, g *colla switch g.Grantee.Type { case provider.GranteeType_GRANTEE_TYPE_USER: eg.Go(func() error { - userid := g.Grantee.GetUserId().GetOpaqueId() rs := &collaboration.ReceivedShare{ Share: s, State: collaboration.ShareState_SHARE_STATE_PENDING, } - err := m.UserReceivedStates.Add(ctx, userid, spaceID, rs) + err := m.UserReceivedStates.Add(ctx, utils.NewFSSafeUserID(g.GetGrantee().GetUserId()), spaceID, rs) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) @@ -512,8 +512,7 @@ func (m *Manager) Share(ctx context.Context, md *provider.ResourceInfo, g *colla }) case provider.GranteeType_GRANTEE_TYPE_GROUP: eg.Go(func() error { - groupid := g.Grantee.GetGroupId().GetOpaqueId() - err := m.GroupReceivedCache.Add(ctx, groupid, shareID) + err := m.GroupReceivedCache.Add(ctx, utils.FSSafeGroupID{ID: g.GetGrantee().GetGroupId()}, shareID) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) @@ -879,7 +878,7 @@ func (m *Manager) listCreatedShares(ctx context.Context, user *userv1beta1.User, defer span.End() sublog := appctx.GetLogger(ctx).With().Str("userid", user.GetId().GetOpaqueId()).Str("useridp", user.GetId().GetIdp()).Str("driver", "jsoncs3").Str("handler", "listCreatedShares").Logger() - list, err := m.CreatedCache.List(ctx, user.Id.OpaqueId) + list, err := m.CreatedCache.List(ctx, utils.NewFSSafeUserID(user.GetId())) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) @@ -1024,7 +1023,10 @@ func (m *Manager) ListReceivedShares(ctx context.Context, filters []*collaborati // first collect all spaceids the user has access to as a group member for _, group := range user.Groups { - list, err := m.GroupReceivedCache.List(ctx, group) + groupid := grouppb.GroupId{ + OpaqueId: group, + } + list, err := m.GroupReceivedCache.List(ctx, utils.FSSafeGroupID{ID: &groupid}) if err != nil { continue // ignore error, cache will be updated on next read } @@ -1049,7 +1051,7 @@ func (m *Manager) ListReceivedShares(ctx context.Context, filters []*collaborati } // add all spaces the user has receved shares for, this includes mount points and share state for groups - spaces, err := m.UserReceivedStates.List(ctx, user.Id.OpaqueId) + spaces, err := m.UserReceivedStates.List(ctx, utils.NewFSSafeUserID(user.GetId())) if err != nil { return nil, err } @@ -1112,7 +1114,7 @@ func (m *Manager) ListReceivedShares(ctx context.Context, filters []*collaborati } if s == nil { sublogr.Warn().Str("shareid", shareID).Msg("share not found. cleaning up") - _ = m.UserReceivedStates.Remove(ctx, user.Id.OpaqueId, w.ssid, shareID) + _ = m.UserReceivedStates.Remove(ctx, utils.NewFSSafeUserID(user.GetId()), w.ssid, shareID) continue } sublogr = sublogr.With().Str("shareid", shareID).Logger() @@ -1190,7 +1192,7 @@ func (m *Manager) ListReceivedShares(ctx context.Context, filters []*collaborati } // convert must be called in a lock-controlled block. -func (m *Manager) convert(ctx context.Context, userID string, s *collaboration.Share) *collaboration.ReceivedShare { +func (m *Manager) convert(ctx context.Context, userID *userv1beta1.UserId, s *collaboration.Share) *collaboration.ReceivedShare { ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "convert") defer span.End() @@ -1201,7 +1203,7 @@ func (m *Manager) convert(ctx context.Context, userID string, s *collaboration.S storageID, spaceID, _ := shareid.Decode(s.Id.OpaqueId) - state, err := m.UserReceivedStates.Get(ctx, userID, storageID+shareid.IDDelimiter+spaceID, s.Id.GetOpaqueId()) + state, err := m.UserReceivedStates.Get(ctx, utils.NewFSSafeUserID(userID), storageID+shareid.IDDelimiter+spaceID, s.Id.GetOpaqueId()) if err == nil && state != nil { rs.State = state.State rs.MountPoint = state.MountPoint @@ -1261,7 +1263,7 @@ func (m *Manager) getReceived(ctx context.Context, ref *collaboration.ShareRefer } } } - return m.convert(ctx, user.Id.GetOpaqueId(), s), nil + return m.convert(ctx, user.GetId(), s), nil } // UpdateReceivedShare updates the received share with share state. @@ -1296,12 +1298,12 @@ func (m *Manager) UpdateReceivedShare(ctx context.Context, receivedShare *collab // write back u := ctxpkg.ContextMustGetUser(ctx) - uid := u.GetId().GetOpaqueId() + uid := u.GetId() if u.GetId().GetType() == userv1beta1.UserType_USER_TYPE_SERVICE { - uid = forUser.GetOpaqueId() + uid = forUser } - err = m.UserReceivedStates.Add(ctx, uid, rs.Share.ResourceId.StorageId+shareid.IDDelimiter+rs.Share.ResourceId.SpaceId, rs) + err = m.UserReceivedStates.Add(ctx, utils.NewFSSafeUserID(uid), rs.Share.ResourceId.StorageId+shareid.IDDelimiter+rs.Share.ResourceId.SpaceId, rs) if err != nil { return nil, err } @@ -1339,7 +1341,7 @@ func (m *Manager) Load(ctx context.Context, shareChan <-chan *collaboration.Shar } else { l.Debug().Str("storageid", s.GetResourceId().GetStorageId()).Str("spaceid", s.GetResourceId().GetSpaceId()).Str("shareid", s.Id.OpaqueId).Msg("imported share") } - if err := m.CreatedCache.Add(ctx, s.GetCreator().GetOpaqueId(), s.Id.OpaqueId); err != nil { + if err := m.CreatedCache.Add(ctx, utils.NewFSSafeUserID(s.GetCreator()), s.Id.OpaqueId); err != nil { l.Error().Err(err).Interface("share", s).Msg("error persisting created cache") } else { l.Debug().Str("creatorid", s.GetCreator().GetOpaqueId()).Str("shareid", s.Id.OpaqueId).Msg("updated created cache") @@ -1355,14 +1357,14 @@ func (m *Manager) Load(ctx context.Context, shareChan <-chan *collaboration.Shar } if s.UserID != nil { spaceid := s.ReceivedShare.GetShare().GetResourceId().GetStorageId() + shareid.IDDelimiter + s.ReceivedShare.GetShare().GetResourceId().GetSpaceId() - if err := m.UserReceivedStates.Add(context.Background(), s.UserID.GetOpaqueId(), spaceid, s.ReceivedShare); err != nil { + if err := m.UserReceivedStates.Add(context.Background(), utils.NewFSSafeUserID(s.UserID), spaceid, s.ReceivedShare); err != nil { l.Error().Err(err).Interface("received share", s).Msg("error persisting received share for user") } else { l.Debug().Str("userid", s.UserID.GetOpaqueId()).Str("spaceid", spaceid).Str("shareid", s.ReceivedShare.GetShare().Id.OpaqueId).Msg("updated received share userdata") } } if s.ReceivedShare.Share.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_GROUP && s.UserID == nil { - if err := m.GroupReceivedCache.Add(context.Background(), s.ReceivedShare.GetShare().GetGrantee().GetGroupId().GetOpaqueId(), s.ReceivedShare.GetShare().GetId().GetOpaqueId()); err != nil { + if err := m.GroupReceivedCache.Add(context.Background(), utils.FSSafeGroupID{ID: s.ReceivedShare.GetShare().GetGrantee().GetGroupId()}, s.ReceivedShare.GetShare().GetId().GetOpaqueId()); err != nil { l.Error().Err(err).Interface("received share", s).Msg("error persisting received share to group cache") } else { l.Debug().Str("groupid", s.ReceivedShare.GetShare().GetGrantee().GetGroupId().GetOpaqueId()).Str("shareid", s.ReceivedShare.GetShare().Id.OpaqueId).Msg("updated received share group cache") @@ -1418,15 +1420,15 @@ func (m *Manager) removeShare(ctx context.Context, s *collaboration.Share, skipS eg.Go(func() error { // remove from created cache - return m.CreatedCache.Remove(ctx, s.GetCreator().GetOpaqueId(), s.Id.OpaqueId) + return m.CreatedCache.Remove(ctx, utils.NewFSSafeUserID(s.GetCreator()), s.Id.OpaqueId) }) eg.Go(func() error { // remove from user received states if s.GetGrantee().Type == provider.GranteeType_GRANTEE_TYPE_USER { - return m.UserReceivedStates.Remove(ctx, s.GetGrantee().GetUserId().GetOpaqueId(), s.GetResourceId().GetStorageId()+shareid.IDDelimiter+s.GetResourceId().GetSpaceId(), s.Id.OpaqueId) + return m.UserReceivedStates.Remove(ctx, utils.NewFSSafeUserID(s.GetGrantee().GetUserId()), s.GetResourceId().GetStorageId()+shareid.IDDelimiter+s.GetResourceId().GetSpaceId(), s.Id.OpaqueId) } else if s.GetGrantee().Type == provider.GranteeType_GRANTEE_TYPE_GROUP { - return m.GroupReceivedCache.Remove(ctx, s.GetGrantee().GetGroupId().GetOpaqueId(), s.Id.OpaqueId) + return m.GroupReceivedCache.Remove(ctx, utils.FSSafeGroupID{ID: s.GetGrantee().GetGroupId()}, s.Id.OpaqueId) } return nil }) diff --git a/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache.go b/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache.go index a0e94361a90..b8e536b0821 100644 --- a/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache.go +++ b/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache.go @@ -34,6 +34,7 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/errtypes" "github.com/opencloud-eu/reva/v2/pkg/storage/utils/decomposedfs/mtimesyncedcache" "github.com/opencloud-eu/reva/v2/pkg/storage/utils/metadata" + "github.com/opencloud-eu/reva/v2/pkg/utils" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" ) @@ -82,8 +83,8 @@ func New(s metadata.Storage, ttl time.Duration) Cache { } } -func (c *Cache) lockUser(userID string) func() { - v, _ := c.lockMap.LoadOrStore(userID, &sync.Mutex{}) +func (c *Cache) lockUser(userID utils.FilenameEncoder) func() { + v, _ := c.lockMap.LoadOrStore(userID.SafeFilename(), &sync.Mutex{}) lock := v.(*sync.Mutex) lock.Lock() @@ -91,14 +92,15 @@ func (c *Cache) lockUser(userID string) func() { } // Add adds a new entry to the cache -func (c *Cache) Add(ctx context.Context, userID, spaceID string, rs *collaboration.ReceivedShare) error { +func (c *Cache) Add(ctx context.Context, userID utils.FilenameEncoder, spaceID string, rs *collaboration.ReceivedShare) error { + userIDKey := userID.SafeFilename() ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "Grab lock") unlock := c.lockUser(userID) span.End() - span.SetAttributes(attribute.String("cs3.userid", userID)) + span.SetAttributes(attribute.String("cs3.userid.key", userIDKey)) defer unlock() - if _, ok := c.ReceivedSpaces.Load(userID); !ok { + if _, ok := c.ReceivedSpaces.Load(userIDKey); !ok { err := c.syncWithLock(ctx, userID) if err != nil { return err @@ -107,12 +109,12 @@ func (c *Cache) Add(ctx context.Context, userID, spaceID string, rs *collaborati ctx, span = appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "Add") defer span.End() - span.SetAttributes(attribute.String("cs3.userid", userID), attribute.String("cs3.spaceid", spaceID)) + span.SetAttributes(attribute.String("cs3.userid.key", userIDKey), attribute.String("cs3.spaceid", spaceID)) persistFunc := func() error { - c.initializeIfNeeded(userID, spaceID) + c.initializeIfNeeded(userIDKey, spaceID) - rss, _ := c.ReceivedSpaces.Load(userID) + rss, _ := c.ReceivedSpaces.Load(userIDKey) receivedSpace := rss.Spaces[spaceID] if receivedSpace.States == nil { receivedSpace.States = map[string]*State{} @@ -128,7 +130,7 @@ func (c *Cache) Add(ctx context.Context, userID, spaceID string, rs *collaborati log := appctx.GetLogger(ctx).With(). Str("hostname", os.Getenv("HOSTNAME")). - Str("userID", userID). + Str("userIDKey", userIDKey). Str("spaceID", spaceID).Logger() var err error @@ -167,18 +169,19 @@ func (c *Cache) Add(ctx context.Context, userID, spaceID string, rs *collaborati } // Get returns one entry from the cache -func (c *Cache) Get(ctx context.Context, userID, spaceID, shareID string) (*State, error) { +func (c *Cache) Get(ctx context.Context, userID utils.FilenameEncoder, spaceID, shareID string) (*State, error) { ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "Grab lock") + userIDKey := userID.SafeFilename() unlock := c.lockUser(userID) span.End() - span.SetAttributes(attribute.String("cs3.userid", userID)) + span.SetAttributes(attribute.String("cs3.userid.key", userIDKey)) defer unlock() err := c.syncWithLock(ctx, userID) if err != nil { return nil, err } - rss, ok := c.ReceivedSpaces.Load(userID) + rss, ok := c.ReceivedSpaces.Load(userIDKey) if !ok || rss.Spaces[spaceID] == nil { return nil, nil } @@ -186,21 +189,22 @@ func (c *Cache) Get(ctx context.Context, userID, spaceID, shareID string) (*Stat } // Remove removes an entry from the cache -func (c *Cache) Remove(ctx context.Context, userID, spaceID, shareID string) error { +func (c *Cache) Remove(ctx context.Context, userID utils.FilenameEncoder, spaceID, shareID string) error { ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "Grab lock") + userIDKey := userID.SafeFilename() unlock := c.lockUser(userID) span.End() - span.SetAttributes(attribute.String("cs3.userid", userID)) + span.SetAttributes(attribute.String("cs3.userid.key", userIDKey)) defer unlock() ctx, span = appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "Add") defer span.End() - span.SetAttributes(attribute.String("cs3.userid", userID), attribute.String("cs3.spaceid", spaceID)) + span.SetAttributes(attribute.String("cs3.userid.key", userIDKey), attribute.String("cs3.spaceid", spaceID)) persistFunc := func() error { - c.initializeIfNeeded(userID, spaceID) + c.initializeIfNeeded(userIDKey, spaceID) - rss, _ := c.ReceivedSpaces.Load(userID) + rss, _ := c.ReceivedSpaces.Load(userIDKey) receivedSpace := rss.Spaces[spaceID] if receivedSpace.States == nil { receivedSpace.States = map[string]*State{} @@ -215,7 +219,7 @@ func (c *Cache) Remove(ctx context.Context, userID, spaceID, shareID string) err log := appctx.GetLogger(ctx).With(). Str("hostname", os.Getenv("HOSTNAME")). - Str("userID", userID). + Str("userIDKey", userIDKey). Str("spaceID", spaceID).Logger() var err error @@ -255,11 +259,12 @@ func (c *Cache) Remove(ctx context.Context, userID, spaceID, shareID string) err // List returns a list of received shares for a given user // The return list is guaranteed to be thread-safe -func (c *Cache) List(ctx context.Context, userID string) (map[string]*Space, error) { +func (c *Cache) List(ctx context.Context, userID utils.FilenameEncoder) (map[string]*Space, error) { + userIDKey := userID.SafeFilename() ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "Grab lock") unlock := c.lockUser(userID) span.End() - span.SetAttributes(attribute.String("cs3.userid", userID)) + span.SetAttributes(attribute.String("cs3.userid.key", userIDKey)) defer unlock() err := c.syncWithLock(ctx, userID) @@ -268,7 +273,7 @@ func (c *Cache) List(ctx context.Context, userID string) (map[string]*Space, err } spaces := map[string]*Space{} - rss, _ := c.ReceivedSpaces.Load(userID) + rss, _ := c.ReceivedSpaces.Load(userIDKey) for spaceID, space := range rss.Spaces { spaceCopy := &Space{ States: map[string]*State{}, @@ -285,19 +290,20 @@ func (c *Cache) List(ctx context.Context, userID string) (map[string]*Space, err return spaces, nil } -func (c *Cache) syncWithLock(ctx context.Context, userID string) error { +func (c *Cache) syncWithLock(ctx context.Context, userID utils.FilenameEncoder) error { + userIDKey := userID.SafeFilename() ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "Sync") defer span.End() - span.SetAttributes(attribute.String("cs3.userid", userID)) + span.SetAttributes(attribute.String("cs3.userid.key", userIDKey)) - log := appctx.GetLogger(ctx).With().Str("userID", userID).Logger() + log := appctx.GetLogger(ctx).With().Str("userIDKey", userIDKey).Logger() - c.initializeIfNeeded(userID, "") + c.initializeIfNeeded(userIDKey, "") - jsonPath := userJSONPath(userID) + jsonPath := userJSONPath(userIDKey) span.AddEvent("updating cache") // - update cached list of created shares for the user in memory if changed - rss, _ := c.ReceivedSpaces.Load(userID) + rss, _ := c.ReceivedSpaces.Load(userIDKey) dlres, err := c.storage.Download(ctx, metadata.DownloadRequest{ Path: jsonPath, IfNoneMatch: []string{rss.etag}, @@ -326,18 +332,19 @@ func (c *Cache) syncWithLock(ctx context.Context, userID string) error { } newSpaces.etag = dlres.Etag - c.ReceivedSpaces.Store(userID, newSpaces) + c.ReceivedSpaces.Store(userIDKey, newSpaces) span.SetStatus(codes.Ok, "") return nil } // persist persists the data for one user to the storage -func (c *Cache) persist(ctx context.Context, userID string) error { +func (c *Cache) persist(ctx context.Context, userID utils.FilenameEncoder) error { + userIDKey := userID.SafeFilename() ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "Persist") defer span.End() - span.SetAttributes(attribute.String("cs3.userid", userID)) + span.SetAttributes(attribute.String("cs3.userid.key", userIDKey)) - rss, ok := c.ReceivedSpaces.Load(userID) + rss, ok := c.ReceivedSpaces.Load(userIDKey) if !ok { span.SetStatus(codes.Ok, "no received shares") return nil @@ -349,7 +356,7 @@ func (c *Cache) persist(ctx context.Context, userID string) error { span.SetStatus(codes.Error, err.Error()) return err } - jsonPath := userJSONPath(userID) + jsonPath := userJSONPath(userIDKey) if err := c.storage.MakeDirIfNotExist(ctx, path.Dir(jsonPath)); err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) diff --git a/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache_test.go b/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache_test.go index 93d5cab0677..fdc45638b99 100644 --- a/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache_test.go +++ b/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache_test.go @@ -23,9 +23,11 @@ import ( "os" "time" + userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1" "github.com/opencloud-eu/reva/v2/pkg/share/manager/jsoncs3/receivedsharecache" "github.com/opencloud-eu/reva/v2/pkg/storage/utils/metadata" + "github.com/opencloud-eu/reva/v2/pkg/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -36,7 +38,9 @@ var _ = Describe("Cache", func() { c receivedsharecache.Cache storage metadata.Storage - userID = "user" + userID = utils.NewFSSafeUserID(&userpb.UserId{ + OpaqueId: "userid", + }) spaceID = "spaceid" shareID = "storageid$spaceid!share1" share = &collaboration.Share{ @@ -111,7 +115,10 @@ var _ = Describe("Cache", func() { Describe("Get", func() { It("handles unknown users", func() { - s, err := c.Get(ctx, "something", spaceID, shareID) + someUserID := utils.NewFSSafeUserID(&userpb.UserId{ + OpaqueId: "something", + }) + s, err := c.Get(ctx, someUserID, spaceID, shareID) Expect(err).ToNot(HaveOccurred()) Expect(s).To(BeNil()) }) diff --git a/pkg/share/manager/jsoncs3/sharecache/sharecache.go b/pkg/share/manager/jsoncs3/sharecache/sharecache.go index d7a830594a7..c5a812dc163 100644 --- a/pkg/share/manager/jsoncs3/sharecache/sharecache.go +++ b/pkg/share/manager/jsoncs3/sharecache/sharecache.go @@ -37,6 +37,7 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/share/manager/jsoncs3/shareid" "github.com/opencloud-eu/reva/v2/pkg/storage/utils/decomposedfs/mtimesyncedcache" "github.com/opencloud-eu/reva/v2/pkg/storage/utils/metadata" + "github.com/opencloud-eu/reva/v2/pkg/utils" ) // name is the Tracer name used to identify this instrumentation library. @@ -89,15 +90,17 @@ func New(s metadata.Storage, namespace, filename string, ttl time.Duration) Cach } // Add adds a share to the cache -func (c *Cache) Add(ctx context.Context, userid, shareID string) error { +func (c *Cache) Add(ctx context.Context, id utils.FilenameEncoder, shareID string) error { + key := id.SafeFilename() + ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "Grab lock") - unlock := c.lockUser(userid) + unlock := c.lockUser(key) span.End() - span.SetAttributes(attribute.String("cs3.userid", userid)) + span.SetAttributes(attribute.String("cs3.userid", key)) defer unlock() - if _, ok := c.UserShares.Load(userid); !ok { - err := c.syncWithLock(ctx, userid) + if _, ok := c.UserShares.Load(key); !ok { + err := c.syncWithLock(ctx, key) if err != nil { return err } @@ -105,23 +108,23 @@ func (c *Cache) Add(ctx context.Context, userid, shareID string) error { ctx, span = appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "Add") defer span.End() - span.SetAttributes(attribute.String("cs3.userid", userid), attribute.String("cs3.shareid", shareID)) + span.SetAttributes(attribute.String("cs3.userid", key), attribute.String("cs3.shareid", shareID)) storageid, spaceid, _ := shareid.Decode(shareID) ssid := storageid + shareid.IDDelimiter + spaceid persistFunc := func() error { - c.initializeIfNeeded(userid, ssid) + c.initializeIfNeeded(key, ssid) // add share id - us, _ := c.UserShares.Load(userid) + us, _ := c.UserShares.Load(key) us.UserShares[ssid].IDs[shareID] = struct{}{} - return c.Persist(ctx, userid) + return c.Persist(ctx, key) } log := appctx.GetLogger(ctx).With(). Str("hostname", os.Getenv("HOSTNAME")). - Str("userID", userid). + Str("userID", key). Str("shareID", shareID).Logger() var err error @@ -149,7 +152,7 @@ func (c *Cache) Add(ctx context.Context, userid, shareID string) error { log.Error().Err(err).Msg("persisting added share failed") return err } - if err := c.syncWithLock(ctx, userid); err != nil { + if err := c.syncWithLock(ctx, key); err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) log.Error().Err(err).Msg("persisting added share failed. giving up.") @@ -160,15 +163,16 @@ func (c *Cache) Add(ctx context.Context, userid, shareID string) error { } // Remove removes a share for the given user -func (c *Cache) Remove(ctx context.Context, userid, shareID string) error { +func (c *Cache) Remove(ctx context.Context, id utils.FilenameEncoder, shareID string) error { ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "Grab lock") - unlock := c.lockUser(userid) + key := id.SafeFilename() + unlock := c.lockUser(key) span.End() - span.SetAttributes(attribute.String("cs3.userid", userid)) + span.SetAttributes(attribute.String("cs3.userid", key)) defer unlock() - if _, ok := c.UserShares.Load(userid); ok { - err := c.syncWithLock(ctx, userid) + if _, ok := c.UserShares.Load(key); ok { + err := c.syncWithLock(ctx, key) if err != nil { return err } @@ -176,13 +180,13 @@ func (c *Cache) Remove(ctx context.Context, userid, shareID string) error { ctx, span = appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "Remove") defer span.End() - span.SetAttributes(attribute.String("cs3.userid", userid), attribute.String("cs3.shareid", shareID)) + span.SetAttributes(attribute.String("cs3.userid", key), attribute.String("cs3.shareid", shareID)) storageid, spaceid, _ := shareid.Decode(shareID) ssid := storageid + shareid.IDDelimiter + spaceid persistFunc := func() error { - us, loaded := c.UserShares.LoadOrStore(userid, &UserShareCache{ + us, loaded := c.UserShares.LoadOrStore(key, &UserShareCache{ UserShares: map[string]*SpaceShareIDs{}, }) @@ -191,12 +195,12 @@ func (c *Cache) Remove(ctx context.Context, userid, shareID string) error { delete(us.UserShares[ssid].IDs, shareID) } - return c.Persist(ctx, userid) + return c.Persist(ctx, key) } log := appctx.GetLogger(ctx).With(). Str("hostname", os.Getenv("HOSTNAME")). - Str("userID", userid). + Str("userID", key). Str("shareID", shareID).Logger() var err error @@ -224,7 +228,7 @@ func (c *Cache) Remove(ctx context.Context, userid, shareID string) error { log.Error().Err(err).Msg("persisting removed share failed") return err } - if err := c.syncWithLock(ctx, userid); err != nil { + if err := c.syncWithLock(ctx, key); err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) return err @@ -235,18 +239,19 @@ func (c *Cache) Remove(ctx context.Context, userid, shareID string) error { } // List return the list of spaces/shares for the given user/group -func (c *Cache) List(ctx context.Context, userid string) (map[string]SpaceShareIDs, error) { +func (c *Cache) List(ctx context.Context, id utils.FilenameEncoder) (map[string]SpaceShareIDs, error) { ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "Grab lock") - unlock := c.lockUser(userid) + key := id.SafeFilename() + unlock := c.lockUser(key) span.End() - span.SetAttributes(attribute.String("cs3.userid", userid)) + span.SetAttributes(attribute.String("cs3.userid", key)) defer unlock() - if err := c.syncWithLock(ctx, userid); err != nil { + if err := c.syncWithLock(ctx, key); err != nil { return nil, err } r := map[string]SpaceShareIDs{} - us, ok := c.UserShares.Load(userid) + us, ok := c.UserShares.Load(key) if !ok { return r, nil } @@ -309,12 +314,12 @@ func (c *Cache) syncWithLock(ctx context.Context, userID string) error { } // Persist persists the data for one user/group to the storage -func (c *Cache) Persist(ctx context.Context, userid string) error { +func (c *Cache) Persist(ctx context.Context, key string) error { ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "Persist") defer span.End() - span.SetAttributes(attribute.String("cs3.userid", userid)) + span.SetAttributes(attribute.String("cs3.userid", key)) - us, ok := c.UserShares.Load(userid) + us, ok := c.UserShares.Load(key) if !ok { span.SetStatus(codes.Ok, "no user shares") return nil @@ -325,7 +330,7 @@ func (c *Cache) Persist(ctx context.Context, userid string) error { span.SetStatus(codes.Error, err.Error()) return err } - jsonPath := c.userCreatedPath(userid) + jsonPath := c.userCreatedPath(key) if err := c.storage.MakeDirIfNotExist(ctx, path.Dir(jsonPath)); err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) @@ -355,8 +360,8 @@ func (c *Cache) Persist(ctx context.Context, userid string) error { return nil } -func (c *Cache) userCreatedPath(userid string) string { - return filepath.Join("/", c.namespace, userid, c.filename) +func (c *Cache) userCreatedPath(key string) string { + return filepath.Join("/", c.namespace, key, c.filename) } func (c *Cache) initializeIfNeeded(userid, ssid string) { diff --git a/pkg/share/manager/jsoncs3/sharecache/sharecache_test.go b/pkg/share/manager/jsoncs3/sharecache/sharecache_test.go index 342339a47d3..6fa457a2bd9 100644 --- a/pkg/share/manager/jsoncs3/sharecache/sharecache_test.go +++ b/pkg/share/manager/jsoncs3/sharecache/sharecache_test.go @@ -26,8 +26,10 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" "github.com/opencloud-eu/reva/v2/pkg/share/manager/jsoncs3/sharecache" "github.com/opencloud-eu/reva/v2/pkg/storage/utils/metadata" + "github.com/opencloud-eu/reva/v2/pkg/utils" ) var _ = Describe("Sharecache", func() { @@ -35,7 +37,9 @@ var _ = Describe("Sharecache", func() { c sharecache.Cache storage metadata.Storage - userid = "user" + userid = utils.NewFSSafeUserID(&userpb.UserId{ + OpaqueId: "userid", + }) shareID = "storageid$spaceid!share1" ctx context.Context tmpdir string @@ -71,13 +75,13 @@ var _ = Describe("Sharecache", func() { }) It("updates the etag", func() { - uc, _ := c.UserShares.Load(userid) + uc, _ := c.UserShares.Load(userid.SafeFilename()) oldEtag := uc.Etag Expect(oldEtag).ToNot(BeEmpty()) - Expect(c.Persist(ctx, userid)).To(Succeed()) + Expect(c.Persist(ctx, userid.SafeFilename())).To(Succeed()) - uc, _ = c.UserShares.Load(userid) + uc, _ = c.UserShares.Load(userid.SafeFilename()) Expect(uc.Etag).ToNot(Equal(oldEtag)) }) }) diff --git a/pkg/storage/pkg/decomposedfs/decomposedfs.go b/pkg/storage/pkg/decomposedfs/decomposedfs.go index ccd7f05d34e..9ae5f281556 100644 --- a/pkg/storage/pkg/decomposedfs/decomposedfs.go +++ b/pkg/storage/pkg/decomposedfs/decomposedfs.go @@ -130,6 +130,7 @@ type Decomposedfs struct { UserCache *ttlcache.Cache userSpaceIndex *spaceidindex.Index groupSpaceIndex *spaceidindex.Index + mailSpaceIndex *spaceidindex.Index spaceTypeIndex *spaceidindex.Index log *zerolog.Logger @@ -218,6 +219,11 @@ func New(o *options.Options, aspects aspects.Aspects, log *zerolog.Logger) (stor if err != nil { return nil, err } + mailSpaceIndex := spaceidindex.New(filepath.Join(o.Root, lookup.IndexesDir), "by-mail") + err = mailSpaceIndex.Init() + if err != nil { + return nil, err + } spaceTypeIndex := spaceidindex.New(filepath.Join(o.Root, lookup.IndexesDir), "by-type") err = spaceTypeIndex.Init() if err != nil { @@ -244,6 +250,7 @@ func New(o *options.Options, aspects aspects.Aspects, log *zerolog.Logger) (stor UserCache: ttlcache.NewCache(), userSpaceIndex: userSpaceIndex, groupSpaceIndex: groupSpaceIndex, + mailSpaceIndex: mailSpaceIndex, spaceTypeIndex: spaceTypeIndex, log: log, } diff --git a/pkg/storage/pkg/decomposedfs/grants.go b/pkg/storage/pkg/decomposedfs/grants.go index 94cc53c355e..4a1eb82ed86 100644 --- a/pkg/storage/pkg/decomposedfs/grants.go +++ b/pkg/storage/pkg/decomposedfs/grants.go @@ -23,6 +23,7 @@ import ( "path/filepath" "strings" + userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/opencloud-eu/reva/v2/internal/grpc/services/storageprovider" "github.com/opencloud-eu/reva/v2/pkg/appctx" @@ -234,9 +235,15 @@ func (fs *Decomposedfs) RemoveGrant(ctx context.Context, ref *provider.Reference // invalidate space grant switch g.Grantee.Type { case provider.GranteeType_GRANTEE_TYPE_USER: - // remove from user index - if err := fs.userSpaceIndex.Remove(g.Grantee.GetUserId().GetOpaqueId(), grantNode.SpaceID); err != nil { - return err + filename := utils.NewFSSafeUserID(g.Grantee.GetUserId()).SafeFilename() + if g.Grantee.GetUserId().GetType() == userpb.UserType_USER_TYPE_GUEST { + if err := fs.mailSpaceIndex.Remove(filename, grantNode.SpaceID); err != nil { + return err + } + } else { + if err := fs.userSpaceIndex.Remove(filename, grantNode.SpaceID); err != nil { + return err + } } case provider.GranteeType_GRANTEE_TYPE_GROUP: // remove from group index @@ -320,7 +327,7 @@ func (fs *Decomposedfs) loadGrant(ctx context.Context, ref *provider.Reference, for _, grant := range grants { switch grant.Grantee.GetType() { case provider.GranteeType_GRANTEE_TYPE_USER: - if g.Grantee.GetUserId().GetOpaqueId() == grant.Grantee.GetUserId().GetOpaqueId() { + if utils.UserIDEqual(g.Grantee.GetUserId(), grant.Grantee.GetUserId()) { return n, unlockFunc, grant, nil } case provider.GranteeType_GRANTEE_TYPE_GROUP: diff --git a/pkg/storage/pkg/decomposedfs/grants_test.go b/pkg/storage/pkg/decomposedfs/grants_test.go index d3b32909759..6cf2fa2926f 100644 --- a/pkg/storage/pkg/decomposedfs/grants_test.go +++ b/pkg/storage/pkg/decomposedfs/grants_test.go @@ -22,6 +22,7 @@ import ( "fmt" "os" "path/filepath" + "strings" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" @@ -30,24 +31,30 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/storage/fs/posix/lookup" "github.com/opencloud-eu/reva/v2/pkg/storage/pkg/decomposedfs/metadata/prefixes" helpers "github.com/opencloud-eu/reva/v2/pkg/storage/pkg/decomposedfs/testhelpers" + "github.com/opencloud-eu/reva/v2/pkg/utils" "github.com/stretchr/testify/mock" ) var _ = Describe("Grants", func() { var ( - env *helpers.DecomposedTestEnv - ref *provider.Reference - grant *provider.Grant + env *helpers.DecomposedTestEnv + ref *provider.Reference + grant *provider.Grant + userid *userpb.UserId ) BeforeEach(func() { + userid = &userpb.UserId{ + OpaqueId: "4c510ada-c86b-4815-8820-42cdf82c3d51", + } + }) + + JustBeforeEach(func() { grant = &provider.Grant{ Grantee: &provider.Grantee{ Type: provider.GranteeType_GRANTEE_TYPE_USER, Id: &provider.Grantee_UserId{ - UserId: &userpb.UserId{ - OpaqueId: "4c510ada-c86b-4815-8820-42cdf82c3d51", - }, + UserId: userid, }, }, Permissions: &provider.ResourcePermissions{ @@ -60,9 +67,7 @@ var _ = Describe("Grants", func() { OpaqueId: helpers.OwnerID, }, } - }) - JustBeforeEach(func() { var err error env, err = helpers.NewTestEnv(nil) Expect(err).ToNot(HaveOccurred()) @@ -181,5 +186,41 @@ var _ = Describe("Grants", func() { Expect(len(grants)).To(Equal(0)) }) }) + + Context("with guest grants", func() { + BeforeEach(func() { + userid = &userpb.UserId{ + OpaqueId: "Foo@Example.COM", + Type: userpb.UserType_USER_TYPE_GUEST, + } + }) + + It("adds, lists and removes the guest grant", func() { + err := env.Fs.AddGrant(env.Ctx, ref, grant) + Expect(err).ToNot(HaveOccurred()) + + n, err := env.Lookup.NodeFromResource(env.Ctx, ref) + Expect(err).ToNot(HaveOccurred()) + principal := prefixes.GrantMailAcePrefix + utils.NewFSSafeUserID(grant.Grantee.GetUserId()).SafeFilename() + _, err = n.XattrString(env.Ctx, principal) + Expect(err).ToNot(HaveOccurred()) + + grants, err := env.Fs.ListGrants(env.Ctx, ref) + Expect(err).ToNot(HaveOccurred()) + Expect(len(grants)).To(Equal(1)) + + g := grants[0] + Expect(g.Grantee.GetUserId().OpaqueId).To(Equal(strings.ToLower(grant.Grantee.GetUserId().OpaqueId))) + Expect(g.Grantee.GetUserId().Type).To(Equal(userpb.UserType_USER_TYPE_GUEST)) + + // the mixed-case request must match the lowercased persisted grant + err = env.Fs.RemoveGrant(env.Ctx, ref, grant) + Expect(err).ToNot(HaveOccurred()) + + grants, err = env.Fs.ListGrants(env.Ctx, ref) + Expect(err).ToNot(HaveOccurred()) + Expect(len(grants)).To(Equal(0)) + }) + }) }) }) diff --git a/pkg/storage/pkg/decomposedfs/metadata/prefixes/prefixes.go b/pkg/storage/pkg/decomposedfs/metadata/prefixes/prefixes.go index 0c364973736..45851fa794e 100644 --- a/pkg/storage/pkg/decomposedfs/metadata/prefixes/prefixes.go +++ b/pkg/storage/pkg/decomposedfs/metadata/prefixes/prefixes.go @@ -20,6 +20,7 @@ package prefixes import ( userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + "github.com/opencloud-eu/reva/v2/pkg/storage/utils/ace" ) // Declare a list of xattr keys @@ -53,8 +54,9 @@ const ( // grantPrefix is the prefix for sharing related extended attributes GrantPrefix string = OcPrefix + "grant." - GrantUserAcePrefix string = OcPrefix + "grant." + UserAcePrefix - GrantGroupAcePrefix string = OcPrefix + "grant." + GroupAcePrefix + GrantUserAcePrefix string = OcPrefix + "grant." + ace.UserAcePrefix + GrantGroupAcePrefix string = OcPrefix + "grant." + ace.GroupAcePrefix + GrantMailAcePrefix string = OcPrefix + "grant." + ace.MailAcePrefix MetadataPrefix string = OcPrefix + "md." // favorite flag, per user @@ -103,9 +105,6 @@ const ( SpaceAliasAttr string = OcPrefix + "space.alias" SpaceTenantIDAttr string = OcPrefix + "space.tenantid" SpaceContentTypeAttr string = OcPrefix + "space.contenttype" - - UserAcePrefix string = "u:" - GroupAcePrefix string = "g:" ) func FavoriteKey(uid *userpb.UserId) string { diff --git a/pkg/storage/pkg/decomposedfs/node/node.go b/pkg/storage/pkg/decomposedfs/node/node.go index dc03ad6b85c..33516a6a649 100644 --- a/pkg/storage/pkg/decomposedfs/node/node.go +++ b/pkg/storage/pkg/decomposedfs/node/node.go @@ -1238,7 +1238,7 @@ func (n *Node) IsDenied(ctx context.Context) bool { isExecutant := func(g *provider.Grantee) bool { switch g.GetType() { case provider.GranteeType_GRANTEE_TYPE_USER: - return g.GetUserId().GetOpaqueId() == u.GetId().GetOpaqueId() + return utils.UserIDEqual(g.GetUserId(), u.GetId()) case provider.GranteeType_GRANTEE_TYPE_GROUP: // check gid gid := g.GetGroupId().GetOpaqueId() @@ -1300,14 +1300,19 @@ func (n *Node) ReadGrant(ctx context.Context, grantee string) (g *provider.Grant return e.Grant(), nil } -// ReadGrant reads a CS3 grant +// DeleteGrant reads a CS3 grant func (n *Node) DeleteGrant(ctx context.Context, g *provider.Grant) (err error) { var attr string - if g.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_GROUP { + switch g.Grantee.Type { + case provider.GranteeType_GRANTEE_TYPE_GROUP: attr = prefixes.GrantGroupAcePrefix + g.Grantee.GetGroupId().OpaqueId - } else { - attr = prefixes.GrantUserAcePrefix + g.Grantee.GetUserId().OpaqueId + case provider.GranteeType_GRANTEE_TYPE_USER: + attr = prefixes.GrantPrefix + ace.UserAce(g.Grantee.GetUserId()) + default: + // Note: We shouldn't actually get here as this is already caught in the upper + // layers (e.g. the storage provider) + return errors.New("grantee type is invalid") } if err = n.RemoveXattr(ctx, attr); err != nil { @@ -1353,7 +1358,7 @@ func (n *Node) getGranteeTypes(ctx context.Context) []provider.GranteeType { hasUserShares, hasGroupShares := false, false for i := range g { switch { - case !hasUserShares && strings.HasPrefix(g[i], prefixes.GrantUserAcePrefix): + case !hasUserShares && (strings.HasPrefix(g[i], prefixes.GrantUserAcePrefix) || strings.HasPrefix(g[i], prefixes.GrantMailAcePrefix)): hasUserShares = true case !hasGroupShares && strings.HasPrefix(g[i], prefixes.GrantGroupAcePrefix): hasGroupShares = true diff --git a/pkg/storage/pkg/decomposedfs/spaces.go b/pkg/storage/pkg/decomposedfs/spaces.go index 00dd5e52460..2421c3c0742 100644 --- a/pkg/storage/pkg/decomposedfs/spaces.go +++ b/pkg/storage/pkg/decomposedfs/spaces.go @@ -345,7 +345,8 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide var err error if requestedUserID != nil { - allMatches, err = fs.userSpaceIndex.Load(requestedUserID.GetOpaqueId()) + filename := utils.NewFSSafeUserID(requestedUserID).SafeFilename() + allMatches, err = fs.userSpaceIndex.Load(filename) // do not return an error if the user has no spaces if err != nil && !os.IsNotExist(err) { return nil, errors.Wrap(err, "error reading user index") @@ -357,6 +358,21 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide matches[allMatches[entry]] = allMatches[entry] } + // guests are granted shares via their mail address, search the mail index as well + if requestedUserID.GetType() == userv1beta1.UserType_USER_TYPE_GUEST { + allMatches, err = fs.mailSpaceIndex.Load(filename) + // do not return an error if the guest has no spaces + if err != nil && !os.IsNotExist(err) { + return nil, errors.Wrap(err, "error reading mail index") + } + + if entry == spaceIDAny { + maps.Copy(matches, allMatches) + } else { + matches[allMatches[entry]] = allMatches[entry] + } + } + // get Groups for userid user := ctxpkg.ContextMustGetUser(ctx) // TODO the user from context may not have groups populated @@ -788,9 +804,17 @@ func (fs *Decomposedfs) DeleteStorageSpace(ctx context.Context, req *provider.De for _, g := range grants { switch g.Grantee.Type { case provider.GranteeType_GRANTEE_TYPE_USER: - // remove from user index - if err := fs.userSpaceIndex.Remove(g.Grantee.GetUserId().GetOpaqueId(), spaceID); err != nil { - sublog.Error().Err(err).Str("grantee", g.Grantee.GetUserId().GetOpaqueId()).Msg("could not remove user from user index") + filename := utils.NewFSSafeUserID(g.Grantee.GetUserId()).SafeFilename() + if g.Grantee.GetUserId().GetType() == userv1beta1.UserType_USER_TYPE_GUEST { + // remove from mail index + if err := fs.mailSpaceIndex.Remove(filename, spaceID); err != nil { + sublog.Error().Err(err).Str("grantee", g.Grantee.GetUserId().GetOpaqueId()).Msg("could not remove guest from mail index") + } + } else { + // remove from user index + if err := fs.userSpaceIndex.Remove(filename, spaceID); err != nil { + sublog.Error().Err(err).Str("grantee", g.Grantee.GetUserId().GetOpaqueId()).Msg("could not remove user from user index") + } } case provider.GranteeType_GRANTEE_TYPE_GROUP: // remove from group index @@ -894,7 +918,11 @@ func (fs *Decomposedfs) updateIndexes(ctx context.Context, grantee *provider.Gra // create space grant index switch grantee.Type { case provider.GranteeType_GRANTEE_TYPE_USER: - return fs.linkSpaceByUser(ctx, grantee.GetUserId().GetOpaqueId(), spaceID, target) + filename := utils.NewFSSafeUserID(grantee.GetUserId()).SafeFilename() + if grantee.GetUserId().GetType() == userv1beta1.UserType_USER_TYPE_GUEST { + return fs.linkSpaceByMail(ctx, filename, spaceID, target) + } + return fs.linkSpaceByUser(ctx, filename, spaceID, target) case provider.GranteeType_GRANTEE_TYPE_GROUP: return fs.linkSpaceByGroup(ctx, grantee.GetGroupId().GetOpaqueId(), spaceID, target) default: @@ -910,6 +938,10 @@ func (fs *Decomposedfs) linkSpaceByGroup(ctx context.Context, groupID, spaceID, return fs.groupSpaceIndex.Add(groupID, spaceID, target) } +func (fs *Decomposedfs) linkSpaceByMail(ctx context.Context, mail, spaceID, target string) error { + return fs.mailSpaceIndex.Add(mail, spaceID, target) +} + func (fs *Decomposedfs) linkStorageSpaceType(ctx context.Context, spaceType, spaceID, target string) error { return fs.spaceTypeIndex.Add(spaceType, spaceID, target) } @@ -983,10 +1015,19 @@ func (fs *Decomposedfs) StorageSpaceFromNode(ctx context.Context, n *node.Node, // invalidate space grant switch g.Grantee.Type { case provider.GranteeType_GRANTEE_TYPE_USER: - // remove from user index - if err := fs.userSpaceIndex.Remove(g.Grantee.GetUserId().GetOpaqueId(), n.GetSpaceID()); err != nil { - sublog.Error().Err(err).Str("grantee", id). - Msg("failed to delete expired user space index") + filename := utils.NewFSSafeUserID(g.Grantee.GetUserId()).SafeFilename() + if g.Grantee.GetUserId().GetType() == userv1beta1.UserType_USER_TYPE_GUEST { + // remove from mail index + if err := fs.mailSpaceIndex.Remove(filename, n.GetSpaceID()); err != nil { + sublog.Error().Err(err).Str("grantee", id). + Msg("failed to delete expired mail space index") + } + } else { + // remove from user index + if err := fs.userSpaceIndex.Remove(filename, n.GetSpaceID()); err != nil { + sublog.Error().Err(err).Str("grantee", id). + Msg("failed to delete expired user space index") + } } case provider.GranteeType_GRANTEE_TYPE_GROUP: // remove from group index diff --git a/pkg/storage/pkg/decomposedfs/spaces_test.go b/pkg/storage/pkg/decomposedfs/spaces_test.go index 799b3bb6063..0e4971f841d 100644 --- a/pkg/storage/pkg/decomposedfs/spaces_test.go +++ b/pkg/storage/pkg/decomposedfs/spaces_test.go @@ -30,11 +30,13 @@ import ( typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/opencloud-eu/reva/v2/internal/grpc/services/storageprovider" ctxpkg "github.com/opencloud-eu/reva/v2/pkg/ctx" "github.com/opencloud-eu/reva/v2/pkg/storage/pkg/decomposedfs/node" "github.com/opencloud-eu/reva/v2/pkg/storage/pkg/decomposedfs/spaceidindex" helpers "github.com/opencloud-eu/reva/v2/pkg/storage/pkg/decomposedfs/testhelpers" "github.com/opencloud-eu/reva/v2/pkg/storagespace" + "github.com/opencloud-eu/reva/v2/pkg/utils" "github.com/stretchr/testify/mock" "google.golang.org/grpc" ) @@ -274,6 +276,7 @@ var _ = Describe("Spaces", func() { userGrantee := helpers.User0ID groupGrantee := "11111111-2222-3333-4444-555555555555" + guestGrantee := "Guest@Example.COM" Expect(env.Fs.AddGrant(env.Ctx, ref, &provider.Grant{ Grantee: &provider.Grantee{Type: provider.GranteeType_GRANTEE_TYPE_USER, Id: &provider.Grantee_UserId{UserId: &userv1beta1.UserId{OpaqueId: userGrantee}}}, Permissions: &provider.ResourcePermissions{Stat: true}, @@ -284,6 +287,11 @@ var _ = Describe("Spaces", func() { Permissions: &provider.ResourcePermissions{Stat: true}, Creator: &userv1beta1.UserId{OpaqueId: helpers.OwnerID}, })).To(Succeed()) + Expect(env.Fs.AddGrant(env.Ctx, ref, &provider.Grant{ + Grantee: &provider.Grantee{Type: provider.GranteeType_GRANTEE_TYPE_USER, Id: &provider.Grantee_UserId{UserId: &userv1beta1.UserId{OpaqueId: guestGrantee, Type: userv1beta1.UserType_USER_TYPE_GUEST}}}, + Permissions: &provider.ResourcePermissions{Stat: true}, + Creator: &userv1beta1.UserId{OpaqueId: helpers.OwnerID}, + })).To(Succeed()) // seed the grantee space-index entries. The real share flow writes // these via a space-typed context; here we add them directly so the @@ -295,6 +303,10 @@ var _ = Describe("Spaces", func() { groupIdx := spaceidindex.New(filepath.Join(env.Root, "indexes"), "by-group-id") Expect(groupIdx.Init()).To(Succeed()) Expect(groupIdx.Add(groupGrantee, spaceID, target)).To(Succeed()) + mailIdx := spaceidindex.New(filepath.Join(env.Root, "indexes"), "by-mail") + Expect(mailIdx.Init()).To(Succeed()) + guestFilename := utils.NewFSSafeUserID(&userv1beta1.UserId{OpaqueId: guestGrantee, Type: userv1beta1.UserType_USER_TYPE_GUEST}).SafeFilename() + Expect(mailIdx.Add(guestFilename, spaceID, target)).To(Succeed()) load := func(indexName, key string) map[string]string { idx := spaceidindex.New(filepath.Join(env.Root, "indexes"), indexName) @@ -304,9 +316,10 @@ var _ = Describe("Spaces", func() { return m } - // precondition: both grantee indexes reference the space + // precondition: the grantee indexes reference the space Expect(load("by-user-id", userGrantee)).To(HaveKey(spaceID)) Expect(load("by-group-id", groupGrantee)).To(HaveKey(spaceID)) + Expect(load("by-mail", guestFilename)).To(HaveKey(spaceID)) // disable, then purge as the space admin ctx := ctxpkg.ContextSetUser(context.Background(), env.DeleteAllSpacesUser) @@ -319,6 +332,7 @@ var _ = Describe("Spaces", func() { // the grantee index entries must be gone after the purge Expect(load("by-user-id", userGrantee)).ToNot(HaveKey(spaceID)) Expect(load("by-group-id", groupGrantee)).ToNot(HaveKey(spaceID)) + Expect(load("by-mail", guestFilename)).ToNot(HaveKey(spaceID)) }) // purging one space must remove only that space's entry: a sibling // space of the same owner must stay in the by-user-id index. @@ -636,4 +650,86 @@ var _ = Describe("Spaces", func() { ), ) }) + + Describe("List storage spaces for guests", func() { + var ( + env *helpers.DecomposedTestEnv + ) + + BeforeEach(func() { + var err error + env, err = helpers.NewTestEnv(nil) + Expect(err).ToNot(HaveOccurred()) + env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Unset() + env.Permissions.On("AssemblePermissions", mock.Anything, mock.Anything).Return(&provider.ResourcePermissions{ + Stat: true, + AddGrant: true, + GetQuota: true, + }, nil) + env.PermissionsClient.On("CheckPermission", mock.Anything, mock.Anything, mock.Anything).Return(&cs3permissions.CheckPermissionResponse{Status: &rpcv1beta1.Status{Code: rpcv1beta1.Code_CODE_OK}}, nil) + }) + + AfterEach(func() { + if env != nil { + env.Cleanup() + } + }) + + It("finds project spaces granted to a guest via the by-mail index", func() { + // create a project space as the owner + resp, err := env.Fs.CreateStorageSpace(env.Ctx, &provider.CreateStorageSpaceRequest{Name: "Guest Space", Type: "project"}) + Expect(err).ToNot(HaveOccurred()) + ref := &provider.Reference{ResourceId: resp.StorageSpace.GetRoot()} + + // grant the space to a guest. A space-typed context makes the storage + // link the space in the by-mail index, like the real share flow. + guest := &userv1beta1.UserId{OpaqueId: "guest@example.com", Type: userv1beta1.UserType_USER_TYPE_GUEST} + ctx := storageprovider.WithSpaceType(ctxpkg.ContextSetUser(context.Background(), env.Users[0]), "project") + Expect(env.Fs.AddGrant(ctx, ref, &provider.Grant{ + Grantee: &provider.Grantee{Type: provider.GranteeType_GRANTEE_TYPE_USER, Id: &provider.Grantee_UserId{UserId: guest}}, + Permissions: &provider.ResourcePermissions{Stat: true}, + Creator: &userv1beta1.UserId{OpaqueId: helpers.OwnerID}, + })).To(Succeed()) + + // the guest can discover the granted space when listing their spaces + spaces, err := env.Fs.ListStorageSpaces(ctxpkg.ContextSetUser(context.Background(), &userv1beta1.User{Id: guest}), []*provider.ListStorageSpacesRequest_Filter{ + {Type: provider.ListStorageSpacesRequest_Filter_TYPE_USER, Term: &provider.ListStorageSpacesRequest_Filter_User{User: guest}}, + }, false) + Expect(err).ToNot(HaveOccurred()) + Expect(len(spaces)).To(Equal(1)) + Expect(spaces[0].SpaceType).To(Equal("project")) + Expect(spaces[0].GetId().GetOpaqueId()).To(Equal(resp.StorageSpace.GetId().GetOpaqueId())) + }) + + It("safely indexes a project space granted to a guest with a path-like ID", func() { + // create a project space as the owner + resp, err := env.Fs.CreateStorageSpace(env.Ctx, &provider.CreateStorageSpaceRequest{Name: "Guest Space", Type: "project"}) + Expect(err).ToNot(HaveOccurred()) + ref := &provider.Reference{ResourceId: resp.StorageSpace.GetRoot()} + + ctx := storageprovider.WithSpaceType(ctxpkg.ContextSetUser(context.Background(), env.Users[0]), "project") + guest := &userv1beta1.UserId{OpaqueId: "my/../email@email.com", Type: userv1beta1.UserType_USER_TYPE_GUEST} + err = env.Fs.AddGrant(ctx, ref, &provider.Grant{ + Grantee: &provider.Grantee{Type: provider.GranteeType_GRANTEE_TYPE_USER, Id: &provider.Grantee_UserId{UserId: guest}}, + Permissions: &provider.ResourcePermissions{Stat: true}, + Creator: &userv1beta1.UserId{OpaqueId: helpers.OwnerID}, + }) + Expect(err).NotTo(HaveOccurred()) + + mailIdx := spaceidindex.New(filepath.Join(env.Root, "indexes"), "by-mail") + Expect(mailIdx.Init()).To(Succeed()) + filename := utils.NewFSSafeUserID(guest).SafeFilename() + index, err := mailIdx.Load(filename) + Expect(err).NotTo(HaveOccurred()) + _, spaceID, _, err := storagespace.SplitID(resp.StorageSpace.GetId().GetOpaqueId()) + Expect(err).NotTo(HaveOccurred()) + Expect(index).To(HaveKey(spaceID)) + + spaces, err := env.Fs.ListStorageSpaces(ctxpkg.ContextSetUser(context.Background(), &userv1beta1.User{Id: guest}), []*provider.ListStorageSpacesRequest_Filter{ + {Type: provider.ListStorageSpacesRequest_Filter_TYPE_USER, Term: &provider.ListStorageSpacesRequest_Filter_User{User: guest}}, + }, false) + Expect(err).NotTo(HaveOccurred()) + Expect(spaces).To(HaveLen(1)) + }) + }) }) diff --git a/pkg/storage/utils/ace/ace.go b/pkg/storage/utils/ace/ace.go index ef3da2ee29e..96f4fa180e2 100644 --- a/pkg/storage/utils/ace/ace.go +++ b/pkg/storage/utils/ace/ace.go @@ -31,6 +31,13 @@ import ( provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/opencloud-eu/reva/v2/pkg/storage/utils/grants" + "github.com/opencloud-eu/reva/v2/pkg/utils" +) + +const ( + UserAcePrefix = "u:" + GroupAcePrefix = "g:" + MailAcePrefix = "m:" ) /* @@ -202,7 +209,7 @@ func FromGrant(g *provider.Grant) *ACE { } if g.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_GROUP { e.flags = "g" - e.principal = "g:" + g.Grantee.GetGroupId().OpaqueId + e.principal = GroupAcePrefix + g.Grantee.GetGroupId().OpaqueId } else { e.principal = UserAce(g.Grantee.GetUserId()) } @@ -215,7 +222,13 @@ func FromGrant(g *provider.Grant) *ACE { } func UserAce(id *userpb.UserId) string { - return "u:" + id.OpaqueId + filename := utils.NewFSSafeUserID(id).SafeFilename() + switch id.GetType() { + case userpb.UserType_USER_TYPE_GUEST: + return MailAcePrefix + filename + default: + return UserAcePrefix + filename + } } // Principal returns the principal of the ACE, eg. `u:` or `g:` @@ -256,13 +269,19 @@ func Unmarshal(principal string, v []byte) (e *ACE, err error) { } // check consistency of Flags and principal type if strings.Contains(e.flags, "g") { - if principal[:1] != "g" { + if !strings.HasPrefix(principal, GroupAcePrefix) { return nil, fmt.Errorf("inconsistent ace: expected group") } } else { - if principal[:1] != "u" { + if !strings.HasPrefix(principal, UserAcePrefix) && !strings.HasPrefix(principal, MailAcePrefix) { return nil, fmt.Errorf("inconsistent ace: expected user") } + if strings.HasPrefix(principal, MailAcePrefix) { + id := &userpb.UserId{Type: userpb.UserType_USER_TYPE_GUEST} + if _, err := utils.NewFSSafeUserID(id).Decode(strings.TrimPrefix(principal, MailAcePrefix)); err != nil { + return nil, fmt.Errorf("invalid guest ace principal: %w", err) + } + } } default: return nil, fmt.Errorf("unknown ace encoding") @@ -288,7 +307,14 @@ func (e *ACE) Grant() *provider.Grant { if e.granteeType() == provider.GranteeType_GRANTEE_TYPE_GROUP { g.Grantee.Id = &provider.Grantee_GroupId{GroupId: &grouppb.GroupId{OpaqueId: id}} } else if e.granteeType() == provider.GranteeType_GRANTEE_TYPE_USER { - g.Grantee.Id = &provider.Grantee_UserId{UserId: &userpb.UserId{OpaqueId: id}} + if strings.HasPrefix(e.principal, MailAcePrefix) { + // Guest principals are validated by Unmarshal or encoded by FromGrant, + // so decoding cannot fail for a valid ACE. + userID, _ := utils.NewFSSafeUserID(&userpb.UserId{Type: userpb.UserType_USER_TYPE_GUEST}).Decode(id) + g.Grantee.Id = &provider.Grantee_UserId{UserId: userID} + } else { + g.Grantee.Id = &provider.Grantee_UserId{UserId: &userpb.UserId{OpaqueId: id, Type: userpb.UserType_USER_TYPE_PRIMARY}} + } } if e.expires != 0 { diff --git a/pkg/storage/utils/ace/ace_test.go b/pkg/storage/utils/ace/ace_test.go index a98ae5aa106..89f5cac5c80 100644 --- a/pkg/storage/utils/ace/ace_test.go +++ b/pkg/storage/utils/ace/ace_test.go @@ -25,6 +25,7 @@ import ( userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/opencloud-eu/reva/v2/pkg/storage/utils/ace" + "github.com/opencloud-eu/reva/v2/pkg/utils" "google.golang.org/protobuf/testing/protocmp" . "github.com/onsi/ginkgo/v2" @@ -34,12 +35,19 @@ import ( var _ = Describe("ACE", func() { var ( + userGrant *provider.Grant + groupGrant *provider.Grant + guestGrant *provider.Grant + ) + + BeforeEach(func() { userGrant = &provider.Grant{ Grantee: &provider.Grantee{ Type: provider.GranteeType_GRANTEE_TYPE_USER, Id: &provider.Grantee_UserId{ UserId: &userpb.UserId{ OpaqueId: "foo", + Type: userpb.UserType_USER_TYPE_PRIMARY, }, }, }, @@ -63,18 +71,47 @@ var _ = Describe("ACE", func() { Permissions: &provider.ResourcePermissions{ CreateContainer: true, }, + Creator: &userpb.UserId{}, } - ) + + guestGrant = &provider.Grant{ + Grantee: &provider.Grantee{ + Type: provider.GranteeType_GRANTEE_TYPE_USER, + Id: &provider.Grantee_UserId{ + UserId: &userpb.UserId{ + OpaqueId: "GUEST@example.com", + Type: userpb.UserType_USER_TYPE_GUEST, + }, + }, + }, + Permissions: &provider.ResourcePermissions{ + CreateContainer: true, + }, + Creator: &userpb.UserId{}, + } + }) Describe("FromGrant", func() { It("creates an ACE from a user grant", func() { - ace := ace.FromGrant(userGrant) - Expect(ace.Principal()).To(Equal("u:foo")) + aceValue := ace.FromGrant(userGrant) + Expect(aceValue.Principal()).To(Equal(ace.UserAcePrefix + "foo")) }) It("creates an ACE from a group grant", func() { - ace := ace.FromGrant(groupGrant) - Expect(ace.Principal()).To(Equal("g:foo")) + aceValue := ace.FromGrant(groupGrant) + Expect(aceValue.Principal()).To(Equal(ace.GroupAcePrefix + "foo")) + }) + + It("creates an ACE from a guest grant", func() { + userid := &userpb.UserId{ + OpaqueId: "GUEST@example.com", + Type: userpb.UserType_USER_TYPE_GUEST, + } + guestGrant.Grantee.Id = &provider.Grantee_UserId{ + UserId: userid, + } + aceValue := ace.FromGrant(guestGrant) + Expect(aceValue.Principal()).To(Equal(ace.MailAcePrefix + utils.NewFSSafeUserID(userid).SafeFilename())) }) }) @@ -86,6 +123,36 @@ var _ = Describe("ACE", func() { grant.Grantee.Opaque = nil Expect(grant).To(BeComparableTo(userGrant, protocmp.Transform())) }) + + It("returns a proper guest Grant with lowercased OpaqueId", func() { + ace := ace.FromGrant(guestGrant) + grant := ace.Grant() + Expect(grant.GetGrantee().GetUserId().GetOpaqueId()).To(Equal("guest@example.com")) + Expect(grant.GetGrantee().GetUserId().GetType()).To(Equal(userpb.UserType_USER_TYPE_GUEST)) + Expect(grant.GetPermissions()).To(BeComparableTo(guestGrant.GetPermissions(), protocmp.Transform())) + }) + + It("returns a proper Grant for group ACE", func() { + ace := ace.FromGrant(groupGrant) + grant := ace.Grant() + // do not check opaque values + grant.Grantee.Opaque = nil + Expect(grant).To(BeComparableTo(groupGrant, protocmp.Transform())) + }) + + It("returns a proper Grant for guest ACE", func() { + guestGrant.Grantee.Id = &provider.Grantee_UserId{ + UserId: &userpb.UserId{ + OpaqueId: "guest@example.com", + Type: userpb.UserType_USER_TYPE_GUEST, + }, + } + ace := ace.FromGrant(guestGrant) + grant := ace.Grant() + // do not check opaque values + grant.Grantee.Opaque = nil + Expect(grant).To(BeComparableTo(guestGrant, protocmp.Transform())) + }) }) Describe("marshalling", func() { diff --git a/pkg/storage/utils/decomposedfs/metadata/prefixes/prefixes.go b/pkg/storage/utils/decomposedfs/metadata/prefixes/prefixes.go index fb0cbef28bb..855b3998efd 100644 --- a/pkg/storage/utils/decomposedfs/metadata/prefixes/prefixes.go +++ b/pkg/storage/utils/decomposedfs/metadata/prefixes/prefixes.go @@ -18,6 +18,8 @@ package prefixes +import "github.com/opencloud-eu/reva/v2/pkg/storage/utils/ace" + // Declare a list of xattr keys // TODO the below comment is currently copied from the owncloud driver, revisit // Currently,extended file attributes have four separated @@ -49,8 +51,8 @@ const ( // grantPrefix is the prefix for sharing related extended attributes GrantPrefix string = OcisPrefix + "grant." - GrantUserAcePrefix string = OcisPrefix + "grant." + UserAcePrefix - GrantGroupAcePrefix string = OcisPrefix + "grant." + GroupAcePrefix + GrantUserAcePrefix string = OcisPrefix + "grant." + ace.UserAcePrefix + GrantGroupAcePrefix string = OcisPrefix + "grant." + ace.GroupAcePrefix MetadataPrefix string = OcisPrefix + "md." // favorite flag, per user @@ -97,7 +99,4 @@ const ( SpaceReadmeAttr string = OcisPrefix + "space.readme" SpaceImageAttr string = OcisPrefix + "space.image" SpaceAliasAttr string = OcisPrefix + "space.alias" - - UserAcePrefix string = "u:" - GroupAcePrefix string = "g:" ) diff --git a/pkg/utils/filename_encoder.go b/pkg/utils/filename_encoder.go new file mode 100644 index 00000000000..13fdf392579 --- /dev/null +++ b/pkg/utils/filename_encoder.go @@ -0,0 +1,62 @@ +package utils + +import ( + "encoding/base64" + "strings" + + grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" + userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + "google.golang.org/protobuf/proto" +) + +type FilenameEncoder interface { + SafeFilename() string +} + +type FSSafeUserID struct { + id *userpb.UserId +} + +func NewFSSafeUserID(id *userpb.UserId) FSSafeUserID { + return FSSafeUserID{id: id} +} + +func (id FSSafeUserID) SafeFilename() string { + opaqueID := id.id.GetOpaqueId() + if id.id.GetType() == userpb.UserType_USER_TYPE_GUEST { + // Guest opaqueID is an email address, which is why we choose to make + // it lowercase: RFC 5321 does specify that email address local-parts + // are case sensitive but, in practice, it's a de-facto standard that + // email providers consider them to be case insensitive: + return base64.RawURLEncoding.EncodeToString([]byte(strings.ToLower(opaqueID))) + } + return opaqueID +} + +// Decode returns a copy of the wrapped user ID with the filename restored as its opaque ID. +// The decoding decision is done based on the Type attribute of the Receiver id +func (id FSSafeUserID) Decode(filename string) (*userpb.UserId, error) { + opaqueID := filename + if id.id.GetType() == userpb.UserType_USER_TYPE_GUEST { + decoded, err := base64.RawURLEncoding.DecodeString(filename) + if err != nil { + return nil, err + } + opaqueID = string(decoded) + } + + decodedID := &userpb.UserId{} + if id.id != nil { + decodedID = proto.Clone(id.id).(*userpb.UserId) + } + decodedID.OpaqueId = opaqueID + return decodedID, nil +} + +type FSSafeGroupID struct { + ID *grouppb.GroupId +} + +func (id FSSafeGroupID) SafeFilename() string { + return id.ID.GetOpaqueId() +} diff --git a/pkg/utils/filename_encoder_suite_test.go b/pkg/utils/filename_encoder_suite_test.go new file mode 100644 index 00000000000..98f7fb6f1dc --- /dev/null +++ b/pkg/utils/filename_encoder_suite_test.go @@ -0,0 +1,13 @@ +package utils_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestFilenameEncoder(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Filename Encoder Suite") +} diff --git a/pkg/utils/filename_encoder_test.go b/pkg/utils/filename_encoder_test.go new file mode 100644 index 00000000000..91ccbedbfd6 --- /dev/null +++ b/pkg/utils/filename_encoder_test.go @@ -0,0 +1,76 @@ +package utils_test + +import ( + "encoding/base64" + "strings" + + userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/opencloud-eu/reva/v2/pkg/utils" +) + +var _ = Describe("FSSafeUserID", func() { + DescribeTable("SafeFilename", + func(userType userpb.UserType, opaqueID string, encoded bool) { + id := utils.NewFSSafeUserID(&userpb.UserId{ + Type: userType, + OpaqueId: opaqueID, + }) + if encoded { + Expect(id.SafeFilename()).To(Equal(base64.RawURLEncoding.EncodeToString([]byte(strings.ToLower(opaqueID))))) + } else { + Expect(id.SafeFilename()).To(Equal(opaqueID)) + } + }, + Entry("preserves a primary user ID", userpb.UserType_USER_TYPE_PRIMARY, "MixedCase/ID", false), + Entry("preserves another non-guest user ID", userpb.UserType_USER_TYPE_FEDERATED, "Federated@Example.COM", false), + Entry("lowercases and encodes a guest user ID", userpb.UserType_USER_TYPE_GUEST, "Guest@Example.COM", true), + Entry("uses URL-safe raw base64 for a guest user ID", userpb.UserType_USER_TYPE_GUEST, "???", true), + Entry("encodes a file system path used as a guest user ID", userpb.UserType_USER_TYPE_GUEST, "../../etc/passwd", true), + Entry("encodes a guest user ID with a quoted local part", userpb.UserType_USER_TYPE_GUEST, `"Guest.User"@Example.COM`, true), + Entry("handles an empty primary user ID", userpb.UserType_USER_TYPE_PRIMARY, "", false), + Entry("handles an empty guest user ID", userpb.UserType_USER_TYPE_GUEST, "", false), + ) + + Describe("Decode", func() { + It("decodes a guest ID and preserves the wrapped ID", func() { + original := &userpb.UserId{ + Type: userpb.UserType_USER_TYPE_GUEST, + OpaqueId: "original@example.com", + Idp: "idp.example.com", + TenantId: "tenant", + } + + decoded, err := utils.NewFSSafeUserID(original).Decode( + base64.RawURLEncoding.EncodeToString([]byte(strings.ToLower("../../etc/passwd"))), + ) + + Expect(err).NotTo(HaveOccurred()) + Expect(decoded.GetOpaqueId()).To(Equal("../../etc/passwd")) + Expect(decoded.GetType()).To(Equal(userpb.UserType_USER_TYPE_GUEST)) + Expect(decoded.GetIdp()).To(Equal("idp.example.com")) + Expect(decoded.GetTenantId()).To(Equal("tenant")) + Expect(original.GetOpaqueId()).To(Equal("original@example.com")) + }) + + It("does not decode a non-guest ID", func() { + id := utils.NewFSSafeUserID(&userpb.UserId{Type: userpb.UserType_USER_TYPE_PRIMARY}) + + someb64 := base64.RawURLEncoding.EncodeToString([]byte("userid")) + decoded, err := id.Decode(someb64) + + Expect(err).NotTo(HaveOccurred()) + Expect(decoded.GetOpaqueId()).To(Equal(someb64)) + }) + + It("rejects malformed Base64 for a guest ID", func() { + id := utils.NewFSSafeUserID(&userpb.UserId{Type: userpb.UserType_USER_TYPE_GUEST}) + + decoded, err := id.Decode("not valid base64") + + Expect(err).To(HaveOccurred()) + Expect(decoded).To(BeNil()) + }) + }) +}) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index bc4833dd27f..78a3c960b17 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -218,12 +218,31 @@ func ExtractGranteeID(grantee *provider.Grantee) (*userpb.UserId, *grouppb.Group // UserEqual returns whether two users have the same field values. func UserEqual(u, v *userpb.UserId) bool { - return u != nil && v != nil && u.Idp == v.Idp && u.OpaqueId == v.OpaqueId + return u != nil && v != nil && u.Idp == v.Idp && UserIDEqual(u, v) } // UserIDEqual returns whether two users have the same opaqueid values. The idp is ignored func UserIDEqual(u, v *userpb.UserId) bool { - return u != nil && v != nil && u.OpaqueId == v.OpaqueId + if u == nil || v == nil { + return false + } + return CanonicalUserID(u) == CanonicalUserID(v) +} + +// CanonicalUserID returns the stable representation of a UserId as e.g. used for user storage keys. +// Currently this is only relevant for UserId of the USER_TYPE_GUEST, which are matched case-insensitively. +func CanonicalUserID(id *userpb.UserId) string { + if id == nil { + return "" + } + if id.GetType() == userpb.UserType_USER_TYPE_GUEST { + // Guest user IDs are lowercased to enable case insensitive + // comparisons: those are email addresses, and while RFC 5321 states + // that the local-part is case sensitive, in practice, it's a de facto + // standard that email providers consider them to be case insensitive. + return strings.ToLower(id.GetOpaqueId()) + } + return id.GetOpaqueId() } // GroupEqual returns whether two groups have the same field values. diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index ca138ead72b..83a7338e825 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -21,6 +21,7 @@ package utils import ( "testing" + userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" ) @@ -171,3 +172,23 @@ func TestMakeRelativePath(t *testing.T) { } } } + +func TestCanonicalUserID(t *testing.T) { + tests := []struct { + name string + id *userpb.UserId + expected string + }{ + {name: "nil", expected: ""}, + {name: "regular user", id: &userpb.UserId{OpaqueId: "MixedCase"}, expected: "MixedCase"}, + {name: "guest", id: &userpb.UserId{OpaqueId: "Guest@Example.COM", Type: userpb.UserType_USER_TYPE_GUEST}, expected: "guest@example.com"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if actual := CanonicalUserID(tt.id); actual != tt.expected { + t.Errorf("expected %q, got %q", tt.expected, actual) + } + }) + } +}