Skip to content

Commit a370115

Browse files
appleboyclaude
andauthored
refactor(tokenstore): extract token storage into reusable sub-package (#14)
* refactor(tokenstore): extract token storage into reusable sub-package - Move token store interface, file store, keyring store, secure store, and file lock into tokenstore/ package - Apply idiomatic Go naming: TokenStore→Store, TokenStorage→Token, FileTokenStore→FileStore, KeyringTokenStore→KeyringStore - Make TokenStorageMap unexported as internal implementation detail - Parameterize NewSecureStore to accept serviceName for keyring probing - Update main.go and main_test.go to import the new tokenstore package Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(tokenstore): address PR review feedback - Add input validation: Save() returns error for nil Token or empty ClientID - Fix FileStore.Save() error handling: propagate read/parse errors instead of silently resetting the token map - Refactor SecureStore: introduce Prober interface so KeyringStore.Probe() replaces the detached probeKeyring() function, removing serviceName parameter - Remove unused useKeyring field from SecureStore - Move stderr output from library to caller (main.go) for reusable package - Fix probeKeyring to treat Delete failure as probe failure - Distinguish ErrNotFound from real errors in main.go run() token loading - Run go mod tidy to fix dependency annotations Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b65c2b1 commit a370115

File tree

13 files changed

+991
-178
lines changed

13 files changed

+991
-178
lines changed

go.mod

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@ require (
99
github.com/appleboy/go-httpretry v0.11.0
1010
github.com/google/uuid v1.6.0
1111
github.com/joho/godotenv v1.5.1
12+
github.com/zalando/go-keyring v0.2.6
1213
golang.org/x/oauth2 v0.35.0
1314
)
1415

1516
require (
17+
al.essio.dev/pkg/shellescape v1.5.1 // indirect
1618
github.com/charmbracelet/colorprofile v0.4.2 // indirect
1719
github.com/charmbracelet/ultraviolet v0.0.0-20260205113103-524a6607adb8 // indirect
1820
github.com/charmbracelet/x/ansi v0.11.6 // indirect
@@ -21,6 +23,8 @@ require (
2123
github.com/charmbracelet/x/windows v0.2.2 // indirect
2224
github.com/clipperhouse/displaywidth v0.11.0 // indirect
2325
github.com/clipperhouse/uax29/v2 v2.7.0 // indirect
26+
github.com/danieljoos/wincred v1.2.2 // indirect
27+
github.com/godbus/dbus/v5 v5.1.0 // indirect
2428
github.com/lucasb-eyer/go-colorful v1.3.0 // indirect
2529
github.com/mattn/go-runewidth v0.0.20 // indirect
2630
github.com/muesli/cancelreader v0.2.2 // indirect

go.sum

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
al.essio.dev/pkg/shellescape v1.5.1 h1:86HrALUujYS/h+GtqoB26SBEdkWfmMI6FubjXlsXyho=
2+
al.essio.dev/pkg/shellescape v1.5.1/go.mod h1:6sIqp7X2P6mThCQ7twERpZTuigpr6KbZWtls1U8I890=
13
charm.land/bubbles/v2 v2.0.0 h1:tE3eK/pHjmtrDiRdoC9uGNLgpopOd8fjhEe31B/ai5s=
24
charm.land/bubbles/v2 v2.0.0/go.mod h1:rCHoleP2XhU8um45NTuOWBPNVHxnkXKTiZqcclL/qOI=
35
charm.land/bubbletea/v2 v2.0.0 h1:p0d6CtWyJXJ9GfzMpUUqbP/XUUhhlk06+vCKWmox1wQ=
@@ -26,6 +28,14 @@ github.com/clipperhouse/displaywidth v0.11.0 h1:lBc6kY44VFw+TDx4I8opi/EtL9m20WSE
2628
github.com/clipperhouse/displaywidth v0.11.0/go.mod h1:bkrFNkf81G8HyVqmKGxsPufD3JhNl3dSqnGhOoSD/o0=
2729
github.com/clipperhouse/uax29/v2 v2.7.0 h1:+gs4oBZ2gPfVrKPthwbMzWZDaAFPGYK72F0NJv2v7Vk=
2830
github.com/clipperhouse/uax29/v2 v2.7.0/go.mod h1:EFJ2TJMRUaplDxHKj1qAEhCtQPW2tJSwu5BF98AuoVM=
31+
github.com/danieljoos/wincred v1.2.2 h1:774zMFJrqaeYCK2W57BgAem/MLi6mtSE47MB6BOJ0i0=
32+
github.com/danieljoos/wincred v1.2.2/go.mod h1:w7w4Utbrz8lqeMbDAK0lkNJUv5sAOkFi7nd/ogr0Uh8=
33+
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
34+
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
35+
github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk=
36+
github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
37+
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4=
38+
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ=
2939
github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
3040
github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
3141
github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0=
@@ -36,10 +46,18 @@ github.com/mattn/go-runewidth v0.0.20 h1:WcT52H91ZUAwy8+HUkdM3THM6gXqXuLJi9O3rjc
3646
github.com/mattn/go-runewidth v0.0.20/go.mod h1:XBkDxAl56ILZc9knddidhrOlY5R/pDhgLpndooCuJAs=
3747
github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELUXHmA=
3848
github.com/muesli/cancelreader v0.2.2/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo=
49+
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
50+
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
3951
github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ=
4052
github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88=
53+
github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
54+
github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
55+
github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
56+
github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
4157
github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no=
4258
github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM=
59+
github.com/zalando/go-keyring v0.2.6 h1:r7Yc3+H+Ux0+M72zacZoItR3UDxeWfKTcabvkI8ua9s=
60+
github.com/zalando/go-keyring v0.2.6/go.mod h1:2TCrxYrbUNYfNS/Kgy/LSrkSQzZ5UPVH85RwfczwvcI=
4361
golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI=
4462
golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo=
4563
golang.org/x/oauth2 v0.35.0 h1:Mv2mzuHuZuY2+bkyWXIHMfhNdJAdwW3FuWeCPYN5GVQ=
@@ -48,3 +66,5 @@ golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4=
4866
golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI=
4967
golang.org/x/sys v0.41.0 h1:Ivj+2Cp/ylzLiEU89QhWblYnOE9zerudt9Ftecq2C6k=
5068
golang.org/x/sys v0.41.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
69+
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
70+
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

main.go

Lines changed: 60 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,26 @@ import (
2323
"golang.org/x/oauth2"
2424

2525
tea "charm.land/bubbletea/v2"
26+
"github.com/go-authgate/device-cli/tokenstore"
2627
"github.com/go-authgate/device-cli/tui"
2728
)
2829

2930
var (
3031
serverURL string
3132
clientID string
3233
tokenFile string
34+
tokenStoreMode string
3335
flagServerURL *string
3436
flagClientID *string
3537
flagTokenFile *string
38+
flagTokenStore *string
3639
configInitialized bool
3740
retryClient *retry.Client
41+
tokenStore tokenstore.Store
3842
)
3943

44+
const defaultKeyringService = "authgate-device-cli"
45+
4046
// Timeout configuration for different operations
4147
const (
4248
deviceCodeRequestTimeout = 10 * time.Second
@@ -91,6 +97,11 @@ func init() {
9197
"",
9298
"Token storage file (default: .authgate-tokens.json or TOKEN_FILE env)",
9399
)
100+
flagTokenStore = flag.String(
101+
"token-store",
102+
"",
103+
"Token storage backend: auto, file, keyring (default: auto or TOKEN_STORE env)",
104+
)
94105
}
95106

96107
// initConfig parses flags and initializes configuration
@@ -107,6 +118,7 @@ func initConfig() {
107118
serverURL = getConfig(*flagServerURL, "SERVER_URL", "http://localhost:8080")
108119
clientID = getConfig(*flagClientID, "CLIENT_ID", "")
109120
tokenFile = getConfig(*flagTokenFile, "TOKEN_FILE", ".authgate-tokens.json")
121+
tokenStoreMode = getConfig(*flagTokenStore, "TOKEN_STORE", "auto")
110122

111123
// Validate SERVER_URL format
112124
if err := validateServerURL(serverURL); err != nil {
@@ -171,6 +183,31 @@ func initConfig() {
171183
if err != nil {
172184
panic(fmt.Sprintf("failed to create retry client: %v", err))
173185
}
186+
187+
// Initialize token store based on mode
188+
fileStore := tokenstore.NewFileStore(tokenFile)
189+
switch tokenStoreMode {
190+
case "file":
191+
tokenStore = fileStore
192+
case "keyring":
193+
tokenStore = tokenstore.NewKeyringStore(defaultKeyringService)
194+
case "auto":
195+
kr := tokenstore.NewKeyringStore(defaultKeyringService)
196+
tokenStore = tokenstore.NewSecureStore(kr, fileStore)
197+
if !tokenStore.(*tokenstore.SecureStore).UseKeyring() {
198+
fmt.Fprintln(
199+
os.Stderr,
200+
"⚠️ OS keyring unavailable, falling back to file-based token storage",
201+
)
202+
}
203+
default:
204+
fmt.Fprintf(
205+
os.Stderr,
206+
"Error: Invalid token-store value: %s (must be auto, file, or keyring)\n",
207+
tokenStoreMode,
208+
)
209+
os.Exit(1)
210+
}
174211
}
175212

176213
// getConfig returns value with priority: flag > env > default
@@ -240,20 +277,6 @@ func validateTokenResponse(accessToken, tokenType string, expiresIn int) error {
240277
return nil
241278
}
242279

243-
// TokenStorage represents saved tokens for a specific client
244-
type TokenStorage struct {
245-
AccessToken string `json:"access_token"`
246-
RefreshToken string `json:"refresh_token"`
247-
TokenType string `json:"token_type"`
248-
ExpiresAt time.Time `json:"expires_at"`
249-
ClientID string `json:"client_id"`
250-
}
251-
252-
// TokenStorageMap manages tokens for multiple clients
253-
type TokenStorageMap struct {
254-
Tokens map[string]*TokenStorage `json:"tokens"` // key = client_id
255-
}
256-
257280
// isTTY reports whether stderr is a character device (interactive terminal).
258281
// We check stderr because the TUI renders to stderr, allowing stdout to be piped.
259282
func isTTY() bool {
@@ -302,11 +325,17 @@ func run(d tui.Displayer) error {
302325
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
303326
defer stop()
304327

305-
var storage *TokenStorage
328+
var storage *tokenstore.Token
306329

307330
// Try to load existing tokens
308-
storage, err := loadTokens()
309-
if err == nil && storage != nil {
331+
storage, err := tokenStore.Load(clientID)
332+
switch {
333+
case err != nil && !errors.Is(err, tokenstore.ErrNotFound):
334+
d.Fatal(err)
335+
return err
336+
case err != nil:
337+
d.TokensNotFound()
338+
case storage != nil:
310339
d.TokensFound()
311340

312341
// Check if access token is still valid
@@ -326,7 +355,7 @@ func run(d tui.Displayer) error {
326355
d.RefreshOK()
327356
}
328357
}
329-
} else {
358+
default:
330359
d.TokensNotFound()
331360
}
332361

@@ -444,7 +473,7 @@ func requestDeviceCode(ctx context.Context) (*oauth2.DeviceAuthResponse, error)
444473
}
445474

446475
// performDeviceFlow performs the OAuth device authorization flow
447-
func performDeviceFlow(ctx context.Context, d tui.Displayer) (*TokenStorage, error) {
476+
func performDeviceFlow(ctx context.Context, d tui.Displayer) (*tokenstore.Token, error) {
448477
config := &oauth2.Config{
449478
ClientID: clientID,
450479
Endpoint: oauth2.Endpoint{
@@ -476,19 +505,19 @@ func performDeviceFlow(ctx context.Context, d tui.Displayer) (*TokenStorage, err
476505

477506
d.AuthSuccess()
478507

479-
// Convert to TokenStorage and save
480-
storage := &TokenStorage{
508+
// Convert to Token and save
509+
storage := &tokenstore.Token{
481510
AccessToken: token.AccessToken,
482511
RefreshToken: token.RefreshToken,
483512
TokenType: token.Type(),
484513
ExpiresAt: token.Expiry,
485514
ClientID: clientID,
486515
}
487516

488-
if err := saveTokens(storage); err != nil {
517+
if err := tokenStore.Save(storage); err != nil {
489518
d.TokenSaveFailed(err)
490519
} else {
491-
d.TokenSaved(tokenFile)
520+
d.TokenSaved(tokenStore.String())
492521
}
493522

494523
return storage, nil
@@ -681,101 +710,12 @@ func verifyToken(ctx context.Context, accessToken string, d tui.Displayer) error
681710
return nil
682711
}
683712

684-
// loadTokens loads tokens from file for the current client
685-
func loadTokens() (*TokenStorage, error) {
686-
data, err := os.ReadFile(tokenFile)
687-
if err != nil {
688-
return nil, err
689-
}
690-
691-
var storageMap TokenStorageMap
692-
if err := json.Unmarshal(data, &storageMap); err != nil {
693-
return nil, fmt.Errorf("failed to parse token file: %w", err)
694-
}
695-
696-
if storageMap.Tokens == nil {
697-
return nil, errors.New("no tokens found in token file")
698-
}
699-
700-
// Look up token for current client_id
701-
if storage, ok := storageMap.Tokens[clientID]; ok {
702-
return storage, nil
703-
}
704-
705-
return nil, fmt.Errorf("no tokens found for client_id: %s", clientID)
706-
}
707-
708-
// saveTokens saves tokens to file (merges with existing tokens for other clients)
709-
// Uses file locking to prevent race conditions when multiple processes access the same file
710-
func saveTokens(storage *TokenStorage) error {
711-
// Ensure ClientID is set
712-
if storage.ClientID == "" {
713-
storage.ClientID = clientID
714-
}
715-
716-
// Acquire file lock to prevent concurrent access
717-
lock, err := acquireFileLock(tokenFile)
718-
if err != nil {
719-
return fmt.Errorf("failed to acquire lock: %w", err)
720-
}
721-
defer func() {
722-
if releaseErr := lock.release(); releaseErr != nil {
723-
fmt.Fprintf(os.Stderr, "failed to release lock: %v\n", releaseErr)
724-
}
725-
}()
726-
727-
// Load existing token map (inside lock to ensure consistency)
728-
var storageMap TokenStorageMap
729-
existingData, err := os.ReadFile(tokenFile)
730-
if err == nil {
731-
// File exists, try to load it
732-
if unmarshalErr := json.Unmarshal(existingData, &storageMap); unmarshalErr != nil {
733-
// If unmarshal fails, start with empty map
734-
storageMap.Tokens = make(map[string]*TokenStorage)
735-
}
736-
}
737-
738-
// Initialize map if nil
739-
if storageMap.Tokens == nil {
740-
storageMap.Tokens = make(map[string]*TokenStorage)
741-
}
742-
743-
// Add or update token for current client
744-
storageMap.Tokens[storage.ClientID] = storage
745-
746-
// Marshal data
747-
data, err := json.MarshalIndent(storageMap, "", " ")
748-
if err != nil {
749-
return err
750-
}
751-
752-
// Write to temp file first (atomic write pattern)
753-
tempFile := tokenFile + ".tmp"
754-
if err := os.WriteFile(tempFile, data, 0o600); err != nil {
755-
return fmt.Errorf("failed to write temp file: %w", err)
756-
}
757-
758-
// Atomic rename (replaces old file)
759-
if err := os.Rename(tempFile, tokenFile); err != nil {
760-
if removeErr := os.Remove(tempFile); removeErr != nil {
761-
return fmt.Errorf(
762-
"failed to rename temp file: %v; additionally failed to remove temp file: %w",
763-
err,
764-
removeErr,
765-
)
766-
}
767-
return fmt.Errorf("failed to rename temp file: %w", err)
768-
}
769-
770-
return nil
771-
}
772-
773713
// refreshAccessToken refreshes the access token using refresh token
774714
func refreshAccessToken(
775715
ctx context.Context,
776716
refreshToken string,
777717
d tui.Displayer,
778-
) (*TokenStorage, error) {
718+
) (*tokenstore.Token, error) {
779719
// Create request with timeout
780720
reqCtx, cancel := context.WithTimeout(ctx, refreshTokenTimeout)
781721
defer cancel()
@@ -844,7 +784,7 @@ func refreshAccessToken(
844784
newRefreshToken = refreshToken
845785
}
846786

847-
storage := &TokenStorage{
787+
storage := &tokenstore.Token{
848788
AccessToken: tokenResp.AccessToken,
849789
RefreshToken: newRefreshToken,
850790
TokenType: tokenResp.TokenType,
@@ -853,15 +793,19 @@ func refreshAccessToken(
853793
}
854794

855795
// Save updated tokens
856-
if err := saveTokens(storage); err != nil {
796+
if err := tokenStore.Save(storage); err != nil {
857797
d.TokenSaveFailed(err)
858798
}
859799

860800
return storage, nil
861801
}
862802

863803
// makeAPICallWithAutoRefresh demonstrates automatic refresh on 401
864-
func makeAPICallWithAutoRefresh(ctx context.Context, storage *TokenStorage, d tui.Displayer) error {
804+
func makeAPICallWithAutoRefresh(
805+
ctx context.Context,
806+
storage *tokenstore.Token,
807+
d tui.Displayer,
808+
) error {
865809
// Try with current access token
866810
reqCtx, cancel := context.WithTimeout(ctx, tokenVerificationTimeout)
867811
defer cancel()

0 commit comments

Comments
 (0)