Make auth-dependent tests async (#2654)

lib/livebook_web/controllers/auth_controller.ex

@@ -22,7 +22,7 @@ defmodule LivebookWeb.AuthController do
   def index(conn, _params) do
     render(conn, "index.html",
       errors: [],
-      authentication_mode: Livebook.Config.authentication().mode
+      authentication_mode: LivebookWeb.AuthPlug.authentication(conn).mode
     )
   end
 

lib/livebook_web/plugs/auth_plug.ex

@@ -41,7 +41,7 @@ defmodule LivebookWeb.AuthPlug do
   """
   @spec authenticated?(map(), non_neg_integer()) :: boolean()
   def authenticated?(session, port) do
-    case Livebook.Config.authentication() do
+    case authentication(session) do
       %{mode: :disabled} ->
         true
 
@@ -52,7 +52,7 @@ defmodule LivebookWeb.AuthPlug do
   end
 
   defp authenticate(conn) do
-    case Livebook.Config.authentication() do
+    case authentication(conn) do
       %{mode: :password} ->
         redirect_to_authenticate(conn)
 
@@ -103,4 +103,21 @@ defmodule LivebookWeb.AuthPlug do
 
   defp key(port, mode), do: "#{port}:#{mode}"
   defp hash(value), do: :crypto.hash(:sha256, value)
+
+  @doc """
+  Returns the authentication configuration for the given `conn` or
+  `session`.
+
+  This mirrors `Livebook.Config.authentication/0`, except the it can
+  be overridden in tests, for each connection.
+  """
+  if Mix.env() == :test do
+    def authentication(%Plug.Conn{} = conn), do: authentication(get_session(conn))
+
+    def authentication(%{} = session) do
+      session["authentication_test_override"] || Livebook.Config.authentication()
+    end
+  else
+    def authentication(_), do: Livebook.Config.authentication()
+  end
 end

test/livebook_web/controllers/session_controller_test.exs

@@ -250,6 +250,8 @@ defmodule LivebookWeb.SessionControllerTest do
   end
 
   describe "show_asset" do
+    @describetag authentication: %{mode: :password, secret: "grumpycat"}
+
     test "fetches assets and redirects to the session-less path", %{conn: conn} do
       %{notebook: notebook, hash: hash} = notebook_with_js_output()
 
@@ -297,6 +299,8 @@ defmodule LivebookWeb.SessionControllerTest do
   end
 
   describe "show_cached_asset" do
+    @describetag authentication: %{mode: :password, secret: "grumpycat"}
+
     test "returns not found when no matching assets are in the cache", %{conn: conn} do
       %{notebook: _notebook, hash: hash} = notebook_with_js_output()
 
@@ -345,7 +349,7 @@ defmodule LivebookWeb.SessionControllerTest do
 
       token = LivebookWeb.SessionHelpers.generate_input_token(view.pid, input_id)
 
-      conn = get(conn, ~p"/public/sessions/audio-input/#{token}")
+      conn = conn |> with_password_auth() |> get(~p"/public/sessions/audio-input/#{token}")
 
       assert conn.status == 200
       assert conn.resp_body == "wav content"
@@ -364,6 +368,7 @@ defmodule LivebookWeb.SessionControllerTest do
 
       conn =
         conn
+        |> with_password_auth()
         |> put_req_header("range", "bytes=4-")
         |> get(~p"/public/sessions/audio-input/#{token}")
 
@@ -382,7 +387,7 @@ defmodule LivebookWeb.SessionControllerTest do
 
       token = LivebookWeb.SessionHelpers.generate_input_token(view.pid, input_id)
 
-      conn = get(conn, ~p"/public/sessions/audio-input/#{token}")
+      conn = conn |> with_password_auth() |> get(~p"/public/sessions/audio-input/#{token}")
 
       assert conn.status == 200
       assert <<_header::44-binary, "pcm content">> = conn.resp_body
@@ -401,6 +406,7 @@ defmodule LivebookWeb.SessionControllerTest do
 
       conn =
         conn
+        |> with_password_auth()
         |> put_req_header("range", "bytes=48-")
         |> get(~p"/public/sessions/audio-input/#{token}")
 
@@ -421,7 +427,7 @@ defmodule LivebookWeb.SessionControllerTest do
 
       token = LivebookWeb.SessionHelpers.generate_input_token(view.pid, input_id)
 
-      conn = get(conn, ~p"/public/sessions/image-input/#{token}")
+      conn = conn |> with_password_auth() |> get(~p"/public/sessions/image-input/#{token}")
 
       assert conn.status == 200
       assert conn.resp_body == "rgb content"
@@ -443,6 +449,11 @@ defmodule LivebookWeb.SessionControllerTest do
     conn
   end
 
+  defp with_password_auth(conn) do
+    authentication = %{mode: :password, secret: "grumpycat"}
+    with_authentication(conn, authentication)
+  end
+
   defp notebook_with_js_output() do
     archive_path = Path.expand("../../support/assets.tar.gz", __DIR__)
     hash = "test-" <> Livebook.Utils.random_id()

test/livebook_web/live/app_auth_live_test.exs

@@ -1,6 +1,5 @@
 defmodule LivebookWeb.AppAuthLiveTest do
-  # Not async, because we alter global config (auth mode)
-  use LivebookWeb.ConnCase, async: false
+  use LivebookWeb.ConnCase, async: true
 
   import Phoenix.LiveViewTest
 
@@ -11,13 +10,7 @@ defmodule LivebookWeb.AppAuthLiveTest do
       Livebook.App.close(app_pid)
     end)
 
-    Application.put_env(:livebook, :authentication, {:password, ctx[:livebook_password]})
-
-    on_exit(fn ->
-      Application.put_env(:livebook, :authentication, :disabled)
-    end)
-
-    %{slug: slug}
+    [slug: slug]
   end
 
   defp create_app(app_settings_attrs) do
@@ -42,6 +35,8 @@ defmodule LivebookWeb.AppAuthLiveTest do
     {slug, app_pid}
   end
 
+  @moduletag authentication: %{mode: :password, secret: "long_livebook_password"}
+
   # Integration tests for the authentication scenarios
 
   describe "public app" do
@@ -58,7 +53,6 @@ defmodule LivebookWeb.AppAuthLiveTest do
   end
 
   describe "protected app" do
-    @describetag livebook_password: "long_livebook_password"
     @describetag app_settings: %{access_type: :protected, password: "long_app_password"}
 
     test "redirect to auth page when not authenticated", %{conn: conn, slug: slug} do

test/livebook_web/plugs/auth_plug_test.exs

@@ -1,87 +1,66 @@
 defmodule LivebookWeb.AuthPlugTest do
-  # Not async, because we alter global config (auth mode)
-  use LivebookWeb.ConnCase, async: false
-
-  setup context do
-    authentication =
-      cond do
-        context[:token] -> :token
-        password = context[:password] -> {:password, password}
-        true -> :disabled
-      end
-
-    unless authentication == :disabled do
-      Application.put_env(:livebook, :authentication, authentication)
-
-      on_exit(fn ->
-        Application.put_env(:livebook, :authentication, :disabled)
-      end)
-    end
+  use LivebookWeb.ConnCase, async: true
+
+  test "skips authentication when it is disabled", %{conn: conn} do
+    conn = get(conn, ~p"/")
 
-    :ok
+    assert conn.status == 200
+    assert conn.resp_body =~ "New notebook"
   end
 
-  describe "token authentication" do
-    test "skips authentication when no token is configured", %{conn: conn} do
-      conn = get(conn, ~p"/")
+  test "/authenticate redirects to / when authentication is disabled", %{conn: conn} do
+    conn = get(conn, ~p"/authenticate")
+    assert redirected_to(conn) == ~p"/"
+  end
 
-      assert conn.status == 200
-      assert conn.resp_body =~ "New notebook"
-    end
+  describe "token authentication" do
+    @describetag authentication: %{mode: :token, secret: "grumpycat"}
 
-    @tag :token
-    test "redirects to /authenticate if not authenticated", %{conn: conn} do
+    test "redirects if not authenticated", %{conn: conn} do
       conn = get(conn, ~p"/")
-      assert redirected_to(conn) == ~p"/authenticate"
+      assert redirected_to(conn) in unauthenticated_homes()
     end
 
-    @tag :token
     test "redirects to the same path when valid token is provided in query params", %{conn: conn} do
-      conn = get(conn, ~p"/?token=#{auth_token()}")
+      conn = get(conn, ~p"/?token=grumpycat")
 
       assert redirected_to(conn) == ~p"/"
     end
 
-    @tag :token
-    test "redirects to /authenticate when invalid token is provided in query params",
-         %{conn: conn} do
+    test "redirects when invalid token is provided in query params", %{conn: conn} do
       conn = get(conn, ~p"/?token=invalid")
-      assert redirected_to(conn) == ~p"/authenticate"
+      assert redirected_to(conn) in unauthenticated_homes()
     end
 
-    @tag :token
     test "persists authentication across requests", %{conn: conn} do
-      conn = get(conn, ~p"/?token=#{auth_token()}")
+      conn = get(conn, ~p"/?token=grumpycat")
       assert get_session(conn, "80:token")
 
       conn = get(conn, ~p"/")
       assert conn.status == 200
       assert conn.resp_body =~ "New notebook"
     end
 
-    @tag :token
     test "redirects to referer on valid authentication", %{conn: conn} do
       referer = "/import?url=example.com"
 
       conn = get(conn, referer)
       assert redirected_to(conn) == ~p"/authenticate"
 
-      conn = post(conn, ~p"/authenticate", token: auth_token())
+      conn = post(conn, ~p"/authenticate", token: "grumpycat")
       assert redirected_to(conn) == referer
     end
 
-    @tag :token
     test "redirects back to /authenticate on invalid token", %{conn: conn} do
       conn = post(conn, ~p"/authenticate?token=invalid_token")
       assert html_response(conn, 200) =~ "Authentication required"
 
       conn = get(conn, ~p"/")
-      assert redirected_to(conn) == ~p"/authenticate"
+      assert redirected_to(conn) in unauthenticated_homes()
     end
 
-    @tag :token
     test "persists authentication across requests (via /authenticate)", %{conn: conn} do
-      conn = post(conn, ~p"/authenticate?token=#{auth_token()}")
+      conn = post(conn, ~p"/authenticate?token=grumpycat")
       assert get_session(conn, "80:token")
 
       conn = get(conn, ~p"/")
@@ -94,33 +73,26 @@ defmodule LivebookWeb.AuthPlugTest do
   end
 
   describe "password authentication" do
-    test "redirects to '/' if no authentication is required", %{conn: conn} do
-      conn = get(conn, ~p"/authenticate")
-      assert redirected_to(conn) == ~p"/"
-    end
+    @describetag authentication: %{mode: :password, secret: "grumpycat"}
 
-    @tag password: "grumpycat"
     test "does not crash when given a token", %{conn: conn} do
       conn = post(conn, ~p"/authenticate?token=grumpycat")
       assert html_response(conn, 200) =~ "token is invalid"
     end
 
-    @tag password: "grumpycat"
-    test "redirects to /authenticate if not authenticated", %{conn: conn} do
+    test "redirects if not authenticated", %{conn: conn} do
       conn = get(conn, ~p"/")
-      assert redirected_to(conn) == ~p"/authenticate"
+      assert redirected_to(conn) in unauthenticated_homes()
     end
 
-    @tag password: "grumpycat"
-    test "redirects to '/' on valid authentication", %{conn: conn} do
+    test "redirects to / on valid authentication", %{conn: conn} do
       conn = post(conn, ~p"/authenticate?password=grumpycat")
       assert redirected_to(conn) == ~p"/"
 
       conn = get(conn, ~p"/")
       assert html_response(conn, 200) =~ "New notebook"
     end
 
-    @tag password: "grumpycat"
     test "redirects to referer on valid authentication", %{conn: conn} do
       referer = "/import?url=example.com"
 
@@ -131,16 +103,14 @@ defmodule LivebookWeb.AuthPlugTest do
       assert redirected_to(conn) == referer
     end
 
-    @tag password: "grumpycat"
     test "redirects back to /authenticate on invalid password", %{conn: conn} do
       conn = post(conn, ~p"/authenticate?password=invalid_password")
       assert html_response(conn, 200) =~ "Authentication required"
 
       conn = get(conn, ~p"/")
-      assert redirected_to(conn) == ~p"/authenticate"
+      assert redirected_to(conn) in unauthenticated_homes()
     end
 
-    @tag password: "grumpycat"
     test "persists authentication across requests", %{conn: conn} do
       conn = post(conn, ~p"/authenticate?password=grumpycat")
       assert get_session(conn, "80:password")
@@ -154,8 +124,5 @@ defmodule LivebookWeb.AuthPlugTest do
     end
   end
 
-  defp auth_token() do
-    %{mode: :token, secret: token} = Livebook.Config.authentication()
-    token
-  end
+  defp unauthenticated_homes(), do: [~p"/authenticate", ~p"/apps"]
 end

test/support/conn_case.ex

@@ -16,7 +16,20 @@ defmodule LivebookWeb.ConnCase do
     end
   end
 
-  setup _tags do
-    {:ok, conn: Phoenix.ConnTest.build_conn()}
+  setup tags do
+    conn = Phoenix.ConnTest.build_conn()
+
+    conn =
+      if authentication = tags[:authentication] do
+        with_authentication(conn, authentication)
+      else
+        conn
+      end
+
+    [conn: conn]
+  end
+
+  def with_authentication(conn, authentication) do
+    Plug.Test.init_test_session(conn, authentication_test_override: authentication)
   end
 end