diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md index 0a87836..2b48de2 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -81,8 +81,8 @@ lib/ ├── membership/ # Membership domain │ ├── membership.ex # Domain definition │ ├── member.ex # Member resource +│ ├── custom_field.ex # Custom field (definition) resource │ ├── custom_field_value.ex # Custom field value resource -│ ├── custom_field.ex # CustomFieldValue type resource │ ├── setting.ex # Global settings (singleton resource) │ └── email.ex # Email custom type ├── membership_fees/ # MembershipFees domain @@ -194,7 +194,8 @@ test/ ├── seeds_test.exs # Database seed tests └── support/ # Test helpers ├── conn_case.ex # Controller test helpers - └── data_case.ex # Data layer test helpers + ├── data_case.ex # Data layer test helpers + └── fixtures.ex # Shared test fixtures (Mv.Fixtures) ``` ### 1.2 Module Organization @@ -1247,7 +1248,8 @@ test/ │ └── components/ └── support/ # Test helpers ├── conn_case.ex # Controller test setup - └── data_case.ex # Database test setup + ├── data_case.ex # Database test setup + └── fixtures.ex # Shared test fixtures (Mv.Fixtures) ``` **Test File Naming:** diff --git a/docs/groups-architecture.md b/docs/groups-architecture.md index b2316d8..735898c 100644 --- a/docs/groups-architecture.md +++ b/docs/groups-architecture.md @@ -4,7 +4,7 @@ **Feature:** Groups Management **Version:** 1.0 **Last Updated:** 2025-01-XX -**Status:** Architecture Design - Ready for Implementation +**Status:** ✅ Implemented (authorization: see [roles-and-permissions-architecture.md](./roles-and-permissions-architecture.md)) --- @@ -412,15 +412,17 @@ lib/ ## Authorization +**Status:** ✅ Implemented. Group and MemberGroup resource policies and PermissionSets are in place. See [roles-and-permissions-architecture.md](./roles-and-permissions-architecture.md) for the full permission matrix and policy patterns. + ### Permission Model (MVP) -**Resource:** `groups` +**Resource:** `Group` (and `MemberGroup`) **Actions:** -- `read` - View groups (all users with member read permission) -- `create` - Create groups (admin only) -- `update` - Edit groups (admin only) -- `destroy` - Delete groups (admin only) +- `read` - View groups (all permission sets) +- `create` - Create groups (normal_user and admin) +- `update` - Edit groups (normal_user and admin) +- `destroy` - Delete groups (normal_user and admin) **Scopes:** - `:all` - All groups (for all permission sets that have read access) @@ -442,7 +444,7 @@ lib/ **Own Data Permission Set:** - `read` action on `Group` resource with `:all` scope - granted -**Note:** All permission sets use `:all` scope for groups. Groups are considered public information that all users with member read permission can view. Only admins can manage (create/update/destroy) groups. +**Note:** All permission sets use `:all` scope for groups. Groups are considered public information that all users with member read permission can view. normal_user and admin can manage (create/update/destroy) groups. ### Member-Group Association Permissions diff --git a/docs/membership-fee-architecture.md b/docs/membership-fee-architecture.md index 4a290b7..6c81169 100644 --- a/docs/membership-fee-architecture.md +++ b/docs/membership-fee-architecture.md @@ -334,20 +334,18 @@ lib/ ### Permission System Integration -**See:** [roles-and-permissions-architecture.md](./roles-and-permissions-architecture.md) +**Status:** ✅ Implemented. See [roles-and-permissions-architecture.md](./roles-and-permissions-architecture.md) for the full permission matrix and policy patterns. -**Required Permissions:** +**PermissionSets (lib/mv/authorization/permission_sets.ex):** -- `MembershipFeeType.create/update/destroy` - Admin only -- `MembershipFeeType.read` - Admin, Treasurer, Board -- `MembershipFeeCycle.update` (status changes) - Admin, Treasurer -- `MembershipFeeCycle.read` - Admin, Treasurer, Board, Own member +- **MembershipFeeType:** All permission sets can read (:all); only admin has create/update/destroy (:all). +- **MembershipFeeCycle:** All can read (:all); read_only has read only; normal_user and admin have read + create + update + destroy (:all). +- **Manual "Regenerate Cycles" (UI + server):** The "Regenerate Cycles" button in the member detail view is shown to users who have MembershipFeeCycle create permission (normal_user and admin). UI access is gated by `can_create_cycle`. The LiveView handler also enforces `can?(:create, MembershipFeeCycle)` server-side before running regeneration (so e.g. a read_only user cannot trigger it via DevTools). Regeneration runs with system actor. -**Policy Patterns:** +**Resource Policies:** -- Use existing HasPermission check -- Leverage existing roles (Admin, Kassenwart) -- Member can read own cycles (linked via member_id) +- **MembershipFeeType** (`lib/membership_fees/membership_fee_type.ex`): `authorizers: [Ash.Policy.Authorizer]`, single policy with `HasPermission` for read/create/update/destroy. +- **MembershipFeeCycle** (`lib/membership_fees/membership_fee_cycle.ex`): Same pattern; update includes mark_as_paid, mark_as_suspended, mark_as_unpaid. ### LiveView Integration @@ -357,7 +355,7 @@ lib/ 2. MembershipFeeCycle table component (member detail view) - Implemented as `MvWeb.MemberLive.Show.MembershipFeesComponent` - Displays all cycles in a table with status management - - Allows changing cycle status, editing amounts, and regenerating cycles + - Allows changing cycle status, editing amounts, and manually regenerating cycles (normal_user and admin) 3. Settings form section (admin) 4. Member list column (membership fee status) diff --git a/docs/roles-and-permissions-architecture.md b/docs/roles-and-permissions-architecture.md index dbf2353..92ad3c5 100644 --- a/docs/roles-and-permissions-architecture.md +++ b/docs/roles-and-permissions-architecture.md @@ -97,6 +97,10 @@ Control CRUD operations on: - CustomFieldValue (custom field values) - CustomField (custom field definitions) - Role (role management) +- Group (group definitions; read all, create/update/destroy normal_user and admin) +- MemberGroup (member–group associations; own_data read :linked, read_only read :all, normal_user/admin create/destroy) +- MembershipFeeType (fee type definitions; all read, admin-only create/update/destroy) +- MembershipFeeCycle (fee cycles; own_data read :linked, read_only read :all, normal_user/admin read+create+update+destroy; manual "Regenerate Cycles" for normal_user and admin) **4. Page-Level Permissions** @@ -105,6 +109,7 @@ Control access to LiveView pages: - Show pages (detail views) - Form pages (create/edit) - Admin pages +- Settings pages: `/settings` and `/membership_fee_settings` are admin-only (explicit in PermissionSets) **5. Granular Scopes** @@ -121,6 +126,8 @@ Three scope levels for permissions: - **Linked Member Email:** Only admins can edit email of member linked to user - **System Roles:** "Mitglied" role cannot be deleted (is_system_role flag) - **User-Member Linking:** Only admins can link/unlink users and members +- **User Role Assignment:** Only admins can change a user's role (via `update_user` with `role_id`). Last-admin validation ensures at least one user keeps the Admin role. +- **Settings Pages:** `/settings` and `/membership_fee_settings` are admin-only (explicit in PermissionSets pages). **7. UI Consistency** @@ -684,6 +691,12 @@ Quick reference table showing what each permission set allows: | **CustomFieldValue** (all) | - | R | R, C, U, D | R, C, U, D | | **CustomField** (all) | R | R | R | R, C, U, D | | **Role** (all) | - | - | - | R, C, U, D | +| **Group** (all) | R | R | R, C, U, D | R, C, U, D | +| **MemberGroup** (linked) | R | - | - | - | +| **MemberGroup** (all) | - | R | R, C, D | R, C, D | +| **MembershipFeeType** (all) | R | R | R | R, C, U, D | +| **MembershipFeeCycle** (linked) | R | - | - | - | +| **MembershipFeeCycle** (all) | - | R | R, C, U, D | R, C, U, D | **Legend:** R=Read, C=Create, U=Update, D=Destroy @@ -1195,6 +1208,36 @@ end *Cannot destroy if `is_system_role=true` +### User Role Assignment (Admin-Only) + +**Location:** `lib/accounts/user.ex` (update_user action), `lib/mv_web/live/user_live/form.ex` + +Only admins can change a user's role. The `update_user` action accepts `role_id`; the User form shows a role dropdown when `can?(actor, :update, Mv.Authorization.Role)`. **Last-admin validation:** If the only non-system admin tries to change their role, the change is rejected with "At least one user must keep the Admin role." (System user is excluded from the admin count.) See [User-Member Linking](#user-member-linking) for the same admin-only pattern. + +### Group Resource Policies + +**Location:** `lib/membership/group.ex` + +Policies use `HasPermission` for read/create/update/destroy. All permission sets can read; normal_user and admin can create, update, destroy. No bypass (scope :all only in PermissionSets). + +### MemberGroup Resource Policies + +**Location:** `lib/membership/member_group.ex` + +Bypass for read restricted to own_data (MemberGroupReadLinkedForOwnData check: own_data only, filter `member_id == actor.member_id`); HasPermission for read (read_only/normal_user/admin :all) and create/destroy (normal_user + admin only). Admin with member_id set still gets :all from HasPermission (bypass does not apply). + +### MembershipFeeType Resource Policies + +**Location:** `lib/membership_fees/membership_fee_type.ex` + +Policies use `HasPermission` for read/create/update/destroy. All permission sets can read; only admin can create, update, destroy. + +### MembershipFeeCycle Resource Policies + +**Location:** `lib/membership_fees/membership_fee_cycle.ex` + +Bypass for read restricted to own_data (MembershipFeeCycleReadLinkedForOwnData: own_data only, filter `member_id == actor.member_id`); HasPermission for read (read_only/normal_user/admin :all) and create/update/destroy. own_data can only read cycles of the linked member; read_only can read all; normal_user and admin can read, create, update, and destroy (including mark_as_paid and manual "Regenerate Cycles"; UI button when `can_create_cycle`). Regenerate-cycles handler enforces `can?(:create, MembershipFeeCycle)` server-side. + --- ## Page Permission System diff --git a/docs/roles-and-permissions-implementation-plan.md b/docs/roles-and-permissions-implementation-plan.md index 23b045c..95db031 100644 --- a/docs/roles-and-permissions-implementation-plan.md +++ b/docs/roles-and-permissions-implementation-plan.md @@ -78,10 +78,11 @@ Stored in database `roles` table, each referencing a `permission_set_name`: - ✅ Hardcoded PermissionSets module with 4 permission sets - ✅ Role database table and CRUD interface - ✅ Custom Ash Policy Check (`HasPermission`) that reads from PermissionSets -- ✅ Policies on all resources (Member, User, CustomFieldValue, CustomField, Role) -- ✅ Page-level permissions via Phoenix Plug +- ✅ Policies on all resources (Member, User, CustomFieldValue, CustomField, Role, Group, MemberGroup, MembershipFeeType, MembershipFeeCycle) +- ✅ Page-level permissions via Phoenix Plug (including admin-only `/settings` and `/membership_fee_settings`) - ✅ UI authorization helpers for conditional rendering - ✅ Special case: Member email validation for linked users +- ✅ User role assignment: admin-only `role_id` in update_user; Last-Admin validation; role dropdown in User form when `can?(actor, :update, Role)` - ✅ Seed data for 5 roles **Benefits of Hardcoded Approach:** diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex index f792973..034177a 100644 --- a/lib/accounts/user.ex +++ b/lib/accounts/user.ex @@ -8,6 +8,9 @@ defmodule Mv.Accounts.User do extensions: [AshAuthentication], authorizers: [Ash.Policy.Authorizer] + require Ash.Query + import Ash.Expr + postgres do table "users" repo Mv.Repo @@ -146,9 +149,10 @@ defmodule Mv.Accounts.User do 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] + + # Accept email and role_id (role_id only used by admins; policy restricts update_user to admins). + # member_id is NOT in accept list - use argument :member for relationship management. + accept [:email, :role_id] # Allow member to be passed as argument for relationship management argument :member, :map, allow_nil?: true @@ -387,6 +391,63 @@ defmodule Mv.Accounts.User do end end + # Last-admin: prevent the only admin from leaving the admin role (at least one admin required). + # Only block when the user is leaving admin (target role is not admin). Switching between + # two admin roles (e.g. "Admin" and "Superadmin" both with permission_set_name "admin") is allowed. + validate fn changeset, _context -> + if Ash.Changeset.changing_attribute?(changeset, :role_id) do + new_role_id = Ash.Changeset.get_attribute(changeset, :role_id) + + if is_nil(new_role_id) do + :ok + else + current_role_id = changeset.data.role_id + + current_role = + Mv.Authorization.Role + |> Ash.get!(current_role_id, authorize?: false) + + new_role = + Mv.Authorization.Role + |> Ash.get!(new_role_id, authorize?: false) + + # Only block when current user is admin and target role is not admin (leaving admin) + if current_role.permission_set_name == "admin" and + new_role.permission_set_name != "admin" do + admin_role_ids = + Mv.Authorization.Role + |> Ash.Query.for_read(:read) + |> Ash.Query.filter(expr(permission_set_name == "admin")) + |> Ash.read!(authorize?: false) + |> Enum.map(& &1.id) + + # Count only non-system users with admin role (system user is for internal ops) + system_email = Mv.Helpers.SystemActor.system_user_email() + + count = + Mv.Accounts.User + |> Ash.Query.for_read(:read) + |> Ash.Query.filter(expr(role_id in ^admin_role_ids)) + |> Ash.Query.filter(expr(email != ^system_email)) + |> Ash.count!(authorize?: false) + + if count <= 1 do + {:error, + field: :role_id, message: "At least one user must keep the Admin role."} + else + :ok + end + else + :ok + end + end + else + :ok + end + end, + on: [:update], + where: [action_is(:update_user)] + # Prevent modification of the system actor user (required for internal operations). # Block update/destroy on UI-exposed actions only; :update_internal is used by bootstrap/tests. validate fn changeset, _context -> diff --git a/lib/membership/group.ex b/lib/membership/group.ex index 14aadc8..d468166 100644 --- a/lib/membership/group.ex +++ b/lib/membership/group.ex @@ -36,7 +36,8 @@ defmodule Mv.Membership.Group do """ use Ash.Resource, domain: Mv.Membership, - data_layer: AshPostgres.DataLayer + data_layer: AshPostgres.DataLayer, + authorizers: [Ash.Policy.Authorizer] require Ash.Query alias Mv.Helpers @@ -63,6 +64,13 @@ defmodule Mv.Membership.Group do end end + policies do + policy action_type([:read, :create, :update, :destroy]) do + description "Check permissions from role (all can read; normal_user and admin can create/update/destroy)" + authorize_if Mv.Authorization.Checks.HasPermission + end + end + validations do validate present(:name) @@ -136,7 +144,7 @@ defmodule Mv.Membership.Group do query = Mv.Membership.Group |> Ash.Query.filter(fragment("LOWER(?) = LOWER(?)", name, ^name)) - |> maybe_exclude_id(exclude_id) + |> Helpers.query_exclude_id(exclude_id) opts = Helpers.ash_actor_opts(actor) @@ -155,7 +163,4 @@ defmodule Mv.Membership.Group do :ok end end - - defp maybe_exclude_id(query, nil), do: query - defp maybe_exclude_id(query, id), do: Ash.Query.filter(query, id != ^id) end diff --git a/lib/membership/member_group.ex b/lib/membership/member_group.ex index 5d29bda..22a1f70 100644 --- a/lib/membership/member_group.ex +++ b/lib/membership/member_group.ex @@ -39,7 +39,8 @@ defmodule Mv.Membership.MemberGroup do """ use Ash.Resource, domain: Mv.Membership, - data_layer: AshPostgres.DataLayer + data_layer: AshPostgres.DataLayer, + authorizers: [Ash.Policy.Authorizer] require Ash.Query @@ -56,6 +57,26 @@ defmodule Mv.Membership.MemberGroup do end end + # Authorization: read uses bypass for :linked (own_data only) then HasPermission for :all; + # create/destroy use HasPermission (normal_user + admin only). + # Single check: own_data gets filter via auto_filter; admin does not match, gets :all from HasPermission. + policies do + bypass action_type(:read) do + description "own_data: read only member_groups where member_id == actor.member_id" + authorize_if Mv.Authorization.Checks.MemberGroupReadLinkedForOwnData + end + + policy action_type(:read) do + description "Check read permission from role (read_only/normal_user/admin :all)" + authorize_if Mv.Authorization.Checks.HasPermission + end + + policy action_type([:create, :destroy]) do + description "Check create/destroy from role (normal_user + admin only)" + authorize_if Mv.Authorization.Checks.HasPermission + end + end + validations do validate present(:member_id) validate present(:group_id) @@ -118,7 +139,7 @@ defmodule Mv.Membership.MemberGroup do query = Mv.Membership.MemberGroup |> Ash.Query.filter(member_id == ^member_id and group_id == ^group_id) - |> maybe_exclude_id(exclude_id) + |> Helpers.query_exclude_id(exclude_id) opts = Helpers.ash_actor_opts(actor) @@ -135,7 +156,4 @@ defmodule Mv.Membership.MemberGroup do :ok end end - - defp maybe_exclude_id(query, nil), do: query - defp maybe_exclude_id(query, id), do: Ash.Query.filter(query, id != ^id) end diff --git a/lib/membership/setting.ex b/lib/membership/setting.ex index 4ba0794..bb7d122 100644 --- a/lib/membership/setting.ex +++ b/lib/membership/setting.ex @@ -155,12 +155,15 @@ defmodule Mv.Membership.Setting do on: [:create, :update] # Validate default_membership_fee_type_id exists if set - validate fn changeset, _context -> + validate fn changeset, context -> fee_type_id = Ash.Changeset.get_attribute(changeset, :default_membership_fee_type_id) if fee_type_id do - case Ash.get(Mv.MembershipFees.MembershipFeeType, fee_type_id) do + # Check existence only; action is already restricted by policy (e.g. admin). + opts = [domain: Mv.MembershipFees, authorize?: false] + + case Ash.get(Mv.MembershipFees.MembershipFeeType, fee_type_id, opts) do {:ok, _} -> :ok diff --git a/lib/membership_fees/changes/set_membership_fee_start_date.ex b/lib/membership_fees/changes/set_membership_fee_start_date.ex index a2e1ad0..0e9cf00 100644 --- a/lib/membership_fees/changes/set_membership_fee_start_date.ex +++ b/lib/membership_fees/changes/set_membership_fee_start_date.ex @@ -31,12 +31,12 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDate do alias Mv.MembershipFees.CalendarCycles @impl true - def change(changeset, _opts, _context) do + def change(changeset, _opts, context) do # Only calculate if membership_fee_start_date is not already set if has_start_date?(changeset) do changeset else - calculate_and_set_start_date(changeset) + calculate_and_set_start_date(changeset, context) end end @@ -56,10 +56,13 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDate do end end - defp calculate_and_set_start_date(changeset) do + defp calculate_and_set_start_date(changeset, context) do + actor = Map.get(context || %{}, :actor) + opts = if actor, do: [actor: actor], else: [] + with {:ok, join_date} <- get_join_date(changeset), {:ok, membership_fee_type_id} <- get_membership_fee_type_id(changeset), - {:ok, interval} <- get_interval(membership_fee_type_id), + {:ok, interval} <- get_interval(membership_fee_type_id, opts), {:ok, include_joining_cycle} <- get_include_joining_cycle() do start_date = calculate_start_date(join_date, interval, include_joining_cycle) Ash.Changeset.force_change_attribute(changeset, :membership_fee_start_date, start_date) @@ -118,8 +121,8 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDate do end end - defp get_interval(membership_fee_type_id) do - case Ash.get(Mv.MembershipFees.MembershipFeeType, membership_fee_type_id) do + defp get_interval(membership_fee_type_id, opts) do + case Ash.get(Mv.MembershipFees.MembershipFeeType, membership_fee_type_id, opts) do {:ok, %{interval: interval}} -> {:ok, interval} {:error, _} -> {:error, :membership_fee_type_not_found} end diff --git a/lib/membership_fees/changes/validate_same_interval.ex b/lib/membership_fees/changes/validate_same_interval.ex index 8c1efb4..0ad32a1 100644 --- a/lib/membership_fees/changes/validate_same_interval.ex +++ b/lib/membership_fees/changes/validate_same_interval.ex @@ -19,9 +19,9 @@ defmodule Mv.MembershipFees.Changes.ValidateSameInterval do use Ash.Resource.Change @impl true - def change(changeset, _opts, _context) do + def change(changeset, _opts, context) do if changing_membership_fee_type?(changeset) do - validate_interval_match(changeset) + validate_interval_match(changeset, context) else changeset end @@ -33,9 +33,10 @@ defmodule Mv.MembershipFees.Changes.ValidateSameInterval do end # Validate that the new type has the same interval as the current type - defp validate_interval_match(changeset) do + defp validate_interval_match(changeset, context) do current_type_id = get_current_type_id(changeset) new_type_id = get_new_type_id(changeset) + actor = Map.get(context || %{}, :actor) cond do # If no current type, allow any change (first assignment) @@ -48,13 +49,13 @@ defmodule Mv.MembershipFees.Changes.ValidateSameInterval do # Both types exist - validate intervals match true -> - validate_intervals_match(changeset, current_type_id, new_type_id) + validate_intervals_match(changeset, current_type_id, new_type_id, actor) end end # Validates that intervals match when both types exist - defp validate_intervals_match(changeset, current_type_id, new_type_id) do - case get_intervals(current_type_id, new_type_id) do + defp validate_intervals_match(changeset, current_type_id, new_type_id, actor) do + case get_intervals(current_type_id, new_type_id, actor) do {:ok, current_interval, new_interval} -> if current_interval == new_interval do changeset @@ -85,11 +86,16 @@ defmodule Mv.MembershipFees.Changes.ValidateSameInterval do end end - # Get intervals for both types - defp get_intervals(current_type_id, new_type_id) do + # Get intervals for both types (actor required for authorization when resource has policies) + defp get_intervals(current_type_id, new_type_id, actor) do alias Mv.MembershipFees.MembershipFeeType - case {Ash.get(MembershipFeeType, current_type_id), Ash.get(MembershipFeeType, new_type_id)} do + opts = if actor, do: [actor: actor], else: [] + + case { + Ash.get(MembershipFeeType, current_type_id, opts), + Ash.get(MembershipFeeType, new_type_id, opts) + } do {{:ok, current_type}, {:ok, new_type}} -> {:ok, current_type.interval, new_type.interval} diff --git a/lib/membership_fees/membership_fee_cycle.ex b/lib/membership_fees/membership_fee_cycle.ex index 4d9c8b7..f0dd1a7 100644 --- a/lib/membership_fees/membership_fee_cycle.ex +++ b/lib/membership_fees/membership_fee_cycle.ex @@ -28,7 +28,8 @@ defmodule Mv.MembershipFees.MembershipFeeCycle do """ use Ash.Resource, domain: Mv.MembershipFees, - data_layer: AshPostgres.DataLayer + data_layer: AshPostgres.DataLayer, + authorizers: [Ash.Policy.Authorizer] postgres do table "membership_fee_cycles" @@ -83,6 +84,19 @@ defmodule Mv.MembershipFees.MembershipFeeCycle do end end + # READ: bypass for own_data (:linked) then HasPermission for :all; create/update/destroy: HasPermission only. + policies do + bypass action_type(:read) do + description "own_data: read only cycles where member_id == actor.member_id" + authorize_if Mv.Authorization.Checks.MembershipFeeCycleReadLinkedForOwnData + end + + policy action_type([:read, :create, :update, :destroy]) do + description "Check permissions from role (all read; normal_user and admin create/update/destroy)" + authorize_if Mv.Authorization.Checks.HasPermission + end + end + attributes do uuid_v7_primary_key :id diff --git a/lib/membership_fees/membership_fee_type.ex b/lib/membership_fees/membership_fee_type.ex index 498ff75..8ec9467 100644 --- a/lib/membership_fees/membership_fee_type.ex +++ b/lib/membership_fees/membership_fee_type.ex @@ -24,7 +24,8 @@ defmodule Mv.MembershipFees.MembershipFeeType do """ use Ash.Resource, domain: Mv.MembershipFees, - data_layer: AshPostgres.DataLayer + data_layer: AshPostgres.DataLayer, + authorizers: [Ash.Policy.Authorizer] postgres do table "membership_fee_types" @@ -61,6 +62,13 @@ defmodule Mv.MembershipFees.MembershipFeeType do end end + policies do + policy action_type([:read, :create, :update, :destroy]) do + description "Check permissions from role (all can read, only admin can create/update/destroy)" + authorize_if Mv.Authorization.Checks.HasPermission + end + end + validations do # Prevent interval changes after creation validate fn changeset, _context -> diff --git a/lib/mv/accounts/user/validations/email_not_used_by_other_member.ex b/lib/mv/accounts/user/validations/email_not_used_by_other_member.ex index 0e693e1..72cc10c 100644 --- a/lib/mv/accounts/user/validations/email_not_used_by_other_member.ex +++ b/lib/mv/accounts/user/validations/email_not_used_by_other_member.ex @@ -81,7 +81,7 @@ defmodule Mv.Accounts.User.Validations.EmailNotUsedByOtherMember do query = Mv.Membership.Member |> Ash.Query.filter(email == ^to_string(email)) - |> maybe_exclude_id(exclude_member_id) + |> Mv.Helpers.query_exclude_id(exclude_member_id) system_actor = SystemActor.get_system_actor() opts = Helpers.ash_actor_opts(system_actor) @@ -101,7 +101,4 @@ defmodule Mv.Accounts.User.Validations.EmailNotUsedByOtherMember do :ok end end - - defp maybe_exclude_id(query, nil), do: query - defp maybe_exclude_id(query, id), do: Ash.Query.filter(query, id != ^id) end diff --git a/lib/mv/authorization/checks/actor_permission_set_is.ex b/lib/mv/authorization/checks/actor_permission_set_is.ex new file mode 100644 index 0000000..deb9382 --- /dev/null +++ b/lib/mv/authorization/checks/actor_permission_set_is.ex @@ -0,0 +1,44 @@ +defmodule Mv.Authorization.Checks.ActorPermissionSetIs do + @moduledoc """ + Policy check: true when the actor's role has the given permission_set_name. + + Used to restrict bypass policies (e.g. MemberGroup read by member_id) to actors + with a specific permission set (e.g. "own_data") so that admin with member_id + still gets :all scope from HasPermission, not the bypass filter. + + ## Usage + + # In a resource policy (both conditions must hold for the bypass) + bypass action_type(:read) do + authorize_if expr(member_id == ^actor(:member_id)) + authorize_if {Mv.Authorization.Checks.ActorPermissionSetIs, permission_set_name: "own_data"} + end + + ## Options + + - `:permission_set_name` (required) - String or atom, e.g. `"own_data"` or `:own_data` + """ + use Ash.Policy.SimpleCheck + + alias Mv.Authorization.Actor + + @impl true + def describe(opts) do + name = opts[:permission_set_name] || "?" + "actor has permission set #{name}" + end + + @impl true + def match?(actor, _context, opts) do + case opts[:permission_set_name] do + nil -> + false + + expected -> + case Actor.permission_set_name(actor) do + nil -> false + actual -> to_string(expected) == to_string(actual) + end + end + end +end diff --git a/lib/mv/authorization/checks/has_permission.ex b/lib/mv/authorization/checks/has_permission.ex index 774e767..1139c3c 100644 --- a/lib/mv/authorization/checks/has_permission.ex +++ b/lib/mv/authorization/checks/has_permission.ex @@ -50,6 +50,7 @@ defmodule Mv.Authorization.Checks.HasPermission do - **:linked** - Filters based on resource type: - Member: `id == actor.member_id` (User.member_id → Member.id, inverse relationship) - CustomFieldValue: `member_id == actor.member_id` (CustomFieldValue.member_id → Member.id → User.member_id) + - MemberGroup: `member_id == actor.member_id` (MemberGroup.member_id → Member.id → User.member_id) ## Error Handling @@ -278,36 +279,28 @@ defmodule Mv.Authorization.Checks.HasPermission do # For :own scope with User resource: id == actor.id # For :linked scope with Member resource: id == actor.member_id defp evaluate_filter_for_strict_check(_filter_expr, actor, record, resource_name) do - case {resource_name, record} do - {"User", %{id: user_id}} when not is_nil(user_id) -> - # Check if this user's ID matches the actor's ID (scope :own) - if user_id == actor.id do - {:ok, true} - else - {:ok, false} - end + result = + case {resource_name, record} do + # Scope :own + {"User", %{id: user_id}} when not is_nil(user_id) -> + user_id == actor.id - {"Member", %{id: member_id}} when not is_nil(member_id) -> - # Check if this member's ID matches the actor's member_id - if member_id == actor.member_id do - {:ok, true} - else - {:ok, false} - end + # Scope :linked + {"Member", %{id: member_id}} when not is_nil(member_id) -> + member_id == actor.member_id - {"CustomFieldValue", %{member_id: cfv_member_id}} when not is_nil(cfv_member_id) -> - # Check if this CFV's member_id matches the actor's member_id - if cfv_member_id == actor.member_id do - {:ok, true} - else - {:ok, false} - end + {"CustomFieldValue", %{member_id: cfv_member_id}} when not is_nil(cfv_member_id) -> + cfv_member_id == actor.member_id - _ -> - # For other cases or when record is not available, return :unknown - # This will cause Ash to use auto_filter instead - {:ok, :unknown} - end + {"MemberGroup", %{member_id: mg_member_id}} when not is_nil(mg_member_id) -> + mg_member_id == actor.member_id + + _ -> + :unknown + end + + out = if result == :unknown, do: {:ok, :unknown}, else: {:ok, result} + out end # Extract resource name from module (e.g., Mv.Membership.Member -> "Member") @@ -347,24 +340,20 @@ defmodule Mv.Authorization.Checks.HasPermission do defp apply_scope(:linked, actor, resource_name) do case resource_name do "Member" -> - # User.member_id → Member.id (inverse relationship) - # Filter: member.id == actor.member_id - # If actor has no member_id, return no results (use false or impossible condition) - if is_nil(actor.member_id) do - {:filter, expr(false)} - else - {:filter, expr(id == ^actor.member_id)} - end + # User.member_id → Member.id (inverse relationship). Filter: member.id == actor.member_id + linked_filter_by_member_id(actor, :id) "CustomFieldValue" -> # CustomFieldValue.member_id → Member.id → User.member_id - # Filter: custom_field_value.member_id == actor.member_id - # If actor has no member_id, return no results - if is_nil(actor.member_id) do - {:filter, expr(false)} - else - {:filter, expr(member_id == ^actor.member_id)} - end + linked_filter_by_member_id(actor, :member_id) + + "MemberGroup" -> + # MemberGroup.member_id → Member.id → User.member_id (own linked member's group associations) + linked_filter_by_member_id(actor, :member_id) + + "MembershipFeeCycle" -> + # MembershipFeeCycle.member_id → Member.id → User.member_id (own linked member's cycles) + linked_filter_by_member_id(actor, :member_id) _ -> # Fallback for other resources @@ -372,6 +361,17 @@ defmodule Mv.Authorization.Checks.HasPermission do end end + # Returns {:filter, expr(false)} if actor has no member_id; otherwise {:filter, expr(field == ^actor.member_id)}. + # Used for :linked scope on Member (field :id), CustomFieldValue and MemberGroup (field :member_id). + defp linked_filter_by_member_id(actor, _field) when is_nil(actor.member_id) do + {:filter, expr(false)} + end + + defp linked_filter_by_member_id(actor, :id), do: {:filter, expr(id == ^actor.member_id)} + + defp linked_filter_by_member_id(actor, :member_id), + do: {:filter, expr(member_id == ^actor.member_id)} + # Log authorization failures for debugging (lazy evaluation) defp log_auth_failure(actor, resource, action, reason) do Logger.debug(fn -> diff --git a/lib/mv/authorization/checks/member_group_read_linked_for_own_data.ex b/lib/mv/authorization/checks/member_group_read_linked_for_own_data.ex new file mode 100644 index 0000000..a553fde --- /dev/null +++ b/lib/mv/authorization/checks/member_group_read_linked_for_own_data.ex @@ -0,0 +1,63 @@ +defmodule Mv.Authorization.Checks.MemberGroupReadLinkedForOwnData do + @moduledoc """ + Policy check for MemberGroup read: true only when actor has permission set "own_data" + AND record.member_id == actor.member_id. + + Used in a bypass so that own_data gets the linked filter (via auto_filter for list queries), + while admin with member_id does not match and gets :all from HasPermission. + + - With a record (e.g. get by id): returns true only when own_data and member_id match. + - Without a record (list query): strict_check returns false; auto_filter adds filter when own_data. + """ + use Ash.Policy.Check + + alias Mv.Authorization.Checks.ActorPermissionSetIs + + @impl true + def type, do: :filter + + @impl true + def describe(_opts), + do: "own_data can read only member_groups where member_id == actor.member_id" + + @impl true + def strict_check(actor, authorizer, _opts) do + record = get_record_from_authorizer(authorizer) + is_own_data = ActorPermissionSetIs.match?(actor, authorizer, permission_set_name: "own_data") + + cond do + # List query + own_data: return :unknown so authorizer applies auto_filter (keyword list) + is_nil(record) and is_own_data -> + {:ok, :unknown} + + is_nil(record) -> + {:ok, false} + + not is_own_data -> + {:ok, false} + + record.member_id == actor.member_id -> + {:ok, true} + + true -> + {:ok, false} + end + end + + @impl true + def auto_filter(actor, _authorizer, _opts) do + if ActorPermissionSetIs.match?(actor, nil, permission_set_name: "own_data") && + Map.get(actor, :member_id) do + [member_id: actor.member_id] + else + [] + end + end + + defp get_record_from_authorizer(authorizer) do + case authorizer.subject do + %{data: data} when not is_nil(data) -> data + _ -> nil + end + end +end diff --git a/lib/mv/authorization/checks/membership_fee_cycle_read_linked_for_own_data.ex b/lib/mv/authorization/checks/membership_fee_cycle_read_linked_for_own_data.ex new file mode 100644 index 0000000..092558c --- /dev/null +++ b/lib/mv/authorization/checks/membership_fee_cycle_read_linked_for_own_data.ex @@ -0,0 +1,62 @@ +defmodule Mv.Authorization.Checks.MembershipFeeCycleReadLinkedForOwnData do + @moduledoc """ + Policy check for MembershipFeeCycle read: true only when actor has permission set "own_data" + AND record.member_id == actor.member_id. + + Used in a bypass so that own_data gets the linked filter (via auto_filter for list queries), + while admin with member_id does not match and gets :all from HasPermission. + + - With a record (e.g. get by id): returns true only when own_data and member_id match. + - Without a record (list query): return :unknown so authorizer applies auto_filter. + """ + use Ash.Policy.Check + + alias Mv.Authorization.Checks.ActorPermissionSetIs + + @impl true + def type, do: :filter + + @impl true + def describe(_opts), + do: "own_data can read only membership_fee_cycles where member_id == actor.member_id" + + @impl true + def strict_check(actor, authorizer, _opts) do + record = get_record_from_authorizer(authorizer) + is_own_data = ActorPermissionSetIs.match?(actor, authorizer, permission_set_name: "own_data") + + cond do + is_nil(record) and is_own_data -> + {:ok, :unknown} + + is_nil(record) -> + {:ok, false} + + not is_own_data -> + {:ok, false} + + record.member_id == actor.member_id -> + {:ok, true} + + true -> + {:ok, false} + end + end + + @impl true + def auto_filter(actor, _authorizer, _opts) do + if ActorPermissionSetIs.match?(actor, nil, permission_set_name: "own_data") && + Map.get(actor, :member_id) do + [member_id: actor.member_id] + else + [] + end + end + + defp get_record_from_authorizer(authorizer) do + case authorizer.subject do + %{data: data} when not is_nil(data) -> data + _ -> nil + end + end +end diff --git a/lib/mv/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex index 858748d..9a5f7a7 100644 --- a/lib/mv/authorization/permission_sets.ex +++ b/lib/mv/authorization/permission_sets.ex @@ -58,6 +58,27 @@ defmodule Mv.Authorization.PermissionSets do pages: [String.t()] } + # DRY helpers for shared resource permission lists (used in own_data, read_only, normal_user, admin) + defp perm(resource, action, scope), + do: %{resource: resource, action: action, scope: scope, granted: true} + + # All four CRUD actions for a resource with scope :all (used for admin) + defp perm_all(resource), + do: [ + perm(resource, :read, :all), + perm(resource, :create, :all), + perm(resource, :update, :all), + perm(resource, :destroy, :all) + ] + + # User: read/update own credentials only (all non-admin sets allow password changes) + defp user_own_credentials, do: [perm("User", :read, :own), perm("User", :update, :own)] + + defp group_read_all, do: [perm("Group", :read, :all)] + defp custom_field_read_all, do: [perm("CustomField", :read, :all)] + defp membership_fee_type_read_all, do: [perm("MembershipFeeType", :read, :all)] + defp membership_fee_cycle_read_all, do: [perm("MembershipFeeCycle", :read, :all)] + @doc """ Returns the list of all valid permission set names. @@ -94,29 +115,21 @@ defmodule Mv.Authorization.PermissionSets do def get_permissions(:own_data) do %{ - resources: [ - # User: Can read/update own credentials only - # IMPORTANT: "read_only" refers to member data, NOT user credentials. - # All permission sets grant User.update :own to allow password changes. - %{resource: "User", action: :read, scope: :own, granted: true}, - %{resource: "User", action: :update, scope: :own, granted: true}, - - # Member: Can read/update linked member - %{resource: "Member", action: :read, scope: :linked, granted: true}, - %{resource: "Member", action: :update, scope: :linked, granted: true}, - - # CustomFieldValue: Can read/update/create/destroy custom field values of linked member - %{resource: "CustomFieldValue", action: :read, scope: :linked, granted: true}, - %{resource: "CustomFieldValue", action: :update, scope: :linked, granted: true}, - %{resource: "CustomFieldValue", action: :create, scope: :linked, granted: true}, - %{resource: "CustomFieldValue", action: :destroy, scope: :linked, granted: true}, - - # CustomField: Can read all (needed for forms) - %{resource: "CustomField", action: :read, scope: :all, granted: true}, - - # Group: Can read all (needed for viewing groups) - %{resource: "Group", action: :read, scope: :all, granted: true} - ], + resources: + user_own_credentials() ++ + [ + perm("Member", :read, :linked), + perm("Member", :update, :linked), + perm("CustomFieldValue", :read, :linked), + perm("CustomFieldValue", :update, :linked), + perm("CustomFieldValue", :create, :linked), + perm("CustomFieldValue", :destroy, :linked) + ] ++ + custom_field_read_all() ++ + group_read_all() ++ + [perm("MemberGroup", :read, :linked)] ++ + membership_fee_type_read_all() ++ + [perm("MembershipFeeCycle", :read, :linked)], pages: [ # No "/" - Mitglied must not see member index at root (same content as /members). # Own profile (sidebar links to /users/:id) and own user edit @@ -133,25 +146,17 @@ defmodule Mv.Authorization.PermissionSets do def get_permissions(:read_only) do %{ - resources: [ - # User: Can read/update own credentials only - # IMPORTANT: "read_only" refers to member data, NOT user credentials. - # All permission sets grant User.update :own to allow password changes. - %{resource: "User", action: :read, scope: :own, granted: true}, - %{resource: "User", action: :update, scope: :own, granted: true}, - - # Member: Can read all members, no modifications - %{resource: "Member", action: :read, scope: :all, granted: true}, - - # CustomFieldValue: Can read all custom field values - %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, - - # CustomField: Can read all - %{resource: "CustomField", action: :read, scope: :all, granted: true}, - - # Group: Can read all - %{resource: "Group", action: :read, scope: :all, granted: true} - ], + resources: + user_own_credentials() ++ + [ + perm("Member", :read, :all), + perm("CustomFieldValue", :read, :all) + ] ++ + custom_field_read_all() ++ + group_read_all() ++ + [perm("MemberGroup", :read, :all)] ++ + membership_fee_type_read_all() ++ + membership_fee_cycle_read_all(), pages: [ "/", # Own profile (sidebar links to /users/:id; redirect target must be allowed) @@ -176,31 +181,37 @@ defmodule Mv.Authorization.PermissionSets do def get_permissions(:normal_user) do %{ - resources: [ - # User: Can read/update own credentials only - # IMPORTANT: "read_only" refers to member data, NOT user credentials. - # All permission sets grant User.update :own to allow password changes. - %{resource: "User", action: :read, scope: :own, granted: true}, - %{resource: "User", action: :update, scope: :own, granted: true}, - - # Member: Full CRUD except destroy (safety) - %{resource: "Member", action: :read, scope: :all, granted: true}, - %{resource: "Member", action: :create, scope: :all, granted: true}, - %{resource: "Member", action: :update, scope: :all, granted: true}, - # Note: destroy intentionally omitted for safety - - # CustomFieldValue: Full CRUD - %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, - %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true}, - %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true}, - %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true}, - - # CustomField: Read only (admin manages definitions) - %{resource: "CustomField", action: :read, scope: :all, granted: true}, - - # Group: Can read all - %{resource: "Group", action: :read, scope: :all, granted: true} - ], + resources: + user_own_credentials() ++ + [ + perm("Member", :read, :all), + perm("Member", :create, :all), + perm("Member", :update, :all), + # destroy intentionally omitted for safety + perm("CustomFieldValue", :read, :all), + perm("CustomFieldValue", :create, :all), + perm("CustomFieldValue", :update, :all), + perm("CustomFieldValue", :destroy, :all) + ] ++ + custom_field_read_all() ++ + [ + perm("Group", :read, :all), + perm("Group", :create, :all), + perm("Group", :update, :all), + perm("Group", :destroy, :all) + ] ++ + [ + perm("MemberGroup", :read, :all), + perm("MemberGroup", :create, :all), + perm("MemberGroup", :destroy, :all) + ] ++ + membership_fee_type_read_all() ++ + [ + perm("MembershipFeeCycle", :read, :all), + perm("MembershipFeeCycle", :create, :all), + perm("MembershipFeeCycle", :update, :all), + perm("MembershipFeeCycle", :destroy, :all) + ], pages: [ "/", # Own profile (sidebar links to /users/:id; redirect target must be allowed) @@ -221,52 +232,39 @@ defmodule Mv.Authorization.PermissionSets do "/custom_field_values/:id/edit", # Groups overview "/groups", + # Create group + "/groups/new", # Group detail - "/groups/:slug" + "/groups/:slug", + # Edit group + "/groups/:slug/edit" ] } end def get_permissions(:admin) do + # MemberGroup has no :update action in the domain; use read/create/destroy only + member_group_perms = [ + perm("MemberGroup", :read, :all), + perm("MemberGroup", :create, :all), + perm("MemberGroup", :destroy, :all) + ] + %{ - resources: [ - # User: Full management including other users - %{resource: "User", action: :read, scope: :all, granted: true}, - %{resource: "User", action: :create, scope: :all, granted: true}, - %{resource: "User", action: :update, scope: :all, granted: true}, - %{resource: "User", action: :destroy, scope: :all, granted: true}, - - # Member: Full CRUD - %{resource: "Member", action: :read, scope: :all, granted: true}, - %{resource: "Member", action: :create, scope: :all, granted: true}, - %{resource: "Member", action: :update, scope: :all, granted: true}, - %{resource: "Member", action: :destroy, scope: :all, granted: true}, - - # CustomFieldValue: Full CRUD - %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, - %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true}, - %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true}, - %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true}, - - # CustomField: Full CRUD (admin manages custom field definitions) - %{resource: "CustomField", action: :read, scope: :all, granted: true}, - %{resource: "CustomField", action: :create, scope: :all, granted: true}, - %{resource: "CustomField", action: :update, scope: :all, granted: true}, - %{resource: "CustomField", action: :destroy, scope: :all, granted: true}, - - # Role: Full CRUD (admin manages roles) - %{resource: "Role", action: :read, scope: :all, granted: true}, - %{resource: "Role", action: :create, scope: :all, granted: true}, - %{resource: "Role", action: :update, scope: :all, granted: true}, - %{resource: "Role", action: :destroy, scope: :all, granted: true}, - - # Group: Full CRUD (admin manages groups) - %{resource: "Group", action: :read, scope: :all, granted: true}, - %{resource: "Group", action: :create, scope: :all, granted: true}, - %{resource: "Group", action: :update, scope: :all, granted: true}, - %{resource: "Group", action: :destroy, scope: :all, granted: true} - ], + resources: + perm_all("User") ++ + perm_all("Member") ++ + perm_all("CustomFieldValue") ++ + perm_all("CustomField") ++ + perm_all("Role") ++ + perm_all("Group") ++ + member_group_perms ++ + perm_all("MembershipFeeType") ++ + perm_all("MembershipFeeCycle"), pages: [ + # Explicit admin-only pages (for clarity and future restrictions) + "/settings", + "/membership_fee_settings", # Wildcard: Admin can access all pages "*" ] diff --git a/lib/mv/email_sync/changes/sync_user_email_to_member.ex b/lib/mv/email_sync/changes/sync_user_email_to_member.ex index eb6770c..624692b 100644 --- a/lib/mv/email_sync/changes/sync_user_email_to_member.ex +++ b/lib/mv/email_sync/changes/sync_user_email_to_member.ex @@ -27,6 +27,10 @@ defmodule Mv.EmailSync.Changes.SyncUserEmailToMember do Modified changeset with email synchronization applied, or original changeset if recursion detected. """ + # Ash 3.12+ calls this to decide whether to run the change in certain contexts. + @impl true + def has_change?, do: true + @impl true def change(changeset, _opts, context) do # Only recursion protection needed - trigger logic is in `where` clauses diff --git a/lib/mv/helpers.ex b/lib/mv/helpers.ex index e20db67..ae22e13 100644 --- a/lib/mv/helpers.ex +++ b/lib/mv/helpers.ex @@ -5,6 +5,8 @@ defmodule Mv.Helpers do Provides utilities that are not specific to a single domain or layer. """ + require Ash.Query + @doc """ Converts an actor to Ash options list for authorization. Returns empty list if actor is nil. @@ -24,4 +26,22 @@ defmodule Mv.Helpers do @spec ash_actor_opts(Mv.Accounts.User.t() | nil) :: keyword() def ash_actor_opts(nil), do: [] def ash_actor_opts(actor) when not is_nil(actor), do: [actor: actor] + + @doc """ + Returns the query unchanged if `exclude_id` is nil; otherwise adds a filter `id != ^exclude_id`. + + Used in uniqueness validations that must exclude the current record (e.g. name uniqueness + on update, duplicate association checks). Call with the record's primary key to exclude it + from the result set. + + ## Examples + + query + |> Ash.Query.filter(name == ^name) + |> Mv.Helpers.query_exclude_id(current_id) + + """ + @spec query_exclude_id(Ash.Query.t(), String.t() | nil) :: Ash.Query.t() + def query_exclude_id(query, nil), do: query + def query_exclude_id(query, id), do: Ash.Query.filter(query, id != ^id) end diff --git a/lib/mv/membership/member/validations/email_not_used_by_other_user.ex b/lib/mv/membership/member/validations/email_not_used_by_other_user.ex index 1ee8ab0..1297515 100644 --- a/lib/mv/membership/member/validations/email_not_used_by_other_user.ex +++ b/lib/mv/membership/member/validations/email_not_used_by_other_user.ex @@ -56,7 +56,7 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do query = Mv.Accounts.User |> Ash.Query.filter(email == ^email) - |> maybe_exclude_id(exclude_user_id) + |> Mv.Helpers.query_exclude_id(exclude_user_id) system_actor = SystemActor.get_system_actor() opts = Helpers.ash_actor_opts(system_actor) @@ -76,7 +76,4 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do :ok end end - - defp maybe_exclude_id(query, nil), do: query - defp maybe_exclude_id(query, id), do: Ash.Query.filter(query, id != ^id) end diff --git a/lib/mv_web/components/core_components.ex b/lib/mv_web/components/core_components.ex index 59e300e..9ef8f2b 100644 --- a/lib/mv_web/components/core_components.ex +++ b/lib/mv_web/components/core_components.ex @@ -545,6 +545,9 @@ defmodule MvWeb.CoreComponents do attr :label, :string attr :class, :string attr :col_click, :any, doc: "optional column-specific click handler that overrides row_click" + + attr :sort_field, :any, + doc: "optional; when equal to table sort_field, aria-sort is set on this th" end slot :action, doc: "the slot for showing user actions in the last table column" @@ -560,7 +563,13 @@ defmodule MvWeb.CoreComponents do - +
{col[:label]} + {col[:label]} + <.live_component module={MvWeb.Components.SortHeaderComponent} @@ -646,6 +655,16 @@ defmodule MvWeb.CoreComponents do """ end + defp table_th_aria_sort(col, sort_field, sort_order) do + col_sort = Map.get(col, :sort_field) + + if not is_nil(col_sort) and col_sort == sort_field and sort_order in [:asc, :desc] do + if sort_order == :asc, do: "ascending", else: "descending" + else + nil + end + end + @doc """ Renders a data list. diff --git a/lib/mv_web/components/table_components.ex b/lib/mv_web/components/table_components.ex index ed94994..6b3c060 100644 --- a/lib/mv_web/components/table_components.ex +++ b/lib/mv_web/components/table_components.ex @@ -20,7 +20,6 @@ defmodule MvWeb.TableComponents do type="button" phx-click="sort" phx-value-field={@field} - aria-sort={aria_sort(@sort_field, @sort_order, @field)} class="flex items-center gap-1 hover:underline focus:outline-none" > {@label} @@ -33,12 +32,4 @@ defmodule MvWeb.TableComponents do """ end - - defp aria_sort(current_field, current_order, this_field) do - cond do - current_field != this_field -> "none" - current_order == :asc -> "ascending" - true -> "descending" - end - end end diff --git a/lib/mv_web/helpers/user_helpers.ex b/lib/mv_web/helpers/user_helpers.ex new file mode 100644 index 0000000..2f9741c --- /dev/null +++ b/lib/mv_web/helpers/user_helpers.ex @@ -0,0 +1,58 @@ +defmodule MvWeb.Helpers.UserHelpers do + @moduledoc """ + Helper functions for user-related display in the web layer. + + Provides utilities for showing authentication status without exposing + sensitive attributes (e.g. hashed_password). + """ + + @doc """ + Returns whether the user has password authentication set. + + Only returns true when `hashed_password` is a non-empty string. This avoids + treating `nil`, empty string, or forbidden/redacted values (e.g. when the + attribute is not visible to the actor) as "has password". + + ## Examples + + iex> user = %{hashed_password: nil} + iex> MvWeb.Helpers.UserHelpers.has_password?(user) + false + + iex> user = %{hashed_password: "$2b$12$..."} + iex> MvWeb.Helpers.UserHelpers.has_password?(user) + true + + iex> user = %{hashed_password: ""} + iex> MvWeb.Helpers.UserHelpers.has_password?(user) + false + """ + @spec has_password?(map() | struct()) :: boolean() + def has_password?(user) when is_map(user) do + case Map.get(user, :hashed_password) do + hash when is_binary(hash) and byte_size(hash) > 0 -> true + _ -> false + end + end + + @doc """ + Returns whether the user is linked via OIDC/SSO (has a non-empty oidc_id). + + ## Examples + + iex> user = %{oidc_id: nil} + iex> MvWeb.Helpers.UserHelpers.has_oidc?(user) + false + + iex> user = %{oidc_id: "sub-from-rauthy"} + iex> MvWeb.Helpers.UserHelpers.has_oidc?(user) + true + """ + @spec has_oidc?(map() | struct()) :: boolean() + def has_oidc?(user) when is_map(user) do + case Map.get(user, :oidc_id) do + id when is_binary(id) and byte_size(id) > 0 -> true + _ -> false + end + end +end diff --git a/lib/mv_web/live/member_live/form.ex b/lib/mv_web/live/member_live/form.ex index b72add6..f9588c0 100644 --- a/lib/mv_web/live/member_live/form.ex +++ b/lib/mv_web/live/member_live/form.ex @@ -177,7 +177,8 @@ defmodule MvWeb.MemberLive.Form do phx-change="validate" value={@form[:membership_fee_type_id].value || ""} > - + <%!-- No "None" option: a membership fee type is required (validated in Member resource). --%> + <%= for fee_type <- @available_fee_types do %>