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>

Author: cwiklik

kagenti-operator/internal/keycloak/audience.go

@@ -275,7 +275,22 @@ func (a *Admin) updateAudienceMapperIfNeeded(ctx context.Context, token, realm,
 		mappers[i].Config["included.custom.audience"] = audience
 		return a.putAudienceMapper(ctx, token, realm, scopeID, mappers[i])
 	}
-	return fmt.Errorf("no matching audience mapper found for scope %q (scopeID %s)", scopeName, scopeID)
+
+	// No oidc-audience-mapper found. A mapper with the same name but a different
+	// protocolMapper type caused the 409 conflict. Delete it and recreate correctly.
+	for i := range mappers {
+		if mappers[i].Name != scopeName {
+			continue
+		}
+		if err := a.deleteMapper(ctx, token, realm, scopeID, mappers[i].ID); err != nil {
+			return fmt.Errorf("delete stale mapper %q (id %s): %w", scopeName, mappers[i].ID, err)
+		}
+		return a.ensureAudienceMapper(ctx, token, realm, scopeID, scopeName, audience)
+	}
+
+	// Scope has no mappers at all despite the 409 — Keycloak phantom conflict.
+	// Create the mapper directly via PUT-style POST (Keycloak may accept it on retry).
+	return a.createAudienceMapperDirect(ctx, token, realm, scopeID, scopeName, audience)
 }
 
 func (a *Admin) putAudienceMapper(ctx context.Context, token, realm, scopeID string, mapper protocolMapperRep) error {
@@ -304,6 +319,72 @@ func (a *Admin) putAudienceMapper(ctx context.Context, token, realm, scopeID str
 	return fmt.Errorf("keycloak update audience mapper: status %d: %s", resp.StatusCode, truncate(body, 256))
 }
 
+// createAudienceMapperDirect creates the audience mapper without conflict handling.
+// Used when a prior 409 was spurious (scope's mapper list is empty) — retrying the
+// POST typically succeeds because the phantom conflict has cleared.
+func (a *Admin) createAudienceMapperDirect(ctx context.Context, token, realm, scopeID, scopeName, audience string) error {
+	mapper := protocolMapperRep{
+		Name:            scopeName,
+		Protocol:        "openid-connect",
+		ProtocolMapper:  "oidc-audience-mapper",
+		ConsentRequired: false,
+		Config: map[string]string{
+			"included.custom.audience": audience,
+			"id.token.claim":           "false",
+			"access.token.claim":       "true",
+			"userinfo.token.claim":     "false",
+		},
+	}
+	payload, err := json.Marshal(mapper)
+	if err != nil {
+		return err
+	}
+	base := trimBaseURL(a.BaseURL)
+	endpoint := base + "/admin/realms/" + url.PathEscape(realm) + "/client-scopes/" + url.PathEscape(scopeID) + "/protocol-mappers/models"
+	req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(payload))
+	if err != nil {
+		return err
+	}
+	req.Header.Set("Authorization", "Bearer "+token)
+	req.Header.Set("Content-Type", "application/json")
+
+	resp, err := a.httpc().Do(req)
+	if err != nil {
+		return err
+	}
+	defer func() { _ = resp.Body.Close() }()
+	if resp.StatusCode == http.StatusCreated || resp.StatusCode == http.StatusNoContent {
+		return nil
+	}
+	if resp.StatusCode == http.StatusConflict {
+		// Another concurrent reconcile created the mapper — treat as success.
+		return nil
+	}
+	body, _ := io.ReadAll(resp.Body)
+	return fmt.Errorf("keycloak create audience mapper (retry): status %d: %s", resp.StatusCode, truncate(body, 256))
+}
+
+func (a *Admin) deleteMapper(ctx context.Context, token, realm, scopeID, mapperID string) error {
+	base := trimBaseURL(a.BaseURL)
+	endpoint := base + "/admin/realms/" + url.PathEscape(realm) + "/client-scopes/" +
+		url.PathEscape(scopeID) + "/protocol-mappers/models/" + url.PathEscape(mapperID)
+	req, err := http.NewRequestWithContext(ctx, http.MethodDelete, endpoint, nil)
+	if err != nil {
+		return err
+	}
+	req.Header.Set("Authorization", "Bearer "+token)
+	resp, err := a.httpc().Do(req)
+	if err != nil {
+		return err
+	}
+	defer func() { _ = resp.Body.Close() }()
+	if resp.StatusCode == http.StatusNoContent || resp.StatusCode == http.StatusNotFound {
+		return nil
+	}
+	body, _ := io.ReadAll(resp.Body)
+	return fmt.Errorf("keycloak delete mapper: status %d: %s", resp.StatusCode, truncate(body, 256))
+}
+
 // verifyAudienceMapper is a defense-in-depth check that runs on every reconcile.
 // It GETs the mappers for a scope and ensures the oidc-audience-mapper exists with the
 // correct audience. If the mapper is missing (e.g. due to a prior transient failure),

kagenti-operator/internal/keycloak/audience_test.go

@@ -353,6 +353,217 @@ func TestEnsureAudienceScope_VerifyRecreatesMissingMapper(t *testing.T) {
 	}
 }
 
+// TestEnsureAudienceScope_DeletesWrongTypeMapper verifies that when a mapper with the
+// correct name exists but has the wrong protocolMapper type (not oidc-audience-mapper),
+// the operator deletes it and recreates the correct mapper. This is the fix for #358.
+func TestEnsureAudienceScope_DeletesWrongTypeMapper(t *testing.T) {
+	var deleteMapperCalls, recreatePostCalls int
+	spiffeURI := "spiffe://example.org/ns/ns/sa/wl"
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		path := r.URL.Path
+		switch {
+		case path == testMasterRealmTokenPath:
+			w.Header().Set("Content-Type", "application/json")
+			_ = json.NewEncoder(w).Encode(map[string]string{"access_token": "tok"})
+
+		// Scope already exists
+		case path == "/admin/realms/kagenti/client-scopes" && r.Method == http.MethodGet:
+			_ = json.NewEncoder(w).Encode([]clientScopeListItem{{ID: "scope-123", Name: "agent-ns-wl-aud"}})
+
+		// ensureAudienceMapper POST — 409 conflict (mapper name taken)
+		case strings.Contains(path, "/client-scopes/scope-123/protocol-mappers/models") && r.Method == http.MethodPost:
+			if deleteMapperCalls > 0 {
+				// After delete, the POST succeeds
+				recreatePostCalls++
+				w.WriteHeader(http.StatusCreated)
+			} else {
+				w.WriteHeader(http.StatusConflict)
+			}
+
+		// GET mappers — returns a mapper with wrong type (e.g. "oidc-hardcoded-claim-mapper")
+		case strings.Contains(path, "/client-scopes/scope-123/protocol-mappers/models") && r.Method == http.MethodGet:
+			if deleteMapperCalls > 0 {
+				// After delete+recreate, verify sees the correct mapper
+				_ = json.NewEncoder(w).Encode([]protocolMapperRep{{
+					ID: "m-new", Name: "agent-ns-wl-aud", Protocol: "openid-connect",
+					ProtocolMapper: "oidc-audience-mapper",
+					Config:         map[string]string{"included.custom.audience": spiffeURI},
+				}})
+			} else {
+				_ = json.NewEncoder(w).Encode([]protocolMapperRep{{
+					ID:             "mapper-stale",
+					Name:           "agent-ns-wl-aud",
+					Protocol:       "openid-connect",
+					ProtocolMapper: "oidc-hardcoded-claim-mapper", // WRONG TYPE
+					Config:         map[string]string{"claim.value": "something"},
+				}})
+			}
+
+		// DELETE the stale mapper
+		case strings.Contains(path, "/protocol-mappers/models/mapper-stale") && r.Method == http.MethodDelete:
+			deleteMapperCalls++
+			w.WriteHeader(http.StatusNoContent)
+
+		// Realm default scope
+		case path == "/admin/realms/kagenti/default-default-client-scopes/scope-123" && r.Method == http.MethodPut:
+			w.WriteHeader(http.StatusNoContent)
+
+		default:
+			t.Fatalf("unexpected %s %s", r.Method, path)
+		}
+	}))
+	defer srv.Close()
+
+	a := Admin{BaseURL: srv.URL, HTTPClient: srv.Client()}
+	token, err := a.PasswordGrantToken(context.Background(), "u", "p")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = a.EnsureAudienceScope(context.Background(), token, AudienceParams{
+		Realm:                "kagenti",
+		ClientName:           "ns/wl",
+		AudienceClientID:     spiffeURI,
+		AudienceScopeEnabled: true,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if deleteMapperCalls != 1 {
+		t.Fatalf("expected 1 DELETE call for stale mapper, got %d", deleteMapperCalls)
+	}
+	if recreatePostCalls != 1 {
+		t.Fatalf("expected 1 POST call to recreate mapper after delete, got %d", recreatePostCalls)
+	}
+}
+
+// TestEnsureAudienceScope_PhantomConflictRetry verifies that when the mapper POST
+// returns 409 but the scope's mapper list is empty (phantom Keycloak conflict),
+// the operator retries the POST directly and succeeds. This is the actual scenario
+// observed on Kind clusters where Keycloak reports a conflict for a non-existent mapper.
+func TestEnsureAudienceScope_PhantomConflictRetry(t *testing.T) {
+	var postCalls int
+	spiffeURI := "spiffe://example.org/ns/ns/sa/wl"
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		path := r.URL.Path
+		switch {
+		case path == testMasterRealmTokenPath:
+			w.Header().Set("Content-Type", "application/json")
+			_ = json.NewEncoder(w).Encode(map[string]string{"access_token": "tok"})
+
+		// Scope already exists
+		case path == "/admin/realms/kagenti/client-scopes" && r.Method == http.MethodGet:
+			_ = json.NewEncoder(w).Encode([]clientScopeListItem{{ID: "scope-123", Name: "agent-ns-wl-aud"}})
+
+		// POST mapper — first call returns 409 (phantom), subsequent calls succeed
+		case strings.Contains(path, "/client-scopes/scope-123/protocol-mappers/models") && r.Method == http.MethodPost:
+			postCalls++
+			if postCalls == 1 {
+				w.WriteHeader(http.StatusConflict)
+			} else {
+				w.WriteHeader(http.StatusCreated)
+			}
+
+		// GET mappers — returns empty (the phantom: 409 but no actual mapper)
+		case strings.Contains(path, "/client-scopes/scope-123/protocol-mappers/models") && r.Method == http.MethodGet:
+			if postCalls >= 2 {
+				// After retry POST succeeds, verify sees the mapper
+				_ = json.NewEncoder(w).Encode([]protocolMapperRep{{
+					ID: "m-new", Name: "agent-ns-wl-aud", Protocol: "openid-connect",
+					ProtocolMapper: "oidc-audience-mapper",
+					Config:         map[string]string{"included.custom.audience": spiffeURI},
+				}})
+			} else {
+				_ = json.NewEncoder(w).Encode([]protocolMapperRep{})
+			}
+
+		// Realm default scope
+		case path == "/admin/realms/kagenti/default-default-client-scopes/scope-123" && r.Method == http.MethodPut:
+			w.WriteHeader(http.StatusNoContent)
+
+		default:
+			t.Fatalf("unexpected %s %s", r.Method, path)
+		}
+	}))
+	defer srv.Close()
+
+	a := Admin{BaseURL: srv.URL, HTTPClient: srv.Client()}
+	token, err := a.PasswordGrantToken(context.Background(), "u", "p")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = a.EnsureAudienceScope(context.Background(), token, AudienceParams{
+		Realm:                "kagenti",
+		ClientName:           "ns/wl",
+		AudienceClientID:     spiffeURI,
+		AudienceScopeEnabled: true,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if postCalls != 2 {
+		t.Fatalf("expected 2 POST calls (first 409, then retry succeeds), got %d", postCalls)
+	}
+}
+
+// TestEnsureAudienceScope_ConcurrentCreate409 verifies that when both the initial
+// and retry POST return 409 (concurrent reconcile created the mapper), the operator
+// treats it as success rather than entering an error loop.
+func TestEnsureAudienceScope_ConcurrentCreate409(t *testing.T) {
+	spiffeURI := "spiffe://example.org/ns/ns/sa/wl"
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		path := r.URL.Path
+		switch {
+		case path == testMasterRealmTokenPath:
+			w.Header().Set("Content-Type", "application/json")
+			_ = json.NewEncoder(w).Encode(map[string]string{"access_token": "tok"})
+
+		case path == "/admin/realms/kagenti/client-scopes" && r.Method == http.MethodGet:
+			_ = json.NewEncoder(w).Encode([]clientScopeListItem{{ID: "scope-123", Name: "agent-ns-wl-aud"}})
+
+		// All POSTs return 409 (concurrent reconcile already created it)
+		case strings.Contains(path, "/client-scopes/scope-123/protocol-mappers/models") && r.Method == http.MethodPost:
+			w.WriteHeader(http.StatusConflict)
+
+		// GET mappers — empty during updateAudienceMapperIfNeeded (race: mapper not yet visible)
+		// but present during verifyAudienceMapper (transaction committed)
+		case strings.Contains(path, "/client-scopes/scope-123/protocol-mappers/models") && r.Method == http.MethodGet:
+			_ = json.NewEncoder(w).Encode([]protocolMapperRep{{
+				ID: "m-concurrent", Name: "agent-ns-wl-aud", Protocol: "openid-connect",
+				ProtocolMapper: "oidc-audience-mapper",
+				Config:         map[string]string{"included.custom.audience": spiffeURI},
+			}})
+
+		case path == "/admin/realms/kagenti/default-default-client-scopes/scope-123" && r.Method == http.MethodPut:
+			w.WriteHeader(http.StatusNoContent)
+
+		default:
+			t.Fatalf("unexpected %s %s", r.Method, path)
+		}
+	}))
+	defer srv.Close()
+
+	a := Admin{BaseURL: srv.URL, HTTPClient: srv.Client()}
+	token, err := a.PasswordGrantToken(context.Background(), "u", "p")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = a.EnsureAudienceScope(context.Background(), token, AudienceParams{
+		Realm:                "kagenti",
+		ClientName:           "ns/wl",
+		AudienceClientID:     spiffeURI,
+		AudienceScopeEnabled: true,
+	})
+	if err != nil {
+		t.Fatalf("expected success when concurrent reconcile created mapper, got: %v", err)
+	}
+}
+
 func TestEnsureAudienceScope_Disabled(t *testing.T) {
 	a := Admin{}
 	err := a.EnsureAudienceScope(context.Background(), "t", AudienceParams{AudienceScopeEnabled: false})