fix: preserve existing refresh token when OAuth response omits it#1055
fix: preserve existing refresh token when OAuth response omits it#1055Daryna Ishchenko (darynaishchenko) wants to merge 1 commit into
Conversation
Co-Authored-By: Daryna Ishchenko <darina.ishchenko17@gmail.com>
🤖 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:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This CDK VersionYou can test this version of the CDK using the following: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1781794789-fix-missing-refresh-token#egg=airbyte-python-cdk[dev]' --help
# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch devin/1781794789-fix-missing-refresh-tokenPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesOptional Refresh Token Handling in SingleUseRefreshTokenOauth2Authenticator
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
SingleUseRefreshTokenOauth2Authenticator.refresh_and_set_access_token()unconditionally overwrites the refresh token with whatever_extract_refresh_token()returns — includingNonewhen the OAuth provider omitsrefresh_tokenfrom the response.This silently destroys the working refresh token and persists
Noneinto the connector config via the control message, causing all subsequent token refreshes to fail.def refresh_and_set_access_token(self) -> None: new_access_token, access_token_expires_in, new_refresh_token = self.refresh_access_token() self.access_token = new_access_token - self.set_refresh_token(new_refresh_token) + if new_refresh_token is not None: + self.set_refresh_token(new_refresh_token) self.set_token_expiry_date(access_token_expires_in)Microsoft's
/oauth2/v2.0/tokenendpoint typically returns a new refresh token whenoffline_accessscope is requested, but this is not guaranteed by spec. Other OAuth providers (e.g., Google) intentionally omit the refresh token on refresh responses. The fix is defensive — keep the existing token when the response doesn't provide a replacement.Link to Devin session: https://app.devin.ai/sessions/c87fd00904c446c6a38fc7313ca7b95c
Requested by: Daryna Ishchenko (@darynaishchenko)
Summary by CodeRabbit
Bug Fixes
Tests