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 c7bcfa6..7e4cee9 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -81,8 +81,8 @@ lib/ ├── membership/ # Membership domain │ ├── membership.ex # Domain definition │ ├── member.ex # Member resource +│ ├── custom_field.ex # Custom field (definition) resource │ ├── custom_field_value.ex # Custom field value resource -│ ├── custom_field.ex # CustomFieldValue type resource │ ├── setting.ex # Global settings (singleton resource) │ ├── group.ex # Group resource │ ├── member_group.ex # MemberGroup join table resource @@ -198,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 @@ -1339,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/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/groups-architecture.md b/docs/groups-architecture.md index b2316d8..735898c 100644 --- a/docs/groups-architecture.md +++ b/docs/groups-architecture.md @@ -4,7 +4,7 @@ **Feature:** Groups Management **Version:** 1.0 **Last Updated:** 2025-01-XX -**Status:** Architecture Design - Ready for Implementation +**Status:** ✅ Implemented (authorization: see [roles-and-permissions-architecture.md](./roles-and-permissions-architecture.md)) --- @@ -412,15 +412,17 @@ lib/ ## Authorization +**Status:** ✅ Implemented. Group and MemberGroup resource policies and PermissionSets are in place. See [roles-and-permissions-architecture.md](./roles-and-permissions-architecture.md) for the full permission matrix and policy patterns. + ### Permission Model (MVP) -**Resource:** `groups` +**Resource:** `Group` (and `MemberGroup`) **Actions:** -- `read` - View groups (all users with member read permission) -- `create` - Create groups (admin only) -- `update` - Edit groups (admin only) -- `destroy` - Delete groups (admin only) +- `read` - View groups (all permission sets) +- `create` - Create groups (normal_user and admin) +- `update` - Edit groups (normal_user and admin) +- `destroy` - Delete groups (normal_user and admin) **Scopes:** - `:all` - All groups (for all permission sets that have read access) @@ -442,7 +444,7 @@ lib/ **Own Data Permission Set:** - `read` action on `Group` resource with `:all` scope - granted -**Note:** All permission sets use `:all` scope for groups. Groups are considered public information that all users with member read permission can view. Only admins can manage (create/update/destroy) groups. +**Note:** All permission sets use `:all` scope for groups. Groups are considered public information that all users with member read permission can view. normal_user and admin can manage (create/update/destroy) groups. ### Member-Group Association Permissions diff --git a/docs/membership-fee-architecture.md b/docs/membership-fee-architecture.md index 4a290b7..6c81169 100644 --- a/docs/membership-fee-architecture.md +++ b/docs/membership-fee-architecture.md @@ -334,20 +334,18 @@ lib/ ### Permission System Integration -**See:** [roles-and-permissions-architecture.md](./roles-and-permissions-architecture.md) +**Status:** ✅ Implemented. See [roles-and-permissions-architecture.md](./roles-and-permissions-architecture.md) for the full permission matrix and policy patterns. -**Required Permissions:** +**PermissionSets (lib/mv/authorization/permission_sets.ex):** -- `MembershipFeeType.create/update/destroy` - Admin only -- `MembershipFeeType.read` - Admin, Treasurer, Board -- `MembershipFeeCycle.update` (status changes) - Admin, Treasurer -- `MembershipFeeCycle.read` - Admin, Treasurer, Board, Own member +- **MembershipFeeType:** All permission sets can read (:all); only admin has create/update/destroy (:all). +- **MembershipFeeCycle:** All can read (:all); read_only has read only; normal_user and admin have read + create + update + destroy (:all). +- **Manual "Regenerate Cycles" (UI + server):** The "Regenerate Cycles" button in the member detail view is shown to users who have MembershipFeeCycle create permission (normal_user and admin). UI access is gated by `can_create_cycle`. The LiveView handler also enforces `can?(:create, MembershipFeeCycle)` server-side before running regeneration (so e.g. a read_only user cannot trigger it via DevTools). Regeneration runs with system actor. -**Policy Patterns:** +**Resource Policies:** -- Use existing HasPermission check -- Leverage existing roles (Admin, Kassenwart) -- Member can read own cycles (linked via member_id) +- **MembershipFeeType** (`lib/membership_fees/membership_fee_type.ex`): `authorizers: [Ash.Policy.Authorizer]`, single policy with `HasPermission` for read/create/update/destroy. +- **MembershipFeeCycle** (`lib/membership_fees/membership_fee_cycle.ex`): Same pattern; update includes mark_as_paid, mark_as_suspended, mark_as_unpaid. ### LiveView Integration @@ -357,7 +355,7 @@ lib/ 2. MembershipFeeCycle table component (member detail view) - Implemented as `MvWeb.MemberLive.Show.MembershipFeesComponent` - Displays all cycles in a table with status management - - Allows changing cycle status, editing amounts, and regenerating cycles + - Allows changing cycle status, editing amounts, and manually regenerating cycles (normal_user and admin) 3. Settings form section (admin) 4. Member list column (membership fee status) diff --git a/docs/roles-and-permissions-architecture.md b/docs/roles-and-permissions-architecture.md index dbf2353..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 ea0ddff..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,34 +148,26 @@ 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) "/users/:id", "/users/:id/edit", "/users/:id/show/edit", - # Member list and CSV export + # Member list "/members", - "/members/export.csv", # Member detail "/members/:id", # Custom field values overview @@ -177,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) @@ -209,7 +223,6 @@ defmodule Mv.Authorization.PermissionSets do "/users/:id/edit", "/users/:id/show/edit", "/members", - "/members/export.csv", # Create member "/members/new", "/members/:id", @@ -223,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/custom_field_value_formatter.ex b/lib/mv/membership/custom_field_value_formatter.ex deleted file mode 100644 index 9709353..0000000 --- a/lib/mv/membership/custom_field_value_formatter.ex +++ /dev/null @@ -1,55 +0,0 @@ -defmodule Mv.Membership.CustomFieldValueFormatter do - @moduledoc """ - Neutral formatter for custom field values (e.g. CSV export). - - Same logic as the member overview Formatter but without Gettext or web helpers, - so it can be used from the Membership context. For boolean: "Yes"/"No"; - for date: European format (dd.mm.yyyy). - """ - @doc """ - Formats a custom field value for plain text (e.g. CSV). - - Handles nil, Ash.Union, JSONB map, and direct values. Uses custom_field.value_type - for typing. Boolean -> "Yes"/"No", Date -> dd.mm.yyyy. - """ - def format_custom_field_value(nil, _custom_field), do: "" - - def format_custom_field_value(%Ash.Union{value: value, type: type}, custom_field) do - format_value_by_type(value, type, custom_field) - end - - def format_custom_field_value(value, custom_field) when is_map(value) do - type = Map.get(value, "type") || Map.get(value, "_union_type") - val = Map.get(value, "value") || Map.get(value, "_union_value") - format_value_by_type(val, type, custom_field) - end - - def format_custom_field_value(value, custom_field) do - format_value_by_type(value, custom_field.value_type, custom_field) - end - - defp format_value_by_type(value, :string, _), do: to_string(value) - defp format_value_by_type(value, :integer, _), do: to_string(value) - - defp format_value_by_type(value, type, _) when type in [:string, :email] and is_binary(value) do - if String.trim(value) == "", do: "", else: value - end - - defp format_value_by_type(value, :email, _), do: to_string(value) - defp format_value_by_type(value, :boolean, _) when value == true, do: "Yes" - defp format_value_by_type(value, :boolean, _) when value == false, do: "No" - defp format_value_by_type(value, :boolean, _), do: to_string(value) - - defp format_value_by_type(%Date{} = date, :date, _) do - Calendar.strftime(date, "%d.%m.%Y") - end - - defp format_value_by_type(value, :date, _) when is_binary(value) do - case Date.from_iso8601(value) do - {:ok, date} -> Calendar.strftime(date, "%d.%m.%Y") - _ -> value - end - end - - defp format_value_by_type(value, _type, _), do: to_string(value) -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/membership/member_export.ex b/lib/mv/membership/member_export.ex deleted file mode 100644 index 5f771cc..0000000 --- a/lib/mv/membership/member_export.ex +++ /dev/null @@ -1,344 +0,0 @@ -defmodule Mv.Membership.MemberExport do - @moduledoc """ - Builds member list and column specs for CSV export. - - Used by `MvWeb.MemberExportController`. Does not perform translations; - the controller applies headers (e.g. via `MemberFields.label` / gettext) - and sends the download. - """ - - require Ash.Query - import Ash.Expr - - alias Mv.Membership.CustomField - alias Mv.Membership.Member - alias Mv.Membership.MemberExportSort - alias MvWeb.MemberLive.Index.MembershipFeeStatus - - @member_fields_allowlist (Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1)) ++ - ["membership_fee_status", "payment_status"] - @computed_export_fields ["membership_fee_status", "payment_status"] - @custom_field_prefix Mv.Constants.custom_field_prefix() - @domain_member_field_strings Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1) - - @doc """ - Fetches members and column specs for export. - - - `actor` - Ash actor (e.g. current user) - - `parsed` - Map from controller's parse_and_validate (selected_ids, member_fields, etc.) - - Returns `{:ok, members, column_specs}` or `{:error, :forbidden}`. - Column specs have `:kind`, `:key`, and for custom fields `:custom_field`; - the controller adds `:header` and optional computed columns to members before CSV export. - """ - @spec fetch(struct(), map()) :: - {:ok, [struct()], [map()]} | {:error, :forbidden} - def fetch(actor, parsed) do - custom_field_ids_union = - (parsed.custom_field_ids ++ Map.keys(parsed.boolean_filters || %{})) |> Enum.uniq() - - with {:ok, custom_fields_by_id} <- load_custom_fields_by_id(custom_field_ids_union, actor), - {:ok, members} <- load_members(actor, parsed, custom_fields_by_id) do - column_specs = build_column_specs(parsed, custom_fields_by_id) - {:ok, members, column_specs} - end - end - - defp load_custom_fields_by_id([], _actor), do: {:ok, %{}} - - defp load_custom_fields_by_id(custom_field_ids, actor) do - query = - CustomField - |> Ash.Query.filter(expr(id in ^custom_field_ids)) - |> Ash.Query.select([:id, :name, :value_type]) - - case Ash.read(query, actor: actor) do - {:ok, custom_fields} -> - by_id = - Enum.reduce(custom_field_ids, %{}, fn id, acc -> - case Enum.find(custom_fields, fn cf -> to_string(cf.id) == to_string(id) end) do - nil -> acc - cf -> Map.put(acc, id, cf) - end - end) - - {:ok, by_id} - - {:error, %Ash.Error.Forbidden{}} -> - {:error, :forbidden} - end - end - - defp build_column_specs(parsed, custom_fields_by_id) do - member_specs = - Enum.map(parsed.member_fields, fn f -> - if f in parsed.selectable_member_fields do - %{kind: :member_field, key: f} - else - %{kind: :computed, key: String.to_existing_atom(f)} - end - end) - - custom_specs = - parsed.custom_field_ids - |> Enum.map(fn id -> Map.get(custom_fields_by_id, id) end) - |> Enum.reject(&is_nil/1) - |> Enum.map(fn cf -> %{kind: :custom_field, key: cf.id, custom_field: cf} end) - - member_specs ++ custom_specs - end - - defp load_members(actor, parsed, custom_fields_by_id) do - select_fields = - [:id] ++ Enum.map(parsed.selectable_member_fields, &String.to_existing_atom/1) - - custom_field_ids_union = parsed.custom_field_ids ++ Map.keys(parsed.boolean_filters || %{}) - - need_cycles = - parsed.show_current_cycle or parsed.cycle_status_filter != nil or - parsed.computed_fields != [] - - query = - Member - |> Ash.Query.new() - |> Ash.Query.select(select_fields) - |> load_custom_field_values_query(custom_field_ids_union) - |> maybe_load_cycles(need_cycles, parsed.show_current_cycle) - - query = - if parsed.selected_ids != [] do - Ash.Query.filter(query, expr(id in ^parsed.selected_ids)) - else - query - |> apply_search(parsed.query) - |> then(fn q -> - {q, _sort_after_load} = maybe_sort(q, parsed.sort_field, parsed.sort_order) - q - end) - end - - case Ash.read(query, actor: actor) do - {:ok, members} -> - members = - if parsed.selected_ids == [] do - members - |> apply_cycle_status_filter(parsed.cycle_status_filter, parsed.show_current_cycle) - |> MvWeb.MemberLive.Index.apply_boolean_custom_field_filters( - parsed.boolean_filters || %{}, - Map.values(custom_fields_by_id) - ) - else - members - end - - members = - if parsed.selected_ids == [] and sort_after_load?(parsed.sort_field) do - sort_members_by_custom_field( - members, - parsed.sort_field, - parsed.sort_order, - Map.values(custom_fields_by_id) - ) - else - members - end - - {:ok, members} - - {:error, %Ash.Error.Forbidden{}} -> - {:error, :forbidden} - end - end - - defp load_custom_field_values_query(query, []), do: query - - defp load_custom_field_values_query(query, custom_field_ids) do - cfv_query = - Mv.Membership.CustomFieldValue - |> Ash.Query.filter(expr(custom_field_id in ^custom_field_ids)) - |> Ash.Query.load(custom_field: [:id, :name, :value_type]) - - Ash.Query.load(query, custom_field_values: cfv_query) - end - - defp apply_search(query, nil), do: query - defp apply_search(query, ""), do: query - - defp apply_search(query, q) when is_binary(q) do - if String.trim(q) != "" do - Member.fuzzy_search(query, %{query: q}) - else - query - end - end - - defp maybe_sort(query, nil, _order), do: {query, false} - defp maybe_sort(query, _field, nil), do: {query, false} - - defp maybe_sort(query, field, order) when is_binary(field) do - if custom_field_sort?(field) do - {query, true} - else - field_atom = String.to_existing_atom(field) - - if field_atom in (Mv.Constants.member_fields() -- [:notes]) do - {Ash.Query.sort(query, [{field_atom, String.to_existing_atom(order)}]), false} - else - {query, false} - end - end - rescue - ArgumentError -> {query, false} - end - - defp sort_after_load?(field) when is_binary(field), - do: String.starts_with?(field, @custom_field_prefix) - - defp sort_after_load?(_), do: false - - defp sort_members_by_custom_field(members, _field, _order, _custom_fields) when members == [], - do: [] - - defp sort_members_by_custom_field(members, field, order, custom_fields) when is_binary(field) do - id_str = String.trim_leading(field, @custom_field_prefix) - custom_field = Enum.find(custom_fields, fn cf -> to_string(cf.id) == id_str end) - if is_nil(custom_field), do: members - - key_fn = fn member -> - cfv = find_cfv(member, custom_field) - raw = if cfv, do: cfv.value, else: nil - MemberExportSort.custom_field_sort_key(custom_field.value_type, raw) - end - - members - |> Enum.map(fn m -> {m, key_fn.(m)} end) - |> Enum.sort(fn {_, ka}, {_, kb} -> MemberExportSort.key_lt(ka, kb, order) end) - |> Enum.map(fn {m, _} -> m end) - end - - defp find_cfv(member, custom_field) do - (member.custom_field_values || []) - |> Enum.find(fn cfv -> - to_string(cfv.custom_field_id) == to_string(custom_field.id) or - (Map.get(cfv, :custom_field) && - to_string(cfv.custom_field.id) == to_string(custom_field.id)) - end) - end - - defp custom_field_sort?(field), do: String.starts_with?(field, @custom_field_prefix) - - defp maybe_load_cycles(query, false, _show_current), do: query - - defp maybe_load_cycles(query, true, show_current) do - MembershipFeeStatus.load_cycles_for_members(query, show_current) - end - - defp apply_cycle_status_filter(members, nil, _show_current), do: members - - defp apply_cycle_status_filter(members, status, show_current) when status in [:paid, :unpaid] do - MembershipFeeStatus.filter_members_by_cycle_status(members, status, show_current) - end - - defp apply_cycle_status_filter(members, _status, _show_current), do: members - - # Called by controller to build parsed map from raw params (kept here so controller stays thin) - @doc """ - Parses and validates export params (from JSON payload). - - Returns a map with :selected_ids, :member_fields, :selectable_member_fields, - :computed_fields, :custom_field_ids, :query, :sort_field, :sort_order, - :show_current_cycle, :cycle_status_filter, :boolean_filters. - """ - @spec parse_params(map()) :: map() - def parse_params(params) do - member_fields = filter_allowed_member_fields(extract_list(params, "member_fields")) - {selectable_member_fields, computed_fields} = split_member_fields(member_fields) - - %{ - selected_ids: filter_valid_uuids(extract_list(params, "selected_ids")), - member_fields: member_fields, - selectable_member_fields: selectable_member_fields, - computed_fields: computed_fields, - custom_field_ids: filter_valid_uuids(extract_list(params, "custom_field_ids")), - query: extract_string(params, "query"), - sort_field: extract_string(params, "sort_field"), - sort_order: extract_sort_order(params), - show_current_cycle: extract_boolean(params, "show_current_cycle"), - cycle_status_filter: extract_cycle_status_filter(params), - boolean_filters: extract_boolean_filters(params) - } - end - - defp split_member_fields(member_fields) do - selectable = Enum.filter(member_fields, fn f -> f in @domain_member_field_strings end) - computed = Enum.filter(member_fields, fn f -> f in @computed_export_fields end) - {selectable, computed} - end - - defp extract_boolean(params, key) do - case Map.get(params, key) do - true -> true - "true" -> true - _ -> false - end - end - - defp extract_cycle_status_filter(params) do - case Map.get(params, "cycle_status_filter") do - "paid" -> :paid - "unpaid" -> :unpaid - _ -> nil - end - end - - defp extract_boolean_filters(params) do - case Map.get(params, "boolean_filters") do - map when is_map(map) -> - map - |> Enum.filter(fn {k, v} -> is_binary(k) and is_boolean(v) end) - |> Enum.filter(fn {k, _} -> match?({:ok, _}, Ecto.UUID.cast(k)) end) - |> Enum.into(%{}) - - _ -> - %{} - end - end - - defp extract_list(params, key) do - case Map.get(params, key) do - list when is_list(list) -> list - _ -> [] - end - end - - defp extract_string(params, key) do - case Map.get(params, key) do - s when is_binary(s) -> s - _ -> nil - end - end - - defp extract_sort_order(params) do - case Map.get(params, "sort_order") do - "asc" -> "asc" - "desc" -> "desc" - _ -> nil - end - end - - defp filter_allowed_member_fields(field_list) do - allowlist = MapSet.new(@member_fields_allowlist) - - field_list - |> Enum.filter(fn field -> is_binary(field) and MapSet.member?(allowlist, field) end) - |> Enum.uniq() - end - - defp filter_valid_uuids(id_list) when is_list(id_list) do - id_list - |> Enum.filter(fn id -> - is_binary(id) and match?({:ok, _}, Ecto.UUID.cast(id)) - end) - |> Enum.uniq() - end -end diff --git a/lib/mv/membership/member_export_sort.ex b/lib/mv/membership/member_export_sort.ex deleted file mode 100644 index 324fb75..0000000 --- a/lib/mv/membership/member_export_sort.ex +++ /dev/null @@ -1,44 +0,0 @@ -defmodule Mv.Membership.MemberExportSort do - @moduledoc """ - Type-stable sort keys for CSV export custom-field sorting. - - Used only by `MvWeb.MemberExportController` when sorting members by a custom field - after load. Nil values sort last in ascending order and first in descending order. - String and email comparison is case-insensitive. - """ - @doc """ - Returns a comparable sort key for (value_type, value). - - - Nil: rank 1 so that in asc order nil sorts last, in desc nil sorts first. - - date: chronological (ISO8601 string). - - boolean: false < true (0 < 1). - - integer: numerical order. - - string / email: case-insensitive (downcased). - - Handles Ash.Union in value; value_type is the custom field's value_type atom. - """ - @spec custom_field_sort_key(:string | :integer | :boolean | :date | :email, term()) :: - {0 | 1, term()} - def custom_field_sort_key(_value_type, nil), do: {1, nil} - - def custom_field_sort_key(value_type, %Ash.Union{value: value, type: _type}) do - custom_field_sort_key(value_type, value) - end - - def custom_field_sort_key(:date, %Date{} = d), do: {0, Date.to_iso8601(d)} - def custom_field_sort_key(:boolean, true), do: {0, 1} - def custom_field_sort_key(:boolean, false), do: {0, 0} - def custom_field_sort_key(:integer, v) when is_integer(v), do: {0, v} - def custom_field_sort_key(:string, v) when is_binary(v), do: {0, String.downcase(v)} - def custom_field_sort_key(:email, v) when is_binary(v), do: {0, String.downcase(v)} - def custom_field_sort_key(_value_type, v), do: {0, to_string(v)} - - @doc """ - Returns true if key_a should sort before key_b for the given order. - - "asc" -> nil last; "desc" -> nil first. No reverse of list needed. - """ - @spec key_lt({0 | 1, term()}, {0 | 1, term()}, String.t()) :: boolean() - def key_lt(key_a, key_b, "asc"), do: key_a < key_b - def key_lt(key_a, key_b, "desc"), do: key_b < key_a -end diff --git a/lib/mv/membership/members_csv.ex b/lib/mv/membership/members_csv.ex deleted file mode 100644 index a0fd463..0000000 --- a/lib/mv/membership/members_csv.ex +++ /dev/null @@ -1,100 +0,0 @@ -defmodule Mv.Membership.MembersCSV do - @moduledoc """ - Exports members to CSV (RFC 4180) as iodata. - - Uses a column-based API: `export(members, columns)` where each column has - `header` (display string, e.g. from Web layer), `kind` (:member_field | :custom_field | :computed), - and `key` (member attribute name, custom_field id, or computed key). Custom field columns - include a `custom_field` struct for value formatting. Domain code does not use Gettext; - headers and computed values come from the caller (e.g. controller). - """ - alias Mv.Membership.CustomFieldValueFormatter - alias NimbleCSV.RFC4180 - - @doc """ - Exports a list of members to CSV iodata. - - - `members` - List of member structs or maps (with optional `custom_field_values` loaded) - - `columns` - List of column specs: `%{header: String.t(), kind: :member_field | :custom_field | :computed, key: term()}` - For `:custom_field`, also pass `custom_field: %CustomField{}`. Header is used as-is (localized by caller). - - Returns iodata suitable for `IO.iodata_to_binary/1` or sending as response body. - RFC 4180 escaping and formula-injection safe_cell are applied. - """ - @spec export([struct() | map()], [map()]) :: iodata() - def export(members, columns) when is_list(members) do - header = build_header(columns) - rows = Enum.map(members, fn member -> build_row(member, columns) end) - RFC4180.dump_to_iodata([header | rows]) - end - - defp build_header(columns) do - columns - |> Enum.map(fn col -> col.header end) - |> Enum.map(&safe_cell/1) - end - - defp build_row(member, columns) do - columns - |> Enum.map(fn col -> cell_value(member, col) end) - |> Enum.map(&safe_cell/1) - end - - defp cell_value(member, %{kind: :member_field, key: key}) do - key_atom = key_to_atom(key) - value = Map.get(member, key_atom) - format_member_value(value) - end - - defp cell_value(member, %{kind: :custom_field, key: id, custom_field: cf}) do - cfv = get_cfv_by_id(member, id) - - if cfv, - do: CustomFieldValueFormatter.format_custom_field_value(cfv.value, cf), - else: "" - end - - defp cell_value(member, %{kind: :computed, key: key}) do - value = Map.get(member, key_to_atom(key)) - if is_binary(value), do: value, else: "" - end - - defp key_to_atom(k) when is_atom(k), do: k - - defp key_to_atom(k) when is_binary(k) do - try do - String.to_existing_atom(k) - rescue - ArgumentError -> k - end - end - - defp get_cfv_by_id(member, id) do - values = - case Map.get(member, :custom_field_values) do - v when is_list(v) -> v - _ -> [] - end - - id_str = to_string(id) - - Enum.find(values, fn cfv -> - to_string(cfv.custom_field_id) == id_str or - (Map.get(cfv, :custom_field) && to_string(cfv.custom_field.id) == id_str) - end) - end - - @doc false - @spec safe_cell(String.t()) :: String.t() - def safe_cell(s) when is_binary(s) do - if String.starts_with?(s, ["=", "+", "-", "@", "\t"]), do: "'" <> s, else: s - end - - defp format_member_value(nil), do: "" - defp format_member_value(true), do: "true" - defp format_member_value(false), do: "false" - defp format_member_value(%Date{} = d), do: Date.to_iso8601(d) - defp format_member_value(%DateTime{} = dt), do: DateTime.to_iso8601(dt) - defp format_member_value(%NaiveDateTime{} = dt), do: NaiveDateTime.to_iso8601(dt) - defp format_member_value(value), do: to_string(value) -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 e74020c..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])) @@ -178,8 +179,7 @@ defmodule MvWeb.CoreComponents do aria-haspopup="menu" aria-expanded={@open} aria-controls={@id} - aria-label={@button_label} - class="btn focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-base-content/20" + class="btn" phx-click="toggle_dropdown" phx-target={@phx_target} data-testid="dropdown-button" @@ -233,12 +233,11 @@ defmodule MvWeb.CoreComponents do