Integration Re-sync Bug Fix

readthedocs/integrations/models.py

@@ -274,6 +274,10 @@ def recreate_secret(self):
         self.secret = get_secret()
         self.save(update_fields=['secret'])
 
+    def remove_secret(self):
+        self.secret = None
+        self.save(update_fields=['secret'])
+
     def __str__(self):
         return (
             _('{0} for {1}')

readthedocs/oauth/services/bitbucket.py

@@ -251,7 +251,6 @@ def setup_webhook(self, project, integration=None):
                     'permissions: project=%s',
                     project,
                 )
-                return (False, resp)
 
         # Catch exceptions with request or deserializing JSON
         except (RequestException, ValueError):
@@ -271,7 +270,8 @@ def setup_webhook(self, project, integration=None):
                 )
             except ValueError:
                 pass
-            return (False, resp)
+
+        return (False, resp)
 
     def update_webhook(self, project, integration):
         """
@@ -308,15 +308,19 @@ def update_webhook(self, project, integration):
             # Bitbucket returns 404 when the webhook doesn't exist. In this
             # case, we call ``setup_webhook`` to re-configure it from scratch
             if resp.status_code == 404:
-                return self.setup_webhook(project)
+                return self.setup_webhook(project, integration)
 
         # Catch exceptions with request or deserializing JSON
         except (KeyError, RequestException, TypeError, ValueError):
+            # We get TypeError when the provider_data is None
+            # it only happens if the webhook attachment was not successful in the first place
+            if not integration.provider_data:
+                return self.setup_webhook(project, integration)
+
             log.exception(
                 'Bitbucket webhook update failed for project: %s',
                 project,
             )
-            return (False, resp)
         else:
             log.error(
                 'Bitbucket webhook update failed for project: %s',
@@ -331,4 +335,5 @@ def update_webhook(self, project, integration):
                 'Bitbucket webhook update failure response: %s',
                 debug_data,
             )
-            return (False, resp)
+
+        return (False, resp)

readthedocs/oauth/services/github.py

@@ -203,7 +203,7 @@ def setup_webhook(self, project, integration=None):
         """
         session = self.get_session()
         owner, repo = build_utils.get_github_username_repo(url=project.repo)
-        if integration:
+        if integration and not integration.secret:
             integration.recreate_secret()
         else:
             integration, _ = Integration.objects.get_or_create(
@@ -221,6 +221,7 @@ def setup_webhook(self, project, integration=None):
                 data=data,
                 headers={'content-type': 'application/json'},
             )
+
             # GitHub will return 200 if already synced
             if resp.status_code in [200, 201]:
                 recv_data = resp.json()
@@ -238,10 +239,9 @@ def setup_webhook(self, project, integration=None):
                     'permissions: project=%s',
                     project,
                 )
-                # Set the secret to None so that the integration can be used manually.
-                integration.secret = None
-                integration.save()
-                return (False, resp)
+
+            # All other status codes will flow to the `else` clause below
+
         # Catch exceptions with request or deserializing JSON
         except (RequestException, ValueError):
             log.exception(
@@ -263,7 +263,10 @@ def setup_webhook(self, project, integration=None):
                 'GitHub webhook creation failure response: %s',
                 debug_data,
             )
-            return (False, resp)
+
+        # Always remove the secret and return False if we don't return True above
+        integration.remove_secret()
+        return (False, resp)
 
     def update_webhook(self, project, integration):
         """
@@ -277,39 +280,46 @@ def update_webhook(self, project, integration):
         :rtype: (Bool, Response)
         """
         session = self.get_session()
-        integration.recreate_secret()
+        if not integration.secret:
+            integration.recreate_secret()
         data = self.get_webhook_data(project, integration)
         resp = None
+
+        # Handle the case where we don't have a proper provider_data set
+        # This happens with a user-managed webhook previously
+        if not integration.provider_data:
+            return self.setup_webhook(project, integration)
+
         try:
             url = integration.provider_data.get('url')
             resp = session.patch(
                 url,
                 data=data,
                 headers={'content-type': 'application/json'},
             )
+
             # GitHub will return 200 if already synced
             if resp.status_code in [200, 201]:
                 recv_data = resp.json()
                 integration.provider_data = recv_data
                 integration.save()
                 log.info(
-                    'GitHub webhook creation successful for project: %s',
+                    'GitHub webhook update successful for project: %s',
                     project,
                 )
                 return (True, resp)
 
             # GitHub returns 404 when the webhook doesn't exist. In this case,
             # we call ``setup_webhook`` to re-configure it from scratch
             if resp.status_code == 404:
-                return self.setup_webhook(project)
+                return self.setup_webhook(project, integration)
 
         # Catch exceptions with request or deserializing JSON
         except (AttributeError, RequestException, ValueError):
             log.exception(
                 'GitHub webhook update failed for project: %s',
                 project,
             )
-            return (False, resp)
         else:
             log.error(
                 'GitHub webhook update failed for project: %s',
@@ -320,10 +330,12 @@ def update_webhook(self, project, integration):
             except ValueError:
                 debug_data = resp.content
             log.debug(
-                'GitHub webhook creation failure response: %s',
+                'GitHub webhook update failure response: %s',
                 debug_data,
             )
-            return (False, resp)
+
+        integration.remove_secret()
+        return (False, resp)
 
     def send_build_status(self, build, commit, state):
         """

readthedocs/oauth/services/gitlab.py

@@ -14,7 +14,7 @@
     RTD_BUILD_STATUS_API_NAME,
     SELECT_BUILD_STATUS,
 )
-from readthedocs.builds.utils import get_gitlab_username_repo
+from readthedocs.builds import utils as build_utils
 from readthedocs.integrations.models import Integration
 from readthedocs.projects.models import Project
 
@@ -52,11 +52,11 @@ def _get_repo_id(self, project):
         # https://docs.gitlab.com/ce/api/README.html#namespaced-path-encoding
         try:
             repo_id = json.loads(project.remote_repository.json).get('id')
-        except Project.remote_repository.RelatedObjectDoesNotExist:
+        except Exception:
             # Handle "Manual Import" when there is no RemoteRepository
             # associated with the project. It only works with gitlab.com at the
             # moment (doesn't support custom gitlab installations)
-            username, repo = get_gitlab_username_repo(project.repo)
+            username, repo = build_utils.get_gitlab_username_repo(project.repo)
             if (username, repo) == (None, None):
                 return None
 
@@ -278,23 +278,23 @@ def setup_webhook(self, project, integration=None):
         :returns: boolean based on webhook set up success
         :rtype: bool
         """
-        if integration:
+        resp = None
+        if integration and not integration.secret:
             integration.recreate_secret()
         else:
             integration, _ = Integration.objects.get_or_create(
                 project=project,
                 integration_type=Integration.GITLAB_WEBHOOK,
             )
         repo_id = self._get_repo_id(project)
+
         if repo_id is None:
             # Set the secret to None so that the integration can be used manually.
-            integration.secret = None
-            integration.save()
-            return (False, None)
+            integration.remove_secret()
+            return (False, resp)
 
         data = self.get_webhook_data(repo_id, project, integration)
         session = self.get_session()
-        resp = None
         try:
             resp = session.post(
                 '{url}/api/v4/projects/{repo_id}/hooks'.format(
@@ -304,6 +304,7 @@ def setup_webhook(self, project, integration=None):
                 data=data,
                 headers={'content-type': 'application/json'},
             )
+
             if resp.status_code == 201:
                 integration.provider_data = resp.json()
                 integration.save()
@@ -319,23 +320,21 @@ def setup_webhook(self, project, integration=None):
                     'permissions: project=%s',
                     project,
                 )
-                # Set the secret to None so that the integration can be used manually.
-                integration.secret = None
-                integration.save()
-                return (False, resp)
 
         except (RequestException, ValueError):
             log.exception(
                 'GitLab webhook creation failed for project: %s',
                 project,
             )
-            return (False, resp)
         else:
             log.error(
                 'GitLab webhook creation failed for project: %s',
                 project,
             )
-            return (False, resp)
+
+        # Always remove secret and return False if we don't return True above
+        integration.remove_secret()
+        return (False, resp)
 
     def update_webhook(self, project, integration):
         """
@@ -352,17 +351,24 @@ def update_webhook(self, project, integration):
 
         :rtype: (Bool, Response)
         """
+        resp = None
         session = self.get_session()
-
         repo_id = self._get_repo_id(project)
+
         if repo_id is None:
-            return (False, None)
+            return (False, resp)
+
+        # When we don't have provider_data, we aren't managing this webhook so setup a new one
+        if not integration.provider_data:
+            return self.setup_webhook(project, integration)
+
+        if not integration.secret:
+            integration.recreate_secret()
 
-        integration.recreate_secret()
         data = self.get_webhook_data(repo_id, project, integration)
-        hook_id = integration.provider_data.get('id')
-        resp = None
+
         try:
+            hook_id = integration.provider_data.get('id')
             resp = session.put(
                 '{url}/api/v4/projects/{repo_id}/hooks/{hook_id}'.format(
                     url=self.adapter.provider_base_url,
@@ -372,6 +378,7 @@ def update_webhook(self, project, integration):
                 data=data,
                 headers={'content-type': 'application/json'},
             )
+
             if resp.status_code == 200:
                 recv_data = resp.json()
                 integration.provider_data = recv_data
@@ -385,10 +392,10 @@ def update_webhook(self, project, integration):
             # GitLab returns 404 when the webhook doesn't exist. In this case,
             # we call ``setup_webhook`` to re-configure it from scratch
             if resp.status_code == 404:
-                return self.setup_webhook(project)
+                return self.setup_webhook(project, integration)
 
         # Catch exceptions with request or deserializing JSON
-        except (RequestException, ValueError):
+        except (AttributeError, RequestException, ValueError):
             log.exception(
                 'GitLab webhook update failed for project: %s',
                 project,
@@ -403,7 +410,9 @@ def update_webhook(self, project, integration):
             except ValueError:
                 debug_data = resp.content
             log.debug('GitLab webhook update failure response: %s', debug_data)
-            return (False, resp)
+
+        integration.remove_secret()
+        return (False, resp)
 
     def send_build_status(self, build, commit, state):
         """
@@ -418,13 +427,14 @@ def send_build_status(self, build, commit, state):
         :returns: boolean based on commit status creation was successful or not.
         :rtype: Bool
         """
+        resp = None
         session = self.get_session()
         project = build.project
 
         repo_id = self._get_repo_id(project)
 
         if repo_id is None:
-            return (False, None)
+            return (False, resp)
 
         # select the correct state and description.
         gitlab_build_state = SELECT_BUILD_STATUS[state]['gitlab']
@@ -443,8 +453,6 @@ def send_build_status(self, build, commit, state):
         }
         url = self.adapter.provider_base_url
 
-        resp = None
-
         try:
             resp = session.post(
                 f'{url}/api/v4/projects/{repo_id}/statuses/{commit}',

readthedocs/oauth/utils.py

@@ -49,6 +49,7 @@ def update_webhook(project, integration, request=None):
         project.has_valid_webhook = True
         project.save()
         return True
+
     messages.error(
         request,
         _(