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

Raise InvalidGrantError if no associated grant exists when invalidating an authorization code #1476

Conversation

mforner13
Copy link
Contributor

@mforner13 mforner13 commented Sep 3, 2024

Description of the Change

This change raises an InvalidGrantError from the OAuth2Validator.invalidate_authorization_code method if the Grant object intended to be deleted does not exist.

Currently, when invalidating an authorization code after it has been used, if for whatever reason the associated grant object no longer exists, an uncaught Grant.DoesNotExist exception is raised. This leads to 500 responses being returned to clients. This could, for example, be caused by concurrent requests being made using the same authorization code.

The change in this PR handles this scenario gracefully by catching Grant.DoesNotExist and raising an InvalidGrantError which will return a 400 'invalid_grant' response to the client.

Checklist

  • PR only contains one change (considered splitting up PR)
  • unit-test added
  • documentation updated (No existing documentation for this validator class)
  • CHANGELOG.md updated (only for user relevant changes)
  • author name in AUTHORS

Previously, when invalidating an authorization code after it has been used, if for whatever reason the associated grant object no longer exists, an uncaught exception would be raised - Grant.DoesNotExist.

This could be caused by concurrent requests being made using the same authorization token.

We now handle this scenario gracefully by catching Grant.DoesNotExist and returning an InvalidGrantError.
@mforner13 mforner13 force-pushed the raise-invalid-grant-error-invalidate-auth-code branch from 3316ac5 to 7becf1c Compare September 3, 2024 13:33
@n2ygk n2ygk self-requested a review September 3, 2024 14:05
Copy link
Member

@n2ygk n2ygk left a comment

Choose a reason for hiding this comment

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

Hey, thanks for this! Nice improvement.

@n2ygk n2ygk added this to the 3.0.0 milestone Sep 3, 2024
@n2ygk n2ygk merged commit 62508b4 into jazzband:master Sep 3, 2024
19 checks passed
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