Update documentation: Remove NoActor bypass references

This commit is contained in:
Moritz 2026-01-23 20:18:28 +01:00
parent 0f48a9b15a
commit d9eb131d96
Signed by: moritz
GPG key ID: 1020A035E5DD0824
5 changed files with 73 additions and 137 deletions

View file

@ -690,16 +690,9 @@ end
**Authorization Bootstrap Patterns:** **Authorization Bootstrap Patterns:**
Three mechanisms exist for bypassing standard authorization: Two mechanisms exist for bypassing standard authorization:
1. **NoActor** (test only) - Allows operations without actor in test environment 1. **system_actor** (systemic operations) - Admin user for operations that must always succeed
```elixir
# Automatically enabled in tests via config/test.exs
# Policies use: bypass action_type(...) do authorize_if NoActor end
member = create_member(%{name: "Test"}) # Works in tests
```
2. **system_actor** (systemic operations) - Admin user for operations that must always succeed
```elixir ```elixir
# Good: Systemic operation # Good: Systemic operation
system_actor = SystemActor.get_system_actor() system_actor = SystemActor.get_system_actor()
@ -709,7 +702,7 @@ Three mechanisms exist for bypassing standard authorization:
# Never use system_actor for user-initiated actions! # Never use system_actor for user-initiated actions!
``` ```
3. **authorize?: false** (bootstrap only) - Skips policies for circular dependencies 2. **authorize?: false** (bootstrap only) - Skips policies for circular dependencies
```elixir ```elixir
# Good: Bootstrap (seeds, SystemActor loading) # Good: Bootstrap (seeds, SystemActor loading)
Accounts.create_user!(%{email: admin_email}, authorize?: false) Accounts.create_user!(%{email: admin_email}, authorize?: false)
@ -719,10 +712,10 @@ Three mechanisms exist for bypassing standard authorization:
``` ```
**Decision Guide:** **Decision Guide:**
- Use **NoActor** for test fixtures (automatic via config) - Use **system_actor** for email sync, cycle generation, validations, and test fixtures
- Use **system_actor** for email sync, cycle generation, validations
- Use **authorize?: false** only for bootstrap (seeds, circular dependencies) - Use **authorize?: false** only for bootstrap (seeds, circular dependencies)
- Always document why `authorize?: false` is necessary - Always document why `authorize?: false` is necessary
- **Note:** NoActor bypass was removed to prevent masking authorization bugs in tests
**See also:** `docs/roles-and-permissions-architecture.md` (Authorization Bootstrap Patterns section) **See also:** `docs/roles-and-permissions-architecture.md` (Authorization Bootstrap Patterns section)
@ -1702,65 +1695,54 @@ case Ash.read(Mv.Membership.Member, actor: actor) do
end end
``` ```
### 5.1a NoActor Pattern - Test Environment Only ### 5.1a Authorization in Tests
**IMPORTANT:** The `Mv.Authorization.Checks.NoActor` check is **ONLY for test environment**. It must NEVER be used in production. **IMPORTANT:** All tests must explicitly provide an actor for Ash operations. The NoActor bypass has been removed to prevent masking authorization bugs.
**What NoActor Does:** **Test Fixtures:**
- Allows CRUD operations without an actor in **test environment only** All test fixtures use `system_actor` for authorization:
- Denies all operations without an actor in **production/dev** (fail-closed)
- Uses compile-time config check to prevent accidental production use (release-safe)
**Security Guards:**
```elixir ```elixir
# config/test.exs # test/support/fixtures.ex
config :mv, :allow_no_actor_bypass, true def member_fixture(attrs \\ %{}) do
system_actor = Mv.Helpers.SystemActor.get_system_actor()
# lib/mv/authorization/checks/no_actor.ex attrs
# Compile-time check from config (release-safe, no Mix.env) |> Enum.into(%{...})
@allow_no_actor_bypass Application.compile_env(:mv, :allow_no_actor_bypass, false) |> Mv.Membership.create_member(actor: system_actor)
# Uses compile-time flag only (no runtime Mix.env needed)
def match?(nil, _context, _opts) do
@allow_no_actor_bypass # true in test, false in prod/dev
end end
``` ```
**Why This Pattern Exists:** **Why Explicit Actors in Tests:**
- Test fixtures often need to create resources without an actor - Prevents masking authorization bugs
- Production operations MUST always have an actor for security - Makes authorization requirements explicit
- Config-based guard (not Mix.env) ensures release-safety - Tests fail if authorization is broken (fail-fast)
- Defaults to `false` (fail-closed) if config not set - Consistent with production code patterns
**NEVER Use NoActor in Production:** **Using system_actor in Tests:**
```elixir ```elixir
# ❌ BAD - Don't do this in production code # ✅ GOOD - Explicit actor in tests
Ash.create!(Member, attrs) # No actor - will fail in prod system_actor = Mv.Helpers.SystemActor.get_system_actor()
# ✅ GOOD - Use admin actor for system operations
admin_user = get_admin_user()
Ash.create!(Member, attrs, actor: admin_user)
```
**Alternative: System Actor Pattern**
For production system operations, use the System Actor Pattern (see Section 3.3) instead of NoActor:
```elixir
# System operations in production
system_actor = get_system_actor()
Ash.create!(Member, attrs, actor: system_actor) Ash.create!(Member, attrs, actor: system_actor)
# ❌ BAD - Missing actor (will fail)
Ash.create!(Member, attrs) # Forbidden error!
``` ```
**Testing:** **For Bootstrap Operations:**
- NoActor tests verify the compile-time config guard Use `authorize?: false` only for bootstrap scenarios (seeds, SystemActor initialization):
- Production safety is guaranteed by config (only set in test.exs, defaults to false)
- See `test/mv/authorization/checks/no_actor_test.exs` ```elixir
# ✅ GOOD - Bootstrap only
Accounts.create_user!(%{email: admin_email}, authorize?: false)
# ❌ BAD - Never use in tests for normal operations
Ash.create!(Member, attrs, authorize?: false) # Never do this!
```
### 5.2 Password Security ### 5.2 Password Security

View file

@ -262,7 +262,7 @@ The bypass is not a design choice but a **technical necessity** due to Ash's pol
- ✅ UPDATE operations via HasPermission with `scope :own` - ✅ UPDATE operations via HasPermission with `scope :own`
- ✅ Admin operations via HasPermission with `scope :all` - ✅ Admin operations via HasPermission with `scope :all`
- ✅ AshAuthentication bypass (registration/login) - ✅ AshAuthentication bypass (registration/login)
- ✅ NoActor bypass (test environment) - ✅ Tests use system_actor for authorization
**Key Tests Proving Pattern:** **Key Tests Proving Pattern:**

View file

@ -946,12 +946,7 @@ defmodule Mv.Accounts.User do
authorize_if always() authorize_if always()
end end
# 2. NoActor Bypass (test environment only, for test fixtures) # 2. SPECIAL CASE: Users can always READ their own account
bypass action_type([:create, :read, :update, :destroy]) do
authorize_if Mv.Authorization.Checks.NoActor
end
# 3. SPECIAL CASE: Users can always READ their own account
# Bypass needed for list queries (expr() triggers auto_filter in Ash) # Bypass needed for list queries (expr() triggers auto_filter in Ash)
# UPDATE is handled by HasPermission below (scope :own works with changesets) # UPDATE is handled by HasPermission below (scope :own works with changesets)
bypass action_type(:read) do bypass action_type(:read) do
@ -959,7 +954,7 @@ defmodule Mv.Accounts.User do
authorize_if expr(id == ^actor(:id)) authorize_if expr(id == ^actor(:id))
end end
# 4. GENERAL: Check permissions from user's role # 3. GENERAL: Check permissions from user's role
# - :own_data → can UPDATE own user (scope :own via HasPermission) # - :own_data → can UPDATE own user (scope :own via HasPermission)
# - :read_only → can UPDATE own user (scope :own via HasPermission) # - :read_only → can UPDATE own user (scope :own via HasPermission)
# - :normal_user → can UPDATE own user (scope :own via HasPermission) # - :normal_user → can UPDATE own user (scope :own via HasPermission)
@ -969,7 +964,7 @@ defmodule Mv.Accounts.User do
authorize_if Mv.Authorization.Checks.HasPermission authorize_if Mv.Authorization.Checks.HasPermission
end end
# 5. DEFAULT: Ash implicitly forbids if no policy authorizes (fail-closed) # 4. DEFAULT: Ash implicitly forbids if no policy authorizes (fail-closed)
end end
# ... # ...
@ -1007,12 +1002,7 @@ defmodule Mv.Membership.Member do
use Ash.Resource, ... use Ash.Resource, ...
policies do policies do
# 1. NoActor Bypass (test environment only, for test fixtures) # 1. SPECIAL CASE: Users can always READ their linked member
bypass action_type([:create, :read, :update, :destroy]) do
authorize_if Mv.Authorization.Checks.NoActor
end
# 2. SPECIAL CASE: Users can always READ their linked member
# Bypass needed for list queries (expr() triggers auto_filter in Ash) # Bypass needed for list queries (expr() triggers auto_filter in Ash)
# UPDATE is handled by HasPermission below (scope :linked works with changesets) # UPDATE is handled by HasPermission below (scope :linked works with changesets)
bypass action_type(:read) do bypass action_type(:read) do
@ -1020,7 +1010,7 @@ defmodule Mv.Membership.Member do
authorize_if expr(id == ^actor(:member_id)) authorize_if expr(id == ^actor(:member_id))
end end
# 3. GENERAL: Check permissions from role # 2. GENERAL: Check permissions from role
# - :own_data → can UPDATE linked member (scope :linked via HasPermission) # - :own_data → can UPDATE linked member (scope :linked via HasPermission)
# - :read_only → can READ all members (scope :all), no update permission # - :read_only → can READ all members (scope :all), no update permission
# - :normal_user → can CRUD all members (scope :all) # - :normal_user → can CRUD all members (scope :all)
@ -2629,45 +2619,16 @@ This section clarifies three different mechanisms for bypassing standard authori
### Overview ### Overview
The codebase uses three authorization bypass mechanisms: The codebase uses two authorization bypass mechanisms:
1. **NoActor** - Test-only bypass (compile-time secured) 1. **system_actor** - Admin user for systemic operations
2. **system_actor** - Admin user for systemic operations 2. **authorize?: false** - Bootstrap bypass for circular dependencies
3. **authorize?: false** - Bootstrap bypass for circular dependencies
**All three are necessary and serve different purposes.** **Both are necessary and serve different purposes.**
### 1. NoActor Check **Note:** The NoActor bypass has been removed to prevent masking authorization bugs in tests. All tests now explicitly use `system_actor` for authorization.
**Purpose:** Allows CRUD operations without actor in test environment only. ### 1. System Actor
**Implementation:**
```elixir
# lib/mv/authorization/checks/no_actor.ex
@allow_no_actor_bypass Application.compile_env(:mv, :allow_no_actor_bypass, false)
def match?(nil, _context, _opts) do
@allow_no_actor_bypass # true in test.exs, false elsewhere
end
```
**Security:**
- Compile-time flag (not runtime `Mix.env()` check)
- Default: false (fail-closed)
- Only enabled in `config/test.exs`
**Use Case:** Test fixtures without verbose actor setup:
```elixir
# With NoActor (test environment only)
member = create_member(%{name: "Test"})
# Production behavior (NoActor returns false)
member = create_member(%{name: "Test"}, actor: user)
```
**Trade-off:** May mask tests that should fail without actor. Mitigated by explicit policy tests (e.g., `test/mv/accounts/user_policies_test.exs`).
### 2. System Actor
**Purpose:** Admin user for systemic operations that must always succeed regardless of user permissions. **Purpose:** Admin user for systemic operations that must always succeed regardless of user permissions.
@ -2708,7 +2669,7 @@ end
- Consistent authorization flow - Consistent authorization flow
- Testable - Testable
### 3. authorize?: false ### 2. authorize?: false
**Purpose:** Skip policies for bootstrap scenarios with circular dependencies. **Purpose:** Skip policies for bootstrap scenarios with circular dependencies.
@ -2759,21 +2720,17 @@ Mv.Authorization.Role
### Comparison ### Comparison
| Aspect | NoActor | system_actor | authorize?: false | | Aspect | system_actor | authorize?: false |
|--------|---------|--------------|-------------------| |--------|--------------|-------------------|
| **Environment** | Test only | All | All | | **Environment** | All | All |
| **Actor** | nil | Admin user | nil | | **Actor** | Admin user | nil |
| **Policies** | Bypassed | Evaluated | Skipped | | **Policies** | Evaluated | Skipped |
| **Audit Trail** | No | Yes (system@mila.local) | No | | **Audit Trail** | Yes (system@mila.local) | No |
| **Use Case** | Test fixtures | Systemic operations | Bootstrap | | **Use Case** | Systemic operations, test fixtures | Bootstrap |
| **Explicit?** | Policy bypass | Function call | Query option | | **Explicit?** | Function call | Query option |
### Decision Guide ### Decision Guide
**Use NoActor when:**
- ✅ Writing test fixtures
- ✅ Compile-time guard ensures test-only
**Use system_actor when:** **Use system_actor when:**
- ✅ Systemic operation must always succeed - ✅ Systemic operation must always succeed
- ✅ Email synchronization - ✅ Email synchronization
@ -2789,7 +2746,7 @@ Mv.Authorization.Role
**DON'T:** **DON'T:**
- ❌ Use `authorize?: false` for user-initiated actions - ❌ Use `authorize?: false` for user-initiated actions
- ❌ Use `authorize?: false` when `system_actor` would work - ❌ Use `authorize?: false` when `system_actor` would work
- ❌ Enable NoActor outside test environment - ❌ Skip actor in tests (always use system_actor)
### The Circular Dependency Problem ### The Circular Dependency Problem
@ -2873,7 +2830,8 @@ end
- Enhanced edge case documentation - Enhanced edge case documentation
**Changes from V2.0:** **Changes from V2.0:**
- Added "Authorization Bootstrap Patterns" section explaining NoActor, system_actor, and authorize?: false - Added "Authorization Bootstrap Patterns" section explaining system_actor and authorize?: false
- Removed NoActor bypass (all tests now use system_actor for explicit authorization)
--- ---

View file

@ -542,7 +542,7 @@ Following the same pattern as Member resource:
1. ✅ Open `lib/accounts/user.ex` 1. ✅ Open `lib/accounts/user.ex`
2. ✅ Add `policies` block 2. ✅ Add `policies` block
3. ✅ Add AshAuthentication bypass (registration/login without actor) 3. ✅ Add AshAuthentication bypass (registration/login without actor)
4. ✅ Add NoActor bypass (test environment only) 4. ✅ ~~Add NoActor bypass (test environment only)~~ **REMOVED** - NoActor bypass was removed to prevent masking authorization bugs. All tests now use `system_actor`.
5. ✅ Add bypass for READ: Allow user to always read their own account 5. ✅ Add bypass for READ: Allow user to always read their own account
```elixir ```elixir
bypass action_type(:read) do bypass action_type(:read) do
@ -556,10 +556,11 @@ Following the same pattern as Member resource:
**Policy Order:** **Policy Order:**
1. ✅ AshAuthentication bypass (registration/login) 1. ✅ AshAuthentication bypass (registration/login)
2. ✅ NoActor bypass (test environment) 2. ✅ Bypass: User can READ own account (id == actor.id)
3. ✅ Bypass: User can READ own account (id == actor.id) 3. ✅ HasPermission: General permission check (UPDATE uses scope :own, admin uses scope :all)
4. ✅ HasPermission: General permission check (UPDATE uses scope :own, admin uses scope :all) 4. ✅ Default: Ash implicitly forbids (fail-closed)
5. ✅ Default: Ash implicitly forbids (fail-closed)
**Note:** NoActor bypass was removed. All tests now use `system_actor` for authorization.
**Why Bypass for READ but not UPDATE?** **Why Bypass for READ but not UPDATE?**
@ -574,7 +575,7 @@ This ensures `scope :own` in PermissionSets is actually used (not redundant).
- ✅ User can always update own credentials (via HasPermission with scope :own) - ✅ User can always update own credentials (via HasPermission with scope :own)
- ✅ Only admin can read/update other users (scope :all) - ✅ Only admin can read/update other users (scope :all)
- ✅ Only admin can destroy users (scope :all) - ✅ Only admin can destroy users (scope :all)
- ✅ Policy order is correct (AshAuth → NoActor → Bypass READ → HasPermission) - ✅ Policy order is correct (AshAuth → Bypass READ → HasPermission)
- ✅ Actor preloads :role relationship - ✅ Actor preloads :role relationship
- ✅ All tests pass (30/31 pass, 1 skipped) - ✅ All tests pass (30/31 pass, 1 skipped)
@ -584,7 +585,7 @@ This ensures `scope :own` in PermissionSets is actually used (not redundant).
- ✅ 31 tests total: 30 passing, 1 skipped (AshAuthentication edge case) - ✅ 31 tests total: 30 passing, 1 skipped (AshAuthentication edge case)
- ✅ Tests for all 4 permission sets: own_data, read_only, normal_user, admin - ✅ Tests for all 4 permission sets: own_data, read_only, normal_user, admin
- ✅ Tests for AshAuthentication bypass (registration/login) - ✅ Tests for AshAuthentication bypass (registration/login)
- ✅ Tests for NoActor bypass (test environment) - ✅ Tests use system_actor for authorization (NoActor bypass removed)
- ✅ Tests verify scope :own is used for UPDATE (not redundant) - ✅ Tests verify scope :own is used for UPDATE (not redundant)
--- ---

View file

@ -22,18 +22,13 @@ policies do
authorize_if always() authorize_if always()
end end
# 2. NoActor Bypass (test environment only) # 2. Bypass for READ (list queries via auto_filter)
bypass action_type([:create, :read, :update, :destroy]) do
authorize_if Mv.Authorization.Checks.NoActor
end
# 3. Bypass for READ (list queries via auto_filter)
bypass action_type(:read) do bypass action_type(:read) do
description "Users can always read their own account" description "Users can always read their own account"
authorize_if expr(id == ^actor(:id)) authorize_if expr(id == ^actor(:id))
end end
# 4. HasPermission for all operations (uses scope from PermissionSets) # 3. HasPermission for all operations (uses scope from PermissionSets)
policy action_type([:read, :create, :update, :destroy]) do policy action_type([:read, :create, :update, :destroy]) do
description "Check permissions from user's role and permission set" description "Check permissions from user's role and permission set"
authorize_if Mv.Authorization.Checks.HasPermission authorize_if Mv.Authorization.Checks.HasPermission
@ -51,7 +46,7 @@ end
- ✅ CREATE operations (admin only) - ✅ CREATE operations (admin only)
- ✅ DESTROY operations (admin only) - ✅ DESTROY operations (admin only)
- ✅ AshAuthentication bypass (registration/login) - ✅ AshAuthentication bypass (registration/login)
- ✅ NoActor bypass (test environment) - ✅ Tests use system_actor for authorization
--- ---
@ -190,7 +185,7 @@ mix test test/mv/accounts/user_policies_test.exs \
**Test Environment:** **Test Environment:**
- ✅ Operations without actor work in test environment - ✅ Operations without actor work in test environment
- ✅ NoActor bypass correctly detects compile-time environment - ✅ All tests explicitly use system_actor for authorization
--- ---