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..5641528 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,9 @@ 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 end diff --git a/priv/repo/migrations/20250926164519_member_relation.exs b/priv/repo/migrations/20250926164519_member_relation.exs new file mode 100644 index 0000000..daaa24c --- /dev/null +++ b/priv/repo/migrations/20250926164519_member_relation.exs @@ -0,0 +1,17 @@ +defmodule Mv.Repo.Migrations.MemberRelation do + @moduledoc """ + Updates resources based on their most recent snapshots. + + This file was autogenerated with `mix ash_postgres.generate_migrations` + """ + + use Ecto.Migration + + def up do + create unique_index(:users, [:member_id], name: "users_unique_member_index") + end + + def down do + drop_if_exists unique_index(:users, [:member_id], name: "users_unique_member_index") + end +end diff --git a/priv/resource_snapshots/repo/users/20250926164519.json b/priv/resource_snapshots/repo/users/20250926164519.json new file mode 100644 index 0000000..7eb68f2 --- /dev/null +++ b/priv/resource_snapshots/repo/users/20250926164519.json @@ -0,0 +1,141 @@ +{ + "attributes": [ + { + "allow_nil?": false, + "default": "fragment(\"gen_random_uuid()\")", + "generated?": false, + "precision": null, + "primary_key?": true, + "references": null, + "scale": null, + "size": null, + "source": "id", + "type": "uuid" + }, + { + "allow_nil?": false, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "email", + "type": "citext" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "hashed_password", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "oidc_id", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": { + "deferrable": false, + "destination_attribute": "id", + "destination_attribute_default": null, + "destination_attribute_generated": null, + "index?": false, + "match_type": null, + "match_with": null, + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "name": "users_member_id_fkey", + "on_delete": null, + "on_update": null, + "primary_key?": true, + "schema": "public", + "table": "members" + }, + "scale": null, + "size": null, + "source": "member_id", + "type": "uuid" + } + ], + "base_filter": null, + "check_constraints": [], + "custom_indexes": [], + "custom_statements": [], + "has_create_action": true, + "hash": "FDEBD4840449609DDA8B50D6741C2EEDE9D81DFBC1E26D4BC77DBD9B5A8EA4DC", + "identities": [ + { + "all_tenants?": false, + "base_filter": null, + "index_name": "users_unique_email_index", + "keys": [ + { + "type": "atom", + "value": "email" + } + ], + "name": "unique_email", + "nils_distinct?": true, + "where": null + }, + { + "all_tenants?": false, + "base_filter": null, + "index_name": "users_unique_oidc_id_index", + "keys": [ + { + "type": "atom", + "value": "oidc_id" + } + ], + "name": "unique_oidc_id", + "nils_distinct?": true, + "where": null + }, + { + "all_tenants?": false, + "base_filter": null, + "index_name": "users_unique_member_index", + "keys": [ + { + "type": "atom", + "value": "member_id" + } + ], + "name": "unique_member", + "nils_distinct?": true, + "where": null + } + ], + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "repo": "Elixir.Mv.Repo", + "schema": null, + "table": "users" +} \ No newline at end of file