Fix oauth/token path, recursive overflow #59
Conversation
WalkthroughThe pull request introduces a minor modification to the Changes
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
trakt/api.py (2)
Line range hint
233-252: Consider adding a retry limit for token refresh.While the error handling is good, consider adding a retry limit to prevent excessive refresh attempts if the server consistently returns errors. This would help prevent token bans due to too many requests.
Example implementation:
def refresh_token(self): """Request Trakt API for a new valid OAuth token using refresh_token""" + MAX_RETRIES = 3 + self._refresh_attempts = getattr(self, '_refresh_attempts', 0) + if self._refresh_attempts >= MAX_RETRIES: + self.logger.error("Max token refresh attempts reached. Manual intervention required.") + return + self._refresh_attempts += 1 self.logger.info("OAuth token has expired, refreshing now...") # ... rest of the method ... try: response = self.client.post('oauth/token', data) + # Reset attempts on success + self._refresh_attempts = 0 except OAuthException: self.logger.debug( "Rejected - Unable to refresh expired OAuth token, " "refresh_token is invalid" ) return
Line range hint
233-252: Enhance security by masking sensitive data in logs.The debug logging could potentially expose sensitive OAuth tokens. Consider masking sensitive information in log messages.
Example implementation:
def refresh_token(self): + def mask_token(token): + if not token: + return None + return token[:4] + '*' * (len(token) - 8) + token[-4:] try: response = self.client.post('oauth/token', data) except OAuthException: self.logger.debug( "Rejected - Unable to refresh expired OAuth token, " "refresh_token is invalid" ) return + # Mask tokens in logs + masked_access_token = mask_token(response.get("access_token")) + masked_refresh_token = mask_token(response.get("refresh_token")) + self.logger.debug( + "Received new tokens - access: %s, refresh: %s", + masked_access_token, + masked_refresh_token + ) self.config.update( OAUTH_TOKEN=response.get("access_token"), OAUTH_REFRESH=response.get("refresh_token"), OAUTH_EXPIRES_AT=response.get("created_at") + response.get("expires_in"), )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
trakt/api.py(1 hunks)
🔇 Additional comments (1)
trakt/api.py (1)
233-233: Fix for recursive overflow and double-slash URL issues.The removal of the leading slash in the URL path addresses two issues:
- Prevents double slashes when combined with
base_urlinHttpClient.request- Fixes a potential recursive overflow where the token refresh request itself would trigger authentication due to the path starting with '/oauth/'
Let's verify the URL construction and potential recursive calls:
Suggestion posted by coderabbit: - #59 (review)
Suggestion posted by coderabbit: - #59 (review)
|
perhaps the token ban was a mitigation of another problem: timeline seems to match |
Even managed to get my token banned with too many requests