diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex index 970e65f..2da15a1 100644 --- a/lib/accounts/user.ex +++ b/lib/accounts/user.ex @@ -76,10 +76,10 @@ defmodule Mv.Accounts.User do update :admin_set_password do accept [:email] argument :password, :string, allow_nil?: false, sensitive?: true - + # Set the strategy context that HashPasswordChange expects change set_context(%{strategy_name: :password}) - + # Use the official Ash Authentication password change change AshAuthentication.Strategy.Password.HashPasswordChange end @@ -117,6 +117,14 @@ defmodule Mv.Accounts.User do end end + # Global validations - applied to all relevant actions + validations do + # Password strength policy: minimum 8 characters for all password-related actions + validate string_length(:password, min: 8) do + where action_is([:register_with_password, :admin_set_password]) + end + end + attributes do uuid_primary_key :id @@ -134,14 +142,6 @@ defmodule Mv.Accounts.User do identity :unique_oidc_id, [:oidc_id] end - # Global validations - applied to all relevant actions - validations do - # Password strength policy: minimum 8 characters for all password-related actions - validate string_length(:password, min: 8) do - where action_is([:register_with_password, :admin_set_password]) - end - end - # You can customize this if you wish, but this is a safe default that # only allows user data to be interacted with via AshAuthentication. # policies do diff --git a/lib/mv_web/live/user_live/form.ex b/lib/mv_web/live/user_live/form.ex index b599bc1..e77283a 100644 --- a/lib/mv_web/live/user_live/form.ex +++ b/lib/mv_web/live/user_live/form.ex @@ -12,8 +12,8 @@ defmodule MvWeb.UserLive.Form do <.form for={@form} id="user-form" phx-change="validate" phx-submit="save"> <.input field={@form[:email]} label={gettext("Email")} required type="email" /> - - + +
- + <%= if @show_password_fields do %>
- <.input - field={@form[:password]} - label={gettext("Password")} - type="password" + <.input + field={@form[:password]} + label={gettext("Password")} + type="password" required autocomplete="new-password" /> - + <%= if !@user do %> - <.input - field={@form[:password_confirmation]} - label={gettext("Confirm Password")} - type="password" + <.input + field={@form[:password_confirmation]} + label={gettext("Confirm Password")} + type="password" required autocomplete="new-password" /> <% end %> - +

{gettext("Password requirements")}:

    @@ -57,7 +57,7 @@ defmodule MvWeb.UserLive.Form do
  • {gettext("Consider using special characters")}
- + <%= if @user do %>

@@ -124,17 +124,15 @@ defmodule MvWeb.UserLive.Form do @impl true def handle_event("toggle_password_section", _params, socket) do show_password_fields = !socket.assigns.show_password_fields - - socket = + + socket = socket |> assign(:show_password_fields, show_password_fields) |> assign_form() - + {:noreply, socket} end - - def handle_event("validate", %{"user" => user_params}, socket) do {:noreply, assign(socket, form: AshPhoenix.Form.validate(socket.assigns.form, user_params))} end @@ -167,6 +165,7 @@ defmodule MvWeb.UserLive.Form do else # For new users, use password registration if password fields are shown action = if show_password_fields, do: :register_with_password, else: :create_user + AshPhoenix.Form.for_create(Mv.Accounts.User, action, domain: Mv.Accounts, as: "user" diff --git a/lib/mv_web/live/user_live/index.html.heex b/lib/mv_web/live/user_live/index.html.heex index 5b313f0..258779e 100644 --- a/lib/mv_web/live/user_live/index.html.heex +++ b/lib/mv_web/live/user_live/index.html.heex @@ -8,11 +8,7 @@ - <.table - id="users" - rows={@users} - row_click={fn user -> JS.navigate(~p"/users/#{user}") end} - > + <.table id="users" rows={@users} row_click={fn user -> JS.navigate(~p"/users/#{user}") end}> <:col :let={user} label={ @@ -72,4 +68,4 @@ - \ No newline at end of file + diff --git a/test/mv_web/user_live/form_test.exs b/test/mv_web/user_live/form_test.exs index 7988f3e..decc789 100644 --- a/test/mv_web/user_live/form_test.exs +++ b/test/mv_web/user_live/form_test.exs @@ -13,7 +13,7 @@ defmodule MvWeb.UserLive.FormTest do {:ok, view, html} = setup_live_view(conn, "/users/new") assert html =~ "New User" - assert html =~ "Email" + assert html =~ "Email" assert html =~ "Set Password" assert has_element?(view, "form#user-form[phx-submit='save']") assert has_element?(view, "input[name='user[email]']") @@ -53,13 +53,15 @@ defmodule MvWeb.UserLive.FormTest do {:ok, view, _html} = setup_live_view(conn, "/users/new") view |> element("input[name='set_password']") |> render_click() - + view - |> form("#user-form", user: %{ - email: "passworduser@example.com", - password: "securepassword123", - password_confirmation: "securepassword123" - }) + |> form("#user-form", + user: %{ + email: "passworduser@example.com", + password: "securepassword123", + password_confirmation: "securepassword123" + } + ) |> render_submit() assert_redirected(view, "/users") @@ -72,10 +74,13 @@ defmodule MvWeb.UserLive.FormTest do |> form("#user-form", user: %{email: "storetest@example.com"}) |> render_submit() - user = Ash.get!(Mv.Accounts.User, - [email: Ash.CiString.new("storetest@example.com")], - domain: Mv.Accounts - ) + user = + Ash.get!( + Mv.Accounts.User, + [email: Ash.CiString.new("storetest@example.com")], + domain: Mv.Accounts + ) + assert to_string(user.email) == "storetest@example.com" assert is_nil(user.hashed_password) end @@ -84,19 +89,24 @@ defmodule MvWeb.UserLive.FormTest do {:ok, view, _html} = setup_live_view(conn, "/users/new") view |> element("input[name='set_password']") |> render_click() - + view - |> form("#user-form", user: %{ - email: "passwordstoretest@example.com", - password: "securepassword123", - password_confirmation: "securepassword123" - }) + |> form("#user-form", + user: %{ + email: "passwordstoretest@example.com", + password: "securepassword123", + password_confirmation: "securepassword123" + } + ) |> render_submit() - user = Ash.get!(Mv.Accounts.User, - [email: Ash.CiString.new("passwordstoretest@example.com")], - domain: Mv.Accounts - ) + user = + Ash.get!( + Mv.Accounts.User, + [email: Ash.CiString.new("passwordstoretest@example.com")], + domain: Mv.Accounts + ) + assert user.hashed_password != nil assert String.starts_with?(user.hashed_password, "$2b$") end @@ -107,9 +117,10 @@ defmodule MvWeb.UserLive.FormTest do _existing_user = create_test_user(%{email: "existing@example.com"}) {:ok, view, _html} = setup_live_view(conn, "/users/new") - html = view - |> form("#user-form", user: %{email: "existing@example.com"}) - |> render_submit() + html = + view + |> form("#user-form", user: %{email: "existing@example.com"}) + |> render_submit() assert html =~ "has already been taken" end @@ -118,14 +129,17 @@ defmodule MvWeb.UserLive.FormTest do {:ok, view, _html} = setup_live_view(conn, "/users/new") view |> element("input[name='set_password']") |> render_click() - - html = view - |> form("#user-form", user: %{ - email: "test@example.com", - password: "123", - password_confirmation: "123" - }) - |> render_submit() + + html = + view + |> form("#user-form", + user: %{ + email: "test@example.com", + password: "123", + password_confirmation: "123" + } + ) + |> render_submit() assert html =~ "length must be greater than or equal to 8" end @@ -165,7 +179,7 @@ defmodule MvWeb.UserLive.FormTest do |> render_submit() assert_redirected(view, "/users") - + updated_user = Ash.reload!(user, domain: Mv.Accounts) assert to_string(updated_user.email) == "new@example.com" assert updated_user.hashed_password == original_password @@ -177,16 +191,18 @@ defmodule MvWeb.UserLive.FormTest do {:ok, view, _html} = setup_live_view(conn, "/users/#{user.id}/edit") view |> element("input[name='set_password']") |> render_click() - + view - |> form("#user-form", user: %{ - email: "user@example.com", - password: "newadminpassword123" - }) + |> form("#user-form", + user: %{ + email: "user@example.com", + password: "newadminpassword123" + } + ) |> render_submit() assert_redirected(view, "/users") - + updated_user = Ash.reload!(user, domain: Mv.Accounts) assert updated_user.hashed_password != original_password assert String.starts_with?(updated_user.hashed_password, "$2b$") @@ -199,9 +215,10 @@ defmodule MvWeb.UserLive.FormTest do user_to_edit = create_test_user(%{email: "original@example.com"}) {:ok, view, _html} = setup_live_view(conn, "/users/#{user_to_edit.id}/edit") - html = view - |> form("#user-form", user: %{email: "taken@example.com"}) - |> render_submit() + html = + view + |> form("#user-form", user: %{email: "taken@example.com"}) + |> render_submit() assert html =~ "has already been taken" end @@ -211,17 +228,21 @@ defmodule MvWeb.UserLive.FormTest do {:ok, view, _html} = setup_live_view(conn, "/users/#{user.id}/edit") view |> element("input[name='set_password']") |> render_click() - - result = view - |> form("#user-form", user: %{ - email: "user@example.com", - password: "123" - }) - |> render_submit() + + result = + view + |> form("#user-form", + user: %{ + email: "user@example.com", + password: "123" + } + ) + |> render_submit() case result do {:error, {:live_redirect, %{to: "/users"}}} -> flunk("Expected validation error but form was submitted successfully") + html when is_binary(html) -> assert html =~ "must have length of at least 8" end @@ -260,4 +281,4 @@ defmodule MvWeb.UserLive.FormTest do assert edit_html =~ "Change Password" end end -end \ No newline at end of file +end diff --git a/test/mv_web/user_live/index_test.exs b/test/mv_web/user_live/index_test.exs index ac4e65a..40756eb 100644 --- a/test/mv_web/user_live/index_test.exs +++ b/test/mv_web/user_live/index_test.exs @@ -65,12 +65,12 @@ defmodule MvWeb.UserLive.IndexTest do # Should show ascending indicator (up arrow) assert html =~ "hero-chevron-up" assert html =~ ~s(aria-sort="ascending") - + # Test actual sort order: alpha should appear before mike, mike before zulu alpha_pos = html |> :binary.match("alpha@example.com") |> elem(0) mike_pos = html |> :binary.match("mike@example.com") |> elem(0) zulu_pos = html |> :binary.match("zulu@example.com") |> elem(0) - + assert alpha_pos < mike_pos, "alpha@example.com should appear before mike@example.com" assert mike_pos < zulu_pos, "mike@example.com should appear before zulu@example.com" end @@ -78,21 +78,24 @@ defmodule MvWeb.UserLive.IndexTest do test "can sort email descending by clicking sort button", %{conn: conn} do conn = conn_with_oidc_user(conn) {:ok, view, _html} = live(conn, "/users") - + # Click on email sort button and get rendered result html = view |> element("button[phx-value-field='email']") |> render_click() # Should now show descending indicator (down arrow) assert html =~ "hero-chevron-down" assert html =~ ~s(aria-sort="descending") - + # Test actual sort order reversed: zulu should now appear before mike, mike before alpha alpha_pos = html |> :binary.match("alpha@example.com") |> elem(0) mike_pos = html |> :binary.match("mike@example.com") |> elem(0) zulu_pos = html |> :binary.match("zulu@example.com") |> elem(0) - - assert zulu_pos < mike_pos, "zulu@example.com should appear before mike@example.com when sorted desc" - assert mike_pos < alpha_pos, "mike@example.com should appear before alpha@example.com when sorted desc" + + assert zulu_pos < mike_pos, + "zulu@example.com should appear before mike@example.com when sorted desc" + + assert mike_pos < alpha_pos, + "mike@example.com should appear before alpha@example.com when sorted desc" end test "toggles back to ascending when clicking sort button twice", %{conn: conn} do @@ -106,12 +109,12 @@ defmodule MvWeb.UserLive.IndexTest do # Should be back to ascending assert html =~ "hero-chevron-up" assert html =~ ~s(aria-sort="ascending") - + # Should be back to original ascending order alpha_pos = html |> :binary.match("alpha@example.com") |> elem(0) mike_pos = html |> :binary.match("mike@example.com") |> elem(0) zulu_pos = html |> :binary.match("zulu@example.com") |> elem(0) - + assert alpha_pos < mike_pos, "Should be back to ascending: alpha before mike" assert mike_pos < zulu_pos, "Should be back to ascending: mike before zulu" end @@ -162,16 +165,20 @@ defmodule MvWeb.UserLive.IndexTest do # Initially, individual checkboxes should exist but not be checked assert view |> element("input[type='checkbox'][name='#{user1.id}']") |> has_element?() assert view |> element("input[type='checkbox'][name='#{user2.id}']") |> has_element?() - + # Initially, select_all should not be checked (since no individual items are selected) - refute view |> element("input[type='checkbox'][name='select_all'][checked]") |> has_element?() + refute view + |> element("input[type='checkbox'][name='select_all'][checked]") + |> has_element?() # Select first user checkbox html = view |> element("input[type='checkbox'][name='#{user1.id}']") |> render_click() # The select_all checkbox should still not be checked (not all users selected) - refute view |> element("input[type='checkbox'][name='select_all'][checked]") |> has_element?() - + refute view + |> element("input[type='checkbox'][name='select_all'][checked]") + |> has_element?() + # Page should still function normally assert html =~ "Email" assert html =~ to_string(user1.email) @@ -186,10 +193,12 @@ defmodule MvWeb.UserLive.IndexTest do # Then deselect user html = view |> element("input[type='checkbox'][name='#{user1.id}']") |> render_click() - + # Select all should not be checked after deselecting individual user - refute view |> element("input[type='checkbox'][name='select_all'][checked]") |> has_element?() - + refute view + |> element("input[type='checkbox'][name='select_all'][checked]") + |> has_element?() + # Page should still function normally assert html =~ "Email" assert html =~ to_string(user1.email) @@ -200,16 +209,26 @@ defmodule MvWeb.UserLive.IndexTest do {:ok, view, _html} = live(conn, "/users") # Initially no checkboxes should be checked - refute view |> element("input[type='checkbox'][name='select_all'][checked]") |> has_element?() - refute view |> element("input[type='checkbox'][name='#{user1.id}'][checked]") |> has_element?() - refute view |> element("input[type='checkbox'][name='#{user2.id}'][checked]") |> has_element?() + refute view + |> element("input[type='checkbox'][name='select_all'][checked]") + |> has_element?() + + refute view + |> element("input[type='checkbox'][name='#{user1.id}'][checked]") + |> has_element?() + + refute view + |> element("input[type='checkbox'][name='#{user2.id}'][checked]") + |> has_element?() # Click select all html = view |> element("input[type='checkbox'][name='select_all']") |> render_click() # After selecting all, the select_all checkbox should be checked - assert view |> element("input[type='checkbox'][name='select_all'][checked]") |> has_element?() - + assert view + |> element("input[type='checkbox'][name='select_all'][checked]") + |> has_element?() + # Page should still function normally and show all users assert html =~ "Email" assert html =~ to_string(user1.email) @@ -222,35 +241,52 @@ defmodule MvWeb.UserLive.IndexTest do # Select all first view |> element("input[type='checkbox'][name='select_all']") |> render_click() - + # Verify that select_all is checked - assert view |> element("input[type='checkbox'][name='select_all'][checked]") |> has_element?() + assert view + |> element("input[type='checkbox'][name='select_all'][checked]") + |> has_element?() # Then deselect all html = view |> element("input[type='checkbox'][name='select_all']") |> render_click() - + # After deselecting all, no checkboxes should be checked - refute view |> element("input[type='checkbox'][name='select_all'][checked]") |> has_element?() - refute view |> element("input[type='checkbox'][name='#{user1.id}'][checked]") |> has_element?() - refute view |> element("input[type='checkbox'][name='#{user2.id}'][checked]") |> has_element?() - + refute view + |> element("input[type='checkbox'][name='select_all'][checked]") + |> has_element?() + + refute view + |> element("input[type='checkbox'][name='#{user1.id}'][checked]") + |> has_element?() + + refute view + |> element("input[type='checkbox'][name='#{user2.id}'][checked]") + |> has_element?() + # Page should still function normally assert html =~ "Email" assert html =~ to_string(user1.email) assert html =~ to_string(user2.email) end - test "select all automatically checks when all individual users are selected", %{conn: conn, users: [user1, user2]} do + test "select all automatically checks when all individual users are selected", %{ + conn: conn, + users: [user1, user2] + } do conn = conn_with_oidc_user(conn) {:ok, view, _html} = live(conn, "/users") # Initially nothing should be checked - refute view |> element("input[type='checkbox'][name='select_all'][checked]") |> has_element?() + refute view + |> element("input[type='checkbox'][name='select_all'][checked]") + |> has_element?() # Select first user view |> element("input[type='checkbox'][name='#{user1.id}']") |> render_click() # Select all should still not be checked (only 1 of 2+ users selected) - refute view |> element("input[type='checkbox'][name='select_all'][checked]") |> has_element?() + refute view + |> element("input[type='checkbox'][name='select_all'][checked]") + |> has_element?() # Select second user html = view |> element("input[type='checkbox'][name='#{user2.id}']") |> render_click() @@ -278,7 +314,8 @@ defmodule MvWeb.UserLive.IndexTest do # The page should still render (basic functionality test) html = render(view) - assert html =~ "Email" # Table header should still be there + # Table header should still be there + assert html =~ "Email" end test "shows delete confirmation", %{conn: conn} do @@ -336,7 +373,8 @@ defmodule MvWeb.UserLive.IndexTest do # Note: English translations might be empty strings by default # This test would verify the structure is there - assert html =~ ~s(aria-label=) # Checking that aria-label attributes exist + # Checking that aria-label attributes exist + assert html =~ ~s(aria-label=) end end @@ -371,5 +409,4 @@ defmodule MvWeb.UserLive.IndexTest do assert html =~ long_email end end - -end \ No newline at end of file +end diff --git a/test/support/conn_case.ex b/test/support/conn_case.ex index 385083d..c51fb61 100644 --- a/test/support/conn_case.ex +++ b/test/support/conn_case.ex @@ -34,14 +34,14 @@ defmodule MvWeb.ConnCase do @doc """ Creates a test user and returns the user struct. Accepts attrs to override default values. - + Password handling: - If `hashed_password` is provided in attrs, it's used directly - If `password` is provided in attrs, it gets hashed automatically - If neither is provided, uses default password "password" - + ## Examples - + create_test_user() # Default user with unique email create_test_user(%{email: "custom@example.com"}) # Custom email create_test_user(%{password: "secret123"}) # Custom password (gets hashed) @@ -50,35 +50,38 @@ defmodule MvWeb.ConnCase do def create_test_user(attrs \\ %{}) do # Generate unique values to avoid conflicts unique_id = System.unique_integer([:positive]) - + default_attrs = %{ email: "user#{unique_id}@example.com", oidc_id: "oidc#{unique_id}" } - + # Merge provided attrs with defaults user_attrs = Map.merge(default_attrs, attrs) - + # Handle password/hashed_password - final_attrs = cond do - # If hashed_password is already provided, use it as-is - Map.has_key?(user_attrs, :hashed_password) -> - user_attrs - - # If password is provided, hash it - Map.has_key?(user_attrs, :password) -> - password = Map.get(user_attrs, :password) - {:ok, hashed_password} = AshAuthentication.BcryptProvider.hash(password) - user_attrs - |> Map.delete(:password) # Remove plain password - |> Map.put(:hashed_password, hashed_password) - - # Neither provided, use default password - true -> - password = "password" - {:ok, hashed_password} = AshAuthentication.BcryptProvider.hash(password) - Map.put(user_attrs, :hashed_password, hashed_password) - end + final_attrs = + cond do + # If hashed_password is already provided, use it as-is + Map.has_key?(user_attrs, :hashed_password) -> + user_attrs + + # If password is provided, hash it + Map.has_key?(user_attrs, :password) -> + password = Map.get(user_attrs, :password) + {:ok, hashed_password} = AshAuthentication.BcryptProvider.hash(password) + + user_attrs + # Remove plain password + |> Map.delete(:password) + |> Map.put(:hashed_password, hashed_password) + + # Neither provided, use default password + true -> + password = "password" + {:ok, hashed_password} = AshAuthentication.BcryptProvider.hash(password) + Map.put(user_attrs, :hashed_password, hashed_password) + end Ash.Seed.seed!(Mv.Accounts.User, final_attrs) end