Do not convert to bytes the refresh_token (#3273)

* Do not convert to bytes the `refresh_token`

When the body is encoded, a mix of bytes and unicode happens and it
fails at this line

https://github.com/requests/requests-oauthlib/blob/c472e6bf0468f1bd52b70a64e9aac92bee41c491/requests_oauthlib/oauth2_session.py#L285

(Pdb) body
u'grant_type=refresh_token&allow_redirects=True&refresh_token=b%27z9BSRKqf2QD4pcDhqj%27&client_id=CLIENT_ID&client_secret=CLIENT_SECRET'

(note those weird `%27` in the string)

* Handle OAuth specific exception properly at `paginate`

* Do not convert the `acess_token` to bytes

* Fix lint :(

* Refactor `paginate` to handle exception in Bitbucket and Github

`paginate` method is shared between both services and each service
implement `get_next_url_to_paginate` and `get_paginated_results`.

* Names and attributes fixes

Author: humitos

readthedocs/oauth/services/base.py

@@ -1,13 +1,14 @@
-"""OAuth utility functions"""
+"""OAuth utility functions."""
 
 from __future__ import absolute_import
-from builtins import str
 from builtins import object
 import logging
 from datetime import datetime
 
 from django.conf import settings
 from requests_oauthlib import OAuth2Session
+from requests.exceptions import RequestException
+from oauthlib.oauth2.rfc6749.errors import InvalidClientIdError
 from allauth.socialaccount.models import SocialAccount
 
 
@@ -69,13 +70,13 @@ def create_session(self):
             return None
 
         token_config = {
-            'access_token': str(token.token),
+            'access_token': token.token,
             'token_type': 'bearer',
         }
         if token.expires_at is not None:
             token_expires = (token.expires_at - datetime.now()).total_seconds()
             token_config.update({
-                'refresh_token': str(token.token_secret),
+                'refresh_token': token.token_secret,
                 'expires_in': token_expires,
             })
 
@@ -114,6 +115,34 @@ def _updater(data):
 
         return _updater
 
+    def paginate(self, url):
+        """Recursively combine results from service's pagination.
+
+        :param url: start url to get the data from.
+        :type url: unicode
+        """
+        try:
+            resp = self.get_session().get(url)
+            next_url = self.get_next_url_to_paginate(resp)
+            results = self.get_paginated_results(resp)
+            if next_url:
+                results.extend(self.paginate(next_url))
+            return results
+        # Catch specific exception related to OAuth
+        except InvalidClientIdError:
+            log.error('access_token or refresh_token failed: %s', url)
+            raise Exception('You should reconnect your account')
+        # Catch exceptions with request or deserializing JSON
+        except (RequestException, ValueError):
+            # Response data should always be JSON, still try to log if not though
+            try:
+                debug_data = resp.json()
+            except ValueError:
+                debug_data = resp.content
+            log.debug('paginate failed at %s with response: %s', url, debug_data)
+        finally:
+            return []
+
     def sync(self):
         raise NotImplementedError
 
@@ -124,6 +153,22 @@ def create_repository(self, fields, privacy=DEFAULT_PRIVACY_LEVEL,
     def create_organization(self, fields):
         raise NotImplementedError
 
+    def get_next_url_to_paginate(self, response):
+        """Return the next url to feed the `paginate` method.
+
+        :param response: response from where to get the `next_url` attribute
+        :type response: requests.Response
+        """
+        raise NotImplementedError
+
+    def get_paginated_results(self, response):
+        """Return the results for the current response/page.
+
+        :param response: response from where to get the results.
+        :type response: requests.Response
+        """
+        raise NotImplementedError
+
     def setup_webhook(self, project):
         raise NotImplementedError
 

readthedocs/oauth/services/bitbucket.py

@@ -164,18 +164,11 @@ def create_organization(self, fields):
         organization.save()
         return organization
 
-    def paginate(self, url):
-        """Recursively combine results from Bitbucket pagination
+    def get_next_url_to_paginate(self, response):
+        return response.json().get('next')
 
-        :param url: start url to get the data from.
-        """
-        resp = self.get_session().get(url)
-        data = resp.json()
-        results = data.get('values', [])
-        next_url = data.get('next')
-        if next_url:
-            results.extend(self.paginate(next_url))
-        return results
+    def get_paginated_results(self, response):
+        return response.json().get('values', [])
 
     def get_webhook_data(self, project, integration):
         """Get webhook JSON data to post to the API"""

readthedocs/oauth/services/github.py

@@ -148,19 +148,11 @@ def create_organization(self, fields):
         organization.save()
         return organization
 
-    def paginate(self, url):
-        """Combines return from GitHub pagination
+    def get_next_url_to_paginate(self, response):
+        return response.links.get('next', {}).get('url')
 
-        :param url: start url to get the data from.
-
-        See https://developer.github.com/v3/#pagination
-        """
-        resp = self.get_session().get(url)
-        result = resp.json()
-        next_url = resp.links.get('next', {}).get('url')
-        if next_url:
-            result.extend(self.paginate(next_url))
-        return result
+    def get_paginated_results(self, response):
+        return response.json()
 
     def get_webhook_data(self, project, integration):
         """Get webhook JSON data to post to the API"""