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

feat: purge OAuth2 tokens when DB changes #31164

Merged
merged 2 commits into from
Nov 26, 2024

Conversation

betodealmeida
Copy link
Member

SUMMARY

Add a new method to the UpdateDatabaseCommand so that, if the OAuth2 client information has changed, existing tokens are purged, since they are unlikely to still be valid. This is done if any of these keys change:

  • Client ID (but not client secret)
  • Scope (tokens are associated with a given scope)
  • Endpoint URLs (likely a new OAuth2 provider, but not necessarily — we purge to be safe)

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

Added tests.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@betodealmeida betodealmeida force-pushed the delete-tokens-scope-change branch from c3dc9cf to ffed4bb Compare November 26, 2024 15:05
@sadpandajoe sadpandajoe added the review:checkpoint Last PR reviewed during the daily review standup label Nov 26, 2024
Copy link
Contributor

@fisjac fisjac left a comment

Choose a reason for hiding this comment

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

Changes look good to me.

When OAuth2 client info is changed through DATABASE_OAUTH2_CLIENTS in the config.py file, will the purge trigger as well, or only when a PUT request is made to the db connection?

@betodealmeida
Copy link
Member Author

Changes look good to me.

When OAuth2 client info is changed through DATABASE_OAUTH2_CLIENTS in the config.py file, will the purge trigger as well, or only when a PUT request is made to the db connection?

Ah, good point! I added a notice to the config.py about this, if people are changing the config.py file they should be able to handle the token deletion themselves.

@betodealmeida betodealmeida force-pushed the delete-tokens-scope-change branch from c6e11a7 to 7589e91 Compare November 26, 2024 20:34
@betodealmeida betodealmeida requested a review from fisjac November 26, 2024 20:34
@betodealmeida betodealmeida merged commit 68499a1 into master Nov 26, 2024
33 checks passed
@sadpandajoe sadpandajoe removed the review:checkpoint Last PR reviewed during the daily review standup label Nov 27, 2024
@rusackas rusackas deleted the delete-tokens-scope-change branch December 5, 2024 02:59
betodealmeida added a commit that referenced this pull request Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants