diff --git a/test/accounts/user_authentication_test.exs b/test/accounts/user_authentication_test.exs new file mode 100644 index 0000000..caa3359 --- /dev/null +++ b/test/accounts/user_authentication_test.exs @@ -0,0 +1,265 @@ +defmodule Mv.Accounts.UserAuthenticationTest do + @moduledoc """ + Tests for user authentication and identification mechanisms. + + This test suite verifies that: + - Password login correctly identifies users via email + - OIDC login correctly identifies users via oidc_id + - Session identifiers work as expected for both authentication methods + """ + use MvWeb.ConnCase, async: true + require Ash.Query + + describe "Password authentication user identification" do + @tag :test_proposal + test "password login uses email as identifier" do + # Create a user with password authentication (no oidc_id) + user = + create_test_user(%{ + email: "password.user@example.com", + password: "securepassword123", + oidc_id: nil + }) + + # Verify that the user can be found by email + email_to_find = to_string(user.email) + + {:ok, users} = + Mv.Accounts.User + |> Ash.Query.filter(email == ^email_to_find) + |> Ash.read() + + assert length(users) == 1 + found_user = List.first(users) + assert found_user.id == user.id + assert to_string(found_user.email) == "password.user@example.com" + assert is_nil(found_user.oidc_id) + end + + @tag :test_proposal + test "password authentication uses email as identity_field" do + # Verify the configuration: password strategy should use email as identity_field + # This test checks the AshAuthentication configuration + + strategies = AshAuthentication.Info.authentication_strategies(Mv.Accounts.User) + password_strategy = Enum.find(strategies, fn s -> s.name == :password end) + + assert password_strategy != nil + assert password_strategy.identity_field == :email + end + + @tag :test_proposal + test "multiple users can exist with different emails" do + user1 = + create_test_user(%{ + email: "user1@example.com", + password: "password123", + oidc_id: nil + }) + + user2 = + create_test_user(%{ + email: "user2@example.com", + password: "password456", + oidc_id: nil + }) + + assert user1.id != user2.id + assert to_string(user1.email) != to_string(user2.email) + end + + @tag :test_proposal + test "users with same password but different emails are separate accounts" do + same_password = "shared_password_123" + + user1 = + create_test_user(%{ + email: "alice@example.com", + password: same_password, + oidc_id: nil + }) + + user2 = + create_test_user(%{ + email: "bob@example.com", + password: same_password, + oidc_id: nil + }) + + # Different users despite same password + assert user1.id != user2.id + + # Both passwords should hash to different values (bcrypt uses salt) + assert user1.hashed_password != user2.hashed_password + end + end + + describe "OIDC authentication user identification" do + @tag :test_proposal + test "OIDC login with matching oidc_id finds correct user" do + # Create user with OIDC authentication + user = + create_test_user(%{ + email: "oidc.user@example.com", + oidc_id: "oidc_identifier_12345" + }) + + # Simulate OIDC callback + user_info = %{ + "sub" => "oidc_identifier_12345", + "preferred_username" => "oidc.user@example.com" + } + + # Use sign_in_with_rauthy to find user by oidc_id + # Note: This test will FAIL until we implement the security fix + # that changes the filter from email to oidc_id + result = + Mv.Accounts.read_sign_in_with_rauthy(%{ + user_info: user_info, + oauth_tokens: %{} + }) + + case result do + {:ok, [found_user]} -> + assert found_user.id == user.id + assert found_user.oidc_id == "oidc_identifier_12345" + + {:ok, []} -> + flunk("User should be found by oidc_id") + + {:error, error} -> + flunk("Unexpected error: #{inspect(error)}") + end + end + + @tag :test_proposal + test "OIDC login creates new user when both email and oidc_id are new" do + # Completely new user from OIDC provider + user_info = %{ + "sub" => "brand_new_oidc_789", + "preferred_username" => "newuser@example.com" + } + + # Should create via register_with_rauthy + {:ok, new_user} = + Mv.Accounts.create_register_with_rauthy(%{ + user_info: user_info, + oauth_tokens: %{} + }) + + assert to_string(new_user.email) == "newuser@example.com" + assert new_user.oidc_id == "brand_new_oidc_789" + assert is_nil(new_user.hashed_password) + end + + @tag :test_proposal + test "OIDC user can be uniquely identified by oidc_id" do + user1 = + create_test_user(%{ + email: "user1@example.com", + oidc_id: "oidc_unique_1" + }) + + user2 = + create_test_user(%{ + email: "user2@example.com", + oidc_id: "oidc_unique_2" + }) + + # Find by oidc_id + {:ok, users1} = + Mv.Accounts.User + |> Ash.Query.filter(oidc_id == "oidc_unique_1") + |> Ash.read() + + {:ok, users2} = + Mv.Accounts.User + |> Ash.Query.filter(oidc_id == "oidc_unique_2") + |> Ash.read() + + assert length(users1) == 1 + assert length(users2) == 1 + assert List.first(users1).id == user1.id + assert List.first(users2).id == user2.id + end + end + + describe "Mixed authentication scenarios" do + @tag :test_proposal + test "user with oidc_id cannot be found by email-only query in sign_in_with_rauthy" do + # This test verifies the security fix: sign_in_with_rauthy should NOT + # match users by email, only by oidc_id + + _user = + create_test_user(%{ + email: "secure@example.com", + oidc_id: "secure_oidc_999" + }) + + # Try to sign in with DIFFERENT oidc_id but SAME email + user_info = %{ + # Different oidc_id! + "sub" => "attacker_oidc_888", + # Same email + "preferred_username" => "secure@example.com" + } + + # Should NOT find the user (security requirement) + result = + Mv.Accounts.read_sign_in_with_rauthy(%{ + user_info: user_info, + oauth_tokens: %{} + }) + + # Either returns empty list OR authentication error - both mean "user not found" + case result do + {:ok, []} -> + :ok + + {:error, %Ash.Error.Forbidden{errors: [%AshAuthentication.Errors.AuthenticationFailed{}]}} -> + :ok + + other -> + flunk("sign_in_with_rauthy should not match by email alone, got: #{inspect(other)}") + end + end + + @tag :test_proposal + test "password user (oidc_id=nil) is not found by sign_in_with_rauthy" do + # Create a password-only user + _user = + create_test_user(%{ + email: "password.only@example.com", + password: "securepass123", + oidc_id: nil + }) + + # Try OIDC sign-in with this email + user_info = %{ + "sub" => "new_oidc_777", + "preferred_username" => "password.only@example.com" + } + + # Should NOT find the user because oidc_id is nil + result = + Mv.Accounts.read_sign_in_with_rauthy(%{ + user_info: user_info, + oauth_tokens: %{} + }) + + # Either returns empty list OR authentication error - both mean "user not found" + case result do + {:ok, []} -> + :ok + + {:error, %Ash.Error.Forbidden{errors: [%AshAuthentication.Errors.AuthenticationFailed{}]}} -> + :ok + + other -> + flunk( + "Password-only user should not be found by sign_in_with_rauthy, got: #{inspect(other)}" + ) + end + end + end +end diff --git a/test/mv_web/controllers/oidc_integration_test.exs b/test/mv_web/controllers/oidc_integration_test.exs index a96e7b1..508ebab 100644 --- a/test/mv_web/controllers/oidc_integration_test.exs +++ b/test/mv_web/controllers/oidc_integration_test.exs @@ -54,10 +54,130 @@ defmodule MvWeb.OidcIntegrationTest do end end + describe "OIDC sign-in security tests" do + @tag :test_proposal + test "sign_in_with_rauthy does NOT match user with only email (no oidc_id)" do + # SECURITY TEST: Ensure password-only users cannot be accessed via OIDC + # Create a password-only user (no oidc_id) + _password_user = + create_test_user(%{ + email: "password.only@example.com", + password: "securepassword123", + oidc_id: nil + }) + + # Try to sign in with OIDC using the same email + user_info = %{ + "sub" => "attacker_oidc_456", + "preferred_username" => "password.only@example.com" + } + + # Should NOT find any user (security requirement) + result = + Mv.Accounts.read_sign_in_with_rauthy(%{ + user_info: user_info, + oauth_tokens: %{} + }) + + # Either returns empty list OR authentication error - both mean "user not found" + case result do + {:ok, []} -> + :ok + + {:error, %Ash.Error.Forbidden{errors: [%AshAuthentication.Errors.AuthenticationFailed{}]}} -> + :ok + + other -> + flunk("Expected no user match, got: #{inspect(other)}") + end + end + + @tag :test_proposal + test "sign_in_with_rauthy only matches when oidc_id matches" do + # Create user with specific OIDC ID + user = + create_test_user(%{ + email: "oidc.user@example.com", + oidc_id: "correct_oidc_789" + }) + + # Try with correct oidc_id + correct_user_info = %{ + "sub" => "correct_oidc_789", + "preferred_username" => "oidc.user@example.com" + } + + {:ok, [found_user]} = + Mv.Accounts.read_sign_in_with_rauthy(%{ + user_info: correct_user_info, + oauth_tokens: %{} + }) + + assert found_user.id == user.id + + # Try with wrong oidc_id but correct email + wrong_user_info = %{ + "sub" => "wrong_oidc_999", + "preferred_username" => "oidc.user@example.com" + } + + result = + Mv.Accounts.read_sign_in_with_rauthy(%{ + user_info: wrong_user_info, + oauth_tokens: %{} + }) + + # Either returns empty list OR authentication error - both mean "user not found" + case result do + {:ok, []} -> + :ok + + {:error, %Ash.Error.Forbidden{errors: [%AshAuthentication.Errors.AuthenticationFailed{}]}} -> + :ok + + other -> + flunk("Expected no user match when oidc_id differs, got: #{inspect(other)}") + end + end + + @tag :test_proposal + test "sign_in_with_rauthy does not match user with empty string oidc_id" do + # Edge case: empty string should be treated like nil + _user = + create_test_user(%{ + email: "empty.oidc@example.com", + oidc_id: "" + }) + + user_info = %{ + "sub" => "new_oidc_111", + "preferred_username" => "empty.oidc@example.com" + } + + result = + Mv.Accounts.read_sign_in_with_rauthy(%{ + user_info: user_info, + oauth_tokens: %{} + }) + + # Either returns empty list OR authentication error - both mean "user not found" + case result do + {:ok, []} -> + :ok + + {:error, %Ash.Error.Forbidden{errors: [%AshAuthentication.Errors.AuthenticationFailed{}]}} -> + :ok + + other -> + flunk("Expected no user match with empty oidc_id, got: #{inspect(other)}") + 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 = + existing_user = create_test_user(%{ email: "conflict@example.com", oidc_id: "oidc_conflict_1" @@ -75,12 +195,15 @@ defmodule MvWeb.OidcIntegrationTest do oauth_tokens: %{} }) - # Should fail due to unique constraint + # Should fail with PasswordVerificationRequired (account conflict) + # This prevents someone with OIDC provider B from taking over an account + # that's already linked to OIDC provider A assert {:error, %Ash.Error.Invalid{errors: errors}} = result + # Should contain PasswordVerificationRequired error assert Enum.any?(errors, fn - %Ash.Error.Changes.InvalidAttribute{field: :email, message: message} -> - String.contains?(message, "has already been taken") + %Mv.Accounts.User.Errors.PasswordVerificationRequired{user_id: user_id} -> + user_id == existing_user.id _ -> false diff --git a/test/mv_web/controllers/oidc_password_linking_test.exs b/test/mv_web/controllers/oidc_password_linking_test.exs new file mode 100644 index 0000000..b59633c --- /dev/null +++ b/test/mv_web/controllers/oidc_password_linking_test.exs @@ -0,0 +1,338 @@ +defmodule MvWeb.OidcPasswordLinkingTest do + @moduledoc """ + Tests for OIDC account linking when email collision occurs. + + This test suite verifies the security flow when an OIDC login attempts + to use an email that already exists in the system with a password account. + """ + use MvWeb.ConnCase, async: true + require Ash.Query + + describe "OIDC login with existing email (no oidc_id) - Email Collision" do + @tag :test_proposal + test "OIDC register with existing password user email fails with PasswordVerificationRequired" do + # Create password-only user + existing_user = + create_test_user(%{ + email: "existing@example.com", + password: "securepassword123", + oidc_id: nil + }) + + # Try OIDC registration with same email + user_info = %{ + "sub" => "new_oidc_12345", + "preferred_username" => "existing@example.com" + } + + result = + Mv.Accounts.create_register_with_rauthy(%{ + user_info: user_info, + oauth_tokens: %{} + }) + + # Should fail with PasswordVerificationRequired error + assert {:error, %Ash.Error.Invalid{errors: errors}} = result + + # Check that the error is our custom PasswordVerificationRequired + password_verification_error = + Enum.find(errors, fn err -> + err.__struct__ == Mv.Accounts.User.Errors.PasswordVerificationRequired + end) + + assert password_verification_error != nil, + "Should contain PasswordVerificationRequired error" + + assert password_verification_error.user_id == existing_user.id + end + + @tag :test_proposal + test "PasswordVerificationRequired error contains necessary context" do + existing_user = + create_test_user(%{ + email: "test@example.com", + password: "password123", + oidc_id: nil + }) + + user_info = %{ + "sub" => "oidc_99999", + "preferred_username" => "test@example.com" + } + + {:error, %Ash.Error.Invalid{errors: errors}} = + Mv.Accounts.create_register_with_rauthy(%{ + user_info: user_info, + oauth_tokens: %{} + }) + + password_error = + Enum.find(errors, fn err -> + err.__struct__ == Mv.Accounts.User.Errors.PasswordVerificationRequired + end) + + # Verify error contains all necessary context + assert password_error.user_id == existing_user.id + assert password_error.oidc_user_info["sub"] == "oidc_99999" + assert password_error.oidc_user_info["preferred_username"] == "test@example.com" + end + + @tag :test_proposal + test "after successful password verification, oidc_id can be set" do + # Create password user + user = + create_test_user(%{ + email: "link@example.com", + password: "mypassword123", + oidc_id: nil + }) + + # Simulate password verification passed, now link OIDC + user_info = %{ + "sub" => "linked_oidc_555", + "preferred_username" => "link@example.com" + } + + # Use the link_oidc_id action + {:ok, updated_user} = + Mv.Accounts.User + |> Ash.Query.filter(id == ^user.id) + |> Ash.read_one!() + |> Ash.Changeset.for_update(:link_oidc_id, %{ + oidc_id: user_info["sub"], + oidc_user_info: user_info + }) + |> Ash.update() + + assert updated_user.id == user.id + assert updated_user.oidc_id == "linked_oidc_555" + assert to_string(updated_user.email) == "link@example.com" + # Password should still exist + assert updated_user.hashed_password == user.hashed_password + end + + @tag :test_proposal + test "password verification with wrong password keeps oidc_id as nil" do + # This test verifies that if password verification fails, + # the oidc_id should NOT be set + + user = + create_test_user(%{ + email: "secure@example.com", + password: "correctpassword", + oidc_id: nil + }) + + # This test verifies the CONCEPT that wrong password should prevent linking + # In practice, the password verification happens BEFORE calling link_oidc_id + # So we just verify that the user still has no oidc_id + + # Attempt to verify with wrong password would fail in the controller/LiveView + # before link_oidc_id is called, so here we just verify the user state + + # User should still have no oidc_id (no linking happened) + {:ok, unchanged_user} = Ash.get(Mv.Accounts.User, user.id) + assert is_nil(unchanged_user.oidc_id) + assert unchanged_user.hashed_password == user.hashed_password + end + end + + describe "OIDC login with email of user having different oidc_id - Account Conflict" do + @tag :test_proposal + test "OIDC register with email of user having different oidc_id fails" do + # User already linked to OIDC provider A + _existing_user = + create_test_user(%{ + email: "linked@example.com", + oidc_id: "oidc_provider_a_123" + }) + + # Someone tries to register with OIDC provider B using same email + user_info = %{ + # Different OIDC ID! + "sub" => "oidc_provider_b_456", + "preferred_username" => "linked@example.com" + } + + result = + Mv.Accounts.create_register_with_rauthy(%{ + user_info: user_info, + oauth_tokens: %{} + }) + + # Should fail - cannot link different OIDC account to same email + assert {:error, %Ash.Error.Invalid{errors: errors}} = result + + # The error should indicate email is already taken + assert Enum.any?(errors, fn err -> + (err.__struct__ == Ash.Error.Changes.InvalidAttribute and err.field == :email) or + err.__struct__ == Mv.Accounts.User.Errors.PasswordVerificationRequired + end) + end + + @tag :test_proposal + test "existing OIDC user email remains unchanged when oidc_id matches" do + user = + create_test_user(%{ + email: "oidc@example.com", + oidc_id: "oidc_stable_789" + }) + + # Same OIDC ID, same email - should just sign in + user_info = %{ + "sub" => "oidc_stable_789", + "preferred_username" => "oidc@example.com" + } + + # This should work via upsert + {:ok, updated_user} = + Mv.Accounts.create_register_with_rauthy(%{ + user_info: user_info, + oauth_tokens: %{} + }) + + assert updated_user.id == user.id + assert updated_user.oidc_id == "oidc_stable_789" + assert to_string(updated_user.email) == "oidc@example.com" + end + end + + describe "Email update during OIDC linking" do + @tag :test_proposal + test "linking OIDC to password account updates email if different in OIDC" do + # Password user with old email + user = + create_test_user(%{ + email: "oldemail@example.com", + password: "password123", + oidc_id: nil + }) + + # OIDC provider returns new email (user changed it there) + user_info = %{ + "sub" => "oidc_link_999", + "preferred_username" => "newemail@example.com" + } + + # After password verification, link and update email + {:ok, updated_user} = + Mv.Accounts.User + |> Ash.Query.filter(id == ^user.id) + |> Ash.read_one!() + |> Ash.Changeset.for_update(:link_oidc_id, %{ + oidc_id: user_info["sub"], + oidc_user_info: user_info + }) + |> Ash.update() + + assert updated_user.oidc_id == "oidc_link_999" + assert to_string(updated_user.email) == "newemail@example.com" + end + + @tag :test_proposal + test "email change during linking triggers member email sync" do + # Create member + member = + Ash.Seed.seed!(Mv.Membership.Member, %{ + email: "member@example.com", + first_name: "Test", + last_name: "User" + }) + + # Create user linked to member + user = + Ash.Seed.seed!(Mv.Accounts.User, %{ + email: "member@example.com", + hashed_password: "dummy_hash", + oidc_id: nil, + member_id: member.id + }) + + # Link OIDC with new email + user_info = %{ + "sub" => "oidc_sync_777", + "preferred_username" => "newemail@example.com" + } + + {:ok, updated_user} = + Mv.Accounts.User + |> Ash.Query.filter(id == ^user.id) + |> Ash.read_one!() + |> Ash.Changeset.for_update(:link_oidc_id, %{ + oidc_id: user_info["sub"], + oidc_user_info: user_info + }) + |> Ash.update() + + # Verify user email changed + assert to_string(updated_user.email) == "newemail@example.com" + + # Verify member email was synced + {:ok, updated_member} = Ash.get(Mv.Membership.Member, member.id) + assert to_string(updated_member.email) == "newemail@example.com" + end + end + + describe "Edge cases" do + @tag :test_proposal + test "user with empty string oidc_id is treated as password-only user" do + _user = + create_test_user(%{ + email: "empty@example.com", + password: "password123", + oidc_id: "" + }) + + # Try OIDC registration with same email + user_info = %{ + "sub" => "oidc_new_111", + "preferred_username" => "empty@example.com" + } + + result = + Mv.Accounts.create_register_with_rauthy(%{ + user_info: user_info, + oauth_tokens: %{} + }) + + # Should trigger PasswordVerificationRequired (empty string = no OIDC) + assert {:error, %Ash.Error.Invalid{errors: errors}} = result + + password_error = + Enum.find(errors, fn err -> + err.__struct__ == Mv.Accounts.User.Errors.PasswordVerificationRequired + end) + + assert password_error != nil + end + + @tag :test_proposal + test "cannot link same oidc_id to multiple users" do + # User 1 with OIDC + _user1 = + create_test_user(%{ + email: "user1@example.com", + oidc_id: "shared_oidc_333" + }) + + # Try to create user 2 with same OIDC ID using raw Ash.Changeset + # (create_test_user uses Ash.Seed which does upsert) + result = + Mv.Accounts.User + |> Ash.Changeset.for_create(:create_user, %{ + email: "user2@example.com" + }) + |> Ash.Changeset.change_attribute(:oidc_id, "shared_oidc_333") + |> Ash.create() + + # Should fail due to unique constraint on oidc_id + assert match?({:error, %Ash.Error.Invalid{}}, result) + + {:error, error} = result + # Verify the error is about oidc_id uniqueness + assert Enum.any?(error.errors, fn err -> + match?(%Ash.Error.Changes.InvalidAttribute{field: :oidc_id}, err) + end) + end + end +end