diff --git a/docs/tenant-claim-contract.md b/docs/tenant-claim-contract.md new file mode 100644 index 0000000..741e8e9 --- /dev/null +++ b/docs/tenant-claim-contract.md @@ -0,0 +1,63 @@ +# Tenant claim contract + +Answers the tenant-alignment question in glas-harness message +`356f6977-d361-4e3b-83ab-b2c7f4759286` (GLAS-WP-0015). It states what KeyCape +owns and emits; it does not create a mapping between tenant vocabularies. + +## What KeyCape emits + +The `tenant` claim is bound at registration or directory-resolution time and is +never influenced by request parameters. + +| Principal | Source of `tenant` | Value in the reviewed registrations | +| --- | --- | --- | +| Service (`client_credentials`) | The client's `tenant` field. Required by config validation — a `client_credentials` client without `serviceSubject` and `tenant` fails startup validation. | `tenant:coulomb` for all four clients in `config/service-clients.example.yaml`, including `secrets-engine-approval` and `approval-engine-operator`. | +| Human (`authorization_code`) | The directory user's tenant, falling back to the platform default when unset. | Directory value, else `tenant:coulomb`. | + +`tenant`, `tenant_hint`, `audience` and `resource` request parameters cannot +change the claim. `tenant_hint` on `/authorize` reaches registration/enrollment +handoffs only; it is not a claim input. + +## The three values in the request + +- **JWT tenant** — `tenant:coulomb`. Owned by KeyCape, emitted verbatim from the + registration above. This is the only one of the three KeyCape owns. +- **Approval store tenant** — `platform`. Owned by approval-engine. +- **Policy tenant / CheckRequest tenant** — `tenant:platform`. Owned by flex-auth. + +KeyCape performs **no** normalization, prefix-stripping or aliasing. A resource +server comparing `tenant` must use exact string comparison, so as things stand a +token issued to `secrets-engine-approval` (`tenant:coulomb`) does not match an +approval store tenant `platform` or a policy tenant `tenant:platform`. + +## What is not decided here + +Whether these three identify the same tenant across layers is not a KeyCape +decision, and spelling similarity is not a mapping. Two admissible resolutions +exist, both owned outside this repository: + +1. The consuming owners accept `tenant:coulomb` as the JWT tenant and record the + layer mapping in their own contract; KeyCape changes nothing. +2. The owners decide the approval clients belong to a different tenant, in which + case KeyCape changes the `tenant` field on those two registrations only, under + an explicit decision reference, and re-issues. + +KeyCape will not change the `tenant` value on a live registration without such a +reference. No unilateral change to live clients or policy subjects was made. + +## Evidence + +`src/internal/server/oidc/tenant_test.go`: + +- `TestServiceTenantIsBoundToRegistrationAndIgnoresRequestParameters` — a request + supplying `tenant=tenant:platform` and `tenant_hint=platform` still yields the + registered `tenant`, signature-verified against `/jwks`. +- `TestServiceTenantsAreDistinctPerRegistration` — two registrations with + different tenants yield their own values and never each other's; this is the + wrong-tenant denial basis for an exact-comparison resource server. +- `TestHumanTenantClaimUsesDirectoryValueThenPlatformDefault` — human tokens carry + the directory tenant, defaulting to `tenant:coulomb` rather than an empty claim. + +These are local issuance proofs. They are not live-rollout evidence; see +`docs/approval-engine-auth-contract.md` and KEY-WP-0013-T02 for the deployment +boundary. No token or secret values appear in this document or in test output. diff --git a/src/internal/server/oidc/tenant_test.go b/src/internal/server/oidc/tenant_test.go new file mode 100644 index 0000000..87e18ac --- /dev/null +++ b/src/internal/server/oidc/tenant_test.go @@ -0,0 +1,106 @@ +package oidc_test + +import ( + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "keycape/internal/domain" + "keycape/internal/server/oidc" +) + +// The tenant claim is bound at registration time. KeyCape emits exactly the +// registered value and performs no mapping between tenant vocabularies used by +// consuming resource servers (approval stores, policy subjects). See +// docs/tenant-claim-contract.md. + +func TestServiceTenantIsBoundToRegistrationAndIgnoresRequestParameters(t *testing.T) { + h := serviceTokenHandler(t) + req := tokenRequest(url.Values{ + "grant_type": {"client_credentials"}, + "scope": {"finance.qonto.read"}, + "tenant": {"tenant:platform"}, + "tenant_hint": {"platform"}, + }) + req.SetBasicAuth("rapp-qonto", "test-service-secret") + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("status %d: %s", w.Code, w.Body.String()) + } + token := decodeTokenResponse(t, w.Body.String())["access_token"].(string) + claims := parseJWTPayload(t, token) + if claims["tenant"] != "tenant:friendly:binky" { + t.Fatalf("tenant claim not bound to registration: %v", claims["tenant"]) + } + verifyWithJWKS(t, h, token) +} + +// A token minted for one registration never carries another registration's +// tenant: this is the wrong-tenant evidence a resource server needs to reject +// a caller by exact string comparison. +func TestServiceTenantsAreDistinctPerRegistration(t *testing.T) { + h := serviceTokenHandler(t) + h.ClientConfig["platform-client"] = &domain.Client{ + ClientID: "platform-client", + AllowedScopes: []string{"finance.qonto.read"}, + GrantTypes: []string{"client_credentials"}, + ClientType: "confidential", + ClientSecret: "other-service-secret", + ServiceSubject: "service:platform-client", + Tenant: "tenant:platform", + Roles: []string{"platform"}, + } + for clientID, want := range map[string]string{ + "rapp-qonto": "tenant:friendly:binky", + "platform-client": "tenant:platform", + } { + secret := "test-service-secret" + if clientID == "platform-client" { + secret = "other-service-secret" + } + req := tokenRequest(url.Values{"grant_type": {"client_credentials"}, "scope": {"finance.qonto.read"}}) + req.SetBasicAuth(clientID, secret) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("%s: status %d", clientID, w.Code) + } + token := decodeTokenResponse(t, w.Body.String())["access_token"].(string) + if got := parseJWTPayload(t, token)["tenant"]; got != want { + t.Fatalf("%s: tenant %v, want %v", clientID, got, want) + } + } +} + +// A human token carries the directory tenant, defaulting to the platform tenant +// (tenant:coulomb) rather than an empty claim. +func TestHumanTenantClaimUsesDirectoryValueThenPlatformDefault(t *testing.T) { + for _, tc := range []struct { + name string + tenant string + want string + }{ + {"directory", "tenant:friendly:binky", "tenant:friendly:binky"}, + {"default", "", "tenant:coulomb"}, + } { + t.Run(tc.name, func(t *testing.T) { + user := aliceUser() + user.Tenant = tc.tenant + sessions := oidc.NewSessionStore() + h, _ := newTokenHandler(t, sessions, &mockUserRepo{users: map[string]*domain.User{"alice": user}}) + verifier := "test-verifier" + code := seededSession(sessions, verifier) + w := httptest.NewRecorder() + h.ServeHTTP(w, tokenRequest(url.Values{"grant_type": {"authorization_code"}, "client_id": {"test-client"}, "code": {code}, "code_verifier": {verifier}})) + if w.Code != http.StatusOK { + t.Fatalf("status %d: %s", w.Code, w.Body.String()) + } + access := decodeTokenResponse(t, w.Body.String())["access_token"].(string) + if got := parseJWTPayload(t, access)["tenant"]; got != tc.want { + t.Fatalf("tenant %v, want %v", got, tc.want) + } + }) + } +} diff --git a/workplans/KEY-WP-0013-approval-engine-resource-audience.md b/workplans/KEY-WP-0013-approval-engine-resource-audience.md index 8c48cf7..d62a791 100644 --- a/workplans/KEY-WP-0013-approval-engine-resource-audience.md +++ b/workplans/KEY-WP-0013-approval-engine-resource-audience.md @@ -8,7 +8,7 @@ status: blocked owner: codex topic_slug: approval-engine-resource-audience created: "2026-09-05" -updated: "2026-09-05" +updated: "2026-09-06" state_hub_workstream_id: "6e815d88-b0e3-5ce0-be5d-13ab15917f7f" --- @@ -56,3 +56,28 @@ Custody routing has no exact admitted lane for these two clients. `warden plan` returned founder_required but matched an unrelated generic database lane; that mismatch is not authority to provision. Human callback clarification is pending. No secrets were read or production resources changed. + +## Reconcile tenant vocabularies across approval layers + +```task +id: KEY-WP-0013-T03 +status: done +priority: high +``` + +Source: glas-harness inbox message 356f6977-d361-4e3b-83ab-b2c7f4759286 +(GLAS-WP-0015), which asks for the exact store tenant, JWT tenant, CheckRequest +tenant, any permitted mapping and wrong-tenant denial evidence. + +Published `docs/tenant-claim-contract.md`. The KeyCape-owned JWT tenant for all +four reviewed service registrations is `tenant:coulomb`, bound at registration +and required by config validation. Approval store `platform` and policy +`tenant:platform` are owned by approval-engine and flex-auth; KeyCape performs no +normalization or aliasing, so exact comparison does not match today. No mapping +was invented and no live registration or policy subject was changed — the two +admissible resolutions are recorded for the owning parties to decide. + +Added `src/internal/server/oidc/tenant_test.go`: request-supplied `tenant` and +`tenant_hint` cannot alter the claim; two registrations never carry each other's +tenant (the wrong-tenant denial basis); human tokens default to `tenant:coulomb` +rather than an empty claim. Local issuance proof only, not live-rollout evidence.