diff --git a/review_r66_lenient_expiry_test.go b/review_r66_lenient_expiry_test.go new file mode 100644 index 0000000..fe1f2be --- /dev/null +++ b/review_r66_lenient_expiry_test.go @@ -0,0 +1,88 @@ +package traefikoidc + +import ( + "net/http/httptest" + "testing" + "time" +) + +// TestLenientAudienceRequiresUnexpiredAccessToken verifies that on the +// default (lenient) audience-validation path, an access token with a valid +// signature but wrong audience is still required to be unexpired before it is +// accepted as authenticated. Previously jwt.Verify short-circuits at the aud +// check before the exp check, so an expired wrong-audience access token (with +// no ID token to corroborate it) was accepted without expiry ever being +// checked, and its claims were trusted for RBAC. +func TestLenientAudienceRequiresUnexpiredAccessToken(t *testing.T) { + ts := NewTestSuite(t) + ts.Setup() + ts.tOidc.audience = "https://my-api.example.com" + ts.tOidc.strictAudienceValidation = false // default lenient + + cleanupReplayCache() + initReplayCache() + defer cleanupReplayCache() + + t.Run("expired wrong-audience token rejected", func(t *testing.T) { + accessToken, err := createTestJWT(ts.rsaPrivateKey, "RS256", "test-key-id", map[string]interface{}{ + "iss": "https://test-issuer.com", + "aud": []interface{}{"https://other-api.example.com"}, // wrong audience + "exp": float64(time.Now().Add(-1 * time.Hour).Unix()), // EXPIRED + "iat": float64(time.Now().Add(-2 * time.Hour).Unix()), + "sub": "test-user", + "scope": "openid profile email", + "jti": "r66-lenient-expired", + }) + if err != nil { + t.Fatalf("failed to create access token: %v", err) + } + + session, err := ts.tOidc.sessionManager.GetSession(httptest.NewRequest("GET", "/", nil)) + if err != nil { + t.Fatal(err) + } + session.SetAuthenticated(true) + session.SetAccessToken(accessToken) + session.SetIDToken("") + session.SetRefreshToken("") // no refresh: must force re-auth, not accept + + rs := (&requestState{}).captureSession(session) + authenticated, _, expired := ts.tOidc.validateStandardTokensRS(rs) + if authenticated { + t.Fatal("expired wrong-audience access token was accepted as authenticated (expiry not checked on lenient path)") + } + if !expired { + t.Log("note: force-re-auth path chosen (expired flag not set)") + } + }) + + t.Run("unexpired wrong-audience token still accepted (lenient preserved)", func(t *testing.T) { + accessToken, err := createTestJWT(ts.rsaPrivateKey, "RS256", "test-key-id", map[string]interface{}{ + "iss": "https://test-issuer.com", + "aud": []interface{}{"https://other-api.example.com"}, // wrong audience + "exp": float64(time.Now().Add(1 * time.Hour).Unix()), // still valid + "iat": float64(time.Now().Unix()), + "sub": "test-user", + "scope": "openid profile email", + "jti": "r66-lenient-valid", + }) + if err != nil { + t.Fatalf("failed to create access token: %v", err) + } + + session, err := ts.tOidc.sessionManager.GetSession(httptest.NewRequest("GET", "/", nil)) + if err != nil { + t.Fatal(err) + } + session.SetAuthenticated(true) + session.SetAccessToken(accessToken) + session.SetIDToken("") + session.SetRefreshToken("") + + rs := (&requestState{}).captureSession(session) + authenticated, _, _ := ts.tOidc.validateStandardTokensRS(rs) + if !authenticated { + t.Fatal("unexpired wrong-audience access token should still be accepted under lenient audience validation") + } + }) +} diff --git a/token_validation_rs.go b/token_validation_rs.go index ebca83a..79b2122 100644 --- a/token_validation_rs.go +++ b/token_validation_rs.go @@ -38,6 +38,23 @@ func (t *TraefikOidc) validateGoogleTokensRS(rs *requestState) (bool, bool, bool return t.validateStandardTokensRS(rs) } +// accessTokenUnexpired reports whether a signature-verified access token's exp +// claim is still in the future. Used on the lenient-audience path where +// jwt.Verify short-circuits at the aud check before validating exp, so expiry +// must be (re)checked here before the token's claims are trusted. Honors the +// same clock-skew tolerance as normal expiration verification. +func (t *TraefikOidc) accessTokenUnexpired(token string) bool { + parsed, err := parseJWT(token) + if err != nil { + return false + } + exp, ok := parsed.Claims["exp"].(float64) + if !ok { + return false + } + return time.Now().Before(time.Unix(int64(exp), 0).Add(ClockSkewToleranceFuture)) +} + // validateTokenExpiryRS is the requestState-aware variant of validateTokenExpiry. // Reads rs.refreshToken instead of session.GetRefreshToken() (4 RLocks avoided). func (t *TraefikOidc) validateTokenExpiryRS(rs *requestState, token string) (bool, bool, bool) { @@ -215,7 +232,17 @@ func (t *TraefikOidc) validateStandardTokensRS(rs *requestState) (bool, bool, bo } if lenientAudienceOnly { // Access token signature verified; audience check was lenient and - // there is no ID token to compare against. Treat as authenticated. + // there is no ID token to compare against. jwt.Verify returns at + // the aud check BEFORE reaching the exp check, so expiry was never + // validated on this path — an expired, wrong-audience token would + // otherwise be accepted as authenticated. Require the token to be + // still unexpired before trusting its claims for authorization. + if !t.accessTokenUnexpired(rs.accessToken) { + if rs.refreshToken != "" { + return false, true, false + } + return false, false, true + } if rs.refreshToken != "" { return true, true, false }