Skip to content

feat: OAuth refresh token grant with configurable TTLs#49

Merged
danieljustus merged 1 commit into
mainfrom
issue/46-oauth-refresh-token-grant
May 15, 2026
Merged

feat: OAuth refresh token grant with configurable TTLs#49
danieljustus merged 1 commit into
mainfrom
issue/46-oauth-refresh-token-grant

Conversation

@danieljustus
Copy link
Copy Markdown
Owner

  • ScopedToken: add RefreshTokenHash and RefreshExpiresAt fields
  • TokenRegistry: add CreateWithRefresh() for access+refresh token pairs
  • TokenRegistry: add RotateViaRefreshToken() with single-use rotation
  • TokenRegistryFile: version 1->2 (backward-compatible Load)
  • handleOAuthToken: support grant_type=refresh_token
  • handleOAuthRegister/wellknown: advertise refresh_token support
  • Config: OAuthConfig with AccessTokenTTL (default 24h) and RefreshTokenTTL (default 720h/30d), file config + merge support
  • Full test coverage: refresh flow, rotation, expiry, config roundtrip

Fixes #46

What

Describe the change in a few sentences.

Why

Explain the problem, user need, or maintenance reason.

Testing

  • make fmt
  • make vet
  • make lint
  • GOWORK=off go test ./...
  • Added or updated tests where needed

Safety

  • I did not include passwords, tokens, private keys, vault contents, or personal data
  • Documentation was updated where behavior changed

- ScopedToken: add RefreshTokenHash and RefreshExpiresAt fields
- TokenRegistry: add CreateWithRefresh() for access+refresh token pairs
- TokenRegistry: add RotateViaRefreshToken() with single-use rotation
- TokenRegistryFile: version 1->2 (backward-compatible Load)
- handleOAuthToken: support grant_type=refresh_token
- handleOAuthRegister/wellknown: advertise refresh_token support
- Config: OAuthConfig with AccessTokenTTL (default 24h) and
  RefreshTokenTTL (default 720h/30d), file config + merge support
- Full test coverage: refresh flow, rotation, expiry, config roundtrip

Fixes #46
Copilot AI review requested due to automatic review settings May 15, 2026 10:07
@danieljustus danieljustus linked an issue May 15, 2026 that may be closed by this pull request
8 tasks
@danieljustus danieljustus merged commit 589ac73 into main May 15, 2026
10 of 11 checks passed
@danieljustus danieljustus deleted the issue/46-oauth-refresh-token-grant branch May 15, 2026 10:08
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

Adds OAuth 2.0 refresh-token grant support (RFC 6749 §6) to the MCP server. The token registry is extended with a refresh-token hash and refresh expiry, exposed through new CreateWithRefresh / RotateViaRefreshToken APIs (single-use rotation). The token endpoint, dynamic client registration response, and .well-known metadata are updated to advertise/handle refresh_token, and a new OAuthConfig block exposes configurable access/refresh TTLs (defaults 24h / 720h).

Changes:

  • Extend ScopedToken/TokenRegistryEntry with RefreshTokenHash and RefreshExpiresAt, bump registry file version 1→2, add CreateWithRefresh and RotateViaRefreshToken.
  • Wire grant_type=refresh_token into handleOAuthToken, plumb TTLs from MCPConfig.OAuth through RunHTTPServerOnListener, and advertise refresh_token in registration and well-known responses.
  • Introduce OAuthConfig/fileOAuthConfig with defaults and merge logic, plus tests for the OAuth flow and config roundtrip.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/mcp/token.go Adds refresh-token fields, lookup, creation, and single-use rotation; bumps file version to 2.
internal/mcp/token_test.go Loosens version assertions to accept 1 or 2.
internal/mcp/serverbootstrap/oauth.go Refactors handleOAuthToken into a grant-type switch and adds refresh_token handling; advertises new grant in registration.
internal/mcp/serverbootstrap/wellknown.go Adds refresh_token to grant_types_supported.
internal/mcp/serverbootstrap/http.go Reads OAuth TTLs from config and passes them to the token handler.
internal/mcp/serverbootstrap/oauth_refresh_test.go New tests for full refresh flow, expired-refresh denial, registration/well-known advertising, and error paths.
internal/config/schema.go Adds OAuthConfig, default TTLs, file-config pointer struct, and merge logic.
internal/config/schema_oauth_test.go New tests for OAuth defaults and merge behavior (including partial/no overrides).
Comments suppressed due to low confidence (3)

internal/mcp/token.go:446

  • After rotation, the new access and refresh TTLs are computed as time.Until(oldEntry.ExpiresAt) / time.Until(oldEntry.RefreshExpiresAt), i.e. the remaining lifetime of the old tokens. This causes refresh tokens to monotonically shrink with every rotation — after enough single-use rotations the refresh window collapses, requiring the user to re-run the full authorization-code flow even though they have been actively using the system. Most OAuth implementations either issue a fresh full-TTL refresh token on each rotation or implement a sliding window. Consider passing the configured accessTokenTTL / refreshTokenTTL into RotateViaRefreshToken (or storing the TTLs on the ScopedToken) so each rotation produces tokens with the intended lifetime.
	var accessTTL, refreshTTL time.Duration
	if oldEntry.ExpiresAt != nil {
		accessTTL = time.Until(*oldEntry.ExpiresAt)
		if accessTTL < 0 {
			accessTTL = 0
		}
	}
	if oldEntry.RefreshExpiresAt != nil {
		refreshTTL = time.Until(*oldEntry.RefreshExpiresAt)
		if refreshTTL < 0 {
			refreshTTL = 0
		}
	}

	newTok, rawAccess, rawRefresh, err := r.CreateWithRefresh(
		oldEntry.Label,
		oldEntry.AllowedTools,
		oldEntry.AgentName,
		accessTTL,
		refreshTTL,
	)

internal/mcp/token.go:446

  • oldEntry.ExpiresAt and oldEntry.RefreshExpiresAt are read here after r.mu.Unlock() on line 424, while CreateWithRefresh is then called which re-acquires the lock and may concurrently mutate the entries map. Although the pointer values on this specific entry are unlikely to change (it was just revoked), reading shared registry state outside the lock is fragile and inconsistent with the surrounding code. Capture the needed values (expiresAt, refreshExpiresAt, label, allowedTools, agentName) into local variables before unlocking on line 424.
	oldEntry.Revoked = true
	now := time.Now().UTC()
	oldEntry.RevokedAt = &now

	r.mu.Unlock()

	var accessTTL, refreshTTL time.Duration
	if oldEntry.ExpiresAt != nil {
		accessTTL = time.Until(*oldEntry.ExpiresAt)
		if accessTTL < 0 {
			accessTTL = 0
		}
	}
	if oldEntry.RefreshExpiresAt != nil {
		refreshTTL = time.Until(*oldEntry.RefreshExpiresAt)
		if refreshTTL < 0 {
			refreshTTL = 0
		}
	}

	newTok, rawAccess, rawRefresh, err := r.CreateWithRefresh(
		oldEntry.Label,
		oldEntry.AllowedTools,
		oldEntry.AgentName,
		accessTTL,
		refreshTTL,
	)

internal/mcp/token.go:334

  • The refresh token hash is compared with a plain string == equality (t.RefreshTokenHash == refreshHash). The PR description and linked issue both call out "constant-time-compare der Hashes" as part of the in-scope work. Although both sides are SHA-256 hex digests (already a one-way function, so timing leakage of the hash itself has limited security value), to satisfy the stated acceptance criterion and to be defensive, use crypto/subtle.ConstantTimeCompare for the comparison.
func (r *TokenRegistry) getByRefreshTokenHash(refreshHash string) *ScopedToken {
	for _, t := range r.entries {
		if t.RefreshTokenHash == refreshHash && !t.Revoked && !t.IsExpired() {
			return t
		}
	}
	return nil
}

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

Comment thread internal/mcp/token.go
Comment on lines +325 to +334
// getByRefreshTokenHash looks up a token by its refresh token hash. Returns
// nil if not found, revoked, or expired.
func (r *TokenRegistry) getByRefreshTokenHash(refreshHash string) *ScopedToken {
for _, t := range r.entries {
if t.RefreshTokenHash == refreshHash && !t.Revoked && !t.IsExpired() {
return t
}
}
return nil
}
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.

feat: OAuth Refresh Token Grant with configurable TTLs

2 participants