fix: update encryptedKey alongside key on OAuth token refresh#28460
Draft
fix: update encryptedKey alongside key on OAuth token refresh#28460
Conversation
When a token is refreshed, updateTokenObjectInDb only updated the key column in the database. If the credential also had encryptedKey set (which happens when the CREDENTIALS keyring is configured), subsequent requests would decrypt the stale encryptedKey and see an expired token, triggering a redundant Google /token call on every request. Now updateTokenObjectInDb re-encrypts the refreshed token and persists it to encryptedKey as well, so the next request reads the fresh token and skips the unnecessary refresh. Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com>
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
This file contains hidden or 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
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.
What does this PR do?
When OAuth tokens are refreshed,
updateTokenObjectInDbupdates thekeycolumn but never updatesencryptedKey. SincegetCalendarsEventsalways prefersencryptedKeyoverkeywhen it exists, subsequent requests decrypt the stale encrypted token (with the originalexpiry_date), see it as expired, and trigger another Google/tokencall — on every single request.This PR re-encrypts the refreshed token and persists it to
encryptedKeyalongsidekey, using the sameencryptSecretpattern as initial credential creation.Changes:
updateTokenObject.ts— addedtryEncryptTokenObjecthelper; spreadsencryptedKeyinto all three DB write paths (delegation update, delegation create, direct credential update). Added requiredcredentialTypeparam to theoauthstrategy branch.CredentialRepository.ts— widenedupdateWhereIdandupdateWhereUserIdAndDelegationCredentialIdsignatures to accept optionalencryptedKey.CrmService.ts(Pipedrive) — passescredentialTypeto satisfy the updated signature.CalendarAuth.ts→OAuthManager→updateTokenObjectInDb) also passescredentialTypeafter this change. This is the primary path that triggered the bug. If the intermediate callback wiring doesn't forwardcredentialType, the encryption won't happen for Google Calendar credentials specifically.createDelegationCredential— The JWT fallback path (line 84) spreadsencryptedKeyintocreateDelegationCredential. Confirm that method's Prismacreatecall acceptsencryptedKeyin its data.tryEncryptTokenObjectreturnsundefinedon error (silently skipsencryptedKeyupdate). This means the stale-token problem persists if encryption breaks, butkeyis still updated so the system degrades gracefully.Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
keyandencryptedKey)expiry_dateto pass (or manually set it to the past in the DB)getScheduleorcalendarOverlayendpoint multiple times with >2s gaps/tokencallAlternatively, inspect the
Credentialtable after a token refresh and confirmencryptedKeywas updated (decrypt it and checkexpiry_datematches the fresh token).Checklist
Link to Devin session: https://app.devin.ai/sessions/d8775f5ea1d6469c9853ca400e8cf286
Requested by: @volnei