Revert "Revert "Block access to Via in the web app (#1739)" (#1742)" (#1745)

This reverts commit 5a39924.

Author: santicomp2014

tests/functional/api/view_pdf_test.py

@@ -1,13 +1,8 @@
-import pytest
-
-from tests.conftest import assert_cache_control
-
-
 class TestViewPDFAPI:
-    @pytest.mark.usefixtures("checkmate_pass")
-    def test_caching_is_disabled(self, test_app):
+    def test_pdf_shows_restricted_page(self, test_app):
         response = test_app.get("/pdf?url=http://example.com/foo.pdf")
 
-        assert_cache_control(
-            response.headers, ["max-age=0", "must-revalidate", "no-cache", "no-store"]
-        )
+        assert response.status_code == 200
+        assert "Access to Via is now" in response.text
+        assert "restricted" in response.text
+        assert "http://example.com/foo.pdf" in response.text

tests/functional/api/views/route_by_content_test.py

@@ -1,62 +1,10 @@
-from urllib.parse import quote_plus
-
-import httpretty
-import pytest
-from h_matchers import Any
-
-from tests.conftest import assert_cache_control
-
-
 class TestRouteByContent:
-    DEFAULT_OPTIONS = {  # noqa: RUF012
-        "via.client.ignoreOtherConfiguration": "1",
-        "via.client.openSidebar": "1",
-        "via.external_link_mode": "new-tab",
-    }
-
-    @pytest.mark.usefixtures("html_response", "checkmate_pass")
-    def test_proxy_html(self, test_app):
-        target_url = "http://example.com"
-
-        response = test_app.get(f"/route?url={target_url}")
-
-        assert response.status_code == 302
-        query = dict(self.DEFAULT_OPTIONS)
-        assert response.location == Any.url.matching(
-            f"https://viahtml.hypothes.is/proxy/{target_url}/"
-        ).with_query(query)
-
-    @pytest.mark.usefixtures("pdf_response", "checkmate_pass")
-    def test_proxy_pdf(self, test_app):
+    def test_route_shows_restricted_page(self, test_app):
         target_url = "http://example.com"
 
         response = test_app.get(f"/route?url={target_url}")
 
-        assert response.status_code == 302
-        query = dict(self.DEFAULT_OPTIONS)
-        query["via.sec"] = Any.string()
-        query["url"] = target_url
-        assert response.location == Any.url.matching(
-            f"http://localhost/pdf?url={quote_plus(target_url)}"
-        ).with_query(query)
-        assert_cache_control(
-            response.headers, ["public", "max-age=300", "stale-while-revalidate=86400"]
-        )
-
-    @pytest.fixture
-    def html_response(self):
-        httpretty.register_uri(
-            httpretty.GET,
-            "http://example.com",
-            status=204,
-            adding_headers={"Content-Type": "text/html"},
-        )
-
-    @pytest.fixture
-    def pdf_response(self):
-        httpretty.register_uri(
-            httpretty.GET,
-            "http://example.com",
-            status=204,
-            adding_headers={"Content-Type": "application/pdf"},
-        )
+        assert response.status_code == 200
+        assert "Access to Via is now" in response.text
+        assert "restricted" in response.text
+        assert target_url in response.text

tests/functional/static_content_test.py

@@ -2,10 +2,12 @@
 
 import re
 
+import importlib_resources
 import pytest
 from h_matchers import Any
 
 from tests.conftest import assert_cache_control
+from via.cache_buster import PathCacheBuster
 
 
 class TestStaticContent:
@@ -38,12 +40,18 @@ def test_immutable_contents(self, test_app):
     def get_salt(self, test_app):
         """Get the salt value being used by the app.
 
-        The most sure fire way to get the exact salt value being used is to
-        actually make a call with immutable assets and then scrape the HTML
-        for the salt value.
+        We use the proxy route to scrape for the salt since the PDF viewer
+        is now restricted. Any page that includes static assets will work.
         """
-        response = test_app.get("/pdf?url=http://example.com")
+        # Use a URL that will hit the proxy route and return the restricted page
+        response = test_app.get("/https://example.com")
         static_match = re.search("/static/([^/]+)/", response.text)
-        assert static_match
+        if not static_match:
+            # The restricted template doesn't reference /static/ paths,
+            # so read the salt directly from stdout capture during app startup.
+            # Fall back to getting it from the cache buster.
+            static_path = str(importlib_resources.files("via") / "static")
+            cache_buster = PathCacheBuster(static_path)
+            return cache_buster.salt
 
         return static_match.group(1)

tests/functional/via/views/index_test.py

@@ -1,35 +1,8 @@
-import pytest
-
-from tests.functional.matchers import temporary_redirect_to
-
-
-@pytest.mark.parametrize(
-    "url,expected_redirect_location",  # noqa: PT006
-    [
-        # When you submit the form on the front page it redirects you to the
-        # page that will proxy the URL that you entered.
-        (
-            "https://example.com/foo/",
-            "http://localhost/https://example.com/foo/",
-        ),
-        (
-            "http://example.com/foo/",
-            "http://localhost/http://example.com/foo/",
-        ),
-        # The submitted URL is normalized to strip leading/trailing spaces and
-        # add a protocol.
-        (
-            "example.com/foo/",
-            "http://localhost/https://example.com/foo/",
-        ),
-        # If you submit an empty form it just reloads the front page again.
-        ("", "http://localhost/"),
-    ],
-)
-def test_index(test_app, url, expected_redirect_location):
-    form = test_app.get("/").form
-
-    form.set("url", url)
-    response = form.submit()
-
-    assert response == temporary_redirect_to(expected_redirect_location)
+def test_index_shows_page_when_not_signed_urls_required(test_app):
+    """When signed_urls_required is False (default test config), index page is served."""
+    response = test_app.get("/")
+
+    assert response.status_code == 200
+    # The normal index page is shown (not restricted) because
+    # signed_urls_required=False means all requests are considered valid.
+    assert "hypothes.is" in response.text

tests/unit/via/services/secure_link_test.py

@@ -50,6 +50,25 @@ def test_request_is_valid_if_signatures_not_required(
 
         service._via_secure_url.verify.assert_not_called()  # noqa: SLF001
 
+    def test_request_has_valid_token(self, service, pyramid_request):
+        assert service.request_has_valid_token(pyramid_request)
+
+    def test_request_has_valid_token_can_fail(self, service, pyramid_request):
+        service._via_secure_url.verify.side_effect = TokenException  # noqa: SLF001
+
+        assert not service.request_has_valid_token(pyramid_request)
+
+    @pytest.mark.usefixtures("with_signed_urls_not_required")
+    def test_request_has_valid_token_always_checks_signature(
+        self, service, pyramid_request
+    ):
+        """request_has_valid_token always verifies the token, even when signed URLs aren't required."""
+        service._via_secure_url.verify.side_effect = TokenException  # noqa: SLF001
+
+        assert not service.request_has_valid_token(pyramid_request)
+
+        service._via_secure_url.verify.assert_called_once_with(pyramid_request.url)  # noqa: SLF001
+
     def test_sign_url(self, service):
         result = service.sign_url(sentinel.url)
 

tests/unit/via/views/index_test.py

@@ -1,60 +1,102 @@
+from unittest.mock import create_autospec
+
 import pytest
-from pyramid.httpexceptions import HTTPFound, HTTPNotFound
+from pyramid.httpexceptions import HTTPBadRequest, HTTPFound, HTTPNotFound
 
 from tests.unit.matchers import temporary_redirect_to
 from via.resources import QueryURLResource
-from via.views.exceptions import BadURL
 from via.views.index import IndexViews
 
 
-class TestIndexViews:
-    def test_get(self, views):
-        assert views.get() == {}
-
-    def test_post(self, views, pyramid_request):
-        pyramid_request.params["url"] = "//site.org?q1=value1&q2=value2"
+class TestIndexGet:
+    def test_it_returns_restricted_page_when_not_lms(
+        self, context, pyramid_request, secure_link_service
+    ):
+        secure_link_service.request_has_valid_token.return_value = False
+        views = IndexViews(context, pyramid_request)
 
-        redirect = views.post()
+        result = views.get()
 
-        assert isinstance(redirect, HTTPFound)
+        assert result == {"target_url": None}
         assert (
-            redirect.location
-            == "http://example.com/https://site.org?q1=value1&q2=value2"
+            pyramid_request.override_renderer == "via:templates/restricted.html.jinja2"
         )
 
-    def test_post_with_no_url(self, views, pyramid_request):
-        assert "url" not in pyramid_request.params
+    def test_it_returns_page_when_lms(
+        self, context, pyramid_request, secure_link_service
+    ):
+        secure_link_service.request_has_valid_token.return_value = True
+        views = IndexViews(context, pyramid_request)
 
-        redirect = views.post()
+        result = views.get()
 
-        assert redirect == temporary_redirect_to(
-            pyramid_request.route_url(route_name="index")
-        )
+        assert result == {}
 
-    def test_post_raises_if_url_invalid(self, views, pyramid_request):
-        # Set a `url` that causes `urlparse` to throw.
-        pyramid_request.params["url"] = "http://::12.34.56.78]/"
+    def test_it_returns_not_found_when_front_page_disabled(
+        self, context, pyramid_request, secure_link_service
+    ):
+        secure_link_service.request_has_valid_token.return_value = True
+        pyramid_request.registry.settings["enable_front_page"] = False
+        views = IndexViews(context, pyramid_request)
 
-        with pytest.raises(BadURL):
-            views.post()
+        result = views.get()
 
-    @pytest.mark.usefixtures("disable_front_page")
-    @pytest.mark.parametrize("view", ["get", "post"])
-    def test_it_404s_if_the_front_page_isnt_enabled(self, view, views):
-        view = getattr(views, view)
+        assert isinstance(result, HTTPNotFound)
 
-        response = view()
+    @pytest.fixture
+    def context(self):
+        return create_autospec(QueryURLResource, spec_set=True, instance=True)
 
-        assert isinstance(response, HTTPNotFound)
 
-    @pytest.fixture
-    def disable_front_page(self, pyramid_settings):
-        pyramid_settings["enable_front_page"] = False
+class TestIndexPost:
+    def test_it_returns_restricted_page_when_not_lms(
+        self, context, pyramid_request, secure_link_service
+    ):
+        secure_link_service.request_has_valid_token.return_value = False
+        views = IndexViews(context, pyramid_request)
 
-    @pytest.fixture
-    def views(self, context, pyramid_request):
-        return IndexViews(context, pyramid_request)
+        result = views.post()
+
+        assert result == {"target_url": None}
+
+    def test_it_returns_not_found_when_front_page_disabled(
+        self, context, pyramid_request, secure_link_service
+    ):
+        secure_link_service.request_has_valid_token.return_value = True
+        pyramid_request.registry.settings["enable_front_page"] = False
+        views = IndexViews(context, pyramid_request)
+
+        result = views.post()
+
+        assert isinstance(result, HTTPNotFound)
+
+    def test_it_redirects_when_lms(self, context, pyramid_request, secure_link_service):
+        secure_link_service.request_has_valid_token.return_value = True
+        context.url_from_query.return_value = "http://example.com/page?q=1"
+        views = IndexViews(context, pyramid_request)
+
+        result = views.post()
+
+        assert isinstance(result, HTTPFound)
+        assert result == temporary_redirect_to(
+            pyramid_request.route_url(
+                route_name="proxy",
+                url="http://example.com/page",
+                _query="q=1",
+            )
+        )
+
+    def test_it_redirects_to_index_on_bad_url(
+        self, context, pyramid_request, secure_link_service
+    ):
+        secure_link_service.request_has_valid_token.return_value = True
+        context.url_from_query.side_effect = HTTPBadRequest("bad url")
+        views = IndexViews(context, pyramid_request)
+
+        result = views.post()
+
+        assert isinstance(result, HTTPFound)
 
     @pytest.fixture
-    def context(self, pyramid_request):
-        return QueryURLResource(pyramid_request)
+    def context(self):
+        return create_autospec(QueryURLResource, spec_set=True, instance=True)

tests/unit/via/views/proxy_test.py

@@ -14,22 +14,47 @@ def test_it(self):
 
 
 class TestProxy:
-    def test_it(
-        self, context, pyramid_request, url_details_service, via_client_service
+    def test_it_returns_restricted_page_when_not_lms(
+        self, context, pyramid_request, secure_link_service
     ):
-        url_details_service.get_url_details.return_value = (
-            sentinel.mime_type,
-            sentinel.status_code,
-        )
+        secure_link_service.request_has_valid_token.return_value = False
         url = context.url_from_path.return_value = "/https://example.org?a=1"
 
         result = proxy(context, pyramid_request)
 
-        url_details_service.get_url_details.assert_called_once_with(url)
-        via_client_service.url_for.assert_called_once_with(
-            url, sentinel.mime_type, pyramid_request.params
+        assert result == {"target_url": url}
+        assert (
+            pyramid_request.override_renderer == "via:templates/restricted.html.jinja2"
+        )
+
+    def test_it_returns_restricted_none_url_on_error_when_not_lms(
+        self, context, pyramid_request, secure_link_service
+    ):
+        secure_link_service.request_has_valid_token.return_value = False
+        context.url_from_path.side_effect = Exception("bad url")
+
+        result = proxy(context, pyramid_request)
+
+        assert result == {"target_url": None}
+
+    def test_it_proxies_when_lms(
+        self,
+        context,
+        pyramid_request,
+        secure_link_service,
+        url_details_service,
+        via_client_service,  # noqa: ARG002
+    ):
+        secure_link_service.request_has_valid_token.return_value = True
+        context.url_from_path.return_value = "http://example.com/page"
+        url_details_service.get_url_details.return_value = ("text/html", 200)
+
+        result = proxy(context, pyramid_request)
+
+        url_details_service.get_url_details.assert_called_once_with(
+            "http://example.com/page"
         )
-        assert result == {"src": via_client_service.url_for.return_value}
+        assert "src" in result
 
     @pytest.fixture
     def context(self):