diff --git a/SCOPE.md b/SCOPE.md index 093110a..ab2522b 100644 --- a/SCOPE.md +++ b/SCOPE.md @@ -171,6 +171,8 @@ or factor enrollment themselves. Run the issuer with `keycape --config PATH`; `keycape server` and `keycape migrate` are not implemented subcommands. Caller commands are `keycape login`, `keycape service-token` and `keycape verify-client`. +`keycape check-config` validates a rendered configuration offline, before a +restart (see [operations](docs/operations.md)). Separate binaries under `src/cmd/` are `validator`, `lldap-export`, `keycape-to-keycloak`, and `lldap-to-ldap`. `make -C src build` places binaries in diff --git a/docs/operations.md b/docs/operations.md index 472f0c2..b920fa2 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -70,6 +70,18 @@ The signing key and all client registrations are read once at startup. Both are changed by editing configuration and restarting; there is no reload signal and no rotation service. +**Check a changed configuration before the restart.** KeyCape exits at startup on +a configuration it refuses, and with one replica that stops sign-in for every +client. `keycape check-config --config PATH [--env-names A,B] [--policy-store]` +runs every startup check that needs no secret value, signing key or upstream, and +prints a JSON report with no values in it. The checks cover validation (including +redirect URIs with `?` or `*`, and duplicate `clientId`s), building the client +registry, the global `tokenLifetime`, and the clients the policy store requires +when `--policy-store` is set. With `--env-names`, it also checks that every +`env:` secretRef is among the variables the Deployment provides. It exits non-zero +when the server would refuse to start. It does not check that the key file is +readable or that a secret is non-empty, because it never sees either. + **The key ID is the constant `key-1`.** Rotating the signing key while keeping that identifier is a trap: a consumer caching JWKS by `kid` may keep the old key and reject freshly issued tokens until its cache expires. Plan rotation as diff --git a/src/cmd/keycape/checkconfig.go b/src/cmd/keycape/checkconfig.go new file mode 100644 index 0000000..5edcfd7 --- /dev/null +++ b/src/cmd/keycape/checkconfig.go @@ -0,0 +1,96 @@ +package main + +import ( + "encoding/json" + "flag" + "fmt" + "io" + "sort" + "strings" + "time" + + "keycape/internal/config" + "keycape/internal/server/policy" +) + +// checkConfigReport is what keycape check-config prints. It carries messages +// and environment variable names only: the check never reads a secret value, +// the signing key or any upstream, so it can run outside the pod before a +// restart (net-kingdom keycape-client.py, oidc-browser-relying-party R4). +type checkConfigReport struct { + Valid bool `json:"valid"` + Errors []string `json:"errors"` + Clients int `json:"clients"` + RequiredEnv []string `json:"required_env"` +} + +// runCheckConfig applies every config check the server makes at startup that +// does not need a secret, a key file or the network, and exits non-zero if the +// server would refuse to start on this config. +// +// --env-names lists the variable names the target Deployment provides; each +// client secretRef must be among them, since an empty secret stops startup. +// --policy-store asserts the policy store is enabled (KEYCAPE_POLICY_PATH set), +// which requires the clients it governs. +func runCheckConfig(args []string, out io.Writer) int { + fs := flag.NewFlagSet("check-config", flag.ContinueOnError) + fs.SetOutput(io.Discard) + cfgPath := fs.String("config", "", "path to YAML config file") + envNames := fs.String("env-names", "", "comma-separated environment variable names the deployment provides") + policyStore := fs.Bool("policy-store", false, "the deployment enables the authentication policy store") + report := checkConfigReport{Errors: []string{}, RequiredEnv: []string{}} + if err := fs.Parse(args); err != nil || *cfgPath == "" { + report.Errors = append(report.Errors, "usage: keycape check-config --config PATH [--env-names A,B] [--policy-store]") + return writeReport(out, report) + } + cfg, err := config.Load(*cfgPath) + if err != nil { + report.Errors = append(report.Errors, "config does not load") + return writeReport(out, report) + } + report.Clients = len(cfg.Clients) + report.Errors = append(report.Errors, config.ValidateConfig(cfg)...) + if cfg.TokenLifetime != "" { + if _, err := time.ParseDuration(cfg.TokenLifetime); err != nil { + report.Errors = append(report.Errors, "tokenLifetime must be a valid duration") + } + } + + provided := map[string]bool{} + for _, name := range strings.Split(*envNames, ",") { + if name = strings.TrimSpace(name); name != "" { + provided[name] = true + } + } + for _, c := range cfg.Clients { + if name, ok := strings.CutPrefix(c.SecretRef, "env:"); ok { + report.RequiredEnv = append(report.RequiredEnv, name) + if *envNames != "" && !provided[name] { + report.Errors = append(report.Errors, fmt.Sprintf("client %q: secret environment variable %q is not provided", c.ClientID, name)) + } + } + } + sort.Strings(report.RequiredEnv) + + // A placeholder stands in for every secret: the values are never needed + // to decide whether the registry builds. + clients, err := buildClientRegistryWith(cfg.Clients, func(string) string { return "check-config" }) + if err != nil { + report.Errors = append(report.Errors, err.Error()) + } else if *policyStore && policy.CheckClients(clients) != nil { + report.Errors = append(report.Errors, "policy store: vergabe-demo-company and user-engine-portal must be authorization_code clients without mfaRequired false") + } + return writeReport(out, report) +} + +func writeReport(out io.Writer, report checkConfigReport) int { + report.Valid = len(report.Errors) == 0 + enc := json.NewEncoder(out) + enc.SetIndent("", " ") + _ = enc.Encode(report) + if report.Valid { + return 0 + } + return 1 +} + diff --git a/src/cmd/keycape/checkconfig_test.go b/src/cmd/keycape/checkconfig_test.go new file mode 100644 index 0000000..93e4c44 --- /dev/null +++ b/src/cmd/keycape/checkconfig_test.go @@ -0,0 +1,92 @@ +package main + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +const checkConfigBase = `issuer: https://kc.example +port: 8080 +privateKeyPem: /nonexistent/key.pem +clients: + - clientId: user-engine-portal + displayName: Portal + redirectUris: ["https://portal.example/cb"] + allowedScopes: ["openid"] + grantTypes: ["authorization_code"] + clientType: public + - clientId: vergabe-demo-company + displayName: Vergabe + redirectUris: ["https://vergabe.example/cb"] + allowedScopes: ["openid"] + grantTypes: ["authorization_code"] + clientType: confidential + secretRef: env:KEYCAPE_VERGABE_CLIENT_SECRET +` + +func checkConfig(t *testing.T, yaml string, args ...string) (int, checkConfigReport) { + t.Helper() + path := filepath.Join(t.TempDir(), "config.yaml") + if err := os.WriteFile(path, []byte(yaml), 0600); err != nil { + t.Fatal(err) + } + var out bytes.Buffer + code := runCheckConfig(append([]string{"--config", path}, args...), &out) + var report checkConfigReport + if err := json.Unmarshal(out.Bytes(), &report); err != nil { + t.Fatalf("report is not JSON: %v", err) + } + return code, report +} + +func TestCheckConfigAcceptsValidConfigWithoutSecretsOrKey(t *testing.T) { + code, report := checkConfig(t, checkConfigBase, "--env-names", "KEYCAPE_VERGABE_CLIENT_SECRET", "--policy-store") + if code != 0 || !report.Valid || report.Clients != 2 { + t.Fatalf("expected valid, got %d %+v", code, report) + } + if len(report.RequiredEnv) != 1 || report.RequiredEnv[0] != "KEYCAPE_VERGABE_CLIENT_SECRET" { + t.Fatalf("required env: %v", report.RequiredEnv) + } +} + +func TestCheckConfigRefusesWhatStopsStartup(t *testing.T) { + cases := map[string]struct { + yaml string + args []string + want string + }{ + "query in redirect": {strings.Replace(checkConfigBase, "portal.example/cb", "portal.example/cb?x=1", 1), nil, "wildcards"}, + "duplicate client": {checkConfigBase + ` - clientId: user-engine-portal + displayName: Again + redirectUris: ["https://other.example/cb"] + allowedScopes: ["openid"] + grantTypes: ["authorization_code"] + clientType: public +`, nil, "more than once"}, + "secret env not provided": {checkConfigBase, []string{"--env-names", "SOMETHING_ELSE"}, "not provided"}, + "policy store client missing": {strings.Replace(checkConfigBase, "clientId: user-engine-portal", "clientId: portal-renamed", 1), []string{"--policy-store"}, "policy store"}, + "explicit mfa false on policy client": {strings.Replace(checkConfigBase, " clientType: public\n", " clientType: public\n mfaRequired: false\n", 1), []string{"--policy-store"}, "policy store"}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + code, report := checkConfig(t, tc.yaml, tc.args...) + if code == 0 || report.Valid { + t.Fatalf("expected refusal, got valid") + } + if !strings.Contains(strings.Join(report.Errors, "\n"), tc.want) { + t.Fatalf("expected %q in %v", tc.want, report.Errors) + } + }) + } +} + +func TestCheckConfigWithoutEnvNamesOnlyListsThem(t *testing.T) { + code, report := checkConfig(t, checkConfigBase) + if code != 0 || len(report.RequiredEnv) != 1 { + t.Fatalf("expected valid with listed env, got %d %+v", code, report) + } +} diff --git a/src/cmd/keycape/main.go b/src/cmd/keycape/main.go index d0b102d..483a613 100644 --- a/src/cmd/keycape/main.go +++ b/src/cmd/keycape/main.go @@ -37,6 +37,10 @@ import ( const version = "0.1.0" func main() { + if len(os.Args) > 1 && os.Args[1] == "check-config" { + os.Exit(runCheckConfig(os.Args[2:], os.Stdout)) + } + if len(os.Args) > 1 && os.Args[1] == "probe-upstream-issuer" { if err := authelia.RunIssuerProbe(context.Background(), os.Args[2:], os.Stdout); err != nil { fmt.Fprintln(os.Stderr, err) @@ -387,6 +391,12 @@ func loadPrivateKey(path string) (*rsa.PrivateKey, error) { // buildClientRegistry converts []ClientConfig into the map used by handlers. func buildClientRegistry(cfgClients []config.ClientConfig) (map[string]*domain.Client, error) { + return buildClientRegistryWith(cfgClients, os.Getenv) +} + +// buildClientRegistryWith resolves env: secretRefs through getenv, so +// check-config can build the registry without any secret value. +func buildClientRegistryWith(cfgClients []config.ClientConfig, getenv func(string) string) (map[string]*domain.Client, error) { m := make(map[string]*domain.Client, len(cfgClients)) for i := range cfgClients { c := &cfgClients[i] @@ -394,7 +404,7 @@ func buildClientRegistry(cfgClients []config.ClientConfig) (map[string]*domain.C var clientTokenLifetime time.Duration if strings.HasPrefix(c.SecretRef, "env:") { envName := strings.TrimPrefix(c.SecretRef, "env:") - clientSecret = os.Getenv(envName) + clientSecret = getenv(envName) if clientSecret == "" { return nil, fmt.Errorf("client %q secret environment variable %q is empty", c.ClientID, envName) } diff --git a/src/internal/config/config_test.go b/src/internal/config/config_test.go index f1ab59b..d4236a6 100644 --- a/src/internal/config/config_test.go +++ b/src/internal/config/config_test.go @@ -616,3 +616,13 @@ func TestValidate_ServiceIdentityFieldsRejectedOnBrowserClients(t *testing.T) { } }) } + +func TestValidate_DuplicateClientID(t *testing.T) { + keyPath := writeTempFile(t, "key") + cfg := validConfig(keyPath) + cfg.Clients = append(cfg.Clients, cfg.Clients[0]) + errs := config.ValidateConfig(cfg) + if !containsErr(errs, "registered more than once") { + t.Errorf("expected duplicate clientId error, got %v", errs) + } +} diff --git a/src/internal/config/validate.go b/src/internal/config/validate.go index da7847c..c020e40 100644 --- a/src/internal/config/validate.go +++ b/src/internal/config/validate.go @@ -68,13 +68,20 @@ func ValidateConfig(cfg *Config) []string { errs = append(errs, "clients: at least one client must be defined") } + // The registry is keyed by clientId, so a duplicate would silently replace + // the earlier registration instead of failing. + seen := map[string]bool{} + // Each client must have at least one redirect URI and a non-empty clientId. for i, c := range cfg.Clients { prefix := fmt.Sprintf("clients[%d] (%s)", i, c.ClientID) if c.ClientID == "" { prefix = fmt.Sprintf("clients[%d]", i) errs = append(errs, prefix+": clientId must not be empty") + } else if seen[c.ClientID] { + errs = append(errs, prefix+": clientId is registered more than once") } + seen[c.ClientID] = true if strings.TrimSpace(c.Audience) != c.Audience || strings.ContainsAny(c.Audience, " \t\r\n") { errs = append(errs, prefix+": audience must be a single non-whitespace identifier") } diff --git a/src/internal/server/policy/policy.go b/src/internal/server/policy/policy.go index 5955ce6..29167da 100644 --- a/src/internal/server/policy/policy.go +++ b/src/internal/server/policy/policy.go @@ -55,9 +55,16 @@ type Store struct { now func() time.Time } -func Open(path string, clients map[string]*domain.Client) (*Store, error) { - s := &Store{path: path, names: map[string]string{}, pending: map[string]ticket{}, now: time.Now} - s.current.Modes = map[string]string{} +// CheckClients reports whether the registry carries the clients the policy +// store governs, in a shape it accepts. Open fails on exactly the same +// condition; keycape check-config uses this to catch it before a restart. +func CheckClients(clients map[string]*domain.Client) error { + _, err := clientModes(clients) + return err +} + +func clientModes(clients map[string]*domain.Client) (map[string]string, error) { + modes := map[string]string{} for _, id := range []string{"vergabe-demo-company", "user-engine-portal"} { c := clients[id] if c == nil || !contains(c.GrantTypes, "authorization_code") || contains(c.GrantTypes, "client_credentials") { @@ -69,8 +76,20 @@ func Open(path string, clients map[string]*domain.Client) (*Store, error) { } else if c.MFARequired != nil && !*c.MFARequired { return nil, ErrUnavailable } - s.current.Modes[id] = mode - s.names[id] = c.DisplayName + modes[id] = mode + } + return modes, nil +} + +func Open(path string, clients map[string]*domain.Client) (*Store, error) { + modes, err := clientModes(clients) + if err != nil { + return nil, err + } + s := &Store{path: path, names: map[string]string{}, pending: map[string]ticket{}, now: time.Now} + s.current.Modes = modes + for id := range modes { + s.names[id] = clients[id].DisplayName } lock, err := os.OpenFile(path+".lock", os.O_CREATE|os.O_RDWR, 0600) if err != nil { diff --git a/workplans/KEY-WP-0013-approval-engine-resource-audience.md b/workplans/KEY-WP-0013-approval-engine-resource-audience.md index fe8f624..7d77aee 100644 --- a/workplans/KEY-WP-0013-approval-engine-resource-audience.md +++ b/workplans/KEY-WP-0013-approval-engine-resource-audience.md @@ -558,3 +558,16 @@ railiance-platform asked for on 2026-09-15, to railiance-platform, approval-engi and net-kingdom: remove the registration (attended), verify `invalid_client`, unbind the env, retire the ESO delivery, then retire custody. The proposal executes nothing. Each owner confirms their step and the founder schedules the attended window. + +**2026-10-10, amended by the owners.** net-kingdom: step 1 (`keycape-client.py +apply --operation remove`, case acceptance CCR-2026-0018) only patches +`sso/keycape-config` and does not restart. The step-3 env removal triggers the one +rollout, and the new pod starts with neither the client nor the env. Step 2's +`invalid_client` check follows that rollout. Rollback runs in reverse: env first, +then the client. railiance-platform (RPF-WP-0035-T10): step 4 also retires the +ClusterSecretStore, the Kubernetes-auth role and the reader policies, through an +attended prune sync run strictly after step 3. Step 5 is a metadata delete, not an +archive, run only after the step-2 receipt. Steps 1-5 can share one attended +window. Still open: approval-engine's reconfirmation that there is no presenter, +and the founder scheduling the window. Before that rollout, run +`keycape check-config` on the patched configuration.