Merge commit '8cfa2ee048145635ded8d55dc157d51fb45dc02c' into design-audit-phase1

Author: alexeygrigorev

core/middleware.py

@@ -69,6 +69,17 @@
     }
 )
 _BODY_METHODS = frozenset({"POST", "PUT", "PATCH", "DELETE"})
+_CREDENTIAL_HEADER_NAMES = frozenset(
+    {
+        "Authorization",
+        "X-CSRFToken",
+        "X-Preview-Token",
+        "X-Management-Token",
+    }
+)
+# These cookies are documented, non-credential preferences.  Every other
+# viewer cookie is treated as unknown credential-like state and is private.
+_ANONYMOUS_COOKIE_NAMES = frozenset({"browser_timezone", "dtc_analytics_consent"})
 
 
 def apply_security_headers(response: HttpResponse) -> None:
@@ -264,16 +275,23 @@ def __call__(self, request: HttpRequest) -> HttpResponse:
             reset_context(tokens)
 
 
-def _is_authenticated_request(request: HttpRequest) -> bool:
+def _is_credential_bearing_request(request: HttpRequest) -> bool:
     user = getattr(request, "user", None)
     if bool(user is not None and user.is_authenticated):
         return True
     # Fail closed when an earlier middleware short-circuits before Django can
-    # resolve the principal (notably SecurityMiddleware and WhiteNoise).
-    if request.headers.get("Authorization"):
+    # resolve the principal (notably SecurityMiddleware and WhiteNoise), and
+    # for viewer-supplied credentials that do not establish a Django user.
+    if any(request.headers.get(name) for name in _CREDENTIAL_HEADER_NAMES):
         return True
     session_cookie_name = getattr(settings, "SESSION_COOKIE_NAME", "sessionid")
-    return session_cookie_name in request.COOKIES
+    csrf_cookie_name = getattr(settings, "CSRF_COOKIE_NAME", "csrftoken")
+    if session_cookie_name in request.COOKIES or csrf_cookie_name in request.COOKIES:
+        return True
+    # Only documented non-credential preferences are anonymous-safe.  Any
+    # other cookie is unknown credential-like state and must not share a
+    # response.
+    return any(name not in _ANONYMOUS_COOKIE_NAMES for name in request.COOKIES)
 
 
 def _is_private_surface(request: HttpRequest) -> bool:
@@ -340,6 +358,6 @@ def __call__(self, request: HttpRequest) -> HttpResponse:
         if settings.NOINDEX or private_surface:
             # Assignment replaces a downstream value instead of appending a second field.
             response["X-Robots-Tag"] = ROBOTS_HEADER_VALUE
-        if _is_authenticated_request(request) or private_surface:
+        if _is_credential_bearing_request(request) or private_surface:
             apply_private_no_store(response)
         return response

core/tests/test_development_seo.py

@@ -13,7 +13,7 @@
 from core.middleware import apply_private_no_store
 from core.preview import SENSITIVE_PREVIEW_QUERY_KEYS, staff_preview_required
 from core.seo import validated_canonical_url
-from core.views import DEVELOPMENT_ROBOTS_BODY
+from core.views import DEVELOPMENT_ROBOTS_BODY, PRODUCTION_ROBOTS_BODY
 from courses.models import Course
 
 FIXTURE_URLCONF = "core.tests.seo_fixture_urls"
@@ -122,6 +122,11 @@ def test_credential_bearing_early_response_fails_closed(self) -> None:
         for headers in (
             {"authorization": "Bearer opaque-input"},
             {"cookie": "sessionid=opaque-session"},
+            {"cookie": "csrftoken=opaque-csrf"},
+            {"cookie": "opaque_credential=opaque-value"},
+            {"x-csrftoken": "opaque-csrf-header"},
+            {"x-preview-token": "opaque-preview"},
+            {"x-management-token": "opaque-management"},
         ):
             with self.subTest(headers=headers):
                 response = self.client.get("/missing", headers=headers)
@@ -164,10 +169,90 @@ def test_sitemap_get_and_head_expose_the_checked_section_index(self) -> None:
         self.assertEqual(head.content, b"")
 
     @override_settings(NOINDEX=False)
-    def test_production_exposes_only_the_public_sitemap(self) -> None:
+    def test_production_robots_contract_and_public_sitemap(self) -> None:
         robots = self.client.get("/robots.txt")
-        self.assertEqual(robots.status_code, 404)
+        self.assertEqual(robots.status_code, 200)
+        self.assertEqual(robots.content, PRODUCTION_ROBOTS_BODY.encode())
+        self.assertEqual(robots.headers["Content-Type"], "text/plain; charset=utf-8")
+        self.assertEqual(robots.headers["Cache-Control"], "max-age=0, must-revalidate")
         self.assertNotIn("X-Robots-Tag", robots.headers)
+        self.assertNotIn("/podwiki/", robots.content.decode())
+        self.assertNotIn("web.dtcdev.click", robots.content.decode())
+
+        head = self.client.head("/robots.txt")
+        self.assertEqual(head.status_code, 200)
+        self.assertEqual(head.content, b"")
+        self.assertEqual(head.headers["Content-Type"], robots.headers["Content-Type"])
+        self.assertEqual(head.headers["Cache-Control"], robots.headers["Cache-Control"])
+        self.assertNotIn("X-Robots-Tag", head.headers)
+
+        for method in ("POST", "PUT", "PATCH", "DELETE", "OPTIONS"):
+            with self.subTest(method=method):
+                response = self.client.generic(method, "/robots.txt", data=b"opaque-input")
+                self.assertEqual(response.status_code, 405)
+                self.assertEqual(response.headers["Allow"], "GET, HEAD")
+                self.assertEqual(response.headers["Cache-Control"], "no-store, max-age=0")
+                self.assertNotIn("public", cache_directives(response))
+                self.assertNotIn("s-maxage=3600", cache_directives(response))
+
+        credential_responses = (
+            (
+                "authorization",
+                self.client.get("/robots.txt", HTTP_AUTHORIZATION="Bearer opaque-input"),
+                self.client.head("/robots.txt", HTTP_AUTHORIZATION="Bearer opaque-input"),
+            ),
+            (
+                "session-cookie",
+                self.client.get("/robots.txt", HTTP_COOKIE="sessionid=opaque-session"),
+                self.client.head("/robots.txt", HTTP_COOKIE="sessionid=opaque-session"),
+            ),
+            (
+                "csrf-cookie",
+                self.client.get("/robots.txt", HTTP_COOKIE="csrftoken=opaque-csrf"),
+                self.client.head("/robots.txt", HTTP_COOKIE="csrftoken=opaque-csrf"),
+            ),
+            (
+                "csrf-token-header",
+                self.client.get("/robots.txt", HTTP_X_CSRFTOKEN="opaque-csrf-header"),
+                self.client.head("/robots.txt", HTTP_X_CSRFTOKEN="opaque-csrf-header"),
+            ),
+            (
+                "unknown-cookie",
+                self.client.get("/robots.txt", HTTP_COOKIE="opaque_credential=opaque-value"),
+                self.client.head("/robots.txt", HTTP_COOKIE="opaque_credential=opaque-value"),
+            ),
+            (
+                "preview-token-header",
+                self.client.get("/robots.txt", HTTP_X_PREVIEW_TOKEN="opaque-preview"),
+                self.client.head("/robots.txt", HTTP_X_PREVIEW_TOKEN="opaque-preview"),
+            ),
+            (
+                "management-token-header",
+                self.client.get("/robots.txt", HTTP_X_MANAGEMENT_TOKEN="opaque-management"),
+                self.client.head("/robots.txt", HTTP_X_MANAGEMENT_TOKEN="opaque-management"),
+            ),
+        )
+        for credential_kind, get_response, head_response in credential_responses:
+            for method, response in (("GET", get_response), ("HEAD", head_response)):
+                with self.subTest(credential_kind=credential_kind, method=method):
+                    directives = cache_directives(response)
+                    self.assertTrue({"private", "no-store", "max-age=0"}.issubset(directives))
+                    self.assertNotIn("public", directives)
+                    self.assertFalse(
+                        any(
+                            item.startswith("s-maxage=") and item != "s-maxage=0"
+                            for item in directives
+                        )
+                    )
+
+        for cookie in (
+            "dtc_analytics_consent=v1.allow",
+            "browser_timezone=Europe%2FBerlin",
+        ):
+            with self.subTest(cookie=cookie):
+                preference = self.client.get("/robots.txt", HTTP_COOKIE=cookie)
+                self.assertEqual(cache_directives(preference), {"max-age=0", "must-revalidate"})
+                self.assertNotIn("private", preference.headers["Cache-Control"])
 
         sitemap = self.client.get("/sitemap.xml")
         self.assertEqual(sitemap.status_code, 200)

core/views.py

@@ -18,6 +18,19 @@
 from content.review_projection import review_projection
 
 DEVELOPMENT_ROBOTS_BODY = "User-agent: *\nDisallow: /\n"
+PRODUCTION_ROBOTS_BODY = (
+    "User-agent: *\n"
+    "Disallow: /admin/\n"
+    "Disallow: /_site/\n"
+    "Disallow: /drafts/\n"
+    "Disallow: /config/\n"
+    "Disallow: /scripts/\n"
+    "Disallow: /styles/\n"
+    "Sitemap: https://datatalks.club/sitemap.xml\n"
+    "Sitemap: https://datatalks.club/sitemaps/wiki.xml\n"
+)
+ROBOTS_CONTENT_TYPE = "text/plain; charset=utf-8"
+ROBOTS_ALLOWED_METHODS = ("GET", "HEAD")
 
 
 def management_slash_redirect(request: HttpRequest) -> HttpResponse:
@@ -59,13 +72,16 @@ def _development_seo_response(body: str, content_type: str) -> HttpResponse:
     return HttpResponse(body, content_type=content_type)
 
 
-@require_safe
 def robots(request: HttpRequest) -> HttpResponse:
-    del request
-    return _development_seo_response(
-        DEVELOPMENT_ROBOTS_BODY,
-        "text/plain; charset=utf-8",
-    )
+    if request.method not in ROBOTS_ALLOWED_METHODS:
+        not_allowed_response = HttpResponseNotAllowed(ROBOTS_ALLOWED_METHODS)
+        not_allowed_response["Cache-Control"] = "no-store, max-age=0"
+        return not_allowed_response
+    if settings.NOINDEX:
+        return _development_seo_response(DEVELOPMENT_ROBOTS_BODY, ROBOTS_CONTENT_TYPE)
+    response = HttpResponse(PRODUCTION_ROBOTS_BODY, content_type=ROBOTS_CONTENT_TYPE)
+    response["Cache-Control"] = "max-age=0, must-revalidate"
+    return response
 
 
 @require_safe