diff --git a/CHANGELOG.md b/CHANGELOG.md index e70caf2..18e93c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,17 @@ to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). server, and edge subjects; a draft API). This surface is not in the public OpenAPI spec, so these first-class commands are the way to reach it. +- **Access-token cache cuts audit-log noise.** c1i now caches the OAuth access + token in the OS keyring when available, with a hardened `0600` file fallback + on Unix-like headless hosts, and reuses it across invocations until it + nears expiry. A burst of one-shot commands no longer mints — and logs — a + `client_credentials` grant each time. Measured live, 12 sequential commands + drop from 12 authentication events to 1. A cached token the server rejects + (clock skew, or a revocation) is dropped and re-minted once automatically. + Opt out with `C1I_NO_TOKEN_CACHE=1`. The cached token is strictly + shorter-lived than the client secret already stored beside it, so it widens + no exposure. + ### Fixed - **MCP gateway dry-run safety.** `mcp gateway call` now rejects `--dry-run` diff --git a/README.md b/README.md index 516aec5..5ef86c7 100644 --- a/README.md +++ b/README.md @@ -1229,6 +1229,19 @@ never written to disk — a new one is minted per invocation. file backend transparently. `c1i auth status` tells you which source served the active credentials. +### Token cache + +To avoid minting a fresh OAuth token on every invocation — each mint writes a +`client_credentials` event to the tenant's audit log — `c1i` caches the access +token and reuses it until it nears expiry. It uses the OS keyring when +available. On Unix-like hosts without a usable keyring, it instead uses a +hardened `0600` file under the config directory (`~/.config/c1i/tokens/` on +Linux). A cached token the server rejects (clock skew, or a revoked credential) +is dropped and re-minted once automatically. +The token is strictly shorter-lived than the client secret already stored +beside it. Set `C1I_NO_TOKEN_CACHE=1` to disable caching and mint per +invocation. + ## Shell Completion ```sh diff --git a/cmd/agents.md b/cmd/agents.md index 8d4018d..2bc6fb4 100644 --- a/cmd/agents.md +++ b/cmd/agents.md @@ -42,6 +42,13 @@ subcommands that also take an external server's address — that one is Credentials resolve in this order: `C1I_CLIENT_ID` + `C1I_CLIENT_SECRET` env vars (read-only — c1i never writes them), the OS keyring, then a `0600` file used automatically where no keyring exists (headless Linux, CI, containers). +Only the bearer c1i attaches automatically for REST commands is cached and +reused across invocations until it nears expiry, so a run of one-shot commands +does not write a `client_credentials` audit event each time. It uses the OS +keychain when available; on Unix-like hosts without a usable keyring, it uses +a hardened `0600` file fallback. `auth token` and `mcp gateway` always mint a +fresh bearer and never persist it. +`C1I_NO_TOKEN_CACHE=1` disables the REST-client cache. `c1i auth login` to authenticate; then `c1i auth whoami` before doing anything else — it reports both the identity you're acting as (principleId, plus userId when the principal has one; email and displayName only when a diff --git a/cmd/auth_logout.go b/cmd/auth_logout.go index 17c5110..4f4595e 100644 --- a/cmd/auth_logout.go +++ b/cmd/auth_logout.go @@ -3,6 +3,7 @@ package cmd import ( "fmt" + "github.com/ConductorOne/c1i/internal/client" "github.com/ConductorOne/c1i/internal/config" "github.com/ConductorOne/c1i/internal/keychain" "github.com/spf13/cobra" @@ -19,6 +20,7 @@ C1I_CLIENT_SECRET) are not affected.`, if err != nil { return err } + client.ClearCachedToken(baseURL) service := config.KeychainService(baseURL) removed, err := keychain.Delete(service) if err != nil { diff --git a/internal/client/client.go b/internal/client/client.go index bb09c0b..1cea458 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -51,6 +51,17 @@ func loadCredentials(baseURL string) (clientID, clientSecret string, err error) return clientID, clientSecret, nil } +// ClearCachedToken removes the cached REST bearer for the credentials currently +// selected for baseURL. It is best-effort so logout can still remove credentials +// if their cache path cannot be resolved. +func ClearCachedToken(baseURL string) { + clientID, clientSecret, err := loadCredentials(baseURL) + if err != nil { + return + } + tokensource.InvalidateCachedToken(baseURL, clientID, clientSecret) +} + // isTokenError reports whether err is (or wraps) a rejected client_credentials // grant. It powers two things: telling transport.Do to fail fast on it rather // than burn the retry budget on credentials that won't fix themselves, and @@ -172,14 +183,22 @@ func New(ctx context.Context, baseURL string, opts ...Option) (*Client, error) { return nil, err } - tokenSource, err := tokensource.NewTokenSource(ctx, clientID, clientSecret, baseURL, transportOpts(opts)...) + tokenSource, err := tokensource.NewCachingTokenSource(ctx, clientID, clientSecret, baseURL, transportOpts(opts)...) if err != nil { return nil, &AuthError{fmt.Errorf("creating token source: %w", err)} } - oauthClient := oauth2.NewClient(ctx, tokenSource) + // oauth2.NewClient wraps its source in a second ReuseTokenSource. The + // cacheTokenSource already owns caching and needs Invalidate to take effect + // before a 401 retry, so compose the transport directly. + var base http.RoundTripper = &oauth2.Transport{Source: tokenSource} + // A cached token can be locally-unexpired yet server-rejected; recover by + // dropping it and re-minting once. Only when the source caches. + if inv, ok := tokenSource.(tokensource.Invalidator); ok { + base = &retryOnTokenReject{base: base, invalidate: inv.Invalidate} + } cfg := resolve(opts) - t := transport.New(oauthClient.Transport, + t := transport.New(base, transport.WithMaxRetries(cfg.maxRetries), transport.WithDebug(cfg.debug), transport.WithNonRetryable(isTokenError), @@ -187,6 +206,49 @@ func New(ctx context.Context, baseURL string, opts ...Option) (*Client, error) { return &Client{t: t, baseURL: baseURL}, nil } +// retryOnTokenReject recovers from a cached access token the server refuses. +// On the first 401 it invalidates the cache and retries once with a freshly +// minted token; a second 401 is a real auth failure and is returned. A 401 is +// rejected before the request is processed, so retrying a mutation is safe. It +// retries only when the body can be rewound, and never on 403 (authenticated +// but forbidden -- a new token of the same identity would not help). +type retryOnTokenReject struct { + base http.RoundTripper + invalidate func() +} + +func (r *retryOnTokenReject) RoundTrip(req *http.Request) (*http.Response, error) { + resp, err := r.base.RoundTrip(req) + if err != nil || resp.StatusCode != http.StatusUnauthorized { + return resp, err + } + req2, ok := rewind(req) + if !ok { + return resp, nil + } + _ = resp.Body.Close() + r.invalidate() + return r.base.RoundTrip(req2) +} + +// rewind clones req with a fresh copy of its body for a retry, reporting false +// when the body exists but cannot be replayed. +func rewind(req *http.Request) (*http.Request, bool) { + clone := req.Clone(req.Context()) + if req.Body == nil || req.Body == http.NoBody { + return clone, true + } + if req.GetBody == nil { + return nil, false + } + body, err := req.GetBody() + if err != nil { + return nil, false + } + clone.Body = body + return clone, true +} + // NewForTesting returns a *Client that sends every request through hc's // transport to baseURL, bypassing loadCredentials and the OAuth mint New // performs. It exists so a test in another package (e.g. cmd's) can drive a diff --git a/internal/client/retry_token_test.go b/internal/client/retry_token_test.go new file mode 100644 index 0000000..843d0c2 --- /dev/null +++ b/internal/client/retry_token_test.go @@ -0,0 +1,179 @@ +package client + +import ( + "io" + "net/http" + "strings" + "testing" + + "golang.org/x/oauth2" +) + +// seqRT returns a programmed status code per call and records each request's +// body and Authorization header, so a test can assert both the retry decision +// and that a rewound body (and a fresh bearer) was resent intact. +type seqRT struct { + codes []int + calls int + bodies []string + auths []string +} + +func (s *seqRT) RoundTrip(req *http.Request) (*http.Response, error) { + body := "" + if req.Body != nil { + b, _ := io.ReadAll(req.Body) + _ = req.Body.Close() + body = string(b) + } + s.bodies = append(s.bodies, body) + s.auths = append(s.auths, req.Header.Get("Authorization")) + code := s.codes[s.calls] + s.calls++ + return &http.Response{StatusCode: code, Body: http.NoBody, Header: make(http.Header)}, nil +} + +func newReq(t *testing.T, body string) *http.Request { + t.Helper() + var r *http.Request + var err error + if body == "" { + r, err = http.NewRequest(http.MethodGet, "https://x/y", nil) + } else { + r, err = http.NewRequest(http.MethodPost, "https://x/y", strings.NewReader(body)) + } + if err != nil { + t.Fatal(err) + } + return r +} + +func TestRetryOnTokenReject_401ThenSuccess(t *testing.T) { + base := &seqRT{codes: []int{401, 200}} + inv := 0 + rt := &retryOnTokenReject{base: base, invalidate: func() { inv++ }} + resp, err := rt.RoundTrip(newReq(t, "")) + if err != nil { + t.Fatal(err) + } + if resp.StatusCode != 200 { + t.Errorf("status = %d, want 200 after re-mint", resp.StatusCode) + } + if inv != 1 { + t.Errorf("invalidate calls = %d, want 1", inv) + } + if base.calls != 2 { + t.Errorf("base calls = %d, want 2", base.calls) + } +} + +func TestRetryOnTokenReject_SecondRejectIsReturned(t *testing.T) { + base := &seqRT{codes: []int{401, 401}} + inv := 0 + rt := &retryOnTokenReject{base: base, invalidate: func() { inv++ }} + resp, err := rt.RoundTrip(newReq(t, "")) + if err != nil { + t.Fatal(err) + } + if resp.StatusCode != 401 { + t.Errorf("status = %d, want the second 401 returned", resp.StatusCode) + } + if inv != 1 || base.calls != 2 { + t.Errorf("invalidate=%d base=%d, want exactly one retry (1 and 2)", inv, base.calls) + } +} + +func TestRetryOnTokenReject_NoRetryOnOtherStatuses(t *testing.T) { + for _, code := range []int{200, 403, 404, 500} { + base := &seqRT{codes: []int{code}} + inv := 0 + rt := &retryOnTokenReject{base: base, invalidate: func() { inv++ }} + resp, err := rt.RoundTrip(newReq(t, "")) + if err != nil { + t.Fatal(err) + } + if resp.StatusCode != code || inv != 0 || base.calls != 1 { + t.Errorf("code %d: got status=%d inv=%d calls=%d, want pass-through (no invalidate, one call)", code, resp.StatusCode, inv, base.calls) + } + } +} + +func TestRetryOnTokenReject_ReplaysBody(t *testing.T) { + base := &seqRT{codes: []int{401, 200}} + rt := &retryOnTokenReject{base: base, invalidate: func() {}} + if _, err := rt.RoundTrip(newReq(t, `{"k":"v"}`)); err != nil { + t.Fatal(err) + } + if len(base.bodies) != 2 { + t.Fatalf("captured %d bodies, want 2", len(base.bodies)) + } + if base.bodies[0] != `{"k":"v"}` || base.bodies[1] != `{"k":"v"}` { + t.Errorf("bodies = %q, want the same JSON resent on retry", base.bodies) + } +} + +// rotatingTokenSource models cacheTokenSource after an invalidation. The retry +// must invoke it again, not let an outer oauth2.ReuseTokenSource resend stale. +type rotatingTokenSource struct { + invalidated bool +} + +func (s *rotatingTokenSource) Token() (*oauth2.Token, error) { + token := "stale" + if s.invalidated { + token = "fresh" + } + return &oauth2.Token{AccessToken: token, TokenType: "Bearer"}, nil +} + +// TestRetryOnTokenReject_ThroughOAuth2Transport drives the real oauth2.Transport +// beneath retryOnTokenReject (the exact composition client.New builds), proving +// a replayed POST gets a freshly attached bearer after cache invalidation. +func TestRetryOnTokenReject_ThroughOAuth2Transport(t *testing.T) { + base := &seqRT{codes: []int{401, 200}} + source := &rotatingTokenSource{} + oauthT := &oauth2.Transport{Source: source, Base: base} + rt := &retryOnTokenReject{ + base: oauthT, + invalidate: func() { + source.invalidated = true + }, + } + req, err := http.NewRequest(http.MethodPost, "https://x/y", strings.NewReader(`{"k":"v"}`)) + if err != nil { + t.Fatal(err) + } + resp, err := rt.RoundTrip(req) + if err != nil { + t.Fatal(err) + } + if resp.StatusCode != 200 { + t.Fatalf("status=%d, want 200", resp.StatusCode) + } + if len(base.bodies) != 2 || base.bodies[0] != `{"k":"v"}` || base.bodies[1] != `{"k":"v"}` { + t.Errorf("bodies=%q, want the JSON resent on retry", base.bodies) + } + if len(base.auths) != 2 || base.auths[0] != "Bearer stale" || base.auths[1] != "Bearer fresh" { + t.Errorf("auth headers=%q, want stale then fresh bearer", base.auths) + } +} + +// unrewindableReq has a body but no GetBody, so it cannot be replayed. +func TestRetryOnTokenReject_NoRetryWhenBodyUnrewindable(t *testing.T) { + base := &seqRT{codes: []int{401, 200}} + inv := 0 + rt := &retryOnTokenReject{base: base, invalidate: func() { inv++ }} + req, err := http.NewRequest(http.MethodPost, "https://x/y", nil) + if err != nil { + t.Fatal(err) + } + req.Body = io.NopCloser(strings.NewReader("payload")) + req.GetBody = nil + resp, err := rt.RoundTrip(req) + if err != nil { + t.Fatal(err) + } + if resp.StatusCode != 401 || inv != 0 || base.calls != 1 { + t.Errorf("got status=%d inv=%d calls=%d, want the first 401 returned with no retry", resp.StatusCode, inv, base.calls) + } +} diff --git a/internal/keychain/keychain.go b/internal/keychain/keychain.go index 7bc05d6..523b393 100644 --- a/internal/keychain/keychain.go +++ b/internal/keychain/keychain.go @@ -127,6 +127,35 @@ func FilePath(service string) (string, error) { return filePath(service) } +// GetSecret reads a non-credential secret from the OS keyring. Internal +// packages use it for short-lived secrets that share the same keychain policy +// as C1 credentials. +func GetSecret(service, account string) (string, error) { + return keyring.Get(service, account) +} + +// SetSecret stores a non-credential secret in the OS keyring. +func SetSecret(service, account, value string) error { + return keyring.Set(service, account, value) +} + +// DeleteSecret removes a non-credential secret from the OS keyring. +func DeleteSecret(service, account string) error { + return keyring.Delete(service, account) +} + +// IsUnavailable reports whether an OS-keyring error warrants falling back to +// the secured file store (for example, a headless Linux host without Secret +// Service). Other keyring failures should not silently downgrade storage. +func IsUnavailable(err error) bool { + return isKeyringUnavailable(err) +} + +// IsNotFound reports whether an OS-keyring lookup found no entry. +func IsNotFound(err error) bool { + return errors.Is(err, keyring.ErrNotFound) +} + func storeKeyring(service, clientID, clientSecret string) error { if err := keyring.Set(service, acctClientID, clientID); err != nil { return err @@ -211,12 +240,24 @@ func storeFile(service, clientID, clientSecret string) error { if err != nil { return err } - tmp := p + ".tmp" - if err := os.WriteFile(tmp, b, 0o600); err != nil { + tmp, err := os.CreateTemp(filepath.Dir(p), "."+filepath.Base(p)+".tmp-*") + if err != nil { + return fmt.Errorf("creating credential temp file: %w", err) + } + tmpName := tmp.Name() + defer func() { _ = os.Remove(tmpName) }() + if err := tmp.Chmod(0o600); err != nil { + _ = tmp.Close() + return fmt.Errorf("setting credential temp file permissions: %w", err) + } + if _, err := tmp.Write(b); err != nil { + _ = tmp.Close() return fmt.Errorf("writing credentials: %w", err) } - if err := os.Rename(tmp, p); err != nil { - _ = os.Remove(tmp) + if err := tmp.Close(); err != nil { + return fmt.Errorf("closing credential temp file: %w", err) + } + if err := os.Rename(tmpName, p); err != nil { return fmt.Errorf("finalizing credentials: %w", err) } return nil diff --git a/internal/keychain/keychain_test.go b/internal/keychain/keychain_test.go index 9a350ac..469bfd5 100644 --- a/internal/keychain/keychain_test.go +++ b/internal/keychain/keychain_test.go @@ -67,6 +67,26 @@ func TestStoreLoadKeyringHappyPath(t *testing.T) { } } +func TestGenericSecretHelpersUseKeyring(t *testing.T) { + keyring.MockInit() + if err := SetSecret("c1i/token-cache-test", "account", "value"); err != nil { + t.Fatalf("SetSecret: %v", err) + } + got, err := GetSecret("c1i/token-cache-test", "account") + if err != nil { + t.Fatalf("GetSecret: %v", err) + } + if got != "value" { + t.Fatalf("GetSecret = %q, want %q", got, "value") + } + if err := DeleteSecret("c1i/token-cache-test", "account"); err != nil { + t.Fatalf("DeleteSecret: %v", err) + } + if _, err := GetSecret("c1i/token-cache-test", "account"); !errors.Is(err, keyring.ErrNotFound) { + t.Fatalf("GetSecret after DeleteSecret error = %v, want ErrNotFound", err) + } +} + func TestEnvOverridesKeyring(t *testing.T) { keyring.MockInit() withTempConfigDir(t) @@ -146,6 +166,39 @@ func TestFileFallbackWhenKeyringUnavailable(t *testing.T) { } } +func TestStoreFileDoesNotFollowPredictableTempSymlink(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink privileges are not portable on Windows") + } + dir := withTempConfigDir(t) + clearEnv(t) + p, err := filePath(testService) + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Dir(p), 0o700); err != nil { + t.Fatal(err) + } + target := filepath.Join(dir, "target") + if err := os.WriteFile(target, []byte("sentinel"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, p+".tmp"); err != nil { + t.Skipf("creating symlink: %v", err) + } + + if err := storeFile(testService, testID, testSecret); err != nil { + t.Fatalf("storeFile: %v", err) + } + got, err := os.ReadFile(target) + if err != nil { + t.Fatal(err) + } + if string(got) != "sentinel" { + t.Fatalf("attacker target was changed to %q", got) + } +} + // configRoot returns the subdirectory of the test root that os.UserConfigDir // resolves to on the current platform. func configRoot(testDir string) string { diff --git a/internal/tokensource/cache.go b/internal/tokensource/cache.go new file mode 100644 index 0000000..08665f4 --- /dev/null +++ b/internal/tokensource/cache.go @@ -0,0 +1,214 @@ +package tokensource + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "time" + + "github.com/ConductorOne/c1i/internal/keychain" + "golang.org/x/oauth2" +) + +// Every c1i process is fresh, so without an on-disk cache each one re-mints an +// access token before its first request. The cost that matters is not latency +// (~25% of a short call) but one client_credentials event in the customer's +// audit log per invocation; agent workloads are long sequences of one-shot +// processes, so a cache collapses a burst to a single mint. +// +// The token sits beside the client secret it was minted from, same 0600/0700 +// perms. It is strictly shorter-lived than that secret and grants nothing the +// secret could not re-mint on demand, so it widens no exposure. Opt out with +// C1I_NO_TOKEN_CACHE=1. +const ( + // expirySkew keeps a token that is about to expire from being handed to a + // request that would outlive it. + expirySkew = 60 * time.Second + + // maxCachedTokenBytes bounds corrupt cache input. OAuth access tokens are + // compact credentials, so 64 KiB leaves substantial room without letting a + // hostile or accidental cache file exhaust the CLI's memory. + maxCachedTokenBytes = 64 << 10 + + noCacheEnv = "C1I_NO_TOKEN_CACHE" +) + +const tokenKeychainService = "com.conductorone.c1i.tokens" + +var ( + tokenKeyringGet = keychain.GetSecret + tokenKeyringSet = keychain.SetSecret + tokenKeyringDelete = keychain.DeleteSecret +) + +type cachedToken struct { + AccessToken string `json:"access_token"` + TokenType string `json:"token_type"` + Expiry time.Time `json:"expiry"` +} + +// cacheKey identifies a token by the host, client id, and credential generation +// it was minted from. Replacing a secret for the same client id must not reuse +// a bearer minted by the old secret. The hash keeps all three out of filenames. +func cacheKey(tokenHost, clientID, clientSecret string) string { + sum := sha256.Sum256([]byte(tokenHost + "\x00" + clientID + "\x00" + clientSecret)) + return hex.EncodeToString(sum[:]) +} + +func cachePath(key string) (string, error) { + dir, err := os.UserConfigDir() + if err != nil { + return "", fmt.Errorf("locating config dir: %w", err) + } + return filepath.Join(dir, "c1i", "tokens", key+".json"), nil +} + +// trustedCacheDir is deliberately stricter than the config root: the token +// cache holds a live bearer, so neither it nor its c1i parent may be a symlink +// or accessible to group/other users. The owning user is the trust boundary. +func trustedCacheDir(dir string) bool { + info, err := os.Lstat(dir) + return err == nil && info.IsDir() && info.Mode()&os.ModeSymlink == 0 && info.Mode().Perm()&0o077 == 0 +} + +// loadCachedToken returns a still-valid token, or nil. Every failure is a cache +// miss, never an error: a corrupt or unreadable cache must degrade to minting, +// not break the command. The OS keychain is the primary store; the hardened +// file cache exists for headless environments without a usable keychain. +func loadCachedToken(key string) *oauth2.Token { + if os.Getenv(noCacheEnv) != "" { + return nil + } + if encoded, err := tokenKeyringGet(tokenKeychainService, key); err == nil { + return decodeCachedToken([]byte(encoded)) + } else if !keychain.IsUnavailable(err) && !keychain.IsNotFound(err) { + return nil + } + return loadFileCachedToken(key) +} + +func decodeCachedToken(b []byte) *oauth2.Token { + var c cachedToken + if err := json.Unmarshal(b, &c); err != nil { + return nil + } + t := &oauth2.Token{AccessToken: c.AccessToken, TokenType: c.TokenType, Expiry: c.Expiry} + if !tokenFresh(t) { + return nil + } + return t +} + +func loadFileCachedToken(key string) *oauth2.Token { + p, err := cachePath(key) + if err != nil { + return nil + } + if !trustedCacheDir(filepath.Dir(filepath.Dir(p))) || !trustedCacheDir(filepath.Dir(p)) { + return nil + } + info, err := os.Lstat(p) + if err != nil || !info.Mode().IsRegular() || info.Mode().Perm()&0o077 != 0 { + return nil + } + f, err := os.Open(p) // #nosec G304 -- p contains only a locally-derived cache key + if err != nil { + return nil + } + defer func() { _ = f.Close() }() + b, err := io.ReadAll(io.LimitReader(f, maxCachedTokenBytes+1)) + if err != nil || len(b) > maxCachedTokenBytes { + return nil + } + return decodeCachedToken(b) +} + +// tokenFresh reports whether t is usable with expirySkew of headroom. Both the +// disk load and cacheTokenSource's in-memory reuse gate on it so the same skew +// applies to every tier -- oauth2.Token.Valid() uses only its own ~10s buffer. +func tokenFresh(t *oauth2.Token) bool { + return t != nil && t.AccessToken != "" && time.Until(t.Expiry) > expirySkew +} + +// storeCachedToken persists a freshly minted token. Errors are deliberately +// swallowed: failing to cache must never fail the request that just succeeded. +func storeCachedToken(key string, t *oauth2.Token) { + if os.Getenv(noCacheEnv) != "" || t == nil || t.AccessToken == "" { + return + } + b, err := json.Marshal(cachedToken{AccessToken: t.AccessToken, TokenType: t.TokenType, Expiry: t.Expiry}) // #nosec G117 -- serializing for the cache, not logging or a response + if err != nil { + return + } + if err := tokenKeyringSet(tokenKeychainService, key, string(b)); err == nil { + invalidateFileCachedToken(key) + return + } else if !keychain.IsUnavailable(err) { + return + } + storeFileCachedToken(key, b) +} + +func storeFileCachedToken(key string, b []byte) { + p, err := cachePath(key) + if err != nil { + return + } + dir := filepath.Dir(p) + parent := filepath.Dir(dir) + if err := os.MkdirAll(parent, 0o700); err != nil || !trustedCacheDir(parent) { + return + } + if err := os.Mkdir(dir, 0o700); err != nil && !os.IsExist(err) { + return + } + if !trustedCacheDir(dir) { + return + } + tmp, err := os.CreateTemp(dir, "."+filepath.Base(p)+".tmp-*") // #nosec G304 -- dir is checked local cache storage + if err != nil { + return + } + tmpName := tmp.Name() + defer func() { _ = os.Remove(tmpName) }() + if err := tmp.Chmod(0o600); err != nil { + _ = tmp.Close() + return + } + if _, err := tmp.Write(b); err != nil { + _ = tmp.Close() + return + } + if err := tmp.Close(); err != nil { + return + } + _ = os.Rename(tmpName, p) +} + +func invalidateFileCachedToken(key string) { + p, err := cachePath(key) + if err != nil { + return + } + _ = os.Remove(p) +} + +// invalidateCachedToken drops the cached token for these credentials. Callers +// use it when the API rejects a token the cache believed was still valid -- +// a revoked credential or a clock far enough out to defeat expirySkew. +func invalidateCachedToken(key string) { + _ = tokenKeyringDelete(tokenKeychainService, key) + invalidateFileCachedToken(key) +} + +// InvalidateCachedToken drops the cache entry for one credential generation. +// It is intentionally best-effort: cache cleanup must not make logout fail. +func InvalidateCachedToken(tokenHost, clientID, clientSecret string) { + host := strings.TrimPrefix(tokenHost, "https://") + invalidateCachedToken(cacheKey(host, clientID, clientSecret)) +} diff --git a/internal/tokensource/cache_test.go b/internal/tokensource/cache_test.go new file mode 100644 index 0000000..87e3c23 --- /dev/null +++ b/internal/tokensource/cache_test.go @@ -0,0 +1,446 @@ +package tokensource + +import ( + "context" + "errors" + "github.com/zalando/go-keyring" + "golang.org/x/oauth2" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" +) + +// useTempConfig redirects os.UserConfigDir at a temp dir so cache files never +// touch the real config directory. Skips where the redirect does not take +func useTempConfig(t *testing.T) string { + t.Helper() + dir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", dir) + t.Setenv("HOME", dir) + oldGet, oldSet, oldDelete := tokenKeyringGet, tokenKeyringSet, tokenKeyringDelete + keyringUnavailable := keyring.ErrUnsupportedPlatform + tokenKeyringGet = func(string, string) (string, error) { return "", keyringUnavailable } + tokenKeyringSet = func(string, string, string) error { return keyringUnavailable } + tokenKeyringDelete = func(string, string) error { return keyringUnavailable } + t.Cleanup(func() { + tokenKeyringGet, tokenKeyringSet, tokenKeyringDelete = oldGet, oldSet, oldDelete + }) + p, err := cachePath(cacheKey("h", "c", "test-secret")) + if err != nil || !strings.HasPrefix(p, dir) { + t.Skipf("os.UserConfigDir not redirected under temp on this platform (path=%q)", p) + } + return dir +} + +func freshToken(d time.Duration) *oauth2.Token { + return &oauth2.Token{AccessToken: "tok", TokenType: "Bearer", Expiry: time.Now().Add(d)} +} + +func testCacheKey(host, clientID string) string { + return cacheKey(host, clientID, "test-secret") +} + +func TestCacheKey_StableAndSeparates(t *testing.T) { + a := cacheKey("host", "client", "secret") + if a != cacheKey("host", "client", "secret") { + t.Fatal("cacheKey not stable for identical inputs") + } + if len(a) != 64 { + t.Errorf("key length = %d, want 64 hex chars", len(a)) + } + // Host, client id, and credential generation must each separate the namespace. + if cacheKey("host2", "client", "secret") == a { + t.Error("cacheKey ignored host") + } + if cacheKey("host", "client2", "secret") == a { + t.Error("cacheKey ignored client id") + } + if cacheKey("host", "client", "secret2") == a { + t.Error("cacheKey ignored client secret") + } + // The NUL joiner must stop ("ab","c") colliding with ("a","bc"). + if cacheKey("ab", "c", "secret") == cacheKey("a", "bc", "secret") { + t.Error("cacheKey joiner failed: (ab,c) collided with (a,bc)") + } +} + +func TestStoreLoadRoundTripAndPerms(t *testing.T) { + useTempConfig(t) + storeCachedToken(testCacheKey("host", "client"), freshToken(30*time.Minute)) + + got := loadCachedToken(testCacheKey("host", "client")) + if got == nil || got.AccessToken != "tok" || got.TokenType != "Bearer" { + t.Fatalf("load after store = %+v, want the stored token", got) + } + + p, _ := cachePath(testCacheKey("host", "client")) + fi, err := os.Stat(p) + if err != nil { + t.Fatalf("stat cache file: %v", err) + } + if fi.Mode().Perm() != 0o600 { + t.Errorf("cache file perm = %o, want 600", fi.Mode().Perm()) + } + di, err := os.Stat(filepath.Dir(p)) + if err != nil { + t.Fatalf("stat cache dir: %v", err) + } + if di.Mode().Perm() != 0o700 { + t.Errorf("cache dir perm = %o, want 700", di.Mode().Perm()) + } + // No temp file left beside the final one. + entries, _ := os.ReadDir(filepath.Dir(p)) + for _, e := range entries { + if strings.Contains(e.Name(), ".tmp") { + t.Errorf("leftover temp file: %s", e.Name()) + } + } +} + +func TestKeychainCachePreferredOverFile(t *testing.T) { + useTempConfig(t) + entries := map[string]string{} + tokenKeyringGet = func(service, key string) (string, error) { + value, ok := entries[service+"\x00"+key] + if !ok { + return "", errors.New("unexpected keychain miss") + } + return value, nil + } + tokenKeyringSet = func(service, key, value string) error { + entries[service+"\x00"+key] = value + return nil + } + tokenKeyringDelete = func(service, key string) error { + delete(entries, service+"\x00"+key) + return nil + } + + key := testCacheKey("host", "client") + storeCachedToken(key, freshToken(30*time.Minute)) + if len(entries) != 1 { + t.Fatalf("keychain entries = %d, want 1", len(entries)) + } + p, _ := cachePath(key) + if _, err := os.Stat(p); !errors.Is(err, os.ErrNotExist) { + t.Errorf("file cache exists despite keychain success: %v", err) + } + if got := loadCachedToken(key); got == nil || got.AccessToken != "tok" { + t.Fatalf("keychain load = %+v, want cached token", got) + } + invalidateCachedToken(key) + if len(entries) != 0 { + t.Error("invalidation did not remove keychain entry") + } +} + +func TestKeychainMissFallsBackToFileCache(t *testing.T) { + useTempConfig(t) + key := testCacheKey("host", "client") + storeCachedToken(key, freshToken(30*time.Minute)) + tokenKeyringGet = func(string, string) (string, error) { + return "", keyring.ErrNotFound + } + + got := loadCachedToken(key) + if got == nil || got.AccessToken != "tok" { + t.Fatalf("load after keychain miss = %+v, want token from file cache", got) + } +} + +func TestLoadMisses(t *testing.T) { + useTempConfig(t) + if loadCachedToken(testCacheKey("host", "client")) != nil { + t.Error("expected miss when no file exists") + } + + p, _ := cachePath(testCacheKey("host", "client")) + if err := os.MkdirAll(filepath.Dir(p), 0o700); err != nil { + t.Fatal(err) + } + cases := map[string]string{ + "corrupt json": "not json {{{", + "empty access": `{"access_token":"","token_type":"Bearer","expiry":"2999-01-01T00:00:00Z"}`, + "expired": `{"access_token":"t","token_type":"Bearer","expiry":"2000-01-01T00:00:00Z"}`, + "within skew (30s)": `{"access_token":"t","token_type":"Bearer","expiry":"` + time.Now().Add(30*time.Second).UTC().Format(time.RFC3339) + `"}`, + } + for name, body := range cases { + if err := os.WriteFile(p, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + if got := loadCachedToken(testCacheKey("host", "client")); got != nil { + t.Errorf("%s: load = %+v, want nil (miss)", name, got) + } + } +} + +func TestStoreNoOps(t *testing.T) { + dir := useTempConfig(t) + storeCachedToken(testCacheKey("host", "client"), nil) + storeCachedToken(testCacheKey("host", "client"), &oauth2.Token{AccessToken: ""}) + if entries, _ := os.ReadDir(filepath.Join(dir, "c1i", "tokens")); len(entries) != 0 { + t.Errorf("nil/empty token should write nothing, found %d files", len(entries)) + } +} + +func TestNoCacheEnvDisablesDisk(t *testing.T) { + useTempConfig(t) + t.Setenv(noCacheEnv, "1") + storeCachedToken(testCacheKey("host", "client"), freshToken(30*time.Minute)) + p, _ := cachePath(testCacheKey("host", "client")) + if _, err := os.Stat(p); !errors.Is(err, os.ErrNotExist) { + t.Error("store must write nothing when C1I_NO_TOKEN_CACHE is set") + } + // And a pre-existing file is ignored on load. + if err := os.MkdirAll(filepath.Dir(p), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(`{"access_token":"t","token_type":"Bearer","expiry":"2999-01-01T00:00:00Z"}`), 0o600); err != nil { + t.Fatal(err) + } + if loadCachedToken(testCacheKey("host", "client")) != nil { + t.Error("load must ignore the cache when C1I_NO_TOKEN_CACHE is set") + } +} + +func TestInvalidateRemovesFile(t *testing.T) { + useTempConfig(t) + storeCachedToken(testCacheKey("host", "client"), freshToken(30*time.Minute)) + invalidateCachedToken(testCacheKey("host", "client")) + if loadCachedToken(testCacheKey("host", "client")) != nil { + t.Error("token still present after Invalidate") + } + invalidateCachedToken(testCacheKey("host", "client")) // absent: must not panic or error +} + +// countingMint records how many times a token was minted, standing in for the +// real client_credentials exchange. +type countingMint struct { + n int + tok *oauth2.Token + err error +} + +func (m *countingMint) Token() (*oauth2.Token, error) { + m.n++ + if m.err != nil { + return nil, m.err + } + return m.tok, nil +} + +func TestCacheTokenSource_InMemoryReuse(t *testing.T) { + useTempConfig(t) + // Disable the disk cache so in-memory reuse is the ONLY thing that can hold + // the mint count at 1 -- otherwise a disk hit would mask a broken in-memory + // path. + t.Setenv(noCacheEnv, "1") + m := &countingMint{tok: freshToken(30 * time.Minute)} + src := &cacheTokenSource{mint: m, key: testCacheKey("h", "c")} + for i := 0; i < 5; i++ { + if _, err := src.Token(); err != nil { + t.Fatal(err) + } + } + if m.n != 1 { + t.Errorf("mints = %d, want 1 (in-memory reuse within a process)", m.n) + } +} + +func TestCacheTokenSource_NearExpiryNotReused(t *testing.T) { + useTempConfig(t) + // A token with less than expirySkew left must not be reused from any tier; + // each call re-mints. Guards against reuse gating on oauth2's ~10s buffer + // instead of the package's 60s skew. + m := &countingMint{tok: freshToken(30 * time.Second)} + src := &cacheTokenSource{mint: m, key: testCacheKey("h", "c")} + if _, err := src.Token(); err != nil { + t.Fatal(err) + } + if _, err := src.Token(); err != nil { + t.Fatal(err) + } + if m.n != 2 { + t.Errorf("mints = %d, want 2 (a near-expiry token must not be reused)", m.n) + } +} + +func TestCacheTokenSource_DiskHitSkipsMint(t *testing.T) { + useTempConfig(t) + storeCachedToken(testCacheKey("h", "c"), freshToken(30*time.Minute)) + m := &countingMint{err: errors.New("mint must not be called on a disk hit")} + src := &cacheTokenSource{mint: m, key: testCacheKey("h", "c")} + if _, err := src.Token(); err != nil { + t.Fatalf("Token: %v", err) + } + if m.n != 0 { + t.Errorf("mints = %d, want 0 (served from disk)", m.n) + } +} + +func TestCacheTokenSource_MintOnMissWritesDisk(t *testing.T) { + useTempConfig(t) + m := &countingMint{tok: freshToken(30 * time.Minute)} + src := &cacheTokenSource{mint: m, key: testCacheKey("h", "c")} + if _, err := src.Token(); err != nil { + t.Fatal(err) + } + // A second, independent process (fresh source) must read the disk, not mint. + m2 := &countingMint{err: errors.New("must not mint; disk was written")} + src2 := &cacheTokenSource{mint: m2, key: testCacheKey("h", "c")} + if _, err := src2.Token(); err != nil { + t.Fatalf("second source Token: %v", err) + } + if m.n != 1 || m2.n != 0 { + t.Errorf("mints first=%d second=%d, want 1 and 0", m.n, m2.n) + } +} + +func TestCacheTokenSource_InvalidateForcesReMint(t *testing.T) { + useTempConfig(t) + m := &countingMint{tok: freshToken(30 * time.Minute)} + src := &cacheTokenSource{mint: m, key: testCacheKey("h", "c")} + if _, err := src.Token(); err != nil { + t.Fatal(err) + } + src.Invalidate() + p, _ := cachePath(testCacheKey("h", "c")) + if _, err := os.Stat(p); !errors.Is(err, os.ErrNotExist) { + t.Error("Invalidate must remove the on-disk token") + } + if _, err := src.Token(); err != nil { + t.Fatal(err) + } + if m.n != 2 { + t.Errorf("mints = %d, want 2 (re-mint after Invalidate)", m.n) + } +} + +func TestCachingSourceIsInvalidatorPlainIsNot(t *testing.T) { + caching, err := NewCachingTokenSource(context.Background(), "client1", validSecret(t), "example.test") + if err != nil { + t.Fatalf("NewCachingTokenSource: %v", err) + } + if _, ok := caching.(Invalidator); !ok { + t.Fatalf("caching source is %T, does not implement Invalidator; the client's self-heal wiring would silently disengage", caching) + } + // The plain source must NOT cache, so it must not be an Invalidator -- this + // is what keeps `auth token` handing out a fresh, unpersisted bearer. + plain, err := NewTokenSource(context.Background(), "client1", validSecret(t), "example.test") + + if err != nil { + t.Fatalf("NewTokenSource: %v", err) + } + if _, ok := plain.(Invalidator); ok { + t.Fatalf("plain source %T implements Invalidator; it must not cache", plain) + } +} +func TestLoadRejectsUnsafeOrOversizedCacheFile(t *testing.T) { + useTempConfig(t) + key := testCacheKey("host", "client") + p, _ := cachePath(key) + if err := os.MkdirAll(filepath.Dir(p), 0o700); err != nil { + t.Fatal(err) + } + valid := []byte(`{"access_token":"t","token_type":"Bearer","expiry":"2999-01-01T00:00:00Z"}`) + if err := os.WriteFile(p, valid, 0o644); err != nil { + t.Fatal(err) + } + if got := loadCachedToken(key); got != nil { + t.Errorf("world-readable cache loaded token %q", got.AccessToken) + } + if err := os.Remove(p); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(strings.Repeat("x", maxCachedTokenBytes+1)), 0o600); err != nil { + t.Fatal(err) + } + if got := loadCachedToken(key); got != nil { + t.Errorf("oversized cache loaded token %q", got.AccessToken) + } +} + +func TestStoreRejectsSymlinkedCacheDirectory(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink privileges are not portable on Windows") + } + dir := useTempConfig(t) + p, _ := cachePath(testCacheKey("host", "client")) + if err := os.MkdirAll(filepath.Dir(filepath.Dir(p)), 0o700); err != nil { + t.Fatal(err) + } + target := filepath.Join(dir, "untrusted") + if err := os.MkdirAll(target, 0o700); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, filepath.Dir(p)); err != nil { + t.Skipf("creating symlink: %v", err) + } + storeCachedToken(testCacheKey("host", "client"), freshToken(30*time.Minute)) + entries, err := os.ReadDir(target) + if err != nil { + t.Fatal(err) + } + if len(entries) != 0 { + t.Errorf("wrote %d token files through symlinked cache directory", len(entries)) + } +} + +func TestStoreRejectsInsecureCacheDirectory(t *testing.T) { + useTempConfig(t) + p, _ := cachePath(testCacheKey("host", "client")) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Chmod(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + storeCachedToken(testCacheKey("host", "client"), freshToken(30*time.Minute)) + if _, err := os.Stat(p); !errors.Is(err, os.ErrNotExist) { + t.Errorf("cache file exists under insecure directory: %v", err) + } +} + +func TestStoreRejectsSymlinkedCacheParent(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink privileges are not portable on Windows") + } + dir := useTempConfig(t) + p, _ := cachePath(testCacheKey("host", "client")) + parent := filepath.Dir(filepath.Dir(p)) + if err := os.MkdirAll(filepath.Dir(parent), 0o700); err != nil { + t.Fatal(err) + } + target := filepath.Join(dir, "untrusted-parent") + if err := os.MkdirAll(target, 0o700); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, parent); err != nil { + t.Skipf("creating symlink: %v", err) + } + storeCachedToken(testCacheKey("host", "client"), freshToken(30*time.Minute)) + entries, err := os.ReadDir(target) + if err != nil { + t.Fatal(err) + } + if len(entries) != 0 { + t.Errorf("wrote %d token files through symlinked cache parent", len(entries)) + } +} + +func TestCacheKeyChangeForcesMint(t *testing.T) { + useTempConfig(t) + storeCachedToken(cacheKey("h", "c", "old-secret"), freshToken(30*time.Minute)) + m := &countingMint{tok: &oauth2.Token{AccessToken: "fresh", TokenType: "Bearer", Expiry: time.Now().Add(30 * time.Minute)}} + src := &cacheTokenSource{mint: m, key: cacheKey("h", "c", "new-secret")} + got, err := src.Token() + if err != nil { + t.Fatal(err) + } + if got.AccessToken != "fresh" || m.n != 1 { + t.Errorf("token=%q mints=%d, want fresh token from exactly one mint", got.AccessToken, m.n) + } +} diff --git a/internal/tokensource/tokensource.go b/internal/tokensource/tokensource.go index acdd7b1..514fec5 100644 --- a/internal/tokensource/tokensource.go +++ b/internal/tokensource/tokensource.go @@ -12,6 +12,7 @@ import ( "net/http" "net/url" "strings" + "sync" "time" "github.com/ConductorOne/c1i/internal/transport" @@ -185,22 +186,85 @@ func (c *c1TokenSource) Token() (*oauth2.Token, error) { }, nil } -// NewTokenSource returns a TokenSource that mints via the client_credentials -// JWT-bearer grant. opts are forwarded to the transport it mints through -// (e.g. to share a caller's --debug/--max-retries with the token request), but -// the request timeout is always tokenRequestTimeout regardless of what opts -// contains. +// NewTokenSource returns a TokenSource that mints a fresh token on every call +// via the client_credentials JWT-bearer grant, without touching the on-disk +// cache. `auth token` and the MCP gateway use it: both hand the bearer onward, +// so it must be freshly minted rather than a possibly-near-expiry cached one. +// opts are forwarded to the transport it mints through (e.g. to share a +// caller's --debug/--max-retries with the token request), but the request +// timeout is always tokenRequestTimeout regardless of what opts contains. func NewTokenSource(ctx context.Context, clientID string, clientSecret string, tokenHost string, opts ...transport.Option) (oauth2.TokenSource, error) { - secret, err := parseSecret([]byte(clientSecret)) + return newMintSource(clientID, clientSecret, tokenHost, opts...) +} + +// NewCachingTokenSource wraps NewTokenSource with the cross-process on-disk +// cache. The REST client uses it so a run of one-shot commands does not mint -- +// and audit-log -- a token each time. It caches only the bearer c1i attaches +// automatically; a bearer handed to the caller (NewTokenSource) is never cached. +func NewCachingTokenSource(ctx context.Context, clientID string, clientSecret string, tokenHost string, opts ...transport.Option) (oauth2.TokenSource, error) { + mint, err := newMintSource(clientID, clientSecret, tokenHost, opts...) if err != nil { return nil, err } + host := strings.TrimPrefix(tokenHost, "https://") + return &cacheTokenSource{mint: mint, key: cacheKey(host, clientID, clientSecret)}, nil +} +func newMintSource(clientID string, clientSecret string, tokenHost string, opts ...transport.Option) (*c1TokenSource, error) { + secret, err := parseSecret([]byte(clientSecret)) + if err != nil { + return nil, err + } t := transport.New(nil, append(opts, transport.WithTimeout(tokenRequestTimeout))...) - return oauth2.ReuseTokenSource(nil, &c1TokenSource{ + return &c1TokenSource{ clientID: clientID, clientSecret: secret, tokenHost: strings.TrimPrefix(tokenHost, "https://"), transport: t, - }), nil + }, nil +} + +// Invalidator is implemented by a caching TokenSource: Invalidate drops the +// cached token (in memory and on disk) so the next Token() re-mints. The REST +// client asserts for it to recover from a cached token the server has begun +// rejecting (clock skew past expirySkew, or a server-side revocation), which +// would otherwise 401 every invocation until the token's local expiry. +type Invalidator interface{ Invalidate() } + +// cacheTokenSource serves a token from three tiers, cheapest first: an in-memory +// token reused for the life of this process, the on-disk cache shared across +// processes, then a fresh mint written back to disk. Cross-process reuse is the +// point: c1i workloads are long sequences of one-shot processes. Concurrency-safe. +type cacheTokenSource struct { + mu sync.Mutex + tok *oauth2.Token + mint oauth2.TokenSource + key string +} + +func (c *cacheTokenSource) Token() (*oauth2.Token, error) { + c.mu.Lock() + defer c.mu.Unlock() + if tokenFresh(c.tok) { + return c.tok, nil + } + if t := loadCachedToken(c.key); t != nil { + c.tok = t + return t, nil + } + t, err := c.mint.Token() + if err != nil { + return nil, err + } + storeCachedToken(c.key, t) + c.tok = t + return t, nil +} + +// Invalidate satisfies Invalidator: drop both the in-memory and on-disk copies. +func (c *cacheTokenSource) Invalidate() { + c.mu.Lock() + c.tok = nil + c.mu.Unlock() + invalidateCachedToken(c.key) }