From e799f0271c7b0ca5534af9fb669389a55f7c1a4a Mon Sep 17 00:00:00 2001
From: Moritz <moritz.m@local-it.org>
Date: Wed, 4 Feb 2026 00:33:58 +0100
Subject: [PATCH] Refactor PermissionSets: define admin permissions via
 perm_all()

Use perm/3 helper for admin resource permissions (DRY). MemberGroup
keeps read/create/destroy only (no update in domain).
---
 lib/mv/authorization/permission_sets.ex | 82 ++++++++-----------------
 1 file changed, 27 insertions(+), 55 deletions(-)

diff --git a/lib/mv/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex
index 61c3fbf..d1bbc3e 100644
--- a/lib/mv/authorization/permission_sets.ex
+++ b/lib/mv/authorization/permission_sets.ex
@@ -58,10 +58,19 @@ defmodule Mv.Authorization.PermissionSets do
           pages: [String.t()]
         }
 
-  # DRY helpers for shared resource permission lists (used in own_data, read_only, normal_user)
+  # DRY helpers for shared resource permission lists (used in own_data, read_only, normal_user, admin)
   defp perm(resource, action, scope),
     do: %{resource: resource, action: action, scope: scope, granted: true}
 
+  # All four CRUD actions for a resource with scope :all (used for admin)
+  defp perm_all(resource),
+    do: [
+      perm(resource, :read, :all),
+      perm(resource, :create, :all),
+      perm(resource, :update, :all),
+      perm(resource, :destroy, :all)
+    ]
+
   # User: read/update own credentials only (all non-admin sets allow password changes)
   defp user_own_credentials, do: [perm("User", :read, :own), perm("User", :update, :own)]
 
@@ -234,61 +243,24 @@ defmodule Mv.Authorization.PermissionSets do
   end
 
   def get_permissions(:admin) do
+    # MemberGroup has no :update action in the domain; use read/create/destroy only
+    member_group_perms = [
+      perm("MemberGroup", :read, :all),
+      perm("MemberGroup", :create, :all),
+      perm("MemberGroup", :destroy, :all)
+    ]
+
     %{
-      resources: [
-        # User: Full management including other users
-        %{resource: "User", action: :read, scope: :all, granted: true},
-        %{resource: "User", action: :create, scope: :all, granted: true},
-        %{resource: "User", action: :update, scope: :all, granted: true},
-        %{resource: "User", action: :destroy, scope: :all, granted: true},
-
-        # Member: Full CRUD
-        %{resource: "Member", action: :read, scope: :all, granted: true},
-        %{resource: "Member", action: :create, scope: :all, granted: true},
-        %{resource: "Member", action: :update, scope: :all, granted: true},
-        %{resource: "Member", action: :destroy, scope: :all, granted: true},
-
-        # CustomFieldValue: Full CRUD
-        %{resource: "CustomFieldValue", action: :read, scope: :all, granted: true},
-        %{resource: "CustomFieldValue", action: :create, scope: :all, granted: true},
-        %{resource: "CustomFieldValue", action: :update, scope: :all, granted: true},
-        %{resource: "CustomFieldValue", action: :destroy, scope: :all, granted: true},
-
-        # CustomField: Full CRUD (admin manages custom field definitions)
-        %{resource: "CustomField", action: :read, scope: :all, granted: true},
-        %{resource: "CustomField", action: :create, scope: :all, granted: true},
-        %{resource: "CustomField", action: :update, scope: :all, granted: true},
-        %{resource: "CustomField", action: :destroy, scope: :all, granted: true},
-
-        # Role: Full CRUD (admin manages roles)
-        %{resource: "Role", action: :read, scope: :all, granted: true},
-        %{resource: "Role", action: :create, scope: :all, granted: true},
-        %{resource: "Role", action: :update, scope: :all, granted: true},
-        %{resource: "Role", action: :destroy, scope: :all, granted: true},
-
-        # Group: Full CRUD (admin manages groups)
-        %{resource: "Group", action: :read, scope: :all, granted: true},
-        %{resource: "Group", action: :create, scope: :all, granted: true},
-        %{resource: "Group", action: :update, scope: :all, granted: true},
-        %{resource: "Group", action: :destroy, scope: :all, granted: true},
-
-        # MemberGroup: Full CRUD
-        %{resource: "MemberGroup", action: :read, scope: :all, granted: true},
-        %{resource: "MemberGroup", action: :create, scope: :all, granted: true},
-        %{resource: "MemberGroup", action: :destroy, scope: :all, granted: true},
-
-        # MembershipFeeType: Full CRUD (admin manages fee types)
-        %{resource: "MembershipFeeType", action: :read, scope: :all, granted: true},
-        %{resource: "MembershipFeeType", action: :create, scope: :all, granted: true},
-        %{resource: "MembershipFeeType", action: :update, scope: :all, granted: true},
-        %{resource: "MembershipFeeType", action: :destroy, scope: :all, granted: true},
-
-        # MembershipFeeCycle: Full CRUD
-        %{resource: "MembershipFeeCycle", action: :read, scope: :all, granted: true},
-        %{resource: "MembershipFeeCycle", action: :create, scope: :all, granted: true},
-        %{resource: "MembershipFeeCycle", action: :update, scope: :all, granted: true},
-        %{resource: "MembershipFeeCycle", action: :destroy, scope: :all, granted: true}
-      ],
+      resources:
+        perm_all("User") ++
+          perm_all("Member") ++
+          perm_all("CustomFieldValue") ++
+          perm_all("CustomField") ++
+          perm_all("Role") ++
+          perm_all("Group") ++
+          member_group_perms ++
+          perm_all("MembershipFeeType") ++
+          perm_all("MembershipFeeCycle"),
       pages: [
         # Explicit admin-only pages (for clarity and future restrictions)
         "/settings",