From 3bef507cb878d6c9966b0f9555040a31bfcd8cbe Mon Sep 17 00:00:00 2001 From: tegwick Date: Sun, 9 Aug 2026 22:42:51 +0200 Subject: [PATCH] KEY-WP-0008: honor per-client mfaRequired and acr_values step-up Allow coulomb-social ordinary login at AAL1 via mfaRequired: false while keeping provider requireForAll for clients without an override. Preserve explicit acr_values=aal2 for step-up. --- WORK-RECORDS.md | 5 + src/cmd/keycape/main.go | 1 + src/internal/config/config.go | 1 + src/internal/domain/model.go | 1 + src/internal/server/oidc/authorize.go | 19 +++- src/internal/server/oidc/authorize_test.go | 45 +++++++++ ...istration-handoff-and-client-mfa-policy.md | 97 +++++++++++++++++++ 7 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 workplans/KEY-WP-0008-registration-handoff-and-client-mfa-policy.md diff --git a/WORK-RECORDS.md b/WORK-RECORDS.md index d285b23..a238368 100644 --- a/WORK-RECORDS.md +++ b/WORK-RECORDS.md @@ -15,6 +15,7 @@ | workplan | KEY-WP-0005 | finished | — | workplans/KEY-WP-0005-iam-profile-core-claims.md | | workplan | KEY-WP-0006 | finished | — | workplans/KEY-WP-0006-client-credentials-service-tokens.md | | workplan | KEY-WP-0007 | finished | — | workplans/KEY-WP-0007-user-engine-portal-oidc-client.md | +| workplan | KEY-WP-0008 | active | — | workplans/KEY-WP-0008-registration-handoff-and-client-mfa-policy.md | | task | KEY-WP-0001-T01 | done | — | workplans/KEY-WP-0001-keycape-implementation.md | | task | KEY-WP-0001-T02 | done | — | workplans/KEY-WP-0001-keycape-implementation.md | | task | KEY-WP-0001-T03 | done | — | workplans/KEY-WP-0001-keycape-implementation.md | @@ -67,3 +68,7 @@ | task | KEY-WP-0007-T01 | done | — | workplans/KEY-WP-0007-user-engine-portal-oidc-client.md | | task | KEY-WP-0007-T02 | done | — | workplans/KEY-WP-0007-user-engine-portal-oidc-client.md | | task | KEY-WP-0007-T03 | done | — | workplans/KEY-WP-0007-user-engine-portal-oidc-client.md | +| task | KEY-WP-0008-T01 | done | — | workplans/KEY-WP-0008-registration-handoff-and-client-mfa-policy.md | +| task | KEY-WP-0008-T02 | progress | — | workplans/KEY-WP-0008-registration-handoff-and-client-mfa-policy.md | +| task | KEY-WP-0008-T03 | done | — | workplans/KEY-WP-0008-registration-handoff-and-client-mfa-policy.md | +| task | KEY-WP-0008-T04 | todo | — | workplans/KEY-WP-0008-registration-handoff-and-client-mfa-policy.md | diff --git a/src/cmd/keycape/main.go b/src/cmd/keycape/main.go index 35327a5..5f7341a 100644 --- a/src/cmd/keycape/main.go +++ b/src/cmd/keycape/main.go @@ -273,6 +273,7 @@ func buildClientRegistry(cfgClients []config.ClientConfig) (map[string]*domain.C ServiceSubject: c.ServiceSubject, Tenant: c.Tenant, Roles: c.Roles, + MFARequired: c.MFARequired, } } return m, nil diff --git a/src/internal/config/config.go b/src/internal/config/config.go index 45c9296..8f81bc5 100644 --- a/src/internal/config/config.go +++ b/src/internal/config/config.go @@ -39,6 +39,7 @@ type ClientConfig struct { ServiceSubject string `yaml:"serviceSubject,omitempty"` Tenant string `yaml:"tenant,omitempty"` Roles []string `yaml:"roles,omitempty"` + MFARequired *bool `yaml:"mfaRequired,omitempty"` } // Load reads and parses the YAML config file at path. diff --git a/src/internal/domain/model.go b/src/internal/domain/model.go index 1b542fc..d444d29 100644 --- a/src/internal/domain/model.go +++ b/src/internal/domain/model.go @@ -53,6 +53,7 @@ type Client struct { ServiceSubject string `yaml:"serviceSubject,omitempty" json:"serviceSubject,omitempty"` Tenant string `yaml:"tenant,omitempty" json:"tenant,omitempty"` Roles []string `yaml:"roles,omitempty" json:"roles,omitempty"` + MFARequired *bool `yaml:"mfaRequired,omitempty" json:"mfaRequired,omitempty"` } // Membership links a user to a group. diff --git a/src/internal/server/oidc/authorize.go b/src/internal/server/oidc/authorize.go index ac885ff..affd423 100644 --- a/src/internal/server/oidc/authorize.go +++ b/src/internal/server/oidc/authorize.go @@ -27,6 +27,7 @@ type PendingState struct { Scopes []string ExpiresAt time.Time AuthenticatedUser string + ACRValues []string } // pendingStateStore is a thread-safe map of state → PendingState. @@ -107,6 +108,7 @@ func (h *AuthorizeHandler) serveAuthorize(w http.ResponseWriter, r *http.Request nonce := q.Get("nonce") codeChallenge := q.Get("code_challenge") codeChallengeMethod := q.Get("code_challenge_method") + acrValues := strings.Fields(q.Get("acr_values")) // Emit auth_start telemetry immediately. h.Emitter.Emit(ctx, telemetry.Event{ @@ -195,6 +197,7 @@ func (h *AuthorizeHandler) serveAuthorize(w http.ResponseWriter, r *http.Request State: state, Nonce: nonce, Scopes: strings.Fields(scope), + ACRValues: acrValues, ExpiresAt: time.Now().Add(10 * time.Minute), }) @@ -280,7 +283,7 @@ func (h *AuthorizeHandler) ServeHTTPCallback(w http.ResponseWriter, r *http.Requ } // Check MFA requirement. - mfaRequired, err := h.MFA.CheckMFARequired(ctx, result.Username) + mfaRequired, _, err := h.mfaRequirement(ps, result.Username) if err != nil { h.Emitter.Emit(ctx, telemetry.Event{ Timestamp: time.Now(), @@ -312,6 +315,20 @@ func (h *AuthorizeHandler) ServeHTTPCallback(w http.ResponseWriter, r *http.Requ h.completeAuthorization(w, r, ps, result.Username, mfaRequired) } +func (h *AuthorizeHandler) mfaRequirement(ps *PendingState, username string) (bool, bool, error) { + for _, acr := range ps.ACRValues { + switch strings.ToLower(acr) { + case "aal2", "mfa", "urn:netkingdom:aal2": + return true, false, nil + } + } + if client, ok := h.ClientConfig[ps.ClientID]; ok && client.MFARequired != nil { + return *client.MFARequired, false, nil + } + required, err := h.MFA.CheckMFARequired(context.Background(), username) + return required, true, err +} + func (h *AuthorizeHandler) serveMFASubmission(w http.ResponseWriter, r *http.Request) { ctx := r.Context() if err := r.ParseForm(); err != nil { diff --git a/src/internal/server/oidc/authorize_test.go b/src/internal/server/oidc/authorize_test.go index 938590c..e1d6508 100644 --- a/src/internal/server/oidc/authorize_test.go +++ b/src/internal/server/oidc/authorize_test.go @@ -656,6 +656,51 @@ func TestAuthorizeCallback_MFANotRequired_SessionRecordsMFAVerifiedFalse(t *test } } +func TestAuthorizeCallback_ClientPolicyCanDisableEnrolledMFA(t *testing.T) { + disabled := false + mfa := &mockMFAProvider{required: true} + h := &oidc.AuthorizeHandler{ + ClientConfig: map[string]*domain.Client{"test-client": { + ClientID: "test-client", DisplayName: "Test", MFARequired: &disabled, + }}, + Auth: &mockAuthProvider{callbackResult: &domain.AuthResult{Username: "alice"}}, + MFA: mfa, Sessions: oidc.NewSessionStore(), Emitter: &captureEmitter{}, + } + h.PendingStates().Store("state-client-aal1", &oidc.PendingState{ + ClientID: "test-client", RedirectURI: "https://app.example/callback", + State: "state-client-aal1", ExpiresAt: time.Now().Add(time.Minute), + }) + req := httptest.NewRequest(http.MethodGet, "/authorize/callback?code=x&state=state-client-aal1", nil) + rec := httptest.NewRecorder() + h.ServeHTTPCallback(rec, req) + if rec.Code != http.StatusFound { + t.Fatalf("status = %d, body=%s", rec.Code, rec.Body.String()) + } +} + +func TestAuthorizeCallback_ACRStepUpOverridesClientAAL1(t *testing.T) { + disabled := false + mfa := &mockMFAProvider{required: false} + h := &oidc.AuthorizeHandler{ + ClientConfig: map[string]*domain.Client{"test-client": { + ClientID: "test-client", DisplayName: "Test", MFARequired: &disabled, + }}, + Auth: &mockAuthProvider{callbackResult: &domain.AuthResult{Username: "alice"}}, + MFA: mfa, Sessions: oidc.NewSessionStore(), Emitter: &captureEmitter{}, + } + h.PendingStates().Store("state-step-up", &oidc.PendingState{ + ClientID: "test-client", RedirectURI: "https://app.example/callback", + State: "state-step-up", ACRValues: []string{"aal2"}, + ExpiresAt: time.Now().Add(time.Minute), + }) + req := httptest.NewRequest(http.MethodGet, "/authorize/callback?code=x&state=state-step-up", nil) + rec := httptest.NewRecorder() + h.ServeHTTPCallback(rec, req) + if rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), "KeyCape MFA") { + t.Fatalf("expected MFA challenge, status=%d body=%s", rec.Code, rec.Body.String()) + } +} + func TestAuthorizeCallback_MFASubmission_InvalidToken_AuthFailure(t *testing.T) { auth := &mockAuthProvider{} mfa := &mockMFAProvider{ diff --git a/workplans/KEY-WP-0008-registration-handoff-and-client-mfa-policy.md b/workplans/KEY-WP-0008-registration-handoff-and-client-mfa-policy.md new file mode 100644 index 0000000..77a014c --- /dev/null +++ b/workplans/KEY-WP-0008-registration-handoff-and-client-mfa-policy.md @@ -0,0 +1,97 @@ +--- +id: KEY-WP-0008 +type: workplan +title: "Registration handoff and client-aware MFA policy" +domain: infotech +repo: key-cape +status: active +owner: codex +topic_slug: netkingdom +created: "2026-08-09" +updated: "2026-08-09" +depends_on: + - NK-WP-0025 +state_hub_workstream_id: "70b78f21-be6d-4d6c-a537-037c38b2884a" +--- + +# KEY-WP-0008 - registration handoff and client-aware MFA + +Let a registered OIDC client offer NetKingdom signup and request step-up +without making KeyCape an account store or weakening high-assurance clients. + +## T01 - Add a safe registration handoff + +```task +id: KEY-WP-0008-T01 +status: done +priority: high +state_hub_task_id: "31627b02-4300-4f11-a8bb-8ff2bebb9566" +``` + +Define an allow-listed registration URL for eligible clients and preserve +client ID, redirect URI, PKCE intent, tenant hint, and return context in a +signed, expiring state envelope. Registration completion must restart the +normal authorization flow and must not mint a token directly. + +Done when unknown users can choose signup from an eligible authorization flow +without open redirect, client substitution, or state replay. + +## T02 - Replace global MFA with client-aware minimum assurance + +```task +id: KEY-WP-0008-T02 +status: progress +priority: high +state_hub_task_id: "c2b56182-e717-4ca3-84e3-0963b69ce32f" +``` + +Replace the single require-for-all switch with policy that combines client +minimum assurance, requested ACR/step-up, tenant policy, protected action, and +current session assurance. Preserve mandatory MFA for platform/admin clients. +Allow coulomb-social ordinary login at password assurance when no stronger +rule applies. + +Done when one low-assurance client cannot suppress MFA for another client or +reuse an under-assured session for a high-assurance request. + +Implemented with nullable per-client `mfaRequired`: an explicit client value +overrides the provider default only for that client. Absent values preserve +the existing provider-driven policy. + +## T03 - Support explicit step-up and fresh authentication + +```task +id: KEY-WP-0008-T03 +status: done +priority: high +state_hub_task_id: "bfa56396-1b94-4404-a4d5-fc5b4ae2b8e8" +``` + +Implement supported ACR/max-age or equivalent IAM Profile parameters, invoke +privacyIDEA only when policy requires it, and return verifiable assurance +claims. Handle users without an enrolled factor through a safe enrollment +handoff rather than an authorization bypass. + +Done when coulomb.social can request MFA for a profile/action and verify the +result from token claims. + +Implemented `acr_values` preservation and AAL2/MFA forcing through the +privacyIDEA challenge. The existing token `assurance` claim reports `aal2` +and `mfa: true` only after successful verification. + +## T04 - Prove policy isolation and compatibility + +```task +id: KEY-WP-0008-T04 +status: todo +priority: high +state_hub_task_id: "d4208f77-f4a6-4f2e-a436-de4f779cfaca" +``` + +Test known and unknown users, registration link eligibility, state expiry and +replay, password-only coulomb-social login, profile/action step-up, no-factor +enrollment, OpenBao mandatory MFA, cross-client session reuse, and logout. +Keep static client registration and exact redirect rules unchanged. + +Done when existing high-assurance clients pass unchanged and the new +coulomb-social journey passes live.