From 69836978bed9946000f6a97e98e2f39d964c8609 Mon Sep 17 00:00:00 2001 From: Moritz Date: Thu, 22 Jan 2026 22:37:09 +0100 Subject: [PATCH] Remove unused PolicyHelpers macro and PolicyConsistency test Dead code - macro was never used in codebase. PolicyConsistency test will be replaced with better implementation. --- lib/mv/authorization/policy_helpers.ex | 40 ------- .../authorization/policy_consistency_test.exs | 104 ------------------ 2 files changed, 144 deletions(-) delete mode 100644 lib/mv/authorization/policy_helpers.ex delete mode 100644 test/mv/authorization/policy_consistency_test.exs diff --git a/lib/mv/authorization/policy_helpers.ex b/lib/mv/authorization/policy_helpers.ex deleted file mode 100644 index f61ecb7..0000000 --- a/lib/mv/authorization/policy_helpers.ex +++ /dev/null @@ -1,40 +0,0 @@ -defmodule Mv.Authorization.PolicyHelpers do - @moduledoc """ - Policy helpers for consistent bypass vs HasPermission patterns. - - ## Pattern: READ Bypass + UPDATE HasPermission - - For resources with scope :own/:linked permissions: - - READ: Use bypass with expr() for auto_filter - - UPDATE/CREATE/DESTROY: Use HasPermission for scope evaluation - - ## Usage - - use Mv.Authorization.PolicyHelpers - - policies do - # Standard pattern for User resource - standard_user_policies() - end - - ## Why This Pattern? - - See `docs/policy-bypass-vs-haspermission.md` for detailed explanation. - """ - - defmacro standard_user_policies do - quote do - # READ: Bypass for auto_filter - bypass action_type(:read) do - description "Users can read their own records" - authorize_if expr(id == ^actor(:id)) - end - - # UPDATE/CREATE/DESTROY: HasPermission - policy action_type([:update, :create, :destroy]) do - description "Check permissions from role" - authorize_if Mv.Authorization.Checks.HasPermission - end - end - end -end diff --git a/test/mv/authorization/policy_consistency_test.exs b/test/mv/authorization/policy_consistency_test.exs deleted file mode 100644 index a7cbfc8..0000000 --- a/test/mv/authorization/policy_consistency_test.exs +++ /dev/null @@ -1,104 +0,0 @@ -defmodule Mv.Authorization.PolicyConsistencyTest do - @moduledoc """ - Tests to ensure policy consistency across resources. - - Verifies that resources with scope :own/:linked permissions for READ - have corresponding READ bypass policies (as required by the two-tier pattern). - """ - use ExUnit.Case, async: true - - alias Mv.Authorization.PermissionSets - - describe "Policy Pattern Consistency" do - test "resources with scope :own/:linked for READ have READ bypass" do - # Get all permission sets - permission_sets = PermissionSets.all_permission_sets() - - # Collect all resources with scope :own/:linked for READ - resources_with_filter_scope = - for permission_set <- permission_sets, - permissions = PermissionSets.get_permissions(permission_set), - perm <- permissions.resources, - perm.action == :read, - perm.scope in [:own, :linked], - perm.granted == true, - do: perm.resource - - # Remove duplicates - unique_resources = Enum.uniq(resources_with_filter_scope) - - # Expected resources that should have READ bypass - expected_resources = ["User", "Member", "CustomFieldValue"] - - # Verify all expected resources are in the list - for resource <- expected_resources do - assert resource in unique_resources, - "Resource #{resource} has scope :own/:linked for READ but may not have READ bypass policy. " <> - "See docs/policy-bypass-vs-haspermission.md for the two-tier pattern." - end - end - - test "resources with scope :own/:linked for UPDATE use HasPermission" do - # Get all permission sets - permission_sets = PermissionSets.all_permission_sets() - - # Collect all resources with scope :own/:linked for UPDATE - resources_with_filter_scope = - for permission_set <- permission_sets, - permissions = PermissionSets.get_permissions(permission_set), - perm <- permissions.resources, - perm.action == :update, - perm.scope in [:own, :linked], - perm.granted == true, - do: perm.resource - - # Remove duplicates - unique_resources = Enum.uniq(resources_with_filter_scope) - - # Expected resources that should use HasPermission for UPDATE - expected_resources = ["User", "Member", "CustomFieldValue"] - - # Verify all expected resources are in the list - for resource <- expected_resources do - assert resource in unique_resources, - "Resource #{resource} should use HasPermission for UPDATE with scope :own/:linked. " <> - "See docs/policy-bypass-vs-haspermission.md for the two-tier pattern." - end - end - - test "all permission sets grant User.update (own or all)" do - # Verify that all permission sets grant User.update - # - :own_data, :read_only, :normal_user grant User.update :own - # - :admin grants User.update :all (can update all users) - permission_sets = PermissionSets.all_permission_sets() - - for permission_set <- permission_sets do - permissions = PermissionSets.get_permissions(permission_set) - - user_update_perm = - Enum.find(permissions.resources, fn perm -> - perm.resource == "User" and perm.action == :update - end) - - assert user_update_perm != nil, - "Permission set #{permission_set} must grant User.update. " <> - "All permission sets must allow users to update credentials." - - assert user_update_perm.scope in [:own, :all], - "Permission set #{permission_set} must grant User.update with scope :own or :all. " <> - "Current scope: #{user_update_perm.scope}" - - assert user_update_perm.granted == true, - "Permission set #{permission_set} must grant User.update. " <> - "Current granted: #{user_update_perm.granted}" - - # Non-admin sets should use :own - if permission_set != :admin do - assert user_update_perm.scope == :own, - "Permission set #{permission_set} must grant User.update with scope :own. " <> - "Current scope: #{user_update_perm.scope}" - end - end - end - end -end