From a5e0c1ec4bd015d36285c428bafb193205323bd5 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Thu, 4 Sep 2025 17:13:50 +0200 Subject: [PATCH] fix(graph): Set the full CS3 user id in the Create Share request Up to now we only set the OpaqueId attribute, which breaks sharing as soon as multi-tenancy is enabled. We need the full UserId (including the tenantId and the idp value). Related Issue: #1194 --- services/graph/pkg/identity/backend.go | 4 +- services/graph/pkg/identity/cache.go | 46 ++++++++++++------- .../service/v0/api_driveitem_permissions.go | 41 ++++++++--------- 3 files changed, 51 insertions(+), 40 deletions(-) diff --git a/services/graph/pkg/identity/backend.go b/services/graph/pkg/identity/backend.go index 02ef000170..ab9ca462e4 100644 --- a/services/graph/pkg/identity/backend.go +++ b/services/graph/pkg/identity/backend.go @@ -121,7 +121,7 @@ func CreateUserModelFromCS3(u *cs3user.User) *libregraph.User { if u.GetId() == nil { u.Id = &cs3user.UserId{} } - userType := cs3UserTypeToGraph(u.GetId().GetType()) + userType := CS3UserTypeToGraph(u.GetId().GetType()) user := &libregraph.User{ Identities: []libregraph.ObjectIdentity{{ Issuer: &u.GetId().Idp, @@ -136,7 +136,7 @@ func CreateUserModelFromCS3(u *cs3user.User) *libregraph.User { return user } -func cs3UserTypeToGraph(cs3type cs3user.UserType) string { +func CS3UserTypeToGraph(cs3type cs3user.UserType) string { switch cs3type { case cs3user.UserType_USER_TYPE_PRIMARY: return UserTypeMember diff --git a/services/graph/pkg/identity/cache.go b/services/graph/pkg/identity/cache.go index 2d0d82161d..e1e158d7ff 100644 --- a/services/graph/pkg/identity/cache.go +++ b/services/graph/pkg/identity/cache.go @@ -7,6 +7,7 @@ import ( gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" cs3Group "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" cs3User "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + cs3user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" "github.com/jellydator/ttlcache/v3" libregraph "github.com/opencloud-eu/libre-graph-api-go" @@ -17,7 +18,7 @@ import ( // IdentityCache implements a simple ttl based cache for looking up users and groups by ID type IdentityCache struct { - users *ttlcache.Cache[string, libregraph.User] + users *ttlcache.Cache[string, *cs3User.User] groups *ttlcache.Cache[string, libregraph.Group] gatewaySelector pool.Selectable[gateway.GatewayAPIClient] } @@ -67,8 +68,8 @@ func NewIdentityCache(opts ...IdentityCacheOption) IdentityCache { var cache IdentityCache cache.users = ttlcache.New( - ttlcache.WithTTL[string, libregraph.User](opt.usersTTL), - ttlcache.WithDisableTouchOnHit[string, libregraph.User](), + ttlcache.WithTTL[string, *cs3user.User](opt.usersTTL), + ttlcache.WithDisableTouchOnHit[string, *cs3user.User](), ) go cache.users.Start() @@ -85,23 +86,30 @@ func NewIdentityCache(opts ...IdentityCacheOption) IdentityCache { // GetUser looks up a user by id, if the user is not cached, yet it will do a lookup via the CS3 API func (cache IdentityCache) GetUser(ctx context.Context, userid string) (libregraph.User, error) { - var user libregraph.User + u, err := cache.GetCS3User(ctx, userid) + if err != nil { + return libregraph.User{}, err + } + return *CreateUserModelFromCS3(u), nil +} + +func (cache IdentityCache) GetCS3User(ctx context.Context, userid string) (*cs3User.User, error) { + var user *cs3User.User if item := cache.users.Get(userid); item == nil { gatewayClient, err := cache.gatewaySelector.Next() if err != nil { - return libregraph.User{}, errorcode.New(errorcode.GeneralException, err.Error()) + return nil, errorcode.New(errorcode.GeneralException, err.Error()) } cs3UserID := &cs3User.UserId{ OpaqueId: userid, } - u, err := revautils.GetUserNoGroups(ctx, cs3UserID, gatewayClient) + user, err = revautils.GetUserNoGroups(ctx, cs3UserID, gatewayClient) if err != nil { if revautils.IsErrNotFound(err) { - return libregraph.User{}, ErrNotFound + return nil, ErrNotFound } - return libregraph.User{}, errorcode.New(errorcode.GeneralException, err.Error()) + return nil, errorcode.New(errorcode.GeneralException, err.Error()) } - user = *CreateUserModelFromCS3(u) cache.users.Set(userid, user, ttlcache.DefaultTTL) } else { @@ -112,25 +120,31 @@ func (cache IdentityCache) GetUser(ctx context.Context, userid string) (libregra // GetAcceptedUser looks up a user by id, if the user is not cached, yet it will do a lookup via the CS3 API func (cache IdentityCache) GetAcceptedUser(ctx context.Context, userid string) (libregraph.User, error) { - var user libregraph.User + u, err := cache.GetAcceptedCS3User(ctx, userid) + if err != nil { + return libregraph.User{}, err + } + return *CreateUserModelFromCS3(u), nil +} + +func (cache IdentityCache) GetAcceptedCS3User(ctx context.Context, userid string) (*cs3User.User, error) { + var user *cs3user.User if item := cache.users.Get(userid); item == nil { gatewayClient, err := cache.gatewaySelector.Next() if err != nil { - return libregraph.User{}, errorcode.New(errorcode.GeneralException, err.Error()) + return nil, errorcode.New(errorcode.GeneralException, err.Error()) } cs3UserID := &cs3User.UserId{ OpaqueId: userid, } - u, err := revautils.GetAcceptedUserWithContext(ctx, cs3UserID, gatewayClient) + user, err = revautils.GetAcceptedUserWithContext(ctx, cs3UserID, gatewayClient) if err != nil { if revautils.IsErrNotFound(err) { - return libregraph.User{}, ErrNotFound + return nil, ErrNotFound } - return libregraph.User{}, errorcode.New(errorcode.GeneralException, err.Error()) + return nil, errorcode.New(errorcode.GeneralException, err.Error()) } - user = *CreateUserModelFromCS3(u) cache.users.Set(userid, user, ttlcache.DefaultTTL) - } else { user = item.Value() } diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions.go b/services/graph/pkg/service/v0/api_driveitem_permissions.go index db160eef24..c7ccb05301 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions.go @@ -184,9 +184,9 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId *sto cTime = createShareResponse.GetShare().GetCtime() expiration = createShareResponse.GetShare().GetExpiration() default: - user, err := s.identityCache.GetUser(ctx, objectID) + user, err := s.identityCache.GetCS3User(ctx, objectID) if errors.Is(err, identity.ErrNotFound) && s.config.IncludeOCMSharees { - user, err = s.identityCache.GetAcceptedUser(ctx, objectID) + user, err = s.identityCache.GetAcceptedCS3User(ctx, objectID) if err == nil && IsSpaceRoot(statResponse.GetInfo().GetId()) { return libregraph.Permission{}, errorcode.New(errorcode.InvalidRequest, "federated user can not become a space member") } @@ -198,19 +198,16 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId *sto permission.GrantedToV2 = &libregraph.SharePointIdentitySet{ User: &libregraph.Identity{ DisplayName: user.GetDisplayName(), - Id: conversions.ToPointer(user.GetId()), - LibreGraphUserType: conversions.ToPointer(user.GetUserType()), + Id: conversions.ToPointer(user.GetId().GetOpaqueId()), + LibreGraphUserType: conversions.ToPointer(identity.CS3UserTypeToGraph(user.GetId().GetType())), }, } - if user.GetUserType() == identity.UserTypeFederated { - if len(user.Identities) < 1 { - return libregraph.Permission{}, errorcode.New(errorcode.InvalidRequest, "user has no federated identity") - } + if user.GetId().GetType() == userpb.UserType_USER_TYPE_FEDERATED { providerInfoResp, err := gatewayClient.GetInfoByDomain(ctx, &ocmprovider.GetInfoByDomainRequest{ - Domain: *user.Identities[0].Issuer, + Domain: user.GetId().GetIdp(), }) - if err := errorcode.FromCS3Status(providerInfoResp.GetStatus(), err); err != nil { + if err = errorcode.FromCS3Status(providerInfoResp.GetStatus(), err); err != nil { s.logger.Error().Err(err).Msg("getting provider info failed") return libregraph.Permission{}, err } @@ -220,7 +217,7 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId *sto createShareRequest.Expiration = utils.TimeToTS(*invite.ExpirationDateTime) } createShareResponse, err := gatewayClient.CreateOCMShare(ctx, createShareRequest) - if err := errorcode.FromCS3Status(createShareResponse.GetStatus(), err); err != nil { + if err = errorcode.FromCS3Status(createShareResponse.GetStatus(), err); err != nil { s.logger.Error().Err(err).Msg("share creation failed") return libregraph.Permission{}, err } @@ -233,7 +230,7 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId *sto createShareRequest.GetGrant().Expiration = utils.TimeToTS(*invite.ExpirationDateTime) } createShareResponse, err := gatewayClient.CreateShare(ctx, createShareRequest) - if err := errorcode.FromCS3Status(createShareResponse.GetStatus(), err); err != nil { + if err = errorcode.FromCS3Status(createShareResponse.GetStatus(), err); err != nil { s.logger.Error().Err(err).Msg("share creation failed") return libregraph.Permission{}, err } @@ -293,15 +290,16 @@ func createShareRequestToGroup(group libregraph.Group, info *storageprovider.Res }, } } -func createShareRequestToUser(user libregraph.User, info *storageprovider.ResourceInfo, cs3ResourcePermissions *storageprovider.ResourcePermissions) *collaboration.CreateShareRequest { + +func createShareRequestToUser(user *userpb.User, info *storageprovider.ResourceInfo, cs3ResourcePermissions *storageprovider.ResourcePermissions) *collaboration.CreateShareRequest { return &collaboration.CreateShareRequest{ ResourceInfo: info, Grant: &collaboration.ShareGrant{ Grantee: &storageprovider.Grantee{ Type: storageprovider.GranteeType_GRANTEE_TYPE_USER, - Id: &storageprovider.Grantee_UserId{UserId: &userpb.UserId{ - OpaqueId: user.GetId(), - }}, + Id: &storageprovider.Grantee_UserId{ + UserId: user.GetId(), + }, }, Permissions: &collaboration.SharePermissions{ Permissions: cs3ResourcePermissions, @@ -309,16 +307,15 @@ func createShareRequestToUser(user libregraph.User, info *storageprovider.Resour }, } } -func createShareRequestToFederatedUser(user libregraph.User, resourceId *storageprovider.ResourceId, providerInfo *ocmprovider.ProviderInfo, cs3ResourcePermissions *storageprovider.ResourcePermissions) *ocm.CreateOCMShareRequest { + +func createShareRequestToFederatedUser(user *userpb.User, resourceId *storageprovider.ResourceId, providerInfo *ocmprovider.ProviderInfo, cs3ResourcePermissions *storageprovider.ResourcePermissions) *ocm.CreateOCMShareRequest { return &ocm.CreateOCMShareRequest{ ResourceId: resourceId, Grantee: &storageprovider.Grantee{ Type: storageprovider.GranteeType_GRANTEE_TYPE_USER, - Id: &storageprovider.Grantee_UserId{UserId: &userpb.UserId{ - Type: userpb.UserType_USER_TYPE_FEDERATED, - OpaqueId: user.GetId(), - Idp: *user.GetIdentities()[0].Issuer, // the domain is persisted in the grant as u:{opaqueid}:{domain} - }}, + Id: &storageprovider.Grantee_UserId{ + UserId: user.GetId(), + }, }, RecipientMeshProvider: providerInfo, AccessMethods: []*ocm.AccessMethod{