bid_api/admin.py

@@ -49,6 +49,7 @@ class WebhookAdmin(ModelAdmin):
     list_display = (
         "name",
         "hook_type",
+        "app",
         "url",
         "enabled",
     )

bid_api/migrations/0010_alter_webhook_app.py

@@ -0,0 +1,21 @@
+# Generated by Django 3.2.9 on 2024-06-17 06:57
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.OAUTH2_PROVIDER_APPLICATION_MODEL),
+        ('bid_api', '0009_remove_webhook_last_flush_attempt'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='webhook',
+            name='app',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='webhooks', to='bid_main.oauth2application'),
+        ),
+    ]

bid_api/models.py

@@ -36,8 +36,6 @@ class Webhook(models.Model):
     )
     app = models.ForeignKey(
         'bid_main.OAuth2Application',
-        null=True,
-        blank=True,
         related_name="webhooks",
         on_delete=models.CASCADE,
     )

bid_api/signals.py

@@ -100,9 +100,10 @@ def inspect_modified_user(sender, user: UserModel, **kwargs):
 
 @receiver(post_save)
 @filter_user_save_hook
-def modified_user_to_webhooks(sender, user: UserModel, **kwargs):
+def handle_modifications(sender, user: UserModel, **kwargs):
-    """Forwards modified user information to webhooks.
+    """Act on various account modifications.
 
+    Clean up orphaned avatars, notify about email change and call webhooks.
     The payload is POSTed, and a HMAC-SHA256 checksum is sent using the
     X-Webhook-HMAC HTTP header.
 
@@ -116,9 +117,6 @@ def modified_user_to_webhooks(sender, user: UserModel, **kwargs):
         # log.debug('Skipping save of %s', user.email)
         return
 
-    hooks = models.Webhook.objects.filter(enabled=True)
-    log.debug("Sending modification of %s to %d webhooks", user.email, len(hooks))
-
     # Get the old email address so that the webhook receiver can match by
     # either database ID or email address.
     webhook_pre_save = getattr(user, "webhook_pre_save", {})
@@ -145,6 +143,19 @@ def modified_user_to_webhooks(sender, user: UserModel, **kwargs):
         log.debug("User changed email from %s to %s", old_email, user.email)
         user_email_changed.send(sender, user=user, old_email=old_email)
 
+    hooks = list(
+        models.Webhook.objects.filter(enabled=True).filter(
+            app__in=user.bid_main_oauth2accesstoken.distinct().values_list('application', flat=True)
+        )
+    )
+
+    hooks_count = len(hooks)
+    if hooks_count == 0:
+        log.info('Not sending modification of pk=%d to any webhooks', user.pk)
+        return
+
+    log.info("Sending modification of pk=%d to %d webhooks", user.pk, hooks_count)
+
     # Do our own JSON encoding so that we can compute the HMAC using the hook's secret.
     payload = {
         "id": user.id,

bid_api/tests/test_webhooks.py

@@ -11,7 +11,7 @@ from freezegun import freeze_time
 import responses
 
 from .abstract import UserModel
-from bid_main.models import Role
+from bid_main.models import Role, OAuth2Application
 from bid_api import models
 import bid_api.tasks as tasks
 
@@ -48,19 +48,24 @@ def _call_task_directly(*args, **kwargs):
     new=_call_task_directly,
 )
 class WebhookTest(TestCase):
-    HOOK_URL = "http://www.unit.test/api/webhook"
+    HOOK_URLS = [
+        "http://www.unit.test/api/webhook/",
+        "http://www.yet-another-unit.test/api/webhook/",
+    ]
 
     def setUp(self):
-        self.hook = models.Webhook(
+        self.oauth_app = OAuth2Application.objects.create()
+        self.hook = models.Webhook.objects.create(
+            app=self.oauth_app,
             name="Unit test webhook",
             enabled=True,
             hook_type="USER_MODIFIED",
-            url=self.HOOK_URL,
+            url=self.HOOK_URLS[0],
             secret="ieBithiGhahh4hee5mac8zu0xiu8kae0",
         )
-        self.hook.save()
         responses.start()
-        responses.add(responses.POST, self.HOOK_URL, json={"status": "success"}, status=200)
+        responses.add(responses.POST, self.HOOK_URLS[0], json={"status": "success"}, status=200)
+        responses.add(responses.POST, self.HOOK_URLS[1], json={"status": "success"}, status=200)
         super().setUp()
 
     def tearDown(self):
@@ -69,6 +74,10 @@ class WebhookTest(TestCase):
 
     def test_modify_user_email_only(self):
         user = UserModel.objects.create_user("test@user.com", "123456")
+        user.bid_main_oauth2accesstoken.create(
+            application=self.oauth_app, expires='2024-01-01 01:01:01'
+        )
+
         user.email = "new@user.com"
         user.save(update_fields={"email"})
         self.assertEqual(user.public_roles_as_string, "")
@@ -77,7 +86,7 @@ class WebhookTest(TestCase):
 
         self.assertEqual(1, len(responses.calls))
         call = responses.calls[0]
-        self.assertEqual(self.HOOK_URL, call.request.url)
+        self.assertEqual(self.HOOK_URLS[0], call.request.url)
 
         payload = json.loads(call.request.body)
         self.assertEqual(
@@ -106,8 +115,81 @@ class WebhookTest(TestCase):
         self.assertEqual(hexdigest, call.request.headers["X-Webhook-HMAC"])
         self.assertEqual("application/json", call.request.headers["Content-Type"])
 
+    def test_modify_user_email_and_full_name_not_sent_because_no_oauth_apps_linked_to_user(self):
+        user = UserModel.objects.create_user("test@user.com", "123456")
+
+        user.email = "new@user.com"
+        user.full_name = "ဖန်စီဘောင်းဘီ"
+        user.save()
+
+        self.assertTrue(user.webhook_user_modified)
+
+        self.assertEqual(0, len(responses.calls))
+
+    def test_modify_user_email_and_full_name_only_sent_to_hooks_linked_to_oauth_app(self):
+        another_oauth_app1 = OAuth2Application.objects.create()
+        models.Webhook.objects.create(
+            app=another_oauth_app1,
+            name="Another webhook linked to a different OAuth app",
+            enabled=True,
+            hook_type="USER_MODIFIED",
+            url="http://www.another-unit.test/api/another-webhook/",
+            secret="asdfasdfasdfasfdasdfasdfas",
+        )
+        another_oauth_app2 = OAuth2Application.objects.create()
+        models.Webhook.objects.create(
+            app=another_oauth_app2,
+            name="Yet another webhook linked to a different OAuth app",
+            enabled=True,
+            hook_type="USER_MODIFIED",
+            url=self.HOOK_URLS[1],
+            secret="asdfasdfasdfasfdasdfasdfas",
+        )
+        models.Webhook.objects.create(
+            app=self.oauth_app,
+            name="Disabled webhook of the same OAuth app",
+            enabled=False,
+            hook_type="USER_MODIFIED",
+            url='http://example.com/disabled-webhook/',
+            secret="asdfasdfasasd12asdfasdfas",
+        )
+        user = UserModel.objects.create_user("test@user.com", "123456")
+        user.bid_main_oauth2accesstoken.create(
+            application=self.oauth_app, expires='2024-01-01 01:01:01', token='foobar1'
+        )
+        user.bid_main_oauth2accesstoken.create(
+            application=another_oauth_app2, expires='2024-01-01 01:01:01', token='fooobar2'
+        )
+
+        user.email = "new@user.com"
+        user.full_name = "ဖန်စီဘောင်းဘီ"
+        user.save()
+
+        self.assertTrue(user.webhook_user_modified)
+
+        self.assertEqual(2, len(responses.calls))
+        expected_payload = {
+            "id": user.id,
+            "old_email": "test@user.com",
+            "full_name": "ဖန်စီဘောင်းဘီ",
+            "email": "new@user.com",
+            "roles": [],
+            "avatar_changed": False,
+            "avatar_url": None,
+            "date_deletion_requested": None,
+            "confirmed_email_at": None,
+            "nickname": "",
+            "old_nickname": "",
+        }
+        self.assertEqual(expected_payload, json.loads(responses.calls[0].request.body))
+        self.assertEqual(expected_payload, json.loads(responses.calls[1].request.body))
+
     def test_modify_user_email_and_full_name(self):
         user = UserModel.objects.create_user("test@user.com", "123456")
+        user.bid_main_oauth2accesstoken.create(
+            application=self.oauth_app, expires='2024-01-01 01:01:01'
+        )
+
         user.email = "new@user.com"
         user.full_name = "ဖန်စီဘောင်းဘီ"
         user.save()
@@ -135,6 +217,10 @@ class WebhookTest(TestCase):
 
     def test_modify_user_nickname(self):
         user = UserModel.objects.create_user("test@user.com", "123456")
+        user.bid_main_oauth2accesstoken.create(
+            application=self.oauth_app, expires='2024-01-01 01:01:01'
+        )
+
         user.nickname = "propellorolleporp"
         user.save()
 
@@ -184,6 +270,10 @@ class WebhookTest(TestCase):
         my_dir = pathlib.Path(__file__).absolute().parent
         with self.settings(MEDIA_ROOT=my_dir / "media"):
             user = UserModel.objects.create_user("test@user.com", "123456")
+            user.bid_main_oauth2accesstoken.create(
+                application=self.oauth_app, expires='2024-01-01 01:01:01'
+            )
+
             user.avatar = "images/t-rex.png"
             user.save()
 
@@ -210,6 +300,10 @@ class WebhookTest(TestCase):
 
     def test_modify_user_add_role(self):
         user = UserModel.objects.create_user("test@user.com", "123456", full_name="ဖန်စီဘောင်းဘီ")
+        user.bid_main_oauth2accesstoken.create(
+            application=self.oauth_app, expires='2024-01-01 01:01:01'
+        )
+
         role1 = Role(name="cloud_subscriber", is_public=True, is_badge=True, is_active=True)
         role1.save()
         role2 = Role(name="admin", is_public=False, is_badge=False, is_active=True)
@@ -271,6 +365,10 @@ class WebhookTest(TestCase):
     @freeze_time('2021-01-01T01:02:03+02:00')
     def test_modify_date_deletion_requested(self):
         user = UserModel.objects.create_user("test@user.com", "123456")
+        user.bid_main_oauth2accesstoken.create(
+            application=self.oauth_app, expires='2024-01-01 01:01:01'
+        )
+
         user.date_deletion_requested = timezone.now()
         user.save(update_fields={"date_deletion_requested"})
 
@@ -278,7 +376,7 @@ class WebhookTest(TestCase):
 
         self.assertEqual(1, len(responses.calls))
         call = responses.calls[0]
-        self.assertEqual(self.HOOK_URL, call.request.url)
+        self.assertEqual(self.HOOK_URLS[0], call.request.url)
 
         payload = json.loads(call.request.body)
         self.assertEqual(
@@ -301,6 +399,10 @@ class WebhookTest(TestCase):
     @freeze_time('2021-01-01T04:05:06+02:00')
     def test_modify_confirmed_email_at(self):
         user = UserModel.objects.create_user("test@user.com", "123456")
+        user.bid_main_oauth2accesstoken.create(
+            application=self.oauth_app, expires='2024-01-01 01:01:01'
+        )
+
         user.confirmed_email_at = timezone.now()
         user.save(update_fields={"confirmed_email_at"})
 
@@ -308,7 +410,7 @@ class WebhookTest(TestCase):
 
         self.assertEqual(1, len(responses.calls))
         call = responses.calls[0]
-        self.assertEqual(self.HOOK_URL, call.request.url)
+        self.assertEqual(self.HOOK_URLS[0], call.request.url)
 
         payload = json.loads(call.request.body)
         self.assertEqual(
@@ -375,19 +477,24 @@ class WebhookQueuedAsTaskTest(TestCase):
 
     def setUp(self):
         self.maxDiff = None
-        self.hook = models.Webhook(
+        self.oauth_app = OAuth2Application.objects.create()
+        self.hook = models.Webhook.objects.create(
+            app=self.oauth_app,
             name="Unit test webhook",
             enabled=True,
             hook_type="USER_MODIFIED",
             url=self.HOOK_URL,
             secret="ieBithiGhahh4hee5mac8zu0xiu8kae0",
         )
-        self.hook.save()
         super().setUp()
         self.assertEqual(Task.objects.count(), 0)
 
     def test_modify_user_email_only(self):
         user = UserModel.objects.create_user("test@user.com", "123456")
+        user.bid_main_oauth2accesstoken.create(
+            application=self.oauth_app, expires='2024-01-01 01:01:01'
+        )
+
         user.email = "new@user.com"
         user.save(update_fields={"email"})
         self.assertEqual(user.public_roles_as_string, "")
@@ -417,8 +524,77 @@ class WebhookQueuedAsTaskTest(TestCase):
             ],
         )
 
+    def test_modify_user_email_and_full_name_not_sent_because_no_oauth_apps_linked_to_user(self):
+        user = UserModel.objects.create_user("test@user.com", "123456")
+
+        user.email = "new@user.com"
+        user.full_name = "ဖန်စီဘောင်းဘီ"
+        user.save()
+
+        self.assertTrue(user.webhook_user_modified)
+
+        self.assertEqual(Task.objects.count(), 0)
+
+    def test_modify_user_email_and_full_name_only_sent_to_hook_linked_to_oauth_app(self):
+        another_oauth_app = OAuth2Application.objects.create()
+        models.Webhook.objects.create(
+            app=another_oauth_app,
+            name="Unit test webhook linked to a different OAuth app",
+            enabled=True,
+            hook_type="USER_MODIFIED",
+            url="http://www.some-other-unit.test/api/another-webhook/",
+            secret="asdfasdfasdfasfdasdfasdfas",
+        )
+        models.Webhook.objects.create(
+            app=self.oauth_app,
+            name="Disabled webhook of the same OAuth app",
+            enabled=False,
+            hook_type="USER_MODIFIED",
+            url=self.HOOK_URL,
+            secret="asdfasdfasasd12asdfasdfas",
+        )
+        user = UserModel.objects.create_user("test@user.com", "123456")
+        user.bid_main_oauth2accesstoken.create(
+            application=self.oauth_app, expires='2024-01-01 01:01:01'
+        )
+
+        user.email = "new@user.com"
+        user.full_name = "ဖန်စီဘောင်းဘီ"
+        user.save()
+
+        self.assertTrue(user.webhook_user_modified)
+
+        self.assertEqual(Task.objects.count(), 1)
+        task = Task.objects.first()
+        self.assertEqual(
+            json.loads(task.task_params),
+            [
+                [
+                    self.hook.pk,
+                    {
+                        "id": user.id,
+                        "old_email": "test@user.com",
+                        "full_name": "ဖန်စီဘောင်းဘီ",
+                        "email": "new@user.com",
+                        "roles": [],
+                        "avatar_changed": False,
+                        "avatar_url": None,
+                        "date_deletion_requested": None,
+                        "confirmed_email_at": None,
+                        "nickname": "",
+                        "old_nickname": "",
+                    },
+                ],
+                {},
+            ],
+        )
+
     def test_modify_user_email_and_full_name(self):
         user = UserModel.objects.create_user("test@user.com", "123456")
+        user.bid_main_oauth2accesstoken.create(
+            application=self.oauth_app, expires='2024-01-01 01:01:01'
+        )
+
         user.email = "new@user.com"
         user.full_name = "ဖန်စီဘောင်းဘီ"
         user.save()
@@ -450,6 +626,10 @@ class WebhookQueuedAsTaskTest(TestCase):
 
     def test_modify_user_nickname(self):
         user = UserModel.objects.create_user("test@user.com", "123456")
+        user.bid_main_oauth2accesstoken.create(
+            application=self.oauth_app, expires='2024-01-01 01:01:01'
+        )
+
         user.nickname = "propellorolleporp"
         user.save()
 
@@ -509,6 +689,10 @@ class WebhookQueuedAsTaskTest(TestCase):
         my_dir = pathlib.Path(__file__).absolute().parent
         with self.settings(MEDIA_ROOT=my_dir / "media"):
             user = UserModel.objects.create_user("test@user.com", "123456")
+            user.bid_main_oauth2accesstoken.create(
+                application=self.oauth_app, expires='2024-01-01 01:01:01'
+            )
+
             user.avatar = "images/t-rex.png"
             user.save()
 
@@ -538,6 +722,10 @@ class WebhookQueuedAsTaskTest(TestCase):
 
     def test_modify_user_add_role(self):
         user = UserModel.objects.create_user("test@user.com", "123456", full_name="ဖန်စီဘောင်းဘီ")
+        user.bid_main_oauth2accesstoken.create(
+            application=self.oauth_app, expires='2024-01-01 01:01:01'
+        )
+
         role1 = Role(name="cloud_subscriber", is_public=True, is_badge=True, is_active=True)
         role1.save()
         role2 = Role(name="admin", is_public=False, is_badge=False, is_active=True)
@@ -611,6 +799,10 @@ class WebhookQueuedAsTaskTest(TestCase):
     @freeze_time('2021-01-01T01:02:03+02:00')
     def test_modify_date_deletion_requested(self):
         user = UserModel.objects.create_user("test@user.com", "123456")
+        user.bid_main_oauth2accesstoken.create(
+            application=self.oauth_app, expires='2024-01-01 01:01:01'
+        )
+
         user.date_deletion_requested = timezone.now()
         user.save(update_fields={"date_deletion_requested"})
 
@@ -642,6 +834,10 @@ class WebhookQueuedAsTaskTest(TestCase):
     @freeze_time('2021-01-01T04:05:06+02:00')
     def test_modify_confirmed_email_at(self):
         user = UserModel.objects.create_user("test@user.com", "123456")
+        user.bid_main_oauth2accesstoken.create(
+            application=self.oauth_app, expires='2024-01-01 01:01:01'
+        )
+
         user.confirmed_email_at = timezone.now()
         user.save(update_fields={"confirmed_email_at"})
 

bid_main/fixtures/blender_extensions_devserver.json

@@ -28,6 +28,7 @@
         "model": "bid_api.webhook",
         "pk": 8111,
         "fields": {
+            "app_id": 8111,
             "description": "Webhook of Blender Extensions Development server; installed from a Django .json fixture",
             "enabled": true,
             "hook_type": "USER_MODIFIED",

bid_main/fixtures/blender_studio_devserver.json

@@ -28,6 +28,7 @@
         "model": "bid_api.webhook",
         "pk": 1,
         "fields": {
+            "app_id": 1,
             "description": "Webhook of Blender Studio Development server; installed from a Django .json fixture",
             "enabled": true,
             "hook_type": "USER_MODIFIED",

bid_main/management/commands/tests/test_process_deletion_requests.py

@@ -22,7 +22,7 @@ class ProcessDeletionRequestsCommandTest(TestCase):
         mock_anonymize.assert_not_called()
 
     @patch('bid_main.models.User.delete')
-    @patch('bid_api.signals.modified_user_to_webhooks')
+    @patch('bid_api.signals.handle_modifications')
     def test_anonymises_users(self, mock_webhook, mock_delete):
         now = timezone.now()
         # create some users
@@ -65,7 +65,7 @@ class ProcessDeletionRequestsCommandTest(TestCase):
             )
 
     @patch('bid_main.models.User.delete')
-    @patch('bid_api.signals.modified_user_to_webhooks')
+    @patch('bid_api.signals.handle_modifications')
     def test_missing_avatar(self, mock_webhook, mock_delete):
         now = timezone.now()
         # a user who requested deletion long enough ago, with a missing avatar