feat(security): mount tenant resolution, scope sessions, wire the chart (#434 stage 3) - #496
Merged
Merged
Conversation
…nt, wire the chart Stage 3 of #434 — the four remaining items. 1. tenantContext is MOUNTED, on the identity surfaces only (/api/v1/security, /api/auth, /api/organizations, /api/invitations, /api/tokens). /health, /metrics and /internal are deliberately excluded: probe and cluster-internal traffic arrives on Service DNS and pod IPs, not a declared tenant host, so requiring a tenant there would fail liveness/readiness the moment a second tenant is declared. 2. Sessions carry their tenant. Every mint stamps `tid`; verification rejects a token whose `tid` is not the tenant serving the request. This is load-bearing: all tenants' sessions are signed with the SAME secret, so without the claim a token minted in one directory is indistinguishable from one minted in another and only the request host — chosen by the token holder — separates them. Pre-tenancy tokens (no `tid`) are accepted only while single-tenant. Logout is covered too, so it cannot delete another directory's session row. 3. Chart wiring. securityService.tenants renders SECURITY_TENANTS plus, per tenant, secretKeyRef-backed SECURITY_TENANT_<ID>_CLIENT_SECRET / _ADMIN_TOKEN / _GOOGLE_CLIENT_SECRET. Secrets are NOT inlined in SECURITY_TENANTS — that renders to a plain env value visible in `kubectl describe pod`, so values reference chart Secret KEYS and the registry reads the secrets from the per-tenant vars. Empty list (default) omits the block entirely and keeps legacy behaviour. 4. Google credentials: BOTH models supported rather than forcing a choice. A tenant may declare its own googleClientId/Secret — needed where the consent screen's branding matters, since a tenant whose premise is that FuzeFront is invisible should not show users a FuzeFront-named consent screen — and otherwise falls back to the platform-wide client, which is today's behaviour. Adding a client later is a values change, not a code change. Per-tenant clients are keyed by tenant id and derive their redirect URI from the tenant's own appBaseUrl. Verified: tsc --noEmit clean; helm lint clean; chart renders inert with an empty tenants list and emits the per-tenant secretKeyRefs when populated; 135 passed / 2 failed across the registry plus the 8 Authentik/OIDC suites — the same two pre-existing failures that reproduce on clean master, no new ones. 32 registry tests including the new per-tenant secret and Google-fallback cases. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session-Id: 55a394fa-d5ba-4da4-b41f-b0ef56fa6ddf
Contributor
Automated code review (gate-code-review)Credit balance is too low Report-only — this check never blocks merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stage 3 of #434 — the four remaining items, on top of #441 and #448.
1.
tenantContextis mountedOn the identity surfaces only:
/api/v1/security,/api/auth,/api/organizations,/api/invitations,/api/tokens./health,/metricsand/internalare deliberately excluded. Probe and cluster-internal traffic arrives on Service DNS and pod IPs, not on a declared tenant host — requiring a tenant there would fail liveness/readiness the moment a second tenant is declared, taking the service down.2. Sessions are scoped to their tenant
Every mint now stamps a
tidclaim; verification rejects a token whosetidis not the tenant serving the request.This is load-bearing, not bookkeeping: all tenants' sessions are signed with the same secret. Without the claim, a token minted in one directory is byte-indistinguishable from one minted in another, and the only thing separating the two account directories is the request host — which the token holder chooses. Logout is covered too, so it cannot delete another directory's session row.
Pre-tenancy tokens (no
tid) are accepted only while single-tenant, where there is nothing to confuse them with.3. Chart wiring
securityService.tenantsrendersSECURITY_TENANTSplus, per tenant,secretKeyRef-backedSECURITY_TENANT_<ID>_CLIENT_SECRET/_ADMIN_TOKEN/_GOOGLE_CLIENT_SECRET.Secrets are not inlined in
SECURITY_TENANTS— that renders to a plain env value, visible inkubectl describe pod. Values reference chart Secret keys; the registry reads the actual secrets from the per-tenant vars. I caught this mid-implementation, having first written a comment claiming an entrypoint injection that does not exist.Empty list (the default) omits the block entirely — verified — so existing deployments are untouched.
4. Google credentials — both models, so it stops being a blocking decision
You asked me to handle this one. Rather than pick, I made it configuration:
googleClientId/googleClientSecretgets its own Google client, so the consent screen carries its branding. That matters here: a tenant whose entire premise is that FuzeFront is invisible to it should not send users to a Google consent screen displaying FuzeFront's application name.Adding a client later is a values change, not a code change. Per-tenant clients are keyed by tenant id and derive their redirect URI from the tenant's own
appBaseUrl, so the callback lands on its own host (that URI must be registered in that Google client).Verification
tsc --noEmithelm lintSECURITY_TENANT_MENDYS_CLIENT_SECRET/_ADMIN_TOKENemitted as secretKeyRefsThe 2 failures are
google-brokered-signinandauthentik-provider, which reproduce on cleanmasterand are pre-existing (confirmed earlier against a stashed tree). Still unowned and worth their own look.Honest gaps
tidround-trip are covered only indirectly by the existing suites passing; a focused integration test that mints under tenant A and presents under tenant B through the real middleware would be stronger./api/v1/security/*and/api/auth/*routed to security-service — that ingress lives in the MendysRobotics repo (izzywdev/MendysRobotics#253), not this chart.securityService.tenants.Related: #428, #433, #439, #441, #448, izzywdev/FuzeInfra#421, izzywdev/MendysRobotics#253.
🤖 Generated with Claude Code