change: verify claims in jwt

Previously we did non require any claims in the token and only verified a subset of them if present.

Now we make the existence of most claims mandatory. Further claims can be enforced with the newly added options

Author: mgoetzegb

auth/auth.go

@@ -9,22 +9,29 @@ import (
 	"errors"
 	"fmt"
 	"net/url"
+	"slices"
 	"strings"
+	"time"
 
 	"github.com/Nerzal/gocloak/v13"
+	"github.com/Nerzal/gocloak/v13/pkg/jwx"
 	"github.com/golang-jwt/jwt/v5"
 )
 
 // KeycloakAuthorizer is used to validate if JWT has a correct signature and is valid and returns keycloak claims
 type KeycloakAuthorizer struct {
-	realmInfo KeycloakRealmInfo
-	client    *gocloak.GoCloak
+	realmInfo        KeycloakRealmInfo
+	client           *gocloak.GoCloak
+	validator        *jwt.Validator
+	validatorOptions []jwt.ParserOption
+	validMethods     []string
 }
 
 // KeycloakRealmInfo provides keycloak realm and server information
 type KeycloakRealmInfo struct {
 	RealmId               string // RealmId is the realm name that is passed to services via env vars
 	AuthServerInternalUrl string // AuthServerInternalUrl should point to keycloak auth server on internal (not public) network, e.g. http://keycloak:8080/auth; used for contacting keycloak for realm certificate for JWT
+	AuthServerPublicUrl   string // AuthServerPublicUrl should point to keycloak auth server on public network; used for validating issuer claim in JWT
 }
 
 func (i *KeycloakRealmInfo) validate() error {
@@ -39,6 +46,11 @@ func (i *KeycloakRealmInfo) validate() error {
 		errs = append(errs, fmt.Errorf("couldn't parse auth server internal url: %w", err))
 	}
 
+	_, err = url.ParseRequestURI(i.AuthServerPublicUrl)
+	if err != nil {
+		errs = append(errs, fmt.Errorf("couldn't parse auth server public url: %w", err))
+	}
+
 	if len(errs) > 0 {
 		return fmt.Errorf("\n%w", errors.Join(errs...))
 	}
@@ -64,6 +76,13 @@ func NewKeycloakAuthorizer(realmInfo KeycloakRealmInfo, options ...func(*Keycloa
 		o(authorizer)
 	}
 
+	authorizer.validatorOptions = append(authorizer.validatorOptions,
+		jwt.WithIssuer(fmt.Sprintf("%s/realms/%s", realmInfo.AuthServerPublicUrl, realmInfo.RealmId)),
+		jwt.WithExpirationRequired(),
+		jwt.WithIssuedAt(),
+	)
+	authorizer.validator = jwt.NewValidator(authorizer.validatorOptions...)
+
 	return authorizer, nil
 }
 
@@ -73,6 +92,30 @@ func ConfigureGoCloak(f func(c *gocloak.GoCloak)) func(a *KeycloakAuthorizer) {
 	}
 }
 
+// WithValidMethods sets the allowed signing methods for the JWT parser.
+// Empty list means all signing methods permitted by gocloak are allowed.
+func WithValidMethods(methods ...string) func(a *KeycloakAuthorizer) {
+	return func(a *KeycloakAuthorizer) {
+		a.validMethods = methods
+	}
+}
+
+// WithLeeway sets the leeway window for the JWT parser.
+// This is used to account for clock skew when validating the token's claims.
+func WithLeeway(leeway time.Duration) func(a *KeycloakAuthorizer) {
+	return func(a *KeycloakAuthorizer) {
+		a.validatorOptions = append(a.validatorOptions, jwt.WithLeeway(leeway))
+	}
+}
+
+// WithAudience configures the validator to require any of the specified audiences in the `aud` claim.
+// Validation will fail if the audience is not listed in the token or the `aud` claim is missing.
+func WithAudience(audience ...string) func(a *KeycloakAuthorizer) {
+	return func(a *KeycloakAuthorizer) {
+		a.validatorOptions = append(a.validatorOptions, jwt.WithAudience(audience...))
+	}
+}
+
 func (a *KeycloakAuthorizer) parseAuthorizationHeader(authorizationHeader string) (string, error) {
 	fields := strings.Fields(authorizationHeader)
 	if len(fields) != 2 {
@@ -131,27 +174,47 @@ func (a *KeycloakAuthorizer) ParseAuthorizationHeader(ctx context.Context, authH
 func (a *KeycloakAuthorizer) ParseJWT(ctx context.Context, token string) (UserContext, error) {
 	type customClaims struct {
 		jwt.RegisteredClaims
-		UserId         string   `json:"sub"`
 		Email          string   `json:"email"`
 		UserName       string   `json:"preferred_username"`
 		Roles          []string `json:"roles"`
 		Groups         []string `json:"groups"`
 		AllowedOrigins []string `json:"allowed-origins"`
 	}
 
-	jwtToken, _, err := jwt.NewParser().ParseUnverified(token, &customClaims{})
+	tokenHeader, err := jwx.DecodeAccessTokenHeader(token)
 	if err != nil {
-		return UserContext{}, fmt.Errorf("parsing of token failed: %w", err)
+		return UserContext{}, fmt.Errorf("could not decode token header: %w", err)
+	}
+	if len(a.validMethods) > 0 {
+		if !slices.Contains(a.validMethods, tokenHeader.Alg) {
+			return UserContext{}, fmt.Errorf("invalid token signing method: %s", tokenHeader.Alg)
+		}
 	}
-	claims := jwtToken.Claims.(*customClaims)
 
+	// verify token signature
 	if _, _, err := a.client.DecodeAccessToken(ctx, token, a.realmInfo.RealmId); err != nil {
 		return UserContext{}, fmt.Errorf("validation of token failed: %w", err)
 	}
 
+	// extract claims from token; signature is validated above; claims are validated below
+	jwtToken, _, err := jwt.NewParser(jwt.WithoutClaimsValidation()).ParseUnverified(token, &customClaims{})
+	if err != nil {
+		return UserContext{}, fmt.Errorf("parsing of token failed: %w", err)
+	}
+	claims := jwtToken.Claims.(*customClaims)
+
+	// verify claims
+	err = a.validator.Validate(claims)
+	if err != nil {
+		return UserContext{}, fmt.Errorf("validation of token claims failed: %w", err)
+	}
+	if claims.Subject == "" { // [jwt.Validator] does not support just checking for non-empty subject claim, so we check it here
+		return UserContext{}, fmt.Errorf("validation of token claims failed: missing subject claim")
+	}
+
 	return UserContext{
 		Realm:          a.realmInfo.RealmId,
-		UserID:         claims.UserId,
+		UserID:         claims.Subject,
 		UserName:       claims.UserName,
 		EmailAddress:   claims.Email,
 		Roles:          claims.Roles,

auth/auth_test.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/golang-jwt/jwt/v5"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -32,16 +33,28 @@ func TestNewKeycloakAuthorizer(t *testing.T) {
 		}
 		t.Run("internal", func(t *testing.T) {
 			for _, test := range tests {
-				t.Run(test, func(t *testing.T) {
+				t.Run("internal url "+test, func(t *testing.T) {
 					authorizer, err := NewKeycloakAuthorizer(KeycloakRealmInfo{
 						AuthServerInternalUrl: test,
 						RealmId:               validRealm,
+						AuthServerPublicUrl:   validPublicUrl,
 					})
 
 					assert.ErrorContains(t, err, "invalid realm info")
 					assert.ErrorContains(t, err, "couldn't parse auth server internal url")
 					assert.Nil(t, authorizer)
 				})
+				t.Run("public url "+test, func(t *testing.T) {
+					authorizer, err := NewKeycloakAuthorizer(KeycloakRealmInfo{
+						AuthServerPublicUrl:   test,
+						RealmId:               validRealm,
+						AuthServerInternalUrl: validInternalUrl,
+					})
+
+					assert.ErrorContains(t, err, "invalid realm info")
+					assert.ErrorContains(t, err, "couldn't parse auth server public url")
+					assert.Nil(t, authorizer)
+				})
 			}
 		})
 	})
@@ -50,6 +63,7 @@ func TestNewKeycloakAuthorizer(t *testing.T) {
 		authorizer, err := NewKeycloakAuthorizer(KeycloakRealmInfo{
 			RealmId:               validRealm,
 			AuthServerInternalUrl: validInternalUrl,
+			AuthServerPublicUrl:   validPublicUrl,
 		})
 
 		assert.NoError(t, err)
@@ -58,49 +72,133 @@ func TestNewKeycloakAuthorizer(t *testing.T) {
 }
 
 func TestParseJWT(t *testing.T) {
+	tokenIssuer, err := NewTokenIssuer(publicKeyALG, publicKeyID)
+	require.NoError(t, err)
+	authServer := FakeAuthServer(t, tokenIssuer)
+
 	authorizer, err := NewKeycloakAuthorizer(KeycloakRealmInfo{
 		RealmId:               validRealm,
-		AuthServerInternalUrl: validInternalUrl,
-	})
+		AuthServerInternalUrl: authServer.URL,
+		AuthServerPublicUrl:   validPublicUrl,
+	},
+		WithValidMethods("RS256"),
+		WithAudience(validAudience, "other valid audience"),
+	)
 	require.NoError(t, err)
 	require.NotNil(t, authorizer)
 
-	FakeCertResponse(t, authorizer)
-
 	t.Run("Wrong algorithm", func(t *testing.T) {
-		userContext, err := authorizer.ParseJWT(context.Background(), invalidAlgorithmToken)
+		token := tokenIssuer.GetTokenWithAlg(t, jwt.SigningMethodHS256.Alg(), validClaims())
 
-		assert.ErrorContains(t, err, "validation of token failed")
-		assert.ErrorContains(t, err, "cannot find a key to decode the token")
+		userContext, err := authorizer.ParseJWT(context.Background(), token)
+
+		assert.ErrorContains(t, err, "invalid token signing method")
 		assert.Zero(t, userContext)
 	})
 
 	t.Run("Wrong signature", func(t *testing.T) {
-		userContext, err := authorizer.ParseJWT(context.Background(), invalidSignatureToken)
+		token := tokenIssuer.GetToken(t, jwt.MapClaims{
+			"iss": validPublicUrl + "/realms/" + validRealm,
+		})
+		token += "XX" // malformed signature
+
+		userContext, err := authorizer.ParseJWT(context.Background(), token)
 
 		assert.ErrorContains(t, err, "validation of token failed")
 		assert.ErrorContains(t, err, "crypto/rsa: verification error")
 		assert.Zero(t, userContext)
 	})
 
 	t.Run("Expired token", func(t *testing.T) {
-		userContext, err := authorizer.ParseJWT(context.Background(), expiredToken)
+		claims := validClaims()
+		claims["exp"] = 1500000000
+		token := tokenIssuer.GetToken(t, claims)
 
-		assert.ErrorContains(t, err, "validation of token failed")
+		userContext, err := authorizer.ParseJWT(context.Background(), token)
+
+		assert.ErrorContains(t, err, "token has invalid claims")
 		assert.ErrorContains(t, err, "token is expired")
 		assert.Zero(t, userContext)
 	})
 
+	t.Run("missing expiration", func(t *testing.T) {
+		claims := validClaims()
+		delete(claims, "exp")
+		token := tokenIssuer.GetToken(t, claims)
+
+		userContext, err := authorizer.ParseJWT(context.Background(), token)
+
+		assert.ErrorContains(t, err, "validation of token claims failed")
+		assert.ErrorContains(t, err, "exp claim is required")
+		assert.Zero(t, userContext)
+	})
+
+	t.Run("invalid issuedAt", func(t *testing.T) {
+		claims := validClaims()
+		claims["iat"] = 9999999999
+		token := tokenIssuer.GetToken(t, claims)
+
+		userContext, err := authorizer.ParseJWT(context.Background(), token)
+
+		assert.ErrorContains(t, err, "validation of token claims failed")
+		assert.ErrorContains(t, err, "token used before issued")
+		assert.Zero(t, userContext)
+	})
+
+	t.Run("Wrong issuer", func(t *testing.T) {
+		claims := validClaims()
+		claims["iss"] = "invalid_issuer"
+		token := tokenIssuer.GetToken(t, claims)
+
+		userContext, err := authorizer.ParseJWT(context.Background(), token)
+
+		assert.ErrorContains(t, err, "validation of token claims failed")
+		assert.ErrorContains(t, err, "token has invalid issuer")
+		assert.Zero(t, userContext)
+	})
+
+	t.Run("Missing subject", func(t *testing.T) {
+		claims := validClaims()
+		delete(claims, "sub")
+		token := tokenIssuer.GetToken(t, claims)
+
+		userContext, err := authorizer.ParseJWT(context.Background(), token)
+
+		assert.ErrorContains(t, err, "validation of token claims failed")
+		assert.ErrorContains(t, err, "missing subject claim")
+		assert.Zero(t, userContext)
+	})
+
+	t.Run("Wrong audience", func(t *testing.T) {
+		claims := validClaims()
+		claims["aud"] = "invalid_audience"
+		token := tokenIssuer.GetToken(t, claims)
+
+		userContext, err := authorizer.ParseJWT(context.Background(), token)
+
+		assert.ErrorContains(t, err, "validation of token claims failed")
+		assert.ErrorContains(t, err, "token has invalid audience")
+		assert.Zero(t, userContext)
+	})
+
 	t.Run("Invalid claims", func(t *testing.T) {
-		userContext, err := authorizer.ParseJWT(context.Background(), invalidClaimsToken)
+		claims := validClaims()
+		claims["email"] = 12345
+		claims["roles"] = 1
+		claims["groups"] = 2
+		token := tokenIssuer.GetToken(t, claims)
+
+		userContext, err := authorizer.ParseJWT(context.Background(), token)
 
 		assert.ErrorContains(t, err, "parsing of token failed")
 		assert.ErrorContains(t, err, "cannot unmarshal number")
 		assert.Zero(t, userContext)
 	})
 
 	t.Run("OK", func(t *testing.T) {
-		userContext, err := authorizer.ParseJWT(context.Background(), validToken)
+		token := tokenIssuer.GetToken(t, validClaims())
+
+		userContext, err := authorizer.ParseJWT(context.Background(), token)
 
 		require.NoError(t, err)
 		require.NotZero(t, userContext)
@@ -111,19 +209,24 @@ func TestParseJWT(t *testing.T) {
 		assert.Equal(t, "initial", userContext.UserName)
 		assert.ElementsMatch(t, []string{"offline_access", "uma_authorization", "user", "default-roles-user-management"}, userContext.Roles)
 		assert.ElementsMatch(t, []string{"user-management-initial"}, userContext.Groups)
+		assert.ElementsMatch(t, []string{validOrigin}, userContext.AllowedOrigins)
 	})
 }
 
 func TestParseAuthorizationHeader(t *testing.T) {
+	tokenIssuer, err := NewTokenIssuer(publicKeyALG, publicKeyID)
+	require.NoError(t, err)
+	authServer := FakeAuthServer(t, tokenIssuer)
+	validToken := tokenIssuer.GetToken(t, validClaims())
+
 	authorizer, err := NewKeycloakAuthorizer(KeycloakRealmInfo{
 		RealmId:               validRealm,
-		AuthServerInternalUrl: validInternalUrl,
+		AuthServerInternalUrl: authServer.URL,
+		AuthServerPublicUrl:   validPublicUrl,
 	})
 	require.NoError(t, err)
 	require.NotNil(t, authorizer)
 
-	FakeCertResponse(t, authorizer)
-
 	t.Run("Invalid fields", func(t *testing.T) {
 		tests := []string{
 			"",
@@ -148,6 +251,10 @@ func TestParseAuthorizationHeader(t *testing.T) {
 	})
 
 	t.Run("Invalid token", func(t *testing.T) {
+		expiredClaims := validClaims()
+		expiredClaims["exp"] = 1500000000
+		expiredToken := tokenIssuer.GetToken(t, expiredClaims)
+
 		userContext, err := authorizer.ParseAuthorizationHeader(context.Background(), "bearer "+expiredToken)
 
 		assert.ErrorContains(t, err, "validation of token failed")
@@ -169,15 +276,19 @@ func TestParseAuthorizationHeader(t *testing.T) {
 }
 
 func TestParseRequest(t *testing.T) {
+	tokenIssuer, err := NewTokenIssuer(publicKeyALG, publicKeyID)
+	require.NoError(t, err)
+	authServer := FakeAuthServer(t, tokenIssuer)
+	validToken := tokenIssuer.GetToken(t, validClaims())
+
 	authorizer, err := NewKeycloakAuthorizer(KeycloakRealmInfo{
 		RealmId:               validRealm,
-		AuthServerInternalUrl: validInternalUrl,
+		AuthServerInternalUrl: authServer.URL,
+		AuthServerPublicUrl:   validPublicUrl,
 	})
 	require.NoError(t, err)
 	require.NotNil(t, authorizer)
 
-	FakeCertResponse(t, authorizer)
-
 	t.Run("Invalid authorization header", func(t *testing.T) {
 		userContext, err := authorizer.ParseRequest(context.Background(), "invalid", validOrigin)