Add verdict administrator review. Fixes #6062.

tests/unit/admin/test_routes.py

@@ -141,4 +141,9 @@ def test_includeme():
         pretend.call(
             "admin.verdicts.detail", "/admin/verdicts/{verdict_id}", domain=warehouse
         ),
+        pretend.call(
+            "admin.verdicts.review",
+            "/admin/verdicts/{verdict_id}/review",
+            domain=warehouse,
+        ),
     ]

tests/unit/admin/views/test_checks.py

@@ -28,15 +28,21 @@ def test_get_checks_none(self, db_request):
 
     def test_get_checks(self, db_request):
         checks = [MalwareCheckFactory.create() for _ in range(10)]
-        assert views.get_checks(db_request) == {"checks": checks}
+        result = views.get_checks(db_request)["checks"]
+        assert len(result) == len(checks)
+        for r in result:
+            assert r in checks
 
     def test_get_checks_different_versions(self, db_request):
         checks = [MalwareCheckFactory.create() for _ in range(5)]
         checks_same = [
             MalwareCheckFactory.create(name="MyCheck", version=i) for i in range(1, 6)
         ]
         checks.append(checks_same[-1])
-        assert views.get_checks(db_request) == {"checks": checks}
+        result = views.get_checks(db_request)["checks"]
+        assert len(result) == len(checks)
+        for r in result:
+            assert r in checks
 
 
 class TestGetCheck:

tests/unit/admin/views/test_verdicts.py

@@ -14,6 +14,7 @@
 
 from random import randint
 
+import pretend
 import pytest
 
 from pyramid.httpexceptions import HTTPBadRequest, HTTPNotFound
@@ -193,10 +194,58 @@ def test_found(self, db_request):
         lookup_id = verdicts[index].id
         db_request.matchdict["verdict_id"] = lookup_id
 
-        assert views.get_verdict(db_request) == {"verdict": verdicts[index]}
+        assert views.get_verdict(db_request) == {
+            "verdict": verdicts[index],
+            "classifications": ["Benign", "Indeterminate", "Threat"],
+        }
 
     def test_not_found(self, db_request):
         db_request.matchdict["verdict_id"] = uuid.uuid4()
 
         with pytest.raises(HTTPNotFound):
             views.get_verdict(db_request)
+
+
+class TestReviewVerdict:
+    @pytest.mark.parametrize(
+        "manually_reviewed, administrator_verdict",
+        [
+            (False, None),  # unreviewed verdict
+            (True, VerdictClassification.Threat),  # previously reviewed
+        ],
+    )
+    def test_set_classification(
+        self, db_request, manually_reviewed, administrator_verdict
+    ):
+        verdict = MalwareVerdictFactory.create(
+            manually_reviewed=manually_reviewed,
+            administrator_verdict=administrator_verdict,
+        )
+
+        db_request.matchdict["verdict_id"] = verdict.id
+        db_request.POST = {"classification": "Benign"}
+        db_request.session = pretend.stub(
+            flash=pretend.call_recorder(lambda *a, **kw: None)
+        )
+
+        db_request.route_path = pretend.call_recorder(
+            lambda *a, **kw: "/admin/verdicts/%s/review" % verdict.id
+        )
+
+        views.review_verdict(db_request)
+
+        assert db_request.session.flash.calls == [
+            pretend.call("Verdict %s marked as reviewed." % verdict.id, queue="success")
+        ]
+
+        assert verdict.manually_reviewed
+        assert verdict.administrator_verdict == VerdictClassification.Benign
+
+    @pytest.mark.parametrize("post_params", [{}, {"classification": "Nope"}])
+    def test_errors(self, db_request, post_params):
+        verdict = MalwareVerdictFactory.create()
+        db_request.matchdict["verdict_id"] = verdict.id
+        db_request.POST = post_params
+
+        with pytest.raises(HTTPBadRequest):
+            views.review_verdict(db_request)

warehouse/admin/routes.py

@@ -148,3 +148,6 @@ def includeme(config):
     config.add_route(
         "admin.verdicts.detail", "/admin/verdicts/{verdict_id}", domain=warehouse
     )
+    config.add_route(
+        "admin.verdicts.review", "/admin/verdicts/{verdict_id}/review", domain=warehouse
+    )

warehouse/admin/templates/admin/malware/verdicts/detail.html

@@ -13,7 +13,7 @@
 -#}
 {% extends "admin/base.html" %}
 
-{% block title %}Verdict {{ verdict.id }}{% endblock %}
+{% block title %}Verdict Details{% endblock %}
 
 {% block breadcrumb %}
   <li><a href="{{ request.route_path('admin.verdicts.list') }}">Verdicts</a></li>
@@ -44,24 +44,20 @@
           <th scope="row">Object</th>
           <td>{% include 'object_link.html' %}</td>
         </tr>
+        {% if verdict.manually_reviewed %}
         <tr>
-          <th scope="row">Verdict Classification</th>
-          <td>{{ verdict.classification.value }}</td>
-        </tr>
-        <tr>
-          <th scope="row">Verdict Confidence</th>
-          <td>{{ verdict.confidence.value }}</td>
+          <th scope="row">Administrator Verdict</th>
+          <td>{{ verdict.administrator_verdict.value }}</td>
         </tr>
+        {% endif %}
         <tr>
-          <th scope="row">Manually Reviewed</th>
-          <td>{{ verdict.manually_reviewed }}</td>
+          <th scope="row">Check Verdict</th>
+          <td>{{ verdict.classification.value }}</td>
         </tr>
-        {% if verdict.manually_reviewed %}
         <tr>
-          <th scope="row">Administrator Verdict</th>
-          <td>{{ verdict.administrator_verdict }}</td>
+          <th scope="row">Confidence</th>
+          <td>{{ verdict.confidence.value }}</td>
         </tr>
-        {% endif %}
         {% if verdict.full_report_link %}
         <tr>
           <th scope="row">Full Report Link</th>
@@ -70,10 +66,25 @@
         {% endif %}
         {% if verdict.details %}
         <tr>
-          <th scope="row">Details</th>
+          <th scope="row">Additional Details</th>
           <td><pre>{{ verdict.details|tojson(indent=4) }}</pre></td>
         </tr>
         {% endif %}
+        <tr>
+          <th scope="row">{{ "Change" if verdict.manually_reviewed else "Set"}} Verdict</th>
+          <td>
+            <form class="form-inline" method="POST" action="{{ request.route_path('admin.verdicts.review', verdict_id=verdict.id) }}">
+              <input name="csrf_token" type="hidden" value="{{ request.session.get_csrf_token() }}">
+              <select class="form-control" name="classification">
+                <option disabled selected></option>
+                {% for c in classifications %}
+                <option>{{ c }}</option>
+                {% endfor %}
+              </select>
+              <button type="submit" class="btn btn-primary">Save</button>
+            </form>
+          </td>
+        </tr>
       </table>
     </div>
   </div>

warehouse/admin/templates/admin/malware/verdicts/index.html

@@ -67,6 +67,7 @@
   <div class="box-body table-responsive no-padding">
     <table class="table table-hover">
       <tr>
+        <th>Investigate</th>
         <th>Object</th>
         <th>Check</th>
         <th>Classification</th>
@@ -75,6 +76,11 @@
       </tr>
       {% for verdict in verdicts %}
       <tr>
+        <td>
+          <a href="{{ request.route_path('admin.verdicts.detail', verdict_id=verdict.id) }}">
+            Detail
+          </a>
+        </td>
         <td>{% include 'object_link.html' %}</td>
         <td>
           <a href="{{ request.route_path('admin.checks.detail', check_name=verdict.check.name) }}">
@@ -104,9 +110,15 @@
           </span>
         </td>
         <td>
-          <a href="{{ request.route_path('admin.verdicts.detail', verdict_id=verdict.id) }}">
-            Detail
-          </a>
+          {% if verdict.manually_reviewed %}
+          Marked as {{ verdict.administrator_verdict.value }}
+          {% else %}
+          <form method="POST" action="{{ request.route_path('admin.verdicts.review', verdict_id=verdict.id, _query=request.params) }}">
+            <input type="hidden" name="classification" value="Benign">
+            <input name="csrf_token" type="hidden" value="{{ request.session.get_csrf_token() }}">
+            <input type="submit" value="Mark Benign" title="{{ 'Marking requires superuser privileges' if not request.has_permission('admin') }}" {{ "disabled" if not request.has_permission('admin') }}>
+          </form>
+	  {% endif %}
         </td>
       </tr>
       {% else %}

warehouse/admin/views/checks.py

@@ -34,6 +34,8 @@ def get_checks(request):
         if not check.is_stale:
             active_checks.append(check)
 
+    active_checks.sort(key=lambda check: check.created, reverse=True)
+
     return {"checks": active_checks}
 
 

warehouse/admin/views/verdicts.py

@@ -11,7 +11,7 @@
 # limitations under the License.
 
 from paginate_sqlalchemy import SqlalchemyOrmPage as SQLAlchemyORMPage
-from pyramid.httpexceptions import HTTPBadRequest, HTTPNotFound
+from pyramid.httpexceptions import HTTPBadRequest, HTTPNotFound, HTTPSeeOther
 from pyramid.view import view_config
 
 from warehouse.malware.models import (
@@ -61,11 +61,43 @@ def get_verdict(request):
     verdict = request.db.query(MalwareVerdict).get(request.matchdict["verdict_id"])
 
     if verdict:
-        return {"verdict": verdict}
+        return {
+            "verdict": verdict,
+            "classifications": list(VerdictClassification.__members__.keys()),
+        }
 
     raise HTTPNotFound
 
 
+@view_config(
+    route_name="admin.verdicts.review",
+    permission="admin",
+    request_method="POST",
+    uses_session=True,
+    require_methods=False,
+    require_csrf=True,
+)
+def review_verdict(request):
+    verdict = request.db.query(MalwareVerdict).get(request.matchdict["verdict_id"])
+
+    try:
+        classification = getattr(VerdictClassification, request.POST["classification"])
+    except (KeyError, AttributeError):
+        raise HTTPBadRequest("Invalid verdict classification.") from None
+
+    verdict.manually_reviewed = True
+    verdict.administrator_verdict = classification
+
+    request.session.flash(
+        "Verdict %s marked as reviewed." % verdict.id, queue="success"
+    )
+
+    # If no query params are provided (e.g. request originating from
+    # admins.verdicts.detail view), then route to the default list view
+    query = request.GET or {"classification": "threat", "manually_reviewed": "0"}
+    return HTTPSeeOther(request.route_path("admin.verdicts.list", _query=query))
+
+
 def validate_fields(request, validators):
     try:
         int(request.params.get("page", 1))