From 6907b2ed3c8ce5d455337b236203b8611d0f72ff Mon Sep 17 00:00:00 2001
From: Moritz <moritz.m@local-it.org>
Date: Thu, 24 Jul 2025 15:30:34 +0200
Subject: [PATCH 1/2] feat: fail if oidc provide does not provide a sub or id

---
 lib/accounts/user.ex | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex
index 2da15a1..d8c7a66 100644
--- a/lib/accounts/user.ex
+++ b/lib/accounts/user.ex
@@ -105,6 +105,8 @@ 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 ->
@@ -125,6 +127,16 @@ 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
 

From 85071096319051e4c617a40acd94abf233d0eb15 Mon Sep 17 00:00:00 2001
From: Moritz <moritz.m@local-it.org>
Date: Wed, 23 Jul 2025 00:26:06 +0200
Subject: [PATCH 2/2] feat: test authentication

---
 lib/accounts/accounts.ex                      |   2 +
 .../controllers/auth_controller_test.exs      | 190 +++++++++++++++---
 .../controllers/oidc_integration_test.exs     | 166 +++++++++++++++
 3 files changed, 335 insertions(+), 23 deletions(-)
 create mode 100644 test/mv_web/controllers/oidc_integration_test.exs

diff --git a/lib/accounts/accounts.ex b/lib/accounts/accounts.ex
index 333e12e..7bc5073 100644
--- a/lib/accounts/accounts.ex
+++ b/lib/accounts/accounts.ex
@@ -15,6 +15,8 @@ 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/test/mv_web/controllers/auth_controller_test.exs b/test/mv_web/controllers/auth_controller_test.exs
index e18ade2..3c3956d 100644
--- a/test/mv_web/controllers/auth_controller_test.exs
+++ b/test/mv_web/controllers/auth_controller_test.exs
@@ -1,40 +1,121 @@
 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 "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"
@@ -45,4 +126,67 @@ 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
new file mode 100644
index 0000000..a96e7b1
--- /dev/null
+++ b/test/mv_web/controllers/oidc_integration_test.exs
@@ -0,0 +1,166 @@
+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