fix(ocm): Fix IDP value in federated user ids

Up to now the IDP for federated users id was set to the domain
of the user's idp. We need to set it to the federation provider's
domain name so that we can check it against the provider domain
as configured in the provider info (as e.g. read from providers.json)

changelog/unreleased/fix-ocm-external-idp.md

@@ -0,0 +1,6 @@
+Bugfix: Fix federated sharing when using an external identity provider
+
+We fixes and issue that caused federated sharing to fail when the identity
+provider url did not match the federation provider url.
+
+https://github.com/cs3org/reva/pull/4933

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

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,

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 {

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

tests/integration/grpc/ocm_invitation_test.go

@@ -119,7 +119,7 @@ var _ = Describe("ocm invitation workflow", func() {
 			Id: &userpb.UserId{
 				Type:     userpb.UserType_USER_TYPE_FEDERATED,
 				Idp:      "cernbox.cern.ch",
-				OpaqueId: base64.URLEncoding.EncodeToString([]byte("[email protected]")),
+				OpaqueId: base64.URLEncoding.EncodeToString([]byte("4c510ada-c86b-4815-8820-42cdf82c3d51@https://cernbox.cern.ch")),
 			},
 			Username:    "einstein",
 			Mail:        "[email protected]",
@@ -139,7 +139,7 @@ var _ = Describe("ocm invitation workflow", func() {
 			Id: &userpb.UserId{
 				Type:     userpb.UserType_USER_TYPE_FEDERATED,
 				Idp:      "cesnet.cz",
-				OpaqueId: base64.URLEncoding.EncodeToString([]byte("[email protected]")),
+				OpaqueId: base64.URLEncoding.EncodeToString([]byte("f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c@https://cesnet.cz")),
 			},
 			Username:    "marie",
 			Mail:        "[email protected]",

tests/integration/grpc/ocm_share_test.go

@@ -127,7 +127,7 @@ var _ = Describe("ocm share", func() {
 		federatedEinsteinID = &userpb.UserId{
 			Type:     userpb.UserType_USER_TYPE_FEDERATED,
 			Idp:      "cernbox.cern.ch",
-			OpaqueId: base64.URLEncoding.EncodeToString([]byte("[email protected]")),
+			OpaqueId: base64.URLEncoding.EncodeToString([]byte("4c510ada-c86b-4815-8820-42cdf82c3d51@https://cernbox.cern.ch")),
 		}
 		marie = &userpb.User{
 			Id: &userpb.UserId{
@@ -142,7 +142,7 @@ var _ = Describe("ocm share", func() {
 		federatedMarieID = &userpb.UserId{
 			Type:     userpb.UserType_USER_TYPE_FEDERATED,
 			Idp:      "cesnet.cz",
-			OpaqueId: base64.URLEncoding.EncodeToString([]byte("[email protected]")),
+			OpaqueId: base64.URLEncoding.EncodeToString([]byte("f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c@https://cesnet.cz")),
 		}
 	)