-
-
Notifications
You must be signed in to change notification settings - Fork 274
fix(gotrue): fix getClaims JWT token decoding #1288
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a critical bug in GoTrueClient.getClaims() where JWT token decoding fails with a null pointer exception when verifying JWTs signed with asymmetric algorithms (RS256, ES256). The fix properly handles the case where _jwks is not yet initialized by providing an empty fallback, and replaces the JWT verification implementation with jose_plus library for proper asymmetric key support.
Key Changes:
- Fixed null safety issue in
getClaims()by providing an emptyJWKSetfallback when_jwksis null - Replaced
dart_jsonwebtokenwithjose_plusfor JWT verification to properly support asymmetric algorithms - Added comprehensive test coverage for JWKS-based verification path with support for both symmetric (HS256) and asymmetric (ES256/RS256) JWT signing
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/gotrue/lib/src/gotrue_client.dart | Fixed null pointer exception by providing fallback for _jwks and implemented JWT verification using jose_plus library |
| packages/gotrue/lib/src/types/jwt.dart | Removed dart_jsonwebtoken dependency and the unused rsaPublicKey getter from JWK class |
| packages/gotrue/pubspec.yaml | Added jose_plus as a production dependency and moved dart_jsonwebtoken to dev dependencies |
| packages/gotrue/test/utils.dart | Added support for generating service role tokens with both symmetric and asymmetric signing algorithms |
| packages/gotrue/test/get_claims_test.dart | Added test case to verify JWKS-based JWT verification for asymmetric algorithms |
| packages/gotrue/test/src/gotrue_admin_oauth_api_test.dart | Refactored to use centralized getServiceRoleToken() utility function |
| packages/gotrue/test/src/gotrue_admin_mfa_api_test.dart | Refactored to use centralized getServiceRoleToken() utility function |
| packages/gotrue/.env.example | Added example environment configuration for both symmetric and asymmetric JWT signing |
| infra/gotrue/docker-compose.jwk.yml | Added docker-compose override file for testing with JWKS/asymmetric JWT signing |
| .github/workflows/gotrue.yml | Added new test-jwks job to run tests with JWKS-enabled configuration |
| .github/workflows/dart-package-test.yml | Enhanced to support multiple docker-compose files for more flexible test configurations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@khensunny thanks for opening this PR, I'll review and provide any feedback if needed. |
7e22b98 to
4db3241
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4db3241 to
c20ec04
Compare
- update tests - add jose_plus package - update CI and docker infra
c20ec04 to
e3898c7
Compare
What kind of change does this PR introduce?
Bug fix (fixes
GoTrueClient.getClaims()JWT decoding for asymmetric algorithms), plus tests, CI/workflow updates, package/pubspec adjustments, and infra (JWKS docker-compose) additions.What is the current behavior?
getClaims()can throwDartError: Unexpected null value.when verifying JWTs signed with asymmetric algorithms (e.g.,RS256,ES256) because_jwksis force-unwrapped before it is initialized — see #1286. CI/tests may not surface this because the local test auth uses HS* signing.What is the new behavior?
getClaims()no longer force-unwraps_jwks; the JWKS path is handled safely by fetching and caching/.well-known/jwks.jsonon first use (or falling back appropriately), so asymmetric (RS*/ES*) JWTs withkidare verified without crashing. Tests updated to cover the JWKS path; CI/workflows, gotrue pubspec, and docker-compose.jwk.yml were updated to support and validate the fix.Feel free to include screenshots if it includes visual changes.
Additional context
Add any other context or screenshots.