From ff71e21fe85f98d6ad61bf3229376fd25340a1f0 Mon Sep 17 00:00:00 2001 From: Adrian Gruntkowski Date: Tue, 3 Sep 2024 11:39:48 +0200 Subject: [PATCH] Revert "Implement token-based sessions (#4463)" This reverts commit 373d4dd6650a170fc11143d0cec46e3ed6feeba8. --- CHANGELOG.md | 1 - config/runtime.exs | 3 - lib/plausible/auth/user.ex | 1 - lib/plausible/auth/user_session.ex | 43 +-- lib/plausible/users.ex | 35 +-- lib/plausible_web.ex | 4 +- lib/plausible_web/live/auth_context.ex | 6 +- lib/plausible_web/live/sentry_context.ex | 14 +- lib/plausible_web/plugs/auth_plug.ex | 20 +- lib/plausible_web/plugs/last_seen_plug.ex | 37 +++ .../plugs/session_timeout_plug.ex | 13 +- .../plugs/super_admin_only_plug.ex | 16 +- lib/plausible_web/plugs/user_session_touch.ex | 28 -- lib/plausible_web/router.ex | 39 +-- lib/plausible_web/user_auth.ex | 208 +++---------- lib/workers/clean_user_sessions.ex | 27 -- .../api/stats_controller/browsers_test.exs | 8 +- .../api/stats_controller/countries_test.exs | 8 +- .../operating_systems_test.exs | 16 +- .../api/stats_controller/pages_test.exs | 24 +- .../stats_controller/screen_sizes_test.exs | 18 +- .../api/stats_controller/sources_test.exs | 96 +++--- .../api/stats_controller/suggestions_test.exs | 4 +- .../controllers/auth_controller_test.exs | 87 +++--- .../controllers/page_controller_test.exs | 9 +- .../controllers/site_controller_test.exs | 57 ++-- .../live/sentry_context_test.exs | 10 +- test/plausible_web/plugs/auth_plug_test.exs | 25 +- .../plugs/session_timeout_plug_test.exs | 16 +- .../plugs/user_session_touch_test.exs | 57 ---- test/plausible_web/user_auth_test.exs | 275 ++++-------------- test/support/test_utils.ex | 4 +- test/workers/clean_user_sessions_test.exs | 34 --- 33 files changed, 364 insertions(+), 879 deletions(-) create mode 100644 lib/plausible_web/plugs/last_seen_plug.ex delete mode 100644 lib/plausible_web/plugs/user_session_touch.ex delete mode 100644 lib/workers/clean_user_sessions.ex delete mode 100644 test/plausible_web/plugs/user_session_touch_test.exs delete mode 100644 test/workers/clean_user_sessions_test.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index be54a7a5153a..e654ffd2698b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,7 +35,6 @@ All notable changes to this project will be documented in this file. - Make `TOTP_VAULT_KEY` optional plausible/analytics#4317 - Sources like 'google' and 'facebook' are now stored in capitalized forms ('Google', 'Facebook') plausible/analytics#4417 - `DATABASE_CACERTFILE` now forces TLS for PostgreSQL connections, so you don't need to add `?ssl=true` in `DATABASE_URL` -- Change auth session cookies to token-based ones with server-side expiration management. ### Fixed diff --git a/config/runtime.exs b/config/runtime.exs index 686ad6fa49e3..0e6a674e7d8a 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -587,8 +587,6 @@ base_cron = [ # Every day at 1am {"0 1 * * *", Plausible.Workers.CleanInvitations}, # Every 2 hours - {"30 */2 * * *", Plausible.Workers.CleanUserSessions}, - # Every 2 hours {"0 */2 * * *", Plausible.Workers.ExpireDomainChangeTransitions}, # Daily at midnight {"0 0 * * *", Plausible.Workers.LocationsSync} @@ -619,7 +617,6 @@ base_queues = [ check_stats_emails: 1, site_setup_emails: 1, clean_invitations: 1, - clean_user_sessions: 1, analytics_imports: 1, analytics_exports: 1, notify_exported_analytics: 1, diff --git a/lib/plausible/auth/user.ex b/lib/plausible/auth/user.ex index 539fc09043d0..47d5503ee4fb 100644 --- a/lib/plausible/auth/user.ex +++ b/lib/plausible/auth/user.ex @@ -47,7 +47,6 @@ defmodule Plausible.Auth.User do embeds_one :grace_period, Plausible.Auth.GracePeriod, on_replace: :update - has_many :sessions, Plausible.Auth.UserSession has_many :site_memberships, Plausible.Site.Membership has_many :sites, through: [:site_memberships, :site] has_many :api_keys, Plausible.Auth.ApiKey diff --git a/lib/plausible/auth/user_session.ex b/lib/plausible/auth/user_session.ex index 48d401808055..84e340d19890 100644 --- a/lib/plausible/auth/user_session.ex +++ b/lib/plausible/auth/user_session.ex @@ -5,48 +5,9 @@ defmodule Plausible.Auth.UserSession do use Ecto.Schema - import Ecto.Changeset - - alias Plausible.Auth - @type t() :: %__MODULE__{} - @rand_size 32 - @timeout Duration.new!(day: 14) - - schema "user_sessions" do - field :token, :binary - field :device, :string - field :last_used_at, :naive_datetime - field :timeout_at, :naive_datetime - - belongs_to :user, Plausible.Auth.User - - timestamps(updated_at: false) - end - - @spec timeout_duration() :: Duration.t() - def timeout_duration(), do: @timeout - - @spec new_session(Auth.User.t(), String.t(), NaiveDateTime.t()) :: Ecto.Changeset.t() - def new_session(user, device, now \\ NaiveDateTime.utc_now(:second)) do - %__MODULE__{} - |> cast(%{device: device}, [:device]) - |> generate_token() - |> put_assoc(:user, user) - |> touch_session(now) - end - - @spec touch_session(t() | Ecto.Changeset.t(), NaiveDateTime.t()) :: Ecto.Changeset.t() - def touch_session(session, now \\ NaiveDateTime.utc_now(:second)) do - session - |> change() - |> put_change(:last_used_at, now) - |> put_change(:timeout_at, NaiveDateTime.shift(now, @timeout)) - end - - defp generate_token(changeset) do - token = :crypto.strong_rand_bytes(@rand_size) - put_change(changeset, :token, token) + embedded_schema do + field :user_id, :integer end end diff --git a/lib/plausible/users.ex b/lib/plausible/users.ex index 39a32d78d3a1..bb0c877eacbf 100644 --- a/lib/plausible/users.ex +++ b/lib/plausible/users.ex @@ -35,19 +35,6 @@ 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 @@ -77,18 +64,19 @@ defmodule Plausible.Users do end end - def with_subscription(%Auth.User{} = user) do - Repo.preload(user, subscription: last_subscription_query()) + def with_subscription(%Auth.User{id: user_id} = user) do + Repo.preload(user, subscription: last_subscription_query(user_id)) end def with_subscription(user_id) when is_integer(user_id) do Repo.one( from(user in Auth.User, - as: :user, - left_lateral_join: s in subquery(last_subscription_join_query()), - on: true, + left_join: last_subscription in subquery(last_subscription_query(user_id)), + on: last_subscription.user_id == user.id, + left_join: subscription in Subscription, + on: subscription.id == last_subscription.id, where: user.id == ^user_id, - preload: [subscription: s] + preload: [subscription: subscription] ) ) end @@ -114,14 +102,9 @@ defmodule Plausible.Users do end end - def last_subscription_join_query() do - from(subscription in last_subscription_query(), - where: subscription.user_id == parent_as(:user).id - ) - end - - defp last_subscription_query() do + defp last_subscription_query(user_id) do from(subscription in Subscription, + where: subscription.user_id == ^user_id, order_by: [desc: subscription.inserted_at], limit: 1 ) diff --git a/lib/plausible_web.ex b/lib/plausible_web.ex index b4f75b69950f..070b6bcc6cf1 100644 --- a/lib/plausible_web.ex +++ b/lib/plausible_web.ex @@ -5,12 +5,12 @@ defmodule PlausibleWeb do use Phoenix.LiveView, global_prefixes: ~w(x-) use PlausibleWeb.Live.Flash - use PlausibleWeb.Live.AuthContext - unless :no_sentry_context in unquote(opts) do use PlausibleWeb.Live.SentryContext end + use PlausibleWeb.Live.AuthContext + alias PlausibleWeb.Router.Helpers, as: Routes alias Phoenix.LiveView.JS end diff --git a/lib/plausible_web/live/auth_context.ex b/lib/plausible_web/live/auth_context.ex index c58e1b612b07..4b982aedf923 100644 --- a/lib/plausible_web/live/auth_context.ex +++ b/lib/plausible_web/live/auth_context.ex @@ -26,8 +26,10 @@ defmodule PlausibleWeb.Live.AuthContext do end end) |> assign_new(:current_user, fn context -> - case context.current_user_session do - %{user: user} -> user + with %{} = user_session <- context.current_user_session, + {:ok, user} <- UserAuth.get_user(user_session) do + user + else _ -> nil end end) diff --git a/lib/plausible_web/live/sentry_context.ex b/lib/plausible_web/live/sentry_context.ex index 1f8c68a44af5..93c5ef07d22a 100644 --- a/lib/plausible_web/live/sentry_context.ex +++ b/lib/plausible_web/live/sentry_context.ex @@ -18,7 +18,7 @@ defmodule PlausibleWeb.Live.SentryContext do end end - def on_mount(:default, _params, _session, socket) do + def on_mount(:default, _params, session, socket) do if Phoenix.LiveView.connected?(socket) do peer = Phoenix.LiveView.get_connect_info(socket, :peer_data) uri = Phoenix.LiveView.get_connect_info(socket, :uri) @@ -49,10 +49,14 @@ defmodule PlausibleWeb.Live.SentryContext do Sentry.Context.set_request_context(request_context) - if current_user = socket.assigns[:current_user] do - Sentry.Context.set_user_context(%{ - id: current_user.id - }) + case PlausibleWeb.UserAuth.get_user_session(session) do + {:ok, user_session} -> + Sentry.Context.set_user_context(%{ + id: user_session.user_id + }) + + _ -> + :pass end end diff --git a/lib/plausible_web/plugs/auth_plug.ex b/lib/plausible_web/plugs/auth_plug.ex index 7f2a0809bc29..68355c565c29 100644 --- a/lib/plausible_web/plugs/auth_plug.ex +++ b/lib/plausible_web/plugs/auth_plug.ex @@ -16,17 +16,15 @@ defmodule PlausibleWeb.AuthPlug do end def call(conn, _opts) do - case UserAuth.get_user_session(conn) do - {:ok, user_session} -> - user = user_session.user - - Plausible.OpenTelemetry.add_user_attributes(user) - Sentry.Context.set_user_context(%{id: user.id, name: user.name, email: user.email}) - - conn - |> assign(:current_user, user) - |> assign(:current_user_session, user_session) - + with {:ok, user_session} <- UserAuth.get_user_session(conn), + {:ok, user} <- UserAuth.get_user(user_session) do + Plausible.OpenTelemetry.add_user_attributes(user) + Sentry.Context.set_user_context(%{id: user.id, name: user.name, email: user.email}) + + conn + |> assign(:current_user, user) + |> assign(:current_user_session, user_session) + else _ -> conn end diff --git a/lib/plausible_web/plugs/last_seen_plug.ex b/lib/plausible_web/plugs/last_seen_plug.ex new file mode 100644 index 000000000000..5a935d62f0c7 --- /dev/null +++ b/lib/plausible_web/plugs/last_seen_plug.ex @@ -0,0 +1,37 @@ +defmodule PlausibleWeb.LastSeenPlug do + import Plug.Conn + use Plausible.Repo + + @one_hour 60 * 60 + + def init(opts) do + opts + end + + def call(conn, _opts) do + last_seen = get_session(conn, :last_seen) + user = conn.assigns[:current_user] + + cond do + user && last_seen && last_seen < unix_now() - @one_hour -> + persist_last_seen(user) + put_session(conn, :last_seen, unix_now()) + + user && !last_seen -> + put_session(conn, :last_seen, unix_now()) + + true -> + conn + end + end + + defp persist_last_seen(user) do + q = from(u in Plausible.Auth.User, where: u.id == ^user.id) + + Repo.update_all(q, set: [last_seen: DateTime.utc_now()]) + end + + defp unix_now do + DateTime.utc_now() |> DateTime.to_unix() + end +end diff --git a/lib/plausible_web/plugs/session_timeout_plug.ex b/lib/plausible_web/plugs/session_timeout_plug.ex index 7ba60f226399..90a4501344dd 100644 --- a/lib/plausible_web/plugs/session_timeout_plug.ex +++ b/lib/plausible_web/plugs/session_timeout_plug.ex @@ -6,19 +6,24 @@ defmodule PlausibleWeb.SessionTimeoutPlug do """ import Plug.Conn + alias PlausibleWeb.UserAuth + def init(opts \\ []) do opts end def call(conn, opts) do timeout_at = get_session(conn, :session_timeout_at) - user_id = get_session(conn, :current_user_id) + + user_id = + case UserAuth.get_user_session(conn) do + {:ok, session} -> session.user_id + _ -> nil + end cond do user_id && timeout_at && now() > timeout_at -> - conn - |> PlausibleWeb.UserAuth.log_out_user() - |> halt() + PlausibleWeb.UserAuth.log_out_user(conn) user_id -> put_session( diff --git a/lib/plausible_web/plugs/super_admin_only_plug.ex b/lib/plausible_web/plugs/super_admin_only_plug.ex index cb67150e99b2..e369a8374dfc 100644 --- a/lib/plausible_web/plugs/super_admin_only_plug.ex +++ b/lib/plausible_web/plugs/super_admin_only_plug.ex @@ -1,24 +1,20 @@ defmodule PlausibleWeb.SuperAdminOnlyPlug do @moduledoc false - use Plausible.Repo - import Plug.Conn + use Plausible.Repo def init(options) do options end def call(conn, _opts) do - current_user = conn.assigns[:current_user] - - if current_user && Plausible.Auth.is_super_admin?(current_user) do - conn + with {:ok, user} <- PlausibleWeb.UserAuth.get_user(conn), + true <- Plausible.Auth.is_super_admin?(user) do + assign(conn, :current_user, user) else - conn - |> PlausibleWeb.UserAuth.log_out_user() - |> send_resp(403, "Not allowed") - |> halt() + _ -> + conn |> send_resp(403, "Not allowed") |> halt end end end diff --git a/lib/plausible_web/plugs/user_session_touch.ex b/lib/plausible_web/plugs/user_session_touch.ex deleted file mode 100644 index 560b68be009f..000000000000 --- a/lib/plausible_web/plugs/user_session_touch.ex +++ /dev/null @@ -1,28 +0,0 @@ -defmodule PlausibleWeb.Plugs.UserSessionTouch do - @moduledoc """ - Plug for bumping timeout on user session on every dashboard request. - """ - - import Plug.Conn - - alias PlausibleWeb.UserAuth - - def init(opts \\ []) do - opts - end - - def call(conn, _opts) do - # NOTE: Needed only during transitional 14-day period - conn = UserAuth.convert_legacy_session(conn) - - if user_session = conn.assigns[:current_user_session] do - assign( - conn, - :current_user_session, - UserAuth.touch_user_session(user_session) - ) - else - conn - end - end -end diff --git a/lib/plausible_web/router.ex b/lib/plausible_web/router.ex index 0e84330a6a32..571676b9e604 100644 --- a/lib/plausible_web/router.ex +++ b/lib/plausible_web/router.ex @@ -13,7 +13,7 @@ defmodule PlausibleWeb.Router do on_ee(do: nil, else: plug(PlausibleWeb.FirstLaunchPlug, redirect_to: "/register")) 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 @@ -30,10 +30,6 @@ defmodule PlausibleWeb.Router do plug :put_root_layout, html: {PlausibleWeb.LayoutView, :app} end - pipeline :external_api do - plug :accepts, ["json"] - end - pipeline :api do plug :accepts, ["json"] plug :fetch_session @@ -43,7 +39,6 @@ defmodule PlausibleWeb.Router do pipeline :internal_stats_api do plug :accepts, ["json"] plug :fetch_session - plug PlausibleWeb.AuthPlug plug PlausibleWeb.AuthorizeSiteAccess plug PlausibleWeb.Plugs.NoRobots end @@ -59,7 +54,6 @@ defmodule PlausibleWeb.Router do plug PlausibleWeb.Plugs.NoRobots plug :fetch_session - plug PlausibleWeb.AuthPlug plug PlausibleWeb.SuperAdminOnlyPlug end end @@ -71,11 +65,7 @@ defmodule PlausibleWeb.Router do on_ee do use Kaffy.Routes, scope: "/crm", - pipe_through: [ - PlausibleWeb.Plugs.NoRobots, - PlausibleWeb.AuthPlug, - PlausibleWeb.SuperAdminOnlyPlug - ] + pipe_through: [PlausibleWeb.Plugs.NoRobots, PlausibleWeb.SuperAdminOnlyPlug] end on_ee do @@ -190,7 +180,7 @@ defmodule PlausibleWeb.Router do scope "/api/docs", PlausibleWeb.Api do get "/query/schema.json", ExternalQueryApiController, :schema - scope [] do + scope assigns: %{} do pipe_through :internal_stats_api post "/query", ExternalQueryApiController, :query @@ -223,24 +213,19 @@ defmodule PlausibleWeb.Router do end scope "/api", PlausibleWeb do - scope [] do - pipe_through :external_api + pipe_through :api - post "/event", Api.ExternalController, :event - get "/error", Api.ExternalController, :error - get "/health", Api.ExternalController, :health - get "/system", Api.ExternalController, :info - end + post "/event", Api.ExternalController, :event + get "/error", Api.ExternalController, :error + get "/health", Api.ExternalController, :health + get "/system", Api.ExternalController, :info - scope [] do - pipe_through :api - post "/paddle/webhook", Api.PaddleController, :webhook - get "/paddle/currency", Api.PaddleController, :currency + post "/paddle/webhook", Api.PaddleController, :webhook + get "/paddle/currency", Api.PaddleController, :currency - put "/:domain/disable-feature", Api.InternalController, :disable_feature + put "/:domain/disable-feature", Api.InternalController, :disable_feature - get "/sites", Api.InternalController, :sites - end + get "/sites", Api.InternalController, :sites end scope "/", PlausibleWeb do diff --git a/lib/plausible_web/user_auth.ex b/lib/plausible_web/user_auth.ex index c850b2c08735..72d46b70340f 100644 --- a/lib/plausible_web/user_auth.ex +++ b/lib/plausible_web/user_auth.ex @@ -5,23 +5,21 @@ defmodule PlausibleWeb.UserAuth do In it's current shape, both current (legacy) and soon to be implemented (new) user sessions are supported side by side. - The legacy token is still accepted from the session cookie. Once 14 days - pass (the current time window for which session cookie is valid without - any activity), the legacy cookies won't be accepted anymore (legacy token - retrieval is tracked with logging) and the logic will be cleaned of branching - for legacy session. + Once the token-based sessions are implemented, `create_user_session/1` will + start returning new token instead of the legacy one. At the same time, + `put_token_in_session/2` will always set the new token. The legacy token will + still be accepted from the session cookie. Once 14 days pass (the current time + window for which session cookie is valid without any activity), the legacy + cookies won't be accepted anymore (token retrieval will most likely be + instrumented to confirm the usage falls in the mentioned time window as + expected) and the logic will be cleaned of branching for legacy session. """ - import Ecto.Query, only: [from: 2] - alias Plausible.Auth - alias Plausible.Repo alias PlausibleWeb.TwoFactor alias PlausibleWeb.Router.Helpers, as: Routes - require Logger - @spec log_in_user(Plug.Conn.t(), Auth.User.t(), String.t() | nil) :: Plug.Conn.t() def log_in_user(conn, user, redirect_path \\ nil) do login_dest = @@ -49,33 +47,27 @@ defmodule PlausibleWeb.UserAuth do |> clear_logged_in_cookie() end - @spec get_user_session(Plug.Conn.t() | map()) :: - {:ok, Auth.UserSession.t()} | {:error, :no_valid_token | :session_not_found} - def get_user_session(%Plug.Conn{assigns: %{current_user_session: user_session}}) do - {:ok, user_session} - end - - def get_user_session(conn_or_session) do - with {:ok, token} <- get_user_token(conn_or_session) do - get_session_by_token(token) + @spec get_user(Plug.Conn.t() | Auth.UserSession.t() | map()) :: + {:ok, Auth.User.t()} | {:error, :no_valid_token | :session_not_found | :user_not_found} + def get_user(%Auth.UserSession{} = user_session) do + if user = Plausible.Users.with_subscription(user_session.user_id) do + {:ok, user} + else + {:error, :user_not_found} end end - @spec touch_user_session(Auth.UserSession.t()) :: Auth.UserSession.t() - def touch_user_session(%{token: nil} = user_session) do - # NOTE: Legacy token sessions can't be touched. - user_session + def get_user(conn_or_session) do + with {:ok, user_session} <- get_user_session(conn_or_session) do + get_user(user_session) + end end - def touch_user_session(user_session, now \\ NaiveDateTime.utc_now(:second)) do - 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 + @spec get_user_session(Plug.Conn.t() | map()) :: + {:ok, map()} | {:error, :no_valid_token | :session_not_found} + def get_user_session(conn_or_session) do + with {:ok, token} <- get_user_token(conn_or_session) do + get_session_by_token(token) end end @@ -94,58 +86,16 @@ defmodule PlausibleWeb.UserAuth do ) end - @spec convert_legacy_session(Plug.Conn.t()) :: Plug.Conn.t() - def convert_legacy_session(conn) do - current_user = conn.assigns[:current_user] - - if current_user && Plug.Conn.get_session(conn, :current_user_id) do - {token, user_session} = create_user_session(conn, current_user) - - conn - |> put_token_in_session(token) - |> Plug.Conn.delete_session(:current_user_id) - |> Plug.Conn.assign(:current_user_session, user_session) - else - conn - end - end - defp get_session_by_token({:legacy, user_id}) do - case Plausible.Users.with_subscription(user_id) do - %Auth.User{} = user -> - {:ok, %Auth.UserSession{user_id: user.id, user: user}} - - nil -> - {:error, :session_not_found} - end + {:ok, %Auth.UserSession{user_id: user_id}} end - defp get_session_by_token({:new, token}) do - now = NaiveDateTime.utc_now(:second) - - last_subscription_query = Plausible.Users.last_subscription_join_query() - - token_query = - from(us in Auth.UserSession, - inner_join: u in assoc(us, :user), - as: :user, - left_lateral_join: s in subquery(last_subscription_query), - on: true, - where: us.token == ^token and us.timeout_at > ^now, - preload: [user: {u, subscription: s}] - ) - - case Repo.one(token_query) do - %Auth.UserSession{} = user_session -> - {:ok, user_session} - - nil -> - {:error, :session_not_found} - end + defp get_session_by_token({:new, _token}) do + {:error, :session_not_found} end defp set_user_session(conn, user) do - {token, _} = create_user_session(conn, user) + {token, _} = create_user_session(user) conn |> renew_session() @@ -168,7 +118,11 @@ defmodule PlausibleWeb.UserAuth do defp put_token_in_session(conn, {:new, token}) do conn |> Plug.Conn.put_session(:user_token, token) - |> Plug.Conn.put_session(:live_socket_id, "user_sessions:#{Base.url_encode64(token)}") + |> Plug.Conn.put_session(:live_socket_id, "users_sessions:#{Base.url_encode64(token)}") + end + + defp put_token_in_session(conn, {:legacy, user_id}) do + Plug.Conn.put_session(conn, :current_user_id, user_id) end defp get_user_token(%Plug.Conn{} = conn) do @@ -177,92 +131,26 @@ defmodule PlausibleWeb.UserAuth do |> get_user_token() end - defp get_user_token(%{"user_token" => token}) when is_binary(token) do - {:ok, {:new, token}} - end - - defp get_user_token(%{"current_user_id" => user_id}) when is_integer(user_id) do - Logger.warning("Legacy user session detected (user: #{user_id})") - {:ok, {:legacy, user_id}} - end - - defp get_user_token(_) do - {:error, :no_valid_token} - end - - defp create_user_session(conn, user) do - device_name = get_device_name(conn) - - user_session = - user - |> Auth.UserSession.new_session(device_name) - |> Repo.insert!() - - {{:new, user_session.token}, user_session} - end - - defp remove_user_session({:legacy, _}), do: :ok - - defp remove_user_session({:new, token}) do - Repo.delete_all(from us in Auth.UserSession, where: us.token == ^token) - :ok - end - - @unknown_label "Unknown" - - defp get_device_name(%Plug.Conn{} = conn) do - conn - |> Plug.Conn.get_req_header("user-agent") - |> List.first() - |> get_device_name() - end - - defp get_device_name(user_agent) when is_binary(user_agent) do - case UAInspector.parse(user_agent) do - %UAInspector.Result{client: %UAInspector.Result.Client{name: "Headless Chrome"}} -> - "Headless Chrome" - - %UAInspector.Result.Bot{name: name} when is_binary(name) -> - name - - %UAInspector.Result{} = ua -> - browser = browser_name(ua) - - if os = os_name(ua) do - browser <> " (#{os})" - else - browser - end - - _ -> - @unknown_label + defp get_user_token(session) do + case Enum.map(["user_token", "current_user_id"], &Map.get(session, &1)) do + [token, nil] when is_binary(token) -> {:ok, {:new, token}} + [nil, current_user_id] when is_integer(current_user_id) -> {:ok, {:legacy, current_user_id}} + [nil, nil] -> {:error, :no_valid_token} end end - defp get_device_name(_), do: @unknown_label - - defp browser_name(ua) do - case ua.client do - :unknown -> @unknown_label - %UAInspector.Result.Client{name: "Mobile Safari"} -> "Safari" - %UAInspector.Result.Client{name: "Chrome Mobile"} -> "Chrome" - %UAInspector.Result.Client{name: "Chrome Mobile iOS"} -> "Chrome" - %UAInspector.Result.Client{name: "Firefox Mobile"} -> "Firefox" - %UAInspector.Result.Client{name: "Firefox Mobile iOS"} -> "Firefox" - %UAInspector.Result.Client{name: "Opera Mobile"} -> "Opera" - %UAInspector.Result.Client{name: "Opera Mini"} -> "Opera" - %UAInspector.Result.Client{name: "Opera Mini iOS"} -> "Opera" - %UAInspector.Result.Client{name: "Yandex Browser Lite"} -> "Yandex Browser" - %UAInspector.Result.Client{name: "Chrome Webview"} -> "Mobile App" - %UAInspector.Result.Client{type: "mobile app"} -> "Mobile App" - client -> client.name || @unknown_label + defp create_user_session(user) do + # NOTE: a temporary fix for for dialyzer + # complaining about unreachable code + # path. + if :erlang.phash2(1, 1) == 0 do + {{:legacy, user.id}, %{}} + else + {{:new, "disabled-for-now"}, %{}} end end - defp os_name(ua) do - case ua.os do - :unknown -> nil - os -> os.name - end + defp remove_user_session(_token) do + :ok end end diff --git a/lib/workers/clean_user_sessions.ex b/lib/workers/clean_user_sessions.ex deleted file mode 100644 index 8b2640e0ad65..000000000000 --- a/lib/workers/clean_user_sessions.ex +++ /dev/null @@ -1,27 +0,0 @@ -defmodule Plausible.Workers.CleanUserSessions do - @moduledoc """ - Job removing expired user sessions. A grace period is applied. - """ - - use Plausible.Repo - use Oban.Worker, queue: :clean_user_sessions - - @grace_period Duration.new!(day: -7) - - @spec grace_period_duration() :: Duration.t() - def grace_period_duration(), do: @grace_period - - @impl Oban.Worker - def perform(_job) do - grace_cutoff = - NaiveDateTime.utc_now(:second) - |> NaiveDateTime.shift(@grace_period) - - Repo.delete_all( - from us in Plausible.Auth.UserSession, - where: us.timeout_at < ^grace_cutoff - ) - - :ok - end -end diff --git a/test/plausible_web/controllers/api/stats_controller/browsers_test.exs b/test/plausible_web/controllers/api/stats_controller/browsers_test.exs index 4020847b87bf..34b1f9867670 100644 --- a/test/plausible_web/controllers/api/stats_controller/browsers_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/browsers_test.exs @@ -123,15 +123,15 @@ defmodule PlausibleWeb.Api.StatsController.BrowsersTest do build(:imported_visitors, visitors: 2) ]) - conn1 = get(conn, "/api/stats/#{site.domain}/browsers?period=day") + conn = get(conn, "/api/stats/#{site.domain}/browsers?period=day") - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"name" => "Chrome", "visitors" => 1, "percentage" => 100} ] - conn2 = get(conn, "/api/stats/#{site.domain}/browsers?period=day&with_imported=true") + conn = get(conn, "/api/stats/#{site.domain}/browsers?period=day&with_imported=true") - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"name" => "Chrome", "visitors" => 2, "percentage" => 66.7}, %{"name" => "Firefox", "visitors" => 1, "percentage" => 33.3} ] diff --git a/test/plausible_web/controllers/api/stats_controller/countries_test.exs b/test/plausible_web/controllers/api/stats_controller/countries_test.exs index c9307a7ce183..565974d061fb 100644 --- a/test/plausible_web/controllers/api/stats_controller/countries_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/countries_test.exs @@ -14,9 +14,9 @@ defmodule PlausibleWeb.Api.StatsController.CountriesTest do build(:imported_visitors, visitors: 2) ]) - conn1 = get(conn, "/api/stats/#{site.domain}/countries?period=day") + conn = get(conn, "/api/stats/#{site.domain}/countries?period=day") - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "code" => "EE", "alpha_3" => "EST", @@ -35,9 +35,9 @@ defmodule PlausibleWeb.Api.StatsController.CountriesTest do } ] - conn2 = get(conn, "/api/stats/#{site.domain}/countries?period=day&with_imported=true") + conn = get(conn, "/api/stats/#{site.domain}/countries?period=day&with_imported=true") - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "code" => "EE", "alpha_3" => "EST", diff --git a/test/plausible_web/controllers/api/stats_controller/operating_systems_test.exs b/test/plausible_web/controllers/api/stats_controller/operating_systems_test.exs index 491e393c5257..79413ad7197f 100644 --- a/test/plausible_web/controllers/api/stats_controller/operating_systems_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/operating_systems_test.exs @@ -29,19 +29,19 @@ defmodule PlausibleWeb.Api.StatsController.OperatingSystemsTest do ) ]) - conn1 = get(conn, "/api/stats/#{site.domain}/operating-systems?period=day") + conn = get(conn, "/api/stats/#{site.domain}/operating-systems?period=day") - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"name" => "(not set)", "visitors" => 1, "percentage" => 50}, %{"name" => "Linux", "visitors" => 1, "percentage" => 50} ] filters = Jason.encode!(%{os: "(not set)"}) - conn2 = + conn = get(conn, "/api/stats/#{site.domain}/operating-systems?period=day&filters=#{filters}") - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"name" => "(not set)", "visitors" => 1, "percentage" => 100} ] end @@ -172,17 +172,17 @@ defmodule PlausibleWeb.Api.StatsController.OperatingSystemsTest do build(:imported_visitors, visitors: 2) ]) - conn1 = get(conn, "/api/stats/#{site.domain}/operating-systems?period=day") + conn = get(conn, "/api/stats/#{site.domain}/operating-systems?period=day") - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"name" => "Mac", "visitors" => 2, "percentage" => 66.7}, %{"name" => "Android", "visitors" => 1, "percentage" => 33.3} ] - conn2 = + conn = get(conn, "/api/stats/#{site.domain}/operating-systems?period=day&with_imported=true") - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"name" => "Mac", "visitors" => 3, "percentage" => 60}, %{"name" => "Android", "visitors" => 2, "percentage" => 40} ] diff --git a/test/plausible_web/controllers/api/stats_controller/pages_test.exs b/test/plausible_web/controllers/api/stats_controller/pages_test.exs index 9f384d693c37..f22ac46e4031 100644 --- a/test/plausible_web/controllers/api/stats_controller/pages_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/pages_test.exs @@ -860,17 +860,17 @@ defmodule PlausibleWeb.Api.StatsController.PagesTest do build(:pageview, pathname: "/contact") ]) - conn1 = get(conn, "/api/stats/#{site.domain}/pages?period=day") + conn = get(conn, "/api/stats/#{site.domain}/pages?period=day") - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"visitors" => 3, "name" => "/"}, %{"visitors" => 2, "name" => "/register"}, %{"visitors" => 1, "name" => "/contact"} ] - conn2 = get(conn, "/api/stats/#{site.domain}/pages?period=day&with_imported=true") + conn = get(conn, "/api/stats/#{site.domain}/pages?period=day&with_imported=true") - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"visitors" => 4, "name" => "/"}, %{"visitors" => 3, "name" => "/register"}, %{"visitors" => 1, "name" => "/contact"} @@ -1529,9 +1529,9 @@ defmodule PlausibleWeb.Api.StatsController.PagesTest do ) ]) - conn1 = get(conn, "/api/stats/#{site.domain}/entry-pages?period=day&date=2021-01-01") + conn = get(conn, "/api/stats/#{site.domain}/entry-pages?period=day&date=2021-01-01") - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "visitors" => 2, "visits" => 2, @@ -1546,13 +1546,13 @@ defmodule PlausibleWeb.Api.StatsController.PagesTest do } ] - conn2 = + conn = get( conn, "/api/stats/#{site.domain}/entry-pages?period=day&date=2021-01-01&with_imported=true" ) - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "visitors" => 3, "visits" => 5, @@ -1947,20 +1947,20 @@ defmodule PlausibleWeb.Api.StatsController.PagesTest do ) ]) - conn1 = get(conn, "/api/stats/#{site.domain}/exit-pages?period=day&date=2021-01-01") + conn = get(conn, "/api/stats/#{site.domain}/exit-pages?period=day&date=2021-01-01") - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"name" => "/page1", "visitors" => 2, "visits" => 2, "exit_rate" => 66}, %{"name" => "/page2", "visitors" => 1, "visits" => 1, "exit_rate" => 100} ] - conn2 = + conn = get( conn, "/api/stats/#{site.domain}/exit-pages?period=day&date=2021-01-01&with_imported=true" ) - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "/page2", "visitors" => 3, diff --git a/test/plausible_web/controllers/api/stats_controller/screen_sizes_test.exs b/test/plausible_web/controllers/api/stats_controller/screen_sizes_test.exs index 9da575fa52cd..63d01916957c 100644 --- a/test/plausible_web/controllers/api/stats_controller/screen_sizes_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/screen_sizes_test.exs @@ -87,17 +87,19 @@ defmodule PlausibleWeb.Api.StatsController.ScreenSizesTest do ) ]) - conn1 = get(conn, "/api/stats/#{site.domain}/screen-sizes?period=day") + conn = get(conn, "/api/stats/#{site.domain}/screen-sizes?period=day") - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"name" => "(not set)", "visitors" => 1, "percentage" => 50}, %{"name" => "Desktop", "visitors" => 1, "percentage" => 50} ] + conn = get(conn, "/api/stats/#{site.domain}/screen-sizes?period=day") + filters = Jason.encode!(%{screen: "(not set)"}) - conn2 = get(conn, "/api/stats/#{site.domain}/screen-sizes?period=day&filters=#{filters}") + conn = get(conn, "/api/stats/#{site.domain}/screen-sizes?period=day&filters=#{filters}") - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"name" => "(not set)", "visitors" => 1, "percentage" => 100} ] end @@ -201,16 +203,16 @@ defmodule PlausibleWeb.Api.StatsController.ScreenSizesTest do build(:imported_visitors, visitors: 2) ]) - conn1 = get(conn, "/api/stats/#{site.domain}/screen-sizes?period=day") + conn = get(conn, "/api/stats/#{site.domain}/screen-sizes?period=day") - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"name" => "Desktop", "visitors" => 2, "percentage" => 66.7}, %{"name" => "Laptop", "visitors" => 1, "percentage" => 33.3} ] - conn2 = get(conn, "/api/stats/#{site.domain}/screen-sizes?period=day&with_imported=true") + conn = get(conn, "/api/stats/#{site.domain}/screen-sizes?period=day&with_imported=true") - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"name" => "Desktop", "visitors" => 2, "percentage" => 40}, %{"name" => "Laptop", "visitors" => 2, "percentage" => 40}, %{"name" => "Mobile", "visitors" => 1, "percentage" => 20} diff --git a/test/plausible_web/controllers/api/stats_controller/sources_test.exs b/test/plausible_web/controllers/api/stats_controller/sources_test.exs index ca00df32d0fb..b3450ab9cdaf 100644 --- a/test/plausible_web/controllers/api/stats_controller/sources_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/sources_test.exs @@ -268,16 +268,16 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do ) ]) - conn1 = get(conn, "/api/stats/#{site.domain}/sources") + conn = get(conn, "/api/stats/#{site.domain}/sources") - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"name" => "Google", "visitors" => 2}, %{"name" => "DuckDuckGo", "visitors" => 1} ] - conn2 = get(conn, "/api/stats/#{site.domain}/sources?with_imported=true") + conn = get(conn, "/api/stats/#{site.domain}/sources?with_imported=true") - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"name" => "Google", "visitors" => 4}, %{"name" => "DuckDuckGo", "visitors" => 2} ] @@ -369,13 +369,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do ) ]) - conn1 = + conn = get( conn, "/api/stats/#{site.domain}/sources?period=day&date=2021-01-01&detailed=true" ) - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "DuckDuckGo", "visitors" => 1, @@ -390,13 +390,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do } ] - conn2 = + conn = get( conn, "/api/stats/#{site.domain}/sources?period=day&date=2021-01-01&detailed=true&with_imported=true" ) - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "Google", "visitors" => 3, @@ -461,15 +461,15 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do ) ]) - conn1 = get(conn, "/api/stats/#{site.domain}/sources?limit=1&page=2") + conn = get(conn, "/api/stats/#{site.domain}/sources?limit=1&page=2") - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"name" => "DuckDuckGo", "visitors" => 1} ] - conn2 = get(conn, "/api/stats/#{site.domain}/sources?limit=1&page=2&with_imported=true") + conn = get(conn, "/api/stats/#{site.domain}/sources?limit=1&page=2&with_imported=true") - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"name" => "Google", "visitors" => 2} ] end @@ -688,13 +688,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do ) ]) - conn1 = + conn = get( conn, "/api/stats/#{site.domain}/utm_mediums?period=day&date=2021-01-01" ) - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "email", "visitors" => 1, @@ -709,13 +709,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do } ] - conn2 = + conn = get( conn, "/api/stats/#{site.domain}/utm_mediums?period=day&date=2021-01-01&with_imported=true" ) - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "email", "visitors" => 2, @@ -768,13 +768,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do ) ]) - conn1 = + conn = get( conn, "/api/stats/#{site.domain}/utm_mediums?period=day&date=2021-01-01" ) - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "social", "visitors" => 1, @@ -783,13 +783,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do } ] - conn2 = + conn = get( conn, "/api/stats/#{site.domain}/utm_mediums?period=day&date=2021-01-01&with_imported=true" ) - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "social", "visitors" => 2, @@ -844,13 +844,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do ) ]) - conn1 = + conn = get( conn, "/api/stats/#{site.domain}/utm_campaigns?period=day&date=2021-01-01" ) - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "august", "visitors" => 2, @@ -865,13 +865,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do } ] - conn2 = + conn = get( conn, "/api/stats/#{site.domain}/utm_campaigns?period=day&date=2021-01-01&with_imported=true" ) - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "august", "visitors" => 3, @@ -928,13 +928,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do ) ]) - conn1 = + conn = get( conn, "/api/stats/#{site.domain}/utm_campaigns?period=day&date=2021-01-01" ) - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "profile", "visitors" => 1, @@ -943,13 +943,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do } ] - conn2 = + conn = get( conn, "/api/stats/#{site.domain}/utm_campaigns?period=day&date=2021-01-01&with_imported=true" ) - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "profile", "visitors" => 2, @@ -1052,13 +1052,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do ) ]) - conn1 = + conn = get( conn, "/api/stats/#{site.domain}/utm_terms?period=day&date=2021-01-01" ) - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "Sweden", "visitors" => 2, @@ -1073,13 +1073,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do } ] - conn2 = + conn = get( conn, "/api/stats/#{site.domain}/utm_terms?period=day&date=2021-01-01&with_imported=true" ) - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "Sweden", "visitors" => 3, @@ -1136,13 +1136,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do ) ]) - conn1 = + conn = get( conn, "/api/stats/#{site.domain}/utm_terms?period=day&date=2021-01-01" ) - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "oat milk", "visitors" => 1, @@ -1151,13 +1151,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do } ] - conn2 = + conn = get( conn, "/api/stats/#{site.domain}/utm_terms?period=day&date=2021-01-01&with_imported=true" ) - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "oat milk", "visitors" => 2, @@ -1212,13 +1212,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do ) ]) - conn1 = + conn = get( conn, "/api/stats/#{site.domain}/utm_contents?period=day&date=2021-01-01" ) - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "blog", "visitors" => 2, @@ -1233,13 +1233,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do } ] - conn2 = + conn = get( conn, "/api/stats/#{site.domain}/utm_contents?period=day&date=2021-01-01&with_imported=true" ) - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "blog", "visitors" => 3, @@ -1296,13 +1296,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do ) ]) - conn1 = + conn = get( conn, "/api/stats/#{site.domain}/utm_contents?period=day&date=2021-01-01" ) - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "ad", "visitors" => 1, @@ -1311,13 +1311,13 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do } ] - conn2 = + conn = get( conn, "/api/stats/#{site.domain}/utm_contents?period=day&date=2021-01-01&with_imported=true" ) - assert json_response(conn2, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{ "name" => "ad", "visitors" => 2, @@ -1757,15 +1757,15 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do ) ]) - conn1 = get(conn, "/api/stats/#{site.domain}/referrers/!Google?period=day") + conn = get(conn, "/api/stats/#{site.domain}/referrers/!Google?period=day") - assert json_response(conn1, 200)["results"] == [ + assert json_response(conn, 200)["results"] == [ %{"name" => "duckduckgo.com", "visitors" => 1} ] - conn2 = get(conn, "/api/stats/#{site.domain}/referrers/Google|DuckDuckGo?period=day") + conn = get(conn, "/api/stats/#{site.domain}/referrers/Google|DuckDuckGo?period=day") - assert [entry1, entry2] = json_response(conn2, 200)["results"] + assert [entry1, entry2] = json_response(conn, 200)["results"] assert %{"name" => "google.com", "visitors" => 2} in [entry1, entry2] assert %{"name" => "duckduckgo.com", "visitors" => 1} in [entry1, entry2] end diff --git a/test/plausible_web/controllers/api/stats_controller/suggestions_test.exs b/test/plausible_web/controllers/api/stats_controller/suggestions_test.exs index ab8530dcfd70..bfe7ae16d488 100644 --- a/test/plausible_web/controllers/api/stats_controller/suggestions_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/suggestions_test.exs @@ -764,13 +764,13 @@ defmodule PlausibleWeb.Api.StatsController.SuggestionsTest do build(:imported_locations, country: "EE", region: "Hiiumaa", pageviews: 1) ]) - conn1 = + conn = get( conn, "/api/stats/#{site.domain}/suggestions/region?q=&with_imported=true" ) - assert json_response(conn1, 200) == [ + assert json_response(conn, 200) == [ %{"value" => "EE-37", "label" => "Harjumaa"}, %{"value" => "Hiiumaa", "label" => "Hiiumaa"} ] diff --git a/test/plausible_web/controllers/auth_controller_test.exs b/test/plausible_web/controllers/auth_controller_test.exs index 2b1a6d02fcab..1081db959f7f 100644 --- a/test/plausible_web/controllers/auth_controller_test.exs +++ b/test/plausible_web/controllers/auth_controller_test.exs @@ -75,15 +75,14 @@ defmodule PlausibleWeb.AuthControllerTest do end test "logs the user in", %{conn: conn} do - user = - Repo.insert!( - User.new(%{ - name: "Jane Doe", - email: "user@example.com", - password: "very-secret-and-very-long-123", - password_confirmation: "very-secret-and-very-long-123" - }) - ) + Repo.insert!( + User.new(%{ + name: "Jane Doe", + email: "user@example.com", + password: "very-secret-and-very-long-123", + password_confirmation: "very-secret-and-very-long-123" + }) + ) conn = post(conn, "/login", @@ -94,8 +93,7 @@ defmodule PlausibleWeb.AuthControllerTest do } ) - assert %{sessions: [%{token: token}]} = user |> Repo.reload!() |> Repo.preload(:sessions) - assert get_session(conn, :user_token) == token + assert get_session(conn, :current_user_id) end end @@ -131,17 +129,16 @@ defmodule PlausibleWeb.AuthControllerTest do role: :admin ) - user = - Repo.insert!( - User.new(%{ - name: "Jane Doe", - email: "user@example.com", - password: "very-secret-and-very-long-123", - password_confirmation: "very-secret-and-very-long-123" - }) - ) + Repo.insert!( + User.new(%{ + name: "Jane Doe", + email: "user@example.com", + password: "very-secret-and-very-long-123", + password_confirmation: "very-secret-and-very-long-123" + }) + ) - {:ok, %{site: site, invitation: invitation, user: user}} + {:ok, %{site: site, invitation: invitation}} end test "registering sends an activation link", %{conn: conn} do @@ -175,7 +172,7 @@ defmodule PlausibleWeb.AuthControllerTest do assert redirected_to(conn, 302) == "/activate?flow=invitation" end - test "logs the user in", %{conn: conn, user: user} do + test "logs the user in", %{conn: conn} do conn = post(conn, "/login", user: %{ @@ -187,8 +184,7 @@ defmodule PlausibleWeb.AuthControllerTest do } ) - assert %{sessions: [%{token: token}]} = user |> Repo.reload!() |> Repo.preload(:sessions) - assert get_session(conn, :user_token) == token + assert get_session(conn, :current_user_id) end end @@ -338,8 +334,7 @@ defmodule PlausibleWeb.AuthControllerTest do conn = post(conn, "/login", email: user.email, password: "password") - assert %{sessions: [%{token: token}]} = user |> Repo.reload!() |> Repo.preload(:sessions) - assert get_session(conn, :user_token) == token + assert get_session(conn, :current_user_id) == user.id assert redirected_to(conn) == "/sites" end @@ -370,7 +365,7 @@ defmodule PlausibleWeb.AuthControllerTest do assert redirected_to(conn, 302) == Routes.auth_path(conn, :verify_2fa_form) assert fetch_cookies(conn).cookies["session_2fa"].current_2fa_user_id == user.id - refute get_session(conn)["user_token"] + refute get_session(conn)["current_user_id"] end test "valid email and password with 2FA enabled and remember 2FA cookie set - logs the user in", @@ -388,8 +383,7 @@ defmodule PlausibleWeb.AuthControllerTest do assert redirected_to(conn, 302) == Routes.site_path(conn, :index) assert conn.resp_cookies["session_2fa"].max_age == 0 - assert %{sessions: [%{token: token}]} = user |> Repo.reload!() |> Repo.preload(:sessions) - assert get_session(conn, :user_token) == token + assert get_session(conn, :current_user_id) == user.id end test "valid email and password with 2FA enabled and rogue remember 2FA cookie set - logs the user in", @@ -408,13 +402,13 @@ defmodule PlausibleWeb.AuthControllerTest do assert redirected_to(conn, 302) == Routes.auth_path(conn, :verify_2fa_form) assert fetch_cookies(conn).cookies["session_2fa"].current_2fa_user_id == user.id - refute get_session(conn, :user_token) + refute get_session(conn, :current_user_id) end test "email does not exist - renders login form again", %{conn: conn} do conn = post(conn, "/login", email: "user@example.com", password: "password") - assert get_session(conn, :user_token) == nil + assert get_session(conn, :current_user_id) == nil assert html_response(conn, 200) =~ "Enter your account credentials" end @@ -422,7 +416,7 @@ defmodule PlausibleWeb.AuthControllerTest do user = insert(:user, password: "password") conn = post(conn, "/login", email: user.email, password: "wrong") - assert get_session(conn, :user_token) == nil + assert get_session(conn, :current_user_id) == nil assert html_response(conn, 200) =~ "Enter your account credentials" end @@ -512,7 +506,7 @@ defmodule PlausibleWeb.AuthControllerTest do # cookie state is as expected for logged out user assert conn.private[:plug_session_info] == :renew assert conn.resp_cookies["logged_in"].max_age == 0 - assert get_session(conn, :user_token) == nil + assert get_session(conn, :current_user_id) == nil {:ok, %{conn: conn}} = PlausibleWeb.FirstLaunchPlug.Test.skip(%{conn: recycle(conn)}) conn = get(conn, location) @@ -531,7 +525,7 @@ defmodule PlausibleWeb.AuthControllerTest do # cookie state is as expected for logged out user assert conn.private[:plug_session_info] == :renew assert conn.resp_cookies["logged_in"].max_age == 0 - assert get_session(conn, :user_token) == nil + assert get_session(conn, :current_user_id) == nil {:ok, %{conn: conn}} = PlausibleWeb.FirstLaunchPlug.Test.skip(%{conn: recycle(conn)}) conn = get(conn, location) @@ -1703,8 +1697,7 @@ defmodule PlausibleWeb.AuthControllerTest do assert redirected_to(conn, 302) == Routes.site_path(conn, :index) - assert %{sessions: [%{token: token}]} = user |> Repo.reload!() |> Repo.preload(:sessions) - assert get_session(conn)["user_token"] == token + assert get_session(conn)["current_user_id"] == user.id # 2FA session terminated assert conn.resp_cookies["session_2fa"].max_age == 0 # Remember cookie unset @@ -1747,8 +1740,7 @@ defmodule PlausibleWeb.AuthControllerTest do assert redirected_to(conn, 302) == Routes.site_path(conn, :index) - assert %{sessions: [%{token: token}]} = user |> Repo.reload!() |> Repo.preload(:sessions) - assert get_session(conn)["user_token"] == token + assert get_session(conn)["current_user_id"] == user.id # 2FA session terminated assert conn.resp_cookies["session_2fa"].max_age == 0 # Remember cookie set @@ -1773,8 +1765,7 @@ defmodule PlausibleWeb.AuthControllerTest do assert redirected_to(conn, 302) == Routes.site_path(conn, :index) - assert %{sessions: [%{token: token}]} = user |> Repo.reload!() |> Repo.preload(:sessions) - assert get_session(conn)["user_token"] == token + assert get_session(conn)["current_user_id"] == user.id # 2FA session terminated assert conn.resp_cookies["session_2fa"].max_age == 0 # Remember cookie set @@ -1800,8 +1791,7 @@ defmodule PlausibleWeb.AuthControllerTest do assert redirected_to(conn, 302) == Routes.site_path(conn, :index) - assert %{sessions: [%{token: token}]} = user |> Repo.reload!() |> Repo.preload(:sessions) - assert get_session(conn, :user_token) == token + assert get_session(conn)["current_user_id"] == user.id # 2FA session terminated assert conn.resp_cookies["session_2fa"].max_age == 0 # Remember cookie cleared @@ -1856,8 +1846,7 @@ defmodule PlausibleWeb.AuthControllerTest do assert redirected_to(conn, 302) == Routes.site_path(conn, :index) - assert %{sessions: [%{token: token}]} = user |> Repo.reload!() |> Repo.preload(:sessions) - assert get_session(conn)["user_token"] == token + assert get_session(conn)["current_user_id"] == user.id # 2FA session terminated assert conn.resp_cookies["session_2fa"].max_age == 0 end @@ -1888,7 +1877,7 @@ defmodule PlausibleWeb.AuthControllerTest do 500 ) - assert get_session(response, :user_token) == nil + assert get_session(response, :current_user_id) == nil # 2FA session terminated assert response.resp_cookies["session_2fa"].max_age == 0 assert html_response(response, 429) =~ "Too many login attempts" @@ -1963,8 +1952,7 @@ defmodule PlausibleWeb.AuthControllerTest do assert redirected_to(conn, 302) == Routes.site_path(conn, :index) - assert %{sessions: [%{token: token}]} = user |> Repo.reload!() |> Repo.preload(:sessions) - assert get_session(conn)["user_token"] == token + assert get_session(conn)["current_user_id"] == user.id # 2FA session terminated assert conn.resp_cookies["session_2fa"].max_age == 0 end @@ -2021,8 +2009,7 @@ defmodule PlausibleWeb.AuthControllerTest do assert redirected_to(conn, 302) == Routes.site_path(conn, :index) - assert %{sessions: [%{token: token}]} = user |> Repo.reload!() |> Repo.preload(:sessions) - assert get_session(conn)["user_token"] == token + assert get_session(conn)["current_user_id"] == user.id # 2FA session terminated assert conn.resp_cookies["session_2fa"].max_age == 0 end @@ -2058,7 +2045,7 @@ defmodule PlausibleWeb.AuthControllerTest do 500 ) - assert get_session(response, :user_token) == nil + assert get_session(response, :current_user_id) == nil # 2FA session terminated assert response.resp_cookies["session_2fa"].max_age == 0 assert html_response(response, 429) =~ "Too many login attempts" diff --git a/test/plausible_web/controllers/page_controller_test.exs b/test/plausible_web/controllers/page_controller_test.exs index 1ba135a5cfc2..b1642112944b 100644 --- a/test/plausible_web/controllers/page_controller_test.exs +++ b/test/plausible_web/controllers/page_controller_test.exs @@ -4,14 +4,15 @@ defmodule PlausibleWeb.PageControllerTest do setup {PlausibleWeb.FirstLaunchPlug.Test, :skip} describe "GET /" do - setup [:create_user, :log_in] - - test "shows landing page when user not authenticated" do - assert build_conn() |> get("/") |> html_response(200) =~ "Welcome to Plausible!" + test "shows landing page when user not authenticated", %{conn: conn} do + assert conn |> get("/") |> html_response(200) =~ "Welcome to Plausible!" end test "redirects to /sites if user is authenticated", %{conn: conn} do + user = insert(:user) + assert conn + |> init_test_session(%{current_user_id: user.id}) |> get("/") |> redirected_to(302) == "/sites" end diff --git a/test/plausible_web/controllers/site_controller_test.exs b/test/plausible_web/controllers/site_controller_test.exs index bd9523a4b83e..7c541dc78d6e 100644 --- a/test/plausible_web/controllers/site_controller_test.exs +++ b/test/plausible_web/controllers/site_controller_test.exs @@ -957,7 +957,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_with_url, get(context.conn, "/some_parent_path"))} + {:ok, Map.put(context, :conn, get(context.conn, "/some_parent_path"))} end setup [:create_user, :log_in, :query_conn_with_some_url] @@ -969,8 +969,7 @@ defmodule PlausibleWeb.SiteControllerTest do } do test "can toggle #{title} with admin access", %{ user: user, - conn: conn0, - conn_with_url: conn_with_url + conn: conn0 } do site = insert(:site, @@ -983,12 +982,7 @@ defmodule PlausibleWeb.SiteControllerTest do conn = put( conn0, - PlausibleWeb.Components.Site.Feature.target( - site, - unquote(setting), - conn_with_url, - false - ) + PlausibleWeb.Components.Site.Feature.target(site, unquote(setting), conn0, false) ) assert Phoenix.Flash.get(conn.assigns.flash, :success) == @@ -1001,12 +995,7 @@ defmodule PlausibleWeb.SiteControllerTest do conn = put( conn0, - PlausibleWeb.Components.Site.Feature.target( - site, - unquote(setting), - conn_with_url, - true - ) + PlausibleWeb.Components.Site.Feature.target(site, unquote(setting), conn0, true) ) assert Phoenix.Flash.get(conn.assigns.flash, :success) == @@ -1025,8 +1014,7 @@ defmodule PlausibleWeb.SiteControllerTest do } do test "cannot toggle #{title} with viewer access", %{ user: user, - conn: conn0, - conn_with_url: conn_with_url + conn: conn0 } do site = insert(:site) insert(:site_membership, user: user, site: site, role: :viewer) @@ -1034,12 +1022,7 @@ defmodule PlausibleWeb.SiteControllerTest do conn = put( conn0, - PlausibleWeb.Components.Site.Feature.target( - site, - unquote(setting), - conn_with_url, - false - ) + PlausibleWeb.Components.Site.Feature.target(site, unquote(setting), conn0, false) ) assert conn.status == 404 @@ -1047,11 +1030,7 @@ defmodule PlausibleWeb.SiteControllerTest do end end - test "setting feature visibility is idempotent", %{ - user: user, - conn: conn0, - conn_with_url: conn_with_url - } do + test "setting feature visibility is idempotent", %{user: user, conn: conn0} do site = insert(:site) insert(:site_membership, user: user, site: site, role: :admin) @@ -1060,7 +1039,7 @@ defmodule PlausibleWeb.SiteControllerTest do conn = put( conn0, - PlausibleWeb.Components.Site.Feature.target(site, setting, conn_with_url, false) + PlausibleWeb.Components.Site.Feature.target(site, setting, conn0, false) ) assert %{^setting => false} = Plausible.Sites.get_by_domain(site.domain) @@ -1069,7 +1048,7 @@ defmodule PlausibleWeb.SiteControllerTest do conn = put( conn0, - PlausibleWeb.Components.Site.Feature.target(site, setting, conn_with_url, false) + PlausibleWeb.Components.Site.Feature.target(site, setting, conn0, false) ) assert %{^setting => false} = Plausible.Sites.get_by_domain(site.domain) @@ -1156,11 +1135,11 @@ defmodule PlausibleWeb.SiteControllerTest do site = insert(:site) insert(:weekly_report, site: site, recipients: ["recipient@email.com"]) - conn1 = delete(conn, "/sites/#{site.domain}/weekly-report/recipients/recipient@email.com") - assert conn1.status == 404 + conn = delete(conn, "/sites/#{site.domain}/weekly-report/recipients/recipient@email.com") + assert conn.status == 404 - conn2 = delete(conn, "/sites/#{site.domain}/weekly-report/recipients/recipient%40email.com") - assert conn2.status == 404 + conn = delete(conn, "/sites/#{site.domain}/weekly-report/recipients/recipient%40email.com") + assert conn.status == 404 report = Repo.get_by(Plausible.Site.WeeklyReport, site_id: site.id) assert [_] = report.recipients @@ -1239,13 +1218,11 @@ defmodule PlausibleWeb.SiteControllerTest do site = insert(:site) insert(:monthly_report, site: site, recipients: ["recipient@email.com"]) - conn1 = delete(conn, "/sites/#{site.domain}/monthly-report/recipients/recipient@email.com") - assert conn1.status == 404 - - conn2 = - delete(conn, "/sites/#{site.domain}/monthly-report/recipients/recipient%40email.com") + conn = delete(conn, "/sites/#{site.domain}/monthly-report/recipients/recipient@email.com") + assert conn.status == 404 - assert conn2.status == 404 + conn = delete(conn, "/sites/#{site.domain}/monthly-report/recipients/recipient%40email.com") + assert conn.status == 404 report = Repo.get_by(Plausible.Site.MonthlyReport, site_id: site.id) assert [_] = report.recipients diff --git a/test/plausible_web/live/sentry_context_test.exs b/test/plausible_web/live/sentry_context_test.exs index 190425e12afb..6ab47718c896 100644 --- a/test/plausible_web/live/sentry_context_test.exs +++ b/test/plausible_web/live/sentry_context_test.exs @@ -54,16 +54,12 @@ defmodule PlausibleWeb.Live.SentryContextTest do assert_receive {:context, context} assert context.request.headers["User-Agent"] == "Firefox" end - end - - describe "sentry context with logged in user" do - setup [:create_user, :log_in] - test "user_id is included", %{conn: conn, user: user} do - context_hook(conn) + test "user_id is included", %{conn: conn} do + context_hook(conn, %{"current_user_id" => 172}) assert_receive {:context, context} - assert context.user.id == user.id + assert context.user.id == 172 end end diff --git a/test/plausible_web/plugs/auth_plug_test.exs b/test/plausible_web/plugs/auth_plug_test.exs index 32911da7d9e4..689195bdc662 100644 --- a/test/plausible_web/plugs/auth_plug_test.exs +++ b/test/plausible_web/plugs/auth_plug_test.exs @@ -1,40 +1,41 @@ defmodule PlausibleWeb.AuthPlugTest do - use PlausibleWeb.ConnCase, async: true - + use Plausible.DataCase, async: true + use Plug.Test alias PlausibleWeb.AuthPlug - setup [:create_user, :log_in] - test "does nothing if user is not logged in" do conn = - build_conn(:get, "/") + conn(:get, "/") |> init_test_session(%{}) |> AuthPlug.call(%{}) assert is_nil(conn.assigns[:current_user]) end - test "looks up current user if they are logged in", %{conn: conn, user: user} do - subscription = insert(:subscription, user: user, inserted_at: Timex.now()) + test "looks up current user if they are logged in" do + user = insert(:user) + subscription = insert(:subscription, user: user) conn = - conn - |> Plug.Adapters.Test.Conn.conn(:get, "/", %{}) + conn(:get, "/") + |> init_test_session(%{current_user_id: user.id}) |> AuthPlug.call(%{}) assert conn.assigns[:current_user].id == user.id assert conn.assigns[:current_user].subscription.id == subscription.id end - test "looks up the latest subscription", %{conn: conn, user: user} do + test "looks up the latest subscription" do + user = insert(:user) + _old_subscription = insert(:subscription, user: user, inserted_at: Timex.now() |> Timex.shift(days: -1)) subscription = insert(:subscription, user: user, inserted_at: Timex.now()) conn = - conn - |> Plug.Adapters.Test.Conn.conn(:get, "/", %{}) + conn(:get, "/") + |> init_test_session(%{current_user_id: user.id}) |> AuthPlug.call(%{}) assert conn.assigns[:current_user].id == user.id diff --git a/test/plausible_web/plugs/session_timeout_plug_test.exs b/test/plausible_web/plugs/session_timeout_plug_test.exs index ce8ee15630c5..adb3baf59a4f 100644 --- a/test/plausible_web/plugs/session_timeout_plug_test.exs +++ b/test/plausible_web/plugs/session_timeout_plug_test.exs @@ -1,15 +1,9 @@ defmodule PlausibleWeb.SessionTimeoutPlugTest do - use Plausible.DataCase, async: true + use ExUnit.Case, async: true use Plug.Test - - import Plausible.Factory - alias PlausibleWeb.SessionTimeoutPlug - @opts %{timeout_after_seconds: 10} - @moduletag :capture_log - test "does nothing if user is not logged in" do conn = conn(:get, "/") @@ -20,11 +14,9 @@ defmodule PlausibleWeb.SessionTimeoutPlugTest do end test "sets session timeout if user is logged in" do - user = insert(:user) - conn = conn(:get, "/") - |> init_test_session(%{current_user_id: user.id}) + |> init_test_session(%{current_user_id: 1}) |> SessionTimeoutPlug.call(@opts) timeout = get_session(conn, :session_timeout_at) @@ -33,11 +25,9 @@ defmodule PlausibleWeb.SessionTimeoutPlugTest do end test "logs user out if timeout passed" do - user = insert(:user) - conn = conn(:get, "/") - |> init_test_session(%{current_user_id: user.id, session_timeout_at: 1}) + |> init_test_session(%{current_user_id: 1, session_timeout_at: 1}) |> SessionTimeoutPlug.call(@opts) assert conn.private[:plug_session_info] == :renew diff --git a/test/plausible_web/plugs/user_session_touch_test.exs b/test/plausible_web/plugs/user_session_touch_test.exs deleted file mode 100644 index 3a0215fb3dcc..000000000000 --- a/test/plausible_web/plugs/user_session_touch_test.exs +++ /dev/null @@ -1,57 +0,0 @@ -defmodule PlausibleWeb.Plugs.UserSessionTouchTest do - use PlausibleWeb.ConnCase, async: true - - alias Plausible.Repo - alias PlausibleWeb.AuthPlug - alias PlausibleWeb.UserAuth - alias PlausibleWeb.Plugs.UserSessionTouch - - setup [:create_user, :log_in] - - @moduletag :capture_log - - test "refreshes session", %{conn: conn, user: user} do - now = NaiveDateTime.utc_now(:second) - one_day_ago = NaiveDateTime.shift(now, day: -1) - %{sessions: [user_session]} = Repo.preload(user, :sessions) - UserAuth.touch_user_session(user_session, one_day_ago) - - assert %{assigns: %{current_user_session: user_session}} = - conn - |> AuthPlug.call([]) - |> UserSessionTouch.call([]) - - assert NaiveDateTime.compare(user_session.last_used_at, now) in [:gt, :eq] - assert NaiveDateTime.compare(user_session.timeout_at, user_session.last_used_at) == :gt - end - - test "passes through when there's no authenticated session" do - conn = - build_conn() - |> init_session() - |> put_session(:login_dest, "/") - |> UserSessionTouch.call([]) - - refute conn.halted - assert get_session(conn, :login_dest) == "/" - refute get_session(conn, :current_user_id) - refute get_session(conn, :user_token) - end - - test "converts legacy session when present", %{user: user} do - %{sessions: [other_session]} = Repo.preload(user, :sessions) - - conn = - build_conn() - |> init_session() - |> put_session(:current_user_id, user.id) - |> AuthPlug.call([]) - |> UserSessionTouch.call([]) - - refute get_session(conn, :current_user_id) - assert user_token = get_session(conn, :user_token) - assert conn.assigns.current_user_session.id - assert conn.assigns.current_user_session.id != other_session.id - assert conn.assigns.current_user_session.token == user_token - end -end diff --git a/test/plausible_web/user_auth_test.exs b/test/plausible_web/user_auth_test.exs index 71311519e15d..11620f3388ac 100644 --- a/test/plausible_web/user_auth_test.exs +++ b/test/plausible_web/user_auth_test.exs @@ -1,17 +1,10 @@ defmodule PlausibleWeb.UserAuthTest do use PlausibleWeb.ConnCase, async: true - import Ecto.Query, only: [from: 2] - import ExUnit.CaptureLog - - alias Plausible.Auth - alias Plausible.Repo alias PlausibleWeb.UserAuth alias PlausibleWeb.Router.Helpers, as: Routes - @moduletag capture_log: true - describe "log_in_user/2,3" do setup [:create_user] @@ -21,17 +14,10 @@ defmodule PlausibleWeb.UserAuthTest do |> init_session() |> UserAuth.log_in_user(user) - now = NaiveDateTime.utc_now(:second) - - assert %{sessions: [session]} = user |> Repo.reload!() |> Repo.preload(:sessions) - assert session.user_id == user.id - assert NaiveDateTime.compare(session.last_used_at, now) in [:eq, :gt] - assert NaiveDateTime.compare(session.timeout_at, session.last_used_at) == :gt - assert redirected_to(conn, 302) == Routes.site_path(conn, :index) assert conn.private[:plug_session_info] == :renew assert conn.resp_cookies["logged_in"].max_age > 0 - assert get_session(conn, :user_token) == session.token + assert get_session(conn, :current_user_id) == user.id assert get_session(conn, :login_dest) == nil end @@ -66,24 +52,15 @@ defmodule PlausibleWeb.UserAuthTest do end describe "log_out_user/1" do - setup [:create_user] - - test "logs user out", %{conn: conn, user: user} do - # another independent session for the same user - {:ok, conn: another_conn} = log_in(%{conn: conn, user: user}) - another_session_token = get_session(another_conn, :user_token) - - {:ok, conn: conn} = log_in(%{conn: conn, user: user}) + setup [:create_user, :log_in] + test "logs user out", %{conn: conn} do conn = conn |> init_session() |> put_session("login_dest", "/ignored") |> UserAuth.log_out_user() - # the other session remains intact - assert %{sessions: [another_session]} = Repo.preload(user, :sessions) - assert another_session.token == another_session_token assert conn.private[:plug_session_info] == :renew assert conn.resp_cookies["logged_in"].max_age == 0 assert get_session(conn, :current_user_id) == nil @@ -91,240 +68,86 @@ defmodule PlausibleWeb.UserAuthTest do end end - describe "get_user_session/1" do + describe "get_user/1" do setup [:create_user, :log_in] - test "gets session from session data in conn", %{conn: conn, user: user} do - assert {:ok, user_session} = UserAuth.get_user_session(conn) - assert user_session.user_id == user.id + test "gets user from session data in conn", %{conn: conn, user: user} do + assert {:ok, session_user} = UserAuth.get_user(conn) + assert session_user.id == user.id end - test "gets session from session data map", %{user: user} do - user_id = user.id - %{sessions: [user_session]} = Repo.preload(user, :sessions) - - assert {:ok, session_from_token} = - UserAuth.get_user_session(%{"user_token" => user_session.token}) + test "gets user from session data map", %{user: user} do + assert {:ok, session_user} = UserAuth.get_user(%{"current_user_id" => user.id}) + assert session_user.id == user.id + end - assert session_from_token.id == user_session.id + test "gets user from session schema", %{user: user} do + assert {:ok, session_user} = + UserAuth.get_user(%Plausible.Auth.UserSession{user_id: user.id}) - capture_log(fn -> - assert {:ok, %Auth.UserSession{user_id: ^user_id, token: nil}} = - UserAuth.get_user_session(%{"current_user_id" => user.id}) - end) =~ "Legacy user session detected" + assert session_user.id == user.id end test "returns error on invalid or missing session data" do conn = init_session(build_conn()) - assert {:error, :no_valid_token} = UserAuth.get_user_session(conn) - assert {:error, :no_valid_token} = UserAuth.get_user_session(%{}) + assert {:error, :no_valid_token} = UserAuth.get_user(conn) + assert {:error, :no_valid_token} = UserAuth.get_user(%{}) end - test "returns error on missing session (new token scaffold; to be revised)", %{ - conn: conn, - user: user - } do - %{sessions: [user_session]} = Repo.preload(user, :sessions) - Repo.delete!(user_session) + test "returns error on missing user", %{conn: conn, user: user} do + Plausible.Repo.delete!(user) - assert {:error, :session_not_found} = UserAuth.get_user_session(conn) + assert {:error, :user_not_found} = UserAuth.get_user(conn) + assert {:error, :user_not_found} = UserAuth.get_user(%{"current_user_id" => user.id}) - assert {:error, :session_not_found} = - UserAuth.get_user_session(%{"user_token" => user_session.token}) + assert {:error, :user_not_found} = + UserAuth.get_user(%Plausible.Auth.UserSession{user_id: user.id}) end - end - describe "set_logged_in_cookie/1" do - test "sets logged_in_cookie", %{conn: conn} do - conn = UserAuth.set_logged_in_cookie(conn) + test "returns error on missing session (new token scaffold; to be revised)" do + conn = build_conn() |> init_session() |> put_session(:user_token, "does_not_exist") - assert cookie = conn.resp_cookies["logged_in"] - assert cookie.max_age > 0 - assert cookie.value == "true" + assert {:error, :session_not_found} = UserAuth.get_user(conn) + assert {:error, :session_not_found} = UserAuth.get_user(%{"user_token" => "does_not_exist"}) end end - describe "touch_user_session/1" do + describe "get_user_session/1" do setup [:create_user, :log_in] - test "refreshes user session timestamps", %{user: user} do - %{sessions: [user_session]} = Repo.preload(user, :sessions) - - two_days_later = - NaiveDateTime.utc_now(:second) - |> NaiveDateTime.shift(day: 2) - - assert refreshed_session = - %Auth.UserSession{} = UserAuth.touch_user_session(user_session, two_days_later) - - 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) - - # concurrent update - now = NaiveDateTime.utc_now(:second) - two_days_later = NaiveDateTime.shift(now, day: 2) - - Repo.update_all( - from(us in Auth.UserSession, where: us.token == ^user_session.token), - set: [timeout_at: two_days_later, last_used_at: now] - ) - - assert refreshed_session = - %Auth.UserSession{} = UserAuth.touch_user_session(user_session) - - assert refreshed_session.id == user_session.id - assert Repo.reload(user_session) - end - - test "handles deleted session case gracefully", %{user: user} do - %{sessions: [user_session]} = Repo.preload(user, :sessions) - Repo.delete!(user_session) - - assert refreshed_session = - %Auth.UserSession{} = UserAuth.touch_user_session(user_session) - - assert refreshed_session.id == user_session.id - - refute Repo.reload(user_session) + test "gets session from session data in conn", %{conn: conn, user: user} do + assert {:ok, user_session} = UserAuth.get_user_session(conn) + assert user_session.user_id == user.id end - test "skips refreshing legacy session", %{user: user} do - user_session = %Auth.UserSession{user_id: user.id} - - assert UserAuth.touch_user_session(user_session) == user_session + test "gets session from session data map", %{user: user} do + assert {:ok, user_session} = UserAuth.get_user_session(%{"current_user_id" => user.id}) + assert user_session.user_id == user.id end - end - - describe "convert_legacy_session/1" do - setup [:create_user, :log_in] - test "does nothing when there's no authenticated session" do - conn = - build_conn() - |> init_session() - |> UserAuth.convert_legacy_session() - - refute get_session(conn, :user_token) - refute get_session(conn, :live_socket_id) - refute conn.assigns[:current_user_session] + test "returns error on invalid or missing session data" do + conn = init_session(build_conn()) + assert {:error, :no_valid_token} = UserAuth.get_user_session(conn) + assert {:error, :no_valid_token} = UserAuth.get_user_session(%{}) end - test "does nothing when there's a new token-based session already", %{conn: conn, user: user} do - %{sessions: [user_session]} = Repo.preload(user, :sessions) - - conn = - conn - |> UserAuth.convert_legacy_session() - |> PlausibleWeb.AuthPlug.call([]) - - assert get_session(conn, :user_token) == user_session.token - - assert get_session(conn, :live_socket_id) == - "user_sessions:#{Base.url_encode64(user_session.token)}" + test "returns error on missing session (new token scaffold; to be revised)" do + conn = build_conn() |> init_session() |> put_session(:user_token, "does_not_exist") - assert conn.assigns.current_user_session.id == user_session.id - end - - test "converts legacy session to a new one", %{user: user} do - %{sessions: [existing_session]} = Repo.preload(user, :sessions) + assert {:error, :session_not_found} = UserAuth.get_user_session(conn) - conn = - build_conn() - |> init_session() - |> put_session(:current_user_id, user.id) - |> PlausibleWeb.AuthPlug.call([]) - |> UserAuth.convert_legacy_session() - - assert conn.assigns.current_user_session.id - assert conn.assigns.current_user_session.id != existing_session.id - assert conn.assigns.current_user_session.token != existing_session.token - assert conn.assigns.current_user_session.user_id == user.id - assert conn.assigns.current_user.id == user.id - refute get_session(conn, :current_user_id) - assert get_session(conn, :user_token) == conn.assigns.current_user_session.token - - assert get_session(conn, :live_socket_id) == - "user_sessions:#{Base.url_encode64(conn.assigns.current_user_session.token)}" + assert {:error, :session_not_found} = + UserAuth.get_user_session(%{"user_token" => "does_not_exist"}) end end - @user_agent "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.110 Safari/537.36" - @user_agent_mobile "Mozilla/5.0 (Linux; Android 6.0; U007 Pro Build/MRA58K; wv) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/44.0.2403.119 Mobile Safari/537.36" - @user_agent_tablet "Mozilla/5.0 (Linux; U; Android 4.2.2; it-it; Surfing TAB B 9.7 3G Build/JDQ39) AppleWebKit/534.30 (KHTML, like Gecko) Version/4.0 Mobile Safari/534.30" - - describe "device name detection" do - setup [:create_user] - - test "detects browser and os when possible", %{conn: conn, user: user} do - assert login_device(conn, user, @user_agent) == "Chrome (Mac)" - assert login_device(conn, user, @user_agent_mobile) == "Mobile App (Android)" - assert login_device(conn, user, @user_agent_tablet) == "Android Browser (Android)" - end - - test "falls back to unknown when can't detect browser", %{conn: conn, user: user} do - assert login_device(conn, user, nil) == "Unknown" - assert login_device(conn, user, "Bogus UA") == "Unknown" - end + describe "set_logged_in_cookie/1" do + test "sets logged_in_cookie", %{conn: conn} do + conn = UserAuth.set_logged_in_cookie(conn) - test "skips os when can't detect it", %{conn: conn, user: user} do - assert login_device(conn, user, "Mozilla Firefox") == "Firefox" + assert cookie = conn.resp_cookies["logged_in"] + assert cookie.max_age > 0 + assert cookie.value == "true" end end - - defp login_device(conn, user, ua_string) do - conn = - if ua_string do - Plug.Conn.put_req_header(conn, "user-agent", ua_string) - else - conn - end - - {:ok, conn: conn} = log_in(%{conn: conn, user: user}) - - {:ok, user_session} = conn |> UserAuth.get_user_session() - - user_session.device - end end diff --git a/test/support/test_utils.ex b/test/support/test_utils.ex index 5326e101975c..780d84f99911 100644 --- a/test/support/test_utils.ex +++ b/test/support/test_utils.ex @@ -101,11 +101,11 @@ defmodule Plausible.TestUtils do def log_in(%{user: user, conn: conn}) do conn = conn - |> init_session() - |> PlausibleWeb.UserAuth.log_in_user(user) + |> PlausibleWeb.UserAuth.set_logged_in_cookie() |> Phoenix.ConnTest.recycle() |> Map.put(:secret_key_base, secret_key_base()) |> init_session() + |> Plug.Conn.put_session(:current_user_id, user.id) {:ok, conn: conn} end diff --git a/test/workers/clean_user_sessions_test.exs b/test/workers/clean_user_sessions_test.exs deleted file mode 100644 index e6006866ea1b..000000000000 --- a/test/workers/clean_user_sessions_test.exs +++ /dev/null @@ -1,34 +0,0 @@ -defmodule Plausible.Workers.CleanUserSessionsTest do - use Plausible.DataCase - - alias Plausible.Auth.UserSession - alias Plausible.Workers.CleanUserSessions - - test "cleans invitation that is more than timeout_at + grace_period days old" do - grace_cutoff = - NaiveDateTime.utc_now(:second) - |> NaiveDateTime.shift(Duration.negate(UserSession.timeout_duration())) - |> NaiveDateTime.shift(CleanUserSessions.grace_period_duration()) - - ten_days_after = NaiveDateTime.shift(grace_cutoff, day: 10) - one_day_after = NaiveDateTime.shift(grace_cutoff, day: 1) - one_day_before = NaiveDateTime.shift(grace_cutoff, day: -1) - session_to_clean = insert_session(one_day_before) - session_to_leave1 = insert_session(one_day_after) - session_to_leave2 = insert_session(ten_days_after) - - CleanUserSessions.perform(nil) - - refute Repo.reload(session_to_clean) - assert Repo.reload(session_to_leave1) - assert Repo.reload(session_to_leave2) - end - - defp insert_session(now) do - user = insert(:user) - - user - |> UserSession.new_session("Unknown", now) - |> Repo.insert!() - end -end