Skip to content

Modifications required to implement GitHub SSO in commercial #7183

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jun 30, 2020
Merged
32 changes: 29 additions & 3 deletions readthedocs/core/permissions.py
Original file line number Diff line number Diff line change
@@ -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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like seeing this logic backported into the .org 👍


@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):
Expand Down
15 changes: 13 additions & 2 deletions readthedocs/oauth/services/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down
9 changes: 3 additions & 6 deletions readthedocs/organizations/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down