Fix CSRF conflicts with Phoenix using automatic pipeline separation

lib/fun_with_flags/ui/router.ex

@@ -2,6 +2,30 @@ defmodule FunWithFlags.UI.Router do
   @moduledoc """
   A `Plug.Router`. This module is meant to be plugged into host applications.
 
+  ## CSRF Protection
+
+  This router uses automatic pipeline separation to handle CSRF protection:
+
+  * **Static assets** (`/assets/*`) - Served directly without CSRF protection
+  * **Interactive routes** (forms, API endpoints) - Full CSRF protection applied
+  * **HTML pages** - Include CSRF tokens for JavaScript to use in AJAX requests
+
+  This approach resolves conflicts with Phoenix applications while maintaining security
+  for form submissions and API calls.
+
+  ## Options
+
+  * `:namespace` - Path prefix for internal redirects. Use when the UI is mounted
+    under a sub-path that differs from the forward path (default: `""`)
+
+  ## Usage
+
+      # Basic usage - UI available at /feature-flags/flags, /feature-flags/new, etc.
+      forward "/feature-flags", FunWithFlags.UI.Router
+
+      # With namespace - forward at /admin but UI redirects go to /admin/feature-flags/*
+      forward "/admin", FunWithFlags.UI.Router, namespace: "feature-flags"
+
   See the [Readme](/fun_with_flags_ui/readme.html#how-to-run) for more detailed instructions.
   """
   require Logger
@@ -14,17 +38,7 @@ defmodule FunWithFlags.UI.Router do
 
   plug Plug.Logger, log: :debug
 
-  plug Plug.Static,
-    gzip: true,
-    at: "/assets",
-    from: :fun_with_flags_ui
-
-  plug :protect_from_forgery, Plug.CSRFProtection.init([])
-
-  plug Plug.Parsers, parsers: [:urlencoded]
-  plug Plug.MethodOverride
-
-  plug :assign_csrf_token
+  plug :route_by_type
   plug :match
   plug :dispatch
 
@@ -287,6 +301,36 @@ defmodule FunWithFlags.UI.Router do
   end
 
 
+  defp route_by_type(conn, _opts) do
+    case conn.request_path do
+      "/assets" <> _ ->
+        asset_pipeline(conn)
+      _ ->
+        form_pipeline(conn)
+    end
+  end
+
+
+  defp asset_pipeline(conn) do
+    # Assets pipeline: only serve static files, no CSRF protection
+    Plug.Static.call(conn,
+      gzip: true,
+      at: "/assets",
+      from: :fun_with_flags_ui
+    )
+  end
+
+
+  defp form_pipeline(conn) do
+    # Form pipeline: full CSRF protection for interactive routes
+    conn
+    |> protect_from_forgery(Plug.CSRFProtection.init([]))
+    |> assign_csrf_token([])
+    |> Plug.Parsers.call(parsers: [:urlencoded])
+    |> Plug.MethodOverride.call([])
+  end
+
+
   defp assign_csrf_token(conn, _opts) do
     csrf_token = Plug.CSRFProtection.get_csrf_token()
     Plug.Conn.assign(conn, :csrf_token, csrf_token)

test/fun_with_flags/ui/router_test.exs

@@ -200,6 +200,134 @@ defmodule FunWithFlags.UI.RouterTest do
   end
 
 
+  describe "pipeline separation" do
+    test "asset requests go through asset pipeline without CSRF tokens" do
+      conn = request!(:get, "/assets/app.js")
+
+      # Asset requests should not have CSRF tokens assigned
+      refute Map.has_key?(conn.assigns, :csrf_token)
+
+      # Should attempt to serve static files (will 404 in tests, but that's expected)
+      # The important thing is that it doesn't crash with CSRF errors
+      assert conn.status in [404, 200]  # 404 because test env doesn't have actual assets
+    end
+
+    test "asset requests with different extensions go through asset pipeline" do
+      assets = ["/assets/app.css", "/assets/main.js", "/assets/image.png", "/assets/fonts/font.woff"]
+
+      for asset_path <- assets do
+        conn = request!(:get, asset_path)
+        refute Map.has_key?(conn.assigns, :csrf_token),
+               "Asset #{asset_path} should not have CSRF token"
+        assert conn.status in [404, 200]
+      end
+    end
+
+    test "form requests go through form pipeline with CSRF tokens" do
+      conn = request!(:get, "/flags")
+
+      # Form requests should have CSRF tokens assigned
+      assert Map.has_key?(conn.assigns, :csrf_token)
+      assert is_binary(conn.assigns[:csrf_token])
+      assert String.length(conn.assigns[:csrf_token]) > 0
+
+      assert 200 = conn.status
+      assert ["text/html; charset=utf-8"] = get_resp_header(conn, "content-type")
+    end
+
+    test "different form routes all get CSRF tokens" do
+      form_paths = ["/", "/flags", "/new", "/flags/test-flag"]
+
+      for path <- form_paths do
+        conn = request!(:get, path)
+        assert Map.has_key?(conn.assigns, :csrf_token),
+               "Form path #{path} should have CSRF token"
+        assert is_binary(conn.assigns[:csrf_token])
+      end
+    end
+
+    test "POST requests go through form pipeline with CSRF protection" do
+      # This should work because form pipeline includes parser
+      conn = request!(:post, "/flags", %{flag_name: "test_csrf_flag"})
+
+      # Should have CSRF token assigned during processing
+      assert Map.has_key?(conn.assigns, :csrf_token)
+
+      # Should process the form (either succeed or fail, but not crash on CSRF)
+      assert conn.status in [302, 400]  # Success redirect or validation error
+    end
+
+    test "asset pipeline doesn't interfere with static file serving" do
+      # Mock asset request - even if file doesn't exist, should go through asset pipeline
+      conn = request!(:get, "/assets/nonexistent.js")
+
+      # Should not have CSRF token
+      refute Map.has_key?(conn.assigns, :csrf_token)
+
+      # Should attempt static file serving (404 is expected for nonexistent files)
+      assert conn.status == 404
+    end
+  end
+
+  describe "CSRF token assignment" do
+    test "HTML responses include CSRF tokens for JavaScript usage" do
+      conn = request!(:get, "/flags")
+
+      # Should have CSRF token in assigns for template usage
+      assert Map.has_key?(conn.assigns, :csrf_token)
+      assert is_binary(conn.assigns[:csrf_token])
+
+      # HTML response should be generated (template can use the CSRF token)
+      assert 200 = conn.status
+      assert is_binary(conn.resp_body)
+    end
+
+    test "asset requests never get CSRF tokens to avoid conflicts" do
+      asset_paths = [
+        "/assets/app.js",
+        "/assets/app.css",
+        "/assets/images/logo.png",
+        "/assets/fonts/main.woff2"
+      ]
+
+      for path <- asset_paths do
+        conn = request!(:get, path)
+        refute Map.has_key?(conn.assigns, :csrf_token),
+               "Asset #{path} incorrectly received CSRF token"
+      end
+    end
+  end
+
+  describe "backward compatibility" do
+    test "all existing functionality continues to work with pipeline separation" do
+      # Test that basic flag operations still work
+      {:ok, true} = FunWithFlags.enable :compat_test_flag
+
+      # GET requests should work
+      conn = request!(:get, "/flags/compat_test_flag")
+      assert 200 = conn.status
+      assert Map.has_key?(conn.assigns, :csrf_token)
+
+      # POST requests should work
+      conn = request!(:post, "/flags", %{flag_name: "new_compat_flag"})
+      assert conn.status in [302, 400]  # Success or validation error
+      assert Map.has_key?(conn.assigns, :csrf_token)
+    end
+
+    test "namespace functionality works with pipeline separation" do
+      # Test with namespace option
+      opts = Router.init([namespace: "test-ns"])
+
+      conn = conn(:get, "/flags")
+           |> Router.call(opts)
+
+      assert 200 = conn.status
+      assert Map.has_key?(conn.assigns, :csrf_token)
+      assert conn.assigns[:namespace] == "/test-ns"
+    end
+  end
+
+
   # For GET and DELETE
   #
   defp request!(method, path) do