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.

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

Return Unauthorized on malformed OAuth token #743

Merged
merged 1 commit into from
Oct 23, 2023

Conversation

matyaskuti
Copy link
Contributor

About this change - What it does

Handle JWT DecodeErrors when extracting the expiration timestamp from an OIDC/OAuth2 JWT token, otherwise this would result in an HTTP 500 response.

Why this way

Catching PyJWT's DecodeError is sufficient, as all other exceptions from the library (see
https://pyjwt.readthedocs.io/en/stable/api.html#exceptions) are related to proper verification, which we do not do at the moment.

Handle JWT DecodeErrors when extracting the expiration timestamp from an
OIDC/OAuth2 JWT token, otherwise this would result in an HTTP 500
response.

Catching PyJWT's `DecodeError` is sufficient, as all other exceptions
from the library (see
https://pyjwt.readthedocs.io/en/stable/api.html#exceptions) are related
to proper verification, which we do not do at the moment.
@matyaskuti matyaskuti force-pushed the matyaskuti/malformed_oauth_token branch from 6866263 to c93bf5d Compare October 23, 2023 09:26
@matyaskuti matyaskuti marked this pull request as ready for review October 23, 2023 11:57
@matyaskuti matyaskuti requested review from a team as code owners October 23, 2023 11:57
Copy link
Contributor

@aiven-anton aiven-anton left a comment

Choose a reason for hiding this comment

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

This looks all good and sound 👍

"auth_header",
(f"Bearer {jwt.encode({'exp': 1697013997}, 'secret')}XX", "Bearer NotAToken"),
)
def test_get_expiration_time_from_header_malformed_bearer_token_raises_unauthorized(auth_header: str) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: Structuring tests this way makes it very easy to review and understand what the expected outcome is 👍

@aiven-anton aiven-anton merged commit 7b88604 into main Oct 23, 2023
8 checks passed
@aiven-anton aiven-anton deleted the matyaskuti/malformed_oauth_token branch October 23, 2023 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants