From e98eb858404e81d9396a4932cebdd76402747adf Mon Sep 17 00:00:00 2001 From: Giuseppe Lo Presti Date: Tue, 17 Sep 2024 12:38:20 +0200 Subject: [PATCH 1/3] cernbox: fix shares to external accounts --- changelog/unreleased/lwshares-fix.md | 7 +++++++ pkg/cbox/utils/conversions.go | 9 +++++++++ 2 files changed, 16 insertions(+) create mode 100644 changelog/unreleased/lwshares-fix.md diff --git a/changelog/unreleased/lwshares-fix.md b/changelog/unreleased/lwshares-fix.md new file mode 100644 index 0000000000..eb23a22a4c --- /dev/null +++ b/changelog/unreleased/lwshares-fix.md @@ -0,0 +1,7 @@ +Bugfix: fix shares for external accounts + +We may have external accounts with regular usernames (and with null uid): +This patch decorates such usernames when storing them as Grantee in a share, +such that when a share is retrieved, permissions are checked correctly. + +https://github.com/cs3org/reva/pull/4849 diff --git a/pkg/cbox/utils/conversions.go b/pkg/cbox/utils/conversions.go index ff6ce9e761..a794ed6179 100644 --- a/pkg/cbox/utils/conversions.go +++ b/pkg/cbox/utils/conversions.go @@ -20,6 +20,7 @@ package utils import ( "database/sql" + "fmt" "strings" "time" @@ -165,6 +166,14 @@ func IntToShareState(g int) collaboration.ShareState { // FormatUserID formats a CS3API user ID to a string. func FormatUserID(u *userpb.UserId) string { + if (u.Type == userpb.UserType_USER_TYPE_LIGHTWEIGHT || u.Type == userpb.UserType_USER_TYPE_FEDERATED) && + !strings.Contains(u.OpaqueId, "guest:") && !strings.Contains(u.OpaqueId, "@") { + // in this case we have an external user, but its username does not contain an Idp identifier: + // this may happen for a SSO (e.g. the CERN SSO) that allows to register external accounts via email + // and generates regular usernames for such accounts. Here we decorate those usernames + // so that ExtractUserID below can do the reverse identification: + return fmt.Sprintf("%s@localidp", u.OpaqueId) + } return u.OpaqueId } From 600027ba928f11ea0a971d39f184dcf2b2d29755 Mon Sep 17 00:00:00 2001 From: Giuseppe Lo Presti Date: Mon, 23 Sep 2024 17:53:40 +0200 Subject: [PATCH 2/3] Different approach: force user type resolution at the upper level --- pkg/cbox/utils/conversions.go | 67 ++++++++----------- .../invite/repository/nextcloud/nextcloud.go | 2 +- pkg/ocm/invite/repository/sql/sql.go | 2 +- 3 files changed, 29 insertions(+), 42 deletions(-) diff --git a/pkg/cbox/utils/conversions.go b/pkg/cbox/utils/conversions.go index a794ed6179..cc547115c7 100644 --- a/pkg/cbox/utils/conversions.go +++ b/pkg/cbox/utils/conversions.go @@ -20,8 +20,6 @@ package utils import ( "database/sql" - "fmt" - "strings" "time" grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" @@ -56,7 +54,7 @@ type DBShare struct { NotifyUploadsExtraRecipients sql.NullString } -// FormatGrantee formats a CS3API grantee to a string. +// FormatGrantee formats a CS3API grantee to a (int, string) tuple. func FormatGrantee(g *provider.Grantee) (int, string) { var granteeType int var formattedID string @@ -73,16 +71,22 @@ func FormatGrantee(g *provider.Grantee) (int, string) { return granteeType, formattedID } -// ExtractGrantee retrieves the CS3API grantee from a formatted string. -func ExtractGrantee(t int, g string) *provider.Grantee { +// ExtractGrantee retrieves the CS3API Grantee from a grantee type and username/groupname. +// The grantee userType is relevant only for users. +func ExtractGrantee(t int, g string, gtype userpb.UserType) *provider.Grantee { var grantee provider.Grantee switch t { case 0: grantee.Type = provider.GranteeType_GRANTEE_TYPE_USER - grantee.Id = &provider.Grantee_UserId{UserId: ExtractUserID(g)} + grantee.Id = &provider.Grantee_UserId{UserId: &userpb.UserId{ + OpaqueId: g, + Type: gtype, + }} case 1: grantee.Type = provider.GranteeType_GRANTEE_TYPE_GROUP - grantee.Id = &provider.Grantee_GroupId{GroupId: ExtractGroupID(g)} + grantee.Id = &provider.Grantee_GroupId{GroupId: &grouppb.GroupId{ + OpaqueId: g, + }} default: grantee.Type = provider.GranteeType_GRANTEE_TYPE_INVALID } @@ -164,42 +168,24 @@ func IntToShareState(g int) collaboration.ShareState { } } -// FormatUserID formats a CS3API user ID to a string. +// FormatUserID formats a CS3API user ID as a string. func FormatUserID(u *userpb.UserId) string { - if (u.Type == userpb.UserType_USER_TYPE_LIGHTWEIGHT || u.Type == userpb.UserType_USER_TYPE_FEDERATED) && - !strings.Contains(u.OpaqueId, "guest:") && !strings.Contains(u.OpaqueId, "@") { - // in this case we have an external user, but its username does not contain an Idp identifier: - // this may happen for a SSO (e.g. the CERN SSO) that allows to register external accounts via email - // and generates regular usernames for such accounts. Here we decorate those usernames - // so that ExtractUserID below can do the reverse identification: - return fmt.Sprintf("%s@localidp", u.OpaqueId) - } return u.OpaqueId } -// ExtractUserID retrieves a CS3API user ID from a string. -func ExtractUserID(u string) *userpb.UserId { - t := userpb.UserType_USER_TYPE_PRIMARY - if strings.HasPrefix(u, "guest:") { - t = userpb.UserType_USER_TYPE_LIGHTWEIGHT - } else if strings.Contains(u, "@") { - t = userpb.UserType_USER_TYPE_FEDERATED - } - return &userpb.UserId{OpaqueId: u, Type: t} -} - // FormatGroupID formats a CS3API group ID to a string. func FormatGroupID(u *grouppb.GroupId) string { return u.OpaqueId } -// ExtractGroupID retrieves a CS3API group ID from a string. -func ExtractGroupID(u string) *grouppb.GroupId { - return &grouppb.GroupId{OpaqueId: u} +// MakeUserID generates a CS3API user ID from a username, ASSUMING user type is primary. +func MakeUserID(u string) *userpb.UserId { + return &userpb.UserId{OpaqueId: u, Type: userpb.UserType_USER_TYPE_PRIMARY} } -// ConvertToCS3Share converts a DBShare to a CS3API collaboration share. -func ConvertToCS3Share(s DBShare) *collaboration.Share { +// ConvertToCS3Share converts a DBShare and a grantee userType to a CS3API collaboration share. +// Here we take the shortcut that the Owner's and Creator's user type is PRIMARY. +func ConvertToCS3Share(s DBShare, gtype userpb.UserType) *collaboration.Share { ts := &typespb.Timestamp{ Seconds: uint64(s.STime), } @@ -213,23 +199,24 @@ func ConvertToCS3Share(s DBShare) *collaboration.Share { OpaqueId: s.ItemSource, }, Permissions: &collaboration.SharePermissions{Permissions: IntTosharePerm(s.Permissions, s.ItemType)}, - Grantee: ExtractGrantee(s.ShareType, s.ShareWith), - Owner: ExtractUserID(s.UIDOwner), - Creator: ExtractUserID(s.UIDInitiator), + Grantee: ExtractGrantee(s.ShareType, s.ShareWith, gtype), + Owner: MakeUserID(s.UIDOwner), + Creator: MakeUserID(s.UIDInitiator), Ctime: ts, Mtime: ts, } } -// ConvertToCS3ReceivedShare converts a DBShare to a CS3API collaboration received share. -func ConvertToCS3ReceivedShare(s DBShare) *collaboration.ReceivedShare { +// ConvertToCS3ReceivedShare converts a DBShare and a grantee userType to a CS3API collaboration received share. +func ConvertToCS3ReceivedShare(s DBShare, gtype userpb.UserType) *collaboration.ReceivedShare { return &collaboration.ReceivedShare{ - Share: ConvertToCS3Share(s), + Share: ConvertToCS3Share(s, gtype), State: IntToShareState(s.State), } } // ConvertToCS3PublicShare converts a DBShare to a CS3API public share. +// Here we take the shortcut that the Owner's and Creator's user type is PRIMARY. func ConvertToCS3PublicShare(s DBShare) *link.PublicShare { ts := &typespb.Timestamp{ Seconds: uint64(s.STime), @@ -256,8 +243,8 @@ func ConvertToCS3PublicShare(s DBShare) *link.PublicShare { OpaqueId: s.ItemSource, }, Permissions: &link.PublicSharePermissions{Permissions: IntTosharePerm(s.Permissions, s.ItemType)}, - Owner: ExtractUserID(s.UIDOwner), - Creator: ExtractUserID(s.UIDInitiator), + Owner: MakeUserID(s.UIDOwner), + Creator: MakeUserID(s.UIDInitiator), Token: s.Token, DisplayName: s.ShareName, PasswordProtected: pwd, diff --git a/pkg/ocm/invite/repository/nextcloud/nextcloud.go b/pkg/ocm/invite/repository/nextcloud/nextcloud.go index 40e7d7c0db..b46a2536e3 100644 --- a/pkg/ocm/invite/repository/nextcloud/nextcloud.go +++ b/pkg/ocm/invite/repository/nextcloud/nextcloud.go @@ -107,7 +107,7 @@ func timestampToTime(ctx context.Context, t *types.Timestamp) time.Time { } func (c *Client) convertToInviteToken(ctx context.Context, tkn *apiToken) (*invitepb.InviteToken, error) { - usr := conversions.ExtractUserID(tkn.Initiator) + usr := conversions.MakeUserID(tkn.Initiator) return &invitepb.InviteToken{ Token: tkn.Token, UserId: usr, diff --git a/pkg/ocm/invite/repository/sql/sql.go b/pkg/ocm/invite/repository/sql/sql.go index fb5e935995..2067685149 100644 --- a/pkg/ocm/invite/repository/sql/sql.go +++ b/pkg/ocm/invite/repository/sql/sql.go @@ -121,7 +121,7 @@ func (m *mgr) GetToken(ctx context.Context, token string) (*invitepb.InviteToken func convertToInviteToken(tkn dbToken) *invitepb.InviteToken { return &invitepb.InviteToken{ Token: tkn.Token, - UserId: conversions.ExtractUserID(tkn.Initiator), + UserId: conversions.MakeUserID(tkn.Initiator), Expiration: &types.Timestamp{ Seconds: uint64(tkn.Expiration.Unix()), }, From f8b054c856c4caffc6a9bd25b647b5cd85f6ebc5 Mon Sep 17 00:00:00 2001 From: Giuseppe Lo Presti Date: Thu, 26 Sep 2024 18:28:51 +0200 Subject: [PATCH 3/3] Updated changelog --- changelog/unreleased/lwshares-fix.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/changelog/unreleased/lwshares-fix.md b/changelog/unreleased/lwshares-fix.md index eb23a22a4c..8659efeb3b 100644 --- a/changelog/unreleased/lwshares-fix.md +++ b/changelog/unreleased/lwshares-fix.md @@ -1,7 +1,8 @@ -Bugfix: fix shares for external accounts +Bugfix: Drop assumptions about user types when dealing with shares -We may have external accounts with regular usernames (and with null uid): -This patch decorates such usernames when storing them as Grantee in a share, -such that when a share is retrieved, permissions are checked correctly. +We may have external accounts with regular usernames (and with null uid), +therefore the current logic to heuristically infer the user type from +a grantee's username is broken. This PR removes those heuristics and +requires the upper level to resolve the user type. https://github.com/cs3org/reva/pull/4849