diff --git a/src/internal/domain/model.go b/src/internal/domain/model.go index 6db7e2a..7c193be 100644 --- a/src/internal/domain/model.go +++ b/src/internal/domain/model.go @@ -7,13 +7,20 @@ import "time" // User is the canonical identity entity — source of truth for all user data. type User struct { - ID string `yaml:"id" json:"id"` - Username string `yaml:"username" json:"username"` - DisplayName string `yaml:"displayName" json:"displayName"` - Email string `yaml:"email" json:"email"` - Enabled bool `yaml:"enabled" json:"enabled"` - Groups []string `yaml:"groups" json:"groups"` - Roles []string `yaml:"roles" json:"roles"` + ID string `yaml:"id" json:"id"` + Username string `yaml:"username" json:"username"` + DisplayName string `yaml:"displayName" json:"displayName"` + Email string `yaml:"email" json:"email"` + Enabled bool `yaml:"enabled" json:"enabled"` + Groups []string `yaml:"groups" json:"groups"` + Roles []string `yaml:"roles" json:"roles"` + // Tenant is the NetKingdom IAM Profile tenant claim value + // (e.g. "tenant:friendly:binky"), per net-kingdom/canon/standards/ + // iam-profile_v0.3.md. Empty means "not yet assigned" -- token + // issuance falls back to the platform default (KEY-WP-0005-T01) rather + // than emitting an empty tenant claim, since the profile requires + // tenant on every token. + Tenant string `yaml:"tenant,omitempty" json:"tenant,omitempty"` MFAEnrollment *MFAEnrollment `yaml:"mfaEnrollment,omitempty" json:"mfaEnrollment,omitempty"` LDAPAttributes map[string]string `yaml:"ldapAttributes,omitempty" json:"ldapAttributes,omitempty"` } diff --git a/src/internal/server/oidc/authorize.go b/src/internal/server/oidc/authorize.go index 94ef3f2..ac885ff 100644 --- a/src/internal/server/oidc/authorize.go +++ b/src/internal/server/oidc/authorize.go @@ -309,7 +309,7 @@ func (h *AuthorizeHandler) ServeHTTPCallback(w http.ResponseWriter, r *http.Requ } h.pending.Delete(state) - h.completeAuthorization(w, r, ps, result.Username) + h.completeAuthorization(w, r, ps, result.Username, mfaRequired) } func (h *AuthorizeHandler) serveMFASubmission(w http.ResponseWriter, r *http.Request) { @@ -350,10 +350,13 @@ func (h *AuthorizeHandler) serveMFASubmission(w http.ResponseWriter, r *http.Req } h.pending.Delete(state) - h.completeAuthorization(w, r, ps, ps.AuthenticatedUser) + // Reached only after ValidateMFAToken succeeded above -- MFA was + // required and passed, unlike the callback path where mfaRequired may + // be false. + h.completeAuthorization(w, r, ps, ps.AuthenticatedUser, true) } -func (h *AuthorizeHandler) completeAuthorization(w http.ResponseWriter, r *http.Request, ps *PendingState, username string) { +func (h *AuthorizeHandler) completeAuthorization(w http.ResponseWriter, r *http.Request, ps *PendingState, username string, mfaVerified bool) { // Generate authorization code and store PKCE session. sess := &PKCESession{ ClientID: ps.ClientID, @@ -365,6 +368,7 @@ func (h *AuthorizeHandler) completeAuthorization(w http.ResponseWriter, r *http. Username: username, Scopes: ps.Scopes, ExpiresAt: time.Now().Add(10 * time.Minute), + MFAVerified: mfaVerified, } authCode := h.Sessions.Create(sess) diff --git a/src/internal/server/oidc/authorize_test.go b/src/internal/server/oidc/authorize_test.go index f06ff0c..938590c 100644 --- a/src/internal/server/oidc/authorize_test.go +++ b/src/internal/server/oidc/authorize_test.go @@ -594,6 +594,66 @@ func TestAuthorizeCallback_MFASubmission_ValidToken_RedirectsWithCode(t *testing if _, ok := h.PendingStates().Load("random-state"); ok { t.Error("expected pending MFA state to be deleted after successful submission") } + + // KEY-WP-0005-T01: the resulting session must record that MFA was + // actually verified in this flow, feeding token.go's assurance claim + // (aal2, not aal1). + code := parsed.Query().Get("code") + sess, ok := sessions.Get(code) + if !ok { + t.Fatal("expected a PKCE session for the issued code") + } + if !sess.MFAVerified { + t.Error("MFAVerified: want true after a successful MFA submission") + } +} + +func TestAuthorizeCallback_MFANotRequired_SessionRecordsMFAVerifiedFalse(t *testing.T) { + auth := &mockAuthProvider{ + callbackResult: &domain.AuthResult{Username: "alice"}, + } + mfa := &mockMFAProvider{required: false} + emitter := &captureEmitter{} + + sessions := oidc.NewSessionStore() + h := &oidc.AuthorizeHandler{ + ClientConfig: testClient(), + Auth: auth, + MFA: mfa, + Sessions: sessions, + Emitter: emitter, + } + + h.PendingStates().Store("random-state", &oidc.PendingState{ + ClientID: "test-client", + RedirectURI: "https://app.example.com/callback", + PKCEChallenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + PKCEChallengeMethod: "S256", + State: "random-state", + Scopes: []string{"openid"}, + ExpiresAt: time.Now().Add(5 * time.Minute), + }) + + req := httptest.NewRequest(http.MethodGet, + "/authorize/callback?code=authelia-code&state=random-state", nil) + w := httptest.NewRecorder() + h.ServeHTTPCallback(w, req) + + if w.Code != http.StatusFound { + t.Fatalf("expected 302 redirect, got %d (body: %s)", w.Code, w.Body.String()) + } + loc, err := url.Parse(w.Header().Get("Location")) + if err != nil { + t.Fatalf("invalid Location header: %v", err) + } + code := loc.Query().Get("code") + sess, ok := sessions.Get(code) + if !ok { + t.Fatal("expected a PKCE session for the issued code") + } + if sess.MFAVerified { + t.Error("MFAVerified: want false when MFA was never required for this flow") + } } func TestAuthorizeCallback_MFASubmission_InvalidToken_AuthFailure(t *testing.T) { diff --git a/src/internal/server/oidc/session.go b/src/internal/server/oidc/session.go index 72405b0..6bd2474 100644 --- a/src/internal/server/oidc/session.go +++ b/src/internal/server/oidc/session.go @@ -12,13 +12,20 @@ type PKCESession struct { Code string ClientID string RedirectURI string - PKCEChallenge string // S256 challenge - PKCEChallengeMethod string // always "S256" + PKCEChallenge string // S256 challenge + PKCEChallengeMethod string // always "S256" State string Nonce string - Username string // set after auth + Username string // set after auth Scopes []string ExpiresAt time.Time + // MFAVerified records whether this authorization actually required and + // passed MFA validation (vs. MFA not being required for this user at + // all). Feeds the assurance claim's level in token.go + // (KEY-WP-0005-T01): aal2 when true, aal1 when false. Set once, at + // completeAuthorization -- never re-derived from stale enrollment state + // at token-exchange time. + MFAVerified bool } // SessionStore is an in-memory PKCE session store. diff --git a/src/internal/server/oidc/token.go b/src/internal/server/oidc/token.go index 35f1d4b..a830612 100644 --- a/src/internal/server/oidc/token.go +++ b/src/internal/server/oidc/token.go @@ -21,7 +21,7 @@ type TokenHandler struct { ClientConfig map[string]*domain.Client Sessions *SessionStore Users domain.UserRepository - SigningKey *rsa.PrivateKey + SigningKey *rsa.PrivateKey Issuer string TokenLifetime time.Duration Emitter telemetry.Emitter @@ -126,9 +126,15 @@ func (h *TokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if scopeSet["email"] { claims["email"] = user.Email } - if scopeSet["groups"] { - claims["groups"] = user.Groups - } + + // 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). + claims["tenant"] = effectiveTenant(user) + claims["principal_type"] = "human" + claims["groups"] = nonNilStrings(user.Groups) + claims["roles"] = nonNilStrings(user.Roles) + claims["assurance"] = assuranceClaim(sess.MFAVerified, now) // 7. Sign JWT with RSA-SHA256. kid := "key-1" // static kid for v0.1 @@ -165,6 +171,58 @@ func (h *TokenHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { _ = json.NewEncoder(w).Encode(resp) } +// --------------------------------------------------------------------------- +// IAM Profile core claims (KEY-WP-0005-T01) +// --------------------------------------------------------------------------- + +// defaultTenant is the fallback tenant claim for users with no explicit +// Tenant assignment yet. This workstation currently operates a single +// tenant (tenant:coulomb, ADR-0006); later tenants (e.g. tenant:friendly:binky, +// ADR-0013) require an explicit domain.User.Tenant value -- this default +// never silently assigns a user to a tenant other than the platform's +// original one. +const defaultTenant = "tenant:coulomb" + +// effectiveTenant resolves the tenant claim for a user, falling back to +// defaultTenant when the user has no explicit tenant assignment. The IAM +// Profile requires a non-empty tenant claim on every token. +func effectiveTenant(user *domain.User) string { + if user.Tenant != "" { + return user.Tenant + } + return defaultTenant +} + +// 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. +func nonNilStrings(s []string) []string { + if s == nil { + return []string{} + } + return s +} + +// assuranceClaim builds the profile's `assurance` object from whether MFA +// was actually verified during this authorization (session.MFAVerified), +// not from static enrollment state -- a user who has MFA enrolled but +// wasn't challenged for it in this particular flow gets aal1, not aal2. +func assuranceClaim(mfaVerified bool, at time.Time) map[string]interface{} { + level := "aal1" + methods := []string{"pwd"} + if mfaVerified { + level = "aal2" + methods = append(methods, "otp") + } + return map[string]interface{}{ + "level": level, + "methods": methods, + "mfa": mfaVerified, + "source": "key-cape", + "at": at.Unix(), + } +} + // --------------------------------------------------------------------------- // PKCE verification // --------------------------------------------------------------------------- diff --git a/src/internal/server/oidc/token_test.go b/src/internal/server/oidc/token_test.go index 555a432..c6827f3 100644 --- a/src/internal/server/oidc/token_test.go +++ b/src/internal/server/oidc/token_test.go @@ -81,13 +81,13 @@ func newTokenHandler(t *testing.T, sessions *oidc.SessionStore, users domain.Use } emitter := &captureEmitter{} h := &oidc.TokenHandler{ - ClientConfig: testClient(), - Sessions: sessions, - Users: users, + ClientConfig: testClient(), + Sessions: sessions, + Users: users, SigningKey: key, - Issuer: "https://auth.netkingdom.local", + Issuer: "https://auth.netkingdom.local", TokenLifetime: 15 * time.Minute, - Emitter: emitter, + Emitter: emitter, } return h, key } @@ -377,9 +377,10 @@ func TestTokenHandler_ScopeFiltering_ProfileScope(t *testing.T) { if _, ok := claims["email"]; ok { t.Error("email must be absent when email scope is not granted") } - // Without groups scope, groups must not be present. - if _, ok := claims["groups"]; ok { - t.Error("groups must be absent when groups scope is not granted") + // groups is a required IAM Profile core claim (iam-profile_v0.3.md) -- + // present regardless of scope, unlike preferred_username/email above. + if _, ok := claims["groups"]; !ok { + t.Error("groups must be present as a core claim even without a groups scope (KEY-WP-0005-T01)") } } @@ -436,7 +437,7 @@ func TestTokenHandler_TokenIssuedTelemetry(t *testing.T) { ClientConfig: testClient(), Sessions: sessions, Users: users, - SigningKey: key, + SigningKey: key, Issuer: "https://auth.netkingdom.local", TokenLifetime: 15 * time.Minute, Emitter: capture, diff --git a/src/tests/profile/profile_test.go b/src/tests/profile/profile_test.go index f49a645..306ba74 100644 --- a/src/tests/profile/profile_test.go +++ b/src/tests/profile/profile_test.go @@ -190,7 +190,7 @@ func newTestServer(t *testing.T) *TestServer { ClientConfig: clients, Sessions: sessions, Users: usersMock, - SigningKey: privateKey, + SigningKey: privateKey, Issuer: issuer, TokenLifetime: 15 * time.Minute, Emitter: emitter, @@ -199,10 +199,10 @@ func newTestServer(t *testing.T) *TestServer { // Userinfo handler. mux.Handle("/userinfo", &oidc.UserinfoHandler{ - Users: usersMock, + Users: usersMock, SigningKey: &privateKey.PublicKey, - Issuer: issuer, - Emitter: emitter, + Issuer: issuer, + Emitter: emitter, }) // Healthz handler. @@ -622,7 +622,12 @@ func TestCompleteTokenFlow(t *testing.T) { t.Fatalf("parse JWT claims: %v", err) } - requiredClaims := []string{"iss", "sub", "aud", "exp", "iat"} + requiredClaims := []string{ + "iss", "sub", "aud", "exp", "iat", + // IAM Profile v0.3 core claims (KEY-WP-0005-T01) -- required on + // every production token, not scope-gated. + "tenant", "principal_type", "groups", "roles", "assurance", + } for _, c := range requiredClaims { if _, ok := claims[c]; !ok { t.Errorf("JWT missing claim %q", c) @@ -632,4 +637,37 @@ func TestCompleteTokenFlow(t *testing.T) { if claims["aud"] != "demo-app" { t.Errorf("aud: want demo-app, got %v", claims["aud"]) } + + // testuser has no explicit Tenant assignment -- falls back to the + // platform default (this workstation's single tenant, ADR-0006). + if claims["tenant"] != "tenant:coulomb" { + t.Errorf("tenant: want tenant:coulomb, got %v", claims["tenant"]) + } + if claims["principal_type"] != "human" { + t.Errorf("principal_type: want human, got %v", claims["principal_type"]) + } + groups, ok := claims["groups"].([]interface{}) + if !ok || len(groups) != 1 || groups[0] != "developers" { + t.Errorf("groups: want [developers], got %v", claims["groups"]) + } + roles, ok := claims["roles"].([]interface{}) + if !ok || len(roles) != 0 { + t.Errorf("roles: want [] (testuser has none), got %v", claims["roles"]) + } + assurance, ok := claims["assurance"].(map[string]interface{}) + if !ok { + t.Fatalf("assurance: want an object, got %v", claims["assurance"]) + } + // This flow's mockMFA has required: false -- MFA was never performed, + // so assurance must reflect aal1, not aal2, regardless of the user's + // static MFA enrollment state. + if assurance["level"] != "aal1" { + t.Errorf("assurance.level: want aal1 (MFA not required in this flow), got %v", assurance["level"]) + } + if assurance["mfa"] != false { + t.Errorf("assurance.mfa: want false, got %v", assurance["mfa"]) + } + if assurance["source"] != "key-cape" { + t.Errorf("assurance.source: want key-cape, got %v", assurance["source"]) + } } diff --git a/workplans/KEY-WP-0005-iam-profile-core-claims.md b/workplans/KEY-WP-0005-iam-profile-core-claims.md index 696a385..1798298 100644 --- a/workplans/KEY-WP-0005-iam-profile-core-claims.md +++ b/workplans/KEY-WP-0005-iam-profile-core-claims.md @@ -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