From 6e689c94db1a61dd439245df7bbb8f9ee3b56b5a Mon Sep 17 00:00:00 2001 From: Mykyta Netipa Date: Wed, 4 Nov 2026 22:20:45 +0000 Subject: [PATCH] feat: keep OAuth tokens in the OS keychain A 0600 file protects tokens from other users but not from other programs running as the same user, backups or synced dotfiles. KeyringStore keeps the tokens in the macOS Keychain, Windows Credential Manager or the Linux Secret Service through zalando/go-keyring, one entry per token to stay within the size limits of the macOS and Windows stores, and everything else in keychain-grants.json, so grants can be listed without reading secrets and the file store never reads a keychain grant as one without tokens. --oauth-store selects the store: file (default) or keychain. Like --oauth-clients, it is not read from a discovered .env. Without a keychain, for example on a headless Linux host with no Secret Service, saving a grant fails and points to --oauth-store file instead of writing the tokens to disk. Refs #98 --- go.mod | 3 + go.sum | 14 +++ internal/README.md | 6 +- internal/cli/auth_login.go | 4 +- internal/cli/oauth.go | 13 ++- internal/cli/oauth_test.go | 96 +++++++++++++++++ internal/cli/root.go | 6 +- internal/oauth/keyringstore.go | 154 ++++++++++++++++++++++++++++ internal/oauth/keyringstore_test.go | 133 ++++++++++++++++++++++++ internal/oauth/store.go | 21 +++- 10 files changed, 439 insertions(+), 11 deletions(-) create mode 100644 internal/oauth/keyringstore.go create mode 100644 internal/oauth/keyringstore_test.go diff --git a/go.mod b/go.mod index 328bd62..2598c11 100644 --- a/go.mod +++ b/go.mod @@ -9,12 +9,15 @@ require ( github.com/joho/godotenv v1.5.1 github.com/spf13/cobra v1.10.2 github.com/spf13/pflag v1.0.10 + github.com/zalando/go-keyring v0.2.8 golang.org/x/sys v0.49.0 google.golang.org/grpc v1.84.0 gopkg.in/yaml.v3 v3.0.1 ) require ( + github.com/danieljoos/wincred v1.2.3 // indirect + github.com/godbus/dbus/v5 v5.2.2 // indirect github.com/google/uuid v1.6.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect golang.org/x/mod v0.42.0 // indirect diff --git a/go.sum b/go.sum index a0d3013..3ef8bdd 100644 --- a/go.sum +++ b/go.sum @@ -3,6 +3,12 @@ github.com/a2aproject/a2a-go v0.3.15/go.mod h1:I7Cm+a1oL+UT6zMoP+roaRE5vdfUa1iQG github.com/a2aproject/a2a-go/v2 v2.6.0 h1:QflB0gHxOP0o5quvA/h9EbYgfOsmRsSmFkeBYtKdwwk= github.com/a2aproject/a2a-go/v2 v2.6.0/go.mod h1:rvFIb0sBJl2qmJRYU2fQxI6hhIQksmGGMEhT/uRwWww= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= +github.com/danieljoos/wincred v1.2.3 h1:v7dZC2x32Ut3nEfRH+vhoZGvN72+dQ/snVXo/vMFLdQ= +github.com/danieljoos/wincred v1.2.3/go.mod h1:6qqX0WNrS4RzPZ1tnroDzq9kY3fu1KwE7MRLQK4X0bs= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/godbus/dbus/v5 v5.2.2 h1:TUR3TgtSVDmjiXOgAAyaZbYmIeP3DPkld3jgKGV8mXQ= +github.com/godbus/dbus/v5 v5.2.2/go.mod h1:3AAv2+hPq5rdnr5txxxRwiGjPXamgoIHgz9FPBfOp3c= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= @@ -13,12 +19,20 @@ github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2 github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0= github.com/joho/godotenv v1.5.1/go.mod h1:f4LDr5Voq0i2e/R5DDNOoa2zzDfwtkZa6DnEwAbqwq4= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU= github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4= github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= +github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +github.com/zalando/go-keyring v0.2.8 h1:6sD/Ucpl7jNq10rM2pgqTs0sZ9V3qMrqfIIy5YPccHs= +github.com/zalando/go-keyring v0.2.8/go.mod h1:tsMo+VpRq5NGyKfxoBVjCuMrG47yj8cmakZDO5QGii0= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= diff --git a/internal/README.md b/internal/README.md index 056eda5..f6cacc7 100644 --- a/internal/README.md +++ b/internal/README.md @@ -44,6 +44,7 @@ These apply to every client-mode command. Each command selects the agent it talk | `--oauth ` | | OAuth sign-in for agents that require it: `auto` (default), `cached`, or `off`. See [OAuth](#oauth). | | `--oauth-clients ` | | OAuth client registration per authorization server. Repeatable. Not read from the local `.env`. | | `--oauth-default-client-id ` | | OAuth client ID for authorization servers that support Client ID Metadata Documents when no registration matches; empty disables it. Not read from the local `.env`. | +| `--oauth-store ` | | Where OAuth grants are kept: `file` (default) or `keychain`. Not read from the local `.env`. | | `--tenant ` | | Tenant identifier. Passed on every request. | | `--timeout ` | | Request timeout. Default `30s`. | | `--verbose` | `-v` | Verbose output to stderr. | @@ -334,6 +335,7 @@ a2a send -a https://agent.example.com --oauth cached "from CI" # never prompt | `--oauth` | `auto` (default), `cached`, `off` | `auto` signs in when an agent requires it and stderr is a terminal; without a terminal it fails like `cached`. `cached` only uses stored grants and otherwise fails with `A2ACLI_ERR_CREDENTIALS_MISSING` (exit 4). `off` disables OAuth. | | `--oauth-clients` | JSON object or array | Client registrations, one per authorization server: `{"authority":"","client_id":"","scopes":[...]}` on the command line and in `A2ACLI_OAUTH_CLIENTS`; a list of such objects in config files. Not read from the local `.env`. See [Client registrations](#client-registrations). | | `--oauth-default-client-id` | Client ID or CIMD URL, empty to disable | Client ID used with servers that support Client ID Metadata Documents when no registration matches. Default `https://a2a-protocol.org/oauth/a2a-cli.json`. Not read from the local `.env`. | +| `--oauth-store` | `file` (default), `keychain` | `file` keeps grants, tokens included, in `~/.config/a2a-cli/oauth/grants.json` (mode 0600). `keychain` keeps the tokens in the macOS Keychain, Windows Credential Manager or Linux Secret Service, one entry per token, and the rest of each grant in `keychain-grants.json`; the two stores keep separate grants. Without a keychain, or when a token is too large for it, saving a grant fails rather than writing the tokens to disk. Not read from the local `.env`. | #### Where a2a learns how to sign in @@ -353,7 +355,7 @@ OAuth is enforced only when the agent card requires it. A card that also accepts Tokens are only sent over `https`, or `http` to a loopback host. Every request to the authorization server carries the agent URL as `resource` ([RFC 8707](https://www.rfc-editor.org/rfc/rfc8707)), so the token only works for that agent. These requests go through the same transport as agent requests, so `--unix-socket` and `--ca-cert` apply to them. Signing in has its own 5-minute limit; `--timeout` covers the agent requests, which start after it. If signing in fails or the agent still rejects the token, the command fails with `A2ACLI_ERR_AUTH_FAILED` (exit 4). -Grants are stored per issuer, client ID, agent URL and scopes in `~/.config/a2a-cli/oauth/grants.json` (mode 0600). A grant also serves agents below its URL, and one obtained with more scopes than an agent asks for is reused. a2a refreshes a token shortly before it expires, or once when the agent rejects it with `error="invalid_token"`, and retries the request once. +Grants are stored per issuer, client ID, agent URL and scopes in `~/.config/a2a-cli/oauth/grants.json` (mode 0600), or in the OS keychain with an index in `keychain-grants.json` under `--oauth-store keychain`. A grant also serves agents below its URL, and one obtained with more scopes than an agent asks for is reused. a2a refreshes a token shortly before it expires, or once when the agent rejects it with `error="invalid_token"`, and retries the request once. #### Client registrations @@ -389,7 +391,7 @@ a2a auth get https://agent.example.com --reveal a2a auth logout --all ``` -A script or a sandboxed agent can sign in once with `a2a auth login` and then run with `--oauth cached`. Both have to use the same `~/.config/a2a-cli`, so a sandbox with its own home directory doesn't see a grant stored outside it. +A script or a sandboxed agent can sign in once with `a2a auth login` and then run with `--oauth cached`. Both have to use the same `~/.config/a2a-cli` and `--oauth-store`, so a sandbox with its own home directory doesn't see a grant stored outside it. --- diff --git a/internal/cli/auth_login.go b/internal/cli/auth_login.go index e2cafc3..7a58f38 100644 --- a/internal/cli/auth_login.go +++ b/internal/cli/auth_login.go @@ -42,8 +42,8 @@ a2a learns where to sign in from the agent card's oauth2 or openIdConnect security scheme, or from the Bearer challenge of a 401 response and the agent's protected resource metadata. It prints a URL to open and waits for the redirect to 127.0.0.1; with --device it prints a URL and a code to enter on any -device. The grant is stored in ~/.config/a2a-cli/oauth/grants.json and -refreshed when it expires. +device. The grant is stored in ~/.config/a2a-cli/oauth/grants.json (or the OS +keychain with --oauth-store keychain) and refreshed when it expires. Other commands sign in the same way when an agent requires it (--oauth auto, the default) if a terminal is attached. --oauth cached only uses stored grants, diff --git a/internal/cli/oauth.go b/internal/cli/oauth.go index cec9c8f..04aa2d8 100644 --- a/internal/cli/oauth.go +++ b/internal/cli/oauth.go @@ -101,7 +101,18 @@ func (g *globalConfig) newOAuthStore(d deps) (oauth.Store, error) { return nil, err } } - return &oauth.FileStore{Dir: dir}, nil + switch g.oauthStore { + case "", "file": + return &oauth.FileStore{Dir: dir}, nil + case "keychain": + kr := d.keyring + if kr == nil { + kr = oauth.SystemKeyring{} + } + return &oauth.KeyringStore{Index: &oauth.FileStore{Dir: dir, Name: oauth.KeyringIndexName}, Keyring: kr}, nil + default: + return nil, clierr.Usage(fmt.Sprintf("invalid --oauth-store %q (want file or keychain)", g.oauthStore)) + } } // wrapSignIn makes every command marked with [agentCall] sign in when the agent diff --git a/internal/cli/oauth_test.go b/internal/cli/oauth_test.go index 0438375..3baf8e8 100644 --- a/internal/cli/oauth_test.go +++ b/internal/cli/oauth_test.go @@ -26,6 +26,7 @@ import ( "path/filepath" "slices" "strings" + "sync" "sync/atomic" "testing" "time" @@ -462,6 +463,101 @@ func TestAuthUsageErrors(t *testing.T) { } } +type memKeyring struct { + mu sync.Mutex + secrets map[string]string +} + +func (k *memKeyring) Get(service, user string) (string, error) { + k.mu.Lock() + defer k.mu.Unlock() + if s, ok := k.secrets[service+"/"+user]; ok { + return s, nil + } + return "", oauth.ErrSecretNotFound +} + +func (k *memKeyring) Set(service, user, secret string) error { + k.mu.Lock() + defer k.mu.Unlock() + if k.secrets == nil { + k.secrets = map[string]string{} + } + k.secrets[service+"/"+user] = secret + return nil +} + +func (k *memKeyring) Delete(service, user string) error { + k.mu.Lock() + defer k.mu.Unlock() + delete(k.secrets, service+"/"+user) + return nil +} + +func TestOAuthKeychainStore(t *testing.T) { + t.Parallel() + e := newOAuthEnv(t, oauthtest.ServerOptions{}, oauthtest.AgentOptions{Card: oauthtest.CardMetadataURL}) + kr := &memKeyring{} + e.deps.keyring = kr + args := []string{"send", "-a", e.agent.URL, e.clients(), "--oauth-store", "keychain", oauthTestMessage} + e.mustRun(t, args...) + + index, err := os.ReadFile(filepath.Join(e.deps.oauthDir, oauth.KeyringIndexName)) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(index), "access_token") || strings.Contains(string(index), "refresh_token") { + t.Fatalf("%s contains tokens: %s", oauth.KeyringIndexName, index) + } + if _, err := os.Stat(filepath.Join(e.deps.oauthDir, "grants.json")); !os.IsNotExist(err) { + t.Fatalf("grants.json exists with --oauth-store keychain (stat error %v)", err) + } + if len(kr.secrets) != 2 { + t.Fatalf("keychain entries = %d, want one per token", len(kr.secrets)) + } + if _, stderr := e.mustRun(t, args...); strings.Contains(stderr, "To authorize") { + t.Fatalf("second send stderr = %q, want the grant read from the keychain", stderr) + } + e.mustRun(t, "auth", "logout", "--all", "--oauth-store", "keychain") + if len(kr.secrets) != 0 { + t.Fatalf("keychain entries after logout = %d, want 0", len(kr.secrets)) + } +} + +func TestOAuthKeychainGrantWithoutTokens(t *testing.T) { + t.Parallel() + tests := []struct { + name string + mutate func(*memKeyring) + store []string + }{ + {name: "file store", store: nil}, + {name: "keychain entries removed", mutate: func(k *memKeyring) { k.secrets = nil }, store: []string{"--oauth-store", "keychain"}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + e := newOAuthEnv(t, oauthtest.ServerOptions{}, oauthtest.AgentOptions{Card: oauthtest.CardMetadataURL}) + kr := &memKeyring{} + e.deps.keyring = kr + e.mustRun(t, "auth", "login", e.agent.URL, e.clients(), "--oauth-store", "keychain") + if tc.mutate != nil { + tc.mutate(kr) + } + sent := len(e.agent.AuthorizationHeaders()) + e.deps.authInteractive = func() bool { return false } + args := append([]string{"send", "-a", e.agent.URL, e.clients(), oauthTestMessage}, tc.store...) + _, _, err := e.run(t, args...) + if got := errCode(err); got != clierr.CodeCredentialsMissing { + t.Fatalf("a2a send error = %v (code %s), want %s", err, got, clierr.CodeCredentialsMissing) + } + if got := e.agent.AuthorizationHeaders()[sent:]; len(got) != 0 { + t.Fatalf("the agent received %q, want no request without a usable token", got) + } + }) + } +} + var cmpIgnoreExpiry = cmp.FilterPath(func(p cmp.Path) bool { return p.Last().String() == ".ExpiresAt" }, cmp.Ignore()) // apiKeyOrOAuthAgent serves a card that accepts an API key or OAuth and counts diff --git a/internal/cli/root.go b/internal/cli/root.go index fb29e56..7b714c2 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -58,6 +58,8 @@ type deps struct { authTimeout time.Duration // authSleep, if set, replaces the wait between device flow polls. authSleep func(context.Context, time.Duration) error + // keyring replaces the OS keychain used by --oauth-store keychain. + keyring oauth.Keyring } func (d *deps) setDefaults() { @@ -104,6 +106,7 @@ type globalConfig struct { oauthMode string oauthClients oauth.ClientsFlag oauthDefaultClientID string + oauthStore string oauth *oauth.Manager // insecureWarning prints insecureCredentialWarning at most once. @@ -248,8 +251,9 @@ func newRootCmd(cfg *globalConfig, deps deps) (*cobra.Command, error) { pf.StringVar(&cfg.oauthMode, "oauth", oauthAuto, "OAuth sign-in for agents that require it: auto (sign in when needed), cached (use stored grants only), or off (see a2a auth login --help)") pf.Var(&cfg.oauthClients, "oauth-clients", `OAuth client registration as JSON: {"authority":"","client_id":"","scopes":[...]} (repeatable; a JSON array is also accepted)`) pf.StringVar(&cfg.oauthDefaultClientID, "oauth-default-client-id", oauth.DefaultClientID, "OAuth client ID used with authorization servers that support Client ID Metadata Documents when no oauth-clients entry matches; empty disables it") + pf.StringVar(&cfg.oauthStore, "oauth-store", "file", "Where OAuth grants are stored: file (~/.config/a2a-cli/oauth/grants.json) or keychain (tokens in the OS keychain)") pf.StringVar(&cfg.configPath, "config", "", "Load configuration from an explicit file (.yaml, .json, or .env) in place of the local .env") - for _, name := range []string{"unix-socket", "credential-helper", "ca-cert", "oauth-clients", "oauth-default-client-id"} { + for _, name := range []string{"unix-socket", "credential-helper", "ca-cert", "oauth-clients", "oauth-default-client-id", "oauth-store"} { if err := pf.SetAnnotation(name, clicfg.AnnotationNotFromDiscoveredFile, []string{"true"}); err != nil { return nil, err } diff --git a/internal/oauth/keyringstore.go b/internal/oauth/keyringstore.go new file mode 100644 index 0000000..b60f160 --- /dev/null +++ b/internal/oauth/keyringstore.go @@ -0,0 +1,154 @@ +// Copyright 2026 The A2A Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package oauth + +import ( + "context" + "errors" + "fmt" + + "github.com/zalando/go-keyring" +) + +// keyringService is the service name grants are stored under in the OS keychain. +const keyringService = "a2a-cli-oauth" + +// Keyring stores secrets in the OS keychain. +type Keyring interface { + // Get returns the secret for user, or ErrSecretNotFound. + Get(service, user string) (string, error) + // Set stores the secret for user. + Set(service, user, secret string) error + // Delete removes the secret for user. + Delete(service, user string) error +} + +// ErrSecretNotFound is returned by a [Keyring] that has no secret for a user. +var ErrSecretNotFound = keyring.ErrNotFound + +// SystemKeyring is the OS keychain: macOS Keychain, Windows Credential Manager, +// or the Secret Service on Linux. +type SystemKeyring struct{} + +// Get implements [Keyring]. +func (SystemKeyring) Get(service, user string) (string, error) { return keyring.Get(service, user) } + +// Set implements [Keyring]. +func (SystemKeyring) Set(service, user, secret string) error { + return keyring.Set(service, user, secret) +} + +// Delete implements [Keyring]. +func (SystemKeyring) Delete(service, user string) error { return keyring.Delete(service, user) } + +// KeyringIndexName is the file name of the [KeyringStore] index, kept apart from +// grants.json so a [FileStore] never mistakes a keychain grant for one without +// tokens. +const KeyringIndexName = "keychain-grants.json" + +// KeyringStore keeps tokens in the OS keychain and everything else in a +// [FileStore] index, so grants can be listed without reading secrets. Each +// token is a separate keychain entry, which keeps entries within the size +// limits of the macOS Keychain and the Windows Credential Manager. +type KeyringStore struct { + Index *FileStore + Keyring Keyring +} + +var _ Store = (*KeyringStore)(nil) + +// keyringUser is the keychain user of a grant's access or refresh token. +func keyringUser(key GrantKey, token string) string { + return key.ID() + "/" + token +} + +// List implements [Store]. +func (s *KeyringStore) List() ([]Grant, error) { + grants, err := s.Index.List() + if err != nil { + return nil, err + } + for i := range grants { + if err := s.fill(&grants[i]); err != nil { + return nil, err + } + } + return grants, nil +} + +// Get implements [Store]. +func (s *KeyringStore) Get(key GrantKey) (*Grant, error) { + g, err := s.Index.Get(key) + if err != nil || g == nil { + return g, err + } + if err := s.fill(g); err != nil { + return nil, err + } + return g, nil +} + +// fill reads the grant's tokens from the keychain. A missing entry leaves the +// token empty, and a grant with no tokens left needs a new sign-in. +func (s *KeyringStore) fill(g *Grant) error { + for _, t := range []struct { + name string + dst *string + }{{"access", &g.AccessToken}, {"refresh", &g.RefreshToken}} { + v, err := s.Keyring.Get(keyringService, keyringUser(g.Key, t.name)) + if err != nil && !errors.Is(err, ErrSecretNotFound) { + return fmt.Errorf("reading the keychain: %w", err) + } + *t.dst = v + } + return nil +} + +// Put implements [Store]. +func (s *KeyringStore) Put(g Grant) error { + for _, t := range []struct{ name, value string }{{"access", g.AccessToken}, {"refresh", g.RefreshToken}} { + user := keyringUser(g.Key, t.name) + if t.value == "" { + if err := s.Keyring.Delete(keyringService, user); err != nil && !errors.Is(err, ErrSecretNotFound) { + return fmt.Errorf("deleting from the keychain: %w", err) + } + continue + } + err := s.Keyring.Set(keyringService, user, t.value) + switch { + case errors.Is(err, keyring.ErrSetDataTooBig): + return fmt.Errorf("the %s token from %s is too large for the OS keychain; use --oauth-store file: %w", t.name, g.Key.Issuer, err) + case err != nil: + return fmt.Errorf("writing to the keychain (use --oauth-store file where none is available): %w", err) + } + } + g.AccessToken, g.RefreshToken = "", "" + return s.Index.Put(g) +} + +// Delete implements [Store]. +func (s *KeyringStore) Delete(key GrantKey) error { + for _, name := range []string{"access", "refresh"} { + if err := s.Keyring.Delete(keyringService, keyringUser(key, name)); err != nil && !errors.Is(err, ErrSecretNotFound) { + return fmt.Errorf("deleting from the keychain: %w", err) + } + } + return s.Index.Delete(key) +} + +// Lock implements [Store]. +func (s *KeyringStore) Lock(ctx context.Context) (func(), error) { + return s.Index.Lock(ctx) +} diff --git a/internal/oauth/keyringstore_test.go b/internal/oauth/keyringstore_test.go new file mode 100644 index 0000000..4df21d0 --- /dev/null +++ b/internal/oauth/keyringstore_test.go @@ -0,0 +1,133 @@ +// Copyright 2026 The A2A Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package oauth + +import ( + "errors" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/zalando/go-keyring" +) + +type fakeKeyring struct { + mu sync.Mutex + secrets map[string]string + err error +} + +func (k *fakeKeyring) Get(service, user string) (string, error) { + k.mu.Lock() + defer k.mu.Unlock() + s, ok := k.secrets[service+"/"+user] + if !ok { + return "", ErrSecretNotFound + } + return s, nil +} + +func (k *fakeKeyring) Set(service, user, secret string) error { + k.mu.Lock() + defer k.mu.Unlock() + if k.err != nil { + return k.err + } + if k.secrets == nil { + k.secrets = map[string]string{} + } + k.secrets[service+"/"+user] = secret + return nil +} + +func (k *fakeKeyring) Delete(service, user string) error { + k.mu.Lock() + defer k.mu.Unlock() + if _, ok := k.secrets[service+"/"+user]; !ok { + return ErrSecretNotFound + } + delete(k.secrets, service+"/"+user) + return nil +} + +func TestKeyringStore(t *testing.T) { + t.Parallel() + dir := t.TempDir() + kr := &fakeKeyring{} + s := &KeyringStore{Index: &FileStore{Dir: dir}, Keyring: kr} + key := NewGrantKey("https://login.example.com", "c", "https://a.example.com", nil) + g := Grant{Key: key, AccessToken: "access-secret", RefreshToken: "refresh-secret", Endpoints: Endpoints{Issuer: "https://login.example.com", Token: "https://login.example.com/token"}} + + if err := s.Put(g); err != nil { + t.Fatalf("KeyringStore.Put() error = %v", err) + } + index, err := os.ReadFile(filepath.Join(dir, grantsFileName)) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(index), "secret") { + t.Fatalf("the index file contains tokens: %s", index) + } + if len(kr.secrets) != 2 { + t.Fatalf("keyring entries = %v, want one per token", kr.secrets) + } + got, err := s.Get(key) + if err != nil { + t.Fatalf("KeyringStore.Get() error = %v", err) + } + if diff := cmp.Diff(&g, got); diff != "" { + t.Fatalf("KeyringStore.Get() wrong result (-want +got) diff = %s", diff) + } + all, err := s.List() + if err != nil || len(all) != 1 || all[0].RefreshToken != "refresh-secret" { + t.Fatalf("KeyringStore.List() = %v, %v, want the grant with its tokens", all, err) + } + if err := s.Delete(key); err != nil { + t.Fatalf("KeyringStore.Delete() error = %v", err) + } + if got, err := s.Get(key); err != nil || got != nil { + t.Fatalf("KeyringStore.Get() after Delete = %v, %v, want nil, nil", got, err) + } + if len(kr.secrets) != 0 { + t.Fatalf("keyring entries after Delete = %v, want none", kr.secrets) + } + + kr.err = errors.New("no secret service") + if err := s.Put(g); err == nil || !strings.Contains(err.Error(), "--oauth-store file") { + t.Fatalf("KeyringStore.Put() without a keychain error = %v, want a hint to use the file store", err) + } + kr.err = keyring.ErrSetDataTooBig + if err := s.Put(g); err == nil || !strings.Contains(err.Error(), "too large for the OS keychain") { + t.Fatalf("KeyringStore.Put() with an oversized token error = %v, want it named", err) + } +} + +func TestKeyringStoreMissingSecret(t *testing.T) { + t.Parallel() + kr := &fakeKeyring{} + s := &KeyringStore{Index: &FileStore{Dir: t.TempDir()}, Keyring: kr} + key := NewGrantKey("https://login.example.com", "c", "https://a.example.com", nil) + if err := s.Put(Grant{Key: key, AccessToken: "a", RefreshToken: "r"}); err != nil { + t.Fatal(err) + } + kr.secrets = nil + g, err := s.Get(key) + if err != nil || g == nil || g.AccessToken != "" || g.RefreshToken != "" { + t.Fatalf("KeyringStore.Get() with the keychain entries gone = %+v, %v, want the grant without tokens", g, err) + } +} diff --git a/internal/oauth/store.go b/internal/oauth/store.go index 1fceb49..e1bd50d 100644 --- a/internal/oauth/store.go +++ b/internal/oauth/store.go @@ -59,6 +59,17 @@ const ( // the user (0600, in a 0700 directory). type FileStore struct { Dir string + // Name replaces grants.json as the file name, so stores with different + // formats in one directory don't read each other's grants. + Name string +} + +func (s *FileStore) path() string { + name := s.Name + if name == "" { + name = grantsFileName + } + return filepath.Join(s.Dir, name) } var _ Store = (*FileStore)(nil) @@ -70,7 +81,7 @@ type grantsFile struct { // List implements [Store]. func (s *FileStore) List() ([]Grant, error) { - data, err := os.ReadFile(filepath.Join(s.Dir, grantsFileName)) + data, err := os.ReadFile(s.path()) if errors.Is(err, os.ErrNotExist) { return nil, nil } @@ -79,10 +90,10 @@ func (s *FileStore) List() ([]Grant, error) { } var f grantsFile if err := json.Unmarshal(data, &f); err != nil { - return nil, fmt.Errorf("parsing %s: %w", filepath.Join(s.Dir, grantsFileName), err) + return nil, fmt.Errorf("parsing %s: %w", s.path(), err) } if f.Version > storeVersion { - return nil, fmt.Errorf("%s was written by a newer a2a (version %d)", filepath.Join(s.Dir, grantsFileName), f.Version) + return nil, fmt.Errorf("%s was written by a newer a2a (version %d)", s.path(), f.Version) } return f.Grants, nil } @@ -146,7 +157,7 @@ func (s *FileStore) write(grants []Grant) error { if err != nil { return err } - tmp, err := os.CreateTemp(s.Dir, grantsFileName+".*.tmp") + tmp, err := os.CreateTemp(s.Dir, filepath.Base(s.path())+".*.tmp") if err != nil { return fmt.Errorf("writing grants: %w", err) } @@ -167,7 +178,7 @@ func (s *FileStore) write(grants []Grant) error { if err := tmp.Close(); err != nil { return fmt.Errorf("writing grants: %w", err) } - if err := os.Rename(tmpName, filepath.Join(s.Dir, grantsFileName)); err != nil { + if err := os.Rename(tmpName, s.path()); err != nil { return fmt.Errorf("writing grants: %w", err) } return nil