-
Notifications
You must be signed in to change notification settings - Fork 428
Making OAuth token refresh more robust #767
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
Conversation
…oken validity handling in CRUD operations
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
✨ No issues found! Your code is sparkling clean! ✨ 🗒️ View all ignored comments in this repo
Need help? Join our Discord for support! |
…e on creation and mark tokens as invalid when refreshed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Summary
This PR enhances OAuth token handling by introducing explicit token validity tracking through a new isValid
boolean field in both OAuthToken
and OAuthAccessToken
models. Key changes include:
- Added
isValid
column (defaulting to true) to track token validity states without deletion - Improved token refresh logic that iterates through available refresh tokens
- Enhanced error handling during token refresh process
The changes make the token refresh system more resilient by maintaining token history and handling invalidation scenarios (particularly with providers like GitHub that can invalidate tokens server-side) more gracefully.
Confidence score: 4.5/5
- This PR is very safe to merge as it adds non-breaking changes with good defaults
- The score reflects the well-structured changes, clear migration path, and improved error handling, with a small deduction for potential edge cases in token refresh logic
- Focus attention on crud.tsx to ensure the token refresh retry logic doesn't introduce any unintended side effects
3 files reviewed, 3 comments
Edit PR Review Bot Settings | Greptile
.../backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/access-token/crud.tsx
Outdated
Show resolved
Hide resolved
.../backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/access-token/crud.tsx
Show resolved
Hide resolved
.../backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/access-token/crud.tsx
Show resolved
Hide resolved
.../backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/access-token/crud.tsx
Show resolved
Hide resolved
.../backend/src/app/api/latest/connected-accounts/[user_id]/[provider_id]/access-token/crud.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! good work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, is it possible to E2E test this, or would that be too hard?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be a bit annoying to do, I will merge this so it get deployed ASAP and create another PR for the tests
…provider_id]/access-token/crud.tsx Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
…oken validity handling in CRUD operations
Important
Enhances OAuth token handling by adding validity checks, marking invalid tokens, and attempting token refreshes in
crud.tsx
.isValid
column toOAuthAccessToken
andOAuthToken
inmigration.sql
andschema.prisma
.crud.tsx
, filters tokens byisValid
status and marks them invalid if server-side checks fail.captureError
to log token refresh errors incrud.tsx
.TokenSet
andcaptureError
incrud.tsx
.This description was created by
for a45b64f. You can customize this summary. It will automatically update as commits are pushed.