Skip to content

fix(auth): reject a getClaims token whose alg does not fit the matched JWK - #1449

Open
AndroidPoet wants to merge 1 commit into
supabase:masterfrom
AndroidPoet:fix/auth-getclaims-alg-key-match
Open

AndroidPoet wants to merge 1 commit into
supabase:masterfrom
AndroidPoet:fix/auth-getclaims-alg-key-match

Conversation

@AndroidPoet

Copy link
Copy Markdown
Contributor

What

getClaims picks the verifier from the token header alg and never compares it with the JWK found for the header kid. A token that says ES256 but names an RSA key, or says RS256 but names an EC key, reaches the DER conversion for the wrong key type and fails with IllegalStateException: Param not found instead of InvalidJwtException. Code that catches InvalidJwtException to reject bad tokens lets that one through as an unexpected error.

While fixing it, JWK.kty turned out to throw ClassCastException on every read. JWK.kt is in the jwt package and does not import the module's JsonObject.getValue delegate, so by jwk resolves to the stdlib Map delegate, which casts the raw JsonPrimitive to KeyType.

Fix

  • Before verifying, check that the matched key fits the header alg: the key's alg when it has one, otherwise its kty (and crv P-256 for ES256). A mismatch throws InvalidJwtException and does not fall back to the Auth server.
  • Import io.github.jan.supabase.auth.getValue in JWK.kt so kty decodes to KeyType.

supabase-flutter made the same check in supabase/supabase-flutter#2091.

Tests

  • testGetClaimsRejectsHeaderAlgNotMatchingKeyAlg: an ES256 header with the RSA key that declares alg: RS256.
  • testGetClaimsRejectsHeaderAlgNotMatchingKeyType: an RS256 header with an EC key that has no alg, which also covers kty.

Both fail without the fix (IllegalStateException: Param not found). All 198 Auth JVM tests pass with it, and detekt is clean.

No public API change.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants