From e72b7ab2e84c443b79e8769a496896d00822a161 Mon Sep 17 00:00:00 2001 From: Moritz Date: Fri, 23 Jan 2026 20:00:18 +0100 Subject: [PATCH] Remove NoActor bypass from User and Member policies This removes the NoActor bypass that was masking authorization bugs in tests. All operations now require an explicit actor for authorization. --- lib/accounts/user.ex | 6 ------ lib/membership/member.ex | 17 ++++++----------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex index 08d1130..badbd72 100644 --- a/lib/accounts/user.ex +++ b/lib/accounts/user.ex @@ -275,12 +275,6 @@ defmodule Mv.Accounts.User do authorize_if always() end - # NoActor bypass (test fixtures only, see no_actor.ex) - bypass action_type([:create, :read, :update, :destroy]) do - description "Allow system operations without actor (test environment only)" - authorize_if Mv.Authorization.Checks.NoActor - end - # READ bypass for list queries (scope :own via expr) bypass action_type(:read) do description "Users can always read their own account" diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 650cf43..0a14efe 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -303,15 +303,6 @@ defmodule Mv.Membership.Member do # Authorization Policies # Order matters: Most specific policies first, then general permission check policies do - # SYSTEM OPERATIONS: Allow CRUD operations without actor (TEST ENVIRONMENT ONLY) - # In test: All operations allowed (for test fixtures) - # In production/dev: ALL operations denied without actor (fail-closed for security) - # NoActor.check uses compile-time environment detection to prevent security issues - bypass action_type([:create, :read, :update, :destroy]) do - description "Allow system operations without actor (test environment only)" - authorize_if Mv.Authorization.Checks.NoActor - end - # SPECIAL CASE: Users can always READ their linked member # This allows users with ANY permission set to read their own linked member # Check using the inverse relationship: User.member_id → Member.id @@ -403,8 +394,12 @@ defmodule Mv.Membership.Member do current_member_id = changeset.data.id # Get actor from changeset context for authorization - # If no actor is present, this will fail in production (fail-closed) - actor = Map.get(changeset.context || %{}, :actor) + # Use system_actor as fallback if no actor is present (for systemic operations) + actor = + case Map.get(changeset.context || %{}, :actor) do + nil -> Mv.Helpers.SystemActor.get_system_actor() + actor -> actor + end # Check the current state of the user in the database # Check if authorization is disabled in the parent operation's context