From a8d57864cfb37d5334eaa6e9810e4cf70105e245 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hugo=20Bara=C3=BAna?= <hugo.barauna@gmail.com>
Date: Mon, 20 Jan 2025 18:32:46 -0300
Subject: [PATCH] Bug fix: show logout template after logging out (#2913)

---
 lib/livebook/config.ex                        |  5 +---
 .../controllers/auth_controller.ex            | 29 ++++++-------------
 .../controllers/user_controller.ex            | 14 +++++++++
 lib/livebook_web/controllers/user_html.ex     |  5 ++++
 .../{auth_html => user_html}/logout.html.heex |  0
 lib/livebook_web/router.ex                    |  2 +-
 .../controllers/user_controller_test.exs      | 29 +++++++++++++++++++
 7 files changed, 59 insertions(+), 25 deletions(-)
 create mode 100644 lib/livebook_web/controllers/user_controller.ex
 create mode 100644 lib/livebook_web/controllers/user_html.ex
 rename lib/livebook_web/controllers/{auth_html => user_html}/logout.html.heex (100%)
 create mode 100644 test/livebook_web/controllers/user_controller_test.exs

diff --git a/lib/livebook/config.ex b/lib/livebook/config.ex
index 367e8569e5b..4dbaa4e1ffa 100644
--- a/lib/livebook/config.ex
+++ b/lib/livebook/config.ex
@@ -275,10 +275,7 @@ defmodule Livebook.Config do
   def logout_enabled?() do
     {_type, module, _key} = Livebook.Config.identity_provider()
 
-    identity_logout? =
-      Code.ensure_loaded?(module) and function_exported?(module, :logout, 2)
-
-    authentication().mode != :disabled or identity_logout?
+    Code.ensure_loaded?(module) and function_exported?(module, :logout, 2)
   end
 
   @doc """
diff --git a/lib/livebook_web/controllers/auth_controller.ex b/lib/livebook_web/controllers/auth_controller.ex
index f067a4bb58c..85f3bc4632c 100644
--- a/lib/livebook_web/controllers/auth_controller.ex
+++ b/lib/livebook_web/controllers/auth_controller.ex
@@ -46,26 +46,6 @@ defmodule LivebookWeb.AuthController do
     end
   end
 
-  def logout(conn, _params) do
-    if get_session(conn, :user_id) do
-      conn
-      |> configure_session(renew: true)
-      |> clear_session()
-      |> render("logout.html")
-    else
-      redirect_to(conn)
-    end
-  end
-
-  defp render_form_error(conn, authentication_mode) do
-    errors = [{"%{authentication_mode} is invalid", [authentication_mode: authentication_mode]}]
-
-    render(conn, "index.html",
-      errors: errors,
-      authentication_mode: authentication_mode
-    )
-  end
-
   defp redirect_to(conn) do
     conn
     |> then(fn conn ->
@@ -79,4 +59,13 @@ defmodule LivebookWeb.AuthController do
     end)
     |> halt()
   end
+
+  defp render_form_error(conn, authentication_mode) do
+    errors = [{"%{authentication_mode} is invalid", [authentication_mode: authentication_mode]}]
+
+    render(conn, "index.html",
+      errors: errors,
+      authentication_mode: authentication_mode
+    )
+  end
 end
diff --git a/lib/livebook_web/controllers/user_controller.ex b/lib/livebook_web/controllers/user_controller.ex
new file mode 100644
index 00000000000..d23e03efae3
--- /dev/null
+++ b/lib/livebook_web/controllers/user_controller.ex
@@ -0,0 +1,14 @@
+defmodule LivebookWeb.UserController do
+  use LivebookWeb, :controller
+
+  def logout(conn, _params) do
+    if get_session(conn, :user_id) do
+      conn
+      |> configure_session(renew: true)
+      |> clear_session()
+      |> render("logout.html")
+    else
+      redirect(conn, to: ~p"/")
+    end
+  end
+end
diff --git a/lib/livebook_web/controllers/user_html.ex b/lib/livebook_web/controllers/user_html.ex
new file mode 100644
index 00000000000..a54be57057c
--- /dev/null
+++ b/lib/livebook_web/controllers/user_html.ex
@@ -0,0 +1,5 @@
+defmodule LivebookWeb.UserHTML do
+  use LivebookWeb, :html
+
+  embed_templates "user_html/*"
+end
diff --git a/lib/livebook_web/controllers/auth_html/logout.html.heex b/lib/livebook_web/controllers/user_html/logout.html.heex
similarity index 100%
rename from lib/livebook_web/controllers/auth_html/logout.html.heex
rename to lib/livebook_web/controllers/user_html/logout.html.heex
diff --git a/lib/livebook_web/router.ex b/lib/livebook_web/router.ex
index c15c3a7f4de..6acb91d2498 100644
--- a/lib/livebook_web/router.ex
+++ b/lib/livebook_web/router.ex
@@ -171,7 +171,7 @@ defmodule LivebookWeb.Router do
 
   scope "/", LivebookWeb do
     pipe_through [:browser]
-    get "/logout", AuthController, :logout
+    get "/logout", UserController, :logout
   end
 
   defp within_iframe_secure_headers(conn, _opts) do
diff --git a/test/livebook_web/controllers/user_controller_test.exs b/test/livebook_web/controllers/user_controller_test.exs
new file mode 100644
index 00000000000..c18b3d4e6b9
--- /dev/null
+++ b/test/livebook_web/controllers/user_controller_test.exs
@@ -0,0 +1,29 @@
+defmodule LivebookWeb.UserControllerTest do
+  use LivebookWeb.ConnCase, async: true
+
+  describe "GET /logout" do
+    test "renders logout template when logged in", %{conn: conn} do
+      conn = login_user(conn)
+
+      conn = get(conn, ~p"/logout")
+
+      assert html_response(conn, 200) =~ "You have been logged out"
+    end
+
+    test "redirects when already logged out", %{conn: conn} do
+      conn = logout_user(conn)
+
+      conn = get(conn, ~p"/logout")
+
+      assert redirected_to(conn) == ~p"/"
+    end
+
+    defp login_user(conn) do
+      Phoenix.ConnTest.init_test_session(conn, %{user_id: 1})
+    end
+
+    defp logout_user(conn) do
+      Phoenix.ConnTest.init_test_session(conn, %{})
+    end
+  end
+end