diff --git a/README.md b/README.md index 3dad384d..78d84878 100644 --- a/README.md +++ b/README.md @@ -77,6 +77,7 @@ Environment variables: | Name | Effect | Example | |------|--------|---------| | `CHECKMATE_BLOCKLIST_URL` | Where to download the blocklist online | `https://some-aws-s3.bucket/file.txt` | +| `CHECKMATE_SECRET` | Secret used for signing URLs | `AB823F97FF2E330C1A20` For details of changing the blocklist see: diff --git a/checkmate/app.py b/checkmate/app.py index 27432310..a2878c20 100644 --- a/checkmate/app.py +++ b/checkmate/app.py @@ -69,12 +69,18 @@ def configure(config, celery_worker=False): # pragma: no cover if celery_worker: # Configure sentry config.add_settings({"h_pyramid_sentry.celery_support": True}) + else: # The celery workers don't need to know about this stuff config.include("pyramid_jinja2") + config.include("pyramid_services") + + # Include the secret for crytographic functions + config.registry.settings["secret"] = os.environ["CHECKMATE_SECRET"] config.scan("checkmate.views") config.include("checkmate.routes") + config.include("checkmate.services") # Configure sentry config.add_settings( diff --git a/checkmate/routes.py b/checkmate/routes.py index f6f8f86c..fc1ca723 100644 --- a/checkmate/routes.py +++ b/checkmate/routes.py @@ -5,3 +5,8 @@ def includeme(config): # pragma: no cover """Pyramid config.""" config.add_route("get_status", "/_status") config.add_route("check_url", "/api/check") + + # Serve content from the static/static directory at /ui/static + config.add_static_view("ui/static", "static/static", cache_max_age=3600) + + config.add_route("present_block", "/ui/block") diff --git a/checkmate/services/__init__.py b/checkmate/services/__init__.py new file mode 100644 index 00000000..85891763 --- /dev/null +++ b/checkmate/services/__init__.py @@ -0,0 +1,8 @@ +from checkmate.services.secure_link import SecureLinkService + + +def includeme(config): # pragma: no cover + config.register_service_factory( + "checkmate.services.secure_link.factory", + iface=SecureLinkService, + ) diff --git a/checkmate/services/secure_link.py b/checkmate/services/secure_link.py new file mode 100644 index 00000000..384cff79 --- /dev/null +++ b/checkmate/services/secure_link.py @@ -0,0 +1,79 @@ +"""Create and check secure links.""" + +from hashlib import sha256 + + +class SecureLinkService: + """A class for generating secure links to our own routes.""" + + TOKEN_ARG = "sec" + VERSION_ARG = "v" + + def __init__(self, secret, route_url): + """Create a new BlockURLService object. + + :param secret: The secret to sign and check args with + :param route_url: The pyramid `request.route_url` function + """ + self._secret = secret + self._route_url = route_url + + def route_url(self, route_name, *elements, **kw): + """Get a secure version of route to our own app. + + This works identically to the Pyramid `Request.route_url()` function + with the exception that the parameters are modified in place to sign + them. This signature can be checked with `is_secure()`. + """ + + args = kw.get("_query") + if args is None: + raise ValueError("You must provide some parameters to sign") + + # Include this to allow us to cope with breaking changes in the + # future if we ever decide to make some. + args[self.VERSION_ARG] = "1" + + # We really should include the elements in the hash, but at present + # we only have one link, and it has no elements. + args[self.TOKEN_ARG] = self._hash_args(route_name, args) + + return self._route_url(route_name, *elements, **kw) + + def is_secure(self, request): + """Check if a request was signed correctly. + + :param request: A pyramid Request object to check + :rtype: bool + """ + args = request.GET + + if self.TOKEN_ARG not in args: + return False + + version = args.get(self.VERSION_ARG) + if version != "1": + return False + + without_token = dict(args) + token = without_token.pop(self.TOKEN_ARG) + + return token == self._hash_args(request.matched_route.name, without_token) + + def _hash_args(self, route_name, args): + digest = sha256() + + digest.update(self._secret.encode("utf-8")) + digest.update(route_name.encode("utf-8")) + + for key, value in sorted(args.items()): + digest.update(key.encode("utf-8")) + digest.update(value.encode("utf-8")) + + return digest.hexdigest() + + +def factory(_context, request): + return SecureLinkService( + secret=request.registry.settings["secret"], route_url=request.route_url + ) diff --git a/checkmate/static/static/css/wrapper-style.css b/checkmate/static/static/css/wrapper-style.css new file mode 100644 index 00000000..8c969238 --- /dev/null +++ b/checkmate/static/static/css/wrapper-style.css @@ -0,0 +1,146 @@ +/** +* Eric Meyer's Reset CSS v2.0 (http://meyerweb.com/eric/tools/css/reset/) +* http://cssreset.com +*/ +html, body, div, span, applet, object, iframe, +h1, h2, h3, h4, h5, h6, p, blockquote, pre, +a, abbr, acronym, address, big, cite, code, +del, dfn, em, img, ins, kbd, q, s, samp, +small, strike, strong, sub, sup, tt, var, +b, u, i, center, +dl, dt, dd, ol, ul, li, +fieldset, form, label, legend, +table, caption, tbody, tfoot, thead, tr, th, td, +article, aside, canvas, details, embed, +figure, figcaption, footer, header, hgroup, +menu, nav, output, ruby, section, summary, +time, mark, audio, video { + margin: 0; + padding: 0; + border: 0; + font-size: 100%; + font: inherit; + vertical-align: baseline; +} +/* HTML5 display-role reset for older browsers */ +article, aside, details, figcaption, figure, +footer, header, hgroup, menu, nav, section { + display: block; +} +body { + line-height: 1; +} +ol, ul { + list-style: none; +} +blockquote, q { + quotes: none; +} +blockquote:before, blockquote:after, +q:before, q:after { + content: ''; + content: none; +} +table { + border-collapse: collapse; + border-spacing: 0; +} + +/* page style */ +body { + background-color: #ececec; + margin: 0 auto; + color: #333333; + max-width: 960px; + font-family: 'Source Sans Pro', sans-serif; + -webkit-font-smoothing: antialiased; + text-align: center; +} + +header { + padding: 100px 0 0 0; +} + +header, article { + text-align: left; + margin: 0 auto; + max-width: 600px; +} + +p, ol { + font-size: 20px; + line-height: 27px; + padding-bottom: 30px; + max-width: 600px; + margin: 0 auto; +} + +strong { + font-weight: 700; +} + +h1 { + font-weight: 700; + font-size: 20px; + line-height: 27px; +} + +p a, li a { + border-bottom: 1px solid #DCDCDC; + text-decoration: none; + color: #333333; +} + +ol { + list-style-type: decimal; + padding-left: 17px; +} + +ol li { + font-size: 20px; + line-height: 27px; + padding-bottom: 30px; +} + +a:hover { + color: #333333; +} + +footer { + padding: 90px 0; +} + +#footer-logo { + display:inline-block; + width: 24px; + height:28px; + background: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAADAAAAA4CAQAAAARWLNhAAABgUlEQVR42u2Yz0rDQBDGhwpKe6wUSXspepE+gGfv+gCtPaTv4EMIHrx7K0iepILXmQ2btAQECYmXnoSWStUYxNLdNmoKsyCab2677P6WbybZPwAwapCDESbMEZEzasDH9GP2yT+DxikiXX1iLsgBA+ZoRoHR6dMoAJsDQBHOC0ABKAB/APCCUzwX+3I7Kfm78kic4Q3NWAFkw4rcA7xlA9AAMhSWacAEcI8hU34zv1HfAr4WXhsGuCdMADwlF6c0o3txEewogD0WgDikZ+UIcrnsSbbojaNMr7QzzoNWYU8cZXqnAV6TEjfgUe8Ly8wAnJgGzP8PgELqeJZnUYdCAwAKZXXRLqs6ggfQ1mqvzQ7wLHWUZ7EDhnV11LDODhBd7Q/W5a+iKKgt2oOafin7adPP+x3EZPvNdKezMS6uUL8PYPoibvwpwfhjiPHnnDyifub6+sCluIJyDSDjCvBJtlYOABPZAl6JngoQPeDXMhOM7mdmgtf9tUzwu69nYlP33wEjM9swn4BymAAAAABJRU5ErkJggg==); + background-size: 24px 28px; + + font-size:0; +} + +@media (max-width: 800px) { + body {padding: 2em 1em;} + header {padding: 50px 0; font-size: 36px; line-height: 42px;} + h1 {font-size: 48px; line-height: 48px;} + p, ol li {font-size: 18px; line-height: 24px;} +} + +/* Add exceptions for malicious view */ + +.icon.danger { + display: block; + height: 64px; + margin-bottom:10px; + background: url('../img/warning.svg'); + background-size: 64px 64px; + background-position: center; + background-repeat: no-repeat; +} + +body.danger { + color:white; + background-color: #aa0000; +} + diff --git a/checkmate/static/static/img/favicon.ico b/checkmate/static/static/img/favicon.ico new file mode 100644 index 00000000..ef4ba6b1 Binary files /dev/null and b/checkmate/static/static/img/favicon.ico differ diff --git a/checkmate/static/static/img/warning.svg b/checkmate/static/static/img/warning.svg new file mode 100644 index 00000000..0de7b75e --- /dev/null +++ b/checkmate/static/static/img/warning.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/checkmate/templates/blocked_page.html.jinja2 b/checkmate/templates/blocked_page.html.jinja2 new file mode 100644 index 00000000..ebb93764 --- /dev/null +++ b/checkmate/templates/blocked_page.html.jinja2 @@ -0,0 +1,90 @@ +{% extends "wrapper.html.jinja2" %} + +{% block title %} + Content not available +{% endblock %} + +{% macro how_to_access() %} +

+ To view the annotations on this page: +

+ +
    +
  1. + Follow the instructions + on our homepage to install our browser extension. +
    2. +
  2. + Visit the page directly at + {{ blocked_url }} + and click on the extension icon to see the annotations. +
    3. +
+{% endmacro %} + +{% macro bad_site_ahead() %} +

+ Attackers on the site may trick you into doing + something dangerous like installing software + or revealing your personal information (for example, passwords, + phone numbers or credit cards). +

+ + {# Do what we can to prevent the user from selecting the text #} +

+ Original URL: {{ blocked_url }} +

+{% endmacro %} + +{% block body_attrs %} + {% if reason == "malicious" %} + class="danger" + {% endif %} +{% endblock %} + +{% block content %} + {% if reason == "malicious" %} + {% set text = { + "heading": "
Deceptive site ahead", + "details": "This site is not available through Hypothesis because + it might be hosting harmful content." + } %} + + {% elif reason == "publisher-blocked" %} + {% set text = { + "heading": "Content not available", + "details": "Unfortunately, the publisher of this page has requested + that we disallow access to it through this Hypothesis service. + That means we can't show you the page you were looking for + right away…" + } %} + + {% else %} + {% set text = { + "heading": "Content cannot be annotated", + "details": "Unfortunately, the contents of this page cannot be + annotated through this Hypothesis service." + } %} + {% endif %} + + +
+

{{ text.heading | safe }}

+
+ +
+

{{ text.details | safe }}

+ + {% if reason == "malicious" %} + {{ bad_site_ahead() }} + {% else %} + {{ how_to_access() }} + {% endif %} +
+ +{% endblock %} \ No newline at end of file diff --git a/checkmate/templates/wrapper.html.jinja2 b/checkmate/templates/wrapper.html.jinja2 new file mode 100644 index 00000000..91033d03 --- /dev/null +++ b/checkmate/templates/wrapper.html.jinja2 @@ -0,0 +1,26 @@ + + + + {% block title %}Via{% endblock %} + + + + + + + + + + {% block content %} + {% endblock %} + + + diff --git a/checkmate/views/api/check_url.py b/checkmate/views/api/check_url.py index ea015c0e..9c84a1f7 100644 --- a/checkmate/views/api/check_url.py +++ b/checkmate/views/api/check_url.py @@ -5,6 +5,7 @@ from checkmate.checker.url import CompoundRules from checkmate.exceptions import BadURLParameter +from checkmate.services import SecureLinkService @view_config(route_name="check_url", renderer="json") @@ -23,11 +24,19 @@ def check_url(request): # If everything is fine give a 204 which is successful, but has no body return HTTPNoContent() + # Reasons are in severity order, worst first + worst_reason = reasons[0] + # https://jsonapi.org/format/#document-top-level return { "data": [reason.serialise() for reason in reasons], "meta": { # Reasons are in severity order, worst first - "maxSeverity": reasons[0].severity.value, + "maxSeverity": worst_reason.severity.value, + }, + "links": { + "html": request.find_service(SecureLinkService).route_url( + "present_block", _query={"url": url, "reason": worst_reason.value} + ) }, } diff --git a/checkmate/views/ui/__init__.py b/checkmate/views/ui/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/checkmate/views/ui/present_block.py b/checkmate/views/ui/present_block.py new file mode 100644 index 00000000..3e47c4d7 --- /dev/null +++ b/checkmate/views/ui/present_block.py @@ -0,0 +1,26 @@ +"""User feedback for blocked pages.""" + +from pyramid.exceptions import HTTPForbidden +from pyramid.view import view_config + +from checkmate.services import SecureLinkService + + +@view_config( + route_name="present_block", + renderer="checkmate:templates/blocked_page.html.jinja2", + # The content of a given page should never change once issued. It should + # be possible to cache them indefinitely, but we want something a bit + # shorter than "forever" so we can modify the page without too much + # delay. + http_cache=(3600, {"public": 1}), +) +def present_block(_context, request): + """Render an HTML version of a blocked URL with explanation.""" + + if not request.find_service(SecureLinkService).is_secure(request): + raise HTTPForbidden() + + # At this point we know the contents of the args came from us, so we'll + # assume that they are correct. + return {"blocked_url": request.GET["url"], "reason": request.GET["reason"]} diff --git a/requirements/dev.txt b/requirements/dev.txt index de9be229..4bec1a66 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -60,8 +60,9 @@ pynacl==1.4.0 # via paramiko pyramid-exclog==1.0 # via -r requirements/requirements.txt pyramid-ipython==0.2 # via -r requirements/dev.in pyramid-jinja2==2.8 # via -r requirements/requirements.txt +pyramid-services==2.2 # via -r requirements/requirements.txt pyramid-tm==2.4 # via -r requirements/requirements.txt -pyramid==1.10.5 # via -r requirements/requirements.txt, h-pyramid-sentry, pyramid-exclog, pyramid-ipython, pyramid-jinja2, pyramid-tm +pyramid==1.10.5 # via -r requirements/requirements.txt, h-pyramid-sentry, pyramid-exclog, pyramid-ipython, pyramid-jinja2, pyramid-services, pyramid-tm pyrsistent==0.17.3 # via jsonschema python-dateutil==2.8.1 # via -r requirements/requirements.txt, alembic, faker python-dotenv==0.15.0 # via docker-compose @@ -84,9 +85,10 @@ vine==5.0.0 # via -r requirements/requirements.txt, amqp, celery wcwidth==0.2.5 # via -r requirements/requirements.txt, prompt-toolkit webob==1.8.6 # via -r requirements/requirements.txt, pyramid websocket-client==0.57.0 # via docker, docker-compose +wired==0.3 # via -r requirements/requirements.txt, pyramid-services zipp==3.4.0 # via -r requirements/requirements.txt, importlib-metadata, importlib-resources zope.deprecation==4.4.0 # via -r requirements/requirements.txt, pyramid, pyramid-jinja2 -zope.interface==5.2.0 # via -r requirements/requirements.txt, pyramid, transaction, zope.sqlalchemy +zope.interface==5.2.0 # via -r requirements/requirements.txt, pyramid, pyramid-services, transaction, wired, zope.sqlalchemy zope.sqlalchemy==1.3 # via -r requirements/requirements.txt # The following packages are considered to be unsafe in a requirements file: diff --git a/requirements/lint.txt b/requirements/lint.txt index 690722d5..1cf0ab04 100644 --- a/requirements/lint.txt +++ b/requirements/lint.txt @@ -51,8 +51,9 @@ pylint==2.6.0 # via -r requirements/lint.in pyparsing==2.4.7 # via -r requirements/tests.txt, packaging pyramid-exclog==1.0 # via -r requirements/requirements.txt, -r requirements/tests.txt pyramid-jinja2==2.8 # via -r requirements/requirements.txt, -r requirements/tests.txt +pyramid-services==2.2 # via -r requirements/requirements.txt, -r requirements/tests.txt pyramid-tm==2.4 # via -r requirements/requirements.txt, -r requirements/tests.txt -pyramid==1.10.5 # via -r requirements/requirements.txt, -r requirements/tests.txt, h-pyramid-sentry, pyramid-exclog, pyramid-jinja2, pyramid-tm +pyramid==1.10.5 # via -r requirements/requirements.txt, -r requirements/tests.txt, h-pyramid-sentry, pyramid-exclog, pyramid-jinja2, pyramid-services, pyramid-tm pytest==6.1.2 # via -r requirements/tests.txt python-dateutil==2.8.1 # via -r requirements/requirements.txt, -r requirements/tests.txt, alembic, faker python-editor==1.0.4 # via -r requirements/requirements.txt, -r requirements/tests.txt, alembic @@ -77,10 +78,11 @@ waitress==1.4.4 # via -r requirements/tests.txt, webtest wcwidth==0.2.5 # via -r requirements/requirements.txt, -r requirements/tests.txt, prompt-toolkit webob==1.8.6 # via -r requirements/requirements.txt, -r requirements/tests.txt, pyramid, webtest webtest==2.0.35 # via -r requirements/tests.txt +wired==0.3 # via -r requirements/requirements.txt, -r requirements/tests.txt, pyramid-services wrapt==1.12.1 # via astroid zipp==3.4.0 # via -r requirements/requirements.txt, -r requirements/tests.txt, importlib-metadata, importlib-resources zope.deprecation==4.4.0 # via -r requirements/requirements.txt, -r requirements/tests.txt, pyramid, pyramid-jinja2 -zope.interface==5.2.0 # via -r requirements/requirements.txt, -r requirements/tests.txt, pyramid, transaction, zope.sqlalchemy +zope.interface==5.2.0 # via -r requirements/requirements.txt, -r requirements/tests.txt, pyramid, pyramid-services, transaction, wired, zope.sqlalchemy zope.sqlalchemy==1.3 # via -r requirements/requirements.txt, -r requirements/tests.txt # The following packages are considered to be unsafe in a requirements file: diff --git a/requirements/requirements.in b/requirements/requirements.in index aa67128f..a9eb18ae 100644 --- a/requirements/requirements.in +++ b/requirements/requirements.in @@ -5,6 +5,7 @@ psycopg2 pyramid pyramid_exclog pyramid-jinja2 +pyramid-services pyramid_tm netaddr newrelic diff --git a/requirements/requirements.txt b/requirements/requirements.txt index 9488d8e6..0e7cf6d0 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -32,8 +32,9 @@ prompt-toolkit==3.0.8 # via click-repl psycopg2==2.8.6 # via -r requirements/requirements.in pyramid-exclog==1.0 # via -r requirements/requirements.in pyramid-jinja2==2.8 # via -r requirements/requirements.in +pyramid-services==2.2 # via -r requirements/requirements.in pyramid-tm==2.4 # via -r requirements/requirements.in -pyramid==1.10.5 # via -r requirements/requirements.in, h-pyramid-sentry, pyramid-exclog, pyramid-jinja2, pyramid-tm +pyramid==1.10.5 # via -r requirements/requirements.in, h-pyramid-sentry, pyramid-exclog, pyramid-jinja2, pyramid-services, pyramid-tm python-dateutil==2.8.1 # via alembic python-editor==1.0.4 # via alembic pytz==2020.4 # via celery @@ -48,9 +49,10 @@ venusian==3.0.0 # via pyramid vine==5.0.0 # via amqp, celery wcwidth==0.2.5 # via prompt-toolkit webob==1.8.6 # via pyramid +wired==0.3 # via pyramid-services zipp==3.4.0 # via importlib-metadata, importlib-resources zope.deprecation==4.4.0 # via pyramid, pyramid-jinja2 -zope.interface==5.2.0 # via pyramid, transaction, zope.sqlalchemy +zope.interface==5.2.0 # via pyramid, pyramid-services, transaction, wired, zope.sqlalchemy zope.sqlalchemy==1.3 # via -r requirements/requirements.in # The following packages are considered to be unsafe in a requirements file: diff --git a/requirements/tests.txt b/requirements/tests.txt index 6de2db56..080541dc 100644 --- a/requirements/tests.txt +++ b/requirements/tests.txt @@ -44,8 +44,9 @@ py==1.9.0 # via pytest pyparsing==2.4.7 # via packaging pyramid-exclog==1.0 # via -r requirements/requirements.txt pyramid-jinja2==2.8 # via -r requirements/requirements.txt +pyramid-services==2.2 # via -r requirements/requirements.txt pyramid-tm==2.4 # via -r requirements/requirements.txt -pyramid==1.10.5 # via -r requirements/requirements.txt, h-pyramid-sentry, pyramid-exclog, pyramid-jinja2, pyramid-tm +pyramid==1.10.5 # via -r requirements/requirements.txt, h-pyramid-sentry, pyramid-exclog, pyramid-jinja2, pyramid-services, pyramid-tm pytest==6.1.2 # via -r requirements/tests.in python-dateutil==2.8.1 # via -r requirements/requirements.txt, alembic, faker python-editor==1.0.4 # via -r requirements/requirements.txt, alembic @@ -66,9 +67,10 @@ waitress==1.4.4 # via webtest wcwidth==0.2.5 # via -r requirements/requirements.txt, prompt-toolkit webob==1.8.6 # via -r requirements/requirements.txt, pyramid, webtest webtest==2.0.35 # via -r requirements/tests.in +wired==0.3 # via -r requirements/requirements.txt, pyramid-services zipp==3.4.0 # via -r requirements/requirements.txt, importlib-metadata, importlib-resources zope.deprecation==4.4.0 # via -r requirements/requirements.txt, pyramid, pyramid-jinja2 -zope.interface==5.2.0 # via -r requirements/requirements.txt, pyramid, transaction, zope.sqlalchemy +zope.interface==5.2.0 # via -r requirements/requirements.txt, pyramid, pyramid-services, transaction, wired, zope.sqlalchemy zope.sqlalchemy==1.3 # via -r requirements/requirements.txt # The following packages are considered to be unsafe in a requirements file: diff --git a/tests/unit/checkmate/services/__init__.py b/tests/unit/checkmate/services/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/unit/checkmate/services/secure_link_test.py b/tests/unit/checkmate/services/secure_link_test.py new file mode 100644 index 00000000..5beb7a34 --- /dev/null +++ b/tests/unit/checkmate/services/secure_link_test.py @@ -0,0 +1,109 @@ +from copy import deepcopy +from unittest.mock import create_autospec, sentinel + +import pytest +from pyramid.urldispatch import Route + +from checkmate.services.secure_link import SecureLinkService, factory + +# pylint: disable=protected-access, too-many-arguments + + +class TestSecureLinkService: + ROUTE_NAME = "my_view_name" + + def test_route_url_works(self, service, route, route_url, args, signed_args): + result = service.route_url(route.name, _query=args) + + assert result == route_url.return_value + + route_url.assert_called_once_with(self.ROUTE_NAME, _query=signed_args) + + def test_route_url_requires_a_query(self, service): + with pytest.raises(ValueError): + service.route_url("irrelevant_route_name", _query=None) + + def test_is_secure_allows_a_signed_value(self, service, signed_request): + assert service.is_secure(signed_request) + + @pytest.mark.parametrize("key", ("arg_1", "arg_2", "v", "sec")) + @pytest.mark.parametrize("value", (None, "different")) + def test_is_secure_detects_mutated_args(self, service, signed_request, key, value): + if value: + signed_request.GET[key] = value + else: + signed_request.GET.pop(key) + + before_call = deepcopy(dict(signed_request.GET)) + + assert not service.is_secure(signed_request) + + # Check we don't mutate the args + assert before_call == signed_request.GET + + def test_is_secure_detects_mismatched_route(self, service, signed_request): + signed_request.matched_route.name = "something_wrong" + + assert not service.is_secure(signed_request) + + def test_is_secure_checks_for_missing_version(self, service, signed_request): + signed_request.GET.pop(SecureLinkService.TOKEN_ARG) + signed_request.GET.pop(SecureLinkService.VERSION_ARG) + + # These args are correctly signed, but don't have the version + signed_request.GET[SecureLinkService.TOKEN_ARG] = service._hash_args( + signed_request.matched_route.name, signed_request.GET + ) + + assert not service.is_secure(signed_request) + + def test_is_secure_detects_additional_args(self, service, signed_request): + signed_request.GET["extra"] = "should not be here" + + assert not service.is_secure(signed_request) + + @pytest.fixture + def route(self): + return Route("my_view_name", "/some/url") + + @pytest.fixture + def args(self): + return {"arg_1": "some_value", "arg_2": "some_other_value"} + + @pytest.fixture + def signed_args(self, args): + signed_args = deepcopy(args) + signed_args.update( + { + "v": "1", + # This just happens to match the route name and args above + "sec": "1706de1486ff38efdea4089ca29a6ca5de3affa7ba919138a5b184365559829a", + } + ) + return signed_args + + @pytest.fixture + def signed_request(self, pyramid_request, signed_args, route): + pyramid_request.GET.update(signed_args) + pyramid_request.matched_route = route + + return pyramid_request + + @pytest.fixture + def route_url(self, pyramid_request): + return create_autospec(pyramid_request.route_url, spec_set=True) + + @pytest.fixture + def service(self, route_url): + return SecureLinkService(secret="not_very_secret", route_url=route_url) + + +class TestFactory: + def test_it(self, pyramid_request): + pyramid_request.registry.settings["secret"] = sentinel.secret + + service = factory(sentinel.context, pyramid_request) + + assert isinstance(service, SecureLinkService) + assert service._secret == sentinel.secret + assert service._route_url == pyramid_request.route_url diff --git a/tests/unit/checkmate/views/api/check_url_test.py b/tests/unit/checkmate/views/api/check_url_test.py index d738fe7b..a95e4747 100644 --- a/tests/unit/checkmate/views/api/check_url_test.py +++ b/tests/unit/checkmate/views/api/check_url_test.py @@ -5,6 +5,7 @@ from checkmate.views.api.check_url import check_url +@pytest.mark.usefixtures("secure_link_service") class TestURLCheck: @pytest.mark.parametrize("allow_all", ("1", None)) def test_a_good_url(self, make_request, allow_all, CompoundRules): @@ -22,13 +23,14 @@ def test_a_good_url(self, make_request, allow_all, CompoundRules): custom_rules = CompoundRules.return_value custom_rules.check_url.assert_called_once_with("http://happy.example.com") - def test_a_bad_url(self, make_request, CompoundRules): + def test_a_bad_url(self, make_request, CompoundRules, secure_link_service): CompoundRules.return_value.check_url.return_value = ( Reason.MALICIOUS, Reason.MEDIA_IMAGE, ) + bad_url = "http://sad.example.com" - request = make_request("/api/check", {"url": "http://sad.example.com"}) + request = make_request("/api/check", {"url": bad_url}) result = check_url(request) @@ -41,8 +43,13 @@ def test_a_bad_url(self, make_request, CompoundRules): "meta": { "maxSeverity": "mandatory", }, + "links": {"html": secure_link_service.route_url.return_value}, } + secure_link_service.route_url.assert_called_once_with( + "present_block", _query={"url": bad_url, "reason": Reason.MALICIOUS.value} + ) + def test_it_returns_an_error_for_no_url(self, make_request): request = make_request("/api/check") diff --git a/tests/unit/checkmate/views/ui/__init__.py b/tests/unit/checkmate/views/ui/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/unit/checkmate/views/ui/present_block_test.py b/tests/unit/checkmate/views/ui/present_block_test.py new file mode 100644 index 00000000..ed36e4b8 --- /dev/null +++ b/tests/unit/checkmate/views/ui/present_block_test.py @@ -0,0 +1,32 @@ +from unittest.mock import sentinel + +import pytest +from pyramid.httpexceptions import HTTPForbidden + +from checkmate.views.ui.present_block import present_block + + +@pytest.mark.usefixtures("secure_link_service") +class TestPresentBlock: + def test_it_passes_through_params(self, make_request, params): + result = present_block(sentinel.context, make_request(params=params)) + + assert result == {"blocked_url": params["url"], "reason": params["reason"]} + + def test_it_checks_param_signing(self, make_request, secure_link_service, params): + secure_link_service.is_secure.return_value = False + request = make_request(params=params) + + with pytest.raises(HTTPForbidden): + present_block(sentinel.context, request) + + secure_link_service.is_secure.assert_called_once_with(request) + + @pytest.fixture + def params(self): + return { + "url": "http://bad.example.com", + "reason": "malicious", + "v": "1", + "sec": "some-long-token", + } diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 2b5c3734..fbbef164 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -2,18 +2,17 @@ """A place to put fixture functions that are useful application-wide.""" import functools import os -from unittest import mock from unittest.mock import MagicMock from urllib.parse import urlencode import httpretty -import pytest from pyramid import testing -from pyramid.request import Request +from pyramid.request import Request, apply_request_extensions from sqlalchemy.orm import sessionmaker from checkmate.db import create_engine from tests import factories +from tests.unit.services import * # pylint: disable=wildcard-import,unused-wildcard-import def autopatcher(request, target, **kwargs): @@ -37,13 +36,15 @@ def pyramid_settings(): "database_url": os.environ.get( "TEST_DATABASE_URL", "postgresql://postgres@localhost:5434/checkmate_test", - ) + ), + "secret": os.environ.get("CHECKMATE_SECRET", "not-very-secret"), } @pytest.fixture def pyramid_config(pyramid_settings): with testing.testConfig(settings=pyramid_settings) as config: + config.include("pyramid_services") yield config @@ -69,6 +70,8 @@ def _make_request(path, pyramid_config, db_session): pyramid_request.tm = MagicMock() pyramid_request.db = db_session + apply_request_extensions(pyramid_request) + return pyramid_request diff --git a/tests/unit/services.py b/tests/unit/services.py new file mode 100644 index 00000000..1d3de830 --- /dev/null +++ b/tests/unit/services.py @@ -0,0 +1,21 @@ +from unittest import mock + +import pytest + +from checkmate.services.secure_link import SecureLinkService + + +@pytest.fixture +def mock_service(pyramid_config): + def mock_service(service_class): + mock_service = mock.create_autospec(service_class, spec_set=True, instance=True) + pyramid_config.register_service(mock_service, iface=service_class) + + return mock_service + + return mock_service + + +@pytest.fixture +def secure_link_service(mock_service): + return mock_service(SecureLinkService) diff --git a/tox.ini b/tox.ini index f0004827..572a077e 100644 --- a/tox.ini +++ b/tox.ini @@ -29,6 +29,7 @@ setenv = dev: CELERY_BROKER_URL={env:CELERY_BROKER_URL:amqp://guest:guest@localhost:5673//} dev: DATABASE_URL={env:DATABASE_URL:postgresql://postgres@localhost:5434/postgres} dev: CHECKMATE_BLOCKLIST_URL = {env:CHECKMATE_BLOCKLIST_URL:http://dummy.example.com/blocklist} + dev: CHECKMATE_SECRET = not_a_secret tests: TEST_DATABASE_URL = {env:TEST_DATABASE_URL:postgresql://postgres@localhost:5434/checkmate_test} OBJC_DISABLE_INITIALIZE_FORK_SAFETY = YES