Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 36 additions & 9 deletions lib/lightning/accounts.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +728 to +732

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accepted — docstring updated to describe validate_password: false for live validation.


## 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
Expand Down
10 changes: 9 additions & 1 deletion lib/lightning_web/live/profile_live/form_component.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
Expand All @@ -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",
Expand Down
3 changes: 1 addition & 2 deletions lib/lightning_web/live/profile_live/form_component.html.heex
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@
field={f[:current_password]}
label="Enter password to confirm"
required="true"
phx-debounce="blur"
autocomplete="current-password"
/>
</div>
Expand All @@ -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
Expand Down
10 changes: 9 additions & 1 deletion lib/lightning_web/live/user_confirmation_required_live.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
Comment on lines 70 to 78

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accepted — added LiveView coverage on the confirmation-required form for no password errors while typing, disabled submit until password present, and clearing a prior mismatch on subsequent change.

Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions test/lightning/accounts_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
49 changes: 48 additions & 1 deletion test/lightning_web/live/profile_live_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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&#39;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)
Expand Down
48 changes: 48 additions & 0 deletions test/lightning_web/live/user_confirmation_required_live_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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&#39;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")
Expand Down