From 38aeb2065fb26b92e1eefe9c7305efd1ecf57812 Mon Sep 17 00:00:00 2001 From: Rex Raphael Date: Tue, 13 Oct 2026 11:19:43 -0500 Subject: [PATCH] fix: park third-party OAuth2 JWTs until a scoped route admits them On an app configured for JWT access tokens, a token the OAuth2 provider issues to a client is now held back by the auth middleware until a route tagged with RequireScope admits it, the same as an opaque token. Before this, that token authenticated as the full user on every route, whatever scopes the client was granted. Userinfo also returned every claim, since it saw no session and took the caller for a first-party login. Parking keys off the session on the request context. The opaque path loads the session row and finds its ClientID there, but the JWT path loads no row, so a client's JWT never looked like a client's token. JWTs now carry the RFC 9068 client_id claim, and tryJWTAuth rebuilds a session from the claims when it's present, which parkOAuthSession then parks. Both places that mint a JWT set the claim: issueTokens in the plugin, and the engine's rotateSession, which the OAuth2 refresh grant goes through. Without the second one a token would escape again on its first refresh. Tokens minted before this change carry no client_id and keep full access until they expire. If you can't wait out one access token lifetime, revoke the outstanding OAuth2 grants when you deploy. Covered by TestOAuth2IssuedJWTIsParkedUntilAScopedRouteAdmitsIt, which issues a token through the real authorize and token endpoints, refreshes it, and presents both tokens to the engine's middleware. --- middleware/auth.go | 21 ++++ oauth2_jwt_scope_e2e_test.go | 167 +++++++++++++++++++++++++++++++ plugins/oauth2provider/plugin.go | 1 + service.go | 10 +- tokenformat/format.go | 8 ++ tokenformat/jwt.go | 4 + 6 files changed, 208 insertions(+), 3 deletions(-) create mode 100644 oauth2_jwt_scope_e2e_test.go diff --git a/middleware/auth.go b/middleware/auth.go index 53843c6e..ae0ebd49 100644 --- a/middleware/auth.go +++ b/middleware/auth.go @@ -707,6 +707,27 @@ func tryJWTAuth( } goCtx = WithUser(goCtx, u) + // A token issued to an OAuth2 client must be parked like its opaque + // counterpart, and parking keys off the session on the context. The + // opaque path gets that session from its row; this path loads no row, so + // rebuild the part parking and RequireScope read from the claims. Without + // it a third-party token authenticated as the user on every route. + if claims.ClientID != "" { + sess := &session.Session{ + AppID: appID, + UserID: subjectID, + ClientID: claims.ClientID, + Scopes: claims.Scopes, + Audience: claims.Audience, + DPoPJKT: claims.DPoPJKT, + ExpiresAt: claims.ExpiresAt, + } + if sid, sidOK := SessionIDFrom(goCtx); sidOK { + sess.ID = sid + } + goCtx = WithSession(goCtx, sess) + } + goCtx = parkOAuthSession(ctx.Context(), goCtx) ctx.WithContext(goCtx) return jwtAuthAuthenticated, nil diff --git a/oauth2_jwt_scope_e2e_test.go b/oauth2_jwt_scope_e2e_test.go new file mode 100644 index 00000000..796f555d --- /dev/null +++ b/oauth2_jwt_scope_e2e_test.go @@ -0,0 +1,167 @@ +package authsome_test + +// An OAuth2 access token is issued to a third party for a set of scopes, so +// the auth middleware parks it until a RequireScope route admits it. In +// opaque mode the middleware finds the session row, sees its ClientID and +// parks it. In JWT mode there is no row lookup, so the token itself has to +// say which client it belongs to. Before it did, a JWT minted by the +// /v1/oauth/token endpoint authenticated as the full user on every route. +// +// This test issues the token through the real grant and presents it to the +// engine's real middleware, because the bug lived at exactly that seam: each +// half had its own passing tests. + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/golang-jwt/jwt/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/xraph/forge" + "golang.org/x/crypto/bcrypt" + + authsome "github.com/xraph/authsome" + "github.com/xraph/authsome/account" + "github.com/xraph/authsome/id" + "github.com/xraph/authsome/middleware" + "github.com/xraph/authsome/plugins/oauth2provider" + "github.com/xraph/authsome/tokenformat" +) + +// issueOAuth2JWT runs an authorization-code flow for scope "profile" against +// a plugin initialised from eng, so the token endpoint mints with the app's +// JWT format. It returns the token response and a function that redeems a +// refresh token at the same endpoint. +func issueOAuth2JWT(t *testing.T, eng *authsome.Engine, appID id.AppID) (*oauth2provider.TokenResponse, func(string) *oauth2provider.TokenResponse) { + t.Helper() + ctx := context.Background() + + _, sess, err := eng.SignUp(ctx, &account.SignUpRequest{ + AppID: appID, + Email: "oauth2-jwt@example.com", + Password: "SecureP@ss123", + FirstName: "JWT Scope", + }) + require.NoError(t, err) + + p := oauth2provider.New(oauth2provider.Config{Issuer: "https://auth.example.com"}) + oauthStore := oauth2provider.NewMemoryStore() + p.SetOAuth2Store(oauthStore) + require.NoError(t, p.OnInit(ctx, eng)) + + hashed, err := bcrypt.GenerateFromPassword([]byte(e2eClientSecret), bcrypt.MinCost) + require.NoError(t, err) + require.NoError(t, oauthStore.CreateClient(ctx, &oauth2provider.OAuth2Client{ + ID: id.NewOAuth2ClientID(), + AppID: appID, + ClientID: e2eClientID, + FirstParty: true, + ClientSecret: string(hashed), + Name: "Third Party", + RedirectURIs: []string{e2eRedirectURI}, + Scopes: []string{"openid", "profile"}, + GrantTypes: []string{"authorization_code"}, + })) + + mux := forge.NewRouter() + require.NoError(t, p.RegisterRoutes(mux)) + + q := url.Values{ + "response_type": {"code"}, + "client_id": {e2eClientID}, + "redirect_uri": {e2eRedirectURI}, + "scope": {"profile"}, + } + authRec := httptest.NewRecorder() + mux.ServeHTTP(authRec, httptest.NewRequestWithContext( + middleware.WithUserID(ctx, sess.UserID), http.MethodGet, + "/v1/oauth/authorize?"+q.Encode(), nil)) + require.Equal(t, http.StatusFound, authRec.Code, "body: %s", authRec.Body.String()) + loc, err := url.Parse(authRec.Header().Get("Location")) + require.NoError(t, err) + code := loc.Query().Get("code") + require.NotEmpty(t, code) + + redeem := func(form map[string]string) *oauth2provider.TokenResponse { + form["client_id"] = e2eClientID + form["client_secret"] = e2eClientSecret + body, err := json.Marshal(form) + require.NoError(t, err) + tokReq := httptest.NewRequestWithContext(ctx, http.MethodPost, "/v1/oauth/token", bytes.NewReader(body)) + tokReq.Header.Set("Content-Type", "application/json") + tokRec := httptest.NewRecorder() + mux.ServeHTTP(tokRec, tokReq) + require.Equal(t, http.StatusOK, tokRec.Code, "body: %s", tokRec.Body.String()) + + var resp oauth2provider.TokenResponse + require.NoError(t, json.Unmarshal(tokRec.Body.Bytes(), &resp)) + // Guard the premise: an opaque token would be parked through its + // session row and this test would pass without exercising the JWT + // path at all. + require.True(t, tokenformat.IsJWT(resp.AccessToken), "the app is JWT-configured, so the grant must mint a JWT") + return &resp + } + + resp := redeem(map[string]string{ + "grant_type": "authorization_code", + "code": code, + "redirect_uri": e2eRedirectURI, + }) + refresh := func(refreshToken string) *oauth2provider.TokenResponse { + return redeem(map[string]string{"grant_type": "refresh_token", "refresh_token": refreshToken}) + } + return resp, refresh +} + +func TestOAuth2IssuedJWTIsParkedUntilAScopedRouteAdmitsIt(t *testing.T) { + jwtFmt, err := tokenformat.NewJWT(tokenformat.JWTConfig{ + SigningMethod: jwt.SigningMethodHS256, + SigningKey: []byte("test-signing-key-0123456789abcdef"), + }) + require.NoError(t, err) + eng, _ := newTestEngine(t, authsome.WithJWTFormat("aapp_01jf0000000000000000000000", jwtFmt)) + + issued, refresh := issueOAuth2JWT(t, eng, testAppID(t)) + require.NotEmpty(t, issued.RefreshToken) + + whoami := func(ctx forge.Context) error { + if _, ok := middleware.UserFrom(ctx.Context()); !ok { + return ctx.NoContent(http.StatusUnauthorized) + } + return ctx.NoContent(http.StatusOK) + } + router := forge.NewRouter() + router.Use(eng.AuthMiddleware()) + require.NoError(t, router.GET("/untagged", whoami)) + require.NoError(t, router.GET("/profile", whoami, forge.WithMiddleware(middleware.RequireScope("profile")))) + require.NoError(t, router.GET("/admin", whoami, forge.WithMiddleware(middleware.RequireScope("admin")))) + + call := func(token, path string) int { + req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, path, nil) + req.Header.Set("Authorization", "Bearer "+token) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + return rec.Code + } + + // The refresh grant re-mints through the engine rather than the plugin, + // so it is a second place the client binding can fall off the token. + tokens := map[string]string{ + "issued by the code grant": issued.AccessToken, + "issued by the refresh grant": refresh(issued.RefreshToken).AccessToken, + } + for name, token := range tokens { + t.Run(name, func(t *testing.T) { + assert.Equal(t, http.StatusUnauthorized, call(token, "/untagged"), + "a third-party JWT must not authenticate as the user on a route that never declared scopes") + assert.Equal(t, http.StatusOK, call(token, "/profile"), "a route asking for a granted scope admits the token") + assert.Equal(t, http.StatusForbidden, call(token, "/admin"), "a route asking for an ungranted scope refuses it") + }) + } +} diff --git a/plugins/oauth2provider/plugin.go b/plugins/oauth2provider/plugin.go index 58828f05..418859d1 100644 --- a/plugins/oauth2provider/plugin.go +++ b/plugins/oauth2provider/plugin.go @@ -1791,6 +1791,7 @@ func (p *Plugin) issueTokens(ctx forge.Context, client *OAuth2Client, userID id. Scopes: scopes, Audience: resources, DPoPJKT: jkt, + ClientID: sess.ClientID, IssuedAt: sess.CreatedAt, ExpiresAt: sess.ExpiresAt, }) diff --git a/service.go b/service.go index a1ca3e2b..f367c545 100644 --- a/service.go +++ b/service.go @@ -830,9 +830,13 @@ func (e *Engine) rotateSession(ctx context.Context, sess *session.Session, opts SessionID: sess.ID.String(), // Without this the first refresh turns a token bound to one // resource server into an unrestricted one, silently. - Audience: sess.Audience, - Scopes: sess.Scopes, - DPoPJKT: sess.DPoPJKT, + Audience: sess.Audience, + Scopes: sess.Scopes, + DPoPJKT: sess.DPoPJKT, + // A refreshed OAuth2 token is still the client's. Dropping + // this would hand the client a token the middleware treats + // as the user's own login. + ClientID: sess.ClientID, IssuedAt: sess.UpdatedAt, ExpiresAt: sess.ExpiresAt, }) diff --git a/tokenformat/format.go b/tokenformat/format.go index 86bba74e..6e1b50c5 100644 --- a/tokenformat/format.go +++ b/tokenformat/format.go @@ -64,6 +64,14 @@ type TokenClaims struct { // means a user. That ambiguity is what made the JWT auth path parse an // empty sub and panic on a machine token. PrincipalID string `json:"pid,omitempty"` + + // ClientID names the OAuth2 client a token was issued to, and is empty + // for a first-party login. It is the RFC 9068 client_id claim. The auth + // middleware reads it to park a third-party token until a RequireScope + // route admits it; an opaque token gets the same answer from its session + // row, but a JWT is validated without loading one, so the token has to + // say it. + ClientID string `json:"client_id,omitempty"` } // Format generates and validates access tokens. Refresh tokens are always diff --git a/tokenformat/jwt.go b/tokenformat/jwt.go index f598f853..44f9d1ad 100644 --- a/tokenformat/jwt.go +++ b/tokenformat/jwt.go @@ -94,6 +94,8 @@ type customClaims struct { // Both stay absent on a human token. PrincipalKind string `json:"pk,omitempty"` PrincipalID string `json:"pid,omitempty"` + // ClientID is the RFC 9068 client_id claim, absent on a first-party token. + ClientID string `json:"client_id,omitempty"` // Confirmation is a pointer with omitempty so an unbound token carries no // cnf member at all, rather than an empty object. Anything reading these // tokens treats the presence of cnf as the signal that a proof is required. @@ -117,6 +119,7 @@ func (j *JWT) GenerateAccessToken(claims TokenClaims) (string, error) { Act: claims.Act, PrincipalKind: claims.PrincipalKind, PrincipalID: claims.PrincipalID, + ClientID: claims.ClientID, } if claims.DPoPJKT != "" { @@ -190,6 +193,7 @@ func (j *JWT) ValidateAccessToken(tokenStr string) (*TokenClaims, error) { Act: claims.Act, PrincipalKind: claims.PrincipalKind, PrincipalID: claims.PrincipalID, + ClientID: claims.ClientID, IssuedAt: issuedAt, ExpiresAt: expiresAt, }, nil