Update documentation: Remove NoActor bypass references
This commit is contained in:
parent
c715a45277
commit
b387897adb
GPG key ID:
40E7A58C4AA1EDB2
5 changed files with 73 additions and 137 deletions
|
|
@ -690,16 +690,9 @@ end
|
|||
|
||||
**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
|
||||
```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
|
||||
1. **system_actor** (systemic operations) - Admin user for operations that must always succeed
|
||||
```elixir
|
||||
# Good: Systemic operation
|
||||
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!
|
||||
```
|
||||
|
||||
3. **authorize?: false** (bootstrap only) - Skips policies for circular dependencies
|
||||
2. **authorize?: false** (bootstrap only) - Skips policies for circular dependencies
|
||||
```elixir
|
||||
# Good: Bootstrap (seeds, SystemActor loading)
|
||||
Accounts.create_user!(%{email: admin_email}, authorize?: false)
|
||||
|
|
@ -719,10 +712,10 @@ Three mechanisms exist for bypassing standard authorization:
|
|||
```
|
||||
|
||||
**Decision Guide:**
|
||||
- Use **NoActor** for test fixtures (automatic via config)
|
||||
- Use **system_actor** for email sync, cycle generation, validations
|
||||
- Use **system_actor** for email sync, cycle generation, validations, and test fixtures
|
||||
- Use **authorize?: false** only for bootstrap (seeds, circular dependencies)
|
||||
- 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)
|
||||
|
||||
|
|
@ -1702,65 +1695,54 @@ case Ash.read(Mv.Membership.Member, actor: actor) do
|
|||
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**
|
||||
- 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:**
|
||||
All test fixtures use `system_actor` for authorization:
|
||||
|
||||
```elixir
|
||||
# config/test.exs
|
||||
config :mv, :allow_no_actor_bypass, true
|
||||
|
||||
# lib/mv/authorization/checks/no_actor.ex
|
||||
# Compile-time check from config (release-safe, no Mix.env)
|
||||
@allow_no_actor_bypass Application.compile_env(:mv, :allow_no_actor_bypass, false)
|
||||
|
||||
# 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
|
||||
# test/support/fixtures.ex
|
||||
def member_fixture(attrs \\ %{}) do
|
||||
system_actor = Mv.Helpers.SystemActor.get_system_actor()
|
||||
|
||||
attrs
|
||||
|> Enum.into(%{...})
|
||||
|> Mv.Membership.create_member(actor: system_actor)
|
||||
end
|
||||
```
|
||||
|
||||
**Why This Pattern Exists:**
|
||||
**Why Explicit Actors in Tests:**
|
||||
|
||||
- Test fixtures often need to create resources without an actor
|
||||
- Production operations MUST always have an actor for security
|
||||
- Config-based guard (not Mix.env) ensures release-safety
|
||||
- Defaults to `false` (fail-closed) if config not set
|
||||
- Prevents masking authorization bugs
|
||||
- Makes authorization requirements explicit
|
||||
- Tests fail if authorization is broken (fail-fast)
|
||||
- Consistent with production code patterns
|
||||
|
||||
**NEVER Use NoActor in Production:**
|
||||
**Using system_actor in Tests:**
|
||||
|
||||
```elixir
|
||||
# ❌ BAD - Don't do this in production code
|
||||
Ash.create!(Member, attrs) # No actor - will fail in prod
|
||||
|
||||
# ✅ 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()
|
||||
# ✅ GOOD - Explicit actor in tests
|
||||
system_actor = Mv.Helpers.SystemActor.get_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
|
||||
- Production safety is guaranteed by config (only set in test.exs, defaults to false)
|
||||
- See `test/mv/authorization/checks/no_actor_test.exs`
|
||||
Use `authorize?: false` only for bootstrap scenarios (seeds, SystemActor initialization):
|
||||
|
||||
```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
|
||||
|
||||
|
|
|
|||
|
|
@ -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`
|
||||
- ✅ Admin operations via HasPermission with `scope :all`
|
||||
- ✅ AshAuthentication bypass (registration/login)
|
||||
- ✅ NoActor bypass (test environment)
|
||||
- ✅ Tests use system_actor for authorization
|
||||
|
||||
**Key Tests Proving Pattern:**
|
||||
|
||||
|
|
|
|||
|
|
@ -946,12 +946,7 @@ defmodule Mv.Accounts.User do
|
|||
authorize_if always()
|
||||
end
|
||||
|
||||
# 2. NoActor Bypass (test environment only, for test fixtures)
|
||||
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
|
||||
# 2. SPECIAL CASE: Users can always READ their own account
|
||||
# Bypass needed for list queries (expr() triggers auto_filter in Ash)
|
||||
# UPDATE is handled by HasPermission below (scope :own works with changesets)
|
||||
bypass action_type(:read) do
|
||||
|
|
@ -959,7 +954,7 @@ defmodule Mv.Accounts.User do
|
|||
authorize_if expr(id == ^actor(:id))
|
||||
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)
|
||||
# - :read_only → 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
|
||||
end
|
||||
|
||||
# 5. DEFAULT: Ash implicitly forbids if no policy authorizes (fail-closed)
|
||||
# 4. DEFAULT: Ash implicitly forbids if no policy authorizes (fail-closed)
|
||||
end
|
||||
|
||||
# ...
|
||||
|
|
@ -1007,12 +1002,7 @@ defmodule Mv.Membership.Member do
|
|||
use Ash.Resource, ...
|
||||
|
||||
policies do
|
||||
# 1. NoActor Bypass (test environment only, for test fixtures)
|
||||
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
|
||||
# 1. SPECIAL CASE: Users can always READ their linked member
|
||||
# Bypass needed for list queries (expr() triggers auto_filter in Ash)
|
||||
# UPDATE is handled by HasPermission below (scope :linked works with changesets)
|
||||
bypass action_type(:read) do
|
||||
|
|
@ -1020,7 +1010,7 @@ defmodule Mv.Membership.Member do
|
|||
authorize_if expr(id == ^actor(:member_id))
|
||||
end
|
||||
|
||||
# 3. GENERAL: Check permissions from role
|
||||
# 2. GENERAL: Check permissions from role
|
||||
# - :own_data → can UPDATE linked member (scope :linked via HasPermission)
|
||||
# - :read_only → can READ all members (scope :all), no update permission
|
||||
# - :normal_user → can CRUD all members (scope :all)
|
||||
|
|
@ -2629,45 +2619,16 @@ This section clarifies three different mechanisms for bypassing standard authori
|
|||
|
||||
### Overview
|
||||
|
||||
The codebase uses three authorization bypass mechanisms:
|
||||
The codebase uses two authorization bypass mechanisms:
|
||||
|
||||
1. **NoActor** - Test-only bypass (compile-time secured)
|
||||
2. **system_actor** - Admin user for systemic operations
|
||||
3. **authorize?: false** - Bootstrap bypass for circular dependencies
|
||||
1. **system_actor** - Admin user for systemic operations
|
||||
2. **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.
|
||||
|
||||
**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
|
||||
### 1. System Actor
|
||||
|
||||
**Purpose:** Admin user for systemic operations that must always succeed regardless of user permissions.
|
||||
|
||||
|
|
@ -2708,7 +2669,7 @@ end
|
|||
- Consistent authorization flow
|
||||
- Testable
|
||||
|
||||
### 3. authorize?: false
|
||||
### 2. authorize?: false
|
||||
|
||||
**Purpose:** Skip policies for bootstrap scenarios with circular dependencies.
|
||||
|
||||
|
|
@ -2759,21 +2720,17 @@ Mv.Authorization.Role
|
|||
|
||||
### Comparison
|
||||
|
||||
| Aspect | NoActor | system_actor | authorize?: false |
|
||||
|--------|---------|--------------|-------------------|
|
||||
| **Environment** | Test only | All | All |
|
||||
| **Actor** | nil | Admin user | nil |
|
||||
| **Policies** | Bypassed | Evaluated | Skipped |
|
||||
| **Audit Trail** | No | Yes (system@mila.local) | No |
|
||||
| **Use Case** | Test fixtures | Systemic operations | Bootstrap |
|
||||
| **Explicit?** | Policy bypass | Function call | Query option |
|
||||
| Aspect | system_actor | authorize?: false |
|
||||
|--------|--------------|-------------------|
|
||||
| **Environment** | All | All |
|
||||
| **Actor** | Admin user | nil |
|
||||
| **Policies** | Evaluated | Skipped |
|
||||
| **Audit Trail** | Yes (system@mila.local) | No |
|
||||
| **Use Case** | Systemic operations, test fixtures | Bootstrap |
|
||||
| **Explicit?** | Function call | Query option |
|
||||
|
||||
### Decision Guide
|
||||
|
||||
**Use NoActor when:**
|
||||
- ✅ Writing test fixtures
|
||||
- ✅ Compile-time guard ensures test-only
|
||||
|
||||
**Use system_actor when:**
|
||||
- ✅ Systemic operation must always succeed
|
||||
- ✅ Email synchronization
|
||||
|
|
@ -2789,7 +2746,7 @@ Mv.Authorization.Role
|
|||
**DON'T:**
|
||||
- ❌ Use `authorize?: false` for user-initiated actions
|
||||
- ❌ 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
|
||||
|
||||
|
|
@ -2873,7 +2830,8 @@ end
|
|||
- Enhanced edge case documentation
|
||||
|
||||
**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)
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -542,7 +542,7 @@ Following the same pattern as Member resource:
|
|||
1. ✅ Open `lib/accounts/user.ex`
|
||||
2. ✅ Add `policies` block
|
||||
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
|
||||
```elixir
|
||||
bypass action_type(:read) do
|
||||
|
|
@ -556,10 +556,11 @@ Following the same pattern as Member resource:
|
|||
|
||||
**Policy Order:**
|
||||
1. ✅ AshAuthentication bypass (registration/login)
|
||||
2. ✅ NoActor bypass (test environment)
|
||||
3. ✅ Bypass: User can READ own account (id == actor.id)
|
||||
4. ✅ HasPermission: General permission check (UPDATE uses scope :own, admin uses scope :all)
|
||||
5. ✅ Default: Ash implicitly forbids (fail-closed)
|
||||
2. ✅ Bypass: User can READ own account (id == actor.id)
|
||||
3. ✅ HasPermission: General permission check (UPDATE uses scope :own, admin uses scope :all)
|
||||
4. ✅ 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?**
|
||||
|
||||
|
|
@ -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)
|
||||
- ✅ Only admin can read/update other 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
|
||||
- ✅ 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)
|
||||
- ✅ Tests for all 4 permission sets: own_data, read_only, normal_user, admin
|
||||
- ✅ 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)
|
||||
|
||||
---
|
||||
|
|
|
|||
|
|
@ -22,18 +22,13 @@ policies do
|
|||
authorize_if always()
|
||||
end
|
||||
|
||||
# 2. NoActor Bypass (test environment only)
|
||||
bypass action_type([:create, :read, :update, :destroy]) do
|
||||
authorize_if Mv.Authorization.Checks.NoActor
|
||||
end
|
||||
|
||||
# 3. Bypass for READ (list queries via auto_filter)
|
||||
# 2. Bypass for READ (list queries via auto_filter)
|
||||
bypass action_type(:read) do
|
||||
description "Users can always read their own account"
|
||||
authorize_if expr(id == ^actor(:id))
|
||||
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
|
||||
description "Check permissions from user's role and permission set"
|
||||
authorize_if Mv.Authorization.Checks.HasPermission
|
||||
|
|
@ -51,7 +46,7 @@ end
|
|||
- ✅ CREATE operations (admin only)
|
||||
- ✅ DESTROY operations (admin only)
|
||||
- ✅ 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:**
|
||||
- ✅ Operations without actor work in test environment
|
||||
- ✅ NoActor bypass correctly detects compile-time environment
|
||||
- ✅ All tests explicitly use system_actor for authorization
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue