Skip to content

Commit 63b8e12

Browse files
appleboyclaude
andcommitted
feat(cli): add configurable timeouts, OIDC discovery, and token revocation
- Make 7 hardcoded timeouts configurable via CLI flags and env vars with graceful fallback to defaults on invalid input - Integrate OIDC Discovery to auto-resolve OAuth endpoints from .well-known/openid-configuration with hardcoded path fallback - Add server-side token revocation (RFC 7009) to `token delete` with --local-only flag to skip remote revocation - Add getDurationConfig and getInt64Config config resolution helpers - Add ResolvedEndpoints struct and resolveEndpoints function - Add discovery_test.go with success and fallback scenario tests - Add revocation tests covering success, graceful degradation, and local-only mode Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 2b9f5ae commit 63b8e12

14 files changed

Lines changed: 777 additions & 58 deletions

auth.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func refreshAccessToken(
6363
cfg *AppConfig,
6464
refreshToken string,
6565
) (*credstore.Token, error) {
66-
ctx, cancel := context.WithTimeout(ctx, refreshTokenTimeout)
66+
ctx, cancel := context.WithTimeout(ctx, cfg.RefreshTokenTimeout)
6767
defer cancel()
6868

6969
data := url.Values{}
@@ -74,7 +74,7 @@ func refreshAccessToken(
7474
data.Set("client_secret", cfg.ClientSecret)
7575
}
7676

77-
tokenResp, err := doTokenExchange(ctx, cfg, cfg.ServerURL+"/oauth/token", data,
77+
tokenResp, err := doTokenExchange(ctx, cfg, cfg.Endpoints.TokenURL, data,
7878
func(errResp ErrorResponse, _ []byte) error {
7979
if errResp.Error == "invalid_grant" || errResp.Error == "invalid_token" {
8080
return ErrRefreshTokenExpired
@@ -101,18 +101,18 @@ func refreshAccessToken(
101101

102102
// verifyToken verifies an access token with the OAuth server.
103103
func verifyToken(ctx context.Context, cfg *AppConfig, accessToken string) (string, error) {
104-
ctx, cancel := context.WithTimeout(ctx, tokenVerificationTimeout)
104+
ctx, cancel := context.WithTimeout(ctx, cfg.TokenVerificationTimeout)
105105
defer cancel()
106106

107-
resp, err := cfg.RetryClient.Get(ctx, cfg.ServerURL+"/oauth/tokeninfo",
107+
resp, err := cfg.RetryClient.Get(ctx, cfg.Endpoints.TokenInfoURL,
108108
retry.WithHeader("Authorization", "Bearer "+accessToken),
109109
)
110110
if err != nil {
111111
return "", fmt.Errorf("request failed: %w", err)
112112
}
113113
defer resp.Body.Close()
114114

115-
body, err := readResponseBody(resp)
115+
body, err := readResponseBody(resp, cfg.MaxResponseBodySize)
116116
if err != nil {
117117
return "", err
118118
}
@@ -131,7 +131,7 @@ func makeAPICallWithAutoRefresh(
131131
storage *credstore.Token,
132132
ui tui.Manager,
133133
) error {
134-
resp, err := cfg.RetryClient.Get(ctx, cfg.ServerURL+"/oauth/tokeninfo",
134+
resp, err := cfg.RetryClient.Get(ctx, cfg.Endpoints.TokenInfoURL,
135135
retry.WithHeader("Authorization", "Bearer "+storage.AccessToken),
136136
)
137137
if err != nil {
@@ -156,7 +156,7 @@ func makeAPICallWithAutoRefresh(
156156

157157
ui.ShowStatus(tui.StatusUpdate{Event: tui.EventTokenRefreshedRetrying})
158158

159-
resp, err = cfg.RetryClient.Get(ctx, cfg.ServerURL+"/oauth/tokeninfo",
159+
resp, err = cfg.RetryClient.Get(ctx, cfg.Endpoints.TokenInfoURL,
160160
retry.WithHeader("Authorization", "Bearer "+storage.AccessToken),
161161
)
162162
if err != nil {
@@ -165,7 +165,7 @@ func makeAPICallWithAutoRefresh(
165165
defer resp.Body.Close()
166166
}
167167

168-
body, err := readResponseBody(resp)
168+
body, err := readResponseBody(resp, cfg.MaxResponseBodySize)
169169
if err != nil {
170170
return err
171171
}

browser_flow.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func buildAuthURL(cfg *AppConfig, state string, pkce *PKCEParams) string {
2121
params.Set("state", state)
2222
params.Set("code_challenge", pkce.Challenge)
2323
params.Set("code_challenge_method", pkce.Method)
24-
return cfg.ServerURL + "/oauth/authorize?" + params.Encode()
24+
return cfg.Endpoints.AuthorizeURL + "?" + params.Encode()
2525
}
2626

2727
// exchangeCode exchanges an authorization code for access + refresh tokens.
@@ -30,7 +30,7 @@ func exchangeCode(
3030
cfg *AppConfig,
3131
code, codeVerifier string,
3232
) (*credstore.Token, error) {
33-
ctx, cancel := context.WithTimeout(ctx, tokenExchangeTimeout)
33+
ctx, cancel := context.WithTimeout(ctx, cfg.TokenExchangeTimeout)
3434
defer cancel()
3535

3636
data := url.Values{}
@@ -44,7 +44,7 @@ func exchangeCode(
4444
data.Set("client_secret", cfg.ClientSecret)
4545
}
4646

47-
tokenResp, err := doTokenExchange(ctx, cfg, cfg.ServerURL+"/oauth/token", data, nil)
47+
tokenResp, err := doTokenExchange(ctx, cfg, cfg.Endpoints.TokenURL, data, nil)
4848
if err != nil {
4949
return nil, err
5050
}
@@ -144,7 +144,7 @@ func performBrowserFlowWithUpdates(
144144
return
145145
case <-ticker.C:
146146
elapsed := time.Since(startTime)
147-
progress := float64(elapsed) / float64(callbackTimeout)
147+
progress := float64(elapsed) / float64(cfg.CallbackTimeout)
148148
if progress > 1.0 {
149149
progress = 1.0
150150
}
@@ -153,7 +153,7 @@ func performBrowserFlowWithUpdates(
153153
Progress: progress,
154154
Data: map[string]any{
155155
"elapsed": elapsed,
156-
"timeout": callbackTimeout,
156+
"timeout": cfg.CallbackTimeout,
157157
},
158158
}
159159
select {
@@ -167,7 +167,7 @@ func performBrowserFlowWithUpdates(
167167
}
168168
}()
169169

170-
storage, err := startCallbackServer(ctx, cfg.CallbackPort, state,
170+
storage, err := startCallbackServer(ctx, cfg.CallbackPort, state, cfg.CallbackTimeout,
171171
func(callbackCtx context.Context, code string) (*credstore.Token, error) {
172172
updates <- tui.FlowUpdate{
173173
Type: tui.StepStart,

callback.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,7 @@ func sanitizeTokenExchangeError(_ error) string {
5151
return "Token exchange failed. Please try again."
5252
}
5353

54-
const (
55-
// callbackTimeout is how long we wait for the browser to deliver the code.
56-
callbackTimeout = 2 * time.Minute
57-
)
58-
59-
// ErrCallbackTimeout is returned when no browser callback is received within callbackTimeout.
54+
// ErrCallbackTimeout is returned when no browser callback is received within the callback timeout.
6055
// Callers can use errors.Is to distinguish a timeout from other authorization errors
6156
// and decide whether to fall back to Device Code Flow.
6257
var ErrCallbackTimeout = errors.New("browser authorization timed out")
@@ -76,6 +71,7 @@ type callbackResult struct {
7671
//
7772
// The server shuts itself down after the first request.
7873
func startCallbackServer(ctx context.Context, port int, expectedState string,
74+
cbTimeout time.Duration,
7975
exchangeFn func(context.Context, string) (*credstore.Token, error),
8076
) (*credstore.Token, error) {
8177
resultCh := make(chan callbackResult, 1)
@@ -158,7 +154,7 @@ func startCallbackServer(ctx context.Context, port int, expectedState string,
158154
_ = srv.Shutdown(shutdownCtx)
159155
}()
160156

161-
timer := time.NewTimer(callbackTimeout)
157+
timer := time.NewTimer(cbTimeout)
162158
defer timer.Stop()
163159

164160
select {
@@ -172,7 +168,7 @@ func startCallbackServer(ctx context.Context, port int, expectedState string,
172168
return result.Storage, nil
173169

174170
case <-timer.C:
175-
return nil, fmt.Errorf("%w after %s", ErrCallbackTimeout, callbackTimeout)
171+
return nil, fmt.Errorf("%w after %s", ErrCallbackTimeout, cbTimeout)
176172

177173
case <-ctx.Done():
178174
return nil, ctx.Err()

callback_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ func startCallbackServerAsync(
158158
t.Helper()
159159
ch := make(chan callbackServerResult, 1)
160160
go func() {
161-
storage, err := startCallbackServer(ctx, port, state, exchangeFn)
161+
storage, err := startCallbackServer(ctx, port, state, defaultCallbackTimeout, exchangeFn)
162162
ch <- callbackServerResult{storage, err}
163163
}()
164164
// Give the server a moment to bind.

config.go

Lines changed: 162 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package main
22

33
import (
4+
"context"
45
"crypto/tls"
56
"errors"
67
"fmt"
@@ -13,6 +14,7 @@ import (
1314
"time"
1415

1516
"github.com/go-authgate/sdk-go/credstore"
17+
"github.com/go-authgate/sdk-go/discovery"
1618

1719
retry "github.com/appleboy/go-httpretry"
1820
"github.com/google/uuid"
@@ -34,17 +36,38 @@ var (
3436
flagTokenFile string
3537
flagTokenStore string
3638
flagDevice bool
39+
40+
flagTokenExchangeTimeout string
41+
flagTokenVerificationTimeout string
42+
flagRefreshTokenTimeout string
43+
flagDeviceCodeRequestTimeout string
44+
flagCallbackTimeout string
45+
flagUserInfoTimeout string
46+
flagMaxResponseBodySize string
3747
)
3848

3949
const (
40-
tokenExchangeTimeout = 10 * time.Second
41-
tokenVerificationTimeout = 10 * time.Second
42-
refreshTokenTimeout = 10 * time.Second
43-
deviceCodeRequestTimeout = 10 * time.Second
44-
maxResponseBodySize = 1 * 1024 * 1024 // 1 MB — guards against oversized server responses
45-
defaultKeyringService = "authgate-cli"
50+
defaultTokenExchangeTimeout = 10 * time.Second
51+
defaultTokenVerificationTimeout = 10 * time.Second
52+
defaultRefreshTokenTimeout = 10 * time.Second
53+
defaultDeviceCodeRequestTimeout = 10 * time.Second
54+
defaultCallbackTimeout = 2 * time.Minute
55+
defaultUserInfoTimeout = 10 * time.Second
56+
defaultMaxResponseBodySize = 1 * 1024 * 1024 // 1 MB — guards against oversized server responses
57+
defaultKeyringService = "authgate-cli"
4658
)
4759

60+
// ResolvedEndpoints holds the absolute URLs for all OAuth endpoints.
61+
// Populated from OIDC Discovery or from hardcoded fallback paths.
62+
type ResolvedEndpoints struct {
63+
AuthorizeURL string
64+
TokenURL string
65+
DeviceAuthorizationURL string
66+
TokenInfoURL string
67+
UserinfoURL string
68+
RevocationURL string
69+
}
70+
4871
// AppConfig holds all resolved configuration for the CLI application.
4972
type AppConfig struct {
5073
ServerURL string
@@ -57,6 +80,20 @@ type AppConfig struct {
5780
TokenStoreMode string // "auto", "file", or "keyring"
5881
RetryClient *retry.Client
5982
Store credstore.Store[credstore.Token]
83+
84+
// Endpoints holds the resolved OAuth endpoint URLs.
85+
// Populated by resolveEndpoints after loadConfig.
86+
Endpoints ResolvedEndpoints
87+
88+
// Timeout configuration (resolved from flag → env → default).
89+
// Only populated by loadConfig; zero in loadStoreConfig paths.
90+
TokenExchangeTimeout time.Duration
91+
TokenVerificationTimeout time.Duration
92+
RefreshTokenTimeout time.Duration
93+
DeviceCodeRequestTimeout time.Duration
94+
CallbackTimeout time.Duration
95+
UserInfoTimeout time.Duration
96+
MaxResponseBodySize int64
6097
}
6198

6299
// IsPublicClient returns true when no client secret is configured —
@@ -85,6 +122,20 @@ func registerFlags(cmd *cobra.Command) {
85122
StringVar(&flagTokenStore, "token-store", "", "Token storage backend: auto, file, keyring (default: auto or TOKEN_STORE env)")
86123
cmd.PersistentFlags().
87124
BoolVar(&flagDevice, "device", false, "Force Device Code Flow (skip browser detection)")
125+
cmd.PersistentFlags().
126+
StringVar(&flagTokenExchangeTimeout, "token-exchange-timeout", "", "Timeout for token exchange requests (e.g. 10s, 1m)")
127+
cmd.PersistentFlags().
128+
StringVar(&flagTokenVerificationTimeout, "token-verification-timeout", "", "Timeout for token verification requests (e.g. 10s, 1m)")
129+
cmd.PersistentFlags().
130+
StringVar(&flagRefreshTokenTimeout, "refresh-token-timeout", "", "Timeout for token refresh requests (e.g. 10s, 1m)")
131+
cmd.PersistentFlags().
132+
StringVar(&flagDeviceCodeRequestTimeout, "device-code-request-timeout", "", "Timeout for device code requests (e.g. 10s, 1m)")
133+
cmd.PersistentFlags().
134+
StringVar(&flagCallbackTimeout, "callback-timeout", "", "Timeout waiting for browser callback (e.g. 2m, 5m)")
135+
cmd.PersistentFlags().
136+
StringVar(&flagUserInfoTimeout, "userinfo-timeout", "", "Timeout for UserInfo requests (e.g. 10s, 1m)")
137+
cmd.PersistentFlags().
138+
StringVar(&flagMaxResponseBodySize, "max-response-body-size", "", "Maximum response body size in bytes (e.g. 1048576)")
88139
}
89140

90141
// loadStoreConfig initialises only the token store and client ID — the minimum
@@ -180,6 +231,25 @@ func loadConfig() *AppConfig {
180231
panic(fmt.Sprintf("failed to create retry client: %v", err))
181232
}
182233

234+
// Resolve timeout configuration.
235+
cfg.TokenExchangeTimeout = getDurationConfig(
236+
flagTokenExchangeTimeout, "TOKEN_EXCHANGE_TIMEOUT", defaultTokenExchangeTimeout)
237+
cfg.TokenVerificationTimeout = getDurationConfig(
238+
flagTokenVerificationTimeout, "TOKEN_VERIFICATION_TIMEOUT", defaultTokenVerificationTimeout)
239+
cfg.RefreshTokenTimeout = getDurationConfig(
240+
flagRefreshTokenTimeout, "REFRESH_TOKEN_TIMEOUT", defaultRefreshTokenTimeout)
241+
cfg.DeviceCodeRequestTimeout = getDurationConfig(
242+
flagDeviceCodeRequestTimeout,
243+
"DEVICE_CODE_REQUEST_TIMEOUT",
244+
defaultDeviceCodeRequestTimeout,
245+
)
246+
cfg.CallbackTimeout = getDurationConfig(
247+
flagCallbackTimeout, "CALLBACK_TIMEOUT", defaultCallbackTimeout)
248+
cfg.UserInfoTimeout = getDurationConfig(
249+
flagUserInfoTimeout, "USERINFO_TIMEOUT", defaultUserInfoTimeout)
250+
cfg.MaxResponseBodySize = getInt64Config(
251+
flagMaxResponseBodySize, "MAX_RESPONSE_BODY_SIZE", defaultMaxResponseBodySize)
252+
183253
if cfg.TokenStoreMode == "auto" {
184254
if ss, ok := cfg.Store.(*credstore.SecureStore[credstore.Token]); ok && !ss.UseKeyring() {
185255
fmt.Fprintln(
@@ -239,6 +309,92 @@ func validateServerURL(rawURL string) error {
239309
return nil
240310
}
241311

312+
// defaultEndpoints returns hardcoded endpoint paths appended to serverURL.
313+
// Used as fallback when OIDC Discovery is unavailable.
314+
func defaultEndpoints(serverURL string) ResolvedEndpoints {
315+
return ResolvedEndpoints{
316+
AuthorizeURL: serverURL + "/oauth/authorize",
317+
TokenURL: serverURL + "/oauth/token",
318+
DeviceAuthorizationURL: serverURL + "/oauth/device/code",
319+
TokenInfoURL: serverURL + "/oauth/tokeninfo",
320+
UserinfoURL: serverURL + "/oauth/userinfo",
321+
RevocationURL: serverURL + "/oauth/revoke",
322+
}
323+
}
324+
325+
// resolveEndpoints attempts OIDC Discovery and falls back to hardcoded paths.
326+
func resolveEndpoints(ctx context.Context, cfg *AppConfig) {
327+
disco, err := discovery.NewClient(
328+
cfg.ServerURL,
329+
discovery.WithHTTPClient(cfg.RetryClient),
330+
)
331+
if err != nil {
332+
fmt.Fprintf(os.Stderr,
333+
"WARNING: OIDC Discovery init failed: %v (using default endpoints)\n", err)
334+
cfg.Endpoints = defaultEndpoints(cfg.ServerURL)
335+
return
336+
}
337+
338+
fetchCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
339+
defer cancel()
340+
341+
meta, err := disco.Fetch(fetchCtx)
342+
if err != nil {
343+
fmt.Fprintf(os.Stderr,
344+
"WARNING: OIDC Discovery fetch failed: %v (using default endpoints)\n", err)
345+
cfg.Endpoints = defaultEndpoints(cfg.ServerURL)
346+
return
347+
}
348+
349+
ep := meta.Endpoints()
350+
cfg.Endpoints = ResolvedEndpoints{
351+
AuthorizeURL: ep.AuthorizeURL,
352+
TokenURL: ep.TokenURL,
353+
DeviceAuthorizationURL: ep.DeviceAuthorizationURL,
354+
TokenInfoURL: ep.TokenInfoURL,
355+
UserinfoURL: ep.UserinfoURL,
356+
RevocationURL: ep.RevocationURL,
357+
}
358+
}
359+
360+
// getDurationConfig resolves a time.Duration from flag → env → default.
361+
// The value is parsed with time.ParseDuration (e.g. "10s", "2m", "1m30s").
362+
// On parse error or non-positive value, it falls back to the default and prints a warning.
363+
func getDurationConfig(flagValue, envKey string, defaultValue time.Duration) time.Duration {
364+
raw := getConfig(flagValue, envKey, "")
365+
if raw == "" {
366+
return defaultValue
367+
}
368+
d, err := time.ParseDuration(raw)
369+
if err != nil {
370+
fmt.Fprintf(os.Stderr, "WARNING: invalid duration %q for %s, using default %s\n",
371+
raw, envKey, defaultValue)
372+
return defaultValue
373+
}
374+
if d <= 0 {
375+
fmt.Fprintf(os.Stderr, "WARNING: %s must be positive, got %s, using default %s\n",
376+
envKey, d, defaultValue)
377+
return defaultValue
378+
}
379+
return d
380+
}
381+
382+
// getInt64Config resolves an int64 from flag → env → default.
383+
// On parse error or non-positive value, it falls back to the default and prints a warning.
384+
func getInt64Config(flagValue, envKey string, defaultValue int64) int64 {
385+
raw := getConfig(flagValue, envKey, "")
386+
if raw == "" {
387+
return defaultValue
388+
}
389+
v, err := strconv.ParseInt(raw, 10, 64)
390+
if err != nil || v <= 0 {
391+
fmt.Fprintf(os.Stderr, "WARNING: invalid value %q for %s, using default %d\n",
392+
raw, envKey, defaultValue)
393+
return defaultValue
394+
}
395+
return v
396+
}
397+
242398
// getVersion returns the build version, preferring the ldflags-injected value
243399
// and falling back to debug.ReadBuildInfo().
244400
func getVersion() string {

0 commit comments

Comments
 (0)