Add keycape check-config; reject duplicate clientIds
All checks were successful
Authentication acceptance / acceptance (push) Successful in 1m25s
Authentication acceptance / provider-contract (push) Successful in 25s
Build and Publish Container Image / build-and-push (push) Successful in 43s

net-kingdom asked for a way to validate a rendered configuration before
keycape-client.py restarts KeyCape (oidc-browser-relying-party review R4).
check-config runs every startup check that needs no secret, key or upstream:
validation, registry build, global tokenLifetime, policy-store clients and,
with --env-names, the secretRef bindings. The report carries no values.

ValidateConfig now refuses a clientId registered twice. The registry is a
map, so a duplicate silently replaced the earlier registration.

Also records the owners' amended CCR-2026-0018 sequence in KEY-WP-0013.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Assistant: claude-code
Assistant-Model: opus
Assistant-Process: 779409@bnt-lap001
Assistant-Session: c658ca06-19b5-468d-be64-2985dc20bfd3
This commit is contained in:
tegwick 2026-10-10 13:28:36 +02:00
parent 5bc4a366b0
commit c528daa5be
9 changed files with 267 additions and 6 deletions

View file

@ -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

View file

@ -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

View file

@ -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
}

View file

@ -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)
}
}

View file

@ -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)
}

View file

@ -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)
}
}

View file

@ -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")
}

View file

@ -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 {

View file

@ -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.