diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex index b085407..668ddd4 100644 --- a/lib/accounts/user.ex +++ b/lib/accounts/user.ex @@ -12,6 +12,12 @@ defmodule Mv.Accounts.User do postgres do table "users" repo Mv.Repo + + references do + # When a member is deleted, set the user's member_id to NULL + # This allows users to continue existing even if their linked member is removed + reference :member, on_delete: :nilify + end end @doc """ @@ -60,15 +66,77 @@ defmodule Mv.Accounts.User do end actions do - defaults [:read, :create, :destroy, :update] + # Default actions kept for framework/tooling integration: + # - :create -> Used by AshAdmin's generated "Create" UI and by generic + # AshPhoenix helpers that assume a default create action. + # It does NOT manage the :member relationship. For admin + # flows that may link an existing member, use :create_user. + # - :read -> Standard read used across the app and by admin tooling. + # - :destroy-> Standard delete used by admin tooling and maintenance tasks. + defaults [:read, :create, :destroy] + + # Primary generic update action: + # - Selected by AshAdmin's generated "Edit" UI and generic AshPhoenix + # helpers that assume a default update action. + # - Intended for simple attribute updates (e.g., :email) and scenarios + # that do NOT need to manage the :member relationship. + # - For linking/unlinking a member (and the related validations), prefer + # the specialized :update_user action below. + update :update do + primary? true + + # Required because custom validation functions (email validation, member relationship validation) + # cannot be executed atomically. These validations need to query the database and perform + # complex checks that are not supported in atomic operations. + require_atomic? false + end create :create_user do + description "Creates a new user with optional member relationship. The member relationship is managed through the :member argument." + # 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 + description "Updates a user and manages the optional member relationship. To change an existing member link, first remove it (set member to nil), then add the new one." + # 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 functions (email validation, member relationship validation) + # cannot be executed atomically. These validations need to query the database and perform + # complex checks that are not supported in atomic operations. + 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 +144,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}) @@ -122,8 +191,54 @@ defmodule Mv.Accounts.User do # Global validations - applied to all relevant actions validations do # Password strength policy: minimum 8 characters for all password-related actions - validate string_length(:password, min: 8) do - where action_is([:register_with_password, :admin_set_password]) + validate string_length(:password, min: 8), + where: [action_is([:register_with_password, :admin_set_password])], + message: "must have length of at least 8" + + # Email validation with EctoCommons.EmailValidator (same as Member) + # This ensures consistency between User and Member email validation + validate fn changeset, _ -> + # Get email from attribute (Ash.CiString) and convert to string + email = Ash.Changeset.get_attribute(changeset, :email) + email_string = if email, do: to_string(email), else: nil + + # Only validate if email is present + if email_string do + changeset2 = + {%{}, %{email: :string}} + |> Ecto.Changeset.cast(%{email: email_string}, [:email]) + |> EctoCommons.EmailValidator.validate_email(:email, checks: [:html_input, :pow]) + + if changeset2.valid? do + :ok + else + {:error, field: :email, message: "is not a valid email"} + end + else + :ok + end + 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 @@ -140,18 +255,28 @@ defmodule Mv.Accounts.User do attributes do uuid_primary_key :id - attribute :email, :ci_string, allow_nil?: false, public?: true + attribute :email, :ci_string do + allow_nil? false + public? true + # Same constraints as Member email for consistency + constraints min_length: 5, max_length: 254 + end + attribute :hashed_password, :string, sensitive?: true, allow_nil?: true attribute :oidc_id, :string, allow_nil?: true 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 7fe69da..4cec072 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)], @@ -175,5 +242,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/membership/property.ex b/lib/membership/property.ex index 2c432a8..de096ca 100644 --- a/lib/membership/property.ex +++ b/lib/membership/property.ex @@ -42,4 +42,10 @@ defmodule Mv.Membership.Property do calculations do calculate :value_to_string, :string, expr(value[:value] <> "") end + + # Ensure a member can only have one property per property type + # For example: A member can have only one "email" property, one "phone" property, etc. + identities do + identity :unique_property_per_member, [:member_id, :property_type_id] + end end diff --git a/lib/mv_web/components/layouts/navbar.ex b/lib/mv_web/components/layouts/navbar.ex index 9009329..9fec3f4 100644 --- a/lib/mv_web/components/layouts/navbar.ex +++ b/lib/mv_web/components/layouts/navbar.ex @@ -16,20 +16,27 @@ defmodule MvWeb.Layouts.Navbar do
Mitgliederverwaltung
-
-