ocm: fix federated userid

Signed-off-by: Jörn Friedrich Dreyer <[email protected]>

changelog/unreleased/ocm-fix-federated-userid.md

@@ -0,0 +1,6 @@
+Bugfix: fix OCM userid encoding
+
+We now base64 encode the remote userid and provider as the local federated user id. This allows us to always differentiate them from local users and unpack the encoded user id and provider when making requests to the remote ocm provider.
+
+https://github.com/cs3org/reva/pull/4833
+https://github.com/owncloud/ocis/issues/9927

internal/grpc/services/gateway/ocmshareprovider.go

@@ -51,11 +51,15 @@ func (s *svc) CreateOCMShare(ctx context.Context, req *ocm.CreateOCMShareRequest
 		}, nil
 	}
 
+	// persist the OCM share in the ocm share provider
 	res, err := c.CreateOCMShare(ctx, req)
 	if err != nil {
 		return nil, errors.Wrap(err, "gateway: error calling CreateOCMShare")
 	}
 
+	// add a grant to the storage provider so the share can efficiently be listed
+	// the grant does not grant any permissions. access is granted by the OCM link token
+	// that is used by the public storage provider to impersonate the resource owner
 	status, err := s.addGrant(ctx, req.ResourceId, req.Grantee, req.AccessMethods[0].GetWebdavOptions().Permissions, req.Expiration, nil)
 	switch {
 	case err != nil:

internal/grpc/services/ocminvitemanager/ocminvitemanager.go

@@ -20,6 +20,8 @@ package ocminvitemanager
 
 import (
 	"context"
+	"encoding/base64"
+	"net/url"
 	"time"
 
 	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
@@ -210,6 +212,9 @@ func (s *service) ForwardInvite(ctx context.Context, req *invitepb.ForwardInvite
 		OpaqueId: remoteUser.UserID,
 	}
 
+	// we need to use a unique identifier for federated users
+	remoteUserID = federatedID(remoteUserID)
+
 	if err := s.repo.AddRemoteUser(ctx, user.Id, &userpb.User{
 		Id:          remoteUserID,
 		Mail:        remoteUser.Email,
@@ -240,6 +245,22 @@ func getOCMEndpoint(originProvider *ocmprovider.ProviderInfo) (string, error) {
 	return "", errors.New("ocm endpoint not specified for mesh provider")
 }
 
+// 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
+	}
+	return &userpb.UserId{
+		Type:     userpb.UserType_USER_TYPE_FEDERATED,
+		Idp:      domain,
+		OpaqueId: base64.URLEncoding.EncodeToString([]byte(id.OpaqueId + "@" + domain)),
+	}
+}
+
 func (s *service) AcceptInvite(ctx context.Context, req *invitepb.AcceptInviteRequest) (*invitepb.AcceptInviteResponse, error) {
 	token, err := s.repo.GetToken(ctx, req.InviteToken.Token)
 	if err != nil {
@@ -266,7 +287,11 @@ func (s *service) AcceptInvite(ctx context.Context, req *invitepb.AcceptInviteRe
 		}, nil
 	}
 
-	if err := s.repo.AddRemoteUser(ctx, token.GetUserId(), req.GetRemoteUser()); err != nil {
+	remoteUser := req.GetRemoteUser()
+	// we need to use a unique identifier for federated users
+	remoteUser.Id = federatedID(remoteUser.Id)
+
+	if err := s.repo.AddRemoteUser(ctx, token.GetUserId(), remoteUser); err != nil {
 		if errors.Is(err, invite.ErrUserAlreadyAccepted) {
 			return &invitepb.AcceptInviteResponse{
 				Status: status.NewAlreadyExists(ctx, err, err.Error()),

internal/grpc/services/ocmshareprovider/ocmshareprovider.go

@@ -20,6 +20,7 @@ package ocmshareprovider
 
 import (
 	"context"
+	"encoding/base64"
 	"fmt"
 	"net/url"
 	"path/filepath"
@@ -35,6 +36,7 @@ import (
 	providerpb "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
 	"github.com/cs3org/reva/v2/internal/http/services/ocmd"
+	"github.com/cs3org/reva/v2/pkg/appctx"
 	ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
 	"github.com/cs3org/reva/v2/pkg/errtypes"
 	"github.com/cs3org/reva/v2/pkg/ocm/client"
@@ -157,6 +159,54 @@ func getOCMEndpoint(originProvider *ocmprovider.ProviderInfo) (string, error) {
 	return "", errors.New("ocm endpoint not specified for mesh provider")
 }
 
+// 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
+	}
+	return &userpb.UserId{
+		Type:     userpb.UserType_USER_TYPE_FEDERATED,
+		Idp:      domain,
+		OpaqueId: base64.URLEncoding.EncodeToString([]byte(id.OpaqueId + "@" + domain)),
+	}
+}
+
+// remoteID creates a remote user id by
+// 1. decoding the base64 encoded opaque id
+// 2. splitting the opaque id at the last @ to get the opaque id and the domain
+func remoteID(id *userpb.UserId) *userpb.UserId {
+	bytes, err := base64.URLEncoding.DecodeString(id.GetOpaqueId())
+	if err != nil {
+		return &userpb.UserId{
+			Type:     userpb.UserType_USER_TYPE_PRIMARY,
+			Idp:      id.Idp,
+			OpaqueId: id.OpaqueId,
+		}
+	}
+	remote := string(bytes)
+	last := strings.LastIndex(remote, "@")
+	if last == -1 {
+		return &userpb.UserId{
+			Type:     userpb.UserType_USER_TYPE_PRIMARY,
+			Idp:      id.Idp,
+			OpaqueId: id.OpaqueId,
+		}
+	}
+	opaqueid := remote[:last]
+	idp := remote[last+1:]
+
+	return &userpb.UserId{
+		Type:     userpb.UserType_USER_TYPE_PRIMARY,
+		Idp:      idp,
+		OpaqueId: opaqueid,
+	}
+}
+
+// formatOCMUser formats a user id in the form of <opaque-id>@<idp> used by the OCM API
 func formatOCMUser(u *userpb.UserId) string {
 	return fmt.Sprintf("%s@%s", u.OpaqueId, u.Idp)
 }
@@ -288,6 +338,7 @@ func (s *service) CreateOCMShare(ctx context.Context, req *ocm.CreateOCMShareReq
 		Nanos:   uint32(now % 1000000000),
 	}
 
+	// 1. persist the share in the repository
 	ocmshare := &ocm.Share{
 		Token:         tkn,
 		Name:          filepath.Base(info.Path),
@@ -314,25 +365,29 @@ func (s *service) CreateOCMShare(ctx context.Context, req *ocm.CreateOCMShareReq
 		}, nil
 	}
 
+	// 2. create the share on the remote provider
+	// 2.a get the ocm endpoint of the remote provider
 	ocmEndpoint, err := getOCMEndpoint(req.RecipientMeshProvider)
 	if err != nil {
 		return &ocm.CreateOCMShareResponse{
 			Status: status.NewInvalidArg(ctx, "the selected provider does not have an OCM endpoint"),
 		}, nil
 	}
 
+	// 2.b replace outgoing user ids with ocm user ids
+	// unpack the federated user id
+	shareWith := formatOCMUser(remoteID(req.GetGrantee().GetUserId()))
+
+	// wrap the local user id in a federated user id
+	owner := formatOCMUser(federatedID(info.Owner))
+	sender := formatOCMUser(federatedID(user.Id))
+
 	newShareReq := &client.NewShareRequest{
-		ShareWith:  formatOCMUser(req.Grantee.GetUserId()),
-		Name:       ocmshare.Name,
-		ProviderID: ocmshare.Id.OpaqueId,
-		Owner: formatOCMUser(&userpb.UserId{
-			OpaqueId: info.Owner.OpaqueId,
-			Idp:      s.conf.ProviderDomain,
-		}),
-		Sender: formatOCMUser(&userpb.UserId{
-			OpaqueId: user.Id.OpaqueId,
-			Idp:      s.conf.ProviderDomain,
-		}),
+		ShareWith:         shareWith,
+		Name:              ocmshare.Name,
+		ProviderID:        ocmshare.Id.OpaqueId,
+		Owner:             owner,
+		Sender:            sender,
 		SenderDisplayName: user.DisplayName,
 		ShareType:         "user",
 		ResourceType:      getResourceType(info),
@@ -343,8 +398,14 @@ func (s *service) CreateOCMShare(ctx context.Context, req *ocm.CreateOCMShareReq
 		newShareReq.Expiration = req.Expiration.Seconds
 	}
 
+	// 2.c make POST /shares request
 	newShareRes, err := s.client.NewShare(ctx, ocmEndpoint, newShareReq)
 	if err != nil {
+		err2 := s.repo.DeleteShare(ctx, user, &ocm.ShareReference{Spec: &ocm.ShareReference_Id{Id: ocmshare.Id}})
+		if err2 != nil {
+			appctx.GetLogger(ctx).Error().Err(err2).Str("shareid", ocmshare.GetId().GetOpaqueId()).Msg("could not delete local ocm share")
+		}
+		// TODO remove the share from the local storage
 		switch {
 		case errors.Is(err, client.ErrInvalidParameters):
 			return &ocm.CreateOCMShareResponse{

tests/integration/grpc/fixtures/ocm-share/ocm-users.demo.json

@@ -2,7 +2,7 @@
 	{
 		"id": {
 			"opaque_id": "4c510ada-c86b-4815-8820-42cdf82c3d51",
-			"idp": "cernbox.cern.ch",
+			"idp": "https://cernbox.cern.ch",
 			"type": 1
 		},
 		"username": "einstein",
@@ -32,7 +32,7 @@
 	{
 		"id": {
 			"opaque_id": "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c",
-			"idp": "cesnet.cz",
+			"idp": "https://cesnet.cz",
 			"type": 1
 		},
 		"username": "marie",

tests/integration/grpc/fixtures/ocm-users.demo.json

@@ -2,7 +2,7 @@
 	{
 		"id": {
 			"opaque_id": "4c510ada-c86b-4815-8820-42cdf82c3d51",
-			"idp": "cernbox.cern.ch",
+			"idp": "https://cernbox.cern.ch",
 			"type": 1
 		},
 		"username": "einstein",
@@ -32,7 +32,7 @@
 	{
 		"id": {
 			"opaque_id": "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c",
-			"idp": "cesnet.cz",
+			"idp": "https://cesnet.cz",
 			"type": 1
 		},
 		"username": "marie",

tests/integration/grpc/ocm_share_test.go

@@ -21,6 +21,7 @@ package grpc_test
 import (
 	"bytes"
 	"context"
+	"encoding/base64"
 	"io"
 	"net/http"
 	"path/filepath"
@@ -116,27 +117,32 @@ var _ = Describe("ocm share", func() {
 		einstein = &userpb.User{
 			Id: &userpb.UserId{
 				OpaqueId: "4c510ada-c86b-4815-8820-42cdf82c3d51",
-				Idp:      "cernbox.cern.ch",
+				Idp:      "https://cernbox.cern.ch",
 				Type:     userpb.UserType_USER_TYPE_PRIMARY,
 			},
 			Username:    "einstein",
 			Mail:        "[email protected]",
 			DisplayName: "Albert Einstein",
 		}
+		federatedEinsteinID = &userpb.UserId{
+			Type:     userpb.UserType_USER_TYPE_FEDERATED,
+			Idp:      "cernbox.cern.ch",
+			OpaqueId: base64.URLEncoding.EncodeToString([]byte("[email protected]")),
+		}
 		marie = &userpb.User{
 			Id: &userpb.UserId{
 				OpaqueId: "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c",
-				Idp:      "cesnet.cz",
+				Idp:      "https://cesnet.cz",
 				Type:     userpb.UserType_USER_TYPE_PRIMARY,
 			},
 			Username:    "marie",
 			Mail:        "[email protected]",
 			DisplayName: "Marie Curie",
 		}
 		federatedMarieID = &userpb.UserId{
-			OpaqueId: marie.Id.OpaqueId,
-			Idp:      marie.Id.Idp,
 			Type:     userpb.UserType_USER_TYPE_FEDERATED,
+			Idp:      "cesnet.cz",
+			OpaqueId: base64.URLEncoding.EncodeToString([]byte("[email protected]")),
 		}
 	)
 
@@ -192,16 +198,25 @@ var _ = Describe("ocm share", func() {
 
 	Describe("marie has already accepted the invitation workflow", func() {
 		JustBeforeEach(func() {
+			// einstein generates an invite token
 			tknRes, err := cernboxgw.GenerateInviteToken(ctxEinstein, &invitev1beta1.GenerateInviteTokenRequest{})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(tknRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
 
+			// marie accepts it and her provider forwards the invite back to the instance of einstein
 			invRes, err := cesnetgw.ForwardInvite(ctxMarie, &invitev1beta1.ForwardInviteRequest{
 				InviteToken:          tknRes.InviteToken,
 				OriginSystemProvider: cernbox,
 			})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(invRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+			// Make sure the user is a federated user
+			// The user type must be a federated user
+			Expect(invRes.UserId.Type).To(Equal(userpb.UserType_USER_TYPE_FEDERATED))
+			// Federated users use the OCM provider id which MUST NOT contain the protocol
+			Expect(invRes.UserId.Idp).To(Equal("cernbox.cern.ch"))
+			// The OpaqueId is the base64 encoded user id and the provider id to provent collisions with other users on the graph API
+			Expect(invRes.UserId.OpaqueId).To(Equal(federatedEinsteinID.OpaqueId))
 		})
 
 		Context("einstein shares a file with view permissions", func() {