From e82b4441c2576a6f888683854117d597c7ce18fe Mon Sep 17 00:00:00 2001 From: Axel Etcheverry Date: Sat, 10 Oct 2026 01:40:50 +0200 Subject: [PATCH 1/4] fix(oauth2): force refresh-token rotation under the BCP and 2.1 profiles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Profile20BCP, the zero value, and Profile21Draft advertise mandatory refresh-token rotation, yet the refresh_token grant rotated only when grant.Config.RotateRefreshTokens was set and a RefreshTokens generator was present; Profile.RequiresRefreshRotation had no caller outside a test. The zero configuration therefore answered a refresh with a new access token, no replacement refresh token, and a presented token that stayed live, so it could be replayed until it expired. With the flag set but no generator, even Profile20 dropped the requested rotation silently. The grant now decides rotation before minting anything, the way authorization_code applies the profile to PKCE: the profile can only tighten RotateRefreshTokens. Under BCP and 2.1 every exchange consumes the presented token and returns a new one in the same family, and a replayed token is refused with invalid_grant and revokes the family (RFC 9700 §4.14.2). A rotation that cannot be honored, for want of a RefreshTokens generator, fails with server_error carrying the new grant.ErrRotationRequiresGenerator as cause, before any token is generated or persisted. The guard runs after request validation, so a misconfigured server still answers invalid or replayed tokens precisely and still revokes a reused family. Non-rotating refresh tokens remain an explicit Profile20 choice. The example drops its now-redundant RotateRefreshTokens, and the godoc, the security considerations and the CHANGELOG describe the mandate. Refs #78. --- CHANGELOG.md | 24 +++- docs/architecture.md | 4 +- docs/security-considerations.md | 19 ++- examples/oauth2/main.go | 18 +-- examples/oauth2/main_test.go | 40 +++++- oauth2/doc.go | 3 +- oauth2/grant/grant.go | 27 ++-- oauth2/grant/grant_more_test.go | 58 ++++++--- oauth2/grant/refresh_token.go | 47 ++++++- oauth2/grant/refresh_token_test.go | 159 ++++++++++++++++++++++++ oauth2/grant_contract.go | 4 +- oauth2/models.go | 14 ++- oauth2/profile.go | 9 +- oauth2/refresh_rotation_test.go | 192 +++++++++++++++++++++++++++++ 14 files changed, 558 insertions(+), 60 deletions(-) create mode 100644 oauth2/grant/refresh_token_test.go create mode 100644 oauth2/refresh_rotation_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a533fce..fecf44f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,8 +42,9 @@ legacy packages (`authentication/`, `authorization/`, the in-tree key rotation, `alg=none` and algorithm-confusion defenses, and a `bearer.TokenVerifier` adapter. - **OAuth2 server** (`oauth2`): `Profile` (2.0 / 2.0-BCP / 2.1-draft), - enforced at runtime on the grants (PKCE required, `plain` PKCE refused - under BCP / 2.1). Grants: `authorization_code` (PKCE), `client_credentials`, + enforced at runtime on the grants (PKCE required, `plain` PKCE refused, + refresh-token rotation forced under BCP / 2.1). Grants: + `authorization_code` (PKCE), `client_credentials`, `refresh_token` (rotation + reuse detection), and the opt-in legacy `password` grant (`grant.NewLegacyPassword`, refused outside `Profile20`). `client_secret_basic`/`_post`/`none` client authentication. Endpoints: @@ -65,6 +66,11 @@ legacy packages (`authentication/`, `authorization/`, the in-tree never puts on the wire — plus the errors it swallows on purpose, such as a best-effort revocation (RFC 7009 §2.2) or a family revocation that failed during reuse detection. Purely observational: the responses are unchanged. +- **OAuth2 refresh-rotation guard**: `grant.ErrRotationRequiresGenerator` + reports a `refresh_token` grant that has to rotate refresh tokens but has + no `grant.Config.RefreshTokens` generator to mint the replacement; it is + the cause of the `server_error` such a grant fails with, before any token + is minted or persisted. - **Observability**: OpenTelemetry spans emitted directly by the core, `httpsec`, `grpcsec`, `connectrpcsec`, `jwtsec`, and `session`. See [docs/observability.md](docs/observability.md). @@ -115,3 +121,17 @@ legacy packages (`authentication/`, `authorization/`, the in-tree a fixed, generic string per error code, so internal context (timestamps, package and authenticator names, consumer-supplied `TokenVerifier`/store errors) can no longer leak to clients. +- The OAuth2 `refresh_token` grant now enforces the refresh-token rotation + that `Profile20BCP` (the default) and `Profile21Draft` mandate (RFC 9700 + §4.14.2) — for every client, stricter than RFC 9700 §2.2.2, which only + requires it for public clients — whatever `grant.Config.RotateRefreshTokens` + says: every exchange consumes the presented refresh token and returns a + new one, and replaying a consumed token is refused with `invalid_grant` + and revokes the whole family. The zero configuration used to let the same + refresh token be replayed until it expired. Clients MUST keep the refresh + token returned by each exchange (RFC 6749 §6). A grant that has to rotate + but has no `grant.Config.RefreshTokens` generator — including under + `Profile20` with `RotateRefreshTokens` set, which used to skip the + rotation silently — fails with `server_error` before any token is minted + or persisted. Non-rotating refresh tokens remain an explicit `Profile20` + choice (`RotateRefreshTokens: false`). diff --git a/docs/architecture.md b/docs/architecture.md index 85f5ee0f..fd3b265a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -142,8 +142,8 @@ The `oauth2` module is an authorization server, not just a provider: - **`Profile`** — `Profile20`, `Profile20BCP` (default), `Profile21Draft`. The profile gates which grants and PKCE methods are allowed, and is - enforced at runtime on the grants — PKCE is required and the `plain` - transformation refused under BCP / 2.1. + enforced at runtime on the grants — PKCE is required, the `plain` + transformation refused, and refresh-token rotation forced under BCP / 2.1. - **Grants** — `authorization_code` (PKCE), `client_credentials`, `refresh_token` (rotation + reuse detection), plus the opt-in legacy `password` grant (`grant.NewLegacyPassword`, refused outside `Profile20`). diff --git a/docs/security-considerations.md b/docs/security-considerations.md index fa53c65f..2e346bb4 100644 --- a/docs/security-considerations.md +++ b/docs/security-considerations.md @@ -49,10 +49,21 @@ configurable clock skew, and resolves keys by `kid` against a JWKS provider - **PKCE** — `authorization_code` requires PKCE. `S256` is the only method allowed under `Profile21Draft`; `plain` is accepted (with a warning) only under the looser profiles. -- **Refresh-token rotation** — every refresh issues a new token and - invalidates the old one. Re-use of an already-rotated token is treated as - theft: the whole token family is revoked (`RotateRefreshToken` returns - `ErrRefreshTokenReused`). +- **Refresh-token rotation** — mandatory under `Profile20BCP` (the default) + and `Profile21Draft`, whatever `grant.Config.RotateRefreshTokens` says: + every refresh exchange consumes the presented token and issues a new one + in the same family (RFC 9700 §4.14.2). The profiles apply it to every + client, stricter than RFC 9700 §2.2.2, which requires rotation — or + sender-constrained refresh tokens, which this library does not issue — + for public clients only. A `refresh_token` grant that cannot rotate (no + `RefreshTokens` generator) fails with `server_error` before minting + anything. Re-use of an already-rotated token is treated as theft: the + whole family is revoked (`ErrRefreshTokenReused`). Clients MUST keep the + refresh token returned by each exchange (RFC 6749 §6) and must not + refresh concurrently with the same token, or they lose the family. Under + `Profile20` rotation is opt-in (`RotateRefreshTokens`); without it a + refresh token stays reusable until it expires, which RFC 9700 §2.2.2 + forbids for public clients. - **Token storage** — access tokens, refresh tokens, and authorization codes are stored **hashed only** (SHA-256, via `oauth2.HashToken`). The store never sees cleartext, so a database compromise does not yield diff --git a/examples/oauth2/main.go b/examples/oauth2/main.go index 0e3c2749..695c9310 100644 --- a/examples/oauth2/main.go +++ b/examples/oauth2/main.go @@ -125,15 +125,17 @@ func buildServer() (http.Handler, error) { }, }} - // Authorization server. + // Authorization server. Profile20BCP mandates refresh-token rotation: + // the refresh_token grant always mints the replacement with + // RefreshTokens. RotateRefreshTokens only matters under the legacy + // Profile20. gcfg := grant.Config{ - Storage: store, - AccessTokens: token.NewOpaque(32), - RefreshTokens: token.OpaqueRefreshAdapter{Opaque: token.NewOpaque(32)}, - AccessTTL: time.Hour, - RefreshTTL: 24 * time.Hour, - RotateRefreshTokens: true, - OnError: logOAuthError, + Storage: store, + AccessTokens: token.NewOpaque(32), + RefreshTokens: token.OpaqueRefreshAdapter{Opaque: token.NewOpaque(32)}, + AccessTTL: time.Hour, + RefreshTTL: 24 * time.Hour, + OnError: logOAuthError, } srv, err := oauth2.NewServer(oauth2.ServerConfig{ diff --git a/examples/oauth2/main_test.go b/examples/oauth2/main_test.go index 55474d8d..1be910bb 100644 --- a/examples/oauth2/main_test.go +++ b/examples/oauth2/main_test.go @@ -149,10 +149,48 @@ func TestExampleOAuth2AuthorizationCodeFlow(t *testing.T) { require.NoError(t, err) var tok struct { - AccessToken string `json:"access_token"` + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token"` } require.NoError(t, json.NewDecoder(resp.Body).Decode(&tok)) _ = resp.Body.Close() require.Equal(t, http.StatusOK, resp.StatusCode) assert.NotEmpty(t, tok.AccessToken) + require.NotEmpty(t, tok.RefreshToken) + + // 4. Profile20BCP mandates refresh-token rotation: the refresh exchange + // returns a replacement refresh token... + resp = refreshExchange(t, client, srv.URL, tok.RefreshToken) + + var rotated struct { + RefreshToken string `json:"refresh_token"` + } + require.NoError(t, json.NewDecoder(resp.Body).Decode(&rotated)) + _ = resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode) + require.NotEmpty(t, rotated.RefreshToken) + assert.NotEqual(t, tok.RefreshToken, rotated.RefreshToken) + + // 5. ...and replaying the consumed one is refused. + resp = refreshExchange(t, client, srv.URL, tok.RefreshToken) + _ = resp.Body.Close() + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) +} + +// refreshExchange POSTs a refresh_token grant for raw to the demo /token +// endpoint, authenticated as the demo client. +func refreshExchange(t *testing.T, client *http.Client, baseURL, raw string) *http.Response { + t.Helper() + + form := url.Values{"grant_type": {"refresh_token"}, "refresh_token": {raw}} + + req, err := http.NewRequest(http.MethodPost, baseURL+"/oauth2/token", strings.NewReader(form.Encode())) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.SetBasicAuth(demoClientID, demoClientSecret) + + resp, err := client.Do(req) + require.NoError(t, err) + + return resp } diff --git a/oauth2/doc.go b/oauth2/doc.go index 47f7ebba..0231fc0e 100644 --- a/oauth2/doc.go +++ b/oauth2/doc.go @@ -8,7 +8,8 @@ // - Server aggregates Profile, Storage, Grants, ClientAuth, IssuerResolver. // - Profile selects the security baseline (OAuth2.0, OAuth2.0-BCP, // OAuth2.1-draft). BCP is the recommended default and is enforced at -// runtime on the grants (PKCE required, "plain" PKCE refused). +// runtime on the grants (PKCE required, "plain" PKCE refused, +// refresh-token rotation forced). // - Endpoints: AuthorizeHandler runs the RFC 6749 §3.1 authorization // endpoint (authorization_code, and the opt-in legacy implicit flow); // TokenHandler, RevokeHandler, IntrospectHandler and MetadataHandler diff --git a/oauth2/grant/grant.go b/oauth2/grant/grant.go index c7718760..c706fbc6 100644 --- a/oauth2/grant/grant.go +++ b/oauth2/grant/grant.go @@ -11,7 +11,8 @@ // - authorization_code (with PKCE; PKCE is mandatory in // [oauth2.Profile20BCP] and [oauth2.Profile21Draft]) // - client_credentials -// - refresh_token (with rotation + reuse detection) +// - refresh_token (rotation + reuse detection; rotation is mandatory in +// [oauth2.Profile20BCP] and [oauth2.Profile21Draft]) // // Legacy grants (password, implicit) live behind explicit opt-in helpers // and are refused outside [oauth2.Profile20]. @@ -31,21 +32,29 @@ type Config struct { Storage oauth2.Storage // AccessTokens issues access tokens (opaque or JWT). AccessTokens token.AccessTokenGenerator - // RefreshTokens issues refresh tokens. Optional — when nil, the - // grant emits no refresh token. + // RefreshTokens issues refresh tokens. Optional for authorization_code + // and password, which emit no refresh token without it. Required by + // refresh_token whenever it rotates — always under + // [oauth2.Profile20BCP] and [oauth2.Profile21Draft] — since the + // replacement is minted with it; see [ErrRotationRequiresGenerator]. RefreshTokens token.RefreshTokenGenerator // AccessTTL is the access-token expiry window. AccessTTL time.Duration // RefreshTTL is the refresh-token expiry window. Honored when // RefreshTokens is non-nil. RefreshTTL time.Duration - // RequirePKCE forces PKCE on authorization_code; default in BCP/21 - // profiles. The authorization_code grant honors this independently - // of public-vs-confidential client type. + // RequirePKCE forces PKCE on authorization_code, independently of + // public-vs-confidential client type. [oauth2.Profile20BCP] and + // [oauth2.Profile21Draft] require PKCE whatever this field says. RequirePKCE bool - // RotateRefreshTokens emits a fresh refresh token on every - // /token?grant_type=refresh_token call and marks the old one - // consumed; reuse triggers family revocation. Default true in BCP/21. + // RotateRefreshTokens makes refresh_token issue a fresh refresh token + // on every exchange and atomically mark the presented one consumed; + // replaying a consumed token revokes the whole family (RFC 9700 + // §4.14.2). [oauth2.Profile20BCP] and [oauth2.Profile21Draft] mandate + // rotation whatever this field says, so it only matters under + // [oauth2.Profile20], where false keeps the legacy behavior: the + // refresh token stays reusable until it expires. Rotation requires + // RefreshTokens. RotateRefreshTokens bool // OnError, when set, observes the errors a grant swallows to keep the // protocol response intact — today, a family revocation that failed diff --git a/oauth2/grant/grant_more_test.go b/oauth2/grant/grant_more_test.go index 0d61ab36..db4facad 100644 --- a/oauth2/grant/grant_more_test.go +++ b/oauth2/grant/grant_more_test.go @@ -12,6 +12,7 @@ import ( "github.com/hyperscale-stack/security/oauth2" "github.com/hyperscale-stack/security/oauth2/grant" + "github.com/hyperscale-stack/security/oauth2/token" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -386,24 +387,47 @@ func TestRefreshTokenRefusesBroadenedScope(t *testing.T) { assert.Equal(t, oauth2.CodeInvalidScope, oauth2.IsCode(err)) } -func TestRefreshTokenWithoutRotation(t *testing.T) { +func TestRefreshTokenWithoutRotationUnderProfile20(t *testing.T) { t.Parallel() - store := newStore() - seedRefresh(t, store, "static-rt", "read:mail", time.Now().Add(time.Hour)) - - // RotateRefreshTokens defaults to false here: the grant issues a new - // access token but no replacement refresh token. - g := grant.NewRefreshToken(grant.Config{ - Storage: store, AccessTokens: newAccessGen(), RefreshTokens: newRefreshGen(), - AccessTTL: time.Hour, RefreshTTL: 24 * time.Hour, RotateRefreshTokens: false, - }) - - form := url.Values{} - form.Set("refresh_token", "static-rt") + cases := []struct { + name string + gen token.RefreshTokenGenerator + }{ + {"with a refresh generator", newRefreshGen()}, + {"without a refresh generator", nil}, + } - resp, err := g.Handle(context.Background(), grant.Request{Client: newClient(), Form: form, Now: time.Now()}) - require.NoError(t, err) - assert.NotEmpty(t, resp.Pair.Access.Token) - assert.Nil(t, resp.Pair.Refresh, "no rotation -> no new refresh token") + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + store := newStore() + seedRefresh(t, store, "static-rt", "read:mail", time.Now().Add(time.Hour)) + + // Rotation is opt-in under Profile20 only: with RotateRefreshTokens + // false the grant issues a new access token but no replacement + // refresh token, and the presented one stays usable. + g := grant.NewRefreshToken(grant.Config{ + Storage: store, AccessTokens: newAccessGen(), RefreshTokens: tc.gen, + AccessTTL: time.Hour, RefreshTTL: 24 * time.Hour, RotateRefreshTokens: false, + }) + + form := url.Values{} + form.Set("refresh_token", "static-rt") + + req := grant.Request{Client: newClient(), Form: form, Now: time.Now(), Profile: oauth2.Profile20} + + for range 2 { + resp, err := g.Handle(context.Background(), req) + require.NoError(t, err) + assert.NotEmpty(t, resp.Pair.Access.Token) + assert.Nil(t, resp.Pair.Refresh, "no rotation -> no new refresh token") + } + + rt, err := store.LookupRefreshToken(context.Background(), oauth2.HashToken(nil, "static-rt")) + require.NoError(t, err) + assert.False(t, rt.Consumed, "a non-rotating refresh never consumes the token") + }) + } } diff --git a/oauth2/grant/refresh_token.go b/oauth2/grant/refresh_token.go index 43667d37..5ca9c17a 100644 --- a/oauth2/grant/refresh_token.go +++ b/oauth2/grant/refresh_token.go @@ -12,8 +12,8 @@ import ( "github.com/hyperscale-stack/security/oauth2/token" ) -// RefreshToken implements RFC 6749 §6 with the OAuth 2.0 BCP §8.10 -// hardening (rotation + reuse detection). +// RefreshToken implements the RFC 6749 §6 refresh_token grant with the +// RFC 9700 §4.14.2 hardening: refresh-token rotation with reuse detection. // // The flow: // @@ -21,13 +21,24 @@ import ( // the family. // 2. Re-bind client (refresh.ClientID MUST match the authenticated client). // 3. Optionally narrow scope (RFC 6749 §6 forbids broadening). -// 4. Issue a fresh access token. -// 5. If RotateRefreshTokens, issue a fresh refresh token in the same -// family AND atomically mark the old one consumed. +// 4. Decide rotation before minting anything: mandatory under +// [oauth2.Profile20BCP] and [oauth2.Profile21Draft], opt-in via +// [Config.RotateRefreshTokens] under [oauth2.Profile20]. A rotation +// that cannot be honored fails with server_error +// ([ErrRotationRequiresGenerator]). +// 5. Issue a fresh access token. +// 6. When rotating, issue a fresh refresh token in the same family AND +// atomically mark the old one consumed. type RefreshToken struct { cfg Config } +// ErrRotationRequiresGenerator reports a refresh_token grant that has to +// rotate refresh tokens but has no [Config.RefreshTokens] generator to mint +// the replacement. [RefreshToken.Handle] then fails with server_error +// carrying it as cause, before any token is minted or persisted. +var ErrRotationRequiresGenerator = errors.New("oauth2/grant: refresh-token rotation requires a RefreshTokens generator") + // NewRefreshToken constructs the handler. func NewRefreshToken(cfg Config) *RefreshToken { if cfg.Storage == nil || cfg.AccessTokens == nil { @@ -87,7 +98,31 @@ func (g *RefreshToken) Handle(ctx context.Context, req Request) (*Response, erro return g.issueRotated(ctx, req, rt, scope) } +// rotates reports whether the exchange rotates the refresh token. The +// profile can only tighten [Config.RotateRefreshTokens], never relax it — +// the rule authorization_code applies to PKCE. Rotating needs a +// [Config.RefreshTokens] generator; without one it returns +// [ErrRotationRequiresGenerator]. +func (g *RefreshToken) rotates(p oauth2.Profile) (bool, error) { + if !g.cfg.RotateRefreshTokens && !p.RequiresRefreshRotation() { + return false, nil + } + + if g.cfg.RefreshTokens == nil { + return false, ErrRotationRequiresGenerator + } + + return true, nil +} + func (g *RefreshToken) issueRotated(ctx context.Context, req Request, old *oauth2.RefreshToken, scope string) (*Response, error) { + // Decided before anything is minted: a rotation that cannot be honored + // must not leave an access token behind. + rotate, err := g.rotates(req.Profile) + if err != nil { + return nil, oauth2.ErrServerError.WithCause(err) + } + expires := req.Now.Add(g.cfg.AccessTTL) atRaw, atHash, err := g.cfg.AccessTokens.Generate(ctx, token.AccessTokenClaims{ @@ -119,7 +154,7 @@ func (g *RefreshToken) issueRotated(ctx context.Context, req Request, old *oauth TokenType: oauth2.TokenTypeBearer, } - if !g.cfg.RotateRefreshTokens || g.cfg.RefreshTokens == nil { + if !rotate { return resp, nil } diff --git a/oauth2/grant/refresh_token_test.go b/oauth2/grant/refresh_token_test.go new file mode 100644 index 00000000..059ffd9a --- /dev/null +++ b/oauth2/grant/refresh_token_test.go @@ -0,0 +1,159 @@ +// Copyright 2026 Hyperscale. All rights reserved. +// Use of this source code is governed by a MIT +// license that can be found in the LICENSE file. + +package grant_test + +import ( + "context" + "net/url" + "testing" + "time" + + "github.com/hyperscale-stack/security/oauth2" + "github.com/hyperscale-stack/security/oauth2/grant" + "github.com/hyperscale-stack/security/oauth2/token" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// spyAccessGen is a [token.AccessTokenGenerator] counting its calls, so a +// test can prove that no access token was minted. +type spyAccessGen struct{ calls int } + +func (s *spyAccessGen) Generate(context.Context, token.AccessTokenClaims) (string, string, error) { + s.calls++ + + const raw = "spy-access-token" + + return raw, oauth2.HashToken(nil, raw), nil +} + +// refreshReq builds the /token request exchanging the raw refresh token +// under profile p. +func refreshReq(raw string, p oauth2.Profile) grant.Request { + return grant.Request{ + Client: newClient(), + Form: url.Values{"refresh_token": {raw}}, + Issuer: "https://auth.example", + Audience: "api", + Now: time.Now(), + Profile: p, + } +} + +func TestRefreshTokenRotationMandatedByProfile(t *testing.T) { + t.Parallel() + + for _, p := range []oauth2.Profile{oauth2.Profile20BCP, oauth2.Profile21Draft} { + t.Run(p.String(), func(t *testing.T) { + t.Parallel() + + ctx := t.Context() + store := newStore() + seedRefresh(t, store, "mandated-rt", "read:mail", time.Now().Add(time.Hour)) + + // RotateRefreshTokens is left false: the profile alone mandates + // the rotation. + g := grant.NewRefreshToken(grant.Config{ + Storage: store, AccessTokens: newAccessGen(), RefreshTokens: newRefreshGen(), + AccessTTL: time.Hour, RefreshTTL: 24 * time.Hour, + }) + + first, err := g.Handle(ctx, refreshReq("mandated-rt", p)) + require.NoError(t, err) + require.NotNil(t, first.Pair.Refresh, "the profile mandates a replacement refresh token") + assert.NotEqual(t, "mandated-rt", first.Pair.Refresh.Token) + assert.Equal(t, "fam-x", first.Pair.Refresh.FamilyID, "the replacement stays in the family") + + presented, err := store.LookupRefreshToken(ctx, oauth2.HashToken(nil, "mandated-rt")) + require.NoError(t, err) + assert.True(t, presented.Consumed, "the rotation consumes the presented token") + + // Replaying the presented token is reuse: refused, family revoked. + _, err = g.Handle(ctx, refreshReq("mandated-rt", p)) + require.ErrorIs(t, err, oauth2.ErrRefreshTokenReused) + assert.Equal(t, oauth2.CodeInvalidGrant, oauth2.IsCode(err)) + + rotated, err := store.LookupRefreshToken(ctx, first.Pair.Refresh.TokenHash) + require.NoError(t, err) + assert.True(t, rotated.Consumed, "the family revocation consumes the replacement too") + + _, err = store.LookupAccessToken(ctx, first.Pair.Access.TokenHash) + assert.Error(t, err, "the family revocation removes the access token of the rotation") + }) + } +} + +func TestRefreshTokenRotationWithoutGeneratorFailsBeforeIssuance(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + profile oauth2.Profile + rotate bool + }{ + {"bcp mandates rotation", oauth2.Profile20BCP, false}, + {"2.1 mandates rotation", oauth2.Profile21Draft, false}, + {"profile20 opts in", oauth2.Profile20, true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + ctx := t.Context() + store := newStore() + seedRefresh(t, store, "stranded-rt", "read:mail", time.Now().Add(time.Hour)) + + spy := &spyAccessGen{} + g := grant.NewRefreshToken(grant.Config{ + Storage: store, AccessTokens: spy, // no RefreshTokens generator + AccessTTL: time.Hour, RefreshTTL: 24 * time.Hour, RotateRefreshTokens: tc.rotate, + }) + + resp, err := g.Handle(ctx, refreshReq("stranded-rt", tc.profile)) + assert.Nil(t, resp) + assert.ErrorIs(t, err, grant.ErrRotationRequiresGenerator) + assert.Equal(t, oauth2.CodeServerError, oauth2.IsCode(err)) + assert.Zero(t, spy.calls, "no access token is minted when the rotation cannot be honored") + + presented, err := store.LookupRefreshToken(ctx, oauth2.HashToken(nil, "stranded-rt")) + require.NoError(t, err) + assert.False(t, presented.Consumed, "the presented token is left untouched") + }) + } +} + +// TestRefreshTokenReuseDetectedWithoutGenerator pins where the rotation +// guard sits: after reuse detection. A grant that cannot rotate still +// answers a replayed token with invalid_grant — not server_error — and +// still revokes its family. +func TestRefreshTokenReuseDetectedWithoutGenerator(t *testing.T) { + t.Parallel() + + ctx := t.Context() + store := newStore() + req := consumedRefreshRequest(ctx, store) // family-1, Profile20BCP + + const sibling = "active-sibling" + + require.NoError(t, store.SaveRefreshToken(ctx, &oauth2.RefreshToken{ + Token: sibling, TokenHash: oauth2.HashToken(nil, sibling), + ClientID: clientID, Subject: subject, Scope: "read:mail", FamilyID: "family-1", + IssuedAt: req.Now, ExpiresAt: req.Now.Add(time.Hour), + })) + + g := grant.NewRefreshToken(grant.Config{ + Storage: store, AccessTokens: newAccessGen(), AccessTTL: time.Hour, // no RefreshTokens generator + }) + + resp, err := g.Handle(ctx, req) + assert.Nil(t, resp) + require.ErrorIs(t, err, oauth2.ErrRefreshTokenReused) + assert.Equal(t, oauth2.CodeInvalidGrant, oauth2.IsCode(err)) + + got, err := store.LookupRefreshToken(ctx, oauth2.HashToken(nil, sibling)) + require.NoError(t, err) + assert.True(t, got.Consumed, "reuse revokes the whole family") +} diff --git a/oauth2/grant_contract.go b/oauth2/grant_contract.go index bab8d2d5..f4f5fd11 100644 --- a/oauth2/grant_contract.go +++ b/oauth2/grant_contract.go @@ -21,8 +21,8 @@ type GrantRequest struct { Now time.Time // Profile is the server's active security profile. Grants use it to // enforce the profile-mandated rules at runtime (e.g. PKCE required, - // "plain" PKCE refused). A profile can only tighten a grant's own - // configuration, never loosen it. + // "plain" PKCE refused, refresh-token rotation forced). A profile can + // only tighten a grant's own configuration, never loosen it. Profile Profile } diff --git a/oauth2/models.go b/oauth2/models.go index d85ffc18..32abdd14 100644 --- a/oauth2/models.go +++ b/oauth2/models.go @@ -60,9 +60,9 @@ type AccessToken struct { Scope string IssuedAt time.Time ExpiresAt time.Time - // FamilyID identifies the token family this access token belongs to, - // used for refresh-token rotation and reuse detection. Empty when - // rotation is disabled. + // FamilyID identifies the refresh-token family the token belongs to, + // so revoking the family revokes it too. Empty for tokens issued + // outside a family (client_credentials, the implicit flow). FamilyID string // Audience is the configured aud claim (typically the resource server // identifier). Single-valued in this model; servers needing multi-aud @@ -100,9 +100,11 @@ func (t *RefreshToken) IsExpired(now time.Time) bool { return now.After(t.ExpiresAt) } -// TokenPair couples an access token with its companion refresh token (when -// rotation is enabled). The grant handlers return this; the response writer -// turns it into the RFC 6749 §5.1 JSON body. +// TokenPair couples an access token with the refresh token issued alongside +// it. Refresh is nil when the grant issues none: client_credentials, a grant +// without a RefreshTokens generator, or a non-rotating refresh under +// Profile20. The grant handlers return this; the response writer turns it +// into the RFC 6749 §5.1 JSON body. type TokenPair struct { Access AccessToken Refresh *RefreshToken diff --git a/oauth2/profile.go b/oauth2/profile.go index 8f06e501..de0ec185 100644 --- a/oauth2/profile.go +++ b/oauth2/profile.go @@ -8,7 +8,8 @@ package oauth2 // are supported: // // - Profile20 — vanilla RFC 6749. Allows implicit and password -// grants when explicitly registered; PKCE is opt-in. +// grants when explicitly registered; PKCE and refresh-token +// rotation are opt-in. // - Profile20BCP — IETF draft-ietf-oauth-security-topics ("OAuth 2.0 // Security Best Current Practice"). Refuses implicit // and password grants outright; mandates PKCE on @@ -52,7 +53,11 @@ func (p Profile) AllowsLegacyGrant() bool { return p == Profile20 } func (p Profile) RequiresPKCE() bool { return p != Profile20 } // RequiresRefreshRotation reports whether the profile mandates refresh- -// token rotation. True for BCP and 21-draft. +// token rotation as described in RFC 9700 §4.14.2, for every client — +// stricter than RFC 9700 §2.2.2, which only requires it (or +// sender-constrained refresh tokens) for public clients. True for BCP and +// 21-draft: the refresh_token grant then rotates whatever +// grant.Config.RotateRefreshTokens says. func (p Profile) RequiresRefreshRotation() bool { return p != Profile20 } // AllowsPKCEPlain reports whether the profile tolerates the "plain" PKCE diff --git a/oauth2/refresh_rotation_test.go b/oauth2/refresh_rotation_test.go new file mode 100644 index 00000000..a249c864 --- /dev/null +++ b/oauth2/refresh_rotation_test.go @@ -0,0 +1,192 @@ +// Copyright 2026 Hyperscale. All rights reserved. +// Use of this source code is governed by a MIT +// license that can be found in the LICENSE file. + +package oauth2_test + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "testing" + "time" + + "github.com/hyperscale-stack/security/oauth2" + "github.com/hyperscale-stack/security/oauth2/grant" + "github.com/hyperscale-stack/security/oauth2/storage/memory" + "github.com/hyperscale-stack/security/oauth2/token" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// spyAccessGen is a [token.AccessTokenGenerator] counting its calls, so a +// test can prove that no access token was minted. +type spyAccessGen struct{ calls int } + +func (s *spyAccessGen) Generate(context.Context, token.AccessTokenClaims) (string, string, error) { + s.calls++ + + const raw = "spy-access-token" + + return raw, oauth2.HashToken(nil, raw), nil +} + +// hiddenGrant wraps a grant the way a metrics or logging decorator would: +// it forwards Type and Handle but none of the optional capabilities the +// wrapped grant implements. +type hiddenGrant struct{ oauth2.Grant } + +// seedServerRefresh stores a live refresh token issued to the test client. +func seedServerRefresh(t *testing.T, store oauth2.Storage, raw string) { + t.Helper() + + require.NoError(t, store.SaveRefreshToken(t.Context(), &oauth2.RefreshToken{ + Token: raw, + TokenHash: oauth2.HashToken(nil, raw), + ClientID: testClientID, + Subject: "alice", + Scope: "api:read", + FamilyID: "family-rot", + IssuedAt: time.Now(), + ExpiresAt: time.Now().Add(time.Hour), + })) +} + +// refreshServerConfig is hookServerConfig serving the single grant g under +// profile p. +func refreshServerConfig(p oauth2.Profile, store oauth2.Storage, g oauth2.Grant) oauth2.ServerConfig { + cfg := hookServerConfig(store) + cfg.Profile = p + cfg.Grants = []oauth2.Grant{g} + + return cfg +} + +// refreshExchange POSTs a refresh_token grant for raw to /token. +func refreshExchange(t *testing.T, srv *oauth2.Server, raw string) *httptest.ResponseRecorder { + t.Helper() + + rec := httptest.NewRecorder() + srv.TokenHandler().ServeHTTP(rec, formRequest("/token", + url.Values{"grant_type": {"refresh_token"}, "refresh_token": {raw}}, true)) + + return rec +} + +// nonRotatingConfig is a refresh-grant configuration with a refresh-token +// generator but RotateRefreshTokens left false. +func nonRotatingConfig(store oauth2.Storage) grant.Config { + return grant.Config{ + Storage: store, + AccessTokens: token.NewOpaque(32), + RefreshTokens: token.OpaqueRefreshAdapter{Opaque: token.NewOpaque(32)}, + AccessTTL: time.Hour, + RefreshTTL: 24 * time.Hour, + } +} + +func TestTokenEndpointRefreshRotationMandatedByProfile(t *testing.T) { + t.Parallel() + + for _, p := range []oauth2.Profile{oauth2.Profile20BCP, oauth2.Profile21Draft} { + t.Run(p.String(), func(t *testing.T) { + t.Parallel() + + store := memory.New() + seedServerRefresh(t, store, "server-rt") + + srv, err := oauth2.NewServer(refreshServerConfig(p, store, + grant.NewRefreshToken(nonRotatingConfig(store)))) + require.NoError(t, err) + + rec := refreshExchange(t, srv, "server-rt") + require.Equal(t, http.StatusOK, rec.Code) + + var first struct { + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token"` + } + + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &first)) + require.NotEmpty(t, first.RefreshToken, "the profile mandates a replacement refresh token") + assert.NotEqual(t, "server-rt", first.RefreshToken) + + // Replaying the presented token is reuse: refused, family revoked. + rec = refreshExchange(t, srv, "server-rt") + assert.Equal(t, http.StatusBadRequest, rec.Code) + assert.Equal(t, oauth2.CodeInvalidGrant, decodeError(t, rec)) + + // The revocation took the replacement and its access token along. + rec = refreshExchange(t, srv, first.RefreshToken) + assert.Equal(t, http.StatusBadRequest, rec.Code) + assert.Equal(t, oauth2.CodeInvalidGrant, decodeError(t, rec)) + + _, err = store.LookupAccessToken(t.Context(), oauth2.HashToken(nil, first.AccessToken)) + assert.Error(t, err, "the family revocation removes the access token of the rotation") + }) + } +} + +func TestTokenEndpointRefreshWithoutRotationUnderProfile20(t *testing.T) { + t.Parallel() + + store := memory.New() + seedServerRefresh(t, store, "legacy-rt") + + srv, err := oauth2.NewServer(refreshServerConfig(oauth2.Profile20, store, + grant.NewRefreshToken(nonRotatingConfig(store)))) + require.NoError(t, err) + + // Rotation is opt-in under Profile20: the same token keeps working and + // no replacement is ever returned. + for range 2 { + rec := refreshExchange(t, srv, "legacy-rt") + require.Equal(t, http.StatusOK, rec.Code) + + var body map[string]any + + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &body)) + assert.NotEmpty(t, body["access_token"]) + assert.NotContains(t, body, "refresh_token") + } +} + +func TestTokenEndpointRefreshFailsClosedWhenRotationUnavailable(t *testing.T) { + t.Parallel() + + sink := &errorSink{} + store := memory.New() + seedServerRefresh(t, store, "stranded-rt") + + // No RefreshTokens generator under BCP, behind a decorator that hides + // the grant from any NewServer-time check: only the grant's runtime + // guard stands between the request and an access token. + spy := &spyAccessGen{} + cfg := refreshServerConfig(oauth2.Profile20BCP, store, hiddenGrant{grant.NewRefreshToken(grant.Config{ + Storage: store, AccessTokens: spy, AccessTTL: time.Hour, + })}) + cfg.OnError = sink.hook + + srv, err := oauth2.NewServer(cfg) + require.NoError(t, err) + + rec := refreshExchange(t, srv, "stranded-rt") + + // The wire only carries the generic server_error envelope... + assert.Equal(t, http.StatusInternalServerError, rec.Code) + assert.JSONEq(t, `{"error":"server_error","error_description":"internal server error"}`, rec.Body.String()) + + // ...the cause reaches the hook only... + got := sink.collected() + require.Len(t, got, 1) + assert.ErrorIs(t, got[0], grant.ErrRotationRequiresGenerator) + + // ...and nothing was minted or consumed. + assert.Zero(t, spy.calls, "no access token is minted when the rotation cannot be honored") + + presented, err := store.LookupRefreshToken(t.Context(), oauth2.HashToken(nil, "stranded-rt")) + require.NoError(t, err) + assert.False(t, presented.Consumed, "the presented token is left untouched") +} From cb82de23d112f20eda85de77d97c8cd6a19477ca Mon Sep 17 00:00:00 2001 From: Axel Etcheverry Date: Sat, 10 Oct 2026 01:44:00 +0200 Subject: [PATCH 2/4] feat(oauth2): refuse at NewServer a refresh grant that cannot rotate The refresh_token grant now fails closed when it has to rotate without a RefreshTokens generator, but a server wired that way still booted and only broke on the first refresh, answering server_error to every client. The mismatch between the grant and the profile is a configuration error, knowable at construction. oauth2.ProfileValidator is a new optional Grant capability, following the SecretMatcher pattern: NewServer calls ValidateProfile on every registered grant implementing it, under every profile, and refuses to build the server on error. The refresh_token grant implements it with the very rotation decision it applies at runtime, so the policy lives in one place, and a compile-time assertion keeps a renamed method from silently disabling the check. NewServer therefore rejects the grant without a generator under Profile20BCP and Profile21Draft, and under Profile20 when RotateRefreshTokens is set, wrapping grant.ErrRotationRequiresGenerator; the legacy-grant ban is unchanged. A grant hidden behind a decorator that does not forward ValidateProfile is not checked at construction; the grant's runtime guard still keeps the exchange closed, and the godoc asks decorators to forward the method. Refs #78. --- CHANGELOG.md | 20 +++++++----- docs/security-considerations.md | 13 ++++---- examples/oauth2/main.go | 4 +-- oauth2/grant/refresh_token.go | 18 ++++++++++- oauth2/grant/refresh_token_test.go | 50 ++++++++++++++++++++++++++++++ oauth2/grant_contract.go | 13 ++++++++ oauth2/profile.go | 3 +- oauth2/refresh_rotation_test.go | 44 ++++++++++++++++++++++++++ oauth2/server.go | 25 +++++++++------ 9 files changed, 164 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fecf44f4..b709bd1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,11 +66,15 @@ legacy packages (`authentication/`, `authorization/`, the in-tree never puts on the wire — plus the errors it swallows on purpose, such as a best-effort revocation (RFC 7009 §2.2) or a family revocation that failed during reuse detection. Purely observational: the responses are unchanged. -- **OAuth2 refresh-rotation guard**: `grant.ErrRotationRequiresGenerator` - reports a `refresh_token` grant that has to rotate refresh tokens but has - no `grant.Config.RefreshTokens` generator to mint the replacement; it is - the cause of the `server_error` such a grant fails with, before any token - is minted or persisted. +- **OAuth2 profile consistency checks**: `oauth2.ProfileValidator`, an + optional `Grant` capability: `oauth2.NewServer` asks every registered + grant implementing it whether it can honor the active `Profile`, and + refuses to build the server otherwise; the shipped `refresh_token` grant + implements it. `grant.ErrRotationRequiresGenerator` reports a + `refresh_token` grant that has to rotate refresh tokens but has no + `grant.Config.RefreshTokens` generator to mint the replacement — returned + by `NewServer`, and the cause of the `server_error` such a grant fails + with at runtime, before any token is minted or persisted. - **Observability**: OpenTelemetry spans emitted directly by the core, `httpsec`, `grpcsec`, `connectrpcsec`, `jwtsec`, and `session`. See [docs/observability.md](docs/observability.md). @@ -132,6 +136,8 @@ legacy packages (`authentication/`, `authorization/`, the in-tree token returned by each exchange (RFC 6749 §6). A grant that has to rotate but has no `grant.Config.RefreshTokens` generator — including under `Profile20` with `RotateRefreshTokens` set, which used to skip the - rotation silently — fails with `server_error` before any token is minted - or persisted. Non-rotating refresh tokens remain an explicit `Profile20` + rotation silently — is refused by `oauth2.NewServer`, and should it run + anyway (called directly, or behind a decorator that hides it from + `NewServer`) it fails with `server_error` before any token is minted or + persisted. Non-rotating refresh tokens remain an explicit `Profile20` choice (`RotateRefreshTokens: false`). diff --git a/docs/security-considerations.md b/docs/security-considerations.md index 2e346bb4..2f787c04 100644 --- a/docs/security-considerations.md +++ b/docs/security-considerations.md @@ -55,12 +55,13 @@ configurable clock skew, and resolves keys by `kid` against a JWKS provider in the same family (RFC 9700 §4.14.2). The profiles apply it to every client, stricter than RFC 9700 §2.2.2, which requires rotation — or sender-constrained refresh tokens, which this library does not issue — - for public clients only. A `refresh_token` grant that cannot rotate (no - `RefreshTokens` generator) fails with `server_error` before minting - anything. Re-use of an already-rotated token is treated as theft: the - whole family is revoked (`ErrRefreshTokenReused`). Clients MUST keep the - refresh token returned by each exchange (RFC 6749 §6) and must not - refresh concurrently with the same token, or they lose the family. Under + for public clients only. `NewServer` refuses a `refresh_token` grant + without a `RefreshTokens` generator, and the grant itself fails with + `server_error` before minting anything when it cannot rotate. Re-use of + an already-rotated token is treated as theft: the whole family is + revoked (`ErrRefreshTokenReused`). Clients MUST keep the refresh token + returned by each exchange (RFC 6749 §6) and must not refresh + concurrently with the same token, or they lose the family. Under `Profile20` rotation is opt-in (`RotateRefreshTokens`); without it a refresh token stays reusable until it expires, which RFC 9700 §2.2.2 forbids for public clients. diff --git a/examples/oauth2/main.go b/examples/oauth2/main.go index 695c9310..791db672 100644 --- a/examples/oauth2/main.go +++ b/examples/oauth2/main.go @@ -127,8 +127,8 @@ func buildServer() (http.Handler, error) { // Authorization server. Profile20BCP mandates refresh-token rotation: // the refresh_token grant always mints the replacement with - // RefreshTokens. RotateRefreshTokens only matters under the legacy - // Profile20. + // RefreshTokens, and NewServer refuses the wiring without it. + // RotateRefreshTokens only matters under the legacy Profile20. gcfg := grant.Config{ Storage: store, AccessTokens: token.NewOpaque(32), diff --git a/oauth2/grant/refresh_token.go b/oauth2/grant/refresh_token.go index 5ca9c17a..e662b971 100644 --- a/oauth2/grant/refresh_token.go +++ b/oauth2/grant/refresh_token.go @@ -35,10 +35,16 @@ type RefreshToken struct { // ErrRotationRequiresGenerator reports a refresh_token grant that has to // rotate refresh tokens but has no [Config.RefreshTokens] generator to mint -// the replacement. [RefreshToken.Handle] then fails with server_error +// the replacement. [oauth2.NewServer] refuses such a grant; should +// [RefreshToken.Handle] run anyway — called directly, or behind a decorator +// hiding [RefreshToken.ValidateProfile] — it fails with server_error // carrying it as cause, before any token is minted or persisted. var ErrRotationRequiresGenerator = errors.New("oauth2/grant: refresh-token rotation requires a RefreshTokens generator") +// Compile-time interface check: renaming ValidateProfile would otherwise +// silently drop the NewServer consistency check. +var _ oauth2.ProfileValidator = (*RefreshToken)(nil) + // NewRefreshToken constructs the handler. func NewRefreshToken(cfg Config) *RefreshToken { if cfg.Storage == nil || cfg.AccessTokens == nil { @@ -115,6 +121,16 @@ func (g *RefreshToken) rotates(p oauth2.Profile) (bool, error) { return true, nil } +// ValidateProfile implements [oauth2.ProfileValidator]: it returns +// [ErrRotationRequiresGenerator] when the grant would have to rotate under +// p without a [Config.RefreshTokens] generator, so [oauth2.NewServer] fails +// at boot rather than on the first refresh. +func (g *RefreshToken) ValidateProfile(p oauth2.Profile) error { + _, err := g.rotates(p) + + return err +} + func (g *RefreshToken) issueRotated(ctx context.Context, req Request, old *oauth2.RefreshToken, scope string) (*Response, error) { // Decided before anything is minted: a rotation that cannot be honored // must not leave an access token behind. diff --git a/oauth2/grant/refresh_token_test.go b/oauth2/grant/refresh_token_test.go index 059ffd9a..722b4c67 100644 --- a/oauth2/grant/refresh_token_test.go +++ b/oauth2/grant/refresh_token_test.go @@ -6,6 +6,7 @@ package grant_test import ( "context" + "fmt" "net/url" "testing" "time" @@ -125,6 +126,55 @@ func TestRefreshTokenRotationWithoutGeneratorFailsBeforeIssuance(t *testing.T) { } } +func TestRefreshTokenValidateProfile(t *testing.T) { + t.Parallel() + + // The grant cannot honor a profile iff it has to rotate — opted in, or + // mandated by any profile but Profile20 — without a refresh generator. + cases := []struct { + profile oauth2.Profile + rotate bool + withGen bool + wantErr bool + }{ + {oauth2.Profile20, false, false, false}, + {oauth2.Profile20, false, true, false}, + {oauth2.Profile20, true, false, true}, + {oauth2.Profile20, true, true, false}, + {oauth2.Profile20BCP, false, false, true}, + {oauth2.Profile20BCP, false, true, false}, + {oauth2.Profile20BCP, true, false, true}, + {oauth2.Profile20BCP, true, true, false}, + {oauth2.Profile21Draft, false, false, true}, + {oauth2.Profile21Draft, false, true, false}, + {oauth2.Profile21Draft, true, false, true}, + {oauth2.Profile21Draft, true, true, false}, + {oauth2.Profile(99), false, false, true}, // an unknown profile fails closed + } + + for _, tc := range cases { + name := fmt.Sprintf("%s/rotate=%t/generator=%t", tc.profile, tc.rotate, tc.withGen) + + t.Run(name, func(t *testing.T) { + t.Parallel() + + cfg := grant.Config{Storage: newStore(), AccessTokens: newAccessGen(), RotateRefreshTokens: tc.rotate} + if tc.withGen { + cfg.RefreshTokens = newRefreshGen() + } + + err := grant.NewRefreshToken(cfg).ValidateProfile(tc.profile) + if tc.wantErr { + assert.ErrorIs(t, err, grant.ErrRotationRequiresGenerator) + + return + } + + assert.NoError(t, err) + }) + } +} + // TestRefreshTokenReuseDetectedWithoutGenerator pins where the rotation // guard sits: after reuse detection. A grant that cannot rotate still // answers a replayed token with invalid_grant — not server_error — and diff --git a/oauth2/grant_contract.go b/oauth2/grant_contract.go index f4f5fd11..d60f189f 100644 --- a/oauth2/grant_contract.go +++ b/oauth2/grant_contract.go @@ -49,3 +49,16 @@ type Grant interface { // server then projects onto the OAuth2 JSON error envelope. Handle(ctx context.Context, req GrantRequest) (*GrantResponse, error) } + +// ProfileValidator is an optional [Grant] capability for grants whose +// configuration must agree with the server's [Profile]. [NewServer] calls +// ValidateProfile on every registered grant implementing it and refuses to +// build the server on error — e.g. a refresh_token grant that cannot rotate +// refresh tokens under [Profile20BCP] or [Profile21Draft]. It only moves +// the failure to boot: grants still enforce the profile at runtime through +// [GrantRequest.Profile]. Grant decorators should forward ValidateProfile; +// a grant that does not implement it is not checked at construction. +type ProfileValidator interface { + // ValidateProfile returns why the grant cannot honor p, or nil. + ValidateProfile(p Profile) error +} diff --git a/oauth2/profile.go b/oauth2/profile.go index de0ec185..4557a54c 100644 --- a/oauth2/profile.go +++ b/oauth2/profile.go @@ -57,7 +57,8 @@ func (p Profile) RequiresPKCE() bool { return p != Profile20 } // stricter than RFC 9700 §2.2.2, which only requires it (or // sender-constrained refresh tokens) for public clients. True for BCP and // 21-draft: the refresh_token grant then rotates whatever -// grant.Config.RotateRefreshTokens says. +// grant.Config.RotateRefreshTokens says, and [NewServer] refuses a refresh +// grant that cannot rotate. func (p Profile) RequiresRefreshRotation() bool { return p != Profile20 } // AllowsPKCEPlain reports whether the profile tolerates the "plain" PKCE diff --git a/oauth2/refresh_rotation_test.go b/oauth2/refresh_rotation_test.go index a249c864..58cc64fd 100644 --- a/oauth2/refresh_rotation_test.go +++ b/oauth2/refresh_rotation_test.go @@ -87,6 +87,50 @@ func nonRotatingConfig(store oauth2.Storage) grant.Config { } } +func TestNewServerRefusesRefreshGrantThatCannotRotate(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + profile oauth2.Profile + rotate bool + withGen bool + wantErr bool + }{ + {"bcp without generator", oauth2.Profile20BCP, false, false, true}, + {"2.1 without generator", oauth2.Profile21Draft, false, false, true}, + {"profile20 rotation without generator", oauth2.Profile20, true, false, true}, + {"bcp with generator", oauth2.Profile20BCP, false, true, false}, + {"profile20 without rotation nor generator", oauth2.Profile20, false, false, false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + store := memory.New() + gcfg := nonRotatingConfig(store) + gcfg.RotateRefreshTokens = tc.rotate + + if !tc.withGen { + gcfg.RefreshTokens = nil + } + + srv, err := oauth2.NewServer(refreshServerConfig(tc.profile, store, grant.NewRefreshToken(gcfg))) + if !tc.wantErr { + require.NoError(t, err) + assert.NotNil(t, srv) + + return + } + + assert.Nil(t, srv) + require.ErrorIs(t, err, grant.ErrRotationRequiresGenerator) + assert.ErrorContains(t, err, `grant "refresh_token"`) + }) + } +} + func TestTokenEndpointRefreshRotationMandatedByProfile(t *testing.T) { t.Parallel() diff --git a/oauth2/server.go b/oauth2/server.go index dc08c7e7..a41e2164 100644 --- a/oauth2/server.go +++ b/oauth2/server.go @@ -38,7 +38,8 @@ type ServerConfig struct { // Use [StaticIssuer] for single-tenant deployments. IssuerResolver IssuerResolver // Grants lists the grant_type handlers active on /token. The Server - // builds a dispatch map keyed on Grant.Type(). + // builds a dispatch map keyed on Grant.Type(). Grants implementing + // [ProfileValidator] are checked against Profile at construction. Grants []Grant // ClientAuth lists the client-authentication methods active on /token // (and /revoke, /introspect). The Server consults them in order and @@ -79,7 +80,8 @@ type Server struct { // NewServer validates cfg and returns a ready-to-mount [Server]. It // returns an error when the configuration is internally inconsistent -// (no storage, no client store, ...). +// (no storage, no client store, a grant the profile forbids or that +// rejects it through [ProfileValidator], ...). func NewServer(cfg ServerConfig) (*Server, error) { if cfg.Storage == nil { return nil, errors.New("oauth2: NewServer: Storage is required") @@ -164,17 +166,22 @@ func (s *Server) resolveIssuer(ctx context.Context, r *http.Request) (string, st return iss, aud, nil } -// profileConstraints enforces the profile-specific bans (e.g. legacy -// grants refused outside Profile20). +// profileConstraints enforces the profile at construction: the legacy +// grants are refused outside Profile20, and every grant implementing +// [ProfileValidator] must accept the profile. func profileConstraints(p Profile, grants []Grant) error { - if p.AllowsLegacyGrant() { - return nil - } - for _, g := range grants { switch g.Type() { case "password", "implicit": - return fmt.Errorf("oauth2: profile %s forbids grant %q", p, g.Type()) + if !p.AllowsLegacyGrant() { + return fmt.Errorf("oauth2: profile %s forbids grant %q", p, g.Type()) + } + } + + if v, ok := g.(ProfileValidator); ok { + if err := v.ValidateProfile(p); err != nil { + return fmt.Errorf("oauth2: NewServer: grant %q is inconsistent with profile %s: %w", g.Type(), p, err) + } } } From 370f6cf77d2805aa46071b7c3252ca18de4c2e82 Mon Sep 17 00:00:00 2001 From: Axel Etcheverry Date: Sat, 10 Oct 2026 01:45:03 +0200 Subject: [PATCH 3/4] fix(oauth2): keep the original scope on a rotated refresh token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC 6749 §6 requires the scope of a newly issued refresh token to be identical to that of the refresh token the client presented. The refresh_token grant minted the replacement with the scope of the request instead, so a client that narrowed one refresh permanently shrank its grant: the next exchange inherited the narrowed scope, and asking for the original scope again was refused with invalid_scope. Rotation being now the default under Profile20BCP and Profile21Draft, every narrowed refresh hit this path. The replacement token now carries the presented token's scope; narrowing only affects the access token issued by the exchange. Refs #78. --- CHANGELOG.md | 5 +++++ oauth2/grant/grant_more_test.go | 17 +++++++++++++++++ oauth2/grant/refresh_token.go | 9 ++++++--- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b709bd1d..886176b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -111,6 +111,11 @@ legacy packages (`authentication/`, `authorization/`, the in-tree 1h TTL is advertised as `3600`, not `3599`) and an already-expired token drops the field rather than sending the negative lifetime RFC 6749 §5.1 does not allow. +- A rotated OAuth2 refresh token keeps the scope of the token it replaces + (RFC 6749 §6); narrowing the scope of a refresh only narrows the issued + access token. The replacement used to inherit the narrowed scope, so the + grant shrank permanently and a later request for the original scope was + refused with `invalid_scope`. ### Removed diff --git a/oauth2/grant/grant_more_test.go b/oauth2/grant/grant_more_test.go index db4facad..3a875d31 100644 --- a/oauth2/grant/grant_more_test.go +++ b/oauth2/grant/grant_more_test.go @@ -366,6 +366,23 @@ func TestRefreshTokenNarrowsScope(t *testing.T) { resp, err := g.Handle(context.Background(), grant.Request{Client: newClient(), Form: form, Now: time.Now()}) require.NoError(t, err) assert.Equal(t, "read:mail", resp.Scope) + assert.Equal(t, "read:mail", resp.Pair.Access.Scope) + + // RFC 6749 §6: narrowing only affects the access token; the rotated + // refresh token keeps the scope of the one it replaces. + require.NotNil(t, resp.Pair.Refresh) + assert.Equal(t, "read:mail write:mail", resp.Pair.Refresh.Scope) + + stored, err := store.LookupRefreshToken(context.Background(), resp.Pair.Refresh.TokenHash) + require.NoError(t, err) + assert.Equal(t, "read:mail write:mail", stored.Scope) + + // So the next exchange without a scope gets the whole original grant back. + next, err := g.Handle(context.Background(), grant.Request{ + Client: newClient(), Form: url.Values{"refresh_token": {resp.Pair.Refresh.Token}}, Now: time.Now(), + }) + require.NoError(t, err) + assert.Equal(t, "read:mail write:mail", next.Scope) } func TestRefreshTokenRefusesBroadenedScope(t *testing.T) { diff --git a/oauth2/grant/refresh_token.go b/oauth2/grant/refresh_token.go index e662b971..1f1e2f42 100644 --- a/oauth2/grant/refresh_token.go +++ b/oauth2/grant/refresh_token.go @@ -27,8 +27,9 @@ import ( // that cannot be honored fails with server_error // ([ErrRotationRequiresGenerator]). // 5. Issue a fresh access token. -// 6. When rotating, issue a fresh refresh token in the same family AND -// atomically mark the old one consumed. +// 6. When rotating, issue a fresh refresh token in the same family with +// the presented token's scope (RFC 6749 §6) AND atomically mark the +// old one consumed. type RefreshToken struct { cfg Config } @@ -179,9 +180,11 @@ func (g *RefreshToken) issueRotated(ctx context.Context, req Request, old *oauth return nil, oauth2.ErrServerError.WithCause(err) } + // RFC 6749 §6: the new refresh token keeps the scope of the one it + // replaces; a narrowed request only narrows the access token. next := &oauth2.RefreshToken{ Token: rtRaw, TokenHash: rtHash, ClientID: req.Client.ID(), Subject: old.Subject, - Scope: scope, IssuedAt: req.Now, ExpiresAt: req.Now.Add(g.cfg.RefreshTTL), + Scope: old.Scope, IssuedAt: req.Now, ExpiresAt: req.Now.Add(g.cfg.RefreshTTL), FamilyID: old.FamilyID, } From d6a0767e9ab13691b07c1c0c7fe2fa8781a3e8e4 Mon Sep 17 00:00:00 2001 From: Axel Etcheverry Date: Sat, 10 Oct 2026 02:06:28 +0200 Subject: [PATCH 4/4] test(oauth2): pin opt-in rotation and every-grant profile checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two behaviors of the rotation change had no test that would notice a regression. An explicit RotateRefreshTokens opt-in under Profile20 must still rotate at runtime, yet the suite kept passing when rotation followed the profile alone or when Profile20 skipped it: the opt-in was only exercised without a generator, where it fails closed. NewServer must check every registered grant, yet the suite kept passing when the check stopped after the first grant or returned on the first validator that accepted the profile. New tests rotate under Profile20 with the opt-in, both at the grant and through the token endpoint: the replacement is returned, the presented token is consumed, and replaying it is refused as reuse. Another builds servers whose offending grant comes second, a legacy grant after a refresh grant that can rotate and a refresh grant that cannot rotate after client_credentials, and expects NewServer to refuse both. The CHANGELOG now names sender-constrained refresh tokens as the alternative RFC 9700 §2.2.2 allows for public clients, and the security considerations say that NewServer also refuses the grant under Profile20 when RotateRefreshTokens is set. Refs #78. --- CHANGELOG.md | 27 ++++++------ docs/security-considerations.md | 15 +++---- oauth2/grant/refresh_token_test.go | 27 ++++++++++++ oauth2/refresh_rotation_test.go | 67 ++++++++++++++++++++++++++++++ 4 files changed, 116 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 886176b1..edf887bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -133,16 +133,17 @@ legacy packages (`authentication/`, `authorization/`, the in-tree - The OAuth2 `refresh_token` grant now enforces the refresh-token rotation that `Profile20BCP` (the default) and `Profile21Draft` mandate (RFC 9700 §4.14.2) — for every client, stricter than RFC 9700 §2.2.2, which only - requires it for public clients — whatever `grant.Config.RotateRefreshTokens` - says: every exchange consumes the presented refresh token and returns a - new one, and replaying a consumed token is refused with `invalid_grant` - and revokes the whole family. The zero configuration used to let the same - refresh token be replayed until it expired. Clients MUST keep the refresh - token returned by each exchange (RFC 6749 §6). A grant that has to rotate - but has no `grant.Config.RefreshTokens` generator — including under - `Profile20` with `RotateRefreshTokens` set, which used to skip the - rotation silently — is refused by `oauth2.NewServer`, and should it run - anyway (called directly, or behind a decorator that hides it from - `NewServer`) it fails with `server_error` before any token is minted or - persisted. Non-rotating refresh tokens remain an explicit `Profile20` - choice (`RotateRefreshTokens: false`). + requires it (or sender-constrained refresh tokens) for public clients — + whatever `grant.Config.RotateRefreshTokens` says: every exchange consumes + the presented refresh token and returns a new one, and replaying a + consumed token is refused with `invalid_grant` and revokes the whole + family. The zero configuration used to let the same refresh token be + replayed until it expired. Clients MUST keep the refresh token returned + by each exchange (RFC 6749 §6). A grant that has to rotate but has no + `grant.Config.RefreshTokens` generator — including under `Profile20` + with `RotateRefreshTokens` set, which used to skip the rotation + silently — is refused by `oauth2.NewServer`, and should it run anyway + (called directly, or behind a decorator that hides it from `NewServer`) + it fails with `server_error` before any token is minted or persisted. + Non-rotating refresh tokens remain an explicit `Profile20` choice + (`RotateRefreshTokens: false`). diff --git a/docs/security-considerations.md b/docs/security-considerations.md index 2f787c04..a7e617bb 100644 --- a/docs/security-considerations.md +++ b/docs/security-considerations.md @@ -55,13 +55,14 @@ configurable clock skew, and resolves keys by `kid` against a JWKS provider in the same family (RFC 9700 §4.14.2). The profiles apply it to every client, stricter than RFC 9700 §2.2.2, which requires rotation — or sender-constrained refresh tokens, which this library does not issue — - for public clients only. `NewServer` refuses a `refresh_token` grant - without a `RefreshTokens` generator, and the grant itself fails with - `server_error` before minting anything when it cannot rotate. Re-use of - an already-rotated token is treated as theft: the whole family is - revoked (`ErrRefreshTokenReused`). Clients MUST keep the refresh token - returned by each exchange (RFC 6749 §6) and must not refresh - concurrently with the same token, or they lose the family. Under + for public clients only. Under these profiles — and under `Profile20` + when `RotateRefreshTokens` is set — `NewServer` refuses a + `refresh_token` grant without a `RefreshTokens` generator, and the grant + itself fails with `server_error` before minting anything when it cannot + rotate. Re-use of an already-rotated token is treated as theft: the + whole family is revoked (`ErrRefreshTokenReused`). Clients MUST keep the + refresh token returned by each exchange (RFC 6749 §6) and must not + refresh concurrently with the same token, or they lose the family. Under `Profile20` rotation is opt-in (`RotateRefreshTokens`); without it a refresh token stays reusable until it expires, which RFC 9700 §2.2.2 forbids for public clients. diff --git a/oauth2/grant/refresh_token_test.go b/oauth2/grant/refresh_token_test.go index 722b4c67..5e14f816 100644 --- a/oauth2/grant/refresh_token_test.go +++ b/oauth2/grant/refresh_token_test.go @@ -86,6 +86,33 @@ func TestRefreshTokenRotationMandatedByProfile(t *testing.T) { } } +func TestRefreshTokenRotationOptInUnderProfile20(t *testing.T) { + t.Parallel() + + ctx := t.Context() + store := newStore() + seedRefresh(t, store, "opt-in-rt", "read:mail", time.Now().Add(time.Hour)) + + // Profile20 mandates nothing: the rotation comes from the explicit opt-in. + g := grant.NewRefreshToken(grant.Config{ + Storage: store, AccessTokens: newAccessGen(), RefreshTokens: newRefreshGen(), + AccessTTL: time.Hour, RefreshTTL: 24 * time.Hour, RotateRefreshTokens: true, + }) + + first, err := g.Handle(ctx, refreshReq("opt-in-rt", oauth2.Profile20)) + require.NoError(t, err) + require.NotNil(t, first.Pair.Refresh, "the opt-in rotates under Profile20 too") + assert.NotEqual(t, "opt-in-rt", first.Pair.Refresh.Token) + + presented, err := store.LookupRefreshToken(ctx, oauth2.HashToken(nil, "opt-in-rt")) + require.NoError(t, err) + assert.True(t, presented.Consumed, "the rotation consumes the presented token") + + _, err = g.Handle(ctx, refreshReq("opt-in-rt", oauth2.Profile20)) + require.ErrorIs(t, err, oauth2.ErrRefreshTokenReused) + assert.Equal(t, oauth2.CodeInvalidGrant, oauth2.IsCode(err)) +} + func TestRefreshTokenRotationWithoutGeneratorFailsBeforeIssuance(t *testing.T) { t.Parallel() diff --git a/oauth2/refresh_rotation_test.go b/oauth2/refresh_rotation_test.go index 58cc64fd..51a4796b 100644 --- a/oauth2/refresh_rotation_test.go +++ b/oauth2/refresh_rotation_test.go @@ -131,6 +131,40 @@ func TestNewServerRefusesRefreshGrantThatCannotRotate(t *testing.T) { } } +// TestNewServerChecksEveryGrantAgainstProfile pins that NewServer checks +// every registered grant, whatever its position: neither a passing +// validator nor a grant without one ends the check early. +func TestNewServerChecksEveryGrantAgainstProfile(t *testing.T) { + t.Parallel() + + t.Run("legacy grant after a refresh grant that can rotate", func(t *testing.T) { + t.Parallel() + + store := memory.New() + cfg := refreshServerConfig(oauth2.Profile20BCP, store, grant.NewRefreshToken(nonRotatingConfig(store))) + cfg.Grants = append(cfg.Grants, legacyGrant{typ: "password"}) + + srv, err := oauth2.NewServer(cfg) + assert.Nil(t, srv) + assert.ErrorContains(t, err, `forbids grant "password"`) + }) + + t.Run("refresh grant that cannot rotate after a grant without validator", func(t *testing.T) { + t.Parallel() + + store := memory.New() + noGen := nonRotatingConfig(store) + noGen.RefreshTokens = nil + + cfg := refreshServerConfig(oauth2.Profile20BCP, store, grant.NewClientCredentials(noGen)) + cfg.Grants = append(cfg.Grants, grant.NewRefreshToken(noGen)) + + srv, err := oauth2.NewServer(cfg) + assert.Nil(t, srv) + assert.ErrorIs(t, err, grant.ErrRotationRequiresGenerator) + }) +} + func TestTokenEndpointRefreshRotationMandatedByProfile(t *testing.T) { t.Parallel() @@ -197,6 +231,39 @@ func TestTokenEndpointRefreshWithoutRotationUnderProfile20(t *testing.T) { } } +func TestTokenEndpointRefreshRotationOptInUnderProfile20(t *testing.T) { + t.Parallel() + + store := memory.New() + seedServerRefresh(t, store, "opt-in-rt") + + // Profile20 mandates nothing: the rotation comes from the explicit opt-in. + gcfg := nonRotatingConfig(store) + gcfg.RotateRefreshTokens = true + + srv, err := oauth2.NewServer(refreshServerConfig(oauth2.Profile20, store, grant.NewRefreshToken(gcfg))) + require.NoError(t, err) + + rec := refreshExchange(t, srv, "opt-in-rt") + require.Equal(t, http.StatusOK, rec.Code) + + var body struct { + RefreshToken string `json:"refresh_token"` + } + + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &body)) + require.NotEmpty(t, body.RefreshToken, "the opt-in rotates under Profile20 too") + assert.NotEqual(t, "opt-in-rt", body.RefreshToken) + + presented, err := store.LookupRefreshToken(t.Context(), oauth2.HashToken(nil, "opt-in-rt")) + require.NoError(t, err) + assert.True(t, presented.Consumed, "the rotation consumes the presented token") + + rec = refreshExchange(t, srv, "opt-in-rt") + assert.Equal(t, http.StatusBadRequest, rec.Code) + assert.Equal(t, oauth2.CodeInvalidGrant, decodeError(t, rec)) +} + func TestTokenEndpointRefreshFailsClosedWhenRotationUnavailable(t *testing.T) { t.Parallel()