Skip to content

Commit bca8317

Browse files
everettraveneverettraven
committed
fixup! address review comments
Signed-off-by: Bryce Palmer <[email protected]>
1 parent e72069c commit bca8317

File tree

2 files changed

+147
-99
lines changed

2 files changed

+147
-99
lines changed

‎test/extended/authentication/keycloak_client.go‎

Lines changed: 141 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"bytes"
55
"crypto/tls"
66
"encoding/json"
7-
"errors"
87
"fmt"
98
"io"
109
"net/http"
@@ -43,11 +42,15 @@ func keycloakClientFor(keycloakURL string) (*keycloakClient, error) {
4342
}, nil
4443
}
4544

45+
type group struct {
46+
Name string `json:"name"`
47+
}
48+
4649
func (kc *keycloakClient) CreateGroup(name string) error {
4750
groupURL := kc.adminURL.JoinPath("groups")
4851

49-
group := map[string]interface{}{
50-
"name": name,
52+
group := group{
53+
Name: name,
5154
}
5255

5356
groupBytes, err := json.Marshal(group)
@@ -69,20 +72,41 @@ func (kc *keycloakClient) CreateGroup(name string) error {
6972
return nil
7073
}
7174

75+
type user struct {
76+
Username string `json:"username"`
77+
Email string `json:"email"`
78+
Enabled bool `json:"enabled"`
79+
EmailVerified bool `json:"emailVerified"`
80+
Groups []string `json:"groups"`
81+
Credentials []credential `json:"credentials"`
82+
}
83+
84+
type credential struct {
85+
Temporary bool `json:"temporary"`
86+
Type credentialType `json:"type"`
87+
Value string `json:"value"`
88+
}
89+
90+
type credentialType string
91+
92+
const (
93+
credentialTypePassword credentialType = "password"
94+
)
95+
7296
func (kc *keycloakClient) CreateUser(username, password string, groups ...string) error {
7397
userURL := kc.adminURL.JoinPath("users")
7498

75-
user := map[string]interface{}{
76-
"username": username,
77-
"email": fmt.Sprintf("%[email protected]", username),
78-
"enabled": true,
79-
"emailVerified": true,
80-
"groups": groups,
81-
"credentials": []map[string]interface{}{
99+
user := user{
100+
Username: username,
101+
Email: fmt.Sprintf("%[email protected]", username),
102+
Enabled: true,
103+
EmailVerified: true,
104+
Groups: groups,
105+
Credentials: []credential{
82106
{
83-
"temporary": false,
84-
"type": "password",
85-
"value": password,
107+
Temporary: true,
108+
Type: credentialTypePassword,
109+
Value: password,
86110
},
87111
},
88112
}
@@ -106,14 +130,18 @@ func (kc *keycloakClient) CreateUser(username, password string, groups ...string
106130
return nil
107131
}
108132

133+
type authenticationResponse struct {
134+
AccessToken string `json:"access_token"`
135+
IDToken string `json:"id_token"`
136+
}
137+
109138
func (kc *keycloakClient) Authenticate(clientID, username, password string) error {
110-
data := url.Values{
111-
"username": []string{username},
112-
"password": []string{password},
113-
"grant_type": []string{"password"},
114-
"client_id": []string{clientID},
115-
"scope": []string{"openid"},
116-
}
139+
data := url.Values{}
140+
data.Set("username", username)
141+
data.Set("password", password)
142+
data.Set("grant_type", "password")
143+
data.Set("client_id", clientID)
144+
data.Set("scope", "openid")
117145

118146
tokenURL := *kc.adminURL
119147
tokenURL.Path = fmt.Sprintf("/realms/%s/protocol/openid-connect/token", kc.realm)
@@ -124,38 +152,15 @@ func (kc *keycloakClient) Authenticate(clientID, username, password string) erro
124152
}
125153
defer resp.Body.Close()
126154

127-
respBody := map[string]interface{}{}
128-
respBodyData, err := io.ReadAll(resp.Body)
129-
if err != nil {
130-
return fmt.Errorf("reading response data: %w", err)
131-
}
155+
respBody := &authenticationResponse{}
132156

133-
err = json.Unmarshal(respBodyData, &respBody)
157+
err = json.NewDecoder(resp.Body).Decode(respBody)
134158
if err != nil {
135-
return fmt.Errorf("unmarshalling response body %s: %w", respBodyData, err)
136-
}
137-
138-
accessTokenData, ok := respBody["access_token"]
139-
if !ok {
140-
return errors.New("unable to extract access token from the response body: access_token field is missing")
141-
}
142-
143-
accessToken, ok := accessTokenData.(string)
144-
if !ok {
145-
return fmt.Errorf("expected accessToken to be of type string but was %T", accessTokenData)
146-
}
147-
kc.accessToken = accessToken
148-
149-
idTokenData, ok := respBody["id_token"]
150-
if !ok {
151-
return errors.New("unable to extract id token from the response body: id_token field is missing")
159+
return fmt.Errorf("unmarshalling response data: %w", err)
152160
}
153161

154-
idToken, ok := idTokenData.(string)
155-
if !ok {
156-
return fmt.Errorf("expected idToken to be of type string but was %T", idTokenData)
157-
}
158-
kc.idToken = idToken
162+
kc.accessToken = respBody.AccessToken
163+
kc.idToken = respBody.IDToken
159164

160165
return nil
161166
}
@@ -191,41 +196,66 @@ func (kc *keycloakClient) ConfigureClient(clientId string) error {
191196
return fmt.Errorf("getting client %q: %w", clientId, err)
192197
}
193198

194-
id, ok := client["id"]
195-
if !ok {
196-
return fmt.Errorf("client %q doesn't have 'id'", clientId)
197-
}
198-
199-
idStr, ok := id.(string)
200-
if !ok {
201-
return fmt.Errorf("client %q 'id' is not of type string: %T", clientId, id)
202-
}
203-
204-
if err := kc.CreateClientGroupMapper(idStr, "test-groups-mapper", "groups"); err != nil {
199+
if err := kc.CreateClientGroupMapper(client.ID, "test-groups-mapper", "groups"); err != nil {
205200
return fmt.Errorf("creating group mapper for client %q: %w", clientId, err)
206201
}
207202

208-
if err := kc.CreateClientAudienceMapper(idStr, "test-aud-mapper"); err != nil {
203+
if err := kc.CreateClientAudienceMapper(client.ID, "test-aud-mapper"); err != nil {
209204
return fmt.Errorf("creating audience mapper for client %q: %w", clientId, err)
210205
}
211206

212207
return nil
213208
}
214209

210+
type groupMapper struct {
211+
Name string `json:"name"`
212+
Protocol protocol `json:"protocol"`
213+
ProtocolMapper protocolMapper `json:"protocolMapper"`
214+
Config groupMapperConfig `json:"config"`
215+
}
216+
217+
type protocol string
218+
219+
const (
220+
protocolOpenIDConnect protocol = "openid-connect"
221+
)
222+
223+
type protocolMapper string
224+
225+
const (
226+
protocolMapperOpenIDConnectGroupMembership protocolMapper = "oidc-group-membership-mapper"
227+
protocolMapperOpenIDConnectAudience protocolMapper = "oidc-audience-mapper"
228+
)
229+
230+
type groupMapperConfig struct {
231+
FullPath booleanString `json:"full.path"`
232+
IDTokenClaim booleanString `json:"id.token.claim"`
233+
AccessTokenClaim booleanString `json:"access.token.claim"`
234+
UserInfoTokenClaim booleanString `json:"userinfo.token.claim"`
235+
ClaimName string `json:"claim.name"`
236+
}
237+
238+
type booleanString string
239+
240+
const (
241+
booleanStringTrue booleanString = "true"
242+
booleanStringFalse booleanString = "false"
243+
)
244+
215245
func (kc *keycloakClient) CreateClientGroupMapper(clientId, name, claim string) error {
216246
mappersURL := *kc.adminURL
217247
mappersURL.Path += fmt.Sprintf("/clients/%s/protocol-mappers/models", clientId)
218248

219-
mapper := map[string]interface{}{
220-
"name": name,
221-
"protocol": "openid-connect",
222-
"protocolMapper": "oidc-group-membership-mapper", // protocol-mapper type provided by Keycloak
223-
"config": map[string]string{
224-
"full.path": "false",
225-
"id.token.claim": "true",
226-
"access.token.claim": "true",
227-
"userinfo.token.claim": "true",
228-
"claim.name": claim,
249+
mapper := &groupMapper{
250+
Name: name,
251+
Protocol: protocolOpenIDConnect,
252+
ProtocolMapper: protocolMapperOpenIDConnectGroupMembership,
253+
Config: groupMapperConfig{
254+
FullPath: booleanStringFalse,
255+
IDTokenClaim: booleanStringTrue,
256+
AccessTokenClaim: booleanStringTrue,
257+
UserInfoTokenClaim: booleanStringTrue,
258+
ClaimName: claim,
229259
},
230260
}
231261

@@ -249,21 +279,36 @@ func (kc *keycloakClient) CreateClientGroupMapper(clientId, name, claim string)
249279
return nil
250280
}
251281

282+
type audienceMapper struct {
283+
Name string `json:"name"`
284+
Protocol protocol `json:"protocol"`
285+
ProtocolMapper protocolMapper `json:"protocolMapper"`
286+
Config audienceMapperConfig `json:"config"`
287+
}
288+
289+
type audienceMapperConfig struct {
290+
IDTokenClaim booleanString `json:"id.token.claim"`
291+
AccessTokenClaim booleanString `json:"access.token.claim"`
292+
IntrospectionTokenClaim booleanString `json:"introspection.token.claim"`
293+
IncludedClientAudience string `json:"included.client.audience"`
294+
IncludedCustomAudience string `json:"included.custom.audience"`
295+
LightweightClaim booleanString `json:"lightweight.claim"`
296+
}
297+
252298
func (kc *keycloakClient) CreateClientAudienceMapper(clientId, name string) error {
253299
mappersURL := *kc.adminURL
254300
mappersURL.Path += fmt.Sprintf("/clients/%s/protocol-mappers/models", clientId)
255301

256-
mapper := map[string]interface{}{
257-
"name": name,
258-
"protocol": "openid-connect",
259-
"protocolMapper": "oidc-audience-mapper", // protocol-mapper type provided by Keycloak
260-
"config": map[string]string{
261-
"id.token.claim": "false",
262-
"access.token.claim": "true",
263-
"introspection.token.claim": "true",
264-
"included.client.audience": "admin-cli",
265-
"included.custom.audience": "",
266-
"lightweight.claim": "false",
302+
mapper := &audienceMapper{
303+
Name: name,
304+
Protocol: protocolOpenIDConnect,
305+
ProtocolMapper: protocolMapperOpenIDConnectAudience,
306+
Config: audienceMapperConfig{
307+
IDTokenClaim: booleanStringFalse,
308+
AccessTokenClaim: booleanStringTrue,
309+
IntrospectionTokenClaim: booleanStringTrue,
310+
IncludedClientAudience: "admin-cli",
311+
LightweightClaim: booleanStringFalse,
267312
},
268313
}
269314

@@ -287,8 +332,13 @@ func (kc *keycloakClient) CreateClientAudienceMapper(clientId, name string) erro
287332
return nil
288333
}
289334

335+
type client struct {
336+
ClientID string `json:"clientID"`
337+
ID string `json:"id"`
338+
}
339+
290340
// ListClients retrieves all clients
291-
func (kc *keycloakClient) ListClients() ([]map[string]interface{}, error) {
341+
func (kc *keycloakClient) ListClients() ([]client, error) {
292342
clientsURL := *kc.adminURL
293343
clientsURL.Path += "/clients"
294344

@@ -298,29 +348,28 @@ func (kc *keycloakClient) ListClients() ([]map[string]interface{}, error) {
298348
}
299349
defer resp.Body.Close()
300350

301-
respBytes, err := io.ReadAll(resp.Body)
302-
if err != nil {
303-
return nil, err
304-
}
305351
if resp.StatusCode != http.StatusOK {
306-
return nil, fmt.Errorf("listing clients failed: %s: %s", resp.Status, respBytes)
352+
return nil, fmt.Errorf("listing clients failed: %s", resp.Status)
307353
}
308354

309-
clients := []map[string]interface{}{}
310-
err = json.Unmarshal(respBytes, &clients)
355+
clients := []client{}
356+
err = json.NewDecoder(resp.Body).Decode(&clients)
357+
if err != nil {
358+
return nil, fmt.Errorf("unmarshalling response data: %w", err)
359+
}
311360

312361
return clients, err
313362
}
314363

315-
func (kc *keycloakClient) GetClientByClientID(clientID string) (map[string]interface{}, error) {
364+
func (kc *keycloakClient) GetClientByClientID(clientID string) (*client, error) {
316365
clients, err := kc.ListClients()
317366
if err != nil {
318367
return nil, err
319368
}
320369

321370
for _, c := range clients {
322-
if c["clientId"].(string) == clientID {
323-
return c, nil
371+
if c.ClientID == clientID {
372+
return &c, nil
324373
}
325374
}
326375

‎test/extended/authentication/oidc.go‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package authentication
33
import (
44
"context"
55
"encoding/json"
6-
"errors"
76
"fmt"
87
"strings"
98
"time"
@@ -17,6 +16,8 @@ import (
1716
authnv1 "k8s.io/api/authentication/v1"
1817
corev1 "k8s.io/api/core/v1"
1918
apierrors "k8s.io/apimachinery/pkg/api/errors"
19+
"k8s.io/apimachinery/pkg/util/errors"
20+
2021
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2122
"k8s.io/apimachinery/pkg/runtime"
2223
"k8s.io/apimachinery/pkg/util/rand"
@@ -64,7 +65,7 @@ var _ = g.Describe("[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow]
6465
keycloakCli, err = keycloakClientFor(kcURL)
6566
o.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error creating a keycloak client")
6667

67-
// First authenticate as the admin keyloak user so we can add new groups and users
68+
// First authenticate as the admin keycloak user so we can add new groups and users
6869
err = keycloakCli.Authenticate("admin-cli", keycloakAdminUsername, keycloakAdminPassword)
6970
o.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error authenticating as keycloak admin")
7071

@@ -379,12 +380,10 @@ func removeResources(ctx context.Context, removalFuncs ...removalFunc) error {
379380
continue
380381
}
381382
err := removal(ctx)
382-
if err != nil && !apierrors.IsNotFound(err) {
383-
errs = append(errs, err)
384-
}
383+
errs = append(errs, err)
385384
}
386385

387-
return errors.Join(errs...)
386+
return errors.FilterOut(errors.NewAggregate(errs), apierrors.IsNotFound)
388387
}
389388

390389
func configureOIDCAuthentication(ctx context.Context, client *exutil.CLI, keycloakNS string, modifier func(*configv1.OIDCProvider)) (*configv1.Authentication, *configv1.Authentication, error) {
@@ -508,7 +507,7 @@ func resetAuthentication(ctx context.Context, client *exutil.CLI, original *conf
508507
func waitForRollout(ctx context.Context, client *exutil.CLI) {
509508
kasCli := client.AdminOperatorClient().OperatorV1().KubeAPIServers()
510509

511-
// First wait for KAS to flip to progessing
510+
// First wait for KAS to flip to progressing
512511
o.Eventually(func(gomega o.Gomega) {
513512
kas, err := kasCli.Get(ctx, "cluster", metav1.GetOptions{})
514513
gomega.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error fetching the KAS")

0 commit comments

Comments
 (0)