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