Skip to content

Commit 4a02c3e

Browse files
rhaferrhafer
committed
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)
1 parent fc2a742 commit 4a02c3e

File tree

7 files changed

+25
-23
lines changed

7 files changed

+25
-23
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
Bugfix: Fix federated sharing when using an external identity provider
2+
3+
We fixes and issue that caused federated sharing to fail when the identity
4+
provider url did not match the federation provider url.
5+
6+
https://github.com/cs3org/reva/pull/4933

‎internal/grpc/services/ocminvitemanager/ocminvitemanager.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -169,12 +169,15 @@ func (s *service) ForwardInvite(ctx context.Context, req *invitepb.ForwardInvite
169169
return nil, err
170170
}
171171

172+
// Accept the invitation on the remote OCM provider
172173
remoteUser, err := s.ocmClient.InviteAccepted(ctx, ocmEndpoint, &client.InviteAcceptedRequest{
173174
Token: req.InviteToken.GetToken(),
174175
RecipientProvider: s.conf.ProviderDomain,
175-
UserID: user.GetId().GetOpaqueId(),
176-
Email: user.GetMail(),
177-
Name: user.GetDisplayName(),
176+
// The UserID is only a string here. To not loose the IDP information we use the FederatedID encoding
177+
// i.e. base64(UserID@IDP)
178+
UserID: ocmuser.FederatedID(user.GetId(), "").GetOpaqueId(),
179+
Email: user.GetMail(),
180+
Name: user.GetDisplayName(),
178181
})
179182
if err != nil {
180183
switch {
@@ -205,15 +208,14 @@ func (s *service) ForwardInvite(ctx context.Context, req *invitepb.ForwardInvite
205208
// and the remote one (the initiator), so at the end of the invitation workflow they
206209
// know each other
207210

211+
// remoteUser.UserID is the federated ID (just a string), to get a unique CS3 userid
212+
// we're using the provider domain as the IDP part of the ID
208213
remoteUserID := &userpb.UserId{
209214
Type: userpb.UserType_USER_TYPE_FEDERATED,
210215
Idp: req.GetOriginSystemProvider().Domain,
211216
OpaqueId: remoteUser.UserID,
212217
}
213218

214-
// we need to use a unique identifier for federated users
215-
remoteUserID = ocmuser.FederatedID(remoteUserID)
216-
217219
if err := s.repo.AddRemoteUser(ctx, user.Id, &userpb.User{
218220
Id: remoteUserID,
219221
Mail: remoteUser.Email,
@@ -271,8 +273,6 @@ func (s *service) AcceptInvite(ctx context.Context, req *invitepb.AcceptInviteRe
271273
}
272274

273275
remoteUser := req.GetRemoteUser()
274-
// we need to use a unique identifier for federated users
275-
remoteUser.Id = ocmuser.FederatedID(remoteUser.Id)
276276

277277
if err := s.repo.AddRemoteUser(ctx, token.GetUserId(), remoteUser); err != nil {
278278
if errors.Is(err, invite.ErrUserAlreadyAccepted) {

‎internal/grpc/services/ocmshareprovider/ocmshareprovider.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -326,8 +326,8 @@ func (s *service) CreateOCMShare(ctx context.Context, req *ocm.CreateOCMShareReq
326326
shareWith := ocmuser.FormatOCMUser(ocmuser.RemoteID(req.GetGrantee().GetUserId()))
327327

328328
// wrap the local user id in a federated user id
329-
owner := ocmuser.FormatOCMUser(ocmuser.FederatedID(info.Owner))
330-
sender := ocmuser.FormatOCMUser(ocmuser.FederatedID(user.Id))
329+
owner := ocmuser.FormatOCMUser(ocmuser.FederatedID(info.Owner, s.conf.ProviderDomain))
330+
sender := ocmuser.FormatOCMUser(ocmuser.FederatedID(user.Id, s.conf.ProviderDomain))
331331

332332
newShareReq := &client.NewShareRequest{
333333
ShareWith: shareWith,

‎internal/http/services/ocmd/invites.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
3333
"github.com/cs3org/reva/v2/internal/http/services/reqres"
3434
"github.com/cs3org/reva/v2/pkg/appctx"
35+
ocmuser "github.com/cs3org/reva/v2/pkg/ocm/user"
3536
"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
3637
"github.com/cs3org/reva/v2/pkg/utils"
3738
)
@@ -145,7 +146,7 @@ func (h *invitesHandler) AcceptInvite(w http.ResponseWriter, r *http.Request) {
145146
}
146147

147148
if err := json.NewEncoder(w).Encode(&user{
148-
UserID: acceptInviteResponse.UserId.OpaqueId,
149+
UserID: ocmuser.FederatedID(acceptInviteResponse.UserId, "").GetOpaqueId(),
149150
Email: acceptInviteResponse.Email,
150151
Name: acceptInviteResponse.DisplayName,
151152
}); err != nil {

‎pkg/ocm/user/user.go‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package user
33
import (
44
"encoding/base64"
55
"fmt"
6-
"net/url"
76
"strings"
87

98
userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
@@ -12,16 +11,12 @@ import (
1211
// FederatedID creates a federated user id by
1312
// 1. stripping the protocol from the domain and
1413
// 2. base64 encoding the opaque id with the domain to get a unique identifier that cannot collide with other users
15-
func FederatedID(id *userpb.UserId) *userpb.UserId {
16-
// strip protocol from the domain
17-
domain := id.Idp
18-
if u, err := url.Parse(domain); err == nil && u.Host != "" {
19-
domain = u.Host
20-
}
14+
func FederatedID(id *userpb.UserId, domain string) *userpb.UserId {
15+
opaqueId := base64.URLEncoding.EncodeToString([]byte(id.OpaqueId + "@" + id.Idp))
2116
return &userpb.UserId{
2217
Type: userpb.UserType_USER_TYPE_FEDERATED,
2318
Idp: domain,
24-
OpaqueId: base64.URLEncoding.EncodeToString([]byte(id.OpaqueId + "@" + domain)),
19+
OpaqueId: opaqueId,
2520
}
2621
}
2722

‎tests/integration/grpc/ocm_invitation_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ var _ = Describe("ocm invitation workflow", func() {
119119
Id: &userpb.UserId{
120120
Type: userpb.UserType_USER_TYPE_FEDERATED,
121121
Idp: "cernbox.cern.ch",
122-
OpaqueId: base64.URLEncoding.EncodeToString([]byte("[email protected]")),
122+
OpaqueId: base64.URLEncoding.EncodeToString([]byte("4c510ada-c86b-4815-8820-42cdf82c3d51@https://cernbox.cern.ch")),
123123
},
124124
Username: "einstein",
125125
Mail: "[email protected]",
@@ -139,7 +139,7 @@ var _ = Describe("ocm invitation workflow", func() {
139139
Id: &userpb.UserId{
140140
Type: userpb.UserType_USER_TYPE_FEDERATED,
141141
Idp: "cesnet.cz",
142-
OpaqueId: base64.URLEncoding.EncodeToString([]byte("[email protected]")),
142+
OpaqueId: base64.URLEncoding.EncodeToString([]byte("f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c@https://cesnet.cz")),
143143
},
144144
Username: "marie",
145145
Mail: "[email protected]",

‎tests/integration/grpc/ocm_share_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ var _ = Describe("ocm share", func() {
127127
federatedEinsteinID = &userpb.UserId{
128128
Type: userpb.UserType_USER_TYPE_FEDERATED,
129129
Idp: "cernbox.cern.ch",
130-
OpaqueId: base64.URLEncoding.EncodeToString([]byte("[email protected]")),
130+
OpaqueId: base64.URLEncoding.EncodeToString([]byte("4c510ada-c86b-4815-8820-42cdf82c3d51@https://cernbox.cern.ch")),
131131
}
132132
marie = &userpb.User{
133133
Id: &userpb.UserId{
@@ -142,7 +142,7 @@ var _ = Describe("ocm share", func() {
142142
federatedMarieID = &userpb.UserId{
143143
Type: userpb.UserType_USER_TYPE_FEDERATED,
144144
Idp: "cesnet.cz",
145-
OpaqueId: base64.URLEncoding.EncodeToString([]byte("[email protected]")),
145+
OpaqueId: base64.URLEncoding.EncodeToString([]byte("f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c@https://cesnet.cz")),
146146
}
147147
)
148148

0 commit comments

Comments
 (0)