Only touch session once an hour and keep user.last_seen in sync

lib/plausible/users.ex

@@ -35,6 +35,19 @@ defmodule Plausible.Users do
     |> Repo.update!()
   end
 
+  @spec bump_last_seen(Auth.User.t() | pos_integer(), NaiveDateTime.t()) :: :ok
+  def bump_last_seen(%Auth.User{id: user_id}, now) do
+    bump_last_seen(user_id, now)
+  end
+
+  def bump_last_seen(user_id, now) do
+    q = from(u in Auth.User, where: u.id == ^user_id)
+
+    Repo.update_all(q, set: [last_seen: now])
+
+    :ok
+  end
+
   @spec accept_traffic_until(Auth.User.t()) :: Date.t()
   on_ee do
     def accept_traffic_until(user) do

lib/plausible_web/router.ex

@@ -14,7 +14,6 @@ defmodule PlausibleWeb.Router do
     plug PlausibleWeb.SessionTimeoutPlug, timeout_after_seconds: @two_weeks_in_seconds
     plug PlausibleWeb.AuthPlug
     plug PlausibleWeb.Plugs.UserSessionTouch
-    plug PlausibleWeb.LastSeenPlug
   end
 
   pipeline :shared_link do

lib/plausible_web/user_auth.ex

@@ -68,9 +68,15 @@ defmodule PlausibleWeb.UserAuth do
   end
 
   def touch_user_session(user_session, now \\ NaiveDateTime.utc_now(:second)) do
-    user_session
-    |> Auth.UserSession.touch_session(now)
-    |> Repo.update!(allow_stale: true)
+    if NaiveDateTime.diff(now, user_session.last_used_at, :hour) >= 1 do
+      Plausible.Users.bump_last_seen(user_session.user_id, now)
+
+      user_session
+      |> Auth.UserSession.touch_session(now)
+      |> Repo.update!(allow_stale: true)
+    else
+      user_session
+    end
   end
 
   @doc """

test/plausible_web/controllers/site_controller_test.exs

@@ -954,7 +954,7 @@ defmodule PlausibleWeb.SiteControllerTest do
 
   describe "PUT /:website/settings/features/visibility/:setting" do
     def query_conn_with_some_url(context) do
-      {:ok, Map.put(context, :conn, get(context.conn, "/some_parent_path"))}
+      {:ok, Map.put(context, :conn_with_url, get(context.conn, "/some_parent_path"))}
     end
 
     setup [:create_user, :log_in, :query_conn_with_some_url]
@@ -966,7 +966,8 @@ defmodule PlausibleWeb.SiteControllerTest do
         } do
       test "can toggle #{title} with admin access", %{
         user: user,
-        conn: conn0
+        conn: conn0,
+        conn_with_url: conn_with_url
       } do
         site =
           insert(:site,
@@ -979,7 +980,12 @@ defmodule PlausibleWeb.SiteControllerTest do
         conn =
           put(
             conn0,
-            PlausibleWeb.Components.Site.Feature.target(site, unquote(setting), conn0, false)
+            PlausibleWeb.Components.Site.Feature.target(
+              site,
+              unquote(setting),
+              conn_with_url,
+              false
+            )
           )
 
         assert Phoenix.Flash.get(conn.assigns.flash, :success) ==
@@ -992,7 +998,12 @@ defmodule PlausibleWeb.SiteControllerTest do
         conn =
           put(
             conn0,
-            PlausibleWeb.Components.Site.Feature.target(site, unquote(setting), conn0, true)
+            PlausibleWeb.Components.Site.Feature.target(
+              site,
+              unquote(setting),
+              conn_with_url,
+              true
+            )
           )
 
         assert Phoenix.Flash.get(conn.assigns.flash, :success) ==
@@ -1011,23 +1022,33 @@ defmodule PlausibleWeb.SiteControllerTest do
         } do
       test "cannot toggle #{title} with viewer access", %{
         user: user,
-        conn: conn0
+        conn: conn0,
+        conn_with_url: conn_with_url
       } do
         site = insert(:site)
         insert(:site_membership, user: user, site: site, role: :viewer)
 
         conn =
           put(
             conn0,
-            PlausibleWeb.Components.Site.Feature.target(site, unquote(setting), conn0, false)
+            PlausibleWeb.Components.Site.Feature.target(
+              site,
+              unquote(setting),
+              conn_with_url,
+              false
+            )
           )
 
         assert conn.status == 404
         assert conn.halted
       end
     end
 
-    test "setting feature visibility is idempotent", %{user: user, conn: conn0} do
+    test "setting feature visibility is idempotent", %{
+      user: user,
+      conn: conn0,
+      conn_with_url: conn_with_url
+    } do
       site = insert(:site)
       insert(:site_membership, user: user, site: site, role: :admin)
 
@@ -1036,7 +1057,7 @@ defmodule PlausibleWeb.SiteControllerTest do
       conn =
         put(
           conn0,
-          PlausibleWeb.Components.Site.Feature.target(site, setting, conn0, false)
+          PlausibleWeb.Components.Site.Feature.target(site, setting, conn_with_url, false)
         )
 
       assert %{^setting => false} = Plausible.Sites.get_by_domain(site.domain)
@@ -1045,7 +1066,7 @@ defmodule PlausibleWeb.SiteControllerTest do
       conn =
         put(
           conn0,
-          PlausibleWeb.Components.Site.Feature.target(site, setting, conn0, false)
+          PlausibleWeb.Components.Site.Feature.target(site, setting, conn_with_url, false)
         )
 
       assert %{^setting => false} = Plausible.Sites.get_by_domain(site.domain)
@@ -1132,11 +1153,11 @@ defmodule PlausibleWeb.SiteControllerTest do
       site = insert(:site)
       insert(:weekly_report, site: site, recipients: ["[email protected]"])
 
-      conn = delete(conn, "/sites/#{site.domain}/weekly-report/recipients/[email protected]")
-      assert conn.status == 404
+      conn1 = delete(conn, "/sites/#{site.domain}/weekly-report/recipients/[email protected]")
+      assert conn1.status == 404
 
-      conn = delete(conn, "/sites/#{site.domain}/weekly-report/recipients/recipient%40email.com")
-      assert conn.status == 404
+      conn2 = delete(conn, "/sites/#{site.domain}/weekly-report/recipients/recipient%40email.com")
+      assert conn2.status == 404
 
       report = Repo.get_by(Plausible.Site.WeeklyReport, site_id: site.id)
       assert [_] = report.recipients
@@ -1215,11 +1236,13 @@ defmodule PlausibleWeb.SiteControllerTest do
       site = insert(:site)
       insert(:monthly_report, site: site, recipients: ["[email protected]"])
 
-      conn = delete(conn, "/sites/#{site.domain}/monthly-report/recipients/[email protected]")
-      assert conn.status == 404
+      conn1 = delete(conn, "/sites/#{site.domain}/monthly-report/recipients/[email protected]")
+      assert conn1.status == 404
 
-      conn = delete(conn, "/sites/#{site.domain}/monthly-report/recipients/recipient%40email.com")
-      assert conn.status == 404
+      conn2 =
+        delete(conn, "/sites/#{site.domain}/monthly-report/recipients/recipient%40email.com")
+
+      assert conn2.status == 404
 
       report = Repo.get_by(Plausible.Site.MonthlyReport, site_id: site.id)
       assert [_] = report.recipients

test/plausible_web/user_auth_test.exs

@@ -159,9 +159,46 @@ defmodule PlausibleWeb.UserAuthTest do
 
       assert refreshed_session.id == user_session.id
       assert NaiveDateTime.compare(refreshed_session.last_used_at, two_days_later) == :eq
+      assert NaiveDateTime.compare(Repo.reload(user).last_seen, two_days_later) == :eq
       assert NaiveDateTime.compare(refreshed_session.timeout_at, user_session.timeout_at) == :gt
     end
 
+    test "does not refresh if timestamps were updated less than hour before", %{user: user} do
+      %{sessions: [user_session]} = Repo.preload(user, :sessions)
+      user_session = Repo.reload(user_session)
+      last_seen = Repo.reload(user).last_seen
+
+      fifty_minutes_later =
+        NaiveDateTime.utc_now(:second)
+        |> NaiveDateTime.shift(minute: 50)
+
+      assert refreshed_session1 =
+               %Auth.UserSession{} =
+               UserAuth.touch_user_session(user_session, fifty_minutes_later)
+
+      assert NaiveDateTime.compare(
+               refreshed_session1.last_used_at,
+               user_session.last_used_at
+             ) == :eq
+
+      assert NaiveDateTime.compare(Repo.reload(user).last_seen, last_seen) == :eq
+
+      sixty_five_minutes_later =
+        NaiveDateTime.utc_now(:second)
+        |> NaiveDateTime.shift(minute: 65)
+
+      assert refreshed_session2 =
+               %Auth.UserSession{} =
+               UserAuth.touch_user_session(user_session, sixty_five_minutes_later)
+
+      assert NaiveDateTime.compare(
+               refreshed_session2.last_used_at,
+               sixty_five_minutes_later
+             ) == :eq
+
+      assert NaiveDateTime.compare(Repo.reload(user).last_seen, sixty_five_minutes_later) == :eq
+    end
+
     test "handles concurrent refresh gracefully", %{user: user} do
       %{sessions: [user_session]} = Repo.preload(user, :sessions)