diff --git a/CHANGELOG.md b/CHANGELOG.md index 8961519a341..596c40a33f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,12 @@ and this project adheres to ## [Unreleased] +### Fixed + +- Profile email updates no longer show password validation errors while typing; + the current password is checked when submitting Update email. + [#2546](https://github.com/OpenFn/lightning/issues/2546) + ### Changed - The AI assistant is the global assistant for everyone. It was behind the diff --git a/lib/lightning/accounts.ex b/lib/lightning/accounts.ex index 5c754bb0a8a..6d5f9294ae6 100644 --- a/lib/lightning/accounts.ex +++ b/lib/lightning/accounts.ex @@ -703,37 +703,64 @@ defmodule Lightning.Accounts do @doc """ Validates the changes for updating a user's email address. - This function ensures that: + By default this function ensures that: - The `email` and `current_password` fields are present. - The new email is in a valid format. - The new email is different from the current one. - The provided `current_password` matches the user's password. + Pass `validate_password: false` for live (`phx-change`) validation so only + the email is checked; password presence and correctness are still enforced + on submit (the default). + ## Parameters - `user`: The `%User{}` struct representing the current user. - - `params`: A map of parameters containing the new email and current password. + - `params`: A map of parameters containing the new email and optionally + the current password. + - `opts`: Keyword options. `:validate_password` (default `true`) controls + whether `current_password` is required and verified. ## Returns An `Ecto.Changeset` containing any validation errors. + ## Options + + * `:validate_password` - when `true` (default), require and check + `current_password`. Set to `false` for live validation while typing so + password errors only appear on submit. + ## Examples iex> validate_change_user_email(user, %{"email" => "new@example.com", "current_password" => "secret"}) %Ecto.Changeset{...} """ - def validate_change_user_email(user, params \\ %{}) do + def validate_change_user_email(user, params \\ %{}, opts \\ []) do + validate_password? = Keyword.get(opts, :validate_password, true) + data = %{email: nil, current_password: nil} types = %{email: :string, current_password: :string} - {data, types} - |> Changeset.cast(params, Map.keys(types)) - |> Changeset.validate_required([:email, :current_password]) - |> User.validate_email() - |> validate_email_changed(user) - |> validate_current_password(user) + changeset = + {data, types} + |> Changeset.cast(params, Map.keys(types)) + |> then(fn changeset -> + if validate_password? do + Changeset.validate_required(changeset, [:email, :current_password]) + else + Changeset.validate_required(changeset, [:email]) + end + end) + |> User.validate_email() + |> validate_email_changed(user) + + if validate_password? do + validate_current_password(changeset, user) + else + changeset + end end defp validate_email_changed(changeset, user) do diff --git a/lib/lightning_web/live/profile_live/form_component.ex b/lib/lightning_web/live/profile_live/form_component.ex index 73ca0044895..f3aa0157e9e 100644 --- a/lib/lightning_web/live/profile_live/form_component.ex +++ b/lib/lightning_web/live/profile_live/form_component.ex @@ -110,7 +110,9 @@ defmodule LightningWeb.ProfileLive.FormComponent do def handle_event("validate_email", %{"user" => user_params}, socket) do changeset = socket.assigns.user - |> Accounts.validate_change_user_email(user_params) + |> Accounts.validate_change_user_email(user_params, + validate_password: false + ) |> Map.put(:action, :validate_email) {:noreply, assign(socket, :email_changeset, changeset)} @@ -126,6 +128,12 @@ defmodule LightningWeb.ProfileLive.FormComponent do {:noreply, assign(socket, :user_info_changeset, changeset)} end + def email_form_submit_disabled?(changeset) do + password = Ecto.Changeset.get_field(changeset, :current_password) + + not changeset.valid? or password in [nil, ""] + end + def enum_options(module, field) do value_label_map = %{ critical: "Critical", diff --git a/lib/lightning_web/live/profile_live/form_component.html.heex b/lib/lightning_web/live/profile_live/form_component.html.heex index 5ada64c6dca..25149631656 100644 --- a/lib/lightning_web/live/profile_live/form_component.html.heex +++ b/lib/lightning_web/live/profile_live/form_component.html.heex @@ -81,7 +81,6 @@ field={f[:current_password]} label="Enter password to confirm" required="true" - phx-debounce="blur" autocomplete="current-password" /> @@ -92,7 +91,7 @@ <.button type="submit" theme="primary" - disabled={!@email_changeset.valid?} + disabled={email_form_submit_disabled?(@email_changeset)} phx-disable-with="Sending confirmation email..." > Update email diff --git a/lib/lightning_web/live/user_confirmation_required_live.ex b/lib/lightning_web/live/user_confirmation_required_live.ex index 1c4d015d0c0..f926d08e0e4 100644 --- a/lib/lightning_web/live/user_confirmation_required_live.ex +++ b/lib/lightning_web/live/user_confirmation_required_live.ex @@ -70,7 +70,9 @@ defmodule LightningWeb.UserConfirmationRequiredLive do def handle_event("validate_email", %{"user" => user_params}, socket) do changeset = socket.assigns.current_user - |> Accounts.validate_change_user_email(user_params) + |> Accounts.validate_change_user_email(user_params, + validate_password: false + ) |> Map.put(:action, :validate_email) {:noreply, assign(socket, :email_changeset, changeset)} @@ -119,4 +121,10 @@ defmodule LightningWeb.UserConfirmationRequiredLive do )} end end + + def email_form_submit_disabled?(changeset) do + password = Ecto.Changeset.get_field(changeset, :current_password) + + not changeset.valid? or password in [nil, ""] + end end diff --git a/lib/lightning_web/live/user_confirmation_required_live.html.heex b/lib/lightning_web/live/user_confirmation_required_live.html.heex index e9623ec47af..086f5c0044c 100644 --- a/lib/lightning_web/live/user_confirmation_required_live.html.heex +++ b/lib/lightning_web/live/user_confirmation_required_live.html.heex @@ -109,13 +109,12 @@ field={f[:current_password]} label="Enter password to confirm" required="true" - phx-debounce="blur" autocomplete="current-password" /> <.button type="submit" theme="primary" - disabled={!@email_changeset.valid?} + disabled={email_form_submit_disabled?(@email_changeset)} phx-disable-with="Sending confirmation email..." > Update email diff --git a/test/lightning/accounts_test.exs b/test/lightning/accounts_test.exs index 539e3d4a574..d8dc95129f8 100644 --- a/test/lightning/accounts_test.exs +++ b/test/lightning/accounts_test.exs @@ -195,6 +195,20 @@ defmodule Lightning.AccountsTest do assert {"can't be blank", _} = errors[:current_password] end + test "skips password checks when validate_password is false" do + user = insert(:user) + + changeset = + Accounts.validate_change_user_email( + user, + %{"email" => "new@example.com", "current_password" => ""}, + validate_password: false + ) + + assert changeset.valid? + refute changeset.errors[:current_password] + end + test "gives a password-less account a changeset error rather than raising" do # AccountHook is adapter-pluggable, so an SSO-provisioned account can have # no hash at all. This form is the one page such an account can reach. diff --git a/test/lightning_web/live/profile_live_test.exs b/test/lightning_web/live/profile_live_test.exs index 6706f48997b..6e81eb8f6e8 100644 --- a/test/lightning_web/live/profile_live_test.exs +++ b/test/lightning_web/live/profile_live_test.exs @@ -43,7 +43,8 @@ defmodule LightningWeb.ProfileLiveTest do } @update_email_attrs %{ - email: "new@example.com" + email: "new@example.com", + current_password: "hello world!" } describe "Edit user profile" do @@ -170,6 +171,52 @@ defmodule LightningWeb.ProfileLiveTest do |> render_change() =~ "Please change your email" end + test "does not show password errors while typing a new email", %{ + conn: conn + } do + {:ok, profile_live, _html} = + live(conn, Routes.profile_edit_path(conn, :edit), on_error: :raise) + + html = + profile_live + |> form("#email-form", + user: %{email: "new_email_123@openfn.org", current_password: ""} + ) + |> render_change() + + refute html =~ "This field can't be blank." + refute html =~ "Your passwords do not match." + assert html =~ ~s(disabled="disabled") + end + + test "clears a prior password error when the user types again", %{ + conn: conn + } do + {:ok, profile_live, _html} = + live(conn, Routes.profile_edit_path(conn, :edit), on_error: :raise) + + assert profile_live + |> form("#email-form", + user: %{ + email: "new_email_123@openfn.org", + current_password: "invalid" + } + ) + |> render_submit() =~ "Your passwords do not match." + + html = + profile_live + |> form("#email-form", + user: %{ + email: "new_email_123@openfn.org", + current_password: "still typing" + } + ) + |> render_change() + + refute html =~ "Your passwords do not match." + end + test "a user can change their email address", %{conn: conn} do {:ok, profile_live, _html} = live(conn, Routes.profile_edit_path(conn, :edit), on_error: :raise) diff --git a/test/lightning_web/live/user_confirmation_required_live_test.exs b/test/lightning_web/live/user_confirmation_required_live_test.exs index 876247d548e..f01e8b1fad5 100644 --- a/test/lightning_web/live/user_confirmation_required_live_test.exs +++ b/test/lightning_web/live/user_confirmation_required_live_test.exs @@ -157,6 +157,54 @@ defmodule LightningWeb.UserConfirmationRequiredLiveTest do refute_email_sent(subject: "Please confirm your new email") end + test "does not show password errors while typing a new email", %{ + conn: conn, + user: user + } do + {:ok, view, _html} = + conn |> log_in_user(user) |> live(~p"/users/confirm-required") + + html = + view + |> form("#email-form", + user: %{email: "typo-fixed@example.com", current_password: ""} + ) + |> render_change() + + refute html =~ "This field can't be blank." + refute html =~ "Your passwords do not match." + assert html =~ ~s(disabled="disabled") + end + + test "clears a prior password error when the user types again", %{ + conn: conn, + user: user + } do + {:ok, view, _html} = + conn |> log_in_user(user) |> live(~p"/users/confirm-required") + + assert view + |> form("#email-form", + user: %{ + email: "typo-fixed@example.com", + current_password: "wrong" + } + ) + |> render_submit() =~ "Your passwords do not match." + + html = + view + |> form("#email-form", + user: %{ + email: "typo-fixed@example.com", + current_password: "still typing" + } + ) + |> render_change() + + refute html =~ "Your passwords do not match." + end + test "sends instructions to the new address", %{conn: conn, user: user} do {:ok, view, _html} = conn |> log_in_user(user) |> live(~p"/users/confirm-required")