From a72d73f21c68a68eaed7920ace7a1314b13bbb7b Mon Sep 17 00:00:00 2001 From: Willem van den Ende Date: Tue, 27 Oct 2026 09:56:25 +0000 Subject: [PATCH] app: logout disconnects open LiveView sockets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `put_token_in_session/2` wrote only `:user_token`, so `log_out_user/1`'s `if live_socket_id = get_session(conn, :live_socket_id)` was always false and logging out left open LiveView sockets running. Write the socket topic alongside the token, copying phx.gen.auth's format verbatim ("users_sessions:" <> Base.url_encode64(token)) so it matches both what Phoenix.LiveView.Socket.id/1 reads and what log_out_user/1 reads. The reissue path goes through the same function, so it follows the new token. Tests (provider/reader agreement, both previously absent): - log_in_user/3 stores the id derived from the session token - fetch_current_scope_for_user/2 reissues the id with the new token - log_out_user/1 broadcasts exactly one "disconnect" on that id - the value written is the topic Socket.id/1 returns for the session the websocket receives (string keys) Not tested end to end: a real disconnect needs the transport process, which Phoenix.LiveViewTest does not start — Phoenix.Socket.__info__/2 handles the broadcast (deps/phoenix/lib/phoenix/socket.ex:571) and phoenix_live_view/test/client_proxy.ex never subscribes to the socket id topic. Gate: make check + make verify green (app coverage 86.97%, blogex 75.16%, crap sensors green). Yak: logout-must-disconnect-existing-liveview-sockets-livesocketid-never-set-6fme --- app/lib/firehose_web/user_auth.ex | 8 ++- app/test/firehose_web/user_auth_test.exs | 73 ++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/app/lib/firehose_web/user_auth.ex b/app/lib/firehose_web/user_auth.ex index 230fd28..85064b7 100644 --- a/app/lib/firehose_web/user_auth.ex +++ b/app/lib/firehose_web/user_auth.ex @@ -171,9 +171,15 @@ defmodule FirehoseWeb.UserAuth do end defp put_token_in_session(conn, token) do - put_session(conn, :user_token, token) + conn + |> put_session(:user_token, token) + |> put_session(:live_socket_id, user_session_topic(token)) end + # Must match what Phoenix.LiveView.Socket.id/1 reads out of the socket's + # connect_info session, so a "disconnect" broadcast reaches open sockets. + defp user_session_topic(token), do: "users_sessions:#{Base.url_encode64(token)}" + @doc """ Plug for routes that require sudo mode. """ diff --git a/app/test/firehose_web/user_auth_test.exs b/app/test/firehose_web/user_auth_test.exs index ba4d685..56ed92e 100644 --- a/app/test/firehose_web/user_auth_test.exs +++ b/app/test/firehose_web/user_auth_test.exs @@ -4,6 +4,7 @@ defmodule FirehoseWeb.UserAuthTest do alias Firehose.Accounts alias Firehose.Accounts.Scope alias FirehoseWeb.UserAuth + alias Phoenix.LiveView.Socket import Firehose.AccountsFixtures @@ -27,6 +28,13 @@ defmodule FirehoseWeb.UserAuthTest do assert Accounts.get_user_by_session_token(token) end + test "stores a live socket id derived from the session token", %{conn: conn, user: user} do + conn = UserAuth.log_in_user(conn, user) + token = get_session(conn, :user_token) + + assert get_session(conn, :live_socket_id) == "users_sessions:#{Base.url_encode64(token)}" + end + test "clears everything previously stored in the session", %{conn: conn, user: user} do conn = conn |> put_session(:to_be_removed, "value") |> UserAuth.log_in_user(user) refute get_session(conn, :to_be_removed) @@ -113,6 +121,21 @@ defmodule FirehoseWeb.UserAuthTest do refute Accounts.get_user_by_session_token(user_token) end + test "broadcasts a disconnect on the session's live socket id", %{conn: conn, user: user} do + logged_in_conn = UserAuth.log_in_user(conn, user) + token = get_session(logged_in_conn, :user_token) + live_socket_id = get_session(logged_in_conn, :live_socket_id) + FirehoseWeb.Endpoint.subscribe(live_socket_id) + + conn + |> put_session(:user_token, token) + |> put_session(:live_socket_id, live_socket_id) + |> UserAuth.log_out_user() + + assert_receive %Phoenix.Socket.Broadcast{event: "disconnect", topic: ^live_socket_id} + refute_receive %Phoenix.Socket.Broadcast{topic: ^live_socket_id} + end + test "works even if user is already logged out", %{conn: conn} do conn = conn |> fetch_cookies() |> UserAuth.log_out_user() refute get_session(conn, :user_token) @@ -183,6 +206,56 @@ defmodule FirehoseWeb.UserAuthTest do assert new_signed_token != signed_token assert max_age == @remember_me_cookie_max_age end + + test "reissues the live socket id together with the token", %{conn: conn, user: user} do + logged_in_conn = + conn |> fetch_cookies() |> UserAuth.log_in_user(user, %{"remember_me" => "true"}) + + token = logged_in_conn.cookies[@remember_me_cookie] + %{value: signed_token} = logged_in_conn.resp_cookies[@remember_me_cookie] + old_live_socket_id = get_session(logged_in_conn, :live_socket_id) + + offset_user_token(token, -10, :day) + + conn = + conn + |> put_session(:user_token, token) + |> put_session(:user_remember_me, true) + |> put_session(:live_socket_id, old_live_socket_id) + |> put_req_cookie(@remember_me_cookie, signed_token) + |> UserAuth.fetch_current_scope_for_user([]) + + assert new_token = get_session(conn, :user_token) + refute new_token == token + + assert get_session(conn, :live_socket_id) == + "users_sessions:#{Base.url_encode64(new_token)}" + + refute get_session(conn, :live_socket_id) == old_live_socket_id + end + end + + describe "live view sockets" do + # A real websocket disconnect cannot be driven through Phoenix.LiveViewTest: + # the "disconnect" broadcast is handled by Phoenix.Socket.__info__/2 on the + # transport process (deps/phoenix/lib/phoenix/socket.ex:571), and the test + # harness (phoenix_live_view/test/client_proxy.ex) never subscribes to the + # socket id topic. What can be pinned is the contract the transport relies + # on: the value in the session is the topic Phoenix.LiveView.Socket.id/1 + # returns for the session the websocket receives (string keys). + test "the id written to the session is the topic Socket.id/1 reads", %{conn: conn, user: user} do + conn = UserAuth.log_in_user(conn, user) + live_socket_id = get_session(conn, :live_socket_id) + + socket_session = + Map.new(conn.private[:plug_session], fn {key, value} -> {to_string(key), value} end) + + socket = %Socket{ + private: %{connect_info: %{session: socket_session}} + } + + assert Socket.id(socket) == live_socket_id + end end describe "require_sudo_mode/2" do