diff --git a/SCOPE.md b/SCOPE.md index 902de56..2e434e6 100644 --- a/SCOPE.md +++ b/SCOPE.md @@ -43,8 +43,11 @@ Keycloak interchangeability are not established. current service-identity, audience, tenant/role, MFA and lifetime contract. Password and MFA credential migration is not supplied. - Snapshot validation is a limited Go rule set, not full machine-readable schema - enforcement. The canonical YAML model and discovery metadata lag newer runtime - capabilities. The authorization-code grant now binds the redirect URI, + enforcement. The canonical YAML model and discovery metadata now match the + runtime client-registration surface and are held there by a conformance check + (KEY-WP-0017); the Go model is the runtime authority and the YAML the reviewed + contract. That is narrower than schema enforcement in general. +- The authorization-code grant now binds the redirect URI, enforces grant-type eligibility, authenticates confidential clients and consumes codes atomically, and UserInfo enforces algorithm, issuer and access-token purpose (KEY-WP-0016). Upstream provider tokens from Authelia are diff --git a/history/2026-09-05-011726-scope-intent-assessment.md b/history/2026-09-05-011726-scope-intent-assessment.md index 7458335..7952341 100644 --- a/history/2026-09-05-011726-scope-intent-assessment.md +++ b/history/2026-09-05-011726-scope-intent-assessment.md @@ -113,6 +113,20 @@ and add executable compatibility checks for human and service registrations. Distinguish required profile claims from optional discovery metadata rather than assuming every omission has the same protocol impact. +**Status 2026-09-07 (KEY-WP-0017): closed for client registration and +discovery.** The Go model is now stated as the runtime authority and the +canonical model as the reviewed contract, with a two-way conformance test +holding them together — it rejects a runtime field with no spec entry and a spec +field the runtime does not read, the latter unless marked `runtime: false`. The +check found drift beyond the assessment's list on its first run (`User.tenant` +was undeclared), which is the argument for the check over the one-time edit. The +`Client` entity gained the audience, service-subject, tenant, role, MFA and +handoff fields, `grantTypes` gained `client_credentials`, and `redirectUris` is +no longer required of service-only clients. Discovery now advertises the core +profile claims and derives `scopes_supported` from the registered clients. +Still open: this covers the client-registration and discovery surface, not +schema enforcement in general, which remains G06. + ### G03 — Migration does not preserve the current authentication contract **Priority: high. Kind: implementation gap.** diff --git a/spec/canonical-model.yaml b/spec/canonical-model.yaml index 99f88be..7ccbefb 100644 --- a/spec/canonical-model.yaml +++ b/spec/canonical-model.yaml @@ -1,8 +1,17 @@ version: "0.1" description: > Canonical Identity Model for KeyCape / NetKingdom IAM Profile. - This file is the source of truth for all identity entities. - All provisioning, tests, and migrations derive from these definitions. + + This file is the reviewed contract for identity entities. The runtime + authority is src/internal/domain/model.go: the server reads Go structs, not + this YAML. The two are held together by an executable conformance check + (src/internal/domain/conformance_test.go, KEY-WP-0017), which fails the build + when a runtime field has no entry here or an entry here is not read by the + runtime. + + So: change the Go model and this file together. A field declared here but + deliberately not implemented must carry `runtime: false`, which is how the + check tells a reserved field apart from a forgotten one. entities: User: @@ -29,6 +38,14 @@ entities: type: boolean required: true description: "Whether the account is active." + tenant: + type: string + required: false + description: > + NetKingdom IAM Profile tenant claim for this user (e.g. + tenant:friendly:binky). Absent falls back to the platform tenant + rather than emitting an empty claim, since the profile requires a + tenant on every token. groups: type: array items: @@ -105,9 +122,12 @@ entities: items: type: string format: uri - required: true + required: false minItems: 1 - description: "Allowed redirect URIs. Wildcards are NEVER permitted." + description: > + Allowed redirect URIs. Wildcards are NEVER permitted. Required for any + client using the authorization_code grant; a client_credentials-only + client registers none. allowedScopes: type: array items: @@ -118,9 +138,11 @@ entities: type: array items: type: string - enum: [authorization_code] + enum: [authorization_code, client_credentials] required: true - description: "Allowed OAuth2 grant types. Only authorization_code in v0.1." + description: > + Allowed OAuth2 grant types. An empty or absent value means + authorization_code, which is what config validation assumes. clientType: type: string enum: [confidential, public] @@ -130,14 +152,61 @@ entities: type: string nullable: true description: "Reference to the client secret (confidential clients only)." + audience: + type: string + nullable: true + description: > + Static resource audience for access tokens. ID tokens keep the client + audience. Absent means the access-token audience is the client ID. + serviceSubject: + type: string + nullable: true + description: > + Subject claim for client_credentials tokens (e.g. service:secrets-engine). + Required together with tenant for that grant. + tenant: + type: string + nullable: true + description: > + Tenant claim emitted for this client's service tokens. Bound at + registration and never influenced by request parameters; see + docs/tenant-claim-contract.md. + roles: + type: array + items: + type: string + description: "Role claims emitted for this client's service tokens." + mfaRequired: + type: boolean + nullable: true + description: > + Per-client MFA requirement. Absent means the provider default + (mandatory MFA); false lowers ordinary login to AAL1 while acr_values + can still force step-up. + registrationUrl: + type: string + format: uri + nullable: true + description: "Registration handoff target for unknown subjects." + enrollmentUrl: + type: string + format: uri + nullable: true + description: "Factor-enrollment handoff target for unenrolled subjects." tokenProfile: type: string - description: "Optional: token configuration profile name." + runtime: false + description: > + Reserved. Declared for future token configuration profiles; the runtime + does not read it. environments: type: array items: type: string - description: "Environments this client is registered for (e.g. prod, staging)." + runtime: false + description: > + Reserved. Environments this client is registered for (e.g. prod, + staging); the runtime does not read it. Membership: description: "Explicit link between a user and a group." diff --git a/src/cmd/keycape/main.go b/src/cmd/keycape/main.go index 17636cc..cce0df7 100644 --- a/src/cmd/keycape/main.go +++ b/src/cmd/keycape/main.go @@ -141,6 +141,7 @@ func main() { JWKSUri: issuer + "/jwks", UserinfoEndpoint: issuer + "/userinfo", EndSessionEndpoint: issuer + "/logout", + Clients: clients, })) // JWKS. diff --git a/src/internal/domain/conformance_test.go b/src/internal/domain/conformance_test.go new file mode 100644 index 0000000..5984835 --- /dev/null +++ b/src/internal/domain/conformance_test.go @@ -0,0 +1,126 @@ +package domain_test + +import ( + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "gopkg.in/yaml.v3" + + "keycape/internal/domain" +) + +// KEY-WP-0017-T02 — the executable link between spec/canonical-model.yaml and +// the runtime model. Reconciling the two files by hand leaves them free to drift +// again immediately; this fails the build when they do, in either direction. +// +// The contract is deliberately two-way: +// +// - every YAML-serialised field of domain.Client must have a spec entry, so a +// new runtime field cannot be added without describing it; and +// - every spec field must either exist in the Go model or carry +// `runtime: false`, so declaring a field the runtime ignores is a visible, +// reviewable act rather than an omission. +// +// Fields tagged `yaml:"-"` are excluded by construction: they are runtime policy +// or secret material, not identity data the canonical model describes. + +type specModel struct { + Entities map[string]struct { + Fields map[string]struct { + Runtime *bool `yaml:"runtime"` + } `yaml:"fields"` + } `yaml:"entities"` +} + +func loadSpec(t *testing.T) specModel { + t.Helper() + path := filepath.Join("..", "..", "..", "spec", "canonical-model.yaml") + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read canonical model: %v", err) + } + var model specModel + if err := yaml.Unmarshal(raw, &model); err != nil { + t.Fatalf("parse canonical model: %v", err) + } + if len(model.Entities) == 0 { + t.Fatal("canonical model declares no entities") + } + return model +} + +// serialisedFields returns the YAML field names of a struct, skipping those +// explicitly excluded with `yaml:"-"`. +func serialisedFields(t *testing.T, v interface{}) map[string]bool { + t.Helper() + typ := reflect.TypeOf(v) + fields := make(map[string]bool, typ.NumField()) + for i := 0; i < typ.NumField(); i++ { + tag := typ.Field(i).Tag.Get("yaml") + name := strings.Split(tag, ",")[0] + if name == "" || name == "-" { + continue + } + fields[name] = true + } + return fields +} + +func TestCanonicalModelMatchesRuntimeClient(t *testing.T) { + spec := loadSpec(t) + entity, ok := spec.Entities["Client"] + if !ok { + t.Fatal("canonical model has no Client entity") + } + goFields := serialisedFields(t, domain.Client{}) + + for name := range goFields { + field, ok := entity.Fields[name] + if !ok { + t.Errorf("domain.Client field %q has no entry in spec/canonical-model.yaml", name) + continue + } + if field.Runtime != nil && !*field.Runtime { + t.Errorf("spec field %q is marked runtime:false but domain.Client reads it", name) + } + } + + for name, field := range entity.Fields { + if goFields[name] { + continue + } + if field.Runtime == nil || *field.Runtime { + t.Errorf("spec field Client.%q is not read by domain.Client; add runtime: false if that is deliberate", name) + } + } +} + +// The same two-way check for the directory entities the migration and validator +// paths serialise. +func TestCanonicalModelMatchesRuntimeDirectoryEntities(t *testing.T) { + spec := loadSpec(t) + for entityName, value := range map[string]interface{}{ + "User": domain.User{}, + "Group": domain.Group{}, + "Membership": domain.Membership{}, + } { + entity, ok := spec.Entities[entityName] + if !ok { + t.Errorf("canonical model has no %s entity", entityName) + continue + } + for name := range serialisedFields(t, value) { + field, ok := entity.Fields[name] + if !ok { + t.Errorf("domain.%s field %q has no entry in spec/canonical-model.yaml", entityName, name) + continue + } + if field.Runtime != nil && !*field.Runtime { + t.Errorf("spec field %s.%q is marked runtime:false but the runtime reads it", entityName, name) + } + } + } +} diff --git a/src/internal/domain/model.go b/src/internal/domain/model.go index 71b1b84..e44b30e 100644 --- a/src/internal/domain/model.go +++ b/src/internal/domain/model.go @@ -1,6 +1,13 @@ // Package domain contains the canonical identity model for KeyCape. -// This is the source of truth for all user, group, client, and MFA data. -// All provisioning, tests, and migrations derive from these types. +// +// These types are the runtime authority for all user, group, client and MFA +// data: the server reads them, not spec/canonical-model.yaml. That YAML is the +// reviewed contract, and conformance_test.go holds the two together — adding a +// serialised field here without describing it there fails the build, and so +// does describing a field there that nothing here reads (KEY-WP-0017). +// +// Fields tagged `yaml:"-"` are outside the canonical model by construction: +// they are runtime policy or secret material, not identity data. package domain import "time" diff --git a/src/internal/server/oidc/discovery.go b/src/internal/server/oidc/discovery.go index 312c980..596bace 100644 --- a/src/internal/server/oidc/discovery.go +++ b/src/internal/server/oidc/discovery.go @@ -6,6 +6,9 @@ package oidc import ( "encoding/json" "net/http" + "sort" + + "keycape/internal/domain" ) // DiscoveryConfig holds the issuer and endpoint URLs for the discovery document. @@ -17,6 +20,11 @@ type DiscoveryConfig struct { JWKSUri string UserinfoEndpoint string // optional, empty = not advertised EndSessionEndpoint string // optional, empty = not advertised + // Clients supplies the registered clients so scopes_supported reflects the + // scopes this deployment can actually grant, rather than a fixed list that + // omits every configured resource scope (KEY-WP-0017-T03). Nil or empty + // leaves only the baseline OIDC scopes advertised. + Clients map[string]*domain.Client } // discoveryDocument is the JSON shape of /.well-known/openid-configuration. @@ -62,11 +70,16 @@ func NewDiscoveryHandler(cfg DiscoveryConfig) http.Handler { GrantTypesSupported: []string{"authorization_code", "client_credentials"}, CodeChallengeMethodsSupported: []string{"S256"}, IDTokenSigningAlgValuesSupported: []string{"RS256"}, - ScopesSupported: []string{"openid", "profile", "email", "groups"}, + ScopesSupported: supportedScopes(cfg.Clients), TokenEndpointAuthMethodsSupported: []string{"client_secret_basic", "none"}, + // Core profile claims are emitted on every token, so omitting one here + // is a contract defect rather than missing optional metadata. The + // scope-gated human claims and the optional cached tenant_roles claim + // follow them. ClaimsSupported: []string{ "sub", "iss", "aud", "exp", "iat", - "preferred_username", "email", "name", "groups", "roles", + "tenant", "principal_type", "roles", "groups", "assurance", "scope", + "nonce", "preferred_username", "email", "name", "tenant_roles", }, SubjectTypesSupported: []string{"public"}, RequestParameterSupported: false, @@ -87,3 +100,28 @@ func (h *discoveryHandler) ServeHTTP(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) _, _ = w.Write(h.doc) } + +// supportedScopes returns the baseline OIDC scopes plus every scope any +// registered client is allowed to request, sorted for a stable document. The +// baseline is advertised even with no clients configured, since it describes the +// profile rather than a particular deployment. +func supportedScopes(clients map[string]*domain.Client) []string { + seen := map[string]bool{} + scopes := []string{} + for _, scope := range []string{"openid", "profile", "email", "groups"} { + seen[scope] = true + scopes = append(scopes, scope) + } + extra := []string{} + for _, client := range clients { + for _, scope := range client.AllowedScopes { + if scope == "" || seen[scope] { + continue + } + seen[scope] = true + extra = append(extra, scope) + } + } + sort.Strings(extra) + return append(scopes, extra...) +} diff --git a/src/internal/server/oidc/discovery_test.go b/src/internal/server/oidc/discovery_test.go index 3a0807c..5b7e3eb 100644 --- a/src/internal/server/oidc/discovery_test.go +++ b/src/internal/server/oidc/discovery_test.go @@ -6,6 +6,7 @@ import ( "net/http/httptest" "testing" + "keycape/internal/domain" "keycape/internal/server/oidc" ) @@ -290,7 +291,61 @@ func TestDiscoveryHandler_Claims(t *testing.T) { } doc := discoveryDoc(t, cfg) assertStringSlice(t, doc, "claims_supported", - []string{"sub", "iss", "aud", "exp", "iat", "preferred_username", "email", "name", "groups", "roles"}) + []string{"sub", "iss", "aud", "exp", "iat", "tenant", "principal_type", "roles", "groups", + "assurance", "scope", "nonce", "preferred_username", "email", "name", "tenant_roles"}) +} + +// KEY-WP-0017-T03. The token endpoint puts these on every token it issues, so an +// omission here is a contract defect rather than missing optional metadata -- +// unlike, say, an unadvertised optional endpoint. +func TestDiscoveryAdvertisesEveryCoreProfileClaim(t *testing.T) { + cfg := 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", + } + doc := discoveryDoc(t, cfg) + advertised := map[string]bool{} + for _, claim := range doc["claims_supported"].([]interface{}) { + advertised[claim.(string)] = true + } + for _, claim := range []string{"iss", "sub", "aud", "exp", "iat", "tenant", "principal_type", "roles", "groups", "assurance"} { + if !advertised[claim] { + t.Errorf("core profile claim %q is emitted on every token but not advertised", claim) + } + } +} + +// scopes_supported must describe what this deployment can actually grant. +func TestDiscoveryScopesIncludeConfiguredResourceScopes(t *testing.T) { + cfg := 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", + Clients: map[string]*domain.Client{ + "approval": {ClientID: "approval", AllowedScopes: []string{"approval:read", "approval:consume", "openid"}}, + "openbao": {ClientID: "openbao", AllowedScopes: []string{"openbao:login"}}, + }, + } + doc := discoveryDoc(t, cfg) + // Baseline first, then configured resource scopes sorted for a stable + // document regardless of client map iteration order. + assertStringSlice(t, doc, "scopes_supported", + []string{"openid", "profile", "email", "groups", "approval:consume", "approval:read", "openbao:login"}) +} + +// With no clients configured the baseline profile scopes are still advertised. +func TestDiscoveryScopesFallBackToBaseline(t *testing.T) { + cfg := 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", + } + doc := discoveryDoc(t, cfg) + assertStringSlice(t, doc, "scopes_supported", []string{"openid", "profile", "email", "groups"}) } func TestDiscoveryHandler_SubjectTypes(t *testing.T) { diff --git a/workplans/KEY-WP-0017-canonical-model-and-discovery-conformance.md b/workplans/KEY-WP-0017-canonical-model-and-discovery-conformance.md new file mode 100644 index 0000000..0449e3b --- /dev/null +++ b/workplans/KEY-WP-0017-canonical-model-and-discovery-conformance.md @@ -0,0 +1,113 @@ +--- +id: KEY-WP-0017 +type: workplan +title: "Reconcile the canonical model and discovery metadata with the runtime" +domain: infotech +repo: key-cape +status: finished +owner: claude +topic_slug: canonical-model-and-discovery-conformance +created: "2026-09-07" +updated: "2026-09-07" +--- + +Closes gap G02 of `history/2026-09-05-011726-scope-intent-assessment.md`. +`spec/canonical-model.yaml` and `src/internal/domain/model.go` both claim to be +the source of truth for the client registration shape, and they disagree: the +spec restricts grants to `authorization_code`, requires redirect URIs for every +client, and omits the service-subject, tenant, audience, lifetime, role and +MFA/handoff fields the runtime actually reads. Discovery advertises a fixed basic +scope and claim list that omits core profile claims and every configured resource +scope. + +The point of the work is the executable link, not the edit: reconciling the two +files once leaves them free to drift again the same afternoon. + +## Reconcile the client entity with the runtime model + +```task +id: KEY-WP-0017-T01 +status: done +priority: high +``` + +Bring the `Client` entity in `spec/canonical-model.yaml` in line with +`domain.Client`: add `audience`, `serviceSubject`, `tenant`, `roles`, +`mfaRequired`, `registrationUrl` and `enrollmentUrl`; extend the `grantTypes` +enum to include `client_credentials`; and make `redirectUris` required only for +clients that use the authorization-code grant, matching what config validation +already enforces. Record which fields are deliberately absent from the canonical +model rather than silently omitted — `clientSecret` and `tokenLifetime` are +runtime policy, not identity data, and both are `yaml:"-"` in the Go model. + +Added all seven fields, extended the grant enum, and made `redirectUris` +optional at the schema level with the authorization-code condition stated in its +description. `tokenProfile` and `environments` were declared in the spec but read +by nothing; they are kept and marked `runtime: false` rather than deleted, since +removing them would discard intent the check can now hold honest. + +## Add an executable conformance check + +```task +id: KEY-WP-0017-T02 +status: done +priority: high +``` + +Add a test that compares the canonical model's `Client` field set against +`domain.Client`'s YAML tags by reflection and fails on divergence in either +direction: a Go field with no spec entry, or a spec field the runtime does not +read. Fields intentionally excluded must be named in one list the test reads, so +excluding a field is a deliberate, reviewable act rather than an omission. This +is the generation/conformance link the assessment asks for. + +`src/internal/domain/conformance_test.go` checks `Client`, `User`, `Group` and +`Membership` by reflection over YAML tags. It earned its place on the first run +by finding drift outside the assessment's list: `User.tenant` was read by the +runtime and undeclared in the spec. `yaml:"-"` fields are excluded by +construction, which is what keeps `clientSecret` and `tokenLifetime` out without +a hand-maintained exception list. + +## Reconcile discovery metadata with the profile surface + +```task +id: KEY-WP-0017-T03 +status: done +priority: medium +``` + +`claims_supported` omits `tenant`, `principal_type`, `assurance`, `scope` and +`nonce`, which the token endpoint emits on every token, and `scopes_supported` is +a fixed list that omits every configured resource scope. Advertise the core +profile claims and derive the scope list from the registered clients. Keep the +distinction the assessment asks for: an omitted core claim is a contract defect, +while optional discovery metadata is not, so document which is which rather than +treating every omission alike. + +`claims_supported` now lists the core profile claims (`tenant`, `principal_type`, +`assurance`, `scope`, `nonce`) alongside the scope-gated human claims and the +optional cached `tenant_roles`. `scopes_supported` is derived from the registered +clients — baseline OIDC scopes first, configured resource scopes sorted after, so +the document is stable regardless of map iteration order — with the baseline +still advertised when no clients are configured. +`TestDiscoveryAdvertisesEveryCoreProfileClaim` states the distinction the +assessment asked for: a missing core claim fails, optional metadata does not. + +## State a single source of truth + +```task +id: KEY-WP-0017-T04 +status: done +priority: medium +``` + +Two files currently claim to be authoritative. State in both that the Go model is +the runtime authority and the canonical model is the reviewed contract the +conformance check holds it to, so the next reader knows which one to edit first. +Update `SCOPE.md` and G02's status in the assessment to say what is now enforced +and what remains — this closes the client-registration and discovery drift, not +schema enforcement in general (G06 is a separate gap). + +Both files now state the split: the Go model is the runtime authority, the YAML +the reviewed contract, and the conformance test the link. SCOPE.md and G02's +status say what is enforced and that G06 remains open.