diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f061a0ab650..8f96107c83e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file. ### Added - Add ability to review and revoke particular logged in user sessions +- Add ability to change password from user settings screen ### Removed diff --git a/lib/plausible/auth/auth.ex b/lib/plausible/auth/auth.ex index 4a65950f131f..ba1ecae1c2c7 100644 --- a/lib/plausible/auth/auth.ex +++ b/lib/plausible/auth/auth.ex @@ -23,6 +23,11 @@ defmodule Plausible.Auth do prefix: "email-change:user", limit: 2, interval: :timer.hours(1) + }, + password_change_user: %{ + prefix: "password-change:user", + limit: 5, + interval: :timer.minutes(20) } } diff --git a/lib/plausible/auth/user.ex b/lib/plausible/auth/user.ex index 4fe35b9f3225..341a4045ad28 100644 --- a/lib/plausible/auth/user.ex +++ b/lib/plausible/auth/user.ex @@ -25,6 +25,7 @@ defmodule Plausible.Auth.User do schema "users" do field :email, :string field :password_hash + field :old_password, :string, virtual: true field :password, :string, virtual: true field :password_confirmation, :string, virtual: true field :name, :string @@ -65,8 +66,7 @@ defmodule Plausible.Auth.User do %Plausible.Auth.User{} |> cast(attrs, @required) |> validate_required(@required) - |> validate_length(:password, min: 12, message: "has to be at least 12 characters") - |> validate_length(:password, max: 128, message: "cannot be longer than 128 characters") + |> validate_password_length() |> validate_confirmation(:password, required: true) |> validate_password_strength() |> hash_password() @@ -142,9 +142,20 @@ defmodule Plausible.Auth.User do user |> cast(%{password: password}, [:password]) |> validate_required([:password]) - |> validate_length(:password, min: 12, message: "has to be at least 12 characters") - |> validate_length(:password, max: 128, message: "cannot be longer than 128 characters") + |> validate_password_length() + |> validate_password_strength() + |> hash_password() + end + + def password_changeset(user, params \\ %{}) do + user + |> cast(params, [:old_password, :password]) + |> check_password(:old_password) + |> validate_required([:old_password, :password]) + |> validate_password_length() + |> validate_confirmation(:password, required: true) |> validate_password_strength() + |> validate_password_changed() |> hash_password() end @@ -226,18 +237,35 @@ defmodule Plausible.Auth.User do end end - defp check_password(changeset) do - if password = get_change(changeset, :password) do + defp validate_password_changed(changeset) do + old_password = get_change(changeset, :old_password) + new_password = get_change(changeset, :password) + + if old_password == new_password do + add_error(changeset, :password, "is too weak", validation: :different_password) + else + changeset + end + end + + defp check_password(changeset, field \\ :password) do + if password = get_change(changeset, field) do if Plausible.Auth.Password.match?(password, changeset.data.password_hash) do changeset else - add_error(changeset, :password, "is invalid", validation: :check_password) + add_error(changeset, field, "is invalid", validation: :check_password) end else changeset end end + defp validate_password_length(changeset) do + changeset + |> validate_length(:password, min: 12, message: "has to be at least 12 characters") + |> validate_length(:password, max: 128, message: "cannot be longer than 128 characters") + end + defp validate_password_strength(changeset) do if get_change(changeset, :password) != nil and password_strength(changeset).score <= 2 do add_error(changeset, :password, "is too weak", validation: :strength) diff --git a/lib/plausible_web/components/two_factor.ex b/lib/plausible_web/components/two_factor.ex index 812d9898780e..14892194772a 100644 --- a/lib/plausible_web/components/two_factor.ex +++ b/lib/plausible_web/components/two_factor.ex @@ -2,7 +2,8 @@ defmodule PlausibleWeb.Components.TwoFactor do @moduledoc """ Reusable components specific to 2FA """ - use Phoenix.Component + use Phoenix.Component, global_prefixes: ~w(x-) + import PlausibleWeb.Components.Generic attr :text, :string, required: true attr :scale, :integer, default: 4 @@ -24,14 +25,26 @@ defmodule PlausibleWeb.Components.TwoFactor do attr :form, :any, required: true attr :field, :any, required: true attr :class, :string, default: "" + attr :show_button?, :boolean, default: true def verify_2fa_input(assigns) do + input_class = + "font-mono tracking-[0.5em] w-36 pl-5 font-medium shadow-sm focus:ring-indigo-500 focus:border-indigo-500 block border-gray-300 dark:border-gray-500 dark:text-gray-200 dark:bg-gray-900 rounded-l-md" + + input_class = + if assigns.show_button? do + input_class + else + [input_class, "rounded-r-md"] + end + + assigns = assign(assigns, :input_class, input_class) + ~H"""
<%= Phoenix.HTML.Form.text_input(@form, @field, autocomplete: "off", - class: - "font-mono tracking-[0.5em] w-36 pl-5 font-medium shadow-sm focus:ring-indigo-500 focus:border-indigo-500 block border-gray-300 dark:border-gray-500 dark:text-gray-200 dark:bg-gray-900 rounded-l-md", + class: @input_class, oninput: "this.value=this.value.replace(/[^0-9]/g, ''); if (this.value.length >= 6) document.getElementById('verify-button').focus()", onclick: "this.select();", @@ -42,6 +55,7 @@ defmodule PlausibleWeb.Components.TwoFactor do required: "required" ) %>
<%= render_slot(@buttons) %> - +
<% end %>
diff --git a/lib/plausible_web/controllers/auth_controller.ex b/lib/plausible_web/controllers/auth_controller.ex index 2226be3a48d3..a1e371774868 100644 --- a/lib/plausible_web/controllers/auth_controller.ex +++ b/lib/plausible_web/controllers/auth_controller.ex @@ -29,6 +29,7 @@ defmodule PlausibleWeb.AuthController do :user_settings, :save_settings, :update_email, + :update_password, :cancel_update_email, :new_api_key, :create_api_key, @@ -261,14 +262,7 @@ defmodule PlausibleWeb.AuthController do end def user_settings(conn, _params) do - user = conn.assigns.current_user - settings_changeset = Auth.User.settings_changeset(user) - email_changeset = Auth.User.settings_changeset(user) - - render_settings(conn, - settings_changeset: settings_changeset, - email_changeset: email_changeset - ) + render_settings(conn, []) end def initiate_2fa_setup(conn, _params) do @@ -453,12 +447,7 @@ defmodule PlausibleWeb.AuthController do |> redirect(to: Routes.auth_path(conn, :user_settings)) {:error, changeset} -> - email_changeset = Auth.User.settings_changeset(user) - - render_settings(conn, - settings_changeset: changeset, - email_changeset: email_changeset - ) + render_settings(conn, settings_changeset: changeset) end end @@ -476,26 +465,42 @@ defmodule PlausibleWeb.AuthController do end else {:error, %Ecto.Changeset{} = changeset} -> - settings_changeset = Auth.User.settings_changeset(user) - - render_settings(conn, - settings_changeset: settings_changeset, - email_changeset: changeset - ) + render_settings(conn, email_changeset: changeset) {:error, {:rate_limit, _}} -> - settings_changeset = Auth.User.settings_changeset(user) - - {:error, changeset} = + changeset = user |> Auth.User.email_changeset(user_params) |> Ecto.Changeset.add_error(:email, "too many requests, try again in an hour") - |> Ecto.Changeset.apply_action(:validate) + |> Map.put(:action, :validate) - render_settings(conn, - settings_changeset: settings_changeset, - email_changeset: changeset - ) + render_settings(conn, email_changeset: changeset) + end + end + + def update_password(conn, %{"user" => params}) do + user = conn.assigns.current_user + user_session = conn.assigns.current_user_session + + with :ok <- Auth.rate_limit(:password_change_user, user), + {:ok, user} <- do_update_password(user, params) do + UserAuth.revoke_all_user_sessions(user, except: user_session) + + conn + |> put_flash(:success, "Your password is now changed") + |> redirect(to: Routes.auth_path(conn, :user_settings) <> "#change-password") + else + {:error, %Ecto.Changeset{} = changeset} -> + render_settings(conn, password_changeset: changeset) + + {:error, {:rate_limit, _}} -> + changeset = + user + |> Auth.User.password_changeset(params) + |> Ecto.Changeset.add_error(:password, "too many attempts, try again in 20 minutes") + |> Map.put(:action, :validate) + + render_settings(conn, password_changeset: changeset) end end @@ -524,18 +529,57 @@ defmodule PlausibleWeb.AuthController do |> redirect(to: Routes.auth_path(conn, :user_settings) <> "#change-email-address") end + defp do_update_password(user, params) do + changes = Auth.User.password_changeset(user, params) + + Repo.transaction(fn -> + with {:ok, user} <- Repo.update(changes), + {:ok, user} <- validate_2fa_code(user, params["two_factor_code"]) do + user + else + {:error, :invalid_2fa} -> + changes + |> Ecto.Changeset.add_error(:password, "invalid 2FA code") + |> Map.put(:action, :validate) + |> Repo.rollback() + + {:error, changeset} -> + Repo.rollback(changeset) + end + end) + end + + defp validate_2fa_code(user, code) do + if Auth.TOTP.enabled?(user) do + case Auth.TOTP.validate_code(user, code) do + {:ok, user} -> {:ok, user} + {:error, :not_enabled} -> {:ok, user} + {:error, _} -> {:error, :invalid_2fa} + end + else + {:ok, user} + end + end + defp render_settings(conn, opts) do current_user = conn.assigns.current_user - settings_changeset = Keyword.fetch!(opts, :settings_changeset) - email_changeset = Keyword.fetch!(opts, :email_changeset) api_keys = Repo.preload(current_user, :api_keys).api_keys user_sessions = Auth.UserSessions.list_for_user(current_user) + settings_changeset = + Keyword.get(opts, :settings_changeset, Auth.User.settings_changeset(current_user)) + + email_changeset = Keyword.get(opts, :email_changeset, Auth.User.email_changeset(current_user)) + + password_changeset = + Keyword.get(opts, :password_changeset, Auth.User.password_changeset(current_user)) + render(conn, "user_settings.html", api_keys: api_keys, user_sessions: user_sessions, settings_changeset: settings_changeset, email_changeset: email_changeset, + password_changeset: password_changeset, subscription: current_user.subscription, invoices: Plausible.Billing.paddle_api().get_invoices(current_user.subscription), theme: current_user.theme || "system", diff --git a/lib/plausible_web/router.ex b/lib/plausible_web/router.ex index fe6ebbe3634a..db5f828eb239 100644 --- a/lib/plausible_web/router.ex +++ b/lib/plausible_web/router.ex @@ -329,6 +329,7 @@ defmodule PlausibleWeb.Router do get "/settings", AuthController, :user_settings put "/settings", AuthController, :save_settings put "/settings/email", AuthController, :update_email + put "/settings/password", AuthController, :update_password post "/settings/email/cancel", AuthController, :cancel_update_email delete "/me", AuthController, :delete_me get "/settings/api-keys/new", AuthController, :new_api_key diff --git a/lib/plausible_web/templates/auth/user_settings.html.heex b/lib/plausible_web/templates/auth/user_settings.html.heex index d631f4631bca..80d4eb4ac74b 100644 --- a/lib/plausible_web/templates/auth/user_settings.html.heex +++ b/lib/plausible_web/templates/auth/user_settings.html.heex @@ -203,6 +203,73 @@ <% end %> +
+

+ Change password +

+ +
+ + <%= form_for @password_changeset, "/settings/password#change-password", [class: "max-w-sm"], fn f -> %> +
+ <%= label(f, :old_password, "Current password", + class: "block text-sm font-medium text-gray-700 dark:text-gray-300" + ) %> +
+ <%= password_input(f, :old_password, + class: + "shadow-sm dark:bg-gray-900 dark:text-gray-300 focus:ring-indigo-500 focus:border-indigo-500 block w-full sm:text-sm border-gray-300 dark:border-gray-500 rounded-md dark:bg-gray-800" + ) %> + <%= error_tag(f, :old_password, only_first?: true) %> +
+
+
+ <%= label(f, :password, "New password", + class: "block text-sm font-medium text-gray-700 dark:text-gray-300" + ) %> +
+ <%= password_input(f, :password, + autocomplete: "new-password", + class: + "shadow-sm dark:bg-gray-900 dark:text-gray-300 focus:ring-indigo-500 focus:border-indigo-500 block w-full sm:text-sm border-gray-300 dark:border-gray-500 rounded-md dark:bg-gray-800" + ) %> + <%= error_tag(f, :password, only_first?: true) %> +
+
+
+ <%= label(f, :password_confirmation, "Confirm new password", + class: "block text-sm font-medium text-gray-700 dark:text-gray-300" + ) %> +
+ <%= password_input(f, :password_confirmation, + autocomplete: "new-password", + class: + "shadow-sm dark:bg-gray-900 dark:text-gray-300 focus:ring-indigo-500 focus:border-indigo-500 block w-full sm:text-sm border-gray-300 dark:border-gray-500 rounded-md dark:bg-gray-800" + ) %> + <%= error_tag(f, :password_confirmation, only_first?: true) %> +
+
+ +
+ <%= label(f, :two_factor_code, "Verify with 2FA", + class: "block text-sm font-medium text-gray-700 dark:text-gray-300" + ) %> +
+ +
+
+ + <%= submit("Change my password", + class: + "inline-block mt-4 px-4 py-2 border border-gray-300 dark:border-gray-500 text-sm leading-5 font-medium rounded-md text-red-600 dark:text-red-500 bg-white dark:bg-gray-800 hover:text-red-400 focus:outline-none focus:border-blue-300 focus:ring active:text-red-800 active:bg-gray-50 transition ease-in-out duration-150" + ) %> + <% end %> +
+

Two-Factor Authentication (2FA) diff --git a/lib/plausible_web/user_auth.ex b/lib/plausible_web/user_auth.ex index e7fbf8e2e2dd..451ecce54b3e 100644 --- a/lib/plausible_web/user_auth.ex +++ b/lib/plausible_web/user_auth.ex @@ -3,7 +3,7 @@ defmodule PlausibleWeb.UserAuth do Functions for user session management. """ - import Ecto.Query, only: [from: 2] + import Ecto.Query alias Plausible.Auth alias Plausible.Repo @@ -85,12 +85,20 @@ defmodule PlausibleWeb.UserAuth do :ok end - @spec revoke_all_user_sessions(Auth.User.t()) :: :ok - def revoke_all_user_sessions(user) do - {_count, tokens} = - Repo.delete_all( - from us in Auth.UserSession, where: us.user_id == ^user.id, select: us.token - ) + @spec revoke_all_user_sessions(Auth.User.t(), Keyword.t()) :: :ok + def revoke_all_user_sessions(user, opts \\ []) do + except = Keyword.get(opts, :except) + + delete_query = from us in Auth.UserSession, where: us.user_id == ^user.id, select: us.token + + delete_query = + if except do + where(delete_query, [us], us.id != ^except.id) + else + delete_query + end + + {_count, tokens} = Repo.delete_all(delete_query) Enum.each(tokens, fn token -> PlausibleWeb.Endpoint.broadcast(live_socket_id(token), "disconnect", %{}) diff --git a/lib/plausible_web/views/error_helpers.ex b/lib/plausible_web/views/error_helpers.ex index c0fdabdfff69..3c438316bce0 100644 --- a/lib/plausible_web/views/error_helpers.ex +++ b/lib/plausible_web/views/error_helpers.ex @@ -1,13 +1,24 @@ defmodule PlausibleWeb.ErrorHelpers do use Phoenix.HTML - def error_tag(%{errors: errors}, field) do - Enum.map(Keyword.get_values(errors, field), fn error -> + def error_tag(map_or_form, field, opts \\ []) + + def error_tag(%{errors: errors}, field, opts) do + error_messages = Keyword.get_values(errors, field) + + error_messages = + if Keyword.get(opts, :only_first?) do + Enum.take(error_messages, 1) + else + error_messages + end + + Enum.map(error_messages, fn error -> content_tag(:div, translate_error(error), class: "mt-2 text-sm text-red-500") end) end - def error_tag(assigns, field) when is_map(assigns) do + def error_tag(assigns, field, _opts) when is_map(assigns) do error = assigns[field] if error do diff --git a/test/plausible_web/controllers/auth_controller_test.exs b/test/plausible_web/controllers/auth_controller_test.exs index 19e749c78fa5..3f904a039eb0 100644 --- a/test/plausible_web/controllers/auth_controller_test.exs +++ b/test/plausible_web/controllers/auth_controller_test.exs @@ -1158,6 +1158,173 @@ defmodule PlausibleWeb.AuthControllerTest do end end + describe "PUT /settings/password" do + setup [:create_user, :log_in] + + test "updates the password and kills other sessions", %{conn: conn, user: user} do + password = "very-long-very-secret-123" + new_password = "super-long-super-secret-999" + + another_session = + user + |> Auth.UserSession.new_session("Some Device") + |> Repo.insert!() + + original = + user + |> User.set_password(password) + |> Repo.update!() + + conn = + put(conn, "/settings/password", %{ + "user" => %{ + "password" => new_password, + "old_password" => password, + "password_confirmation" => new_password + } + }) + + assert redirected_to(conn, 302) == + Routes.auth_path(conn, :user_settings) <> "#change-password" + + current_hash = Repo.reload!(user).password_hash + assert current_hash != original.password_hash + assert Plausible.Auth.Password.match?(new_password, current_hash) + + assert [remaining_session] = Repo.preload(user, :sessions).sessions + assert remaining_session.id != another_session.id + end + + test "fails to update weak password", %{conn: conn} do + password = "very-long-very-secret-123" + new_password = "weak" + + conn = + put(conn, "/settings/password", %{ + "user" => %{ + "password" => new_password, + "old_password" => password, + "password_confirmation" => new_password + } + }) + + assert html = html_response(conn, 200) + assert html =~ "is too weak" + end + + test "fails to update confirmation mismatch", %{conn: conn} do + password = "very-long-very-secret-123" + new_password = "super-long-super-secret-999" + + conn = + put(conn, "/settings/password", %{ + "user" => %{ + "password" => new_password, + "old_password" => password, + "password_confirmation" => new_password <> "mismatch" + } + }) + + assert html = html_response(conn, 200) + assert html =~ "does not match confirmation" + end + + test "updates the password when 2FA is enabled", %{conn: conn, user: user} do + password = "very-long-very-secret-123" + new_password = "super-long-super-secret-999" + + original = + user + |> User.set_password(password) + |> Repo.update!() + + {:ok, user, _} = Auth.TOTP.initiate(user) + {:ok, user, _} = Auth.TOTP.enable(user, :skip_verify) + + code = NimbleTOTP.verification_code(user.totp_secret) + + conn = + put(conn, "/settings/password", %{ + "user" => %{ + "password" => new_password, + "old_password" => password, + "password_confirmation" => new_password, + "two_factor_code" => code + } + }) + + assert redirected_to(conn, 302) == + Routes.auth_path(conn, :user_settings) <> "#change-password" + + current_hash = Repo.reload!(user).password_hash + assert current_hash != original.password_hash + assert Plausible.Auth.Password.match?(new_password, current_hash) + end + + test "fails to update with wrong 2fa code", %{conn: conn, user: user} do + password = "very-long-very-secret-123" + + user = + user + |> User.set_password(password) + |> Repo.update!() + + new_password = "super-long-super-secret-999" + + {:ok, user, _} = Auth.TOTP.initiate(user) + {:ok, _, _} = Auth.TOTP.enable(user, :skip_verify) + + conn = + put(conn, "/settings/password", %{ + "user" => %{ + "password" => new_password, + "old_password" => password, + "password_confirmation" => new_password, + "two_factor_code" => "111111" + } + }) + + assert html = html_response(conn, 200) + assert html =~ "invalid 2FA code" + end + + test "fails to update with missing 2fa code", %{conn: conn, user: user} do + password = "very-long-very-secret-123" + + user = + user + |> User.set_password(password) + |> Repo.update!() + + new_password = "super-long-super-secret-999" + + {:ok, user, _} = Auth.TOTP.initiate(user) + {:ok, _, _} = Auth.TOTP.enable(user, :skip_verify) + + conn = + put(conn, "/settings/password", %{ + "user" => %{ + "password" => new_password, + "old_password" => password, + "password_confirmation" => new_password + } + }) + + assert html = html_response(conn, 200) + assert html =~ "invalid 2FA code" + end + + test "fails to update with no input", %{conn: conn} do + conn = + put(conn, "/settings/password", %{ + "user" => %{} + }) + + assert html = html_response(conn, 200) + assert text(html) =~ "can't be blank" + end + end + describe "PUT /settings/email" do setup [:create_user, :log_in]