diff --git a/.drone.yml b/.drone.yml index 0f874a9..cca4ca4 100644 --- a/.drone.yml +++ b/.drone.yml @@ -273,7 +273,7 @@ environment: steps: - name: renovate - image: renovate/renovate:42.81 + image: renovate/renovate:42.95 environment: RENOVATE_CONFIG_FILE: "renovate_backend_config.js" RENOVATE_TOKEN: diff --git a/.env.example b/.env.example index 13154f3..d5d35ed 100644 --- a/.env.example +++ b/.env.example @@ -11,9 +11,22 @@ PHX_HOST=localhost # Recommended: Association settings ASSOCIATION_NAME="Sportsclub XYZ" +# Optional: Admin user (created/updated on container start via Release.seed_admin) +# In production, set these so the first admin can log in. Change password without redeploy: +# bin/mv eval "Mv.Release.seed_admin()" (with new ADMIN_PASSWORD or ADMIN_PASSWORD_FILE) +# ADMIN_EMAIL=admin@example.com +# ADMIN_PASSWORD=secure-password +# ADMIN_PASSWORD_FILE=/run/secrets/admin_password + # Optional: OIDC Configuration # These have defaults in docker-compose.prod.yml, only override if needed # OIDC_CLIENT_ID=mv # OIDC_BASE_URL=http://localhost:8080/auth/v1 # OIDC_REDIRECT_URI=http://localhost:4001/auth/user/rauthy/callback # OIDC_CLIENT_SECRET=your-rauthy-client-secret + +# Optional: OIDC group → Admin role sync (e.g. Authentik groups from profile scope) +# If OIDC_ADMIN_GROUP_NAME is set, users in that group get Admin role on registration/sign-in. +# OIDC_GROUPS_CLAIM defaults to "groups" (JWT claim name for group list). +# OIDC_ADMIN_GROUP_NAME=admin +# OIDC_GROUPS_CLAIM=groups diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md index 0a87836..7e4cee9 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -81,9 +81,11 @@ lib/ ├── membership/ # Membership domain │ ├── membership.ex # Domain definition │ ├── member.ex # Member resource +│ ├── custom_field.ex # Custom field (definition) resource │ ├── custom_field_value.ex # Custom field value resource -│ ├── custom_field.ex # CustomFieldValue type resource │ ├── setting.ex # Global settings (singleton resource) +│ ├── group.ex # Group resource +│ ├── member_group.ex # MemberGroup join table resource │ └── email.ex # Email custom type ├── membership_fees/ # MembershipFees domain │ ├── membership_fees.ex # Domain definition @@ -149,6 +151,8 @@ lib/ │ │ ├── membership_fee_type_live/ # Membership fee type LiveViews │ │ ├── membership_fee_settings_live.ex # Membership fee settings │ │ ├── global_settings_live.ex # Global settings +│ │ ├── group_live/ # Group management LiveViews +│ │ ├── import_export_live.ex # CSV import/export LiveView │ │ └── contribution_type_live/ # Contribution types (mock-up) │ ├── auth_overrides.ex # AshAuthentication overrides │ ├── endpoint.ex # Phoenix endpoint @@ -194,7 +198,8 @@ test/ ├── seeds_test.exs # Database seed tests └── support/ # Test helpers ├── conn_case.ex # Controller test helpers - └── data_case.ex # Data layer test helpers + ├── data_case.ex # Data layer test helpers + └── fixtures.ex # Shared test fixtures (Mv.Fixtures) ``` ### 1.2 Module Organization @@ -641,7 +646,95 @@ def card(assigns) do end ``` -### 3.3 System Actor Pattern +### 3.3 CSV Import Configuration + +**CSV Import Limits:** + +CSV import functionality supports configurable limits to prevent resource exhaustion: + +```elixir +# config/config.exs +config :mv, + csv_import: [ + max_file_size_mb: 10, # Maximum file size in megabytes + max_rows: 1000 # Maximum number of data rows (excluding header) + ] +``` + +**Accessing Configuration:** + +Use `Mv.Config` helper functions: + +```elixir +# Get max file size in bytes +max_bytes = Mv.Config.csv_import_max_file_size_bytes() + +# Get max file size in megabytes +max_mb = Mv.Config.csv_import_max_file_size_mb() + +# Get max rows +max_rows = Mv.Config.csv_import_max_rows() +``` + +**Best Practices:** +- Set reasonable limits based on server resources +- Display limits to users in UI +- Validate file size before upload +- Process imports in chunks (default: 200 rows per chunk) +- Cap error collection (default: 50 errors per import) + +### 3.4 Page-Level Authorization + +**CheckPagePermission Plug:** + +Use `MvWeb.Plugs.CheckPagePermission` for page-level authorization: + +```elixir +# lib/mv_web/router.ex +defmodule MvWeb.Router do + use MvWeb, :router + + # Add plug to router pipeline + pipeline :browser do + plug :accepts, ["html"] + plug :fetch_session + plug :fetch_live_flash + plug :put_root_layout, html: {MvWeb.Layouts, :root} + plug :protect_from_forgery + plug :put_secure_browser_headers + plug MvWeb.Plugs.CheckPagePermission # Page-level authorization + end +end +``` + +**Permission Set Route Matrix:** + +Routes are mapped to permission sets: +- `own_data`: Can access `/profile` and `/members/:id` (own linked member only) +- `read_only`: Can read all data, cannot modify +- `normal_user`: Can read and modify most data +- `admin`: Full access to all routes + +**Usage in LiveViews:** + +```elixir +# Check page access before mount +def mount(_params, _session, socket) do + actor = current_actor(socket) + + if MvWeb.Authorization.can_access_page?(actor, "/admin/roles") do + {:ok, assign(socket, :roles, load_roles(actor))} + else + {:ok, redirect(socket, to: ~p"/")} + end +end +``` + +**Public Paths:** + +Public paths (login, OIDC callbacks) are excluded from permission checks automatically. + +### 3.5 System Actor Pattern **When to Use System Actor:** @@ -726,7 +819,7 @@ Two mechanisms exist for bypassing standard authorization: **See also:** `docs/roles-and-permissions-architecture.md` (Authorization Bootstrap Patterns section) -### 3.4 Ash Framework +### 3.6 Ash Framework **Resource Definition Best Practices:** @@ -1247,7 +1340,8 @@ test/ │ └── components/ └── support/ # Test helpers ├── conn_case.ex # Controller test setup - └── data_case.ex # Database test setup + ├── data_case.ex # Database test setup + └── fixtures.ex # Shared test fixtures (Mv.Fixtures) ``` **Test File Naming:** diff --git a/config/config.exs b/config/config.exs index 64f3604..6720a5d 100644 --- a/config/config.exs +++ b/config/config.exs @@ -58,6 +58,11 @@ config :mv, max_rows: 1000 ] +# OIDC group → role sync (optional). Overridden in runtime.exs from ENV in production. +config :mv, :oidc_role_sync, + admin_group_name: nil, + groups_claim: "groups" + # Configures the endpoint config :mv, MvWeb.Endpoint, url: [host: "localhost"], diff --git a/config/runtime.exs b/config/runtime.exs index 06a2cd8..f1df5b7 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -89,6 +89,11 @@ if System.get_env("PHX_SERVER") do config :mv, MvWeb.Endpoint, server: true end +# OIDC group → Admin role sync: read from ENV in all environments (dev/test/prod) +config :mv, :oidc_role_sync, + admin_group_name: System.get_env("OIDC_ADMIN_GROUP_NAME"), + groups_claim: System.get_env("OIDC_GROUPS_CLAIM") || "groups" + if config_env() == :prod do database_url = build_database_url.() diff --git a/docker-compose.yml b/docker-compose.yml index 4c169b5..626f353 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -25,7 +25,7 @@ services: rauthy: container_name: rauthy-dev - image: ghcr.io/sebadob/rauthy:0.33.4 + image: ghcr.io/sebadob/rauthy:0.34.2 environment: - LOCAL_TEST=true - SMTP_URL=mailcrab diff --git a/docs/admin-bootstrap-and-oidc-role-sync.md b/docs/admin-bootstrap-and-oidc-role-sync.md new file mode 100644 index 0000000..b0da019 --- /dev/null +++ b/docs/admin-bootstrap-and-oidc-role-sync.md @@ -0,0 +1,54 @@ +# Admin Bootstrap and OIDC Role Sync + +## Overview + +- **Admin bootstrap:** In production, no seeds run. The first admin user is created/updated from environment variables in the Docker entrypoint (after migrate, before server). Password can be changed without redeploy via `bin/mv eval "Mv.Release.seed_admin()"`. +- **OIDC role sync:** Optional mapping from OIDC groups (e.g. from Authentik profile scope) to the Admin role. Users in the configured admin group get the Admin role on registration and on each sign-in. + +## Admin Bootstrap (Part A) + +### Environment Variables + +- `ADMIN_EMAIL` – Email of the admin user to create/update. If unset, seed_admin/0 does nothing. +- `ADMIN_PASSWORD` – Password for the admin user. If unset (and no file), no new user is created; if a user with ADMIN_EMAIL already exists (e.g. OIDC-only), their role is set to Admin (no password change). +- `ADMIN_PASSWORD_FILE` – Path to a file containing the password (e.g. Docker secret). + +### Release Task + +- `Mv.Release.seed_admin/0` – Reads ADMIN_EMAIL and password from ADMIN_PASSWORD or ADMIN_PASSWORD_FILE. If both email and password are set: creates or updates the user with the Admin role. If only ADMIN_EMAIL is set: sets the Admin role on an existing user with that email (for OIDC-only admins); does not create a user. Idempotent. + +### Entrypoint + +- rel/overlays/bin/docker-entrypoint.sh – After migrate, runs seed_admin(), then starts the server. + +### Seeds (Dev/Test) + +- priv/repo/seeds.exs – Uses ADMIN_PASSWORD or ADMIN_PASSWORD_FILE when set; otherwise fallback "testpassword" only in dev/test. + +## OIDC Role Sync (Part B) + +### Configuration + +- `OIDC_ADMIN_GROUP_NAME` – OIDC group name that maps to the Admin role. If unset, no role sync. +- `OIDC_GROUPS_CLAIM` – JWT claim name for group list (default "groups"). +- Module: Mv.OidcRoleSyncConfig (oidc_admin_group_name/0, oidc_groups_claim/0). + +### Sync Logic + +- Mv.OidcRoleSync.apply_admin_role_from_user_info(user, user_info) – If admin group configured, sets user role to Admin or Mitglied based on user_info groups. + +### Where It Runs + +1. Registration: register_with_rauthy after_action calls OidcRoleSync. +2. Sign-in: sign_in_with_rauthy prepare after_action calls OidcRoleSync for each user. + +### Internal Action + +- User.set_role_from_oidc_sync – Internal update (role_id only). Used by OidcRoleSync; not exposed. + +## See Also + +- .env.example – Admin and OIDC group env vars. +- lib/mv/release.ex – seed_admin/0. +- lib/mv/oidc_role_sync.ex – Sync implementation. +- docs/oidc-account-linking.md – OIDC account linking. diff --git a/docs/database-schema-readme.md b/docs/database-schema-readme.md index 15e4e33..6bf11de 100644 --- a/docs/database-schema-readme.md +++ b/docs/database-schema-readme.md @@ -15,10 +15,10 @@ This document provides a comprehensive overview of the Mila Membership Managemen | Metric | Count | |--------|-------| -| **Tables** | 9 | +| **Tables** | 11 | | **Domains** | 4 (Accounts, Membership, MembershipFees, Authorization) | -| **Relationships** | 7 | -| **Indexes** | 20+ | +| **Relationships** | 9 | +| **Indexes** | 25+ | | **Triggers** | 1 (Full-text search) | ## Tables Overview @@ -77,6 +77,23 @@ This document provides a comprehensive overview of the Mila Membership Managemen - Membership fee default settings - Environment variable support for club name +#### `groups` +- **Purpose:** Group definitions for organizing members +- **Rows (Estimated):** Low (typically 5-20 groups per club) +- **Key Features:** + - Unique group names (case-insensitive) + - URL-friendly slugs (auto-generated, immutable) + - Optional descriptions + - Many-to-many relationship with members + +#### `member_groups` +- **Purpose:** Join table for many-to-many relationship between members and groups +- **Rows (Estimated):** Medium to High (multiple groups per member) +- **Key Features:** + - Unique constraint on (member_id, group_id) + - CASCADE delete on both sides + - Efficient indexes for queries + ### Authorization Domain #### `roles` @@ -100,6 +117,10 @@ Member (1) → (N) MembershipFeeCycles ↓ MembershipFeeType (1) +Member (N) ←→ (N) Group + ↓ ↓ + MemberGroups (N) MemberGroups (N) + Settings (1) → MembershipFeeType (0..1) ``` @@ -145,6 +166,12 @@ Settings (1) → MembershipFeeType (0..1) - Settings can reference a default fee type - `ON DELETE SET NULL` - if fee type is deleted, setting is cleared +9. **Member ↔ Group (N:N via MemberGroup)** + - Many-to-many relationship through `member_groups` join table + - `ON DELETE CASCADE` on both sides - removing member/group removes associations + - Unique constraint on (member_id, group_id) prevents duplicates + - Groups searchable via member search vector + ## Important Business Rules ### Email Synchronization @@ -509,7 +536,7 @@ mix run priv/repo/seeds.exs --- -**Last Updated:** 2026-01-13 -**Schema Version:** 1.4 +**Last Updated:** 2026-01-27 +**Schema Version:** 1.5 **Database:** PostgreSQL 17.6 (dev) / 16 (prod) diff --git a/docs/development-progress-log.md b/docs/development-progress-log.md index 928558e..1dcf994 100644 --- a/docs/development-progress-log.md +++ b/docs/development-progress-log.md @@ -1752,8 +1752,151 @@ This project demonstrates a modern Phoenix application built with: --- -**Document Version:** 1.4 -**Last Updated:** 2026-01-13 +--- + +## Recent Updates (2026-01-13 to 2026-01-27) + +### Groups Feature Implementation (2026-01-27) + +**PR #378:** *Add groups resource* (closes #371) +- Created `Mv.Membership.Group` resource with name, slug, description +- Created `Mv.Membership.MemberGroup` join table for many-to-many relationship +- Automatic slug generation from name (immutable after creation) +- Case-insensitive name uniqueness via LOWER(name) index +- Database migration: `20260127141620_add_groups_and_member_groups.exs` + +**PR #382:** *Groups Admin UI* (closes #372) +- Groups management LiveViews (`/groups`) +- Create, edit, delete groups with confirmation +- Member count display per group +- Add/remove members from groups +- Groups displayed in member overview and detail views +- Filter and sort by groups in member list + +**Key Features:** +- Many-to-many relationship: Members can belong to multiple groups +- Groups searchable via member search vector (full-text search) +- CASCADE delete: Removing member/group removes associations +- Unique constraint prevents duplicate member-group associations + +### CSV Import Feature Implementation (2026-01-27) + +**PR #359:** *Implements CSV Import UI* (closes #335) +- Import/Export LiveView (`/import_export`) +- CSV file upload with auto-upload +- Real-time import progress tracking +- Error and warning reporting +- Chunked processing (200 rows per chunk) + +**PR #394:** *Adds config for import limits* (closes #336) +- Configurable maximum file size (default: 10 MB) +- Configurable maximum rows (default: 1000) +- Configuration via `config :mv, csv_import: [max_file_size_mb: ..., max_rows: ...]` +- UI displays limits to users + +**PR #395:** *Implements custom field CSV import* (closes #338) +- Support for importing custom field values via CSV +- Custom field mapping by slug or name +- Validation of custom field value types +- Error reporting with line numbers and field names +- CSV templates (German and English) available for download + +**Key Features:** +- Member field import (email, first_name, last_name, etc.) +- Custom field value import (all types: string, integer, boolean, date, email) +- Error capping (max 50 errors per import to prevent memory issues) +- Async chunk processing with progress updates +- Admin-only access (requires `:create` permission on Member resource) + +### Page Permission Router Plug (2026-01-27) + +**PR #390:** *Page Permission Router Plug* (closes #388) +- `MvWeb.Plugs.CheckPagePermission` plug for page-level authorization +- Route-based permission checking +- Automatic redirects for unauthorized access +- Integration with permission sets (own_data, read_only, normal_user, admin) +- Documentation: `docs/page-permission-route-coverage.md` + +**Key Features:** +- Page-level access control before LiveView mount +- Permission set-based route matrix +- Redirect targets for different permission levels +- Public paths (login, OIDC callbacks) excluded from checks + +### Resource Policies Implementation (2026-01-27) + +**PR #387:** *CustomField Resource Policies* (closes #386) +- CustomField resource policies with actor-based authorization +- Admin-only create/update/destroy operations +- Read access for authenticated users +- No system-actor fallback (explicit actor required) + +**PR #377:** *CustomFieldValue Resource Policies* (closes #369) +- CustomFieldValue resource policies +- own_data permission set: can create/update own linked member's custom field values +- Admin and normal_user: full access +- Bypass read rule for CustomFieldValue pattern (documented) + +**PR #364:** *User Resource Policies* (closes #363) +- User resource policies with scope filtering +- own_data: can read/update own user record +- Admin: full access +- Email change validation for linked members + +### System Actor Improvements (2026-01-27) + +**PR #379:** *Fix System missing system actor in prod and prevent deletion* +- System actor user creation in migrations +- Block update/destroy on system-actor user +- System user handling in UserLive forms +- Normalize system actor email + +**PR #361:** *System Actor Mode for Systemic Flows* (closes #348) +- System actor pattern for systemic operations +- Email synchronization uses system actor +- Cycle generation uses system actor +- Documentation: `docs/roles-and-permissions-architecture.md` (Authorization Bootstrap Patterns) + +**PR #367:** *Remove NoActor bypass* +- Removed NoActor bypass to prevent masking authorization bugs +- All tests now require explicit actor +- Exception: AshAuthentication bypass tests (conscious exception) + +### Email Sync Fixes (2026-01-27) + +**PR #380:** *Fix email sync (user->member) when changing password and email* +- Email sync when admin sets password via `admin_set_password` +- Bidirectional email synchronization improvements +- Validation fixes for linked user-member pairs + +### UI/UX Improvements (2026-01-27) + +**PR #389:** *Change Logo* (closes #385) +- Updated application logo +- Logo display in sidebar and navigation + +**PR #362:** *Add boolean custom field filters to member overview* (closes #309) +- Boolean custom field filtering in member list +- Filter by true/false values +- Integration with existing filter system + +### Test Performance Optimization (2026-01-27) + +**PR #384:** *Minor test refactoring to improve on performance* (closes #383) +- Moved slow tests to nightly test suite +- Optimized policy tests +- Reduced test complexity in seeds tests +- Documentation: `docs/test-performance-optimization.md` + +**Key Changes:** +- Fast tests (standard CI): Business logic, validations, data persistence +- Slow tests (nightly): Performance tests, large datasets, query optimization +- UI tests: Basic HTML rendering, navigation, translations + +--- + +**Document Version:** 1.5 +**Last Updated:** 2026-01-27 **Maintainer:** Development Team **Status:** Living Document (update as project evolves) diff --git a/docs/email-sync.md b/docs/email-sync.md index c191ff4..2f765f0 100644 --- a/docs/email-sync.md +++ b/docs/email-sync.md @@ -4,6 +4,7 @@ 2. **DB constraints** - Prevent duplicates within same table (users.email, members.email) 3. **Custom validations** - Prevent cross-table conflicts only for linked entities 4. **Sync is bidirectional**: User ↔ Member (but User always wins on link) +5. **Linked member email change** - When a member is linked, only administrators or the linked user may change that member's email (Member resource validation `EmailChangePermission`). Because User.email wins on link and changes sync Member → User, allowing anyone to change a linked member's email would overwrite that user's account email; this rule keeps sync under control. --- diff --git a/docs/feature-roadmap.md b/docs/feature-roadmap.md index 1df3eb6..7e28eea 100644 --- a/docs/feature-roadmap.md +++ b/docs/feature-roadmap.md @@ -1,7 +1,7 @@ # Feature Roadmap & Implementation Plan **Project:** Mila - Membership Management System -**Last Updated:** 2026-01-13 +**Last Updated:** 2026-01-27 **Status:** Active Development --- @@ -29,6 +29,10 @@ - ✅ **OIDC account linking with password verification** (PR #192, closes #171) - ✅ **Secure OIDC email collision handling** (PR #192) - ✅ **Automatic linking for passwordless users** (PR #192) +- ✅ **Page Permission Router Plug** - Page-level authorization (PR #390, closes #388, 2026-01-27) + - Route-based permission checking + - Automatic redirects for unauthorized access + - Integration with permission sets **Closed Issues:** - ✅ [#171](https://git.local-it.org/local-it/mitgliederverwaltung/issues/171) - OIDC handling and linking (closed 2025-11-13) @@ -55,6 +59,10 @@ - ✅ [#191](https://git.local-it.org/local-it/mitgliederverwaltung/issues/191) - Implement Roles in Ash (M) - Completed - ✅ [#190](https://git.local-it.org/local-it/mitgliederverwaltung/issues/190) - Implement Permissions in Ash (M) - Completed - ✅ [#151](https://git.local-it.org/local-it/mitgliederverwaltung/issues/151) - Define implementation plan for roles and permissions (M) - Completed +- ✅ [#388](https://git.local-it.org/local-it/mitgliederverwaltung/issues/388) - Page Permission Router Plug (closed 2026-01-27) +- ✅ [#386](https://git.local-it.org/local-it/mitgliederverwaltung/issues/386) - CustomField Resource Policies (closed 2026-01-27) +- ✅ [#369](https://git.local-it.org/local-it/mitgliederverwaltung/issues/369) - CustomFieldValue Resource Policies (closed 2026-01-27) +- ✅ [#363](https://git.local-it.org/local-it/mitgliederverwaltung/issues/363) - User Resource Policies (closed 2026-01-27) --- @@ -73,9 +81,24 @@ - ✅ User-Member linking (optional 1:1) - ✅ Email synchronization between User and Member - ✅ **Bulk email copy** - Copy selected members' email addresses to clipboard (Issue #230) +- ✅ **Groups** - Organize members into groups (PR #378, #382, closes #371, #372, 2026-01-27) + - Many-to-many relationship with groups + - Groups management UI (`/groups`) + - Filter and sort by groups in member list + - Groups displayed in member overview and detail views +- ✅ **CSV Import** - Import members from CSV files (PR #359, #394, #395, closes #335, #336, #338, 2026-01-27) + - Member field import + - Custom field value import + - Real-time progress tracking + - Error reporting **Closed Issues:** - ✅ [#162](https://git.local-it.org/local-it/mitgliederverwaltung/issues/162) - Fuzzy and substring search (closed 2025-11-12) +- ✅ [#371](https://git.local-it.org/local-it/mitgliederverwaltung/issues/371) - Add groups resource (closed 2026-01-27) +- ✅ [#372](https://git.local-it.org/local-it/mitgliederverwaltung/issues/372) - Groups Admin UI (closed 2026-01-27) +- ✅ [#335](https://git.local-it.org/local-it/mitgliederverwaltung/issues/335) - CSV Import UI (closed 2026-01-27) +- ✅ [#336](https://git.local-it.org/local-it/mitgliederverwaltung/issues/336) - Config for import limits (closed 2026-01-27) +- ✅ [#338](https://git.local-it.org/local-it/mitgliederverwaltung/issues/338) - Custom field CSV import (closed 2026-01-27) **Open Issues:** - [#169](https://git.local-it.org/local-it/mitgliederverwaltung/issues/169) - Allow combined creation of Users/Members (M, Low priority) @@ -88,7 +111,7 @@ - ❌ Advanced filters (date ranges, multiple criteria) - ❌ Pagination (currently all members loaded) - ❌ Bulk operations (bulk delete, bulk update) -- ❌ Member import/export (CSV, Excel) +- ❌ Excel import for members - ❌ Member profile photos/avatars - ❌ Member history/audit log - ❌ Duplicate detection @@ -288,12 +311,24 @@ - ✅ **CSV Import Templates** - German and English templates (#329, 2026-01-13) - Template files in `priv/static/templates/member_import_de.csv` and `member_import_en.csv` - CSV specification documented in `docs/csv-member-import-v1.md` +- ✅ **CSV Import Implementation** - Full CSV import feature (#335, #336, #338, 2026-01-27) + - Import/Export LiveView (`/import_export`) + - Member field import (email, first_name, last_name, etc.) + - Custom field value import (all types: string, integer, boolean, date, email) + - Real-time progress tracking + - Error and warning reporting with line numbers + - Configurable limits (max file size, max rows) + - Chunked processing (200 rows per chunk) + - Admin-only access + +**Closed Issues:** +- ✅ [#335](https://git.local-it.org/local-it/mitgliederverwaltung/issues/335) - CSV Import UI (closed 2026-01-27) +- ✅ [#336](https://git.local-it.org/local-it/mitgliederverwaltung/issues/336) - Config for import limits (closed 2026-01-27) +- ✅ [#338](https://git.local-it.org/local-it/mitgliederverwaltung/issues/338) - Custom field CSV import (closed 2026-01-27) **Missing Features:** -- ❌ CSV import implementation (templates ready, import logic pending) - ❌ Excel import for members -- ❌ Import validation and preview -- ❌ Import error handling +- ❌ Import validation preview (before import) - ❌ Bulk data export - ❌ Backup export - ❌ Data migration tools diff --git a/docs/groups-architecture.md b/docs/groups-architecture.md index 8251a4b..735898c 100644 --- a/docs/groups-architecture.md +++ b/docs/groups-architecture.md @@ -4,7 +4,7 @@ **Feature:** Groups Management **Version:** 1.0 **Last Updated:** 2025-01-XX -**Status:** Architecture Design - Ready for Implementation +**Status:** ✅ Implemented (authorization: see [roles-and-permissions-architecture.md](./roles-and-permissions-architecture.md)) --- @@ -314,23 +314,9 @@ lib/ - Display group name and description - List all members in group - Link to member detail pages -- Add members to group (via modal with search/autocomplete) -- Remove members from group (via remove button per member) - Edit group button (navigates to `/groups/:slug/edit`) - Delete group button (with confirmation modal) -**Add Member Functionality:** -- "Add Member" button displayed above member table (only for users with `:update` permission) -- Opens modal with member search/autocomplete -- Search filters out members already in the group -- Selecting a member adds them to the group immediately -- Success/error flash messages provide feedback - -**Remove Member Functionality:** -- "Remove" button (icon button) for each member in table (only for users with `:update` permission) -- Clicking remove immediately removes member from group (no confirmation dialog) -- Success/error flash messages provide feedback - **Note:** Uses slug for routing to provide URL-friendly, readable group URLs (e.g., `/groups/board-members`). ### Group Form Pages @@ -426,15 +412,17 @@ lib/ ## Authorization +**Status:** ✅ Implemented. Group and MemberGroup resource policies and PermissionSets are in place. See [roles-and-permissions-architecture.md](./roles-and-permissions-architecture.md) for the full permission matrix and policy patterns. + ### Permission Model (MVP) -**Resource:** `groups` +**Resource:** `Group` (and `MemberGroup`) **Actions:** -- `read` - View groups (all users with member read permission) -- `create` - Create groups (admin only) -- `update` - Edit groups (admin only) -- `destroy` - Delete groups (admin only) +- `read` - View groups (all permission sets) +- `create` - Create groups (normal_user and admin) +- `update` - Edit groups (normal_user and admin) +- `destroy` - Delete groups (normal_user and admin) **Scopes:** - `:all` - All groups (for all permission sets that have read access) @@ -456,7 +444,7 @@ lib/ **Own Data Permission Set:** - `read` action on `Group` resource with `:all` scope - granted -**Note:** All permission sets use `:all` scope for groups. Groups are considered public information that all users with member read permission can view. Only admins can manage (create/update/destroy) groups. +**Note:** All permission sets use `:all` scope for groups. Groups are considered public information that all users with member read permission can view. normal_user and admin can manage (create/update/destroy) groups. ### Member-Group Association Permissions @@ -766,7 +754,6 @@ Each functional unit can be implemented as a **separate issue**: - **Issue 4:** Groups in Member Detail (Unit 5) - **Issue 5:** Groups in Member Search (Unit 6) - **Issue 6:** Permissions (Unit 7) -- **Issue 7:** Add/Remove Members in Group Detail View **Alternative:** Issues 3 and 4 can be combined, as they both concern the display of groups. @@ -812,27 +799,6 @@ Each functional unit can be implemented as a **separate issue**: **Estimation:** 3-4h -### Phase 2a: Add/Remove Members in Group Detail View - -**Goal:** Enable adding and removing members from groups via UI - -**Tasks:** -1. Add "Add Member" button above member table in Group Detail View -2. Implement modal with member search/autocomplete -3. Add "Remove" button for each member in table -4. Implement add/remove functionality with flash messages -5. Ensure proper authorization checks - -**Deliverables:** -- Members can be added to groups via UI -- Members can be removed from groups via UI -- Proper feedback via flash messages -- Authorization enforced - -**Estimation:** 2-3h - -**Note:** This phase extends Phase 2 and can be implemented as Issue 7 after Issue 2 is complete. - ### Phase 3: Member Overview Integration **Goal:** Display and filter groups in member overview @@ -899,9 +865,9 @@ Each functional unit can be implemented as a **separate issue**: **Estimation:** 1-2h -### Total Estimation: 15-21h +### Total Estimation: 13-18h -**Note:** This includes all 7 issues. The original MVP estimation was 13-15h, with Issue 7 adding 2-3h for the add/remove members functionality in the Group Detail View. +**Note:** This aligns with the issue estimation of 15h. --- @@ -994,55 +960,6 @@ Each functional unit can be implemented as a **separate issue**: - Only admins can manage groups - All users can view groups (if they can view members) -### Issue 7: Add/Remove Members in Group Detail View -**Type:** Frontend -**Estimation:** 2-3h -**Dependencies:** Issue 1 (Backend must be functional), Issue 2 (Group Detail View must exist) - -**Tasks:** -- Add "Add Member" button above member table in Group Detail View (`/groups/:slug`) -- Implement modal for member selection with search/autocomplete -- Add "Remove" button for each member in the member table -- Implement add member functionality (create MemberGroup association) -- Implement remove member functionality (destroy MemberGroup association) -- Add flash messages for success/error feedback -- Ensure proper authorization checks (only users with `:update` permission on Group can add/remove) -- Filter out members already in the group from search results -- Reload group data after add/remove operations - -**Acceptance Criteria:** -- "Add Member" button is visible above member table (only for users with `:update` permission) -- Clicking "Add Member" opens a modal with member search/autocomplete -- Search filters members and excludes those already in the group -- Selecting a member from search adds them to the group -- Success flash message is displayed when member is added -- Error flash message is displayed if member is already in group or other error occurs -- Each member row in the table has a "Remove" button (only visible for users with `:update` permission) -- Clicking "Remove" immediately removes the member from the group (no confirmation dialog) -- Success flash message is displayed when member is removed -- Group member list and member count update automatically after add/remove -- Modal closes after successful member addition -- Authorization is enforced server-side in event handlers -- UI respects permission checks (buttons hidden for unauthorized users) - -**Technical Notes:** -- Reuse member search pattern from `UserLive.Form` (ComboBox hook with autocomplete) -- Use `Membership.create_member_group/1` for adding members -- Use `Membership.destroy_member_group/1` for removing members -- Filter search results to exclude members already in the group (check `group.members`) -- Reload group with `:members` and `:member_count` after operations -- Follow existing modal patterns (similar to delete confirmation modal) -- Ensure accessibility: proper ARIA labels, keyboard navigation, focus management - -**UI/UX Details:** -- Modal title: "Add Member to Group" -- Search input placeholder: "Search for a member..." -- Search results show member name and email -- "Add" button in modal (disabled until member selected) -- "Cancel" button to close modal -- Remove button can be an icon button (trash icon) with tooltip -- Flash messages: "Member added successfully" / "Member removed successfully" / error messages - --- ## Testing Strategy diff --git a/docs/membership-fee-architecture.md b/docs/membership-fee-architecture.md index 4a290b7..6c81169 100644 --- a/docs/membership-fee-architecture.md +++ b/docs/membership-fee-architecture.md @@ -334,20 +334,18 @@ lib/ ### Permission System Integration -**See:** [roles-and-permissions-architecture.md](./roles-and-permissions-architecture.md) +**Status:** ✅ Implemented. See [roles-and-permissions-architecture.md](./roles-and-permissions-architecture.md) for the full permission matrix and policy patterns. -**Required Permissions:** +**PermissionSets (lib/mv/authorization/permission_sets.ex):** -- `MembershipFeeType.create/update/destroy` - Admin only -- `MembershipFeeType.read` - Admin, Treasurer, Board -- `MembershipFeeCycle.update` (status changes) - Admin, Treasurer -- `MembershipFeeCycle.read` - Admin, Treasurer, Board, Own member +- **MembershipFeeType:** All permission sets can read (:all); only admin has create/update/destroy (:all). +- **MembershipFeeCycle:** All can read (:all); read_only has read only; normal_user and admin have read + create + update + destroy (:all). +- **Manual "Regenerate Cycles" (UI + server):** The "Regenerate Cycles" button in the member detail view is shown to users who have MembershipFeeCycle create permission (normal_user and admin). UI access is gated by `can_create_cycle`. The LiveView handler also enforces `can?(:create, MembershipFeeCycle)` server-side before running regeneration (so e.g. a read_only user cannot trigger it via DevTools). Regeneration runs with system actor. -**Policy Patterns:** +**Resource Policies:** -- Use existing HasPermission check -- Leverage existing roles (Admin, Kassenwart) -- Member can read own cycles (linked via member_id) +- **MembershipFeeType** (`lib/membership_fees/membership_fee_type.ex`): `authorizers: [Ash.Policy.Authorizer]`, single policy with `HasPermission` for read/create/update/destroy. +- **MembershipFeeCycle** (`lib/membership_fees/membership_fee_cycle.ex`): Same pattern; update includes mark_as_paid, mark_as_suspended, mark_as_unpaid. ### LiveView Integration @@ -357,7 +355,7 @@ lib/ 2. MembershipFeeCycle table component (member detail view) - Implemented as `MvWeb.MemberLive.Show.MembershipFeesComponent` - Displays all cycles in a table with status management - - Allows changing cycle status, editing amounts, and regenerating cycles + - Allows changing cycle status, editing amounts, and manually regenerating cycles (normal_user and admin) 3. Settings form section (admin) 4. Member list column (membership fee status) diff --git a/docs/roles-and-permissions-architecture.md b/docs/roles-and-permissions-architecture.md index dbf2353..216c6c9 100644 --- a/docs/roles-and-permissions-architecture.md +++ b/docs/roles-and-permissions-architecture.md @@ -97,6 +97,10 @@ Control CRUD operations on: - CustomFieldValue (custom field values) - CustomField (custom field definitions) - Role (role management) +- Group (group definitions; read all, create/update/destroy normal_user and admin) +- MemberGroup (member–group associations; own_data read :linked, read_only read :all, normal_user/admin create/destroy) +- MembershipFeeType (fee type definitions; all read, admin-only create/update/destroy) +- MembershipFeeCycle (fee cycles; own_data read :linked, read_only read :all, normal_user/admin read+create+update+destroy; manual "Regenerate Cycles" for normal_user and admin) **4. Page-Level Permissions** @@ -105,6 +109,7 @@ Control access to LiveView pages: - Show pages (detail views) - Form pages (create/edit) - Admin pages +- Settings pages: `/settings` and `/membership_fee_settings` are admin-only (explicit in PermissionSets) **5. Granular Scopes** @@ -121,6 +126,8 @@ Three scope levels for permissions: - **Linked Member Email:** Only admins can edit email of member linked to user - **System Roles:** "Mitglied" role cannot be deleted (is_system_role flag) - **User-Member Linking:** Only admins can link/unlink users and members +- **User Role Assignment:** Only admins can change a user's role (via `update_user` with `role_id`). Last-admin validation ensures at least one user keeps the Admin role. +- **Settings Pages:** `/settings` and `/membership_fee_settings` are admin-only (explicit in PermissionSets pages). **7. UI Consistency** @@ -684,6 +691,12 @@ Quick reference table showing what each permission set allows: | **CustomFieldValue** (all) | - | R | R, C, U, D | R, C, U, D | | **CustomField** (all) | R | R | R | R, C, U, D | | **Role** (all) | - | - | - | R, C, U, D | +| **Group** (all) | R | R | R, C, U, D | R, C, U, D | +| **MemberGroup** (linked) | R | - | - | - | +| **MemberGroup** (all) | - | R | R, C, D | R, C, D | +| **MembershipFeeType** (all) | R | R | R | R, C, U, D | +| **MembershipFeeCycle** (linked) | R | - | - | - | +| **MembershipFeeCycle** (all) | - | R | R, C, U, D | R, C, U, D | **Legend:** R=Read, C=Create, U=Update, D=Destroy @@ -1012,16 +1025,21 @@ defmodule Mv.Membership.Member do authorize_if expr(id == ^actor(:member_id)) end - # 2. GENERAL: Check permissions from role - # - :own_data → can UPDATE linked member (scope :linked via HasPermission) - # - :read_only → can READ all members (scope :all), no update permission - # - :normal_user → can CRUD all members (scope :all) - # - :admin → can CRUD all members (scope :all) - policy action_type([:read, :create, :update, :destroy]) do + # 2. READ/DESTROY: Check permissions only (no :user argument on these actions) + policy action_type([:read, :destroy]) do description "Check permissions from user's role" authorize_if Mv.Authorization.Checks.HasPermission end - + + # 3. CREATE/UPDATE: Forbid user link unless admin; then check permissions + # ForbidMemberUserLinkUnlessAdmin: only admins may pass :user (link or unlink via nil/empty). + # HasPermission: :own_data → update linked; :read_only → no update; :normal_user/admin → update all + policy action_type([:create, :update]) do + description "Forbid user link unless admin; then check permissions" + forbid_if Mv.Authorization.Checks.ForbidMemberUserLinkUnlessAdmin + authorize_if Mv.Authorization.Checks.HasPermission + end + # 4. DEFAULT: Ash implicitly forbids if no policy authorizes (fail-closed) end @@ -1041,6 +1059,8 @@ end - **READ list queries**: No record at strict_check time → bypass with `expr(id == ^actor(:member_id))` needed for auto_filter ✅ - **UPDATE operations**: Changeset contains record → HasPermission evaluates `scope :linked` correctly ✅ +**User–member link:** Only admins may pass the `:user` argument on create_member or update_member (link or unlink via `user: nil`/`user: %{}`). The check uses **argument presence** (key in arguments), not value, to avoid bypass (see [User-Member Linking](#user-member-linking)). + **Permission Matrix:** | Action | Mitglied | Vorstand | Kassenwart | Buchhaltung | Admin | @@ -1135,23 +1155,20 @@ end **Location:** `lib/mv/authorization/role.ex` -**Special Protection:** System roles cannot be deleted. +**Defense-in-depth:** The Role resource uses `authorizers: [Ash.Policy.Authorizer]` and policies with `Mv.Authorization.Checks.HasPermission`. **Read** is allowed for all permission sets (own_data, read_only, normal_user, admin) via `perm("Role", :read, :all)` in PermissionSets; reading roles is not a security concern. **Create, update, and destroy** are allowed only for admin (admin has full Role CRUD in PermissionSets). Seeds and bootstrap use `authorize?: false` where necessary. + +**Special Protection:** System roles cannot be deleted (validation on destroy). ```elixir defmodule Mv.Authorization.Role do - use Ash.Resource, ... + use Ash.Resource, + authorizers: [Ash.Policy.Authorizer] policies do - # Only admin can manage roles policy action_type([:read, :create, :update, :destroy]) do - description "Check permissions from user's role" + description "Check permissions from user's role (read all, create/update/destroy admin only)" authorize_if Mv.Authorization.Checks.HasPermission end - - # DEFAULT: Forbid - policy action_type([:read, :create, :update, :destroy]) do - forbid_if always() - end end # Prevent deletion of system roles @@ -1188,13 +1205,43 @@ end | Action | Mitglied | Vorstand | Kassenwart | Buchhaltung | Admin | |--------|----------|----------|------------|-------------|-------| -| Read | ❌ | ❌ | ❌ | ❌ | ✅ | +| Read | ✅ | ✅ | ✅ | ✅ | ✅ | | Create | ❌ | ❌ | ❌ | ❌ | ✅ | | Update | ❌ | ❌ | ❌ | ❌ | ✅ | | Destroy* | ❌ | ❌ | ❌ | ❌ | ✅ | *Cannot destroy if `is_system_role=true` +### User Role Assignment (Admin-Only) + +**Location:** `lib/accounts/user.ex` (update_user action), `lib/mv_web/live/user_live/form.ex` + +Only admins can change a user's role. The `update_user` action accepts `role_id`; the User form shows a role dropdown when `can?(actor, :update, Mv.Authorization.Role)`. **Last-admin validation:** If the only non-system admin tries to change their role, the change is rejected with "At least one user must keep the Admin role." (System user is excluded from the admin count.) See [User-Member Linking](#user-member-linking) for the same admin-only pattern. + +### Group Resource Policies + +**Location:** `lib/membership/group.ex` + +Policies use `HasPermission` for read/create/update/destroy. All permission sets can read; normal_user and admin can create, update, destroy. No bypass (scope :all only in PermissionSets). + +### MemberGroup Resource Policies + +**Location:** `lib/membership/member_group.ex` + +Bypass for read restricted to own_data (MemberGroupReadLinkedForOwnData check: own_data only, filter `member_id == actor.member_id`); HasPermission for read (read_only/normal_user/admin :all) and create/destroy (normal_user + admin only). Admin with member_id set still gets :all from HasPermission (bypass does not apply). + +### MembershipFeeType Resource Policies + +**Location:** `lib/membership_fees/membership_fee_type.ex` + +Policies use `HasPermission` for read/create/update/destroy. All permission sets can read; only admin can create, update, destroy. + +### MembershipFeeCycle Resource Policies + +**Location:** `lib/membership_fees/membership_fee_cycle.ex` + +Bypass for read restricted to own_data (MembershipFeeCycleReadLinkedForOwnData: own_data only, filter `member_id == actor.member_id`); HasPermission for read (read_only/normal_user/admin :all) and create/update/destroy. own_data can only read cycles of the linked member; read_only can read all; normal_user and admin can read, create, update, and destroy (including mark_as_paid and manual "Regenerate Cycles"; UI button when `can_create_cycle`). Regenerate-cycles handler enforces `can?(:create, MembershipFeeCycle)` server-side. + --- ## Page Permission System @@ -2002,7 +2049,10 @@ Users and Members are separate entities that can be linked. Special rules: - A user cannot link themselves to an existing member - A user CAN create a new member and be directly linked to it (self-service) -**Enforcement:** The User resource restricts the `update_user` action (which accepts the `member` argument for link/unlink) to admins only via `Mv.Authorization.Checks.ActorIsAdmin`. The UserLive.Form shows the Member-Linking UI and runs member link/unlink on save only when the current user is admin; non-admins use the `:update` action (email only) for profile edit. +**Enforcement:** + +- **User side:** The User resource restricts the `update_user` action (which accepts the `member` argument for link/unlink) to admins only via `Mv.Authorization.Checks.ActorIsAdmin`. The UserLive.Form shows the Member-Linking UI and runs member link/unlink on save only when the current user is admin; non-admins use the `:update` action (email only) for profile edit. +- **Member side:** Only admins may set or change the user–member link on **Member** create or update. When creating or updating a member, the `:user` argument (which links the member to a user account) is forbidden for non-admins. This is enforced by `Mv.Authorization.Checks.ForbidMemberUserLinkUnlessAdmin` in the Member resource policies (`forbid_if` before `authorize_if HasPermission`). Non-admins can still create and update members as long as they do **not** pass the `:user` argument. The Member resource uses **`on_missing: :ignore`** for the `:user` relationship on update_member, so **omitting** `:user` from params does **not** change the link (no "unlink by omission"); unlink is only possible by explicitly passing `:user` (e.g. `user: nil`), which is admin-only. ### Approach: Separate Ash Actions diff --git a/docs/roles-and-permissions-implementation-plan.md b/docs/roles-and-permissions-implementation-plan.md index 23b045c..95db031 100644 --- a/docs/roles-and-permissions-implementation-plan.md +++ b/docs/roles-and-permissions-implementation-plan.md @@ -78,10 +78,11 @@ Stored in database `roles` table, each referencing a `permission_set_name`: - ✅ Hardcoded PermissionSets module with 4 permission sets - ✅ Role database table and CRUD interface - ✅ Custom Ash Policy Check (`HasPermission`) that reads from PermissionSets -- ✅ Policies on all resources (Member, User, CustomFieldValue, CustomField, Role) -- ✅ Page-level permissions via Phoenix Plug +- ✅ Policies on all resources (Member, User, CustomFieldValue, CustomField, Role, Group, MemberGroup, MembershipFeeType, MembershipFeeCycle) +- ✅ Page-level permissions via Phoenix Plug (including admin-only `/settings` and `/membership_fee_settings`) - ✅ UI authorization helpers for conditional rendering - ✅ Special case: Member email validation for linked users +- ✅ User role assignment: admin-only `role_id` in update_user; Last-Admin validation; role dropdown in User form when `can?(actor, :update, Role)` - ✅ Seed data for 5 roles **Benefits of Hardcoded Approach:** diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex index f792973..92b9ef2 100644 --- a/lib/accounts/user.ex +++ b/lib/accounts/user.ex @@ -8,6 +8,9 @@ defmodule Mv.Accounts.User do extensions: [AshAuthentication], authorizers: [Ash.Policy.Authorizer] + require Ash.Query + import Ash.Expr + postgres do table "users" repo Mv.Repo @@ -146,9 +149,10 @@ defmodule Mv.Accounts.User do update :update_user do description "Updates a user and manages the optional member relationship. To change an existing member link, first remove it (set member to nil), then add the new one." - # Only accept email directly - member_id is NOT in accept list - # This prevents direct foreign key manipulation, forcing use of manage_relationship - accept [:email] + + # Accept email and role_id (role_id only used by admins; policy restricts update_user to admins). + # member_id is NOT in accept list - use argument :member for relationship management. + accept [:email, :role_id] # Allow member to be passed as argument for relationship management argument :member, :map, allow_nil?: true @@ -183,6 +187,13 @@ defmodule Mv.Accounts.User do require_atomic? false end + # Internal: set role from OIDC group sync (Mv.OidcRoleSync). Bypass policy when context.private.oidc_role_sync. + # Same "at least one admin" validation as update_user (see validations where action_is). + update :set_role_from_oidc_sync do + accept [:role_id] + require_atomic? false + end + # Admin action for direct password changes in admin panel # Uses the official Ash Authentication HashPasswordChange with correct context update :admin_set_password do @@ -247,6 +258,8 @@ defmodule Mv.Accounts.User do end read :sign_in_with_rauthy do + # Single record expected; required for AshAuthentication OAuth2 strategy (returns list of 0 or 1). + get? true argument :user_info, :map, allow_nil?: false argument :oauth_tokens, :map, allow_nil?: false prepare AshAuthentication.Strategy.OAuth2.SignInPreparation @@ -256,6 +269,27 @@ defmodule Mv.Accounts.User do # linked their account via OIDC. Password-only users (oidc_id = nil) # cannot be accessed via OIDC login without password verification. filter expr(oidc_id == get_path(^arg(:user_info), [:sub])) + + # Sync role from OIDC groups after sign-in (e.g. admin group → Admin role) + # get? true can return nil, a single %User{}, or a list; normalize to list for Enum.each + prepare Ash.Resource.Preparation.Builtins.after_action(fn query, result, _context -> + user_info = Ash.Query.get_argument(query, :user_info) || %{} + oauth_tokens = Ash.Query.get_argument(query, :oauth_tokens) || %{} + + users = + case result do + nil -> [] + u when is_struct(u, User) -> [u] + list when is_list(list) -> list + _ -> [] + end + + Enum.each(users, fn user -> + Mv.OidcRoleSync.apply_admin_role_from_user_info(user, user_info, oauth_tokens) + end) + + {:ok, result} + end) end create :register_with_rauthy do @@ -293,6 +327,18 @@ defmodule Mv.Accounts.User do # Sync user email to member when linking (User → Member) change Mv.EmailSync.Changes.SyncUserEmailToMember + + # Sync role from OIDC groups (e.g. admin group → Admin role) after user is created/updated + change fn changeset, _ctx -> + user_info = Ash.Changeset.get_argument(changeset, :user_info) + oauth_tokens = Ash.Changeset.get_argument(changeset, :oauth_tokens) || %{} + + Ash.Changeset.after_action(changeset, fn _cs, record -> + Mv.OidcRoleSync.apply_admin_role_from_user_info(record, user_info, oauth_tokens) + # Return original record so __metadata__.token (from GenerateTokenChange) is preserved + {:ok, record} + end) + end end end @@ -319,6 +365,13 @@ defmodule Mv.Accounts.User do authorize_if Mv.Authorization.Checks.ActorIsAdmin end + # set_role_from_oidc_sync: internal only (called from Mv.OidcRoleSync on registration/sign-in). + # Not exposed in code_interface; only allowed when context.private.oidc_role_sync is set. + bypass action(:set_role_from_oidc_sync) do + description "Internal: OIDC role sync (server-side only)" + authorize_if Mv.Authorization.Checks.OidcRoleSyncContext + end + # UPDATE/DESTROY via HasPermission (evaluates PermissionSets scope) policy action_type([:read, :create, :update, :destroy]) do description "Check permissions from user's role and permission set" @@ -387,6 +440,63 @@ defmodule Mv.Accounts.User do end end + # Last-admin: prevent the only admin from leaving the admin role (at least one admin required). + # Only block when the user is leaving admin (target role is not admin). Switching between + # two admin roles (e.g. "Admin" and "Superadmin" both with permission_set_name "admin") is allowed. + validate fn changeset, _context -> + if Ash.Changeset.changing_attribute?(changeset, :role_id) do + new_role_id = Ash.Changeset.get_attribute(changeset, :role_id) + + if is_nil(new_role_id) do + :ok + else + current_role_id = changeset.data.role_id + + current_role = + Mv.Authorization.Role + |> Ash.get!(current_role_id, authorize?: false) + + new_role = + Mv.Authorization.Role + |> Ash.get!(new_role_id, authorize?: false) + + # Only block when current user is admin and target role is not admin (leaving admin) + if current_role.permission_set_name == "admin" and + new_role.permission_set_name != "admin" do + admin_role_ids = + Mv.Authorization.Role + |> Ash.Query.for_read(:read) + |> Ash.Query.filter(expr(permission_set_name == "admin")) + |> Ash.read!(authorize?: false) + |> Enum.map(& &1.id) + + # Count only non-system users with admin role (system user is for internal ops) + system_email = Mv.Helpers.SystemActor.system_user_email() + + count = + Mv.Accounts.User + |> Ash.Query.for_read(:read) + |> Ash.Query.filter(expr(role_id in ^admin_role_ids)) + |> Ash.Query.filter(expr(email != ^system_email)) + |> Ash.count!(authorize?: false) + + if count <= 1 do + {:error, + field: :role_id, message: "At least one user must keep the Admin role."} + else + :ok + end + else + :ok + end + end + else + :ok + end + end, + on: [:update], + where: [action_is([:update_user, :set_role_from_oidc_sync])] + # Prevent modification of the system actor user (required for internal operations). # Block update/destroy on UI-exposed actions only; :update_internal is used by bootstrap/tests. validate fn changeset, _context -> diff --git a/lib/membership/group.ex b/lib/membership/group.ex index 14aadc8..d468166 100644 --- a/lib/membership/group.ex +++ b/lib/membership/group.ex @@ -36,7 +36,8 @@ defmodule Mv.Membership.Group do """ use Ash.Resource, domain: Mv.Membership, - data_layer: AshPostgres.DataLayer + data_layer: AshPostgres.DataLayer, + authorizers: [Ash.Policy.Authorizer] require Ash.Query alias Mv.Helpers @@ -63,6 +64,13 @@ defmodule Mv.Membership.Group do end end + policies do + policy action_type([:read, :create, :update, :destroy]) do + description "Check permissions from role (all can read; normal_user and admin can create/update/destroy)" + authorize_if Mv.Authorization.Checks.HasPermission + end + end + validations do validate present(:name) @@ -136,7 +144,7 @@ defmodule Mv.Membership.Group do query = Mv.Membership.Group |> Ash.Query.filter(fragment("LOWER(?) = LOWER(?)", name, ^name)) - |> maybe_exclude_id(exclude_id) + |> Helpers.query_exclude_id(exclude_id) opts = Helpers.ash_actor_opts(actor) @@ -155,7 +163,4 @@ defmodule Mv.Membership.Group do :ok end end - - defp maybe_exclude_id(query, nil), do: query - defp maybe_exclude_id(query, id), do: Ash.Query.filter(query, id != ^id) end diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 7b49c86..476501c 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -25,6 +25,7 @@ defmodule Mv.Membership.Member do - 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 + - Linked member email change: only admins or the linked user may change a linked member's email (see `Mv.Membership.Member.Validations.EmailChangePermission`) ## Full-Text Search Members have a `search_vector` attribute (tsvector) that is automatically @@ -152,16 +153,18 @@ defmodule Mv.Membership.Member do change manage_relationship(:custom_field_values, on_match: :update, on_no_match: :create) + # When :user argument is present and nil/empty, unrelate (admin-only via policy). + # Must run before manage_relationship; on_missing: :ignore then does nothing for nil input. + change Mv.Membership.Member.Changes.UnrelateUserWhenArgumentNil + # Manage the user relationship during member update + # on_missing: :ignore so that omitting :user does NOT unlink (security: only admins may + # change the link; unlink is explicit via user: nil, forbidden for non-admins by policy). change manage_relationship(:user, :user, - # Look up existing user and relate to it on_lookup: :relate, - # Error if user doesn't exist in database on_no_match: :error, - # Error if user is already linked to another member (prevents "stealing") on_match: :error, - # If no user provided, remove existing relationship (allows user removal) - on_missing: :unrelate + on_missing: :ignore ) # Sync member email to user when email changes (Member → User) @@ -311,14 +314,18 @@ defmodule Mv.Membership.Member do authorize_if expr(id == ^actor(:member_id)) end - # GENERAL: Check permissions from user's role - # HasPermission handles update permissions correctly: - # - :own_data → can update linked member (scope :linked) - # - :read_only → cannot update any member (no update permission) - # - :normal_user → can update all members (scope :all) - # - :admin → can update all members (scope :all) - policy action_type([:read, :create, :update, :destroy]) do - description "Check permissions from user's role and permission set" + # READ/DESTROY: Check permissions only (no :user argument on these actions) + policy action_type([:read, :destroy]) do + description "Check permissions from user's role" + authorize_if Mv.Authorization.Checks.HasPermission + end + + # CREATE/UPDATE: Forbid member–user link unless admin, then check permissions + # ForbidMemberUserLinkUnlessAdmin: only admins may pass :user (link or unlink via nil/empty). + # HasPermission: :own_data → update linked; :read_only → no update; :normal_user/admin → update all. + policy action_type([:create, :update]) do + description "Forbid user link unless admin; then check permissions" + forbid_if Mv.Authorization.Checks.ForbidMemberUserLinkUnlessAdmin authorize_if Mv.Authorization.Checks.HasPermission end @@ -381,6 +388,9 @@ defmodule Mv.Membership.Member do # Validates that member email is not already used by another (unlinked) user validate Mv.Membership.Member.Validations.EmailNotUsedByOtherUser + # Only admins or the linked user may change a linked member's email (prevents breaking sync) + validate Mv.Membership.Member.Validations.EmailChangePermission, on: [:update] + # Prevent linking to a user that already has a member # This validation prevents "stealing" users from other members by checking # if the target user is already linked to a different member diff --git a/lib/membership/member/changes/unrelate_user_when_argument_nil.ex b/lib/membership/member/changes/unrelate_user_when_argument_nil.ex new file mode 100644 index 0000000..dc4d097 --- /dev/null +++ b/lib/membership/member/changes/unrelate_user_when_argument_nil.ex @@ -0,0 +1,50 @@ +defmodule Mv.Membership.Member.Changes.UnrelateUserWhenArgumentNil do + @moduledoc """ + When :user argument is present and nil/empty on update_member, unrelate the current user. + + With on_missing: :ignore, manage_relationship does not unrelate when input is nil/[]. + This change handles explicit unlink (user: nil or user: %{}) by updating the linked + User to set member_id = nil. Only runs when the argument key is present (policy + ForbidMemberUserLinkUnlessAdmin ensures only admins can pass :user). + """ + use Ash.Resource.Change + + @spec change(Ash.Changeset.t(), keyword(), Ash.Resource.Change.context()) :: Ash.Changeset.t() + def change(changeset, _opts, _context) do + if unlink_requested?(changeset) do + unrelate_current_user(changeset) + else + changeset + end + end + + defp unlink_requested?(changeset) do + args = changeset.arguments || %{} + + if Map.has_key?(args, :user) or Map.has_key?(args, "user") do + user_arg = Ash.Changeset.get_argument(changeset, :user) + user_arg == nil or (is_map(user_arg) and map_size(user_arg) == 0) + else + false + end + end + + defp unrelate_current_user(changeset) do + member = changeset.data + actor = Map.get(changeset.context || %{}, :actor) + + case Ash.load(member, :user, domain: Mv.Membership, authorize?: false) do + {:ok, %{user: user}} when not is_nil(user) -> + # User's :update action only accepts [:email]; use :update_user so + # manage_relationship(:member, ..., on_missing: :unrelate) runs and clears member_id. + user + |> Ash.Changeset.for_update(:update_user, %{member: nil}, domain: Mv.Accounts) + |> Ash.update(domain: Mv.Accounts, actor: actor, authorize?: false) + + changeset + + _ -> + changeset + end + end +end diff --git a/lib/membership/member_group.ex b/lib/membership/member_group.ex index 5d29bda..22a1f70 100644 --- a/lib/membership/member_group.ex +++ b/lib/membership/member_group.ex @@ -39,7 +39,8 @@ defmodule Mv.Membership.MemberGroup do """ use Ash.Resource, domain: Mv.Membership, - data_layer: AshPostgres.DataLayer + data_layer: AshPostgres.DataLayer, + authorizers: [Ash.Policy.Authorizer] require Ash.Query @@ -56,6 +57,26 @@ defmodule Mv.Membership.MemberGroup do end end + # Authorization: read uses bypass for :linked (own_data only) then HasPermission for :all; + # create/destroy use HasPermission (normal_user + admin only). + # Single check: own_data gets filter via auto_filter; admin does not match, gets :all from HasPermission. + policies do + bypass action_type(:read) do + description "own_data: read only member_groups where member_id == actor.member_id" + authorize_if Mv.Authorization.Checks.MemberGroupReadLinkedForOwnData + end + + policy action_type(:read) do + description "Check read permission from role (read_only/normal_user/admin :all)" + authorize_if Mv.Authorization.Checks.HasPermission + end + + policy action_type([:create, :destroy]) do + description "Check create/destroy from role (normal_user + admin only)" + authorize_if Mv.Authorization.Checks.HasPermission + end + end + validations do validate present(:member_id) validate present(:group_id) @@ -118,7 +139,7 @@ defmodule Mv.Membership.MemberGroup do query = Mv.Membership.MemberGroup |> Ash.Query.filter(member_id == ^member_id and group_id == ^group_id) - |> maybe_exclude_id(exclude_id) + |> Helpers.query_exclude_id(exclude_id) opts = Helpers.ash_actor_opts(actor) @@ -135,7 +156,4 @@ defmodule Mv.Membership.MemberGroup do :ok end end - - defp maybe_exclude_id(query, nil), do: query - defp maybe_exclude_id(query, id), do: Ash.Query.filter(query, id != ^id) end diff --git a/lib/membership/setting.ex b/lib/membership/setting.ex index 4ba0794..bb7d122 100644 --- a/lib/membership/setting.ex +++ b/lib/membership/setting.ex @@ -155,12 +155,15 @@ defmodule Mv.Membership.Setting do on: [:create, :update] # Validate default_membership_fee_type_id exists if set - validate fn changeset, _context -> + validate fn changeset, context -> fee_type_id = Ash.Changeset.get_attribute(changeset, :default_membership_fee_type_id) if fee_type_id do - case Ash.get(Mv.MembershipFees.MembershipFeeType, fee_type_id) do + # Check existence only; action is already restricted by policy (e.g. admin). + opts = [domain: Mv.MembershipFees, authorize?: false] + + case Ash.get(Mv.MembershipFees.MembershipFeeType, fee_type_id, opts) do {:ok, _} -> :ok diff --git a/lib/membership_fees/changes/set_membership_fee_start_date.ex b/lib/membership_fees/changes/set_membership_fee_start_date.ex index a2e1ad0..0e9cf00 100644 --- a/lib/membership_fees/changes/set_membership_fee_start_date.ex +++ b/lib/membership_fees/changes/set_membership_fee_start_date.ex @@ -31,12 +31,12 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDate do alias Mv.MembershipFees.CalendarCycles @impl true - def change(changeset, _opts, _context) do + def change(changeset, _opts, context) do # Only calculate if membership_fee_start_date is not already set if has_start_date?(changeset) do changeset else - calculate_and_set_start_date(changeset) + calculate_and_set_start_date(changeset, context) end end @@ -56,10 +56,13 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDate do end end - defp calculate_and_set_start_date(changeset) do + defp calculate_and_set_start_date(changeset, context) do + actor = Map.get(context || %{}, :actor) + opts = if actor, do: [actor: actor], else: [] + with {:ok, join_date} <- get_join_date(changeset), {:ok, membership_fee_type_id} <- get_membership_fee_type_id(changeset), - {:ok, interval} <- get_interval(membership_fee_type_id), + {:ok, interval} <- get_interval(membership_fee_type_id, opts), {:ok, include_joining_cycle} <- get_include_joining_cycle() do start_date = calculate_start_date(join_date, interval, include_joining_cycle) Ash.Changeset.force_change_attribute(changeset, :membership_fee_start_date, start_date) @@ -118,8 +121,8 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDate do end end - defp get_interval(membership_fee_type_id) do - case Ash.get(Mv.MembershipFees.MembershipFeeType, membership_fee_type_id) do + defp get_interval(membership_fee_type_id, opts) do + case Ash.get(Mv.MembershipFees.MembershipFeeType, membership_fee_type_id, opts) do {:ok, %{interval: interval}} -> {:ok, interval} {:error, _} -> {:error, :membership_fee_type_not_found} end diff --git a/lib/membership_fees/changes/validate_same_interval.ex b/lib/membership_fees/changes/validate_same_interval.ex index 8c1efb4..0ad32a1 100644 --- a/lib/membership_fees/changes/validate_same_interval.ex +++ b/lib/membership_fees/changes/validate_same_interval.ex @@ -19,9 +19,9 @@ defmodule Mv.MembershipFees.Changes.ValidateSameInterval do use Ash.Resource.Change @impl true - def change(changeset, _opts, _context) do + def change(changeset, _opts, context) do if changing_membership_fee_type?(changeset) do - validate_interval_match(changeset) + validate_interval_match(changeset, context) else changeset end @@ -33,9 +33,10 @@ defmodule Mv.MembershipFees.Changes.ValidateSameInterval do end # Validate that the new type has the same interval as the current type - defp validate_interval_match(changeset) do + defp validate_interval_match(changeset, context) do current_type_id = get_current_type_id(changeset) new_type_id = get_new_type_id(changeset) + actor = Map.get(context || %{}, :actor) cond do # If no current type, allow any change (first assignment) @@ -48,13 +49,13 @@ defmodule Mv.MembershipFees.Changes.ValidateSameInterval do # Both types exist - validate intervals match true -> - validate_intervals_match(changeset, current_type_id, new_type_id) + validate_intervals_match(changeset, current_type_id, new_type_id, actor) end end # Validates that intervals match when both types exist - defp validate_intervals_match(changeset, current_type_id, new_type_id) do - case get_intervals(current_type_id, new_type_id) do + defp validate_intervals_match(changeset, current_type_id, new_type_id, actor) do + case get_intervals(current_type_id, new_type_id, actor) do {:ok, current_interval, new_interval} -> if current_interval == new_interval do changeset @@ -85,11 +86,16 @@ defmodule Mv.MembershipFees.Changes.ValidateSameInterval do end end - # Get intervals for both types - defp get_intervals(current_type_id, new_type_id) do + # Get intervals for both types (actor required for authorization when resource has policies) + defp get_intervals(current_type_id, new_type_id, actor) do alias Mv.MembershipFees.MembershipFeeType - case {Ash.get(MembershipFeeType, current_type_id), Ash.get(MembershipFeeType, new_type_id)} do + opts = if actor, do: [actor: actor], else: [] + + case { + Ash.get(MembershipFeeType, current_type_id, opts), + Ash.get(MembershipFeeType, new_type_id, opts) + } do {{:ok, current_type}, {:ok, new_type}} -> {:ok, current_type.interval, new_type.interval} diff --git a/lib/membership_fees/membership_fee_cycle.ex b/lib/membership_fees/membership_fee_cycle.ex index 4d9c8b7..f0dd1a7 100644 --- a/lib/membership_fees/membership_fee_cycle.ex +++ b/lib/membership_fees/membership_fee_cycle.ex @@ -28,7 +28,8 @@ defmodule Mv.MembershipFees.MembershipFeeCycle do """ use Ash.Resource, domain: Mv.MembershipFees, - data_layer: AshPostgres.DataLayer + data_layer: AshPostgres.DataLayer, + authorizers: [Ash.Policy.Authorizer] postgres do table "membership_fee_cycles" @@ -83,6 +84,19 @@ defmodule Mv.MembershipFees.MembershipFeeCycle do end end + # READ: bypass for own_data (:linked) then HasPermission for :all; create/update/destroy: HasPermission only. + policies do + bypass action_type(:read) do + description "own_data: read only cycles where member_id == actor.member_id" + authorize_if Mv.Authorization.Checks.MembershipFeeCycleReadLinkedForOwnData + end + + policy action_type([:read, :create, :update, :destroy]) do + description "Check permissions from role (all read; normal_user and admin create/update/destroy)" + authorize_if Mv.Authorization.Checks.HasPermission + end + end + attributes do uuid_v7_primary_key :id diff --git a/lib/membership_fees/membership_fee_type.ex b/lib/membership_fees/membership_fee_type.ex index 498ff75..8ec9467 100644 --- a/lib/membership_fees/membership_fee_type.ex +++ b/lib/membership_fees/membership_fee_type.ex @@ -24,7 +24,8 @@ defmodule Mv.MembershipFees.MembershipFeeType do """ use Ash.Resource, domain: Mv.MembershipFees, - data_layer: AshPostgres.DataLayer + data_layer: AshPostgres.DataLayer, + authorizers: [Ash.Policy.Authorizer] postgres do table "membership_fee_types" @@ -61,6 +62,13 @@ defmodule Mv.MembershipFees.MembershipFeeType do end end + policies do + policy action_type([:read, :create, :update, :destroy]) do + description "Check permissions from role (all can read, only admin can create/update/destroy)" + authorize_if Mv.Authorization.Checks.HasPermission + end + end + validations do # Prevent interval changes after creation validate fn changeset, _context -> diff --git a/lib/mv/accounts/user/validations/email_not_used_by_other_member.ex b/lib/mv/accounts/user/validations/email_not_used_by_other_member.ex index 0e693e1..72cc10c 100644 --- a/lib/mv/accounts/user/validations/email_not_used_by_other_member.ex +++ b/lib/mv/accounts/user/validations/email_not_used_by_other_member.ex @@ -81,7 +81,7 @@ defmodule Mv.Accounts.User.Validations.EmailNotUsedByOtherMember do query = Mv.Membership.Member |> Ash.Query.filter(email == ^to_string(email)) - |> maybe_exclude_id(exclude_member_id) + |> Mv.Helpers.query_exclude_id(exclude_member_id) system_actor = SystemActor.get_system_actor() opts = Helpers.ash_actor_opts(system_actor) @@ -101,7 +101,4 @@ defmodule Mv.Accounts.User.Validations.EmailNotUsedByOtherMember do :ok end end - - defp maybe_exclude_id(query, nil), do: query - defp maybe_exclude_id(query, id), do: Ash.Query.filter(query, id != ^id) end diff --git a/lib/mv/authorization/actor.ex b/lib/mv/authorization/actor.ex index 3482043..edc6b8b 100644 --- a/lib/mv/authorization/actor.ex +++ b/lib/mv/authorization/actor.ex @@ -1,6 +1,7 @@ defmodule Mv.Authorization.Actor do @moduledoc """ - Helper functions for ensuring User actors have required data loaded. + Helper functions for ensuring User actors have required data loaded + and for querying actor capabilities (e.g. admin, permission set). ## Actor Invariant @@ -27,8 +28,11 @@ defmodule Mv.Authorization.Actor do assign(socket, :current_user, user) end - # In tests - user = Actor.ensure_loaded(user) + # Check if actor is admin (policy checks, validations) + if Actor.admin?(actor), do: ... + + # Get permission set name (string or nil) + ps_name = Actor.permission_set_name(actor) ## Security Note @@ -47,6 +51,8 @@ defmodule Mv.Authorization.Actor do require Logger + alias Mv.Helpers.SystemActor + @doc """ Ensures the actor (User) has their `:role` relationship loaded. @@ -96,4 +102,45 @@ defmodule Mv.Authorization.Actor do actor end end + + @doc """ + Returns the actor's permission set name (string or atom) from their role, or nil. + + Ensures role is loaded (including when role is nil). Supports both atom and + string keys for session/socket assigns. Use for capability checks consistent + with `ActorIsAdmin` and `HasPermission`. + """ + @spec permission_set_name(Mv.Accounts.User.t() | map() | nil) :: String.t() | atom() | nil + def permission_set_name(nil), do: nil + + def permission_set_name(actor) do + actor = actor |> ensure_loaded() |> maybe_load_role() + + get_in(actor, [Access.key(:role), Access.key(:permission_set_name)]) || + get_in(actor, [Access.key("role"), Access.key("permission_set_name")]) + end + + @doc """ + Returns true if the actor is the system user or has the admin permission set. + + Use for validations and policy checks that require admin capability (e.g. + changing a linked member's email). Consistent with `ActorIsAdmin` policy check. + """ + @spec admin?(Mv.Accounts.User.t() | map() | nil) :: boolean() + def admin?(nil), do: false + + def admin?(actor) do + SystemActor.system_user?(actor) or permission_set_name(actor) in ["admin", :admin] + end + + # Load role only when it is nil (e.g. actor from session without role). ensure_loaded/1 + # already handles %Ash.NotLoaded{}, so we do not double-load in the normal Ash path. + defp maybe_load_role(%Mv.Accounts.User{role: nil} = user) do + case Ash.load(user, :role, domain: Mv.Accounts, authorize?: false) do + {:ok, loaded} -> loaded + _ -> user + end + end + + defp maybe_load_role(actor), do: actor end diff --git a/lib/mv/authorization/checks/actor_is_admin.ex b/lib/mv/authorization/checks/actor_is_admin.ex index 2328876..413c6c7 100644 --- a/lib/mv/authorization/checks/actor_is_admin.ex +++ b/lib/mv/authorization/checks/actor_is_admin.ex @@ -1,22 +1,18 @@ defmodule Mv.Authorization.Checks.ActorIsAdmin do @moduledoc """ - Policy check: true when the actor's role has permission_set_name "admin". + Policy check: true when the actor is the system user or has permission_set_name "admin". Used to restrict actions (e.g. User.update_user for member link/unlink) to admins only. + Delegates to `Mv.Authorization.Actor.admin?/1`, which returns true for the system actor + or for a user whose role has permission_set_name "admin". """ use Ash.Policy.SimpleCheck + alias Mv.Authorization.Actor + @impl true def describe(_opts), do: "actor has admin permission set" @impl true - def match?(nil, _context, _opts), do: false - - def match?(actor, _context, _opts) do - ps_name = - get_in(actor, [Access.key(:role), Access.key(:permission_set_name)]) || - get_in(actor, [Access.key("role"), Access.key("permission_set_name")]) - - ps_name == "admin" - end + def match?(actor, _context, _opts), do: Actor.admin?(actor) end diff --git a/lib/mv/authorization/checks/actor_permission_set_is.ex b/lib/mv/authorization/checks/actor_permission_set_is.ex new file mode 100644 index 0000000..deb9382 --- /dev/null +++ b/lib/mv/authorization/checks/actor_permission_set_is.ex @@ -0,0 +1,44 @@ +defmodule Mv.Authorization.Checks.ActorPermissionSetIs do + @moduledoc """ + Policy check: true when the actor's role has the given permission_set_name. + + Used to restrict bypass policies (e.g. MemberGroup read by member_id) to actors + with a specific permission set (e.g. "own_data") so that admin with member_id + still gets :all scope from HasPermission, not the bypass filter. + + ## Usage + + # In a resource policy (both conditions must hold for the bypass) + bypass action_type(:read) do + authorize_if expr(member_id == ^actor(:member_id)) + authorize_if {Mv.Authorization.Checks.ActorPermissionSetIs, permission_set_name: "own_data"} + end + + ## Options + + - `:permission_set_name` (required) - String or atom, e.g. `"own_data"` or `:own_data` + """ + use Ash.Policy.SimpleCheck + + alias Mv.Authorization.Actor + + @impl true + def describe(opts) do + name = opts[:permission_set_name] || "?" + "actor has permission set #{name}" + end + + @impl true + def match?(actor, _context, opts) do + case opts[:permission_set_name] do + nil -> + false + + expected -> + case Actor.permission_set_name(actor) do + nil -> false + actual -> to_string(expected) == to_string(actual) + end + end + end +end diff --git a/lib/mv/authorization/checks/forbid_member_user_link_unless_admin.ex b/lib/mv/authorization/checks/forbid_member_user_link_unless_admin.ex new file mode 100644 index 0000000..1e7cb77 --- /dev/null +++ b/lib/mv/authorization/checks/forbid_member_user_link_unless_admin.ex @@ -0,0 +1,71 @@ +defmodule Mv.Authorization.Checks.ForbidMemberUserLinkUnlessAdmin do + @moduledoc """ + Policy check: forbids setting or changing the member–user link unless the actor is admin. + + Used on Member create_member and update_member actions. When the `:user` argument + **is present** (key in arguments, regardless of value), only admins may perform the action. + This covers: + - **Linking:** `user: %{id: user_id}` → only admin + - **Unlinking:** explicit `user: nil` or `user: %{}` on update_member → only admin + Non-admin users can create and update members only when they do **not** pass the + `:user` argument; omitting `:user` leaves the relationship unchanged. + + ## Unlink semantics (update_member) + + The Member resource uses `on_missing: :ignore` for the `:user` relationship on update. + So **omitting** `:user` from params does **not** change the link (no "unlink by omission"). + Unlink is only possible by **explicitly** passing `:user` (e.g. `user: nil`), which this + check forbids for non-admins. Admins may link or unlink via the `:user` argument. + + ## Usage + + In Member resource policies, restrict to create/update only: + + policy action_type([:create, :update]) do + forbid_if Mv.Authorization.Checks.ForbidMemberUserLinkUnlessAdmin + authorize_if Mv.Authorization.Checks.HasPermission + end + + ## Behaviour + + - If the `:user` argument **key is not present** → does not forbid. + - If `:user` is present (any value, including nil or %{}) and actor is not admin → forbids. + - If actor is nil → treated as non-admin (forbid when :user present). `Actor.admin?(nil)` is defined and returns false. + - If actor is admin (or system actor) → does not forbid. + """ + use Ash.Policy.Check + + alias Mv.Authorization.Actor + + @impl true + def describe(_opts), do: "forbid setting member–user link unless actor is admin" + + @impl true + def strict_check(actor, authorizer, _opts) do + # Nil actor: treat as non-admin (Actor.admin?(nil) returns false; no crash) + actor = if is_nil(actor), do: nil, else: Actor.ensure_loaded(actor) + + if user_argument_present?(authorizer) and not Actor.admin?(actor) do + {:ok, true} + else + {:ok, false} + end + end + + # Forbid when :user was passed at all (link, unlink via nil/empty, or invalid value). + # Check argument key presence (atom or string) for defense-in-depth. + defp user_argument_present?(authorizer) do + args = get_arguments(authorizer) || %{} + Map.has_key?(args, :user) or Map.has_key?(args, "user") + end + + defp get_arguments(authorizer) do + subject = authorizer.changeset || authorizer.subject + + cond do + is_struct(subject, Ash.Changeset) -> subject.arguments + is_struct(subject, Ash.ActionInput) -> subject.arguments + true -> %{} + end + end +end diff --git a/lib/mv/authorization/checks/has_permission.ex b/lib/mv/authorization/checks/has_permission.ex index 774e767..721cee7 100644 --- a/lib/mv/authorization/checks/has_permission.ex +++ b/lib/mv/authorization/checks/has_permission.ex @@ -50,6 +50,7 @@ defmodule Mv.Authorization.Checks.HasPermission do - **:linked** - Filters based on resource type: - Member: `id == actor.member_id` (User.member_id → Member.id, inverse relationship) - CustomFieldValue: `member_id == actor.member_id` (CustomFieldValue.member_id → Member.id → User.member_id) + - MemberGroup: `member_id == actor.member_id` (MemberGroup.member_id → Member.id → User.member_id) ## Error Handling @@ -131,26 +132,10 @@ defmodule Mv.Authorization.Checks.HasPermission do resource_name ) do :authorized -> - # For :all scope, authorize directly {:ok, true} {:filter, filter_expr} -> - # For :own/:linked scope: - # - With a record, evaluate filter against record for strict authorization - # - Without a record (queries/lists), return false - # - # NOTE: Returning false here forces the use of expr-based bypass policies. - # This is necessary because Ash's policy evaluation doesn't reliably call auto_filter - # when strict_check returns :unknown. Instead, resources should use bypass policies - # with expr() directly for filter-based authorization (see User resource). - if record do - evaluate_filter_for_strict_check(filter_expr, actor, record, resource_name) - else - # No record yet (e.g., read/list queries) - deny at strict_check level - # Resources must use expr-based bypass policies for list filtering - # Create: use a dedicated check that does not return a filter (e.g. CustomFieldValueCreateScope) - {:ok, false} - end + strict_check_filter_scope(record, filter_expr, actor, resource_name) false -> {:ok, false} @@ -174,6 +159,15 @@ defmodule Mv.Authorization.Checks.HasPermission do end end + # For :own/:linked scope: with record evaluate filter; without record deny (resources use bypass + expr). + defp strict_check_filter_scope(record, filter_expr, actor, resource_name) do + if record do + evaluate_filter_for_strict_check(filter_expr, actor, record, resource_name) + else + {:ok, false} + end + end + @impl true def auto_filter(actor, authorizer, _opts) do resource = authorizer.resource @@ -278,36 +272,28 @@ defmodule Mv.Authorization.Checks.HasPermission do # For :own scope with User resource: id == actor.id # For :linked scope with Member resource: id == actor.member_id defp evaluate_filter_for_strict_check(_filter_expr, actor, record, resource_name) do - case {resource_name, record} do - {"User", %{id: user_id}} when not is_nil(user_id) -> - # Check if this user's ID matches the actor's ID (scope :own) - if user_id == actor.id do - {:ok, true} - else - {:ok, false} - end + result = + case {resource_name, record} do + # Scope :own + {"User", %{id: user_id}} when not is_nil(user_id) -> + user_id == actor.id - {"Member", %{id: member_id}} when not is_nil(member_id) -> - # Check if this member's ID matches the actor's member_id - if member_id == actor.member_id do - {:ok, true} - else - {:ok, false} - end + # Scope :linked + {"Member", %{id: member_id}} when not is_nil(member_id) -> + member_id == actor.member_id - {"CustomFieldValue", %{member_id: cfv_member_id}} when not is_nil(cfv_member_id) -> - # Check if this CFV's member_id matches the actor's member_id - if cfv_member_id == actor.member_id do - {:ok, true} - else - {:ok, false} - end + {"CustomFieldValue", %{member_id: cfv_member_id}} when not is_nil(cfv_member_id) -> + cfv_member_id == actor.member_id - _ -> - # For other cases or when record is not available, return :unknown - # This will cause Ash to use auto_filter instead - {:ok, :unknown} - end + {"MemberGroup", %{member_id: mg_member_id}} when not is_nil(mg_member_id) -> + mg_member_id == actor.member_id + + _ -> + :unknown + end + + out = if result == :unknown, do: {:ok, :unknown}, else: {:ok, result} + out end # Extract resource name from module (e.g., Mv.Membership.Member -> "Member") @@ -347,24 +333,20 @@ defmodule Mv.Authorization.Checks.HasPermission do defp apply_scope(:linked, actor, resource_name) do case resource_name do "Member" -> - # User.member_id → Member.id (inverse relationship) - # Filter: member.id == actor.member_id - # If actor has no member_id, return no results (use false or impossible condition) - if is_nil(actor.member_id) do - {:filter, expr(false)} - else - {:filter, expr(id == ^actor.member_id)} - end + # User.member_id → Member.id (inverse relationship). Filter: member.id == actor.member_id + linked_filter_by_member_id(actor, :id) "CustomFieldValue" -> # CustomFieldValue.member_id → Member.id → User.member_id - # Filter: custom_field_value.member_id == actor.member_id - # If actor has no member_id, return no results - if is_nil(actor.member_id) do - {:filter, expr(false)} - else - {:filter, expr(member_id == ^actor.member_id)} - end + linked_filter_by_member_id(actor, :member_id) + + "MemberGroup" -> + # MemberGroup.member_id → Member.id → User.member_id (own linked member's group associations) + linked_filter_by_member_id(actor, :member_id) + + "MembershipFeeCycle" -> + # MembershipFeeCycle.member_id → Member.id → User.member_id (own linked member's cycles) + linked_filter_by_member_id(actor, :member_id) _ -> # Fallback for other resources @@ -372,6 +354,17 @@ defmodule Mv.Authorization.Checks.HasPermission do end end + # Returns {:filter, expr(false)} if actor has no member_id; otherwise {:filter, expr(field == ^actor.member_id)}. + # Used for :linked scope on Member (field :id), CustomFieldValue and MemberGroup (field :member_id). + defp linked_filter_by_member_id(actor, _field) when is_nil(actor.member_id) do + {:filter, expr(false)} + end + + defp linked_filter_by_member_id(actor, :id), do: {:filter, expr(id == ^actor.member_id)} + + defp linked_filter_by_member_id(actor, :member_id), + do: {:filter, expr(member_id == ^actor.member_id)} + # Log authorization failures for debugging (lazy evaluation) defp log_auth_failure(actor, resource, action, reason) do Logger.debug(fn -> diff --git a/lib/mv/authorization/checks/member_group_read_linked_for_own_data.ex b/lib/mv/authorization/checks/member_group_read_linked_for_own_data.ex new file mode 100644 index 0000000..a553fde --- /dev/null +++ b/lib/mv/authorization/checks/member_group_read_linked_for_own_data.ex @@ -0,0 +1,63 @@ +defmodule Mv.Authorization.Checks.MemberGroupReadLinkedForOwnData do + @moduledoc """ + Policy check for MemberGroup read: true only when actor has permission set "own_data" + AND record.member_id == actor.member_id. + + Used in a bypass so that own_data gets the linked filter (via auto_filter for list queries), + while admin with member_id does not match and gets :all from HasPermission. + + - With a record (e.g. get by id): returns true only when own_data and member_id match. + - Without a record (list query): strict_check returns false; auto_filter adds filter when own_data. + """ + use Ash.Policy.Check + + alias Mv.Authorization.Checks.ActorPermissionSetIs + + @impl true + def type, do: :filter + + @impl true + def describe(_opts), + do: "own_data can read only member_groups where member_id == actor.member_id" + + @impl true + def strict_check(actor, authorizer, _opts) do + record = get_record_from_authorizer(authorizer) + is_own_data = ActorPermissionSetIs.match?(actor, authorizer, permission_set_name: "own_data") + + cond do + # List query + own_data: return :unknown so authorizer applies auto_filter (keyword list) + is_nil(record) and is_own_data -> + {:ok, :unknown} + + is_nil(record) -> + {:ok, false} + + not is_own_data -> + {:ok, false} + + record.member_id == actor.member_id -> + {:ok, true} + + true -> + {:ok, false} + end + end + + @impl true + def auto_filter(actor, _authorizer, _opts) do + if ActorPermissionSetIs.match?(actor, nil, permission_set_name: "own_data") && + Map.get(actor, :member_id) do + [member_id: actor.member_id] + else + [] + end + end + + defp get_record_from_authorizer(authorizer) do + case authorizer.subject do + %{data: data} when not is_nil(data) -> data + _ -> nil + end + end +end diff --git a/lib/mv/authorization/checks/membership_fee_cycle_read_linked_for_own_data.ex b/lib/mv/authorization/checks/membership_fee_cycle_read_linked_for_own_data.ex new file mode 100644 index 0000000..092558c --- /dev/null +++ b/lib/mv/authorization/checks/membership_fee_cycle_read_linked_for_own_data.ex @@ -0,0 +1,62 @@ +defmodule Mv.Authorization.Checks.MembershipFeeCycleReadLinkedForOwnData do + @moduledoc """ + Policy check for MembershipFeeCycle read: true only when actor has permission set "own_data" + AND record.member_id == actor.member_id. + + Used in a bypass so that own_data gets the linked filter (via auto_filter for list queries), + while admin with member_id does not match and gets :all from HasPermission. + + - With a record (e.g. get by id): returns true only when own_data and member_id match. + - Without a record (list query): return :unknown so authorizer applies auto_filter. + """ + use Ash.Policy.Check + + alias Mv.Authorization.Checks.ActorPermissionSetIs + + @impl true + def type, do: :filter + + @impl true + def describe(_opts), + do: "own_data can read only membership_fee_cycles where member_id == actor.member_id" + + @impl true + def strict_check(actor, authorizer, _opts) do + record = get_record_from_authorizer(authorizer) + is_own_data = ActorPermissionSetIs.match?(actor, authorizer, permission_set_name: "own_data") + + cond do + is_nil(record) and is_own_data -> + {:ok, :unknown} + + is_nil(record) -> + {:ok, false} + + not is_own_data -> + {:ok, false} + + record.member_id == actor.member_id -> + {:ok, true} + + true -> + {:ok, false} + end + end + + @impl true + def auto_filter(actor, _authorizer, _opts) do + if ActorPermissionSetIs.match?(actor, nil, permission_set_name: "own_data") && + Map.get(actor, :member_id) do + [member_id: actor.member_id] + else + [] + end + end + + defp get_record_from_authorizer(authorizer) do + case authorizer.subject do + %{data: data} when not is_nil(data) -> data + _ -> nil + end + end +end diff --git a/lib/mv/authorization/checks/oidc_role_sync_context.ex b/lib/mv/authorization/checks/oidc_role_sync_context.ex new file mode 100644 index 0000000..1214d75 --- /dev/null +++ b/lib/mv/authorization/checks/oidc_role_sync_context.ex @@ -0,0 +1,18 @@ +defmodule Mv.Authorization.Checks.OidcRoleSyncContext do + @moduledoc """ + Policy check: true when the action is run from OIDC role sync (context.private.oidc_role_sync). + + Used to allow the internal set_role_from_oidc_sync action only when called by Mv.OidcRoleSync, + which sets context.private.oidc_role_sync when performing the update. + """ + use Ash.Policy.SimpleCheck + + @impl true + def describe(_opts), do: "called from OIDC role sync (context.private.oidc_role_sync)" + + @impl true + def match?(_actor, authorizer, _opts) do + context = Map.get(authorizer, :context) || %{} + get_in(context, [:private, :oidc_role_sync]) == true + end +end diff --git a/lib/mv/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex index 858748d..b0e7015 100644 --- a/lib/mv/authorization/permission_sets.ex +++ b/lib/mv/authorization/permission_sets.ex @@ -58,6 +58,28 @@ defmodule Mv.Authorization.PermissionSets do pages: [String.t()] } + # DRY helpers for shared resource permission lists (used in own_data, read_only, normal_user, admin) + defp perm(resource, action, scope), + do: %{resource: resource, action: action, scope: scope, granted: true} + + # All four CRUD actions for a resource with scope :all (used for admin) + defp perm_all(resource), + do: [ + perm(resource, :read, :all), + perm(resource, :create, :all), + perm(resource, :update, :all), + perm(resource, :destroy, :all) + ] + + # User: read/update own credentials only (all non-admin sets allow password changes) + defp user_own_credentials, do: [perm("User", :read, :own), perm("User", :update, :own)] + + defp group_read_all, do: [perm("Group", :read, :all)] + defp custom_field_read_all, do: [perm("CustomField", :read, :all)] + defp membership_fee_type_read_all, do: [perm("MembershipFeeType", :read, :all)] + defp membership_fee_cycle_read_all, do: [perm("MembershipFeeCycle", :read, :all)] + defp role_read_all, do: [perm("Role", :read, :all)] + @doc """ Returns the list of all valid permission set names. @@ -94,29 +116,22 @@ defmodule Mv.Authorization.PermissionSets do def get_permissions(:own_data) do %{ - resources: [ - # User: Can read/update own credentials only - # IMPORTANT: "read_only" refers to member data, NOT user credentials. - # All permission sets grant User.update :own to allow password changes. - %{resource: "User", action: :read, scope: :own, granted: true}, - %{resource: "User", action: :update, scope: :own, granted: true}, - - # Member: Can read/update linked member - %{resource: "Member", action: :read, scope: :linked, granted: true}, - %{resource: "Member", action: :update, scope: :linked, granted: true}, - - # CustomFieldValue: Can read/update/create/destroy custom field values of linked member - %{resource: "CustomFieldValue", action: :read, scope: :linked, granted: true}, - %{resource: "CustomFieldValue", action: :update, scope: :linked, granted: true}, - %{resource: "CustomFieldValue", action: :create, scope: :linked, granted: true}, - %{resource: "CustomFieldValue", action: :destroy, scope: :linked, granted: true}, - - # CustomField: Can read all (needed for forms) - %{resource: "CustomField", action: :read, scope: :all, granted: true}, - - # Group: Can read all (needed for viewing groups) - %{resource: "Group", action: :read, scope: :all, granted: true} - ], + resources: + user_own_credentials() ++ + [ + perm("Member", :read, :linked), + perm("Member", :update, :linked), + perm("CustomFieldValue", :read, :linked), + perm("CustomFieldValue", :update, :linked), + perm("CustomFieldValue", :create, :linked), + perm("CustomFieldValue", :destroy, :linked) + ] ++ + custom_field_read_all() ++ + group_read_all() ++ + [perm("MemberGroup", :read, :linked)] ++ + membership_fee_type_read_all() ++ + [perm("MembershipFeeCycle", :read, :linked)] ++ + role_read_all(), pages: [ # No "/" - Mitglied must not see member index at root (same content as /members). # Own profile (sidebar links to /users/:id) and own user edit @@ -133,25 +148,18 @@ defmodule Mv.Authorization.PermissionSets do def get_permissions(:read_only) do %{ - resources: [ - # User: Can read/update own credentials only - # IMPORTANT: "read_only" refers to member data, NOT user credentials. - # All permission sets grant User.update :own to allow password changes. - %{resource: "User", action: :read, scope: :own, granted: true}, - %{resource: "User", action: :update, scope: :own, granted: true}, - - # Member: Can read all members, no modifications - %{resource: "Member", action: :read, scope: :all, granted: true}, - - # CustomFieldValue: Can read all custom field values - %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, - - # CustomField: Can read all - %{resource: "CustomField", action: :read, scope: :all, granted: true}, - - # Group: Can read all - %{resource: "Group", action: :read, scope: :all, granted: true} - ], + resources: + user_own_credentials() ++ + [ + perm("Member", :read, :all), + perm("CustomFieldValue", :read, :all) + ] ++ + custom_field_read_all() ++ + group_read_all() ++ + [perm("MemberGroup", :read, :all)] ++ + membership_fee_type_read_all() ++ + membership_fee_cycle_read_all() ++ + role_read_all(), pages: [ "/", # Own profile (sidebar links to /users/:id; redirect target must be allowed) @@ -176,31 +184,38 @@ defmodule Mv.Authorization.PermissionSets do def get_permissions(:normal_user) do %{ - resources: [ - # User: Can read/update own credentials only - # IMPORTANT: "read_only" refers to member data, NOT user credentials. - # All permission sets grant User.update :own to allow password changes. - %{resource: "User", action: :read, scope: :own, granted: true}, - %{resource: "User", action: :update, scope: :own, granted: true}, - - # Member: Full CRUD except destroy (safety) - %{resource: "Member", action: :read, scope: :all, granted: true}, - %{resource: "Member", action: :create, scope: :all, granted: true}, - %{resource: "Member", action: :update, scope: :all, granted: true}, - # Note: destroy intentionally omitted for safety - - # CustomFieldValue: Full CRUD - %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, - %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true}, - %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true}, - %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true}, - - # CustomField: Read only (admin manages definitions) - %{resource: "CustomField", action: :read, scope: :all, granted: true}, - - # Group: Can read all - %{resource: "Group", action: :read, scope: :all, granted: true} - ], + resources: + user_own_credentials() ++ + [ + perm("Member", :read, :all), + perm("Member", :create, :all), + perm("Member", :update, :all), + # destroy intentionally omitted for safety + perm("CustomFieldValue", :read, :all), + perm("CustomFieldValue", :create, :all), + perm("CustomFieldValue", :update, :all), + perm("CustomFieldValue", :destroy, :all) + ] ++ + custom_field_read_all() ++ + [ + perm("Group", :read, :all), + perm("Group", :create, :all), + perm("Group", :update, :all), + perm("Group", :destroy, :all) + ] ++ + [ + perm("MemberGroup", :read, :all), + perm("MemberGroup", :create, :all), + perm("MemberGroup", :destroy, :all) + ] ++ + membership_fee_type_read_all() ++ + [ + perm("MembershipFeeCycle", :read, :all), + perm("MembershipFeeCycle", :create, :all), + perm("MembershipFeeCycle", :update, :all), + perm("MembershipFeeCycle", :destroy, :all) + ] ++ + role_read_all(), pages: [ "/", # Own profile (sidebar links to /users/:id; redirect target must be allowed) @@ -221,52 +236,39 @@ defmodule Mv.Authorization.PermissionSets do "/custom_field_values/:id/edit", # Groups overview "/groups", + # Create group + "/groups/new", # Group detail - "/groups/:slug" + "/groups/:slug", + # Edit group + "/groups/:slug/edit" ] } end def get_permissions(:admin) do + # MemberGroup has no :update action in the domain; use read/create/destroy only + member_group_perms = [ + perm("MemberGroup", :read, :all), + perm("MemberGroup", :create, :all), + perm("MemberGroup", :destroy, :all) + ] + %{ - resources: [ - # User: Full management including other users - %{resource: "User", action: :read, scope: :all, granted: true}, - %{resource: "User", action: :create, scope: :all, granted: true}, - %{resource: "User", action: :update, scope: :all, granted: true}, - %{resource: "User", action: :destroy, scope: :all, granted: true}, - - # Member: Full CRUD - %{resource: "Member", action: :read, scope: :all, granted: true}, - %{resource: "Member", action: :create, scope: :all, granted: true}, - %{resource: "Member", action: :update, scope: :all, granted: true}, - %{resource: "Member", action: :destroy, scope: :all, granted: true}, - - # CustomFieldValue: Full CRUD - %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, - %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true}, - %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true}, - %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true}, - - # CustomField: Full CRUD (admin manages custom field definitions) - %{resource: "CustomField", action: :read, scope: :all, granted: true}, - %{resource: "CustomField", action: :create, scope: :all, granted: true}, - %{resource: "CustomField", action: :update, scope: :all, granted: true}, - %{resource: "CustomField", action: :destroy, scope: :all, granted: true}, - - # Role: Full CRUD (admin manages roles) - %{resource: "Role", action: :read, scope: :all, granted: true}, - %{resource: "Role", action: :create, scope: :all, granted: true}, - %{resource: "Role", action: :update, scope: :all, granted: true}, - %{resource: "Role", action: :destroy, scope: :all, granted: true}, - - # Group: Full CRUD (admin manages groups) - %{resource: "Group", action: :read, scope: :all, granted: true}, - %{resource: "Group", action: :create, scope: :all, granted: true}, - %{resource: "Group", action: :update, scope: :all, granted: true}, - %{resource: "Group", action: :destroy, scope: :all, granted: true} - ], + resources: + perm_all("User") ++ + perm_all("Member") ++ + perm_all("CustomFieldValue") ++ + perm_all("CustomField") ++ + perm_all("Role") ++ + perm_all("Group") ++ + member_group_perms ++ + perm_all("MembershipFeeType") ++ + perm_all("MembershipFeeCycle"), pages: [ + # Explicit admin-only pages (for clarity and future restrictions) + "/settings", + "/membership_fee_settings", # Wildcard: Admin can access all pages "*" ] diff --git a/lib/mv/authorization/role.ex b/lib/mv/authorization/role.ex index 9c33e2d..8700a33 100644 --- a/lib/mv/authorization/role.ex +++ b/lib/mv/authorization/role.ex @@ -37,7 +37,8 @@ defmodule Mv.Authorization.Role do """ use Ash.Resource, domain: Mv.Authorization, - data_layer: AshPostgres.DataLayer + data_layer: AshPostgres.DataLayer, + authorizers: [Ash.Policy.Authorizer] postgres do table "roles" @@ -86,6 +87,13 @@ defmodule Mv.Authorization.Role do end end + policies do + policy action_type([:read, :create, :update, :destroy]) do + description "Role access: read for all permission sets, create/update/destroy for admin only (PermissionSets)" + authorize_if Mv.Authorization.Checks.HasPermission + end + end + validations do validate one_of( :permission_set_name, @@ -173,4 +181,18 @@ defmodule Mv.Authorization.Role do |> Ash.Query.filter(name == "Mitglied") |> Ash.read_one(authorize?: false, domain: Mv.Authorization) end + + @doc """ + Returns the Admin role if it exists. + + Used by release tasks (e.g. seed_admin) and OIDC role sync to assign the admin role. + """ + @spec get_admin_role() :: {:ok, t() | nil} | {:error, term()} + def get_admin_role do + require Ash.Query + + __MODULE__ + |> Ash.Query.filter(name == "Admin") + |> Ash.read_one(authorize?: false, domain: Mv.Authorization) + end end diff --git a/lib/mv/email_sync/changes/sync_user_email_to_member.ex b/lib/mv/email_sync/changes/sync_user_email_to_member.ex index eb6770c..26b26d4 100644 --- a/lib/mv/email_sync/changes/sync_user_email_to_member.ex +++ b/lib/mv/email_sync/changes/sync_user_email_to_member.ex @@ -27,6 +27,10 @@ defmodule Mv.EmailSync.Changes.SyncUserEmailToMember do Modified changeset with email synchronization applied, or original changeset if recursion detected. """ + # Ash 3.12+ calls this to decide whether to run the change in certain contexts. + @impl true + def has_change?, do: true + @impl true def change(changeset, _opts, context) do # Only recursion protection needed - trigger logic is in `where` clauses @@ -40,26 +44,29 @@ defmodule Mv.EmailSync.Changes.SyncUserEmailToMember do defp sync_email(changeset) do Ash.Changeset.around_transaction(changeset, fn cs, callback -> result = callback.(cs) - - with {:ok, record} <- Helpers.extract_record(result), - {:ok, user, member} <- get_user_and_member(record) do - # When called from Member-side, we need to update the member in the result - # When called from User-side, we update the linked member in DB only - case record do - %Mv.Membership.Member{} -> - # Member-side: Override member email in result with user email - Helpers.override_with_linked_email(result, user.email) - - %Mv.Accounts.User{} -> - # User-side: Sync user email to linked member in DB - Helpers.sync_email_to_linked_record(result, member, user.email) - end - else - _ -> result - end + apply_sync(result) end) end + defp apply_sync(result) do + with {:ok, record} <- Helpers.extract_record(result), + {:ok, user, member} <- get_user_and_member(record) do + sync_by_record_type(result, record, user, member) + else + _ -> result + end + end + + # When called from Member-side, we update the member in the result. + # When called from User-side, we sync user email to the linked member in DB. + defp sync_by_record_type(result, %Mv.Membership.Member{}, user, _member) do + Helpers.override_with_linked_email(result, user.email) + end + + defp sync_by_record_type(result, %Mv.Accounts.User{}, user, member) do + Helpers.sync_email_to_linked_record(result, member, user.email) + end + # Retrieves user and member - works for both resource types # Uses system actor via Loader functions defp get_user_and_member(%Mv.Accounts.User{} = user) do diff --git a/lib/mv/email_sync/loader.ex b/lib/mv/email_sync/loader.ex index 98f85df..31e0468 100644 --- a/lib/mv/email_sync/loader.ex +++ b/lib/mv/email_sync/loader.ex @@ -3,13 +3,15 @@ defmodule Mv.EmailSync.Loader do Helper functions for loading linked records in email synchronization. Centralizes the logic for retrieving related User/Member entities. - ## Authorization + ## Authorization-independent link checks - This module runs systemically and uses the system actor for all operations. - This ensures that email synchronization always works, regardless of user permissions. - - All functions use `Mv.Helpers.SystemActor.get_system_actor/0` to bypass - user permission checks, as email sync is a mandatory side effect. + All functions use the **system actor** for the load. Link existence + (linked vs not linked) is therefore determined **independently of the + current request actor**. This is required so that validations (e.g. + `EmailChangePermission`, `EmailNotUsedByOtherUser`) can correctly decide + "member is linked" even when the current user would not have read permission + on the related User. Using the request actor would otherwise allow + treating a linked member as unlinked and bypass the permission rule. """ alias Mv.Helpers alias Mv.Helpers.SystemActor diff --git a/lib/mv/helpers.ex b/lib/mv/helpers.ex index e20db67..ae22e13 100644 --- a/lib/mv/helpers.ex +++ b/lib/mv/helpers.ex @@ -5,6 +5,8 @@ defmodule Mv.Helpers do Provides utilities that are not specific to a single domain or layer. """ + require Ash.Query + @doc """ Converts an actor to Ash options list for authorization. Returns empty list if actor is nil. @@ -24,4 +26,22 @@ defmodule Mv.Helpers do @spec ash_actor_opts(Mv.Accounts.User.t() | nil) :: keyword() def ash_actor_opts(nil), do: [] def ash_actor_opts(actor) when not is_nil(actor), do: [actor: actor] + + @doc """ + Returns the query unchanged if `exclude_id` is nil; otherwise adds a filter `id != ^exclude_id`. + + Used in uniqueness validations that must exclude the current record (e.g. name uniqueness + on update, duplicate association checks). Call with the record's primary key to exclude it + from the result set. + + ## Examples + + query + |> Ash.Query.filter(name == ^name) + |> Mv.Helpers.query_exclude_id(current_id) + + """ + @spec query_exclude_id(Ash.Query.t(), String.t() | nil) :: Ash.Query.t() + def query_exclude_id(query, nil), do: query + def query_exclude_id(query, id), do: Ash.Query.filter(query, id != ^id) end diff --git a/lib/mv/membership/member/validations/email_change_permission.ex b/lib/mv/membership/member/validations/email_change_permission.ex new file mode 100644 index 0000000..2b1c041 --- /dev/null +++ b/lib/mv/membership/member/validations/email_change_permission.ex @@ -0,0 +1,75 @@ +defmodule Mv.Membership.Member.Validations.EmailChangePermission do + @moduledoc """ + Validates that only admins or the linked user may change a linked member's email. + + This validation runs on member update when the email attribute is changing. + It allows the change only if: + - The member is not linked to a user, or + - The actor has the admin permission set (via `Mv.Authorization.Actor.admin?/1`), or + - The actor is the user linked to this member (actor.member_id == member.id). + + This prevents non-admins from changing another user's linked member email, + which would sync to that user's account and break email synchronization. + + Missing actor is not allowed; the system actor counts as admin (via `Actor.admin?/1`). + """ + use Ash.Resource.Validation + use Gettext, backend: MvWeb.Gettext, otp_app: :mv + + alias Mv.Authorization.Actor + alias Mv.EmailSync.Loader + + @doc """ + Validates that the actor may change the member's email when the member is linked. + + Only runs when the email attribute is changing (checked inside). Skips when + member is not linked. Allows when actor is admin or owns the linked member. + """ + @impl true + def validate(changeset, _opts, context) do + if Ash.Changeset.changing_attribute?(changeset, :email) do + validate_linked_member_email_change(changeset, context) + else + :ok + end + end + + defp validate_linked_member_email_change(changeset, context) do + linked_user = Loader.get_linked_user(changeset.data) + + if is_nil(linked_user) do + :ok + else + actor = resolve_actor(changeset, context) + member_id = changeset.data.id + + if Actor.admin?(actor) or actor_owns_member?(actor, member_id) do + :ok + else + msg = + dgettext( + "default", + "Only administrators or the linked user can change the email for members linked to users" + ) + + {:error, field: :email, message: msg} + end + end + end + + # Ash stores actor in changeset.context.private.actor; validation context has .actor; some callsites use context.actor + defp resolve_actor(changeset, context) do + ctx = changeset.context || %{} + + get_in(ctx, [:private, :actor]) || + Map.get(ctx, :actor) || + (context && Map.get(context, :actor)) + end + + defp actor_owns_member?(nil, _member_id), do: false + + defp actor_owns_member?(actor, member_id) do + actor_member_id = Map.get(actor, :member_id) || Map.get(actor, "member_id") + actor_member_id == member_id + end +end diff --git a/lib/mv/membership/member/validations/email_not_used_by_other_user.ex b/lib/mv/membership/member/validations/email_not_used_by_other_user.ex index f9fba1b..1297515 100644 --- a/lib/mv/membership/member/validations/email_not_used_by_other_user.ex +++ b/lib/mv/membership/member/validations/email_not_used_by_other_user.ex @@ -8,6 +8,8 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do This allows creating members with the same email as unlinked users. """ use Ash.Resource.Validation + + alias Mv.EmailSync.Loader alias Mv.Helpers require Logger @@ -32,7 +34,8 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do def validate(changeset, _opts, _context) do email_changing? = Ash.Changeset.changing_attribute?(changeset, :email) - linked_user_id = get_linked_user_id(changeset.data) + linked_user = Loader.get_linked_user(changeset.data) + linked_user_id = if linked_user, do: linked_user.id, else: nil is_linked? = not is_nil(linked_user_id) # Only validate if member is already linked AND email is changing @@ -53,7 +56,7 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do query = Mv.Accounts.User |> Ash.Query.filter(email == ^email) - |> maybe_exclude_id(exclude_user_id) + |> Mv.Helpers.query_exclude_id(exclude_user_id) system_actor = SystemActor.get_system_actor() opts = Helpers.ash_actor_opts(system_actor) @@ -73,19 +76,4 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do :ok end end - - defp maybe_exclude_id(query, nil), do: query - defp maybe_exclude_id(query, id), do: Ash.Query.filter(query, id != ^id) - - defp get_linked_user_id(member_data) do - alias Mv.Helpers.SystemActor - - system_actor = SystemActor.get_system_actor() - opts = Helpers.ash_actor_opts(system_actor) - - case Ash.load(member_data, :user, opts) do - {:ok, %{user: %{id: id}}} -> id - _ -> nil - end - end end diff --git a/lib/mv/oidc_role_sync.ex b/lib/mv/oidc_role_sync.ex new file mode 100644 index 0000000..f268154 --- /dev/null +++ b/lib/mv/oidc_role_sync.ex @@ -0,0 +1,148 @@ +defmodule Mv.OidcRoleSync do + @moduledoc """ + Syncs user role from OIDC user_info (e.g. groups claim → Admin role). + + Used after OIDC registration (register_with_rauthy) and on sign-in so that + users in the configured admin group get the Admin role; others get Mitglied. + Configure via OIDC_ADMIN_GROUP_NAME and OIDC_GROUPS_CLAIM (see OidcRoleSyncConfig). + + Groups are read from user_info (ID token claims) first; if missing or empty, + the access_token from oauth_tokens is decoded as JWT and the groups claim is + read from there (e.g. Rauthy puts groups in the access token when scope + includes "groups"). + + ## JWT access token (security) + + The access_token payload is read without signature verification (peek only). + We rely on the fact that `oauth_tokens` is only ever passed from the + verified OIDC callback (Assent/AshAuthentication after provider token + exchange). If callers passed untrusted or tampered tokens, group claims + could be forged and a user could be assigned the Admin role. Therefore: + do not call this module with user-supplied tokens; it is intended only + for the internal flow from the OIDC callback. + """ + alias Mv.Accounts.User + alias Mv.Authorization.Role + alias Mv.OidcRoleSyncConfig + + @doc """ + Applies Admin or Mitglied role to the user based on OIDC groups claim. + + - If OIDC_ADMIN_GROUP_NAME is not configured: no-op, returns :ok without changing the user. + - If groups (from user_info or access_token) contain the configured admin group: assigns Admin role. + - Otherwise: assigns Mitglied role (downgrade if user was Admin). + + user_info is a map (e.g. from ID token claims); oauth_tokens is optional and may + contain "access_token" (JWT) from which the groups claim is read when not in user_info. + """ + @spec apply_admin_role_from_user_info(User.t(), map(), map() | nil) :: :ok + def apply_admin_role_from_user_info(user, user_info, oauth_tokens \\ nil) + when is_map(user_info) do + admin_group = OidcRoleSyncConfig.oidc_admin_group_name() + + if is_nil(admin_group) or admin_group == "" do + :ok + else + claim = OidcRoleSyncConfig.oidc_groups_claim() + groups = groups_from_user_info(user_info, claim) + + groups = + if Enum.empty?(groups), do: groups_from_access_token(oauth_tokens, claim), else: groups + + target_role = if admin_group in groups, do: :admin, else: :mitglied + set_user_role(user, target_role) + end + end + + defp groups_from_user_info(user_info, claim) do + value = user_info[claim] || user_info[String.to_existing_atom(claim)] + normalize_groups(value) + rescue + ArgumentError -> normalize_groups(user_info[claim]) + end + + defp groups_from_access_token(nil, _claim), do: [] + defp groups_from_access_token(oauth_tokens, _claim) when not is_map(oauth_tokens), do: [] + + defp groups_from_access_token(oauth_tokens, claim) do + access_token = oauth_tokens["access_token"] || oauth_tokens[:access_token] + + if is_binary(access_token) do + case peek_jwt_claims(access_token) do + {:ok, claims} -> + value = claims[claim] || safe_get_atom(claims, claim) + normalize_groups(value) + + _ -> + [] + end + else + [] + end + end + + defp safe_get_atom(map, key) when is_binary(key) do + try do + Map.get(map, String.to_existing_atom(key)) + rescue + ArgumentError -> nil + end + end + + defp safe_get_atom(_map, _key), do: nil + + defp peek_jwt_claims(token) do + parts = String.split(token, ".") + + if length(parts) == 3 do + [_h, payload_b64, _sig] = parts + + case Base.url_decode64(payload_b64, padding: false) do + {:ok, payload} -> Jason.decode(payload) + _ -> :error + end + else + :error + end + end + + defp normalize_groups(nil), do: [] + defp normalize_groups(list) when is_list(list), do: Enum.map(list, &to_string/1) + defp normalize_groups(single) when is_binary(single), do: [single] + defp normalize_groups(_), do: [] + + defp set_user_role(user, :admin) do + case Role.get_admin_role() do + {:ok, %Role{} = role} -> + do_set_role(user, role) + + _ -> + :ok + end + end + + defp set_user_role(user, :mitglied) do + case Role.get_mitglied_role() do + {:ok, %Role{} = role} -> + do_set_role(user, role) + + _ -> + :ok + end + end + + defp do_set_role(user, role) do + if user.role_id == role.id do + :ok + else + user + |> Ash.Changeset.for_update(:set_role_from_oidc_sync, %{role_id: role.id}) + |> Ash.Changeset.set_context(%{private: %{oidc_role_sync: true}}) + |> Ash.update(domain: Mv.Accounts, context: %{private: %{oidc_role_sync: true}}) + |> case do + {:ok, _} -> :ok + {:error, _} -> :ok + end + end + end +end diff --git a/lib/mv/oidc_role_sync_config.ex b/lib/mv/oidc_role_sync_config.ex new file mode 100644 index 0000000..493a435 --- /dev/null +++ b/lib/mv/oidc_role_sync_config.ex @@ -0,0 +1,24 @@ +defmodule Mv.OidcRoleSyncConfig do + @moduledoc """ + Runtime configuration for OIDC group → role sync (e.g. admin group → Admin role). + + Reads from Application config `:mv, :oidc_role_sync`: + - `:admin_group_name` – OIDC group name that maps to Admin role (optional; when nil, no sync). + - `:groups_claim` – JWT/user_info claim name for groups (default: `"groups"`). + + Set via ENV in production: OIDC_ADMIN_GROUP_NAME, OIDC_GROUPS_CLAIM (see config/runtime.exs). + """ + @doc "Returns the OIDC group name that maps to Admin role, or nil if not configured." + def oidc_admin_group_name do + get(:admin_group_name) + end + + @doc "Returns the JWT/user_info claim name for groups; defaults to \"groups\"." + def oidc_groups_claim do + get(:groups_claim) || "groups" + end + + defp get(key) do + Application.get_env(:mv, :oidc_role_sync, []) |> Keyword.get(key) + end +end diff --git a/lib/mv/release.ex b/lib/mv/release.ex index c0c2c8a..54bc245 100644 --- a/lib/mv/release.ex +++ b/lib/mv/release.ex @@ -2,9 +2,22 @@ defmodule Mv.Release do @moduledoc """ Used for executing DB release tasks when run in production without Mix installed. + + ## Tasks + + - `migrate/0` - Runs all pending Ecto migrations. + - `seed_admin/0` - Ensures an admin user exists from ENV (ADMIN_EMAIL, ADMIN_PASSWORD + or ADMIN_PASSWORD_FILE). Idempotent; can be run on every deployment or via shell + to update the admin password without redeploying. """ @app :mv + alias Mv.Accounts + alias Mv.Accounts.User + alias Mv.Authorization.Role + + require Ash.Query + def migrate do load_app() @@ -18,6 +31,158 @@ defmodule Mv.Release do {:ok, _, _} = Ecto.Migrator.with_repo(repo, &Ecto.Migrator.run(&1, :down, to: version)) end + @doc """ + Ensures an admin user exists from ENV (ADMIN_EMAIL, ADMIN_PASSWORD or ADMIN_PASSWORD_FILE). + + Starts the application if not already running (required when called via `bin/mv eval`; + Ash/Telemetry need the running app). Idempotent. + + - If ADMIN_EMAIL is unset: no-op (idempotent). + - If ADMIN_PASSWORD (and ADMIN_PASSWORD_FILE) are unset and the user does not exist: + no user is created (no fallback password in production). + - If both ADMIN_EMAIL and ADMIN_PASSWORD are set: creates or updates the user with + Admin role and the given password. Safe to run on every deployment or via + `bin/mv eval "Mv.Release.seed_admin()"` to change the admin password without redeploying. + """ + def seed_admin do + # Ensure app (and Telemetry/Ash deps) are started when run via bin/mv eval + case Application.ensure_all_started(@app) do + {:ok, _} -> :ok + {:error, {app, reason}} -> raise "Failed to start #{inspect(app)}: #{inspect(reason)}" + end + + admin_email = get_env("ADMIN_EMAIL", nil) + admin_password = get_env_or_file("ADMIN_PASSWORD", nil) + + cond do + is_nil(admin_email) or admin_email == "" -> + :ok + + is_nil(admin_password) or admin_password == "" -> + ensure_admin_role_only(admin_email) + + true -> + ensure_admin_user(admin_email, admin_password) + end + end + + defp ensure_admin_role_only(email) do + case Role.get_admin_role() do + {:ok, nil} -> + :ok + + {:ok, %Role{} = admin_role} -> + case get_user_by_email(email) do + {:ok, %User{} = user} -> + user + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) + |> Ash.update!(authorize?: false) + + :ok + + _ -> + :ok + end + + {:error, _} -> + :ok + end + end + + defp ensure_admin_user(email, password) do + if is_nil(password) or password == "" do + :ok + else + do_ensure_admin_user(email, password) + end + end + + defp do_ensure_admin_user(email, password) do + case Role.get_admin_role() do + {:ok, nil} -> + # Admin role does not exist (e.g. migrations not run); skip + :ok + + {:ok, %Role{} = admin_role} -> + case get_user_by_email(email) do + {:ok, nil} -> + create_admin_user(email, password, admin_role) + + {:ok, user} -> + update_admin_user(user, password, admin_role) + + {:error, _} -> + :ok + end + + {:error, _} -> + :ok + end + end + + defp create_admin_user(email, password, admin_role) do + case Accounts.create_user(%{email: email}, authorize?: false) do + {:ok, user} -> + user + |> Ash.Changeset.for_update(:admin_set_password, %{password: password}) + |> Ash.update!(authorize?: false) + |> then(fn u -> + u + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) + |> Ash.update!(authorize?: false) + end) + + :ok + + {:error, _} -> + :ok + end + end + + defp update_admin_user(user, password, admin_role) do + user + |> Ash.Changeset.for_update(:admin_set_password, %{password: password}) + |> Ash.update!(authorize?: false) + |> then(fn u -> + u + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) + |> Ash.update!(authorize?: false) + end) + + :ok + end + + defp get_user_by_email(email) do + User + |> Ash.Query.filter(email == ^email) + |> Ash.read_one(authorize?: false, domain: Mv.Accounts) + end + + defp get_env(key, default) do + System.get_env(key, default) + end + + defp get_env_or_file(var_name, default) do + file_var = "#{var_name}_FILE" + + case System.get_env(file_var) do + nil -> + System.get_env(var_name, default) + + file_path -> + case File.read(file_path) do + {:ok, content} -> + String.trim_trailing(content) + + {:error, _} -> + default + end + end + end + defp repos do Application.fetch_env!(@app, :ecto_repos) end diff --git a/lib/mv_web/authorization.ex b/lib/mv_web/authorization.ex index d20be7d..d821416 100644 --- a/lib/mv_web/authorization.ex +++ b/lib/mv_web/authorization.ex @@ -97,12 +97,18 @@ defmodule MvWeb.Authorization do @doc """ Checks if user can access a specific page. + Nil-safe: returns false when user is nil (e.g. unauthenticated or layout + assigns regression), so callers do not need to guard. + ## Examples iex> admin = %{role: %{permission_set_name: "admin"}} iex> can_access_page?(admin, "/admin/roles") true + iex> can_access_page?(nil, "/members") + false + iex> mitglied = %{role: %{permission_set_name: "own_data"}} iex> can_access_page?(mitglied, "/members") false diff --git a/lib/mv_web/components/core_components.ex b/lib/mv_web/components/core_components.ex index 45bcae0..9ef8f2b 100644 --- a/lib/mv_web/components/core_components.ex +++ b/lib/mv_web/components/core_components.ex @@ -97,12 +97,13 @@ defmodule MvWeb.CoreComponents do <.button navigate={~p"/"}>Home <.button disabled={true}>Disabled """ - attr :rest, :global, include: ~w(href navigate patch method) + attr :rest, :global, include: ~w(href navigate patch method data-testid) attr :variant, :string, values: ~w(primary) attr :disabled, :boolean, default: false, doc: "Whether the button is disabled" slot :inner_block, required: true - def button(%{rest: rest} = assigns) do + def button(assigns) do + rest = assigns.rest variants = %{"primary" => "btn-primary", nil => "btn-primary btn-soft"} assigns = assign(assigns, :class, Map.fetch!(variants, assigns[:variant])) @@ -544,6 +545,9 @@ defmodule MvWeb.CoreComponents do attr :label, :string attr :class, :string attr :col_click, :any, doc: "optional column-specific click handler that overrides row_click" + + attr :sort_field, :any, + doc: "optional; when equal to table sort_field, aria-sort is set on this th" end slot :action, doc: "the slot for showing user actions in the last table column" @@ -559,7 +563,13 @@ defmodule MvWeb.CoreComponents do - + - <%= if can?(@current_user, :update, Mv.Membership.Group) do %> - - <% end %> @@ -281,20 +156,6 @@ defmodule MvWeb.GroupLive.Show do — <% end %> - <%= if can?(@current_user, :update, Mv.Membership.Group) do %> - - <% end %> <% end %> @@ -375,13 +236,11 @@ defmodule MvWeb.GroupLive.Show do """ end - # Delete Modal Events @impl true def handle_event("open_delete_modal", _params, socket) do {:noreply, assign(socket, show_delete_modal: true, name_confirmation: "")} end - @impl true def handle_event("cancel_delete", _params, socket) do {:noreply, socket @@ -389,12 +248,10 @@ defmodule MvWeb.GroupLive.Show do |> assign(:name_confirmation, "")} end - @impl true def handle_event("update_name_confirmation", %{"name" => name}, socket) do {:noreply, assign(socket, :name_confirmation, name)} end - @impl true def handle_event("confirm_delete", %{"slug" => slug}, socket) do actor = current_actor(socket) group = socket.assigns.group @@ -418,416 +275,6 @@ defmodule MvWeb.GroupLive.Show do end end - # Add Member Events - @impl true - def handle_event("show_add_member_input", _params, socket) do - # Reload group to ensure we have the latest members list - actor = current_actor(socket) - group = socket.assigns.group - socket = reload_group(socket, group.slug, actor) - - {:noreply, - socket - |> assign(:show_add_member_input, true) - |> assign(:member_search_query, "") - |> assign(:available_members, []) - |> assign(:selected_member_ids, []) - |> assign(:selected_members, []) - |> assign(:show_member_dropdown, false) - |> assign(:focused_member_index, nil)} - end - - @impl true - def handle_event("show_member_dropdown", _params, socket) do - # Reload group to ensure we have the latest members list before filtering - actor = current_actor(socket) - group = socket.assigns.group - socket = reload_group(socket, group.slug, actor) - - # Load available members with empty query when input is focused - socket = - socket - |> load_available_members("") - |> assign(:show_member_dropdown, true) - |> assign(:focused_member_index, nil) - - {:noreply, socket} - end - - @impl true - def handle_event("hide_member_dropdown", _params, socket) do - {:noreply, assign(socket, show_member_dropdown: false, focused_member_index: nil)} - end - - @impl true - def handle_event("member_dropdown_keydown", %{"key" => "ArrowDown"}, socket) do - return_if_dropdown_closed(socket, fn -> - max_index = length(socket.assigns.available_members) - 1 - current = socket.assigns.focused_member_index - - new_index = - case current do - nil -> 0 - index when index < max_index -> index + 1 - _ -> current - end - - {:noreply, assign(socket, focused_member_index: new_index)} - end) - end - - @impl true - def handle_event("member_dropdown_keydown", %{"key" => "ArrowUp"}, socket) do - return_if_dropdown_closed(socket, fn -> - current = socket.assigns.focused_member_index - - new_index = - case current do - nil -> 0 - 0 -> 0 - index -> index - 1 - end - - {:noreply, assign(socket, focused_member_index: new_index)} - end) - end - - @impl true - def handle_event("member_dropdown_keydown", %{"key" => "Enter"}, socket) do - return_if_dropdown_closed(socket, fn -> - select_focused_member(socket) - end) - end - - @impl true - def handle_event("member_dropdown_keydown", %{"key" => "Escape"}, socket) do - return_if_dropdown_closed(socket, fn -> - {:noreply, assign(socket, show_member_dropdown: false, focused_member_index: nil)} - end) - end - - @impl true - def handle_event("member_dropdown_keydown", _params, socket) do - # Ignore other keys - {:noreply, socket} - end - - @impl true - def handle_event("search_members", %{"member_search" => query}, socket) do - # Reload group to ensure we have the latest members list before filtering - actor = current_actor(socket) - group = socket.assigns.group - socket = reload_group(socket, group.slug, actor) - - socket = - socket - |> assign(:member_search_query, query) - |> load_available_members(query) - |> assign(:show_member_dropdown, true) - |> assign(:focused_member_index, nil) - - {:noreply, socket} - end - - @impl true - def handle_event("select_member", %{"id" => member_id}, socket) do - # Check if member is already selected - if member_id in socket.assigns.selected_member_ids do - {:noreply, socket} - else - # Find the selected member - selected_member = Enum.find(socket.assigns.available_members, &(&1.id == member_id)) - - if selected_member do - socket = - socket - |> assign(:selected_member_ids, [member_id | socket.assigns.selected_member_ids]) - |> assign(:selected_members, [selected_member | socket.assigns.selected_members]) - |> assign(:member_search_query, "") - |> assign(:show_member_dropdown, false) - |> assign(:focused_member_index, nil) - - {:noreply, socket} - else - {:noreply, socket} - end - end - end - - @impl true - def handle_event("remove_selected_member", %{"member_id" => member_id}, socket) do - socket = - socket - |> assign(:selected_member_ids, List.delete(socket.assigns.selected_member_ids, member_id)) - |> assign( - :selected_members, - Enum.reject(socket.assigns.selected_members, &(&1.id == member_id)) - ) - - {:noreply, socket} - end - - @impl true - def handle_event("add_selected_members", _params, socket) do - actor = current_actor(socket) - group = socket.assigns.group - - # Server-side authorization check - if can?(actor, :update, group) do - perform_add_members(socket, group, socket.assigns.selected_member_ids, actor) - else - {:noreply, - socket - |> put_flash(:error, gettext("Not authorized.")) - |> redirect(to: ~p"/groups/#{group.slug}")} - end - end - - @impl true - def handle_event("remove_member", %{"member_id" => member_id}, socket) do - actor = current_actor(socket) - group = socket.assigns.group - - # Server-side authorization check - if can?(actor, :update, group) do - perform_remove_member(socket, group, member_id, actor) - else - {:noreply, - socket - |> put_flash(:error, gettext("Not authorized.")) - |> redirect(to: ~p"/groups/#{group.slug}")} - end - end - - # Helper functions - defp return_if_dropdown_closed(socket, fun) do - if socket.assigns.show_member_dropdown do - fun.() - else - {:noreply, socket} - end - end - - defp select_focused_member(socket) do - case socket.assigns.focused_member_index do - nil -> - {:noreply, socket} - - index -> - select_member_by_index(socket, index) - end - end - - defp select_member_by_index(socket, index) do - case Enum.at(socket.assigns.available_members, index) do - nil -> - {:noreply, socket} - - member -> - add_member_to_selection(socket, member) - end - end - - defp add_member_to_selection(socket, member) do - # Check if member is already selected - if member.id in socket.assigns.selected_member_ids do - {:noreply, socket} - else - socket = - socket - |> assign(:selected_member_ids, [member.id | socket.assigns.selected_member_ids]) - |> assign(:selected_members, [member | socket.assigns.selected_members]) - |> assign(:member_search_query, "") - |> assign(:show_member_dropdown, false) - |> assign(:focused_member_index, nil) - - {:noreply, socket} - end - end - - defp load_available_members(socket, query) do - require Ash.Query - - base_query = available_members_base_query(query) - limited_query = Ash.Query.limit(base_query, 10) - actor = current_actor(socket) - - case Ash.read(limited_query, actor: actor, domain: Mv.Membership) do - {:ok, members} -> - current_member_ids = group_member_ids_set(socket.assigns.group) - - filtered_members = - Enum.reject(members, fn member -> - MapSet.member?(current_member_ids, member.id) - end) - - assign(socket, available_members: filtered_members) - - {:error, _} -> - assign(socket, available_members: []) - end - end - - defp available_members_base_query(query) do - search_query = if query && String.trim(query) != "", do: String.trim(query), else: nil - - if search_query do - Mv.Membership.Member - |> Ash.Query.for_read(:search, %{query: search_query}) - else - Mv.Membership.Member - |> Ash.Query.new() - end - end - - defp group_member_ids_set(group) do - cond do - is_list(group.members) and group.members != [] -> - group.members - |> Enum.map(& &1.id) - |> MapSet.new() - - is_list(group.members) -> - MapSet.new() - - true -> - MapSet.new() - end - end - - defp perform_add_members(socket, group, member_ids, actor) when is_list(member_ids) do - # Add all members in a transaction-like manner - results = - Enum.map(member_ids, fn member_id -> - Membership.create_member_group( - %{member_id: member_id, group_id: group.id}, - actor: actor - ) - end) - - # Check for errors - errors = Enum.filter(results, &match?({:error, _}, &1)) - - if Enum.empty?(errors) do - handle_successful_add_members(socket, group, actor) - else - handle_failed_add_members(socket, group, errors, actor) - end - end - - defp perform_add_members(socket, _group, _member_ids, _actor) do - {:noreply, - socket - |> put_flash(:error, gettext("No members selected."))} - end - - defp handle_successful_add_members(socket, group, actor) do - socket = reload_group(socket, group.slug, actor) - - {:noreply, - socket - |> assign(:show_add_member_input, false) - |> assign(:member_search_query, "") - |> assign(:available_members, []) - |> assign(:selected_member_ids, []) - |> assign(:selected_members, []) - |> assign(:show_member_dropdown, false) - |> assign(:focused_member_index, nil)} - end - - defp handle_failed_add_members(socket, group, errors, actor) do - error_messages = extract_error_messages(errors) - - # Still reload to show any successful additions - socket = reload_group(socket, group.slug, actor) - - {:noreply, - socket - |> put_flash( - :error, - gettext("Some members could not be added: %{errors}", errors: error_messages) - ) - |> assign(:show_add_member_input, true)} - end - - defp extract_error_messages(errors) do - Enum.map(errors, fn {:error, error} -> - format_single_error(error) - end) - |> Enum.uniq() - |> Enum.join(", ") - end - - defp format_single_error(%{errors: [%{message: message}]}) when is_binary(message), do: message - - defp format_single_error(%{errors: [%{field: :member_id, message: message}]}) - when is_binary(message), - do: message - - defp format_single_error(error), do: format_error(error) - - defp perform_remove_member(socket, group, member_id, actor) do - require Ash.Query - - # Find the MemberGroup association - query = - Mv.Membership.MemberGroup - |> Ash.Query.filter(member_id == ^member_id and group_id == ^group.id) - - case Ash.read_one(query, actor: actor, domain: Mv.Membership) do - {:ok, nil} -> - {:noreply, - socket - |> put_flash(:error, gettext("Member is not in this group."))} - - {:ok, member_group} -> - case Membership.destroy_member_group(member_group, actor: actor) do - :ok -> - # Reload group with members and member_count - socket = reload_group(socket, group.slug, actor) - - {:noreply, socket} - - {:error, error} -> - error_message = format_error(error) - - {:noreply, - socket - |> put_flash( - :error, - gettext("Failed to remove member: %{error}", error: error_message) - )} - end - - {:error, error} -> - error_message = format_error(error) - - {:noreply, - socket - |> put_flash( - :error, - gettext("Failed to remove member: %{error}", error: error_message) - )} - end - end - - defp reload_group(socket, slug, actor) do - require Ash.Query - - query = - Mv.Membership.Group - |> Ash.Query.filter(slug == ^slug) - |> Ash.Query.load([:members, :member_count]) - - case Ash.read_one(query, actor: actor, domain: Mv.Membership) do - {:ok, group} -> - assign(socket, :group, group) - - {:error, _} -> - socket - end - end - defp handle_delete_confirmation(socket, group, actor) do if socket.assigns.name_confirmation == group.name do perform_group_deletion(socket, group, actor) diff --git a/lib/mv_web/live/import_export_live.ex b/lib/mv_web/live/import_export_live.ex new file mode 100644 index 0000000..384c39b --- /dev/null +++ b/lib/mv_web/live/import_export_live.ex @@ -0,0 +1,828 @@ +defmodule MvWeb.ImportExportLive do + @moduledoc """ + LiveView for importing and exporting members via CSV. + + ## Features + - CSV member import (admin only) + - Real-time import progress tracking + - Error and warning reporting + - Custom fields support + + ## CSV Import + + The CSV import feature allows administrators to upload CSV files and import members. + + ### File Upload + + Files are uploaded automatically when selected (`auto_upload: true`). No manual + upload trigger is required. + + ### Rate Limiting + + Currently, there is no rate limiting for CSV imports. Administrators can start + multiple imports in quick succession. This is intentional for bulk data migration + scenarios, but should be monitored in production. + + ### Limits + + - Maximum file size: configurable via `config :mv, csv_import: [max_file_size_mb: ...]` + - Maximum rows: configurable via `config :mv, csv_import: [max_rows: ...]` (excluding header) + - Processing: chunks of 200 rows + - Errors: capped at 50 per import + """ + use MvWeb, :live_view + + alias Mv.Authorization.Actor + alias Mv.Config + alias Mv.Membership + alias Mv.Membership.Import.MemberCSV + alias MvWeb.Authorization + + on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded} + + # Maximum number of errors to collect per import to prevent memory issues + # and keep error display manageable. Additional errors are silently dropped + # after this limit is reached. + @max_errors 50 + + # Maximum length for error messages before truncation + @max_error_message_length 200 + + @impl true + def mount(_params, session, socket) do + # Get locale from session for translations + locale = session["locale"] || "de" + Gettext.put_locale(MvWeb.Gettext, locale) + + # Get club name from settings + club_name = + case Membership.get_settings() do + {:ok, settings} -> settings.club_name + _ -> "Mitgliederverwaltung" + end + + socket = + socket + |> assign(:page_title, gettext("Import/Export")) + |> assign(:club_name, club_name) + |> assign(:import_state, nil) + |> assign(:import_progress, nil) + |> assign(:import_status, :idle) + |> assign(:locale, locale) + |> assign(:max_errors, @max_errors) + |> assign(:csv_import_max_rows, Config.csv_import_max_rows()) + |> assign(:csv_import_max_file_size_mb, Config.csv_import_max_file_size_mb()) + # Configure file upload with auto-upload enabled + # Files are uploaded automatically when selected, no need for manual trigger + |> allow_upload(:csv_file, + accept: ~w(.csv), + max_entries: 1, + max_file_size: Config.csv_import_max_file_size_bytes(), + auto_upload: true + ) + + {:ok, socket} + end + + @impl true + def render(assigns) do + ~H""" + + <.header> + {gettext("Import/Export")} + <:subtitle> + {gettext("Import members from CSV files or export member data.")} + + + + <%= if Authorization.can?(@current_user, :create, Mv.Membership.Member) do %> + <%!-- CSV Import Section --%> + <.form_section title={gettext("Import Members (CSV)")}> + {import_info_box(assigns)} + {template_links(assigns)} + {import_form(assigns)} + <%= if @import_status == :running or @import_status == :done do %> + {import_progress(assigns)} + <% end %> + + + <%!-- Export Section (Placeholder) --%> + <.form_section title={gettext("Export Members (CSV)")}> +
+ <.icon name="hero-information-circle" class="size-5" aria-hidden="true" /> +
+

+ {gettext("Export functionality will be available in a future release.")} +

+
+
+ + <% else %> + + <% end %> + + """ + end + + # Renders the info box explaining CSV import requirements + defp import_info_box(assigns) do + ~H""" +
+ <.icon name="hero-information-circle" class="size-5" aria-hidden="true" /> +
+

+ {gettext( + "Use the data field name as the CSV column header in your file. Data fields must exist in Mila before importing, so they must be listed in the list of member data (like e-mail or first name). Unknown data field columns will be ignored with a warning." + )} +

+

+ <.link + href={~p"/settings#custom_fields"} + class="link" + data-testid="custom-fields-link" + > + {gettext("Manage Member Data")} + +

+
+
+ """ + end + + # Renders template download links + defp template_links(assigns) do + ~H""" +
+

+ {gettext("Download CSV templates:")} +

+ +
+ """ + end + + # Renders the CSV upload form + defp import_form(assigns) do + ~H""" + <.form + id="csv-upload-form" + for={%{}} + multipart={true} + phx-change="validate_csv_upload" + phx-submit="start_import" + data-testid="csv-upload-form" + > +
+ + <.live_file_input + upload={@uploads.csv_file} + id="csv_file" + class="file-input file-input-bordered w-full" + aria-describedby="csv_file_help" + /> +

+ {gettext("CSV files only, maximum %{size} MB", size: @csv_import_max_file_size_mb)} +

+
+ + <.button + type="submit" + phx-disable-with={gettext("Starting import...")} + variant="primary" + disabled={import_button_disabled?(@import_status, @uploads.csv_file.entries)} + data-testid="start-import-button" + > + {gettext("Start Import")} + + + """ + end + + # Renders import progress and results + defp import_progress(assigns) do + ~H""" + <%= if @import_progress do %> +
+ <%= if @import_progress.status == :running do %> +

+ {gettext("Processing chunk %{current} of %{total}...", + current: @import_progress.current_chunk, + total: @import_progress.total_chunks + )} +

+ <% end %> + + <%= if @import_progress.status == :done do %> + {import_results(assigns)} + <% end %> +
+ <% end %> + """ + end + + # Renders import results summary, errors, and warnings + defp import_results(assigns) do + ~H""" +
+

+ {gettext("Import Results")} +

+ +
+
+

+ {gettext("Summary")} +

+
+

+ <.icon + name="hero-check-circle" + class="size-4 inline mr-1" + aria-hidden="true" + /> + {gettext("Successfully inserted: %{count} member(s)", + count: @import_progress.inserted + )} +

+ <%= if @import_progress.failed > 0 do %> +

+ <.icon + name="hero-exclamation-circle" + class="size-4 inline mr-1" + aria-hidden="true" + /> + {gettext("Failed: %{count} row(s)", count: @import_progress.failed)} +

+ <% end %> + <%= if @import_progress.errors_truncated? do %> +

+ <.icon + name="hero-information-circle" + class="size-4 inline mr-1" + aria-hidden="true" + /> + {gettext("Error list truncated to %{count} entries", count: @max_errors)} +

+ <% end %> +
+
+ + <%= if length(@import_progress.errors) > 0 do %> +
+

+ <.icon + name="hero-exclamation-circle" + class="size-4 inline mr-1" + aria-hidden="true" + /> + {gettext("Errors")} +

+
    + <%= for error <- @import_progress.errors do %> +
  • + {gettext("Line %{line}: %{message}", + line: error.csv_line_number || "?", + message: error.message || gettext("Unknown error") + )} + <%= if error.field do %> + {gettext(" (Field: %{field})", field: error.field)} + <% end %> +
    • + <% end %> +
+
+ <% end %> + + <%= if length(@import_progress.warnings) > 0 do %> + + <% end %> +
+
+ """ + end + + @impl true + def handle_event("validate_csv_upload", _params, socket) do + {:noreply, socket} + end + + @impl true + def handle_event("start_import", _params, socket) do + case check_import_prerequisites(socket) do + {:error, message} -> + {:noreply, put_flash(socket, :error, message)} + + :ok -> + process_csv_upload(socket) + end + end + + # Checks if all prerequisites for starting an import are met. + # + # Validates: + # - User has admin permissions + # - No import is currently running + # - CSV file is uploaded and ready + # + # Returns `:ok` if all checks pass, `{:error, message}` otherwise. + # + # Note: on_mount :ensure_user_role_loaded already guarantees the role is loaded, + # so ensure_actor_loaded is primarily for clarity. + @spec check_import_prerequisites(Phoenix.LiveView.Socket.t()) :: + :ok | {:error, String.t()} + defp check_import_prerequisites(socket) do + # on_mount already ensures role is loaded, but we keep this for clarity + user_with_role = ensure_actor_loaded(socket) + + cond do + not Authorization.can?(user_with_role, :create, Mv.Membership.Member) -> + {:error, gettext("Only administrators can import members from CSV files.")} + + socket.assigns.import_status == :running -> + {:error, gettext("Import is already running. Please wait for it to complete.")} + + Enum.empty?(socket.assigns.uploads.csv_file.entries) -> + {:error, gettext("Please select a CSV file to import.")} + + not List.first(socket.assigns.uploads.csv_file.entries).done? -> + {:error, + gettext("Please wait for the file upload to complete before starting the import.")} + + true -> + :ok + end + end + + # Processes CSV upload and starts import process. + # + # Reads the uploaded CSV file, prepares it for import, and initiates + # the chunked processing workflow. + @spec process_csv_upload(Phoenix.LiveView.Socket.t()) :: + {:noreply, Phoenix.LiveView.Socket.t()} + defp process_csv_upload(socket) do + actor = MvWeb.LiveHelpers.current_actor(socket) + + with {:ok, content} <- consume_and_read_csv(socket), + {:ok, import_state} <- + MemberCSV.prepare(content, max_rows: Config.csv_import_max_rows(), actor: actor) do + start_import(socket, import_state) + else + {:error, reason} when is_binary(reason) -> + {:noreply, + put_flash( + socket, + :error, + gettext("Failed to prepare CSV import: %{reason}", reason: reason) + )} + + {:error, error} -> + error_message = format_error_message(error) + + {:noreply, + put_flash( + socket, + :error, + gettext("Failed to prepare CSV import: %{reason}", reason: error_message) + )} + end + end + + # Starts the import process by initializing progress tracking and scheduling the first chunk. + @spec start_import(Phoenix.LiveView.Socket.t(), map()) :: + {:noreply, Phoenix.LiveView.Socket.t()} + defp start_import(socket, import_state) do + progress = initialize_import_progress(import_state) + + socket = + socket + |> assign(:import_state, import_state) + |> assign(:import_progress, progress) + |> assign(:import_status, :running) + + send(self(), {:process_chunk, 0}) + + {:noreply, socket} + end + + # Initializes the import progress tracking structure with default values. + @spec initialize_import_progress(map()) :: map() + defp initialize_import_progress(import_state) do + %{ + inserted: 0, + failed: 0, + errors: [], + warnings: import_state.warnings || [], + status: :running, + current_chunk: 0, + total_chunks: length(import_state.chunks), + errors_truncated?: false + } + end + + # Formats error messages for user-friendly display. + # + # Handles various error types including Ash errors, maps with message fields, + # lists of errors, and fallback formatting for unknown types. + @spec format_error_message(any()) :: String.t() + defp format_error_message(error) do + case error do + %Ash.Error.Invalid{} = ash_error -> + format_ash_error(ash_error) + + %{message: msg} when is_binary(msg) -> + msg + + %{errors: errors} when is_list(errors) -> + format_error_list(errors) + + reason when is_binary(reason) -> + reason + + other -> + format_unknown_error(other) + end + end + + # Formats Ash validation errors for display + defp format_ash_error(%Ash.Error.Invalid{errors: errors}) when is_list(errors) do + Enum.map_join(errors, ", ", &format_single_error/1) + end + + defp format_ash_error(error) do + format_unknown_error(error) + end + + # Formats a list of errors into a readable string + defp format_error_list(errors) do + Enum.map_join(errors, ", ", &format_single_error/1) + end + + # Formats a single error item + defp format_single_error(error) when is_map(error) do + Map.get(error, :message) || Map.get(error, :field) || inspect(error, limit: :infinity) + end + + defp format_single_error(error) do + to_string(error) + end + + # Formats unknown error types with truncation for very long messages + defp format_unknown_error(other) do + error_str = inspect(other, limit: :infinity, pretty: true) + + if String.length(error_str) > @max_error_message_length do + String.slice(error_str, 0, @max_error_message_length - 3) <> "..." + else + error_str + end + end + + @impl true + def handle_info({:process_chunk, idx}, socket) do + case socket.assigns do + %{import_state: import_state, import_progress: progress} + when is_map(import_state) and is_map(progress) -> + if idx < length(import_state.chunks) do + start_chunk_processing_task(socket, import_state, progress, idx) + else + handle_chunk_error(socket, :invalid_index, idx) + end + + _ -> + # Missing required assigns - mark as error + handle_chunk_error(socket, :missing_state, idx) + end + end + + @impl true + def handle_info({:chunk_done, idx, result}, socket) do + case socket.assigns do + %{import_state: import_state, import_progress: progress} + when is_map(import_state) and is_map(progress) -> + handle_chunk_result(socket, import_state, progress, idx, result) + + _ -> + # Missing required assigns - mark as error + handle_chunk_error(socket, :missing_state, idx) + end + end + + @impl true + def handle_info({:chunk_error, idx, reason}, socket) do + handle_chunk_error(socket, :processing_failed, idx, reason) + end + + # Processes a chunk with error handling and sends result message to LiveView. + # + # Handles errors from MemberCSV.process_chunk and sends appropriate messages + # to the LiveView process for progress tracking. + @spec process_chunk_with_error_handling( + list(), + map(), + map(), + keyword(), + pid(), + non_neg_integer() + ) :: :ok + defp process_chunk_with_error_handling( + chunk, + column_map, + custom_field_map, + opts, + live_view_pid, + idx + ) do + result = + try do + MemberCSV.process_chunk(chunk, column_map, custom_field_map, opts) + rescue + e -> + {:error, Exception.message(e)} + catch + :exit, reason -> + {:error, inspect(reason)} + + :throw, reason -> + {:error, inspect(reason)} + end + + case result do + {:ok, chunk_result} -> + send(live_view_pid, {:chunk_done, idx, chunk_result}) + + {:error, reason} -> + send(live_view_pid, {:chunk_error, idx, reason}) + end + end + + # Starts async task to process a chunk of CSV rows. + # + # In tests (SQL sandbox mode), runs synchronously to avoid Ecto Sandbox issues. + @spec start_chunk_processing_task( + Phoenix.LiveView.Socket.t(), + map(), + map(), + non_neg_integer() + ) :: {:noreply, Phoenix.LiveView.Socket.t()} + defp start_chunk_processing_task(socket, import_state, progress, idx) do + chunk = Enum.at(import_state.chunks, idx) + actor = ensure_actor_loaded(socket) + live_view_pid = self() + + # Process chunk with existing error count for capping + opts = [ + custom_field_lookup: import_state.custom_field_lookup, + existing_error_count: length(progress.errors), + max_errors: @max_errors, + actor: actor + ] + + # Get locale from socket for translations in background tasks + locale = socket.assigns[:locale] || "de" + Gettext.put_locale(MvWeb.Gettext, locale) + + if Config.sql_sandbox?() do + # Run synchronously in tests to avoid Ecto Sandbox issues with async tasks + # In test mode, send the message - it will be processed when render() is called + # in the test. The test helper wait_for_import_completion() handles message processing + process_chunk_with_error_handling( + chunk, + import_state.column_map, + import_state.custom_field_map, + opts, + live_view_pid, + idx + ) + else + # Start async task to process chunk in production + # Use start_child for fire-and-forget: no monitor, no Task messages + # We only use our own send/2 messages for communication + Task.Supervisor.start_child(Mv.TaskSupervisor, fn -> + # Set locale in task process for translations + Gettext.put_locale(MvWeb.Gettext, locale) + + process_chunk_with_error_handling( + chunk, + import_state.column_map, + import_state.custom_field_map, + opts, + live_view_pid, + idx + ) + end) + end + + {:noreply, socket} + end + + # Handles chunk processing result from async task and schedules the next chunk. + @spec handle_chunk_result( + Phoenix.LiveView.Socket.t(), + map(), + map(), + non_neg_integer(), + map() + ) :: {:noreply, Phoenix.LiveView.Socket.t()} + defp handle_chunk_result(socket, import_state, progress, idx, chunk_result) do + # Merge progress + new_progress = merge_progress(progress, chunk_result, idx) + + socket = + socket + |> assign(:import_progress, new_progress) + |> assign(:import_status, new_progress.status) + + # Schedule next chunk or mark as done + socket = schedule_next_chunk(socket, idx, length(import_state.chunks)) + + {:noreply, socket} + end + + # Handles chunk processing errors and updates socket with error status. + @spec handle_chunk_error( + Phoenix.LiveView.Socket.t(), + :invalid_index | :missing_state | :processing_failed, + non_neg_integer(), + any() + ) :: {:noreply, Phoenix.LiveView.Socket.t()} + defp handle_chunk_error(socket, error_type, idx, reason \\ nil) do + error_message = + case error_type do + :invalid_index -> + gettext("Invalid chunk index: %{idx}", idx: idx) + + :missing_state -> + gettext("Import state is missing. Cannot process chunk %{idx}.", idx: idx) + + :processing_failed -> + gettext("Failed to process chunk %{idx}: %{reason}", + idx: idx, + reason: inspect(reason) + ) + end + + socket = + socket + |> assign(:import_status, :error) + |> put_flash(:error, error_message) + + {:noreply, socket} + end + + # Consumes uploaded CSV file entries and reads the file content. + # + # Returns the file content as a binary string or an error tuple. + @spec consume_and_read_csv(Phoenix.LiveView.Socket.t()) :: + {:ok, String.t()} | {:error, String.t()} + defp consume_and_read_csv(socket) do + raw = consume_uploaded_entries(socket, :csv_file, &read_file_entry/2) + + case raw do + [{:ok, content}] when is_binary(content) -> + {:ok, content} + + # Phoenix LiveView test (render_upload) can return raw content list when callback return is treated as value + [content] when is_binary(content) -> + {:ok, content} + + [{:error, reason}] -> + {:error, gettext("Failed to read file: %{reason}", reason: reason)} + + [] -> + {:error, gettext("No file was uploaded")} + + _other -> + {:error, gettext("Failed to read uploaded file: unexpected format")} + end + end + + # Reads a single file entry from the uploaded path + @spec read_file_entry(map(), map()) :: {:ok, String.t()} | {:error, String.t()} + defp read_file_entry(%{path: path}, _entry) do + case File.read(path) do + {:ok, content} -> + {:ok, content} + + {:error, reason} when is_atom(reason) -> + # POSIX error atoms (e.g., :enoent) need to be formatted + {:error, :file.format_error(reason)} + + {:error, %File.Error{reason: reason}} -> + # File.Error struct with reason atom + {:error, :file.format_error(reason)} + + {:error, reason} -> + # Fallback for other error types + {:error, Exception.message(reason)} + end + end + + # Merges chunk processing results into the overall import progress. + # + # Handles error capping, warning merging, and status updates. + @spec merge_progress(map(), map(), non_neg_integer()) :: map() + defp merge_progress(progress, chunk_result, current_chunk_idx) do + # Merge errors with cap of @max_errors overall + all_errors = progress.errors ++ chunk_result.errors + new_errors = Enum.take(all_errors, @max_errors) + errors_truncated? = length(all_errors) > @max_errors + + # Merge warnings (optional dedupe - simple append for now) + new_warnings = progress.warnings ++ Map.get(chunk_result, :warnings, []) + + # Update status based on whether we're done + # current_chunk_idx is 0-based, so after processing chunk 0, we've processed 1 chunk + chunks_processed = current_chunk_idx + 1 + new_status = if chunks_processed >= progress.total_chunks, do: :done, else: :running + + %{ + inserted: progress.inserted + chunk_result.inserted, + failed: progress.failed + chunk_result.failed, + errors: new_errors, + warnings: new_warnings, + status: new_status, + current_chunk: chunks_processed, + total_chunks: progress.total_chunks, + errors_truncated?: errors_truncated? || chunk_result.errors_truncated? + } + end + + # Schedules the next chunk for processing or marks import as complete. + @spec schedule_next_chunk(Phoenix.LiveView.Socket.t(), non_neg_integer(), non_neg_integer()) :: + Phoenix.LiveView.Socket.t() + defp schedule_next_chunk(socket, current_idx, total_chunks) do + next_idx = current_idx + 1 + + if next_idx < total_chunks do + # Schedule next chunk + send(self(), {:process_chunk, next_idx}) + socket + else + # All chunks processed - status already set to :done in merge_progress + socket + end + end + + # Determines if the import button should be disabled based on import status and upload state + @spec import_button_disabled?(:idle | :running | :done | :error, [map()]) :: boolean() + defp import_button_disabled?(:running, _entries), do: true + defp import_button_disabled?(_status, []), do: true + defp import_button_disabled?(_status, [entry | _]) when not entry.done?, do: true + defp import_button_disabled?(_status, _entries), do: false + + # Ensures the actor (user with role) is loaded from socket assigns. + # + # Note: on_mount :ensure_user_role_loaded already guarantees the role is loaded, + # so this is primarily for clarity and defensive programming. + @spec ensure_actor_loaded(Phoenix.LiveView.Socket.t()) :: Mv.Accounts.User.t() | nil + defp ensure_actor_loaded(socket) do + user = socket.assigns[:current_user] + # on_mount already ensures role is loaded, but we keep this for clarity + Actor.ensure_loaded(user) + end +end diff --git a/lib/mv_web/live/member_live/form.ex b/lib/mv_web/live/member_live/form.ex index b72add6..f9588c0 100644 --- a/lib/mv_web/live/member_live/form.ex +++ b/lib/mv_web/live/member_live/form.ex @@ -177,7 +177,8 @@ defmodule MvWeb.MemberLive.Form do phx-change="validate" value={@form[:membership_fee_type_id].value || ""} > - + <%!-- No "None" option: a membership fee type is required (validated in Member resource). --%> + <%= for fee_type <- @available_fee_types do %>
{col[:label]} + {col[:label]} + <.live_component module={MvWeb.Components.SortHeaderComponent} @@ -645,6 +655,16 @@ defmodule MvWeb.CoreComponents do """ end + defp table_th_aria_sort(col, sort_field, sort_order) do + col_sort = Map.get(col, :sort_field) + + if not is_nil(col_sort) and col_sort == sort_field and sort_order in [:asc, :desc] do + if sort_order == :asc, do: "ascending", else: "descending" + else + nil + end + end + @doc """ Renders a data list. diff --git a/lib/mv_web/components/layouts/sidebar.ex b/lib/mv_web/components/layouts/sidebar.ex index 1d564c1..89519ae 100644 --- a/lib/mv_web/components/layouts/sidebar.ex +++ b/lib/mv_web/components/layouts/sidebar.ex @@ -4,6 +4,8 @@ defmodule MvWeb.Layouts.Sidebar do """ use MvWeb, :html + alias MvWeb.PagePaths + attr :current_user, :map, default: nil, doc: "The current user" attr :club_name, :string, required: true, doc: "The name of the club" attr :mobile, :boolean, default: false, doc: "Whether this is mobile view" @@ -70,33 +72,57 @@ defmodule MvWeb.Layouts.Sidebar do defp sidebar_menu(assigns) do ~H""" """ end + defp admin_menu_visible?(user) do + Enum.any?(PagePaths.admin_menu_paths(), &can_access_page?(user, &1)) + end + attr :href, :string, required: true, doc: "Navigation path" attr :icon, :string, required: true, doc: "Heroicon name" attr :label, :string, required: true, doc: "Menu item label" @@ -119,12 +145,13 @@ defmodule MvWeb.Layouts.Sidebar do attr :icon, :string, required: true, doc: "Heroicon name for the menu group" attr :label, :string, required: true, doc: "Menu group label" + attr :testid, :string, default: nil, doc: "data-testid for stable test selectors" slot :inner_block, required: true, doc: "Submenu items" defp menu_group(assigns) do ~H""" -
  • +
    • -
    +
    """ end - - defp aria_sort(current_field, current_order, this_field) do - cond do - current_field != this_field -> "none" - current_order == :asc -> "ascending" - true -> "descending" - end - end end diff --git a/lib/mv_web/helpers/user_helpers.ex b/lib/mv_web/helpers/user_helpers.ex new file mode 100644 index 0000000..2f9741c --- /dev/null +++ b/lib/mv_web/helpers/user_helpers.ex @@ -0,0 +1,58 @@ +defmodule MvWeb.Helpers.UserHelpers do + @moduledoc """ + Helper functions for user-related display in the web layer. + + Provides utilities for showing authentication status without exposing + sensitive attributes (e.g. hashed_password). + """ + + @doc """ + Returns whether the user has password authentication set. + + Only returns true when `hashed_password` is a non-empty string. This avoids + treating `nil`, empty string, or forbidden/redacted values (e.g. when the + attribute is not visible to the actor) as "has password". + + ## Examples + + iex> user = %{hashed_password: nil} + iex> MvWeb.Helpers.UserHelpers.has_password?(user) + false + + iex> user = %{hashed_password: "$2b$12$..."} + iex> MvWeb.Helpers.UserHelpers.has_password?(user) + true + + iex> user = %{hashed_password: ""} + iex> MvWeb.Helpers.UserHelpers.has_password?(user) + false + """ + @spec has_password?(map() | struct()) :: boolean() + def has_password?(user) when is_map(user) do + case Map.get(user, :hashed_password) do + hash when is_binary(hash) and byte_size(hash) > 0 -> true + _ -> false + end + end + + @doc """ + Returns whether the user is linked via OIDC/SSO (has a non-empty oidc_id). + + ## Examples + + iex> user = %{oidc_id: nil} + iex> MvWeb.Helpers.UserHelpers.has_oidc?(user) + false + + iex> user = %{oidc_id: "sub-from-rauthy"} + iex> MvWeb.Helpers.UserHelpers.has_oidc?(user) + true + """ + @spec has_oidc?(map() | struct()) :: boolean() + def has_oidc?(user) when is_map(user) do + case Map.get(user, :oidc_id) do + id when is_binary(id) and byte_size(id) > 0 -> true + _ -> false + end + end +end diff --git a/lib/mv_web/live/global_settings_live.ex b/lib/mv_web/live/global_settings_live.ex index bbd19ca..fafc955 100644 --- a/lib/mv_web/live/global_settings_live.ex +++ b/lib/mv_web/live/global_settings_live.ex @@ -7,7 +7,6 @@ defmodule MvWeb.GlobalSettingsLive do - Manage custom fields - Real-time form validation - Success/error feedback - - CSV member import (admin only) ## Settings - `club_name` - The name of the association/club (required) @@ -15,47 +14,19 @@ defmodule MvWeb.GlobalSettingsLive do ## Events - `validate` - Real-time form validation - `save` - Save settings changes - - `start_import` - Start CSV member import (admin only) - - ## CSV Import - - The CSV import feature allows administrators to upload CSV files and import members. - - ### File Upload - - Files are uploaded automatically when selected (`auto_upload: true`). No manual - upload trigger is required. - - ### Rate Limiting - - Currently, there is no rate limiting for CSV imports. Administrators can start - multiple imports in quick succession. This is intentional for bulk data migration - scenarios, but should be monitored in production. - - ### Limits - - - Maximum file size: configurable via `config :mv, csv_import: [max_file_size_mb: ...]` - - Maximum rows: configurable via `config :mv, csv_import: [max_rows: ...]` (excluding header) - - Processing: chunks of 200 rows - - Errors: capped at 50 per import ## Note Settings is a singleton resource - there is only one settings record. The club_name can also be set via the `ASSOCIATION_NAME` environment variable. + + CSV member import has been moved to the Import/Export page (`/admin/import-export`). """ use MvWeb, :live_view - alias Mv.Authorization.Actor - alias Mv.Config alias Mv.Membership - alias Mv.Membership.Import.MemberCSV - alias MvWeb.Authorization on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded} - # CSV Import configuration constants - @max_errors 50 - @impl true def mount(_params, session, socket) do {:ok, settings} = Membership.get_settings() @@ -69,22 +40,8 @@ defmodule MvWeb.GlobalSettingsLive do |> assign(:page_title, gettext("Settings")) |> assign(:settings, settings) |> assign(:active_editing_section, nil) - |> assign(:import_state, nil) - |> assign(:import_progress, nil) - |> assign(:import_status, :idle) |> assign(:locale, locale) - |> assign(:max_errors, @max_errors) - |> assign(:csv_import_max_rows, Config.csv_import_max_rows()) - |> assign(:csv_import_max_file_size_mb, Config.csv_import_max_file_size_mb()) |> assign_form() - # Configure file upload with auto-upload enabled - # Files are uploaded automatically when selected, no need for manual trigger - |> allow_upload(:csv_file, - accept: ~w(.csv), - max_entries: 1, - max_file_size: Config.csv_import_max_file_size_bytes(), - auto_upload: true - ) {:ok, socket} end @@ -133,211 +90,6 @@ defmodule MvWeb.GlobalSettingsLive do actor={@current_user} /> - - <%!-- CSV Import Section (Admin only) --%> - <%= if Authorization.can?(@current_user, :create, Mv.Membership.Member) do %> - <.form_section title={gettext("Import Members (CSV)")}> -
    - <.icon name="hero-information-circle" class="size-5" aria-hidden="true" /> -
    -

    - {gettext( - "Use the data field name as the CSV column header in your file. Data fields must exist in Mila before importing, so they must be listed in the list of memberdate (like e-mail or first name). Unknown data field columns will be ignored with a warning." - )} -

    -

    - <.link - href="#custom_fields" - class="link" - data-testid="custom-fields-link" - > - {gettext("Manage Memberdata")} - -

    -
    -
    - -
    -

    - {gettext("Download CSV templates:")} -

    -
      -
    • - <.link - href={~p"/templates/member_import_en.csv"} - download="member_import_en.csv" - class="link link-primary" - > - {gettext("English Template")} - -
      • -
    • - <.link - href={~p"/templates/member_import_de.csv"} - download="member_import_de.csv" - class="link link-primary" - > - {gettext("German Template")} - -
      • -
    -
    - - <.form - id="csv-upload-form" - for={%{}} - multipart={true} - phx-change="validate_csv_upload" - phx-submit="start_import" - data-testid="csv-upload-form" - > -
    - - <.live_file_input - upload={@uploads.csv_file} - id="csv_file" - class="file-input file-input-bordered w-full" - aria-describedby="csv_file_help" - /> - -
    - - <.button - type="submit" - phx-disable-with={gettext("Starting import...")} - variant="primary" - disabled={ - @import_status == :running or - Enum.empty?(@uploads.csv_file.entries) or - @uploads.csv_file.entries |> List.first() |> then(&(&1 && not &1.done?)) - } - data-testid="start-import-button" - > - {gettext("Start Import")} - - - - <%= if @import_status == :running or @import_status == :done do %> - <%= if @import_progress do %> -
    - <%= if @import_progress.status == :running do %> -

    - {gettext("Processing chunk %{current} of %{total}...", - current: @import_progress.current_chunk, - total: @import_progress.total_chunks - )} -

    - <% end %> - - <%= if @import_progress.status == :done do %> -
    -

    - {gettext("Import Results")} -

    - -
    -
    -

    - {gettext("Summary")} -

    -
    -

    - <.icon - name="hero-check-circle" - class="size-4 inline mr-1" - aria-hidden="true" - /> - {gettext("Successfully inserted: %{count} member(s)", - count: @import_progress.inserted - )} -

    - <%= if @import_progress.failed > 0 do %> -

    - <.icon - name="hero-exclamation-circle" - class="size-4 inline mr-1" - aria-hidden="true" - /> - {gettext("Failed: %{count} row(s)", count: @import_progress.failed)} -

    - <% end %> - <%= if @import_progress.errors_truncated? do %> -

    - <.icon - name="hero-information-circle" - class="size-4 inline mr-1" - aria-hidden="true" - /> - {gettext("Error list truncated to %{count} entries", - count: @max_errors - )} -

    - <% end %> -
    -
    - - <%= if length(@import_progress.errors) > 0 do %> -
    -

    - <.icon - name="hero-exclamation-circle" - class="size-4 inline mr-1" - aria-hidden="true" - /> - {gettext("Errors")} -

    -
      - <%= for error <- @import_progress.errors do %> -
    • - {gettext("Line %{line}: %{message}", - line: error.csv_line_number || "?", - message: error.message || gettext("Unknown error") - )} - <%= if error.field do %> - {gettext(" (Field: %{field})", field: error.field)} - <% end %> -
      • - <% end %> -
    -
    - <% end %> - - <%= if length(@import_progress.warnings) > 0 do %> -
    - <.icon name="hero-information-circle" class="size-5" aria-hidden="true" /> -
    -

    - {gettext("Warnings")} -

    -
      - <%= for warning <- @import_progress.warnings do %> -
    • {warning}
      • - <% end %> -
    -
    -
    - <% end %> -
    -
    - <% end %> -
    - <% end %> - <% end %> - - <% end %> """ end @@ -370,115 +122,6 @@ defmodule MvWeb.GlobalSettingsLive do end end - @impl true - def handle_event("validate_csv_upload", _params, socket) do - {:noreply, socket} - end - - @impl true - def handle_event("start_import", _params, socket) do - case check_import_prerequisites(socket) do - {:error, message} -> - {:noreply, put_flash(socket, :error, message)} - - :ok -> - process_csv_upload(socket) - end - end - - # Checks if import can be started (admin permission, status, upload ready) - defp check_import_prerequisites(socket) do - # Ensure user role is loaded before authorization check - user = socket.assigns[:current_user] - user_with_role = Actor.ensure_loaded(user) - - cond do - not Authorization.can?(user_with_role, :create, Mv.Membership.Member) -> - {:error, gettext("Only administrators can import members from CSV files.")} - - socket.assigns.import_status == :running -> - {:error, gettext("Import is already running. Please wait for it to complete.")} - - Enum.empty?(socket.assigns.uploads.csv_file.entries) -> - {:error, gettext("Please select a CSV file to import.")} - - not List.first(socket.assigns.uploads.csv_file.entries).done? -> - {:error, - gettext("Please wait for the file upload to complete before starting the import.")} - - true -> - :ok - end - end - - # Processes CSV upload and starts import - defp process_csv_upload(socket) do - actor = MvWeb.LiveHelpers.current_actor(socket) - - with {:ok, content} <- consume_and_read_csv(socket), - {:ok, import_state} <- - MemberCSV.prepare(content, max_rows: Config.csv_import_max_rows(), actor: actor) do - start_import(socket, import_state) - else - {:error, reason} when is_binary(reason) -> - {:noreply, - put_flash( - socket, - :error, - gettext("Failed to prepare CSV import: %{reason}", reason: reason) - )} - - {:error, error} -> - error_message = format_error_message(error) - - {:noreply, - put_flash( - socket, - :error, - gettext("Failed to prepare CSV import: %{error}", error: error_message) - )} - end - end - - # Starts the import process - defp start_import(socket, import_state) do - progress = initialize_import_progress(import_state) - - socket = - socket - |> assign(:import_state, import_state) - |> assign(:import_progress, progress) - |> assign(:import_status, :running) - - send(self(), {:process_chunk, 0}) - - {:noreply, socket} - end - - # Initializes import progress structure - defp initialize_import_progress(import_state) do - %{ - inserted: 0, - failed: 0, - errors: [], - warnings: import_state.warnings || [], - status: :running, - current_chunk: 0, - total_chunks: length(import_state.chunks), - errors_truncated?: false - } - end - - # Formats error messages for display - defp format_error_message(error) do - case error do - %{message: msg} when is_binary(msg) -> msg - %{errors: errors} when is_list(errors) -> inspect(errors) - reason when is_binary(reason) -> reason - other -> inspect(other) - end - end - @impl true def handle_info({:custom_field_saved, _custom_field, action}, socket) do send_update(MvWeb.CustomFieldLive.IndexComponent, @@ -558,139 +201,6 @@ defmodule MvWeb.GlobalSettingsLive do {:noreply, assign(socket, :settings, updated_settings)} end - @impl true - def handle_info({:process_chunk, idx}, socket) do - case socket.assigns do - %{import_state: import_state, import_progress: progress} - when is_map(import_state) and is_map(progress) -> - if idx >= 0 and idx < length(import_state.chunks) do - start_chunk_processing_task(socket, import_state, progress, idx) - else - handle_chunk_error(socket, :invalid_index, idx) - end - - _ -> - # Missing required assigns - mark as error - handle_chunk_error(socket, :missing_state, idx) - end - end - - @impl true - def handle_info({:chunk_done, idx, result}, socket) do - case socket.assigns do - %{import_state: import_state, import_progress: progress} - when is_map(import_state) and is_map(progress) -> - handle_chunk_result(socket, import_state, progress, idx, result) - - _ -> - # Missing required assigns - mark as error - handle_chunk_error(socket, :missing_state, idx) - end - end - - @impl true - def handle_info({:chunk_error, idx, reason}, socket) do - handle_chunk_error(socket, :processing_failed, idx, reason) - end - - # Starts async task to process a chunk - # In tests (SQL sandbox mode), runs synchronously to avoid Ecto Sandbox issues - defp start_chunk_processing_task(socket, import_state, progress, idx) do - chunk = Enum.at(import_state.chunks, idx) - # Ensure user role is loaded before using as actor - user = socket.assigns[:current_user] - actor = Actor.ensure_loaded(user) - live_view_pid = self() - - # Process chunk with existing error count for capping - opts = [ - custom_field_lookup: import_state.custom_field_lookup, - existing_error_count: length(progress.errors), - max_errors: @max_errors, - actor: actor - ] - - # Get locale from socket for translations in background tasks - locale = socket.assigns[:locale] || "de" - Gettext.put_locale(MvWeb.Gettext, locale) - - if Config.sql_sandbox?() do - # Run synchronously in tests to avoid Ecto Sandbox issues with async tasks - {:ok, chunk_result} = - MemberCSV.process_chunk( - chunk, - import_state.column_map, - import_state.custom_field_map, - opts - ) - - # In test mode, send the message - it will be processed when render() is called - # in the test. The test helper wait_for_import_completion() handles message processing - send(live_view_pid, {:chunk_done, idx, chunk_result}) - else - # Start async task to process chunk in production - # Use start_child for fire-and-forget: no monitor, no Task messages - # We only use our own send/2 messages for communication - Task.Supervisor.start_child(Mv.TaskSupervisor, fn -> - # Set locale in task process for translations - Gettext.put_locale(MvWeb.Gettext, locale) - - {:ok, chunk_result} = - MemberCSV.process_chunk( - chunk, - import_state.column_map, - import_state.custom_field_map, - opts - ) - - send(live_view_pid, {:chunk_done, idx, chunk_result}) - end) - end - - {:noreply, socket} - end - - # Handles chunk processing result from async task - defp handle_chunk_result(socket, import_state, progress, idx, chunk_result) do - # Merge progress - new_progress = merge_progress(progress, chunk_result, idx) - - socket = - socket - |> assign(:import_progress, new_progress) - |> assign(:import_status, new_progress.status) - - # Schedule next chunk or mark as done - socket = schedule_next_chunk(socket, idx, length(import_state.chunks)) - - {:noreply, socket} - end - - # Handles chunk processing errors - defp handle_chunk_error(socket, error_type, idx, reason \\ nil) do - error_message = - case error_type do - :invalid_index -> - gettext("Invalid chunk index: %{idx}", idx: idx) - - :missing_state -> - gettext("Import state is missing. Cannot process chunk %{idx}.", idx: idx) - - :processing_failed -> - gettext("Failed to process chunk %{idx}: %{reason}", - idx: idx, - reason: inspect(reason) - ) - end - - socket = - socket - |> assign(:import_status, :error) - |> put_flash(:error, error_message) - - {:noreply, socket} - end - defp assign_form(%{assigns: %{settings: settings}} = socket) do form = AshPhoenix.Form.for_update( @@ -703,71 +213,4 @@ defmodule MvWeb.GlobalSettingsLive do assign(socket, form: to_form(form)) end - - defp consume_and_read_csv(socket) do - result = - consume_uploaded_entries(socket, :csv_file, fn %{path: path}, _entry -> - case File.read(path) do - {:ok, content} -> {:ok, content} - {:error, reason} -> {:error, Exception.message(reason)} - end - end) - - result - |> case do - [content] when is_binary(content) -> - {:ok, content} - - [{:ok, content}] when is_binary(content) -> - {:ok, content} - - [{:error, reason}] -> - {:error, gettext("Failed to read file: %{reason}", reason: reason)} - - [] -> - {:error, gettext("No file was uploaded")} - - _other -> - {:error, gettext("Failed to read uploaded file")} - end - end - - defp merge_progress(progress, chunk_result, current_chunk_idx) do - # Merge errors with cap of @max_errors overall - all_errors = progress.errors ++ chunk_result.errors - new_errors = Enum.take(all_errors, @max_errors) - errors_truncated? = length(all_errors) > @max_errors - - # Merge warnings (optional dedupe - simple append for now) - new_warnings = progress.warnings ++ Map.get(chunk_result, :warnings, []) - - # Update status based on whether we're done - # current_chunk_idx is 0-based, so after processing chunk 0, we've processed 1 chunk - chunks_processed = current_chunk_idx + 1 - new_status = if chunks_processed >= progress.total_chunks, do: :done, else: :running - - %{ - inserted: progress.inserted + chunk_result.inserted, - failed: progress.failed + chunk_result.failed, - errors: new_errors, - warnings: new_warnings, - status: new_status, - current_chunk: chunks_processed, - total_chunks: progress.total_chunks, - errors_truncated?: errors_truncated? || chunk_result.errors_truncated? - } - end - - defp schedule_next_chunk(socket, current_idx, total_chunks) do - next_idx = current_idx + 1 - - if next_idx < total_chunks do - # Schedule next chunk - send(self(), {:process_chunk, next_idx}) - socket - else - # All chunks processed - status already set to :done in merge_progress - socket - end - end end diff --git a/lib/mv_web/live/group_live/show.ex b/lib/mv_web/live/group_live/show.ex index af8582f..0899728 100644 --- a/lib/mv_web/live/group_live/show.ex +++ b/lib/mv_web/live/group_live/show.ex @@ -22,15 +22,7 @@ defmodule MvWeb.GroupLive.Show do @impl true def mount(_params, _session, socket) do - {:ok, - socket - |> assign(:show_add_member_input, false) - |> assign(:member_search_query, "") - |> assign(:available_members, []) - |> assign(:selected_member_ids, []) - |> assign(:selected_members, []) - |> assign(:show_member_dropdown, false) - |> assign(:focused_member_index, nil)} + {:ok, socket} end @impl true @@ -130,120 +122,6 @@ defmodule MvWeb.GroupLive.Show do )}

    - <%= if can?(@current_user, :update, Mv.Membership.Group) do %> -
    - <%= if assigns[:show_add_member_input] do %> -
    -
    -
    -
    - <%= for member <- @selected_members do %> - - {MvWeb.Helpers.MemberHelpers.display_name(member)} - - - <% end %> - -
    - - <%= if length(@available_members) > 0 do %> -
    - <%= for {member, index} <- Enum.with_index(@available_members) do %> -
    -

    - {MvWeb.Helpers.MemberHelpers.display_name(member)} -

    -

    - {member.email || gettext("No email")} -

    -
    - <% end %> -
    - <% end %> -
    -
    - -
    - <% else %> - <.button - variant="primary" - phx-click="show_add_member_input" - aria-label={gettext("Add Member")} - > - {gettext("Add Member")} - - <% end %> -
    - <% end %> - <%= if Enum.empty?(@group.members || []) do %>

    {gettext("No members in this group")}

    <% else %> @@ -253,9 +131,6 @@ defmodule MvWeb.GroupLive.Show do
    {gettext("Name")} {gettext("Email")}{gettext("Actions")}
    - -