From 1c00f2d9185246518ce4c016e6cffa01138cfb94 Mon Sep 17 00:00:00 2001 From: Mike Fiedler Date: Tue, 19 Nov 2024 09:39:03 -0500 Subject: [PATCH] feat(admin): freeze on quarantine, add user observation on removal (#17109) --- .../unit/admin/views/test_malware_reports.py | 19 ++++++++--- warehouse/admin/static/js/warehouse.js | 27 +++++++++------ .../admin/malware_reports/detail.html | 1 + .../admin/malware_reports/project_list.html | 1 + .../admin/templates/admin/users/detail.html | 34 ++++++++++++++++++- warehouse/admin/views/malware_reports.py | 16 ++++++++- warehouse/observations/models.py | 1 + warehouse/utils/project.py | 4 +++ 8 files changed, 85 insertions(+), 18 deletions(-) diff --git a/tests/unit/admin/views/test_malware_reports.py b/tests/unit/admin/views/test_malware_reports.py index cad8e71af6ba..aa98ff30c34d 100644 --- a/tests/unit/admin/views/test_malware_reports.py +++ b/tests/unit/admin/views/test_malware_reports.py @@ -102,7 +102,9 @@ def test_malware_reports_project_verdict_not_malware(self, db_request): assert action_record["reason"] == "This is a test" def test_malware_reports_project_verdict_quarantine(self, db_request): + owner_user = UserFactory.create(is_frozen=False) project = ProjectFactory.create() + RoleFactory(user=owner_user, project=project, role_name="Owner") report = ProjectObservationFactory.create(kind="is_malware", related=project) db_request.route_path = lambda a: "/admin/malware_reports/" @@ -130,6 +132,7 @@ def test_malware_reports_project_verdict_quarantine(self, db_request): == f"Quarantined by {db_request.user.username}." ) assert len(report.actions) == 0 + assert owner_user.is_frozen is True def test_malware_reports_project_verdict_remove_malware(self, db_request): owner_user = UserFactory.create(is_frozen=False) @@ -148,7 +151,7 @@ def test_malware_reports_project_verdict_remove_malware(self, db_request): db_request.session = pretend.stub( flash=pretend.call_recorder(lambda *a, **kw: None) ) - db_request.user = owner_user + db_request.user = UserFactory.create() result = views.malware_reports_project_verdict_remove_malware( project, db_request @@ -167,7 +170,8 @@ def test_malware_reports_project_verdict_remove_malware(self, db_request): assert len(report.actions) == 1 assert db_request.db.get(Project, project.id) is None - assert db_request.user.is_frozen is True + assert owner_user.is_frozen is True + assert owner_user.observations[0].kind == "account_abuse" class TestMalwareReportsDetail: @@ -211,7 +215,10 @@ def test_detail_not_malware_for_project(self, db_request): assert action_record["reason"] == "This is a test" def test_detail_verdict_quarantine_project(self, db_request): - report = ProjectObservationFactory.create(kind="is_malware") + owner_user = UserFactory.create(is_frozen=False) + project = ProjectFactory.create() + RoleFactory(user=owner_user, project=project, role_name="Owner") + report = ProjectObservationFactory.create(kind="is_malware", related=project) db_request.matchdict["observation_id"] = str(report.id) db_request.route_path = lambda a: "/admin/malware_reports/" db_request.session = pretend.stub( @@ -237,6 +244,7 @@ def test_detail_verdict_quarantine_project(self, db_request): f"Quarantined by {db_request.user.username}." ) assert len(report.actions) == 0 + assert owner_user.is_frozen is True def test_detail_remove_malware_for_project(self, db_request): owner_user = UserFactory.create(is_frozen=False) @@ -257,7 +265,7 @@ def test_detail_remove_malware_for_project(self, db_request): db_request.session = pretend.stub( flash=pretend.call_recorder(lambda *a, **kw: None) ) - db_request.user = owner_user + db_request.user = UserFactory.create() result = views.remove_malware_for_project(db_request) @@ -273,4 +281,5 @@ def test_detail_remove_malware_for_project(self, db_request): assert len(report.actions) == 1 assert db_request.db.get(Project, project.id) is None - assert db_request.user.is_frozen is True + assert owner_user.is_frozen is True + assert owner_user.observations[0].kind == "account_abuse" diff --git a/warehouse/admin/static/js/warehouse.js b/warehouse/admin/static/js/warehouse.js index a2327fb45752..6d038cf08089 100644 --- a/warehouse/admin/static/js/warehouse.js +++ b/warehouse/admin/static/js/warehouse.js @@ -159,17 +159,22 @@ if (tokenTable.length) { } // Observations -let observationsTable = $("#observations"); -if (observationsTable.length) { - let table = observationsTable.DataTable({ - responsive: true, - lengthChange: false, - }); - table.column(".time").order("desc").draw(); - table.columns([".payload"]).visible(false); - new $.fn.dataTable.Buttons(table, {buttons: ["copy", "csv", "colvis"]}); - table.buttons().container().appendTo($(".col-md-6:eq(0)", table.table().container())); -} +// Note: Each of these tables **must** have the same columns for this to work. +const tableSelectors = ["#observations", "#user_observations"]; + +tableSelectors.forEach(selector => { + let tableElement = $(selector); + if (tableElement.length) { + let table = tableElement.DataTable({ + responsive: true, + lengthChange: false, + }); + table.column(".time").order("desc").draw(); + table.columns([".payload"]).visible(false); + new $.fn.dataTable.Buttons(table, {buttons: ["copy", "csv", "colvis"]}); + table.buttons().container().appendTo($(".col-md-6:eq(0)", table.table().container())); + } +}); // Malware Reports let malwareReportsTable = $("#malware-reports"); diff --git a/warehouse/admin/templates/admin/malware_reports/detail.html b/warehouse/admin/templates/admin/malware_reports/detail.html index d282d1f3be50..368682ec87c8 100644 --- a/warehouse/admin/templates/admin/malware_reports/detail.html +++ b/warehouse/admin/templates/admin/malware_reports/detail.html @@ -175,6 +175,7 @@

Quarantine Project

This will remove the Project from being installable, + freeze the Owner's account, and prohibit the Project from being changed by the Owner.

diff --git a/warehouse/admin/templates/admin/malware_reports/project_list.html b/warehouse/admin/templates/admin/malware_reports/project_list.html index b42553d3b24c..6588ba457a8b 100644 --- a/warehouse/admin/templates/admin/malware_reports/project_list.html +++ b/warehouse/admin/templates/admin/malware_reports/project_list.html @@ -221,6 +221,7 @@

Quarantine Project

This will remove the Project from being installable, + freeze the Owner's account, and prohibit the Project from being changed by the Owner.

diff --git a/warehouse/admin/templates/admin/users/detail.html b/warehouse/admin/templates/admin/users/detail.html index ae738e6618d4..5d8ecd9eeaea 100644 --- a/warehouse/admin/templates/admin/users/detail.html +++ b/warehouse/admin/templates/admin/users/detail.html @@ -883,10 +883,42 @@

Account activity

+ {% if user.observations %} +
+
+

Observations about {{ user.username }}

+
+
+ + + + + + + + + + + + {% for observation in user.observations %} + + + + + + + + {% endfor %} + +
TimeRelatedKindSummaryPayload
{{ observation.created }}{{ observation.related }}{{ observation.kind_display }}{{ observation.summary }}{{ observation.payload }}
+
+
+ {% endif %} + {% if user.observer.observations %}
-

Observations

+

Admin-submitted Observations

diff --git a/warehouse/admin/views/malware_reports.py b/warehouse/admin/views/malware_reports.py index 55a86e4bea27..173352114d3d 100644 --- a/warehouse/admin/views/malware_reports.py +++ b/warehouse/admin/views/malware_reports.py @@ -21,7 +21,7 @@ from warehouse.authnz import Permissions from warehouse.helpdesk.interfaces import IHelpDeskService -from warehouse.observations.models import Observation +from warehouse.observations.models import Observation, ObservationKind from warehouse.utils.project import ( confirm_project, prohibit_and_remove_project, @@ -161,6 +161,13 @@ def malware_reports_project_verdict_remove_malware(project, request): # freeze associated user accounts for user in project.users: user.is_frozen = True + user.record_observation( + request=request, + kind=ObservationKind.AccountAbuse, + actor=request.user, + summary="Account frozen due to malware report", + payload={"project_name": project.name}, + ) # add action to all Malware Observations, **before we remove the project** observations = ( @@ -289,6 +296,13 @@ def remove_malware_for_project(request): # freeze associated user accounts for user in project.users: user.is_frozen = True + user.record_observation( + request=request, + kind=ObservationKind.AccountAbuse, + actor=request.user, + summary="Account frozen due to malware report", + payload={"project_name": project.name}, + ) now = datetime.now(tz=timezone.utc) diff --git a/warehouse/observations/models.py b/warehouse/observations/models.py index 25c777322227..e506f4e6e816 100644 --- a/warehouse/observations/models.py +++ b/warehouse/observations/models.py @@ -113,6 +113,7 @@ class ObservationKind(enum.Enum): IsMalware = ("is_malware", "Is Malware") IsSpam = ("is_spam", "Is Spam") SomethingElse = ("something_else", "Something Else") + AccountAbuse = ("account_abuse", "Account Abuse") AccountRecovery = ( "account_recovery", "Account Recovery", diff --git a/warehouse/utils/project.py b/warehouse/utils/project.py index 8d79072e9fbd..5e4d83882b3e 100644 --- a/warehouse/utils/project.py +++ b/warehouse/utils/project.py @@ -119,6 +119,10 @@ def quarantine_project(project: Project, request, flash=True) -> None: ) ) + # freeze associated user accounts + for user in project.users: + user.is_frozen = True + if flash: request.session.flash( f"Project {project.name} quarantined.\n"