diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex index b085407..c65b882 100644 --- a/lib/accounts/user.ex +++ b/lib/accounts/user.ex @@ -60,15 +60,54 @@ defmodule Mv.Accounts.User do end actions do - defaults [:read, :create, :destroy, :update] + defaults [:read, :create, :destroy] + + update :update do + primary? true + require_atomic? false + end create :create_user do + # Only accept email directly - member_id is NOT in accept list + # This prevents direct foreign key manipulation, forcing use of manage_relationship accept [:email] + # Allow member to be passed as argument for relationship management + argument :member, :map, allow_nil?: true upsert? true + + # Manage the member relationship during user creation + change manage_relationship(:member, :member, + # Look up existing member and relate to it + on_lookup: :relate, + # Error if member doesn't exist in database + on_no_match: :error, + # If member already linked to this user, ignore (shouldn't happen in create) + on_match: :ignore, + # If no member provided, that's fine (optional relationship) + on_missing: :ignore + ) end update :update_user do + # Only accept email directly - member_id is NOT in accept list + # This prevents direct foreign key manipulation, forcing use of manage_relationship accept [:email] + # Allow member to be passed as argument for relationship management + argument :member, :map, allow_nil?: true + # Required because custom validation function cannot be done atomically + require_atomic? false + + # Manage the member relationship during user update + change manage_relationship(:member, :member, + # Look up existing member and relate to it + on_lookup: :relate, + # Error if member doesn't exist in database + on_no_match: :error, + # If same member provided, that's fine (allows updates with same member) + on_match: :ignore, + # If no member provided, remove existing relationship (allows member removal) + on_missing: :unrelate + ) end # Admin action for direct password changes in admin panel @@ -76,6 +115,7 @@ defmodule Mv.Accounts.User do update :admin_set_password do accept [:email] argument :password, :string, allow_nil?: false, sensitive?: true + require_atomic? false # Set the strategy context that HashPasswordChange expects change set_context(%{strategy_name: :password}) @@ -125,6 +165,28 @@ defmodule Mv.Accounts.User do validate string_length(:password, min: 8) do where action_is([:register_with_password, :admin_set_password]) end + + # Prevent overwriting existing member relationship + # This validation ensures race condition safety by requiring explicit two-step process: + # 1. Remove existing member (set member to nil) + # 2. Add new member + # This prevents accidental overwrites when multiple admins work simultaneously + validate fn changeset, _context -> + member_arg = Ash.Changeset.get_argument(changeset, :member) + current_member_id = changeset.data.member_id + + # Only trigger if: + # - member argument is provided AND has an ID + # - user currently has a member + # - the new member ID is different from current member ID + if member_arg && member_arg[:id] && current_member_id && + member_arg[:id] != current_member_id do + {:error, + field: :member, message: "User already has a member. Remove existing member first."} + else + :ok + end + end end def validate_oidc_id_present(changeset, _context) do @@ -146,12 +208,16 @@ defmodule Mv.Accounts.User do end relationships do + # 1:1 relationship - User can optionally belong to one Member + # This automatically creates a `member_id` attribute in the User table + # The relationship is optional (allow_nil? true by default) belongs_to :member, Mv.Membership.Member end identities do identity :unique_email, [:email] identity :unique_oidc_id, [:oidc_id] + identity :unique_member, [:member_id] end # You can customize this if you wish, but this is a safe default that diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 583f173..7b898a8 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -13,7 +13,11 @@ defmodule Mv.Membership.Member do create :create_member do primary? true + # Properties can be created along with member argument :properties, {:array, :map} + # Allow user to be passed as argument for relationship management + # user_id is NOT in accept list to prevent direct foreign key manipulation + argument :user, :map, allow_nil?: true accept [ :first_name, @@ -32,12 +36,29 @@ defmodule Mv.Membership.Member do ] change manage_relationship(:properties, type: :create) + + # Manage the user relationship during member creation + change manage_relationship(:user, :user, + # Look up existing user and relate to it + on_lookup: :relate, + # Error if user doesn't exist in database + on_no_match: :error, + # Error if user is already linked to another member (prevents "stealing") + on_match: :error, + # If no user provided, that's fine (optional relationship) + on_missing: :ignore + ) end update :update_member do primary? true + # Required because custom validation function cannot be done atomically require_atomic? false + # Properties can be updated or created along with member argument :properties, {:array, :map} + # Allow user to be passed as argument for relationship management + # user_id is NOT in accept list to prevent direct foreign key manipulation + argument :user, :map, allow_nil?: true accept [ :first_name, @@ -56,6 +77,18 @@ defmodule Mv.Membership.Member do ] change manage_relationship(:properties, on_match: :update, on_no_match: :create) + + # Manage the user relationship during member update + change manage_relationship(:user, :user, + # Look up existing user and relate to it + on_lookup: :relate, + # Error if user doesn't exist in database + on_no_match: :error, + # Error if user is already linked to another member (prevents "stealing") + on_match: :error, + # If no user provided, remove existing relationship (allows user removal) + on_missing: :unrelate + ) end end @@ -67,6 +100,40 @@ defmodule Mv.Membership.Member do validate present(:last_name) validate present(:email) + # Prevent linking to a user that already has a member + # This validation prevents "stealing" users from other members by checking + # if the target user is already linked to a different member + # This is necessary because manage_relationship's on_match: :error only checks + # if the user is already linked to THIS specific member, not ANY member + validate fn changeset, _context -> + user_arg = Ash.Changeset.get_argument(changeset, :user) + + if user_arg && user_arg[:id] do + user_id = user_arg[:id] + current_member_id = changeset.data.id + + # Check the current state of the user in the database + case Ash.get(Mv.Accounts.User, user_id) do + # User is free to be linked + {:ok, %{member_id: nil}} -> + :ok + + # User already linked to this member (update scenario) + {:ok, %{member_id: ^current_member_id}} -> + :ok + + {:ok, %{member_id: _other_member_id}} -> + # User is linked to a different member - prevent "stealing" + {:error, field: :user, message: "User is already linked to another member"} + + {:error, _} -> + {:error, field: :user, message: "User not found"} + end + else + :ok + end + end + # Birth date not in the future validate compare(:birth_date, less_than_or_equal_to: &Date.utc_today/0), where: [present(:birth_date)], @@ -170,5 +237,14 @@ defmodule Mv.Membership.Member do relationships do has_many :properties, Mv.Membership.Property + # 1:1 relationship - Member can optionally have one User + # This references the User's member_id attribute + # The relationship is optional (allow_nil? true by default) + has_one :user, Mv.Accounts.User + end + + # Define identities for upsert operations + identities do + identity :unique_email, [:email] end end diff --git a/lib/mv_web/components/layouts/navbar.ex b/lib/mv_web/components/layouts/navbar.ex index b917ddc..5efaa1b 100644 --- a/lib/mv_web/components/layouts/navbar.ex +++ b/lib/mv_web/components/layouts/navbar.ex @@ -18,13 +18,20 @@ defmodule MvWeb.Layouts.Navbar do
-
-