infrastructure/blender-id
0
4
Fork 0
Code Issues 11 Pull Requests Projects Releases Wiki Activity

Send user-modified only to webhooks of relevant OAuth apps #93585

Closed
Anna Sirota wants to merge 5 commits from webhook-oauth-app-required into main
pull from: webhook-oauth-app-required
merge into: infrastructure:main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 0 Commits 5 Files Changed 8 +246 -20
8 changed files with 246 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 6a95b03d70 - Show all commits

1
bid_api/admin.py
View File

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

21
bid_api/migrations/0010_alter_webhook_app.py Normal file
View File

@ -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'),
),
]

2
bid_api/models.py
View File

@ -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,
)

18
bid_api/signals.py
View File

@ -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):
"""Forwards modified user information to webhooks.
def handle_modifications(sender, user: UserModel, **kwargs):
"""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,16 @@ 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 = models.Webhook.objects.filter(enabled=True).filter(
app__in=user.bid_main_oauth2accesstoken.distinct().values_list('application', flat=True)
)
if not hooks.count():
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, len(hooks))
# Do our own JSON encoding so that we can compute the HMAC using the hook's secret.
payload = {
"id": user.id,

218
bid_api/tests/test_webhooks.py
View File

@ -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"})

1
bid_main/fixtures/blender_extensions_devserver.json
View File

@ -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",

1
bid_main/fixtures/blender_studio_devserver.json
View File

@ -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",

4
bid_main/management/commands/tests/test_process_deletion_requests.py
View File

@ -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