From b128ffb51c4c8f8de82a297decbb7dae25773982 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 27 Jan 2026 13:04:27 +0100 Subject: [PATCH 01/25] docs: add groups concept --- docs/groups-architecture.md | 1033 +++++++++++++++++++++++++++++++++++ 1 file changed, 1033 insertions(+) create mode 100644 docs/groups-architecture.md diff --git a/docs/groups-architecture.md b/docs/groups-architecture.md new file mode 100644 index 0000000..c401e2a --- /dev/null +++ b/docs/groups-architecture.md @@ -0,0 +1,1033 @@ +# Groups - Technical Architecture + +**Project:** Mila - Membership Management System +**Feature:** Groups Management +**Version:** 1.0 +**Last Updated:** 2025-01-XX +**Status:** Architecture Design - Ready for Implementation + +--- + +## Purpose + +This document defines the technical architecture for the Groups feature. It focuses on architectural decisions, patterns, module structure, and integration points **without** concrete implementation details. + +**Related Documents:** + +- [database-schema-readme.md](./database-schema-readme.md) - Database documentation +- [roles-and-permissions-architecture.md](./roles-and-permissions-architecture.md) - Authorization system + +--- + +## Table of Contents + +1. [Architecture Principles](#architecture-principles) +2. [Domain Structure](#domain-structure) +3. [Data Architecture](#data-architecture) +4. [Business Logic Architecture](#business-logic-architecture) +5. [UI/UX Architecture](#uiux-architecture) +6. [Integration Points](#integration-points) +7. [Authorization](#authorization) +8. [Performance Considerations](#performance-considerations) +9. [Future Extensibility](#future-extensibility) +10. [Implementation Phases](#implementation-phases) + +--- + +## Architecture Principles + +### Core Design Decisions + +1. **Many-to-Many Relationship:** + - Members can belong to multiple groups + - Groups can contain multiple members + - Implemented via join table (`member_groups`) as separate Ash resource + +2. **Flat Structure (MVP):** + - Groups are initially flat (no hierarchy) + - Architecture designed to allow hierarchical extension later + - No parent/child relationships in MVP + +3. **Minimal Attributes (MVP):** + - Only `name` and `description` in initial version + - Extensible for future attributes (dates, status, etc.) + +4. **Cascade Deletion:** + - Deleting a group removes all member-group associations + - Members themselves are not deleted (CASCADE on join table only) + - Requires explicit confirmation with group name input + +5. **Search Integration:** + - Groups searchable within member search (not separate search) + - Group names included in member search vector for full-text search + +--- + +## Domain Structure + +### Ash Domain: `Mv.Membership` + +**Purpose:** Groups are part of the Membership domain, alongside Members and CustomFields + +**New Resources:** + +- `Group` - Group definitions (name, description) +- `MemberGroup` - Join table for many-to-many relationship between Members and Groups + +**Extended Resources:** + +- `Member` - Extended with `has_many :groups` relationship (through MemberGroup) + +### Module Organization + +``` +lib/ +├── membership/ +│ ├── membership.ex # Domain definition (extended) +│ ├── group.ex # Group resource +│ ├── member_group.ex # MemberGroup join table resource +│ └── member.ex # Extended with groups relationship +├── mv_web/ +│ └── live/ +│ ├── group_live/ +│ │ ├── index.ex # Groups management page +│ │ ├── form.ex # Create/edit group form +│ │ └── show.ex # Group detail view +│ └── member_live/ +│ ├── index.ex # Extended with group filtering/sorting +│ └── show.ex # Extended with group display +└── mv/ + └── membership/ + └── group/ # Future: Group-specific business logic + └── helpers.ex # Group-related helper functions +``` + +--- + +## Data Architecture + +### Database Schema + +#### `groups` Table + +**Attributes:** +- `id` - UUID v7 primary key +- `name` - Unique group name (required, max 100 chars) +- `description` - Optional description (max 500 chars) +- `inserted_at` / `updated_at` - Timestamps + +**Constraints:** +- `name` must be unique (case-insensitive, using LOWER(name)) +- `name` cannot be null +- `name` max length: 100 characters +- `description` max length: 500 characters + +#### `member_groups` Table (Join Table) + +**Attributes:** +- `id` - UUID v7 primary key +- `member_id` - Foreign key to members (CASCADE delete) +- `group_id` - Foreign key to groups (CASCADE delete) +- `inserted_at` / `updated_at` - Timestamps + +**Constraints:** +- Unique constraint on `(member_id, group_id)` - prevents duplicate memberships +- CASCADE delete: Removing member removes all group associations +- CASCADE delete: Removing group removes all member associations + +**Indexes:** +- Index on `member_id` for efficient member → groups queries +- Index on `group_id` for efficient group → members queries + +### Ash Resources + +#### `Mv.Membership.Group` + +**Relationships:** +- `has_many :member_groups` - Relationship to MemberGroup join table +- `many_to_many :members` - Relationship to Members through MemberGroup + +**Calculations:** +- `member_count` - Integer calculation counting associated members + +**Actions:** +- `create` - Create new group +- `read` - List/search groups +- `update` - Update group name/description +- `destroy` - Delete group (with confirmation) + +**Validations:** +- `name` required, unique (case-insensitive), max 100 chars +- `description` optional, max 500 chars + +#### `Mv.Membership.MemberGroup` + +**Relationships:** +- `belongs_to :member` - Relationship to Member +- `belongs_to :group` - Relationship to Group + +**Actions:** +- `create` - Add member to group +- `read` - Query member-group associations +- `destroy` - Remove member from group + +**Validations:** +- Unique constraint on `(member_id, group_id)` + +#### `Mv.Membership.Member` (Extended) + +**New Relationships:** +- `has_many :member_groups` - Relationship to MemberGroup join table +- `many_to_many :groups` - Relationship to Groups through MemberGroup + +**New Actions:** +- `add_to_groups` - Add member to one or more groups +- `remove_from_groups` - Remove member from one or more groups + +--- + +## Business Logic Architecture + +### Group Management + +**Create Group:** +- Validate name uniqueness +- Generate slug (if needed for future URL-friendly identifiers) +- Return created group + +**Update Group:** +- Validate name uniqueness (if name changed) +- Update description +- Return updated group + +**Delete Group:** +- Check if group has members (for warning display) +- Require explicit confirmation (group name input) +- Cascade delete all `member_groups` associations +- Group itself deleted + +### Member-Group Association + +**Add Member to Group:** +- Validate member exists +- Validate group exists +- Check for duplicate association +- Create `MemberGroup` record + +**Remove Member from Group:** +- Find `MemberGroup` record +- Delete association +- Member and group remain intact + +**Bulk Operations:** +- Add member to multiple groups in single transaction +- Remove member from multiple groups in single transaction + +### Search Integration + +**Member Search Enhancement:** +- Include group names in member search vector +- When searching for member, also search in associated group names +- Example: Searching for a group name finds all members in groups with that name + +**Implementation:** +- Extend `member.search_vector` trigger to include group names +- Update trigger on `member_groups` changes +- Use PostgreSQL `tsvector` for full-text search + +--- + +## UI/UX Architecture + +### Groups Management Page (`/groups`) + +**Route:** `/groups` - Groups management index page + +**Features:** +- List all groups in table +- Create new group button +- Edit group (inline or modal) +- Delete group with confirmation modal +- Show member count per group + +**Table Columns:** +- Name (sortable, searchable) +- Description +- Member Count +- Actions (Edit, Delete) + +**Delete Confirmation Modal:** +- Warning: "X members are in this group" +- Confirmation: "All member-group associations will be permanently deleted" +- Input field: Enter group name to confirm +- Delete button disabled until name matches +- Cancel button + +### Member Overview Integration + +**New Column: "Groups"** +- Display group badges for each member +- Badge shows group name +- Multiple badges if member in multiple groups +- *(Optional)* Click badge to filter by that group (enhanced UX, can be added later) + +**Filtering:** +- Dropdown/select to filter by group +- "All groups" option (default) +- Filter persists in URL query params +- Works with existing search/sort + +**Sorting:** +- Sort by group name (members with groups first, then alphabetically) +- Sort by number of groups (members with most groups first) + +**Search:** +- Group names included in member search +- Searching group name shows all members in that group + +### Member Detail View Integration + +**New Section: "Groups"** +- List all groups member belongs to +- Display as badges or list +- Add/remove groups inline +- Link to group detail page + +### Group Detail View (`/groups/:id`) + +**Route:** `/groups/:id` - Group detail page + +**Features:** +- Display group name and description +- List all members in group +- Link to member detail pages +- Edit group button +- Delete group button (with confirmation) + +### Accessibility (A11y) Considerations + +**Requirements:** +- All UI elements must be keyboard accessible +- Screen readers must be able to navigate and understand the interface +- ARIA labels and roles must be properly set + +**Group Badges in Member Overview:** +- Badges must have `role="status"` and appropriate `aria-label` attributes +- Badge title should indicate group membership + +**Clickable Group Badge (for filtering) - Optional:** + +**Note:** This is an optional enhancement. The dropdown filter provides the same functionality. The clickable badge improves UX by showing the active filter visually and allowing quick removal. + +**Estimated effort:** 1.5-2.5 hours + +- Clickable badges must be proper button elements with `type="button"` +- Must include `aria-label` describing the filter action +- Icon for removal should have `aria-hidden="true"` + +**Group Filter Dropdown:** +- Select element must have appropriate `id`, `name`, and `aria-label` attributes +- Options should clearly indicate selected state + +**Screen Reader Announcements:** +- Use `role="status"` with `aria-live="polite"` for dynamic content +- Announce filter changes and member count updates + +**Delete Confirmation Modal:** +- Modal must use proper `role="dialog"` with `aria-labelledby` and `aria-describedby` +- Warning messages must be clearly associated with the modal description +- Form inputs must be properly labeled + +**Keyboard Navigation:** +- All interactive elements (buttons, links, form inputs) must be focusable via Tab key +- Modal dialogs must trap focus (Tab key cycles within modal) +- Escape key closes modals +- Enter/Space activates buttons when focused + +--- + +## Integration Points + +### Member Search Vector + +**Trigger Update:** +- When `member_groups` record created/deleted +- Update `members.search_vector` to include group names +- Use PostgreSQL trigger for automatic updates + +**Search Query:** +- Extend existing `fuzzy_search` to include group names +- Group names added with weight 'B' (same as city, etc.) + +### Member Form + +**Future Enhancement:** +- Add groups selection in member form +- Multi-select dropdown for groups +- Add/remove groups during member creation/edit + +### Authorization Integration + +**Current (MVP):** +- Only admins can manage groups +- Uses existing `Mv.Authorization.Checks.HasPermission` +- Permission: `groups` resource with `:all` scope + +**Future:** +- Group-specific permissions +- Role-based group management +- Member-level group assignment permissions + +--- + +## Authorization + +### Permission Model (MVP) + +**Resource:** `groups` + +**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) + +**Scopes:** +- `:all` - All groups (for all permission sets that have read access) + +### Permission Sets Update + +**Admin Permission Set:** +- `read` action on `Group` resource with `:all` scope - granted +- `create` action on `Group` resource with `:all` scope - granted +- `update` action on `Group` resource with `:all` scope - granted +- `destroy` action on `Group` resource with `:all` scope - granted + +**Read-Only Permission Set:** +- `read` action on `Group` resource with `:all` scope - granted + +**Normal User Permission Set:** +- `read` action on `Group` resource with `:all` scope - granted + +**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. + +### Member-Group Association Permissions + +**Current (MVP):** +- Adding/removing members from groups requires group update permission +- Managed through group edit interface + +**Future:** +- Separate permission for member-group management +- Member-level permissions for self-assignment + +--- + +## Performance Considerations + +### Database Indexes + +**Critical Indexes:** +- `groups.name` - For uniqueness and search +- `member_groups.member_id` - For member → groups queries +- `member_groups.group_id` - For group → members queries +- Composite index on `(member_id, group_id)` - For uniqueness check + +### Query Optimization + +**Member Overview:** +- Load groups with members in single query using query preloading +- Preload only necessary group attributes (id, name) to minimize data transfer +- Filter groups at database level when filtering by group + +**N+1 Query Prevention:** +- Always preload groups relationship when querying members +- Never access member groups without preloading (would trigger N+1 queries) +- Use query preloading mechanisms to load all required relationships in a single query + +**Performance Threshold:** +- With proper preloading: Works efficiently up to **100 members** (MVP scope) +- For larger datasets (>100 members), consider: + - Pagination (limit number of members loaded) + - Lazy loading of groups (only load when groups column is visible) + - Database-level aggregation for group counts + +**Group Detail:** +- Paginate member list for large groups (>50 members) +- Load member count via calculation (not separate query) +- Use `Ash.Query.load` for member details when displaying + +### Search Performance + +**Search Vector:** +- Group names included in `search_vector` (tsvector) +- GIN index on `search_vector` for fast full-text search +- Trigger updates on `member_groups` changes + +**Filtering:** +- Use database-level filtering (not in-memory) +- Leverage indexes for group filtering + +--- + +## Future Extensibility + +### Hierarchical Groups + +**Design for Future:** +- Add `parent_group_id` to `groups` table (nullable) +- Add `parent_group` relationship (self-referential) +- Add validation to prevent circular references +- Add calculation for `path` (e.g., "Parent > Child > Grandchild") + +**Migration Path:** +- Add column with `NULL` default (all groups initially root-level) +- Add foreign key constraint +- Add validation logic +- Update UI to show hierarchy + +### Group Attributes + +**Future Attributes:** +- `created_at` / `founded_date` - Group creation date +- `dissolved_at` - Group dissolution date +- `status` - Active/inactive/suspended +- `color` - UI color for badges +- `icon` - Icon identifier + +**Migration Path:** +- Add nullable columns +- Set defaults for existing groups +- Update UI to display new attributes + +### Roles/Positions in Groups + +**Future Feature:** +- Add `member_group_roles` table +- Link `MemberGroup` to `Role` (e.g., "Leiter", "Mitglied") +- Extend `MemberGroup` with `role_id` foreign key +- Display role in member detail and group detail views + +### Group Permissions + +**Future Feature:** +- Group-specific permission sets +- Role-based group access +- Member-level group management permissions + +--- + +## Feature Breakdown: Functional Units and MVP + +### Strategy: Vertical Slicing + +The Groups feature is divided into **functionally complete, vertical units**. Each unit delivers a complete, usable functional area that can be independently tested and delivered. + +### MVP Definition + +**Minimal Viable Product (MVP):** +The MVP includes the **core functionality** necessary to manage groups and assign them to members: + +1. ✅ Create groups (Name + Description) +2. ✅ Edit groups +3. ✅ Delete groups (with confirmation) +4. ✅ Assign members to groups +5. ✅ Remove members from groups +6. ✅ Display groups in member overview +7. ✅ Filter by groups +8. ✅ Sort by groups +9. ✅ Display groups in member detail +10. ✅ Groups in member search (automatically via search_vector) + +**Not in MVP:** +- ❌ Hierarchical groups +- ❌ Roles/positions in groups +- ❌ Extended group attributes (dates, status, etc.) +- ❌ Group-specific permissions + +### Functional Units (Vertical Slices) + +#### Unit 1: Group Management (Backend) +**Functional Scope:** Administrators can manage groups in the system + +**Scope:** +- Group resource (Name, Description) +- CRUD operations for groups +- Validations (unique name, length limits) +- Delete logic with cascade behavior + +**Deliverable:** Groups can be created, edited, and deleted via API + +**Dependencies:** None + +**Estimation:** 4-5h + +--- + +#### Unit 2: Member-Group Assignment (Backend) +**Functional Scope:** Members can be assigned to groups + +**Scope:** +- MemberGroup join table +- Many-to-Many relationship +- Add/Remove member-group associations +- Cascade delete behavior + +**Deliverable:** Members can be assigned to and removed from groups + +**Dependencies:** Unit 1 (Groups must exist) + +**Estimation:** 2-3h (can be combined with Unit 1) + +--- + +#### Unit 3: Group Management UI +**Functional Scope:** Administrators can manage groups via web interface + +**Scope:** +- Groups overview page (`/groups`) +- Group form (Create/Edit) +- Group detail page (member list) +- Delete confirmation modal (with name input) + +**Deliverable:** Complete group management via UI possible + +**Dependencies:** Unit 1 + 2 (Backend must be functional) + +**Estimation:** 3-4h + +--- + +#### Unit 4: Groups in Member Overview +**Functional Scope:** Groups are displayed in member overview and can be filtered/sorted + +**Scope:** +- "Groups" column with badges +- Group filter dropdown +- Sorting by groups +- URL parameter persistence + +**Deliverable:** Groups visible, filterable, and sortable in member overview + +**Dependencies:** Unit 1 + 2 (Groups and assignments must exist) + +**Estimation:** 2-3h + +--- + +#### Unit 5: Groups in Member Detail +**Functional Scope:** Groups are displayed in member detail view + +**Scope:** +- "Groups" section in Member Show +- Badge display +- Links to group detail pages + +**Deliverable:** Groups visible in member detail + +**Dependencies:** Unit 3 (Group detail page must exist) + +**Estimation:** 1-2h + +--- + +#### Unit 6: Groups in Member Search +**Functional Scope:** Group names are searchable in member search + +**Scope:** +- Search Vector Update (Trigger) +- Fuzzy Search extension +- Test search functionality + +**Deliverable:** Searching for group names finds associated members + +**Dependencies:** Unit 1 + 2 (Groups and assignments must exist) + +**Estimation:** 2h + +--- + +#### Unit 7: Permissions +**Functional Scope:** Only administrators can manage groups + +**Scope:** +- Add groups to permission sets +- Implement authorization policies +- UI permission checks + +**Deliverable:** Permissions correctly implemented + +**Dependencies:** All previous units (Feature must be functional) + +**Estimation:** 1-2h + +--- + +### MVP Composition + +**MVP consists of:** +- ✅ Unit 1: Group Management (Backend) +- ✅ Unit 2: Member-Group Assignment (Backend) +- ✅ Unit 3: Group Management UI +- ✅ Unit 4: Groups in Member Overview +- ✅ Unit 5: Groups in Member Detail +- ✅ Unit 6: Groups in Member Search (automatically via search_vector) +- ✅ Unit 7: Permissions + +**Total MVP Estimation:** 13-15h + +### Implementation Order + +**Recommended Order:** + +1. **Phase 1: Backend Foundation** (Unit 1 + 2) + - Group resource + - MemberGroup join table + - CRUD operations + - **Result:** Groups can be managed via API + +2. **Phase 2: Management UI** (Unit 3) + - Groups overview + - Group form + - Group detail + - **Result:** Groups can be managed via UI + +3. **Phase 3: Member Integration** (Unit 4 + 5) + - Groups in overview + - Groups in detail + - **Result:** Groups visible in member views + +4. **Phase 4: Search Integration** (Unit 6) + - Search Vector Update + - **Result:** Groups searchable + +5. **Phase 5: Permissions** (Unit 7) + - Permission Sets + - Policies + - **Result:** Permissions correct + +### Issue Structure + +Each functional unit can be implemented as a **separate issue**: + +- **Issue 1:** Group Resource & Database Schema (Unit 1 + 2) +- **Issue 2:** Group Management UI (Unit 3) +- **Issue 3:** Groups in Member Overview (Unit 4) +- **Issue 4:** Groups in Member Detail (Unit 5) +- **Issue 5:** Groups in Member Search (Unit 6) +- **Issue 6:** Permissions (Unit 7) + +**Alternative:** Issues 3 and 4 can be combined, as they both concern the display of groups. + +--- + +## Implementation Phases + +### Phase 1: MVP Core (Foundation) + +**Goal:** Basic group management and member assignment + +**Tasks:** +1. Create `Group` resource (name, description) +2. Create `MemberGroup` join table resource +3. Extend `Member` with groups relationship +4. Database migrations +5. Basic CRUD actions for groups +6. Add/remove members from groups (via group management) + +**Deliverables:** +- Groups can be created, edited, deleted +- Members can be added/removed from groups +- Basic validation and constraints + +**Estimation:** 4-5h + +### Phase 2: UI - Groups Management + +**Goal:** Complete groups management interface + +**Tasks:** +1. Groups index page (`/groups`) +2. Group form (create/edit) +3. Group show page (list members) +4. Delete confirmation modal (with name input) +5. Member count display + +**Deliverables:** +- Full groups management UI +- Delete confirmation workflow +- Group detail view + +**Estimation:** 3-4h + +### Phase 3: Member Overview Integration + +**Goal:** Display and filter groups in member overview + +**Tasks:** +1. Add "Groups" column to member overview table +2. Display group badges +3. Group filter dropdown +4. Group sorting +5. URL query param persistence +6. *(Optional)* Clickable group badges for filtering (enhanced UX) + +**Deliverables:** +- Groups visible in member overview +- Filter by group (via dropdown) +- Sort by group +- *(Optional)* Clickable badges for quick filtering + +**Estimation:** 2-3h + +### Phase 4: Member Detail Integration + +**Goal:** Display groups in member detail view + +**Tasks:** +1. Add "Groups" section to member show page +2. Display group badges +3. Link to group detail pages + +**Deliverables:** +- Groups visible in member detail +- Navigation to group pages + +**Estimation:** 1-2h + +### Phase 5: Search Integration + +**Goal:** Include groups in member search + +**Tasks:** +1. Update `search_vector` trigger to include group names +2. Extend `fuzzy_search` to search group names +3. Test search functionality + +**Deliverables:** +- Group names searchable in member search +- Search finds members by group name + +**Estimation:** 2h + +### Phase 6: Authorization + +**Goal:** Implement permission-based access control + +**Tasks:** +1. Add groups to permission sets +2. Implement authorization policies +3. Test permission enforcement +4. Update UI to respect permissions + +**Deliverables:** +- Only admins can manage groups +- All users can view groups (if they can view members) + +**Estimation:** 1-2h + +### Total Estimation: 13-18h + +**Note:** This aligns with the issue estimation of 15h. + +--- + +## Issue Breakdown + +### Issue 1: Groups Resource & Database Schema +**Type:** Backend +**Estimation:** 4-5h +**Tasks:** +- Create `Group` resource +- Create `MemberGroup` join table resource +- Extend `Member` resource +- Database migrations +- Basic validations + +**Acceptance Criteria:** +- Groups can be created via Ash API +- Members can be associated with groups +- Database constraints enforced + +### Issue 2: Groups Management UI +**Type:** Frontend +**Estimation:** 3-4h +**Tasks:** +- Groups index page +- Group form (create/edit) +- Group show page +- Delete confirmation modal + +**Acceptance Criteria:** +- Groups can be created/edited/deleted via UI +- Delete requires name confirmation +- Member count displayed + +### Issue 3: Member Overview - Groups Integration +**Type:** Frontend +**Estimation:** 2-3h +**Tasks:** +- Add groups column with badges +- Group filter dropdown +- Group sorting +- URL persistence +- *(Optional)* Clickable group badges for filtering + +**Acceptance Criteria:** +- Groups visible in member overview +- Can filter by group (via dropdown) +- Can sort by group +- Filter persists in URL +- *(Optional)* Can filter by clicking group badge + +### Issue 4: Member Detail - Groups Display +**Type:** Frontend +**Estimation:** 1-2h +**Tasks:** +- Add groups section to member show +- Display group badges +- Link to group pages + +**Acceptance Criteria:** +- Groups visible in member detail +- Links to group pages work + +### Issue 5: Search Integration +**Type:** Backend +**Estimation:** 2h +**Tasks:** +- Update search vector trigger to include group names +- Extend fuzzy search to search group names +- Test search functionality + +**Acceptance Criteria:** +- Group names searchable in member search (automatic via search_vector) +- Search finds members by group name +- Search vector updates automatically when member-group associations change + +**Note:** This is part of MVP as group names are automatically included in full-text search once the search_vector trigger is updated. + +### Issue 6: Authorization +**Type:** Backend/Frontend +**Estimation:** 1-2h +**Tasks:** +- Add groups to permission sets +- Implement policies +- Test permissions + +**Acceptance Criteria:** +- Only admins can manage groups +- All users can view groups (if they can view members) + +--- + +## Testing Strategy + +### Unit Tests + +#### Group Resource Tests + +**File:** `test/membership/group_test.exs` + +**Test Cases:** +- Create group with valid attributes +- Return error when name is missing +- Return error when name exceeds 100 characters +- Return error when name is not unique +- Name uniqueness is case-insensitive +- Allow description to be nil +- Trim whitespace from name +- Description max length is 500 characters +- Update group name and description +- Prevent duplicate name on update +- Delete group and all member associations +- Do not delete members themselves +- Member count calculation returns 0 for empty group +- Member count calculation returns correct count when members added +- Member count updates correctly when members removed + +#### MemberGroup Resource Tests + +**File:** `test/membership/member_group_test.exs` + +**Test Cases:** +- Create association between member and group +- Prevent duplicate associations +- Cascade delete when member deleted +- Cascade delete when group deleted + +### Integration Tests + +#### Member-Group Relationships + +**File:** `test/membership/group_integration_test.exs` + +**Test Cases:** +- Member can belong to multiple groups +- Group can contain multiple members + +### UI Tests + +#### Groups Management + +**File:** `test/mv_web/live/group_live/index_test.exs` + +**Test Cases:** +- List all groups +- Create new group +- Delete group with confirmation + +#### Member Overview Integration + +**File:** `test/mv_web/live/member_live/index_groups_test.exs` + +**Test Cases:** +- Display group badges +- Filter members by group + +--- + +## Migration Strategy + +### Database Migrations + +**Migration 1: Create groups table** +- Create table with UUID v7 primary key +- Add name field (required, unique, case-insensitive) +- Add description field (optional) +- Add timestamps +- Create unique index on lowercased name +- Create index on lowercased name for search + +**Migration 2: Create member_groups join table** +- Create table with UUID v7 primary key +- Add member_id and group_id foreign keys +- Add timestamps +- Create unique index on (member_id, group_id) +- Create indexes on member_id and group_id +- Add foreign key constraints with CASCADE delete + +**Migration 3: Update search_vector trigger (if needed)** +- Extend trigger to include group names +- Update trigger function + +### Code Migration + +**Ash Resources:** +- Use code generation tools to generate migrations +- Manually adjust if needed + +**Domain Updates:** +- Add groups resources to domain +- Define domain actions + +--- + +## Summary + +This architecture provides a solid foundation for the Groups feature while maintaining flexibility for future enhancements. The many-to-many relationship is implemented via a join table, following existing patterns in the codebase. The MVP focuses on core functionality (create, edit, delete groups, assign members) with clear extension points for hierarchical groups, roles, and advanced permissions. + +The implementation is split into 6 manageable issues, totaling approximately 15 hours of work, aligning with the original estimation. Each phase builds on the previous one, allowing for incremental development and testing. From b93b419246c7b1ba3296c739708c45d4bd99b8e6 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:12 +0100 Subject: [PATCH 02/25] Add CustomFieldValue create/destroy :linked to own_data permission set Allows members to create and delete custom field values for their linked member. --- lib/mv/authorization/permission_sets.ex | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/mv/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex index 22132cb..e133ed7 100644 --- a/lib/mv/authorization/permission_sets.ex +++ b/lib/mv/authorization/permission_sets.ex @@ -105,9 +105,11 @@ defmodule Mv.Authorization.PermissionSets do %{resource: "Member", action: :read, scope: :linked, granted: true}, %{resource: "Member", action: :update, scope: :linked, granted: true}, - # CustomFieldValue: Can read/update custom field values of linked member + # 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} From 80efa5b3bd3742764a6741bf876bcb0aef85ff2e Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:17 +0100 Subject: [PATCH 03/25] Add CustomFieldValueCreateScope check for create actions Ash cannot apply filters to create; this check enforces :linked/:all scope via strict_check only (no filter). --- .../checks/custom_field_value_create_scope.ex | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 lib/mv/authorization/checks/custom_field_value_create_scope.ex diff --git a/lib/mv/authorization/checks/custom_field_value_create_scope.ex b/lib/mv/authorization/checks/custom_field_value_create_scope.ex new file mode 100644 index 0000000..f5be53d --- /dev/null +++ b/lib/mv/authorization/checks/custom_field_value_create_scope.ex @@ -0,0 +1,64 @@ +defmodule Mv.Authorization.Checks.CustomFieldValueCreateScope do + @moduledoc """ + Policy check for CustomFieldValue create actions only. + + Use this for create instead of HasPermission because Ash cannot apply + filters to create actions ("Cannot use a filter to authorize a create"). + This check performs the same scope logic as HasPermission for create + (PermissionSets + :linked/:all) but only implements strict_check, so it + never adds a filter. + + Used in CustomFieldValue policies: + policy action_type(:create) do + authorize_if Mv.Authorization.Checks.CustomFieldValueCreateScope + end + """ + use Ash.Policy.Check + alias Mv.Authorization.PermissionSets + require Logger + + @impl true + def describe(_opts), + do: "CustomFieldValue create allowed by permission set scope (:linked or :all)" + + @impl true + def strict_check(actor, authorizer, _opts) do + actor = ensure_role_loaded(actor) + + with %{role: %{permission_set_name: ps_name}} when not is_nil(ps_name) <- actor, + {:ok, ps_atom} <- PermissionSets.permission_set_name_to_atom(ps_name), + permissions <- PermissionSets.get_permissions(ps_atom), + perm <- find_custom_field_value_create(permissions.resources) do + case perm do + nil -> {:ok, false} + %{scope: :all} -> {:ok, true} + %{scope: :linked} -> {:ok, member_id_matches?(authorizer, actor)} + end + else + _ -> {:ok, false} + end + end + + defp find_custom_field_value_create(resources) do + Enum.find(resources, fn p -> + p.resource == "CustomFieldValue" and p.action == :create and p.granted + end) + end + + defp member_id_matches?(authorizer, actor) do + member_id = get_create_member_id(authorizer) + !is_nil(member_id) and member_id == actor.member_id + end + + defp get_create_member_id(authorizer) do + changeset = authorizer.changeset || authorizer.subject + + if changeset && function_exported?(Ash.Changeset, :get_attribute, 2) do + Ash.Changeset.get_attribute(changeset, :member_id) + else + nil + end + end + + defp ensure_role_loaded(actor), do: Mv.Authorization.Actor.ensure_loaded(actor) +end From 1afb97b6df8b1977e17baf98e7dd1542a16104c4 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:22 +0100 Subject: [PATCH 04/25] Add authorization policies to CustomFieldValue resource - Authorizer and policies: bypass for read (member_id == actor.member_id), CustomFieldValueCreateScope for create, HasPermission for read/update/destroy. - HasPermission: pass authorizer into strict_check helper; document that create must use a dedicated check (no filter). --- lib/membership/custom_field_value.ex | 36 ++++++++++++++++++- lib/mv/authorization/checks/has_permission.ex | 7 ++-- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/lib/membership/custom_field_value.ex b/lib/membership/custom_field_value.ex index 232ba99..d6d91a6 100644 --- a/lib/membership/custom_field_value.ex +++ b/lib/membership/custom_field_value.ex @@ -39,7 +39,11 @@ defmodule Mv.Membership.CustomFieldValue do """ use Ash.Resource, domain: Mv.Membership, - data_layer: AshPostgres.DataLayer + data_layer: AshPostgres.DataLayer, + authorizers: [Ash.Policy.Authorizer] + + require Ash.Query + import Ash.Expr postgres do table "custom_field_values" @@ -62,6 +66,36 @@ defmodule Mv.Membership.CustomFieldValue do end end + # Authorization Policies + # Order matters: Most specific policies first, then general permission check + # Pattern aligns with User and Member resources (bypass for READ, HasPermission for update/destroy) + # Create uses CustomFieldValueCreateScope because Ash cannot apply filters to create actions. + policies do + # SPECIAL CASE: Users can READ custom field values of their linked member + # Bypass needed for list queries (expr triggers auto_filter in Ash) + bypass action_type(:read) do + description "Users can read custom field values of their linked member" + authorize_if expr(member_id == ^actor(:member_id)) + end + + # CREATE: CustomFieldValueCreateScope (no filter; Ash rejects filters on create) + # - :own_data -> create allowed when member_id == actor.member_id (scope :linked) + # - :read_only -> no create permission + # - :normal_user / :admin -> create allowed (scope :all) + policy action_type(:create) do + description "CustomFieldValue create allowed by permission set scope" + authorize_if Mv.Authorization.Checks.CustomFieldValueCreateScope + end + + # READ/UPDATE/DESTROY: HasPermission (scope :linked / :all) + policy action_type([:read, :update, :destroy]) do + description "Check permissions from user's role and permission set" + authorize_if Mv.Authorization.Checks.HasPermission + end + + # DEFAULT: Ash implicitly forbids if no policy authorized (fail-closed) + end + attributes do uuid_primary_key :id diff --git a/lib/mv/authorization/checks/has_permission.ex b/lib/mv/authorization/checks/has_permission.ex index 1a478b8..1cf1e39 100644 --- a/lib/mv/authorization/checks/has_permission.ex +++ b/lib/mv/authorization/checks/has_permission.ex @@ -110,12 +110,12 @@ defmodule Mv.Authorization.Checks.HasPermission do {:ok, false} true -> - strict_check_with_permissions(actor, resource, action, record) + strict_check_with_permissions(actor, resource, action, record, authorizer) end end # Helper function to reduce nesting depth - defp strict_check_with_permissions(actor, resource, action, record) do + defp strict_check_with_permissions(actor, resource, action, record, _authorizer) do # Ensure role is loaded (fallback if on_mount didn't run) actor = ensure_role_loaded(actor) @@ -148,6 +148,7 @@ defmodule Mv.Authorization.Checks.HasPermission do else # No record yet (e.g., read/list queries) - deny at strict_check level # Resources must use expr-based bypass policies for list filtering + # Create: use a dedicated check that does not return a filter (e.g. CustomFieldValueCreateScope) {:ok, false} end @@ -213,7 +214,7 @@ defmodule Mv.Authorization.Checks.HasPermission do {:filter, filter_expr} -> # :linked or :own scope - apply filter - # filter_expr is a keyword list from expr(...), return it directly + # Create actions must not use HasPermission (use a dedicated check, e.g. CustomFieldValueCreateScope) filter_expr false -> From 62dd939efa8be4608f415411e632f176fc949377 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:29 +0100 Subject: [PATCH 05/25] Pass actor to CustomFieldValue destroy and load in existing tests Required after CustomFieldValue gained authorization policies. --- test/membership/member_search_with_custom_fields_test.exs | 2 +- test/mv/membership/import/member_csv_test.exs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/membership/member_search_with_custom_fields_test.exs b/test/membership/member_search_with_custom_fields_test.exs index bd28ce5..284fcff 100644 --- a/test/membership/member_search_with_custom_fields_test.exs +++ b/test/membership/member_search_with_custom_fields_test.exs @@ -348,7 +348,7 @@ defmodule Mv.Membership.MemberSearchWithCustomFieldsTest do assert List.first(results).id == member1.id # Delete custom field value - assert :ok = Ash.destroy(cfv) + assert :ok = Ash.destroy(cfv, actor: system_actor) # Value should no longer be found deleted_results = diff --git a/test/mv/membership/import/member_csv_test.exs b/test/mv/membership/import/member_csv_test.exs index 778e82b..0304989 100644 --- a/test/mv/membership/import/member_csv_test.exs +++ b/test/mv/membership/import/member_csv_test.exs @@ -247,7 +247,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do member = Enum.find(members, &(&1.email == "withcustom@example.com")) assert member != nil - {:ok, member_with_cf} = Ash.load(member, :custom_field_values) + {:ok, member_with_cf} = Ash.load(member, :custom_field_values, actor: actor) assert length(member_with_cf.custom_field_values) == 1 cfv = List.first(member_with_cf.custom_field_values) assert cfv.custom_field_id == custom_field.id From 49af921336bfe19e0087909f432ad8389163ea59 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:34 +0100 Subject: [PATCH 06/25] Add CustomFieldValue policy tests (own_data, read_only, normal_user, admin) Covers read/update/create/destroy for linked vs unlinked members and CRUD permissions per permission set. --- .../custom_field_value_policies_test.exs | 487 ++++++++++++++++++ 1 file changed, 487 insertions(+) create mode 100644 test/mv/membership/custom_field_value_policies_test.exs diff --git a/test/mv/membership/custom_field_value_policies_test.exs b/test/mv/membership/custom_field_value_policies_test.exs new file mode 100644 index 0000000..c7a80db --- /dev/null +++ b/test/mv/membership/custom_field_value_policies_test.exs @@ -0,0 +1,487 @@ +defmodule Mv.Membership.CustomFieldValuePoliciesTest do + @moduledoc """ + Tests for CustomFieldValue resource authorization policies. + + Tests all 4 permission sets (own_data, read_only, normal_user, admin) + and verifies that policies correctly enforce access control based on + user roles and permission sets. + """ + # async: false because we need database commits to be visible across queries + use Mv.DataCase, async: false + + alias Mv.Membership.{CustomField, CustomFieldValue, Member} + alias Mv.Accounts + alias Mv.Authorization + + require Ash.Query + + setup do + system_actor = Mv.Helpers.SystemActor.get_system_actor() + %{actor: system_actor} + end + + # Helper to create a role with a specific permission set + defp create_role_with_permission_set(permission_set_name, actor) do + role_name = "Test Role #{permission_set_name} #{System.unique_integer([:positive])}" + + case Authorization.create_role( + %{ + name: role_name, + description: "Test role for #{permission_set_name}", + permission_set_name: permission_set_name + }, + actor: actor + ) do + {:ok, role} -> role + {:error, error} -> raise "Failed to create role: #{inspect(error)}" + end + end + + # Helper to create a user with a specific permission set + # Returns user with role preloaded (required for authorization) + defp create_user_with_permission_set(permission_set_name, actor) do + role = create_role_with_permission_set(permission_set_name, actor) + + {:ok, user} = + Accounts.User + |> Ash.Changeset.for_create(:register_with_password, %{ + email: "user#{System.unique_integer([:positive])}@example.com", + password: "testpassword123" + }) + |> Ash.create(actor: actor) + + {:ok, user} = + user + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.manage_relationship(:role, role, type: :append_and_remove) + |> Ash.update(actor: actor) + + {:ok, user_with_role} = Ash.load(user, :role, domain: Mv.Accounts, actor: actor) + user_with_role + end + + defp create_admin_user(actor) do + create_user_with_permission_set("admin", actor) + end + + defp create_linked_member_for_user(user, actor) do + admin = create_admin_user(actor) + + {:ok, member} = + Member + |> Ash.Changeset.for_create(:create_member, %{ + first_name: "Linked", + last_name: "Member", + email: "linked#{System.unique_integer([:positive])}@example.com" + }) + |> Ash.create(actor: admin, return_notifications?: false) + + user + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.force_change_attribute(:member_id, member.id) + |> Ash.update(actor: admin, domain: Mv.Accounts, return_notifications?: false) + + member + end + + defp create_unlinked_member(actor) do + admin = create_admin_user(actor) + + {:ok, member} = + Member + |> Ash.Changeset.for_create(:create_member, %{ + first_name: "Unlinked", + last_name: "Member", + email: "unlinked#{System.unique_integer([:positive])}@example.com" + }) + |> Ash.create(actor: admin) + + member + end + + defp create_custom_field(actor) do + {:ok, field} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "test_field_#{System.unique_integer([:positive])}", + value_type: :string + }) + |> Ash.create(actor: actor) + + field + end + + defp create_custom_field_value(member_id, custom_field_id, value, actor) do + {:ok, cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: member_id, + custom_field_id: custom_field_id, + value: %{"_union_type" => "string", "_union_value" => value} + }) + |> Ash.create(actor: actor, domain: Mv.Membership) + + cfv + end + + describe "own_data permission set (Mitglied)" do + setup %{actor: actor} do + user = create_user_with_permission_set("own_data", actor) + linked_member = create_linked_member_for_user(user, actor) + unlinked_member = create_unlinked_member(actor) + custom_field = create_custom_field(actor) + + cfv_linked = create_custom_field_value(linked_member.id, custom_field.id, "linked", actor) + + cfv_unlinked = + create_custom_field_value(unlinked_member.id, custom_field.id, "unlinked", actor) + + {:ok, user} = + Ash.get(Accounts.User, user.id, domain: Mv.Accounts, load: [:role], actor: actor) + + {:ok, user} = Ash.load(user, :member, domain: Mv.Accounts, actor: actor) + + %{ + user: user, + linked_member: linked_member, + unlinked_member: unlinked_member, + custom_field: custom_field, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked, + actor: actor + } + end + + test "can read custom field values of linked member", %{user: user, cfv_linked: cfv_linked} do + {:ok, cfv} = + Ash.get(CustomFieldValue, cfv_linked.id, actor: user, domain: Mv.Membership) + + assert cfv.id == cfv_linked.id + end + + test "can list custom field values returns only linked member's values", %{ + user: user, + cfv_linked: cfv_linked + } do + {:ok, values} = Ash.read(CustomFieldValue, actor: user, domain: Mv.Membership) + + assert length(values) == 1 + assert hd(values).id == cfv_linked.id + end + + test "can update custom field value of linked member", %{user: user, cfv_linked: cfv_linked} do + {:ok, updated} = + cfv_linked + |> Ash.Changeset.for_update(:update, %{ + value: %{"_union_type" => "string", "_union_value" => "updated"} + }) + |> Ash.update(actor: user, domain: Mv.Membership) + + assert %Ash.Union{value: "updated", type: :string} = updated.value + end + + test "can create custom field value for linked member", %{ + user: user, + linked_member: linked_member, + actor: actor + } do + # Create a second custom field via admin (own_data cannot create CustomField) + custom_field2 = create_custom_field(actor) + + {:ok, cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: linked_member.id, + custom_field_id: custom_field2.id, + value: %{"_union_type" => "string", "_union_value" => "new"} + }) + |> Ash.create(actor: user, domain: Mv.Membership) + + assert cfv.member_id == linked_member.id + assert cfv.custom_field_id == custom_field2.id + end + + test "can destroy custom field value of linked member", %{user: user, cfv_linked: cfv_linked} do + result = Ash.destroy(cfv_linked, actor: user, domain: Mv.Membership) + assert :ok = result + + assert {:error, _} = Ash.get(CustomFieldValue, cfv_linked.id, domain: Mv.Membership) + end + + test "cannot read custom field values of unlinked member", %{ + user: user, + cfv_unlinked: cfv_unlinked + } do + assert_raise Ash.Error.Invalid, fn -> + Ash.get!(CustomFieldValue, cfv_unlinked.id, actor: user, domain: Mv.Membership) + end + end + + test "cannot update custom field value of unlinked member", %{ + user: user, + cfv_unlinked: cfv_unlinked + } do + assert_raise Ash.Error.Forbidden, fn -> + cfv_unlinked + |> Ash.Changeset.for_update(:update, %{ + value: %{"_union_type" => "string", "_union_value" => "hacked"} + }) + |> Ash.update!(actor: user, domain: Mv.Membership) + end + end + + test "cannot create custom field value for unlinked member", %{ + user: user, + unlinked_member: unlinked_member, + custom_field: custom_field + } do + assert_raise Ash.Error.Forbidden, fn -> + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: unlinked_member.id, + custom_field_id: custom_field.id, + value: %{"_union_type" => "string", "_union_value" => "forbidden"} + }) + |> Ash.create!(actor: user, domain: Mv.Membership) + end + end + + test "cannot destroy custom field value of unlinked member", %{ + user: user, + cfv_unlinked: cfv_unlinked + } do + assert_raise Ash.Error.Forbidden, fn -> + Ash.destroy!(cfv_unlinked, actor: user, domain: Mv.Membership) + end + end + end + + describe "read_only permission set (Vorstand/Buchhaltung)" do + setup %{actor: actor} do + user = create_user_with_permission_set("read_only", actor) + linked_member = create_linked_member_for_user(user, actor) + unlinked_member = create_unlinked_member(actor) + custom_field = create_custom_field(actor) + + cfv_linked = create_custom_field_value(linked_member.id, custom_field.id, "linked", actor) + + cfv_unlinked = + create_custom_field_value(unlinked_member.id, custom_field.id, "unlinked", actor) + + {:ok, user} = + Ash.get(Accounts.User, user.id, domain: Mv.Accounts, load: [:role], actor: actor) + + %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked, + custom_field: custom_field, + linked_member: linked_member, + unlinked_member: unlinked_member + } + end + + test "can read all custom field values", %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked + } do + {:ok, values} = Ash.read(CustomFieldValue, actor: user, domain: Mv.Membership) + + ids = Enum.map(values, & &1.id) + assert cfv_linked.id in ids + assert cfv_unlinked.id in ids + end + + test "can read individual custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + {:ok, cfv} = + Ash.get(CustomFieldValue, cfv_unlinked.id, actor: user, domain: Mv.Membership) + + assert cfv.id == cfv_unlinked.id + end + + test "cannot create custom field value (returns forbidden)", %{ + user: user, + linked_member: linked_member, + custom_field: custom_field + } do + assert_raise Ash.Error.Forbidden, fn -> + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: linked_member.id, + custom_field_id: custom_field.id, + value: %{"_union_type" => "string", "_union_value" => "forbidden"} + }) + |> Ash.create!(actor: user, domain: Mv.Membership) + end + end + + test "cannot update custom field value (returns forbidden)", %{ + user: user, + cfv_linked: cfv_linked + } do + assert_raise Ash.Error.Forbidden, fn -> + cfv_linked + |> Ash.Changeset.for_update(:update, %{ + value: %{"_union_type" => "string", "_union_value" => "hacked"} + }) + |> Ash.update!(actor: user, domain: Mv.Membership) + end + end + + test "cannot destroy custom field value (returns forbidden)", %{ + user: user, + cfv_linked: cfv_linked + } do + assert_raise Ash.Error.Forbidden, fn -> + Ash.destroy!(cfv_linked, actor: user, domain: Mv.Membership) + end + end + end + + describe "normal_user permission set (Kassenwart)" do + setup %{actor: actor} do + user = create_user_with_permission_set("normal_user", actor) + linked_member = create_linked_member_for_user(user, actor) + unlinked_member = create_unlinked_member(actor) + custom_field = create_custom_field(actor) + + cfv_linked = create_custom_field_value(linked_member.id, custom_field.id, "linked", actor) + + cfv_unlinked = + create_custom_field_value(unlinked_member.id, custom_field.id, "unlinked", actor) + + {:ok, user} = + Ash.get(Accounts.User, user.id, domain: Mv.Accounts, load: [:role], actor: actor) + + %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked, + custom_field: custom_field, + linked_member: linked_member, + unlinked_member: unlinked_member, + actor: actor + } + end + + test "can read all custom field values", %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked + } do + {:ok, values} = Ash.read(CustomFieldValue, actor: user, domain: Mv.Membership) + + ids = Enum.map(values, & &1.id) + assert cfv_linked.id in ids + assert cfv_unlinked.id in ids + end + + test "can create custom field value", %{ + user: user, + unlinked_member: unlinked_member, + actor: actor + } do + # normal_user cannot create CustomField; use actor (admin) to create it + custom_field = create_custom_field(actor) + + {:ok, cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: unlinked_member.id, + custom_field_id: custom_field.id, + value: %{"_union_type" => "string", "_union_value" => "new"} + }) + |> Ash.create(actor: user, domain: Mv.Membership) + + assert cfv.member_id == unlinked_member.id + end + + test "can update any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + {:ok, updated} = + cfv_unlinked + |> Ash.Changeset.for_update(:update, %{ + value: %{"_union_type" => "string", "_union_value" => "updated"} + }) + |> Ash.update(actor: user, domain: Mv.Membership) + + assert %Ash.Union{value: "updated", type: :string} = updated.value + end + + test "can destroy any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + :ok = Ash.destroy(cfv_unlinked, actor: user, domain: Mv.Membership) + + assert {:error, _} = Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership) + end + end + + describe "admin permission set" do + setup %{actor: actor} do + user = create_user_with_permission_set("admin", actor) + linked_member = create_linked_member_for_user(user, actor) + unlinked_member = create_unlinked_member(actor) + custom_field = create_custom_field(actor) + + cfv_linked = create_custom_field_value(linked_member.id, custom_field.id, "linked", actor) + + cfv_unlinked = + create_custom_field_value(unlinked_member.id, custom_field.id, "unlinked", actor) + + {:ok, user} = + Ash.get(Accounts.User, user.id, domain: Mv.Accounts, load: [:role], actor: actor) + + %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked, + custom_field: custom_field, + linked_member: linked_member, + unlinked_member: unlinked_member + } + end + + test "can read all custom field values", %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked + } do + {:ok, values} = Ash.read(CustomFieldValue, actor: user, domain: Mv.Membership) + + ids = Enum.map(values, & &1.id) + assert cfv_linked.id in ids + assert cfv_unlinked.id in ids + end + + test "can create custom field value", %{user: user, unlinked_member: unlinked_member} do + custom_field = create_custom_field(user) + + {:ok, cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: unlinked_member.id, + custom_field_id: custom_field.id, + value: %{"_union_type" => "string", "_union_value" => "new"} + }) + |> Ash.create(actor: user, domain: Mv.Membership) + + assert cfv.member_id == unlinked_member.id + end + + test "can update any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + {:ok, updated} = + cfv_unlinked + |> Ash.Changeset.for_update(:update, %{ + value: %{"_union_type" => "string", "_union_value" => "updated"} + }) + |> Ash.update(actor: user, domain: Mv.Membership) + + assert %Ash.Union{value: "updated", type: :string} = updated.value + end + + test "can destroy any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + :ok = Ash.destroy(cfv_unlinked, actor: user, domain: Mv.Membership) + + assert {:error, _} = Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership) + end + end +end From 7bd4b4154696e83e76b665a5846f374bb167204b Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:38 +0100 Subject: [PATCH 07/25] Document CustomFieldValue policies and own_data create/destroy in architecture Update roles-and-permissions-architecture.md with policy layout and permission matrix for CustomFieldValue (linked). --- docs/roles-and-permissions-architecture.md | 44 +++++++++++----------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/docs/roles-and-permissions-architecture.md b/docs/roles-and-permissions-architecture.md index 8934688..acea99e 100644 --- a/docs/roles-and-permissions-architecture.md +++ b/docs/roles-and-permissions-architecture.md @@ -501,9 +501,11 @@ defmodule Mv.Authorization.PermissionSets do %{resource: "Member", action: :read, scope: :linked, granted: true}, %{resource: "Member", action: :update, scope: :linked, granted: true}, - # CustomFieldValue: Can read/update custom field values of linked member + # 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} @@ -678,7 +680,7 @@ Quick reference table showing what each permission set allows: | **User** (all) | - | - | - | R, C, U, D | | **Member** (linked) | R, U | - | - | - | | **Member** (all) | - | R | R, C, U | R, C, U, D | -| **CustomFieldValue** (linked) | R, U | - | - | - | +| **CustomFieldValue** (linked) | R, U, C, D | - | - | - | | **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 | @@ -1053,35 +1055,33 @@ end ### CustomFieldValue Resource Policies -**Location:** `lib/mv/membership/custom_field_value.ex` +**Location:** `lib/membership/custom_field_value.ex` -**Special Case:** Users can access custom field values of their linked member. +**Pattern:** Bypass for READ (list queries), CustomFieldValueCreateScope for create (no filter), HasPermission for read/update/destroy. Create uses a dedicated check because Ash cannot apply filters to create actions. ```elixir defmodule Mv.Membership.CustomFieldValue do use Ash.Resource, ... policies do - # SPECIAL CASE: Users can access custom field values of their linked member - # Note: This uses member_id relationship (CustomFieldValue.member_id → Member.id → User.member_id) - policy action_type([:read, :update]) do - description "Users can access custom field values of their linked member" + # Bypass for READ (list queries; expr triggers auto_filter) + bypass action_type(:read) do + description "Users can read custom field values of their linked member" authorize_if expr(member_id == ^actor(:member_id)) end - # GENERAL: Check permissions from role - policy action_type([:read, :create, :update, :destroy]) do - description "Check permissions from user's role" - authorize_if Mv.Authorization.Checks.HasPermission + # CREATE: CustomFieldValueCreateScope (no filter; Ash rejects filters on create) + # own_data -> create when member_id == actor.member_id; normal_user/admin -> create (scope :all) + policy action_type(:create) do + authorize_if Mv.Authorization.Checks.CustomFieldValueCreateScope end - # DEFAULT: Forbid - policy action_type([:read, :create, :update, :destroy]) do - forbid_if always() + # READ/UPDATE/DESTROY: HasPermission (scope :linked / :all) + policy action_type([:read, :update, :destroy]) do + authorize_if Mv.Authorization.Checks.HasPermission end + # DEFAULT: Ash implicitly forbids if no policy authorized (fail-closed) end - - # ... end ``` @@ -1089,11 +1089,13 @@ end | Action | Mitglied | Vorstand | Kassenwart | Buchhaltung | Admin | |--------|----------|----------|------------|-------------|-------| -| Read linked | ✅ (special) | ✅ (if linked) | ✅ | ✅ (if linked) | ✅ | -| Update linked | ✅ (special) | ❌ | ✅ | ❌ | ✅ | +| Read linked | ✅ (bypass) | ✅ (if linked) | ✅ | ✅ (if linked) | ✅ | +| Update linked | ✅ (scope :linked) | ❌ | ✅ | ❌ | ✅ | +| Create linked | ✅ (CustomFieldValueCreateScope) | ❌ | ✅ | ❌ | ✅ | +| Destroy linked | ✅ (scope :linked) | ❌ | ✅ | ❌ | ✅ | | Read all | ❌ | ✅ | ✅ | ✅ | ✅ | -| Create | ❌ | ❌ | ✅ | ❌ | ✅ | -| Destroy | ❌ | ❌ | ✅ | ❌ | ✅ | +| Create all | ❌ | ❌ | ✅ | ❌ | ✅ | +| Destroy all | ❌ | ❌ | ✅ | ❌ | ✅ | ### CustomField Resource Policies From ba8c12f3ea1e65776e0362c95c6f08d9aee60c04 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:41:33 +0100 Subject: [PATCH 08/25] chore: remove start-database from test action --- Justfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Justfile b/Justfile index c68c473..f25041c 100644 --- a/Justfile +++ b/Justfile @@ -41,7 +41,7 @@ audit: mix deps.audit mix hex.audit -test *args: install-dependencies start-database +test *args: install-dependencies mix test {{args}} format: From 5d82769d2de69eedaa521384e15363a1ae1148e5 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 15:44:38 +0100 Subject: [PATCH 09/25] CustomFieldValueCreateScope: use get_argument_or_attribute for member_id - Read member_id via Ash.Changeset.get_argument_or_attribute/2 so it works when set as attribute or argument - Remove unused require Logger - Document member_id source in moduledoc --- .../checks/custom_field_value_create_scope.ex | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/mv/authorization/checks/custom_field_value_create_scope.ex b/lib/mv/authorization/checks/custom_field_value_create_scope.ex index f5be53d..0b24e74 100644 --- a/lib/mv/authorization/checks/custom_field_value_create_scope.ex +++ b/lib/mv/authorization/checks/custom_field_value_create_scope.ex @@ -8,6 +8,14 @@ defmodule Mv.Authorization.Checks.CustomFieldValueCreateScope do (PermissionSets + :linked/:all) but only implements strict_check, so it never adds a filter. + ## member_id source + + The check reads `member_id` from the create changeset via + `Ash.Changeset.get_argument_or_attribute/2`, so it works when member_id + is set as an attribute or as an action argument. The CustomFieldValue + resource's default create action must accept and require `member_id` + (e.g. via `default_accept [:value, :member_id, :custom_field_id]`). + Used in CustomFieldValue policies: policy action_type(:create) do authorize_if Mv.Authorization.Checks.CustomFieldValueCreateScope @@ -15,7 +23,6 @@ defmodule Mv.Authorization.Checks.CustomFieldValueCreateScope do """ use Ash.Policy.Check alias Mv.Authorization.PermissionSets - require Logger @impl true def describe(_opts), @@ -53,8 +60,8 @@ defmodule Mv.Authorization.Checks.CustomFieldValueCreateScope do defp get_create_member_id(authorizer) do changeset = authorizer.changeset || authorizer.subject - if changeset && function_exported?(Ash.Changeset, :get_attribute, 2) do - Ash.Changeset.get_attribute(changeset, :member_id) + if changeset && function_exported?(Ash.Changeset, :get_argument_or_attribute, 2) do + Ash.Changeset.get_argument_or_attribute(changeset, :member_id) else nil end From 1fb65ace8db5717a7774a4c74306dd3450cd7607 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 15:44:39 +0100 Subject: [PATCH 10/25] CustomFieldValue: remove unused require Ash.Query --- lib/membership/custom_field_value.ex | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/membership/custom_field_value.ex b/lib/membership/custom_field_value.ex index d6d91a6..623455d 100644 --- a/lib/membership/custom_field_value.ex +++ b/lib/membership/custom_field_value.ex @@ -42,7 +42,6 @@ defmodule Mv.Membership.CustomFieldValue do data_layer: AshPostgres.DataLayer, authorizers: [Ash.Policy.Authorizer] - require Ash.Query import Ash.Expr postgres do From 185ccb02175e8b8664f05b0c1525409d39c75035 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 15:44:40 +0100 Subject: [PATCH 11/25] HasPermission: remove unused _authorizer from strict_check helper --- lib/mv/authorization/checks/has_permission.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mv/authorization/checks/has_permission.ex b/lib/mv/authorization/checks/has_permission.ex index 1cf1e39..774e767 100644 --- a/lib/mv/authorization/checks/has_permission.ex +++ b/lib/mv/authorization/checks/has_permission.ex @@ -110,12 +110,12 @@ defmodule Mv.Authorization.Checks.HasPermission do {:ok, false} true -> - strict_check_with_permissions(actor, resource, action, record, authorizer) + strict_check_with_permissions(actor, resource, action, record) end end # Helper function to reduce nesting depth - defp strict_check_with_permissions(actor, resource, action, record, _authorizer) do + defp strict_check_with_permissions(actor, resource, action, record) do # Ensure role is loaded (fallback if on_mount didn't run) actor = ensure_role_loaded(actor) From 6e01af10f5ee69e94a058fc305dfeffe75100ca5 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 15:44:43 +0100 Subject: [PATCH 12/25] CFV policies test: system_actor for setup, verify destroy with actor - create_linked_member_for_user and create_unlinked_member use actor (system_actor) directly instead of creating admin user per call - Remove create_admin_user helper - After destroy, verify with Ash.get(..., actor: actor) to avoid false positive from Forbidden vs NotFound --- .../custom_field_value_policies_test.exs | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/test/mv/membership/custom_field_value_policies_test.exs b/test/mv/membership/custom_field_value_policies_test.exs index c7a80db..d400aec 100644 --- a/test/mv/membership/custom_field_value_policies_test.exs +++ b/test/mv/membership/custom_field_value_policies_test.exs @@ -60,13 +60,7 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do user_with_role end - defp create_admin_user(actor) do - create_user_with_permission_set("admin", actor) - end - defp create_linked_member_for_user(user, actor) do - admin = create_admin_user(actor) - {:ok, member} = Member |> Ash.Changeset.for_create(:create_member, %{ @@ -74,19 +68,17 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do last_name: "Member", email: "linked#{System.unique_integer([:positive])}@example.com" }) - |> Ash.create(actor: admin, return_notifications?: false) + |> Ash.create(actor: actor, return_notifications?: false) user |> Ash.Changeset.for_update(:update, %{}) |> Ash.Changeset.force_change_attribute(:member_id, member.id) - |> Ash.update(actor: admin, domain: Mv.Accounts, return_notifications?: false) + |> Ash.update(actor: actor, domain: Mv.Accounts, return_notifications?: false) member end defp create_unlinked_member(actor) do - admin = create_admin_user(actor) - {:ok, member} = Member |> Ash.Changeset.for_create(:create_member, %{ @@ -94,7 +86,7 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do last_name: "Member", email: "unlinked#{System.unique_integer([:positive])}@example.com" }) - |> Ash.create(actor: admin) + |> Ash.create(actor: actor) member end @@ -201,11 +193,16 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do assert cfv.custom_field_id == custom_field2.id end - test "can destroy custom field value of linked member", %{user: user, cfv_linked: cfv_linked} do + test "can destroy custom field value of linked member", %{ + user: user, + cfv_linked: cfv_linked, + actor: actor + } do result = Ash.destroy(cfv_linked, actor: user, domain: Mv.Membership) assert :ok = result - assert {:error, _} = Ash.get(CustomFieldValue, cfv_linked.id, domain: Mv.Membership) + assert {:error, _} = + Ash.get(CustomFieldValue, cfv_linked.id, domain: Mv.Membership, actor: actor) end test "cannot read custom field values of unlinked member", %{ @@ -408,10 +405,15 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do assert %Ash.Union{value: "updated", type: :string} = updated.value end - test "can destroy any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + test "can destroy any custom field value", %{ + user: user, + cfv_unlinked: cfv_unlinked, + actor: actor + } do :ok = Ash.destroy(cfv_unlinked, actor: user, domain: Mv.Membership) - assert {:error, _} = Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership) + assert {:error, _} = + Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership, actor: actor) end end @@ -478,10 +480,15 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do assert %Ash.Union{value: "updated", type: :string} = updated.value end - test "can destroy any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + test "can destroy any custom field value", %{ + user: user, + cfv_unlinked: cfv_unlinked, + actor: actor + } do :ok = Ash.destroy(cfv_unlinked, actor: user, domain: Mv.Membership) - assert {:error, _} = Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership) + assert {:error, _} = + Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership, actor: actor) end end end From c48feb2128b6a3e5d86ab2fcb097d6a62e3016bf Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 15:44:44 +0100 Subject: [PATCH 13/25] Docs: document bypass read rule for CustomFieldValue pattern - Bypass action_type(:read) is production-side rule: reading own CFVs always allowed, overrides Permission-Sets. Applies to get/list/load. --- docs/roles-and-permissions-architecture.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/roles-and-permissions-architecture.md b/docs/roles-and-permissions-architecture.md index acea99e..063de32 100644 --- a/docs/roles-and-permissions-architecture.md +++ b/docs/roles-and-permissions-architecture.md @@ -1059,6 +1059,8 @@ end **Pattern:** Bypass for READ (list queries), CustomFieldValueCreateScope for create (no filter), HasPermission for read/update/destroy. Create uses a dedicated check because Ash cannot apply filters to create actions. +The bypass `action_type(:read)` is a production-side rule: reading own CFVs (where `member_id == actor.member_id`) is always allowed and overrides Permission-Sets; no further policies are needed for that. It applies to all read actions (get, list, load). + ```elixir defmodule Mv.Membership.CustomFieldValue do use Ash.Resource, ... From 8f5f69744c939923d172878e09075a1138622d10 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:12 +0100 Subject: [PATCH 14/25] Add CustomFieldValue create/destroy :linked to own_data permission set Allows members to create and delete custom field values for their linked member. --- lib/mv/authorization/permission_sets.ex | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/mv/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex index 22132cb..e133ed7 100644 --- a/lib/mv/authorization/permission_sets.ex +++ b/lib/mv/authorization/permission_sets.ex @@ -105,9 +105,11 @@ defmodule Mv.Authorization.PermissionSets do %{resource: "Member", action: :read, scope: :linked, granted: true}, %{resource: "Member", action: :update, scope: :linked, granted: true}, - # CustomFieldValue: Can read/update custom field values of linked member + # 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} From c7c6b318acef273edb2110070bf805244ec21dd0 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:17 +0100 Subject: [PATCH 15/25] Add CustomFieldValueCreateScope check for create actions Ash cannot apply filters to create; this check enforces :linked/:all scope via strict_check only (no filter). --- .../checks/custom_field_value_create_scope.ex | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 lib/mv/authorization/checks/custom_field_value_create_scope.ex diff --git a/lib/mv/authorization/checks/custom_field_value_create_scope.ex b/lib/mv/authorization/checks/custom_field_value_create_scope.ex new file mode 100644 index 0000000..f5be53d --- /dev/null +++ b/lib/mv/authorization/checks/custom_field_value_create_scope.ex @@ -0,0 +1,64 @@ +defmodule Mv.Authorization.Checks.CustomFieldValueCreateScope do + @moduledoc """ + Policy check for CustomFieldValue create actions only. + + Use this for create instead of HasPermission because Ash cannot apply + filters to create actions ("Cannot use a filter to authorize a create"). + This check performs the same scope logic as HasPermission for create + (PermissionSets + :linked/:all) but only implements strict_check, so it + never adds a filter. + + Used in CustomFieldValue policies: + policy action_type(:create) do + authorize_if Mv.Authorization.Checks.CustomFieldValueCreateScope + end + """ + use Ash.Policy.Check + alias Mv.Authorization.PermissionSets + require Logger + + @impl true + def describe(_opts), + do: "CustomFieldValue create allowed by permission set scope (:linked or :all)" + + @impl true + def strict_check(actor, authorizer, _opts) do + actor = ensure_role_loaded(actor) + + with %{role: %{permission_set_name: ps_name}} when not is_nil(ps_name) <- actor, + {:ok, ps_atom} <- PermissionSets.permission_set_name_to_atom(ps_name), + permissions <- PermissionSets.get_permissions(ps_atom), + perm <- find_custom_field_value_create(permissions.resources) do + case perm do + nil -> {:ok, false} + %{scope: :all} -> {:ok, true} + %{scope: :linked} -> {:ok, member_id_matches?(authorizer, actor)} + end + else + _ -> {:ok, false} + end + end + + defp find_custom_field_value_create(resources) do + Enum.find(resources, fn p -> + p.resource == "CustomFieldValue" and p.action == :create and p.granted + end) + end + + defp member_id_matches?(authorizer, actor) do + member_id = get_create_member_id(authorizer) + !is_nil(member_id) and member_id == actor.member_id + end + + defp get_create_member_id(authorizer) do + changeset = authorizer.changeset || authorizer.subject + + if changeset && function_exported?(Ash.Changeset, :get_attribute, 2) do + Ash.Changeset.get_attribute(changeset, :member_id) + else + nil + end + end + + defp ensure_role_loaded(actor), do: Mv.Authorization.Actor.ensure_loaded(actor) +end From bf2d0352c14acc27cae32d31b648a15d9964c89c Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:22 +0100 Subject: [PATCH 16/25] Add authorization policies to CustomFieldValue resource - Authorizer and policies: bypass for read (member_id == actor.member_id), CustomFieldValueCreateScope for create, HasPermission for read/update/destroy. - HasPermission: pass authorizer into strict_check helper; document that create must use a dedicated check (no filter). --- lib/membership/custom_field_value.ex | 36 ++++++++++++++++++- lib/mv/authorization/checks/has_permission.ex | 7 ++-- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/lib/membership/custom_field_value.ex b/lib/membership/custom_field_value.ex index 232ba99..d6d91a6 100644 --- a/lib/membership/custom_field_value.ex +++ b/lib/membership/custom_field_value.ex @@ -39,7 +39,11 @@ defmodule Mv.Membership.CustomFieldValue do """ use Ash.Resource, domain: Mv.Membership, - data_layer: AshPostgres.DataLayer + data_layer: AshPostgres.DataLayer, + authorizers: [Ash.Policy.Authorizer] + + require Ash.Query + import Ash.Expr postgres do table "custom_field_values" @@ -62,6 +66,36 @@ defmodule Mv.Membership.CustomFieldValue do end end + # Authorization Policies + # Order matters: Most specific policies first, then general permission check + # Pattern aligns with User and Member resources (bypass for READ, HasPermission for update/destroy) + # Create uses CustomFieldValueCreateScope because Ash cannot apply filters to create actions. + policies do + # SPECIAL CASE: Users can READ custom field values of their linked member + # Bypass needed for list queries (expr triggers auto_filter in Ash) + bypass action_type(:read) do + description "Users can read custom field values of their linked member" + authorize_if expr(member_id == ^actor(:member_id)) + end + + # CREATE: CustomFieldValueCreateScope (no filter; Ash rejects filters on create) + # - :own_data -> create allowed when member_id == actor.member_id (scope :linked) + # - :read_only -> no create permission + # - :normal_user / :admin -> create allowed (scope :all) + policy action_type(:create) do + description "CustomFieldValue create allowed by permission set scope" + authorize_if Mv.Authorization.Checks.CustomFieldValueCreateScope + end + + # READ/UPDATE/DESTROY: HasPermission (scope :linked / :all) + policy action_type([:read, :update, :destroy]) do + description "Check permissions from user's role and permission set" + authorize_if Mv.Authorization.Checks.HasPermission + end + + # DEFAULT: Ash implicitly forbids if no policy authorized (fail-closed) + end + attributes do uuid_primary_key :id diff --git a/lib/mv/authorization/checks/has_permission.ex b/lib/mv/authorization/checks/has_permission.ex index 1a478b8..1cf1e39 100644 --- a/lib/mv/authorization/checks/has_permission.ex +++ b/lib/mv/authorization/checks/has_permission.ex @@ -110,12 +110,12 @@ defmodule Mv.Authorization.Checks.HasPermission do {:ok, false} true -> - strict_check_with_permissions(actor, resource, action, record) + strict_check_with_permissions(actor, resource, action, record, authorizer) end end # Helper function to reduce nesting depth - defp strict_check_with_permissions(actor, resource, action, record) do + defp strict_check_with_permissions(actor, resource, action, record, _authorizer) do # Ensure role is loaded (fallback if on_mount didn't run) actor = ensure_role_loaded(actor) @@ -148,6 +148,7 @@ defmodule Mv.Authorization.Checks.HasPermission do else # No record yet (e.g., read/list queries) - deny at strict_check level # Resources must use expr-based bypass policies for list filtering + # Create: use a dedicated check that does not return a filter (e.g. CustomFieldValueCreateScope) {:ok, false} end @@ -213,7 +214,7 @@ defmodule Mv.Authorization.Checks.HasPermission do {:filter, filter_expr} -> # :linked or :own scope - apply filter - # filter_expr is a keyword list from expr(...), return it directly + # Create actions must not use HasPermission (use a dedicated check, e.g. CustomFieldValueCreateScope) filter_expr false -> From 17831a09482f4193415d5db6ae8e6c09eb38751d Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:29 +0100 Subject: [PATCH 17/25] Pass actor to CustomFieldValue destroy and load in existing tests Required after CustomFieldValue gained authorization policies. --- test/membership/member_search_with_custom_fields_test.exs | 2 +- test/mv/membership/import/member_csv_test.exs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/membership/member_search_with_custom_fields_test.exs b/test/membership/member_search_with_custom_fields_test.exs index bd28ce5..284fcff 100644 --- a/test/membership/member_search_with_custom_fields_test.exs +++ b/test/membership/member_search_with_custom_fields_test.exs @@ -348,7 +348,7 @@ defmodule Mv.Membership.MemberSearchWithCustomFieldsTest do assert List.first(results).id == member1.id # Delete custom field value - assert :ok = Ash.destroy(cfv) + assert :ok = Ash.destroy(cfv, actor: system_actor) # Value should no longer be found deleted_results = diff --git a/test/mv/membership/import/member_csv_test.exs b/test/mv/membership/import/member_csv_test.exs index 778e82b..0304989 100644 --- a/test/mv/membership/import/member_csv_test.exs +++ b/test/mv/membership/import/member_csv_test.exs @@ -247,7 +247,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do member = Enum.find(members, &(&1.email == "withcustom@example.com")) assert member != nil - {:ok, member_with_cf} = Ash.load(member, :custom_field_values) + {:ok, member_with_cf} = Ash.load(member, :custom_field_values, actor: actor) assert length(member_with_cf.custom_field_values) == 1 cfv = List.first(member_with_cf.custom_field_values) assert cfv.custom_field_id == custom_field.id From 4e032ea778ac60f701fa79ee353b4ce5fc994ad8 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:34 +0100 Subject: [PATCH 18/25] Add CustomFieldValue policy tests (own_data, read_only, normal_user, admin) Covers read/update/create/destroy for linked vs unlinked members and CRUD permissions per permission set. --- .../custom_field_value_policies_test.exs | 487 ++++++++++++++++++ 1 file changed, 487 insertions(+) create mode 100644 test/mv/membership/custom_field_value_policies_test.exs diff --git a/test/mv/membership/custom_field_value_policies_test.exs b/test/mv/membership/custom_field_value_policies_test.exs new file mode 100644 index 0000000..c7a80db --- /dev/null +++ b/test/mv/membership/custom_field_value_policies_test.exs @@ -0,0 +1,487 @@ +defmodule Mv.Membership.CustomFieldValuePoliciesTest do + @moduledoc """ + Tests for CustomFieldValue resource authorization policies. + + Tests all 4 permission sets (own_data, read_only, normal_user, admin) + and verifies that policies correctly enforce access control based on + user roles and permission sets. + """ + # async: false because we need database commits to be visible across queries + use Mv.DataCase, async: false + + alias Mv.Membership.{CustomField, CustomFieldValue, Member} + alias Mv.Accounts + alias Mv.Authorization + + require Ash.Query + + setup do + system_actor = Mv.Helpers.SystemActor.get_system_actor() + %{actor: system_actor} + end + + # Helper to create a role with a specific permission set + defp create_role_with_permission_set(permission_set_name, actor) do + role_name = "Test Role #{permission_set_name} #{System.unique_integer([:positive])}" + + case Authorization.create_role( + %{ + name: role_name, + description: "Test role for #{permission_set_name}", + permission_set_name: permission_set_name + }, + actor: actor + ) do + {:ok, role} -> role + {:error, error} -> raise "Failed to create role: #{inspect(error)}" + end + end + + # Helper to create a user with a specific permission set + # Returns user with role preloaded (required for authorization) + defp create_user_with_permission_set(permission_set_name, actor) do + role = create_role_with_permission_set(permission_set_name, actor) + + {:ok, user} = + Accounts.User + |> Ash.Changeset.for_create(:register_with_password, %{ + email: "user#{System.unique_integer([:positive])}@example.com", + password: "testpassword123" + }) + |> Ash.create(actor: actor) + + {:ok, user} = + user + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.manage_relationship(:role, role, type: :append_and_remove) + |> Ash.update(actor: actor) + + {:ok, user_with_role} = Ash.load(user, :role, domain: Mv.Accounts, actor: actor) + user_with_role + end + + defp create_admin_user(actor) do + create_user_with_permission_set("admin", actor) + end + + defp create_linked_member_for_user(user, actor) do + admin = create_admin_user(actor) + + {:ok, member} = + Member + |> Ash.Changeset.for_create(:create_member, %{ + first_name: "Linked", + last_name: "Member", + email: "linked#{System.unique_integer([:positive])}@example.com" + }) + |> Ash.create(actor: admin, return_notifications?: false) + + user + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.force_change_attribute(:member_id, member.id) + |> Ash.update(actor: admin, domain: Mv.Accounts, return_notifications?: false) + + member + end + + defp create_unlinked_member(actor) do + admin = create_admin_user(actor) + + {:ok, member} = + Member + |> Ash.Changeset.for_create(:create_member, %{ + first_name: "Unlinked", + last_name: "Member", + email: "unlinked#{System.unique_integer([:positive])}@example.com" + }) + |> Ash.create(actor: admin) + + member + end + + defp create_custom_field(actor) do + {:ok, field} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "test_field_#{System.unique_integer([:positive])}", + value_type: :string + }) + |> Ash.create(actor: actor) + + field + end + + defp create_custom_field_value(member_id, custom_field_id, value, actor) do + {:ok, cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: member_id, + custom_field_id: custom_field_id, + value: %{"_union_type" => "string", "_union_value" => value} + }) + |> Ash.create(actor: actor, domain: Mv.Membership) + + cfv + end + + describe "own_data permission set (Mitglied)" do + setup %{actor: actor} do + user = create_user_with_permission_set("own_data", actor) + linked_member = create_linked_member_for_user(user, actor) + unlinked_member = create_unlinked_member(actor) + custom_field = create_custom_field(actor) + + cfv_linked = create_custom_field_value(linked_member.id, custom_field.id, "linked", actor) + + cfv_unlinked = + create_custom_field_value(unlinked_member.id, custom_field.id, "unlinked", actor) + + {:ok, user} = + Ash.get(Accounts.User, user.id, domain: Mv.Accounts, load: [:role], actor: actor) + + {:ok, user} = Ash.load(user, :member, domain: Mv.Accounts, actor: actor) + + %{ + user: user, + linked_member: linked_member, + unlinked_member: unlinked_member, + custom_field: custom_field, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked, + actor: actor + } + end + + test "can read custom field values of linked member", %{user: user, cfv_linked: cfv_linked} do + {:ok, cfv} = + Ash.get(CustomFieldValue, cfv_linked.id, actor: user, domain: Mv.Membership) + + assert cfv.id == cfv_linked.id + end + + test "can list custom field values returns only linked member's values", %{ + user: user, + cfv_linked: cfv_linked + } do + {:ok, values} = Ash.read(CustomFieldValue, actor: user, domain: Mv.Membership) + + assert length(values) == 1 + assert hd(values).id == cfv_linked.id + end + + test "can update custom field value of linked member", %{user: user, cfv_linked: cfv_linked} do + {:ok, updated} = + cfv_linked + |> Ash.Changeset.for_update(:update, %{ + value: %{"_union_type" => "string", "_union_value" => "updated"} + }) + |> Ash.update(actor: user, domain: Mv.Membership) + + assert %Ash.Union{value: "updated", type: :string} = updated.value + end + + test "can create custom field value for linked member", %{ + user: user, + linked_member: linked_member, + actor: actor + } do + # Create a second custom field via admin (own_data cannot create CustomField) + custom_field2 = create_custom_field(actor) + + {:ok, cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: linked_member.id, + custom_field_id: custom_field2.id, + value: %{"_union_type" => "string", "_union_value" => "new"} + }) + |> Ash.create(actor: user, domain: Mv.Membership) + + assert cfv.member_id == linked_member.id + assert cfv.custom_field_id == custom_field2.id + end + + test "can destroy custom field value of linked member", %{user: user, cfv_linked: cfv_linked} do + result = Ash.destroy(cfv_linked, actor: user, domain: Mv.Membership) + assert :ok = result + + assert {:error, _} = Ash.get(CustomFieldValue, cfv_linked.id, domain: Mv.Membership) + end + + test "cannot read custom field values of unlinked member", %{ + user: user, + cfv_unlinked: cfv_unlinked + } do + assert_raise Ash.Error.Invalid, fn -> + Ash.get!(CustomFieldValue, cfv_unlinked.id, actor: user, domain: Mv.Membership) + end + end + + test "cannot update custom field value of unlinked member", %{ + user: user, + cfv_unlinked: cfv_unlinked + } do + assert_raise Ash.Error.Forbidden, fn -> + cfv_unlinked + |> Ash.Changeset.for_update(:update, %{ + value: %{"_union_type" => "string", "_union_value" => "hacked"} + }) + |> Ash.update!(actor: user, domain: Mv.Membership) + end + end + + test "cannot create custom field value for unlinked member", %{ + user: user, + unlinked_member: unlinked_member, + custom_field: custom_field + } do + assert_raise Ash.Error.Forbidden, fn -> + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: unlinked_member.id, + custom_field_id: custom_field.id, + value: %{"_union_type" => "string", "_union_value" => "forbidden"} + }) + |> Ash.create!(actor: user, domain: Mv.Membership) + end + end + + test "cannot destroy custom field value of unlinked member", %{ + user: user, + cfv_unlinked: cfv_unlinked + } do + assert_raise Ash.Error.Forbidden, fn -> + Ash.destroy!(cfv_unlinked, actor: user, domain: Mv.Membership) + end + end + end + + describe "read_only permission set (Vorstand/Buchhaltung)" do + setup %{actor: actor} do + user = create_user_with_permission_set("read_only", actor) + linked_member = create_linked_member_for_user(user, actor) + unlinked_member = create_unlinked_member(actor) + custom_field = create_custom_field(actor) + + cfv_linked = create_custom_field_value(linked_member.id, custom_field.id, "linked", actor) + + cfv_unlinked = + create_custom_field_value(unlinked_member.id, custom_field.id, "unlinked", actor) + + {:ok, user} = + Ash.get(Accounts.User, user.id, domain: Mv.Accounts, load: [:role], actor: actor) + + %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked, + custom_field: custom_field, + linked_member: linked_member, + unlinked_member: unlinked_member + } + end + + test "can read all custom field values", %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked + } do + {:ok, values} = Ash.read(CustomFieldValue, actor: user, domain: Mv.Membership) + + ids = Enum.map(values, & &1.id) + assert cfv_linked.id in ids + assert cfv_unlinked.id in ids + end + + test "can read individual custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + {:ok, cfv} = + Ash.get(CustomFieldValue, cfv_unlinked.id, actor: user, domain: Mv.Membership) + + assert cfv.id == cfv_unlinked.id + end + + test "cannot create custom field value (returns forbidden)", %{ + user: user, + linked_member: linked_member, + custom_field: custom_field + } do + assert_raise Ash.Error.Forbidden, fn -> + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: linked_member.id, + custom_field_id: custom_field.id, + value: %{"_union_type" => "string", "_union_value" => "forbidden"} + }) + |> Ash.create!(actor: user, domain: Mv.Membership) + end + end + + test "cannot update custom field value (returns forbidden)", %{ + user: user, + cfv_linked: cfv_linked + } do + assert_raise Ash.Error.Forbidden, fn -> + cfv_linked + |> Ash.Changeset.for_update(:update, %{ + value: %{"_union_type" => "string", "_union_value" => "hacked"} + }) + |> Ash.update!(actor: user, domain: Mv.Membership) + end + end + + test "cannot destroy custom field value (returns forbidden)", %{ + user: user, + cfv_linked: cfv_linked + } do + assert_raise Ash.Error.Forbidden, fn -> + Ash.destroy!(cfv_linked, actor: user, domain: Mv.Membership) + end + end + end + + describe "normal_user permission set (Kassenwart)" do + setup %{actor: actor} do + user = create_user_with_permission_set("normal_user", actor) + linked_member = create_linked_member_for_user(user, actor) + unlinked_member = create_unlinked_member(actor) + custom_field = create_custom_field(actor) + + cfv_linked = create_custom_field_value(linked_member.id, custom_field.id, "linked", actor) + + cfv_unlinked = + create_custom_field_value(unlinked_member.id, custom_field.id, "unlinked", actor) + + {:ok, user} = + Ash.get(Accounts.User, user.id, domain: Mv.Accounts, load: [:role], actor: actor) + + %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked, + custom_field: custom_field, + linked_member: linked_member, + unlinked_member: unlinked_member, + actor: actor + } + end + + test "can read all custom field values", %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked + } do + {:ok, values} = Ash.read(CustomFieldValue, actor: user, domain: Mv.Membership) + + ids = Enum.map(values, & &1.id) + assert cfv_linked.id in ids + assert cfv_unlinked.id in ids + end + + test "can create custom field value", %{ + user: user, + unlinked_member: unlinked_member, + actor: actor + } do + # normal_user cannot create CustomField; use actor (admin) to create it + custom_field = create_custom_field(actor) + + {:ok, cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: unlinked_member.id, + custom_field_id: custom_field.id, + value: %{"_union_type" => "string", "_union_value" => "new"} + }) + |> Ash.create(actor: user, domain: Mv.Membership) + + assert cfv.member_id == unlinked_member.id + end + + test "can update any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + {:ok, updated} = + cfv_unlinked + |> Ash.Changeset.for_update(:update, %{ + value: %{"_union_type" => "string", "_union_value" => "updated"} + }) + |> Ash.update(actor: user, domain: Mv.Membership) + + assert %Ash.Union{value: "updated", type: :string} = updated.value + end + + test "can destroy any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + :ok = Ash.destroy(cfv_unlinked, actor: user, domain: Mv.Membership) + + assert {:error, _} = Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership) + end + end + + describe "admin permission set" do + setup %{actor: actor} do + user = create_user_with_permission_set("admin", actor) + linked_member = create_linked_member_for_user(user, actor) + unlinked_member = create_unlinked_member(actor) + custom_field = create_custom_field(actor) + + cfv_linked = create_custom_field_value(linked_member.id, custom_field.id, "linked", actor) + + cfv_unlinked = + create_custom_field_value(unlinked_member.id, custom_field.id, "unlinked", actor) + + {:ok, user} = + Ash.get(Accounts.User, user.id, domain: Mv.Accounts, load: [:role], actor: actor) + + %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked, + custom_field: custom_field, + linked_member: linked_member, + unlinked_member: unlinked_member + } + end + + test "can read all custom field values", %{ + user: user, + cfv_linked: cfv_linked, + cfv_unlinked: cfv_unlinked + } do + {:ok, values} = Ash.read(CustomFieldValue, actor: user, domain: Mv.Membership) + + ids = Enum.map(values, & &1.id) + assert cfv_linked.id in ids + assert cfv_unlinked.id in ids + end + + test "can create custom field value", %{user: user, unlinked_member: unlinked_member} do + custom_field = create_custom_field(user) + + {:ok, cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: unlinked_member.id, + custom_field_id: custom_field.id, + value: %{"_union_type" => "string", "_union_value" => "new"} + }) + |> Ash.create(actor: user, domain: Mv.Membership) + + assert cfv.member_id == unlinked_member.id + end + + test "can update any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + {:ok, updated} = + cfv_unlinked + |> Ash.Changeset.for_update(:update, %{ + value: %{"_union_type" => "string", "_union_value" => "updated"} + }) + |> Ash.update(actor: user, domain: Mv.Membership) + + assert %Ash.Union{value: "updated", type: :string} = updated.value + end + + test "can destroy any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + :ok = Ash.destroy(cfv_unlinked, actor: user, domain: Mv.Membership) + + assert {:error, _} = Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership) + end + end +end From db95979bf54c1ed32c90ab070516b0fffcac9ca9 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:40:38 +0100 Subject: [PATCH 19/25] Document CustomFieldValue policies and own_data create/destroy in architecture Update roles-and-permissions-architecture.md with policy layout and permission matrix for CustomFieldValue (linked). --- docs/roles-and-permissions-architecture.md | 44 +++++++++++----------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/docs/roles-and-permissions-architecture.md b/docs/roles-and-permissions-architecture.md index 8934688..acea99e 100644 --- a/docs/roles-and-permissions-architecture.md +++ b/docs/roles-and-permissions-architecture.md @@ -501,9 +501,11 @@ defmodule Mv.Authorization.PermissionSets do %{resource: "Member", action: :read, scope: :linked, granted: true}, %{resource: "Member", action: :update, scope: :linked, granted: true}, - # CustomFieldValue: Can read/update custom field values of linked member + # 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} @@ -678,7 +680,7 @@ Quick reference table showing what each permission set allows: | **User** (all) | - | - | - | R, C, U, D | | **Member** (linked) | R, U | - | - | - | | **Member** (all) | - | R | R, C, U | R, C, U, D | -| **CustomFieldValue** (linked) | R, U | - | - | - | +| **CustomFieldValue** (linked) | R, U, C, D | - | - | - | | **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 | @@ -1053,35 +1055,33 @@ end ### CustomFieldValue Resource Policies -**Location:** `lib/mv/membership/custom_field_value.ex` +**Location:** `lib/membership/custom_field_value.ex` -**Special Case:** Users can access custom field values of their linked member. +**Pattern:** Bypass for READ (list queries), CustomFieldValueCreateScope for create (no filter), HasPermission for read/update/destroy. Create uses a dedicated check because Ash cannot apply filters to create actions. ```elixir defmodule Mv.Membership.CustomFieldValue do use Ash.Resource, ... policies do - # SPECIAL CASE: Users can access custom field values of their linked member - # Note: This uses member_id relationship (CustomFieldValue.member_id → Member.id → User.member_id) - policy action_type([:read, :update]) do - description "Users can access custom field values of their linked member" + # Bypass for READ (list queries; expr triggers auto_filter) + bypass action_type(:read) do + description "Users can read custom field values of their linked member" authorize_if expr(member_id == ^actor(:member_id)) end - # GENERAL: Check permissions from role - policy action_type([:read, :create, :update, :destroy]) do - description "Check permissions from user's role" - authorize_if Mv.Authorization.Checks.HasPermission + # CREATE: CustomFieldValueCreateScope (no filter; Ash rejects filters on create) + # own_data -> create when member_id == actor.member_id; normal_user/admin -> create (scope :all) + policy action_type(:create) do + authorize_if Mv.Authorization.Checks.CustomFieldValueCreateScope end - # DEFAULT: Forbid - policy action_type([:read, :create, :update, :destroy]) do - forbid_if always() + # READ/UPDATE/DESTROY: HasPermission (scope :linked / :all) + policy action_type([:read, :update, :destroy]) do + authorize_if Mv.Authorization.Checks.HasPermission end + # DEFAULT: Ash implicitly forbids if no policy authorized (fail-closed) end - - # ... end ``` @@ -1089,11 +1089,13 @@ end | Action | Mitglied | Vorstand | Kassenwart | Buchhaltung | Admin | |--------|----------|----------|------------|-------------|-------| -| Read linked | ✅ (special) | ✅ (if linked) | ✅ | ✅ (if linked) | ✅ | -| Update linked | ✅ (special) | ❌ | ✅ | ❌ | ✅ | +| Read linked | ✅ (bypass) | ✅ (if linked) | ✅ | ✅ (if linked) | ✅ | +| Update linked | ✅ (scope :linked) | ❌ | ✅ | ❌ | ✅ | +| Create linked | ✅ (CustomFieldValueCreateScope) | ❌ | ✅ | ❌ | ✅ | +| Destroy linked | ✅ (scope :linked) | ❌ | ✅ | ❌ | ✅ | | Read all | ❌ | ✅ | ✅ | ✅ | ✅ | -| Create | ❌ | ❌ | ✅ | ❌ | ✅ | -| Destroy | ❌ | ❌ | ✅ | ❌ | ✅ | +| Create all | ❌ | ❌ | ✅ | ❌ | ✅ | +| Destroy all | ❌ | ❌ | ✅ | ❌ | ✅ | ### CustomField Resource Policies From 9e6c79bf40af2906db8d9ef9ff6061ba60d6de91 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 13:41:33 +0100 Subject: [PATCH 20/25] chore: remove start-database from test action --- Justfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Justfile b/Justfile index c68c473..f25041c 100644 --- a/Justfile +++ b/Justfile @@ -41,7 +41,7 @@ audit: mix deps.audit mix hex.audit -test *args: install-dependencies start-database +test *args: install-dependencies mix test {{args}} format: From 7153af23eee18b81478779ba0fe124b3a29d74be Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 15:44:38 +0100 Subject: [PATCH 21/25] CustomFieldValueCreateScope: use get_argument_or_attribute for member_id - Read member_id via Ash.Changeset.get_argument_or_attribute/2 so it works when set as attribute or argument - Remove unused require Logger - Document member_id source in moduledoc --- .../checks/custom_field_value_create_scope.ex | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/mv/authorization/checks/custom_field_value_create_scope.ex b/lib/mv/authorization/checks/custom_field_value_create_scope.ex index f5be53d..0b24e74 100644 --- a/lib/mv/authorization/checks/custom_field_value_create_scope.ex +++ b/lib/mv/authorization/checks/custom_field_value_create_scope.ex @@ -8,6 +8,14 @@ defmodule Mv.Authorization.Checks.CustomFieldValueCreateScope do (PermissionSets + :linked/:all) but only implements strict_check, so it never adds a filter. + ## member_id source + + The check reads `member_id` from the create changeset via + `Ash.Changeset.get_argument_or_attribute/2`, so it works when member_id + is set as an attribute or as an action argument. The CustomFieldValue + resource's default create action must accept and require `member_id` + (e.g. via `default_accept [:value, :member_id, :custom_field_id]`). + Used in CustomFieldValue policies: policy action_type(:create) do authorize_if Mv.Authorization.Checks.CustomFieldValueCreateScope @@ -15,7 +23,6 @@ defmodule Mv.Authorization.Checks.CustomFieldValueCreateScope do """ use Ash.Policy.Check alias Mv.Authorization.PermissionSets - require Logger @impl true def describe(_opts), @@ -53,8 +60,8 @@ defmodule Mv.Authorization.Checks.CustomFieldValueCreateScope do defp get_create_member_id(authorizer) do changeset = authorizer.changeset || authorizer.subject - if changeset && function_exported?(Ash.Changeset, :get_attribute, 2) do - Ash.Changeset.get_attribute(changeset, :member_id) + if changeset && function_exported?(Ash.Changeset, :get_argument_or_attribute, 2) do + Ash.Changeset.get_argument_or_attribute(changeset, :member_id) else nil end From 3f95a2dd84eceffc84a36cbd65b232b1036e318c Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 15:44:39 +0100 Subject: [PATCH 22/25] CustomFieldValue: remove unused require Ash.Query --- lib/membership/custom_field_value.ex | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/membership/custom_field_value.ex b/lib/membership/custom_field_value.ex index d6d91a6..623455d 100644 --- a/lib/membership/custom_field_value.ex +++ b/lib/membership/custom_field_value.ex @@ -42,7 +42,6 @@ defmodule Mv.Membership.CustomFieldValue do data_layer: AshPostgres.DataLayer, authorizers: [Ash.Policy.Authorizer] - require Ash.Query import Ash.Expr postgres do From 4d3a249b0c2aec4165fea2d8918f0dc293006029 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 15:44:40 +0100 Subject: [PATCH 23/25] HasPermission: remove unused _authorizer from strict_check helper --- lib/mv/authorization/checks/has_permission.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mv/authorization/checks/has_permission.ex b/lib/mv/authorization/checks/has_permission.ex index 1cf1e39..774e767 100644 --- a/lib/mv/authorization/checks/has_permission.ex +++ b/lib/mv/authorization/checks/has_permission.ex @@ -110,12 +110,12 @@ defmodule Mv.Authorization.Checks.HasPermission do {:ok, false} true -> - strict_check_with_permissions(actor, resource, action, record, authorizer) + strict_check_with_permissions(actor, resource, action, record) end end # Helper function to reduce nesting depth - defp strict_check_with_permissions(actor, resource, action, record, _authorizer) do + defp strict_check_with_permissions(actor, resource, action, record) do # Ensure role is loaded (fallback if on_mount didn't run) actor = ensure_role_loaded(actor) From 0219073d338a55d0d4c4a09a29a24c2f87d7c870 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 15:44:43 +0100 Subject: [PATCH 24/25] CFV policies test: system_actor for setup, verify destroy with actor - create_linked_member_for_user and create_unlinked_member use actor (system_actor) directly instead of creating admin user per call - Remove create_admin_user helper - After destroy, verify with Ash.get(..., actor: actor) to avoid false positive from Forbidden vs NotFound --- .../custom_field_value_policies_test.exs | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/test/mv/membership/custom_field_value_policies_test.exs b/test/mv/membership/custom_field_value_policies_test.exs index c7a80db..d400aec 100644 --- a/test/mv/membership/custom_field_value_policies_test.exs +++ b/test/mv/membership/custom_field_value_policies_test.exs @@ -60,13 +60,7 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do user_with_role end - defp create_admin_user(actor) do - create_user_with_permission_set("admin", actor) - end - defp create_linked_member_for_user(user, actor) do - admin = create_admin_user(actor) - {:ok, member} = Member |> Ash.Changeset.for_create(:create_member, %{ @@ -74,19 +68,17 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do last_name: "Member", email: "linked#{System.unique_integer([:positive])}@example.com" }) - |> Ash.create(actor: admin, return_notifications?: false) + |> Ash.create(actor: actor, return_notifications?: false) user |> Ash.Changeset.for_update(:update, %{}) |> Ash.Changeset.force_change_attribute(:member_id, member.id) - |> Ash.update(actor: admin, domain: Mv.Accounts, return_notifications?: false) + |> Ash.update(actor: actor, domain: Mv.Accounts, return_notifications?: false) member end defp create_unlinked_member(actor) do - admin = create_admin_user(actor) - {:ok, member} = Member |> Ash.Changeset.for_create(:create_member, %{ @@ -94,7 +86,7 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do last_name: "Member", email: "unlinked#{System.unique_integer([:positive])}@example.com" }) - |> Ash.create(actor: admin) + |> Ash.create(actor: actor) member end @@ -201,11 +193,16 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do assert cfv.custom_field_id == custom_field2.id end - test "can destroy custom field value of linked member", %{user: user, cfv_linked: cfv_linked} do + test "can destroy custom field value of linked member", %{ + user: user, + cfv_linked: cfv_linked, + actor: actor + } do result = Ash.destroy(cfv_linked, actor: user, domain: Mv.Membership) assert :ok = result - assert {:error, _} = Ash.get(CustomFieldValue, cfv_linked.id, domain: Mv.Membership) + assert {:error, _} = + Ash.get(CustomFieldValue, cfv_linked.id, domain: Mv.Membership, actor: actor) end test "cannot read custom field values of unlinked member", %{ @@ -408,10 +405,15 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do assert %Ash.Union{value: "updated", type: :string} = updated.value end - test "can destroy any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + test "can destroy any custom field value", %{ + user: user, + cfv_unlinked: cfv_unlinked, + actor: actor + } do :ok = Ash.destroy(cfv_unlinked, actor: user, domain: Mv.Membership) - assert {:error, _} = Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership) + assert {:error, _} = + Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership, actor: actor) end end @@ -478,10 +480,15 @@ defmodule Mv.Membership.CustomFieldValuePoliciesTest do assert %Ash.Union{value: "updated", type: :string} = updated.value end - test "can destroy any custom field value", %{user: user, cfv_unlinked: cfv_unlinked} do + test "can destroy any custom field value", %{ + user: user, + cfv_unlinked: cfv_unlinked, + actor: actor + } do :ok = Ash.destroy(cfv_unlinked, actor: user, domain: Mv.Membership) - assert {:error, _} = Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership) + assert {:error, _} = + Ash.get(CustomFieldValue, cfv_unlinked.id, domain: Mv.Membership, actor: actor) end end end From bfe9fba2e03e87f7a4d985830a2a299939050a37 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 27 Jan 2026 15:44:44 +0100 Subject: [PATCH 25/25] Docs: document bypass read rule for CustomFieldValue pattern - Bypass action_type(:read) is production-side rule: reading own CFVs always allowed, overrides Permission-Sets. Applies to get/list/load. --- docs/roles-and-permissions-architecture.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/roles-and-permissions-architecture.md b/docs/roles-and-permissions-architecture.md index acea99e..063de32 100644 --- a/docs/roles-and-permissions-architecture.md +++ b/docs/roles-and-permissions-architecture.md @@ -1059,6 +1059,8 @@ end **Pattern:** Bypass for READ (list queries), CustomFieldValueCreateScope for create (no filter), HasPermission for read/update/destroy. Create uses a dedicated check because Ash cannot apply filters to create actions. +The bypass `action_type(:read)` is a production-side rule: reading own CFVs (where `member_id == actor.member_id`) is always allowed and overrides Permission-Sets; no further policies are needed for that. It applies to all read actions (get, list, load). + ```elixir defmodule Mv.Membership.CustomFieldValue do use Ash.Resource, ...