From 9c399ad4c5e70a83a6a2844a4c993425d016e57b Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Thu, 12 Mar 2026 18:02:03 +0800 Subject: [PATCH 1/3] feat(cli): migrate CLI to Cobra with token management - Migrate the CLI from the standard flag package to Cobra, introducing a root command, subcommands, and proper version handling - Refactor configuration and flag handling to use Cobra persistent flags and plain variables instead of pointer flags - Split configuration initialization to allow minimal setup for local-only commands like token get - Add a token command with a get subcommand to print stored access tokens, with optional JSON output - Add tests covering token get behavior, including JSON output and missing-token errors - Update dependencies to include Cobra and its related packages - Simplify string prefix handling in the TUI token storage display using newer standard library helpers Signed-off-by: Bo-Yi Wu --- config.go | 175 +++++++++++++++++++++------------------------- go.mod | 3 + go.sum | 10 +++ main.go | 42 +++++++++-- token_cmd.go | 85 ++++++++++++++++++++++ token_cmd_test.go | 94 +++++++++++++++++++++++++ tui/token_view.go | 9 ++- 7 files changed, 311 insertions(+), 107 deletions(-) create mode 100644 token_cmd.go create mode 100644 token_cmd_test.go diff --git a/config.go b/config.go index c48ab12..aa11965 100644 --- a/config.go +++ b/config.go @@ -3,7 +3,6 @@ package main import ( "crypto/tls" "errors" - "flag" "fmt" "net/http" "net/url" @@ -18,35 +17,36 @@ import ( retry "github.com/appleboy/go-httpretry" "github.com/google/uuid" "github.com/joho/godotenv" + "github.com/spf13/cobra" ) // version is set at build time via -ldflags "-X main.version=...". var version string var ( - serverURL string - clientID string - clientSecret string - redirectURI string - callbackPort int - scope string - tokenFile string - tokenStoreMode string - forceDevice bool - configInitialized bool - retryClient *retry.Client - tokenStore credstore.Store[credstore.Token] + serverURL string + clientID string + clientSecret string + redirectURI string + callbackPort int + scope string + tokenFile string + tokenStoreMode string + forceDevice bool + storeConfigInitialized bool + configInitialized bool + retryClient *retry.Client + tokenStore credstore.Store[credstore.Token] - flagServerURL *string - flagClientID *string - flagClientSecret *string - flagRedirectURI *string - flagCallbackPort *int - flagScope *string - flagTokenFile *string - flagTokenStore *string - flagDevice *bool - flagVersion *bool + flagServerURL string + flagClientID string + flagClientSecret string + flagRedirectURI string + flagCallbackPort int + flagScope string + flagTokenFile string + flagTokenStore string + flagDevice bool ) const ( @@ -58,47 +58,56 @@ const ( defaultKeyringService = "authgate-cli" ) -func init() { +func registerFlags(cmd *cobra.Command) { _ = godotenv.Load() + cmd.PersistentFlags(). + StringVar(&flagServerURL, "server-url", "", "OAuth server URL (default: http://localhost:8080 or SERVER_URL env)") + cmd.PersistentFlags(). + StringVar(&flagClientID, "client-id", "", "OAuth client ID (required, or set CLIENT_ID env)") + cmd.PersistentFlags(). + StringVar(&flagClientSecret, "client-secret", "", "OAuth client secret (confidential clients only; omit for public/PKCE clients)") + cmd.PersistentFlags(). + StringVar(&flagRedirectURI, "redirect-uri", "", "Redirect URI registered with the OAuth server (default: http://localhost:PORT/callback)") + cmd.PersistentFlags(). + IntVar(&flagCallbackPort, "port", 0, "Local callback port for browser flow (default: 8888 or CALLBACK_PORT env)") + cmd.PersistentFlags(). + StringVar(&flagScope, "scope", "", "Space-separated OAuth scopes (default: \"read write\")") + cmd.PersistentFlags(). + StringVar(&flagTokenFile, "token-file", "", "Token storage file (default: .authgate-tokens.json or TOKEN_FILE env)") + cmd.PersistentFlags(). + StringVar(&flagTokenStore, "token-store", "", "Token storage backend: auto, file, keyring (default: auto or TOKEN_STORE env)") + cmd.PersistentFlags(). + BoolVar(&flagDevice, "device", false, "Force Device Code Flow (skip browser detection)") +} + +// initStoreConfig initialises only the token store and client ID — the minimum +// needed for commands like `token get` that read local credentials without +// making any network calls. +func initStoreConfig() { + if storeConfigInitialized { + return + } + storeConfigInitialized = true + + clientID = getConfig(flagClientID, "CLIENT_ID", "") + tokenFile = getConfig(flagTokenFile, "TOKEN_FILE", ".authgate-tokens.json") + tokenStoreMode = getConfig(flagTokenStore, "TOKEN_STORE", "auto") + + if clientID == "" { + fmt.Fprintln(os.Stderr, "Error: CLIENT_ID not set. Please provide it via:") + fmt.Fprintln(os.Stderr, " 1. Command-line flag: --client-id=") + fmt.Fprintln(os.Stderr, " 2. Environment variable: CLIENT_ID=") + fmt.Fprintln(os.Stderr, " 3. .env file: CLIENT_ID=") + fmt.Fprintln(os.Stderr, "\nYou can find the client_id in the server startup logs.") + os.Exit(1) + } - flagServerURL = flag.String( - "server-url", - "", - "OAuth server URL (default: http://localhost:8080 or SERVER_URL env)", - ) - flagClientID = flag.String("client-id", "", "OAuth client ID (required, or set CLIENT_ID env)") - flagClientSecret = flag.String( - "client-secret", - "", - "OAuth client secret (confidential clients only; omit for public/PKCE clients)", - ) - flagRedirectURI = flag.String( - "redirect-uri", - "", - "Redirect URI registered with the OAuth server (default: http://localhost:PORT/callback)", - ) - flagCallbackPort = flag.Int( - "port", - 0, - "Local callback port for browser flow (default: 8888 or CALLBACK_PORT env)", - ) - flagScope = flag.String("scope", "", "Space-separated OAuth scopes (default: \"read write\")") - flagTokenFile = flag.String( - "token-file", - "", - "Token storage file (default: .authgate-tokens.json or TOKEN_FILE env)", - ) - flagTokenStore = flag.String( - "token-store", - "", - "Token storage backend: auto, file, keyring (default: auto or TOKEN_STORE env)", - ) - flagDevice = flag.Bool( - "device", - false, - "Force Device Code Flow (skip browser detection)", - ) - flagVersion = flag.Bool("version", false, "Print version and exit") + var storeErr error + tokenStore, storeErr = newTokenStore(tokenStoreMode, tokenFile, defaultKeyringService) + if storeErr != nil { + fmt.Fprintln(os.Stderr, storeErr) + os.Exit(1) + } } func initConfig() { @@ -107,27 +116,20 @@ func initConfig() { } configInitialized = true - flag.Parse() - - // --version prints build version and exits immediately. - if *flagVersion { - fmt.Println(getVersion()) - os.Exit(0) - } + // initStoreConfig sets clientID, tokenFile, tokenStoreMode, and tokenStore. + initStoreConfig() // --device forces Device Code Flow unconditionally. - forceDevice = *flagDevice + forceDevice = flagDevice - serverURL = getConfig(*flagServerURL, "SERVER_URL", "http://localhost:8080") - clientID = getConfig(*flagClientID, "CLIENT_ID", "") - clientSecret = getConfig(*flagClientSecret, "CLIENT_SECRET", "") - scope = getConfig(*flagScope, "SCOPE", "read write") - tokenFile = getConfig(*flagTokenFile, "TOKEN_FILE", ".authgate-tokens.json") + serverURL = getConfig(flagServerURL, "SERVER_URL", "http://localhost:8080") + clientSecret = getConfig(flagClientSecret, "CLIENT_SECRET", "") + scope = getConfig(flagScope, "SCOPE", "read write") // Resolve callback port (int flag needs special handling). portStr := "" - if *flagCallbackPort != 0 { - portStr = strconv.Itoa(*flagCallbackPort) + if flagCallbackPort != 0 { + portStr = strconv.Itoa(flagCallbackPort) } portStr = getConfig(portStr, "CALLBACK_PORT", "8888") if _, err := fmt.Sscanf(portStr, "%d", &callbackPort); err != nil || callbackPort <= 0 { @@ -136,7 +138,7 @@ func initConfig() { // Resolve redirect URI (depends on port, so compute after port is known). defaultRedirectURI := fmt.Sprintf("http://localhost:%d/callback", callbackPort) - redirectURI = getConfig(*flagRedirectURI, "REDIRECT_URI", defaultRedirectURI) + redirectURI = getConfig(flagRedirectURI, "REDIRECT_URI", defaultRedirectURI) if err := validateServerURL(serverURL); err != nil { fmt.Fprintf(os.Stderr, "Error: Invalid SERVER_URL: %v\n", err) @@ -155,15 +157,6 @@ func initConfig() { fmt.Fprintln(os.Stderr) } - if clientID == "" { - fmt.Println("Error: CLIENT_ID not set. Please provide it via:") - fmt.Println(" 1. Command-line flag: -client-id=") - fmt.Println(" 2. Environment variable: CLIENT_ID=") - fmt.Println(" 3. .env file: CLIENT_ID=") - fmt.Println("\nYou can find the client_id in the server startup logs.") - os.Exit(1) - } - if _, err := uuid.Parse(clientID); err != nil { fmt.Fprintf( os.Stderr, @@ -188,14 +181,6 @@ func initConfig() { panic(fmt.Sprintf("failed to create retry client: %v", err)) } - // Initialize token store based on mode - tokenStoreMode = getConfig(*flagTokenStore, "TOKEN_STORE", "auto") - var storeErr error - tokenStore, storeErr = newTokenStore(tokenStoreMode, tokenFile, defaultKeyringService) - if storeErr != nil { - fmt.Fprintln(os.Stderr, storeErr) - os.Exit(1) - } if tokenStoreMode == "auto" { if ss, ok := tokenStore.(*credstore.SecureStore[credstore.Token]); ok && !ss.UseKeyring() { fmt.Fprintln( diff --git a/go.mod b/go.mod index 01b2ae6..70d50c8 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/google/uuid v1.6.0 github.com/joho/godotenv v1.5.1 github.com/mattn/go-isatty v0.0.20 + github.com/spf13/cobra v1.10.2 golang.org/x/oauth2 v0.36.0 golang.org/x/term v0.41.0 ) @@ -28,10 +29,12 @@ require ( github.com/clipperhouse/uax29/v2 v2.7.0 // indirect github.com/danieljoos/wincred v1.2.3 // indirect github.com/godbus/dbus/v5 v5.2.2 // indirect + github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/lucasb-eyer/go-colorful v1.3.0 // indirect github.com/mattn/go-runewidth v0.0.21 // indirect github.com/muesli/cancelreader v0.2.2 // indirect github.com/rivo/uniseg v0.4.7 // indirect + github.com/spf13/pflag v1.0.9 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect github.com/zalando/go-keyring v0.2.6 // indirect golang.org/x/sync v0.20.0 // indirect diff --git a/go.sum b/go.sum index 308c5b1..91aef2e 100644 --- a/go.sum +++ b/go.sum @@ -30,6 +30,7 @@ github.com/clipperhouse/displaywidth v0.11.0 h1:lBc6kY44VFw+TDx4I8opi/EtL9m20WSE github.com/clipperhouse/displaywidth v0.11.0/go.mod h1:bkrFNkf81G8HyVqmKGxsPufD3JhNl3dSqnGhOoSD/o0= github.com/clipperhouse/uax29/v2 v2.7.0 h1:+gs4oBZ2gPfVrKPthwbMzWZDaAFPGYK72F0NJv2v7Vk= github.com/clipperhouse/uax29/v2 v2.7.0/go.mod h1:EFJ2TJMRUaplDxHKj1qAEhCtQPW2tJSwu5BF98AuoVM= +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= @@ -42,6 +43,8 @@ github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaU github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= +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/lucasb-eyer/go-colorful v1.3.0 h1:2/yBRLdWBZKrf7gB40FoiKfAWYQ0lqNcbuQwVHXptag= @@ -56,6 +59,11 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= +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 h1:9exaQaMOCwffKiiiYk6/BndUBv+iRViNW+4lEMi0PvY= +github.com/spf13/pflag v1.0.9/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= @@ -64,6 +72,7 @@ github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavM github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= github.com/zalando/go-keyring v0.2.6 h1:r7Yc3+H+Ux0+M72zacZoItR3UDxeWfKTcabvkI8ua9s= github.com/zalando/go-keyring v0.2.6/go.mod h1:2TCrxYrbUNYfNS/Kgy/LSrkSQzZ5UPVH85RwfczwvcI= +go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo= golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= @@ -75,5 +84,6 @@ golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.41.0 h1:QCgPso/Q3RTJx2Th4bDLqML4W6iJiaXFq2/ftQF13YU= golang.org/x/term v0.41.0/go.mod h1:3pfBgksrReYfZ5lvYM0kSO0LIkAl4Yl2bXOkKP7Ec2A= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/main.go b/main.go index f74dcfa..8d985f5 100644 --- a/main.go +++ b/main.go @@ -18,18 +18,46 @@ import ( retry "github.com/appleboy/go-httpretry" "github.com/go-authgate/cli/tui" + "github.com/spf13/cobra" ) func main() { - initConfig() + if err := buildRootCmd().Execute(); err != nil { + os.Exit(1) + } +} - // Select UI manager based on environment detection - uiManager := tui.SelectManager() +func buildRootCmd() *cobra.Command { + rootCmd := &cobra.Command{ + Use: "authgate-cli", + Short: "OAuth 2.0 authentication CLI", + Version: getVersion(), + SilenceUsage: true, + SilenceErrors: true, + RunE: func(cmd *cobra.Command, args []string) error { + initConfig() + uiManager := tui.SelectManager() + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + exitCode := run(ctx, uiManager) + stop() + os.Exit(exitCode) + return nil + }, + } + registerFlags(rootCmd) + rootCmd.AddCommand(buildVersionCmd()) + rootCmd.AddCommand(buildTokenCmd()) + return rootCmd +} - ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) - exitCode := run(ctx, uiManager) - stop() - os.Exit(exitCode) +func buildVersionCmd() *cobra.Command { + return &cobra.Command{ + Use: "version", + Short: "Print version", + Run: func(cmd *cobra.Command, args []string) { + fmt.Println(getVersion()) + }, + } } func run(ctx context.Context, ui tui.Manager) int { diff --git a/token_cmd.go b/token_cmd.go new file mode 100644 index 0000000..cbcd736 --- /dev/null +++ b/token_cmd.go @@ -0,0 +1,85 @@ +package main + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "os" + "time" + + "github.com/go-authgate/sdk-go/credstore" + "github.com/spf13/cobra" +) + +type tokenGetOutput struct { + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token"` + TokenType string `json:"token_type"` + ExpiresAt time.Time `json:"expires_at"` + Expired bool `json:"expired"` + ClientID string `json:"client_id"` +} + +func buildTokenCmd() *cobra.Command { + tokenCmd := &cobra.Command{ + Use: "token", + Short: "Manage stored tokens", + } + tokenCmd.AddCommand(buildTokenGetCmd()) + return tokenCmd +} + +func buildTokenGetCmd() *cobra.Command { + var jsonOutput bool + cmd := &cobra.Command{ + Use: "get", + Short: "Print the stored access token", + RunE: func(cmd *cobra.Command, args []string) error { + initStoreConfig() + code := runTokenGet(tokenStore, clientID, jsonOutput, os.Stdout, os.Stderr) + if code != 0 { + os.Exit(code) + } + return nil + }, + } + cmd.Flags().BoolVar(&jsonOutput, "json", false, "Output full token details as JSON") + return cmd +} + +// runTokenGet is the testable core of `token get`. +func runTokenGet( + store credstore.Store[credstore.Token], + id string, + jsonOut bool, + stdout io.Writer, + stderr io.Writer, +) int { + tok, err := store.Load(id) + if err != nil { + if errors.Is(err, credstore.ErrNotFound) { + fmt.Fprintf(stderr, "Error: no stored token for client-id %q\n", id) + fmt.Fprintf(stderr, "Hint: run 'authgate-cli' first to authenticate.\n") + return 1 + } + fmt.Fprintf(stderr, "Error: failed to load token: %v\n", err) + return 1 + } + if jsonOut { + out := tokenGetOutput{ + AccessToken: tok.AccessToken, + RefreshToken: tok.RefreshToken, + TokenType: tok.TokenType, + ExpiresAt: tok.ExpiresAt, + Expired: time.Now().After(tok.ExpiresAt), + ClientID: tok.ClientID, + } + enc := json.NewEncoder(stdout) + enc.SetIndent("", " ") + _ = enc.Encode(out) + return 0 + } + fmt.Fprintln(stdout, tok.AccessToken) + return 0 +} diff --git a/token_cmd_test.go b/token_cmd_test.go new file mode 100644 index 0000000..67d413e --- /dev/null +++ b/token_cmd_test.go @@ -0,0 +1,94 @@ +package main + +import ( + "bytes" + "encoding/json" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/go-authgate/sdk-go/credstore" +) + +func TestRunTokenGet(t *testing.T) { + tests := []struct { + name string + setup func(credstore.Store[credstore.Token]) + jsonOut bool + wantCode int + checkOut func(t *testing.T, out string) + checkErr func(t *testing.T, errOut string) + }{ + { + name: "token found plain output", + setup: func(s credstore.Store[credstore.Token]) { + _ = s.Save("test-id", credstore.Token{ + AccessToken: "my-access-token", + ExpiresAt: time.Now().Add(time.Hour), + ClientID: "test-id", + }) + }, + jsonOut: false, + wantCode: 0, + checkOut: func(t *testing.T, out string) { + if out != "my-access-token\n" { + t.Errorf("got %q, want %q", out, "my-access-token\n") + } + }, + }, + { + name: "token found json output", + setup: func(s credstore.Store[credstore.Token]) { + _ = s.Save("test-id", credstore.Token{ + AccessToken: "my-access-token", + ExpiresAt: time.Now().Add(time.Hour), + ClientID: "test-id", + }) + }, + jsonOut: true, + wantCode: 0, + checkOut: func(t *testing.T, out string) { + var result tokenGetOutput + if err := json.Unmarshal([]byte(out), &result); err != nil { + t.Fatalf("invalid JSON: %v", err) + } + if result.AccessToken != "my-access-token" { + t.Errorf("access_token: got %q", result.AccessToken) + } + if result.Expired { + t.Error("expected expired=false") + } + }, + }, + { + name: "no token stored", + setup: func(s credstore.Store[credstore.Token]) {}, + jsonOut: false, + wantCode: 1, + checkErr: func(t *testing.T, errOut string) { + if !strings.Contains(errOut, "no stored token") { + t.Errorf("expected 'no stored token' in stderr, got: %q", errOut) + } + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + store := credstore.NewTokenFileStore(filepath.Join(t.TempDir(), "tokens.json")) + tc.setup(store) + var stdout, stderr bytes.Buffer + code := runTokenGet(store, "test-id", tc.jsonOut, &stdout, &stderr) + if code != tc.wantCode { + t.Errorf("exit code: got %d, want %d", code, tc.wantCode) + } + if tc.checkOut != nil { + tc.checkOut(t, stdout.String()) + } + if tc.checkErr != nil { + tc.checkErr(t, stderr.String()) + } + }) + } +} diff --git a/tui/token_view.go b/tui/token_view.go index e893d8a..9cdb806 100644 --- a/tui/token_view.go +++ b/tui/token_view.go @@ -142,12 +142,11 @@ func getTokenFilePath() string { // formatStorageLocation returns a human-readable storage location from the backend string. // The backend string is in the form "keyring: " or "file: ". func formatStorageLocation(backend string) string { - if strings.HasPrefix(backend, "keyring:") { - service := strings.TrimSpace(strings.TrimPrefix(backend, "keyring:")) - return "OS keyring (service: " + service + ")" + if service, ok := strings.CutPrefix(backend, "keyring:"); ok { + return "OS keyring (service: " + strings.TrimSpace(service) + ")" } - if strings.HasPrefix(backend, "file:") { - return strings.TrimSpace(strings.TrimPrefix(backend, "file:")) + if path, ok := strings.CutPrefix(backend, "file:"); ok { + return strings.TrimSpace(path) } if backend != "" { return backend From 0d6e509f12b4f05816eec6b872785459f562aba1 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Thu, 12 Mar 2026 18:05:09 +0800 Subject: [PATCH 2/3] ci: remove make generate step from testing workflow Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .github/workflows/testing.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 1908fbd..1c7bbaf 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -32,10 +32,6 @@ jobs: with: go-version: ${{ matrix.go }} - - name: Run Generate - run: | - make generate - - name: Setup golangci-lint uses: golangci/golangci-lint-action@v9 with: From f96e7fa109da7050b70cbf58ef76ac9b0eabf785 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Thu, 12 Mar 2026 18:28:56 +0800 Subject: [PATCH 3/3] fix(cli): address code review feedback on Cobra migration - Print Cobra errors to stderr in main() instead of silently exiting; introduce exitCodeError to propagate non-zero exit codes through Cobras error chain without printing a redundant message - Replace os.Exit calls inside RunE with exitCodeError returns so defers run correctly and commands remain testable - Add Args: cobra.NoArgs to token get to reject unexpected positional args - Handle JSON encode error in runTokenGet instead of ignoring it - Remove refresh_token from --json output to avoid leaking long-lived secrets into logs and shell history - Check Store.Save errors in tests with t.Fatalf for clearer diagnostics Co-Authored-By: Claude Sonnet 4.6 (1M context) --- main.go | 18 +++++++++++++++--- token_cmd.go | 45 +++++++++++++++++++++++++++------------------ token_cmd_test.go | 12 ++++++++---- 3 files changed, 50 insertions(+), 25 deletions(-) diff --git a/main.go b/main.go index 8d985f5..1eb39ca 100644 --- a/main.go +++ b/main.go @@ -21,8 +21,19 @@ import ( "github.com/spf13/cobra" ) +// exitCodeError carries a non-zero exit code through Cobra's error chain +// without printing a redundant message (the command already wrote to stderr). +type exitCodeError int + +func (e exitCodeError) Error() string { return "" } + func main() { if err := buildRootCmd().Execute(); err != nil { + var exitErr exitCodeError + if errors.As(err, &exitErr) { + os.Exit(int(exitErr)) + } + fmt.Fprintln(os.Stderr, err) os.Exit(1) } } @@ -38,9 +49,10 @@ func buildRootCmd() *cobra.Command { initConfig() uiManager := tui.SelectManager() ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) - exitCode := run(ctx, uiManager) - stop() - os.Exit(exitCode) + defer stop() + if code := run(ctx, uiManager); code != 0 { + return exitCodeError(code) + } return nil }, } diff --git a/token_cmd.go b/token_cmd.go index cbcd736..1882d0c 100644 --- a/token_cmd.go +++ b/token_cmd.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "io" - "os" "time" "github.com/go-authgate/sdk-go/credstore" @@ -13,12 +12,13 @@ import ( ) type tokenGetOutput struct { - AccessToken string `json:"access_token"` - RefreshToken string `json:"refresh_token"` - TokenType string `json:"token_type"` - ExpiresAt time.Time `json:"expires_at"` - Expired bool `json:"expired"` - ClientID string `json:"client_id"` + AccessToken string `json:"access_token"` + TokenType string `json:"token_type"` + ExpiresAt time.Time `json:"expires_at"` + Expired bool `json:"expired"` + ClientID string `json:"client_id"` + // RefreshToken is intentionally omitted — it is a long-lived secret + // that should not be casually printed to stdout or captured in logs. } func buildTokenCmd() *cobra.Command { @@ -35,16 +35,23 @@ func buildTokenGetCmd() *cobra.Command { cmd := &cobra.Command{ Use: "get", Short: "Print the stored access token", + Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { initStoreConfig() - code := runTokenGet(tokenStore, clientID, jsonOutput, os.Stdout, os.Stderr) - if code != 0 { - os.Exit(code) + if code := runTokenGet( + tokenStore, + clientID, + jsonOutput, + cmd.OutOrStdout(), + cmd.ErrOrStderr(), + ); code != 0 { + return exitCodeError(code) } return nil }, } - cmd.Flags().BoolVar(&jsonOutput, "json", false, "Output full token details as JSON") + cmd.Flags(). + BoolVar(&jsonOutput, "json", false, "Output token details as JSON (access_token, token_type, expires_at, expired, client_id)") return cmd } @@ -68,16 +75,18 @@ func runTokenGet( } if jsonOut { out := tokenGetOutput{ - AccessToken: tok.AccessToken, - RefreshToken: tok.RefreshToken, - TokenType: tok.TokenType, - ExpiresAt: tok.ExpiresAt, - Expired: time.Now().After(tok.ExpiresAt), - ClientID: tok.ClientID, + AccessToken: tok.AccessToken, + TokenType: tok.TokenType, + ExpiresAt: tok.ExpiresAt, + Expired: time.Now().After(tok.ExpiresAt), + ClientID: tok.ClientID, } enc := json.NewEncoder(stdout) enc.SetIndent("", " ") - _ = enc.Encode(out) + if err := enc.Encode(out); err != nil { + fmt.Fprintf(stderr, "Error: failed to write output: %v\n", err) + return 1 + } return 0 } fmt.Fprintln(stdout, tok.AccessToken) diff --git a/token_cmd_test.go b/token_cmd_test.go index 67d413e..1941e4f 100644 --- a/token_cmd_test.go +++ b/token_cmd_test.go @@ -23,11 +23,13 @@ func TestRunTokenGet(t *testing.T) { { name: "token found plain output", setup: func(s credstore.Store[credstore.Token]) { - _ = s.Save("test-id", credstore.Token{ + if err := s.Save("test-id", credstore.Token{ AccessToken: "my-access-token", ExpiresAt: time.Now().Add(time.Hour), ClientID: "test-id", - }) + }); err != nil { + t.Fatalf("setup: failed to save token: %v", err) + } }, jsonOut: false, wantCode: 0, @@ -40,11 +42,13 @@ func TestRunTokenGet(t *testing.T) { { name: "token found json output", setup: func(s credstore.Store[credstore.Token]) { - _ = s.Save("test-id", credstore.Token{ + if err := s.Save("test-id", credstore.Token{ AccessToken: "my-access-token", ExpiresAt: time.Now().Add(time.Hour), ClientID: "test-id", - }) + }); err != nil { + t.Fatalf("setup: failed to save token: %v", err) + } }, jsonOut: true, wantCode: 0,