Skip to content

Commit

Permalink
process override decision payloads from webhook without cinder job (#…
Browse files Browse the repository at this point in the history 
…22829)
  • Loading branch information
@eviljeff
eviljeff authored Nov 8, 2024
1 parent e19b438 commit e4edcab
Show file tree
Hide file tree
Showing 3 changed files with 310 additions and 16 deletions.
218 changes: 218 additions & 0 deletions src/olympia/abuse/tests/assets/cinder_webhook_payloads/override_change_to_approve.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,218 @@
{
"event": "decision.created",
"payload": {
"appeal": {
"appealed_decision": {
"enforcement_actions": [
"amo-ban-user",
"amo-delete-collection",
"amo-delete-rating",
"amo-disable-addon"
],
"enforcement_actions_removed": [],
"id": "4dec6a52-41c1-43ea-9897-d914a51e57ff",
"notes": "",
"policies": [
{
"enforcement_actions": [],
"id": "0d9df565-f249-40f8-8954-e73e65932ca2",
"is_illegal": false,
"is_non_violating": false,
"name": "Acceptable Use"
},
{
"enforcement_actions": [
"amo-ban-user",
"amo-delete-collection",
"amo-delete-rating",
"amo-disable-addon"
],
"id": "4e401e5d-2720-4cea-a367-0d163bad1dcd",
"is_illegal": false,
"is_non_violating": false,
"name": "Controlled substances",
"parent_id": "0d9df565-f249-40f8-8954-e73e65932ca2"
}
],
"policies_removed": [],
"type": "queue_review",
"user": {
"email": "[email protected]",
"groups": [
{
"name": "API Manager"
},
{
"name": "Base User"
},
{
"name": "Everyone"
},
{
"name": "QA"
},
{
"name": "TaskUs moderators"
}
],
"name": "Ioana rusiczki"
}
}
},
"appeals_resolved": [],
"enforcement_actions": [
"amo-approve"
],
"enforcement_actions_removed": [
"amo-ban-user",
"amo-delete-collection",
"amo-delete-rating",
"amo-disable-addon"
],
"entity": {
"attributes": {
"average_daily_users": 0,
"created": "2024-11-06T06:48:24.833984",
"description": "",
"guid": "{bf8f936b-d9a9-4bb7-bc17-6176920251e5}",
"id": "633963",
"last_updated": "2024-11-06T06:48:24.982270",
"name": "Override policy 3",
"previews": [
{
"mime_type": "image/jpeg",
"value": "https://storage.googleapis.com/dev_addons_server_for_svcse_1619/95e66ed19bc60249d21493e91cf7f03f037e0adbc874e6077ee0a23e090829ff.jpg?Expires=1749451847&GoogleAccessId=dev-svcse-1619-uploader%40moz-fx-amo-nonprod.iam.gserviceaccount.com&Signature=E7GxwIq0rkmwubkHMk3MzLtqD9uFqKFAVhE7OWPZZdGX317ZJY%2BtzTq1zhYwMHgNZIW0nMTkt21dCKXEDb2AIkdN%2FEtSxermUXT7%2FTLYgC7mv6mEs%2BqDFBsJQDk7ygi%2FZ3rNNz6Bij6tGf%2BL%2FQIzoDtA8nCMPhudxjgCjaNgHPDCUQq0c24i94GY8ipT73tl6GUEXt%2F5vxDUj7px9pfpI4Xa0vbAwO3%2BENI%2Bh5nCedXRqZhvQWEDFy1mKByBmBh0UnVscbFuniOvrbUk5vCGdg7qBe0RicEgt5bK7k6AdCZ9KNvpiub5BG4PW2VhhC2lUWIYm5mK8%2FpxMAuKow1rVA%3D%3D"
}
],
"privacy_policy": "",
"promoted": "",
"release_notes": "",
"slug": "override-policy-3",
"summary": "Override policy 3",
"version": "1.0"
},
"entity_schema": "amo_addon"
},
"notes": "changed our mind",
"point_updates": [],
"policies": [
{
"enforcement_actions": [
"amo-approve"
],
"id": "085f6a1c-46b6-44c2-a6ae-c3a73488aa1e",
"is_illegal": false,
"is_non_violating": true,
"name": "Approve"
}
],
"policies_removed": [
{
"enforcement_actions": [],
"id": "0d9df565-f249-40f8-8954-e73e65932ca2",
"is_illegal": false,
"is_non_violating": false,
"name": "Acceptable Use"
},
{
"enforcement_actions": [
"amo-ban-user",
"amo-delete-collection",
"amo-delete-rating",
"amo-disable-addon"
],
"id": "4e401e5d-2720-4cea-a367-0d163bad1dcd",
"is_illegal": false,
"is_non_violating": false,
"name": "Controlled substances",
"parent_id": "0d9df565-f249-40f8-8954-e73e65932ca2"
}
],
"previous_decision": {
"enforcement_actions": [
"amo-ban-user",
"amo-delete-collection",
"amo-delete-rating",
"amo-disable-addon"
],
"enforcement_actions_removed": [],
"id": "d1f01fae-3bce-41d5-af8a-e0b4b5ceaaed",
"metadata": {},
"notes": "",
"policies": [
{
"enforcement_actions": [],
"id": "0d9df565-f249-40f8-8954-e73e65932ca2",
"is_illegal": false,
"is_non_violating": false,
"name": "Acceptable Use"
},
{
"enforcement_actions": [
"amo-ban-user",
"amo-delete-collection",
"amo-delete-rating",
"amo-disable-addon"
],
"id": "4e401e5d-2720-4cea-a367-0d163bad1dcd",
"is_illegal": false,
"is_non_violating": false,
"name": "Controlled substances",
"parent_id": "0d9df565-f249-40f8-8954-e73e65932ca2"
}
],
"policies_removed": [],
"type": "queue_review",
"user": {
"email": "[email protected]",
"groups": [
{
"name": "API Manager"
},
{
"name": "Base User"
},
{
"name": "Everyone"
},
{
"name": "QA"
},
{
"name": "TaskUs moderators"
}
],
"name": "Ioana rusiczki"
}
},
"source": {
"decision": {
"id": "3eacdc09-c292-4fcb-a56f-a3d45d5eefeb",
"metadata": {},
"type": "manual_override"
},
"user": {
"email": "[email protected]",
"groups": [
{
"name": "API Manager"
},
{
"name": "Base User"
},
{
"name": "Everyone"
},
{
"name": "QA"
},
{
"name": "TaskUs moderators"
}
],
"name": "Ioana rusiczki"
}
},
"timestamp": "2024-11-06T06:54:36.962174+00:00"
}
}
64 changes: 63 additions & 1 deletion src/olympia/abuse/tests/test_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1347,6 +1347,30 @@ def test_process_decision_called_for_appeal_change_to_disable(self):
assert response.status_code == 201
assert response.data == {'amo': {'received': True, 'handled': True}}

def test_process_decision_called_for_override_to_approve(self):
abuse_report = self._setup_reports()
CinderJob.objects.get().update(
decision=ContentDecision.objects.create(
cinder_id='d1f01fae-3bce-41d5-af8a-e0b4b5ceaaed',
action=DECISION_ACTIONS.AMO_DISABLE_ADDON,
addon=addon_factory(guid=abuse_report.guid),
),
)
req = self.get_request(
data=self.get_data(filename='override_change_to_approve.json')
)
with mock.patch.object(CinderJob, 'process_decision') as process_mock:
response = cinder_webhook(req)
assert process_mock.call_count == 1, response.data
process_mock.assert_called_with(
decision_cinder_id='3eacdc09-c292-4fcb-a56f-a3d45d5eefeb',
decision_action=DECISION_ACTIONS.AMO_APPROVE.value,
decision_notes='changed our mind',
policy_ids=['085f6a1c-46b6-44c2-a6ae-c3a73488aa1e'],
)
assert response.status_code == 201
assert response.data == {'amo': {'received': True, 'handled': True}}

def test_process_decision_triggers_emails_when_disable_confirmed(self):
data = self.get_data(filename='target_appeal_confirm_disable.json')
abuse_report = self._setup_reports()
Expand Down Expand Up @@ -1533,7 +1557,7 @@ def check(response):
data['payload'] = {}
check(cinder_webhook(self.get_request(data=data)))

def test_no_cinder_report(self):
def test_no_cinder_job(self):
req = self.get_request()
with mock.patch.object(CinderJob, 'process_decision') as process_mock:
response = cinder_webhook(req)
Expand All @@ -1547,6 +1571,44 @@ def test_no_cinder_report(self):
}
}

def test_no_decision(self):
req = self.get_request(
data=self.get_data(filename='override_change_to_approve.json')
)
with mock.patch.object(CinderJob, 'process_decision') as process_mock:
response = cinder_webhook(req)
process_mock.assert_not_called()
assert response.status_code == 200
assert response.data == {
'amo': {
'received': True,
'handled': False,
'not_handled_reason': 'No matching decision id found',
}
}

def test_valid_decision_but_no_cinder_job(self):
abuse_report = self._setup_reports()
ContentDecision.objects.create(
cinder_id='d1f01fae-3bce-41d5-af8a-e0b4b5ceaaed',
action=DECISION_ACTIONS.AMO_DISABLE_ADDON,
addon=addon_factory(guid=abuse_report.guid),
)
req = self.get_request(
data=self.get_data(filename='override_change_to_approve.json')
)
with mock.patch.object(CinderJob, 'process_decision') as process_mock:
response = cinder_webhook(req)
process_mock.assert_not_called()
assert response.status_code == 200
assert response.data == {
'amo': {
'received': True,
'handled': False,
'not_handled_reason': 'No matching job found for decision id',
}
}

def test_reviewer_tools_resolved_cinder_job(self):
report = self._setup_reports()
report.cinder_job.update(resolvable_in_reviewer_tools=True)
Expand Down
44 changes: 29 additions & 15 deletions src/olympia/abuse/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,21 +185,35 @@ def filter_enforcement_actions(enforcement_actions, cinder_job):

def process_webhook_payload_decision(payload):
source = payload.get('source', {})
job = source.get('job', {})
if (
queue_name := job.get('queue', {}).get('slug')
) == CinderAddonHandledByReviewers.queue:
log.info('Payload from queue handled by reviewers: %s', queue_name)
raise ValidationError('Queue handled by AMO reviewers')

log.info('Valid Payload from AMO queue: %s', payload)
job_id = job.get('id', '')

try:
cinder_job = CinderJob.objects.get(job_id=job_id)
except CinderJob.DoesNotExist as exc:
log.debug('CinderJob instance not found for job id %s', job_id)
raise ValidationError('No matching job id found') from exc
if 'job' in source:
job = source.get('job', {})
if (
queue_name := job.get('queue', {}).get('slug')
) == CinderAddonHandledByReviewers.queue:
log.info('Payload from queue handled by reviewers: %s', queue_name)
raise ValidationError('Queue handled by AMO reviewers')

log.info('Valid Payload from AMO queue: %s', payload)
job_id = job.get('id', '')

try:
cinder_job = CinderJob.objects.get(job_id=job_id)
except CinderJob.DoesNotExist as exc:
log.debug('CinderJob instance not found for job id %s', job_id)
raise ValidationError('No matching job id found') from exc
else:
decision_id = payload.get('previous_decision', {}).get('id', '')

try:
decision = ContentDecision.objects.get(cinder_id=decision_id)
except ContentDecision.DoesNotExist as exc:
log.debug('ContentDecision instance not found for id %s', decision_id)
raise ValidationError('No matching decision id found') from exc

cinder_job = getattr(decision, 'cinder_job', None)
if not cinder_job:
log.debug('No job for ContentDecision with id %s', decision_id)
raise ValidationError('No matching job found for decision id')

if cinder_job.resolvable_in_reviewer_tools:
log.debug('Cinder webhook decision for reviewer resolvable job skipped.')
Expand Down

0 comments on commit e4edcab

Please sign in to comment.