add oidc tests

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

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} ->
+               %Mv.Accounts.User.Errors.PasswordVerificationRequired{user_id: user_id} ->
-                 String.contains?(message, "has already been taken")
+                 user_id == existing_user.id
 
                _ ->
                  false

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