Skip to content

feat(cli): add configurable timeouts, OIDC discovery, and token revocation#28

Merged
appleboy merged 5 commits intomainfrom
feat/configurable-timeouts-oidc-discovery-token-revocation
Apr 2, 2026
Merged

feat(cli): add configurable timeouts, OIDC discovery, and token revocation#28
appleboy merged 5 commits intomainfrom
feat/configurable-timeouts-oidc-discovery-token-revocation

Conversation

@appleboy
Copy link
Copy Markdown
Member

@appleboy appleboy commented Apr 1, 2026

Summary

  • Configurable Timeouts: 7 hardcoded timeout constants are now configurable via CLI flags (e.g. --callback-timeout 5m) and environment variables (e.g. CALLBACK_TIMEOUT=5m), with graceful fallback to defaults on invalid input
  • OIDC Discovery: OAuth endpoints are now auto-discovered from /.well-known/openid-configuration using the SDK's discovery package, with transparent fallback to hardcoded paths for servers that don't support OIDC Discovery
  • Token Revocation (RFC 7009): token delete now revokes tokens on the server before local deletion, with --local-only flag to skip server revocation and graceful degradation when the server is unreachable

Details

Configurable Timeouts

Env Var Flag Default
TOKEN_EXCHANGE_TIMEOUT --token-exchange-timeout 10s
TOKEN_VERIFICATION_TIMEOUT --token-verification-timeout 10s
REFRESH_TOKEN_TIMEOUT --refresh-token-timeout 10s
DEVICE_CODE_REQUEST_TIMEOUT --device-code-request-timeout 10s
CALLBACK_TIMEOUT --callback-timeout 2m
USERINFO_TIMEOUT --userinfo-timeout 10s
MAX_RESPONSE_BODY_SIZE --max-response-body-size 1048576

OIDC Discovery

  • Fetches endpoints from /.well-known/openid-configuration on startup
  • Falls back to hardcoded paths (/oauth/authorize, /oauth/token, etc.) if discovery fails
  • Replaced 10 hardcoded endpoint concatenations across 4 files

Token Revocation

  • Revokes refresh token first (long-lived), then access token
  • Server errors produce a warning but local deletion still proceeds (exit 0)
  • --local-only flag skips network calls entirely

Test plan

  • make test — all existing + new tests pass
  • make lint — 0 issues
  • New tests for getDurationConfig / getInt64Config helpers
  • New discovery_test.go with success + 3 fallback scenarios (404, timeout, network error)
  • New revocation tests: success, server error degradation, local-only, access-only token

🤖 Generated with Claude Code

…ation

- 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>
Copilot AI review requested due to automatic review settings April 1, 2026 17:30
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 1, 2026

Codecov Report

❌ Patch coverage is 53.11005% with 98 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
config.go 51.02% 47 Missing and 1 partial ⚠️
token_cmd.go 60.49% 28 Missing and 4 partials ⚠️
auth.go 25.00% 6 Missing ⚠️
browser_flow.go 16.66% 5 Missing ⚠️
userinfo.go 0.00% 3 Missing ⚠️
device_flow.go 71.42% 2 Missing ⚠️
callback.go 50.00% 1 Missing ⚠️
main.go 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enhances the CLI’s OAuth behavior by making network timeouts configurable, resolving OAuth/OIDC endpoints via OIDC Discovery (with fallback), and revoking tokens on the server during token delete before deleting them locally.

Changes:

  • Added CLI flag + env-var configuration for multiple request timeouts and maximum response body size, with parsing helpers and tests.
  • Introduced endpoint resolution via OIDC Discovery (/.well-known/openid-configuration) with fallback to hardcoded /oauth/* paths, and updated flows to use resolved endpoints.
  • Updated token delete to attempt RFC 7009 token revocation (with --local-only to skip), and added revocation-focused tests.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
config.go Adds configurable timeouts/body size, endpoint structs, and OIDC discovery-based endpoint resolution with fallback.
config_test.go Adds unit tests for duration/int64 config parsing helpers.
main.go Resolves endpoints at startup before running flows that rely on endpoint URLs.
main_test.go Updates test config construction to include resolved endpoints and new config fields.
discovery_test.go Adds tests for default endpoints and discovery success/fallback scenarios.
browser_flow.go Switches browser flow URLs to cfg.Endpoints.* and uses configurable callback/token-exchange timeouts.
callback.go Makes callback server timeout configurable via parameter (used by browser flow).
callback_test.go Updates callback server helper to pass the callback timeout parameter.
device_flow.go Switches device flow URLs to cfg.Endpoints.* and uses configurable timeouts/body size.
auth.go Switches token verification/token exchange to resolved endpoints and configurable timeouts/body size.
userinfo.go Uses cfg.Endpoints.UserinfoURL, configurable UserInfo timeout, and response body size limit.
tokens.go Makes readResponseBody accept a configurable max size and updates call sites.
token_cmd.go Adds server-side token revocation to token delete with --local-only option.
token_cmd_test.go Updates existing delete tests and adds revocation behavior tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

appleboy and others added 2 commits April 2, 2026 11:56
…outs

- Replace ResolvedEndpoints with oauth.Endpoints from SDK
- Simplify resolveEndpoints to assign meta.Endpoints directly
- Parallelize refresh and access token revocation with WaitGroup.Go
- Drain response body in doRevoke for proper HTTP connection reuse
- Accept 2xx status range in doRevoke instead of only 200
- Cap user-supplied timeout durations at 10 minutes
- Add configurable --discovery-timeout and --revocation-timeout flags

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Include client_id and client_secret in RFC 7009 revocation requests
- Return error from revokeTokenOnServer on refresh-only failure to
  prevent misleading "Token revoked on server." message
- Wire test server into local-only test config for meaningful assertion
- Assert TokenInfoURL in OIDC discovery success test

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

tokens.go:50

  • readResponseBody uses io.LimitReader, but it doesn’t detect when the response is truncated at maxSize. That can turn an oversized response into a confusing JSON parse/format error later. Consider reading maxSize+1 and returning a clear "response body too large" error when the limit is exceeded.
// readResponseBody reads the response body with a size limit to guard against oversized responses.
func readResponseBody(resp *http.Response, maxSize int64) ([]byte, error) {
	body, err := io.ReadAll(io.LimitReader(resp.Body, maxSize))
	if err != nil {
		return nil, fmt.Errorf("failed to read response body: %w", err)
	}
	return body, nil

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Limit io.Copy drain in doRevoke to 4KB to prevent unbounded reads
- Cap MAX_RESPONSE_BODY_SIZE at 100MB to prevent OOM via io.ReadAll

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@appleboy appleboy merged commit 3a06cc7 into main Apr 2, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants