Skip to content

feat: source IP and User-Agent from identity Sessions API#32

Merged
zachsmith1 merged 1 commit intomainfrom
feat/sessions-data-source
Apr 29, 2026
Merged

feat: source IP and User-Agent from identity Sessions API#32
zachsmith1 merged 1 commit intomainfrom
feat/sessions-data-source

Conversation

@zachsmith1
Copy link
Copy Markdown
Collaborator

@zachsmith1 zachsmith1 commented Apr 29, 2026

Summary

Replace the per-evaluation AuditLogQuery with a List against identity.miloapis.com/v1alpha1.Sessions (filtered by status.userUID) to source IP and User-Agent for the fraud evaluation pipeline. The Sessions API is served by the auth-provider-zitadel aggregated apiserver landed in datum-cloud/auth-provider-zitadel#69 + datum-cloud/infra#2285.

  • Resolver swap (internal/datasource/resolver.go): resolveAuditLogresolveSession. Sorts results by LastUpdatedAt desc, falls back to CreatedAt desc, takes [0]. Drops the activity package import. Empty list still returns an error so the existing recent-user requeue path in resolveInput continues to fire for brand-new users whose first session hasn't landed yet.
  • Cache bypass (cmd/main.go): Client.Cache.DisableFor: []client.Object{&identityv1alpha1.Session{}} so reads against Session route straight to the API server. The aggregated apiserver implements only Lister/Getter/GracefulDeleter — no Watch — so caching it would fail informer startup and would also be wrong (Sessions are high-churn per-user state).
  • RBAC marker swap (internal/controller/fraudevaluation_controller.go): activity.miloapis.com/auditlogqueries:createidentity.miloapis.com/sessions:list. make manifests regenerates config/rbac/role.yaml.
  • milo bump (go.mod): v0.21.0v0.24.8 for SessionStatus.UserAgent and SessionStatus.LastUpdatedAt.
  • Naming refresh: auditDataRetryDelaysessionDataRetryDelay; "audit data incomplete" log message → "session data incomplete". The recentUserThreshold constant and 5s/2min retry window are preserved.

UserRef.Name is already the user UID — same key the previous AuditLog filter (user.uid == '<name>') used and the same key the Sessions REST handler indexes on, so no FraudEvaluation schema change is needed.

Behavior change

  • IP/UserAgent now come from the user's most-recent zitadel session rather than from the most-recent audit-log event. For users who have logged in at least once this is strictly better data (a real session, not "whoever last touched something on their behalf"). For users with zero sessions, the recent-user requeue path is identical to today.
  • provider.Input shape is unchanged. FingerprintID is intentionally not plumbed through — that's zitadel's internal device ID and not useful to MaxMind.

Test plan

  • go vet ./... clean
  • go build ./... clean
  • go test -count=1 ./... (excluding test/e2e) — all pass
  • make generate manifests — only regenerated config/rbac/role.yaml (already included in commit)
  • e2e in CI (needs kind cluster — not run locally)
  • manual verification post-deploy: trigger a FraudEvaluation for a user with at least one zitadel session and confirm input.ip/input.userAgent populate from the session

🤖 Generated with Claude Code

Replace the per-evaluation AuditLogQuery with a List against
identity.miloapis.com/v1alpha1.Sessions filtered by status.userUID, then
take the most-recent session (LastUpdatedAt desc, falling back to
CreatedAt) and copy IP and UserAgent into provider.Input. The Sessions
API is served by the auth-provider-zitadel aggregated apiserver, so the
manager client is configured with Cache.DisableFor on Session to skip
the informer (Sessions does not implement Watch).

Bumps go.miloapis.com/milo to v0.24.8 for the SessionStatus.UserAgent
and LastUpdatedAt fields, swaps the activity scheme for identity, and
replaces the auditlogqueries:create RBAC with sessions:list. The recent
-user requeue path is preserved unchanged so a brand-new user with no
session yet still gets the same retry window.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zachsmith1 zachsmith1 requested a review from scotwells April 29, 2026 23:01
@zachsmith1 zachsmith1 merged commit 7ed7325 into main Apr 29, 2026
12 checks passed
@zachsmith1 zachsmith1 deleted the feat/sessions-data-source branch April 29, 2026 23:06
zachsmith1 added a commit to datum-cloud/milo that referenced this pull request Apr 29, 2026
## 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

- [x] `go build ./...` clean
- [x] `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

- datum-cloud/auth-provider-zitadel#105 — auth-provider-zitadel side of
the same registration (already merged + rolled)
- datum-cloud/auth-provider-zitadel#69 — added the Session REST handler
with the `status.userUID` selector consumer
- datum-cloud/fraud#32 — fraud-operator caller that surfaced the bug

🤖 Generated with [Claude Code](https://claude.com/claude-code)
zachsmith1 added a commit to datum-cloud/milo 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