OBACK-290: Fix issue with python Authenticate JWT locally logic #209
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bgier-stytch
commented
Jul 16, 2024
| @@ -1 +1 @@ | |||
| __version__ = "10.1.0" | |||
| __version__ = "11.0.0" | |||
There was a problem hiding this comment.
Since we're changing the return type of Authenticate this should be a major version bump.
logan-stytch
approved these changes
Jul 16, 2024
| @@ -258,7 +259,7 @@ def authenticate_jwt( | |||
| session_jwt: str, | |||
There was a problem hiding this comment.
I can't comment any higher, but # ADDIMPORT: from stytch.consumer.models.sessions import AuthenticateJWTLocalResponse or it'll get removed the next time codegen runs
stytch/consumer/api/sessions.py
Outdated
| self.authenticate_jwt_local( | ||
| session_jwt=session_jwt, | ||
| max_token_age_seconds=max_token_age_seconds, | ||
| # Return the local_result if available, otherwise call the Stytch API |
stytch/consumer/api/sessions.py
Outdated
| session_jwt=session_jwt, | ||
| max_token_age_seconds=max_token_age_seconds, | ||
| # Return the local_result if available, otherwise call the Stytch API | ||
| local_token = self.authenticate_jwt_local( |
There was a problem hiding this comment.
[dust] local_resp or local_res feels like a better name than local_token
| @@ -232,6 +234,23 @@ def test_webauthn(self) -> None: | |||
| # TODO: No test public key credential (see skipTest above) | |||
| self.assertTrue(api.authenticate(public_key_credential="").is_success) | |||
|
|
|||
| def test_authenticate(self) -> None: | |||
There was a problem hiding this comment.
Sweet! Thanks for adding a test!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes an issue related to how pyJWT handles verifications. The verification logic raises Exceptions in the event that data is bad (such as an expired token being too old), which was breaking our function's ability to call the underlying authenticate endpoint and receive a new JWT token.
We now except this exception and return None, which then calls our underlying authenticate endpoint to grab a new JWT.
This change also updates our response type for this endpoint to return a dedicated type here, which is more in line with how Node handles it.