From 155392a809d7890e2bb1766962831b2e6f121793 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sun, 30 Aug 2026 15:31:25 +1000 Subject: [PATCH] Add agentvault seed-oauth for oauth2-proxy credentials Seeding an oauth2-proxy secret by hand means an agent shell-plumbing a client secret and a cookie secret, which the classifier blocks. seed-oauth does it in one self-contained invocation: it reads the KV path, fills in only the keys that are missing, preserves everything else and prints key names and the new version, never a value. - Add SeedOAuth in internal/agent: read-modify-write of the client_id, client_secret and cookie_secret keys with per-key created/kept/rotated actions and a no-op when nothing changed. - Generate secrets from 32 crypto/rand bytes; cookie_secret is base64url so it decodes to exactly the 32 bytes oauth2-proxy requires. - Add ReadKVOptional (missing secret = empty) and WriteKVAny (non-string fields survive a round trip) to the KV-v2 client. - Wire the seed-oauth subcommand and document it in README and AGENTS.md. - Cover fresh create, patch-preserves-client_secret, other-key preservation, --rotate, idempotence, denial errors and secret leakage. --- AGENTS.md | 30 ++- README.md | 36 +++ cmd/agentvault/main.go | 46 +++- cmd/agentvault/main_test.go | 84 ++++++ internal/agent/seedoauth.go | 183 ++++++++++++++ internal/agent/seedoauth_test.go | 421 +++++++++++++++++++++++++++++++ internal/agent/vaultkv.go | 32 +++ 7 files changed, 829 insertions(+), 3 deletions(-) create mode 100644 internal/agent/seedoauth.go create mode 100644 internal/agent/seedoauth_test.go diff --git a/AGENTS.md b/AGENTS.md index b500548..801c0c6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,7 +31,7 @@ parsing, watch-state comparison, git worktree helpers). cmd/agentpr/main.go # agentpr CLI (pr create / pr comment / whoami) cmd/watchpr/main.go # watchpr CLI (poll + meaningful-change exit) cmd/agentws/main.go # agentws CLI (new / list / rm / clean / token / credential) -cmd/agentvault/main.go # agentvault CLI (seed-outpost) +cmd/agentvault/main.go # agentvault CLI (seed-outpost / seed-oauth) internal/agent/ # shared plumbing: token.go # env config + in-process Gitea-token cache vault.go # AppRole login + read the gitea creds path @@ -42,6 +42,7 @@ internal/agent/ # shared plumbing: vaultkv.go # AppRole-authenticated Vault client + KV-v2 read/write authentik.go # Authentik REST client (outpost search, token view_key) seedoutpost.go # seed-outpost flow (Authentik token -> Vault KV) + seedoauth.go # seed-oauth flow (oauth2-proxy credential set in Vault KV) go.mod # module git.unkin.net/unkin/agent-tools Makefile # build / test / lint / completions / rpm / version-bump packaging/nfpm.yaml # nfpm spec (envsubst-templated) for the RPM (all binaries) @@ -148,12 +149,37 @@ printed. Errors are wrapped per stage (login / read denied / outpost missing / view_key / write denied) via the `ErrVaultDenied`, `ErrVaultNotFound` and `ErrOutpostNotFound` sentinels. +## agentvault seed-oauth + +`agentvault seed-oauth --path --client-id ` makes a KV-v2 path +hold a complete oauth2-proxy credential set, in-process: + +1. AppRole login (shared `approleLogin`), then a KV-v2 read via + `ReadKVOptional` — a 404 or a deleted version means "empty", not an error, + so the first seed of a path works. +2. Desired keys are computed over the existing map: `client_id` from the flag + (`kept`/`created`/`updated`), `client_secret` and `cookie_secret` generated + from 32 `crypto/rand` bytes only when absent or when `--rotate` is set + (`kept`/`created`/`rotated`). `cookie_secret` is base64url so it decodes to + exactly the 32 bytes oauth2-proxy demands; `client_secret` is standard + base64. +3. Any other key on the path is carried through unchanged (`preserved`), which + is why the write goes through `WriteKVAny` rather than `WriteKV`. +4. The write is skipped entirely when nothing changed; the command then prints + `version: unchanged`. + +Only key names, per-key actions and the new KV version are printed. Errors are +wrapped per stage (login / read denied / write denied) via `ErrVaultDenied`. + ## Gotchas - `watchpr` exits 0 with no output changes on `--once` (just prints state). - The token cache is process-wide (`sync.Once`); tests call the unexported `fetchGiteaToken` to avoid it. - `agentvault` never puts a secret in an error string: Vault decode failures and - Authentik `view_key` responses are reported without their bodies. + Authentik `view_key` responses are reported without their bodies, and + `seed-oauth` reports key names only. +- `--rotate` regenerates the `client_secret` too, which then no longer matches + the IdP provider unless that is rotated alongside. - CI "combined status" comes from `/commits/{sha}/status`; an empty head SHA yields an empty state without an API call. diff --git a/README.md b/README.md index d15bcc0..788eb3e 100644 --- a/README.md +++ b/README.md @@ -162,6 +162,42 @@ Errors name the failing stage: AppRole login, KV read denied (policy not applied), outpost not found (terraform not applied), `view_key` failure, or KV write denied. +### seed-oauth + +Make a Vault KV-v2 path hold a complete oauth2-proxy credential set. It is a +read-modify-write: `client_id` is set from the flag, `client_secret` and +`cookie_secret` are generated (32 bytes from `crypto/rand`) only when missing, +every other key on the path is written back untouched, and nothing is written +at all when the secret is already correct. `cookie_secret` is base64url so it +decodes to exactly the 32 bytes oauth2-proxy requires. + +```bash +agentvault seed-oauth \ + --path kubernetes/namespace/repospawner/default/oauth-credentials \ + --client-id 4f1c… +``` + +``` +path: kv/kubernetes/namespace/repospawner/default/oauth-credentials +keys: client_id, client_secret, cookie_secret + client_id: created + client_secret: kept + cookie_secret: created +version: 4 +``` + +That is the common case: the provider's `client_secret` already lives on the +path, so only the missing keys are added. A run with nothing to do prints +`version: unchanged` and issues no write. + +Flags: `--path` and `--client-id` are required; `--kv-mount` (default `kv`) and +`--rotate` override the rest. `--rotate` regenerates both secrets — only use it +when the IdP provider's secret is being rotated alongside, since a rotated +`client_secret` no longer matches the provider. + +Errors name the failing stage: AppRole login, KV read denied, or KV write +denied. Only key names, actions and the KV version are printed. + ## Build & package ```bash diff --git a/cmd/agentvault/main.go b/cmd/agentvault/main.go index 72fab30..35d17e6 100644 --- a/cmd/agentvault/main.go +++ b/cmd/agentvault/main.go @@ -3,11 +3,13 @@ // with the same Vault AppRole as agentpr (role_id only, no secret_id). // // agentvault seed-outpost --outpost --dest-path +// agentvault seed-oauth --path --client-id package main import ( "fmt" "os" + "strings" "git.unkin.net/unkin/agent-tools/internal/agent" @@ -33,7 +35,7 @@ func newRootCmd() *cobra.Command { SilenceUsage: true, } root.SetVersionTemplate("{{.Version}}\n") - root.AddCommand(newSeedOutpostCmd(), newVersionCmd()) + root.AddCommand(newSeedOutpostCmd(), newSeedOAuthCmd(), newVersionCmd()) return root } @@ -73,6 +75,48 @@ func newSeedOutpostCmd() *cobra.Command { return cmd } +func newSeedOAuthCmd() *cobra.Command { + opts := agent.SeedOAuthOptions{} + cmd := &cobra.Command{ + Use: "seed-oauth", + Short: "Seed an oauth2-proxy credential set into Vault KV", + Long: "Make a Vault KV-v2 path hold a complete oauth2-proxy credential set: the\n" + + "given client_id, plus a client_secret and a 32-byte cookie_secret that are\n" + + "generated only when missing (or with --rotate). Existing keys are preserved\n" + + "and nothing is written when the secret is already correct. Secret values are\n" + + "never printed or logged.", + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + opts.VaultAddr = agent.VaultAddr() + opts.RoleID = agent.RoleID() + res, err := agent.SeedOAuth(opts) + if err != nil { + return err + } + out := cmd.OutOrStdout() + _, _ = fmt.Fprintf(out, "path: %s/%s\n", res.KVMount, res.Path) + _, _ = fmt.Fprintf(out, "keys: %s\n", strings.Join(res.KeyNames(), ", ")) + for _, k := range res.Keys { + _, _ = fmt.Fprintf(out, " %-14s %s\n", k.Name+":", k.Action) + } + if res.Changed { + _, _ = fmt.Fprintf(out, "version: %d\n", res.Version) + } else { + _, _ = fmt.Fprintf(out, "version: unchanged\n") + } + return nil + }, + } + f := cmd.Flags() + f.StringVar(&opts.Path, "path", "", "KV-v2 path holding the credentials, e.g. kubernetes/namespace/repospawner/default/oauth-credentials (required)") + f.StringVar(&opts.ClientID, "client-id", "", "OIDC client id to store (required)") + f.StringVar(&opts.KVMount, "kv-mount", agent.DefaultKVMount, "KV-v2 mount holding the path") + f.BoolVar(&opts.Rotate, "rotate", false, "Regenerate client_secret and cookie_secret even when they already exist") + _ = cmd.MarkFlagRequired("path") + _ = cmd.MarkFlagRequired("client-id") + return cmd +} + func newVersionCmd() *cobra.Command { return &cobra.Command{ Use: "version", diff --git a/cmd/agentvault/main_test.go b/cmd/agentvault/main_test.go index 7a027c1..529d667 100644 --- a/cmd/agentvault/main_test.go +++ b/cmd/agentvault/main_test.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "encoding/json" "io" "net/http" "net/http/httptest" @@ -78,6 +79,89 @@ func TestSeedOutpostOutputHasNoSecrets(t *testing.T) { } } +const oauthPath = "kubernetes/namespace/repospawner/default/oauth-credentials" + +// fakeOAuthVault serves approle login plus a KV-v2 path that already holds a +// client_secret, and records what gets written back. +func fakeOAuthVault(t *testing.T, existing map[string]string) (vaultURL string, written *map[string]string) { + t.Helper() + writes := map[string]string{} + mux := http.NewServeMux() + mux.HandleFunc("/v1/auth/approle/login", func(w http.ResponseWriter, r *http.Request) { + _, _ = io.WriteString(w, `{"auth":{"client_token":"s.vaulttoken"}}`) + }) + mux.HandleFunc("/v1/kv/data/"+oauthPath, func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost { + var body struct { + Data map[string]string `json:"data"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + for k, v := range body.Data { + writes[k] = v + } + _, _ = io.WriteString(w, `{"data":{"version":4}}`) + return + } + payload, _ := json.Marshal(map[string]any{"data": map[string]any{"data": existing}}) + _, _ = w.Write(payload) + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv.URL, &writes +} + +// The command prints key names and the KV version only — never a value. +func TestSeedOAuthOutputHasNoSecrets(t *testing.T) { + const existingSecret = "existing-client-secret-value" + vaultURL, written := fakeOAuthVault(t, map[string]string{"client_secret": existingSecret}) + t.Setenv("VAULT_ADDR", vaultURL) + t.Setenv("AGENT_APPROLE_ROLE_ID", "role-xyz") + + var out bytes.Buffer + cmd := newRootCmd() + cmd.SetOut(&out) + cmd.SetErr(&out) + cmd.SetArgs([]string{"seed-oauth", "--path", oauthPath, "--client-id", "mediamark-client-id"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute: %v", err) + } + + got := out.String() + for _, want := range []string{"kv/" + oauthPath, "client_id, client_secret, cookie_secret", "client_secret: kept", "cookie_secret: created", "version: 4"} { + if !strings.Contains(got, want) { + t.Errorf("output missing %q:\n%s", want, got) + } + } + for key, value := range *written { + if key == "client_id" { + continue + } + if strings.Contains(got, value) { + t.Fatalf("output leaks the %s value:\n%s", key, got) + } + } + if strings.Contains(got, existingSecret) { + t.Fatalf("output leaks the existing client_secret:\n%s", got) + } +} + +func TestSeedOAuthRequiresFlags(t *testing.T) { + for name, args := range map[string][]string{ + "no path": {"seed-oauth", "--client-id", "mediamark-client-id"}, + "no client-id": {"seed-oauth", "--path", oauthPath}, + } { + t.Run(name, func(t *testing.T) { + cmd := newRootCmd() + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + cmd.SetArgs(args) + if err := cmd.Execute(); err == nil { + t.Fatal("Execute() = nil, want a missing-required-flag error") + } + }) + } +} + func TestSeedOutpostRequiresFlags(t *testing.T) { for name, args := range map[string][]string{ "no outpost": {"seed-outpost", "--dest-path", destPath}, diff --git a/internal/agent/seedoauth.go b/internal/agent/seedoauth.go new file mode 100644 index 0000000..8a30141 --- /dev/null +++ b/internal/agent/seedoauth.go @@ -0,0 +1,183 @@ +package agent + +import ( + "crypto/rand" + "encoding/base64" + "errors" + "fmt" + "sort" +) + +const ( + // OAuthClientIDKey, OAuthClientSecretKey and OAuthCookieSecretKey are the + // KV fields oauth2-proxy deployments read their credentials from. + OAuthClientIDKey = "client_id" + OAuthClientSecretKey = "client_secret" + OAuthCookieSecretKey = "cookie_secret" + + // oauthSecretBytes is the raw length of generated secrets. oauth2-proxy + // rejects a cookie secret that does not decode to exactly 32 bytes. + oauthSecretBytes = 32 +) + +// Per-key outcomes reported by SeedOAuth. +const ( + ActionCreated = "created" + ActionKept = "kept" + ActionRotated = "rotated" + ActionUpdated = "updated" + ActionPreserved = "preserved" +) + +// SeedOAuthOptions configures SeedOAuth. The CLI supplies the defaults. +type SeedOAuthOptions struct { + VaultAddr string + RoleID string + KVMount string + Path string + ClientID string + Rotate bool +} + +// SeedOAuthKey names a key present in the secret and what happened to it. +type SeedOAuthKey struct { + Name string + Action string +} + +// SeedOAuthResult is the non-secret summary of a seed run. Version is zero when +// nothing changed and no write was issued. +type SeedOAuthResult struct { + KVMount string + Path string + Keys []SeedOAuthKey + Version int + Changed bool +} + +// KeyNames returns the key names present in the secret after the run. +func (r SeedOAuthResult) KeyNames() []string { + names := make([]string, 0, len(r.Keys)) + for _, k := range r.Keys { + names = append(names, k.Name) + } + return names +} + +// SeedOAuth makes a Vault KV-v2 path hold a complete oauth2-proxy credential +// set: client_id from the caller, plus a client_secret and cookie_secret that +// are generated only when absent (or when Rotate is set). It is a +// read-modify-write, so keys it does not own survive untouched, and it writes +// nothing when the secret is already correct. Secret material never leaves this +// function: results and errors carry only key names. +func SeedOAuth(o SeedOAuthOptions) (SeedOAuthResult, error) { + var res SeedOAuthResult + if o.Path == "" { + return res, errors.New("a KV-v2 path is required") + } + if o.ClientID == "" { + return res, errors.New("a client id is required") + } + + vc, err := NewVaultClient(o.VaultAddr, o.RoleID) + if err != nil { + return res, fmt.Errorf("vault approle login failed against %s (check VAULT_ADDR and AGENT_APPROLE_ROLE_ID): %w", o.VaultAddr, err) + } + + existing, err := vc.ReadKVOptional(o.KVMount, o.Path) + if err != nil { + if errors.Is(err, ErrVaultDenied) { + return res, fmt.Errorf("reading %s/%s denied: the agent AppRole policy does not grant read on this path (apply the terraform-vault policy change): %w", o.KVMount, o.Path, err) + } + return res, fmt.Errorf("reading %s/%s: %w", o.KVMount, o.Path, err) + } + + data := make(map[string]any, len(existing)+3) + for k, v := range existing { + data[k] = v + } + + var ( + keys []SeedOAuthKey + changed bool + ) + + current, _ := existing[OAuthClientIDKey].(string) + switch current { + case o.ClientID: + keys = append(keys, SeedOAuthKey{OAuthClientIDKey, ActionKept}) + case "": + keys = append(keys, SeedOAuthKey{OAuthClientIDKey, ActionCreated}) + changed = true + default: + keys = append(keys, SeedOAuthKey{OAuthClientIDKey, ActionUpdated}) + changed = true + } + data[OAuthClientIDKey] = o.ClientID + + for _, gen := range []struct { + name string + enc *base64.Encoding + }{ + // oauth2-proxy accepts a standard-base64 client secret, but the cookie + // secret goes into a cookie and must be URL-safe. + {OAuthClientSecretKey, base64.StdEncoding}, + {OAuthCookieSecretKey, base64.RawURLEncoding}, + } { + current, _ := existing[gen.name].(string) + if current != "" && !o.Rotate { + keys = append(keys, SeedOAuthKey{gen.name, ActionKept}) + continue + } + value, err := randomSecret(gen.enc) + if err != nil { + return res, fmt.Errorf("generating %s: %w", gen.name, err) + } + action := ActionCreated + if current != "" { + action = ActionRotated + } + data[gen.name] = value + keys = append(keys, SeedOAuthKey{gen.name, action}) + changed = true + } + + var others []string + for k := range existing { + switch k { + case OAuthClientIDKey, OAuthClientSecretKey, OAuthCookieSecretKey: + default: + others = append(others, k) + } + } + sort.Strings(others) + for _, k := range others { + keys = append(keys, SeedOAuthKey{k, ActionPreserved}) + } + + res = SeedOAuthResult{KVMount: o.KVMount, Path: o.Path, Keys: keys} + if !changed { + return res, nil + } + + version, err := vc.WriteKVAny(o.KVMount, o.Path, data) + if err != nil { + if errors.Is(err, ErrVaultDenied) { + return SeedOAuthResult{}, fmt.Errorf("writing %s/%s denied: the agent AppRole policy does not grant create/update on this path (apply the terraform-vault policy change): %w", o.KVMount, o.Path, err) + } + return SeedOAuthResult{}, fmt.Errorf("writing %s/%s: %w", o.KVMount, o.Path, err) + } + res.Version = version + res.Changed = true + return res, nil +} + +// randomSecret returns oauthSecretBytes of crypto/rand entropy in the given +// base64 encoding. +func randomSecret(enc *base64.Encoding) (string, error) { + buf := make([]byte, oauthSecretBytes) + if _, err := rand.Read(buf); err != nil { + return "", err + } + return enc.EncodeToString(buf), nil +} diff --git a/internal/agent/seedoauth_test.go b/internal/agent/seedoauth_test.go new file mode 100644 index 0000000..f4523af --- /dev/null +++ b/internal/agent/seedoauth_test.go @@ -0,0 +1,421 @@ +package agent + +import ( + "encoding/base64" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" +) + +const ( + oauthPath = "kubernetes/namespace/repospawner/default/oauth-credentials" + oauthClientID = "mediamark-client-id" + existingClientSec = "existing-client-secret-from-authentik" + existingCookieSec = "existing-cookie-secret-value-abcdefghij" + oauthExtraKeyValue = "extra-key-secret-value" + oauthVaultClientTok = "s.vaulttoken" +) + +// oauthVaultStub is a KV-v2 stand-in that actually stores what is written, so +// read-modify-write behaviour can be asserted end to end. +type oauthVaultStub struct { + data map[string]any + exists bool + version int + readStatus int + writeStatus int + writes []map[string]any +} + +func newOAuthVaultStub() *oauthVaultStub { + return &oauthVaultStub{readStatus: http.StatusOK, writeStatus: http.StatusOK} +} + +// seed makes the path exist with the given fields at version 1. +func (v *oauthVaultStub) seed(data map[string]any) *oauthVaultStub { + v.data = data + v.exists = true + v.version = 1 + return v +} + +func (v *oauthVaultStub) server(t *testing.T) *httptest.Server { + t.Helper() + mux := http.NewServeMux() + mux.HandleFunc("/v1/auth/approle/login", func(w http.ResponseWriter, r *http.Request) { + var body map[string]string + _ = json.NewDecoder(r.Body).Decode(&body) + if _, ok := body["secret_id"]; ok { + t.Errorf("secret_id must not be sent") + } + _, _ = io.WriteString(w, `{"auth":{"client_token":"`+oauthVaultClientTok+`"}}`) + }) + mux.HandleFunc("/v1/kv/data/"+oauthPath, func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("X-Vault-Token"); got != oauthVaultClientTok { + t.Errorf("X-Vault-Token = %q, want %q", got, oauthVaultClientTok) + } + switch r.Method { + case http.MethodGet: + if v.readStatus != http.StatusOK { + w.WriteHeader(v.readStatus) + _, _ = io.WriteString(w, `{"errors":["permission denied"]}`) + return + } + if !v.exists { + w.WriteHeader(http.StatusNotFound) + _, _ = io.WriteString(w, `{"errors":[]}`) + return + } + payload, _ := json.Marshal(map[string]any{ + "data": map[string]any{"data": v.data, "metadata": map[string]any{"version": v.version}}, + }) + _, _ = w.Write(payload) + case http.MethodPost: + if v.writeStatus != http.StatusOK { + w.WriteHeader(v.writeStatus) + _, _ = io.WriteString(w, `{"errors":["permission denied"]}`) + return + } + var body struct { + Data map[string]any `json:"data"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + v.writes = append(v.writes, body.Data) + v.data = body.Data + v.exists = true + v.version++ + _, _ = io.WriteString(w, `{"data":{"version":`+strconv.Itoa(v.version)+`}}`) + default: + t.Errorf("unexpected method %s", r.Method) + } + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv +} + +func oauthOpts(vaultURL string) SeedOAuthOptions { + return SeedOAuthOptions{ + VaultAddr: vaultURL, + RoleID: "role-xyz", + KVMount: DefaultKVMount, + Path: oauthPath, + ClientID: oauthClientID, + } +} + +// actions flattens a result into key -> action for order-independent asserts. +func actions(res SeedOAuthResult) map[string]string { + m := make(map[string]string, len(res.Keys)) + for _, k := range res.Keys { + m[k.Name] = k.Action + } + return m +} + +func stringField(t *testing.T, data map[string]any, key string) string { + t.Helper() + s, ok := data[key].(string) + if !ok { + t.Fatalf("written %s = %v, want a string", key, data[key]) + } + return s +} + +// assertDecodesTo32 fails unless the value is base64 of exactly 32 bytes, which +// is what oauth2-proxy requires of a cookie secret. +func assertDecodesTo32(t *testing.T, enc *base64.Encoding, value, name string) { + t.Helper() + raw, err := enc.DecodeString(value) + if err != nil { + t.Fatalf("%s is not valid base64: %v", name, err) + } + if len(raw) != oauthSecretBytes { + t.Errorf("%s decodes to %d bytes, want %d", name, len(raw), oauthSecretBytes) + } +} + +func TestSeedOAuthFreshCreate(t *testing.T) { + v := newOAuthVaultStub() + res, err := SeedOAuth(oauthOpts(v.server(t).URL)) + if err != nil { + t.Fatalf("SeedOAuth: %v", err) + } + if !res.Changed || res.Version != 1 { + t.Errorf("Changed=%v Version=%d, want a first write at version 1", res.Changed, res.Version) + } + for key, want := range map[string]string{ + OAuthClientIDKey: ActionCreated, + OAuthClientSecretKey: ActionCreated, + OAuthCookieSecretKey: ActionCreated, + } { + if got := actions(res)[key]; got != want { + t.Errorf("%s action = %q, want %q", key, got, want) + } + } + if len(v.writes) != 1 { + t.Fatalf("%d writes, want exactly 1", len(v.writes)) + } + w := v.writes[0] + if got := stringField(t, w, OAuthClientIDKey); got != oauthClientID { + t.Errorf("written client_id = %q, want %q", got, oauthClientID) + } + assertDecodesTo32(t, base64.StdEncoding, stringField(t, w, OAuthClientSecretKey), OAuthClientSecretKey) + assertDecodesTo32(t, base64.RawURLEncoding, stringField(t, w, OAuthCookieSecretKey), OAuthCookieSecretKey) +} + +// The mediamark case: a client_secret already issued by Authentik must survive +// while the missing keys are filled in. +func TestSeedOAuthPreservesExistingClientSecret(t *testing.T) { + v := newOAuthVaultStub().seed(map[string]any{OAuthClientSecretKey: existingClientSec}) + res, err := SeedOAuth(oauthOpts(v.server(t).URL)) + if err != nil { + t.Fatalf("SeedOAuth: %v", err) + } + got := actions(res) + for key, want := range map[string]string{ + OAuthClientIDKey: ActionCreated, + OAuthClientSecretKey: ActionKept, + OAuthCookieSecretKey: ActionCreated, + } { + if got[key] != want { + t.Errorf("%s action = %q, want %q", key, got[key], want) + } + } + if len(v.writes) != 1 { + t.Fatalf("%d writes, want exactly 1", len(v.writes)) + } + if s := stringField(t, v.writes[0], OAuthClientSecretKey); s != existingClientSec { + t.Errorf("client_secret was replaced, want the existing value kept") + } +} + +func TestSeedOAuthPreservesOtherKeys(t *testing.T) { + v := newOAuthVaultStub().seed(map[string]any{ + OAuthClientIDKey: oauthClientID, + OAuthClientSecretKey: existingClientSec, + "redirect_url": "https://mediamark.unkin.net/oauth2/callback", + "extra": oauthExtraKeyValue, + }) + res, err := SeedOAuth(oauthOpts(v.server(t).URL)) + if err != nil { + t.Fatalf("SeedOAuth: %v", err) + } + got := actions(res) + for _, key := range []string{"redirect_url", "extra"} { + if got[key] != ActionPreserved { + t.Errorf("%s action = %q, want %q", key, got[key], ActionPreserved) + } + } + if len(v.writes) != 1 { + t.Fatalf("%d writes, want exactly 1", len(v.writes)) + } + w := v.writes[0] + if stringField(t, w, "extra") != oauthExtraKeyValue { + t.Errorf("extra key was not written back unchanged") + } + if stringField(t, w, "redirect_url") != "https://mediamark.unkin.net/oauth2/callback" { + t.Errorf("redirect_url was not written back unchanged") + } +} + +func TestSeedOAuthRotateRegenerates(t *testing.T) { + v := newOAuthVaultStub().seed(map[string]any{ + OAuthClientIDKey: oauthClientID, + OAuthClientSecretKey: existingClientSec, + OAuthCookieSecretKey: existingCookieSec, + }) + o := oauthOpts(v.server(t).URL) + o.Rotate = true + + res, err := SeedOAuth(o) + if err != nil { + t.Fatalf("SeedOAuth: %v", err) + } + got := actions(res) + for key, want := range map[string]string{ + OAuthClientIDKey: ActionKept, + OAuthClientSecretKey: ActionRotated, + OAuthCookieSecretKey: ActionRotated, + } { + if got[key] != want { + t.Errorf("%s action = %q, want %q", key, got[key], want) + } + } + if len(v.writes) != 1 { + t.Fatalf("%d writes, want exactly 1", len(v.writes)) + } + w := v.writes[0] + if stringField(t, w, OAuthClientSecretKey) == existingClientSec { + t.Errorf("client_secret unchanged under --rotate") + } + if stringField(t, w, OAuthCookieSecretKey) == existingCookieSec { + t.Errorf("cookie_secret unchanged under --rotate") + } + assertDecodesTo32(t, base64.RawURLEncoding, stringField(t, w, OAuthCookieSecretKey), OAuthCookieSecretKey) +} + +// A complete, correct secret must not produce a new KV version. +func TestSeedOAuthIdempotentWritesNothing(t *testing.T) { + v := newOAuthVaultStub() + url := v.server(t).URL + if _, err := SeedOAuth(oauthOpts(url)); err != nil { + t.Fatalf("first SeedOAuth: %v", err) + } + res, err := SeedOAuth(oauthOpts(url)) + if err != nil { + t.Fatalf("second SeedOAuth: %v", err) + } + if res.Changed || res.Version != 0 { + t.Errorf("Changed=%v Version=%d, want an unchanged result", res.Changed, res.Version) + } + if len(v.writes) != 1 { + t.Errorf("%d writes, want the second run to write nothing", len(v.writes)) + } + for _, k := range res.Keys { + if k.Action != ActionKept { + t.Errorf("%s action = %q, want %q", k.Name, k.Action, ActionKept) + } + } +} + +func TestSeedOAuthClientIDUpdated(t *testing.T) { + v := newOAuthVaultStub().seed(map[string]any{ + OAuthClientIDKey: "stale-client-id", + OAuthClientSecretKey: existingClientSec, + OAuthCookieSecretKey: existingCookieSec, + }) + res, err := SeedOAuth(oauthOpts(v.server(t).URL)) + if err != nil { + t.Fatalf("SeedOAuth: %v", err) + } + if got := actions(res)[OAuthClientIDKey]; got != ActionUpdated { + t.Errorf("client_id action = %q, want %q", got, ActionUpdated) + } + if len(v.writes) != 1 || stringField(t, v.writes[0], OAuthClientIDKey) != oauthClientID { + t.Errorf("writes = %v, want the new client_id written", v.writes) + } +} + +func TestSeedOAuthLoginFailure(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/v1/auth/approle/login", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _, _ = io.WriteString(w, `{"errors":["invalid role ID"]}`) + }) + vs := httptest.NewServer(mux) + defer vs.Close() + + _, err := SeedOAuth(oauthOpts(vs.URL)) + if err == nil { + t.Fatal("SeedOAuth() = nil, want an approle login error") + } + if !strings.Contains(err.Error(), "approle login failed") { + t.Errorf("error = %v, want it to name the approle login", err) + } +} + +func TestSeedOAuthReadDenied(t *testing.T) { + v := newOAuthVaultStub() + v.readStatus = http.StatusForbidden + + _, err := SeedOAuth(oauthOpts(v.server(t).URL)) + if err == nil { + t.Fatal("SeedOAuth() = nil, want a KV read error") + } + msg := err.Error() + if !strings.Contains(msg, oauthPath) || !strings.Contains(msg, "policy") { + t.Errorf("error = %v, want it to name the path and point at the policy", err) + } + if len(v.writes) != 0 { + t.Errorf("wrote %v, want no write when the read is denied", v.writes) + } +} + +func TestSeedOAuthWriteDenied(t *testing.T) { + v := newOAuthVaultStub() + v.writeStatus = http.StatusForbidden + + _, err := SeedOAuth(oauthOpts(v.server(t).URL)) + if err == nil { + t.Fatal("SeedOAuth() = nil, want a KV write error") + } + msg := err.Error() + if !strings.Contains(msg, oauthPath) || !strings.Contains(msg, "create/update") { + t.Errorf("error = %v, want it to name the path and the missing capability", err) + } +} + +// A missing path is normal (first seed), not a not-found error. +func TestSeedOAuthMissingPathIsNotAnError(t *testing.T) { + v := newOAuthVaultStub() + if _, err := SeedOAuth(oauthOpts(v.server(t).URL)); err != nil { + t.Fatalf("SeedOAuth on a missing path: %v", err) + } +} + +func TestSeedOAuthRequiresPathAndClientID(t *testing.T) { + v := newOAuthVaultStub() + url := v.server(t).URL + for name, mutate := range map[string]func(*SeedOAuthOptions){ + "no path": func(o *SeedOAuthOptions) { o.Path = "" }, + "no client id": func(o *SeedOAuthOptions) { o.ClientID = "" }, + } { + t.Run(name, func(t *testing.T) { + o := oauthOpts(url) + mutate(&o) + if _, err := SeedOAuth(o); err == nil { + t.Fatal("SeedOAuth() = nil, want a required-input error") + } + }) + } +} + +// No failure path may leak stored or generated secret material. +func TestSeedOAuthErrorsNeverLeakSecrets(t *testing.T) { + cases := map[string]func(*oauthVaultStub){ + "read denied": func(v *oauthVaultStub) { v.readStatus = http.StatusForbidden }, + "write denied": func(v *oauthVaultStub) { v.writeStatus = http.StatusForbidden }, + "read error": func(v *oauthVaultStub) { v.readStatus = http.StatusInternalServerError }, + "write error": func(v *oauthVaultStub) { v.writeStatus = http.StatusInternalServerError }, + } + for name, mutate := range cases { + t.Run(name, func(t *testing.T) { + v := newOAuthVaultStub().seed(map[string]any{ + OAuthClientSecretKey: existingClientSec, + OAuthCookieSecretKey: existingCookieSec, + "extra": oauthExtraKeyValue, + }) + mutate(v) + _, err := SeedOAuth(oauthOpts(v.server(t).URL)) + if err == nil { + t.Fatal("SeedOAuth() = nil, want an error") + } + for _, secret := range []string{existingClientSec, existingCookieSec, oauthExtraKeyValue} { + if strings.Contains(err.Error(), secret) { + t.Errorf("error %q leaks a secret", err) + } + } + }) + } +} + +// The successful result carries key names and a version, never values. +func TestSeedOAuthResultNeverCarriesSecrets(t *testing.T) { + v := newOAuthVaultStub().seed(map[string]any{OAuthClientSecretKey: existingClientSec}) + res, err := SeedOAuth(oauthOpts(v.server(t).URL)) + if err != nil { + t.Fatalf("SeedOAuth: %v", err) + } + rendered := strings.Join(append(res.KeyNames(), res.Path, res.KVMount), " ") + written := v.writes[0] + for _, key := range []string{OAuthClientSecretKey, OAuthCookieSecretKey} { + if value := stringField(t, written, key); strings.Contains(rendered, value) { + t.Errorf("result leaks the %s value", key) + } + } +} diff --git a/internal/agent/vaultkv.go b/internal/agent/vaultkv.go index 5587a67..49817ac 100644 --- a/internal/agent/vaultkv.go +++ b/internal/agent/vaultkv.go @@ -106,8 +106,40 @@ func (c *VaultClient) ReadKV(mount, path string) (map[string]any, error) { return out.Data.Data, nil } +// ReadKVOptional is ReadKV but treats a missing secret as empty: a 404 or a +// deleted version (data: null) yields an empty map, not an error. Callers that +// read-modify-write a path that may not exist yet use this. +func (c *VaultClient) ReadKVOptional(mount, path string) (map[string]any, error) { + var out struct { + Data struct { + Data map[string]any `json:"data"` + } `json:"data"` + } + if err := c.do(http.MethodGet, kvDataPath(mount, path), nil, &out); err != nil { + if errors.Is(err, ErrVaultNotFound) { + return map[string]any{}, nil + } + return nil, err + } + if out.Data.Data == nil { + return map[string]any{}, nil + } + return out.Data.Data, nil +} + // WriteKV writes a KV-v2 secret and returns the version it created. func (c *VaultClient) WriteKV(mount, path string, data map[string]string) (int, error) { + fields := make(map[string]any, len(data)) + for k, v := range data { + fields[k] = v + } + return c.WriteKVAny(mount, path, fields) +} + +// WriteKVAny writes a KV-v2 secret whose fields are not all strings (so a +// read-modify-write can put back values it did not author) and returns the +// version it created. +func (c *VaultClient) WriteKVAny(mount, path string, data map[string]any) (int, error) { var out struct { Data struct { Version int `json:"version"`