From 786809d75013de7e727b151d8dfe123997ea7238 Mon Sep 17 00:00:00 2001 From: Axel Etcheverry Date: Sun, 23 Aug 2026 21:37:32 +0200 Subject: [PATCH] fix(oauth2): compute expires_in from the server clock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ServerConfig.Now is documented as the clock the server stamps issuance and expiry with, and every other site honoured it. writeTokenResponse did not: it measured the remaining lifetime with time.Until, i.e. against the wall clock, while ExpiresAt came from the injected clock. With a pinned clock the advertised expires_in stopped describing the token it shipped with, and a clock pinned far enough in the past put a negative lifetime on the wire — which RFC 6749 §5.1 does not allow. The stored expiry was right, so a client trusting expires_in refreshed at the wrong moment while introspection still reported the token as valid. serveToken now reads the clock once and passes that instant to both the grant and the response, so expires_in is exactly the configured TTL. The value is rounded to the nearest second — truncation advertised a 1h TTL as 3599 as soon as a millisecond elapsed between two clock reads — and an already-expired token drops the field instead of carrying a negative lifetime. Refs #64. --- CHANGELOG.md | 6 ++ oauth2/server.go | 5 +- oauth2/token_endpoint.go | 31 +++++- oauth2/token_endpoint_test.go | 178 ++++++++++++++++++++++++++++++++++ 4 files changed, 213 insertions(+), 7 deletions(-) create mode 100644 oauth2/token_endpoint_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b58fc40..8a533fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -95,6 +95,12 @@ legacy packages (`authentication/`, `authorization/`, the in-tree short-circuits on first success and aggregates failures. - The OAuth2 client-secret mismatch is now a typed error (`ErrClientSecretMismatch`) instead of a silent failure. +- The OAuth2 `/token` response now computes `expires_in` from + `ServerConfig.Now` instead of the wall clock, so an injected clock stays + authoritative on the wire; the value is rounded to the nearest second (a + 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. ### Removed diff --git a/oauth2/server.go b/oauth2/server.go index 57d36ad..dc08c7e 100644 --- a/oauth2/server.go +++ b/oauth2/server.go @@ -55,8 +55,9 @@ type ServerConfig struct { // trimmed ("/" yields a root mount). The .well-known endpoints are not // affected — they live at the host root per RFC 8615. RoutePrefix string - // Now is the clock used to stamp issuance / expiry. Defaults to - // time.Now (wall clock); inject a fixed clock in tests. + // Now is the clock used to stamp issuance / expiry, and to compute the + // expires_in the /token response advertises. Defaults to time.Now + // (wall clock); inject a fixed clock in tests. Now func() time.Time // OnError, when set, observes every error the server turns into an // RFC 6749 §5.2 response — including the cause of a server_error, which diff --git a/oauth2/token_endpoint.go b/oauth2/token_endpoint.go index a95640d..7b87041 100644 --- a/oauth2/token_endpoint.go +++ b/oauth2/token_endpoint.go @@ -66,12 +66,17 @@ func (s *Server) serveToken(w http.ResponseWriter, r *http.Request) { return } + // One clock read for the whole exchange: the grant stamps the expiry + // from it, and expires_in is measured against the very same instant, so + // the advertised lifetime is exactly the configured TTL. + now := s.cfg.Now() + resp, err := handler.Handle(r.Context(), GrantRequest{ Client: client, Form: r.PostForm, Issuer: issuer, Audience: audience, - Now: s.cfg.Now(), + Now: now, Profile: s.cfg.Profile, }) if err != nil { @@ -80,7 +85,7 @@ func (s *Server) serveToken(w http.ResponseWriter, r *http.Request) { return } - writeTokenResponse(w, resp) + writeTokenResponse(w, resp, now) } // tokenResponse is the on-wire JSON body per RFC 6749 §5.1. The @@ -96,12 +101,14 @@ type tokenResponse struct { } // writeTokenResponse serializes resp to the standard JSON body and adds -// Cache-Control / Pragma headers per RFC 6749 §5.1. -func writeTokenResponse(w http.ResponseWriter, resp *GrantResponse) { +// Cache-Control / Pragma headers per RFC 6749 §5.1. now is the instant the +// grant was stamped with — the server clock, never the wall clock, so an +// injected [ServerConfig.Now] stays authoritative on the wire too. +func writeTokenResponse(w http.ResponseWriter, resp *GrantResponse, now time.Time) { body := tokenResponse{ AccessToken: resp.Pair.Access.Token, TokenType: resp.TokenType, - ExpiresIn: int(time.Until(resp.Pair.Access.ExpiresAt).Seconds()), + ExpiresIn: expiresIn(resp.Pair.Access.ExpiresAt, now), Scope: resp.Scope, } @@ -122,6 +129,20 @@ func writeTokenResponse(w http.ResponseWriter, resp *GrantResponse) { } } +// expiresIn returns the RFC 6749 §5.1 expires_in value: the token lifetime +// in seconds, rounded to the nearest second so a whole-second TTL is +// advertised as itself rather than one short. An already-expired token +// yields 0 — the field is omitempty, so it drops off the wire instead of +// carrying a negative lifetime the RFC does not allow. +func expiresIn(expiresAt, now time.Time) int { + d := expiresAt.Sub(now) + if d <= 0 { + return 0 + } + + return int(d.Round(time.Second).Seconds()) +} + // errorResponse is the on-wire JSON body per RFC 6749 §5.2. type errorResponse struct { Error string `json:"error"` diff --git a/oauth2/token_endpoint_test.go b/oauth2/token_endpoint_test.go new file mode 100644 index 0000000..b411adb --- /dev/null +++ b/oauth2/token_endpoint_test.go @@ -0,0 +1,178 @@ +// 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/clientauth" + "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" +) + +// testAccessTTL is the access-token lifetime the clocked servers below are +// configured with; expires_in must advertise exactly this. +const testAccessTTL = time.Hour + +// fixedExpiryGrant mints a token whose expiry the test picks, so the +// expires_in edge cases can be driven without waiting for a clock. +type fixedExpiryGrant struct{ expiresAt time.Time } + +func (fixedExpiryGrant) Type() string { return "client_credentials" } + +func (g fixedExpiryGrant) Handle(_ context.Context, req oauth2.GrantRequest) (*oauth2.GrantResponse, error) { + return &oauth2.GrantResponse{ + Pair: oauth2.TokenPair{Access: oauth2.AccessToken{ + Token: "opaque-access-token", + TokenHash: oauth2.HashToken(nil, "opaque-access-token"), + ClientID: req.Client.ID(), + IssuedAt: req.Now, + ExpiresAt: g.expiresAt, + }}, + TokenType: oauth2.TokenTypeBearer, + }, nil +} + +// newClockedServer builds a /token-capable server pinned to a fixed clock. +// A nil now keeps the default wall clock; a nil grant keeps the real +// client_credentials handler. +func newClockedServer(t *testing.T, now func() time.Time, g oauth2.Grant) *oauth2.Server { + t.Helper() + + store := memory.New() + clients := &staticClientStore{clients: map[string]oauth2.Client{ + testClientID: &oauth2.DefaultClient{ + IDValue: testClientID, + Secret: testClientSecret, + TypeValue: oauth2.ClientConfidential, + }, + }} + + if g == nil { + g = grant.NewClientCredentials(grant.Config{ + Storage: store, + AccessTokens: token.NewOpaque(32), + AccessTTL: testAccessTTL, + }) + } + + srv, err := oauth2.NewServer(oauth2.ServerConfig{ + Profile: oauth2.Profile20BCP, + Storage: store, + ClientStore: clients, + IssuerResolver: oauth2.StaticIssuer("https://auth.example", "api"), + Grants: []oauth2.Grant{g}, + ClientAuth: []oauth2.ClientAuthenticator{clientauth.NewBasic()}, + Now: now, + }) + require.NoError(t, err) + + return srv +} + +// issueToken runs a client_credentials exchange and returns the decoded +// JSON body. +func issueToken(t *testing.T, srv *oauth2.Server) map[string]any { + t.Helper() + + rec := httptest.NewRecorder() + srv.TokenHandler().ServeHTTP(rec, formRequest("/token", + url.Values{"grant_type": {"client_credentials"}}, true)) + require.Equal(t, http.StatusOK, rec.Code) + + var body map[string]any + + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &body)) + + return body +} + +func TestTokenExpiresInHonorsServerClock(t *testing.T) { + t.Parallel() + + // A clock pinned well in the past: measuring against the wall clock + // would advertise a large negative lifetime. + pinned := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC) + + body := issueToken(t, newClockedServer(t, func() time.Time { return pinned }, nil)) + + assert.InDelta(t, testAccessTTL.Seconds(), body["expires_in"], 0) +} + +func TestTokenExpiresInMatchesTTLOnWallClock(t *testing.T) { + t.Parallel() + + // The default clock must advertise the configured TTL exactly, not one + // second short because a millisecond elapsed between two clock reads. + body := issueToken(t, newClockedServer(t, nil, nil)) + + assert.InDelta(t, testAccessTTL.Seconds(), body["expires_in"], 0) +} + +func TestTokenExpiresInRoundsToNearestSecond(t *testing.T) { + t.Parallel() + + pinned := time.Date(2026, 5, 20, 12, 0, 0, 0, time.UTC) + + cases := []struct { + name string + ttl time.Duration + want float64 + }{ + {"whole seconds", 90 * time.Second, 90}, + {"rounds up", 90*time.Second + 600*time.Millisecond, 91}, + {"rounds down", 90*time.Second + 400*time.Millisecond, 90}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + srv := newClockedServer(t, func() time.Time { return pinned }, + fixedExpiryGrant{expiresAt: pinned.Add(tc.ttl)}) + + assert.InDelta(t, tc.want, issueToken(t, srv)["expires_in"], 0) + }) + } +} + +func TestTokenExpiresInNeverNegative(t *testing.T) { + t.Parallel() + + pinned := time.Date(2026, 5, 20, 12, 0, 0, 0, time.UTC) + + cases := []struct { + name string + expiresAt time.Time + }{ + {"already expired", pinned.Add(-time.Hour)}, + {"expiring now", pinned}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + srv := newClockedServer(t, func() time.Time { return pinned }, + fixedExpiryGrant{expiresAt: tc.expiresAt}) + + // RFC 6749 §5.1: expires_in is a lifetime in seconds, so a + // negative value is not allowed. It drops off the wire instead. + body := issueToken(t, srv) + assert.NotContains(t, body, "expires_in") + assert.Equal(t, "opaque-access-token", body["access_token"]) + }) + } +}