diff --git a/reviewboard/oauth/__init__.py b/reviewboard/oauth/__init__.py index 398372e8c079e836ed97b53f44984b53b8eb883b..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 --- a/reviewboard/oauth/__init__.py +++ b/reviewboard/oauth/__init__.py @@ -1,21 +0,0 @@ -"""OAuth initialization. - -This module loads the WebAPI scopes when Review Board initializes. -""" - -from django.dispatch import receiver - -from reviewboard.signals import initializing - - -@receiver(initializing) -def _on_initializing(**kwargs): - """Enable OAuth scopes for the API when initializing Review Board. - - Args: - **kwargs (dict): - Keyword arguments from the signal. - """ - from djblets.webapi.oauth2_scopes import enable_web_api_scopes - - enable_web_api_scopes() diff --git a/reviewboard/oauth/apps.py b/reviewboard/oauth/apps.py index 15e99a37d28d45c6fa5fe4dcaf8360e68af0f497..3e8d5d6bffb477b2202373a623dfa274aa0c28fd 100644 --- a/reviewboard/oauth/apps.py +++ b/reviewboard/oauth/apps.py @@ -4,4 +4,18 @@ from django.apps import AppConfig class OAuthAppConfig(AppConfig): + """App configuration for reviewboard.oauth.""" + name = 'reviewboard.oauth' + + def ready(self): + """Configure the app once it's ready. + + This will connect signal handlers for the app. + + Version Added: + 5.0 + """ + from reviewboard.oauth.signal_handlers import connect_signal_handlers + + connect_signal_handlers() diff --git a/reviewboard/oauth/signal_handlers.py b/reviewboard/oauth/signal_handlers.py new file mode 100644 index 0000000000000000000000000000000000000000..32d414aa5e92fe6dad1d8ef63abc7496d84daf1f --- /dev/null +++ b/reviewboard/oauth/signal_handlers.py @@ -0,0 +1,110 @@ +"""Signal handlers for OAuth2. + +Version Added: + 5.0 +""" + +from django.contrib.auth.models import User +from django.db.models.signals import m2m_changed +from djblets.webapi.oauth2_scopes import enable_web_api_scopes + +from reviewboard.oauth.models import Application +from reviewboard.signals import initializing +from reviewboard.site.models import LocalSite + + +def _on_local_site_users_changed(instance, action, pk_set, reverse, **kwargs): + """Update OAuth apps when users change on a Local Site. + + This method ensures that any + :py:class:`Applications ` owned by + users removed from a a :py:class:`~reviewboard.site.models.LocalSite` will + be re-assigned to an administrator on that Local Site and disabled so the + client secret can be changed. + + Version Added: + 5.0: + This logic used to live in :py:mod:`reviewboard.site.signal_handlers`. + + Args: + instance (django.contrib.auth.models.User or + reviewboard.reviews.models.review_group.Group): + The model that changed. + + action (unicode): + The change action on the Local Site. + + pk_set (list of int): + The primary keys of the objects changed. + + reverse (bool): + Whether or not the relation or the reverse relation is changing. + + **kwargs (dict): + Ignored arguments from the signal. + """ + users = None + + # When reverse is True, `instance` will be a user that was changed (i.e., + # the signal triggered from user.local_sites.add(site)). Otherwise, + # `instance` will be the `local_site` that changed and `pk_set` will be + # the list of user primary keys that were added/removed. + if action == 'post_remove': + if reverse: + users = [instance] + else: + users = list(User.objects.filter(pk__in=pk_set)) + elif action == 'pre_clear': + if reverse: + users = [instance] + else: + # We have to grab the list of associated users in the pre_clear + # phase because pk_set is always empty for pre_ and post_clear. + users = list(instance.users.all()) + + if not users: + return + + applications = list( + Application.objects + .filter(user__in=users, + local_site__isnull=False) + .prefetch_related('local_site__admins') + ) + + if not applications: + return + + users_by_pk = { + user.pk: user + for user in users + } + + for application in applications: + user = users_by_pk[application.user_id] + + if not application.local_site.is_accessible_by(user): + # The user who owns this application no longer has access to the + # Local Site. We must disable the application and reassign it. + application.enabled = False + application.user = application.local_site.admins.first() + application.original_user = user + application.save(update_fields=[ + 'enabled', + 'original_user', + 'user', + ]) + + +def connect_signal_handlers(): + """Connect LocalSite-related signal handlers. + + Version Added: + 5.0 + """ + # Enable only after initializing, as we want to include anything from + # extensions that have been loaded. + initializing.connect(enable_web_api_scopes) + + m2m_changed.connect(_on_local_site_users_changed, + sender=LocalSite.users.through) diff --git a/reviewboard/site/__init__.py b/reviewboard/site/__init__.py index aaf1ce7df33f33a2b5c44eb1e3ec1045d8f82b14..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 --- a/reviewboard/site/__init__.py +++ b/reviewboard/site/__init__.py @@ -1,16 +0,0 @@ -"""Local Site-specfic initialization.""" - -from reviewboard.signals import initializing - - -def _on_initializing(**kwargs): - """Set up signal handlers for Local Sites.""" - from django.db.models.signals import m2m_changed - - from reviewboard.site.models import LocalSite - from reviewboard.site.signal_handlers import on_users_changed - - m2m_changed.connect(on_users_changed, sender=LocalSite.users.through) - - -initializing.connect(_on_initializing) diff --git a/reviewboard/site/apps.py b/reviewboard/site/apps.py new file mode 100644 index 0000000000000000000000000000000000000000..d16cef944d14870dc9a83cad80e8149a44caf176 --- /dev/null +++ b/reviewboard/site/apps.py @@ -0,0 +1,18 @@ +"""Django app information for reviewboard.site.""" + +from django.apps import AppConfig + + +class SiteAppConfig(AppConfig): + """App configuration for reviewboard.site.""" + + name = 'reviewboard.site' + + def ready(self): + """Configure the app once it's ready. + + This will connect signal handlers for the app. + """ + from reviewboard.site.signal_handlers import connect_signal_handlers + + connect_signal_handlers() diff --git a/reviewboard/site/models.py b/reviewboard/site/models.py index 7507cb93d7893eb0396968958cde5d13c810f4ec..567a707d72e630d34e78baf3372989c016eca7a0 100644 --- a/reviewboard/site/models.py +++ b/reviewboard/site/models.py @@ -89,27 +89,3 @@ class LocalSite(models.Model): db_table = 'site_localsite' verbose_name = _('Local Site') verbose_name_plural = _('Local Sites') - - -@receiver(m2m_changed, sender=LocalSite.users.through) -def _on_local_site_users_changed(sender, instance, model, - action, pk_set, **kwargs): - """Handle the m2m_changed event for LocalSite and User. - - This function handles both the case where users are added to local sites - and local sites are added to the set of a user's local sites. In both of - these cases, the local_site_user_added signal is dispatched. - """ - if action == 'post_add': - if isinstance(instance, User): - users = [instance] - local_sites = LocalSite.objects.filter(id__in=pk_set) - else: - users = User.objects.filter(id__in=pk_set) - local_sites = [instance] - - for user in users: - for local_site in local_sites: - local_site_user_added.send(sender=LocalSite, - user=user, - local_site=local_site) diff --git a/reviewboard/site/signal_handlers.py b/reviewboard/site/signal_handlers.py index cb06f40179fbd0cbf0109d97593a559a6b84e530..8794ce45ff9fa37e64d12660496c5600a824c4de 100644 --- a/reviewboard/site/signal_handlers.py +++ b/reviewboard/site/signal_handlers.py @@ -1,84 +1,62 @@ """Signal handlers.""" from django.contrib.auth.models import User +from django.db.models.signals import m2m_changed -from reviewboard.oauth.models import Application +from reviewboard.site.models import LocalSite +from reviewboard.site.signals import local_site_user_added -def on_users_changed(instance, action, pk_set, reverse, **kwargs): - """Handle the users of a Local Site changing. +def _emit_local_site_user_signals(instance, action, pk_set, **kwargs): + """Handle the m2m_changed event for LocalSite and User. - This method ensures that any - :py:class:`Applications ` owned by - users removed from a a :py:class:`~reviewboard.site.models.LocalSite` will - be re-assigned to an administrator on that Local Site and disabled so the - client secret can be changed. + This function handles both the case where users are added to local sites + and local sites are added to the set of a user's local sites. In both of + these cases, the :py:data:`reviewboard.site.signals.local_site_user_added` + signal is dispatched. + + Version Added: + 5.0: + This logic used to live in :py:mod:`reviewboard.site.models`. Args: instance (django.contrib.auth.models.User or - reviewboard.reviews.models.review_group.Group): - The model that changed. + reviewboard.site.models.LocalSite): + The Local Site or User that caused the signal to be emitted, + depending on the side of the relation that changed. action (unicode): - The change action on the Local Site. + The action that was performed. This handler only responds to + ``post_add``. pk_set (list of int): - The primary keys of the objects changed. - - reverse (bool): - Whether or not the relation or the reverse relation is changing. + The list of primary keys that were added. - **kwargs (dict): - Ignored arguments from the signal. + **kwargs (dict, unused): + Additional keyword arguments from the signal. """ - users = None - - # When reverse is True, `instance` will be a user that was changed (i.e., - # the signal triggered from user.local_sites.add(site)). Otherwise, - # `instance` will be the `local_site` that changed and `pk_set `will be - # the list of user primary keys that were added/removed. - if action == 'post_remove': - if reverse: - users = [instance] - else: - users = list(User.objects.filter(pk__in=pk_set)) - elif action == 'pre_clear': - if reverse: - users = [instance] - else: - # We have to grab the list of associated users in the pre_clear - # phase because pk_set is always empty for pre_ and post_clear. - users = list(instance.users.all()) - - if not users: + if action != 'post_add': return - applications = list( - Application.objects - .filter(user__in=users, - local_site__isnull=False) - .prefetch_related('local_site__admins') - ) + if isinstance(instance, User): + users = [instance] + local_sites = LocalSite.objects.filter(id__in=pk_set) + else: + users = User.objects.filter(id__in=pk_set) + local_sites = [instance] - if not applications: - return + for user in users: + for local_site in local_sites: + local_site_user_added.send(sender=LocalSite, + user=user, + local_site=local_site) - users_by_pk = { - user.pk: user - for user in users - } - for application in applications: - user = users_by_pk[application.user_id] +def connect_signal_handlers(): + """Connect LocalSite-related signal handlers. - if not application.local_site.is_accessible_by(user): - # The user who owns this application no longer has access to the - # Local Site. We must disable the application and - application.enabled = False - application.user = application.local_site.admins.first() - application.original_user = user - application.save(update_fields=[ - 'enabled', - 'original_user', - 'user', - ]) + Version Added: + 5.0 + """ + m2m_changed.connect(_emit_local_site_user_signals, + sender=LocalSite.users.through)