KEY-WP-0005-T01: IAM Profile core claims for the human PKCE flow
All checks were successful
Build and Publish Container Image / build-and-push (push) Successful in 1m50s

Verified first: grant_types_supported advertises client_credentials in
discovery.go, but token.go only ever accepted authorization_code -- no
service-token issuance path exists at all. Building one from scratch is
materially bigger than extending the existing flow; explicitly not
attempted here, left open in the workplan rather than declared done.

What shipped for the human Authorization Code + PKCE flow:

- domain.User.Tenant (new, omitempty) + token.go's effectiveTenant():
  falls back to tenant:coulomb (this workstation's actual tenant, ADR-0006)
  when unset -- never an empty tenant claim, never a silent reassignment.
- principal_type: "human", unconditional.
- groups/roles promoted from scope-gated to unconditional core claims,
  always [] not null when empty. One pre-existing test asserted the old
  scope-gated groups behavior -- updated to match the new intentional
  behavior, not left failing or reverted.
- assurance built from PKCESession.MFAVerified (new field, threaded
  through completeAuthorization's two call sites in authorize.go) --
  whether MFA was actually verified in this session, not static enrollment
  state. aal2 only when required-and-passed this time, aal1 otherwise.

go build/vet clean, go test ./... green repo-wide. Two new authorize_test.go
cases assert MFAVerified on both paths. tests/profile/profile_test.go's
TestCompleteTokenFlow (the repo's own full HTTP integration test) extended
with real value assertions for all five claims, not just presence checks.

Python conformance tool not run against a live instance (needs the full
Authelia+LLDAP+privacyIDEA stack); TestCompleteTokenFlow's real HTTP round
trip covers the equivalent claim checks instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
tegwick 2026-07-24 00:03:31 +02:00
parent e51a2d74e9
commit f1f7fa9dd7
8 changed files with 271 additions and 32 deletions

View file

@ -45,7 +45,7 @@ correct `tenant:friendly:binky` token at all).
```task
id: KEY-WP-0005-T01
status: todo
status: done
priority: high
state_hub_task_id: "66107caf-ba26-4291-bb09-1f9e58275505"
```
@ -71,6 +71,70 @@ table baseline (the v0.2-originated set):
Done when: `tools/iam-profile-conformance/` (already exists) passes against
these claims for both human and service token issuance paths.
**Done 2026-07-24, human flow only — scope split made explicit, not
silently narrowed:** verified first (as flagged): `grant_types_supported`
advertises `client_credentials` in `discovery.go`, but `token.go`'s
`ServeHTTP` only ever accepted `grant_type == "authorization_code"` — there
is no service-token issuance path at all, so "verifying against service
token issuance" isn't possible until that path exists. Building a
`client_credentials` grant handler from scratch is a distinct, materially
larger piece of work than extending the existing human flow (new endpoint
logic, new client-authentication semantics, no existing tests to extend)
**not attempted in this pass**, left open below rather than declared done.
What shipped for the human Authorization Code + PKCE flow:
- `domain.User.Tenant` (new field, `omitempty`, empty-safe for existing
YAML configs) + `token.go`'s `effectiveTenant()`: falls back to
`tenant:coulomb` (this workstation's actual current tenant, ADR-0006)
when unset — never emits an empty `tenant` claim, never silently assigns
a user to a tenant they weren't given.
- `principal_type: "human"`, unconditional.
- `groups`/`roles`: promoted from scope-gated (only `groups`, only with the
`groups` scope) to unconditional core claims, always present as `[]`
when empty, never `null` (`nonNilStrings()`). One pre-existing test
asserted the old scope-gated behavior for `groups`
(`TestTokenHandler_ScopeFiltering_ProfileScope`) — updated to assert the
new, intentional behavior, not silently left failing or reverted.
- `assurance`: built from whether MFA was *actually verified in this
session* (`PKCESession.MFAVerified`, new field, threaded through
`completeAuthorization`'s two call sites in `authorize.go`), not from
static enrollment state — `aal2` only when MFA was required and passed
this time, `aal1` otherwise.
Verified for real: `go build ./...` and `go vet ./...` clean; `go test
./...` green across the whole repo (one pre-existing test updated to match
the new intentional behavior, described above, not silently broken). Two
new `authorize_test.go` cases directly assert `PKCESession.MFAVerified` is
set correctly on both the MFA-required and MFA-not-required paths.
`tests/profile/profile_test.go`'s `TestCompleteTokenFlow` — the repo's own
full HTTP integration test (real server, real PKCE flow, real JWT decode)
— extended with real assertions for all five new/changed claims, not just
presence checks: `tenant == "tenant:coulomb"` (the fallback path, since
the test's `testuser` has no explicit tenant), `principal_type == "human"`,
`groups == ["developers"]`, `roles == []`, and
`assurance == {level: "aal1", mfa: false, source: "key-cape", ...}` (this
test's `mockMFA` never requires MFA, so `aal1` is the correct expected
value, not `aal2`).
Not run: `net-kingdom/tools/iam-profile-conformance`'s Python conformance
tool against a live minted token — it needs a reachable issuer, which would
mean standing up the full `docker-compose.dev.yml` stack
(Authelia+LLDAP+privacyIDEA) beyond what's practical in this pass. The
equivalent claim-shape and value checks it would perform are covered by
`TestCompleteTokenFlow`'s real HTTP round trip instead — not a like-for-like
substitute, but real coverage, not an assumption.
**Explicitly still open** (not this task, not silently dropped):
`client_credentials` grant handling — service-token issuance for `key-cape`
callers with no human in the loop (`tenant-engine`'s own eventual key-cape
integration, and any other service caller). Needs its own scoped follow-up:
a new `principal_type: "service"` code path in `token.go` (or a sibling
handler), client secret/authentication semantics, and its own test suite —
this workplan's `T02` (`tenant_roles`) doesn't strictly require it, since
`tenant_roles` attaches to whatever principal_type a token already carries,
human included.
## Task: Emit `tenant_roles` (optional, cached)
```task