Merge pull request #10570 from rhafer/enterprise/6967

[full-ci] fix(ocm): Adjust for recend change for federated user IDs
2026-03-09 15:28:37 -05:00 · 2024-11-15 12:32:32 +01:00
parent cb7c4fc3ec e6ea4faf01
commit e2547e79a9
11 changed files with 46 additions and 35 deletions
--- a/changelog/unreleased/fix-ocm-external-idp.md
+++ 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
--- 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
--- 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=
--- 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
 }

--- 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,
--- 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" => []
 			],
--- 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) {
--- 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,
--- 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 {
--- 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,
 	}
 }

--- 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