Address round-2 PR review: rotation 401 blocker + cleanups

Blocker: the unknown-kid cooldown gate 401'd legitimate tokens during a
real Keycloak key rotation. The SPA fires /api/me, /api/models and
/api/keys concurrently on page load, so on the first load after a
rotation one request won the CAS and fetched while the others got
errKIDRefreshCooldown and were rejected. Move the cooldown check inside
the singleflight callback so concurrent callers block on the one
in-flight fetch and share its result, and re-check the cache in keyFunc
regardless of the refresh outcome. The DoS bound (one outbound fetch per
30s window) is preserved. Add TestRotatedKIDResolvesForConcurrentBurst.

Take the steady-state JWKS refresh off the request path: a synchronous
hourly refresh stalled every concurrent request for up to the fetch
timeout when Keycloak was unreachable as the cache went stale. Move it
to a background ticker (refreshLoop) and drop the now-dead lastFetch.

Other review items:
- Cap the JWKS response body at 1 MiB (io.LimitReader).
- Call validator.Stop() on shutdown.
- Add security headers (X-Frame-Options, CSP frame-ancestors,
  X-Content-Type-Options) and a no-store /config.json block to both
  nginx configs; the UI exposes one-click revoke.
- Template the NebariApp groups mapper claim.name from
  auth.oidc.groupsClaim so it can't drift from what the key-manager reads.
- Comment provider: keycloak pointing at #61.
- Note pinClientAudience is only meaningful with frontend.enabled=true.
- Fix the stale ui-development.md shipping instructions.
- Delete the stale frontend-rewrite-plan.md.
- Drop an em dash in test-auth.sh.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: jbouder

charts/nebari-llm-serving/templates/frontend-configmap.yaml

@@ -62,6 +62,15 @@ data:
             root /usr/share/nginx/html;
             index index.html;
 
+            # Security headers. add_header does NOT inherit into a location that
+            # sets its own add_header, so these cover the SPA document (location
+            # /); the config.json block below repeats the ones that matter for it.
+            # The framing headers matter because the UI exposes one-click "revoke
+            # key" — deny embedding to block clickjacking.
+            add_header X-Frame-Options "DENY" always;
+            add_header Content-Security-Policy "frame-ancestors 'none'" always;
+            add_header X-Content-Type-Options "nosniff" always;
+
             gzip on;
             gzip_types text/plain text/css application/json application/javascript
                        text/xml application/xml application/xml+rss text/javascript
@@ -91,6 +100,14 @@ data:
                 add_header Content-Type text/plain;
             }
 
+            # Runtime config: never cache, so Keycloak settings changes take
+            # effect on the next page load instead of being pinned by a stale copy.
+            location = /config.json {
+                add_header Cache-Control "no-store" always;
+                add_header X-Content-Type-Options "nosniff" always;
+                try_files $uri =404;
+            }
+
             # SPA fallback.
             location / {
                 try_files $uri $uri/ /index.html;

charts/nebari-llm-serving/templates/key-manager-nebariapp.yaml

@@ -33,18 +33,23 @@ spec:
   # provisions the public SPA client but emits no gateway SecurityPolicy.
   auth:
     enabled: true
+    # Hardcoded to keycloak: the key-manager only validates Keycloak-minted
+    # tokens today. generic-oidc support is tracked in nebari-dev/#61.
     provider: keycloak
     enforceAtGateway: false
     scopes:
       {{- toYaml .Values.keyManager.nebariApp.auth.scopes | nindent 6 }}
     # The groups mapper ensures access tokens carry group claims so the
-    # key-manager can filter models by group membership.
+    # key-manager can filter models by group membership. claim.name is derived
+    # from the same value the key-manager reads (auth.oidc.groupsClaim, wired to
+    # LLM_OIDC_GROUPS_CLAIM) so overriding the claim name can't drift the mapper
+    # and the reader out of sync.
     keycloakConfig:
       protocolMappers:
         - name: group-membership
           protocolMapper: oidc-group-membership-mapper
           config:
-            claim.name: groups
+            claim.name: {{ .Values.auth.oidc.groupsClaim | quote }}
             full.path: "false"
             id.token.claim: "true"
             access.token.claim: "true"

charts/nebari-llm-serving/values.yaml

@@ -100,6 +100,11 @@ keyManager:
     # realm (issuer-only validation). The client ID is derived from
     # frontend.keycloak.clientId (or the operator convention), so it always
     # matches the token the SPA actually presents.
+    #
+    # Only meaningful with frontend.enabled=true: the pinned client is the SPA
+    # client the NebariApp provisions, so enabling this on an API-only install
+    # (frontend.enabled=false) pins an `azp` that no token in the realm carries
+    # and every request 401s.
     pinClientAudience: false
   # nebariApp - the NebariApp CR that exposes the key-manager UI via the
   # nebari-operator (HTTPRoute, TLS cert, Keycloak client, landing-page tile).

dev/test-auth.sh

@@ -112,7 +112,7 @@ check() { # name expected actual
   if [[ "$2" == "$3" ]]; then
     echo "  PASS: $1 (HTTP $3)"
   else
-    echo "  FAIL: $1 — expected $2, got $3"
+    echo "  FAIL: $1 - expected $2, got $3"
     FAILED=1
   fi
 }

docs/src/content/docs/ui-development.md

@@ -92,14 +92,14 @@ npm run check   # biome lint + format
 The UI ships as its own image
 (`ghcr.io/nebari-dev/nebari-llm-serving-pack/frontend`, nginx serving the built
 bundle) - it is no longer embedded in the Go key-manager binary. Committing your
-edits to `frontend/` is all that is needed for them to ship in the next image
-build. To see your changes in the actual in-cluster pod (rather than the dev
-server), rebuild and reload:
-
-```bash
-make build-images && make load-images
-kubectl -n llm-operator-system rollout restart deployment/llm-frontend
-```
+edits to `frontend/` is all that is needed for them to ship: CI builds and
+publishes the frontend image on merge, and a chart upgrade rolls it out.
+
+During local development you iterate against the Vite dev server (`make ui` in
+`dev/`), which hot-reloads on save and proxies `/api` to the port-forwarded
+key-manager - there is no in-cluster frontend pod in the dev stack to rebuild or
+restart. (`make build-images` builds only the operator, key-manager, and
+mock-vllm images.)
 
 ## API reference (what the UI calls)
 

frontend-rewrite-plan.md

@@ -36,6 +36,15 @@ http {
         root /usr/share/nginx/html;
         index index.html;
 
+        # Security headers. add_header does NOT inherit into a location that sets
+        # its own add_header, so these cover the SPA document (location /); the
+        # config.json block below repeats the ones that matter for it. The framing
+        # headers matter because the UI exposes one-click "revoke key" — deny
+        # embedding to block clickjacking.
+        add_header X-Frame-Options "DENY" always;
+        add_header Content-Security-Policy "frame-ancestors 'none'" always;
+        add_header X-Content-Type-Options "nosniff" always;
+
         gzip on;
         gzip_types text/plain text/css application/json application/javascript
                    text/xml application/xml application/xml+rss text/javascript
@@ -55,6 +64,14 @@ http {
             add_header Content-Type text/plain;
         }
 
+        # Runtime config: never cache, so Keycloak settings changes take effect on
+        # the next page load instead of being pinned by a stale cached copy.
+        location = /config.json {
+            add_header Cache-Control "no-store" always;
+            add_header X-Content-Type-Options "nosniff" always;
+            try_files $uri =404;
+        }
+
         # SPA fallback.
         location / {
             try_files $uri $uri/ /index.html;