diff --git a/.drone.yml b/.drone.yml index 8c7f325..06db32b 100644 --- a/.drone.yml +++ b/.drone.yml @@ -4,7 +4,7 @@ name: check services: - name: postgres - image: docker.io/library/postgres:17.7 + image: docker.io/library/postgres:18.1 environment: POSTGRES_USER: postgres POSTGRES_PASSWORD: postgres @@ -57,7 +57,7 @@ steps: - mix gettext.extract --check-up-to-date - name: wait_for_postgres - image: docker.io/library/postgres:17.7 + image: docker.io/library/postgres:18.1 commands: # Wait for postgres to become available - | @@ -166,7 +166,7 @@ environment: steps: - name: renovate - image: renovate/renovate:42.44 + image: renovate/renovate:42.71 environment: RENOVATE_CONFIG_FILE: "renovate_backend_config.js" RENOVATE_TOKEN: diff --git a/.tool-versions b/.tool-versions index 489262a..275206c 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,3 +1,3 @@ elixir 1.18.3-otp-27 erlang 27.3.4 -just 1.45.0 +just 1.46.0 diff --git a/config/config.exs b/config/config.exs index 5fcfcf5..cc338b2 100644 --- a/config/config.exs +++ b/config/config.exs @@ -49,7 +49,7 @@ config :spark, config :mv, ecto_repos: [Mv.Repo], generators: [timestamp_type: :utc_datetime], - ash_domains: [Mv.Membership, Mv.Accounts, Mv.MembershipFees] + ash_domains: [Mv.Membership, Mv.Accounts, Mv.MembershipFees, Mv.Authorization] # Configures the endpoint config :mv, MvWeb.Endpoint, diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index 5b35e10..1ed863a 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -33,7 +33,7 @@ services: restart: unless-stopped db-prod: - image: postgres:17.7-alpine + image: postgres:18.1-alpine container_name: mv-prod-db environment: POSTGRES_USER: postgres diff --git a/docker-compose.yml b/docker-compose.yml index feff34c..8621603 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -4,7 +4,7 @@ networks: services: db: - image: postgres:17.7-alpine + image: postgres:18.1-alpine environment: POSTGRES_USER: postgres POSTGRES_PASSWORD: postgres @@ -29,7 +29,7 @@ services: rauthy: container_name: rauthy-dev - image: ghcr.io/sebadob/rauthy:0.33.1 + image: ghcr.io/sebadob/rauthy:0.33.4 environment: - LOCAL_TEST=true - SMTP_URL=mailcrab diff --git a/docs/csv-member-import-v1.md b/docs/csv-member-import-v1.md new file mode 100644 index 0000000..2bdbe69 --- /dev/null +++ b/docs/csv-member-import-v1.md @@ -0,0 +1,666 @@ +# CSV Member Import v1 - Implementation Plan + +**Version:** 1.0 +**Date:** 2025-01-XX +**Status:** Ready for Implementation +**Related Documents:** +- [Feature Roadmap](./feature-roadmap.md) - Overall feature planning + +--- + +## Table of Contents + +- [Overview & Scope](#overview--scope) +- [UX Flow](#ux-flow) +- [CSV Specification](#csv-specification) +- [Technical Design Notes](#technical-design-notes) +- [Implementation Issues](#implementation-issues) +- [Rollout & Risks](#rollout--risks) + +--- + +## Overview & Scope + +### What We're Building + +A **basic CSV member import feature** that allows administrators to upload a CSV file and import new members into the system. This is a **v1 minimal implementation** focused on establishing the import structure without advanced features. + +**Core Functionality (v1 Minimal):** +- Upload CSV file via LiveView file upload +- Parse CSV with bilingual header support for core member fields (English/German) +- Auto-detect delimiter (`;` or `,`) using header recognition +- Map CSV columns to core member fields (`first_name`, `last_name`, `email`, `street`, `postal_code`, `city`) +- **Import custom field values** - Map CSV columns to existing custom fields by name (unknown custom field columns will be ignored with a warning) +- Validate each row (required field: `email`) +- Create members via Ash resource (one-by-one, **no background jobs**, processed in chunks of 200 rows via LiveView messages) +- Display import results: success count, error count, and error details +- Provide static CSV templates (EN/DE) + +**Key Constraints (v1):** +- ✅ **Admin-only feature** +- ✅ **No upsert** (create only) +- ✅ **No deduplication** (duplicate emails fail and show as errors) +- ✅ **No mapping wizard** (fixed header mapping via bilingual variants) +- ✅ **No background jobs** (progress via LiveView `handle_info`) +- ✅ **Best-effort import** (row-by-row, no rollback) +- ✅ **UI-only error display** (no error CSV export) +- ✅ **Safety limits** (10 MB, 1,000 rows, chunks of 200) + +### Out of Scope (v1) + +**Deferred to Future Versions:** +- ❌ Upsert/update existing members +- ❌ Advanced deduplication strategies +- ❌ Column mapping wizard UI +- ❌ Background job processing (Oban/GenStage) +- ❌ Transactional all-or-nothing import +- ❌ Error CSV export/download +- ❌ Batch validation preview before import +- ❌ Dynamic template generation +- ❌ Import history/audit log +- ❌ Import templates for other entities + +--- + +## UX Flow + +### Access & Location + +**Entry Point:** +- **Location:** Global Settings page (`/settings`) +- **UI Element:** New section "Import Members (CSV)" below "Custom Fields" section +- **Access Control:** Admin-only (enforced at LiveView event level, not entire `/settings` route) + +### User Journey + +1. **Navigate to Global Settings** +2. **Access Import Section** + - **Important notice:** Custom fields should be created in Mila before importing CSV files with custom field columns (unknown columns will be ignored with a warning) + - Upload area (drag & drop or file picker) + - Template download links (English / German) + - Help text explaining CSV format and custom field requirements +3. **Ensure Custom Fields Exist (if importing custom fields)** + - Navigate to Custom Fields section and create required custom fields + - Note the name/identifier for each custom field (used as CSV header) +4. **Download Template (Optional)** +5. **Prepare CSV File** + - Include custom field columns using the custom field name as header (e.g., `membership_number`, `birth_date`) +6. **Upload CSV** +7. **Start Import** + - Runs server-side via LiveView messages (may take up to ~30 seconds for large files) + - Warning messages if custom field columns reference non-existent custom fields (columns will be ignored) +8. **View Results** + - Success count + - Error count + - First 50 errors, each with: + - **CSV line number** (header is line 1, first data record begins at line 2) + - Error message + - Field name (if applicable) + +### Error Handling + +- **File too large:** Flash error before upload starts +- **Too many rows:** Flash error before import starts +- **Invalid CSV format:** Error shown in results +- **Partial success:** Results show both success and error counts + +--- + +## CSV Specification + +### Delimiter + +**Recommended:** Semicolon (`;`) +**Supported:** `;` and `,` + +**Auto-Detection (Header Recognition):** +- Remove UTF-8 BOM *first* +- Extract header record and try parsing with both delimiters +- For each delimiter, count how many recognized headers are present (via normalized variants) +- Choose delimiter with higher recognition; prefer `;` if tied +- If neither yields recognized headers, default to `;` + +### Quoting Rules + +- Fields may be quoted with double quotes (`"`) +- Escaped quotes: `""` inside quoted field represents a single `"` +- **v1 assumption:** CSV records do **not** contain embedded newlines inside quoted fields. (If they do, parsing may fail or line numbers may be inaccurate.) + +### Column Headers + +**v1 Supported Fields:** + +**Core Member Fields:** +- `first_name` / `Vorname` (optional) +- `last_name` / `Nachname` (optional) +- `email` / `E-Mail` (required) +- `street` / `Straße` (optional) +- `postal_code` / `PLZ` / `Postleitzahl` (optional) +- `city` / `Stadt` (optional) + +**Custom Fields:** +- Any custom field column using the custom field's **name** as the header (e.g., `membership_number`, `birth_date`) +- **Important:** Custom fields must be created in Mila before importing. The CSV header must match the custom field name exactly (same normalization as member fields). +- **Behavior:** If the CSV contains custom field columns that don't exist in Mila, a warning message will be shown and those columns will be ignored during import. + +**Member Field Header Mapping:** + +| Canonical Field | English Variants | German Variants | +|---|---|---| +| `first_name` | `first_name`, `firstname` | `Vorname`, `vorname` | +| `last_name` | `last_name`, `lastname`, `surname` | `Nachname`, `nachname`, `Familienname` | +| `email` | `email`, `e-mail`, `e_mail` | `E-Mail`, `e-mail`, `e_mail` | +| `street` | `street`, `address` | `Straße`, `strasse`, `Strasse` | +| `postal_code` | `postal_code`, `zip`, `postcode` | `PLZ`, `plz`, `Postleitzahl`, `postleitzahl` | +| `city` | `city`, `town` | `Stadt`, `stadt`, `Ort` | + +**Header Normalization (used consistently for both input headers AND mapping variants):** +- Trim whitespace +- Convert to lowercase +- Normalize Unicode: `ß` → `ss` (e.g., `Straße` → `strasse`) +- Replace hyphens/whitespace with underscores: `E-Mail` → `e_mail`, `phone number` → `phone_number` +- Collapse multiple underscores: `e__mail` → `e_mail` +- Case-insensitive matching + +**Unknown columns:** ignored (no error) + +**Required fields:** `email` + +**Custom Field Columns:** +- Custom field columns are identified by matching the normalized CSV header to the custom field `name` (not slug) +- Same normalization rules apply as for member fields (trim, lowercase, Unicode normalization, underscore replacement) +- Unknown custom field columns (non-existent names) will be ignored with a warning message + +### CSV Template Files + +**Location:** +- `priv/static/templates/member_import_en.csv` +- `priv/static/templates/member_import_de.csv` + +**Content:** +- Header row with required + common optional fields +- **Note:** Custom field columns are not included in templates by default (users add them based on their custom field configuration) +- One example row +- Uses semicolon delimiter (`;`) +- UTF-8 encoding **with BOM** (Excel compatibility) + +**Template Access:** +- Templates are static files in `priv/static/templates/` +- Served at: + - `/templates/member_import_en.csv` + - `/templates/member_import_de.csv` +- In LiveView, link them using Phoenix static path helpers (e.g. `~p` or `Routes.static_path/2`, depending on Phoenix version). + +### File Limits + +- **Max file size:** 10 MB +- **Max rows:** 1,000 rows (excluding header) +- **Processing:** chunks of 200 (via LiveView messages) +- **Encoding:** UTF-8 (BOM handled) + +--- + +## Technical Design Notes + +### Architecture Overview + +``` +┌─────────────────┐ +│ LiveView UI │ (GlobalSettingsLive or component) +│ - Upload area │ +│ - Progress │ +│ - Results │ +└────────┬────────┘ + │ prepare + ▼ +┌─────────────────────────────┐ +│ Import Service │ (Mv.Membership.Import.MemberCSV) +│ - parse + map + limit checks│ -> returns import_state +│ - process_chunk(chunk) │ -> returns chunk results +└────────┬────────────────────┘ + │ create + ▼ +┌─────────────────┐ +│ Ash Resource │ (Mv.Membership.Member) +│ - Create │ +└─────────────────┘ +``` + +### Technology Stack + +- **Phoenix LiveView:** file upload via `allow_upload/3` +- **NimbleCSV:** CSV parsing (add explicit dependency if missing) +- **Ash Resource:** member creation via `Membership.create_member/1` +- **Gettext:** bilingual UI/error messages + +### Module Structure + +**New Modules:** +- `lib/mv/membership/import/member_csv.ex` - import orchestration + chunk processing + custom field handling +- `lib/mv/membership/import/csv_parser.ex` - delimiter detection + parsing + BOM handling +- `lib/mv/membership/import/header_mapper.ex` - normalization + header mapping (core fields + custom fields) + +**Modified Modules:** +- `lib/mv_web/live/global_settings_live.ex` - render import section, handle upload/events/messages + +### Data Flow + +1. **Upload:** LiveView receives file via `allow_upload` +2. **Consume:** `consume_uploaded_entries/3` reads file content +3. **Prepare:** `MemberCSV.prepare/2` + - Strip BOM + - Detect delimiter (header recognition) + - Parse header + rows + - Map headers to canonical fields (core member fields) + - **Query existing custom fields and map custom field columns by name** (using same normalization as member fields) + - **Warn about unknown custom field columns** (non-existent names will be ignored with warning) + - Early abort if required headers missing + - Row count check + - Return `import_state` containing chunks, column_map, and custom_field_map +4. **Process:** LiveView drives chunk processing via `handle_info` + - For each chunk: validate + create member + create custom field values + collect errors +5. **Results:** LiveView shows progress + final summary + +### Types & Key Consistency + +- **Raw CSV parsing:** returns headers as list of strings, and rows **with csv line numbers** +- **Header mapping:** operates on normalized strings; mapping table variants are normalized once +- **Ash attrs:** built as atom-keyed map (`%{first_name: ..., ...}`) + +### Error Model + +```elixir +%{ + csv_line_number: 5, # physical line number in the CSV file + field: :email, # optional + message: "is not a valid email" +} +``` + +### CSV Line Numbers (Important) + +To keep error reporting user-friendly and accurate, **row errors must reference the physical line number in the original file**, even if empty lines are skipped. + +**Design decision:** the parser returns rows as: + +```elixir +rows :: [{csv_line_number :: pos_integer(), row_map :: map()}] +``` + +Downstream logic must **not** recompute line numbers from row indexes. + +### Authorization + +**Enforcement points:** +1. **LiveView event level:** check admin permission in `handle_event("start_import", ...)` +2. **UI level:** render import section only for admin users +3. **Static templates:** public assets (no authorization needed) + +Use `Mv.Authorization.PermissionSets` (preferred) instead of hard-coded string checks where possible. + +### Safety Limits + +- File size enforced by `allow_upload` (`max_file_size`) +- Row count enforced in `MemberCSV.prepare/2` before processing starts +- Chunking is done via **LiveView `handle_info` loop** (sequential, cooperative scheduling) + +--- + +## Implementation Issues + +### Issue #1: CSV Specification & Static Template Files + +**Dependencies:** None + +**Goal:** Define CSV contract and add static templates. + +**Tasks:** +- [ ] Finalize header mapping variants +- [ ] Document normalization rules +- [ ] Document delimiter detection strategy +- [ ] Create templates in `priv/static/templates/` (UTF-8 with BOM) +- [ ] Document template URLs and how to link them from LiveView +- [ ] Document line number semantics (physical CSV line numbers) + +**Definition of Done:** +- [ ] Templates open cleanly in Excel/LibreOffice +- [ ] CSV spec section complete + +--- + +### Issue #2: Import Service Module Skeleton + +**Dependencies:** None + +**Goal:** Create service API and error types. + +**API (recommended):** +- `prepare/2` — parse + map + limit checks, returns import_state +- `process_chunk/3` — process one chunk (pure-ish), returns per-chunk results + +**Tasks:** +- [ ] Create `lib/mv/membership/import/member_csv.ex` +- [ ] Define public function: `prepare/2 (file_content, opts \\ [])` +- [ ] Define public function: `process_chunk/3 (chunk_rows_with_lines, column_map, opts \\ [])` +- [ ] Define error struct: `%MemberCSV.Error{csv_line_number: integer, field: atom | nil, message: String.t}` +- [ ] Document module + API + +--- + +### Issue #3: CSV Parsing + Delimiter Auto-Detection + BOM Handling + +**Dependencies:** Issue #2 + +**Goal:** Parse CSV robustly with correct delimiter detection and BOM handling. + +**Tasks:** +- [ ] Verify/add NimbleCSV dependency (`{:nimble_csv, "~> 1.0"}`) +- [ ] Create `lib/mv/membership/import/csv_parser.ex` +- [ ] Implement `strip_bom/1` and apply it **before** any header handling +- [ ] Handle `\r\n` and `\n` line endings (trim `\r` on header record) +- [ ] Detect delimiter via header recognition (try `;` and `,`) +- [ ] Parse CSV and return: + - `headers :: [String.t()]` + - `rows :: [{csv_line_number, [String.t()]}]` or directly `[{csv_line_number, row_map}]` +- [ ] Skip completely empty records (but preserve correct physical line numbers) +- [ ] Return `{:ok, headers, rows}` or `{:error, reason}` + +**Definition of Done:** +- [ ] BOM handling works (Excel exports) +- [ ] Delimiter detection works reliably +- [ ] Rows carry correct `csv_line_number` + +--- + +### Issue #4: Header Normalization + Per-Header Mapping (No Language Detection) + +**Dependencies:** Issue #3 + +**Goal:** Map each header individually to canonical fields (normalized comparison). + +**Tasks:** +- [ ] Create `lib/mv/membership/import/header_mapper.ex` +- [ ] Implement `normalize_header/1` +- [ ] Normalize mapping variants once and compare normalized strings +- [ ] Build `column_map` (canonical field -> column index) +- [ ] **Early abort if required headers missing** (`email`) +- [ ] Ignore unknown columns (member fields only) +- [ ] **Separate custom field column detection** (by name, with normalization) + +**Definition of Done:** +- [ ] English/German headers map correctly +- [ ] Missing required columns fails fast + +--- + +### Issue #5: Validation (Required Fields) + Error Formatting + +**Dependencies:** Issue #4 + +**Goal:** Validate each row and return structured, translatable errors. + +**Tasks:** +- [ ] Implement `validate_row/3 (row_map, csv_line_number, opts)` +- [ ] Required field presence (`email`) +- [ ] Email format validation (EctoCommons.EmailValidator) +- [ ] Trim values before validation +- [ ] Gettext-backed error messages + +--- + +### Issue #6: Persistence via Ash Create + Per-Row Error Capture (Chunked Processing) + +**Dependencies:** Issue #5 + +**Goal:** Create members and capture errors per row with correct CSV line numbers. + +**Tasks:** +- [ ] Implement `process_chunk/3` in service: + - Input: `[{csv_line_number, row_map}]` + - Validate + create sequentially + - Collect counts + first 50 errors (per import overall; LiveView enforces cap across chunks) +- [ ] Implement Ash error formatter helper: + - Convert `Ash.Error.Invalid` into `%MemberCSV.Error{}` + - Prefer field-level errors where possible (attach `field` atom) + - Handle unique email constraint error as user-friendly message +- [ ] Map row_map to Ash attrs (`%{first_name: ..., ...}`) + +**Important:** **Do not recompute line numbers** in this layer—use the ones provided by the parser. + +--- + +### Issue #7: Admin Global Settings LiveView UI (Upload + Start Import + Results + Template Links) + +**Dependencies:** Issue #6 + +**Goal:** UI section with upload, progress, results, and template links. + +**Tasks:** +- [ ] Render import section only for admins +- [ ] **Add prominent UI notice about custom fields:** + - Display alert/info box: "Custom fields must be created in Mila before importing CSV files with custom field columns" + - Explain: "Use the custom field name as the CSV column header (same normalization as member fields applies)" + - Add link to custom fields management section +- [ ] Configure `allow_upload/3`: + - `.csv` only, `max_entries: 1`, `max_file_size: 10MB`, `auto_upload: false` +- [ ] `handle_event("start_import", ...)`: + - Admin permission check + - Consume upload -> read file content + - Call `MemberCSV.prepare/2` + - Store `import_state` in assigns (chunks + column_map + metadata) + - Initialize progress assigns + - `send(self(), {:process_chunk, 0})` +- [ ] `handle_info({:process_chunk, idx}, socket)`: + - Fetch chunk from `import_state` + - Call `MemberCSV.process_chunk/3` + - Merge counts/errors into progress assigns (cap errors at 50 overall) + - Schedule next chunk (or finish and show results) +- [ ] Results UI: + - Success count + - Failure count + - Error list (line number + message + field) + - **Warning messages for unknown custom field columns** (non-existent names) shown in results + +**Template links:** +- Link `/templates/member_import_en.csv` and `/templates/member_import_de.csv` via Phoenix static path helpers. + +--- + +### Issue #8: Authorization + Limits + +**Dependencies:** None (can be parallelized) + +**Goal:** Ensure admin-only access and enforce limits. + +**Tasks:** +- [ ] Admin check in start import event handler +- [ ] File size enforced in upload config +- [ ] Row limit enforced in `MemberCSV.prepare/2` (max_rows from config) +- [ ] Configuration: + ```elixir + config :mv, csv_import: [ + max_file_size_mb: 10, + max_rows: 1000 + ] + ``` + +--- + +### Issue #9: End-to-End LiveView Tests + Fixtures + +**Dependencies:** Issue #7 and #8 + +**Tasks:** +- [ ] Fixtures: + - valid EN/DE (core fields only) + - valid with custom fields + - invalid + - unknown custom field name (non-existent, should show warning) + - too many rows (1,001) + - BOM + `;` delimiter fixture + - fixture with empty line(s) to validate correct line numbers +- [ ] LiveView tests: + - admin sees section, non-admin does not + - upload + start import + - success + error rendering + - row limit + file size errors + - custom field import success + - custom field import warning (non-existent name, column ignored) + +--- + +### Issue #10: Documentation Polish (Inline Help Text + Docs) + +**Dependencies:** Issue #9 + +**Tasks:** +- [ ] UI help text + translations +- [ ] CHANGELOG entry +- [ ] Ensure moduledocs/docs + +--- + +### Issue #11: Custom Field Import + +**Dependencies:** Issue #6 (Persistence) + +**Priority:** High (Core v1 Feature) + +**Goal:** Support importing custom field values from CSV columns. Custom fields should exist in Mila before import for best results. + +**Important Requirements:** +- **Custom fields should be created in Mila first** - Unknown custom field columns will be ignored with a warning message +- CSV headers for custom fields must match the custom field **name** exactly (same normalization as member fields applies) +- Custom field values are validated according to the custom field type (string, integer, boolean, date, email) +- Unknown custom field columns (non-existent names) will be ignored with a warning - import continues + +**Tasks:** +- [ ] Extend `header_mapper.ex` to detect custom field columns by name (using same normalization as member fields) +- [ ] Query existing custom fields during `prepare/2` to map custom field columns +- [ ] Collect unknown custom field columns and add warning messages (don't fail import) +- [ ] Map custom field CSV values to `CustomFieldValue` creation in `process_chunk/3` +- [ ] Handle custom field type validation (string, integer, boolean, date, email) +- [ ] Create `CustomFieldValue` records linked to members during import +- [ ] Update error messages to include custom field validation errors +- [ ] Add UI help text explaining custom field requirements: + - "Custom fields must be created in Mila before importing" + - "Use the custom field name as the CSV column header (same normalization as member fields)" + - Link to custom fields management section +- [ ] Update CSV templates documentation to explain custom field columns +- [ ] Add tests for custom field import (valid, invalid name, type validation, warning for unknown) + +**Definition of Done:** +- [ ] Custom field columns are recognized by name (with normalization) +- [ ] Warning messages shown for unknown custom field columns (import continues) +- [ ] Custom field values are created and linked to members +- [ ] Type validation works for all custom field types +- [ ] UI clearly explains custom field requirements +- [ ] Tests cover custom field import scenarios (including warning for unknown names) + +--- + +## Rollout & Risks + +### Rollout Strategy +- Dev → Staging → Production (with anonymized real-world CSV tests) + +### Risks & Mitigations + +| Risk | Impact | Likelihood | Mitigation | +|---|---:|---:|---| +| Large import timeout | High | Medium | 10 MB + 1,000 rows, chunking via `handle_info` | +| Encoding issues | Medium | Medium | BOM stripping, templates with BOM | +| Invalid CSV format | Medium | High | Clear errors + templates | +| Duplicate emails | Low | High | Ash constraint error -> user-friendly message | +| Performance (no background jobs) | Medium | Low | Small limits, sequential chunk processing | +| Admin access bypass | High | Low | Event-level auth + UI hiding | +| Data corruption | High | Low | Per-row validation + best-effort | + +--- + +## Appendix + +### Module File Structure + +``` +lib/ +├── mv/ +│ └── membership/ +│ └── import/ +│ ├── member_csv.ex # prepare + process_chunk +│ ├── csv_parser.ex # delimiter detection + parsing + BOM handling +│ └── header_mapper.ex # normalization + header mapping +└── mv_web/ + └── live/ + └── global_settings_live.ex # add import section + LV message loop + +priv/ +└── static/ + └── templates/ + ├── member_import_en.csv + └── member_import_de.csv + +test/ +├── mv/ +│ └── membership/ +│ └── import/ +│ ├── member_csv_test.exs +│ ├── csv_parser_test.exs +│ └── header_mapper_test.exs +└── fixtures/ + ├── member_import_en.csv + ├── member_import_de.csv + ├── member_import_invalid.csv + ├── member_import_large.csv + └── member_import_empty_lines.csv +``` + +### Example Usage (LiveView) + +```elixir +def handle_event("start_import", _params, socket) do + assert_admin!(socket.assigns.current_user) + + [{_name, content}] = + consume_uploaded_entries(socket, :csv_file, fn %{path: path}, _entry -> + {:ok, File.read!(path)} + end) + + case Mv.Membership.Import.MemberCSV.prepare(content) do + {:ok, import_state} -> + socket = + socket + |> assign(:import_state, import_state) + |> assign(:import_progress, %{processed: 0, inserted: 0, failed: 0, errors: []}) + |> assign(:importing?, true) + + send(self(), {:process_chunk, 0}) + {:noreply, socket} + + {:error, reason} -> + {:noreply, put_flash(socket, :error, reason)} + end +end + +def handle_info({:process_chunk, idx}, socket) do + %{chunks: chunks, column_map: column_map} = socket.assigns.import_state + + case Enum.at(chunks, idx) do + nil -> + {:noreply, assign(socket, importing?: false)} + + chunk_rows_with_lines -> + {:ok, chunk_result} = + Mv.Membership.Import.MemberCSV.process_chunk(chunk_rows_with_lines, column_map) + + socket = merge_progress(socket, chunk_result) # caps errors at 50 overall + + send(self(), {:process_chunk, idx + 1}) + {:noreply, socket} + end +end +``` + +--- + +**End of Implementation Plan** \ No newline at end of file diff --git a/docs/roles-and-permissions-architecture.md b/docs/roles-and-permissions-architecture.md index fa45d86..b44604b 100644 --- a/docs/roles-and-permissions-architecture.md +++ b/docs/roles-and-permissions-architecture.md @@ -93,8 +93,8 @@ Five predefined roles stored in the `roles` table: Control CRUD operations on: - User (credentials, profile) - Member (member data) -- Property (custom field values) -- PropertyType (custom field definitions) +- CustomFieldValue (custom field values) +- CustomField (custom field definitions) - Role (role management) **4. Page-Level Permissions** @@ -111,7 +111,7 @@ Three scope levels for permissions: - **:own** - Only records where `record.id == user.id` (for User resource) - **:linked** - Only records linked to user via relationships - Member: `member.user_id == user.id` - - Property: `property.member.user_id == user.id` + - CustomFieldValue: `custom_field_value.member.user_id == user.id` - **:all** - All records, no filtering **6. Special Cases** @@ -414,7 +414,7 @@ defmodule Mv.Authorization.PermissionSets do ## Permission Sets 1. **own_data** - Default for "Mitglied" role - - Can only access own user data and linked member/properties + - Can only access own user data and linked member/custom field values - Cannot create new members or manage system 2. **read_only** - For "Vorstand" and "Buchhaltung" roles @@ -423,11 +423,11 @@ defmodule Mv.Authorization.PermissionSets do 3. **normal_user** - For "Kassenwart" role - Create/Read/Update members (no delete), full CRUD on properties - - Cannot manage property types or users + - Cannot manage custom fields or users 4. **admin** - For "Admin" role - Unrestricted access to all resources - - Can manage users, roles, property types + - Can manage users, roles, custom fields ## Usage @@ -500,12 +500,12 @@ defmodule Mv.Authorization.PermissionSets do %{resource: "Member", action: :read, scope: :linked, granted: true}, %{resource: "Member", action: :update, scope: :linked, granted: true}, - # Property: Can read/update properties of linked member - %{resource: "Property", action: :read, scope: :linked, granted: true}, - %{resource: "Property", action: :update, scope: :linked, granted: true}, + # CustomFieldValue: Can read/update custom field values of linked member + %{resource: "CustomFieldValue", action: :read, scope: :linked, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :linked, granted: true}, - # PropertyType: Can read all (needed for forms) - %{resource: "PropertyType", action: :read, scope: :all, granted: true} + # CustomField: Can read all (needed for forms) + %{resource: "CustomField", action: :read, scope: :all, granted: true} ], pages: [ "/", # Home page @@ -525,17 +525,17 @@ defmodule Mv.Authorization.PermissionSets do # Member: Can read all members, no modifications %{resource: "Member", action: :read, scope: :all, granted: true}, - # Property: Can read all properties - %{resource: "Property", action: :read, scope: :all, granted: true}, + # CustomFieldValue: Can read all custom field values + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, - # PropertyType: Can read all - %{resource: "PropertyType", action: :read, scope: :all, granted: true} + # CustomField: Can read all + %{resource: "CustomField", action: :read, scope: :all, granted: true} ], pages: [ "/", "/members", # Member list "/members/:id", # Member detail - "/properties", # Property overview + "/custom_field_values" # Custom field values overview "/profile" # Own profile ] } @@ -554,14 +554,14 @@ defmodule Mv.Authorization.PermissionSets do %{resource: "Member", action: :update, scope: :all, granted: true}, # Note: destroy intentionally omitted for safety - # Property: Full CRUD - %{resource: "Property", action: :read, scope: :all, granted: true}, - %{resource: "Property", action: :create, scope: :all, granted: true}, - %{resource: "Property", action: :update, scope: :all, granted: true}, - %{resource: "Property", action: :destroy, scope: :all, granted: true}, + # CustomFieldValue: Full CRUD + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true}, - # PropertyType: Read only (admin manages definitions) - %{resource: "PropertyType", action: :read, scope: :all, granted: true} + # CustomField: Read only (admin manages definitions) + %{resource: "CustomField", action: :read, scope: :all, granted: true} ], pages: [ "/", @@ -569,9 +569,9 @@ defmodule Mv.Authorization.PermissionSets do "/members/new", # Create member "/members/:id", "/members/:id/edit", # Edit member - "/properties", - "/properties/new", - "/properties/:id/edit", + "/custom_field_values", + "/custom_field_values/new", + "/custom_field_values/:id/edit", "/profile" ] } @@ -592,17 +592,17 @@ defmodule Mv.Authorization.PermissionSets do %{resource: "Member", action: :update, scope: :all, granted: true}, %{resource: "Member", action: :destroy, scope: :all, granted: true}, - # Property: Full CRUD - %{resource: "Property", action: :read, scope: :all, granted: true}, - %{resource: "Property", action: :create, scope: :all, granted: true}, - %{resource: "Property", action: :update, scope: :all, granted: true}, - %{resource: "Property", action: :destroy, scope: :all, granted: true}, + # CustomFieldValue: Full CRUD + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true}, - # PropertyType: Full CRUD (admin manages custom field definitions) - %{resource: "PropertyType", action: :read, scope: :all, granted: true}, - %{resource: "PropertyType", action: :create, scope: :all, granted: true}, - %{resource: "PropertyType", action: :update, scope: :all, granted: true}, - %{resource: "PropertyType", action: :destroy, scope: :all, granted: true}, + # CustomField: Full CRUD (admin manages custom field definitions) + %{resource: "CustomField", action: :read, scope: :all, granted: true}, + %{resource: "CustomField", action: :create, scope: :all, granted: true}, + %{resource: "CustomField", action: :update, scope: :all, granted: true}, + %{resource: "CustomField", action: :destroy, scope: :all, granted: true}, # Role: Full CRUD (admin manages roles) %{resource: "Role", action: :read, scope: :all, granted: true}, @@ -677,9 +677,9 @@ 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 | -| **Property** (linked) | R, U | - | - | - | -| **Property** (all) | - | R | R, C, U, D | R, C, U, D | -| **PropertyType** (all) | R | R | R | R, C, U, D | +| **CustomFieldValue** (linked) | R, U | - | - | - | +| **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 | **Legend:** R=Read, C=Create, U=Update, D=Destroy @@ -715,7 +715,7 @@ defmodule Mv.Authorization.Checks.HasPermission do - **:own** - Filters to records where record.id == actor.id - **:linked** - Filters based on resource type: - Member: member.user_id == actor.id - - Property: property.member.user_id == actor.id (traverses relationship!) + - CustomFieldValue: custom_field_value.member.user_id == actor.id (traverses relationship!) ## Error Handling @@ -802,8 +802,8 @@ defmodule Mv.Authorization.Checks.HasPermission do # Member.user_id == actor.id (direct relationship) {:filter, expr(user_id == ^actor.id)} - "Property" -> - # Property.member.user_id == actor.id (traverse through member!) + "CustomFieldValue" -> + # CustomFieldValue.member.user_id == actor.id (traverse through member!) {:filter, expr(member.user_id == ^actor.id)} _ -> @@ -832,7 +832,7 @@ end **Key Design Decisions:** -1. **Resource-Specific :linked Scope:** Property needs to traverse `member` relationship to check `user_id` +1. **Resource-Specific :linked Scope:** CustomFieldValue needs to traverse `member` relationship to check `user_id` 2. **Error Handling:** All errors log for debugging but return generic forbidden to user 3. **Module Name Extraction:** Uses `Module.split() |> List.last()` to match against PermissionSets strings 4. **Pure Function:** No side effects, deterministic, easily testable @@ -966,21 +966,21 @@ end *Email editing has additional validation (see Special Cases) -### Property Resource Policies +### CustomFieldValue Resource Policies -**Location:** `lib/mv/membership/property.ex` +**Location:** `lib/mv/membership/custom_field_value.ex` -**Special Case:** Users can access properties of their linked member. +**Special Case:** Users can access custom field values of their linked member. ```elixir -defmodule Mv.Membership.Property do +defmodule Mv.Membership.CustomFieldValue do use Ash.Resource, ... policies do - # SPECIAL CASE: Users can access properties of their linked member + # SPECIAL CASE: Users can access custom field values of their linked member # Note: This traverses the member relationship! policy action_type([:read, :update]) do - description "Users can access properties of their linked member" + description "Users can access custom field values of their linked member" authorize_if expr(member.user_id == ^actor(:id)) end @@ -1010,18 +1010,18 @@ end | Create | ❌ | ❌ | ✅ | ❌ | ✅ | | Destroy | ❌ | ❌ | ✅ | ❌ | ✅ | -### PropertyType Resource Policies +### CustomField Resource Policies -**Location:** `lib/mv/membership/property_type.ex` +**Location:** `lib/mv/membership/custom_field.ex` **No Special Cases:** All users can read, only admin can write. ```elixir -defmodule Mv.Membership.PropertyType do +defmodule Mv.Membership.CustomField do use Ash.Resource, ... policies do - # All authenticated users can read property types (needed for forms) + # All authenticated users can read custom fields (needed for forms) # Write operations are admin-only policy action_type([:read, :create, :update, :destroy]) do description "Check permissions from user's role" @@ -1308,12 +1308,12 @@ end - ❌ Cannot access: `/members`, `/members/new`, `/admin/roles` **Vorstand (read_only):** -- ✅ Can access: `/`, `/members`, `/members/123`, `/properties`, `/profile` +- ✅ Can access: `/`, `/members`, `/members/123`, `/custom_field_values`, `/profile` - ❌ Cannot access: `/members/new`, `/members/123/edit`, `/admin/roles` **Kassenwart (normal_user):** -- ✅ Can access: `/`, `/members`, `/members/new`, `/members/123/edit`, `/properties`, `/profile` -- ❌ Cannot access: `/admin/roles`, `/admin/property_types/new` +- ✅ Can access: `/`, `/members`, `/members/new`, `/members/123/edit`, `/custom_field_values`, `/profile` +- ❌ Cannot access: `/admin/roles`, `/admin/custom_fields/new` **Admin:** - ✅ Can access: `*` (all pages, including `/admin/roles`) @@ -1479,9 +1479,9 @@ defmodule MvWeb.Authorization do # Direct relationship: member.user_id Map.get(record, :user_id) == user.id - "Property" -> - # Need to traverse: property.member.user_id - # Note: In UI, property should have member preloaded + "CustomFieldValue" -> + # Need to traverse: custom_field_value.member.user_id + # Note: In UI, custom_field_value should have member preloaded case Map.get(record, :member) do %{user_id: member_user_id} -> member_user_id == user.id _ -> false @@ -1569,7 +1569,7 @@ end Admin <% end %> @@ -2409,8 +2409,8 @@ The `HasPermission` check extracts resource names via `Module.split() |> List.la |------------|------------------------| | `Mv.Accounts.User` | "User" | | `Mv.Membership.Member` | "Member" | -| `Mv.Membership.Property` | "Property" | -| `Mv.Membership.PropertyType` | "PropertyType" | +| `Mv.Membership.CustomFieldValue` | "CustomFieldValue" | +| `Mv.Membership.CustomField` | "CustomField" | | `Mv.Authorization.Role` | "Role" | These strings must match exactly in `PermissionSets` module. @@ -2450,7 +2450,7 @@ These strings must match exactly in `PermissionSets` module. **Integration:** - [ ] One complete user journey per role -- [ ] Cross-resource scenarios (e.g., Member -> Property) +- [ ] Cross-resource scenarios (e.g., Member -> CustomFieldValue) - [ ] Special cases in context (e.g., linked member email during full edit flow) ### Useful Commands diff --git a/docs/roles-and-permissions-implementation-plan.md b/docs/roles-and-permissions-implementation-plan.md index 0b173fa..2c29b8d 100644 --- a/docs/roles-and-permissions-implementation-plan.md +++ b/docs/roles-and-permissions-implementation-plan.md @@ -53,7 +53,7 @@ This document defines the implementation plan for the **MVP (Phase 1)** of the R Hardcoded in `Mv.Authorization.PermissionSets` module: 1. **own_data** - User can only access their own data (default for "Mitglied") -2. **read_only** - Read access to all members/properties (for "Vorstand", "Buchhaltung") +2. **read_only** - Read access to all members/custom field values (for "Vorstand", "Buchhaltung") 3. **normal_user** - Create/Read/Update members (no delete), full CRUD on properties (for "Kassenwart") 4. **admin** - Unrestricted access including user/role management (for "Admin") @@ -77,7 +77,7 @@ Stored in database `roles` table, each referencing a `permission_set_name`: - ✅ Hardcoded PermissionSets module with 4 permission sets - ✅ Role database table and CRUD interface - ✅ Custom Ash Policy Check (`HasPermission`) that reads from PermissionSets -- ✅ Policies on all resources (Member, User, Property, PropertyType, Role) +- ✅ Policies on all resources (Member, User, CustomFieldValue, CustomField, Role) - ✅ Page-level permissions via Phoenix Plug - ✅ UI authorization helpers for conditional rendering - ✅ Special case: Member email validation for linked users @@ -228,32 +228,32 @@ Create the core `PermissionSets` module that defines all four permission sets wi - Resources: - User: read/update :own - Member: read/update :linked - - Property: read/update :linked - - PropertyType: read :all + - CustomFieldValue: read/update :linked + - CustomField: read :all - Pages: `["/", "/profile", "/members/:id"]` **2. read_only (Vorstand, Buchhaltung):** - Resources: - User: read :own, update :own - Member: read :all - - Property: read :all - - PropertyType: read :all -- Pages: `["/", "/members", "/members/:id", "/properties"]` + - CustomFieldValue: read :all + - CustomField: read :all +- Pages: `["/", "/members", "/members/:id", "/custom_field_values"]` **3. normal_user (Kassenwart):** - Resources: - User: read/update :own - Member: read/create/update :all (no destroy for safety) - - Property: read/create/update/destroy :all - - PropertyType: read :all -- Pages: `["/", "/members", "/members/new", "/members/:id", "/members/:id/edit", "/properties", "/properties/new", "/properties/:id/edit"]` + - CustomFieldValue: read/create/update/destroy :all + - CustomField: read :all +- Pages: `["/", "/members", "/members/new", "/members/:id", "/members/:id/edit", "/custom_field_values", "/custom_field_values/new", "/custom_field_values/:id/edit"]` **4. admin:** - Resources: - User: read/update/destroy :all - Member: read/create/update/destroy :all - - Property: read/create/update/destroy :all - - PropertyType: read/create/update/destroy :all + - CustomFieldValue: read/create/update/destroy :all + - CustomField: read/create/update/destroy :all - Role: read/create/update/destroy :all - Pages: `["*"]` (wildcard = all pages) @@ -276,10 +276,10 @@ Create the core `PermissionSets` module that defines all four permission sets wi **Permission Content Tests:** - `:own_data` allows User read/update with scope :own -- `:own_data` allows Member/Property read/update with scope :linked -- `:read_only` allows Member/Property read with scope :all -- `:read_only` does NOT allow Member/Property create/update/destroy -- `:normal_user` allows Member/Property full CRUD with scope :all +- `:own_data` allows Member/CustomFieldValue read/update with scope :linked +- `:read_only` allows Member/CustomFieldValue read with scope :all +- `:read_only` does NOT allow Member/CustomFieldValue create/update/destroy +- `:normal_user` allows Member/CustomFieldValue full CRUD with scope :all - `:admin` allows everything with scope :all - `:admin` has wildcard page permission "*" @@ -387,7 +387,7 @@ Create the core custom Ash Policy Check that reads permissions from the `Permiss - `:own` → `{:filter, expr(id == ^actor.id)}` - `:linked` → resource-specific logic: - Member: `{:filter, expr(user_id == ^actor.id)}` - - Property: `{:filter, expr(member.user_id == ^actor.id)}` (traverse relationship!) + - CustomFieldValue: `{:filter, expr(member.user_id == ^actor.id)}` (traverse relationship!) 6. Handle errors gracefully: - No actor → `{:error, :no_actor}` - No role → `{:error, :no_role}` @@ -401,7 +401,7 @@ Create the core custom Ash Policy Check that reads permissions from the `Permiss - [ ] Check module implements `Ash.Policy.Check` behavior - [ ] `match?/3` correctly evaluates permissions from PermissionSets - [ ] Scope filters work correctly (:all, :own, :linked) -- [ ] `:linked` scope handles Member and Property differently +- [ ] `:linked` scope handles Member and CustomFieldValue differently - [ ] Errors are handled gracefully (no crashes) - [ ] Authorization failures are logged - [ ] Module is well-documented @@ -425,7 +425,7 @@ Create the core custom Ash Policy Check that reads permissions from the `Permiss **Scope Application Tests - :linked:** - Actor with scope :linked can access Member where member.user_id == actor.id -- Actor with scope :linked can access Property where property.member.user_id == actor.id (relationship traversal!) +- Actor with scope :linked can access CustomFieldValue where custom_field_value.member.user_id == actor.id (relationship traversal!) - Actor with scope :linked cannot access unlinked member - Query correctly filters based on user_id relationship @@ -581,7 +581,7 @@ Add authorization policies to the User resource. Special case: Users can always --- -#### Issue #9: Property Resource Policies +#### Issue #9: CustomFieldValue Resource Policies **Size:** M (2 days) **Dependencies:** #6 (HasPermission check) @@ -590,20 +590,20 @@ Add authorization policies to the User resource. Special case: Users can always **Description:** -Add authorization policies to the Property resource. Properties are linked to members, which are linked to users. +Add authorization policies to the CustomFieldValue resource. CustomFieldValues are linked to members, which are linked to users. **Tasks:** -1. Open `lib/mv/membership/property.ex` +1. Open `lib/mv/membership/custom_field_value.ex` 2. Add `policies` block -3. Add special policy: Allow user to read/update properties of their linked member +3. Add special policy: Allow user to read/update custom field values of their linked member ```elixir policy action_type([:read, :update]) do authorize_if expr(member.user_id == ^actor(:id)) end ``` 4. Add general policy: Check HasPermission -5. Ensure Property preloads :member relationship for scope checks +5. Ensure CustomFieldValue preloads :member relationship for scope checks 6. Preload :role relationship for actor **Policy Order:** @@ -620,27 +620,27 @@ Add authorization policies to the Property resource. Properties are linked to me **Test Strategy (TDD):** -**Linked Properties Tests (:own_data):** -- User can read properties of their linked member -- User can update properties of their linked member -- User cannot read properties of unlinked members -- Verify relationship traversal works (property.member.user_id) +**Linked CustomFieldValues Tests (:own_data):** +- User can read custom field values of their linked member +- User can update custom field values of their linked member +- User cannot read custom field values of unlinked members +- Verify relationship traversal works (custom_field_value.member.user_id) **Read-Only Tests:** -- User with :read_only can read all properties -- User with :read_only cannot create/update properties +- User with :read_only can read all custom field values +- User with :read_only cannot create/update custom field values **Normal User Tests:** -- User with :normal_user can CRUD properties +- User with :normal_user can CRUD custom field values **Admin Tests:** - Admin can perform all operations -**Test File:** `test/mv/membership/property_policies_test.exs` +**Test File:** `test/mv/membership/custom_field_value_policies_test.exs` --- -#### Issue #10: PropertyType Resource Policies +#### Issue #10: CustomField Resource Policies **Size:** S (1 day) **Dependencies:** #6 (HasPermission check) @@ -649,11 +649,11 @@ Add authorization policies to the Property resource. Properties are linked to me **Description:** -Add authorization policies to the PropertyType resource. PropertyTypes are admin-managed, but readable by all. +Add authorization policies to the CustomField resource. CustomFields are admin-managed, but readable by all. **Tasks:** -1. Open `lib/mv/membership/property_type.ex` +1. Open `lib/mv/membership/custom_field.ex` 2. Add `policies` block 3. Add read policy: All authenticated users can read (scope :all) 4. Add write policies: Only admin can create/update/destroy @@ -661,27 +661,27 @@ Add authorization policies to the PropertyType resource. PropertyTypes are admin **Acceptance Criteria:** -- [ ] All users can read property types -- [ ] Only admin can create/update/destroy property types +- [ ] All users can read custom fields +- [ ] Only admin can create/update/destroy custom fields - [ ] Policies tested **Test Strategy (TDD):** **Read Access (All Roles):** -- User with :own_data can read all property types -- User with :read_only can read all property types -- User with :normal_user can read all property types -- User with :admin can read all property types +- User with :own_data can read all custom fields +- User with :read_only can read all custom fields +- User with :normal_user can read all custom fields +- User with :admin can read all custom fields **Write Access (Admin Only):** -- Non-admin cannot create property type (Forbidden) -- Non-admin cannot update property type (Forbidden) -- Non-admin cannot destroy property type (Forbidden) -- Admin can create property type -- Admin can update property type -- Admin can destroy property type +- Non-admin cannot create custom field (Forbidden) +- Non-admin cannot update custom field (Forbidden) +- Non-admin cannot destroy custom field (Forbidden) +- Admin can create custom field +- Admin can update custom field +- Admin can destroy custom field -**Test File:** `test/mv/membership/property_type_policies_test.exs` +**Test File:** `test/mv/membership/custom_field_policies_test.exs` --- @@ -924,7 +924,7 @@ Create helper functions for UI-level authorization checks. These will be used in ``` 5. All functions use `PermissionSets.get_permissions/1` (same logic as HasPermission) 6. All functions handle nil user gracefully (return false) -7. Implement resource-specific scope checking (Member vs Property for :linked) +7. Implement resource-specific scope checking (Member vs CustomFieldValue for :linked) 8. Add comprehensive `@doc` with template examples 9. Import helper in `mv_web.ex` `html_helpers` section @@ -957,9 +957,9 @@ Create helper functions for UI-level authorization checks. These will be used in **can?/3 with Record Struct - Scope :linked:** - User can update linked Member (member.user_id == user.id) - User cannot update unlinked Member -- User can update Property of linked Member (property.member.user_id == user.id) -- User cannot update Property of unlinked Member -- Scope checking is resource-specific (Member vs Property) +- User can update CustomFieldValue of linked Member (custom_field_value.member.user_id == user.id) +- User cannot update CustomFieldValue of unlinked Member +- Scope checking is resource-specific (Member vs CustomFieldValue) **can_access_page?/2:** - User with page in list can access (returns true) @@ -1046,7 +1046,7 @@ Update Role management LiveViews to use authorization helpers for conditional re **Description:** -Update all existing LiveViews (Member, User, Property, PropertyType) to use authorization helpers for conditional rendering. +Update all existing LiveViews (Member, User, CustomFieldValue, CustomField) to use authorization helpers for conditional rendering. **Tasks:** @@ -1061,10 +1061,10 @@ Update all existing LiveViews (Member, User, Property, PropertyType) to use auth - Show: Only show other users if admin, always show own profile - Edit: Only allow editing own profile or admin editing anyone -3. **Property LiveViews:** +3. **CustomFieldValue LiveViews:** - Similar to Member (hide create/edit/delete based on permissions) -4. **PropertyType LiveViews:** +4. **CustomField LiveViews:** - All users can view - Only admin can create/edit/delete @@ -1110,13 +1110,13 @@ Update all existing LiveViews (Member, User, Property, PropertyType) to use auth - Vorstand: Sees "Home", "Members" (read-only), "Profile" - Kassenwart: Sees "Home", "Members", "Properties", "Profile" - Buchhaltung: Sees "Home", "Members" (read-only), "Profile" -- Admin: Sees "Home", "Members", "Properties", "Property Types", "Admin", "Profile" +- Admin: Sees "Home", "Members", "Custom Field Values", "Custom Fields", "Admin", "Profile" **Test Files:** - `test/mv_web/live/member_live_authorization_test.exs` - `test/mv_web/live/user_live_authorization_test.exs` -- `test/mv_web/live/property_live_authorization_test.exs` -- `test/mv_web/live/property_type_live_authorization_test.exs` +- `test/mv_web/live/custom_field_value_live_authorization_test.exs` +- `test/mv_web/live/custom_field_live_authorization_test.exs` - `test/mv_web/components/navbar_authorization_test.exs` --- @@ -1192,7 +1192,7 @@ Write comprehensive integration tests that follow complete user journeys for eac 4. Can edit any member (except email if linked - see special case) 5. Cannot delete member 6. Can manage properties -7. Cannot manage property types (read-only) +7. Cannot manage custom fields (read-only) 8. Cannot access /admin/roles **Buchhaltung Journey:** @@ -1266,7 +1266,7 @@ Write comprehensive integration tests that follow complete user journeys for eac │ │ │ ┌────▼─────┐ ┌──────▼──────┐ │ │ Issue #9 │ │ Issue #10 │ │ - │ Property │ │ PropType │ │ + │ CustomFieldValue │ │ CustomField │ │ │ Policies │ │ Policies │ │ └────┬─────┘ └──────┬──────┘ │ │ │ │ @@ -1384,8 +1384,8 @@ test/ ├── mv/membership/ │ ├── member_policies_test.exs # Issue #7 │ ├── member_email_validation_test.exs # Issue #12 -│ ├── property_policies_test.exs # Issue #9 -│ └── property_type_policies_test.exs # Issue #10 +│ ├── custom_field_value_policies_test.exs # Issue #9 +│ └── custom_field_policies_test.exs # Issue #10 ├── mv_web/ │ ├── authorization_test.exs # Issue #14 │ ├── plugs/ @@ -1395,8 +1395,8 @@ test/ │ ├── role_live_authorization_test.exs # Issue #15 │ ├── member_live_authorization_test.exs # Issue #16 │ ├── user_live_authorization_test.exs # Issue #16 -│ ├── property_live_authorization_test.exs # Issue #16 -│ └── property_type_live_authorization_test.exs # Issue #16 +│ ├── custom_field_value_live_authorization_test.exs # Issue #16 +│ └── custom_field_live_authorization_test.exs # Issue #16 ├── integration/ │ ├── mitglied_journey_test.exs # Issue #17 │ ├── vorstand_journey_test.exs # Issue #17 diff --git a/docs/roles-and-permissions-overview.md b/docs/roles-and-permissions-overview.md index 191e8b7..86e7273 100644 --- a/docs/roles-and-permissions-overview.md +++ b/docs/roles-and-permissions-overview.md @@ -201,7 +201,7 @@ When runtime permission editing becomes a business requirement, migrate to Appro **Resource Level (MVP):** - Controls create, read, update, destroy actions on resources -- Resources: Member, User, Property, PropertyType, Role +- Resources: Member, User, CustomFieldValue, CustomField, Role **Page Level (MVP):** - Controls access to LiveView pages @@ -280,7 +280,7 @@ Contains: Each Permission Set contains: **Resources:** List of resource permissions -- resource: "Member", "User", "Property", etc. +- resource: "Member", "User", "CustomFieldValue", etc. - action: :read, :create, :update, :destroy - scope: :own, :linked, :all - granted: true/false diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex index dbc62b2..ceedeae 100644 --- a/lib/accounts/user.ex +++ b/lib/accounts/user.ex @@ -17,6 +17,10 @@ defmodule Mv.Accounts.User do # When a member is deleted, set the user's member_id to NULL # This allows users to continue existing even if their linked member is removed reference :member, on_delete: :nilify + + # When a role is deleted, prevent deletion if users are assigned to it + # This protects critical roles from accidental deletion + reference :role, on_delete: :restrict end end @@ -357,6 +361,12 @@ defmodule Mv.Accounts.User do # This automatically creates a `member_id` attribute in the User table # The relationship is optional (allow_nil? true by default) belongs_to :member, Mv.Membership.Member + + # 1:1 relationship - User belongs to a Role + # This automatically creates a `role_id` attribute in the User table + # The relationship is optional (allow_nil? true by default) + # Foreign key constraint: on_delete: :restrict (prevents deleting roles assigned to users) + belongs_to :role, Mv.Authorization.Role end identities do diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 1d6d96e..d2ea07d 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -5,7 +5,7 @@ defmodule Mv.Membership.Member do ## Overview Members are the core entity in the membership management system. Each member can have: - - Personal information (name, email, phone, address) + - Personal information (name, email, address) - Optional link to a User account (1:1 relationship) - Dynamic custom field values via CustomField system - Full-text searchable profile @@ -20,9 +20,8 @@ defmodule Mv.Membership.Member do - `has_one :user` - Optional authentication account link ## Validations - - Required: first_name, last_name, email + - Required: email (all other fields are optional) - Email format validation (using EctoCommons.EmailValidator) - - Phone number format: international format with 6-20 digits - Postal code format: exactly 5 digits (German format) - Date validations: join_date not in future, exit_date after join_date - Email uniqueness: prevents conflicts with unlinked users @@ -31,7 +30,7 @@ defmodule Mv.Membership.Member do Members have a `search_vector` attribute (tsvector) that is automatically updated via database trigger. Search includes name, email, notes, contact fields, and all custom field values. Custom field values are automatically included in - the search vector with weight 'C' (same as phone_number, city, etc.). + the search vector with weight 'C' (same as city, etc.). """ use Ash.Resource, domain: Mv.Membership, @@ -41,6 +40,8 @@ defmodule Mv.Membership.Member do import Ash.Expr require Logger + alias Mv.Membership.Helpers.VisibilityConfig + # Module constants @member_search_limit 10 @@ -343,9 +344,7 @@ defmodule Mv.Membership.Member do validations do # Required fields are covered by allow_nil? false - # First name and last name must not be empty - validate present(:first_name) - validate present(:last_name) + # Email is required validate present(:email) # Email uniqueness check for all actions that change the email attribute @@ -396,11 +395,6 @@ defmodule Mv.Membership.Member do where: [present([:join_date, :exit_date])], message: "cannot be before join date" - # Phone number format (only if set) - validate match(:phone_number, ~r/^\+?[0-9\- ]{6,20}$/), - where: [present(:phone_number)], - message: "is not a valid phone number" - # Postal code format (only if set) validate match(:postal_code, ~r/^\d{5}$/), where: [present(:postal_code)], @@ -453,12 +447,12 @@ defmodule Mv.Membership.Member do uuid_v7_primary_key :id attribute :first_name, :string do - allow_nil? false + allow_nil? true constraints min_length: 1 end attribute :last_name, :string do - allow_nil? false + allow_nil? true constraints min_length: 1 end @@ -474,10 +468,6 @@ defmodule Mv.Membership.Member do constraints min_length: 5, max_length: 254 end - attribute :phone_number, :string do - allow_nil? true - end - attribute :join_date, :date do allow_nil? true end @@ -612,18 +602,21 @@ defmodule Mv.Membership.Member do """ @spec show_in_overview?(atom()) :: boolean() def show_in_overview?(field) when is_atom(field) do + # exit_date defaults to false (hidden) instead of true + default_visibility = if field == :exit_date, do: false, else: true + case Mv.Membership.get_settings() do {:ok, settings} -> visibility_config = settings.member_field_visibility || %{} # Normalize map keys to atoms (JSONB may return string keys) - normalized_config = normalize_visibility_config(visibility_config) + normalized_config = VisibilityConfig.normalize(visibility_config) - # Get value from normalized config, default to true - Map.get(normalized_config, field, true) + # Get value from normalized config, use field-specific default + Map.get(normalized_config, field, default_visibility) {:error, _} -> - # If settings can't be loaded, default to visible - true + # If settings can't be loaded, use field-specific default + default_visibility end end @@ -968,29 +961,6 @@ defmodule Mv.Membership.Member do defp error_type(error) when is_atom(error), do: error defp error_type(_), do: :unknown - # Normalizes visibility config map keys from strings to atoms. - # JSONB in PostgreSQL converts atom keys to string keys when storing. - defp normalize_visibility_config(config) when is_map(config) do - Enum.reduce(config, %{}, fn - {key, value}, acc when is_atom(key) -> - Map.put(acc, key, value) - - {key, value}, acc when is_binary(key) -> - try do - atom_key = String.to_existing_atom(key) - Map.put(acc, atom_key, value) - rescue - ArgumentError -> - acc - end - - _, acc -> - acc - end) - end - - defp normalize_visibility_config(_), do: %{} - @doc """ Performs fuzzy search on members using PostgreSQL trigram similarity. @@ -1073,7 +1043,6 @@ defmodule Mv.Membership.Member do expr( contains(postal_code, ^query) or contains(house_number, ^query) or - contains(phone_number, ^query) or contains(email, ^query) or contains(city, ^query) ) diff --git a/lib/membership/membership.ex b/lib/membership/membership.ex index 4917c7c..982b837 100644 --- a/lib/membership/membership.ex +++ b/lib/membership/membership.ex @@ -57,6 +57,9 @@ defmodule Mv.Membership do # Settings should be created via seed script define :update_settings, action: :update define :update_member_field_visibility, action: :update_member_field_visibility + + define :update_single_member_field_visibility, + action: :update_single_member_field_visibility end end @@ -89,7 +92,10 @@ defmodule Mv.Membership do default_club_name = System.get_env("ASSOCIATION_NAME") || "Club Name" Mv.Membership.Setting - |> Ash.Changeset.for_create(:create, %{club_name: default_club_name}) + |> Ash.Changeset.for_create(:create, %{ + club_name: default_club_name, + member_field_visibility: %{"exit_date" => false} + }) |> Ash.create!(domain: __MODULE__) |> then(fn settings -> {:ok, settings} end) @@ -183,4 +189,42 @@ defmodule Mv.Membership do }) |> Ash.update(domain: __MODULE__) end + + @doc """ + Atomically updates a single field in the member field visibility configuration. + + This action uses PostgreSQL's jsonb_set function to atomically update a single key + in the JSONB map, preventing lost updates in concurrent scenarios. This is the + preferred method for updating individual field visibility settings. + + ## Parameters + + - `settings` - The settings record to update + - `field` - The member field name as a string (e.g., "street", "house_number") + - `show_in_overview` - Boolean value indicating visibility + + ## Returns + + - `{:ok, updated_settings}` - Successfully updated settings + - `{:error, error}` - Validation or update error + + ## Examples + + iex> {:ok, settings} = Mv.Membership.get_settings() + iex> {:ok, updated} = Mv.Membership.update_single_member_field_visibility(settings, field: "street", show_in_overview: false) + iex> updated.member_field_visibility["street"] + false + + """ + def update_single_member_field_visibility(settings, + field: field, + show_in_overview: show_in_overview + ) do + settings + |> Ash.Changeset.new() + |> Ash.Changeset.set_argument(:field, field) + |> Ash.Changeset.set_argument(:show_in_overview, show_in_overview) + |> Ash.Changeset.for_update(:update_single_member_field_visibility, %{}) + |> Ash.update(domain: __MODULE__) + end end diff --git a/lib/membership/setting.ex b/lib/membership/setting.ex index eedc47c..4ba0794 100644 --- a/lib/membership/setting.ex +++ b/lib/membership/setting.ex @@ -91,6 +91,16 @@ defmodule Mv.Membership.Setting do accept [:member_field_visibility] end + update :update_single_member_field_visibility do + description "Atomically updates a single field in the member_field_visibility JSONB map" + require_atomic? false + + argument :field, :string, allow_nil?: false + argument :show_in_overview, :boolean, allow_nil?: false + + change Mv.Membership.Setting.Changes.UpdateSingleMemberFieldVisibility + end + update :update_membership_fee_settings do description "Updates the membership fee configuration" require_atomic? false diff --git a/lib/membership/setting/changes/update_single_member_field_visibility.ex b/lib/membership/setting/changes/update_single_member_field_visibility.ex new file mode 100644 index 0000000..e047cdf --- /dev/null +++ b/lib/membership/setting/changes/update_single_member_field_visibility.ex @@ -0,0 +1,164 @@ +defmodule Mv.Membership.Setting.Changes.UpdateSingleMemberFieldVisibility do + @moduledoc """ + Ash change that atomically updates a single field in the member_field_visibility JSONB map. + + This change uses PostgreSQL's jsonb_set function to atomically update a single key + in the JSONB map, preventing lost updates in concurrent scenarios. + + ## Arguments + - `field` - The member field name as a string (e.g., "street", "house_number") + - `show_in_overview` - Boolean value indicating visibility + + ## Example + settings + |> Ash.Changeset.for_update(:update_single_member_field_visibility, + %{}, + arguments: %{field: "street", show_in_overview: false} + ) + |> Ash.update(domain: Mv.Membership) + """ + use Ash.Resource.Change + + alias Ash.Error.Invalid + alias Ecto.Adapters.SQL + require Logger + + def change(changeset, _opts, _context) do + with {:ok, field} <- get_and_validate_field(changeset), + {:ok, show_in_overview} <- get_and_validate_boolean(changeset, :show_in_overview) do + add_after_action(changeset, field, show_in_overview) + else + {:error, updated_changeset} -> updated_changeset + end + end + + defp get_and_validate_field(changeset) do + case Ash.Changeset.get_argument(changeset, :field) do + nil -> + {:error, + add_error(changeset, + field: :member_field_visibility, + message: "field argument is required" + )} + + field -> + valid_fields = Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1) + + if field in valid_fields do + {:ok, field} + else + {:error, + add_error( + changeset, + field: :member_field_visibility, + message: "Invalid member field: #{field}" + )} + end + end + end + + defp get_and_validate_boolean(changeset, arg_name) do + case Ash.Changeset.get_argument(changeset, arg_name) do + nil -> + {:error, + add_error( + changeset, + field: :member_field_visibility, + message: "#{arg_name} argument is required" + )} + + value when is_boolean(value) -> + {:ok, value} + + _ -> + {:error, + add_error( + changeset, + field: :member_field_visibility, + message: "#{arg_name} must be a boolean" + )} + end + end + + defp add_error(changeset, opts) do + Ash.Changeset.add_error(changeset, opts) + end + + defp add_after_action(changeset, field, show_in_overview) do + # Use after_action to execute atomic SQL update + Ash.Changeset.after_action(changeset, fn _changeset, settings -> + # Use PostgreSQL jsonb_set for atomic update + # jsonb_set(target, path, new_value, create_missing?) + # path is an array: ['field_name'] + # new_value must be JSON: to_jsonb(boolean) + sql = """ + UPDATE settings + SET member_field_visibility = jsonb_set( + COALESCE(member_field_visibility, '{}'::jsonb), + ARRAY[$1::text], + to_jsonb($2::boolean), + true + ) + WHERE id = $3 + RETURNING member_field_visibility + """ + + # Convert UUID string to binary for PostgreSQL + uuid_binary = Ecto.UUID.dump!(settings.id) + + case SQL.query(Mv.Repo, sql, [field, show_in_overview, uuid_binary]) do + {:ok, %{rows: [[updated_jsonb] | _]}} -> + updated_visibility = normalize_jsonb_result(updated_jsonb) + + # Update the settings struct with the new visibility + updated_settings = %{settings | member_field_visibility: updated_visibility} + {:ok, updated_settings} + + {:ok, %{rows: []}} -> + {:error, + Invalid.exception( + field: :member_field_visibility, + message: "Settings not found" + )} + + {:error, error} -> + Logger.error("Failed to atomically update member_field_visibility: #{inspect(error)}") + + {:error, + Invalid.exception( + field: :member_field_visibility, + message: "Failed to update visibility" + )} + end + end) + end + + defp normalize_jsonb_result(updated_jsonb) do + case updated_jsonb do + map when is_map(map) -> + # Convert atom keys to strings if needed + Enum.reduce(map, %{}, fn + {k, v}, acc when is_atom(k) -> Map.put(acc, Atom.to_string(k), v) + {k, v}, acc -> Map.put(acc, k, v) + end) + + binary when is_binary(binary) -> + case Jason.decode(binary) do + {:ok, decoded} when is_map(decoded) -> + decoded + + # Not a map after decode + {:ok, _} -> + %{} + + {:error, reason} -> + Logger.warning("Failed to decode JSONB: #{inspect(reason)}") + %{} + end + + _ -> + Logger.warning("Unexpected JSONB format: #{inspect(updated_jsonb)}") + %{} + end + end +end diff --git a/lib/mv/authorization/authorization.ex b/lib/mv/authorization/authorization.ex new file mode 100644 index 0000000..aac07a9 --- /dev/null +++ b/lib/mv/authorization/authorization.ex @@ -0,0 +1,31 @@ +defmodule Mv.Authorization do + @moduledoc """ + Ash Domain for authorization and role management. + + ## Resources + - `Role` - User roles that reference permission sets + + ## Public API + The domain exposes these main actions: + - Role CRUD: `create_role/1`, `list_roles/0`, `update_role/2`, `destroy_role/1` + + ## Admin Interface + The domain is configured with AshAdmin for management UI. + """ + use Ash.Domain, + extensions: [AshAdmin.Domain, AshPhoenix] + + admin do + show? true + end + + resources do + resource Mv.Authorization.Role do + define :create_role, action: :create_role + define :list_roles, action: :read + define :get_role, action: :read, get_by: [:id] + define :update_role, action: :update_role + define :destroy_role, action: :destroy + end + end +end diff --git a/lib/mv/authorization/checks/has_permission.ex b/lib/mv/authorization/checks/has_permission.ex new file mode 100644 index 0000000..345d6e4 --- /dev/null +++ b/lib/mv/authorization/checks/has_permission.ex @@ -0,0 +1,239 @@ +defmodule Mv.Authorization.Checks.HasPermission do + @moduledoc """ + Custom Ash Policy Check that evaluates permissions from the PermissionSets module. + + This check: + 1. Reads the actor's role and permission_set_name + 2. Looks up permissions from PermissionSets.get_permissions/1 + 3. Finds matching permission for current resource + action + 4. Applies scope filter (:own, :linked, :all) + + ## Usage in Ash Resource + + policies do + policy action_type(:read) do + authorize_if Mv.Authorization.Checks.HasPermission + end + end + + ## Scope Behavior + + - **:all** - Authorizes without filtering (returns all records) + - **:own** - Filters to records where record.id == actor.id + - **:linked** - Filters based on resource type: + - Member: member.user.id == actor.id (via has_one :user relationship) + - CustomFieldValue: custom_field_value.member.user.id == actor.id (traverses member → user relationship!) + + ## Error Handling + + Returns `false` for: + - Missing actor + - Actor without role + - Invalid permission_set_name + - No matching permission found + + All errors result in Forbidden (policy fails). + + ## Examples + + # In a resource policy + policies do + policy action_type([:read, :create, :update, :destroy]) do + authorize_if Mv.Authorization.Checks.HasPermission + end + end + """ + + use Ash.Policy.Check + require Ash.Query + import Ash.Expr + alias Mv.Authorization.PermissionSets + require Logger + + @impl true + def describe(_opts) do + "checks if actor has permission via their role's permission set" + end + + @impl true + def strict_check(actor, authorizer, _opts) do + resource = authorizer.resource + action = get_action_from_authorizer(authorizer) + + cond do + is_nil(actor) -> + log_auth_failure(actor, resource, action, "no actor") + {:ok, false} + + is_nil(action) -> + log_auth_failure( + actor, + resource, + action, + "authorizer subject shape unsupported (no action)" + ) + + {:ok, false} + + true -> + strict_check_with_permissions(actor, resource, action) + end + end + + # Helper function to reduce nesting depth + defp strict_check_with_permissions(actor, resource, action) do + 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), + resource_name <- get_resource_name(resource) do + case check_permission( + permissions.resources, + resource_name, + action, + actor, + resource_name + ) do + :authorized -> {:ok, true} + {:filter, _} -> {:ok, :unknown} + false -> {:ok, false} + end + else + %{role: nil} -> + log_auth_failure(actor, resource, action, "no role assigned") + {:ok, false} + + %{role: %{permission_set_name: nil}} -> + log_auth_failure(actor, resource, action, "role has no permission_set_name") + {:ok, false} + + {:error, :invalid_permission_set} -> + log_auth_failure(actor, resource, action, "invalid permission_set_name") + {:ok, false} + + _ -> + log_auth_failure(actor, resource, action, "missing data") + {:ok, false} + end + end + + @impl true + def auto_filter(actor, authorizer, _opts) do + resource = authorizer.resource + action = get_action_from_authorizer(authorizer) + + cond do + is_nil(actor) -> nil + is_nil(action) -> nil + true -> auto_filter_with_permissions(actor, resource, action) + end + end + + # Helper function to reduce nesting depth + defp auto_filter_with_permissions(actor, resource, action) do + 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), + resource_name <- get_resource_name(resource) do + case check_permission( + permissions.resources, + resource_name, + action, + actor, + resource_name + ) do + :authorized -> nil + {:filter, filter_expr} -> filter_expr + false -> nil + end + else + _ -> nil + end + end + + # Helper to extract action from authorizer + defp get_action_from_authorizer(authorizer) do + case authorizer.subject do + %{action: %{name: action}} -> action + %{action: action} when is_atom(action) -> action + _ -> nil + end + end + + # Extract resource name from module (e.g., Mv.Membership.Member -> "Member") + defp get_resource_name(resource) when is_atom(resource) do + resource |> Module.split() |> List.last() + end + + # Find matching permission and apply scope + defp check_permission(resource_perms, resource_name, action, actor, resource_name_for_logging) do + case Enum.find(resource_perms, fn perm -> + perm.resource == resource_name and perm.action == action and perm.granted + end) do + nil -> + log_auth_failure(actor, resource_name_for_logging, action, "no matching permission found") + false + + perm -> + apply_scope(perm.scope, actor, resource_name) + end + end + + # Scope: all - No filtering, access to all records + defp apply_scope(:all, _actor, _resource) do + :authorized + end + + # Scope: own - Filter to records where record.id == actor.id + # Used for User resource (users can access their own user record) + defp apply_scope(:own, actor, _resource) do + {:filter, expr(id == ^actor.id)} + end + + # Scope: linked - Filter based on user relationship (resource-specific!) + # Uses Ash relationships: Member has_one :user, CustomFieldValue belongs_to :member + defp apply_scope(:linked, actor, resource_name) do + case resource_name do + "Member" -> + # Member has_one :user → filter by user.id == actor.id + {:filter, expr(user.id == ^actor.id)} + + "CustomFieldValue" -> + # CustomFieldValue belongs_to :member → member has_one :user + # Traverse: custom_field_value.member.user.id == actor.id + {:filter, expr(member.user.id == ^actor.id)} + + _ -> + # Fallback for other resources: try user relationship first, then user_id + {:filter, expr(user.id == ^actor.id or user_id == ^actor.id)} + end + end + + # Log authorization failures for debugging (lazy evaluation) + defp log_auth_failure(actor, resource, action, reason) do + Logger.debug(fn -> + actor_id = if is_map(actor), do: Map.get(actor, :id), else: "nil" + resource_name = get_resource_name_for_logging(resource) + + """ + Authorization failed: + Actor: #{actor_id} + Resource: #{resource_name} + Action: #{inspect(action)} + Reason: #{reason} + """ + end) + end + + # Helper to extract resource name for logging (handles both atoms and strings) + defp get_resource_name_for_logging(resource) when is_atom(resource) do + resource |> Module.split() |> List.last() + end + + defp get_resource_name_for_logging(resource) when is_binary(resource) do + resource + end + + defp get_resource_name_for_logging(_resource) do + "unknown" + end +end diff --git a/lib/mv/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex new file mode 100644 index 0000000..11ddb5a --- /dev/null +++ b/lib/mv/authorization/permission_sets.ex @@ -0,0 +1,294 @@ +defmodule Mv.Authorization.PermissionSets do + @moduledoc """ + Defines the four hardcoded permission sets for the application. + + Each permission set specifies: + - Resource permissions (what CRUD operations on which resources) + - Page permissions (which LiveView pages can be accessed) + - Scopes (own, linked, all) + + ## Permission Sets + + 1. **own_data** - Default for "Mitglied" role + - Can only access own user data and linked member/custom field values + - Cannot create new members or manage system + + 2. **read_only** - For "Vorstand" and "Buchhaltung" roles + - Can read all member data + - Cannot create, update, or delete + + 3. **normal_user** - For "Kassenwart" role + - Create/Read/Update members (no delete for safety), full CRUD on custom field values + - Cannot manage custom fields or users + + 4. **admin** - For "Admin" role + - Unrestricted access to all resources + - Can manage users, roles, custom fields + + ## Usage + + # Get permissions for a role's permission set + permissions = PermissionSets.get_permissions(:admin) + + # Check if a permission set name is valid + PermissionSets.valid_permission_set?("read_only") # => true + + # Convert string to atom safely + {:ok, atom} = PermissionSets.permission_set_name_to_atom("own_data") + + ## Performance + + All functions are pure and intended to be constant-time. Permission lookups + are very fast (typically < 1 microsecond in practice) as they are simple + pattern matches and map lookups with no database queries or external calls. + """ + + @type scope :: :own | :linked | :all + @type action :: :read | :create | :update | :destroy + + @type resource_permission :: %{ + resource: String.t(), + action: action(), + scope: scope(), + granted: boolean() + } + + @type permission_set :: %{ + resources: [resource_permission()], + pages: [String.t()] + } + + @doc """ + Returns the list of all valid permission set names. + + ## Examples + + iex> PermissionSets.all_permission_sets() + [:own_data, :read_only, :normal_user, :admin] + """ + @spec all_permission_sets() :: [atom()] + def all_permission_sets do + [:own_data, :read_only, :normal_user, :admin] + end + + @doc """ + Returns permissions for the given permission set. + + ## Examples + + iex> permissions = PermissionSets.get_permissions(:admin) + iex> Enum.any?(permissions.resources, fn p -> + ...> p.resource == "User" and p.action == :destroy + ...> end) + true + + iex> PermissionSets.get_permissions(:invalid) + ** (ArgumentError) invalid permission set: :invalid. Must be one of: [:own_data, :read_only, :normal_user, :admin] + """ + @spec get_permissions(atom()) :: permission_set() + + def get_permissions(set) when set not in [:own_data, :read_only, :normal_user, :admin] do + raise ArgumentError, + "invalid permission set: #{inspect(set)}. Must be one of: #{inspect(all_permission_sets())}" + end + + def get_permissions(:own_data) do + %{ + resources: [ + # User: Can always read/update own credentials + %{resource: "User", action: :read, scope: :own, granted: true}, + %{resource: "User", action: :update, scope: :own, granted: true}, + + # Member: Can read/update linked member + %{resource: "Member", action: :read, scope: :linked, granted: true}, + %{resource: "Member", action: :update, scope: :linked, granted: true}, + + # CustomFieldValue: Can read/update custom field values of linked member + %{resource: "CustomFieldValue", action: :read, scope: :linked, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :linked, granted: true}, + + # CustomField: Can read all (needed for forms) + %{resource: "CustomField", action: :read, scope: :all, granted: true} + ], + pages: [ + # Home page + "/", + # Own profile + "/profile", + # Linked member detail (filtered by policy) + "/members/:id" + ] + } + end + + def get_permissions(:read_only) do + %{ + resources: [ + # User: Can read/update own credentials only + %{resource: "User", action: :read, scope: :own, granted: true}, + %{resource: "User", action: :update, scope: :own, granted: true}, + + # Member: Can read all members, no modifications + %{resource: "Member", action: :read, scope: :all, granted: true}, + + # CustomFieldValue: Can read all custom field values + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, + + # CustomField: Can read all + %{resource: "CustomField", action: :read, scope: :all, granted: true} + ], + pages: [ + "/", + # Own profile + "/profile", + # Member list + "/members", + # Member detail + "/members/:id", + # Custom field values overview + "/custom_field_values", + # Custom field value detail + "/custom_field_values/:id" + ] + } + end + + def get_permissions(:normal_user) do + %{ + resources: [ + # User: Can read/update own credentials only + %{resource: "User", action: :read, scope: :own, granted: true}, + %{resource: "User", action: :update, scope: :own, granted: true}, + + # Member: Full CRUD except destroy (safety) + %{resource: "Member", action: :read, scope: :all, granted: true}, + %{resource: "Member", action: :create, scope: :all, granted: true}, + %{resource: "Member", action: :update, scope: :all, granted: true}, + # Note: destroy intentionally omitted for safety + + # CustomFieldValue: Full CRUD + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true}, + + # CustomField: Read only (admin manages definitions) + %{resource: "CustomField", action: :read, scope: :all, granted: true} + ], + pages: [ + "/", + # Own profile + "/profile", + "/members", + # Create member + "/members/new", + "/members/:id", + # Edit member + "/members/:id/edit", + "/custom_field_values", + # Custom field value detail + "/custom_field_values/:id", + "/custom_field_values/new", + "/custom_field_values/:id/edit" + ] + } + end + + def get_permissions(:admin) do + %{ + resources: [ + # User: Full management including other users + %{resource: "User", action: :read, scope: :all, granted: true}, + %{resource: "User", action: :create, scope: :all, granted: true}, + %{resource: "User", action: :update, scope: :all, granted: true}, + %{resource: "User", action: :destroy, scope: :all, granted: true}, + + # Member: Full CRUD + %{resource: "Member", action: :read, scope: :all, granted: true}, + %{resource: "Member", action: :create, scope: :all, granted: true}, + %{resource: "Member", action: :update, scope: :all, granted: true}, + %{resource: "Member", action: :destroy, scope: :all, granted: true}, + + # CustomFieldValue: Full CRUD + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true}, + + # CustomField: Full CRUD (admin manages custom field definitions) + %{resource: "CustomField", action: :read, scope: :all, granted: true}, + %{resource: "CustomField", action: :create, scope: :all, granted: true}, + %{resource: "CustomField", action: :update, scope: :all, granted: true}, + %{resource: "CustomField", action: :destroy, scope: :all, granted: true}, + + # Role: Full CRUD (admin manages roles) + %{resource: "Role", action: :read, scope: :all, granted: true}, + %{resource: "Role", action: :create, scope: :all, granted: true}, + %{resource: "Role", action: :update, scope: :all, granted: true}, + %{resource: "Role", action: :destroy, scope: :all, granted: true} + ], + pages: [ + # Wildcard: Admin can access all pages + "*" + ] + } + end + + def get_permissions(invalid) do + raise ArgumentError, + "invalid permission set: #{inspect(invalid)}. Must be one of: #{inspect(all_permission_sets())}" + end + + @doc """ + Checks if a permission set name (string or atom) is valid. + + ## Examples + + iex> PermissionSets.valid_permission_set?("admin") + true + + iex> PermissionSets.valid_permission_set?(:read_only) + true + + iex> PermissionSets.valid_permission_set?("invalid") + false + """ + @spec valid_permission_set?(any()) :: boolean() + def valid_permission_set?(name) when is_binary(name) do + case permission_set_name_to_atom(name) do + {:ok, _atom} -> true + {:error, _} -> false + end + end + + def valid_permission_set?(name) when is_atom(name) do + name in all_permission_sets() + end + + def valid_permission_set?(_), do: false + + @doc """ + Converts a permission set name string to atom safely. + + ## Examples + + iex> PermissionSets.permission_set_name_to_atom("admin") + {:ok, :admin} + + iex> PermissionSets.permission_set_name_to_atom("invalid") + {:error, :invalid_permission_set} + """ + @spec permission_set_name_to_atom(String.t()) :: + {:ok, atom()} | {:error, :invalid_permission_set} + def permission_set_name_to_atom(name) when is_binary(name) do + atom = String.to_existing_atom(name) + + if valid_permission_set?(atom) do + {:ok, atom} + else + {:error, :invalid_permission_set} + end + rescue + ArgumentError -> {:error, :invalid_permission_set} + end +end diff --git a/lib/mv/authorization/role.ex b/lib/mv/authorization/role.ex new file mode 100644 index 0000000..da43510 --- /dev/null +++ b/lib/mv/authorization/role.ex @@ -0,0 +1,142 @@ +defmodule Mv.Authorization.Role do + @moduledoc """ + Represents a user role that references a permission set. + + Roles are stored in the database and link users to permission sets. + Each role has a `permission_set_name` that references one of the four + hardcoded permission sets defined in `Mv.Authorization.PermissionSets`. + + ## Fields + + - `name` - Unique role name (e.g., "Vorstand", "Admin") + - `description` - Human-readable description of the role + - `permission_set_name` - Must be one of: "own_data", "read_only", "normal_user", "admin" + - `is_system_role` - If true, role cannot be deleted (protects critical roles like "Mitglied") + + ## Relationships + + - `has_many :users` - Users assigned to this role + + ## Validations + + - `permission_set_name` must be a valid permission set (checked against PermissionSets.all_permission_sets/0) + - `name` must be unique + - System roles cannot be deleted (enforced via validation) + + ## Examples + + # Create a new role + {:ok, role} = Mv.Authorization.create_role(%{ + name: "Vorstand", + description: "Board member with read access", + permission_set_name: "read_only" + }) + + # List all roles + {:ok, roles} = Mv.Authorization.list_roles() + """ + use Ash.Resource, + domain: Mv.Authorization, + data_layer: AshPostgres.DataLayer + + postgres do + table "roles" + repo Mv.Repo + + references do + # Prevent deletion of roles that are assigned to users + reference :users, on_delete: :restrict + end + end + + code_interface do + define :create_role + define :list_roles, action: :read + define :update_role + define :destroy_role, action: :destroy + end + + actions do + defaults [:read] + + create :create_role do + primary? true + # is_system_role is intentionally excluded - should only be set via seeds/internal actions + accept [:name, :description, :permission_set_name] + # Note: In Ash 3.0, require_atomic? is not available for create actions + # Custom validations will still work + end + + update :update_role do + primary? true + # is_system_role is intentionally excluded - should only be set via seeds/internal actions + accept [:name, :description, :permission_set_name] + # Required because custom validation functions cannot be executed atomically + require_atomic? false + end + + destroy :destroy do + # Required because custom validation functions cannot be executed atomically + require_atomic? false + end + end + + validations do + validate one_of( + :permission_set_name, + Mv.Authorization.PermissionSets.all_permission_sets() + |> Enum.map(&Atom.to_string/1) + ), + message: + "must be one of: #{Mv.Authorization.PermissionSets.all_permission_sets() |> Enum.map_join(", ", &Atom.to_string/1)}" + + validate fn changeset, _context -> + if changeset.data.is_system_role do + {:error, + field: :is_system_role, + message: + "Cannot delete system role. System roles are required for the application to function."} + else + :ok + end + end, + on: [:destroy] + end + + attributes do + uuid_v7_primary_key :id + + attribute :name, :string do + allow_nil? false + public? true + end + + attribute :description, :string do + allow_nil? true + public? true + end + + attribute :permission_set_name, :string do + allow_nil? false + public? true + end + + attribute :is_system_role, :boolean do + allow_nil? false + default false + public? true + end + + timestamps() + end + + relationships do + has_many :users, Mv.Accounts.User do + destination_attribute :role_id + end + end + + identities do + identity :unique_name, [:name] + end +end diff --git a/lib/mv/constants.ex b/lib/mv/constants.ex index c81dbd6..82a8400 100644 --- a/lib/mv/constants.ex +++ b/lib/mv/constants.ex @@ -7,7 +7,6 @@ defmodule Mv.Constants do :first_name, :last_name, :email, - :phone_number, :join_date, :exit_date, :notes, diff --git a/lib/mv/helpers/type_parsers.ex b/lib/mv/helpers/type_parsers.ex new file mode 100644 index 0000000..6c07e6e --- /dev/null +++ b/lib/mv/helpers/type_parsers.ex @@ -0,0 +1,49 @@ +defmodule Mv.Helpers.TypeParsers do + @moduledoc """ + Helper functions for parsing various input types to common Elixir types. + + Provides safe parsing functions for common type conversions, especially useful + when dealing with form data or external APIs. + """ + + @doc """ + Parses various input types to boolean. + + Handles: booleans, strings ("true"/"false"), integers (1/0), and other values (defaults to false). + + ## Parameters + + - `value` - The value to parse (boolean, string, integer, or other) + + ## Returns + + A boolean value + + ## Examples + + iex> parse_boolean(true) + true + + iex> parse_boolean("true") + true + + iex> parse_boolean("false") + false + + iex> parse_boolean(1) + true + + iex> parse_boolean(0) + false + + iex> parse_boolean(nil) + false + """ + @spec parse_boolean(any()) :: boolean() + def parse_boolean(value) when is_boolean(value), do: value + def parse_boolean("true"), do: true + def parse_boolean("false"), do: false + def parse_boolean(1), do: true + def parse_boolean(0), do: false + def parse_boolean(_), do: false +end diff --git a/lib/mv/membership/helpers/visibility_config.ex b/lib/mv/membership/helpers/visibility_config.ex new file mode 100644 index 0000000..886d575 --- /dev/null +++ b/lib/mv/membership/helpers/visibility_config.ex @@ -0,0 +1,55 @@ +defmodule Mv.Membership.Helpers.VisibilityConfig do + @moduledoc """ + Helper functions for normalizing member field visibility configuration. + + Handles conversion between string keys (from JSONB) and atom keys (Elixir convention). + JSONB in PostgreSQL converts atom keys to string keys when storing. + This module provides functions to normalize these back to atoms for Elixir usage. + """ + + @doc """ + Normalizes visibility config map keys from strings to atoms. + + JSONB in PostgreSQL converts atom keys to string keys when storing. + This function converts them back to atoms for Elixir usage. + + ## Parameters + + - `config` - A map with either string or atom keys + + ## Returns + + A map with atom keys (where possible) + + ## Examples + + iex> normalize(%{"first_name" => true, "email" => false}) + %{first_name: true, email: false} + + iex> normalize(%{first_name: true, email: false}) + %{first_name: true, email: false} + + iex> normalize(%{"invalid_field" => true}) + %{} + """ + @spec normalize(map()) :: map() + def normalize(config) when is_map(config) do + Enum.reduce(config, %{}, fn + {key, value}, acc when is_atom(key) -> + Map.put(acc, key, value) + + {key, value}, acc when is_binary(key) -> + try do + atom_key = String.to_existing_atom(key) + Map.put(acc, atom_key, value) + rescue + ArgumentError -> acc + end + + _, acc -> + acc + end) + end + + def normalize(_), do: %{} +end diff --git a/lib/mv_web.ex b/lib/mv_web.ex index 46e4e8b..8589be1 100644 --- a/lib/mv_web.ex +++ b/lib/mv_web.ex @@ -89,6 +89,9 @@ defmodule MvWeb do # Core UI components import MvWeb.CoreComponents + # Authorization helpers + import MvWeb.Authorization, only: [can?: 3, can_access_page?: 2] + # Common modules used in templates alias Phoenix.LiveView.JS alias MvWeb.Layouts diff --git a/lib/mv_web/authorization.ex b/lib/mv_web/authorization.ex new file mode 100644 index 0000000..95a8524 --- /dev/null +++ b/lib/mv_web/authorization.ex @@ -0,0 +1,206 @@ +defmodule MvWeb.Authorization do + @moduledoc """ + UI-level authorization helpers for LiveView templates. + + These functions check if the current user has permission to perform actions + or access pages. They use the same PermissionSets module as the backend policies, + ensuring UI and backend authorization are consistent. + + ## Usage in Templates + + + <%= if can?(@current_user, :create, Mv.Membership.Member) do %> + <.link patch={~p"/members/new"}>New Member + <% end %> + + + <%= if can?(@current_user, :update, @member) do %> + <.button>Edit + <% end %> + + + <%= if can_access_page?(@current_user, "/admin/roles") do %> + <.link navigate="/admin/roles">Manage Roles + <% end %> + + ## Performance + + All checks are pure function calls using the hardcoded PermissionSets module. + No database queries, < 1 microsecond per check. + """ + + alias Mv.Authorization.PermissionSets + + @doc """ + Checks if user has permission for an action on a resource. + + This function has two variants: + 1. Resource atom: Checks if user has permission for action on resource type + 2. Record struct: Checks if user has permission for action on specific record (with scope checking) + + ## Examples + + # Resource-level check (atom) + iex> admin = %{role: %{permission_set_name: "admin"}} + iex> can?(admin, :create, Mv.Membership.Member) + true + + iex> mitglied = %{role: %{permission_set_name: "own_data"}} + iex> can?(mitglied, :create, Mv.Membership.Member) + false + + # Record-level check (struct with scope) + iex> user = %{id: "user-123", role: %{permission_set_name: "own_data"}} + iex> member = %Member{id: "member-456", user: %User{id: "user-123"}} + iex> can?(user, :update, member) + true + """ + @spec can?(map() | nil, atom(), atom() | struct()) :: boolean() + def can?(nil, _action, _resource), do: false + + def can?(user, action, resource) when is_atom(action) and is_atom(resource) do + with %{role: %{permission_set_name: ps_name}} when not is_nil(ps_name) <- user, + {:ok, ps_atom} <- PermissionSets.permission_set_name_to_atom(ps_name), + permissions <- PermissionSets.get_permissions(ps_atom) do + resource_name = get_resource_name(resource) + + Enum.any?(permissions.resources, fn perm -> + perm.resource == resource_name and perm.action == action and perm.granted + end) + else + _ -> false + end + end + + def can?(user, action, %resource{} = record) when is_atom(action) do + with %{role: %{permission_set_name: ps_name}} when not is_nil(ps_name) <- user, + {:ok, ps_atom} <- PermissionSets.permission_set_name_to_atom(ps_name), + permissions <- PermissionSets.get_permissions(ps_atom) do + resource_name = get_resource_name(resource) + + # Find matching permission + matching_perm = + Enum.find(permissions.resources, fn perm -> + perm.resource == resource_name and perm.action == action and perm.granted + end) + + case matching_perm do + nil -> false + perm -> check_scope(perm.scope, user, record, resource_name) + end + else + _ -> false + end + end + + @doc """ + Checks if user can access a specific page. + + ## Examples + + iex> admin = %{role: %{permission_set_name: "admin"}} + iex> can_access_page?(admin, "/admin/roles") + true + + iex> mitglied = %{role: %{permission_set_name: "own_data"}} + iex> can_access_page?(mitglied, "/members") + false + """ + @spec can_access_page?(map() | nil, String.t() | Phoenix.VerifiedRoutes.unverified_path()) :: + boolean() + def can_access_page?(nil, _page_path), do: false + + def can_access_page?(user, page_path) do + # Convert verified route to string if needed + page_path_str = if is_binary(page_path), do: page_path, else: to_string(page_path) + + with %{role: %{permission_set_name: ps_name}} when not is_nil(ps_name) <- user, + {:ok, ps_atom} <- PermissionSets.permission_set_name_to_atom(ps_name), + permissions <- PermissionSets.get_permissions(ps_atom) do + page_matches?(permissions.pages, page_path_str) + else + _ -> false + end + end + + # Check if scope allows access to record + defp check_scope(:all, _user, _record, _resource_name), do: true + + defp check_scope(:own, user, record, _resource_name) do + record.id == user.id + end + + defp check_scope(:linked, user, record, resource_name) do + case resource_name do + "Member" -> check_member_linked(user, record) + "CustomFieldValue" -> check_custom_field_value_linked(user, record) + _ -> check_fallback_linked(user, record) + end + end + + defp check_member_linked(user, record) do + # Member has_one :user (inverse of User belongs_to :member) + # Check if member.user.id == user.id (user must be preloaded) + case Map.get(record, :user) do + %{id: user_id} -> user_id == user.id + _ -> false + end + end + + defp check_custom_field_value_linked(user, record) do + # Need to traverse: custom_field_value.member.user.id + # Note: In UI, custom_field_value should have member.user preloaded + case Map.get(record, :member) do + %{user: %{id: member_user_id}} -> member_user_id == user.id + _ -> false + end + end + + defp check_fallback_linked(user, record) do + # Fallback: try user_id or user relationship + case Map.get(record, :user_id) do + nil -> check_user_relationship_linked(user, record) + user_id -> user_id == user.id + end + end + + defp check_user_relationship_linked(user, record) do + # Try user relationship + case Map.get(record, :user) do + %{id: user_id} -> user_id == user.id + _ -> false + end + end + + # Check if page path matches any allowed pattern + defp page_matches?(allowed_pages, requested_path) do + Enum.any?(allowed_pages, fn pattern -> + cond do + pattern == "*" -> true + pattern == requested_path -> true + String.contains?(pattern, ":") -> match_pattern?(pattern, requested_path) + true -> false + end + end) + end + + # Match dynamic route pattern + defp match_pattern?(pattern, path) do + pattern_segments = String.split(pattern, "/", trim: true) + path_segments = String.split(path, "/", trim: true) + + if length(pattern_segments) == length(path_segments) do + Enum.zip(pattern_segments, path_segments) + |> Enum.all?(fn {pattern_seg, path_seg} -> + String.starts_with?(pattern_seg, ":") or pattern_seg == path_seg + end) + else + false + end + end + + # Extract resource name from module + defp get_resource_name(resource) when is_atom(resource) do + resource |> Module.split() |> List.last() + end +end diff --git a/lib/mv_web/components/core_components.ex b/lib/mv_web/components/core_components.ex index 996a349..45bcae0 100644 --- a/lib/mv_web/components/core_components.ex +++ b/lib/mv_web/components/core_components.ex @@ -692,7 +692,7 @@ defmodule MvWeb.CoreComponents do """ attr :name, :string, required: true attr :class, :string, default: "size-4" - attr :rest, :global, include: ~w[aria-hidden] + attr :rest, :global, include: ~w(aria-hidden) def icon(%{name: "hero-" <> _} = assigns) do ~H""" diff --git a/lib/mv_web/components/layouts/navbar.ex b/lib/mv_web/components/layouts/navbar.ex index 52b80d2..88ce380 100644 --- a/lib/mv_web/components/layouts/navbar.ex +++ b/lib/mv_web/components/layouts/navbar.ex @@ -3,6 +3,7 @@ defmodule MvWeb.Layouts.Navbar do Navbar that is used in the rootlayout shown on every page """ use MvWeb, :html + import MvWeb.Authorization attr :current_user, :map, required: true, @@ -22,12 +23,26 @@ defmodule MvWeb.Layouts.Navbar do {@club_name}