Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: add get_claims method #681

Merged
merged 15 commits into from
Mar 25, 2025
Merged

feat: add get_claims method #681

merged 15 commits into from
Mar 25, 2025

Conversation

grdsdev
Copy link
Contributor

@grdsdev grdsdev commented Feb 25, 2025 •
edited
Loading

What kind of change does this PR introduce?

Add get_claims method for verifying both symmetric and asymmetric JWT and returning whole set of claims

Reference PR: supabase/auth-js#1030

@linear Linear
Copy link

linear bot commented Feb 25, 2025

CLIBS-102 [auth-py] add getClaims method

@coveralls
Copy link

coveralls commented Feb 25, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 13930276879

Details

  • 166 of 166 (100.0%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+19.8%) to 93.7%
Totals Coverage Status
Change from base Build 13632531815: 19.8%
Covered Lines: 1993
Relevant Lines: 2127
💛 - Coveralls

@grdsdev grdsdev requested a review from hf March 12, 2025 10:20
@grdsdev grdsdev force-pushed the guilherme/clibs-102-auth-py-add-getclaims-method branch from 5eed291 to 46c4c37 Compare March 12, 2025 10:22
grdsdev
grdsdev commented Mar 12, 2025
View reviewed changes
@grdsdev grdsdev requested a review from silentworks March 12, 2025 13:44
@grdsdev grdsdev marked this pull request as ready for review March 12, 2025 13:46
hf
hf reviewed Mar 13, 2025
View reviewed changes
supabase_auth/_async/gotrue_client.py
Comment on lines +1174 to +1178
# try fetching from the cache.
jwk = next(
(jwk for jwk in self._jwks["keys"] if jwk["kid"] == kid),
None,
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just want to check, how long is this kept in the cache?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the application lifecycle, how long it is supposed to be in the cache?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we've decided to go with 10mins in c7a1903

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hf talking with @kangmingtay about this, decided to put a TTL on the cache, please review it again c7a1903

hf
hf approved these changes Mar 13, 2025
View reviewed changes
Copy link

@hf hf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good! I'll reach out to you on how to get an asymmetric JWT on staging to try it!

@grdsdev grdsdev force-pushed the guilherme/clibs-102-auth-py-add-getclaims-method branch from 3c46e99 to c7a1903 Compare March 14, 2025 09:58
@grdsdev grdsdev requested a review from hf March 14, 2025 10:02
@silentworks
Copy link
Contributor

Is the decode_jwt function backward compatible? will it still work with existing JWTs with this change?

@grdsdev
Copy link
Contributor Author

grdsdev commented Mar 18, 2025

Is the decode_jwt function backward compatible? will it still work with existing JWTs with this change?

@silentworks yes it is, we should have a test for that, let me check, if not I'll add one.

@grdsdev
Copy link
Contributor Author

grdsdev commented Mar 18, 2025

  • Fixed base64 token validation and added more tests to make sure decode_jwt is backward compatible in 6a90f66
  • Noticed a warning while running tests complaining about the refresh token Timer not being released, fixed in 66ffe5d

@silentworks please review again.

@grdsdev grdsdev merged commit f5c9926 into main Mar 25, 2025
11 checks passed
@grdsdev grdsdev deleted the guilherme/clibs-102-auth-py-add-getclaims-method branch March 25, 2025 09:10
@github-actions github-actions bot mentioned this pull request Mar 25, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kangmingtay kangmingtay

@silentworks silentworks

@hf hf

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@grdsdev @coveralls @silentworks @hf @kangmingtay