From 67bc5a8de0c84263ed73fb3eab0e224d850c5867 Mon Sep 17 00:00:00 2001 From: "David A. Wheeler" Date: Thu, 25 Jun 2026 23:23:02 -0400 Subject: [PATCH] Fix GitHub login broken by conditional CSRF tag We're trying to dramatically increase what the CDN caches. We're now fixing a problem we found while trying to do this. The CDN-caching change (commit 9f8e5ae2) made csrf_meta_tags conditional on logged_in?, on the documented assumption that the only rails-ujs "method:" links live on logged-in pages. However, that assumption was wrong: the login page's "Log in with GitHub" button is a `link_to ... method: 'post'`, and rails-ujs reads the CSRF token from the meta tag. With the tag gone for anonymous users, the POST to /auth/github carried no token, OmniAuth rejected it with ActionController::InvalidAuthenticityToken, and login 404'd via /auth/failure. This commit fixes the problem via the plan's own Section 9.1 opt-in escape hatch: * Gate the layout tag on `logged_in? || content_for?(:needs_csrf_meta)`. * Have the login page (sessions/new) opt in. This is scoped to the login *page*, not the header's "Login" link (a plain GET link_to), so every other anonymous page stays cookie-free and CDN-cacheable; the never-cached login page is the sole, documented exception. Also correct a subtle gotcha in the documented snippet: the block form `content_for(:needs_csrf_meta) { true }` does NOT work -- content_for captures the block's rendered output, and a block returning bare `true` outputs nothing, so content_for? stays false. The working form is the non-block `content_for :needs_csrf_meta, 'true'`. Fixed in code and docs. Add a regression test that reproduces the production failure: it asserts the anonymous login page emits the CSRF meta tag, run WITH forgery protection enabled (it is disabled by default in the test env, which is why the original change shipped without catching this). Also add script/verify_cdn_caching.sh, the staging/production verification harness used throughout this work (anonymous cacheable + no cookie, session and remember-me cookies bypass the cache, JSON still caches; --verbose dumps full headers). It exits non-zero on any failure, doubling as the periodic synthetic monitor recommended in Section 9.4. Co-Authored-By: Claude Opus 4.8 Signed-off-by: David A. Wheeler --- app/views/layouts/application.html.erb | 16 +- app/views/sessions/new.html.erb | 10 ++ docs/cdn-cache-not-logged-in.md | 132 +++++++++++++-- script/verify_cdn_caching.sh | 216 +++++++++++++++++++++++++ test/integration/cdn_caching_test.rb | 17 ++ 5 files changed, 372 insertions(+), 19 deletions(-) create mode 100755 script/verify_cdn_caching.sh diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 952f6f3a2..32352c0a0 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -8,13 +8,17 @@ Only emit the CSRF token when it can actually be used. Emitting it calls form_authenticity_token, which writes session[:_csrf_token] and forces a _BadgeApp_session cookie -- defeating CDN caching for anonymous visitors. - Anonymous read-only pages need no token: form_with embeds its own hidden - authenticity_token (so login/signup/reset forms still work), and the only - JavaScript consumer of this meta tag (jQuery UJS "method:" links such as - logout and user-delete) appears only on logged-in pages. See - docs/cdn-cache-not-logged-in.md. + Most anonymous read-only pages need no token: form_with embeds its own + hidden authenticity_token (so login/signup/reset forms still work). + The exception is an anonymous page with a rails-ujs "method:" link, which + reads the token from this meta tag -- e.g. the login page's "Log in with + GitHub" button (a link_to ... method: 'post'). Such a page must opt in + explicitly with `content_for :needs_csrf_meta, 'true'` (the non-block form; + a block returning bare `true` outputs nothing, so content_for? stays + false), keeping the safe, cacheable default (no token) for other pages. See + docs/cdn-cache-not-logged-in.md (Change 1 and Section 9.1). -%> - <% if logged_in? %><%= csrf_meta_tags %><% end %> + <% if logged_in? || content_for?(:needs_csrf_meta) %><%= csrf_meta_tags %><% end %> <%= yield :special_head_values %> <%# The following are the same for a given locale, cache for speed -%> <% cache_frozen locale do # do not cache csrf_meta_tags -%> diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb index cd023b139..0b4346a87 100644 --- a/app/views/sessions/new.html.erb +++ b/app/views/sessions/new.html.erb @@ -1,3 +1,13 @@ +<%# The "Log in with GitHub" button below is a rails-ujs link_to ... method: + 'post'; rails-ujs reads the CSRF token from the + tag, which the layout omits for anonymous users (CDN caching, Change 1). + Opt back in so the token is present and the POST to /auth/github passes + CSRF. This page is never CDN-cached, so emitting the tag here is harmless. + See docs/cdn-cache-not-logged-in.md Section 9.1. %> +<%# Use the non-block form: a content_for block captures the block's *output*, + and a block returning a bare `true` outputs nothing, so content_for? would + stay false. A non-empty string value makes content_for? true. %> +<% content_for :needs_csrf_meta, 'true' %> <% content_for :special_head_values do %> <%# same-origin (must NOT be no-referrer): Cross-origin requests (e.g. the OmniAuth redirect to github.com) carry no diff --git a/docs/cdn-cache-not-logged-in.md b/docs/cdn-cache-not-logged-in.md index 66f979d63..d6ac1c4a9 100644 --- a/docs/cdn-cache-not-logged-in.md +++ b/docs/cdn-cache-not-logged-in.md @@ -354,18 +354,50 @@ with a guarded version: Only emit the CSRF token when it can actually be used. Emitting it calls form_authenticity_token, which writes session[:_csrf_token] and forces a _BadgeApp_session cookie -- defeating CDN caching for anonymous visitors. - Anonymous read-only pages need no token: form_with embeds its own hidden - authenticity_token (so login/signup/reset forms still work), and the only - JavaScript consumer of this meta tag (jQuery UJS "method:" links such as - logout and user-delete) appears only on logged-in pages. See - docs/cdn-cache-not-logged-in.md. + Most anonymous read-only pages need no token: form_with embeds its own + hidden authenticity_token (so login/signup/reset forms still work). + The exception is an anonymous page with a rails-ujs "method:" link, which + reads the token from this meta tag -- e.g. the login page's "Log in with + GitHub" button (a link_to ... method: 'post'). Such a page must opt in + explicitly with `content_for :needs_csrf_meta, 'true'` (the non-block form; + a block returning bare `true` outputs nothing, so content_for? stays + false), keeping the safe, cacheable default (no token) for other pages. See + docs/cdn-cache-not-logged-in.md (Change 1 and Section 9.1). -%> - <% if logged_in? %><%= csrf_meta_tags %><% end %> + <% if logged_in? || content_for?(:needs_csrf_meta) %><%= csrf_meta_tags %><% end %> ``` `logged_in?` is the existing `SessionsHelper` predicate (`@session_user_id.present?`) and is available in views. +> **The login page is a real anonymous UJS consumer — do not skip the opt-in.** +> An earlier draft of this plan claimed UJS `method:` links "appear only on +> logged-in pages." That is **wrong**: `app/views/sessions/new.html.erb` +> renders the "Log in with GitHub" button as `link_to … method: 'post'`, and +> rails-ujs reads the token from this meta tag. Without the opt-in, the +> anonymous login page has no meta tag, the generated POST to `/auth/github` +> carries no token, and OmniAuth rejects it +> (`ActionController::InvalidAuthenticityToken` → `/auth/failure` → 404). The +> login page therefore sets `content_for :needs_csrf_meta, 'true'` (see the +> per-page opt-in note below). The header's "Login"/"Sign up" entries are +> ordinary GET `link_to`s, so they do **not** trigger this and every other +> anonymous page stays cookie-free and cacheable. A regression test +> (`test/integration/cdn_caching_test.rb`, "anonymous login page emits the CSRF +> meta tag for GitHub OAuth") locks this in **with forgery protection enabled** +> — without that flag the bug is invisible in the test environment (Section 6). + +**Per-page opt-in — use the non-block `content_for`.** On a page that needs the +token, set the flag at the top of its template: + +```erb +<% content_for :needs_csrf_meta, 'true' %> +``` + +Use the **non-block** form with a non-empty string value. The block form +`content_for(:needs_csrf_meta) { true }` does **not** work: `content_for` +captures the block's rendered *output*, and a block returning a bare `true` +outputs nothing, so `content_for?` stays false and the tag is never emitted. + ### Change 2: Cache anonymous `projects#show` HTML In [`app/controllers/projects_controller.rb`](../app/controllers/projects_controller.rb), @@ -464,6 +496,14 @@ cookies. #### Option A: Fastly Web UI Request Setting (recommended) +This is the configuration actually used in production. Because it lives in the +Fastly service (not in this repository), it is **not** under the project's +normal version control — so the exact steps are recorded here. It is a +load-bearing security control (Section 9.4): without it, a logged-in or +remember-me user can be served a cached anonymous page. + +The setting in summary: + * **Name:** `Bypass cache for personalized requests` * **Action:** `Pass` * **Condition** — Apply if: @@ -472,6 +512,55 @@ cookies. req.http.Cookie ~ "(_BadgeApp_session|remember_token|user_id)=" && req.url.path !~ "\.(css|js|png|gif|jpg|jpeg|svg|json|csv|txt|ico|woff2?|map)$" && req.url.path !~ "/(badge|baseline)$" ``` +**Step-by-step in the Fastly web UI:** + +1. **Identify the service.** The Fastly service is named by the `FASTLY_SERVICE_ID` + environment variable on the corresponding Heroku app + (`heroku config -a | grep FASTLY_SERVICE_ID`). Match that ID in the + dashboard. Do staging first, then production. +2. **Open and clone the active version.** At + [manage.fastly.com](https://manage.fastly.com), open the service, then click + **Edit configuration → Clone version N to edit**. Fastly will not let you + edit the live version directly; cloning creates an editable draft, and + nothing goes live until you Activate (step 6). +3. **Create the request setting.** In the left sidebar click **Settings** (the + gear). In the **Request Settings** section click **Create a request + setting**, then set **Name** = `Bypass cache for personalized requests` and + **Action** = **Pass**. Leave the other fields at their defaults. +4. **Attach the condition.** In the same form, under **Conditions** / **Request + condition**, click **Create a new condition**. Set **Name** = + `Personalized page request` and paste the **Apply if** expression above + verbatim (it is valid VCL; if the UI rejects it, the usual cause is a + smart-quote introduced by copy/paste — make sure the quotes are plain `"`). + Save the condition, then save the request setting. +5. **(Optional) Cookie-stripping for hit rate.** The `unset req.http.Cookie` + optimization in Option B (which lets guests carrying only unrelated cookies, + e.g. analytics, still share one cached object) is **not** expressible as a + simple Request Setting. It is *not required for correctness* — Option A above + is sufficient and safe on its own. If you want the hit-rate improvement, add + it via a small custom VCL snippet (Option B) instead. +6. **Activate.** Click **Activate** on the draft version to make it live (a few + seconds to propagate). + +> **If your UI has no "Pass" action on Request Settings:** the equivalent is a +> **Cache Setting** with **Action: Pass** plus a **Cache condition** using the +> same expression. + +**Companion Rails kill switch.** The Fastly rule and the Rails kill switch +(`BADGEAPP_CACHE_SHOW_PROJECT`, Change 2) are independent. Caching HTML only +actually happens when *both* are enabled: the Fastly bypass rule is active *and* +`BADGEAPP_CACHE_SHOW_PROJECT` is not `false`. If show pages return +`private, no-store` with no `Surrogate-Control` even after the Rails deploy, +check `heroku config -a | grep BADGEAPP_CACHE_SHOW_PROJECT` — set it to +`true` (or unset it; the default is on) and restart. Conversely, setting it to +`false` is the instant rollback for HTML caching, leaving this Fastly rule +harmlessly in place. + +**Verify after activating** with `script/verify_cdn_caching.sh -v ` +(Section 8): a request carrying `_BadgeApp_session` or `remember_token` must +return a non-`HIT` `X-Cache` even when an anonymous cached object exists, while +an anonymous request gets `MISS` then `HIT`. + #### Option B: Custom VCL snippet (placement: `recv`) ```vcl @@ -755,6 +844,13 @@ The rendered show page is the canonical section URL (`/en/projects/:id/:section`); `/en/projects/:id` only redirects to it, so the tests below target the section URL directly. +[`script/verify_cdn_caching.sh`](../script/verify_cdn_caching.sh) automates all +of the checks in this section (run it as +`script/verify_cdn_caching.sh -v https://staging.bestpractices.dev 1`); it +exits non-zero on any failure, so it also serves as the periodic synthetic +monitor recommended in Section 9.4. The raw `curl` recipes below remain useful +for ad-hoc inspection. + ### Anonymous request is cached ```bash @@ -817,18 +913,28 @@ which our change omits for anonymous users. cookie" test iterates over a list of anonymous pages; adding UJS/AJAX to any of them flips the meta tag back on and trips the test. Keep that list current as anonymous pages are added. -* **Do better (opt-in escape hatch).** If a specific anonymous page ever - legitimately needs the token, it should opt in explicitly rather than - forcing it globally. For example, gate the layout on login *or* an - explicit request: +* **Opt-in escape hatch (implemented).** A specific anonymous page that + legitimately needs the token opts in explicitly rather than forcing it + globally. The layout is gated on login *or* an explicit request: ```erb <% if logged_in? || content_for?(:needs_csrf_meta) %><%= csrf_meta_tags %><% end %> ``` - and have that one page set `content_for(:needs_csrf_meta) { true }`. This - keeps the default (no token, cacheable) safe while making the exception - loud and local. + and the page sets the flag at the top of its template: + + ```erb + <% content_for :needs_csrf_meta, 'true' %> + ``` + + This is **already in use** by the login page + (`app/views/sessions/new.html.erb`), whose "Log in with GitHub" button is a + rails-ujs `link_to … method: 'post'` that needs the token (see the warning + under Change 1). Use the **non-block** form with a non-empty string: + `content_for(:needs_csrf_meta) { true }` does **not** work because + `content_for` captures the block's *output*, and a block returning a bare + `true` outputs nothing, leaving `content_for?` false. The default (no token, + cacheable) stays safe for every page that does not opt in. ### 9.2 A persistent flash reaches an anonymous user diff --git a/script/verify_cdn_caching.sh b/script/verify_cdn_caching.sh new file mode 100755 index 000000000..c34b4255f --- /dev/null +++ b/script/verify_cdn_caching.sh @@ -0,0 +1,216 @@ +#!/usr/bin/env bash + +# Copyright the Linux Foundation and the +# OpenSSF Best Practices badge contributors +# SPDX-License-Identifier: MIT + +# Verify CDN (Fastly) caching of anonymous "project show" pages, per +# docs/cdn-cache-not-logged-in.md Section 8. Runs items 1-4 of the staging +# verification plan: +# +# 1. An anonymous request is cacheable and served from cache (HIT), and +# sets no _BadgeApp_session cookie. +# 2. A request carrying _BadgeApp_session bypasses the cache (never HIT), +# even when a cached anonymous object exists. +# 3. A request carrying the remember-me cookie bypasses the cache likewise. +# 4. JSON still caches regardless of cookies (no regression). +# +# Items 2 and 3 exercise the Fastly bypass rule (Change 3), which lives in +# the Fastly config, NOT in this repo. If that rule is not yet deployed to +# the target, those checks will fail -- that is a real signal, not a bug in +# this script. Note: until cacheable HTML actually ships, items 2 and 3 pass +# trivially (nothing is cached, so nothing can be a HIT); they only become +# meaningful once item 1 passes. +# +# This is also suitable as a periodic synthetic monitor (Section 9.4): a +# session-cookie or remember-me request returning HIT is a security +# regression in the bypass rule. +# +# Usage: script/verify_cdn_caching.sh [-v|--verbose] [BASE_URL] [PROJECT_ID] +# -v, --verbose Print the full response header block for every request +# (useful for inspecting the existing JSON/badge VCL while +# hunting for the rule to copy, and confirming the CDN +# strips Surrogate-Control before the browser). +# BASE_URL default: https://staging.bestpractices.dev +# PROJECT_ID default: 1 +# +# Exit status is 0 only if every check passes. + +set -u + +usage() { + cat <<'EOF' +Usage: script/verify_cdn_caching.sh [-v|--verbose] [BASE_URL] [PROJECT_ID] + -v, --verbose Print the full response header block for every request. + BASE_URL default: https://staging.bestpractices.dev + PROJECT_ID default: 1 +EOF +} + +VERBOSE='' +positional=() +for arg in "$@"; do + case "$arg" in + -v|--verbose) VERBOSE=1 ;; + -h|--help) usage; exit 0 ;; + -*) printf 'Unknown option: %s\n' "$arg" >&2; usage >&2; exit 2 ;; + *) positional+=("$arg") ;; + esac +done + +BASE="${positional[0]:-https://staging.bestpractices.dev}" +PROJECT_ID="${positional[1]:-1}" +BASE="${BASE%/}" # strip any trailing slash + +SHOW_URL="$BASE/en/projects/$PROJECT_ID/passing" +JSON_URL="$BASE/projects/$PROJECT_ID.json" + +# Colorize only when stdout is a terminal. +if [ -t 1 ]; then + RED=$(printf '\033[31m'); GREEN=$(printf '\033[32m') + YELLOW=$(printf '\033[33m'); CYAN=$(printf '\033[36m') + BOLD=$(printf '\033[1m'); RESET=$(printf '\033[0m') +else + RED=''; GREEN=''; YELLOW=''; CYAN=''; BOLD=''; RESET='' +fi + +passes=0 +failures=0 + +# Fetch a URL with Fastly debugging on, discarding the body and printing +# only the response headers (no "< " prefix, unlike curl -v). Extra args +# (e.g. -H "Cookie: ...") are passed through. +fetch_headers() { + curl -sS -o /dev/null -D - -H 'Fastly-Debug: 1' "$@" +} + +# In --verbose mode, print a labeled, indented dump of a header block (with +# CR characters stripped so it reads cleanly). No-op otherwise. +# Usage: vlog "label" "$headers" +vlog() { + [ -n "$VERBOSE" ] || return 0 + printf '%s --- %s ---%s\n' "$CYAN" "$1" "$RESET" + printf '%s\n' "$2" | tr -d '\r' | sed '/^$/d; s/^/ /' +} + +# Fetch + (optionally) dump in one step, returning the headers on stdout. +# The dump goes to stderr so it never pollutes the captured value. +# Usage: headers=$(fetch_and_log "label" [curl args...] URL) +fetch_and_log() { + local label="$1"; shift + local headers + headers=$(fetch_headers "$@") + vlog "$label" "$headers" >&2 + printf '%s' "$headers" +} + +# Extract the last value of a (case-insensitive) header from header text. +# Usage: header_value "X-Cache" "$headers" +header_value() { + printf '%s\n' "$2" | grep -i "^$1:" | tail -1 | + sed 's/^[^:]*:[[:space:]]*//' | tr -d '\r' +} + +# Does the response indicate a Fastly cache HIT? Fastly may chain nodes +# (e.g. "X-Cache: MISS, HIT"), so we look for HIT anywhere in the value. +is_hit() { + printf '%s\n' "$1" | grep -iq '^X-Cache:.*HIT' +} + +report() { # status_bool description detail + if [ "$1" = 'true' ]; then + passes=$((passes + 1)) + printf ' %sPASS%s %s\n' "$GREEN" "$RESET" "$2" + else + failures=$((failures + 1)) + printf ' %sFAIL%s %s\n' "$RED" "$RESET" "$2" + fi + [ -n "${3:-}" ] && printf ' %s%s%s\n' "$YELLOW" "$3" "$RESET" + return 0 +} + +printf '%sVerifying CDN caching%s\n' "$BOLD" "$RESET" +printf ' Base URL: %s\n' "$BASE" +printf ' Show page: %s\n' "$SHOW_URL" +printf ' JSON: %s\n' "$JSON_URL" +[ -n "$VERBOSE" ] && printf ' Verbose: on\n' +printf '\n' + +# --- Item 1: anonymous request is cacheable and served from cache ---------- +printf '%s1. Anonymous show page is cacheable (MISS then HIT), no cookie%s\n' \ + "$BOLD" "$RESET" +h1=$(fetch_and_log 'anon request 1 (warm)' "$SHOW_URL") +h2=$(fetch_and_log 'anon request 2 (expect HIT)' "$SHOW_URL") +xc1=$(header_value 'X-Cache' "$h1") +xc2=$(header_value 'X-Cache' "$h2") +surrogate=$(header_value 'Surrogate-Control' "$h1") +setcookie=$(printf '%s\n' "$h1" "$h2" | grep -i '^Set-Cookie:.*_BadgeApp_session') + +if [ -n "$surrogate" ]; then + report true "advertises Surrogate-Control to the CDN" "Surrogate-Control: $surrogate" +else + report false "advertises Surrogate-Control to the CDN" \ + 'missing Surrogate-Control: Rails is not marking this page cacheable' +fi + +if [ -z "$setcookie" ]; then + report true 'sets no _BadgeApp_session cookie' +else + report false 'sets no _BadgeApp_session cookie' "$setcookie" +fi + +if is_hit "$h2"; then + report true 'second request is a Fastly HIT' "X-Cache: $xc1 -> $xc2" +else + report false 'second request is a Fastly HIT' \ + "X-Cache: $xc1 -> $xc2 (Fastly bypass rule may be passing all HTML, or caching not yet enabled)" +fi + +# --- Item 2: session cookie bypasses the cache ----------------------------- +printf '\n%s2. _BadgeApp_session request bypasses the cache (never HIT)%s\n' \ + "$BOLD" "$RESET" +fetch_and_log 'anon warm 1' "$SHOW_URL" >/dev/null # ensure a cached object exists +fetch_and_log 'anon warm 2' "$SHOW_URL" >/dev/null +hs=$(fetch_and_log 'with _BadgeApp_session cookie' \ + -H 'Cookie: _BadgeApp_session=anything' "$SHOW_URL") +xcs=$(header_value 'X-Cache' "$hs") +if is_hit "$hs"; then + report false 'session-cookie request is not served from cache' \ + "X-Cache: $xcs (SECURITY: a personalized request got a cached anonymous page -- check the Fastly bypass rule)" +else + report true 'session-cookie request is not served from cache' "X-Cache: $xcs" +fi + +# --- Item 3: remember-me cookie bypasses the cache ------------------------- +printf '\n%s3. remember_token request bypasses the cache (never HIT)%s\n' \ + "$BOLD" "$RESET" +hr=$(fetch_and_log 'with remember_token cookie' \ + -H 'Cookie: remember_token=anything' "$SHOW_URL") +xcr=$(header_value 'X-Cache' "$hr") +if is_hit "$hr"; then + report false 'remember-me request is not served from cache' \ + "X-Cache: $xcr (SECURITY: check the Fastly bypass rule)" +else + report true 'remember-me request is not served from cache' "X-Cache: $xcr" +fi + +# --- Item 4: JSON still caches regardless of cookies (no regression) ------- +printf '\n%s4. JSON still caches despite a session cookie (no regression)%s\n' \ + "$BOLD" "$RESET" +fetch_and_log 'json warm (with cookie)' \ + -H 'Cookie: _BadgeApp_session=anything' "$JSON_URL" >/dev/null +hj=$(fetch_and_log 'json with cookie (expect HIT)' \ + -H 'Cookie: _BadgeApp_session=anything' "$JSON_URL") +xcj=$(header_value 'X-Cache' "$hj") +if is_hit "$hj"; then + report true 'JSON is served from cache even with a cookie' "X-Cache: $xcj" +else + report false 'JSON is served from cache even with a cookie' \ + "X-Cache: $xcj (JSON is meant to cache regardless of cookies)" +fi + +# --- Summary --------------------------------------------------------------- +printf '\n%sSummary:%s %s%d passed%s, %s%d failed%s\n' \ + "$BOLD" "$RESET" "$GREEN" "$passes" "$RESET" "$RED" "$failures" "$RESET" + +[ "$failures" -eq 0 ] diff --git a/test/integration/cdn_caching_test.rb b/test/integration/cdn_caching_test.rb index b4d73ee41..68ec4ed46 100644 --- a/test/integration/cdn_caching_test.rb +++ b/test/integration/cdn_caching_test.rb @@ -5,6 +5,7 @@ require 'test_helper' +# rubocop:disable Metrics/ClassLength class CdnCachingTest < ActionDispatch::IntegrationTest setup do @project = projects(:one) @@ -102,6 +103,21 @@ class CdnCachingTest < ActionDispatch::IntegrationTest end end + # The login page initiates GitHub OAuth with a rails-ujs "method: post" + # link, which reads the CSRF token from the meta tag. Change 1 removes that + # tag for anonymous users, so the login page must opt back in via + # content_for(:needs_csrf_meta). Without the tag the POST to /auth/github + # fails CSRF (ActionController::InvalidAuthenticityToken) and login 404s. + # This must be tested with forgery protection ON: it is off by default in + # the test env, so the bug is invisible otherwise (see Section 6 caveat). + test 'anonymous login page emits the CSRF meta tag for GitHub OAuth' do + with_forgery_protection do + get login_path(locale: :en) + assert_response :success + assert_select 'meta[name="csrf-token"]', count: 1 + end + end + # An obsolete-section 301 from inside show must NOT be cached: the # "return if performed?" guard keeps cache_on_cdn from attaching to it. test 'obsolete-section redirect is not CDN-cacheable' do @@ -150,3 +166,4 @@ def with_forgery_protection ActionController::Base.allow_forgery_protection = original end end +# rubocop:enable Metrics/ClassLength