diff --git a/config/service-clients.example.yaml b/config/service-clients.example.yaml index da75f16..9e77c95 100644 --- a/config/service-clients.example.yaml +++ b/config/service-clients.example.yaml @@ -67,3 +67,37 @@ clients: tenant: "tenant:platform" roles: ["approval-operator"] tokenLifetime: "15m" + + # Human approver browser client. Submitted by informed-decision 2026-09-10 + # (INFD-WP-0001-T07) with the origin already live: both / and /auth/callback + # return 200 from 92.205.62.239 on a Let's Encrypt certificate + # CN=decisions.coulomb.social valid 2026-09-10 to 2026-12-09, deployed by + # railiance-apps. The host is decisions.coulomb.social, NOT the + # decide.coulomb.social an earlier draft proposed; register the string verbatim. + # + # The path currently serves an nginx placeholder while their surface is gated on + # APPROVAL-WP-0002-T01. That does not affect this registration: the redirect is + # matched as an exact string at /authorize and never fetched. + # + # No secretRef: this is a public client and authenticates with PKCE alone. No + # serviceSubject or roles either — on a browser client both are silently ignored + # and config validation rejects them (KEY-WP-0028); the subject and roles come + # from the directory user. + - clientId: "informed-decision-approver" + displayName: "informed-decision approver surface" + audience: "approval-engine" + redirectUris: + - "https://decisions.coulomb.social/auth/callback" + allowedScopes: ["openid", "approval:read", "approval:approve"] + grantTypes: ["authorization_code"] + clientType: "public" + # Declared, not inherited. Nothing populates domain.User.Tenant for approver + # users, so this reaches the token by the GH-DEC-2026-013 declared-gap route + # by construction, and the token says so: tenant_source is "registration", + # never "directory". Admissible for approval-engine's store-isolation gate; + # NOT admissible for any doctrine turning on this person's membership of the + # zone. See docs/tenant-claim-contract.md. + tenant: "tenant:platform" + # Approval is a human-in-the-loop control, so MFA is required rather than + # left to the provider default. + mfaRequired: true diff --git a/docs/approval-engine-auth-contract.md b/docs/approval-engine-auth-contract.md index e2b8ff9..3c35fe6 100644 --- a/docs/approval-engine-auth-contract.md +++ b/docs/approval-engine-auth-contract.md @@ -13,12 +13,32 @@ gets lifecycle/observation scopes without consume. Service tokens contain audience, issue time and expiry; the lifetime is 15 minutes. The issuer signs with RS256 and publishes its public key through `/jwks`. -Human approvers need a separate authorization-code/PKCE registration with an -exact deployment-owned callback, `audience: approval-engine`, -`allowedScopes: [openid, approval:read, approval:approve]`, and -`mfaRequired: true`. Do not add consume or other approval grants to that client. -No callback is invented here. The ID token is for the login client; present the -access token to approval-engine. +Human approvers use the `informed-decision-approver` registration, submitted by +informed-decision on 2026-09-10 (INFD-WP-0001-T07) and published in +`config/service-clients.example.yaml`: + +| Field | Value | +| --- | --- | +| `clientId` | `informed-decision-approver` | +| `redirectUris` | `https://decisions.coulomb.social/auth/callback` (exact, sole) | +| `clientType` | `public` — PKCE is the whole proof; no secret | +| `grantTypes` | `authorization_code` with S256 PKCE | +| `audience` | `approval-engine` | +| `allowedScopes` | `openid`, `approval:read`, `approval:approve` | +| `mfaRequired` | `true` | +| `tenant` | `tenant:platform`, declared | + +The host is `decisions.coulomb.social`, not the `decide.coulomb.social` an +earlier draft proposed; the origin was verified live before submission. The path +currently serves a placeholder while the surface is gated on +APPROVAL-WP-0002-T01, which does not affect the registration — the redirect is +matched as an exact string at `/authorize` and never fetched. + +Do not add consume or other approval grants to that client. The ID token is for +the login client; present the access token to approval-engine. +`TestApproverRegistrationShapeIsExact` pins every field above, so widening a +scope or relaxing the MFA requirement fails the build rather than passing as an +edit. That client must also declare `tenant: tenant:platform`. A human token's tenant comes from the directory record, which assigns none today, so without the @@ -63,10 +83,10 @@ These fragments are not live registrations. The two service registrations requir custody-managed values for the named environment references and a reviewed rollout of this version, including the upstream issuer precondition. Platform's CCR-2026-0017/0018 use OpenBao field `CLIENT_SECRET`; their approval remains open. -The separate human registration needs its actual UI-owned callback. A bearer-only -approval resource server has no such callback; its absence does not prevent -service-client issuance or service startup, and service credentials cannot be -counted as human approval evidence. Never log the token or secret. Verify the resulting +The human registration now carries its real UI-owned callback (above); a +bearer-only approval resource server never owned that string, which is why it +came from informed-decision. Service credentials still cannot be counted as human +approval evidence. Never log the token or secret. Verify the resulting access token against the deployed issuer's `/jwks`, checking issuer, audience, expiry, subject, principal type, tenant, roles, scope and assurance. Verify that operator consume and human consume requests are rejected. Local tests verify diff --git a/docs/approval-engine-provisioning-request.yaml b/docs/approval-engine-provisioning-request.yaml index 1de4348..902c20c 100644 --- a/docs/approval-engine-provisioning-request.yaml +++ b/docs/approval-engine-provisioning-request.yaml @@ -34,7 +34,16 @@ requests: custody_owner: railiance-platform consumer: approval-engine-operator human_registration: - status: awaiting-exact-callback + status: submitted-2026-09-10 + client_id: informed-decision-approver + redirect_uri: https://decisions.coulomb.social/auth/callback + # Origin verified live before submission: / and /auth/callback both return 200 + # from 92.205.62.239 on a Let's Encrypt certificate CN=decisions.coulomb.social + # valid 2026-09-10 to 2026-12-09, deployed by railiance-apps. The host is + # decisions.coulomb.social, NOT the decide.coulomb.social of an earlier draft. + # The path serves a placeholder while the surface is gated on + # APPROVAL-WP-0002-T01; the redirect is matched as a string and never fetched. + origin_verified: true blocks_service_client_rollout: false # Owner identified 2026-09-09: informed-decision (hub repo cf4c7da8). It # supplies client_id and callback URI from INFD-WP-0001-T07 once it has a @@ -58,9 +67,16 @@ human_registration: directory has not placed the user, requires agreement when it has, and refuses issuance on conflict rather than relabelling. Safe only because registrations are static and deployment-owned. Still outstanding from - elsewhere: the exact client_id and callback URI from informed-decision - (INFD-WP-0001-T07), and whether the owners prefer the directory-sourced - resolution instead, which this implementation degrades into cleanly. + elsewhere: whether the owners prefer the directory-sourced resolution + instead, which this implementation degrades into cleanly. + resolved_2026_09_10: | + client_id and callback URI received and registered. GH-DEC-2026-013 granted + the registration-bound shape as a declared bounded gap, so the declared + tenant is admissible; the token says how it got there via tenant_source = + "registration", never "directory". Per gate-house's later correction the + tenant field carries three facts, and a registration-supplied value is + adequate for approval-engine's store-isolation gate and NEVER for doctrine + turning on this person's membership of the zone. verification: # The first two lines are now one runnable command per client; see # docs/native-authentication.md, "Verifying a live registration". It writes diff --git a/src/cmd/keycape/clients_test.go b/src/cmd/keycape/clients_test.go index 60c0351..1012d36 100644 --- a/src/cmd/keycape/clients_test.go +++ b/src/cmd/keycape/clients_test.go @@ -2,6 +2,9 @@ package main import ( "keycape/internal/config" + "os" + "path/filepath" + "strings" "testing" "time" ) @@ -12,7 +15,12 @@ func TestServiceRegistrationAudienceAndScopeIsolation(t *testing.T) { t.Fatal(err) } for _, c := range cfg.Clients { - t.Setenv(c.SecretRef[4:], "test-only-secret") + // Public clients carry no secretRef -- the approver registration added in + // KEY-WP-0013-T05 is the first in this fixture. Slicing unconditionally + // assumed every entry was confidential, which was true when written. + if strings.HasPrefix(c.SecretRef, "env:") { + t.Setenv(c.SecretRef[len("env:"):], "test-only-secret") + } } registry, err := buildClientRegistry(cfg.Clients) if err != nil { @@ -51,6 +59,11 @@ func TestServiceRegistrationTenantsAreExactPerDecision(t *testing.T) { "approval-engine-operator": "tenant:platform", "codex-railiance-platform": "tenant:coulomb", "secrets-engine-openbao": "tenant:coulomb", + // The only browser client here, and the only registration-supplied + // human tenant. Reviewed and added deliberately: this guard exists so a + // new client carrying a tenant cannot arrive unnoticed, and it did its + // job when the approver registration first landed (KEY-WP-0013-T05). + "informed-decision-approver": "tenant:platform", } seen := map[string]bool{} for _, c := range cfg.Clients { @@ -69,3 +82,87 @@ func TestServiceRegistrationTenantsAreExactPerDecision(t *testing.T) { } } } + +// The approver registration is a human-in-the-loop control's entry point, so its +// shape is pinned rather than left to review: a widened scope, a relaxed MFA +// requirement or an added redirect would each be a security change that reads +// like an edit (KEY-WP-0013-T05). +func TestApproverRegistrationShapeIsExact(t *testing.T) { + cfg, err := config.Load("../../../config/service-clients.example.yaml") + if err != nil { + t.Fatal(err) + } + var approver *config.ClientConfig + for i := range cfg.Clients { + if cfg.Clients[i].ClientID == "informed-decision-approver" { + approver = &cfg.Clients[i] + } + } + if approver == nil { + t.Fatal("the approver registration is absent") + } + + // The exact string informed-decision submitted, verified live on 2026-09-10. + // decisions.coulomb.social, not the decide.coulomb.social of an earlier draft. + if len(approver.RedirectURIs) != 1 || approver.RedirectURIs[0] != "https://decisions.coulomb.social/auth/callback" { + t.Errorf("redirect URIs = %v; exactly one exact callback is registered", approver.RedirectURIs) + } + if approver.ClientType != "public" || len(approver.GrantTypes) != 1 || approver.GrantTypes[0] != "authorization_code" { + t.Errorf("client type %q grants %v; want a public authorization_code client", approver.ClientType, approver.GrantTypes) + } + // A public client must carry no secret reference: PKCE is the whole proof. + if approver.SecretRef != "" { + t.Errorf("public approver client carries a secretRef: %q", approver.SecretRef) + } + if approver.MFARequired == nil || !*approver.MFARequired { + t.Error("mfaRequired must be explicitly true: approval is a human-in-the-loop control") + } + if approver.Tenant != "tenant:platform" { + t.Errorf("tenant = %q, want tenant:platform", approver.Tenant) + } + if approver.Audience != "approval-engine" { + t.Errorf("audience = %q, want approval-engine", approver.Audience) + } + + // Exactly these scopes. consume is the one that must never appear, but an + // unreviewed addition of any kind is what this pins. + want := map[string]bool{"openid": true, "approval:read": true, "approval:approve": true} + if len(approver.AllowedScopes) != len(want) { + t.Errorf("scopes = %v; want exactly %v", approver.AllowedScopes, want) + } + for _, scope := range approver.AllowedScopes { + if !want[scope] { + t.Errorf("unreviewed scope %q on the approver client", scope) + } + if scope == "approval:consume" { + t.Fatal("approval:consume on the human approver client") + } + } + + // It must also survive startup validation. KEY-WP-0028 rejects + // serviceSubject and roles on a browser client, and tenant is deliberately + // exempt from that rule -- this asserts the exemption actually holds for the + // registration that depends on it, rather than only in a synthetic case. + key := writeTestKeyPEM(t) + full := &config.Config{ + Issuer: "https://kc.coulomb.social", + Port: 8080, + TokenLifetime: "15m", + PrivateKeyPEM: key, + Clients: []config.ClientConfig{*approver}, + } + if errs := config.ValidateConfig(full); len(errs) != 0 { + t.Fatalf("the approver registration fails startup validation: %v", errs) + } +} + +// writeTestKeyPEM writes a placeholder key file; ValidateConfig checks the path +// exists, not the key material. +func writeTestKeyPEM(t *testing.T) string { + t.Helper() + path := filepath.Join(t.TempDir(), "key.pem") + if err := os.WriteFile(path, []byte("placeholder"), 0o600); err != nil { + t.Fatal(err) + } + return path +} diff --git a/workplans/KEY-WP-0013-approval-engine-resource-audience.md b/workplans/KEY-WP-0013-approval-engine-resource-audience.md index a4f98a5..049188a 100644 --- a/workplans/KEY-WP-0013-approval-engine-resource-audience.md +++ b/workplans/KEY-WP-0013-approval-engine-resource-audience.md @@ -348,10 +348,9 @@ indefinitely.** ```task id: KEY-WP-0013-T05 -status: wait +status: done priority: high assignee: the-custodian -blocking_reason: "Client ID and deployed callback pending INFD-WP-0001-T07 from informed-decision. The tenant blocker is resolved in code." state_hub_task_id: "9a782909-91db-59fa-aae7-83766f4fbb0d" ``` @@ -435,10 +434,49 @@ the relabel refusal. Verified with teeth — neutering the conflict check makes `docs/approval-engine-auth-contract.md` states the requirement on the approver client, and the provisioning packet now declares `tenant: tenant:platform`. -Task stays `wait` on one thing only: `client_id` and callback URI from -INFD-WP-0001-T07, once informed-decision has a deployed origin. The owners were -asked which resolution they prefer and have not answered; that answer is no -longer blocking, and this implementation is compatible with either. +**Cleared and registered 2026-09-10.** informed-decision submitted the two +strings with the origin already live: + + client_id informed-decision-approver + redirect_uri https://decisions.coulomb.social/auth/callback + +The host is `decisions.coulomb.social`, not the `decide.coulomb.social` of an +earlier draft. They verified the origin rather than reporting it: `/` and +`/auth/callback` both return 200 from 92.205.62.239 on a Let's Encrypt +certificate CN=decisions.coulomb.social valid 2026-09-10 to 2026-12-09. The path +serves a placeholder while their surface is gated on APPROVAL-WP-0002-T01, which +does not affect the registration — the redirect is matched as an exact string at +`/authorize` and never fetched. + +Published in `config/service-clients.example.yaml`: public `authorization_code` +with S256 PKCE, `audience: approval-engine`, scopes +`[openid, approval:read, approval:approve]`, `mfaRequired: true`, declared +`tenant: tenant:platform`. No `secretRef` — PKCE is the whole proof — and no +`serviceSubject` or `roles`, which validation rejects on a browser client. + +`TestApproverRegistrationShapeIsExact` pins every field, so widening a scope or +relaxing MFA fails the build rather than reading as an edit, and asserts the +registration passes startup validation — which proves the KEY-WP-0028 tenant +exemption holds for the registration that depends on it, not only in a synthetic +case. + +Two existing guards fired on the way in, both correctly, and neither was loosened: + +- `TestServiceRegistrationTenantsAreExactPerDecision` refused an unreviewed client + carrying a tenant, which is exactly its purpose. The approver was added to its + reviewed set as a deliberate act. +- `TestServiceRegistrationAudienceAndScopeIsolation` panicked on + `secretRef[4:]`, an assumption that was safe while the fixture held only + confidential clients. The approver is the first public one; the loop now guards + on the `env:` prefix. + +The tenant is declared, and the token says so: `tenant_source` is `registration`, +never `directory`. Per gate-house's correction that is adequate for +approval-engine's store-isolation gate and **never** for doctrine turning on this +person's membership of the zone. + +Not live: this is a published registration fragment. Deployment is owner-side and +nothing has been issued to a real approver. ## Make negative rollout evidence discriminate actual issuer refusal