From 10573b30a8f75daa7e9a0f80b7932c77cd05312b Mon Sep 17 00:00:00 2001 From: Cristina Date: Mon, 3 Feb 2020 08:27:51 -0800 Subject: [PATCH] Add verdicts view filtering capabilities #6062. (#7322) * Add verdicts view filtering capabilities #6062. * Code review changes. - Refactor tests to be parametrized. - Pass `_query` to `route_path` in template. - Remove `is None` from filter query, it adds nothing. --- tests/unit/admin/views/test_verdicts.py | 163 ++++++++++++++++-- warehouse/admin/templates/admin/base.html | 2 +- .../admin/malware/verdicts/index.html | 43 ++++- warehouse/admin/views/verdicts.py | 67 +++++-- 4 files changed, 249 insertions(+), 26 deletions(-) diff --git a/tests/unit/admin/views/test_verdicts.py b/tests/unit/admin/views/test_verdicts.py index 7d28820ca9cf..1492ce853f6e 100644 --- a/tests/unit/admin/views/test_verdicts.py +++ b/tests/unit/admin/views/test_verdicts.py @@ -14,37 +14,176 @@ from random import randint -import pretend import pytest from pyramid.httpexceptions import HTTPBadRequest, HTTPNotFound from warehouse.admin.views import verdicts as views +from warehouse.malware.models import VerdictClassification, VerdictConfidence -from ....common.db.malware import MalwareVerdictFactory +from ....common.db.malware import MalwareCheckFactory, MalwareVerdictFactory class TestListVerdicts: def test_none(self, db_request): - assert views.get_verdicts(db_request) == {"verdicts": []} + assert views.get_verdicts(db_request) == { + "verdicts": [], + "check_names": set(), + "classifications": set(["threat", "indeterminate", "benign"]), + "confidences": set(["low", "medium", "high"]), + } def test_some(self, db_request): - verdicts = [MalwareVerdictFactory.create() for _ in range(10)] + check = MalwareCheckFactory.create() + verdicts = [MalwareVerdictFactory.create(check=check) for _ in range(10)] - assert views.get_verdicts(db_request) == {"verdicts": verdicts} + assert views.get_verdicts(db_request) == { + "verdicts": verdicts, + "check_names": set([check.name]), + "classifications": set(["threat", "indeterminate", "benign"]), + "confidences": set(["low", "medium", "high"]), + } def test_some_with_multipage(self, db_request): - verdicts = [MalwareVerdictFactory.create() for _ in range(60)] + check1 = MalwareCheckFactory.create() + check2 = MalwareCheckFactory.create() + verdicts = [MalwareVerdictFactory.create(check=check2) for _ in range(60)] db_request.GET["page"] = "2" - assert views.get_verdicts(db_request) == {"verdicts": verdicts[25:50]} - - def test_with_invalid_page(self): - request = pretend.stub(params={"page": "not an integer"}) - + assert views.get_verdicts(db_request) == { + "verdicts": verdicts[25:50], + "check_names": set([check1.name, check2.name]), + "classifications": set(["threat", "indeterminate", "benign"]), + "confidences": set(["low", "medium", "high"]), + } + + @pytest.mark.parametrize( + "check_name", ["check0", "check1", ""], + ) + def test_check_name_filter(self, db_request, check_name): + result_verdicts, all_verdicts = [], [] + for i in range(3): + check = MalwareCheckFactory.create(name="check%d" % i) + verdicts = [MalwareVerdictFactory.create(check=check) for _ in range(5)] + all_verdicts.extend(verdicts) + if check.name == check_name: + result_verdicts = verdicts + + # Emptry string + if not result_verdicts: + result_verdicts = all_verdicts + + response = { + "verdicts": result_verdicts, + "check_names": set(["check0", "check1", "check2"]), + "classifications": set(["threat", "indeterminate", "benign"]), + "confidences": set(["low", "medium", "high"]), + } + + db_request.GET["check_name"] = check_name + assert views.get_verdicts(db_request) == response + + @pytest.mark.parametrize( + "classification", ["benign", "indeterminate", "threat", ""], + ) + def test_classification_filter(self, db_request, classification): + check1 = MalwareCheckFactory.create() + result_verdicts, all_verdicts = [], [] + for c in VerdictClassification: + verdicts = [ + MalwareVerdictFactory.create(check=check1, classification=c) + for _ in range(5) + ] + all_verdicts.extend(verdicts) + if c.value == classification: + result_verdicts = verdicts + + # Emptry string + if not result_verdicts: + result_verdicts = all_verdicts + + db_request.GET["classification"] = classification + response = { + "verdicts": result_verdicts, + "check_names": set([check1.name]), + "classifications": set(["threat", "indeterminate", "benign"]), + "confidences": set(["low", "medium", "high"]), + } + assert views.get_verdicts(db_request) == response + + @pytest.mark.parametrize( + "confidence", ["low", "medium", "high", ""], + ) + def test_confidence_filter(self, db_request, confidence): + check1 = MalwareCheckFactory.create() + result_verdicts, all_verdicts = [], [] + for c in VerdictConfidence: + verdicts = [ + MalwareVerdictFactory.create(check=check1, confidence=c) + for _ in range(5) + ] + all_verdicts.extend(verdicts) + if c.value == confidence: + result_verdicts = verdicts + + # Emptry string + if not result_verdicts: + result_verdicts = all_verdicts + + response = { + "verdicts": result_verdicts, + "check_names": set([check1.name]), + "classifications": set(["threat", "indeterminate", "benign"]), + "confidences": set(["low", "medium", "high"]), + } + + db_request.GET["confidence"] = confidence + assert views.get_verdicts(db_request) == response + + @pytest.mark.parametrize( + "manually_reviewed", [1, 0], + ) + def test_manually_reviewed_filter(self, db_request, manually_reviewed): + check1 = MalwareCheckFactory.create() + result_verdicts = [ + MalwareVerdictFactory.create( + check=check1, manually_reviewed=bool(manually_reviewed) + ) + for _ in range(5) + ] + + # Create other verdicts to ensure filter works properly + for _ in range(10): + MalwareVerdictFactory.create( + check=check1, manually_reviewed=not bool(manually_reviewed) + ) + + db_request.GET["manually_reviewed"] = str(manually_reviewed) + + response = { + "verdicts": result_verdicts, + "check_names": set([check1.name]), + "classifications": set(["threat", "indeterminate", "benign"]), + "confidences": set(["low", "medium", "high"]), + } + + assert views.get_verdicts(db_request) == response + + @pytest.mark.parametrize( + "invalid_param", + [ + ("page", "invalid"), + ("check_name", "NotACheck"), + ("confidence", "NotAConfidence"), + ("classification", "NotAClassification"), + ("manually_reviewed", "False"), + ], + ) + def test_errors(self, db_request, invalid_param): + db_request.GET[invalid_param[0]] = invalid_param[1] with pytest.raises(HTTPBadRequest): - views.get_verdicts(request) + views.get_verdicts(db_request) class TestGetVerdict: diff --git a/warehouse/admin/templates/admin/base.html b/warehouse/admin/templates/admin/base.html index fffa7ccfcec5..18be097d6576 100644 --- a/warehouse/admin/templates/admin/base.html +++ b/warehouse/admin/templates/admin/base.html @@ -131,7 +131,7 @@
  • - + Verdicts
    • diff --git a/warehouse/admin/templates/admin/malware/verdicts/index.html b/warehouse/admin/templates/admin/malware/verdicts/index.html index d6ab7ef6b028..443dd295aa15 100644 --- a/warehouse/admin/templates/admin/malware/verdicts/index.html +++ b/warehouse/admin/templates/admin/malware/verdicts/index.html @@ -17,12 +17,53 @@ {% block title %}Malware Verdicts{% endblock %} +{% set check_name = request.params.get("check_name") %} +{% set classification = request.params.get("classification") %} +{% set confidence = request.params.get("confidence") %} +{% set manually_reviewed = request.params.get("manually_reviewed") %} + {% block breadcrumb %}
  • Verdicts
    • {% endblock %} {% block content %}
    +
    +
    +
    + +
    +
    + +
    +
    + +
    +
    + +
    + +
    +
    @@ -30,7 +71,7 @@ - + {% for verdict in verdicts %} diff --git a/warehouse/admin/views/verdicts.py b/warehouse/admin/views/verdicts.py index bd9c2eae68ca..ab8dec6dd514 100644 --- a/warehouse/admin/views/verdicts.py +++ b/warehouse/admin/views/verdicts.py @@ -14,7 +14,12 @@ from pyramid.httpexceptions import HTTPBadRequest, HTTPNotFound from pyramid.view import view_config -from warehouse.malware.models import MalwareVerdict +from warehouse.malware.models import ( + MalwareCheck, + MalwareVerdict, + VerdictClassification, + VerdictConfidence, +) from warehouse.utils.paginate import paginate_url_factory @@ -26,23 +31,23 @@ uses_session=True, ) def get_verdicts(request): - try: - page_num = int(request.params.get("page", 1)) - except ValueError: - raise HTTPBadRequest("'page' must be an integer.") from None - - verdicts_query = request.db.query(MalwareVerdict).order_by( - MalwareVerdict.run_date.desc() + result = {} + result["check_names"] = set( + [name for (name,) in request.db.query(MalwareCheck.name)] ) + result["classifications"] = set([c.value for c in VerdictClassification]) + result["confidences"] = set([c.value for c in VerdictConfidence]) + + validate_fields(request, result) - verdicts = SQLAlchemyORMPage( - verdicts_query, - page=page_num, + result["verdicts"] = SQLAlchemyORMPage( + generate_query(request.db, request.params), + page=int(request.params.get("page", 1)), items_per_page=25, url_maker=paginate_url_factory(request), ) - return {"verdicts": verdicts} + return result @view_config( @@ -59,3 +64,41 @@ def get_verdict(request): return {"verdict": verdict} raise HTTPNotFound + + +def validate_fields(request, validators): + try: + int(request.params.get("page", 1)) + except ValueError: + raise HTTPBadRequest("'page' must be an integer.") from None + + validators = {**validators, **{"manually_revieweds": set(["0", "1"])}} + + for key, possible_values in validators.items(): + # Remove the trailing 's' + value = request.params.get(key[:-1]) + additional_values = set([None, ""]) + if value not in possible_values | additional_values: + raise HTTPBadRequest( + "Invalid value for '%s': %s." % (key[:-1], value) + ) from None + + +def generate_query(db, params): + """ + Returns an SQLAlchemy query wth request params applied as filters. + """ + query = db.query(MalwareVerdict) + if params.get("check_name"): + query = query.join(MalwareCheck) + query = query.filter(MalwareCheck.name == params["check_name"]) + if params.get("confidence"): + query = query.filter(MalwareVerdict.confidence == params["confidence"]) + if params.get("classification"): + query = query.filter(MalwareVerdict.classification == params["classification"]) + if params.get("manually_reviewed"): + query = query.filter( + MalwareVerdict.manually_reviewed == bool(int(params["manually_reviewed"])) + ) + + return query.order_by(MalwareVerdict.run_date.desc())
    Check Classification ConfidenceDetailReview