From de187190e44cbd31506cbb46666de4cf621a6c59 Mon Sep 17 00:00:00 2001
From: Moritz <moritz.m@local-it.org>
Date: Thu, 22 Jan 2026 19:19:22 +0100
Subject: [PATCH] feat(auth): add User resource authorization policies

Implement bypass for READ + HasPermission for UPDATE pattern
Extend HasPermission check to support User resource scope :own
---
 lib/accounts/user.ex                          | 50 +++++++++++++++++--
 lib/mv/authorization/checks/has_permission.ex | 27 +++++++++-
 2 files changed, 72 insertions(+), 5 deletions(-)

diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex
index 9598b76..3f0381d 100644
--- a/lib/accounts/user.ex
+++ b/lib/accounts/user.ex
@@ -5,9 +5,8 @@ defmodule Mv.Accounts.User do
   use Ash.Resource,
     domain: Mv.Accounts,
     data_layer: AshPostgres.DataLayer,
-    extensions: [AshAuthentication]
-
-  # authorizers: [Ash.Policy.Authorizer]
+    extensions: [AshAuthentication],
+    authorizers: [Ash.Policy.Authorizer]
 
   postgres do
     table "users"
@@ -267,6 +266,51 @@ defmodule Mv.Accounts.User do
     end
   end
 
+  # Authorization Policies
+  # Order matters: Most specific policies first, then general permission check
+  policies do
+    # ASHAUTHENTICATION BYPASS: Allow authentication actions (registration, login)
+    # These actions are called internally by AshAuthentication and need to bypass
+    # normal authorization policies. This must come FIRST because User is an
+    # authentication resource and authentication flows should have priority.
+    bypass AshAuthentication.Checks.AshAuthenticationInteraction do
+      description "Allow AshAuthentication internal operations (registration, login)"
+      authorize_if always()
+    end
+
+    # 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 own account
+    # This allows users with ANY permission set to read their own user record
+    # Uses bypass with expr filter to enable auto_filter behavior for reads/lists
+    # (consistent with Member "always read linked member" pattern)
+    bypass action_type(:read) do
+      description "Users can always read their own account"
+      authorize_if expr(id == ^actor(:id))
+    end
+
+    # GENERAL: Check permissions from user's role
+    # HasPermission handles permissions correctly:
+    # - :own_data → can update own user (scope :own)
+    # - :read_only → can update own user (scope :own)
+    # - :normal_user → can update own user (scope :own)
+    # - :admin → can read/create/update/destroy all users (scope :all)
+    policy action_type([:read, :create, :update, :destroy]) do
+      description "Check permissions from user's role and permission set"
+      authorize_if Mv.Authorization.Checks.HasPermission
+    end
+
+    # DEFAULT: Ash implicitly forbids if no policy authorizes
+    # No explicit forbid needed, as Ash's default behavior is fail-closed
+  end
+
   # Global validations - applied to all relevant actions
   validations do
     # Password strength policy: minimum 8 characters for all password-related actions
diff --git a/lib/mv/authorization/checks/has_permission.ex b/lib/mv/authorization/checks/has_permission.ex
index 18ffe5b..f3192fb 100644
--- a/lib/mv/authorization/checks/has_permission.ex
+++ b/lib/mv/authorization/checks/has_permission.ex
@@ -95,11 +95,25 @@ defmodule Mv.Authorization.Checks.HasPermission do
              resource_name
            ) do
         :authorized ->
+          # For :all scope, authorize directly
           {:ok, true}
 
         {:filter, filter_expr} ->
-          # For strict_check on single records, evaluate the filter against the record
-          evaluate_filter_for_strict_check(filter_expr, actor, record, resource_name)
+          # For :own/:linked scope:
+          # - With a record, evaluate filter against record for strict authorization
+          # - Without a record (queries/lists), return false
+          #
+          # NOTE: Returning false here forces the use of expr-based bypass policies.
+          # This is necessary because Ash's policy evaluation doesn't reliably call auto_filter
+          # when strict_check returns :unknown. Instead, resources should use bypass policies
+          # with expr() directly for filter-based authorization (see User resource).
+          if record do
+            evaluate_filter_for_strict_check(filter_expr, actor, record, resource_name)
+          else
+            # No record yet (e.g., read/list queries) - deny at strict_check level
+            # Resources must use expr-based bypass policies for list filtering
+            {:ok, false}
+          end
 
         false ->
           {:ok, false}
@@ -224,9 +238,18 @@ defmodule Mv.Authorization.Checks.HasPermission do
   end
 
   # Evaluate filter expression for strict_check on single records
+  # For :own scope with User resource: id == actor.id
   # For :linked scope with Member resource: id == actor.member_id
   defp evaluate_filter_for_strict_check(_filter_expr, actor, record, resource_name) do
     case {resource_name, record} do
+      {"User", %{id: user_id}} when not is_nil(user_id) ->
+        # Check if this user's ID matches the actor's ID (scope :own)
+        if user_id == actor.id do
+          {:ok, true}
+        else
+          {:ok, false}
+        end
+
       {"Member", %{id: member_id}} when not is_nil(member_id) ->
         # Check if this member's ID matches the actor's member_id
         if member_id == actor.member_id do