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.