Let a human token carry the zone it is issued into, without relabelling anyone
All checks were successful
Build and Publish Container Image / build-and-push (push) Successful in 46s

KEY-WP-0013-T05's tenant blocker did not need the decision it was waiting on. The
two proposed resolutions differ in where a human's tenant comes from -- the
directory record, or the client registration -- and an implementation exists that
is correct under either, so the choice can be made later without another
migration.

A client registration may now declare a tenant. humanTenant() resolves it by four
rules: no declaration keeps the directory answer unchanged; a declared zone
applies where the directory has placed the user nowhere; agreement passes; and a
declared zone conflicting with a directory assignment refuses issuance rather
than relabelling the user.

The refusal is the design, not an edge case. A registration can bind a zone for
unplaced users and can never move a placed one, so this gets the approval chain
its tenant:platform without writing a general cross-tenant override into the
issuer. It fails closed rather than picking a winner, because either answer would
be a silent cross-tenant assertion, and it reports 403 with
error_type: tenant_binding so an operator can tell a misconfigured registration
from a rejected login. If the owners later populate directory tenants, the same
code stops supplying the zone and starts enforcing agreement with it.

Safe only because client registrations are static and deployment-owned. The
tenant contract records that this rule must be revisited if dynamic client
registration is ever admitted.

Tests cover all four rules; neutering the conflict check fails the relabel test
rather than passing silently.

T05 now waits on one thing only: the client_id and callback URI from
informed-decision once it has a deployed origin.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uV8zoCKpA1WRAxsKRYbdH

Assistant: claude-code
Assistant-Model: opus
Assistant-Process: 1182213@bnt-lap001
Assistant-Session: 966597b9-ae61-46a4-8b9e-1594ab3ec4ad
This commit is contained in:
tegwick 2026-09-09 14:40:36 +02:00
parent a73da29093
commit 329e48f64a
13 changed files with 310 additions and 14 deletions

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

View file

@ -20,6 +20,12 @@ exact deployment-owned callback, `audience: approval-engine`,
No callback is invented here. The ID token is for the login client; present the
access token to approval-engine.
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
declaration the token would carry `tenant:coulomb` and be refused here. See
[the tenant contract](tenant-claim-contract.md) for the four resolution rules and
for why a declared zone can never relabel a user the directory has placed.
`approval:read` is present because approval-engine showed the surface cannot
render a decision without it: `GET /v1/approvals/{id}` and `/claim` both require
it, so the earlier `[openid, approval:approve]` would have let an approver submit

View file

@ -49,14 +49,18 @@ human_registration:
client_type: public
grant: authorization_code + S256 PKCE
never: [approval:consume]
blocked_on_keycape_side: |
A human access token cannot carry tenant:platform today. The tenant claim on
a human token is resolved from the directory user (effectiveTenant in
src/internal/server/oidc/token.go), not from the client registration, and no
adapter populates User.Tenant -- so every human token defaults to
tenant:coulomb. approval-engine compares tenant by exact string equality and
refuses near-miss spellings, so an approver token would be rejected. This
must be resolved before the registration is issued, not after.
# Required. Without it the token carries tenant:coulomb from the directory
# default and approval-engine refuses it. See docs/tenant-claim-contract.md.
tenant: tenant:platform
keycape_side_resolved: |
2026-09-09: a human token can now carry tenant:platform. A client
registration may declare a tenant; humanTenant() supplies it when the
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.
verification:
# The first two lines are now one runnable command per client; see
# docs/native-authentication.md, "Verifying a live registration". It writes

View file

@ -84,6 +84,46 @@ KEY-WP-0013-T02; KeyCape changed no live registration or policy subject.
drift or an alias reintroduced into `config/service-clients.example.yaml` fails
the build.
## How a human token's tenant is resolved (KEY-WP-0013-T05)
A human's tenant is normally a property of the person, read from the directory
record. That alone could not serve the approval chain: decision
`5ed3fb35-eca9-413a-82b9-95171ba85bf6` binds it to the landlord zone,
approval-engine compares the claim by exact string equality, and no adapter
populates `domain.User.Tenant` — so every human token fell back to
`tenant:coulomb` and an approver token would have been refused downstream. It
would have presented as a failed approval rather than as a registration defect.
A client registration may therefore declare a `tenant`, and `humanTenant` in
`src/internal/server/oidc/token.go` resolves it by these four rules:
| Client declares | Directory assigns | Result |
| --- | --- | --- |
| nothing | anything | the directory answer, or `tenant:coulomb` — unchanged |
| a zone | nothing | the declared zone |
| a zone | the same zone | that zone; client and directory agree |
| a zone | a **different** zone | **issuance is refused** |
The last row is the point. A registration can bind a zone for users the
directory has not placed, and can never relabel a user it *has* placed. That
case fails closed rather than picking a winner, because either answer would be a
silent cross-tenant assertion. The refusal is a `403` with
`error_type: tenant_binding`, distinct from an authentication failure, so an
operator can tell a misconfigured registration from a rejected login.
This also means the design survives the other resolution. If the directory later
carries tenants, the same code stops supplying the zone and starts enforcing
agreement with it — no second migration, and no window in which a stale
registration silently wins.
It is safe only because client registrations are static and deployment-owned.
KeyCape excludes dynamic client registration by design; a self-service client
able to name its users' tenant would be a straightforward escalation, and this
rule must be revisited if that exclusion is ever lifted.
Covered by `src/internal/server/oidc/human_tenant_test.go`, including the
relabel refusal.
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.

View file

@ -3,6 +3,7 @@ package config_test
import (
"os"
"path/filepath"
"strings"
"testing"
"keycape/internal/config"
@ -529,3 +530,81 @@ func TestValidate_TenantEngine(t *testing.T) {
})
}
}
// Service-identity fields are read only on the client_credentials path. Silently
// ignoring them on a browser client is the trap behind KEY-WP-0013-T05: an
// approver client registered with tenant: tenant:platform starts cleanly and
// issues the directory tenant instead, which a resource server comparing tenant
// exactly refuses — surfacing as a failed approval, not a registration defect
// (KEY-WP-0028).
func TestValidate_ServiceIdentityFieldsRejectedOnBrowserClients(t *testing.T) {
browserClient := func() config.ClientConfig {
return config.ClientConfig{
ClientID: "approver-ui",
RedirectURIs: []string{"https://approve.example.com/callback"},
AllowedScopes: []string{"openid", "approval:approve"},
GrantTypes: []string{"authorization_code"},
ClientType: "public",
}
}
cases := map[string]struct {
mutate func(*config.ClientConfig)
want string
}{
"tenant": {func(c *config.ClientConfig) { c.Tenant = "tenant:platform" }, "tenant"},
"serviceSubject": {func(c *config.ClientConfig) { c.ServiceSubject = "service:approver" }, "serviceSubject"},
"roles": {func(c *config.ClientConfig) { c.Roles = []string{"approver"} }, "roles"},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
cfg := validConfig(writeTempFile(t, "key"))
client := browserClient()
tc.mutate(&client)
cfg.Clients = append(cfg.Clients, client)
errs := config.ValidateConfig(cfg)
found := false
for _, e := range errs {
if strings.Contains(e, tc.want) {
found = true
}
}
if !found {
t.Fatalf("expected an error naming %q, got %v", tc.want, errs)
}
})
}
// A client that omits grantTypes is an implicit authorization-code client,
// so the same rule applies to it.
t.Run("implicit authorization_code client", func(t *testing.T) {
cfg := validConfig(writeTempFile(t, "key"))
client := browserClient()
client.GrantTypes = nil
client.Tenant = "tenant:platform"
if errs := config.ValidateConfig(cfg); len(errs) != 0 {
t.Fatalf("baseline config invalid: %v", errs)
}
cfg.Clients = append(cfg.Clients, client)
if errs := config.ValidateConfig(cfg); len(errs) == 0 {
t.Fatal("expected tenant to be rejected on an implicit authorization-code client")
}
})
// The service clients that legitimately carry these fields still validate.
t.Run("service clients unaffected", func(t *testing.T) {
cfg := validConfig(writeTempFile(t, "key"))
cfg.Clients = append(cfg.Clients, config.ClientConfig{
ClientID: "svc",
AllowedScopes: []string{"approval:read"},
GrantTypes: []string{"client_credentials"},
ClientType: "confidential",
SecretRef: "env:SVC_SECRET",
ServiceSubject: "service:svc",
Tenant: "tenant:platform",
Roles: []string{"svc"},
})
if errs := config.ValidateConfig(cfg); len(errs) != 0 {
t.Fatalf("service client rejected: %v", errs)
}
})
}

View file

@ -89,8 +89,28 @@ func ValidateConfig(cfg *Config) []string {
errs = append(errs, prefix+": tokenLifetime must be between 1m and 1h")
}
}
} else if c.TokenLifetime != "" {
errs = append(errs, prefix+": tokenLifetime is only supported for client_credentials clients")
} else {
if c.TokenLifetime != "" {
errs = append(errs, prefix+": tokenLifetime is only supported for client_credentials clients")
}
// Service-identity fields are read only on the client_credentials
// path. On a browser client they are silently ignored, and the
// tenant one is a trap worth failing on: an approver client
// registered with tenant: tenant:platform starts cleanly and then
// issues tokens carrying the directory tenant, which a resource
// server comparing tenant exactly will refuse. That surfaces as a
// mysterious failed approval, not as a registration defect
// (KEY-WP-0028). Human tenant comes from the directory user; see
// docs/approval-engine-auth-contract.md.
if c.Tenant != "" {
errs = append(errs, prefix+": tenant is only read for client_credentials clients; a browser client's tenant claim comes from the directory user, so this value would be silently ignored")
}
if c.ServiceSubject != "" {
errs = append(errs, prefix+": serviceSubject is only read for client_credentials clients; a browser client's subject is the directory user")
}
if len(c.Roles) > 0 {
errs = append(errs, prefix+": roles is only read for client_credentials clients; a browser client's roles come from the directory user")
}
}
// Warn about wildcard redirect URIs (they are blocked at runtime anyway).
for _, uri := range c.RedirectURIs {

View file

@ -0,0 +1,75 @@
package oidc
import (
"testing"
"keycape/internal/domain"
)
// KEY-WP-0013-T05. Without a client-declared tenant every human token fell back
// to the platform default, so an approver token would have carried
// tenant:coulomb and been refused by approval-engine's exact string comparison
// -- surfacing as a failed approval rather than as a registration defect.
func TestHumanTenantBindsTheZoneAClientDeclares(t *testing.T) {
approver := &domain.Client{ClientID: "informed-decision", Tenant: "tenant:platform"}
unplaced := &domain.User{ID: "user:alice"}
got, err := humanTenant(approver, unplaced)
if err != nil {
t.Fatalf("declared zone refused for an unassigned user: %v", err)
}
if got != "tenant:platform" {
t.Errorf("tenant = %q, want tenant:platform", got)
}
}
func TestHumanTenantKeepsDirectoryAnswerWhenNoClientTenantIsDeclared(t *testing.T) {
for _, tc := range []struct {
name string
client *domain.Client
user *domain.User
want string
}{
{"no client at all", nil, &domain.User{ID: "u"}, defaultTenant},
{"client declares nothing", &domain.Client{ClientID: "demo-app"}, &domain.User{ID: "u"}, defaultTenant},
{"directory answer wins", &domain.Client{ClientID: "demo-app"}, &domain.User{ID: "u", Tenant: "tenant:friendly:binky"}, "tenant:friendly:binky"},
} {
t.Run(tc.name, func(t *testing.T) {
got, err := humanTenant(tc.client, tc.user)
if err != nil || got != tc.want {
t.Errorf("humanTenant = %q, %v; want %q, nil", got, err, tc.want)
}
})
}
}
// The escalation this design has to refuse: a registration must never be able to
// relabel a user the directory has already placed in another tenant. Neither
// answer is safe there, so it fails closed rather than picking a winner.
func TestHumanTenantRefusesToRelabelAPlacedUser(t *testing.T) {
approver := &domain.Client{ClientID: "informed-decision", Tenant: "tenant:platform"}
placed := &domain.User{ID: "user:bob", Tenant: "tenant:friendly:binky"}
got, err := humanTenant(approver, placed)
if err == nil {
t.Fatalf("client relabelled a placed user into %q", got)
}
if got != "" {
t.Errorf("a refused binding still returned a tenant: %q", got)
}
}
// Once the directory populates Tenant, the same code turns from supplying the
// zone into enforcing agreement with it. No second migration.
func TestHumanTenantAcceptsAgreementBetweenClientAndDirectory(t *testing.T) {
approver := &domain.Client{ClientID: "informed-decision", Tenant: "tenant:platform"}
placed := &domain.User{ID: "user:carol", Tenant: "tenant:platform"}
got, err := humanTenant(approver, placed)
if err != nil {
t.Fatalf("agreeing client and directory refused: %v", err)
}
if got != "tenant:platform" {
t.Errorf("tenant = %q, want tenant:platform", got)
}
}

View file

@ -8,6 +8,7 @@ import (
"crypto/subtle"
"encoding/base64"
"encoding/json"
"fmt"
"net/http"
"net/url"
"strings"
@ -196,7 +197,14 @@ func (h *TokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
// Core claims required by net-kingdom/canon/standards/iam-profile_v0.3.md
// for every production token -- not scope-gated, unlike the recommended
// human claims above (KEY-WP-0005-T01).
tenant := effectiveTenant(user)
tenant, err := humanTenant(h.ClientConfig[clientID], user)
if err != nil {
profileerrors.RejectedForSafety(
"tenant binding conflict",
"tenant_binding",
).Write(w, http.StatusForbidden)
return
}
claims["tenant"] = tenant
claims["principal_type"] = "human"
claims["groups"] = nonNilStrings(user.Groups)
@ -398,6 +406,43 @@ func effectiveTenant(user *domain.User) string {
return defaultTenant
}
// humanTenant resolves the tenant claim for a human token (KEY-WP-0013-T05).
//
// A human's tenant is normally a property of the person, read from the
// directory. But the approval chain is bound to the landlord zone by decision
// 5ed3fb35-eca9-413a-82b9-95171ba85bf6, and approval-engine compares the claim
// by exact string equality, so an approver client has to be able to state the
// zone it issues into. Without this every human token fell back to
// defaultTenant and would have been refused downstream -- as a failed approval
// rather than as a registration defect.
//
// The rule is deliberately not an override:
//
// - no client tenant declared -> the directory answer, unchanged;
// - declared, user unassigned -> the declared zone;
// - declared and equal -> agreement, no ambiguity;
// - declared and different -> refuse to issue.
//
// So a registration can bind a zone for users the directory has not placed, and
// can never relabel a user the directory HAS placed into a different one. That
// last case fails closed rather than picking a winner, because either answer
// would be a silent cross-tenant assertion. It also means this stays correct if
// the directory later populates Tenant: the same code turns from supplying the
// zone into enforcing agreement with it, with no second migration.
//
// This is only safe because client registrations are static and
// deployment-owned; KeyCape excludes dynamic client registration by design. A
// self-service client that could name its users' tenant would be an escalation.
func humanTenant(client *domain.Client, user *domain.User) (string, error) {
if client == nil || client.Tenant == "" {
return effectiveTenant(user), nil
}
if user.Tenant != "" && user.Tenant != client.Tenant {
return "", fmt.Errorf("client %q binds tenant %q but the directory assigns this user a different tenant", client.ClientID, client.Tenant)
}
return client.Tenant, nil
}
// nonNilStrings returns s, or an empty (non-nil) slice if s is nil, so the
// claim always serializes as `[]`, never `null` -- the profile requires
// groups/roles to be present, "possibly empty", not absent.

View file

@ -230,7 +230,7 @@ id: KEY-WP-0013-T05
status: wait
priority: high
assignee: the-custodian
blocking_reason: "Owner found 2026-09-09 (informed-decision); client ID and deployed callback still pending INFD-WP-0001-T07, and a human token cannot carry tenant:platform yet."
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"
```
@ -289,8 +289,35 @@ design. KeyCape leans to (2) and deliberately implemented neither: (1) is a
directory-ownership question and (2) writes a cross-tenant capability into the
issuer.
Task stays `wait` on the tenant decision, then `client_id` and callback URI from
INFD-WP-0001-T07 once that repo has a deployed origin.
**Tenant blocker resolved in code, 2026-09-09.** On review the choice did not
have to be made to unblock: the two resolutions differ in where a human's tenant
*comes from*, and an implementation exists that is correct under either. A client
registration may now declare a `tenant`, and `humanTenant()` resolves it by four
rules — no declaration keeps the directory answer unchanged; a declared zone
applies when the directory has placed the user nowhere; agreement passes; and a
declared zone that **conflicts** with a directory assignment refuses issuance
(`403`, `error_type: tenant_binding`) rather than relabelling the user.
That refusal is the whole design. A registration can bind a zone for unplaced
users and can never move a placed one, so option (2) is available now without
writing a general cross-tenant override into the issuer, and if the owners prefer
option (1) the same code stops supplying the zone and starts enforcing agreement
with it — no second migration and no window where a stale registration silently
wins. It is safe only because registrations are static and deployment-owned;
`docs/tenant-claim-contract.md` records that this rule must be revisited if
dynamic client registration is ever admitted.
Evidence: `src/internal/server/oidc/human_tenant_test.go`, four cases including
the relabel refusal. Verified with teeth — neutering the conflict check makes
`TestHumanTenantRefusesToRelabelAPlacedUser` fail rather than pass silently.
`docs/tenant-claim-contract.md` carries the resolution table,
`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.
## Make negative rollout evidence discriminate actual issuer refusal