Remove version from live_session (#3715)

Previously, live_session's had a version field that was generated randomly
whenever the router was compiled. We checked the version field on
live_redirects and enforce a full redirect (as when a user navigates,
that is a good point in time to force a full navigation, right?).

It turns out that live navigation on the client is actually implemented
in a way that every reconnect after the first live navigation is also
treated as a navigation. Therefore, after a deployment that changed the
router, LiveViews that were mounted through a live navigation were never
remounted, but always fully reloaded, losing any state and preventing
form recovery from working.

As the security mechanism of live_session is primarily based on the
live_session name, checking the name is generally enough. There could be
a case where previously a live_session called `:admin` was defined where
a user had access to and after deployment, those routes were instead
moved to a `:semiadmin` live session and now super sensitive routes are
accessible in the `:admin` live session. In this case, a user could try
to mount a route from this super sensitive section, but even then, those
routes SHOULD be protected by on_mount hooks that run and properly check
authorization, e.g. based on the user_id in the session.

So to sum this up, the version field of the live session is not needed,
causes problems at the moment and is therefore removed.

Author: SteffenDE

lib/phoenix_live_view/channel.ex

@@ -1287,8 +1287,7 @@ defmodule Phoenix.LiveView.Channel do
     %{
       root_view: root_view,
       assign_new: assign_new,
-      live_session_name: live_session_name,
-      live_session_vsn: live_session_vsn
+      live_session_name: live_session_name
     } = session
 
     {:ok,
@@ -1299,8 +1298,7 @@ defmodule Phoenix.LiveView.Channel do
        lifecycle: lifecycle,
        root_view: root_view,
        live_temp: %{},
-       live_session_name: live_session_name,
-       live_session_vsn: live_session_vsn
+       live_session_name: live_session_name
      }}
   end
 
@@ -1313,8 +1311,7 @@ defmodule Phoenix.LiveView.Channel do
     %{
       root_view: root_view,
       assign_new: assign_new,
-      live_session_name: live_session_name,
-      live_session_vsn: live_session_vsn
+      live_session_name: live_session_name
     } = session
 
     case sync_with_parent(parent, assign_new) do
@@ -1329,8 +1326,7 @@ defmodule Phoenix.LiveView.Channel do
            lifecycle: lifecycle,
            root_view: root_view,
            live_temp: %{},
-           live_session_name: live_session_name,
-           live_session_vsn: live_session_vsn
+           live_session_name: live_session_name
          }}
 
       {:error, :noproc} ->

lib/phoenix_live_view/router.ex

@@ -240,7 +240,6 @@ defmodule Phoenix.LiveView.Router do
   @doc false
   def __live_session__(module, opts, name) do
     Module.register_attribute(module, :phoenix_live_sessions, accumulate: true)
-    vsn = session_vsn(module)
 
     if not is_atom(name) do
       raise ArgumentError, """
@@ -264,7 +263,7 @@ defmodule Phoenix.LiveView.Router do
       """
     end
 
-    current = %{name: name, extra: extra, vsn: vsn}
+    current = %{name: name, extra: extra}
     Module.put_attribute(module, :phoenix_live_session_current, current)
 
     Module.put_attribute(module, :phoenix_live_sessions, name)
@@ -375,7 +374,7 @@ defmodule Phoenix.LiveView.Router do
       when is_atom(action) and is_list(opts) do
     live_session =
       Module.get_attribute(router, :phoenix_live_session_current) ||
-        %{name: :default, extra: %{}, vsn: session_vsn(router)}
+        %{name: :default, extra: %{}}
 
     helpers = Module.get_attribute(router, :phoenix_helpers)
 
@@ -488,14 +487,4 @@ defmodule Phoenix.LiveView.Router do
   end
 
   defp cookie_flash(%Plug.Conn{} = conn), do: {conn, nil}
-
-  defp session_vsn(module) do
-    if vsn = Module.get_attribute(module, :phoenix_session_vsn) do
-      vsn
-    else
-      vsn = System.system_time()
-      Module.put_attribute(module, :phoenix_session_vsn, vsn)
-      vsn
-    end
-  end
 end

lib/phoenix_live_view/session.ex

@@ -12,18 +12,15 @@ defmodule Phoenix.LiveView.Session do
             router: nil,
             flash: nil,
             live_session_name: nil,
-            live_session_vsn: nil,
             assign_new: []
 
   def main?(%Session{} = session), do: session.router != nil and session.parent_pid == nil
 
   def authorize_root_redirect(%Session{} = session, %Route{} = route) do
-    %Session{live_session_name: session_name, live_session_vsn: session_vsn} = session
+    %Session{live_session_name: session_name} = session
 
     case route.live_session do
-      # We check the version because if there was a new deploy,
-      # we can use this opportunity to reload the whole thing.
-      %{name: ^session_name, vsn: ^session_vsn} ->
+      %{name: ^session_name} ->
         {:ok, replace_root(session, route.view, self())}
 
       %{} ->
@@ -63,7 +60,7 @@ defmodule Phoenix.LiveView.Session do
          :ok <- verify_topic(topic, id),
          {:ok, static} <- verify_static_token(endpoint, id, static_token) do
       merged_session = Map.merge(session, static)
-      {live_session_name, vsn} = merged_session[:live_session] || {nil, nil}
+      live_session_name = merged_session[:live_session_name]
 
       session = %Session{
         id: id,
@@ -74,7 +71,6 @@ defmodule Phoenix.LiveView.Session do
         session: merged_session.session,
         assign_new: merged_session.assign_new,
         live_session_name: live_session_name,
-        live_session_vsn: vsn,
         # optional keys
         router: merged_session[:router],
         flash: merged_session[:flash]

lib/phoenix_live_view/static.ex

@@ -9,7 +9,7 @@ defmodule Phoenix.LiveView.Static do
   alias Phoenix.LiveView.{Socket, Utils, Diff, Route, Lifecycle}
 
   # Token version. Should be changed whenever new data is stored.
-  @token_vsn 5
+  @token_vsn 6
   @phoenix_reload_status "__phoenix_reload_status__"
 
   def token_vsn, do: @token_vsn
@@ -39,7 +39,7 @@ defmodule Phoenix.LiveView.Static do
 
   defp live_session(%Plug.Conn{} = conn) do
     case conn.private[:phoenix_live_view] do
-      {_view, _opts, %{name: _name, extra: _extra, vsn: _vsn} = lv_session} -> lv_session
+      {_view, _opts, %{name: _name, extra: _extra} = lv_session} -> lv_session
       nil -> nil
     end
   end
@@ -354,9 +354,9 @@ defmodule Phoenix.LiveView.Static do
   end
 
   defp sign_root_session(%Socket{} = socket, router, view, session, live_session) do
-    live_session_pair =
+    live_session_name =
       case live_session do
-        %{name: name, vsn: vsn} -> {name, vsn}
+        %{name: name} -> name
         nil -> nil
       end
 
@@ -366,7 +366,7 @@ defmodule Phoenix.LiveView.Static do
       view: view,
       root_view: view,
       router: router,
-      live_session: live_session_pair,
+      live_session_name: live_session_name,
       parent_pid: nil,
       root_pid: nil,
       session: session

test/phoenix_live_view/router_test.exs

@@ -86,7 +86,6 @@ defmodule Phoenix.LiveView.RouterTest do
                )
 
       assert route.live_session.name == :test
-      assert route.live_session.vsn
 
       assert conn |> get(path) |> html_response(200) |> verified_session() == %{}
     end
@@ -102,7 +101,6 @@ defmodule Phoenix.LiveView.RouterTest do
                )
 
       assert route.live_session.name == :admin
-      assert route.live_session.vsn
 
       assert conn |> get(path) |> html_response(200) |> verified_session() ==
                %{"admin" => true}
@@ -119,7 +117,6 @@ defmodule Phoenix.LiveView.RouterTest do
                )
 
       assert route.live_session.name == :mfa
-      assert route.live_session.vsn
 
       assert conn |> get(path) |> html_response(200) |> verified_session() ==
                %{"inlined" => true, "called" => true}
@@ -323,7 +320,7 @@ defmodule Phoenix.LiveView.RouterTest do
     test "classifies route as external when same view, but different session" do
       # previously, a patch to the same LV, but a different path in a different live_session
       # would succeed when it should not
-      {_, %Route{live_session: %{vsn: vsn}}} =
+      {_, %Route{live_session: %{name: :test}}} =
         Route.live_link_info_without_checks(
           @endpoint,
           Phoenix.LiveViewTest.Support.Router,
@@ -333,7 +330,7 @@ defmodule Phoenix.LiveView.RouterTest do
       socket = %Phoenix.LiveView.Socket{
         router: Phoenix.LiveViewTest.Support.Router,
         endpoint: @endpoint,
-        private: %{live_session_name: :test, live_session_vsn: vsn}
+        private: %{live_session_name: :test}
       }
 
       assert {:external, _} =