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/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/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" => [] ], 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