Skip to content

fix: register status.userUID field selector for Session#588

Merged
zachsmith1 merged 1 commit intomainfrom
fix/session-userid-field-selector
Apr 29, 2026
Merged

fix: register status.userUID field selector for Session#588
zachsmith1 merged 1 commit intomainfrom
fix/session-userid-field-selector

Conversation

@zachsmith1
Copy link
Copy Markdown
Contributor

Summary

The milo aggregator pre-validates List field selectors against its own scheme before proxying to aggregated apiservers. Without a registered FieldLabelConversionFunc for the Session GVK, the default convertor rejects status.userUID with:

"status.userUID" is not a known field selector: only "metadata.name", "metadata.namespace"

…and the request never reaches the auth-provider-zitadel apiserver that actually serves sessions. Observed live in staging:

LIST /apis/identity.miloapis.com/v1alpha1/sessions?fieldSelector=status.userUID%3D...  resp:400
                                                                                       ^^^^^^^^

Companion PR datum-cloud/auth-provider-zitadel#105 already added the same registration on the auth-provider-zitadel side, but that wasn't enough — milo is the entry point and needs to allow the selector through too.

Fix

Add a Session registration to identity.Install in pkg/apis/identity/scheme.go, mirroring the existing ServiceAccountKey block. Allows status.userUID (plus the standard metadata.name/metadata.namespace). SAR enforcement is unchanged — the REST handler in auth-provider-zitadel still gates unauthorized cross-user lookups.

Test plan

  • go build ./... clean
  • go test ./pkg/apis/identity/... clean
  • Once merged + rolled to staging milo, fraud-operator's Sessions list returns instead of 400. Verify via kubectl -n datum-system logs deploy/milo-apiserver | grep sessions showing resp:200.

Related

🤖 Generated with Claude Code

The milo aggregator pre-validates List field selectors against its own
scheme before proxying to aggregated apiservers. Without a registered
FieldLabelConversionFunc for the Session GVK, the default convertor
rejects "status.userUID" with `"status.userUID" is not a known field
selector: only "metadata.name", "metadata.namespace"` and the request
never reaches the auth-provider-zitadel apiserver that actually serves
sessions.

Add Session to identity.Install alongside the existing ServiceAccountKey
registration so milo passes the selector through. The auth-provider-
zitadel REST handler still gates cross-user lookups via SAR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@joggrbot
Copy link
Copy Markdown
Contributor

joggrbot Bot commented Apr 29, 2026

📝 Documentation Analysis

All docs are up to date! 🎉


✅ Latest commit analyzed: 8e9843d | Powered by Joggr

@zachsmith1 zachsmith1 merged commit 300fbe6 into main Apr 29, 2026
9 checks passed
@zachsmith1 zachsmith1 deleted the fix/session-userid-field-selector branch April 29, 2026 23:47
zachsmith1 added a commit that referenced this pull request Apr 30, 2026
## Summary

milo's local Sessions REST handler at
`internal/apiserver/identity/sessions/rest.go:43` was building a fresh
empty `ListOptions{}` and **discarding the caller's `FieldSelector`**
before delegating to the `DynamicProvider` backend that proxies to
auth-provider-zitadel. The comment said "ignore selectors; self-scoped
list delegated to provider" — written before the cross-user lookup
pattern landed (#588 /
datum-cloud/auth-provider-zitadel#69).

## Symptom (observed live in staging)

After #588 merged, the field-selector validation 400 went away — but the
request still returned no sessions for the targeted user. Tracing
showed:

- fraud-operator → milo: `LIST
sessions?fieldSelector=status.userUID=<uid>` ✓
- milo local REST handler → backend: `ListOptions{}` (selector dropped)
- backend → auth-provider-zitadel: `LIST sessions?timeout=3s` (no
selector)
- auth-provider-zitadel REST handler: `opts.FieldSelector == nil` →
falls back to `u.GetUID()` → caller is
`system:control@fraud.miloapis.com` (service account, no UID) → `uid=""`
→ 0 sessions

apiserver logs confirmed the empty UID:
```
"Listing sessions" uid=""
"ListSessions: found 0 session(s) for userID=""
URI="/apis/identity.miloapis.com/v1alpha1/sessions?timeout=3s"
```

## Fix

Forward `opts.FieldSelector` (and `opts.LabelSelector` for completeness)
into the `metav1.ListOptions` passed to `Backend.ListSessions`. The
`DynamicProvider` backend already preserves the caller identity via
X-Remote-* headers
(`internal/apiserver/identity/sessions/dynamic.go:108-119`), so
auth-provider-zitadel's REST handler still runs its own SAR check
against milo and rejects unauthorized cross-user reads.

## Test plan

- [x] `go build ./...` clean
- [x] `go test ./internal/apiserver/identity/sessions/...` clean
- [ ] Once merged + rolled to staging milo, fraud-operator's Sessions
list returns the user's actual sessions instead of `0 session(s) for
userID=""`. Verify via `kubectl -n datum-iam-system logs
deploy/auth-provider-zitadel-apiserver | grep "Listing sessions"`
showing `uid=<actual-target-uid>`.

## Related

- #588 — fixed the field-selector 400 at the conversion
layer (necessary precondition; this PR is the missing piece)
- datum-cloud/auth-provider-zitadel#105 — same field-selector
registration on the auth-provider-zitadel scheme
- datum-cloud/auth-provider-zitadel#69 — added the Session REST handler
with the `status.userUID` selector consumer + SAR enforcement
- datum-cloud/fraud#32 — fraud-operator caller that surfaced the issue

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants