Make the dashboard work under host CSRF pipelines (no config needed)

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## v1.2.0 (unreleased)
 
+* `FunWithFlags.UI.Router` now works under Phoenix's standard `:browser` pipeline (or any host pipeline that uses `Plug.CSRFProtection`) without extra configuration. The router opts its own static asset `GET`/`HEAD` requests out of the host's CSRF protection, so the dashboard's cross-origin `<script>` and `<link>` requests no longer raise `Plug.CSRFProtection.InvalidCrossOriginRequestError`. State-changing requests remain fully CSRF-protected, and the previously documented `:mounted_apps` pipeline (one that omits `:protect_from_forgery`) is no longer necessary. ([issue/52](https://github.com/tompave/fun_with_flags_ui/issues/52))
 * Drop support for Erlang/OTP 25, and Erlang/OTP >= 26 is now required. Dropping support for older versions of Erlang/OTP simply means that this package is not tested with them in CI, and that no compatibility issues are considered bugs.
 
 ## v1.1.0

README.md

@@ -16,25 +16,22 @@ It's primarily meant to be embedded in a host Plug application, either Phoenix o
 
 ### Mounted in Phoenix
 
-The router plug can be mounted inside the Phoenix router with [`Phoenix.Router.forward/4`](https://hexdocs.pm/phoenix/Phoenix.Router.html#forward/4).
+The router plug can be mounted inside the Phoenix router with [`Phoenix.Router.forward/4`](https://hexdocs.pm/phoenix/Phoenix.Router.html#forward/4). It works under Phoenix's standard `:browser` pipeline, so you can reuse the same pipeline that already handles sessions, secure headers, and authentication:
 
 ```elixir
 defmodule MyPhoenixAppWeb.Router do
   use MyPhoenixAppWeb, :router
 
-  pipeline :mounted_apps do
-    plug :accepts, ["html"]
-    plug :put_secure_browser_headers
-  end
-
-  scope path: "/feature-flags" do
-    pipe_through :mounted_apps
+  scope "/feature-flags" do
+    pipe_through [:browser, :require_authenticated_admin]
     forward "/", FunWithFlags.UI.Router, namespace: "feature-flags"
   end
 end
 ```
 
-Note: There is no need to add `:protect_from_forgery` to the `:mounted_apps` pipeline because this package already implements CSRF protection. In order to enable it, your host application must use the `Plug.Session` plug, which is usually configured in the endpoint module in Phoenix.
+The dashboard serves its own JavaScript and CSS from `/assets/*`. Because browsers request those `<script>` and `<link>` tags cross-origin, a host CSRF plug such as Phoenix's `:protect_from_forgery` would otherwise reject them with `Plug.CSRFProtection.InvalidCrossOriginRequestError`. `FunWithFlags.UI.Router` handles this for you: it opts its own asset `GET`/`HEAD` requests out of CSRF protection, while every state-changing request (creating, toggling, and deleting flags) stays fully CSRF-protected. No special pipeline and no extra configuration are required.
+
+CSRF protection for those state-changing requests requires your host application to use the `Plug.Session` plug, which Phoenix configures in the endpoint module by default.
 
 ### Mounted in another Plug application
 
@@ -47,7 +44,7 @@ defmodule Another.App do
 end
 ```
 
-Note: If your plug router uses `Plug.CSRFProtection`, `FunWithFlags.UI.Router` should be added before your CSRF protection plug because it already implements its own CSRF protection. If you declare `FunWithFlags.UI.Router` after, your CSRF plug will likely block GET requests for the JS assets of the dashboard.
+The same asset handling described above applies here: the router opts its own asset requests out of CSRF protection, so it works regardless of where your host application's `Plug.CSRFProtection` sits in the pipeline.
 
 ### Standalone
 
@@ -84,14 +81,13 @@ defmodule MyPhoenixAppWeb.Router do
   use MyPhoenixAppWeb, :router
 + import Plug.BasicAuth
 
-  pipeline :mounted_apps do
-    plug :accepts, ["html"]
-    plug :put_secure_browser_headers
++ pipeline :feature_flags_auth do
 +   plug :basic_auth, username: "foo", password: "bar"
-  end
++ end
 
-  scope path: "/feature-flags" do
-    pipe_through :mounted_apps
+  scope "/feature-flags" do
+-   pipe_through :browser
++   pipe_through [:browser, :feature_flags_auth]
     forward "/", FunWithFlags.UI.Router, namespace: "feature-flags"
   end
 end

lib/fun_with_flags/ui/router.ex

@@ -14,6 +14,20 @@ defmodule FunWithFlags.UI.Router do
 
   plug Plug.Logger, log: :debug
 
+  # When the UI is forwarded under a host pipeline that already runs
+  # `Plug.CSRFProtection` (for example, Phoenix's standard `:browser`
+  # pipeline), that plug has registered a `before_send` callback that raises
+  # `Plug.CSRFProtection.InvalidCrossOriginRequestError` whenever a non-XHR
+  # `GET` returns a JavaScript response. The dashboard's `<script>` tag for
+  # `/assets/details.js` is exactly such a request, so the host would reject
+  # the asset and the UI would fail to load.
+  #
+  # `Plug.CSRFProtection` reads `conn.private[:plug_skip_csrf_protection]` at
+  # send time, so setting it here — after the host registered its callback but
+  # before the asset is sent — suppresses that check for asset requests only.
+  # It must run before `Plug.Static` sends the response.
+  plug :skip_csrf_for_assets
+
   plug Plug.Static,
     gzip: true,
     at: "/assets",
@@ -293,6 +307,26 @@ defmodule FunWithFlags.UI.Router do
   end
 
 
+  @safe_methods ~w(GET HEAD)
+
+  # Opt safe asset requests out of CSRF protection (both the host's and this
+  # router's own) by setting the documented `:plug_skip_csrf_protection`
+  # escape hatch. Only `GET`/`HEAD` requests for `/assets/<file>` are skipped:
+  # those are read-only and are the only requests that trip the host's
+  # cross-origin-JavaScript check. State-changing requests are never skipped
+  # and continue to flow through `protect_from_forgery` unchanged.
+  #
+  # `forward` strips the mount prefix from `conn.path_info`, so the asset
+  # subtree is always `["assets" | file]` here regardless of where the router
+  # is mounted — no mount/namespace configuration is needed.
+  defp skip_csrf_for_assets(%Plug.Conn{method: method, path_info: ["assets", _ | _]} = conn, _opts)
+       when method in @safe_methods do
+    Plug.Conn.put_private(conn, :plug_skip_csrf_protection, true)
+  end
+
+  defp skip_csrf_for_assets(conn, _opts), do: conn
+
+
   # Custom CSRF protection plug. It wraps the default plug provided
   # by `Plug`, it calls `Plug.Conn.fetch_session/1` (no-op if already
   # fetched), and it bails out gracefully if no session is configured.

test/fun_with_flags/ui/router_test.exs

@@ -200,6 +200,87 @@ defmodule FunWithFlags.UI.RouterTest do
   end
 
 
+  # Simulates a host application (for example, a Phoenix `:browser` pipeline)
+  # that runs `Plug.CSRFProtection` *around* the forwarded router. The host's
+  # CSRF plug registers a `before_send` callback that raises
+  # `InvalidCrossOriginRequestError` for non-XHR `GET`s returning JavaScript.
+  # The router must suppress that for its own assets so the dashboard loads.
+  describe "embedded under a host CSRF pipeline" do
+    @session_opts Plug.Session.init(
+                    store: :cookie,
+                    key: "_test_session",
+                    signing_salt: "test-salt",
+                    encryption_salt: "test-enc"
+                  )
+    @csrf_opts Plug.CSRFProtection.init([])
+
+    defp through_host_csrf(conn) do
+      conn
+      |> Map.put(:secret_key_base, String.duplicate("a", 64))
+      |> Plug.Session.call(@session_opts)
+      |> fetch_session()
+      |> Plug.CSRFProtection.call(@csrf_opts)
+      |> Router.call(@opts)
+    end
+
+    test "a JavaScript asset loads without raising InvalidCrossOriginRequestError" do
+      conn = through_host_csrf(conn(:get, "/assets/details.js"))
+
+      assert conn.status == 200
+      assert conn.private[:plug_skip_csrf_protection] == true
+      # `Plug.Static` serves `.js` as `text/javascript`, which is exactly the
+      # content type the host's CSRF `before_send` callback would reject.
+      assert ["text/javascript"] = get_resp_header(conn, "content-type")
+    end
+
+    test "a CSS asset loads as well" do
+      conn = through_host_csrf(conn(:get, "/assets/style.css"))
+
+      assert conn.status == 200
+      assert conn.private[:plug_skip_csrf_protection] == true
+    end
+
+    test "an HTML page is served normally and is not opted out of CSRF" do
+      conn = through_host_csrf(conn(:get, "/flags"))
+
+      assert conn.status == 200
+      refute conn.private[:plug_skip_csrf_protection]
+    end
+
+    test "a forged state-changing request is still rejected" do
+      # No `_csrf_token`, so the host CSRF plug rejects it before the router
+      # can act, and the asset skip never applies to mutations.
+      assert_raise Plug.CSRFProtection.InvalidCSRFTokenError, fn ->
+        through_host_csrf(conn(:post, "/flags", "flag_name=nope"))
+      end
+    end
+  end
+
+
+  describe "asset CSRF skip" do
+    test "is set only for safe methods on /assets/<file>" do
+      for method <- [:get, :head] do
+        conn = conn(method, "/assets/details.js") |> Router.call(@opts)
+        assert conn.private[:plug_skip_csrf_protection] == true,
+               "expected #{method} /assets/details.js to be skipped"
+      end
+    end
+
+    test "is not set for non-asset paths" do
+      for path <- ["/flags", "/new", "/"] do
+        conn = conn(:get, path) |> Router.call(@opts)
+        refute conn.private[:plug_skip_csrf_protection],
+               "expected #{path} to retain CSRF protection"
+      end
+    end
+
+    test "is not set for the bare /assets path with no file" do
+      conn = conn(:get, "/assets") |> Router.call(@opts)
+      refute conn.private[:plug_skip_csrf_protection]
+    end
+  end
+
+
   # For GET and DELETE
   #
   defp request!(method, path) do