From a0f3cac12217cce148bbb30a77d307dc82de09a1 Mon Sep 17 00:00:00 2001 From: tegwick Date: Tue, 8 Sep 2026 08:59:40 +0200 Subject: [PATCH] Wire tenant_roles as explicit opt-in configuration Closes gap G07. The tenant-engine client and TokenHandler.TenantEngine were implemented and tested, but main.go supplied no client and exposed no configuration, so the stock executable left the field nil and the capability existed at library level only. A tenantEngine block with baseURL and an optional timeout now wires it. An empty baseURL leaves the stock server's behaviour exactly as it was, so enabling the claim is a deliberate act. Validation treats a configured source as one that must work: http/https with a host, and a timeout in (0, 10s] since it sits on the synchronous token-issuance path. A timeout set without a baseURL is rejected rather than ignored -- it means someone expected the claim to be on. Verified in the built executable, which is what G07 asks for, rather than at the wiring: against a stub source a real token carries tenant_roles; with no block the claim is absent; with the source configured but down, issuance succeeds without it, confirming the documented fail-open path end to end. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NV9oijZukGyGbRQGGKnK4P Assistant: claude-code Assistant-Model: opus Assistant-Process: 713576@bnt-lap001 Assistant-Session: 384c511d-9bce-4cb8-a676-2aef6c0c8df6 --- SCOPE.md | 8 ++- ...26-09-05-011726-scope-intent-assessment.md | 8 +++ src/cmd/keycape/main.go | 21 +++++++ src/internal/config/config.go | 16 +++++ src/internal/config/config_test.go | 33 +++++++++++ src/internal/config/validate.go | 21 +++++++ .../KEY-WP-0024-tenant-roles-opt-in-wiring.md | 59 +++++++++++++++++++ 7 files changed, 163 insertions(+), 3 deletions(-) create mode 100644 workplans/KEY-WP-0024-tenant-roles-opt-in-wiring.md diff --git a/SCOPE.md b/SCOPE.md index 279f3e7..de3a831 100644 --- a/SCOPE.md +++ b/SCOPE.md @@ -35,9 +35,11 @@ Keycloak interchangeability are not established. session store, general refresh-token flow, token introspection/revocation API, or automatic signing-key/client-secret rotation service. Logout clears the local KeyCape session, not every upstream or downstream session/token. -- The optional tenant-engine `tenant_roles` adapter and handler support exist, - but the server entry point does not configure them. That claim is not an - enabled capability of the stock executable. +- The optional tenant-engine `tenant_roles` adapter is wired through a + `tenantEngine` config block and is off unless `baseURL` is set (KEY-WP-0024). + When enabled it fails open: an unreachable source omits the claim rather than + failing issuance, so `tenant_roles` is a cache and must not be trusted for + privileged decisions. - The LLDAP export enumerates the group subtree directly, so groups with no members are present, and every snapshot carries a `groupEnumeration` field saying whether that enumeration ran or the membership-derived fallback did diff --git a/history/2026-09-05-011726-scope-intent-assessment.md b/history/2026-09-05-011726-scope-intent-assessment.md index ef75bd3..7ac1e37 100644 --- a/history/2026-09-05-011726-scope-intent-assessment.md +++ b/history/2026-09-05-011726-scope-intent-assessment.md @@ -355,6 +355,14 @@ no configuration for it. The stock server therefore leaves it nil and omits executable, or document this as library-only support. It is an optional cache claim, so absence is not itself a resource authorization failure. +**Status 2026-09-08 (KEY-WP-0024): closed.** A `tenantEngine` block with +`baseURL` and optional `timeout` now wires the client; an empty `baseURL` leaves +the stock server's behaviour unchanged. Verified in the built executable rather +than at the wiring: with a stub source a real token carries `tenant_roles`, with +no block the claim is absent, and with the source down issuance succeeds without +it — the documented fail-open path, confirmed end to end. Validation rejects an +unusable URL, an out-of-range timeout, and a timeout set without a base URL. + ### G08 — Runtime lifecycle and readiness are intentionally minimal **Priority: medium. Kind: operational maturity gap.** diff --git a/src/cmd/keycape/main.go b/src/cmd/keycape/main.go index cce0df7..c79d710 100644 --- a/src/cmd/keycape/main.go +++ b/src/cmd/keycape/main.go @@ -21,6 +21,7 @@ import ( "keycape/internal/adapters/authelia" "keycape/internal/adapters/lldap" "keycape/internal/adapters/privacyidea" + "keycape/internal/adapters/tenantengine" "keycape/internal/authclient" "keycape/internal/config" "keycape/internal/domain" @@ -178,6 +179,11 @@ func main() { Issuer: issuer, TokenLifetime: tokenLifetime, Emitter: emitter, + // Opt-in: nil unless tenantEngine.baseURL is configured, which is what + // leaves tenant_roles off the stock server (KEY-WP-0024). The adapter + // fails open, so a configured-but-unreachable cache omits the claim + // rather than failing issuance. + TenantEngine: buildTenantEngine(cfg.TenantEngine), } mux.Handle("/token", enforcement.Middleware(tokenHandler)) @@ -320,3 +326,18 @@ func withEmitter(next http.Handler, e telemetry.Emitter) http.Handler { next.ServeHTTP(w, r.WithContext(ctx)) }) } + +// buildTenantEngine returns the tenant_roles cache client, or nil when the +// claim is not configured. Validation has already accepted the URL and timeout. +func buildTenantEngine(cfg config.TenantEngineConfig) *tenantengine.Client { + if cfg.BaseURL == "" { + return nil + } + var httpClient *http.Client + if cfg.Timeout != "" { + if timeout, err := time.ParseDuration(cfg.Timeout); err == nil { + httpClient = &http.Client{Timeout: timeout} + } + } + return tenantengine.New(cfg.BaseURL, httpClient) +} diff --git a/src/internal/config/config.go b/src/internal/config/config.go index dc1aa7b..6bb6827 100644 --- a/src/internal/config/config.go +++ b/src/internal/config/config.go @@ -27,6 +27,22 @@ type Config struct { PrivacyIDEA privacyidea.Config `yaml:"privacyidea"` Clients []ClientConfig `yaml:"clients"` Environment string `yaml:"environment"` + TenantEngine TenantEngineConfig `yaml:"tenantEngine,omitempty"` +} + +// TenantEngineConfig configures the optional tenant_roles cache claim. +// +// Opt-in by design: an empty baseURL leaves the claim off entirely, which is +// what the stock server does. tenant_roles is a cache callers must not trust for +// privileged decisions, and the adapter fails open, so enabling it is a +// performance choice rather than a security one (KEY-WP-0024). +type TenantEngineConfig struct { + // BaseURL is tenant-engine's cache-read endpoint. Empty disables the claim. + BaseURL string `yaml:"baseURL,omitempty"` + + // Timeout bounds the lookup, which sits on the synchronous token-issuance + // path. Empty uses the adapter's own short default. + Timeout string `yaml:"timeout,omitempty"` } // ClientConfig is a static OIDC client registration. diff --git a/src/internal/config/config_test.go b/src/internal/config/config_test.go index 2d94f11..2d0a8ee 100644 --- a/src/internal/config/config_test.go +++ b/src/internal/config/config_test.go @@ -496,3 +496,36 @@ func TestValidateConfigAudience(t *testing.T) { } } } + +// tenant_roles is opt-in, but a configured cache source must be usable: it sits +// on the synchronous token-issuance path (KEY-WP-0024). +func TestValidate_TenantEngine(t *testing.T) { + cases := map[string]struct { + engine config.TenantEngineConfig + valid bool + }{ + "absent is valid": {config.TenantEngineConfig{}, true}, + "http url": {config.TenantEngineConfig{BaseURL: "http://tenant-engine:8080"}, true}, + "https url and timeout": {config.TenantEngineConfig{BaseURL: "https://tenant-engine", Timeout: "2s"}, true}, + "not a url": {config.TenantEngineConfig{BaseURL: "tenant-engine"}, false}, + "wrong scheme": {config.TenantEngineConfig{BaseURL: "ldap://tenant-engine"}, false}, + "unparseable timeout": {config.TenantEngineConfig{BaseURL: "http://t", Timeout: "soon"}, false}, + "zero timeout": {config.TenantEngineConfig{BaseURL: "http://t", Timeout: "0s"}, false}, + "timeout far too long": {config.TenantEngineConfig{BaseURL: "http://t", Timeout: "5m"}, false}, + // A timeout with no base URL means someone expected the claim to be on. + "timeout without url": {config.TenantEngineConfig{Timeout: "2s"}, false}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + cfg := validConfig(writeTempFile(t, "key")) + cfg.TenantEngine = tc.engine + errs := config.ValidateConfig(cfg) + if tc.valid && len(errs) != 0 { + t.Fatalf("expected valid, got %v", errs) + } + if !tc.valid && len(errs) == 0 { + t.Fatal("expected a validation error") + } + }) + } +} diff --git a/src/internal/config/validate.go b/src/internal/config/validate.go index d09138e..e0a982a 100644 --- a/src/internal/config/validate.go +++ b/src/internal/config/validate.go @@ -30,6 +30,27 @@ func ValidateConfig(cfg *Config) []string { errs = append(errs, fmt.Sprintf("port: must be between 1 and 65535, got %d", cfg.Port)) } + // tenant_roles is opt-in: an empty baseURL disables it. A configured one + // must be usable, since a misconfigured cache source on the token path is + // worse than no cache source (KEY-WP-0024). + if cfg.TenantEngine.BaseURL != "" { + u, err := url.Parse(cfg.TenantEngine.BaseURL) + if err != nil || u.Scheme == "" || u.Host == "" { + errs = append(errs, fmt.Sprintf("tenantEngine.baseURL: %q is not a valid URL (must include scheme and host)", cfg.TenantEngine.BaseURL)) + } else if u.Scheme != "http" && u.Scheme != "https" { + errs = append(errs, fmt.Sprintf("tenantEngine.baseURL: scheme must be http or https, got %q", u.Scheme)) + } + } + if cfg.TenantEngine.Timeout != "" { + if cfg.TenantEngine.BaseURL == "" { + errs = append(errs, "tenantEngine.timeout: set without a baseURL, so tenant_roles stays disabled") + } else if timeout, err := time.ParseDuration(cfg.TenantEngine.Timeout); err != nil { + errs = append(errs, "tenantEngine.timeout: must be a valid duration") + } else if timeout <= 0 || timeout > 10*time.Second { + errs = append(errs, "tenantEngine.timeout: must be greater than 0 and at most 10s; it sits on the token-issuance path") + } + } + // At least one client must be registered. if len(cfg.Clients) == 0 { errs = append(errs, "clients: at least one client must be defined") diff --git a/workplans/KEY-WP-0024-tenant-roles-opt-in-wiring.md b/workplans/KEY-WP-0024-tenant-roles-opt-in-wiring.md new file mode 100644 index 0000000..9d06810 --- /dev/null +++ b/workplans/KEY-WP-0024-tenant-roles-opt-in-wiring.md @@ -0,0 +1,59 @@ +--- +id: KEY-WP-0024 +type: workplan +title: "Wire tenant_roles as explicit opt-in configuration" +domain: infotech +repo: key-cape +status: finished +owner: claude +topic_slug: tenant-roles-opt-in-wiring +created: "2026-09-08" +updated: "2026-09-08" +--- + +Closes gap G07. The tenant-engine client and `TokenHandler.TenantEngine` were +implemented and tested, but `main.go` supplied no client and exposed no +configuration, so the stock executable left the field nil and never emitted +`tenant_roles`. The capability existed at library level only. + +Wired rather than documented away as library-only: the claim is cheap, the +adapter already fails open, and leaving a tested capability unreachable from the +binary invites the same gap being rediscovered later. + +## Add opt-in configuration and wire the client + +```task +id: KEY-WP-0024-T01 +status: done +priority: medium +``` + +Added a `tenantEngine` config block with `baseURL` and an optional `timeout`. +An empty `baseURL` disables the claim, which keeps the stock server's behaviour +exactly as it was — enabling it is a deliberate act. + +Validation treats a configured source as one that must work: the URL must be +http/https with a host, and the timeout must parse and fall in (0, 10s], since it +sits on the synchronous token-issuance path. A `timeout` set without a `baseURL` +is rejected rather than ignored — it means someone expected the claim to be on +and it silently would not have been. + +## Verify it in the built executable + +```task +id: KEY-WP-0024-T02 +status: done +priority: medium +``` + +G07's closure criterion is the built executable, not the wiring, so all three +behaviours were checked by running `bin/keycape` and inspecting real tokens +rather than by reading the code: + +- configured against a stub tenant-engine, a `client_credentials` token carries + `tenant_roles: [capability:approve, capability:observe]`; +- with no `tenantEngine` block, the claim is absent; +- configured but with the source down, issuance still succeeds and the claim is + absent — the fail-open behaviour the adapter documents, confirmed end to end. + +Nine validation cases cover the accepted and rejected configurations.