From 37d165522776acc12418531f33c8a9be2cc4ad84 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 17:18:29 +0100 Subject: [PATCH 01/24] feat: add PermissionSets stub module for role validation Add minimal PermissionSets module with all_permission_sets/0 function to support permission_set_name validation in Role resource. --- lib/mv/authorization/permission_sets.ex | 34 +++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 lib/mv/authorization/permission_sets.ex diff --git a/lib/mv/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex new file mode 100644 index 0000000..fb9d249 --- /dev/null +++ b/lib/mv/authorization/permission_sets.ex @@ -0,0 +1,34 @@ +defmodule Mv.Authorization.PermissionSets do + @moduledoc """ + Defines the four hardcoded permission sets for the application. + + This is a minimal stub implementation for Issue #1. The full implementation + with all permission details will be added in Issue #2. + + ## Permission Sets + + 1. **own_data** - Default for "Mitglied" role + 2. **read_only** - For "Vorstand" and "Buchhaltung" roles + 3. **normal_user** - For "Kassenwart" role + 4. **admin** - For "Admin" role + + ## Usage + + # Get list of all valid permission set names + PermissionSets.all_permission_sets() + # => [:own_data, :read_only, :normal_user, :admin] + """ + + @doc """ + Returns the list of all valid permission set names. + + ## Examples + + iex> PermissionSets.all_permission_sets() + [:own_data, :read_only, :normal_user, :admin] + """ + @spec all_permission_sets() :: [atom()] + def all_permission_sets do + [:own_data, :read_only, :normal_user, :admin] + end +end From 1b2927ce40a5722da6f223874fae143f51d04c35 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 17:18:30 +0100 Subject: [PATCH 02/24] feat: create Authorization domain Add Mv.Authorization domain with AshAdmin and AshPhoenix extensions. Register domain in config for role management. --- lib/mv/authorization/authorization.ex | 30 +++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 lib/mv/authorization/authorization.ex diff --git a/lib/mv/authorization/authorization.ex b/lib/mv/authorization/authorization.ex new file mode 100644 index 0000000..23672f1 --- /dev/null +++ b/lib/mv/authorization/authorization.ex @@ -0,0 +1,30 @@ +defmodule Mv.Authorization do + @moduledoc """ + Ash Domain for authorization and role management. + + ## Resources + - `Role` - User roles that reference permission sets + + ## Public API + The domain exposes these main actions: + - Role CRUD: `create_role/1`, `list_roles/0`, `update_role/2`, `destroy_role/1` + + ## Admin Interface + The domain is configured with AshAdmin for management UI. + """ + use Ash.Domain, + extensions: [AshAdmin.Domain, AshPhoenix] + + admin do + show? true + end + + resources do + resource Mv.Authorization.Role do + define :create_role, action: :create_role + define :list_roles, action: :read + define :update_role, action: :update_role + define :destroy_role, action: :destroy + end + end +end From 4535551b8d83e8e52e11ef9fbeda5aed3a2fb529 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 17:18:32 +0100 Subject: [PATCH 03/24] feat: add Role resource with validations Create Role resource with name, description, permission_set_name, and is_system_role fields. Add validations for permission_set_name and system role deletion protection. --- lib/mv/authorization/role.ex | 152 +++++++++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 lib/mv/authorization/role.ex diff --git a/lib/mv/authorization/role.ex b/lib/mv/authorization/role.ex new file mode 100644 index 0000000..e5b9795 --- /dev/null +++ b/lib/mv/authorization/role.ex @@ -0,0 +1,152 @@ +defmodule Mv.Authorization.Role do + @moduledoc """ + Represents a user role that references a permission set. + + Roles are stored in the database and link users to permission sets. + Each role has a `permission_set_name` that references one of the four + hardcoded permission sets defined in `Mv.Authorization.PermissionSets`. + + ## Fields + + - `name` - Unique role name (e.g., "Vorstand", "Admin") + - `description` - Human-readable description of the role + - `permission_set_name` - Must be one of: "own_data", "read_only", "normal_user", "admin" + - `is_system_role` - If true, role cannot be deleted (protects critical roles like "Mitglied") + + ## Relationships + + - `has_many :users` - Users assigned to this role + + ## Validations + + - `permission_set_name` must be a valid permission set (checked against PermissionSets.all_permission_sets/0) + - `name` must be unique + - System roles cannot be deleted (enforced via validation) + + ## Examples + + # Create a new role + {:ok, role} = Mv.Authorization.create_role(%{ + name: "Vorstand", + description: "Board member with read access", + permission_set_name: "read_only" + }) + + # List all roles + {:ok, roles} = Mv.Authorization.list_roles() + """ + use Ash.Resource, + domain: Mv.Authorization, + data_layer: AshPostgres.DataLayer + + postgres do + table "roles" + repo Mv.Repo + end + + code_interface do + define :create_role + define :list_roles, action: :read + define :update_role + define :destroy_role, action: :destroy + end + + actions do + defaults [:read] + + create :create_role do + primary? true + accept [:name, :description, :permission_set_name, :is_system_role] + # Note: In Ash 3.0, require_atomic? is not available for create actions + # Custom validations will still work + end + + update :update_role do + primary? true + accept [:name, :description, :permission_set_name, :is_system_role] + # Required because custom validation functions cannot be executed atomically + require_atomic? false + end + + destroy :destroy do + # Required because custom validation functions cannot be executed atomically + require_atomic? false + end + end + + validations do + validate fn changeset, _context -> + permission_set_name = Ash.Changeset.get_attribute(changeset, :permission_set_name) + + if permission_set_name do + valid_sets = + Mv.Authorization.PermissionSets.all_permission_sets() + |> Enum.map(&Atom.to_string/1) + + if permission_set_name in valid_sets do + :ok + else + valid_sets_string = Enum.join(valid_sets, ", ") + + {:error, + field: :permission_set_name, + message: "Invalid permission set name. Must be one of: #{valid_sets_string}"} + end + else + :ok + end + end + + validate fn changeset, _context -> + if changeset.action_type == :destroy do + if changeset.data.is_system_role do + {:error, + message: + "Cannot delete system role. System roles are required for the application to function."} + else + :ok + end + else + :ok + end + end, + on: [:destroy] + end + + attributes do + uuid_primary_key :id + + attribute :name, :string do + allow_nil? false + public? true + end + + attribute :description, :string do + allow_nil? true + public? true + end + + attribute :permission_set_name, :string do + allow_nil? false + public? true + end + + attribute :is_system_role, :boolean do + allow_nil? false + default false + public? true + end + + timestamps() + end + + relationships do + has_many :users, Mv.Accounts.User do + destination_attribute :role_id + end + end + + identities do + identity :unique_name, [:name] + end +end From 90c32c2afdc8e21431ff4356326ea1e4e135aa71 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 17:18:33 +0100 Subject: [PATCH 04/24] feat: add role relationship to User resource Add belongs_to :role relationship to User resource and register Authorization domain in config. --- config/config.exs | 2 +- lib/accounts/user.ex | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/config/config.exs b/config/config.exs index 5fcfcf5..cc338b2 100644 --- a/config/config.exs +++ b/config/config.exs @@ -49,7 +49,7 @@ config :spark, config :mv, ecto_repos: [Mv.Repo], generators: [timestamp_type: :utc_datetime], - ash_domains: [Mv.Membership, Mv.Accounts, Mv.MembershipFees] + ash_domains: [Mv.Membership, Mv.Accounts, Mv.MembershipFees, Mv.Authorization] # Configures the endpoint config :mv, MvWeb.Endpoint, diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex index dbc62b2..b0d919b 100644 --- a/lib/accounts/user.ex +++ b/lib/accounts/user.ex @@ -357,6 +357,11 @@ defmodule Mv.Accounts.User do # This automatically creates a `member_id` attribute in the User table # The relationship is optional (allow_nil? true by default) belongs_to :member, Mv.Membership.Member + + # 1:1 relationship - User belongs to a Role + # This automatically creates a `role_id` attribute in the User table + # The relationship is optional (allow_nil? true by default) + belongs_to :role, Mv.Authorization.Role end identities do From 851d63f626162bc599af6651a0ab230221ea731b Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 17:18:34 +0100 Subject: [PATCH 05/24] feat: add authorization domain migration Create roles table and add role_id to users table with indexes and foreign key constraints. --- ...0260106161215_add_authorization_domain.exs | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 priv/repo/migrations/20260106161215_add_authorization_domain.exs diff --git a/priv/repo/migrations/20260106161215_add_authorization_domain.exs b/priv/repo/migrations/20260106161215_add_authorization_domain.exs new file mode 100644 index 0000000..02edcd3 --- /dev/null +++ b/priv/repo/migrations/20260106161215_add_authorization_domain.exs @@ -0,0 +1,79 @@ +defmodule Mv.Repo.Migrations.AddAuthorizationDomain do + @moduledoc """ + Updates resources based on their most recent snapshots. + + This file was autogenerated with `mix ash_postgres.generate_migrations` + """ + + use Ecto.Migration + + def up do + alter table(:users) do + add :role_id, :uuid + end + + create table(:roles, primary_key: false) do + add :id, :uuid, null: false, default: fragment("gen_random_uuid()"), primary_key: true + end + + alter table(:users) do + modify :role_id, + references(:roles, + column: :id, + name: "users_role_id_fkey", + type: :uuid, + prefix: "public" + ) + end + + alter table(:roles) do + add :name, :text, null: false + add :description, :text + add :permission_set_name, :text, null: false + add :is_system_role, :boolean, null: false, default: false + + add :inserted_at, :utc_datetime_usec, + null: false, + default: fragment("(now() AT TIME ZONE 'utc')") + + add :updated_at, :utc_datetime_usec, + null: false, + default: fragment("(now() AT TIME ZONE 'utc')") + end + + create unique_index(:roles, [:name], name: "roles_unique_name_index") + + create index(:roles, [:permission_set_name], name: "roles_permission_set_name_index") + + create index(:users, [:role_id], name: "users_role_id_index") + end + + def down do + drop_if_exists index(:users, [:role_id], name: "users_role_id_index") + + drop_if_exists index(:roles, [:permission_set_name], name: "roles_permission_set_name_index") + + drop_if_exists unique_index(:roles, [:name], name: "roles_unique_name_index") + + alter table(:roles) do + remove :updated_at + remove :inserted_at + remove :is_system_role + remove :permission_set_name + remove :description + remove :name + end + + drop constraint(:users, "users_role_id_fkey") + + alter table(:users) do + modify :role_id, :uuid + end + + drop table(:roles) + + alter table(:users) do + remove :role_id + end + end +end From b569612a63687b9a1ffbf16f061771e336998f3d Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 17:18:45 +0100 Subject: [PATCH 06/24] feat: add resource snapshots for roles and users Add Ash resource snapshots generated during migration creation. --- .../repo/roles/20260106161215.json | 118 ++++++++++++ .../repo/users/20260106161215.json | 172 ++++++++++++++++++ 2 files changed, 290 insertions(+) create mode 100644 priv/resource_snapshots/repo/roles/20260106161215.json create mode 100644 priv/resource_snapshots/repo/users/20260106161215.json diff --git a/priv/resource_snapshots/repo/roles/20260106161215.json b/priv/resource_snapshots/repo/roles/20260106161215.json new file mode 100644 index 0000000..78c5636 --- /dev/null +++ b/priv/resource_snapshots/repo/roles/20260106161215.json @@ -0,0 +1,118 @@ +{ + "attributes": [ + { + "allow_nil?": false, + "default": "fragment(\"gen_random_uuid()\")", + "generated?": false, + "precision": null, + "primary_key?": true, + "references": null, + "scale": null, + "size": null, + "source": "id", + "type": "uuid" + }, + { + "allow_nil?": false, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "name", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "description", + "type": "text" + }, + { + "allow_nil?": false, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "permission_set_name", + "type": "text" + }, + { + "allow_nil?": false, + "default": "false", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "is_system_role", + "type": "boolean" + }, + { + "allow_nil?": false, + "default": "fragment(\"(now() AT TIME ZONE 'utc')\")", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "inserted_at", + "type": "utc_datetime_usec" + }, + { + "allow_nil?": false, + "default": "fragment(\"(now() AT TIME ZONE 'utc')\")", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "updated_at", + "type": "utc_datetime_usec" + } + ], + "base_filter": null, + "check_constraints": [], + "custom_indexes": [], + "custom_statements": [], + "has_create_action": true, + "hash": "FFDA74F44B5F11381D4C1F4DACA54901A1E02C3D181A88484AEED4E1ADA21B87", + "identities": [ + { + "all_tenants?": false, + "base_filter": null, + "index_name": "roles_unique_name_index", + "keys": [ + { + "type": "atom", + "value": "name" + } + ], + "name": "unique_name", + "nils_distinct?": true, + "where": null + } + ], + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "repo": "Elixir.Mv.Repo", + "schema": null, + "table": "roles" +} \ No newline at end of file diff --git a/priv/resource_snapshots/repo/users/20260106161215.json b/priv/resource_snapshots/repo/users/20260106161215.json new file mode 100644 index 0000000..886e1a1 --- /dev/null +++ b/priv/resource_snapshots/repo/users/20260106161215.json @@ -0,0 +1,172 @@ +{ + "attributes": [ + { + "allow_nil?": false, + "default": "fragment(\"gen_random_uuid()\")", + "generated?": false, + "precision": null, + "primary_key?": true, + "references": null, + "scale": null, + "size": null, + "source": "id", + "type": "uuid" + }, + { + "allow_nil?": false, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "email", + "type": "citext" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "hashed_password", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "oidc_id", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": { + "deferrable": false, + "destination_attribute": "id", + "destination_attribute_default": null, + "destination_attribute_generated": null, + "index?": false, + "match_type": null, + "match_with": null, + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "name": "users_member_id_fkey", + "on_delete": "nilify", + "on_update": null, + "primary_key?": true, + "schema": "public", + "table": "members" + }, + "scale": null, + "size": null, + "source": "member_id", + "type": "uuid" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": { + "deferrable": false, + "destination_attribute": "id", + "destination_attribute_default": null, + "destination_attribute_generated": null, + "index?": false, + "match_type": null, + "match_with": null, + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "name": "users_role_id_fkey", + "on_delete": null, + "on_update": null, + "primary_key?": true, + "schema": "public", + "table": "roles" + }, + "scale": null, + "size": null, + "source": "role_id", + "type": "uuid" + } + ], + "base_filter": null, + "check_constraints": [], + "custom_indexes": [], + "custom_statements": [], + "has_create_action": true, + "hash": "E381FA10CFC1D8D4CCD09AC1AD4B0CC9F8931436F22139CCF3A4558E84C422D3", + "identities": [ + { + "all_tenants?": false, + "base_filter": null, + "index_name": "users_unique_email_index", + "keys": [ + { + "type": "atom", + "value": "email" + } + ], + "name": "unique_email", + "nils_distinct?": true, + "where": null + }, + { + "all_tenants?": false, + "base_filter": null, + "index_name": "users_unique_member_index", + "keys": [ + { + "type": "atom", + "value": "member_id" + } + ], + "name": "unique_member", + "nils_distinct?": true, + "where": null + }, + { + "all_tenants?": false, + "base_filter": null, + "index_name": "users_unique_oidc_id_index", + "keys": [ + { + "type": "atom", + "value": "oidc_id" + } + ], + "name": "unique_oidc_id", + "nils_distinct?": true, + "where": null + } + ], + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "repo": "Elixir.Mv.Repo", + "schema": null, + "table": "users" +} \ No newline at end of file From 82ec4e565a209e6a66974898342d08f1a332715e Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 18:14:16 +0100 Subject: [PATCH 07/24] refactor: use UUIDv7 and improve Role validations - Change id from uuid_primary_key to uuid_v7_primary_key - Replace custom validation with built-in one_of validation - Add explicit on_delete: :restrict for users foreign key - Update postgres references configuration --- lib/mv/authorization/role.ex | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/lib/mv/authorization/role.ex b/lib/mv/authorization/role.ex index e5b9795..3397172 100644 --- a/lib/mv/authorization/role.ex +++ b/lib/mv/authorization/role.ex @@ -42,6 +42,11 @@ defmodule Mv.Authorization.Role do postgres do table "roles" repo Mv.Repo + + references do + # Prevent deletion of roles that are assigned to users + reference :users, on_delete: :restrict + end end code_interface do @@ -75,27 +80,12 @@ defmodule Mv.Authorization.Role do end validations do - validate fn changeset, _context -> - permission_set_name = Ash.Changeset.get_attribute(changeset, :permission_set_name) - - if permission_set_name do - valid_sets = - Mv.Authorization.PermissionSets.all_permission_sets() - |> Enum.map(&Atom.to_string/1) - - if permission_set_name in valid_sets do - :ok - else - valid_sets_string = Enum.join(valid_sets, ", ") - - {:error, - field: :permission_set_name, - message: "Invalid permission set name. Must be one of: #{valid_sets_string}"} - end - else - :ok - end - end + validate one_of( + :permission_set_name, + Mv.Authorization.PermissionSets.all_permission_sets() + |> Enum.map(&Atom.to_string/1) + ), + message: "must be one of: own_data, read_only, normal_user, admin" validate fn changeset, _context -> if changeset.action_type == :destroy do @@ -114,7 +104,7 @@ defmodule Mv.Authorization.Role do end attributes do - uuid_primary_key :id + uuid_v7_primary_key :id attribute :name, :string do allow_nil? false From 402a78dd0a363b9b7db5870e0b079b54bdc4ccbe Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 18:14:18 +0100 Subject: [PATCH 08/24] refactor: update migration for UUIDv7 and explicit FK constraint - Add on_delete: :restrict to users.role_id foreign key - Update roles.id to use uuid_generate_v7() default - Regenerate resource snapshots --- ...0260106161215_add_authorization_domain.exs | 1 + .../20260106165250_update_role_to_uuidv7.exs | 21 ++++ .../repo/roles/20260106165250.json | 118 ++++++++++++++++++ 3 files changed, 140 insertions(+) create mode 100644 priv/repo/migrations/20260106165250_update_role_to_uuidv7.exs create mode 100644 priv/resource_snapshots/repo/roles/20260106165250.json diff --git a/priv/repo/migrations/20260106161215_add_authorization_domain.exs b/priv/repo/migrations/20260106161215_add_authorization_domain.exs index 02edcd3..445fd19 100644 --- a/priv/repo/migrations/20260106161215_add_authorization_domain.exs +++ b/priv/repo/migrations/20260106161215_add_authorization_domain.exs @@ -22,6 +22,7 @@ defmodule Mv.Repo.Migrations.AddAuthorizationDomain do column: :id, name: "users_role_id_fkey", type: :uuid, + on_delete: :restrict, prefix: "public" ) end diff --git a/priv/repo/migrations/20260106165250_update_role_to_uuidv7.exs b/priv/repo/migrations/20260106165250_update_role_to_uuidv7.exs new file mode 100644 index 0000000..9be7534 --- /dev/null +++ b/priv/repo/migrations/20260106165250_update_role_to_uuidv7.exs @@ -0,0 +1,21 @@ +defmodule Mv.Repo.Migrations.UpdateRoleToUuidv7 do + @moduledoc """ + Updates resources based on their most recent snapshots. + + This file was autogenerated with `mix ash_postgres.generate_migrations` + """ + + use Ecto.Migration + + def up do + alter table(:roles) do + modify :id, :uuid, default: fragment("uuid_generate_v7()") + end + end + + def down do + alter table(:roles) do + modify :id, :uuid, default: fragment("gen_random_uuid()") + end + end +end diff --git a/priv/resource_snapshots/repo/roles/20260106165250.json b/priv/resource_snapshots/repo/roles/20260106165250.json new file mode 100644 index 0000000..56fedf5 --- /dev/null +++ b/priv/resource_snapshots/repo/roles/20260106165250.json @@ -0,0 +1,118 @@ +{ + "attributes": [ + { + "allow_nil?": false, + "default": "fragment(\"uuid_generate_v7()\")", + "generated?": false, + "precision": null, + "primary_key?": true, + "references": null, + "scale": null, + "size": null, + "source": "id", + "type": "uuid" + }, + { + "allow_nil?": false, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "name", + "type": "text" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "description", + "type": "text" + }, + { + "allow_nil?": false, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "permission_set_name", + "type": "text" + }, + { + "allow_nil?": false, + "default": "false", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "is_system_role", + "type": "boolean" + }, + { + "allow_nil?": false, + "default": "fragment(\"(now() AT TIME ZONE 'utc')\")", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "inserted_at", + "type": "utc_datetime_usec" + }, + { + "allow_nil?": false, + "default": "fragment(\"(now() AT TIME ZONE 'utc')\")", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "updated_at", + "type": "utc_datetime_usec" + } + ], + "base_filter": null, + "check_constraints": [], + "custom_indexes": [], + "custom_statements": [], + "has_create_action": true, + "hash": "8822483B2830DB45988E3B673F36EAE43311B336EE34FBDA1FA24BF9867D7494", + "identities": [ + { + "all_tenants?": false, + "base_filter": null, + "index_name": "roles_unique_name_index", + "keys": [ + { + "type": "atom", + "value": "name" + } + ], + "name": "unique_name", + "nils_distinct?": true, + "where": null + } + ], + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "repo": "Elixir.Mv.Repo", + "schema": null, + "table": "roles" +} \ No newline at end of file From 12c08cabee79538f3737325f11e95c71de720924 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 18:14:19 +0100 Subject: [PATCH 09/24] docs: clean up PermissionSets documentation Remove issue number references from moduledoc --- lib/mv/authorization/permission_sets.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mv/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex index fb9d249..d01e285 100644 --- a/lib/mv/authorization/permission_sets.ex +++ b/lib/mv/authorization/permission_sets.ex @@ -2,8 +2,8 @@ defmodule Mv.Authorization.PermissionSets do @moduledoc """ Defines the four hardcoded permission sets for the application. - This is a minimal stub implementation for Issue #1. The full implementation - with all permission details will be added in Issue #2. + This is a minimal stub implementation. The full implementation + with all permission details will be added in a subsequent issue. ## Permission Sets From 9bb0fe5e375cb8e0a9d985af9a548b622c1be694 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 18:14:20 +0100 Subject: [PATCH 10/24] test: add unit tests for Role validations Add tests for permission_set_name validation, system role deletion protection, and name uniqueness constraints. --- test/mv/authorization/role_test.exs | 93 +++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 test/mv/authorization/role_test.exs diff --git a/test/mv/authorization/role_test.exs b/test/mv/authorization/role_test.exs new file mode 100644 index 0000000..ab1ebeb --- /dev/null +++ b/test/mv/authorization/role_test.exs @@ -0,0 +1,93 @@ +defmodule Mv.Authorization.RoleTest do + @moduledoc """ + Unit tests for Role resource validations and constraints. + """ + use Mv.DataCase, async: true + + alias Mv.Authorization + + describe "permission_set_name validation" do + test "accepts valid permission set names" do + attrs = %{ + name: "Test Role", + permission_set_name: "own_data" + } + + assert {:ok, role} = Authorization.create_role(attrs) + assert role.permission_set_name == "own_data" + end + + test "rejects invalid permission set names" do + attrs = %{ + name: "Test Role", + permission_set_name: "invalid_set" + } + + assert {:error, %Ash.Error.Invalid{errors: errors}} = Authorization.create_role(attrs) + assert error_message(errors, :permission_set_name) =~ "must be one of" + end + + test "accepts all four valid permission sets" do + valid_sets = ["own_data", "read_only", "normal_user", "admin"] + + for permission_set <- valid_sets do + attrs = %{ + name: "Role #{permission_set}", + permission_set_name: permission_set + } + + assert {:ok, _role} = Authorization.create_role(attrs) + end + end + end + + describe "system role deletion protection" do + test "prevents deletion of system roles" do + {:ok, system_role} = + Authorization.create_role(%{ + name: "System Role", + permission_set_name: "own_data", + is_system_role: true + }) + + assert {:error, %Ash.Error.Invalid{errors: errors}} = + Authorization.destroy_role(system_role) + + message = error_message(errors, nil) + assert message =~ "Cannot delete system role" + end + + test "allows deletion of non-system roles" do + {:ok, regular_role} = + Authorization.create_role(%{ + name: "Regular Role", + permission_set_name: "read_only", + is_system_role: false + }) + + assert :ok = Authorization.destroy_role(regular_role) + end + end + + describe "name uniqueness" do + test "enforces unique role names" do + attrs = %{ + name: "Unique Role", + permission_set_name: "own_data" + } + + assert {:ok, _} = Authorization.create_role(attrs) + + assert {:error, %Ash.Error.Invalid{errors: errors}} = Authorization.create_role(attrs) + assert error_message(errors, :name) =~ "has already been taken" + end + end + + # Helper function for error evaluation + defp error_message(errors, field) do + errors + |> Enum.filter(fn err -> Map.get(err, :field) == field end) + |> Enum.map(&Map.get(&1, :message, "")) + |> List.first() || "" + end +end From 557eb4d27d5865feb15663156c7c70cb30268596 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 18:37:34 +0100 Subject: [PATCH 11/24] refactor: simplify system role deletion validation Remove redundant action_type check since validation already runs only on destroy actions. Add field to error for better error handling. --- lib/mv/authorization/role.ex | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/mv/authorization/role.ex b/lib/mv/authorization/role.ex index 3397172..ff1f2c1 100644 --- a/lib/mv/authorization/role.ex +++ b/lib/mv/authorization/role.ex @@ -88,14 +88,11 @@ defmodule Mv.Authorization.Role do message: "must be one of: own_data, read_only, normal_user, admin" validate fn changeset, _context -> - if changeset.action_type == :destroy do - if changeset.data.is_system_role do - {:error, - message: - "Cannot delete system role. System roles are required for the application to function."} - else - :ok - end + if changeset.data.is_system_role do + {:error, + field: :is_system_role, + message: + "Cannot delete system role. System roles are required for the application to function."} else :ok end From f63405052fb1ceba40aaeae16f970b93197dbfb6 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 18:37:35 +0100 Subject: [PATCH 12/24] feat: add get_role action to Authorization domain Add get_role action for retrieving single role by ID through code interface. --- lib/mv/authorization/authorization.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/mv/authorization/authorization.ex b/lib/mv/authorization/authorization.ex index 23672f1..aac07a9 100644 --- a/lib/mv/authorization/authorization.ex +++ b/lib/mv/authorization/authorization.ex @@ -23,6 +23,7 @@ defmodule Mv.Authorization do resource Mv.Authorization.Role do define :create_role, action: :create_role define :list_roles, action: :read + define :get_role, action: :read, get_by: [:id] define :update_role, action: :update_role define :destroy_role, action: :destroy end From deacc43030290b4d737a11f48f136f8201512d22 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 18:37:37 +0100 Subject: [PATCH 13/24] docs: document FK constraint behavior for role relationship Add comment explaining on_delete: :restrict behavior for users.role_id foreign key constraint. --- lib/accounts/user.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex index b0d919b..655dcc6 100644 --- a/lib/accounts/user.ex +++ b/lib/accounts/user.ex @@ -361,6 +361,7 @@ defmodule Mv.Accounts.User do # 1:1 relationship - User belongs to a Role # This automatically creates a `role_id` attribute in the User table # The relationship is optional (allow_nil? true by default) + # Foreign key constraint: on_delete: :restrict (prevents deleting roles assigned to users) belongs_to :role, Mv.Authorization.Role end From c6a766377a55473cf597e326e7777cb937acd99c Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 18:37:38 +0100 Subject: [PATCH 14/24] refactor: improve error_message test helper Add pattern matching for nil field case to handle errors without specific field (e.g., system role deletion). --- test/mv/authorization/role_test.exs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/mv/authorization/role_test.exs b/test/mv/authorization/role_test.exs index ab1ebeb..effa000 100644 --- a/test/mv/authorization/role_test.exs +++ b/test/mv/authorization/role_test.exs @@ -84,6 +84,13 @@ defmodule Mv.Authorization.RoleTest do end # Helper function for error evaluation + # When field is nil, returns first error message (for errors without specific field) + defp error_message(errors, field) when is_nil(field) do + errors + |> Enum.map(&Map.get(&1, :message, "")) + |> List.first() || "" + end + defp error_message(errors, field) do errors |> Enum.filter(fn err -> Map.get(err, :field) == field end) From ce1d5790a37ef2bd678283eb8097ce84611996aa Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 18:37:39 +0100 Subject: [PATCH 15/24] refactor: squash migrations into single authorization domain migration Combine initial authorization migration with UUIDv7 update into one migration. Migration now creates roles table with UUIDv7 default and explicit on_delete: :restrict FK constraint. --- ...0260106161215_add_authorization_domain.exs | 2 +- .../20260106165250_update_role_to_uuidv7.exs | 21 ------------------- 2 files changed, 1 insertion(+), 22 deletions(-) delete mode 100644 priv/repo/migrations/20260106165250_update_role_to_uuidv7.exs diff --git a/priv/repo/migrations/20260106161215_add_authorization_domain.exs b/priv/repo/migrations/20260106161215_add_authorization_domain.exs index 445fd19..7631043 100644 --- a/priv/repo/migrations/20260106161215_add_authorization_domain.exs +++ b/priv/repo/migrations/20260106161215_add_authorization_domain.exs @@ -13,7 +13,7 @@ defmodule Mv.Repo.Migrations.AddAuthorizationDomain do end create table(:roles, primary_key: false) do - add :id, :uuid, null: false, default: fragment("gen_random_uuid()"), primary_key: true + add :id, :uuid, null: false, default: fragment("uuid_generate_v7()"), primary_key: true end alter table(:users) do diff --git a/priv/repo/migrations/20260106165250_update_role_to_uuidv7.exs b/priv/repo/migrations/20260106165250_update_role_to_uuidv7.exs deleted file mode 100644 index 9be7534..0000000 --- a/priv/repo/migrations/20260106165250_update_role_to_uuidv7.exs +++ /dev/null @@ -1,21 +0,0 @@ -defmodule Mv.Repo.Migrations.UpdateRoleToUuidv7 do - @moduledoc """ - Updates resources based on their most recent snapshots. - - This file was autogenerated with `mix ash_postgres.generate_migrations` - """ - - use Ecto.Migration - - def up do - alter table(:roles) do - modify :id, :uuid, default: fragment("uuid_generate_v7()") - end - end - - def down do - alter table(:roles) do - modify :id, :uuid, default: fragment("gen_random_uuid()") - end - end -end From 73763b1f584d976c8a42b13001d2d9e2b308b514 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 18:44:04 +0100 Subject: [PATCH 16/24] refactor: improve error_message test helper robustness Use Enum.reject for nil field case to explicitly filter errors without field. Update test to use :is_system_role field since validation error includes field. --- test/mv/authorization/role_test.exs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/mv/authorization/role_test.exs b/test/mv/authorization/role_test.exs index effa000..be297f2 100644 --- a/test/mv/authorization/role_test.exs +++ b/test/mv/authorization/role_test.exs @@ -53,7 +53,7 @@ defmodule Mv.Authorization.RoleTest do assert {:error, %Ash.Error.Invalid{errors: errors}} = Authorization.destroy_role(system_role) - message = error_message(errors, nil) + message = error_message(errors, :is_system_role) assert message =~ "Cannot delete system role" end @@ -84,14 +84,15 @@ defmodule Mv.Authorization.RoleTest do end # Helper function for error evaluation - # When field is nil, returns first error message (for errors without specific field) - defp error_message(errors, field) when is_nil(field) do + # When field is nil, returns first error message for errors without specific field + defp error_message(errors, nil) do errors + |> Enum.reject(fn err -> Map.has_key?(err, :field) end) |> Enum.map(&Map.get(&1, :message, "")) |> List.first() || "" end - defp error_message(errors, field) do + defp error_message(errors, field) when is_atom(field) do errors |> Enum.filter(fn err -> Map.get(err, :field) == field end) |> Enum.map(&Map.get(&1, :message, "")) From 5f13901ca59835e557694c344fde7eaf405f24dc Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 19:04:03 +0100 Subject: [PATCH 17/24] security: remove is_system_role from public API Remove is_system_role from accept lists in create_role and update_role actions. This field should only be set via seeds or internal actions to prevent users from creating unkillable roles through the public API. --- lib/accounts/user.ex | 4 + lib/mv/authorization/role.ex | 9 +- .../repo/roles/20260106161215.json | 118 ------------------ .../repo/users/20260106161215.json | 2 +- 4 files changed, 11 insertions(+), 122 deletions(-) delete mode 100644 priv/resource_snapshots/repo/roles/20260106161215.json diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex index 655dcc6..ceedeae 100644 --- a/lib/accounts/user.ex +++ b/lib/accounts/user.ex @@ -17,6 +17,10 @@ defmodule Mv.Accounts.User do # When a member is deleted, set the user's member_id to NULL # This allows users to continue existing even if their linked member is removed reference :member, on_delete: :nilify + + # When a role is deleted, prevent deletion if users are assigned to it + # This protects critical roles from accidental deletion + reference :role, on_delete: :restrict end end diff --git a/lib/mv/authorization/role.ex b/lib/mv/authorization/role.ex index ff1f2c1..da43510 100644 --- a/lib/mv/authorization/role.ex +++ b/lib/mv/authorization/role.ex @@ -61,14 +61,16 @@ defmodule Mv.Authorization.Role do create :create_role do primary? true - accept [:name, :description, :permission_set_name, :is_system_role] + # is_system_role is intentionally excluded - should only be set via seeds/internal actions + accept [:name, :description, :permission_set_name] # Note: In Ash 3.0, require_atomic? is not available for create actions # Custom validations will still work end update :update_role do primary? true - accept [:name, :description, :permission_set_name, :is_system_role] + # is_system_role is intentionally excluded - should only be set via seeds/internal actions + accept [:name, :description, :permission_set_name] # Required because custom validation functions cannot be executed atomically require_atomic? false end @@ -85,7 +87,8 @@ defmodule Mv.Authorization.Role do Mv.Authorization.PermissionSets.all_permission_sets() |> Enum.map(&Atom.to_string/1) ), - message: "must be one of: own_data, read_only, normal_user, admin" + message: + "must be one of: #{Mv.Authorization.PermissionSets.all_permission_sets() |> Enum.map_join(", ", &Atom.to_string/1)}" validate fn changeset, _context -> if changeset.data.is_system_role do diff --git a/priv/resource_snapshots/repo/roles/20260106161215.json b/priv/resource_snapshots/repo/roles/20260106161215.json deleted file mode 100644 index 78c5636..0000000 --- a/priv/resource_snapshots/repo/roles/20260106161215.json +++ /dev/null @@ -1,118 +0,0 @@ -{ - "attributes": [ - { - "allow_nil?": false, - "default": "fragment(\"gen_random_uuid()\")", - "generated?": false, - "precision": null, - "primary_key?": true, - "references": null, - "scale": null, - "size": null, - "source": "id", - "type": "uuid" - }, - { - "allow_nil?": false, - "default": "nil", - "generated?": false, - "precision": null, - "primary_key?": false, - "references": null, - "scale": null, - "size": null, - "source": "name", - "type": "text" - }, - { - "allow_nil?": true, - "default": "nil", - "generated?": false, - "precision": null, - "primary_key?": false, - "references": null, - "scale": null, - "size": null, - "source": "description", - "type": "text" - }, - { - "allow_nil?": false, - "default": "nil", - "generated?": false, - "precision": null, - "primary_key?": false, - "references": null, - "scale": null, - "size": null, - "source": "permission_set_name", - "type": "text" - }, - { - "allow_nil?": false, - "default": "false", - "generated?": false, - "precision": null, - "primary_key?": false, - "references": null, - "scale": null, - "size": null, - "source": "is_system_role", - "type": "boolean" - }, - { - "allow_nil?": false, - "default": "fragment(\"(now() AT TIME ZONE 'utc')\")", - "generated?": false, - "precision": null, - "primary_key?": false, - "references": null, - "scale": null, - "size": null, - "source": "inserted_at", - "type": "utc_datetime_usec" - }, - { - "allow_nil?": false, - "default": "fragment(\"(now() AT TIME ZONE 'utc')\")", - "generated?": false, - "precision": null, - "primary_key?": false, - "references": null, - "scale": null, - "size": null, - "source": "updated_at", - "type": "utc_datetime_usec" - } - ], - "base_filter": null, - "check_constraints": [], - "custom_indexes": [], - "custom_statements": [], - "has_create_action": true, - "hash": "FFDA74F44B5F11381D4C1F4DACA54901A1E02C3D181A88484AEED4E1ADA21B87", - "identities": [ - { - "all_tenants?": false, - "base_filter": null, - "index_name": "roles_unique_name_index", - "keys": [ - { - "type": "atom", - "value": "name" - } - ], - "name": "unique_name", - "nils_distinct?": true, - "where": null - } - ], - "multitenancy": { - "attribute": null, - "global": null, - "strategy": null - }, - "repo": "Elixir.Mv.Repo", - "schema": null, - "table": "roles" -} \ No newline at end of file diff --git a/priv/resource_snapshots/repo/users/20260106161215.json b/priv/resource_snapshots/repo/users/20260106161215.json index 886e1a1..3fcf712 100644 --- a/priv/resource_snapshots/repo/users/20260106161215.json +++ b/priv/resource_snapshots/repo/users/20260106161215.json @@ -99,7 +99,7 @@ "strategy": null }, "name": "users_role_id_fkey", - "on_delete": null, + "on_delete": "restrict", "on_update": null, "primary_key?": true, "schema": "public", From 3265468bd61e459dd3569fbee36bc5f8ea853fdf Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 19:04:05 +0100 Subject: [PATCH 18/24] test: update role tests for is_system_role API change Use Ash.Changeset.force_change_attribute to set is_system_role in tests since it's no longer settable via public API. Remove unused nil clause from error_message helper. --- test/mv/authorization/role_test.exs | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/test/mv/authorization/role_test.exs b/test/mv/authorization/role_test.exs index be297f2..b263455 100644 --- a/test/mv/authorization/role_test.exs +++ b/test/mv/authorization/role_test.exs @@ -43,12 +43,16 @@ defmodule Mv.Authorization.RoleTest do describe "system role deletion protection" do test "prevents deletion of system roles" do - {:ok, system_role} = - Authorization.create_role(%{ + # is_system_role is not settable via public API, so we use Ash.Changeset directly + changeset = + Mv.Authorization.Role + |> Ash.Changeset.for_create(:create_role, %{ name: "System Role", - permission_set_name: "own_data", - is_system_role: true + permission_set_name: "own_data" }) + |> Ash.Changeset.force_change_attribute(:is_system_role, true) + + {:ok, system_role} = Ash.create(changeset) assert {:error, %Ash.Error.Invalid{errors: errors}} = Authorization.destroy_role(system_role) @@ -58,11 +62,11 @@ defmodule Mv.Authorization.RoleTest do end test "allows deletion of non-system roles" do + # is_system_role defaults to false, so regular create works {:ok, regular_role} = Authorization.create_role(%{ name: "Regular Role", - permission_set_name: "read_only", - is_system_role: false + permission_set_name: "read_only" }) assert :ok = Authorization.destroy_role(regular_role) @@ -84,14 +88,6 @@ defmodule Mv.Authorization.RoleTest do end # Helper function for error evaluation - # When field is nil, returns first error message for errors without specific field - defp error_message(errors, nil) do - errors - |> Enum.reject(fn err -> Map.has_key?(err, :field) end) - |> Enum.map(&Map.get(&1, :message, "")) - |> List.first() || "" - end - defp error_message(errors, field) when is_atom(field) do errors |> Enum.filter(fn err -> Map.get(err, :field) == field end) From 3a0fb4e84f626695f91d4cb9127b6750c7832a3a Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 19:50:00 +0100 Subject: [PATCH 19/24] feat: implement PermissionSets module with all 4 permission sets - Add types for scope, action, resource_permission, permission_set - Implement get_permissions/1 for all 4 sets (own_data, read_only, normal_user, admin) - Implement valid_permission_set?/1 for string and atom validation - Implement permission_set_name_to_atom/1 with error handling --- lib/mv/authorization/permission_sets.ex | 250 +++++++- .../mv/authorization/permission_sets_test.exs | 568 ++++++++++++++++++ 2 files changed, 813 insertions(+), 5 deletions(-) create mode 100644 test/mv/authorization/permission_sets_test.exs diff --git a/lib/mv/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex index d01e285..22b1648 100644 --- a/lib/mv/authorization/permission_sets.ex +++ b/lib/mv/authorization/permission_sets.ex @@ -2,23 +2,60 @@ defmodule Mv.Authorization.PermissionSets do @moduledoc """ Defines the four hardcoded permission sets for the application. - This is a minimal stub implementation. The full implementation - with all permission details will be added in a subsequent issue. + Each permission set specifies: + - Resource permissions (what CRUD operations on which resources) + - Page permissions (which LiveView pages can be accessed) + - Scopes (own, linked, all) ## Permission Sets 1. **own_data** - Default for "Mitglied" role + - Can only access own user data and linked member/custom field values + - Cannot create new members or manage system + 2. **read_only** - For "Vorstand" and "Buchhaltung" roles + - Can read all member data + - Cannot create, update, or delete + 3. **normal_user** - For "Kassenwart" role + - Create/Read/Update members (no delete for safety), full CRUD on custom field values + - Cannot manage custom fields or users + 4. **admin** - For "Admin" role + - Unrestricted access to all resources + - Can manage users, roles, custom fields ## Usage - # Get list of all valid permission set names - PermissionSets.all_permission_sets() - # => [:own_data, :read_only, :normal_user, :admin] + # Get permissions for a role's permission set + permissions = PermissionSets.get_permissions(:admin) + + # Check if a permission set name is valid + PermissionSets.valid_permission_set?("read_only") # => true + + # Convert string to atom safely + {:ok, atom} = PermissionSets.permission_set_name_to_atom("own_data") + + ## Performance + + All functions are pure and compile-time. Permission lookups are < 1 microsecond. """ + @type scope :: :own | :linked | :all + @type action :: :read | :create | :update | :destroy + + @type resource_permission :: %{ + resource: String.t(), + action: action(), + scope: scope(), + granted: boolean() + } + + @type permission_set :: %{ + resources: [resource_permission()], + pages: [String.t()] + } + @doc """ Returns the list of all valid permission set names. @@ -31,4 +68,207 @@ defmodule Mv.Authorization.PermissionSets do def all_permission_sets do [:own_data, :read_only, :normal_user, :admin] end + + @doc """ + Returns permissions for the given permission set. + + ## Examples + + iex> permissions = PermissionSets.get_permissions(:admin) + iex> Enum.any?(permissions.resources, fn p -> + ...> p.resource == "User" and p.action == :destroy + ...> end) + true + + iex> PermissionSets.get_permissions(:invalid) + ** (FunctionClauseError) no function clause matching + """ + @spec get_permissions(atom()) :: permission_set() + + def get_permissions(:own_data) do + %{ + resources: [ + # User: Can always read/update own credentials + %{resource: "User", action: :read, scope: :own, granted: true}, + %{resource: "User", action: :update, scope: :own, granted: true}, + + # Member: Can read/update linked member + %{resource: "Member", action: :read, scope: :linked, granted: true}, + %{resource: "Member", action: :update, scope: :linked, granted: true}, + + # CustomFieldValue: Can read/update custom field values of linked member + %{resource: "CustomFieldValue", action: :read, scope: :linked, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :linked, granted: true}, + + # CustomField: Can read all (needed for forms) + %{resource: "CustomField", action: :read, scope: :all, granted: true} + ], + pages: [ + # Home page + "/", + # Own profile + "/profile", + # Linked member detail (filtered by policy) + "/members/:id" + ] + } + end + + def get_permissions(:read_only) do + %{ + resources: [ + # User: Can read/update own credentials only + %{resource: "User", action: :read, scope: :own, granted: true}, + %{resource: "User", action: :update, scope: :own, granted: true}, + + # Member: Can read all members, no modifications + %{resource: "Member", action: :read, scope: :all, granted: true}, + + # CustomFieldValue: Can read all custom field values + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, + + # CustomField: Can read all + %{resource: "CustomField", action: :read, scope: :all, granted: true} + ], + pages: [ + "/", + # Member list + "/members", + # Member detail + "/members/:id", + # Custom field values overview + "/custom_field_values" + ] + } + end + + def get_permissions(:normal_user) do + %{ + resources: [ + # User: Can read/update own credentials only + %{resource: "User", action: :read, scope: :own, granted: true}, + %{resource: "User", action: :update, scope: :own, granted: true}, + + # Member: Full CRUD except destroy (safety) + %{resource: "Member", action: :read, scope: :all, granted: true}, + %{resource: "Member", action: :create, scope: :all, granted: true}, + %{resource: "Member", action: :update, scope: :all, granted: true}, + # Note: destroy intentionally omitted for safety + + # CustomFieldValue: Full CRUD + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true}, + + # CustomField: Read only (admin manages definitions) + %{resource: "CustomField", action: :read, scope: :all, granted: true} + ], + pages: [ + "/", + "/members", + # Create member + "/members/new", + "/members/:id", + # Edit member + "/members/:id/edit", + "/custom_field_values", + "/custom_field_values/new", + "/custom_field_values/:id/edit" + ] + } + end + + def get_permissions(:admin) do + %{ + resources: [ + # User: Full management including other users + %{resource: "User", action: :read, scope: :all, granted: true}, + %{resource: "User", action: :create, scope: :all, granted: true}, + %{resource: "User", action: :update, scope: :all, granted: true}, + %{resource: "User", action: :destroy, scope: :all, granted: true}, + + # Member: Full CRUD + %{resource: "Member", action: :read, scope: :all, granted: true}, + %{resource: "Member", action: :create, scope: :all, granted: true}, + %{resource: "Member", action: :update, scope: :all, granted: true}, + %{resource: "Member", action: :destroy, scope: :all, granted: true}, + + # CustomFieldValue: Full CRUD + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true}, + + # CustomField: Full CRUD (admin manages custom field definitions) + %{resource: "CustomField", action: :read, scope: :all, granted: true}, + %{resource: "CustomField", action: :create, scope: :all, granted: true}, + %{resource: "CustomField", action: :update, scope: :all, granted: true}, + %{resource: "CustomField", action: :destroy, scope: :all, granted: true}, + + # Role: Full CRUD (admin manages roles) + %{resource: "Role", action: :read, scope: :all, granted: true}, + %{resource: "Role", action: :create, scope: :all, granted: true}, + %{resource: "Role", action: :update, scope: :all, granted: true}, + %{resource: "Role", action: :destroy, scope: :all, granted: true} + ], + pages: [ + # Wildcard: Admin can access all pages + "*" + ] + } + end + + @doc """ + Checks if a permission set name (string or atom) is valid. + + ## Examples + + iex> PermissionSets.valid_permission_set?("admin") + true + + iex> PermissionSets.valid_permission_set?(:read_only) + true + + iex> PermissionSets.valid_permission_set?("invalid") + false + """ + @spec valid_permission_set?(String.t() | atom()) :: boolean() + def valid_permission_set?(name) when is_binary(name) do + case permission_set_name_to_atom(name) do + {:ok, _atom} -> true + {:error, _} -> false + end + end + + def valid_permission_set?(name) when is_atom(name) do + name in all_permission_sets() + end + + def valid_permission_set?(_), do: false + + @doc """ + Converts a permission set name string to atom safely. + + ## Examples + + iex> PermissionSets.permission_set_name_to_atom("admin") + {:ok, :admin} + + iex> PermissionSets.permission_set_name_to_atom("invalid") + {:error, :invalid_permission_set} + """ + @spec permission_set_name_to_atom(String.t()) :: + {:ok, atom()} | {:error, :invalid_permission_set} + def permission_set_name_to_atom(name) when is_binary(name) do + atom = String.to_existing_atom(name) + + if valid_permission_set?(atom) do + {:ok, atom} + else + {:error, :invalid_permission_set} + end + rescue + ArgumentError -> {:error, :invalid_permission_set} + end end diff --git a/test/mv/authorization/permission_sets_test.exs b/test/mv/authorization/permission_sets_test.exs new file mode 100644 index 0000000..51dc797 --- /dev/null +++ b/test/mv/authorization/permission_sets_test.exs @@ -0,0 +1,568 @@ +defmodule Mv.Authorization.PermissionSetsTest do + @moduledoc """ + Tests for the PermissionSets module that defines hardcoded permission sets. + """ + use ExUnit.Case, async: true + + alias Mv.Authorization.PermissionSets + + describe "all_permission_sets/0" do + test "returns all four permission sets" do + sets = PermissionSets.all_permission_sets() + + assert length(sets) == 4 + assert :own_data in sets + assert :read_only in sets + assert :normal_user in sets + assert :admin in sets + end + end + + describe "get_permissions/1" do + test "returns map with :resources and :pages keys for :own_data" do + permissions = PermissionSets.get_permissions(:own_data) + + assert Map.has_key?(permissions, :resources) + assert Map.has_key?(permissions, :pages) + assert is_list(permissions.resources) + assert is_list(permissions.pages) + end + + test "returns map with :resources and :pages keys for :read_only" do + permissions = PermissionSets.get_permissions(:read_only) + + assert Map.has_key?(permissions, :resources) + assert Map.has_key?(permissions, :pages) + assert is_list(permissions.resources) + assert is_list(permissions.pages) + end + + test "returns map with :resources and :pages keys for :normal_user" do + permissions = PermissionSets.get_permissions(:normal_user) + + assert Map.has_key?(permissions, :resources) + assert Map.has_key?(permissions, :pages) + assert is_list(permissions.resources) + assert is_list(permissions.pages) + end + + test "returns map with :resources and :pages keys for :admin" do + permissions = PermissionSets.get_permissions(:admin) + + assert Map.has_key?(permissions, :resources) + assert Map.has_key?(permissions, :pages) + assert is_list(permissions.resources) + assert is_list(permissions.pages) + end + + test "each resource permission has required keys" do + permissions = PermissionSets.get_permissions(:own_data) + + Enum.each(permissions.resources, fn perm -> + assert Map.has_key?(perm, :resource) + assert Map.has_key?(perm, :action) + assert Map.has_key?(perm, :scope) + assert Map.has_key?(perm, :granted) + assert is_binary(perm.resource) + assert perm.action in [:read, :create, :update, :destroy] + assert perm.scope in [:own, :linked, :all] + assert is_boolean(perm.granted) + end) + end + + test "pages lists are non-empty for all permission sets" do + for set <- [:own_data, :read_only, :normal_user, :admin] do + permissions = PermissionSets.get_permissions(set) + + assert permissions.pages != [], + "Permission set #{set} should have at least one page" + end + end + end + + describe "get_permissions/1 - :own_data permission content" do + test "allows User read/update with scope :own" do + permissions = PermissionSets.get_permissions(:own_data) + + user_read = + Enum.find(permissions.resources, fn p -> p.resource == "User" && p.action == :read end) + + user_update = + Enum.find(permissions.resources, fn p -> p.resource == "User" && p.action == :update end) + + assert user_read.scope == :own + assert user_read.granted == true + assert user_update.scope == :own + assert user_update.granted == true + end + + test "allows Member read/update with scope :linked" do + permissions = PermissionSets.get_permissions(:own_data) + + member_read = + Enum.find(permissions.resources, fn p -> p.resource == "Member" && p.action == :read end) + + member_update = + Enum.find(permissions.resources, fn p -> p.resource == "Member" && p.action == :update end) + + assert member_read.scope == :linked + assert member_read.granted == true + assert member_update.scope == :linked + assert member_update.granted == true + end + + test "allows CustomFieldValue read/update with scope :linked" do + permissions = PermissionSets.get_permissions(:own_data) + + cfv_read = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomFieldValue" && p.action == :read + end) + + cfv_update = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomFieldValue" && p.action == :update + end) + + assert cfv_read.scope == :linked + assert cfv_read.granted == true + assert cfv_update.scope == :linked + assert cfv_update.granted == true + end + + test "allows CustomField read with scope :all" do + permissions = PermissionSets.get_permissions(:own_data) + + cf_read = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomField" && p.action == :read + end) + + assert cf_read.scope == :all + assert cf_read.granted == true + end + + test "includes correct pages" do + permissions = PermissionSets.get_permissions(:own_data) + + assert "/" in permissions.pages + assert "/profile" in permissions.pages + assert "/members/:id" in permissions.pages + end + end + + describe "get_permissions/1 - :read_only permission content" do + test "allows User read/update with scope :own" do + permissions = PermissionSets.get_permissions(:read_only) + + user_read = + Enum.find(permissions.resources, fn p -> p.resource == "User" && p.action == :read end) + + user_update = + Enum.find(permissions.resources, fn p -> p.resource == "User" && p.action == :update end) + + assert user_read.scope == :own + assert user_read.granted == true + assert user_update.scope == :own + assert user_update.granted == true + end + + test "allows Member read with scope :all" do + permissions = PermissionSets.get_permissions(:read_only) + + member_read = + Enum.find(permissions.resources, fn p -> p.resource == "Member" && p.action == :read end) + + assert member_read.scope == :all + assert member_read.granted == true + end + + test "does NOT allow Member create/update/destroy" do + permissions = PermissionSets.get_permissions(:read_only) + + member_create = + Enum.find(permissions.resources, fn p -> p.resource == "Member" && p.action == :create end) + + member_update = + Enum.find(permissions.resources, fn p -> p.resource == "Member" && p.action == :update end) + + member_destroy = + Enum.find(permissions.resources, fn p -> + p.resource == "Member" && p.action == :destroy + end) + + assert member_create == nil || member_create.granted == false + assert member_update == nil || member_update.granted == false + assert member_destroy == nil || member_destroy.granted == false + end + + test "allows CustomFieldValue read with scope :all" do + permissions = PermissionSets.get_permissions(:read_only) + + cfv_read = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomFieldValue" && p.action == :read + end) + + assert cfv_read.scope == :all + assert cfv_read.granted == true + end + + test "does NOT allow CustomFieldValue create/update/destroy" do + permissions = PermissionSets.get_permissions(:read_only) + + cfv_create = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomFieldValue" && p.action == :create + end) + + cfv_update = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomFieldValue" && p.action == :update + end) + + cfv_destroy = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomFieldValue" && p.action == :destroy + end) + + assert cfv_create == nil || cfv_create.granted == false + assert cfv_update == nil || cfv_update.granted == false + assert cfv_destroy == nil || cfv_destroy.granted == false + end + + test "allows CustomField read with scope :all" do + permissions = PermissionSets.get_permissions(:read_only) + + cf_read = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomField" && p.action == :read + end) + + assert cf_read.scope == :all + assert cf_read.granted == true + end + + test "includes correct pages" do + permissions = PermissionSets.get_permissions(:read_only) + + assert "/" in permissions.pages + assert "/members" in permissions.pages + assert "/members/:id" in permissions.pages + assert "/custom_field_values" in permissions.pages + end + end + + describe "get_permissions/1 - :normal_user permission content" do + test "allows User read/update with scope :own" do + permissions = PermissionSets.get_permissions(:normal_user) + + user_read = + Enum.find(permissions.resources, fn p -> p.resource == "User" && p.action == :read end) + + user_update = + Enum.find(permissions.resources, fn p -> p.resource == "User" && p.action == :update end) + + assert user_read.scope == :own + assert user_read.granted == true + assert user_update.scope == :own + assert user_update.granted == true + end + + test "allows Member read/create/update with scope :all" do + permissions = PermissionSets.get_permissions(:normal_user) + + member_read = + Enum.find(permissions.resources, fn p -> p.resource == "Member" && p.action == :read end) + + member_create = + Enum.find(permissions.resources, fn p -> p.resource == "Member" && p.action == :create end) + + member_update = + Enum.find(permissions.resources, fn p -> p.resource == "Member" && p.action == :update end) + + assert member_read.scope == :all + assert member_read.granted == true + assert member_create.scope == :all + assert member_create.granted == true + assert member_update.scope == :all + assert member_update.granted == true + end + + test "does NOT allow Member destroy (safety)" do + permissions = PermissionSets.get_permissions(:normal_user) + + member_destroy = + Enum.find(permissions.resources, fn p -> + p.resource == "Member" && p.action == :destroy + end) + + assert member_destroy == nil || member_destroy.granted == false + end + + test "allows CustomFieldValue full CRUD with scope :all" do + permissions = PermissionSets.get_permissions(:normal_user) + + cfv_read = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomFieldValue" && p.action == :read + end) + + cfv_create = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomFieldValue" && p.action == :create + end) + + cfv_update = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomFieldValue" && p.action == :update + end) + + cfv_destroy = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomFieldValue" && p.action == :destroy + end) + + assert cfv_read.scope == :all + assert cfv_read.granted == true + assert cfv_create.scope == :all + assert cfv_create.granted == true + assert cfv_update.scope == :all + assert cfv_update.granted == true + assert cfv_destroy.scope == :all + assert cfv_destroy.granted == true + end + + test "allows CustomField read with scope :all" do + permissions = PermissionSets.get_permissions(:normal_user) + + cf_read = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomField" && p.action == :read + end) + + assert cf_read.scope == :all + assert cf_read.granted == true + end + + test "includes correct pages" do + permissions = PermissionSets.get_permissions(:normal_user) + + assert "/" in permissions.pages + assert "/members" in permissions.pages + assert "/members/new" in permissions.pages + assert "/members/:id" in permissions.pages + assert "/members/:id/edit" in permissions.pages + assert "/custom_field_values" in permissions.pages + assert "/custom_field_values/new" in permissions.pages + assert "/custom_field_values/:id/edit" in permissions.pages + end + end + + describe "get_permissions/1 - :admin permission content" do + test "allows User full CRUD with scope :all" do + permissions = PermissionSets.get_permissions(:admin) + + user_read = + Enum.find(permissions.resources, fn p -> p.resource == "User" && p.action == :read end) + + user_create = + Enum.find(permissions.resources, fn p -> p.resource == "User" && p.action == :create end) + + user_update = + Enum.find(permissions.resources, fn p -> p.resource == "User" && p.action == :update end) + + user_destroy = + Enum.find(permissions.resources, fn p -> p.resource == "User" && p.action == :destroy end) + + assert user_read.scope == :all + assert user_read.granted == true + assert user_create.scope == :all + assert user_create.granted == true + assert user_update.scope == :all + assert user_update.granted == true + assert user_destroy.scope == :all + assert user_destroy.granted == true + end + + test "allows Member full CRUD with scope :all" do + permissions = PermissionSets.get_permissions(:admin) + + member_read = + Enum.find(permissions.resources, fn p -> p.resource == "Member" && p.action == :read end) + + member_create = + Enum.find(permissions.resources, fn p -> p.resource == "Member" && p.action == :create end) + + member_update = + Enum.find(permissions.resources, fn p -> p.resource == "Member" && p.action == :update end) + + member_destroy = + Enum.find(permissions.resources, fn p -> + p.resource == "Member" && p.action == :destroy + end) + + assert member_read.scope == :all + assert member_read.granted == true + assert member_create.scope == :all + assert member_create.granted == true + assert member_update.scope == :all + assert member_update.granted == true + assert member_destroy.scope == :all + assert member_destroy.granted == true + end + + test "allows CustomFieldValue full CRUD with scope :all" do + permissions = PermissionSets.get_permissions(:admin) + + cfv_read = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomFieldValue" && p.action == :read + end) + + cfv_create = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomFieldValue" && p.action == :create + end) + + cfv_update = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomFieldValue" && p.action == :update + end) + + cfv_destroy = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomFieldValue" && p.action == :destroy + end) + + assert cfv_read.scope == :all + assert cfv_read.granted == true + assert cfv_create.scope == :all + assert cfv_create.granted == true + assert cfv_update.scope == :all + assert cfv_update.granted == true + assert cfv_destroy.scope == :all + assert cfv_destroy.granted == true + end + + test "allows CustomField full CRUD with scope :all" do + permissions = PermissionSets.get_permissions(:admin) + + cf_read = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomField" && p.action == :read + end) + + cf_create = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomField" && p.action == :create + end) + + cf_update = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomField" && p.action == :update + end) + + cf_destroy = + Enum.find(permissions.resources, fn p -> + p.resource == "CustomField" && p.action == :destroy + end) + + assert cf_read.scope == :all + assert cf_read.granted == true + assert cf_create.scope == :all + assert cf_create.granted == true + assert cf_update.scope == :all + assert cf_update.granted == true + assert cf_destroy.scope == :all + assert cf_destroy.granted == true + end + + test "allows Role full CRUD with scope :all" do + permissions = PermissionSets.get_permissions(:admin) + + role_read = + Enum.find(permissions.resources, fn p -> p.resource == "Role" && p.action == :read end) + + role_create = + Enum.find(permissions.resources, fn p -> p.resource == "Role" && p.action == :create end) + + role_update = + Enum.find(permissions.resources, fn p -> p.resource == "Role" && p.action == :update end) + + role_destroy = + Enum.find(permissions.resources, fn p -> p.resource == "Role" && p.action == :destroy end) + + assert role_read.scope == :all + assert role_read.granted == true + assert role_create.scope == :all + assert role_create.granted == true + assert role_update.scope == :all + assert role_update.granted == true + assert role_destroy.scope == :all + assert role_destroy.granted == true + end + + test "has wildcard page permission" do + permissions = PermissionSets.get_permissions(:admin) + + assert "*" in permissions.pages + end + end + + describe "valid_permission_set?/1" do + test "returns true for valid permission set string" do + assert PermissionSets.valid_permission_set?("own_data") == true + assert PermissionSets.valid_permission_set?("read_only") == true + assert PermissionSets.valid_permission_set?("normal_user") == true + assert PermissionSets.valid_permission_set?("admin") == true + end + + test "returns true for valid permission set atom" do + assert PermissionSets.valid_permission_set?(:own_data) == true + assert PermissionSets.valid_permission_set?(:read_only) == true + assert PermissionSets.valid_permission_set?(:normal_user) == true + assert PermissionSets.valid_permission_set?(:admin) == true + end + + test "returns false for invalid permission set string" do + assert PermissionSets.valid_permission_set?("invalid") == false + assert PermissionSets.valid_permission_set?("") == false + assert PermissionSets.valid_permission_set?("admin_user") == false + end + + test "returns false for invalid permission set atom" do + assert PermissionSets.valid_permission_set?(:invalid) == false + assert PermissionSets.valid_permission_set?(:unknown) == false + end + + test "returns false for nil input" do + assert PermissionSets.valid_permission_set?(nil) == false + end + end + + describe "permission_set_name_to_atom/1" do + test "returns {:ok, atom} for valid permission set name" do + assert PermissionSets.permission_set_name_to_atom("own_data") == {:ok, :own_data} + assert PermissionSets.permission_set_name_to_atom("read_only") == {:ok, :read_only} + assert PermissionSets.permission_set_name_to_atom("normal_user") == {:ok, :normal_user} + assert PermissionSets.permission_set_name_to_atom("admin") == {:ok, :admin} + end + + test "returns {:error, :invalid_permission_set} for invalid permission set name" do + assert PermissionSets.permission_set_name_to_atom("invalid") == + {:error, :invalid_permission_set} + + assert PermissionSets.permission_set_name_to_atom("") == {:error, :invalid_permission_set} + + assert PermissionSets.permission_set_name_to_atom("admin_user") == + {:error, :invalid_permission_set} + end + + test "handles non-existent atom gracefully" do + # String.to_existing_atom will raise ArgumentError for non-existent atoms + assert PermissionSets.permission_set_name_to_atom("nonexistent_atom_12345") == + {:error, :invalid_permission_set} + end + end +end From 19a20635a77161ca65bcc110a9db6d6f320d3e44 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 19:52:58 +0100 Subject: [PATCH 20/24] docs: update documentation to use CustomFieldValue/CustomField instead of Property/PropertyType --- docs/roles-and-permissions-architecture.md | 126 ++++++++--------- ...les-and-permissions-implementation-plan.md | 130 +++++++++--------- docs/roles-and-permissions-overview.md | 4 +- 3 files changed, 130 insertions(+), 130 deletions(-) diff --git a/docs/roles-and-permissions-architecture.md b/docs/roles-and-permissions-architecture.md index fa45d86..b44604b 100644 --- a/docs/roles-and-permissions-architecture.md +++ b/docs/roles-and-permissions-architecture.md @@ -93,8 +93,8 @@ Five predefined roles stored in the `roles` table: Control CRUD operations on: - User (credentials, profile) - Member (member data) -- Property (custom field values) -- PropertyType (custom field definitions) +- CustomFieldValue (custom field values) +- CustomField (custom field definitions) - Role (role management) **4. Page-Level Permissions** @@ -111,7 +111,7 @@ Three scope levels for permissions: - **:own** - Only records where `record.id == user.id` (for User resource) - **:linked** - Only records linked to user via relationships - Member: `member.user_id == user.id` - - Property: `property.member.user_id == user.id` + - CustomFieldValue: `custom_field_value.member.user_id == user.id` - **:all** - All records, no filtering **6. Special Cases** @@ -414,7 +414,7 @@ defmodule Mv.Authorization.PermissionSets do ## Permission Sets 1. **own_data** - Default for "Mitglied" role - - Can only access own user data and linked member/properties + - Can only access own user data and linked member/custom field values - Cannot create new members or manage system 2. **read_only** - For "Vorstand" and "Buchhaltung" roles @@ -423,11 +423,11 @@ defmodule Mv.Authorization.PermissionSets do 3. **normal_user** - For "Kassenwart" role - Create/Read/Update members (no delete), full CRUD on properties - - Cannot manage property types or users + - Cannot manage custom fields or users 4. **admin** - For "Admin" role - Unrestricted access to all resources - - Can manage users, roles, property types + - Can manage users, roles, custom fields ## Usage @@ -500,12 +500,12 @@ defmodule Mv.Authorization.PermissionSets do %{resource: "Member", action: :read, scope: :linked, granted: true}, %{resource: "Member", action: :update, scope: :linked, granted: true}, - # Property: Can read/update properties of linked member - %{resource: "Property", action: :read, scope: :linked, granted: true}, - %{resource: "Property", action: :update, scope: :linked, granted: true}, + # CustomFieldValue: Can read/update custom field values of linked member + %{resource: "CustomFieldValue", action: :read, scope: :linked, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :linked, granted: true}, - # PropertyType: Can read all (needed for forms) - %{resource: "PropertyType", action: :read, scope: :all, granted: true} + # CustomField: Can read all (needed for forms) + %{resource: "CustomField", action: :read, scope: :all, granted: true} ], pages: [ "/", # Home page @@ -525,17 +525,17 @@ defmodule Mv.Authorization.PermissionSets do # Member: Can read all members, no modifications %{resource: "Member", action: :read, scope: :all, granted: true}, - # Property: Can read all properties - %{resource: "Property", action: :read, scope: :all, granted: true}, + # CustomFieldValue: Can read all custom field values + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, - # PropertyType: Can read all - %{resource: "PropertyType", action: :read, scope: :all, granted: true} + # CustomField: Can read all + %{resource: "CustomField", action: :read, scope: :all, granted: true} ], pages: [ "/", "/members", # Member list "/members/:id", # Member detail - "/properties", # Property overview + "/custom_field_values" # Custom field values overview "/profile" # Own profile ] } @@ -554,14 +554,14 @@ defmodule Mv.Authorization.PermissionSets do %{resource: "Member", action: :update, scope: :all, granted: true}, # Note: destroy intentionally omitted for safety - # Property: Full CRUD - %{resource: "Property", action: :read, scope: :all, granted: true}, - %{resource: "Property", action: :create, scope: :all, granted: true}, - %{resource: "Property", action: :update, scope: :all, granted: true}, - %{resource: "Property", action: :destroy, scope: :all, granted: true}, + # CustomFieldValue: Full CRUD + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true}, - # PropertyType: Read only (admin manages definitions) - %{resource: "PropertyType", action: :read, scope: :all, granted: true} + # CustomField: Read only (admin manages definitions) + %{resource: "CustomField", action: :read, scope: :all, granted: true} ], pages: [ "/", @@ -569,9 +569,9 @@ defmodule Mv.Authorization.PermissionSets do "/members/new", # Create member "/members/:id", "/members/:id/edit", # Edit member - "/properties", - "/properties/new", - "/properties/:id/edit", + "/custom_field_values", + "/custom_field_values/new", + "/custom_field_values/:id/edit", "/profile" ] } @@ -592,17 +592,17 @@ defmodule Mv.Authorization.PermissionSets do %{resource: "Member", action: :update, scope: :all, granted: true}, %{resource: "Member", action: :destroy, scope: :all, granted: true}, - # Property: Full CRUD - %{resource: "Property", action: :read, scope: :all, granted: true}, - %{resource: "Property", action: :create, scope: :all, granted: true}, - %{resource: "Property", action: :update, scope: :all, granted: true}, - %{resource: "Property", action: :destroy, scope: :all, granted: true}, + # CustomFieldValue: Full CRUD + %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true}, + %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true}, - # PropertyType: Full CRUD (admin manages custom field definitions) - %{resource: "PropertyType", action: :read, scope: :all, granted: true}, - %{resource: "PropertyType", action: :create, scope: :all, granted: true}, - %{resource: "PropertyType", action: :update, scope: :all, granted: true}, - %{resource: "PropertyType", action: :destroy, scope: :all, granted: true}, + # CustomField: Full CRUD (admin manages custom field definitions) + %{resource: "CustomField", action: :read, scope: :all, granted: true}, + %{resource: "CustomField", action: :create, scope: :all, granted: true}, + %{resource: "CustomField", action: :update, scope: :all, granted: true}, + %{resource: "CustomField", action: :destroy, scope: :all, granted: true}, # Role: Full CRUD (admin manages roles) %{resource: "Role", action: :read, scope: :all, granted: true}, @@ -677,9 +677,9 @@ Quick reference table showing what each permission set allows: | **User** (all) | - | - | - | R, C, U, D | | **Member** (linked) | R, U | - | - | - | | **Member** (all) | - | R | R, C, U | R, C, U, D | -| **Property** (linked) | R, U | - | - | - | -| **Property** (all) | - | R | R, C, U, D | R, C, U, D | -| **PropertyType** (all) | R | R | R | R, C, U, D | +| **CustomFieldValue** (linked) | R, U | - | - | - | +| **CustomFieldValue** (all) | - | R | R, C, U, D | R, C, U, D | +| **CustomField** (all) | R | R | R | R, C, U, D | | **Role** (all) | - | - | - | R, C, U, D | **Legend:** R=Read, C=Create, U=Update, D=Destroy @@ -715,7 +715,7 @@ defmodule Mv.Authorization.Checks.HasPermission do - **:own** - Filters to records where record.id == actor.id - **:linked** - Filters based on resource type: - Member: member.user_id == actor.id - - Property: property.member.user_id == actor.id (traverses relationship!) + - CustomFieldValue: custom_field_value.member.user_id == actor.id (traverses relationship!) ## Error Handling @@ -802,8 +802,8 @@ defmodule Mv.Authorization.Checks.HasPermission do # Member.user_id == actor.id (direct relationship) {:filter, expr(user_id == ^actor.id)} - "Property" -> - # Property.member.user_id == actor.id (traverse through member!) + "CustomFieldValue" -> + # CustomFieldValue.member.user_id == actor.id (traverse through member!) {:filter, expr(member.user_id == ^actor.id)} _ -> @@ -832,7 +832,7 @@ end **Key Design Decisions:** -1. **Resource-Specific :linked Scope:** Property needs to traverse `member` relationship to check `user_id` +1. **Resource-Specific :linked Scope:** CustomFieldValue needs to traverse `member` relationship to check `user_id` 2. **Error Handling:** All errors log for debugging but return generic forbidden to user 3. **Module Name Extraction:** Uses `Module.split() |> List.last()` to match against PermissionSets strings 4. **Pure Function:** No side effects, deterministic, easily testable @@ -966,21 +966,21 @@ end *Email editing has additional validation (see Special Cases) -### Property Resource Policies +### CustomFieldValue Resource Policies -**Location:** `lib/mv/membership/property.ex` +**Location:** `lib/mv/membership/custom_field_value.ex` -**Special Case:** Users can access properties of their linked member. +**Special Case:** Users can access custom field values of their linked member. ```elixir -defmodule Mv.Membership.Property do +defmodule Mv.Membership.CustomFieldValue do use Ash.Resource, ... policies do - # SPECIAL CASE: Users can access properties of their linked member + # SPECIAL CASE: Users can access custom field values of their linked member # Note: This traverses the member relationship! policy action_type([:read, :update]) do - description "Users can access properties of their linked member" + description "Users can access custom field values of their linked member" authorize_if expr(member.user_id == ^actor(:id)) end @@ -1010,18 +1010,18 @@ end | Create | ❌ | ❌ | ✅ | ❌ | ✅ | | Destroy | ❌ | ❌ | ✅ | ❌ | ✅ | -### PropertyType Resource Policies +### CustomField Resource Policies -**Location:** `lib/mv/membership/property_type.ex` +**Location:** `lib/mv/membership/custom_field.ex` **No Special Cases:** All users can read, only admin can write. ```elixir -defmodule Mv.Membership.PropertyType do +defmodule Mv.Membership.CustomField do use Ash.Resource, ... policies do - # All authenticated users can read property types (needed for forms) + # All authenticated users can read custom fields (needed for forms) # Write operations are admin-only policy action_type([:read, :create, :update, :destroy]) do description "Check permissions from user's role" @@ -1308,12 +1308,12 @@ end - ❌ Cannot access: `/members`, `/members/new`, `/admin/roles` **Vorstand (read_only):** -- ✅ Can access: `/`, `/members`, `/members/123`, `/properties`, `/profile` +- ✅ Can access: `/`, `/members`, `/members/123`, `/custom_field_values`, `/profile` - ❌ Cannot access: `/members/new`, `/members/123/edit`, `/admin/roles` **Kassenwart (normal_user):** -- ✅ Can access: `/`, `/members`, `/members/new`, `/members/123/edit`, `/properties`, `/profile` -- ❌ Cannot access: `/admin/roles`, `/admin/property_types/new` +- ✅ Can access: `/`, `/members`, `/members/new`, `/members/123/edit`, `/custom_field_values`, `/profile` +- ❌ Cannot access: `/admin/roles`, `/admin/custom_fields/new` **Admin:** - ✅ Can access: `*` (all pages, including `/admin/roles`) @@ -1479,9 +1479,9 @@ defmodule MvWeb.Authorization do # Direct relationship: member.user_id Map.get(record, :user_id) == user.id - "Property" -> - # Need to traverse: property.member.user_id - # Note: In UI, property should have member preloaded + "CustomFieldValue" -> + # Need to traverse: custom_field_value.member.user_id + # Note: In UI, custom_field_value should have member preloaded case Map.get(record, :member) do %{user_id: member_user_id} -> member_user_id == user.id _ -> false @@ -1569,7 +1569,7 @@ end Admin
  • <.link navigate="/admin/roles">Roles
  • -
  • <.link navigate="/admin/property_types">Property Types
  • +
  • <.link navigate="/admin/custom_fields">Custom Fields
<% end %> @@ -2409,8 +2409,8 @@ The `HasPermission` check extracts resource names via `Module.split() |> List.la |------------|------------------------| | `Mv.Accounts.User` | "User" | | `Mv.Membership.Member` | "Member" | -| `Mv.Membership.Property` | "Property" | -| `Mv.Membership.PropertyType` | "PropertyType" | +| `Mv.Membership.CustomFieldValue` | "CustomFieldValue" | +| `Mv.Membership.CustomField` | "CustomField" | | `Mv.Authorization.Role` | "Role" | These strings must match exactly in `PermissionSets` module. @@ -2450,7 +2450,7 @@ These strings must match exactly in `PermissionSets` module. **Integration:** - [ ] One complete user journey per role -- [ ] Cross-resource scenarios (e.g., Member -> Property) +- [ ] Cross-resource scenarios (e.g., Member -> CustomFieldValue) - [ ] Special cases in context (e.g., linked member email during full edit flow) ### Useful Commands diff --git a/docs/roles-and-permissions-implementation-plan.md b/docs/roles-and-permissions-implementation-plan.md index 0b173fa..2c29b8d 100644 --- a/docs/roles-and-permissions-implementation-plan.md +++ b/docs/roles-and-permissions-implementation-plan.md @@ -53,7 +53,7 @@ This document defines the implementation plan for the **MVP (Phase 1)** of the R Hardcoded in `Mv.Authorization.PermissionSets` module: 1. **own_data** - User can only access their own data (default for "Mitglied") -2. **read_only** - Read access to all members/properties (for "Vorstand", "Buchhaltung") +2. **read_only** - Read access to all members/custom field values (for "Vorstand", "Buchhaltung") 3. **normal_user** - Create/Read/Update members (no delete), full CRUD on properties (for "Kassenwart") 4. **admin** - Unrestricted access including user/role management (for "Admin") @@ -77,7 +77,7 @@ Stored in database `roles` table, each referencing a `permission_set_name`: - ✅ Hardcoded PermissionSets module with 4 permission sets - ✅ Role database table and CRUD interface - ✅ Custom Ash Policy Check (`HasPermission`) that reads from PermissionSets -- ✅ Policies on all resources (Member, User, Property, PropertyType, Role) +- ✅ Policies on all resources (Member, User, CustomFieldValue, CustomField, Role) - ✅ Page-level permissions via Phoenix Plug - ✅ UI authorization helpers for conditional rendering - ✅ Special case: Member email validation for linked users @@ -228,32 +228,32 @@ Create the core `PermissionSets` module that defines all four permission sets wi - Resources: - User: read/update :own - Member: read/update :linked - - Property: read/update :linked - - PropertyType: read :all + - CustomFieldValue: read/update :linked + - CustomField: read :all - Pages: `["/", "/profile", "/members/:id"]` **2. read_only (Vorstand, Buchhaltung):** - Resources: - User: read :own, update :own - Member: read :all - - Property: read :all - - PropertyType: read :all -- Pages: `["/", "/members", "/members/:id", "/properties"]` + - CustomFieldValue: read :all + - CustomField: read :all +- Pages: `["/", "/members", "/members/:id", "/custom_field_values"]` **3. normal_user (Kassenwart):** - Resources: - User: read/update :own - Member: read/create/update :all (no destroy for safety) - - Property: read/create/update/destroy :all - - PropertyType: read :all -- Pages: `["/", "/members", "/members/new", "/members/:id", "/members/:id/edit", "/properties", "/properties/new", "/properties/:id/edit"]` + - CustomFieldValue: read/create/update/destroy :all + - CustomField: read :all +- Pages: `["/", "/members", "/members/new", "/members/:id", "/members/:id/edit", "/custom_field_values", "/custom_field_values/new", "/custom_field_values/:id/edit"]` **4. admin:** - Resources: - User: read/update/destroy :all - Member: read/create/update/destroy :all - - Property: read/create/update/destroy :all - - PropertyType: read/create/update/destroy :all + - CustomFieldValue: read/create/update/destroy :all + - CustomField: read/create/update/destroy :all - Role: read/create/update/destroy :all - Pages: `["*"]` (wildcard = all pages) @@ -276,10 +276,10 @@ Create the core `PermissionSets` module that defines all four permission sets wi **Permission Content Tests:** - `:own_data` allows User read/update with scope :own -- `:own_data` allows Member/Property read/update with scope :linked -- `:read_only` allows Member/Property read with scope :all -- `:read_only` does NOT allow Member/Property create/update/destroy -- `:normal_user` allows Member/Property full CRUD with scope :all +- `:own_data` allows Member/CustomFieldValue read/update with scope :linked +- `:read_only` allows Member/CustomFieldValue read with scope :all +- `:read_only` does NOT allow Member/CustomFieldValue create/update/destroy +- `:normal_user` allows Member/CustomFieldValue full CRUD with scope :all - `:admin` allows everything with scope :all - `:admin` has wildcard page permission "*" @@ -387,7 +387,7 @@ Create the core custom Ash Policy Check that reads permissions from the `Permiss - `:own` → `{:filter, expr(id == ^actor.id)}` - `:linked` → resource-specific logic: - Member: `{:filter, expr(user_id == ^actor.id)}` - - Property: `{:filter, expr(member.user_id == ^actor.id)}` (traverse relationship!) + - CustomFieldValue: `{:filter, expr(member.user_id == ^actor.id)}` (traverse relationship!) 6. Handle errors gracefully: - No actor → `{:error, :no_actor}` - No role → `{:error, :no_role}` @@ -401,7 +401,7 @@ Create the core custom Ash Policy Check that reads permissions from the `Permiss - [ ] Check module implements `Ash.Policy.Check` behavior - [ ] `match?/3` correctly evaluates permissions from PermissionSets - [ ] Scope filters work correctly (:all, :own, :linked) -- [ ] `:linked` scope handles Member and Property differently +- [ ] `:linked` scope handles Member and CustomFieldValue differently - [ ] Errors are handled gracefully (no crashes) - [ ] Authorization failures are logged - [ ] Module is well-documented @@ -425,7 +425,7 @@ Create the core custom Ash Policy Check that reads permissions from the `Permiss **Scope Application Tests - :linked:** - Actor with scope :linked can access Member where member.user_id == actor.id -- Actor with scope :linked can access Property where property.member.user_id == actor.id (relationship traversal!) +- Actor with scope :linked can access CustomFieldValue where custom_field_value.member.user_id == actor.id (relationship traversal!) - Actor with scope :linked cannot access unlinked member - Query correctly filters based on user_id relationship @@ -581,7 +581,7 @@ Add authorization policies to the User resource. Special case: Users can always --- -#### Issue #9: Property Resource Policies +#### Issue #9: CustomFieldValue Resource Policies **Size:** M (2 days) **Dependencies:** #6 (HasPermission check) @@ -590,20 +590,20 @@ Add authorization policies to the User resource. Special case: Users can always **Description:** -Add authorization policies to the Property resource. Properties are linked to members, which are linked to users. +Add authorization policies to the CustomFieldValue resource. CustomFieldValues are linked to members, which are linked to users. **Tasks:** -1. Open `lib/mv/membership/property.ex` +1. Open `lib/mv/membership/custom_field_value.ex` 2. Add `policies` block -3. Add special policy: Allow user to read/update properties of their linked member +3. Add special policy: Allow user to read/update custom field values of their linked member ```elixir policy action_type([:read, :update]) do authorize_if expr(member.user_id == ^actor(:id)) end ``` 4. Add general policy: Check HasPermission -5. Ensure Property preloads :member relationship for scope checks +5. Ensure CustomFieldValue preloads :member relationship for scope checks 6. Preload :role relationship for actor **Policy Order:** @@ -620,27 +620,27 @@ Add authorization policies to the Property resource. Properties are linked to me **Test Strategy (TDD):** -**Linked Properties Tests (:own_data):** -- User can read properties of their linked member -- User can update properties of their linked member -- User cannot read properties of unlinked members -- Verify relationship traversal works (property.member.user_id) +**Linked CustomFieldValues Tests (:own_data):** +- User can read custom field values of their linked member +- User can update custom field values of their linked member +- User cannot read custom field values of unlinked members +- Verify relationship traversal works (custom_field_value.member.user_id) **Read-Only Tests:** -- User with :read_only can read all properties -- User with :read_only cannot create/update properties +- User with :read_only can read all custom field values +- User with :read_only cannot create/update custom field values **Normal User Tests:** -- User with :normal_user can CRUD properties +- User with :normal_user can CRUD custom field values **Admin Tests:** - Admin can perform all operations -**Test File:** `test/mv/membership/property_policies_test.exs` +**Test File:** `test/mv/membership/custom_field_value_policies_test.exs` --- -#### Issue #10: PropertyType Resource Policies +#### Issue #10: CustomField Resource Policies **Size:** S (1 day) **Dependencies:** #6 (HasPermission check) @@ -649,11 +649,11 @@ Add authorization policies to the Property resource. Properties are linked to me **Description:** -Add authorization policies to the PropertyType resource. PropertyTypes are admin-managed, but readable by all. +Add authorization policies to the CustomField resource. CustomFields are admin-managed, but readable by all. **Tasks:** -1. Open `lib/mv/membership/property_type.ex` +1. Open `lib/mv/membership/custom_field.ex` 2. Add `policies` block 3. Add read policy: All authenticated users can read (scope :all) 4. Add write policies: Only admin can create/update/destroy @@ -661,27 +661,27 @@ Add authorization policies to the PropertyType resource. PropertyTypes are admin **Acceptance Criteria:** -- [ ] All users can read property types -- [ ] Only admin can create/update/destroy property types +- [ ] All users can read custom fields +- [ ] Only admin can create/update/destroy custom fields - [ ] Policies tested **Test Strategy (TDD):** **Read Access (All Roles):** -- User with :own_data can read all property types -- User with :read_only can read all property types -- User with :normal_user can read all property types -- User with :admin can read all property types +- User with :own_data can read all custom fields +- User with :read_only can read all custom fields +- User with :normal_user can read all custom fields +- User with :admin can read all custom fields **Write Access (Admin Only):** -- Non-admin cannot create property type (Forbidden) -- Non-admin cannot update property type (Forbidden) -- Non-admin cannot destroy property type (Forbidden) -- Admin can create property type -- Admin can update property type -- Admin can destroy property type +- Non-admin cannot create custom field (Forbidden) +- Non-admin cannot update custom field (Forbidden) +- Non-admin cannot destroy custom field (Forbidden) +- Admin can create custom field +- Admin can update custom field +- Admin can destroy custom field -**Test File:** `test/mv/membership/property_type_policies_test.exs` +**Test File:** `test/mv/membership/custom_field_policies_test.exs` --- @@ -924,7 +924,7 @@ Create helper functions for UI-level authorization checks. These will be used in ``` 5. All functions use `PermissionSets.get_permissions/1` (same logic as HasPermission) 6. All functions handle nil user gracefully (return false) -7. Implement resource-specific scope checking (Member vs Property for :linked) +7. Implement resource-specific scope checking (Member vs CustomFieldValue for :linked) 8. Add comprehensive `@doc` with template examples 9. Import helper in `mv_web.ex` `html_helpers` section @@ -957,9 +957,9 @@ Create helper functions for UI-level authorization checks. These will be used in **can?/3 with Record Struct - Scope :linked:** - User can update linked Member (member.user_id == user.id) - User cannot update unlinked Member -- User can update Property of linked Member (property.member.user_id == user.id) -- User cannot update Property of unlinked Member -- Scope checking is resource-specific (Member vs Property) +- User can update CustomFieldValue of linked Member (custom_field_value.member.user_id == user.id) +- User cannot update CustomFieldValue of unlinked Member +- Scope checking is resource-specific (Member vs CustomFieldValue) **can_access_page?/2:** - User with page in list can access (returns true) @@ -1046,7 +1046,7 @@ Update Role management LiveViews to use authorization helpers for conditional re **Description:** -Update all existing LiveViews (Member, User, Property, PropertyType) to use authorization helpers for conditional rendering. +Update all existing LiveViews (Member, User, CustomFieldValue, CustomField) to use authorization helpers for conditional rendering. **Tasks:** @@ -1061,10 +1061,10 @@ Update all existing LiveViews (Member, User, Property, PropertyType) to use auth - Show: Only show other users if admin, always show own profile - Edit: Only allow editing own profile or admin editing anyone -3. **Property LiveViews:** +3. **CustomFieldValue LiveViews:** - Similar to Member (hide create/edit/delete based on permissions) -4. **PropertyType LiveViews:** +4. **CustomField LiveViews:** - All users can view - Only admin can create/edit/delete @@ -1110,13 +1110,13 @@ Update all existing LiveViews (Member, User, Property, PropertyType) to use auth - Vorstand: Sees "Home", "Members" (read-only), "Profile" - Kassenwart: Sees "Home", "Members", "Properties", "Profile" - Buchhaltung: Sees "Home", "Members" (read-only), "Profile" -- Admin: Sees "Home", "Members", "Properties", "Property Types", "Admin", "Profile" +- Admin: Sees "Home", "Members", "Custom Field Values", "Custom Fields", "Admin", "Profile" **Test Files:** - `test/mv_web/live/member_live_authorization_test.exs` - `test/mv_web/live/user_live_authorization_test.exs` -- `test/mv_web/live/property_live_authorization_test.exs` -- `test/mv_web/live/property_type_live_authorization_test.exs` +- `test/mv_web/live/custom_field_value_live_authorization_test.exs` +- `test/mv_web/live/custom_field_live_authorization_test.exs` - `test/mv_web/components/navbar_authorization_test.exs` --- @@ -1192,7 +1192,7 @@ Write comprehensive integration tests that follow complete user journeys for eac 4. Can edit any member (except email if linked - see special case) 5. Cannot delete member 6. Can manage properties -7. Cannot manage property types (read-only) +7. Cannot manage custom fields (read-only) 8. Cannot access /admin/roles **Buchhaltung Journey:** @@ -1266,7 +1266,7 @@ Write comprehensive integration tests that follow complete user journeys for eac │ │ │ ┌────▼─────┐ ┌──────▼──────┐ │ │ Issue #9 │ │ Issue #10 │ │ - │ Property │ │ PropType │ │ + │ CustomFieldValue │ │ CustomField │ │ │ Policies │ │ Policies │ │ └────┬─────┘ └──────┬──────┘ │ │ │ │ @@ -1384,8 +1384,8 @@ test/ ├── mv/membership/ │ ├── member_policies_test.exs # Issue #7 │ ├── member_email_validation_test.exs # Issue #12 -│ ├── property_policies_test.exs # Issue #9 -│ └── property_type_policies_test.exs # Issue #10 +│ ├── custom_field_value_policies_test.exs # Issue #9 +│ └── custom_field_policies_test.exs # Issue #10 ├── mv_web/ │ ├── authorization_test.exs # Issue #14 │ ├── plugs/ @@ -1395,8 +1395,8 @@ test/ │ ├── role_live_authorization_test.exs # Issue #15 │ ├── member_live_authorization_test.exs # Issue #16 │ ├── user_live_authorization_test.exs # Issue #16 -│ ├── property_live_authorization_test.exs # Issue #16 -│ └── property_type_live_authorization_test.exs # Issue #16 +│ ├── custom_field_value_live_authorization_test.exs # Issue #16 +│ └── custom_field_live_authorization_test.exs # Issue #16 ├── integration/ │ ├── mitglied_journey_test.exs # Issue #17 │ ├── vorstand_journey_test.exs # Issue #17 diff --git a/docs/roles-and-permissions-overview.md b/docs/roles-and-permissions-overview.md index 191e8b7..86e7273 100644 --- a/docs/roles-and-permissions-overview.md +++ b/docs/roles-and-permissions-overview.md @@ -201,7 +201,7 @@ When runtime permission editing becomes a business requirement, migrate to Appro **Resource Level (MVP):** - Controls create, read, update, destroy actions on resources -- Resources: Member, User, Property, PropertyType, Role +- Resources: Member, User, CustomFieldValue, CustomField, Role **Page Level (MVP):** - Controls access to LiveView pages @@ -280,7 +280,7 @@ Contains: Each Permission Set contains: **Resources:** List of resource permissions -- resource: "Member", "User", "Property", etc. +- resource: "Member", "User", "CustomFieldValue", etc. - action: :read, :create, :update, :destroy - scope: :own, :linked, :all - granted: true/false From 4bd08e85bb3bc6dc5600810fa727e590d5eb097b Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 21:35:59 +0100 Subject: [PATCH 21/24] fix: use Enum.empty? instead of != [] to fix type warning Replace comparison with empty list using Enum.empty?/1 to satisfy type checker and avoid redundant comparison warning --- test/mv/authorization/permission_sets_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/mv/authorization/permission_sets_test.exs b/test/mv/authorization/permission_sets_test.exs index 51dc797..84bdc2f 100644 --- a/test/mv/authorization/permission_sets_test.exs +++ b/test/mv/authorization/permission_sets_test.exs @@ -74,7 +74,7 @@ defmodule Mv.Authorization.PermissionSetsTest do for set <- [:own_data, :read_only, :normal_user, :admin] do permissions = PermissionSets.get_permissions(set) - assert permissions.pages != [], + assert not Enum.empty?(permissions.pages), "Permission set #{set} should have at least one page" end end From 9b0d022767a258b2195764c7a1588a043cd6e686 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 21:55:13 +0100 Subject: [PATCH 22/24] fix: add missing /profile page to read_only and normal_user permission sets Both permission sets allow User:update :own, so users should be able to access their profile page. This makes the implementation consistent with the documentation and the logical permission model. --- lib/mv/authorization/permission_sets.ex | 4 ++++ test/mv/authorization/permission_sets_test.exs | 2 ++ 2 files changed, 6 insertions(+) diff --git a/lib/mv/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex index 22b1648..6139f7f 100644 --- a/lib/mv/authorization/permission_sets.ex +++ b/lib/mv/authorization/permission_sets.ex @@ -132,6 +132,8 @@ defmodule Mv.Authorization.PermissionSets do ], pages: [ "/", + # Own profile + "/profile", # Member list "/members", # Member detail @@ -166,6 +168,8 @@ defmodule Mv.Authorization.PermissionSets do ], pages: [ "/", + # Own profile + "/profile", "/members", # Create member "/members/new", diff --git a/test/mv/authorization/permission_sets_test.exs b/test/mv/authorization/permission_sets_test.exs index 84bdc2f..06e2110 100644 --- a/test/mv/authorization/permission_sets_test.exs +++ b/test/mv/authorization/permission_sets_test.exs @@ -247,6 +247,7 @@ defmodule Mv.Authorization.PermissionSetsTest do permissions = PermissionSets.get_permissions(:read_only) assert "/" in permissions.pages + assert "/profile" in permissions.pages assert "/members" in permissions.pages assert "/members/:id" in permissions.pages assert "/custom_field_values" in permissions.pages @@ -349,6 +350,7 @@ defmodule Mv.Authorization.PermissionSetsTest do permissions = PermissionSets.get_permissions(:normal_user) assert "/" in permissions.pages + assert "/profile" in permissions.pages assert "/members" in permissions.pages assert "/members/new" in permissions.pages assert "/members/:id" in permissions.pages From 7845117fadcdaa4417d18c24d5bd213083eb13ec Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 21:55:52 +0100 Subject: [PATCH 23/24] refactor: improve error handling and documentation in PermissionSets - Add explicit ArgumentError for invalid permission set names with helpful message - Soften performance claim in documentation (intended to be constant-time) - Add tests for error handling - Improve maintainability with guard clause for invalid inputs --- lib/mv/authorization/permission_sets.ex | 11 +++++++-- .../mv/authorization/permission_sets_test.exs | 23 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/mv/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex index 6139f7f..f9197de 100644 --- a/lib/mv/authorization/permission_sets.ex +++ b/lib/mv/authorization/permission_sets.ex @@ -38,7 +38,9 @@ defmodule Mv.Authorization.PermissionSets do ## Performance - All functions are pure and compile-time. Permission lookups are < 1 microsecond. + All functions are pure and intended to be constant-time. Permission lookups + are very fast (typically < 1 microsecond in practice) as they are simple + pattern matches and map lookups with no database queries or external calls. """ @type scope :: :own | :linked | :all @@ -81,10 +83,15 @@ defmodule Mv.Authorization.PermissionSets do true iex> PermissionSets.get_permissions(:invalid) - ** (FunctionClauseError) no function clause matching + ** (ArgumentError) invalid permission set: :invalid. Must be one of: [:own_data, :read_only, :normal_user, :admin] """ @spec get_permissions(atom()) :: permission_set() + def get_permissions(set) when set not in [:own_data, :read_only, :normal_user, :admin] do + raise ArgumentError, + "invalid permission set: #{inspect(set)}. Must be one of: #{inspect(all_permission_sets())}" + end + def get_permissions(:own_data) do %{ resources: [ diff --git a/test/mv/authorization/permission_sets_test.exs b/test/mv/authorization/permission_sets_test.exs index 06e2110..de960a9 100644 --- a/test/mv/authorization/permission_sets_test.exs +++ b/test/mv/authorization/permission_sets_test.exs @@ -567,4 +567,27 @@ defmodule Mv.Authorization.PermissionSetsTest do {:error, :invalid_permission_set} end end + + describe "get_permissions/1 - error handling" do + test "raises ArgumentError for invalid permission set with helpful message" do + assert_raise ArgumentError, + ~r/invalid permission set: :invalid\. Must be one of:/, + fn -> + PermissionSets.get_permissions(:invalid) + end + end + + test "error message includes all valid permission sets" do + error = + assert_raise ArgumentError, fn -> + PermissionSets.get_permissions(:unknown) + end + + error_message = Exception.message(error) + assert error_message =~ "own_data" + assert error_message =~ "read_only" + assert error_message =~ "normal_user" + assert error_message =~ "admin" + end + end end From 18ec4bfd1682de0555d07fe84dc6c5966c334b45 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 6 Jan 2026 22:17:33 +0100 Subject: [PATCH 24/24] fix: add missing /custom_field_values/:id page to read_only and normal_user - Add /custom_field_values/:id to read_only pages (users can view list, should also view details) - Add /custom_field_values/:id to normal_user pages - Refactor tests to reduce duplication (use for-comprehension for structure tests) - Add tests for invalid input types in valid_permission_set?/1 - Update @spec for valid_permission_set?/1 to accept any() type --- lib/mv/authorization/permission_sets.ex | 13 ++++- .../mv/authorization/permission_sets_test.exs | 51 ++++++++----------- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/lib/mv/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex index f9197de..11ddb5a 100644 --- a/lib/mv/authorization/permission_sets.ex +++ b/lib/mv/authorization/permission_sets.ex @@ -146,7 +146,9 @@ defmodule Mv.Authorization.PermissionSets do # Member detail "/members/:id", # Custom field values overview - "/custom_field_values" + "/custom_field_values", + # Custom field value detail + "/custom_field_values/:id" ] } end @@ -184,6 +186,8 @@ defmodule Mv.Authorization.PermissionSets do # Edit member "/members/:id/edit", "/custom_field_values", + # Custom field value detail + "/custom_field_values/:id", "/custom_field_values/new", "/custom_field_values/:id/edit" ] @@ -230,6 +234,11 @@ defmodule Mv.Authorization.PermissionSets do } end + def get_permissions(invalid) do + raise ArgumentError, + "invalid permission set: #{inspect(invalid)}. Must be one of: #{inspect(all_permission_sets())}" + end + @doc """ Checks if a permission set name (string or atom) is valid. @@ -244,7 +253,7 @@ defmodule Mv.Authorization.PermissionSets do iex> PermissionSets.valid_permission_set?("invalid") false """ - @spec valid_permission_set?(String.t() | atom()) :: boolean() + @spec valid_permission_set?(any()) :: boolean() def valid_permission_set?(name) when is_binary(name) do case permission_set_name_to_atom(name) do {:ok, _atom} -> true diff --git a/test/mv/authorization/permission_sets_test.exs b/test/mv/authorization/permission_sets_test.exs index de960a9..dcd0680 100644 --- a/test/mv/authorization/permission_sets_test.exs +++ b/test/mv/authorization/permission_sets_test.exs @@ -19,40 +19,22 @@ defmodule Mv.Authorization.PermissionSetsTest do end describe "get_permissions/1" do - test "returns map with :resources and :pages keys for :own_data" do - permissions = PermissionSets.get_permissions(:own_data) + test "all permission sets return map with :resources and :pages keys" do + for set <- PermissionSets.all_permission_sets() do + permissions = PermissionSets.get_permissions(set) - assert Map.has_key?(permissions, :resources) - assert Map.has_key?(permissions, :pages) - assert is_list(permissions.resources) - assert is_list(permissions.pages) - end + assert Map.has_key?(permissions, :resources), + "#{set} missing :resources key" - test "returns map with :resources and :pages keys for :read_only" do - permissions = PermissionSets.get_permissions(:read_only) + assert Map.has_key?(permissions, :pages), + "#{set} missing :pages key" - assert Map.has_key?(permissions, :resources) - assert Map.has_key?(permissions, :pages) - assert is_list(permissions.resources) - assert is_list(permissions.pages) - end + assert is_list(permissions.resources), + "#{set} :resources must be a list" - test "returns map with :resources and :pages keys for :normal_user" do - permissions = PermissionSets.get_permissions(:normal_user) - - assert Map.has_key?(permissions, :resources) - assert Map.has_key?(permissions, :pages) - assert is_list(permissions.resources) - assert is_list(permissions.pages) - end - - test "returns map with :resources and :pages keys for :admin" do - permissions = PermissionSets.get_permissions(:admin) - - assert Map.has_key?(permissions, :resources) - assert Map.has_key?(permissions, :pages) - assert is_list(permissions.resources) - assert is_list(permissions.pages) + assert is_list(permissions.pages), + "#{set} :pages must be a list" + end end test "each resource permission has required keys" do @@ -251,6 +233,7 @@ defmodule Mv.Authorization.PermissionSetsTest do assert "/members" in permissions.pages assert "/members/:id" in permissions.pages assert "/custom_field_values" in permissions.pages + assert "/custom_field_values/:id" in permissions.pages end end @@ -356,6 +339,7 @@ defmodule Mv.Authorization.PermissionSetsTest do assert "/members/:id" in permissions.pages assert "/members/:id/edit" in permissions.pages assert "/custom_field_values" in permissions.pages + assert "/custom_field_values/:id" in permissions.pages assert "/custom_field_values/new" in permissions.pages assert "/custom_field_values/:id/edit" in permissions.pages end @@ -541,6 +525,13 @@ defmodule Mv.Authorization.PermissionSetsTest do test "returns false for nil input" do assert PermissionSets.valid_permission_set?(nil) == false end + + test "returns false for invalid types" do + assert PermissionSets.valid_permission_set?(123) == false + assert PermissionSets.valid_permission_set?([]) == false + assert PermissionSets.valid_permission_set?(%{}) == false + assert PermissionSets.valid_permission_set?("") == false + end end describe "permission_set_name_to_atom/1" do