Skip to content

Commit 32a37b3

Browse files
zmotsoMykolaMarusenko
authored andcommitted
feat: Add validation for user identity provider (#183)
Problem When assigning a non-existent identity provider, Keycloak returns a 409 Conflict error. This error message is confusing and doesn't indicate that the root cause is a missing identity provider. Change Added explicit identity provider existence validation before attempting to create a user federated identity to prevent confusing 409 Conflict errors and provide more explicit error messages.
1 parent 804061e commit 32a37b3

File tree

7 files changed

+105
-5
lines changed

7 files changed

+105
-5
lines changed

api/v1/keycloakrealmuser_types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ type KeycloakRealmUserSpec struct {
8383
// +optional
8484
PasswordSecret PasswordSecret `json:"passwordSecret,omitempty"`
8585

86-
// IdentityProviders linked to the user.
86+
// IdentityProviders is a list of identity providers aliases linked to the user.
8787
// +nullable
8888
// +optional
8989
IdentityProviders *[]string `json:"identityProviders,omitempty"`

config/crd/bases/v1.edp.epam.com_keycloakrealmusers.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ spec:
8888
nullable: true
8989
type: array
9090
identityProviders:
91-
description: IdentityProviders linked to the user.
91+
description: IdentityProviders is a list of identity providers aliases
92+
linked to the user.
9293
items:
9394
type: string
9495
nullable: true

deploy-templates/_crd_examples/keycloakrealmidentityprovider.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ metadata:
55
spec:
66
realmRef:
77
kind: KeycloakRealm
8-
name: realm
8+
name: keycloakrealm-sample
99
alias: instagram
1010
authenticateByDefault: false
1111
enabled: true

deploy-templates/crds/v1.edp.epam.com_keycloakrealmusers.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ spec:
8888
nullable: true
8989
type: array
9090
identityProviders:
91-
description: IdentityProviders linked to the user.
91+
description: IdentityProviders is a list of identity providers aliases
92+
linked to the user.
9293
items:
9394
type: string
9495
nullable: true

docs/api.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6154,7 +6154,7 @@ KeycloakRealmUserSpec defines the desired state of KeycloakRealmUser.
61546154
<td><b>identityProviders</b></td>
61556155
<td>[]string</td>
61566156
<td>
6157-
IdentityProviders linked to the user.<br/>
6157+
IdentityProviders is a list of identity providers aliases linked to the user.<br/>
61586158
</td>
61596159
<td>false</td>
61606160
</tr><tr>

pkg/client/keycloak/adapter/gocloak_adapter_user.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,15 @@ func (a GoCloakAdapter) addMissingIdentityProviders(
448448
continue
449449
}
450450

451+
exists, err := a.IdentityProviderExists(ctx, realmName, provider)
452+
if err != nil {
453+
return fmt.Errorf("unable to check if identity provider exists: %w", err)
454+
}
455+
456+
if !exists {
457+
return fmt.Errorf("identity provider %s does not exist", provider)
458+
}
459+
451460
federatedIdentity := gocloak.FederatedIdentityRepresentation{
452461
IdentityProvider: &provider,
453462
UserID: &userID,

pkg/client/keycloak/adapter/gocloak_adapter_user_test.go

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,22 @@ func TestGoCloakAdapter_SyncRealmUser(t *testing.T) {
3232
return
3333
}
3434

35+
if strings.Contains(r.URL.Path, "identity-provider/instances/idp1") {
36+
w.WriteHeader(http.StatusOK)
37+
_, err := w.Write([]byte(`{"alias":"idp1"}`))
38+
assert.NoError(t, err)
39+
40+
return
41+
}
42+
43+
if strings.Contains(r.URL.Path, "identity-provider/instances/non-existent-idp") {
44+
w.WriteHeader(http.StatusNotFound)
45+
_, err := w.Write([]byte(`{"error":"idp not found"}`))
46+
assert.NoError(t, err)
47+
48+
return
49+
}
50+
3551
w.WriteHeader(http.StatusOK)
3652
}))
3753

@@ -381,6 +397,79 @@ func TestGoCloakAdapter_SyncRealmUser(t *testing.T) {
381397
assert.Contains(t, err.Error(), "failed to get user")
382398
},
383399
},
400+
{
401+
name: "identity provider does not exist",
402+
userDto: &KeycloakUser{
403+
Username: "user",
404+
Enabled: true,
405+
EmailVerified: true,
406+
Email: "mail@mail.com",
407+
FirstName: "first-name",
408+
LastName: "last-name",
409+
RequiredUserActions: []string{"change-password"},
410+
Roles: []string{"role1"},
411+
Groups: []string{"group1"},
412+
Attributes: map[string]string{"attr1": "attr1value"},
413+
Password: "password",
414+
IdentityProviders: &[]string{"non-existent-idp"},
415+
},
416+
client: func(t *testing.T) *mocks.MockGoCloak {
417+
m := mocks.NewMockGoCloak(t)
418+
419+
m.On("GetUsers", mock.Anything, "", "realm", mock.Anything).
420+
Return(nil, nil)
421+
m.On("CreateUser",
422+
mock.Anything,
423+
"",
424+
"realm",
425+
mock.MatchedBy(func(user gocloak.User) bool {
426+
return assert.Equal(t, "user", *user.Username)
427+
})).
428+
Return("user-id", nil)
429+
m.On("GetRoleMappingByUserID", mock.Anything, "", "realm", "user-id").
430+
Return(&gocloak.MappingsRepresentation{
431+
RealmMappings: &[]gocloak.Role{},
432+
ClientMappings: map[string]*gocloak.ClientMappingsRepresentation{},
433+
}, nil)
434+
m.On("GetRealmRole", mock.Anything, "", "realm", "role1").
435+
Return(&gocloak.Role{
436+
Name: gocloak.StringP("role1"),
437+
ID: gocloak.StringP("role1-id"),
438+
}, nil)
439+
m.On("AddRealmRoleToUser",
440+
mock.Anything,
441+
"",
442+
"realm",
443+
"user-id",
444+
mock.MatchedBy(func(roles []gocloak.Role) bool {
445+
return assert.Len(t, roles, 1) &&
446+
assert.Equal(t, "role1-id", *roles[0].ID)
447+
})).
448+
Return(nil)
449+
m.On("GetGroups",
450+
mock.Anything,
451+
"",
452+
"realm",
453+
mock.Anything).
454+
Return([]*gocloak.Group{{
455+
Name: gocloak.StringP("group1"),
456+
ID: gocloak.StringP("group1-id"),
457+
}}, nil)
458+
m.On("RestyClient").Return(resty.New())
459+
m.On("GetUserFederatedIdentities",
460+
mock.Anything,
461+
"",
462+
"realm",
463+
"user-id").
464+
Return([]*gocloak.FederatedIdentityRepresentation{}, nil)
465+
466+
return m
467+
},
468+
wantErr: func(t require.TestingT, err error, i ...interface{}) {
469+
require.Error(t, err)
470+
assert.Contains(t, err.Error(), "identity provider non-existent-idp does not exist")
471+
},
472+
},
384473
}
385474

386475
for _, tt := range tests {

0 commit comments

Comments
 (0)