From 6f90abd22d16150aff8aaea97edd230f64e93b8b Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Thu, 14 Nov 2024 11:55:58 +0100 Subject: [PATCH 1/2] fix(ocm): Adjust for recend change for federated user IDs The UserIds as returned by e.g. GetAcceptedUser do already contain the provider domain in the IDP field now. Also adjust the provider domain in the OCM config to be really a domain without URI scheme and path. --- changelog/unreleased/fix-ocm-external-idp.md | 7 +++++++ services/graph/pkg/identity/backend.go | 7 ------- services/ocm/pkg/revaconfig/config.go | 20 ++++++++++++++++--- tests/acceptance/bootstrap/FeatureContext.php | 2 +- 4 files changed, 25 insertions(+), 11 deletions(-) create mode 100644 changelog/unreleased/fix-ocm-external-idp.md diff --git a/changelog/unreleased/fix-ocm-external-idp.md b/changelog/unreleased/fix-ocm-external-idp.md new file mode 100644 index 0000000000..6e13949c3f --- /dev/null +++ b/changelog/unreleased/fix-ocm-external-idp.md @@ -0,0 +1,7 @@ +Bugfix: Fix federated sharing when using an external IDP + +We fixed a bug that caused federated sharing to fail, when the +federated oCIS instances where sharing the same external IDP. + +https://github.com/owncloud/ocis/pull/10567 +https://github.com/cs3org/reva/pull/4933 diff --git a/services/graph/pkg/identity/backend.go b/services/graph/pkg/identity/backend.go index 8f3d09686c..976f97d5ce 100644 --- a/services/graph/pkg/identity/backend.go +++ b/services/graph/pkg/identity/backend.go @@ -8,7 +8,6 @@ import ( "github.com/CiscoM31/godata" cs3group "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" cs3user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" - ocmuser "github.com/cs3org/reva/v2/pkg/ocm/user" libregraph "github.com/owncloud/libre-graph-api-go" "github.com/owncloud/ocis/v2/services/graph/pkg/errorcode" ) @@ -134,12 +133,6 @@ func CreateUserModelFromCS3(u *cs3user.User) *libregraph.User { OnPremisesSamAccountName: u.GetUsername(), Id: &u.GetId().OpaqueId, } - // decode the remote id if the user is federated - if u.GetId().GetType() == cs3user.UserType_USER_TYPE_FEDERATED { - remoteID := ocmuser.RemoteID(u.GetId()) - user.Identities[0].Issuer = &remoteID.Idp - user.Identities[0].IssuerAssignedId = &remoteID.OpaqueId - } return user } diff --git a/services/ocm/pkg/revaconfig/config.go b/services/ocm/pkg/revaconfig/config.go index 601cfc3bb8..9a33ce13c5 100644 --- a/services/ocm/pkg/revaconfig/config.go +++ b/services/ocm/pkg/revaconfig/config.go @@ -2,6 +2,7 @@ package revaconfig import ( "math" + "net/url" "github.com/owncloud/ocis/v2/ocis-pkg/log" "github.com/owncloud/ocis/v2/services/ocm/pkg/config" @@ -9,6 +10,19 @@ import ( // OCMConfigFromStruct will adapt an oCIS config struct into a reva mapstructure to start a reva service. func OCMConfigFromStruct(cfg *config.Config, logger log.Logger) map[string]interface{} { + + // Construct the ocm provider domain from the oCIS URL + providerDomain := "" + u, err := url.Parse(cfg.Commons.OcisURL) + switch { + case err != nil: + logger.Error().Err(err).Msg("could not parse oCIS URL") + case u.Host == "": + logger.Error().Msg("oCIS URL has no host") + default: + providerDomain = u.Host + } + return map[string]interface{}{ "shared": map[string]interface{}{ "jwt_secret": cfg.TokenManager.JWTSecret, @@ -59,7 +73,7 @@ func OCMConfigFromStruct(cfg *config.Config, logger log.Logger) map[string]inter "smtp_credentials": map[string]string{}, "gatewaysvc": cfg.Reva.Address, "mesh_directory_url": cfg.ScienceMesh.MeshDirectoryURL, - "provider_domain": cfg.Commons.OcisURL, + "provider_domain": providerDomain, "events": map[string]interface{}{ "natsaddress": cfg.Events.Endpoint, "natsclusterid": cfg.Events.Cluster, @@ -121,7 +135,7 @@ func OCMConfigFromStruct(cfg *config.Config, logger log.Logger) map[string]inter "file": cfg.OCMInviteManager.Drivers.JSON.File, }, }, - "provider_domain": cfg.Commons.OcisURL, + "provider_domain": providerDomain, "token_expiration": cfg.OCMInviteManager.TokenExpiration.String(), "ocm_timeout": int(math.Round(cfg.OCMInviteManager.Timeout.Seconds())), "ocm_insecure": cfg.OCMInviteManager.Insecure, @@ -142,7 +156,7 @@ func OCMConfigFromStruct(cfg *config.Config, logger log.Logger) map[string]inter }, }, "gatewaysvc": cfg.Reva.Address, - "provider_domain": cfg.Commons.OcisURL, + "provider_domain": providerDomain, "webdav_endpoint": cfg.Commons.OcisURL, "webapp_template": cfg.OCMShareProvider.WebappTemplate, "client_insecure": cfg.OCMShareProvider.Insecure, diff --git a/tests/acceptance/bootstrap/FeatureContext.php b/tests/acceptance/bootstrap/FeatureContext.php index 842a71d320..5bf1cb956c 100644 --- a/tests/acceptance/bootstrap/FeatureContext.php +++ b/tests/acceptance/bootstrap/FeatureContext.php @@ -2396,7 +2396,7 @@ class FeatureContext extends BehatVariablesContext { "code" => "%identities_issuer_id_pattern%", "function" => [ __NAMESPACE__ . '\TestHelpers\GraphHelper', - "getUUIDv4Regex" + "getFederatedUserRegex" ], "parameter" => [] ], From e6ea4faf01d732ebce13b3e9f2e972b953f39b70 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Thu, 14 Nov 2024 14:56:59 +0100 Subject: [PATCH 2/2] Bump reva to get https://github.com/cs3org/reva/pull/4933 --- go.mod | 5 +++-- go.sum | 4 ++-- .../ocminvitemanager/ocminvitemanager.go | 16 ++++++++-------- .../ocmshareprovider/ocmshareprovider.go | 4 ++-- .../v2/internal/http/services/ocmd/invites.go | 3 ++- .../cs3org/reva/v2/pkg/ocm/user/user.go | 11 +++-------- vendor/modules.txt | 2 +- 7 files changed, 21 insertions(+), 24 deletions(-) diff --git a/go.mod b/go.mod index acab018c40..9c1dc31a51 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,7 @@ module github.com/owncloud/ocis/v2 -go 1.22.0 +go 1.22.7 + toolchain go1.22.9 require ( @@ -16,7 +17,7 @@ require ( github.com/cenkalti/backoff v2.2.1+incompatible github.com/coreos/go-oidc/v3 v3.11.0 github.com/cs3org/go-cs3apis v0.0.0-20241105092511-3ad35d174fc1 - github.com/cs3org/reva/v2 v2.26.5 + github.com/cs3org/reva/v2 v2.26.6-0.20241114152615-4ce61d4d09fc github.com/davidbyttow/govips/v2 v2.15.0 github.com/dhowden/tag v0.0.0-20240417053706-3d75831295e8 github.com/dutchcoders/go-clamd v0.0.0-20170520113014-b970184f4d9e diff --git a/go.sum b/go.sum index d31afbccde..3686ca1e0d 100644 --- a/go.sum +++ b/go.sum @@ -255,8 +255,8 @@ github.com/crewjam/saml v0.4.14 h1:g9FBNx62osKusnFzs3QTN5L9CVA/Egfgm+stJShzw/c= github.com/crewjam/saml v0.4.14/go.mod h1:UVSZCf18jJkk6GpWNVqcyQJMD5HsRugBPf4I1nl2mME= github.com/cs3org/go-cs3apis v0.0.0-20241105092511-3ad35d174fc1 h1:RU6LT6mkD16xZs011+8foU7T3LrPvTTSWeTQ9OgfhkA= github.com/cs3org/go-cs3apis v0.0.0-20241105092511-3ad35d174fc1/go.mod h1:DedpcqXl193qF/08Y04IO0PpxyyMu8+GrkD6kWK2MEQ= -github.com/cs3org/reva/v2 v2.26.5 h1:LWIOSpmgoVQDfe9S2renzqqAXorFs6lT+5Vodhr3M68= -github.com/cs3org/reva/v2 v2.26.5/go.mod h1:KP0Zomt3dNIr/kU2M1mXzTIVFOtxBVS4qmBDMRCfrOQ= +github.com/cs3org/reva/v2 v2.26.6-0.20241114152615-4ce61d4d09fc h1:uhBs3S2G8SpVd38uDzEWazIvR0U3GqL7VfevtkBequ4= +github.com/cs3org/reva/v2 v2.26.6-0.20241114152615-4ce61d4d09fc/go.mod h1:KP0Zomt3dNIr/kU2M1mXzTIVFOtxBVS4qmBDMRCfrOQ= github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4= github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg= github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= diff --git a/vendor/github.com/cs3org/reva/v2/internal/grpc/services/ocminvitemanager/ocminvitemanager.go b/vendor/github.com/cs3org/reva/v2/internal/grpc/services/ocminvitemanager/ocminvitemanager.go index 9a7b3a1295..f9bdc8baf9 100644 --- a/vendor/github.com/cs3org/reva/v2/internal/grpc/services/ocminvitemanager/ocminvitemanager.go +++ b/vendor/github.com/cs3org/reva/v2/internal/grpc/services/ocminvitemanager/ocminvitemanager.go @@ -169,12 +169,15 @@ func (s *service) ForwardInvite(ctx context.Context, req *invitepb.ForwardInvite return nil, err } + // Accept the invitation on the remote OCM provider remoteUser, err := s.ocmClient.InviteAccepted(ctx, ocmEndpoint, &client.InviteAcceptedRequest{ Token: req.InviteToken.GetToken(), RecipientProvider: s.conf.ProviderDomain, - UserID: user.GetId().GetOpaqueId(), - Email: user.GetMail(), - Name: user.GetDisplayName(), + // The UserID is only a string here. To not loose the IDP information we use the FederatedID encoding + // i.e. base64(UserID@IDP) + UserID: ocmuser.FederatedID(user.GetId(), "").GetOpaqueId(), + Email: user.GetMail(), + Name: user.GetDisplayName(), }) if err != nil { switch { @@ -205,15 +208,14 @@ func (s *service) ForwardInvite(ctx context.Context, req *invitepb.ForwardInvite // and the remote one (the initiator), so at the end of the invitation workflow they // know each other + // remoteUser.UserID is the federated ID (just a string), to get a unique CS3 userid + // we're using the provider domain as the IDP part of the ID remoteUserID := &userpb.UserId{ Type: userpb.UserType_USER_TYPE_FEDERATED, Idp: req.GetOriginSystemProvider().Domain, OpaqueId: remoteUser.UserID, } - // we need to use a unique identifier for federated users - remoteUserID = ocmuser.FederatedID(remoteUserID) - if err := s.repo.AddRemoteUser(ctx, user.Id, &userpb.User{ Id: remoteUserID, Mail: remoteUser.Email, @@ -271,8 +273,6 @@ func (s *service) AcceptInvite(ctx context.Context, req *invitepb.AcceptInviteRe } remoteUser := req.GetRemoteUser() - // we need to use a unique identifier for federated users - remoteUser.Id = ocmuser.FederatedID(remoteUser.Id) if err := s.repo.AddRemoteUser(ctx, token.GetUserId(), remoteUser); err != nil { if errors.Is(err, invite.ErrUserAlreadyAccepted) { diff --git a/vendor/github.com/cs3org/reva/v2/internal/grpc/services/ocmshareprovider/ocmshareprovider.go b/vendor/github.com/cs3org/reva/v2/internal/grpc/services/ocmshareprovider/ocmshareprovider.go index b35eca3dbf..ad7c36ca93 100644 --- a/vendor/github.com/cs3org/reva/v2/internal/grpc/services/ocmshareprovider/ocmshareprovider.go +++ b/vendor/github.com/cs3org/reva/v2/internal/grpc/services/ocmshareprovider/ocmshareprovider.go @@ -326,8 +326,8 @@ func (s *service) CreateOCMShare(ctx context.Context, req *ocm.CreateOCMShareReq shareWith := ocmuser.FormatOCMUser(ocmuser.RemoteID(req.GetGrantee().GetUserId())) // wrap the local user id in a federated user id - owner := ocmuser.FormatOCMUser(ocmuser.FederatedID(info.Owner)) - sender := ocmuser.FormatOCMUser(ocmuser.FederatedID(user.Id)) + owner := ocmuser.FormatOCMUser(ocmuser.FederatedID(info.Owner, s.conf.ProviderDomain)) + sender := ocmuser.FormatOCMUser(ocmuser.FederatedID(user.Id, s.conf.ProviderDomain)) newShareReq := &client.NewShareRequest{ ShareWith: shareWith, diff --git a/vendor/github.com/cs3org/reva/v2/internal/http/services/ocmd/invites.go b/vendor/github.com/cs3org/reva/v2/internal/http/services/ocmd/invites.go index 439d40a3c9..1fda2059fd 100644 --- a/vendor/github.com/cs3org/reva/v2/internal/http/services/ocmd/invites.go +++ b/vendor/github.com/cs3org/reva/v2/internal/http/services/ocmd/invites.go @@ -32,6 +32,7 @@ import ( rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" "github.com/cs3org/reva/v2/internal/http/services/reqres" "github.com/cs3org/reva/v2/pkg/appctx" + ocmuser "github.com/cs3org/reva/v2/pkg/ocm/user" "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/v2/pkg/utils" ) @@ -145,7 +146,7 @@ func (h *invitesHandler) AcceptInvite(w http.ResponseWriter, r *http.Request) { } if err := json.NewEncoder(w).Encode(&user{ - UserID: acceptInviteResponse.UserId.OpaqueId, + UserID: ocmuser.FederatedID(acceptInviteResponse.UserId, "").GetOpaqueId(), Email: acceptInviteResponse.Email, Name: acceptInviteResponse.DisplayName, }); err != nil { diff --git a/vendor/github.com/cs3org/reva/v2/pkg/ocm/user/user.go b/vendor/github.com/cs3org/reva/v2/pkg/ocm/user/user.go index 6a48f4fe77..9be8add2b4 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/ocm/user/user.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/ocm/user/user.go @@ -3,7 +3,6 @@ package user import ( "encoding/base64" "fmt" - "net/url" "strings" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -12,16 +11,12 @@ import ( // FederatedID creates a federated user id by // 1. stripping the protocol from the domain and // 2. base64 encoding the opaque id with the domain to get a unique identifier that cannot collide with other users -func FederatedID(id *userpb.UserId) *userpb.UserId { - // strip protocol from the domain - domain := id.Idp - if u, err := url.Parse(domain); err == nil && u.Host != "" { - domain = u.Host - } +func FederatedID(id *userpb.UserId, domain string) *userpb.UserId { + opaqueId := base64.URLEncoding.EncodeToString([]byte(id.OpaqueId + "@" + id.Idp)) return &userpb.UserId{ Type: userpb.UserType_USER_TYPE_FEDERATED, Idp: domain, - OpaqueId: base64.URLEncoding.EncodeToString([]byte(id.OpaqueId + "@" + domain)), + OpaqueId: opaqueId, } } diff --git a/vendor/modules.txt b/vendor/modules.txt index a15e8b0fa2..fe589ecce0 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -367,7 +367,7 @@ github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1 github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1 github.com/cs3org/go-cs3apis/cs3/tx/v1beta1 github.com/cs3org/go-cs3apis/cs3/types/v1beta1 -# github.com/cs3org/reva/v2 v2.26.5 +# github.com/cs3org/reva/v2 v2.26.6-0.20241114152615-4ce61d4d09fc ## explicit; go 1.22.0 github.com/cs3org/reva/v2/cmd/revad/internal/grace github.com/cs3org/reva/v2/cmd/revad/runtime