From 17804cb9dec4cdd24961cc16122bdbf0939de4b3 Mon Sep 17 00:00:00 2001 From: Cristina Date: Thu, 26 Dec 2019 06:03:27 -0800 Subject: [PATCH] Add admin interface to view and enable checks (#7134) * Add admin interface to view and enable checks - Implement list, detail and change_state views (#7133) - Add unit tests for check admin view * Add comprehensive test coverage for check admin --- tests/common/db/malware.py | 40 ++++++ tests/unit/admin/test_routes.py | 9 ++ tests/unit/admin/views/test_checks.py | 122 ++++++++++++++++++ warehouse/admin/routes.py | 11 ++ warehouse/admin/templates/admin/base.html | 5 + .../admin/malware/checks/detail.html | 70 ++++++++++ .../templates/admin/malware/checks/index.html | 57 ++++++++ warehouse/admin/views/checks.py | 89 +++++++++++++ warehouse/malware/models.py | 31 +++-- ...1ff3d24c22_add_malware_detection_tables.py | 2 +- 10 files changed, 425 insertions(+), 11 deletions(-) create mode 100644 tests/common/db/malware.py create mode 100644 tests/unit/admin/views/test_checks.py create mode 100644 warehouse/admin/templates/admin/malware/checks/detail.html create mode 100644 warehouse/admin/templates/admin/malware/checks/index.html create mode 100644 warehouse/admin/views/checks.py diff --git a/tests/common/db/malware.py b/tests/common/db/malware.py new file mode 100644 index 000000000000..263fa82fa684 --- /dev/null +++ b/tests/common/db/malware.py @@ -0,0 +1,40 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import datetime + +import factory +import factory.fuzzy + +from warehouse.malware.models import MalwareCheck, MalwareCheckState, MalwareCheckType + +from .base import WarehouseFactory + + +class MalwareCheckFactory(WarehouseFactory): + class Meta: + model = MalwareCheck + + name = factory.fuzzy.FuzzyText(length=12) + version = 1 + short_description = factory.fuzzy.FuzzyText(length=80) + long_description = factory.fuzzy.FuzzyText(length=300) + check_type = factory.fuzzy.FuzzyChoice([e for e in MalwareCheckType]) + hook_name = ( + "project:release:file:upload" + if check_type == MalwareCheckType.event_hook + else None + ) + state = factory.fuzzy.FuzzyChoice([e for e in MalwareCheckState]) + created = factory.fuzzy.FuzzyNaiveDateTime( + datetime.datetime.utcnow() - datetime.timedelta(days=7) + ) diff --git a/tests/unit/admin/test_routes.py b/tests/unit/admin/test_routes.py index 2600365a7ed6..e10962ac54dc 100644 --- a/tests/unit/admin/test_routes.py +++ b/tests/unit/admin/test_routes.py @@ -123,4 +123,13 @@ def test_includeme(): pretend.call("admin.flags.edit", "/admin/flags/edit/", domain=warehouse), pretend.call("admin.squats", "/admin/squats/", domain=warehouse), pretend.call("admin.squats.review", "/admin/squats/review/", domain=warehouse), + pretend.call("admin.checks.list", "/admin/checks/", domain=warehouse), + pretend.call( + "admin.checks.detail", "/admin/checks/{check_name}", domain=warehouse + ), + pretend.call( + "admin.checks.change_state", + "/admin/checks/{check_name}/change_state", + domain=warehouse, + ), ] diff --git a/tests/unit/admin/views/test_checks.py b/tests/unit/admin/views/test_checks.py new file mode 100644 index 000000000000..55a45b2d5ac0 --- /dev/null +++ b/tests/unit/admin/views/test_checks.py @@ -0,0 +1,122 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import uuid + +import pretend +import pytest + +from pyramid.httpexceptions import HTTPNotFound + +from warehouse.admin.views import checks as views +from warehouse.malware.models import MalwareCheckState + +from ....common.db.malware import MalwareCheckFactory + + +class TestListChecks: + def test_get_checks_none(self, db_request): + assert views.get_checks(db_request) == {"checks": []} + + def test_get_checks(self, db_request): + checks = [MalwareCheckFactory.create() for _ in range(10)] + assert views.get_checks(db_request) == {"checks": 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} + + +class TestGetCheck: + def test_get_check(self, db_request): + check = MalwareCheckFactory.create() + db_request.matchdict["check_name"] = check.name + assert views.get_check(db_request) == { + "check": check, + "checks": [check], + "states": MalwareCheckState, + } + + def test_get_check_many_versions(self, db_request): + check1 = MalwareCheckFactory.create(name="MyCheck", version="1") + check2 = MalwareCheckFactory.create(name="MyCheck", version="2") + db_request.matchdict["check_name"] = check1.name + assert views.get_check(db_request) == { + "check": check2, + "checks": [check2, check1], + "states": MalwareCheckState, + } + + def test_get_check_not_found(self, db_request): + db_request.matchdict["check_name"] = "DoesNotExist" + with pytest.raises(HTTPNotFound): + views.get_check(db_request) + + +class TestChangeCheckState: + def test_change_to_enabled(self, db_request): + check = MalwareCheckFactory.create( + name="MyCheck", state=MalwareCheckState.disabled + ) + + db_request.POST = {"id": check.id, "check_state": "enabled"} + db_request.matchdict["check_name"] = check.name + + db_request.session = pretend.stub( + flash=pretend.call_recorder(lambda *a, **kw: None) + ) + db_request.route_path = pretend.call_recorder( + lambda *a, **kw: "/admin/checks/MyCheck/change_state" + ) + + views.change_check_state(db_request) + + assert db_request.session.flash.calls == [ + pretend.call("Changed 'MyCheck' check to 'enabled'!", queue="success") + ] + assert check.state == MalwareCheckState.enabled + + def test_change_to_invalid_state(self, db_request): + check = MalwareCheckFactory.create(name="MyCheck") + initial_state = check.state + invalid_check_state = "cancelled" + db_request.POST = {"id": check.id, "check_state": invalid_check_state} + db_request.matchdict["check_name"] = check.name + + db_request.session = pretend.stub( + flash=pretend.call_recorder(lambda *a, **kw: None) + ) + db_request.route_path = pretend.call_recorder( + lambda *a, **kw: "/admin/checks/MyCheck/change_state" + ) + + views.change_check_state(db_request) + + assert db_request.session.flash.calls == [ + pretend.call("Invalid check state provided.", queue="error") + ] + assert check.state == initial_state + + def test_check_not_found(self, db_request): + db_request.POST = {"id": uuid.uuid4(), "check_state": "enabled"} + db_request.matchdict["check_name"] = "DoesNotExist" + + db_request.route_path = pretend.call_recorder( + lambda *a, **kw: "/admin/checks/DoesNotExist/change_state" + ) + + with pytest.raises(HTTPNotFound): + views.change_check_state(db_request) diff --git a/warehouse/admin/routes.py b/warehouse/admin/routes.py index b0f1afde424d..4180df8188a6 100644 --- a/warehouse/admin/routes.py +++ b/warehouse/admin/routes.py @@ -128,3 +128,14 @@ def includeme(config): # Squats config.add_route("admin.squats", "/admin/squats/", domain=warehouse) config.add_route("admin.squats.review", "/admin/squats/review/", domain=warehouse) + + # Malware checks + config.add_route("admin.checks.list", "/admin/checks/", domain=warehouse) + config.add_route( + "admin.checks.detail", "/admin/checks/{check_name}", domain=warehouse + ) + config.add_route( + "admin.checks.change_state", + "/admin/checks/{check_name}/change_state", + domain=warehouse, + ) diff --git a/warehouse/admin/templates/admin/base.html b/warehouse/admin/templates/admin/base.html index 410d70fe1e60..cc74b4675b81 100644 --- a/warehouse/admin/templates/admin/base.html +++ b/warehouse/admin/templates/admin/base.html @@ -125,6 +125,11 @@ Squats +
  • + + Checks + +
    • diff --git a/warehouse/admin/templates/admin/malware/checks/detail.html b/warehouse/admin/templates/admin/malware/checks/detail.html new file mode 100644 index 000000000000..2cf80f20cc8a --- /dev/null +++ b/warehouse/admin/templates/admin/malware/checks/detail.html @@ -0,0 +1,70 @@ +{# + # Licensed under the Apache License, Version 2.0 (the "License"); + # you may not use this file except in compliance with the License. + # You may obtain a copy of the License at + # + # http://www.apache.org/licenses/LICENSE-2.0 + # + # Unless required by applicable law or agreed to in writing, software + # distributed under the License is distributed on an "AS IS" BASIS, + # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + # See the License for the specific language governing permissions and + # limitations under the License. +-#} +{% extends "admin/base.html" %} + +{% block title %}{{ check.name }}{% endblock %} + +{% block breadcrumb %} +
  • Checks
    • +
  • {{ check.name }}
    • +{% endblock %} + +{% block content %} +
    +
    +

    {{ check.long_description }}

    +

    Revision History

    +
    + + + + + + + {% for c in checks %} + + + + + + {% endfor %} +
    VersionStateCreated
    {{ c.version }}{{ c.state.value }}{{ c.created }}
    +
    +
    +
    +
    +
    +

    Change State

    +
    +
    + + +
    +
    + +
    +
    + +
    +
    +
    +
    + +{% endblock %} diff --git a/warehouse/admin/templates/admin/malware/checks/index.html b/warehouse/admin/templates/admin/malware/checks/index.html new file mode 100644 index 000000000000..5717849e2579 --- /dev/null +++ b/warehouse/admin/templates/admin/malware/checks/index.html @@ -0,0 +1,57 @@ +{# + # Licensed under the Apache License, Version 2.0 (the "License"); + # you may not use this file except in compliance with the License. + # You may obtain a copy of the License at + # + # http://www.apache.org/licenses/LICENSE-2.0 + # + # Unless required by applicable law or agreed to in writing, software + # distributed under the License is distributed on an "AS IS" BASIS, + # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + # See the License for the specific language governing permissions and + # limitations under the License. +-#} +{% extends "admin/base.html" %} + +{% block title %}Malware Checks{% endblock %} + +{% block breadcrumb %} +
  • Checks
    • +{% endblock %} + +{% block content %} +
    +
    + + + + + + + + + {% for check in checks %} + + + + + + + + {% else %} + + + + {% endfor %} +
    Check NameStateRevisionsLast ModifiedDescription
    + + {{ check.name }} + + {{ check.state.value }}{{ check.version }}{{ check.created }}{{ check.short_description }}
    +
    + No checks! +
    +
    +
    +
    +{% endblock content %} diff --git a/warehouse/admin/views/checks.py b/warehouse/admin/views/checks.py new file mode 100644 index 000000000000..e3d38a88a0c0 --- /dev/null +++ b/warehouse/admin/views/checks.py @@ -0,0 +1,89 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from pyramid.httpexceptions import HTTPNotFound, HTTPSeeOther +from pyramid.view import view_config +from sqlalchemy.orm.exc import NoResultFound + +from warehouse.malware.models import MalwareCheck, MalwareCheckState + + +@view_config( + route_name="admin.checks.list", + renderer="admin/malware/checks/index.html", + permission="moderator", + request_method="GET", + uses_session=True, +) +def get_checks(request): + all_checks = request.db.query(MalwareCheck).all() + active_checks = [] + for check in all_checks: + if not check.is_stale: + active_checks.append(check) + + return {"checks": active_checks} + + +@view_config( + route_name="admin.checks.detail", + renderer="admin/malware/checks/detail.html", + permission="moderator", + request_method="GET", + uses_session=True, +) +def get_check(request): + query = ( + request.db.query(MalwareCheck) + .filter(MalwareCheck.name == request.matchdict["check_name"]) + .order_by(MalwareCheck.version.desc()) + ) + + try: + # Throw an exception if and only if no results are returned. + newest = query.limit(1).one() + except NoResultFound: + raise HTTPNotFound + + return {"check": newest, "checks": query.all(), "states": MalwareCheckState} + + +@view_config( + route_name="admin.checks.change_state", + permission="admin", + request_method="POST", + uses_session=True, + require_methods=False, + require_csrf=True, +) +def change_check_state(request): + try: + check = ( + request.db.query(MalwareCheck) + .filter(MalwareCheck.id == request.POST["id"]) + .one() + ) + except NoResultFound: + raise HTTPNotFound + + try: + check.state = getattr(MalwareCheckState, request.POST["check_state"]) + except (AttributeError, KeyError): + request.session.flash("Invalid check state provided.", queue="error") + else: + request.session.flash( + f"Changed {check.name!r} check to {check.state.value!r}!", queue="success" + ) + finally: + return HTTPSeeOther( + request.route_path("admin.checks.detail", check_name=check.name) + ) diff --git a/warehouse/malware/models.py b/warehouse/malware/models.py index fc6576c7dfcd..0c51e3006991 100644 --- a/warehouse/malware/models.py +++ b/warehouse/malware/models.py @@ -34,23 +34,23 @@ class MalwareCheckType(enum.Enum): - EventHook = "event_hook" - Scheduled = "scheduled" + event_hook = "event_hook" + scheduled = "scheduled" class MalwareCheckState(enum.Enum): - Enabled = "enabled" - Evaluation = "evaluation" - Disabled = "disabled" - WipedOut = "wiped_out" + enabled = "enabled" + evaluation = "evaluation" + disabled = "disabled" + wiped_out = "wiped_out" class VerdictClassification(enum.Enum): - Threat = "threat" - Indeterminate = "indeterminate" - Benign = "benign" + threat = "threat" + indeterminate = "indeterminate" + benign = "benign" class VerdictConfidence(enum.Enum): @@ -67,7 +67,7 @@ class MalwareCheck(db.Model): __repr__ = make_repr("name", "version") name = Column(CIText, nullable=False) - version = Column(Integer, default=0, nullable=False) + version = Column(Integer, default=1, nullable=False) short_description = Column(String(length=128), nullable=False) long_description = Column(Text, nullable=False) check_type = Column( @@ -84,6 +84,17 @@ class MalwareCheck(db.Model): ) created = Column(DateTime, nullable=False, server_default=sql.func.now()) + @property + def is_stale(self): + session = orm.object_session(self) + newest = ( + session.query(MalwareCheck) + .filter(MalwareCheck.name == self.name) + .order_by(MalwareCheck.version.desc()) + .first() + ) + return self.version != newest.version + class MalwareVerdict(db.Model): __tablename__ = "malware_verdicts" diff --git a/warehouse/migrations/versions/061ff3d24c22_add_malware_detection_tables.py b/warehouse/migrations/versions/061ff3d24c22_add_malware_detection_tables.py index 899cc51e0f57..569cc0f100b1 100644 --- a/warehouse/migrations/versions/061ff3d24c22_add_malware_detection_tables.py +++ b/warehouse/migrations/versions/061ff3d24c22_add_malware_detection_tables.py @@ -47,7 +47,7 @@ def upgrade(): nullable=False, ), sa.Column("name", citext.CIText(), nullable=False), - sa.Column("version", sa.Integer(), nullable=False), + sa.Column("version", sa.Integer(), default=1, nullable=False), sa.Column("short_description", sa.String(length=128), nullable=False), sa.Column("long_description", sa.Text(), nullable=False), sa.Column("check_type", MalwareCheckTypes, nullable=False),