From 180bd488e1723b167105d81852ee41c5d0d010c9 Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Sat, 7 Feb 2026 01:01:30 +0100 Subject: [PATCH] chore: Add `JWT()` method for convenience and clarity (#11067) This slightly simplifies calling code by centralizing the common 3-liner to create a JWT from claims, signed by a key. But more importantly, it reduces the risk of `key.PreProcessToken()` being forgotten, which will become relevant in upcoming PRs: `key.PreProcessToken()` adds the key id to the JWT header, which is important to efficiently validate tokens when multiple validation keys are supported (that is not the case yet) Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11067 Co-authored-by: Nils Goroll Co-committed-by: Nils Goroll --- modules/jwtx/signingkey.go | 24 +++++++++++++++++ modules/jwtx/signingkey_test.go | 39 ++++++++++++++++++++++++++++ routers/api/actions/id_token.go | 5 +--- services/auth/source/oauth2/token.go | 8 ++---- 4 files changed, 66 insertions(+), 10 deletions(-) diff --git a/modules/jwtx/signingkey.go b/modules/jwtx/signingkey.go index 9047c9d472..2aef70440d 100644 --- a/modules/jwtx/signingkey.go +++ b/modules/jwtx/signingkey.go @@ -34,6 +34,12 @@ func (err ErrInvalidAlgorithmType) Error() string { return fmt.Sprintf("JWT signing algorithm is not supported: %s", err.Algorithm) } +func jwtHelper(key SigningKey, claims jwt.Claims, opts ...jwt.TokenOption) (string, error) { + jwt := jwt.NewWithClaims(key.SigningMethod(), claims, opts...) + key.PreProcessToken(jwt) + return jwt.SignedString(key.SignKey()) +} + // SigningKey represents a algorithm/key pair to sign JWTs type SigningKey interface { IsSymmetric() bool @@ -42,6 +48,8 @@ type SigningKey interface { VerifyKey() any ToJWK() (map[string]string, error) PreProcessToken(*jwt.Token) + // convenience: jwt.NewWithClaims + PreProcessToken + SignedString + JWT(jwt.Claims, ...jwt.TokenOption) (string, error) } type hmacSigningKey struct { @@ -74,6 +82,10 @@ func (key hmacSigningKey) ToJWK() (map[string]string, error) { func (key hmacSigningKey) PreProcessToken(*jwt.Token) {} +func (key hmacSigningKey) JWT(claims jwt.Claims, opts ...jwt.TokenOption) (string, error) { + return jwtHelper(key, claims, opts...) +} + type rsaSigningKey struct { signingMethod jwt.SigningMethod key *rsa.PrivateKey @@ -125,6 +137,10 @@ func (key rsaSigningKey) PreProcessToken(token *jwt.Token) { token.Header["kid"] = key.id } +func (key rsaSigningKey) JWT(claims jwt.Claims, opts ...jwt.TokenOption) (string, error) { + return jwtHelper(key, claims, opts...) +} + type eddsaSigningKey struct { signingMethod jwt.SigningMethod key ed25519.PrivateKey @@ -176,6 +192,10 @@ func (key eddsaSigningKey) PreProcessToken(token *jwt.Token) { token.Header["kid"] = key.id } +func (key eddsaSigningKey) JWT(claims jwt.Claims, opts ...jwt.TokenOption) (string, error) { + return jwtHelper(key, claims, opts...) +} + type ecdsaSigningKey struct { signingMethod jwt.SigningMethod key *ecdsa.PrivateKey @@ -228,6 +248,10 @@ func (key ecdsaSigningKey) PreProcessToken(token *jwt.Token) { token.Header["kid"] = key.id } +func (key ecdsaSigningKey) JWT(claims jwt.Claims, opts ...jwt.TokenOption) (string, error) { + return jwtHelper(key, claims, opts...) +} + var allowedAlgorithms = map[string]bool{ "HS256": true, "HS384": true, diff --git a/modules/jwtx/signingkey_test.go b/modules/jwtx/signingkey_test.go index 0b81a03682..eb30473cad 100644 --- a/modules/jwtx/signingkey_test.go +++ b/modules/jwtx/signingkey_test.go @@ -13,6 +13,7 @@ import ( "path/filepath" "testing" + "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -112,6 +113,44 @@ func TestLoadOrCreateAsymmetricKey(t *testing.T) { }) } +type testClaims struct { + Foo string `json:"Foo"` + jwt.RegisteredClaims +} + +func TestJWTHasKid(t *testing.T) { + keyPath := filepath.Join(t.TempDir(), "jwt-rsa-2048.priv") + algorithm := "RS256" + key, err := InitAsymmetricSigningKey(keyPath, algorithm) + require.NoError(t, err) + + claimsIn := testClaims{ + Foo: "bar", + RegisteredClaims: jwt.RegisteredClaims{}, + } + token, err := key.JWT(&claimsIn) + require.NoError(t, err) + + var claimsOut testClaims + parsed, err := jwt.ParseWithClaims(token, &claimsOut, func(valToken *jwt.Token) (any, error) { + assert.NotNil(t, valToken.Method) + assert.Equal(t, key.SigningMethod().Alg(), valToken.Method.Alg()) + kid, ok := valToken.Header["kid"] + assert.True(t, ok) + assert.NotNil(t, kid) + + return key.VerifyKey(), nil + }) + require.NoError(t, err) + assert.NotNil(t, parsed) + assert.Equal(t, "bar", parsed.Claims.(*testClaims).Foo) + assert.Equal(t, "bar", claimsOut.Foo) + // dup to keyFunc above + kid, ok := parsed.Header["kid"] + assert.True(t, ok) + assert.NotNil(t, kid) +} + func TestCannotCreatePrivateKey(t *testing.T) { _, err := InitAsymmetricSigningKey("/dev/directory-does-not-exist-and-you-should-not-have-permission-to-create/privatekey.pem", "RS256") require.Error(t, err) diff --git a/routers/api/actions/id_token.go b/routers/api/actions/id_token.go index cd0432acb7..5eacc77fc9 100644 --- a/routers/api/actions/id_token.go +++ b/routers/api/actions/id_token.go @@ -131,10 +131,7 @@ func generateIDToken(ctx *IDTokenContext) { claims["nbf"] = jwt.NewNumericDate(now) claims["iss"] = strings.TrimSuffix(setting.AppURL, "/") + "/api/actions" - jwtToken := jwt.NewWithClaims(jwtSigningKey.SigningMethod(), claims) - jwtSigningKey.PreProcessToken(jwtToken) - - signedToken, err := jwtToken.SignedString(jwtSigningKey.SignKey()) + signedToken, err := jwtSigningKey.JWT(claims) if err != nil { ctx.Error(http.StatusInternalServerError, "Error signing token") } diff --git a/services/auth/source/oauth2/token.go b/services/auth/source/oauth2/token.go index 6b5c3676aa..20efc78ab5 100644 --- a/services/auth/source/oauth2/token.go +++ b/services/auth/source/oauth2/token.go @@ -66,9 +66,7 @@ func ParseToken(jwtToken string, signingKey jwtx.SigningKey) (*Token, error) { // SignToken signs the token with the JWT secret func (token *Token) SignToken(signingKey jwtx.SigningKey) (string, error) { token.IssuedAt = jwt.NewNumericDate(time.Now()) - jwtToken := jwt.NewWithClaims(signingKey.SigningMethod(), token) - signingKey.PreProcessToken(jwtToken) - return jwtToken.SignedString(signingKey.SignKey()) + return signingKey.JWT(token) } // OIDCToken represents an OpenID Connect id_token @@ -96,7 +94,5 @@ type OIDCToken struct { // SignToken signs an id_token with the (symmetric) client secret key func (token *OIDCToken) SignToken(signingKey jwtx.SigningKey) (string, error) { token.IssuedAt = jwt.NewNumericDate(time.Now()) - jwtToken := jwt.NewWithClaims(signingKey.SigningMethod(), token) - signingKey.PreProcessToken(jwtToken) - return jwtToken.SignedString(signingKey.SignKey()) + return signingKey.JWT(token) }