Skip to content

Commit 3a06cc7

Browse files
appleboyclaude
andauthored
feat(cli): add configurable timeouts, OIDC discovery, and token revocation (#28)
## Summary - Add configurable timeouts (flag → env → default) for all OAuth operations with 10-minute cap - Resolve OAuth endpoints via OIDC Discovery with hardcoded fallback - Add server-side token revocation (RFC 7009) on `token delete` with `--local-only` option - Replace duplicate `ResolvedEndpoints` with `oauth.Endpoints` from SDK - Parallelize refresh/access token revocation with `sync.WaitGroup.Go` - Include client_id/client_secret in revocation requests per RFC 7009 - Cap `MAX_RESPONSE_BODY_SIZE` at 100MB to prevent OOM Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 2b9f5ae commit 3a06cc7

14 files changed

Lines changed: 854 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.

0 commit comments

Comments
 (0)