Fix GitHub login broken by conditional CSRF tag (#2859)

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.

Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Author: david-a-wheeler

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 -%>

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 <meta name="csrf-token">
+    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

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 <app> | 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 <app> | 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 <base-url>`
+(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