Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions internal/service/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,17 @@ func (s *OAuthService) jwtBearer(ctx context.Context, req TokenRequest) (*domain
return nil, oauthBadRequestCause("invalid_grant", "assertion JWT validation failed", err)
}

// RFC 7523 §3 mandatory claims. jwx's WithValidate(true) honors them
// when present but does not require them — supplement here.
// §3 (2): "The JWT MUST contain a 'sub' (subject) claim ..."
// §3 (4): "The JWT MUST contain an 'exp' (expiration) claim ..."
if _, ok := assertionToken.Subject(); !ok {
return nil, oauthBadRequest("invalid_grant", "assertion JWT missing required sub claim")
}
if _, ok := assertionToken.Expiration(); !ok {
return nil, oauthBadRequest("invalid_grant", "assertion JWT missing required exp claim")
}

// iss must match the identity's WIMSE URI.
if iss, _ := assertionToken.Issuer(); iss != identity.WIMSEURI {
return nil, oauthBadRequest("invalid_grant", "iss claim does not match identity WIMSE URI")
Expand Down Expand Up @@ -461,6 +472,17 @@ func (s *OAuthService) tokenExchange(ctx context.Context, req TokenRequest) (*do
if err != nil {
return nil, oauthBadRequestCause("invalid_grant", "actor_token validation failed", err)
}
// RFC 7523 §3 mandatory claims apply to the actor_token too (RFC 8693
// §1.2 inherits the JWT-bearer assertion contract). jwx's validator
// honors them when present but does not require them — supplement here.
// §3 (2): sub REQUIRED
// §3 (4): exp REQUIRED
if _, ok := validatedActorToken.Subject(); !ok {
return nil, oauthBadRequest("invalid_grant", "actor_token missing required sub claim")
}
if _, ok := validatedActorToken.Expiration(); !ok {
return nil, oauthBadRequest("invalid_grant", "actor_token missing required exp claim")
}
if iss, _ := validatedActorToken.Issuer(); iss != actorIdentity.WIMSEURI {
return nil, oauthBadRequest("invalid_grant", "actor_token iss does not match actor identity WIMSE URI")
}
Expand Down
5 changes: 4 additions & 1 deletion tests/integration/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -384,12 +384,15 @@ func ecPublicKeyPEM(t *testing.T, key *ecdsa.PrivateKey) string {
}

// buildAssertion creates a self-signed ES256 JWT assertion for jwt_bearer and token_exchange flows.
// issuerWIMSE is the agent's WIMSE URI used as the iss claim.
// issuerWIMSE is the agent's WIMSE URI used as both the iss and sub claims —
// for self-asserting agents, RFC 7523 §3 (2) requires sub to identify the
// principal, which is the same agent the assertion is for.
func buildAssertion(t *testing.T, privKey *ecdsa.PrivateKey, issuerWIMSE string) string {
t.Helper()
now := time.Now()
tok, err := jwt.NewBuilder().
Issuer(issuerWIMSE).
Subject(issuerWIMSE).
Audience([]string{testIssuer}).
IssuedAt(now).
Expiration(now.Add(5 * time.Minute)).
Expand Down
193 changes: 193 additions & 0 deletions tests/integration/jwt_bearer_exp_required_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
// Regression guards for the RFC 7523 §3 (2) + §3 (4) sub-and-exp-required fix.
//
// jwx's `WithValidate(true)` honors `sub` and `exp` when present but does
// not require them. The OAuth service now supplements with explicit
// checks for both required claims on both the jwt_bearer assertion and
// the token_exchange actor_token; these tests ensure those supplements
// stay in place across future refactors.

package integration_test

import (
"crypto/ecdsa"
"crypto/elliptic"
"crypto/rand"
"net/http"
"testing"
"time"

"github.com/lestrrat-go/jwx/v4/jwa"
"github.com/lestrrat-go/jwx/v4/jwt"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// signAssertionWithClaims builds a jwt-bearer assertion from a caller-supplied
// claims map. Tests deliberately omit either `sub` or `exp` to exercise the
// negative-space MUSTs of RFC 7523 §3 (2) and §3 (4).
func signAssertionWithClaims(t *testing.T, key *ecdsa.PrivateKey, claims map[string]any) string {
t.Helper()
b := jwt.NewBuilder()
for k, v := range claims {
b = b.Claim(k, v)
}
tok, err := b.Build()
require.NoError(t, err)
signed, err := jwt.Sign(tok, jwt.WithKey(jwa.ES256(), key))
require.NoError(t, err)
return string(signed)
}

func TestJwtBearer_ExpClaimRequired(t *testing.T) {
// RFC 7523 §3 (4): "The JWT MUST contain an 'exp' (expiration) claim
// that limits the time window during which the JWT can be used."
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
require.NoError(t, err)
agentID := uid("exp-required-jwt-bearer")
identity := registerIdentity(t, agentID, []string{"data:read"}, ecPublicKeyPEM(t, key))

now := time.Now()
bad := signAssertionWithClaims(t, key, map[string]any{
"iss": identity.WIMSEURI,
"sub": identity.WIMSEURI,
"aud": testIssuer,
"iat": now.Unix(),
// exp deliberately omitted
})

resp := post(t, "/oauth2/token", map[string]any{
"grant_type": "urn:ietf:params:oauth:grant-type:jwt-bearer",
"subject": bad,
"scope": "data:read",
}, nil)
require.Equal(t, http.StatusBadRequest, resp.StatusCode,
"jwt-bearer assertion without exp MUST be rejected")
body := decode(t, resp)
assert.Equal(t, "invalid_grant", body["error"])
desc, _ := body["error_description"].(string)
assert.Contains(t, desc, "exp",
"error_description should name the missing claim — proves the new check fired, not some other validator")
}

func TestTokenExchange_ActorTokenExpClaimRequired(t *testing.T) {
// RFC 7523 §3 (4) applies to the actor_token via RFC 8693 §1.2 —
// the actor_token is a JWT-bearer-style assertion and inherits the same
// MUST-have-exp contract.
orchID := uid("exp-required-tx-orch")
registerIdentity(t, orchID, []string{"data:read"})
orchClient := registerOAuthClient(t, orchID, []string{"data:read"})
resp := post(t, "/oauth2/token", map[string]any{
"grant_type": "client_credentials",
"client_id": orchClient.ClientID,
"client_secret": orchClient.ClientSecret,
"account_id": testAccountID,
"project_id": testProjectID,
"scope": "data:read",
}, nil)
require.Equal(t, http.StatusOK, resp.StatusCode)
orchToken, _ := decode(t, resp)["access_token"].(string)

subKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
require.NoError(t, err)
subID := uid("exp-required-tx-sub")
subIdentity := registerIdentity(t, subID, []string{"data:read"}, ecPublicKeyPEM(t, subKey))

now := time.Now()
actorNoExp := signAssertionWithClaims(t, subKey, map[string]any{
"iss": subIdentity.WIMSEURI,
"sub": subIdentity.WIMSEURI,
"aud": testIssuer,
"iat": now.Unix(),
// exp deliberately omitted
})

exch := post(t, "/oauth2/token", map[string]any{
"grant_type": "urn:ietf:params:oauth:grant-type:token-exchange",
"subject_token": orchToken,
"actor_token": actorNoExp,
"scope": "data:read",
}, nil)
require.Equal(t, http.StatusBadRequest, exch.StatusCode,
"actor_token without exp MUST be rejected")
body := decode(t, exch)
assert.Equal(t, "invalid_grant", body["error"])
desc, _ := body["error_description"].(string)
assert.Contains(t, desc, "exp",
"error_description should name the missing claim for token-exchange too")
}

func TestJwtBearer_SubClaimRequired(t *testing.T) {
// RFC 7523 §3 (2): "The JWT MUST contain a 'sub' (subject) claim
// identifying the principal that is the subject of the JWT."
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
require.NoError(t, err)
agentID := uid("sub-required-jwt-bearer")
identity := registerIdentity(t, agentID, []string{"data:read"}, ecPublicKeyPEM(t, key))

now := time.Now()
bad := signAssertionWithClaims(t, key, map[string]any{
"iss": identity.WIMSEURI,
"aud": testIssuer,
"exp": now.Add(5 * time.Minute).Unix(),
"iat": now.Unix(),
// sub deliberately omitted
})

resp := post(t, "/oauth2/token", map[string]any{
"grant_type": "urn:ietf:params:oauth:grant-type:jwt-bearer",
"subject": bad,
"scope": "data:read",
}, nil)
require.Equal(t, http.StatusBadRequest, resp.StatusCode,
"jwt-bearer assertion without sub MUST be rejected")
body := decode(t, resp)
assert.Equal(t, "invalid_grant", body["error"])
desc, _ := body["error_description"].(string)
assert.Contains(t, desc, "sub",
"error_description should name the missing claim")
}

func TestTokenExchange_ActorTokenSubClaimRequired(t *testing.T) {
// RFC 7523 §3 (2) applies to actor_token via RFC 8693 §1.2.
orchID := uid("sub-required-tx-orch")
registerIdentity(t, orchID, []string{"data:read"})
orchClient := registerOAuthClient(t, orchID, []string{"data:read"})
resp := post(t, "/oauth2/token", map[string]any{
"grant_type": "client_credentials",
"client_id": orchClient.ClientID,
"client_secret": orchClient.ClientSecret,
"account_id": testAccountID,
"project_id": testProjectID,
"scope": "data:read",
}, nil)
require.Equal(t, http.StatusOK, resp.StatusCode)
orchToken, _ := decode(t, resp)["access_token"].(string)

subKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
require.NoError(t, err)
subID := uid("sub-required-tx-sub")
subIdentity := registerIdentity(t, subID, []string{"data:read"}, ecPublicKeyPEM(t, subKey))

now := time.Now()
actorNoSub := signAssertionWithClaims(t, subKey, map[string]any{
"iss": subIdentity.WIMSEURI,
"aud": testIssuer,
"exp": now.Add(5 * time.Minute).Unix(),
"iat": now.Unix(),
// sub deliberately omitted
})

exch := post(t, "/oauth2/token", map[string]any{
"grant_type": "urn:ietf:params:oauth:grant-type:token-exchange",
"subject_token": orchToken,
"actor_token": actorNoSub,
"scope": "data:read",
}, nil)
require.Equal(t, http.StatusBadRequest, exch.StatusCode,
"actor_token without sub MUST be rejected")
body := decode(t, exch)
assert.Equal(t, "invalid_grant", body["error"])
desc, _ := body["error_description"].(string)
assert.Contains(t, desc, "sub",
"error_description should name the missing claim for token-exchange too")
}