Skip to content

Commit 6abaa5e

Browse files
committed
fix(keycloak): delete wrong-type mapper before recreating audience mapper (#358)
When a protocol mapper exists with the correct name but the wrong type (not oidc-audience-mapper), the POST returns 409 and updateAudienceMapperIfNeeded fails to find a matching mapper — entering an infinite error loop that blocks audience scope propagation. ## Root Cause The 409 Conflict from Keycloak means "a mapper with that name already exists." But updateAudienceMapperIfNeeded only looks for mappers matching BOTH Name == scopeName AND ProtocolMapper == "oidc-audience-mapper". When the existing mapper has the right name but wrong type, the loop skips it and falls through to "no matching audience mapper found." This also prevents verifyAudienceMapper (defense-in-depth from PR #350) from running, since getOrCreateAudienceClientScope returns early on the error — no self-healing is possible. ## Fix In updateAudienceMapperIfNeeded, after failing to find an oidc-audience-mapper, perform a second pass looking for any mapper with a matching name (regardless of type). If found, DELETE it via the Keycloak Admin API, then re-POST the correct oidc-audience-mapper. This is the minimal targeted fix — it handles the exact broken state (wrong-type name collision) without restructuring the flow. ## Observed Symptoms - Operator logs: "ensure audience mapper for existing scope ... no matching audience mapper found" repeating every few seconds - Agent tokens lack the correct audience claim - AuthBridge/Envoy rejects requests with 401 Unauthorized - Affects fresh installs with operator v0.2.0-rc.4 Fixes #358 Signed-off-by: cwiklik <cwiklik@users.noreply.github.com> Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: cwiklik <cwiklikj@gmail.com>
1 parent e8882b4 commit 6abaa5e

2 files changed

Lines changed: 118 additions & 0 deletions

File tree

kagenti-operator/internal/keycloak/audience.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,18 @@ func (a *Admin) updateAudienceMapperIfNeeded(ctx context.Context, token, realm,
275275
mappers[i].Config["included.custom.audience"] = audience
276276
return a.putAudienceMapper(ctx, token, realm, scopeID, mappers[i])
277277
}
278+
279+
// No oidc-audience-mapper found. A mapper with the same name but a different
280+
// protocolMapper type caused the 409 conflict. Delete it and recreate correctly.
281+
for i := range mappers {
282+
if mappers[i].Name != scopeName {
283+
continue
284+
}
285+
if err := a.deleteMapper(ctx, token, realm, scopeID, mappers[i].ID); err != nil {
286+
return fmt.Errorf("delete stale mapper %q (id %s): %w", scopeName, mappers[i].ID, err)
287+
}
288+
return a.ensureAudienceMapper(ctx, token, realm, scopeID, scopeName, audience)
289+
}
278290
return fmt.Errorf("no matching audience mapper found for scope %q (scopeID %s)", scopeName, scopeID)
279291
}
280292

@@ -304,6 +316,27 @@ func (a *Admin) putAudienceMapper(ctx context.Context, token, realm, scopeID str
304316
return fmt.Errorf("keycloak update audience mapper: status %d: %s", resp.StatusCode, truncate(body, 256))
305317
}
306318

319+
func (a *Admin) deleteMapper(ctx context.Context, token, realm, scopeID, mapperID string) error {
320+
base := trimBaseURL(a.BaseURL)
321+
endpoint := base + "/admin/realms/" + url.PathEscape(realm) + "/client-scopes/" +
322+
url.PathEscape(scopeID) + "/protocol-mappers/models/" + url.PathEscape(mapperID)
323+
req, err := http.NewRequestWithContext(ctx, http.MethodDelete, endpoint, nil)
324+
if err != nil {
325+
return err
326+
}
327+
req.Header.Set("Authorization", "Bearer "+token)
328+
resp, err := a.httpc().Do(req)
329+
if err != nil {
330+
return err
331+
}
332+
defer func() { _ = resp.Body.Close() }()
333+
if resp.StatusCode == http.StatusNoContent || resp.StatusCode == http.StatusNotFound {
334+
return nil
335+
}
336+
body, _ := io.ReadAll(resp.Body)
337+
return fmt.Errorf("keycloak delete mapper: status %d: %s", resp.StatusCode, truncate(body, 256))
338+
}
339+
307340
// verifyAudienceMapper is a defense-in-depth check that runs on every reconcile.
308341
// It GETs the mappers for a scope and ensures the oidc-audience-mapper exists with the
309342
// correct audience. If the mapper is missing (e.g. due to a prior transient failure),

kagenti-operator/internal/keycloak/audience_test.go

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,91 @@ func TestEnsureAudienceScope_VerifyRecreatesMissingMapper(t *testing.T) {
353353
}
354354
}
355355

356+
// TestEnsureAudienceScope_DeletesWrongTypeMapper verifies that when a mapper with the
357+
// correct name exists but has the wrong protocolMapper type (not oidc-audience-mapper),
358+
// the operator deletes it and recreates the correct mapper. This is the fix for #358.
359+
func TestEnsureAudienceScope_DeletesWrongTypeMapper(t *testing.T) {
360+
var deleteMapperCalls, recreatePostCalls int
361+
spiffeURI := "spiffe://example.org/ns/ns/sa/wl"
362+
363+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
364+
path := r.URL.Path
365+
switch {
366+
case path == testMasterRealmTokenPath:
367+
w.Header().Set("Content-Type", "application/json")
368+
_ = json.NewEncoder(w).Encode(map[string]string{"access_token": "tok"})
369+
370+
// Scope already exists
371+
case path == "/admin/realms/kagenti/client-scopes" && r.Method == http.MethodGet:
372+
_ = json.NewEncoder(w).Encode([]clientScopeListItem{{ID: "scope-123", Name: "agent-ns-wl-aud"}})
373+
374+
// ensureAudienceMapper POST — 409 conflict (mapper name taken)
375+
case strings.Contains(path, "/client-scopes/scope-123/protocol-mappers/models") && r.Method == http.MethodPost:
376+
if deleteMapperCalls > 0 {
377+
// After delete, the POST succeeds
378+
recreatePostCalls++
379+
w.WriteHeader(http.StatusCreated)
380+
} else {
381+
w.WriteHeader(http.StatusConflict)
382+
}
383+
384+
// GET mappers — returns a mapper with wrong type (e.g. "oidc-hardcoded-claim-mapper")
385+
case strings.Contains(path, "/client-scopes/scope-123/protocol-mappers/models") && r.Method == http.MethodGet:
386+
if deleteMapperCalls > 0 {
387+
// After delete+recreate, verify sees the correct mapper
388+
_ = json.NewEncoder(w).Encode([]protocolMapperRep{{
389+
ID: "m-new", Name: "agent-ns-wl-aud", Protocol: "openid-connect",
390+
ProtocolMapper: "oidc-audience-mapper",
391+
Config: map[string]string{"included.custom.audience": spiffeURI},
392+
}})
393+
} else {
394+
_ = json.NewEncoder(w).Encode([]protocolMapperRep{{
395+
ID: "mapper-stale",
396+
Name: "agent-ns-wl-aud",
397+
Protocol: "openid-connect",
398+
ProtocolMapper: "oidc-hardcoded-claim-mapper", // WRONG TYPE
399+
Config: map[string]string{"claim.value": "something"},
400+
}})
401+
}
402+
403+
// DELETE the stale mapper
404+
case strings.Contains(path, "/protocol-mappers/models/mapper-stale") && r.Method == http.MethodDelete:
405+
deleteMapperCalls++
406+
w.WriteHeader(http.StatusNoContent)
407+
408+
// Realm default scope
409+
case path == "/admin/realms/kagenti/default-default-client-scopes/scope-123" && r.Method == http.MethodPut:
410+
w.WriteHeader(http.StatusNoContent)
411+
412+
default:
413+
t.Fatalf("unexpected %s %s", r.Method, path)
414+
}
415+
}))
416+
defer srv.Close()
417+
418+
a := Admin{BaseURL: srv.URL, HTTPClient: srv.Client()}
419+
token, err := a.PasswordGrantToken(context.Background(), "u", "p")
420+
if err != nil {
421+
t.Fatal(err)
422+
}
423+
424+
err = a.EnsureAudienceScope(context.Background(), token, AudienceParams{
425+
Realm: "kagenti",
426+
ClientName: "ns/wl",
427+
AudienceClientID: spiffeURI,
428+
AudienceScopeEnabled: true,
429+
})
430+
if err != nil {
431+
t.Fatal(err)
432+
}
433+
if deleteMapperCalls != 1 {
434+
t.Fatalf("expected 1 DELETE call for stale mapper, got %d", deleteMapperCalls)
435+
}
436+
if recreatePostCalls != 1 {
437+
t.Fatalf("expected 1 POST call to recreate mapper after delete, got %d", recreatePostCalls)
438+
}
439+
}
440+
356441
func TestEnsureAudienceScope_Disabled(t *testing.T) {
357442
a := Admin{}
358443
err := a.EnsureAudienceScope(context.Background(), "t", AudienceParams{AudienceScopeEnabled: false})

0 commit comments

Comments
 (0)