key-cape/src/internal/server/oidc/tenant_precondition_test.go

148 lines
7.1 KiB
Go
Raw Normal View History

Make the static-registration precondition a checked condition informed-decision asked for the caveat under which a registration-bound human tenant is safe to be a condition of the capability rather than reasoning in a message, so a future change to registration policy has to confront it. They were right that it was only prose: the contract stated it, a separate test asserted registration_endpoint is absent, and nothing connected the two -- so a session adding dynamic registration would have seen a test about discovery metadata, not a warning about relabelling users. The test asserts both halves together: that a client-declared tenant is issued, and that dynamic registration is not advertised. Whichever is removed first, the failure points at the other. The message carries the reasoning rather than the observation -- anyone able to register a client could relabel the users who log in through it -- and names the two ways out. Verified in both directions rather than assumed: advertising a registration_endpoint fails it with the escalation message, and neutering humanTenant fails it with the message saying to remove the guard along with the capability it protects. This is not a vote on the tenant question. It makes one option's precondition checkable; if option 1 lands, the capability and this guard are removed together. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NV9oijZukGyGbRQGGKnK4P Assistant: claude-code Assistant-Model: opus Assistant-Process: 713576@bnt-lap001 Assistant-Session: 384c511d-9bce-4cb8-a676-2aef6c0c8df6
2026-09-09 23:23:48 +02:00
package oidc_test
import (
"net/http"
"net/http/httptest"
"net/url"
"testing"
"keycape/internal/domain"
"keycape/internal/server/oidc"
)
// KEY-WP-0030. A client registration may declare the tenant its users' tokens
// carry (humanTenant). That is safe for exactly one reason: registrations here
// are static and deployment-owned. A self-service client able to name its users'
// tenant would be a straightforward escalation — register a client, declare the
// landlord zone, and any user who logs in through it is labelled with it.
//
// informed-decision asked for that caveat to be a condition of the capability
// rather than a paragraph, so that a future change to registration policy has to
// confront it. This test is that condition: it fails if the exclusion is ever
// lifted while the capability is present, and its failure message says what to do
// about it rather than merely reporting the endpoint.
//
Carry the tenant claim's provenance, and correct a guard the ruling voided GH-DEC-2026-013 §5 is a finding nobody asked for and ours to implement: tenant is a bare string, so a consumer cannot tell a zone the directory asserted about the person from one a registration supplied about the client they came through. approval-engine exact-matches that string while its contract reads as though it relies on the first -- the check is sound and the property a reader infers from it is absent. gate-house named the property and left the mechanism to us. Every token now carries tenant_source beside tenant: directory, registration or default. Advertised in claims_supported, and asserted at the token level on both grants rather than only in the resolution function, since the claim a consumer reads is the thing under obligation. Three values where the ruling names two, which is the judgement here. Labelling an unasserted profile default as directory would reproduce the same defect one level down -- a consumer reading an assertion the identity layer never made. The ruling cites GH-DEC-2026-011 §3 on unknown versus absent for the case it examined; the same rule applies to our own fallback. The agreement case resolves to directory deliberately: if a registration declares the zone the directory also assigned, the directory did assert it, and reporting the weaker source would understate what is known. Also corrects the guard shipped in 5f516a0. Its failure message offered two ways out of adding dynamic registration, and condition (b) voids the second: admitting dynamic registration voids the registration-bound shape that day, whatever state the adapter is in. The message named an inadmissible resolution in the exact place someone would read it while making that change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NV9oijZukGyGbRQGGKnK4P Assistant: claude-code Assistant-Model: opus Assistant-Process: 713576@bnt-lap001 Assistant-Session: 384c511d-9bce-4cb8-a676-2aef6c0c8df6
2026-09-10 07:58:29 +02:00
// gate-house GH-DEC-2026-013 condition (b) then strengthened the rule against us.
// We had written that it "must be revisited" if the exclusion is lifted; the
// ruling holds that revisited implies the answer might survive review, and it
// would not. Admitting dynamic registration voids the registration-bound shape
// outright. The failure message says that, because the earlier wording offered a
// way out the ruling forbids, in the exact place someone would read it while
// making the change.
//
Make the static-registration precondition a checked condition informed-decision asked for the caveat under which a registration-bound human tenant is safe to be a condition of the capability rather than reasoning in a message, so a future change to registration policy has to confront it. They were right that it was only prose: the contract stated it, a separate test asserted registration_endpoint is absent, and nothing connected the two -- so a session adding dynamic registration would have seen a test about discovery metadata, not a warning about relabelling users. The test asserts both halves together: that a client-declared tenant is issued, and that dynamic registration is not advertised. Whichever is removed first, the failure points at the other. The message carries the reasoning rather than the observation -- anyone able to register a client could relabel the users who log in through it -- and names the two ways out. Verified in both directions rather than assumed: advertising a registration_endpoint fails it with the escalation message, and neutering humanTenant fails it with the message saying to remove the guard along with the capability it protects. This is not a vote on the tenant question. It makes one option's precondition checkable; if option 1 lands, the capability and this guard are removed together. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NV9oijZukGyGbRQGGKnK4P Assistant: claude-code Assistant-Model: opus Assistant-Process: 713576@bnt-lap001 Assistant-Session: 384c511d-9bce-4cb8-a676-2aef6c0c8df6
2026-09-09 23:23:48 +02:00
// The two halves are asserted together on purpose. Whichever is removed first,
// the test points at the other.
func TestRegistrationBoundTenantRequiresStaticRegistration(t *testing.T) {
// Half one: the capability exists. If this stops holding, the precondition
// below is no longer load-bearing and this test can go with it.
sessions := oidc.NewSessionStore()
h, _ := newTokenHandler(t, sessions, &mockUserRepo{users: map[string]*domain.User{"alice": aliceUser()}})
h.ClientConfig["test-client"].Tenant = "tenant:platform"
verifier := "test-verifier"
code := seededSession(sessions, verifier)
w := httptest.NewRecorder()
h.ServeHTTP(w, codeExchange(t, url.Values{
"grant_type": {"authorization_code"}, "client_id": {"test-client"},
"code": {code}, "code_verifier": {verifier}, "redirect_uri": {seededRedirectURI},
}))
if w.Code != http.StatusOK {
t.Fatalf("client-declared tenant no longer issues (status %d): if the capability was "+
"removed deliberately, remove this test too — the dynamic-registration "+
"precondition below only exists to protect it", w.Code)
}
if got := parseJWTPayload(t, decodeTokenResponse(t, w.Body.String())["access_token"].(string))["tenant"]; got != "tenant:platform" {
t.Fatalf("client-declared tenant resolved to %v, want tenant:platform", got)
}
// Half two: the precondition holds. Dynamic client registration must stay
// absent while any client can declare a tenant.
doc := discoveryDoc(t, oidc.DiscoveryConfig{
Issuer: "https://auth.netkingdom.local",
AuthorizationEndpoint: "https://auth.netkingdom.local/authorize",
TokenEndpoint: "https://auth.netkingdom.local/token",
JWKSUri: "https://auth.netkingdom.local/jwks",
})
if _, advertised := doc["registration_endpoint"]; advertised {
t.Fatal("dynamic client registration is advertised while a client registration " +
"may declare its users' tenant. That combination lets anyone who can register " +
"a client relabel the users who log in through it into a zone of their choosing. " +
Carry the tenant claim's provenance, and correct a guard the ruling voided GH-DEC-2026-013 §5 is a finding nobody asked for and ours to implement: tenant is a bare string, so a consumer cannot tell a zone the directory asserted about the person from one a registration supplied about the client they came through. approval-engine exact-matches that string while its contract reads as though it relies on the first -- the check is sound and the property a reader infers from it is absent. gate-house named the property and left the mechanism to us. Every token now carries tenant_source beside tenant: directory, registration or default. Advertised in claims_supported, and asserted at the token level on both grants rather than only in the resolution function, since the claim a consumer reads is the thing under obligation. Three values where the ruling names two, which is the judgement here. Labelling an unasserted profile default as directory would reproduce the same defect one level down -- a consumer reading an assertion the identity layer never made. The ruling cites GH-DEC-2026-011 §3 on unknown versus absent for the case it examined; the same rule applies to our own fallback. The agreement case resolves to directory deliberately: if a registration declares the zone the directory also assigned, the directory did assert it, and reporting the weaker source would understate what is known. Also corrects the guard shipped in 5f516a0. Its failure message offered two ways out of adding dynamic registration, and condition (b) voids the second: admitting dynamic registration voids the registration-bound shape that day, whatever state the adapter is in. The message named an inadmissible resolution in the exact place someone would read it while making that change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NV9oijZukGyGbRQGGKnK4P Assistant: claude-code Assistant-Model: opus Assistant-Process: 713576@bnt-lap001 Assistant-Session: 384c511d-9bce-4cb8-a676-2aef6c0c8df6
2026-09-10 07:58:29 +02:00
"Per gate-house GH-DEC-2026-013 condition (b) this is not a trade-off to " +
"rebalance: admitting dynamic registration VOIDS the registration-bound tenant " +
"that day, and the directory becomes the only source, whatever state the " +
"directory adapter is in. Gating the capability to statically configured " +
"registrations is not an available answer. Remove the client-declared tenant, " +
"or do not admit dynamic registration. See docs/tenant-claim-contract.md, " +
"'How a human token's tenant is resolved'.")
}
}
// GH-DEC-2026-013 §5: the claim must carry its provenance, so a consumer can
// tell a zone the identity layer asserted about the person from one a
// registration supplied about the client they came through. The unit rule is
// covered in human_tenant_test.go; this asserts the claim actually reaches
// issued tokens, on both grants, which is what a consumer reads.
func TestIssuedTokensCarryTenantProvenance(t *testing.T) {
t.Run("human, registration-supplied", func(t *testing.T) {
sessions := oidc.NewSessionStore()
h, _ := newTokenHandler(t, sessions, &mockUserRepo{users: map[string]*domain.User{"alice": aliceUser()}})
h.ClientConfig["test-client"].Tenant = "tenant:platform"
claims := issueHumanToken(t, h, sessions)
if claims["tenant"] != "tenant:platform" || claims["tenant_source"] != oidc.TenantSourceRegistration {
t.Fatalf("tenant %v / source %v", claims["tenant"], claims["tenant_source"])
}
})
t.Run("human, directory-asserted", func(t *testing.T) {
user := aliceUser()
user.Tenant = "tenant:friendly:binky"
sessions := oidc.NewSessionStore()
h, _ := newTokenHandler(t, sessions, &mockUserRepo{users: map[string]*domain.User{"alice": user}})
claims := issueHumanToken(t, h, sessions)
if claims["tenant"] != "tenant:friendly:binky" || claims["tenant_source"] != oidc.TenantSourceDirectory {
t.Fatalf("tenant %v / source %v", claims["tenant"], claims["tenant_source"])
}
})
// Nobody asserted a zone. Reporting "directory" here would be the same defect
// one level down: a consumer reading an assertion the directory never made.
t.Run("human, nobody asserted", func(t *testing.T) {
sessions := oidc.NewSessionStore()
h, _ := newTokenHandler(t, sessions, &mockUserRepo{users: map[string]*domain.User{"alice": aliceUser()}})
claims := issueHumanToken(t, h, sessions)
if claims["tenant_source"] != oidc.TenantSourceDefault {
t.Fatalf("source %v, want %v", claims["tenant_source"], oidc.TenantSourceDefault)
}
})
t.Run("service is always registration-supplied", func(t *testing.T) {
h := serviceTokenHandler(t)
req := tokenRequest(url.Values{"grant_type": {"client_credentials"}, "scope": {"finance.qonto.read"}})
req.SetBasicAuth("rapp-qonto", "test-service-secret")
w := httptest.NewRecorder()
h.ServeHTTP(w, req)
if w.Code != http.StatusOK {
t.Fatalf("status %d", w.Code)
}
claims := parseJWTPayload(t, decodeTokenResponse(t, w.Body.String())["access_token"].(string))
if claims["tenant_source"] != oidc.TenantSourceRegistration {
t.Fatalf("source %v, want %v", claims["tenant_source"], oidc.TenantSourceRegistration)
}
})
}
func issueHumanToken(t *testing.T, h *oidc.TokenHandler, sessions *oidc.SessionStore) map[string]interface{} {
t.Helper()
verifier := "test-verifier"
code := seededSession(sessions, verifier)
w := httptest.NewRecorder()
h.ServeHTTP(w, codeExchange(t, url.Values{
"grant_type": {"authorization_code"}, "client_id": {"test-client"},
"code": {code}, "code_verifier": {verifier}, "redirect_uri": {seededRedirectURI},
}))
if w.Code != http.StatusOK {
t.Fatalf("status %d: %s", w.Code, w.Body.String())
Make the static-registration precondition a checked condition informed-decision asked for the caveat under which a registration-bound human tenant is safe to be a condition of the capability rather than reasoning in a message, so a future change to registration policy has to confront it. They were right that it was only prose: the contract stated it, a separate test asserted registration_endpoint is absent, and nothing connected the two -- so a session adding dynamic registration would have seen a test about discovery metadata, not a warning about relabelling users. The test asserts both halves together: that a client-declared tenant is issued, and that dynamic registration is not advertised. Whichever is removed first, the failure points at the other. The message carries the reasoning rather than the observation -- anyone able to register a client could relabel the users who log in through it -- and names the two ways out. Verified in both directions rather than assumed: advertising a registration_endpoint fails it with the escalation message, and neutering humanTenant fails it with the message saying to remove the guard along with the capability it protects. This is not a vote on the tenant question. It makes one option's precondition checkable; if option 1 lands, the capability and this guard are removed together. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NV9oijZukGyGbRQGGKnK4P Assistant: claude-code Assistant-Model: opus Assistant-Process: 713576@bnt-lap001 Assistant-Session: 384c511d-9bce-4cb8-a676-2aef6c0c8df6
2026-09-09 23:23:48 +02:00
}
Carry the tenant claim's provenance, and correct a guard the ruling voided GH-DEC-2026-013 §5 is a finding nobody asked for and ours to implement: tenant is a bare string, so a consumer cannot tell a zone the directory asserted about the person from one a registration supplied about the client they came through. approval-engine exact-matches that string while its contract reads as though it relies on the first -- the check is sound and the property a reader infers from it is absent. gate-house named the property and left the mechanism to us. Every token now carries tenant_source beside tenant: directory, registration or default. Advertised in claims_supported, and asserted at the token level on both grants rather than only in the resolution function, since the claim a consumer reads is the thing under obligation. Three values where the ruling names two, which is the judgement here. Labelling an unasserted profile default as directory would reproduce the same defect one level down -- a consumer reading an assertion the identity layer never made. The ruling cites GH-DEC-2026-011 §3 on unknown versus absent for the case it examined; the same rule applies to our own fallback. The agreement case resolves to directory deliberately: if a registration declares the zone the directory also assigned, the directory did assert it, and reporting the weaker source would understate what is known. Also corrects the guard shipped in 5f516a0. Its failure message offered two ways out of adding dynamic registration, and condition (b) voids the second: admitting dynamic registration voids the registration-bound shape that day, whatever state the adapter is in. The message named an inadmissible resolution in the exact place someone would read it while making that change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NV9oijZukGyGbRQGGKnK4P Assistant: claude-code Assistant-Model: opus Assistant-Process: 713576@bnt-lap001 Assistant-Session: 384c511d-9bce-4cb8-a676-2aef6c0c8df6
2026-09-10 07:58:29 +02:00
return parseJWTPayload(t, decodeTokenResponse(t, w.Body.String())["access_token"].(string))
Make the static-registration precondition a checked condition informed-decision asked for the caveat under which a registration-bound human tenant is safe to be a condition of the capability rather than reasoning in a message, so a future change to registration policy has to confront it. They were right that it was only prose: the contract stated it, a separate test asserted registration_endpoint is absent, and nothing connected the two -- so a session adding dynamic registration would have seen a test about discovery metadata, not a warning about relabelling users. The test asserts both halves together: that a client-declared tenant is issued, and that dynamic registration is not advertised. Whichever is removed first, the failure points at the other. The message carries the reasoning rather than the observation -- anyone able to register a client could relabel the users who log in through it -- and names the two ways out. Verified in both directions rather than assumed: advertising a registration_endpoint fails it with the escalation message, and neutering humanTenant fails it with the message saying to remove the guard along with the capability it protects. This is not a vote on the tenant question. It makes one option's precondition checkable; if option 1 lands, the capability and this guard are removed together. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NV9oijZukGyGbRQGGKnK4P Assistant: claude-code Assistant-Model: opus Assistant-Process: 713576@bnt-lap001 Assistant-Session: 384c511d-9bce-4cb8-a676-2aef6c0c8df6
2026-09-09 23:23:48 +02:00
}