Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Nov 14, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelog/unreleased/fix-ocm-external-idp.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
16 changes: 8 additions & 8 deletions internal/grpc/services/ocminvitemanager/ocminvitemanager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions internal/grpc/services/ocmshareprovider/ocmshareprovider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion internal/http/services/ocmd/invites.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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 {
Expand Down
11 changes: 3 additions & 8 deletions pkg/ocm/user/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package user
import (
"encoding/base64"
"fmt"
"net/url"
"strings"

userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
Expand All @@ -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,
}
}

Expand Down
4 changes: 2 additions & 2 deletions tests/integration/grpc/ocm_invitation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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("4c510ada-c86b-4815-8820-42cdf82c3d51@cernbox.cern.ch")),
OpaqueId: base64.URLEncoding.EncodeToString([]byte("4c510ada-c86b-4815-8820-42cdf82c3d51@https://cernbox.cern.ch")),
},
Username: "einstein",
Mail: "einstein@cern.ch",
Expand All @@ -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("f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c@cesnet.cz")),
OpaqueId: base64.URLEncoding.EncodeToString([]byte("f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c@https://cesnet.cz")),
},
Username: "marie",
Mail: "marie@cesnet.cz",
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/grpc/ocm_share_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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("4c510ada-c86b-4815-8820-42cdf82c3d51@cernbox.cern.ch")),
OpaqueId: base64.URLEncoding.EncodeToString([]byte("4c510ada-c86b-4815-8820-42cdf82c3d51@https://cernbox.cern.ch")),
}
marie = &userpb.User{
Id: &userpb.UserId{
Expand All @@ -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("f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c@cesnet.cz")),
OpaqueId: base64.URLEncoding.EncodeToString([]byte("f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c@https://cesnet.cz")),
}
)

Expand Down
Loading