diff --git a/lib/accounts/accounts.ex b/lib/accounts/accounts.ex index 7bc5073..333e12e 100644 --- a/lib/accounts/accounts.ex +++ b/lib/accounts/accounts.ex @@ -15,8 +15,6 @@ defmodule Mv.Accounts do define :list_users, action: :read define :update_user, action: :update_user define :destroy_user, action: :destroy - define :create_register_with_rauthy, action: :register_with_rauthy - define :read_sign_in_with_rauthy, action: :sign_in_with_rauthy end resource Mv.Accounts.Token diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex index d8c7a66..2da15a1 100644 --- a/lib/accounts/user.ex +++ b/lib/accounts/user.ex @@ -105,8 +105,6 @@ defmodule Mv.Accounts.User do upsert? true upsert_identity :unique_oidc_id - validate &__MODULE__.validate_oidc_id_present/2 - change AshAuthentication.GenerateTokenChange change fn changeset, _ctx -> @@ -127,16 +125,6 @@ defmodule Mv.Accounts.User do end end - def validate_oidc_id_present(changeset, _context) do - user_info = Ash.Changeset.get_argument(changeset, :user_info) || %{} - - if is_binary(user_info["sub"]) or is_binary(user_info["id"]) do - :ok - else - {:error, [user_info: "OIDC user_info must contain a non-empty 'sub' or 'id' field"]} - end - end - attributes do uuid_primary_key :id diff --git a/test/mv_web/controllers/auth_controller_test.exs b/test/mv_web/controllers/auth_controller_test.exs index 3c3956d..e18ade2 100644 --- a/test/mv_web/controllers/auth_controller_test.exs +++ b/test/mv_web/controllers/auth_controller_test.exs @@ -1,121 +1,40 @@ defmodule MvWeb.AuthControllerTest do use MvWeb.ConnCase, async: true - import Phoenix.LiveViewTest - # Basic UI tests test "GET /sign-in shows sign in form", %{conn: conn} do conn = get(conn, ~p"/sign-in") assert html_response(conn, 200) =~ "Sign in" end - test "GET /sign-out redirects to home", %{conn: conn} do + test "POST /sign-in with valid credentials redirects to home", %{conn: conn} do + # Create a test user first conn = conn_with_oidc_user(conn) + conn = get(conn, ~p"/sign-in") + + assert redirected_to(conn) == ~p"/" + end + + test "POST /sign-in with invalid credentials shows error", %{conn: conn} do + conn = + post(conn, ~p"/auth/sign_in", %{ + "user" => %{ + "email" => "wrong@example.com", + "password" => "wrongpassword" + } + }) + + assert conn.status == 404 + end + + test "GET /sign-out redirects to home", %{conn: conn} do + # First sign in a user + conn = conn_with_oidc_user(conn) + + # Then sign out conn = get(conn, ~p"/sign-out") assert redirected_to(conn) == ~p"/" end - # Password authentication (LiveView) - test "password user can sign in with valid credentials via LiveView", %{conn: conn} do - _user = - create_test_user(%{ - email: "password@example.com", - password: "secret123", - oidc_id: nil - }) - - {:ok, view, _html} = live(conn, "/sign-in") - - {:error, {:redirect, %{to: to}}} = - view - |> form("#user-password-sign-in-with-password", - user: %{email: "password@example.com", password: "secret123"} - ) - |> render_submit() - - assert to =~ "/auth/user/password/sign_in_with_token" - end - - test "password user with invalid credentials shows error via LiveView", %{conn: conn} do - _user = - create_test_user(%{ - email: "test@example.com", - password: "correct_password", - oidc_id: nil - }) - - {:ok, view, _html} = live(conn, "/sign-in") - - html = - view - |> form("#user-password-sign-in-with-password", - user: %{email: "test@example.com", password: "wrong_password"} - ) - |> render_submit() - - assert html =~ "Email or password was incorrect" - end - - test "password user with non-existent email shows error via LiveView", %{conn: conn} do - {:ok, view, _html} = live(conn, "/sign-in") - - html = - view - |> form("#user-password-sign-in-with-password", - user: %{email: "nonexistent@example.com", password: "anypassword"} - ) - |> render_submit() - - assert html =~ "Email or password was incorrect" - end - - # Registration (LiveView) - test "user can register with valid credentials via LiveView", %{conn: conn} do - {:ok, view, _html} = live(conn, "/register") - - {:error, {:redirect, %{to: to}}} = - view - |> form("#user-password-register-with-password-wrapper form", - user: %{email: "newuser@example.com", password: "newpassword123"} - ) - |> render_submit() - - assert to =~ "/auth/user/password/sign_in_with_token" - end - - test "registration with existing email shows error via LiveView", %{conn: conn} do - _user = - create_test_user(%{ - email: "existing@example.com", - password: "secret123", - oidc_id: nil - }) - - {:ok, view, _html} = live(conn, "/register") - - html = - view - |> form("#user-password-register-with-password-wrapper form", - user: %{email: "existing@example.com", password: "anotherpassword"} - ) - |> render_submit() - - assert html =~ "has already been taken" - end - - test "registration with weak password shows error via LiveView", %{conn: conn} do - {:ok, view, _html} = live(conn, "/register") - - html = - view - |> form("#user-password-register-with-password-wrapper form", - user: %{email: "weakpass@example.com", password: "123"} - ) - |> render_submit() - - assert html =~ "length must be greater than or equal to 8" - end - - # Access control test "unauthenticated user accessing protected route gets redirected to sign-in", %{conn: conn} do conn = get(conn, ~p"/members") assert redirected_to(conn) == ~p"/sign-in" @@ -126,67 +45,4 @@ defmodule MvWeb.AuthControllerTest do conn = get(conn, ~p"/members") assert conn.status == 200 end - - test "password authenticated user can access protected route via LiveView", %{conn: conn} do - _user = - create_test_user(%{ - email: "auth@example.com", - password: "secret123", - oidc_id: nil - }) - - {:ok, view, _html} = live(conn, "/sign-in") - - {:error, {:redirect, %{to: to}}} = - view - |> form("#user-password-sign-in-with-password", - user: %{email: "auth@example.com", password: "secret123"} - ) - |> render_submit() - - assert to =~ "/auth/user/password/sign_in_with_token" - - # After login, user is redirected to /auth/user/password/sign_in_with_token. Session handling for protected routes should be tested in integration or E2E tests. - end - - # Edge cases - test "user with nil oidc_id can still sign in with password via LiveView", %{conn: conn} do - _user = - create_test_user(%{ - email: "nil_oidc@example.com", - password: "secret123", - oidc_id: nil - }) - - {:ok, view, _html} = live(conn, "/sign-in") - - {:error, {:redirect, %{to: to}}} = - view - |> form("#user-password-sign-in-with-password", - user: %{email: "nil_oidc@example.com", password: "secret123"} - ) - |> render_submit() - - assert to =~ "/auth/user/password/sign_in_with_token" - end - - test "user with empty string oidc_id is handled correctly via LiveView", %{conn: conn} do - _user = - create_test_user(%{ - email: "empty_oidc@example.com", - password: "secret123", - oidc_id: "" - }) - - {:ok, view, _html} = live(conn, "/sign-in") - - {:error, {:redirect, %{to: to}}} = - view - |> form("#user-password-sign-in-with-password", - user: %{email: "empty_oidc@example.com", password: "secret123"} - ) - |> render_submit() - - assert to =~ "/auth/user/password/sign_in_with_token" - end end diff --git a/test/mv_web/controllers/oidc_integration_test.exs b/test/mv_web/controllers/oidc_integration_test.exs deleted file mode 100644 index a96e7b1..0000000 --- a/test/mv_web/controllers/oidc_integration_test.exs +++ /dev/null @@ -1,166 +0,0 @@ -defmodule MvWeb.OidcIntegrationTest do - use MvWeb.ConnCase, async: true - - # Test OIDC callback scenarios by directly calling the actions - # This simulates what happens during real OIDC authentication - - describe "OIDC sign-in scenarios" do - test "existing OIDC user with unchanged email can sign in" do - # Create user with OIDC ID - user = - create_test_user(%{ - email: "existing@example.com", - oidc_id: "existing_oidc_123" - }) - - # Simulate OIDC callback data - user_info = %{ - "sub" => "existing_oidc_123", - "preferred_username" => "existing@example.com" - } - - # Test sign_in_with_rauthy action directly - {:ok, [found_user]} = - Mv.Accounts.read_sign_in_with_rauthy(%{ - user_info: user_info, - oauth_tokens: %{} - }) - - assert found_user.id == user.id - assert to_string(found_user.email) == "existing@example.com" - assert found_user.oidc_id == "existing_oidc_123" - end - - test "new OIDC user gets created via register_with_rauthy" do - # Simulate OIDC callback for completely new user - user_info = %{ - "sub" => "brand_new_oidc_456", - "preferred_username" => "newuser@example.com" - } - - # Test register_with_rauthy action - case Mv.Accounts.create_register_with_rauthy(%{ - user_info: user_info, - oauth_tokens: %{} - }) do - {:ok, new_user} -> - assert to_string(new_user.email) == "newuser@example.com" - assert new_user.oidc_id == "brand_new_oidc_456" - assert is_nil(new_user.hashed_password) - - {:error, error} -> - flunk("Should have created new user: #{inspect(error)}") - end - end - end - - describe "OIDC error and edge case scenarios" do - test "OIDC registration with conflicting email and OIDC ID shows error" do - # Create user with email and OIDC ID - _existing_user = - create_test_user(%{ - email: "conflict@example.com", - oidc_id: "oidc_conflict_1" - }) - - # Try to register with same email but different OIDC ID - user_info = %{ - "sub" => "oidc_conflict_2", - "preferred_username" => "conflict@example.com" - } - - result = - Mv.Accounts.create_register_with_rauthy(%{ - user_info: user_info, - oauth_tokens: %{} - }) - - # Should fail due to unique constraint - assert {:error, %Ash.Error.Invalid{errors: errors}} = result - - assert Enum.any?(errors, fn - %Ash.Error.Changes.InvalidAttribute{field: :email, message: message} -> - String.contains?(message, "has already been taken") - - _ -> - false - end) - end - - test "OIDC registration with missing sub and id should fail" do - user_info = %{ - "preferred_username" => "nosub@example.com" - } - - result = - Mv.Accounts.create_register_with_rauthy(%{ - user_info: user_info, - oauth_tokens: %{} - }) - - assert {:error, - %Ash.Error.Invalid{ - errors: [%Ash.Error.Changes.InvalidChanges{vars: [user_info: msg]}] - }} = result - - assert String.contains?(msg, "OIDC user_info must contain a non-empty 'sub' or 'id' field") - end - - test "OIDC registration with missing preferred_username should fail" do - user_info = %{ - "sub" => "noemail_oidc_123" - } - - result = - Mv.Accounts.create_register_with_rauthy(%{ - user_info: user_info, - oauth_tokens: %{} - }) - - assert {:error, %Ash.Error.Invalid{errors: errors}} = result - - assert Enum.any?(errors, fn err -> - match?(%Ash.Error.Changes.Required{field: :email}, err) - end) - end - - test "OIDC registration with existing OIDC ID and different email updates email" do - existing_user = - create_test_user(%{ - email: "old@example.com", - oidc_id: "oidc_update_email" - }) - - user_info = %{ - "sub" => "oidc_update_email", - "preferred_username" => "new@example.com" - } - - {:ok, user} = - Mv.Accounts.create_register_with_rauthy(%{ - user_info: user_info, - oauth_tokens: %{} - }) - - assert user.id == existing_user.id - assert to_string(user.email) == "new@example.com" - assert user.oidc_id == "oidc_update_email" - end - - test "OIDC registration with alternative OIDC ID field (id instead of sub)" do - user_info = %{ - "id" => "alt_oidc_id_123", - "preferred_username" => "altid@example.com" - } - - {:ok, user} = - Mv.Accounts.create_register_with_rauthy(%{ - user_info: user_info, - oauth_tokens: %{} - }) - - assert user.oidc_id == "alt_oidc_id_123" - assert to_string(user.email) == "altid@example.com" - end - end -end