Merge pull request #7183 from readthedocs/humitos/db-user-github-sso

Author: humitos

readthedocs/core/permissions.py

@@ -1,21 +1,47 @@
 # -*- coding: utf-8 -*-
 
 """Objects for User permission checks."""
+from django.contrib.auth.models import User
+from django.db.models import Q
 
 from readthedocs.core.utils.extend import SettingsOverrideObject
 
 
 class AdminPermissionBase:
 
     @classmethod
-    def is_admin(cls, user, project):
+    def admins(cls, obj):
+        from readthedocs.projects.models import Project
+        from readthedocs.organizations.models import Organization
+
+        if isinstance(obj, Project):
+            return obj.users.all()
+
+        if isinstance(obj, Organization):
+            return obj.owners.all()
+
+    @classmethod
+    def members(cls, obj):
+        from readthedocs.projects.models import Project
+        from readthedocs.organizations.models import Organization
+
+        if isinstance(obj, Project):
+            return obj.users.all()
+
+        if isinstance(obj, Organization):
+            return User.objects.filter(
+                Q(teams__organization=obj) | Q(owner_organizations=obj),
+            ).distinct()
+
+    @classmethod
+    def is_admin(cls, user, obj):
         # This explicitly uses "user in project.users.all" so that
         # users on projects can be cached using prefetch_related or prefetch_related_objects
-        return user in project.users.all() or user.is_superuser
+        return user in cls.admins(obj) or user.is_superuser
 
     @classmethod
     def is_member(cls, user, obj):
-        return user in obj.users.all() or user.is_superuser
+        return user in cls.members(obj) or user.is_superuser
 
 
 class AdminPermission(SettingsOverrideObject):

readthedocs/oauth/services/github.py

@@ -6,6 +6,8 @@
 
 from allauth.socialaccount.models import SocialToken
 from allauth.socialaccount.providers.github.views import GitHubOAuth2Adapter
+
+from django.db.models import Q
 from django.conf import settings
 from django.urls import reverse
 from requests.exceptions import RequestException
@@ -35,15 +37,23 @@ class GitHubService(Service):
 
     def sync(self):
         """Sync repositories and organizations."""
-        self.sync_repositories()
-        self.sync_organizations()
+        repos = self.sync_repositories()
+        organization_repos = self.sync_organizations()
+
+        # Delete RemoteRepository where the user doesn't have access anymore
+        # (skip RemoteRepository tied to a Project on this user)
+        full_names = {repo.get('full_name') for repo in repos + organization_repos}
+        self.user.oauth_repositories.exclude(
+            Q(full_name__in=full_names) | Q(project__isnull=False)
+        ).delete()
 
     def sync_repositories(self):
         """Sync repositories from GitHub API."""
         repos = self.paginate('https://api.github.com/user/repos?per_page=100')
         try:
             for repo in repos:
                 self.create_repository(repo)
+            return repos
         except (TypeError, ValueError):
             log.warning('Error syncing GitHub repositories')
             raise SyncServiceError(
@@ -65,6 +75,7 @@ def sync_organizations(self):
                 )
                 for repo in org_repos:
                     self.create_repository(repo, organization=org_obj)
+                return org_repos
         except (TypeError, ValueError):
             log.warning('Error syncing GitHub organizations')
             raise SyncServiceError(

readthedocs/organizations/models.py

@@ -3,12 +3,12 @@
 from autoslug import AutoSlugField
 from django.contrib.auth.models import User
 from django.db import models
-from django.db.models import Q
 from django.urls import reverse
 from django.utils.crypto import salted_hmac
 from django.utils.translation import ugettext_lazy as _
 
 from readthedocs.core.utils import slugify
+from readthedocs.core.permissions import AdminPermission
 
 from . import constants
 from .managers import TeamManager, TeamMemberManager
@@ -93,14 +93,11 @@ def get_absolute_url(self):
 
     @property
     def users(self):
-        return (self.members.all() | self.owners.all().distinct()).distinct()
+        return AdminPermission.members(self)
 
     @property
     def members(self):
-        """Return members as an aggregate over all organization teams."""
-        return User.objects.filter(
-            Q(teams__organization=self) | Q(owner_organizations=self),
-        ).distinct()
+        return AdminPermission.members(self)
 
     def save(self, *args, **kwargs):  # pylint: disable=arguments-differ
         if not self.slug: