fix(oauth)!: authenticate and authorize clients on refresh grant

[appleboy]

Summary

Hardens the OAuth refresh_token grant — previously the weakest token path — by adding the three client/authorization gates the other grants already enforce, and by making requested scope-narrowing actually take effect. Addresses backlog items #3, #5, and #12.

#	Problem	Fix
#3	The grant never validated client_secret, only the public client_id. A leaked refresh token was replayable using only the (public) client_id, violating RFC 6749 §6.	Confidential clients must now present and pass a valid client_secret (HTTP Basic Auth or form body), matching authorization_code / client_credentials. Public (PKCE) clients keep authenticating by client_id only.
#5	The grant never checked client.IsActive(), so a disabled/rejected client could refresh forever.	Disabled clients are rejected with invalid_client.
#12	Requested scope narrowing was validated as a subset but never applied — the issued token kept the original (wider) scope.	The narrowed scope is now signed into the access token's scope claim. The rotated refresh token keeps the original grant scope, so a later refresh can re-request up to it (mirroring how RFC 8707 resource is already handled).

The client lookup also changes from a best-effort cached fetch (fail-open) to a fail-closed GetClientWithSecret — the secret and active status cannot be verified otherwise, so a missing/failed client load now rejects the refresh rather than silently falling back to provider defaults.

⚠️ Breaking changes

• Confidential clients must send client_secret on grant_type=refresh_token. Clients that previously refreshed without a secret will now receive 401 invalid_client. Update them to send their secret via HTTP Basic Auth or the client_secret form field. Public clients are unaffected.
• TokenProvider.RefreshAccessToken gains an accessScope parameter. Required so scope narrowing reaches the signed JWT rather than only the persisted row. Internal interface; affects custom TokenProvider implementations only.

Implementation notes

• internal/services/token_refresh.go — client authentication/authorization gate + scope narrowing.
• internal/token/local.go, internal/core/token.go — accessScope override threaded through the provider and interface.
• internal/handlers/token.go — refresh handler reads client credentials via the shared parseClientCredentials (Basic Auth + form).
• internal/util/string.go — small shared ScopeOrDefault helper for the "narrow-or-keep-original" convention, used by both the service and provider sites.
• internal/mocks/mock_token.go — regenerated for the new interface signature.

Verification

• New internal/handlers/token_refresh_test.go covers: scope-narrowing happy path (asserts the signed JWT scope, not just the DB row), rotation keeps original scope, missing/wrong secret → 401 invalid_client, disabled client → 401, and a public-client no-secret regression.
• make generate, make fmt, make lint (0 issues), and make test (all 14 packages) pass.

Follow-ups (out of scope, tracked in backlog)

• The client-auth gate is now inlined in three grant paths (authorization_code, client_credentials, refresh); extracting a shared gate is deferred to a separate refactor PR.

AI authorship

This change was implemented with Claude Code (Opus 4.8). It was planned (plan.md), implemented, reviewed by parallel cleanup agents, and verified against the test suite; the diff has been reviewed by a human before merge.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 44.44444% with 20 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/services/token_refresh.go	50.00%	8 Missing and 4 partials ⚠️
internal/mocks/mock_token.go	0.00%	4 Missing ⚠️
internal/util/string.go	0.00%	4 Missing ⚠️

📢 Thoughts on this report? Let us know!

[Copilot]

Pull request overview

Hardens the OAuth refresh_token grant by adding the client authentication, active-status, and scope-narrowing enforcement that other grants already had. The refresh path now fails closed on client load and threads a per-request accessScope through the TokenProvider interface so a narrowed scope actually reaches the signed JWT, not just the DB row.

Changes:

• Service-side: TokenService.RefreshAccessToken requires clientSecret, swaps the best-effort cached lookup for GetClientWithSecret, rejects disabled clients, validates confidential secrets, and signs the narrowed accessScope into the new access token while keeping the rotated refresh token's scope at the original grant.
• Provider/interface: core.TokenProvider.RefreshAccessToken and LocalTokenProvider.RefreshAccessToken gain an accessScope parameter (mocks regenerated, all call sites updated); new util.ScopeOrDefault helper.
• Handler/docs/tests: handleRefreshTokenGrant reads credentials via the shared parseClientCredentials; new internal/handlers/token_refresh_test.go covers narrowing-to-JWT, rotation-keeps-original-scope, missing/wrong/disabled confidential cases, and a public-client regression; docs/SECURITY.md documents the breaking change.

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
internal/services/token_refresh.go	Adds client auth/active gate, fail-closed lookup, scope narrowing applied to JWT and persisted row.
internal/handlers/token.go	Refresh handler parses Basic-Auth/form client credentials and passes clientSecret through.
internal/core/token.go, internal/token/local.go	RefreshAccessToken interface/impl gain accessScope override; doc comments updated.
internal/util/string.go	New ScopeOrDefault helper for narrow-or-keep-original convention.
internal/mocks/mock_token.go	Regenerated mock for new signature.
internal/handlers/token_refresh_test.go	New handler-level tests for narrowing, rotation, missing/wrong secret, disabled client, public client.
internal/services/token_*_test.go, internal/token/local*_test.go	Test call sites updated for the new clientSecret/accessScope parameters and the new fail-closed expectation (ErrAccessDenied).
docs/SECURITY.md	Documents new refresh authentication, disabled-client rejection, scope narrowing, and the breaking change.

Files not reviewed (1)

• internal/mocks/mock_token.go: Language not supported

---

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