Skip to content

Commit a549c58

Browse files
committed
feat(auth): support required keyring credentials
1 parent 400843a commit a549c58

8 files changed

Lines changed: 211 additions & 15 deletions

File tree

go.mod

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,21 @@ require (
66
github.com/alecthomas/kong v1.16.0
77
github.com/google/go-cmp v0.7.0
88
github.com/google/go-github/v89 v89.0.0
9+
github.com/google/uuid v1.6.0
910
github.com/modelcontextprotocol/go-sdk v1.6.1
1011
github.com/pelletier/go-toml/v2 v2.4.3
1112
github.com/pressly/goose/v3 v3.24.0
13+
github.com/zalando/go-keyring v0.2.8
1214
golang.org/x/time v0.15.0
1315
modernc.org/sqlite v1.54.0
1416
)
1517

1618
require (
19+
github.com/danieljoos/wincred v1.2.3 // indirect
1720
github.com/dustin/go-humanize v1.0.1 // indirect
21+
github.com/godbus/dbus/v5 v5.2.2 // indirect
1822
github.com/google/go-querystring v1.2.0 // indirect
1923
github.com/google/jsonschema-go v0.4.3 // indirect
20-
github.com/google/uuid v1.6.0 // indirect
2124
github.com/mattn/go-isatty v0.0.20 // indirect
2225
github.com/mfridman/interpolate v0.0.2 // indirect
2326
github.com/ncruces/go-strftime v1.0.0 // indirect

go.sum

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,14 @@ github.com/alecthomas/kong v1.16.0 h1:g92/kUxBcdcTPOM79yE63viJgtcp5dNyrB3/O2cjYT
44
github.com/alecthomas/kong v1.16.0/go.mod h1:wrlbXem1CWqUV5Vbmss5ISYhsVPkBb1Yo7YKJghju2I=
55
github.com/alecthomas/repr v0.5.2 h1:SU73FTI9D1P5UNtvseffFSGmdNci/O6RsqzeXJtP0Qs=
66
github.com/alecthomas/repr v0.5.2/go.mod h1:Fr0507jx4eOXV7AlPV6AVZLYrLIuIeSOWtW57eE/O/4=
7+
github.com/danieljoos/wincred v1.2.3 h1:v7dZC2x32Ut3nEfRH+vhoZGvN72+dQ/snVXo/vMFLdQ=
8+
github.com/danieljoos/wincred v1.2.3/go.mod h1:6qqX0WNrS4RzPZ1tnroDzq9kY3fu1KwE7MRLQK4X0bs=
79
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
810
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
911
github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY=
1012
github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto=
13+
github.com/godbus/dbus/v5 v5.2.2 h1:TUR3TgtSVDmjiXOgAAyaZbYmIeP3DPkld3jgKGV8mXQ=
14+
github.com/godbus/dbus/v5 v5.2.2/go.mod h1:3AAv2+hPq5rdnr5txxxRwiGjPXamgoIHgz9FPBfOp3c=
1115
github.com/golang-jwt/jwt/v5 v5.3.1 h1:kYf81DTWFe7t+1VvL7eS+jKFVWaUnK9cB1qbwn63YCY=
1216
github.com/golang-jwt/jwt/v5 v5.3.1/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE=
1317
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
@@ -49,10 +53,14 @@ github.com/segmentio/encoding v0.5.4 h1:OW1VRern8Nw6ITAtwSZ7Idrl3MXCFwXHPgqESYfv
4953
github.com/segmentio/encoding v0.5.4/go.mod h1:HS1ZKa3kSN32ZHVZ7ZLPLXWvOVIiZtyJnO1gPH1sKt0=
5054
github.com/sethvargo/go-retry v0.3.0 h1:EEt31A35QhrcRZtrYFDTBg91cqZVnFL2navjDrah2SE=
5155
github.com/sethvargo/go-retry v0.3.0/go.mod h1:mNX17F0C/HguQMyMyJxcnU471gOZGxCLyYaFyAZraas=
52-
github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
53-
github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
56+
github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
57+
github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
58+
github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
59+
github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
5460
github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zIM+UJPGz4=
5561
github.com/yosida95/uritemplate/v3 v3.0.2/go.mod h1:ILOh0sOhIJR3+L/8afwt/kE++YT040gmv5BQTMR2HP4=
62+
github.com/zalando/go-keyring v0.2.8 h1:6sD/Ucpl7jNq10rM2pgqTs0sZ9V3qMrqfIIy5YPccHs=
63+
github.com/zalando/go-keyring v0.2.8/go.mod h1:tsMo+VpRq5NGyKfxoBVjCuMrG47yj8cmakZDO5QGii0=
5664
go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0=
5765
go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y=
5866
golang.org/x/mod v0.37.0 h1:vF1DjpVEshcIqoEaauuHebaLk1O1forxjxBaVn884JQ=

internal/app/app.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,9 @@ func (s *Service) loadConfig(save bool) (*config.Config, error) {
8787
if err := config.ApplyEnv(cfg, os.Getenv); err != nil {
8888
return nil, err
8989
}
90+
if err := config.Validate(cfg); err != nil {
91+
return nil, fmt.Errorf("validate config: %w", err)
92+
}
9093
if save && !exists {
9194
dir := filepath.Dir(cfgFile)
9295
if err := os.MkdirAll(dir, 0755); err != nil {
@@ -157,9 +160,6 @@ func (s *Service) newGitHubReader() (github.Reader, error) {
157160
if cfg == nil {
158161
return nil, errors.New("configuration is not loaded")
159162
}
160-
if strings.EqualFold(cfg.TokenSource.Method, "keyring") {
161-
return nil, errors.New("keyring token source is not supported")
162-
}
163163
tokenSrc := tokenSource(cfg)
164164
client, err := github.NewClient(github.Config{TokenSource: tokenSrc})
165165
if err != nil {
@@ -176,9 +176,11 @@ func tokenSource(cfg *config.Config) github.TokenSource {
176176
if name == "" {
177177
name = github.DefaultEnvToken
178178
}
179-
return github.EnvTokenSource(name)
179+
return github.RequireToken(github.EnvTokenSource(name))
180180
case "gh-cli":
181-
return github.GhCLITokenSource(nil)
181+
return github.RequireToken(github.GhCLITokenSource(nil))
182+
case "keyring":
183+
return github.RequireToken(github.KeyringTokenSource(cfg.TokenSource.Key))
182184
}
183185
return github.StaticTokenSource("")
184186
}

internal/app/app_test.go

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ func TestSearchReportsDefaultLimit(t *testing.T) {
359359
}
360360
}
361361

362-
func TestLocalInitializationDoesNotResolveUnsupportedAuth(t *testing.T) {
362+
func TestLocalInitializationDoesNotResolveKeyringAuth(t *testing.T) {
363363
ctx := context.Background()
364364
paths := config.NewPaths(&config.Env{Home: t.TempDir()})
365365
configPath, err := paths.ConfigFile()
@@ -368,6 +368,7 @@ func TestLocalInitializationDoesNotResolveUnsupportedAuth(t *testing.T) {
368368
}
369369
cfg := config.Default()
370370
cfg.TokenSource.Method = "keyring"
371+
cfg.TokenSource.Key = "test-account"
371372
if err := config.ApplyDefaults(cfg, paths); err != nil {
372373
t.Fatal(err)
373374
}
@@ -382,8 +383,40 @@ func TestLocalInitializationDoesNotResolveUnsupportedAuth(t *testing.T) {
382383
if _, err := svc.Init(ctx); err != nil {
383384
t.Fatalf("local init resolved GitHub auth: %v", err)
384385
}
385-
if _, err := svc.Sync(ctx, cli.RepoRef{Owner: "owner", Repo: "repo"}); err == nil || !strings.Contains(err.Error(), "keyring") {
386-
t.Fatalf("network sync error = %v, want explicit unsupported auth", err)
386+
}
387+
388+
func TestConfiguredAuthenticationIsRequired(t *testing.T) {
389+
t.Setenv("GITCONTRIBUTE_TEST_MISSING_TOKEN", "")
390+
cfg := config.Default()
391+
cfg.TokenSource = config.TokenSource{
392+
Method: "env",
393+
Key: "GITCONTRIBUTE_TEST_MISSING_TOKEN",
394+
}
395+
396+
_, err := tokenSource(cfg).Token(context.Background())
397+
if !errors.Is(err, github.ErrRequiredToken) {
398+
t.Fatalf("configured auth error = %v, want ErrRequiredToken", err)
399+
}
400+
}
401+
402+
func TestNewRejectsInvalidConfiguredTokenSource(t *testing.T) {
403+
paths := config.NewPaths(&config.Env{Home: t.TempDir()})
404+
configPath, err := paths.ConfigFile()
405+
if err != nil {
406+
t.Fatal(err)
407+
}
408+
cfg := config.Default()
409+
cfg.TokenSource.Method = "keyrign"
410+
if err := config.ApplyDefaults(cfg, paths); err != nil {
411+
t.Fatal(err)
412+
}
413+
if err := config.Save(configPath, cfg); err != nil {
414+
t.Fatal(err)
415+
}
416+
417+
_, err = New(paths, "test")
418+
if err == nil || !strings.Contains(err.Error(), "invalid token_source method") {
419+
t.Fatalf("New error = %v, want invalid token source", err)
387420
}
388421
}
389422

internal/config/config.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -237,10 +237,10 @@ func Validate(cfg *Config) error {
237237
}
238238

239239
switch cfg.TokenSource.Method {
240-
case "none", "keyring", "gh-cli":
241-
case "env":
242-
if cfg.TokenSource.Key == "" {
243-
return errors.New("token_source key is required when method is env")
240+
case "none", "gh-cli":
241+
case "env", "keyring":
242+
if strings.TrimSpace(cfg.TokenSource.Key) == "" {
243+
return fmt.Errorf("token_source key is required when method is %s", cfg.TokenSource.Method)
244244
}
245245
default:
246246
return fmt.Errorf("invalid token_source method %q", cfg.TokenSource.Method)

internal/config/config_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,16 @@ func TestConfigValidation(t *testing.T) {
190190
cfg: &Config{Database: "/tmp/db", TokenSource: TokenSource{Method: "env"}, Crawl: Crawl{Budget: 1, Concurrency: 1}, Output: Output{Format: "text"}},
191191
wantErr: "token_source key is required when method is env",
192192
},
193+
{
194+
name: "token keyring missing account",
195+
cfg: &Config{Database: "/tmp/db", TokenSource: TokenSource{Method: "keyring"}, Crawl: Crawl{Budget: 1, Concurrency: 1}, Output: Output{Format: "text"}},
196+
wantErr: "token_source key is required when method is keyring",
197+
},
198+
{
199+
name: "token keyring blank account",
200+
cfg: &Config{Database: "/tmp/db", TokenSource: TokenSource{Method: "keyring", Key: " \t"}, Crawl: Crawl{Budget: 1, Concurrency: 1}, Output: Output{Format: "text"}},
201+
wantErr: "token_source key is required when method is keyring",
202+
},
193203
{
194204
name: "invalid token method",
195205
cfg: &Config{Database: "/tmp/db", TokenSource: TokenSource{Method: "magic"}, Crawl: Crawl{Budget: 1, Concurrency: 1}, Output: Output{Format: "text"}},

internal/github/auth.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,12 @@ package github
33
import (
44
"context"
55
"errors"
6+
"fmt"
67
"os"
78
"os/exec"
89
"strings"
10+
11+
"github.com/zalando/go-keyring"
912
)
1013

1114
// TokenSource resolves a GitHub authentication token.
@@ -16,6 +19,14 @@ type TokenSource interface {
1619
// ErrNoToken indicates that a token source could not provide a token.
1720
var ErrNoToken = errors.New("no GitHub token available")
1821

22+
// ErrRequiredToken indicates that an explicitly configured authentication
23+
// source did not provide a token.
24+
var ErrRequiredToken = errors.New("configured GitHub token unavailable")
25+
26+
// KeyringService is the service name used for credentials owned by
27+
// gitcontribute.
28+
const KeyringService = "gitcontribute"
29+
1930
// CommandRunner abstracts process execution so that tests can inject behavior.
2031
type CommandRunner interface {
2132
Run(ctx context.Context, name string, args ...string) (string, error)
@@ -65,6 +76,39 @@ func (s *envTokenSource) Token(ctx context.Context) (string, error) {
6576
return v, nil
6677
}
6778

79+
// KeyringTokenSource resolves a token from the operating system credential
80+
// store. account identifies the credential within the gitcontribute service.
81+
func KeyringTokenSource(account string) TokenSource {
82+
return &keyringTokenSource{account: account, get: keyring.Get}
83+
}
84+
85+
type keyringTokenSource struct {
86+
account string
87+
get func(service, user string) (string, error)
88+
}
89+
90+
func (s *keyringTokenSource) Token(ctx context.Context) (string, error) {
91+
if err := ctx.Err(); err != nil {
92+
return "", err
93+
}
94+
if strings.TrimSpace(s.account) == "" {
95+
return "", ErrNoToken
96+
}
97+
98+
token, err := s.get(KeyringService, s.account)
99+
if errors.Is(err, keyring.ErrNotFound) {
100+
return "", ErrNoToken
101+
}
102+
if err != nil {
103+
return "", fmt.Errorf("read GitHub token from keyring: %w", err)
104+
}
105+
token = strings.TrimSpace(token)
106+
if token == "" {
107+
return "", ErrNoToken
108+
}
109+
return token, nil
110+
}
111+
68112
// GhCLITokenSource resolves a token by running `gh auth token`.
69113
// Optional args are passed through to `gh` (for example a `--hostname` flag).
70114
func GhCLITokenSource(runner CommandRunner, args ...string) TokenSource {
@@ -115,6 +159,27 @@ func (c chainTokenSource) Token(ctx context.Context) (string, error) {
115159
return "", ErrNoToken
116160
}
117161

162+
// RequireToken prevents an explicitly configured source from silently falling
163+
// back to anonymous GitHub access.
164+
func RequireToken(source TokenSource) TokenSource {
165+
return requiredTokenSource{source: source}
166+
}
167+
168+
type requiredTokenSource struct {
169+
source TokenSource
170+
}
171+
172+
func (s requiredTokenSource) Token(ctx context.Context) (string, error) {
173+
if s.source == nil {
174+
return "", ErrRequiredToken
175+
}
176+
token, err := s.source.Token(ctx)
177+
if errors.Is(err, ErrNoToken) || (err == nil && token == "") {
178+
return "", ErrRequiredToken
179+
}
180+
return token, err
181+
}
182+
118183
// DefaultEnvToken is the conventional environment variable name for a
119184
// GitHub token.
120185
const DefaultEnvToken = "GITHUB_TOKEN"

internal/github/client_test.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313

1414
"github.com/google/go-cmp/cmp"
1515
"github.com/google/go-cmp/cmp/cmpopts"
16+
"github.com/zalando/go-keyring"
1617
)
1718

1819
const testOwner = "octocat"
@@ -556,6 +557,80 @@ func TestTokenResolution(t *testing.T) {
556557
t.Fatalf("got %q, %v", tok, err)
557558
}
558559
})
560+
561+
t.Run("required missing", func(t *testing.T) {
562+
src := RequireToken(StaticTokenSource(""))
563+
tok, err := src.Token(context.Background())
564+
if !errors.Is(err, ErrRequiredToken) || errors.Is(err, ErrNoToken) || tok != "" {
565+
t.Fatalf("got %q, %v", tok, err)
566+
}
567+
})
568+
569+
t.Run("required present", func(t *testing.T) {
570+
src := RequireToken(StaticTokenSource("required-token"))
571+
tok, err := src.Token(context.Background())
572+
if err != nil || tok != "required-token" {
573+
t.Fatalf("got %q, %v", tok, err)
574+
}
575+
})
576+
577+
t.Run("keyring", func(t *testing.T) {
578+
var service, account string
579+
src := &keyringTokenSource{
580+
account: "github.com",
581+
get: func(gotService, gotAccount string) (string, error) {
582+
service, account = gotService, gotAccount
583+
return " keyring-token\n", nil
584+
},
585+
}
586+
tok, err := src.Token(context.Background())
587+
if err != nil || tok != "keyring-token" {
588+
t.Fatalf("got %q, %v", tok, err)
589+
}
590+
if service != KeyringService || account != "github.com" {
591+
t.Fatalf("lookup = %q/%q, want %q/github.com", service, account, KeyringService)
592+
}
593+
})
594+
595+
t.Run("keyring missing", func(t *testing.T) {
596+
src := &keyringTokenSource{
597+
account: "missing",
598+
get: func(string, string) (string, error) { return "", keyring.ErrNotFound },
599+
}
600+
tok, err := src.Token(context.Background())
601+
if !errors.Is(err, ErrNoToken) || tok != "" {
602+
t.Fatalf("got %q, %v", tok, err)
603+
}
604+
})
605+
606+
t.Run("keyring backend failure", func(t *testing.T) {
607+
backendErr := errors.New("backend unavailable")
608+
src := &keyringTokenSource{
609+
account: "github.com",
610+
get: func(string, string) (string, error) { return "", backendErr },
611+
}
612+
_, err := src.Token(context.Background())
613+
if !errors.Is(err, backendErr) || !strings.Contains(err.Error(), "keyring") {
614+
t.Fatalf("got %v, want wrapped keyring backend error", err)
615+
}
616+
})
617+
618+
t.Run("keyring canceled", func(t *testing.T) {
619+
ctx, cancel := context.WithCancel(context.Background())
620+
cancel()
621+
called := false
622+
src := &keyringTokenSource{
623+
account: "github.com",
624+
get: func(string, string) (string, error) {
625+
called = true
626+
return "token", nil
627+
},
628+
}
629+
_, err := src.Token(ctx)
630+
if !errors.Is(err, context.Canceled) || called {
631+
t.Fatalf("got err=%v called=%t, want canceled without lookup", err, called)
632+
}
633+
})
559634
}
560635

561636
type fakeRunner struct {

0 commit comments

Comments
 (0)