Merge branch 'main' into feature/filter-boolean-custom-fields
Some checks failed
continuous-integration/drone/push Build is failing
Some checks failed
continuous-integration/drone/push Build is failing
This commit is contained in:
commit
672b4a8250
45 changed files with 3166 additions and 359 deletions
|
|
@ -641,7 +641,92 @@ def card(assigns) do
|
||||||
end
|
end
|
||||||
```
|
```
|
||||||
|
|
||||||
### 3.3 Ash Framework
|
### 3.3 System Actor Pattern
|
||||||
|
|
||||||
|
**When to Use System Actor:**
|
||||||
|
|
||||||
|
Some operations must always run regardless of user permissions. These are **systemic operations** that are mandatory side effects:
|
||||||
|
|
||||||
|
- **Email synchronization** (Member ↔ User)
|
||||||
|
- **Email uniqueness validation** (data integrity requirement)
|
||||||
|
- **Cycle generation** (if defined as mandatory side effect)
|
||||||
|
- **Background jobs**
|
||||||
|
- **Seeds**
|
||||||
|
|
||||||
|
**Implementation:**
|
||||||
|
|
||||||
|
Use `Mv.Helpers.SystemActor.get_system_actor/0` for all systemic operations:
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
# Good - Email sync uses system actor
|
||||||
|
def get_linked_member(user) do
|
||||||
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
opts = Helpers.ash_actor_opts(system_actor)
|
||||||
|
|
||||||
|
case Ash.get(Mv.Membership.Member, id, opts) do
|
||||||
|
{:ok, member} -> member
|
||||||
|
{:error, _} -> nil
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Bad - Using user actor for systemic operation
|
||||||
|
def get_linked_member(user, actor) do
|
||||||
|
opts = Helpers.ash_actor_opts(actor) # May fail if user lacks permissions!
|
||||||
|
# ...
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
**System Actor Details:**
|
||||||
|
|
||||||
|
- System actor is a user with admin role (email: "system@mila.local")
|
||||||
|
- Cached in Agent for performance
|
||||||
|
- Falls back to admin user from seeds if system user doesn't exist
|
||||||
|
- Should NEVER be used for user-initiated actions (only systemic operations)
|
||||||
|
|
||||||
|
**User Mode vs System Mode:**
|
||||||
|
|
||||||
|
- **User Mode**: User-initiated actions use the actual user actor, policies are enforced
|
||||||
|
- **System Mode**: Systemic operations use system actor, bypass user permissions
|
||||||
|
|
||||||
|
**Authorization Bootstrap Patterns:**
|
||||||
|
|
||||||
|
Three 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
|
||||||
|
```elixir
|
||||||
|
# Good: Systemic operation
|
||||||
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
Ash.read(Member, actor: system_actor)
|
||||||
|
|
||||||
|
# Bad: User-initiated action
|
||||||
|
# Never use system_actor for user-initiated actions!
|
||||||
|
```
|
||||||
|
|
||||||
|
3. **authorize?: false** (bootstrap only) - Skips policies for circular dependencies
|
||||||
|
```elixir
|
||||||
|
# Good: Bootstrap (seeds, SystemActor loading)
|
||||||
|
Accounts.create_user!(%{email: admin_email}, authorize?: false)
|
||||||
|
|
||||||
|
# Bad: User-initiated action
|
||||||
|
Ash.destroy(member, authorize?: false) # Never do this!
|
||||||
|
```
|
||||||
|
|
||||||
|
**Decision Guide:**
|
||||||
|
- Use **NoActor** for test fixtures (automatic via config)
|
||||||
|
- Use **system_actor** for email sync, cycle generation, validations
|
||||||
|
- Use **authorize?: false** only for bootstrap (seeds, circular dependencies)
|
||||||
|
- Always document why `authorize?: false` is necessary
|
||||||
|
|
||||||
|
**See also:** `docs/roles-and-permissions-architecture.md` (Authorization Bootstrap Patterns section)
|
||||||
|
|
||||||
|
### 3.4 Ash Framework
|
||||||
|
|
||||||
**Resource Definition Best Practices:**
|
**Resource Definition Best Practices:**
|
||||||
|
|
||||||
|
|
@ -1617,6 +1702,66 @@ case Ash.read(Mv.Membership.Member, actor: actor) do
|
||||||
end
|
end
|
||||||
```
|
```
|
||||||
|
|
||||||
|
### 5.1a NoActor Pattern - Test Environment Only
|
||||||
|
|
||||||
|
**IMPORTANT:** The `Mv.Authorization.Checks.NoActor` check is **ONLY for test environment**. It must NEVER be used in production.
|
||||||
|
|
||||||
|
**What NoActor Does:**
|
||||||
|
|
||||||
|
- 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:**
|
||||||
|
|
||||||
|
```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
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
**Why This Pattern Exists:**
|
||||||
|
|
||||||
|
- 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
|
||||||
|
|
||||||
|
**NEVER Use NoActor in Production:**
|
||||||
|
|
||||||
|
```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()
|
||||||
|
Ash.create!(Member, attrs, actor: system_actor)
|
||||||
|
```
|
||||||
|
|
||||||
|
**Testing:**
|
||||||
|
|
||||||
|
- 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`
|
||||||
|
|
||||||
### 5.2 Password Security
|
### 5.2 Password Security
|
||||||
|
|
||||||
**Use bcrypt for Password Hashing:**
|
**Use bcrypt for Password Hashing:**
|
||||||
|
|
|
||||||
|
|
@ -49,3 +49,7 @@ config :mv, :require_token_presence_for_authentication, false
|
||||||
# Enable SQL Sandbox for async LiveView tests
|
# Enable SQL Sandbox for async LiveView tests
|
||||||
# This flag controls sync vs async behavior in CycleGenerator after_action hooks
|
# This flag controls sync vs async behavior in CycleGenerator after_action hooks
|
||||||
config :mv, :sql_sandbox, true
|
config :mv, :sql_sandbox, true
|
||||||
|
|
||||||
|
# Allow operations without actor in test environment (NoActor check)
|
||||||
|
# SECURITY: This must ONLY be true in test.exs, never in prod/dev
|
||||||
|
config :mv, :allow_no_actor_bypass, true
|
||||||
|
|
|
||||||
330
docs/policy-bypass-vs-haspermission.md
Normal file
330
docs/policy-bypass-vs-haspermission.md
Normal file
|
|
@ -0,0 +1,330 @@
|
||||||
|
# Policy Pattern: Bypass vs. HasPermission
|
||||||
|
|
||||||
|
**Date:** 2026-01-22
|
||||||
|
**Status:** Implemented and Tested
|
||||||
|
**Applies to:** User Resource, Member Resource
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
For filter-based permissions (`scope :own`, `scope :linked`), we use a **two-tier authorization pattern**:
|
||||||
|
|
||||||
|
1. **Bypass with `expr()` for READ operations** - Handles list queries via auto_filter
|
||||||
|
2. **HasPermission for UPDATE/CREATE/DESTROY** - Uses scope from PermissionSets when record is present
|
||||||
|
|
||||||
|
This pattern ensures that the scope concept in PermissionSets is actually used and not redundant.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## The Problem
|
||||||
|
|
||||||
|
### Initial Assumption (INCORRECT)
|
||||||
|
|
||||||
|
> "No separate Own Credentials Bypass needed, as all permission sets already have User read/update :own. HasPermission with scope :own handles this correctly."
|
||||||
|
|
||||||
|
This assumption was based on the idea that `HasPermission` returning `{:filter, expr(...)}` would automatically trigger Ash's `auto_filter` for list queries.
|
||||||
|
|
||||||
|
### Reality
|
||||||
|
|
||||||
|
**When HasPermission returns `{:filter, expr(...)}`:**
|
||||||
|
|
||||||
|
1. `strict_check` is called first
|
||||||
|
2. For list queries (no record yet), `strict_check` returns `{:ok, false}`
|
||||||
|
3. Ash **STOPS** evaluation and does **NOT** call `auto_filter`
|
||||||
|
4. Result: List queries fail with empty results ❌
|
||||||
|
|
||||||
|
**Example:**
|
||||||
|
```elixir
|
||||||
|
# This FAILS for list queries:
|
||||||
|
policy action_type([:read, :update]) do
|
||||||
|
authorize_if Mv.Authorization.Checks.HasPermission
|
||||||
|
end
|
||||||
|
|
||||||
|
# User tries to list all users:
|
||||||
|
Ash.read(User, actor: user)
|
||||||
|
# Expected: Returns [user] (filtered to own record)
|
||||||
|
# Actual: Returns [] (empty list)
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## The Solution
|
||||||
|
|
||||||
|
### Pattern: Bypass for READ, HasPermission for UPDATE
|
||||||
|
|
||||||
|
**User Resource Example:**
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
policies do
|
||||||
|
# Bypass for READ (handles 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
|
||||||
|
|
||||||
|
# HasPermission for UPDATE (scope :own works with changesets)
|
||||||
|
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
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
**Why This Works:**
|
||||||
|
|
||||||
|
| Operation | Record Available? | Method | Result |
|
||||||
|
|-----------|-------------------|--------|--------|
|
||||||
|
| **READ (list)** | ❌ No | `bypass` with `expr()` | Ash applies expr as SQL WHERE → ✅ Filtered list |
|
||||||
|
| **READ (single)** | ✅ Yes | `bypass` with `expr()` | Ash evaluates expr → ✅ true/false |
|
||||||
|
| **UPDATE** | ✅ Yes (changeset) | `HasPermission` with `scope :own` | strict_check evaluates record → ✅ Authorized |
|
||||||
|
| **CREATE** | ✅ Yes (changeset) | `HasPermission` with `scope :own` | strict_check evaluates record → ✅ Authorized |
|
||||||
|
| **DESTROY** | ✅ Yes | `HasPermission` with `scope :own` | strict_check evaluates record → ✅ Authorized |
|
||||||
|
|
||||||
|
**Important: UPDATE Strategy**
|
||||||
|
|
||||||
|
UPDATE is **NOT** a hardcoded bypass. It is controlled by **PermissionSets**:
|
||||||
|
|
||||||
|
- All permission sets (`:own_data`, `:read_only`, `:normal_user`, `:admin`) explicitly grant `User.update :own`
|
||||||
|
- `HasPermission` evaluates `scope :own` when a changeset with record is present
|
||||||
|
- If a permission set is changed to remove `User.update :own`, users with that set will lose the ability to update their credentials
|
||||||
|
- This is intentional - UPDATE is controlled by PermissionSets, not hardcoded
|
||||||
|
|
||||||
|
**Example:** The `read_only` permission set grants `User.update :own` even though it's "read-only" for member data. This allows password changes while keeping member data read-only.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Why `scope :own` Is NOT Redundant
|
||||||
|
|
||||||
|
### The Question
|
||||||
|
|
||||||
|
> "If we use a bypass with `expr(id == ^actor(:id))` for READ, isn't `scope :own` in PermissionSets redundant?"
|
||||||
|
|
||||||
|
### The Answer: NO! ✅
|
||||||
|
|
||||||
|
**`scope :own` is ONLY used for operations where a record is present:**
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
# PermissionSets.ex
|
||||||
|
%{resource: "User", action: :read, scope: :own, granted: true}, # Not used (bypass handles it)
|
||||||
|
%{resource: "User", action: :update, scope: :own, granted: true}, # USED by HasPermission ✅
|
||||||
|
```
|
||||||
|
|
||||||
|
**Test Proof:**
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
# test/mv/accounts/user_policies_test.exs:82
|
||||||
|
test "can update own email", %{user: user} do
|
||||||
|
new_email = "updated@example.com"
|
||||||
|
|
||||||
|
# This works via HasPermission with scope :own (NOT via bypass)
|
||||||
|
{:ok, updated_user} =
|
||||||
|
user
|
||||||
|
|> Ash.Changeset.for_update(:update_user, %{email: new_email})
|
||||||
|
|> Ash.update(actor: user)
|
||||||
|
|
||||||
|
assert updated_user.email == Ash.CiString.new(new_email)
|
||||||
|
end
|
||||||
|
# ✅ Test passes - proves scope :own is used!
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Consistency Across Resources
|
||||||
|
|
||||||
|
### User Resource
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
# Bypass for READ list queries
|
||||||
|
bypass action_type(:read) do
|
||||||
|
authorize_if expr(id == ^actor(:id))
|
||||||
|
end
|
||||||
|
|
||||||
|
# HasPermission for UPDATE (uses scope :own from PermissionSets)
|
||||||
|
policy action_type([:read, :create, :update, :destroy]) do
|
||||||
|
authorize_if Mv.Authorization.Checks.HasPermission
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
**PermissionSets:**
|
||||||
|
- `own_data`, `read_only`, `normal_user`: `scope :own` for read/update
|
||||||
|
- `admin`: `scope :all` for all operations
|
||||||
|
|
||||||
|
### Member Resource
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
# Bypass for READ list queries
|
||||||
|
bypass action_type(:read) do
|
||||||
|
authorize_if expr(id == ^actor(:member_id))
|
||||||
|
end
|
||||||
|
|
||||||
|
# HasPermission for UPDATE (uses scope :linked from PermissionSets)
|
||||||
|
policy action_type([:read, :create, :update, :destroy]) do
|
||||||
|
authorize_if Mv.Authorization.Checks.HasPermission
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
**PermissionSets:**
|
||||||
|
- `own_data`: `scope :linked` for read/update
|
||||||
|
- `read_only`: `scope :all` for read (no update permission)
|
||||||
|
- `normal_user`, `admin`: `scope :all` for all operations
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Technical Deep Dive
|
||||||
|
|
||||||
|
### Why Does `expr()` in Bypass Work?
|
||||||
|
|
||||||
|
**Ash treats `expr()` natively in two contexts:**
|
||||||
|
|
||||||
|
1. **strict_check** (single record):
|
||||||
|
- Ash evaluates the expression against the record
|
||||||
|
- Returns true/false based on match
|
||||||
|
|
||||||
|
2. **auto_filter** (list queries):
|
||||||
|
- Ash compiles the expression to SQL WHERE clause
|
||||||
|
- Applies filter directly in database query
|
||||||
|
|
||||||
|
**Example:**
|
||||||
|
```elixir
|
||||||
|
bypass action_type(:read) do
|
||||||
|
authorize_if expr(id == ^actor(:id))
|
||||||
|
end
|
||||||
|
|
||||||
|
# For list query: Ash.read(User, actor: user)
|
||||||
|
# Compiled SQL: SELECT * FROM users WHERE id = $1 (user.id)
|
||||||
|
# Result: [user] ✅
|
||||||
|
```
|
||||||
|
|
||||||
|
### Why Doesn't HasPermission Trigger auto_filter?
|
||||||
|
|
||||||
|
**HasPermission.strict_check logic:**
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
def strict_check(actor, authorizer, _opts) do
|
||||||
|
# ...
|
||||||
|
case check_permission(...) do
|
||||||
|
{:filter, filter_expr} ->
|
||||||
|
if record do
|
||||||
|
# Evaluate filter against record
|
||||||
|
evaluate_filter_for_strict_check(filter_expr, actor, record, resource_name)
|
||||||
|
else
|
||||||
|
# No record (list query) - return false
|
||||||
|
# Ash STOPS here, does NOT call auto_filter
|
||||||
|
{:ok, false}
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
**Why return false instead of :unknown?**
|
||||||
|
|
||||||
|
We tested returning `:unknown`, but Ash's policy evaluation still didn't reliably call `auto_filter`. The `bypass` with `expr()` is the only consistent solution.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Design Principles
|
||||||
|
|
||||||
|
### 1. Consistency
|
||||||
|
|
||||||
|
Both User and Member follow the same pattern:
|
||||||
|
- Bypass for READ (list queries)
|
||||||
|
- HasPermission for UPDATE/CREATE/DESTROY (with scope)
|
||||||
|
|
||||||
|
### 2. Scope Concept Is Essential
|
||||||
|
|
||||||
|
PermissionSets define scopes for all operations:
|
||||||
|
- `:own` - User can access their own records
|
||||||
|
- `:linked` - User can access linked records (e.g., their member)
|
||||||
|
- `:all` - User can access all records (admin)
|
||||||
|
|
||||||
|
**These scopes are NOT redundant** - they are used for UPDATE/CREATE/DESTROY.
|
||||||
|
|
||||||
|
### 3. Bypass Is a Technical Workaround
|
||||||
|
|
||||||
|
The bypass is not a design choice but a **technical necessity** due to Ash's policy evaluation behavior:
|
||||||
|
- Ash doesn't call `auto_filter` when `strict_check` returns `false`
|
||||||
|
- `expr()` in bypass is handled natively by Ash for both contexts
|
||||||
|
- This is consistent with Ash's documentation and best practices
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Test Coverage
|
||||||
|
|
||||||
|
### User Resource Tests
|
||||||
|
|
||||||
|
**File:** `test/mv/accounts/user_policies_test.exs`
|
||||||
|
|
||||||
|
**Coverage:**
|
||||||
|
- ✅ 31 tests: 30 passing, 1 skipped
|
||||||
|
- ✅ All 4 permission sets: `own_data`, `read_only`, `normal_user`, `admin`
|
||||||
|
- ✅ READ operations (list and single) via bypass
|
||||||
|
- ✅ UPDATE operations via HasPermission with `scope :own`
|
||||||
|
- ✅ Admin operations via HasPermission with `scope :all`
|
||||||
|
- ✅ AshAuthentication bypass (registration/login)
|
||||||
|
- ✅ NoActor bypass (test environment)
|
||||||
|
|
||||||
|
**Key Tests Proving Pattern:**
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
# Test 1: READ list uses bypass (returns filtered list)
|
||||||
|
test "list users returns only own user", %{user: user} do
|
||||||
|
{:ok, users} = Ash.read(Accounts.User, actor: user, domain: Mv.Accounts)
|
||||||
|
assert length(users) == 1 # Filtered to own user ✅
|
||||||
|
assert hd(users).id == user.id
|
||||||
|
end
|
||||||
|
|
||||||
|
# Test 2: UPDATE uses HasPermission with scope :own
|
||||||
|
test "can update own email", %{user: user} do
|
||||||
|
{:ok, updated_user} =
|
||||||
|
user
|
||||||
|
|> Ash.Changeset.for_update(:update_user, %{email: "new@example.com"})
|
||||||
|
|> Ash.update(actor: user)
|
||||||
|
|
||||||
|
assert updated_user.email # Uses scope :own from PermissionSets ✅
|
||||||
|
end
|
||||||
|
|
||||||
|
# Test 3: Admin uses HasPermission with scope :all
|
||||||
|
test "admin can update other users", %{admin: admin, other_user: other_user} do
|
||||||
|
{:ok, updated_user} =
|
||||||
|
other_user
|
||||||
|
|> Ash.Changeset.for_update(:update_user, %{email: "admin-changed@example.com"})
|
||||||
|
|> Ash.update(actor: admin)
|
||||||
|
|
||||||
|
assert updated_user.email # Uses scope :all from PermissionSets ✅
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Lessons Learned
|
||||||
|
|
||||||
|
1. **Don't assume** that returning a filter from `strict_check` will trigger `auto_filter` - test it!
|
||||||
|
2. **Bypass with `expr()` is necessary** for list queries with filter-based permissions
|
||||||
|
3. **Scope concept is NOT redundant** - it's used for operations with records (UPDATE/CREATE/DESTROY)
|
||||||
|
4. **Consistency matters** - following the same pattern across resources improves maintainability
|
||||||
|
5. **Documentation is key** - explaining WHY the pattern exists prevents future confusion
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Future Considerations
|
||||||
|
|
||||||
|
### If Ash Changes Policy Evaluation
|
||||||
|
|
||||||
|
If a future version of Ash reliably calls `auto_filter` when `strict_check` returns `:unknown` or `{:filter, expr}`:
|
||||||
|
|
||||||
|
1. We could **remove** the bypass for READ
|
||||||
|
2. Keep only the HasPermission policy for all operations
|
||||||
|
3. Update tests to verify the new behavior
|
||||||
|
|
||||||
|
**However, for now (Ash 3.13.1), the bypass pattern is necessary and correct.**
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## References
|
||||||
|
|
||||||
|
- **Ash Policy Documentation**: [https://hexdocs.pm/ash/policies.html](https://hexdocs.pm/ash/policies.html)
|
||||||
|
- **Implementation**: `lib/accounts/user.ex` (lines 271-315)
|
||||||
|
- **Tests**: `test/mv/accounts/user_policies_test.exs`
|
||||||
|
- **Architecture Doc**: `docs/roles-and-permissions-architecture.md`
|
||||||
|
- **Permission Sets**: `lib/mv/authorization/permission_sets.ex`
|
||||||
|
|
@ -871,79 +871,166 @@ end
|
||||||
|
|
||||||
**Policy Order Matters!** Ash evaluates policies top-to-bottom, first match wins.
|
**Policy Order Matters!** Ash evaluates policies top-to-bottom, first match wins.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Bypass vs. HasPermission: When to Use Which?
|
||||||
|
|
||||||
|
**Key Finding:** For filter-based permissions (`scope :own`, `scope :linked`), we use a **two-tier approach**:
|
||||||
|
|
||||||
|
1. **Bypass with `expr()` for READ** - Handles list queries (auto_filter)
|
||||||
|
2. **HasPermission for UPDATE/CREATE/DESTROY** - Handles operations with records
|
||||||
|
|
||||||
|
### Why This Pattern?
|
||||||
|
|
||||||
|
**The Problem with HasPermission for List Queries:**
|
||||||
|
|
||||||
|
When `HasPermission` returns `{:filter, expr(...)}` for `scope :own` or `scope :linked`:
|
||||||
|
- `strict_check` returns `{:ok, false}` for queries without a record
|
||||||
|
- Ash does **NOT** reliably call `auto_filter` when `strict_check` returns `false`
|
||||||
|
- Result: List queries fail ❌
|
||||||
|
|
||||||
|
**The Solution:**
|
||||||
|
|
||||||
|
Use `bypass` with `expr()` directly for READ operations:
|
||||||
|
- Ash handles `expr()` natively for both `strict_check` and `auto_filter`
|
||||||
|
- List queries work correctly ✅
|
||||||
|
- Single-record reads work correctly ✅
|
||||||
|
|
||||||
|
### Pattern Summary
|
||||||
|
|
||||||
|
| Operation | Has Record? | Use | Why |
|
||||||
|
|-----------|-------------|-----|-----|
|
||||||
|
| **READ (list)** | ❌ No | `bypass` with `expr()` | Triggers auto_filter |
|
||||||
|
| **READ (single)** | ✅ Yes | `bypass` with `expr()` | expr() evaluates to true/false |
|
||||||
|
| **UPDATE** | ✅ Yes (changeset) | `HasPermission` | strict_check can evaluate record |
|
||||||
|
| **CREATE** | ✅ Yes (changeset) | `HasPermission` | strict_check can evaluate record |
|
||||||
|
| **DESTROY** | ✅ Yes | `HasPermission` | strict_check can evaluate record |
|
||||||
|
|
||||||
|
### Is scope :own/:linked Still Useful?
|
||||||
|
|
||||||
|
**YES! ✅** The scope concept is essential:
|
||||||
|
|
||||||
|
1. **Documentation** - Clearly expresses intent in PermissionSets
|
||||||
|
2. **UPDATE/CREATE/DESTROY** - Works perfectly via HasPermission when record is present
|
||||||
|
3. **Consistency** - All permissions are centralized in PermissionSets
|
||||||
|
4. **Maintainability** - Easy to see what each role can do
|
||||||
|
|
||||||
|
The bypass is a **technical workaround** for Ash's auto_filter limitation, not a replacement for the scope concept.
|
||||||
|
|
||||||
|
### Consistency Across Resources
|
||||||
|
|
||||||
|
Both `User` and `Member` follow this pattern:
|
||||||
|
|
||||||
|
- **User**: Bypass for READ (`id == ^actor(:id)`), HasPermission for UPDATE (`scope :own`)
|
||||||
|
- **Member**: Bypass for READ (`id == ^actor(:member_id)`), HasPermission for UPDATE (`scope :linked`)
|
||||||
|
|
||||||
|
This ensures consistent behavior and predictable authorization logic throughout the application.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
### User Resource Policies
|
### User Resource Policies
|
||||||
|
|
||||||
**Location:** `lib/mv/accounts/user.ex`
|
**Location:** `lib/accounts/user.ex`
|
||||||
|
|
||||||
**Special Case:** Users can ALWAYS read/update their own credentials, regardless of role.
|
**Pattern:** Bypass for READ (list queries), HasPermission for UPDATE (with scope :own).
|
||||||
|
|
||||||
|
**Key Insight:** Bypass with `expr()` is needed ONLY for READ list queries because HasPermission's strict_check cannot properly trigger auto_filter. UPDATE operations work correctly via HasPermission because a changeset with record is available.
|
||||||
|
|
||||||
```elixir
|
```elixir
|
||||||
defmodule Mv.Accounts.User do
|
defmodule Mv.Accounts.User do
|
||||||
use Ash.Resource, ...
|
use Ash.Resource, ...
|
||||||
|
|
||||||
policies do
|
policies do
|
||||||
# SPECIAL CASE: Users can always access their own account
|
# 1. AshAuthentication Bypass (registration/login without actor)
|
||||||
# This takes precedence over permission checks
|
bypass AshAuthentication.Checks.AshAuthenticationInteraction do
|
||||||
policy action_type([:read, :update]) do
|
authorize_if always()
|
||||||
description "Users can always read and update their own account"
|
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
|
||||||
|
# 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
|
||||||
|
description "Users can always read their own account"
|
||||||
authorize_if expr(id == ^actor(:id))
|
authorize_if expr(id == ^actor(:id))
|
||||||
end
|
end
|
||||||
|
|
||||||
# GENERAL: Other operations require permission
|
# 4. GENERAL: Check permissions from user's role
|
||||||
# (e.g., admin reading/updating other users, admin destroying users)
|
# - :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)
|
||||||
|
# - :admin → can read/create/update/destroy all users (scope :all)
|
||||||
policy action_type([:read, :create, :update, :destroy]) do
|
policy action_type([:read, :create, :update, :destroy]) do
|
||||||
description "Check permissions from user's role"
|
description "Check permissions from user's role and permission set"
|
||||||
authorize_if Mv.Authorization.Checks.HasPermission
|
authorize_if Mv.Authorization.Checks.HasPermission
|
||||||
end
|
end
|
||||||
|
|
||||||
# DEFAULT: Forbid if no policy matched
|
# 5. DEFAULT: Ash implicitly forbids if no policy authorizes (fail-closed)
|
||||||
policy action_type([:read, :create, :update, :destroy]) do
|
|
||||||
forbid_if always()
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
# ...
|
# ...
|
||||||
end
|
end
|
||||||
```
|
```
|
||||||
|
|
||||||
|
**Why Bypass for READ but not UPDATE?**
|
||||||
|
|
||||||
|
- **READ list queries** (`Ash.read(User, actor: user)`): No record at strict_check time → HasPermission returns `{:ok, false}` → auto_filter not called → bypass with `expr()` needed ✅
|
||||||
|
- **UPDATE operations** (`Ash.update(changeset, actor: user)`): Changeset contains record → HasPermission can evaluate `scope :own` correctly → works via HasPermission ✅
|
||||||
|
|
||||||
**Permission Matrix:**
|
**Permission Matrix:**
|
||||||
|
|
||||||
| Action | Mitglied | Vorstand | Kassenwart | Buchhaltung | Admin |
|
| Action | Mitglied | Vorstand | Kassenwart | Buchhaltung | Admin |
|
||||||
|--------|----------|----------|------------|-------------|-------|
|
|--------|----------|----------|------------|-------------|-------|
|
||||||
| Read own | ✅ (special) | ✅ (special) | ✅ (special) | ✅ (special) | ✅ |
|
| Read own | ✅ (bypass) | ✅ (bypass) | ✅ (bypass) | ✅ (bypass) | ✅ (scope :all) |
|
||||||
| Update own | ✅ (special) | ✅ (special) | ✅ (special) | ✅ (special) | ✅ |
|
| Update own | ✅ (scope :own) | ✅ (scope :own) | ✅ (scope :own) | ✅ (scope :own) | ✅ (scope :all) |
|
||||||
| Read others | ❌ | ❌ | ❌ | ❌ | ✅ |
|
| Read others | ❌ | ❌ | ❌ | ❌ | ✅ (scope :all) |
|
||||||
| Update others | ❌ | ❌ | ❌ | ❌ | ✅ |
|
| Update others | ❌ | ❌ | ❌ | ❌ | ✅ (scope :all) |
|
||||||
| Create | ❌ | ❌ | ❌ | ❌ | ✅ |
|
| Create | ❌ | ❌ | ❌ | ❌ | ✅ (scope :all) |
|
||||||
| Destroy | ❌ | ❌ | ❌ | ❌ | ✅ |
|
| Destroy | ❌ | ❌ | ❌ | ❌ | ✅ (scope :all) |
|
||||||
|
|
||||||
|
**Note:** This pattern is consistent with Member resource policies (bypass for READ, HasPermission for UPDATE).
|
||||||
|
|
||||||
### Member Resource Policies
|
### Member Resource Policies
|
||||||
|
|
||||||
**Location:** `lib/mv/membership/member.ex`
|
**Location:** `lib/mv/membership/member.ex`
|
||||||
|
|
||||||
**Special Case:** Users can always READ their linked member (where `id == user.member_id`).
|
**Pattern:** Bypass for READ (list queries), HasPermission for UPDATE (with scope :linked).
|
||||||
|
|
||||||
|
**Key Insight:** Same pattern as User - bypass with `expr()` is needed ONLY for READ list queries. UPDATE operations work correctly via HasPermission because a changeset with record is available.
|
||||||
|
|
||||||
```elixir
|
```elixir
|
||||||
defmodule Mv.Membership.Member do
|
defmodule Mv.Membership.Member do
|
||||||
use Ash.Resource, ...
|
use Ash.Resource, ...
|
||||||
|
|
||||||
policies do
|
policies do
|
||||||
# SPECIAL CASE: Users can always access their linked member
|
# 1. NoActor Bypass (test environment only, for test fixtures)
|
||||||
policy action_type([:read, :update]) do
|
bypass action_type([:create, :read, :update, :destroy]) do
|
||||||
description "Users can access member linked to their account"
|
authorize_if Mv.Authorization.Checks.NoActor
|
||||||
authorize_if expr(user_id == ^actor(:id))
|
|
||||||
end
|
end
|
||||||
|
|
||||||
# GENERAL: Check permissions from role
|
# 2. 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
|
||||||
|
description "Users can always read member linked to their account"
|
||||||
|
authorize_if expr(id == ^actor(:member_id))
|
||||||
|
end
|
||||||
|
|
||||||
|
# 3. 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)
|
||||||
|
# - :admin → can CRUD all members (scope :all)
|
||||||
policy action_type([:read, :create, :update, :destroy]) do
|
policy action_type([:read, :create, :update, :destroy]) do
|
||||||
description "Check permissions from user's role"
|
description "Check permissions from user's role"
|
||||||
authorize_if Mv.Authorization.Checks.HasPermission
|
authorize_if Mv.Authorization.Checks.HasPermission
|
||||||
end
|
end
|
||||||
|
|
||||||
# DEFAULT: Forbid
|
# 4. DEFAULT: Ash implicitly forbids if no policy authorizes (fail-closed)
|
||||||
policy action_type([:read, :create, :update, :destroy]) do
|
|
||||||
forbid_if always()
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
# Custom validation for email editing (see Special Cases section)
|
# Custom validation for email editing (see Special Cases section)
|
||||||
|
|
@ -957,6 +1044,11 @@ defmodule Mv.Membership.Member do
|
||||||
end
|
end
|
||||||
```
|
```
|
||||||
|
|
||||||
|
**Why Bypass for READ but not UPDATE?**
|
||||||
|
|
||||||
|
- **READ list queries**: No record at strict_check time → bypass with `expr(id == ^actor(:member_id))` needed for auto_filter ✅
|
||||||
|
- **UPDATE operations**: Changeset contains record → HasPermission evaluates `scope :linked` correctly ✅
|
||||||
|
|
||||||
**Permission Matrix:**
|
**Permission Matrix:**
|
||||||
|
|
||||||
| Action | Mitglied | Vorstand | Kassenwart | Buchhaltung | Admin |
|
| Action | Mitglied | Vorstand | Kassenwart | Buchhaltung | Admin |
|
||||||
|
|
@ -1652,17 +1744,21 @@ end
|
||||||
|
|
||||||
**Implementation:**
|
**Implementation:**
|
||||||
|
|
||||||
Policy in `User` resource places this check BEFORE the general `HasPermission` check:
|
Policy in `User` resource uses a two-tier approach:
|
||||||
|
- **READ**: Bypass with `expr()` for list queries (auto_filter)
|
||||||
|
- **UPDATE**: HasPermission with `scope :own` (evaluates PermissionSets)
|
||||||
|
|
||||||
```elixir
|
```elixir
|
||||||
policies do
|
policies do
|
||||||
# SPECIAL CASE: Takes precedence over role permissions
|
# SPECIAL CASE: Users can always READ their own account
|
||||||
policy action_type([:read, :update]) do
|
# Bypass needed for list queries (expr() triggers auto_filter in Ash)
|
||||||
description "Users can always read and update their own account"
|
bypass action_type(:read) do
|
||||||
|
description "Users can always read their own account"
|
||||||
authorize_if expr(id == ^actor(:id))
|
authorize_if expr(id == ^actor(:id))
|
||||||
end
|
end
|
||||||
|
|
||||||
# GENERAL: For other operations (e.g., admin reading other users)
|
# GENERAL: Check permissions from user's role
|
||||||
|
# UPDATE uses scope :own from PermissionSets (all sets grant User.update :own)
|
||||||
policy action_type([:read, :create, :update, :destroy]) do
|
policy action_type([:read, :create, :update, :destroy]) do
|
||||||
authorize_if Mv.Authorization.Checks.HasPermission
|
authorize_if Mv.Authorization.Checks.HasPermission
|
||||||
end
|
end
|
||||||
|
|
@ -1670,10 +1766,53 @@ end
|
||||||
```
|
```
|
||||||
|
|
||||||
**Why this works:**
|
**Why this works:**
|
||||||
- Ash evaluates policies top-to-bottom
|
- READ bypass handles list queries correctly (auto_filter)
|
||||||
- First matching policy wins
|
- UPDATE is handled by HasPermission with `scope :own` from PermissionSets
|
||||||
- Special case catches own-account access before checking permissions
|
- All permission sets (`:own_data`, `:read_only`, `:normal_user`, `:admin`) grant `User.update :own`
|
||||||
- Even a user with `own_data` (no admin permissions) can update their credentials
|
- Even a user with `read_only` (read-only for member data) can update their own credentials
|
||||||
|
|
||||||
|
**Important:** UPDATE is NOT an unverrückbarer Spezialfall (hardcoded bypass). It is controlled by PermissionSets. If a permission set is changed to remove `User.update :own`, users with that set will lose the ability to update their credentials. See "User Credentials: Why read_only Can Still Update" below for details.
|
||||||
|
|
||||||
|
### 1a. User Credentials: Why read_only Can Still Update
|
||||||
|
|
||||||
|
**Question:** If `read_only` means "read-only", why can users with this permission set still update their own credentials?
|
||||||
|
|
||||||
|
**Answer:** The `read_only` permission set refers to **member data**, NOT user credentials. All permission sets grant `User.update :own` to allow password changes and profile updates.
|
||||||
|
|
||||||
|
**Implementation Details:**
|
||||||
|
|
||||||
|
1. **UPDATE is controlled by PermissionSets**, not a hardcoded bypass
|
||||||
|
2. **All 4 permission sets** (`:own_data`, `:read_only`, `:normal_user`, `:admin`) explicitly grant:
|
||||||
|
```elixir
|
||||||
|
%{resource: "User", action: :update, scope: :own, granted: true}
|
||||||
|
```
|
||||||
|
3. **HasPermission** evaluates `scope :own` for UPDATE operations (when a changeset with record is present)
|
||||||
|
4. **No special bypass** is needed for UPDATE - it works correctly via HasPermission
|
||||||
|
|
||||||
|
**Why This Design?**
|
||||||
|
|
||||||
|
- **Flexibility:** Permission sets can be modified to change UPDATE behavior
|
||||||
|
- **Consistency:** All permissions are centralized in PermissionSets
|
||||||
|
- **Clarity:** The name "read_only" refers to member data, not user credentials
|
||||||
|
- **Maintainability:** Easy to see what each role can do in PermissionSets module
|
||||||
|
|
||||||
|
**Warning:** If a permission set is changed to remove `User.update :own`, users with that set will **lose the ability to update their credentials**. This is intentional - UPDATE is controlled by PermissionSets, not hardcoded.
|
||||||
|
|
||||||
|
**Example:**
|
||||||
|
```elixir
|
||||||
|
# In PermissionSets.get_permissions(:read_only)
|
||||||
|
resources: [
|
||||||
|
# User: Can read/update own credentials only
|
||||||
|
# IMPORTANT: "read_only" refers to member data, NOT user credentials.
|
||||||
|
# All permission sets grant User.update :own to allow password changes.
|
||||||
|
%{resource: "User", action: :read, scope: :own, granted: true},
|
||||||
|
%{resource: "User", action: :update, scope: :own, granted: true},
|
||||||
|
|
||||||
|
# Member: Can read all members, no modifications
|
||||||
|
%{resource: "Member", action: :read, scope: :all, granted: true},
|
||||||
|
# Note: No Member.update permission - this is the "read_only" part
|
||||||
|
]
|
||||||
|
```
|
||||||
|
|
||||||
### 2. Linked Member Email Editing
|
### 2. Linked Member Email Editing
|
||||||
|
|
||||||
|
|
@ -2484,8 +2623,241 @@ iex> MvWeb.Authorization.can_access_page?(user, "/members/new")
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## Authorization Bootstrap Patterns
|
||||||
|
|
||||||
|
This section clarifies three different mechanisms for bypassing standard authorization, their purposes, and when to use each.
|
||||||
|
|
||||||
|
### Overview
|
||||||
|
|
||||||
|
The codebase uses three 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
|
||||||
|
|
||||||
|
**All three are necessary and serve different purposes.**
|
||||||
|
|
||||||
|
### 1. NoActor Check
|
||||||
|
|
||||||
|
**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
|
||||||
|
|
||||||
|
**Purpose:** Admin user for systemic operations that must always succeed regardless of user permissions.
|
||||||
|
|
||||||
|
**Implementation:**
|
||||||
|
```elixir
|
||||||
|
system_actor = Mv.Helpers.SystemActor.get_system_actor()
|
||||||
|
# => %User{email: "system@mila.local", role: %{permission_set_name: "admin"}}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Security:**
|
||||||
|
- No password (hashed_password = nil) → cannot login
|
||||||
|
- No OIDC ID (oidc_id = nil) → cannot authenticate
|
||||||
|
- Cached in Agent for performance
|
||||||
|
- Created automatically in test environment if missing
|
||||||
|
|
||||||
|
**Use Cases:**
|
||||||
|
- **Email synchronization** (User ↔ Member email sync)
|
||||||
|
- **Email uniqueness validation** (cross-resource checks)
|
||||||
|
- **Cycle generation** (mandatory side effect)
|
||||||
|
- **OIDC account linking** (user not yet logged in)
|
||||||
|
- **Cross-resource validations** (must work regardless of actor)
|
||||||
|
|
||||||
|
**Example:**
|
||||||
|
```elixir
|
||||||
|
def get_linked_member(%{member_id: id}) do
|
||||||
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
opts = Helpers.ash_actor_opts(system_actor)
|
||||||
|
|
||||||
|
# Email sync must work regardless of user permissions
|
||||||
|
Ash.get(Mv.Membership.Member, id, opts)
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
**Why not `authorize?: false`?**
|
||||||
|
- System actor is explicit (clear intent: "systemic operation")
|
||||||
|
- Policies are evaluated (with admin rights)
|
||||||
|
- Audit trail (actor.email = "system@mila.local")
|
||||||
|
- Consistent authorization flow
|
||||||
|
- Testable
|
||||||
|
|
||||||
|
### 3. authorize?: false
|
||||||
|
|
||||||
|
**Purpose:** Skip policies for bootstrap scenarios with circular dependencies.
|
||||||
|
|
||||||
|
**Use Cases:**
|
||||||
|
|
||||||
|
**1. Seeds** - No admin exists yet to use as actor:
|
||||||
|
```elixir
|
||||||
|
# priv/repo/seeds.exs
|
||||||
|
Accounts.create_user!(%{email: admin_email},
|
||||||
|
authorize?: false # Bootstrap: no admin exists yet
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
**2. SystemActor Bootstrap** - Chicken-and-egg problem:
|
||||||
|
```elixir
|
||||||
|
# lib/mv/helpers/system_actor.ex
|
||||||
|
defp find_user_by_email(email) do
|
||||||
|
# Need to find system actor, but loading requires system actor!
|
||||||
|
Mv.Accounts.User
|
||||||
|
|> Ash.Query.filter(email == ^email)
|
||||||
|
|> Ash.read_one(authorize?: false) # Bootstrap only
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
**3. Actor.ensure_loaded** - Circular dependency:
|
||||||
|
```elixir
|
||||||
|
# lib/mv/authorization/actor.ex
|
||||||
|
defp load_role(actor) do
|
||||||
|
# Actor needs role for authorization,
|
||||||
|
# but loading role requires authorization!
|
||||||
|
Ash.load(actor, :role, authorize?: false) # Bootstrap only
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
**4. assign_default_role** - User creation:
|
||||||
|
```elixir
|
||||||
|
# User doesn't have actor during creation
|
||||||
|
Mv.Authorization.Role
|
||||||
|
|> Ash.Query.filter(name == "Mitglied")
|
||||||
|
|> Ash.read_one(authorize?: false) # Bootstrap only
|
||||||
|
```
|
||||||
|
|
||||||
|
**Security:**
|
||||||
|
- Very powerful - skips ALL policies
|
||||||
|
- Use sparingly and document every usage
|
||||||
|
- Only for bootstrap scenarios
|
||||||
|
- All current usages are legitimate
|
||||||
|
|
||||||
|
### 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 |
|
||||||
|
|
||||||
|
### 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
|
||||||
|
- ✅ Cycle generation
|
||||||
|
- ✅ Cross-resource validations
|
||||||
|
- ✅ OIDC flows (user not logged in)
|
||||||
|
|
||||||
|
**Use authorize?: false when:**
|
||||||
|
- ✅ Bootstrap scenario (seeds)
|
||||||
|
- ✅ Circular dependency (SystemActor bootstrap, Actor.ensure_loaded)
|
||||||
|
- ⚠️ Document with comment explaining why
|
||||||
|
|
||||||
|
**DON'T:**
|
||||||
|
- ❌ Use `authorize?: false` for user-initiated actions
|
||||||
|
- ❌ Use `authorize?: false` when `system_actor` would work
|
||||||
|
- ❌ Enable NoActor outside test environment
|
||||||
|
|
||||||
|
### The Circular Dependency Problem
|
||||||
|
|
||||||
|
**SystemActor Bootstrap:**
|
||||||
|
```
|
||||||
|
SystemActor.get_system_actor()
|
||||||
|
↓ calls find_user_by_email()
|
||||||
|
↓ needs to query User
|
||||||
|
↓ User policies require actor
|
||||||
|
↓ but we're loading the actor!
|
||||||
|
|
||||||
|
Solution: authorize?: false for bootstrap query
|
||||||
|
```
|
||||||
|
|
||||||
|
**Actor.ensure_loaded:**
|
||||||
|
```
|
||||||
|
Authorization check (HasPermission)
|
||||||
|
↓ needs actor.role.permission_set_name
|
||||||
|
↓ but role is %Ash.NotLoaded{}
|
||||||
|
↓ load role with Ash.load(actor, :role)
|
||||||
|
↓ but loading requires authorization
|
||||||
|
↓ which needs actor.role!
|
||||||
|
|
||||||
|
Solution: authorize?: false for role load
|
||||||
|
```
|
||||||
|
|
||||||
|
**Why this is safe:**
|
||||||
|
- Actor is loading their OWN data (role relationship)
|
||||||
|
- Actor already passed authentication boundary
|
||||||
|
- Role contains no sensitive data (just permission_set reference)
|
||||||
|
- Alternative (denormalize permission_set_name) adds complexity
|
||||||
|
|
||||||
|
### Examples
|
||||||
|
|
||||||
|
**Good - system_actor for systemic operation:**
|
||||||
|
```elixir
|
||||||
|
defp check_if_email_used(email) do
|
||||||
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
opts = Helpers.ash_actor_opts(system_actor)
|
||||||
|
|
||||||
|
# Validation must work regardless of current actor
|
||||||
|
Ash.read(User, opts)
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
**Good - authorize?: false for bootstrap:**
|
||||||
|
```elixir
|
||||||
|
# Seeds - no admin exists yet
|
||||||
|
Accounts.create_user!(%{email: admin_email}, authorize?: false)
|
||||||
|
```
|
||||||
|
|
||||||
|
**Bad - authorize?: false for user action:**
|
||||||
|
```elixir
|
||||||
|
# WRONG: Bypasses all policies for user-initiated action
|
||||||
|
def delete_member(member) do
|
||||||
|
Ash.destroy(member, authorize?: false) # ❌ Don't do this!
|
||||||
|
end
|
||||||
|
|
||||||
|
# CORRECT: Use actor
|
||||||
|
def delete_member(member, actor) do
|
||||||
|
Ash.destroy(member, actor: actor) # ✅ Policies enforced
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
**Document Version:** 2.0 (Clean Rewrite)
|
**Document Version:** 2.0 (Clean Rewrite)
|
||||||
**Last Updated:** 2026-01-13
|
**Last Updated:** 2026-01-23
|
||||||
**Implementation Status:** ✅ Complete (2026-01-08)
|
**Implementation Status:** ✅ Complete (2026-01-08)
|
||||||
**Status:** Ready for Implementation
|
**Status:** Ready for Implementation
|
||||||
|
|
||||||
|
|
@ -2500,6 +2872,9 @@ iex> MvWeb.Authorization.can_access_page?(user, "/members/new")
|
||||||
- Added comprehensive security section
|
- Added comprehensive security section
|
||||||
- Enhanced edge case documentation
|
- Enhanced edge case documentation
|
||||||
|
|
||||||
|
**Changes from V2.0:**
|
||||||
|
- Added "Authorization Bootstrap Patterns" section explaining NoActor, system_actor, and authorize?: false
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
**End of Architecture Document**
|
**End of Architecture Document**
|
||||||
|
|
|
||||||
|
|
@ -525,60 +525,67 @@ Add authorization policies to the Member resource using the new `HasPermission`
|
||||||
**Dependencies:** #6 (HasPermission check)
|
**Dependencies:** #6 (HasPermission check)
|
||||||
**Can work in parallel:** Yes (parallel with #7, #9, #10)
|
**Can work in parallel:** Yes (parallel with #7, #9, #10)
|
||||||
**Assignable to:** Backend Developer
|
**Assignable to:** Backend Developer
|
||||||
|
**Status:** ✅ **COMPLETED**
|
||||||
|
|
||||||
**Description:**
|
**Description:**
|
||||||
|
|
||||||
Add authorization policies to the User resource. Special case: Users can always read/update their own credentials.
|
Add authorization policies to the User resource. Users can always read their own credentials (via bypass), and update their own credentials (via HasPermission with scope :own).
|
||||||
|
|
||||||
|
**Implementation Pattern:**
|
||||||
|
|
||||||
|
Following the same pattern as Member resource:
|
||||||
|
- **Bypass for READ** - Handles list queries (auto_filter)
|
||||||
|
- **HasPermission for UPDATE** - Handles updates with scope :own
|
||||||
|
|
||||||
**Tasks:**
|
**Tasks:**
|
||||||
|
|
||||||
1. Open `lib/mv/accounts/user.ex`
|
1. ✅ Open `lib/accounts/user.ex`
|
||||||
2. Add `policies` block
|
2. ✅ Add `policies` block
|
||||||
3. Add special policy: Allow user to always access their own account (before general policy)
|
3. ✅ Add AshAuthentication bypass (registration/login without actor)
|
||||||
|
4. ✅ Add NoActor bypass (test environment only)
|
||||||
|
5. ✅ Add bypass for READ: Allow user to always read their own account
|
||||||
```elixir
|
```elixir
|
||||||
policy action_type([:read, :update]) do
|
bypass action_type(:read) do
|
||||||
|
description "Users can always read their own account"
|
||||||
authorize_if expr(id == ^actor(:id))
|
authorize_if expr(id == ^actor(:id))
|
||||||
end
|
end
|
||||||
```
|
```
|
||||||
4. Add general policy: Check HasPermission for all actions
|
6. ✅ Add general policy: Check HasPermission for all actions (including UPDATE with scope :own)
|
||||||
5. Ensure :destroy is admin-only (via HasPermission)
|
7. ✅ Ensure :destroy is admin-only (via HasPermission)
|
||||||
6. Preload :role relationship for actor
|
8. ✅ Preload :role relationship for actor in tests
|
||||||
|
|
||||||
**Policy Order:**
|
**Policy Order:**
|
||||||
1. Allow user to read/update own account (id == actor.id)
|
1. ✅ AshAuthentication bypass (registration/login)
|
||||||
2. Check HasPermission (for admin operations)
|
2. ✅ NoActor bypass (test environment)
|
||||||
3. Default: Forbid
|
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)
|
||||||
|
|
||||||
|
**Why Bypass for READ but not UPDATE?**
|
||||||
|
|
||||||
|
- **READ list queries**: No record at strict_check time → bypass with `expr()` needed for auto_filter ✅
|
||||||
|
- **UPDATE operations**: Changeset contains record → HasPermission evaluates `scope :own` correctly ✅
|
||||||
|
|
||||||
|
This ensures `scope :own` in PermissionSets is actually used (not redundant).
|
||||||
|
|
||||||
**Acceptance Criteria:**
|
**Acceptance Criteria:**
|
||||||
|
|
||||||
- [ ] User can always read/update own credentials
|
- ✅ User can always read own credentials (via bypass)
|
||||||
- [ ] Only admin can read/update other users
|
- ✅ User can always update own credentials (via HasPermission with scope :own)
|
||||||
- [ ] Only admin can destroy users
|
- ✅ Only admin can read/update other users (scope :all)
|
||||||
- [ ] Policy order is correct
|
- ✅ Only admin can destroy users (scope :all)
|
||||||
- [ ] Actor preloads :role relationship
|
- ✅ Policy order is correct (AshAuth → NoActor → Bypass READ → HasPermission)
|
||||||
|
- ✅ Actor preloads :role relationship
|
||||||
|
- ✅ All tests pass (30/31 pass, 1 skipped)
|
||||||
|
|
||||||
**Test Strategy (TDD):**
|
**Test Results:**
|
||||||
|
|
||||||
**Own Data Tests (All Roles):**
|
|
||||||
- User with :own_data can read own user record
|
|
||||||
- User with :own_data can update own email/password
|
|
||||||
- User with :own_data cannot read other users
|
|
||||||
- User with :read_only can read own data
|
|
||||||
- User with :normal_user can read own data
|
|
||||||
- Verify special policy takes precedence
|
|
||||||
|
|
||||||
**Admin Tests:**
|
|
||||||
- Admin can read all users
|
|
||||||
- Admin can update any user's credentials
|
|
||||||
- Admin can destroy users
|
|
||||||
- Admin has unrestricted access
|
|
||||||
|
|
||||||
**Forbidden Tests:**
|
|
||||||
- Non-admin cannot read other users
|
|
||||||
- Non-admin cannot update other users
|
|
||||||
- Non-admin cannot destroy users
|
|
||||||
|
|
||||||
**Test File:** `test/mv/accounts/user_policies_test.exs`
|
**Test File:** `test/mv/accounts/user_policies_test.exs`
|
||||||
|
- ✅ 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 verify scope :own is used for UPDATE (not redundant)
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|
|
||||||
274
docs/user-resource-policies-implementation-summary.md
Normal file
274
docs/user-resource-policies-implementation-summary.md
Normal file
|
|
@ -0,0 +1,274 @@
|
||||||
|
# User Resource Authorization Policies - Implementation Summary
|
||||||
|
|
||||||
|
**Date:** 2026-01-22
|
||||||
|
**Status:** ✅ COMPLETED
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Overview
|
||||||
|
|
||||||
|
Successfully implemented authorization policies for the User resource following the Bypass + HasPermission pattern, ensuring consistency with Member resource policies and proper use of the scope concept from PermissionSets.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## What Was Implemented
|
||||||
|
|
||||||
|
### 1. Policy Structure in `lib/accounts/user.ex`
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
policies do
|
||||||
|
# 1. AshAuthentication Bypass
|
||||||
|
bypass AshAuthentication.Checks.AshAuthenticationInteraction 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)
|
||||||
|
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)
|
||||||
|
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
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
### 2. Test Suite in `test/mv/accounts/user_policies_test.exs`
|
||||||
|
|
||||||
|
**Coverage:**
|
||||||
|
- ✅ 31 tests total: 30 passing, 1 skipped
|
||||||
|
- ✅ All 4 permission sets tested: `own_data`, `read_only`, `normal_user`, `admin`
|
||||||
|
- ✅ READ operations (list and single record)
|
||||||
|
- ✅ UPDATE operations (own and other users)
|
||||||
|
- ✅ CREATE operations (admin only)
|
||||||
|
- ✅ DESTROY operations (admin only)
|
||||||
|
- ✅ AshAuthentication bypass (registration/login)
|
||||||
|
- ✅ NoActor bypass (test environment)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Key Design Decisions
|
||||||
|
|
||||||
|
### Decision 1: Bypass for READ, HasPermission for UPDATE
|
||||||
|
|
||||||
|
**Rationale:**
|
||||||
|
- READ list queries have no record at `strict_check` time
|
||||||
|
- `HasPermission` returns `{:ok, false}` for queries without record
|
||||||
|
- Ash doesn't call `auto_filter` when `strict_check` returns `false`
|
||||||
|
- `expr()` in bypass is handled natively by Ash for `auto_filter`
|
||||||
|
|
||||||
|
**Result:**
|
||||||
|
- Bypass handles READ list queries ✅
|
||||||
|
- HasPermission handles UPDATE with `scope :own` ✅
|
||||||
|
- No redundancy - both are necessary ✅
|
||||||
|
|
||||||
|
### Decision 2: No Explicit `forbid_if always()`
|
||||||
|
|
||||||
|
**Rationale:**
|
||||||
|
- Ash implicitly forbids if no policy authorizes (fail-closed by default)
|
||||||
|
- Explicit `forbid_if always()` at the end breaks tests
|
||||||
|
- It would forbid valid operations that should be authorized by previous policies
|
||||||
|
|
||||||
|
**Result:**
|
||||||
|
- Policies rely on Ash's implicit forbid ✅
|
||||||
|
- Tests pass with this approach ✅
|
||||||
|
|
||||||
|
### Decision 3: Consistency with Member Resource
|
||||||
|
|
||||||
|
**Rationale:**
|
||||||
|
- Member resource uses same pattern: Bypass for READ, HasPermission for UPDATE
|
||||||
|
- Consistent patterns improve maintainability and predictability
|
||||||
|
- Developers can understand authorization logic across resources
|
||||||
|
|
||||||
|
**Result:**
|
||||||
|
- User and Member follow identical pattern ✅
|
||||||
|
- Authorization logic is consistent throughout the app ✅
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## The Scope Concept Is NOT Redundant
|
||||||
|
|
||||||
|
### Initial Concern
|
||||||
|
|
||||||
|
> "If we use a bypass with `expr(id == ^actor(:id))` for READ, isn't `scope :own` in PermissionSets redundant?"
|
||||||
|
|
||||||
|
### Resolution
|
||||||
|
|
||||||
|
**NO! The scope concept is essential:**
|
||||||
|
|
||||||
|
1. **Documentation** - `scope :own` clearly expresses intent in PermissionSets
|
||||||
|
2. **UPDATE operations** - `scope :own` is USED by HasPermission when changeset contains record
|
||||||
|
3. **Admin operations** - `scope :all` allows admins full access
|
||||||
|
4. **Maintainability** - All permissions centralized in one place
|
||||||
|
|
||||||
|
**Test Proof:**
|
||||||
|
|
||||||
|
```elixir
|
||||||
|
test "can update own email", %{user: user} do
|
||||||
|
# This works via HasPermission with scope :own (NOT bypass)
|
||||||
|
{:ok, updated_user} =
|
||||||
|
user
|
||||||
|
|> Ash.Changeset.for_update(:update_user, %{email: "new@example.com"})
|
||||||
|
|> Ash.update(actor: user)
|
||||||
|
|
||||||
|
assert updated_user.email # ✅ Proves scope :own is used
|
||||||
|
end
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Documentation Updates
|
||||||
|
|
||||||
|
### 1. Created `docs/policy-bypass-vs-haspermission.md`
|
||||||
|
|
||||||
|
Comprehensive documentation explaining:
|
||||||
|
- Why bypass is needed for READ
|
||||||
|
- Why HasPermission works for UPDATE
|
||||||
|
- Technical deep dive into Ash policy evaluation
|
||||||
|
- Test coverage proving the pattern
|
||||||
|
- Lessons learned
|
||||||
|
|
||||||
|
### 2. Updated `docs/roles-and-permissions-architecture.md`
|
||||||
|
|
||||||
|
- Added "Bypass vs. HasPermission: When to Use Which?" section
|
||||||
|
- Updated User Resource Policies section with correct implementation
|
||||||
|
- Updated Member Resource Policies section for consistency
|
||||||
|
- Added pattern comparison table
|
||||||
|
|
||||||
|
### 3. Updated `docs/roles-and-permissions-implementation-plan.md`
|
||||||
|
|
||||||
|
- Marked Issue #8 as COMPLETED ✅
|
||||||
|
- Added implementation details
|
||||||
|
- Documented why bypass is needed
|
||||||
|
- Added test results
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Test Results
|
||||||
|
|
||||||
|
### All Relevant Tests Pass
|
||||||
|
|
||||||
|
```bash
|
||||||
|
mix test test/mv/accounts/user_policies_test.exs \
|
||||||
|
test/mv/authorization/checks/has_permission_test.exs \
|
||||||
|
test/mv/membership/member_policies_test.exs
|
||||||
|
|
||||||
|
# Results:
|
||||||
|
# 75 tests: 74 passing, 1 skipped
|
||||||
|
# ✅ User policies: 30/31 (1 skipped)
|
||||||
|
# ✅ HasPermission check: 21/21
|
||||||
|
# ✅ Member policies: 23/23
|
||||||
|
```
|
||||||
|
|
||||||
|
### Specific Test Coverage
|
||||||
|
|
||||||
|
**Own Data Access (All Roles):**
|
||||||
|
- ✅ Can read own user record (via bypass)
|
||||||
|
- ✅ Can update own email (via HasPermission with scope :own)
|
||||||
|
- ✅ Cannot read other users (filtered by bypass)
|
||||||
|
- ✅ Cannot update other users (forbidden by HasPermission)
|
||||||
|
- ✅ List returns only own user (auto_filter via bypass)
|
||||||
|
|
||||||
|
**Admin Access:**
|
||||||
|
- ✅ Can read all users (HasPermission with scope :all)
|
||||||
|
- ✅ Can update other users (HasPermission with scope :all)
|
||||||
|
- ✅ Can create users (HasPermission with scope :all)
|
||||||
|
- ✅ Can destroy users (HasPermission with scope :all)
|
||||||
|
|
||||||
|
**AshAuthentication:**
|
||||||
|
- ✅ Registration works without actor
|
||||||
|
- ✅ OIDC registration works
|
||||||
|
- ✅ OIDC sign-in works
|
||||||
|
|
||||||
|
**Test Environment:**
|
||||||
|
- ✅ Operations without actor work in test environment
|
||||||
|
- ✅ NoActor bypass correctly detects compile-time environment
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Files Changed
|
||||||
|
|
||||||
|
### Implementation
|
||||||
|
1. ✅ `lib/accounts/user.ex` - Added policies block (lines 271-315)
|
||||||
|
2. ✅ `lib/mv/authorization/checks/has_permission.ex` - Added User resource support in `evaluate_filter_for_strict_check`
|
||||||
|
|
||||||
|
### Tests
|
||||||
|
3. ✅ `test/mv/accounts/user_policies_test.exs` - Created comprehensive test suite (435 lines)
|
||||||
|
4. ✅ `test/mv/authorization/checks/has_permission_test.exs` - Updated to expect `false` instead of `:unknown`
|
||||||
|
|
||||||
|
### Documentation
|
||||||
|
5. ✅ `docs/policy-bypass-vs-haspermission.md` - New comprehensive guide (created)
|
||||||
|
6. ✅ `docs/roles-and-permissions-architecture.md` - Updated User and Member sections
|
||||||
|
7. ✅ `docs/roles-and-permissions-implementation-plan.md` - Marked Issue #8 as completed
|
||||||
|
8. ✅ `docs/user-resource-policies-implementation-summary.md` - This file (created)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Lessons Learned
|
||||||
|
|
||||||
|
### 1. Test Before Assuming
|
||||||
|
|
||||||
|
The initial plan assumed HasPermission with `scope :own` would be sufficient. Testing revealed that Ash's policy evaluation doesn't reliably call `auto_filter` when `strict_check` returns `false` or `:unknown`.
|
||||||
|
|
||||||
|
### 2. Bypass Is Not a Workaround, It's a Pattern
|
||||||
|
|
||||||
|
The bypass with `expr()` is not a hack or workaround - it's the **correct pattern** for filter-based authorization in Ash when dealing with list queries.
|
||||||
|
|
||||||
|
### 3. Scope Concept Remains Essential
|
||||||
|
|
||||||
|
Even with bypass for READ, the scope concept in PermissionSets is essential for:
|
||||||
|
- UPDATE/CREATE/DESTROY operations
|
||||||
|
- Documentation and maintainability
|
||||||
|
- Centralized permission management
|
||||||
|
|
||||||
|
### 4. Consistency Across Resources
|
||||||
|
|
||||||
|
Following the same pattern (Bypass for READ, HasPermission for UPDATE) across User and Member resources makes the codebase more maintainable and predictable.
|
||||||
|
|
||||||
|
### 5. Documentation Is Key
|
||||||
|
|
||||||
|
Thorough documentation explaining **WHY** the pattern exists prevents future confusion and ensures the pattern is applied correctly in future resources.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Future Considerations
|
||||||
|
|
||||||
|
### If Adding New Resources with Filter-Based Permissions
|
||||||
|
|
||||||
|
Follow the same pattern:
|
||||||
|
1. Bypass with `expr()` for READ (list queries)
|
||||||
|
2. HasPermission for UPDATE/CREATE/DESTROY (uses scope from PermissionSets)
|
||||||
|
3. Define appropriate scopes in PermissionSets (`:own`, `:linked`, `:all`)
|
||||||
|
|
||||||
|
### If Ash Framework Changes
|
||||||
|
|
||||||
|
If a future version of Ash reliably calls `auto_filter` when `strict_check` returns `:unknown`:
|
||||||
|
1. Consider removing bypass for READ
|
||||||
|
2. Keep only HasPermission policy
|
||||||
|
3. Update tests to verify new behavior
|
||||||
|
4. Update documentation
|
||||||
|
|
||||||
|
**For now (Ash 3.13.1), the current pattern is correct and necessary.**
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Conclusion
|
||||||
|
|
||||||
|
✅ **User Resource Authorization Policies are fully implemented, tested, and documented.**
|
||||||
|
|
||||||
|
The implementation:
|
||||||
|
- Follows best practices for Ash policies
|
||||||
|
- Is consistent with Member resource pattern
|
||||||
|
- Uses the scope concept from PermissionSets effectively
|
||||||
|
- Has comprehensive test coverage
|
||||||
|
- Is thoroughly documented for future developers
|
||||||
|
|
||||||
|
**Status: PRODUCTION READY** 🎉
|
||||||
|
|
@ -5,9 +5,8 @@ defmodule Mv.Accounts.User do
|
||||||
use Ash.Resource,
|
use Ash.Resource,
|
||||||
domain: Mv.Accounts,
|
domain: Mv.Accounts,
|
||||||
data_layer: AshPostgres.DataLayer,
|
data_layer: AshPostgres.DataLayer,
|
||||||
extensions: [AshAuthentication]
|
extensions: [AshAuthentication],
|
||||||
|
authorizers: [Ash.Policy.Authorizer]
|
||||||
# authorizers: [Ash.Policy.Authorizer]
|
|
||||||
|
|
||||||
postgres do
|
postgres do
|
||||||
table "users"
|
table "users"
|
||||||
|
|
@ -267,6 +266,36 @@ defmodule Mv.Accounts.User do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Authorization Policies
|
||||||
|
# Order matters: Most specific policies first, then general permission check
|
||||||
|
policies do
|
||||||
|
# AshAuthentication bypass (registration/login without actor)
|
||||||
|
bypass AshAuthentication.Checks.AshAuthenticationInteraction do
|
||||||
|
description "Allow AshAuthentication internal operations (registration, login)"
|
||||||
|
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"
|
||||||
|
authorize_if expr(id == ^actor(:id))
|
||||||
|
end
|
||||||
|
|
||||||
|
# UPDATE/DESTROY via HasPermission (evaluates PermissionSets scope)
|
||||||
|
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 (fail-closed)
|
||||||
|
end
|
||||||
|
|
||||||
# Global validations - applied to all relevant actions
|
# Global validations - applied to all relevant actions
|
||||||
validations do
|
validations do
|
||||||
# Password strength policy: minimum 8 characters for all password-related actions
|
# Password strength policy: minimum 8 characters for all password-related actions
|
||||||
|
|
|
||||||
|
|
@ -42,25 +42,29 @@ defmodule Mv.Accounts.User.Validations.OidcEmailCollision do
|
||||||
if email && oidc_id && user_info do
|
if email && oidc_id && user_info do
|
||||||
# Check if a user with this oidc_id already exists
|
# Check if a user with this oidc_id already exists
|
||||||
# If yes, this will be an upsert (email update), not a new registration
|
# If yes, this will be an upsert (email update), not a new registration
|
||||||
|
# Use SystemActor for authorization during OIDC registration (no logged-in actor)
|
||||||
|
system_actor = Mv.Helpers.SystemActor.get_system_actor()
|
||||||
|
|
||||||
existing_oidc_user =
|
existing_oidc_user =
|
||||||
case Mv.Accounts.User
|
case Mv.Accounts.User
|
||||||
|> Ash.Query.filter(oidc_id == ^to_string(oidc_id))
|
|> Ash.Query.filter(oidc_id == ^to_string(oidc_id))
|
||||||
|> Ash.read_one() do
|
|> Ash.read_one(actor: system_actor) do
|
||||||
{:ok, user} -> user
|
{:ok, user} -> user
|
||||||
_ -> nil
|
_ -> nil
|
||||||
end
|
end
|
||||||
|
|
||||||
check_email_collision(email, oidc_id, user_info, existing_oidc_user)
|
check_email_collision(email, oidc_id, user_info, existing_oidc_user, system_actor)
|
||||||
else
|
else
|
||||||
:ok
|
:ok
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
defp check_email_collision(email, new_oidc_id, user_info, existing_oidc_user) do
|
defp check_email_collision(email, new_oidc_id, user_info, existing_oidc_user, system_actor) do
|
||||||
# Find existing user with this email
|
# Find existing user with this email
|
||||||
|
# Use SystemActor for authorization during OIDC registration (no logged-in actor)
|
||||||
case Mv.Accounts.User
|
case Mv.Accounts.User
|
||||||
|> Ash.Query.filter(email == ^to_string(email))
|
|> Ash.Query.filter(email == ^to_string(email))
|
||||||
|> Ash.read_one() do
|
|> Ash.read_one(actor: system_actor) do
|
||||||
{:ok, nil} ->
|
{:ok, nil} ->
|
||||||
# No user exists with this email - OK to create new user
|
# No user exists with this email - OK to create new user
|
||||||
:ok
|
:ok
|
||||||
|
|
|
||||||
|
|
@ -124,8 +124,9 @@ defmodule Mv.Membership.Member do
|
||||||
case result do
|
case result do
|
||||||
{:ok, member} ->
|
{:ok, member} ->
|
||||||
if member.membership_fee_type_id && member.join_date do
|
if member.membership_fee_type_id && member.join_date do
|
||||||
actor = Map.get(changeset.context, :actor)
|
# Capture initiator for audit trail (if available)
|
||||||
handle_cycle_generation(member, actor: actor)
|
initiator = Map.get(changeset.context, :actor)
|
||||||
|
handle_cycle_generation(member, initiator: initiator)
|
||||||
end
|
end
|
||||||
|
|
||||||
{:error, _} ->
|
{:error, _} ->
|
||||||
|
|
@ -196,16 +197,12 @@ defmodule Mv.Membership.Member do
|
||||||
Ash.Changeset.changing_attribute?(changeset, :membership_fee_type_id)
|
Ash.Changeset.changing_attribute?(changeset, :membership_fee_type_id)
|
||||||
|
|
||||||
if fee_type_changed && member.membership_fee_type_id && member.join_date do
|
if fee_type_changed && member.membership_fee_type_id && member.join_date do
|
||||||
actor = Map.get(changeset.context, :actor)
|
case regenerate_cycles_on_type_change(member) do
|
||||||
|
|
||||||
case regenerate_cycles_on_type_change(member, actor: actor) do
|
|
||||||
{:ok, notifications} ->
|
{:ok, notifications} ->
|
||||||
# Return notifications to Ash - they will be sent automatically after commit
|
# Return notifications to Ash - they will be sent automatically after commit
|
||||||
{:ok, member, notifications}
|
{:ok, member, notifications}
|
||||||
|
|
||||||
{:error, reason} ->
|
{:error, reason} ->
|
||||||
require Logger
|
|
||||||
|
|
||||||
Logger.warning(
|
Logger.warning(
|
||||||
"Failed to regenerate cycles for member #{member.id}: #{inspect(reason)}"
|
"Failed to regenerate cycles for member #{member.id}: #{inspect(reason)}"
|
||||||
)
|
)
|
||||||
|
|
@ -230,8 +227,9 @@ defmodule Mv.Membership.Member do
|
||||||
exit_date_changed = Ash.Changeset.changing_attribute?(changeset, :exit_date)
|
exit_date_changed = Ash.Changeset.changing_attribute?(changeset, :exit_date)
|
||||||
|
|
||||||
if (join_date_changed || exit_date_changed) && member.membership_fee_type_id do
|
if (join_date_changed || exit_date_changed) && member.membership_fee_type_id do
|
||||||
actor = Map.get(changeset.context, :actor)
|
# Capture initiator for audit trail (if available)
|
||||||
handle_cycle_generation(member, actor: actor)
|
initiator = Map.get(changeset.context, :actor)
|
||||||
|
handle_cycle_generation(member, initiator: initiator)
|
||||||
end
|
end
|
||||||
|
|
||||||
{:error, _} ->
|
{:error, _} ->
|
||||||
|
|
@ -409,8 +407,16 @@ defmodule Mv.Membership.Member do
|
||||||
actor = Map.get(changeset.context || %{}, :actor)
|
actor = Map.get(changeset.context || %{}, :actor)
|
||||||
|
|
||||||
# Check the current state of the user in the database
|
# Check the current state of the user in the database
|
||||||
# Pass actor to ensure proper authorization (User might have policies in future)
|
# Check if authorization is disabled in the parent operation's context
|
||||||
case Ash.get(Mv.Accounts.User, user_id, actor: actor) do
|
# Access private context where authorize? flag is stored
|
||||||
|
authorize? =
|
||||||
|
case get_in(changeset.context, [:private, :authorize?]) do
|
||||||
|
false -> false
|
||||||
|
_ -> true
|
||||||
|
end
|
||||||
|
|
||||||
|
# Pass actor and authorize? to ensure proper authorization (User might have policies in future)
|
||||||
|
case Ash.get(Mv.Accounts.User, user_id, actor: actor, authorize?: authorize?) do
|
||||||
# User is free to be linked
|
# User is free to be linked
|
||||||
{:ok, %{member_id: nil}} ->
|
{:ok, %{member_id: nil}} ->
|
||||||
:ok
|
:ok
|
||||||
|
|
@ -790,37 +796,37 @@ defmodule Mv.Membership.Member do
|
||||||
# Returns {:ok, notifications} or {:error, reason} where notifications are collected
|
# Returns {:ok, notifications} or {:error, reason} where notifications are collected
|
||||||
# to be sent after transaction commits
|
# to be sent after transaction commits
|
||||||
@doc false
|
@doc false
|
||||||
def regenerate_cycles_on_type_change(member, opts \\ []) do
|
# Uses system actor for cycle regeneration (mandatory side effect)
|
||||||
|
def regenerate_cycles_on_type_change(member, _opts \\ []) do
|
||||||
|
alias Mv.Helpers
|
||||||
|
alias Mv.Helpers.SystemActor
|
||||||
|
|
||||||
today = Date.utc_today()
|
today = Date.utc_today()
|
||||||
lock_key = :erlang.phash2(member.id)
|
lock_key = :erlang.phash2(member.id)
|
||||||
actor = Keyword.get(opts, :actor)
|
|
||||||
|
|
||||||
# Use advisory lock to prevent concurrent deletion and regeneration
|
# Use advisory lock to prevent concurrent deletion and regeneration
|
||||||
# This ensures atomicity when multiple updates happen simultaneously
|
# This ensures atomicity when multiple updates happen simultaneously
|
||||||
if Mv.Repo.in_transaction?() do
|
if Mv.Repo.in_transaction?() do
|
||||||
regenerate_cycles_in_transaction(member, today, lock_key, actor: actor)
|
regenerate_cycles_in_transaction(member, today, lock_key)
|
||||||
else
|
else
|
||||||
regenerate_cycles_new_transaction(member, today, lock_key, actor: actor)
|
regenerate_cycles_new_transaction(member, today, lock_key)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
# Already in transaction: use advisory lock directly
|
# Already in transaction: use advisory lock directly
|
||||||
# Returns {:ok, notifications} - notifications should be returned to after_action hook
|
# Returns {:ok, notifications} - notifications should be returned to after_action hook
|
||||||
defp regenerate_cycles_in_transaction(member, today, lock_key, opts) do
|
defp regenerate_cycles_in_transaction(member, today, lock_key) do
|
||||||
actor = Keyword.get(opts, :actor)
|
|
||||||
Ecto.Adapters.SQL.query!(Mv.Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key])
|
Ecto.Adapters.SQL.query!(Mv.Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key])
|
||||||
do_regenerate_cycles_on_type_change(member, today, skip_lock?: true, actor: actor)
|
do_regenerate_cycles_on_type_change(member, today, skip_lock?: true)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Not in transaction: start new transaction with advisory lock
|
# Not in transaction: start new transaction with advisory lock
|
||||||
# Returns {:ok, notifications} - notifications should be sent by caller (e.g., via after_action)
|
# Returns {:ok, notifications} - notifications should be sent by caller (e.g., via after_action)
|
||||||
defp regenerate_cycles_new_transaction(member, today, lock_key, opts) do
|
defp regenerate_cycles_new_transaction(member, today, lock_key) do
|
||||||
actor = Keyword.get(opts, :actor)
|
|
||||||
|
|
||||||
Mv.Repo.transaction(fn ->
|
Mv.Repo.transaction(fn ->
|
||||||
Ecto.Adapters.SQL.query!(Mv.Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key])
|
Ecto.Adapters.SQL.query!(Mv.Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key])
|
||||||
|
|
||||||
case do_regenerate_cycles_on_type_change(member, today, skip_lock?: true, actor: actor) do
|
case do_regenerate_cycles_on_type_change(member, today, skip_lock?: true) do
|
||||||
{:ok, notifications} ->
|
{:ok, notifications} ->
|
||||||
# Return notifications - they will be sent by the caller
|
# Return notifications - they will be sent by the caller
|
||||||
notifications
|
notifications
|
||||||
|
|
@ -838,11 +844,16 @@ defmodule Mv.Membership.Member do
|
||||||
# Performs the actual cycle deletion and regeneration
|
# Performs the actual cycle deletion and regeneration
|
||||||
# Returns {:ok, notifications} or {:error, reason}
|
# Returns {:ok, notifications} or {:error, reason}
|
||||||
# notifications are collected to be sent after transaction commits
|
# notifications are collected to be sent after transaction commits
|
||||||
|
# Uses system actor for all operations
|
||||||
defp do_regenerate_cycles_on_type_change(member, today, opts) do
|
defp do_regenerate_cycles_on_type_change(member, today, opts) do
|
||||||
|
alias Mv.Helpers
|
||||||
|
alias Mv.Helpers.SystemActor
|
||||||
|
|
||||||
require Ash.Query
|
require Ash.Query
|
||||||
|
|
||||||
skip_lock? = Keyword.get(opts, :skip_lock?, false)
|
skip_lock? = Keyword.get(opts, :skip_lock?, false)
|
||||||
actor = Keyword.get(opts, :actor)
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
actor_opts = Helpers.ash_actor_opts(system_actor)
|
||||||
|
|
||||||
# Find all unpaid cycles for this member
|
# Find all unpaid cycles for this member
|
||||||
# We need to check cycle_end for each cycle using its own interval
|
# We need to check cycle_end for each cycle using its own interval
|
||||||
|
|
@ -852,20 +863,16 @@ defmodule Mv.Membership.Member do
|
||||||
|> Ash.Query.filter(status == :unpaid)
|
|> Ash.Query.filter(status == :unpaid)
|
||||||
|> Ash.Query.load([:membership_fee_type])
|
|> Ash.Query.load([:membership_fee_type])
|
||||||
|
|
||||||
result =
|
case Ash.read(all_unpaid_cycles_query, actor_opts) do
|
||||||
if actor do
|
|
||||||
Ash.read(all_unpaid_cycles_query, actor: actor)
|
|
||||||
else
|
|
||||||
Ash.read(all_unpaid_cycles_query)
|
|
||||||
end
|
|
||||||
|
|
||||||
case result do
|
|
||||||
{:ok, all_unpaid_cycles} ->
|
{:ok, all_unpaid_cycles} ->
|
||||||
cycles_to_delete = filter_future_cycles(all_unpaid_cycles, today)
|
cycles_to_delete = filter_future_cycles(all_unpaid_cycles, today)
|
||||||
|
|
||||||
delete_and_regenerate_cycles(cycles_to_delete, member.id, today,
|
delete_and_regenerate_cycles(
|
||||||
skip_lock?: skip_lock?,
|
cycles_to_delete,
|
||||||
actor: actor
|
member.id,
|
||||||
|
today,
|
||||||
|
actor_opts,
|
||||||
|
skip_lock?: skip_lock?
|
||||||
)
|
)
|
||||||
|
|
||||||
{:error, reason} ->
|
{:error, reason} ->
|
||||||
|
|
@ -893,26 +900,27 @@ defmodule Mv.Membership.Member do
|
||||||
# Deletes future cycles and regenerates them with the new type/amount
|
# Deletes future cycles and regenerates them with the new type/amount
|
||||||
# Passes today to ensure consistent date across deletion and regeneration
|
# Passes today to ensure consistent date across deletion and regeneration
|
||||||
# Returns {:ok, notifications} or {:error, reason}
|
# Returns {:ok, notifications} or {:error, reason}
|
||||||
defp delete_and_regenerate_cycles(cycles_to_delete, member_id, today, opts) do
|
# Uses system actor for cycle generation and deletion
|
||||||
|
defp delete_and_regenerate_cycles(cycles_to_delete, member_id, today, actor_opts, opts) do
|
||||||
skip_lock? = Keyword.get(opts, :skip_lock?, false)
|
skip_lock? = Keyword.get(opts, :skip_lock?, false)
|
||||||
actor = Keyword.get(opts, :actor)
|
|
||||||
|
|
||||||
if Enum.empty?(cycles_to_delete) do
|
if Enum.empty?(cycles_to_delete) do
|
||||||
# No cycles to delete, just regenerate
|
# No cycles to delete, just regenerate
|
||||||
regenerate_cycles(member_id, today, skip_lock?: skip_lock?, actor: actor)
|
regenerate_cycles(member_id, today, skip_lock?: skip_lock?)
|
||||||
else
|
else
|
||||||
case delete_cycles(cycles_to_delete) do
|
case delete_cycles(cycles_to_delete, actor_opts) do
|
||||||
:ok -> regenerate_cycles(member_id, today, skip_lock?: skip_lock?, actor: actor)
|
:ok -> regenerate_cycles(member_id, today, skip_lock?: skip_lock?)
|
||||||
{:error, reason} -> {:error, reason}
|
{:error, reason} -> {:error, reason}
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
# Deletes cycles and returns :ok if all succeeded, {:error, reason} otherwise
|
# Deletes cycles and returns :ok if all succeeded, {:error, reason} otherwise
|
||||||
defp delete_cycles(cycles_to_delete) do
|
# Uses system actor for authorization to ensure deletion always works
|
||||||
|
defp delete_cycles(cycles_to_delete, actor_opts) do
|
||||||
delete_results =
|
delete_results =
|
||||||
Enum.map(cycles_to_delete, fn cycle ->
|
Enum.map(cycles_to_delete, fn cycle ->
|
||||||
Ash.destroy(cycle)
|
Ash.destroy(cycle, actor_opts)
|
||||||
end)
|
end)
|
||||||
|
|
||||||
if Enum.any?(delete_results, &match?({:error, _}, &1)) do
|
if Enum.any?(delete_results, &match?({:error, _}, &1)) do
|
||||||
|
|
@ -928,13 +936,11 @@ defmodule Mv.Membership.Member do
|
||||||
# Returns {:ok, notifications} - notifications should be returned to after_action hook
|
# Returns {:ok, notifications} - notifications should be returned to after_action hook
|
||||||
defp regenerate_cycles(member_id, today, opts) do
|
defp regenerate_cycles(member_id, today, opts) do
|
||||||
skip_lock? = Keyword.get(opts, :skip_lock?, false)
|
skip_lock? = Keyword.get(opts, :skip_lock?, false)
|
||||||
actor = Keyword.get(opts, :actor)
|
|
||||||
|
|
||||||
case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(
|
case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(
|
||||||
member_id,
|
member_id,
|
||||||
today: today,
|
today: today,
|
||||||
skip_lock?: skip_lock?,
|
skip_lock?: skip_lock?
|
||||||
actor: actor
|
|
||||||
) do
|
) do
|
||||||
{:ok, _cycles, notifications} when is_list(notifications) ->
|
{:ok, _cycles, notifications} when is_list(notifications) ->
|
||||||
{:ok, notifications}
|
{:ok, notifications}
|
||||||
|
|
@ -948,49 +954,57 @@ defmodule Mv.Membership.Member do
|
||||||
# based on environment (test vs production)
|
# based on environment (test vs production)
|
||||||
# This function encapsulates the common logic for cycle generation
|
# This function encapsulates the common logic for cycle generation
|
||||||
# to avoid code duplication across different hooks
|
# to avoid code duplication across different hooks
|
||||||
|
# Uses system actor for cycle generation (mandatory side effect)
|
||||||
|
# Captures initiator for audit trail (if available in opts)
|
||||||
defp handle_cycle_generation(member, opts) do
|
defp handle_cycle_generation(member, opts) do
|
||||||
actor = Keyword.get(opts, :actor)
|
initiator = Keyword.get(opts, :initiator)
|
||||||
|
|
||||||
if Mv.Config.sql_sandbox?() do
|
if Mv.Config.sql_sandbox?() do
|
||||||
handle_cycle_generation_sync(member, actor: actor)
|
handle_cycle_generation_sync(member, initiator)
|
||||||
else
|
else
|
||||||
handle_cycle_generation_async(member, actor: actor)
|
handle_cycle_generation_async(member, initiator)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
# Runs cycle generation synchronously (for test environment)
|
# Runs cycle generation synchronously (for test environment)
|
||||||
defp handle_cycle_generation_sync(member, opts) do
|
defp handle_cycle_generation_sync(member, initiator) do
|
||||||
require Logger
|
|
||||||
actor = Keyword.get(opts, :actor)
|
|
||||||
|
|
||||||
case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(
|
case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(
|
||||||
member.id,
|
member.id,
|
||||||
today: Date.utc_today(),
|
today: Date.utc_today(),
|
||||||
actor: actor
|
initiator: initiator
|
||||||
) do
|
) do
|
||||||
{:ok, cycles, notifications} ->
|
{:ok, cycles, notifications} ->
|
||||||
send_notifications_if_any(notifications)
|
send_notifications_if_any(notifications)
|
||||||
log_cycle_generation_success(member, cycles, notifications, sync: true)
|
|
||||||
|
log_cycle_generation_success(member, cycles, notifications,
|
||||||
|
sync: true,
|
||||||
|
initiator: initiator
|
||||||
|
)
|
||||||
|
|
||||||
{:error, reason} ->
|
{:error, reason} ->
|
||||||
log_cycle_generation_error(member, reason, sync: true)
|
log_cycle_generation_error(member, reason, sync: true, initiator: initiator)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
# Runs cycle generation asynchronously (for production environment)
|
# Runs cycle generation asynchronously (for production environment)
|
||||||
defp handle_cycle_generation_async(member, opts) do
|
defp handle_cycle_generation_async(member, initiator) do
|
||||||
actor = Keyword.get(opts, :actor)
|
|
||||||
|
|
||||||
Task.Supervisor.async_nolink(Mv.TaskSupervisor, fn ->
|
Task.Supervisor.async_nolink(Mv.TaskSupervisor, fn ->
|
||||||
case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(member.id, actor: actor) do
|
case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(member.id,
|
||||||
|
initiator: initiator
|
||||||
|
) do
|
||||||
{:ok, cycles, notifications} ->
|
{:ok, cycles, notifications} ->
|
||||||
send_notifications_if_any(notifications)
|
send_notifications_if_any(notifications)
|
||||||
log_cycle_generation_success(member, cycles, notifications, sync: false)
|
|
||||||
|
log_cycle_generation_success(member, cycles, notifications,
|
||||||
|
sync: false,
|
||||||
|
initiator: initiator
|
||||||
|
)
|
||||||
|
|
||||||
{:error, reason} ->
|
{:error, reason} ->
|
||||||
log_cycle_generation_error(member, reason, sync: false)
|
log_cycle_generation_error(member, reason, sync: false, initiator: initiator)
|
||||||
end
|
end
|
||||||
end)
|
end)
|
||||||
|
|> Task.await(:infinity)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Sends notifications if any are present
|
# Sends notifications if any are present
|
||||||
|
|
@ -1001,13 +1015,15 @@ defmodule Mv.Membership.Member do
|
||||||
end
|
end
|
||||||
|
|
||||||
# Logs successful cycle generation
|
# Logs successful cycle generation
|
||||||
defp log_cycle_generation_success(member, cycles, notifications, sync: sync?) do
|
defp log_cycle_generation_success(member, cycles, notifications,
|
||||||
require Logger
|
sync: sync?,
|
||||||
|
initiator: initiator
|
||||||
|
) do
|
||||||
sync_label = if sync?, do: "", else: " (async)"
|
sync_label = if sync?, do: "", else: " (async)"
|
||||||
|
initiator_info = get_initiator_info(initiator)
|
||||||
|
|
||||||
Logger.debug(
|
Logger.debug(
|
||||||
"Successfully generated cycles for member#{sync_label}",
|
"Successfully generated cycles for member#{sync_label} (initiator: #{initiator_info})",
|
||||||
member_id: member.id,
|
member_id: member.id,
|
||||||
cycles_count: length(cycles),
|
cycles_count: length(cycles),
|
||||||
notifications_count: length(notifications)
|
notifications_count: length(notifications)
|
||||||
|
|
@ -1015,13 +1031,12 @@ defmodule Mv.Membership.Member do
|
||||||
end
|
end
|
||||||
|
|
||||||
# Logs cycle generation errors
|
# Logs cycle generation errors
|
||||||
defp log_cycle_generation_error(member, reason, sync: sync?) do
|
defp log_cycle_generation_error(member, reason, sync: sync?, initiator: initiator) do
|
||||||
require Logger
|
|
||||||
|
|
||||||
sync_label = if sync?, do: "", else: " (async)"
|
sync_label = if sync?, do: "", else: " (async)"
|
||||||
|
initiator_info = get_initiator_info(initiator)
|
||||||
|
|
||||||
Logger.error(
|
Logger.error(
|
||||||
"Failed to generate cycles for member#{sync_label}",
|
"Failed to generate cycles for member#{sync_label} (initiator: #{initiator_info})",
|
||||||
member_id: member.id,
|
member_id: member.id,
|
||||||
member_email: member.email,
|
member_email: member.email,
|
||||||
error: inspect(reason),
|
error: inspect(reason),
|
||||||
|
|
@ -1029,6 +1044,11 @@ defmodule Mv.Membership.Member do
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Extracts initiator information for audit trail
|
||||||
|
defp get_initiator_info(nil), do: "system"
|
||||||
|
defp get_initiator_info(%{email: email}), do: email
|
||||||
|
defp get_initiator_info(_), do: "unknown"
|
||||||
|
|
||||||
# Helper to extract error type for structured logging
|
# Helper to extract error type for structured logging
|
||||||
defp error_type(%{__struct__: struct_name}), do: struct_name
|
defp error_type(%{__struct__: struct_name}), do: struct_name
|
||||||
defp error_type(error) when is_atom(error), do: error
|
defp error_type(error) when is_atom(error), do: error
|
||||||
|
|
|
||||||
|
|
@ -9,6 +9,8 @@ defmodule Mv.Accounts.User.Validations.EmailNotUsedByOtherMember do
|
||||||
"""
|
"""
|
||||||
use Ash.Resource.Validation
|
use Ash.Resource.Validation
|
||||||
|
|
||||||
|
require Logger
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
Validates email uniqueness across linked User-Member pairs.
|
Validates email uniqueness across linked User-Member pairs.
|
||||||
|
|
||||||
|
|
@ -73,19 +75,29 @@ defmodule Mv.Accounts.User.Validations.EmailNotUsedByOtherMember do
|
||||||
end
|
end
|
||||||
|
|
||||||
defp check_email_uniqueness(email, exclude_member_id) do
|
defp check_email_uniqueness(email, exclude_member_id) do
|
||||||
|
alias Mv.Helpers
|
||||||
|
alias Mv.Helpers.SystemActor
|
||||||
|
|
||||||
query =
|
query =
|
||||||
Mv.Membership.Member
|
Mv.Membership.Member
|
||||||
|> Ash.Query.filter(email == ^to_string(email))
|
|> Ash.Query.filter(email == ^to_string(email))
|
||||||
|> maybe_exclude_id(exclude_member_id)
|
|> maybe_exclude_id(exclude_member_id)
|
||||||
|
|
||||||
case Ash.read(query) do
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
opts = Helpers.ash_actor_opts(system_actor)
|
||||||
|
|
||||||
|
case Ash.read(query, opts) do
|
||||||
{:ok, []} ->
|
{:ok, []} ->
|
||||||
:ok
|
:ok
|
||||||
|
|
||||||
{:ok, _} ->
|
{:ok, _} ->
|
||||||
{:error, field: :email, message: "is already used by another member", value: email}
|
{:error, field: :email, message: "is already used by another member", value: email}
|
||||||
|
|
||||||
{:error, _} ->
|
{:error, reason} ->
|
||||||
|
Logger.warning(
|
||||||
|
"Email uniqueness validation query failed for user email '#{email}': #{inspect(reason)}. Allowing operation to proceed (fail-open)."
|
||||||
|
)
|
||||||
|
|
||||||
:ok
|
:ok
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
||||||
|
|
@ -14,6 +14,7 @@ defmodule Mv.Application do
|
||||||
{DNSCluster, query: Application.get_env(:mv, :dns_cluster_query) || :ignore},
|
{DNSCluster, query: Application.get_env(:mv, :dns_cluster_query) || :ignore},
|
||||||
{Phoenix.PubSub, name: Mv.PubSub},
|
{Phoenix.PubSub, name: Mv.PubSub},
|
||||||
{AshAuthentication.Supervisor, otp_app: :my},
|
{AshAuthentication.Supervisor, otp_app: :my},
|
||||||
|
Mv.Helpers.SystemActor,
|
||||||
# Start a worker by calling: Mv.Worker.start_link(arg)
|
# Start a worker by calling: Mv.Worker.start_link(arg)
|
||||||
# {Mv.Worker, arg},
|
# {Mv.Worker, arg},
|
||||||
# Start to serve requests, typically the last entry
|
# Start to serve requests, typically the last entry
|
||||||
|
|
|
||||||
99
lib/mv/authorization/actor.ex
Normal file
99
lib/mv/authorization/actor.ex
Normal file
|
|
@ -0,0 +1,99 @@
|
||||||
|
defmodule Mv.Authorization.Actor do
|
||||||
|
@moduledoc """
|
||||||
|
Helper functions for ensuring User actors have required data loaded.
|
||||||
|
|
||||||
|
## Actor Invariant
|
||||||
|
|
||||||
|
Authorization policies (especially HasPermission) require that the User actor
|
||||||
|
has their `:role` relationship loaded. This module provides helpers to
|
||||||
|
ensure this invariant is maintained across all entry points:
|
||||||
|
|
||||||
|
- LiveView on_mount hooks
|
||||||
|
- Plug pipelines
|
||||||
|
- Background jobs
|
||||||
|
- Tests
|
||||||
|
|
||||||
|
## Scope
|
||||||
|
|
||||||
|
This module ONLY handles `Mv.Accounts.User` resources. Other resources with
|
||||||
|
a `:role` field are ignored (returned as-is). This prevents accidental
|
||||||
|
authorization bypasses and keeps the logic focused.
|
||||||
|
|
||||||
|
## Usage
|
||||||
|
|
||||||
|
# In LiveView on_mount
|
||||||
|
def ensure_user_role_loaded(_name, socket) do
|
||||||
|
user = Actor.ensure_loaded(socket.assigns[:current_user])
|
||||||
|
assign(socket, :current_user, user)
|
||||||
|
end
|
||||||
|
|
||||||
|
# In tests
|
||||||
|
user = Actor.ensure_loaded(user)
|
||||||
|
|
||||||
|
## Security Note
|
||||||
|
|
||||||
|
`ensure_loaded/1` loads the role with `authorize?: false` to avoid circular
|
||||||
|
dependency (actor needs role loaded to be authorized, but loading role requires
|
||||||
|
authorization). This is safe because:
|
||||||
|
|
||||||
|
- The actor (User) is loading their OWN role (user.role relationship)
|
||||||
|
- This load is needed FOR authorization checks to work
|
||||||
|
- The role itself contains no sensitive data (just permission_set reference)
|
||||||
|
- The actor is already authenticated (passed auth boundary)
|
||||||
|
|
||||||
|
Alternative would be to denormalize permission_set_name on User, but that
|
||||||
|
adds complexity and potential for inconsistency.
|
||||||
|
"""
|
||||||
|
|
||||||
|
require Logger
|
||||||
|
|
||||||
|
@doc """
|
||||||
|
Ensures the actor (User) has their `:role` relationship loaded.
|
||||||
|
|
||||||
|
- If actor is nil, returns nil
|
||||||
|
- If role is already loaded, returns actor as-is
|
||||||
|
- If role is %Ash.NotLoaded{}, loads it and returns updated actor
|
||||||
|
- If actor is not a User, returns as-is (no-op)
|
||||||
|
|
||||||
|
## Examples
|
||||||
|
|
||||||
|
iex> Actor.ensure_loaded(nil)
|
||||||
|
nil
|
||||||
|
|
||||||
|
iex> Actor.ensure_loaded(%User{role: %Role{}})
|
||||||
|
%User{role: %Role{}}
|
||||||
|
|
||||||
|
iex> Actor.ensure_loaded(%User{role: %Ash.NotLoaded{}})
|
||||||
|
%User{role: %Role{}} # role loaded
|
||||||
|
"""
|
||||||
|
def ensure_loaded(nil), do: nil
|
||||||
|
|
||||||
|
# Only handle Mv.Accounts.User - clear intention, no accidental other resources
|
||||||
|
def ensure_loaded(%Mv.Accounts.User{role: %Ash.NotLoaded{}} = user) do
|
||||||
|
load_role(user)
|
||||||
|
end
|
||||||
|
|
||||||
|
def ensure_loaded(actor), do: actor
|
||||||
|
|
||||||
|
defp load_role(actor) do
|
||||||
|
# SECURITY: We skip authorization here because this is a bootstrap scenario:
|
||||||
|
# - The actor is loading their OWN role (actor.role relationship)
|
||||||
|
# - This load is needed FOR authorization checks to work (circular dependency)
|
||||||
|
# - The role itself contains no sensitive data (just permission_set reference)
|
||||||
|
# - The actor is already authenticated (passed auth boundary)
|
||||||
|
# Alternative would be to denormalize permission_set_name on User.
|
||||||
|
case Ash.load(actor, :role, domain: Mv.Accounts, authorize?: false) do
|
||||||
|
{:ok, loaded_actor} ->
|
||||||
|
loaded_actor
|
||||||
|
|
||||||
|
{:error, error} ->
|
||||||
|
# Log error but don't crash - fail-closed for authorization
|
||||||
|
Logger.warning(
|
||||||
|
"Failed to load actor role: #{inspect(error)}. " <>
|
||||||
|
"Authorization may fail if role is required."
|
||||||
|
)
|
||||||
|
|
||||||
|
actor
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
@ -8,10 +8,37 @@ defmodule Mv.Authorization.Checks.HasPermission do
|
||||||
3. Finds matching permission for current resource + action
|
3. Finds matching permission for current resource + action
|
||||||
4. Applies scope filter (:own, :linked, :all)
|
4. Applies scope filter (:own, :linked, :all)
|
||||||
|
|
||||||
|
## Important: strict_check Behavior
|
||||||
|
|
||||||
|
For filter-based scopes (`:own`, `:linked`):
|
||||||
|
- **WITH record**: Evaluates filter against record (returns `true`/`false`)
|
||||||
|
- **WITHOUT record** (queries/lists): Returns `false`
|
||||||
|
|
||||||
|
**Why `false` instead of `:unknown`?**
|
||||||
|
|
||||||
|
Ash's policy evaluation doesn't reliably call `auto_filter` when `strict_check`
|
||||||
|
returns `:unknown`. To ensure list queries work correctly, resources **MUST** use
|
||||||
|
bypass policies with `expr()` for READ operations (see `docs/policy-bypass-vs-haspermission.md`).
|
||||||
|
|
||||||
|
This means `HasPermission` is **NOT** generically reusable for query authorization
|
||||||
|
with filter scopes - it requires companion bypass policies.
|
||||||
|
|
||||||
|
## Usage Pattern
|
||||||
|
|
||||||
|
See `docs/policy-bypass-vs-haspermission.md` for the two-tier pattern:
|
||||||
|
- **READ**: `bypass` with `expr()` (handles auto_filter)
|
||||||
|
- **UPDATE/CREATE/DESTROY**: `HasPermission` (handles scope evaluation)
|
||||||
|
|
||||||
## Usage in Ash Resource
|
## Usage in Ash Resource
|
||||||
|
|
||||||
policies do
|
policies do
|
||||||
policy action_type(:read) do
|
# READ: Bypass for list queries
|
||||||
|
bypass action_type(:read) do
|
||||||
|
authorize_if expr(id == ^actor(:id))
|
||||||
|
end
|
||||||
|
|
||||||
|
# UPDATE: HasPermission for scope evaluation
|
||||||
|
policy action_type([:update, :create, :destroy]) do
|
||||||
authorize_if Mv.Authorization.Checks.HasPermission
|
authorize_if Mv.Authorization.Checks.HasPermission
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
@ -34,6 +61,12 @@ defmodule Mv.Authorization.Checks.HasPermission do
|
||||||
|
|
||||||
All errors result in Forbidden (policy fails).
|
All errors result in Forbidden (policy fails).
|
||||||
|
|
||||||
|
## Role Loading Fallback
|
||||||
|
|
||||||
|
If the actor's `:role` relationship is `%Ash.NotLoaded{}`, this check will
|
||||||
|
attempt to load it automatically. This provides a fallback if `on_mount` hooks
|
||||||
|
didn't run (e.g., in non-LiveView contexts).
|
||||||
|
|
||||||
## Examples
|
## Examples
|
||||||
|
|
||||||
# In a resource policy
|
# In a resource policy
|
||||||
|
|
@ -83,6 +116,9 @@ defmodule Mv.Authorization.Checks.HasPermission do
|
||||||
|
|
||||||
# Helper function to reduce nesting depth
|
# Helper function to reduce nesting depth
|
||||||
defp strict_check_with_permissions(actor, resource, action, record) do
|
defp strict_check_with_permissions(actor, resource, action, record) do
|
||||||
|
# Ensure role is loaded (fallback if on_mount didn't run)
|
||||||
|
actor = ensure_role_loaded(actor)
|
||||||
|
|
||||||
with %{role: %{permission_set_name: ps_name}} when not is_nil(ps_name) <- actor,
|
with %{role: %{permission_set_name: ps_name}} when not is_nil(ps_name) <- actor,
|
||||||
{:ok, ps_atom} <- PermissionSets.permission_set_name_to_atom(ps_name),
|
{:ok, ps_atom} <- PermissionSets.permission_set_name_to_atom(ps_name),
|
||||||
permissions <- PermissionSets.get_permissions(ps_atom),
|
permissions <- PermissionSets.get_permissions(ps_atom),
|
||||||
|
|
@ -95,11 +131,25 @@ defmodule Mv.Authorization.Checks.HasPermission do
|
||||||
resource_name
|
resource_name
|
||||||
) do
|
) do
|
||||||
:authorized ->
|
:authorized ->
|
||||||
|
# For :all scope, authorize directly
|
||||||
{:ok, true}
|
{:ok, true}
|
||||||
|
|
||||||
{:filter, filter_expr} ->
|
{:filter, filter_expr} ->
|
||||||
# For strict_check on single records, evaluate the filter against the record
|
# 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)
|
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 ->
|
false ->
|
||||||
{:ok, false}
|
{:ok, false}
|
||||||
|
|
@ -224,9 +274,18 @@ defmodule Mv.Authorization.Checks.HasPermission do
|
||||||
end
|
end
|
||||||
|
|
||||||
# Evaluate filter expression for strict_check on single records
|
# 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
|
# For :linked scope with Member resource: id == actor.member_id
|
||||||
defp evaluate_filter_for_strict_check(_filter_expr, actor, record, resource_name) do
|
defp evaluate_filter_for_strict_check(_filter_expr, actor, record, resource_name) do
|
||||||
case {resource_name, record} 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) ->
|
{"Member", %{id: member_id}} when not is_nil(member_id) ->
|
||||||
# Check if this member's ID matches the actor's member_id
|
# Check if this member's ID matches the actor's member_id
|
||||||
if member_id == actor.member_id do
|
if member_id == actor.member_id do
|
||||||
|
|
@ -330,4 +389,10 @@ defmodule Mv.Authorization.Checks.HasPermission do
|
||||||
defp get_resource_name_for_logging(_resource) do
|
defp get_resource_name_for_logging(_resource) do
|
||||||
"unknown"
|
"unknown"
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Fallback: Load role if not loaded (in case on_mount didn't run)
|
||||||
|
# Delegates to centralized Actor helper
|
||||||
|
defp ensure_role_loaded(actor) do
|
||||||
|
Mv.Authorization.Actor.ensure_loaded(actor)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
||||||
|
|
@ -42,9 +42,9 @@ defmodule Mv.Authorization.Checks.NoActor do
|
||||||
use Ash.Policy.SimpleCheck
|
use Ash.Policy.SimpleCheck
|
||||||
|
|
||||||
# Compile-time check: Only allow no-actor bypass in test environment
|
# Compile-time check: Only allow no-actor bypass in test environment
|
||||||
@allow_no_actor_bypass Mix.env() == :test
|
# SECURITY: This must ONLY be true in test.exs, never in prod/dev
|
||||||
# Alternative (if you want to control via config):
|
# Using compile_env instead of Mix.env() for release-safety
|
||||||
# @allow_no_actor_bypass Application.compile_env(:mv, :allow_no_actor_bypass, false)
|
@allow_no_actor_bypass Application.compile_env(:mv, :allow_no_actor_bypass, false)
|
||||||
|
|
||||||
@impl true
|
@impl true
|
||||||
def describe(_opts) do
|
def describe(_opts) do
|
||||||
|
|
@ -58,13 +58,9 @@ defmodule Mv.Authorization.Checks.NoActor do
|
||||||
@impl true
|
@impl true
|
||||||
def match?(nil, _context, _opts) do
|
def match?(nil, _context, _opts) do
|
||||||
# Actor is nil
|
# Actor is nil
|
||||||
if @allow_no_actor_bypass do
|
# SECURITY: Only allow if compile_env flag is set (test.exs only)
|
||||||
# Test environment: Allow all operations
|
# No runtime Mix.env() check - fail-closed by default (false)
|
||||||
true
|
@allow_no_actor_bypass
|
||||||
else
|
|
||||||
# Production/dev: Deny all operations (fail-closed for security)
|
|
||||||
false
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def match?(_actor, _context, _opts) do
|
def match?(_actor, _context, _opts) do
|
||||||
|
|
|
||||||
|
|
@ -95,7 +95,9 @@ defmodule Mv.Authorization.PermissionSets do
|
||||||
def get_permissions(:own_data) do
|
def get_permissions(:own_data) do
|
||||||
%{
|
%{
|
||||||
resources: [
|
resources: [
|
||||||
# User: Can always read/update own credentials
|
# User: Can read/update own credentials only
|
||||||
|
# IMPORTANT: "read_only" refers to member data, NOT user credentials.
|
||||||
|
# All permission sets grant User.update :own to allow password changes.
|
||||||
%{resource: "User", action: :read, scope: :own, granted: true},
|
%{resource: "User", action: :read, scope: :own, granted: true},
|
||||||
%{resource: "User", action: :update, scope: :own, granted: true},
|
%{resource: "User", action: :update, scope: :own, granted: true},
|
||||||
|
|
||||||
|
|
@ -125,6 +127,8 @@ defmodule Mv.Authorization.PermissionSets do
|
||||||
%{
|
%{
|
||||||
resources: [
|
resources: [
|
||||||
# User: Can read/update own credentials only
|
# User: Can read/update own credentials only
|
||||||
|
# IMPORTANT: "read_only" refers to member data, NOT user credentials.
|
||||||
|
# All permission sets grant User.update :own to allow password changes.
|
||||||
%{resource: "User", action: :read, scope: :own, granted: true},
|
%{resource: "User", action: :read, scope: :own, granted: true},
|
||||||
%{resource: "User", action: :update, scope: :own, granted: true},
|
%{resource: "User", action: :update, scope: :own, granted: true},
|
||||||
|
|
||||||
|
|
@ -157,6 +161,8 @@ defmodule Mv.Authorization.PermissionSets do
|
||||||
%{
|
%{
|
||||||
resources: [
|
resources: [
|
||||||
# User: Can read/update own credentials only
|
# User: Can read/update own credentials only
|
||||||
|
# IMPORTANT: "read_only" refers to member data, NOT user credentials.
|
||||||
|
# All permission sets grant User.update :own to allow password changes.
|
||||||
%{resource: "User", action: :read, scope: :own, granted: true},
|
%{resource: "User", action: :read, scope: :own, granted: true},
|
||||||
%{resource: "User", action: :update, scope: :own, granted: true},
|
%{resource: "User", action: :update, scope: :own, granted: true},
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -41,10 +41,8 @@ defmodule Mv.EmailSync.Changes.SyncMemberEmailToUser do
|
||||||
Ash.Changeset.around_transaction(changeset, fn cs, callback ->
|
Ash.Changeset.around_transaction(changeset, fn cs, callback ->
|
||||||
result = callback.(cs)
|
result = callback.(cs)
|
||||||
|
|
||||||
actor = Map.get(changeset.context, :actor)
|
|
||||||
|
|
||||||
with {:ok, member} <- Helpers.extract_record(result),
|
with {:ok, member} <- Helpers.extract_record(result),
|
||||||
linked_user <- Loader.get_linked_user(member, actor) do
|
linked_user <- Loader.get_linked_user(member) do
|
||||||
Helpers.sync_email_to_linked_record(result, linked_user, new_email)
|
Helpers.sync_email_to_linked_record(result, linked_user, new_email)
|
||||||
else
|
else
|
||||||
_ -> result
|
_ -> result
|
||||||
|
|
|
||||||
|
|
@ -33,17 +33,7 @@ defmodule Mv.EmailSync.Changes.SyncUserEmailToMember do
|
||||||
if Map.get(context, :syncing_email, false) do
|
if Map.get(context, :syncing_email, false) do
|
||||||
changeset
|
changeset
|
||||||
else
|
else
|
||||||
# Ensure actor is in changeset context - get it from context if available
|
sync_email(changeset)
|
||||||
actor = Map.get(changeset.context, :actor) || Map.get(context, :actor)
|
|
||||||
|
|
||||||
changeset_with_actor =
|
|
||||||
if actor && !Map.has_key?(changeset.context, :actor) do
|
|
||||||
Ash.Changeset.put_context(changeset, :actor, actor)
|
|
||||||
else
|
|
||||||
changeset
|
|
||||||
end
|
|
||||||
|
|
||||||
sync_email(changeset_with_actor)
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
@ -52,7 +42,7 @@ defmodule Mv.EmailSync.Changes.SyncUserEmailToMember do
|
||||||
result = callback.(cs)
|
result = callback.(cs)
|
||||||
|
|
||||||
with {:ok, record} <- Helpers.extract_record(result),
|
with {:ok, record} <- Helpers.extract_record(result),
|
||||||
{:ok, user, member} <- get_user_and_member(record, cs) do
|
{:ok, user, member} <- get_user_and_member(record) do
|
||||||
# When called from Member-side, we need to update the member in the result
|
# When called from Member-side, we need to update the member in the result
|
||||||
# When called from User-side, we update the linked member in DB only
|
# When called from User-side, we update the linked member in DB only
|
||||||
case record do
|
case record do
|
||||||
|
|
@ -71,19 +61,16 @@ defmodule Mv.EmailSync.Changes.SyncUserEmailToMember do
|
||||||
end
|
end
|
||||||
|
|
||||||
# Retrieves user and member - works for both resource types
|
# Retrieves user and member - works for both resource types
|
||||||
defp get_user_and_member(%Mv.Accounts.User{} = user, changeset) do
|
# Uses system actor via Loader functions
|
||||||
actor = Map.get(changeset.context, :actor)
|
defp get_user_and_member(%Mv.Accounts.User{} = user) do
|
||||||
|
case Loader.get_linked_member(user) do
|
||||||
case Loader.get_linked_member(user, actor) do
|
|
||||||
nil -> {:error, :no_member}
|
nil -> {:error, :no_member}
|
||||||
member -> {:ok, user, member}
|
member -> {:ok, user, member}
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
defp get_user_and_member(%Mv.Membership.Member{} = member, changeset) do
|
defp get_user_and_member(%Mv.Membership.Member{} = member) do
|
||||||
actor = Map.get(changeset.context, :actor)
|
case Loader.load_linked_user!(member) do
|
||||||
|
|
||||||
case Loader.load_linked_user!(member, actor) do
|
|
||||||
{:ok, user} -> {:ok, user, member}
|
{:ok, user} -> {:ok, user, member}
|
||||||
error -> error
|
error -> error
|
||||||
end
|
end
|
||||||
|
|
|
||||||
|
|
@ -5,25 +5,26 @@ defmodule Mv.EmailSync.Loader do
|
||||||
|
|
||||||
## Authorization
|
## Authorization
|
||||||
|
|
||||||
This module runs systemically and accepts optional actor parameters.
|
This module runs systemically and uses the system actor for all operations.
|
||||||
When called from hooks/changes, actor is extracted from changeset context.
|
This ensures that email synchronization always works, regardless of user permissions.
|
||||||
When called directly, actor should be provided for proper authorization.
|
|
||||||
|
|
||||||
All functions accept an optional `actor` parameter that is passed to Ash operations
|
All functions use `Mv.Helpers.SystemActor.get_system_actor/0` to bypass
|
||||||
to ensure proper authorization checks are performed.
|
user permission checks, as email sync is a mandatory side effect.
|
||||||
"""
|
"""
|
||||||
alias Mv.Helpers
|
alias Mv.Helpers
|
||||||
|
alias Mv.Helpers.SystemActor
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
Loads the member linked to a user, returns nil if not linked or on error.
|
Loads the member linked to a user, returns nil if not linked or on error.
|
||||||
|
|
||||||
Accepts optional actor for authorization.
|
Uses system actor for authorization to ensure email sync always works.
|
||||||
"""
|
"""
|
||||||
def get_linked_member(user, actor \\ nil)
|
def get_linked_member(user)
|
||||||
def get_linked_member(%{member_id: nil}, _actor), do: nil
|
def get_linked_member(%{member_id: nil}), do: nil
|
||||||
|
|
||||||
def get_linked_member(%{member_id: id}, actor) do
|
def get_linked_member(%{member_id: id}) do
|
||||||
opts = Helpers.ash_actor_opts(actor)
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
opts = Helpers.ash_actor_opts(system_actor)
|
||||||
|
|
||||||
case Ash.get(Mv.Membership.Member, id, opts) do
|
case Ash.get(Mv.Membership.Member, id, opts) do
|
||||||
{:ok, member} -> member
|
{:ok, member} -> member
|
||||||
|
|
@ -34,10 +35,11 @@ defmodule Mv.EmailSync.Loader do
|
||||||
@doc """
|
@doc """
|
||||||
Loads the user linked to a member, returns nil if not linked or on error.
|
Loads the user linked to a member, returns nil if not linked or on error.
|
||||||
|
|
||||||
Accepts optional actor for authorization.
|
Uses system actor for authorization to ensure email sync always works.
|
||||||
"""
|
"""
|
||||||
def get_linked_user(member, actor \\ nil) do
|
def get_linked_user(member) do
|
||||||
opts = Helpers.ash_actor_opts(actor)
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
opts = Helpers.ash_actor_opts(system_actor)
|
||||||
|
|
||||||
case Ash.load(member, :user, opts) do
|
case Ash.load(member, :user, opts) do
|
||||||
{:ok, %{user: user}} -> user
|
{:ok, %{user: user}} -> user
|
||||||
|
|
@ -49,10 +51,11 @@ defmodule Mv.EmailSync.Loader do
|
||||||
Loads the user linked to a member, returning an error tuple if not linked.
|
Loads the user linked to a member, returning an error tuple if not linked.
|
||||||
Useful when a link is required for the operation.
|
Useful when a link is required for the operation.
|
||||||
|
|
||||||
Accepts optional actor for authorization.
|
Uses system actor for authorization to ensure email sync always works.
|
||||||
"""
|
"""
|
||||||
def load_linked_user!(member, actor \\ nil) do
|
def load_linked_user!(member) do
|
||||||
opts = Helpers.ash_actor_opts(actor)
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
opts = Helpers.ash_actor_opts(system_actor)
|
||||||
|
|
||||||
case Ash.load(member, :user, opts) do
|
case Ash.load(member, :user, opts) do
|
||||||
{:ok, %{user: user}} when not is_nil(user) -> {:ok, user}
|
{:ok, %{user: user}} when not is_nil(user) -> {:ok, user}
|
||||||
|
|
|
||||||
436
lib/mv/helpers/system_actor.ex
Normal file
436
lib/mv/helpers/system_actor.ex
Normal file
|
|
@ -0,0 +1,436 @@
|
||||||
|
defmodule Mv.Helpers.SystemActor do
|
||||||
|
@moduledoc """
|
||||||
|
Provides access to the system actor for systemic operations.
|
||||||
|
|
||||||
|
The system actor is a user with admin permissions that is used
|
||||||
|
for operations that must always run regardless of user permissions:
|
||||||
|
- Email synchronization
|
||||||
|
- Email uniqueness validation
|
||||||
|
- Cycle generation (if mandatory)
|
||||||
|
- Background jobs
|
||||||
|
- Seeds
|
||||||
|
|
||||||
|
## Usage
|
||||||
|
|
||||||
|
# Get system actor for systemic operations
|
||||||
|
system_actor = Mv.Helpers.SystemActor.get_system_actor()
|
||||||
|
Ash.read(query, actor: system_actor)
|
||||||
|
|
||||||
|
## Implementation
|
||||||
|
|
||||||
|
The system actor is cached in an Agent for performance. On first access,
|
||||||
|
it attempts to load a user with email "system@mila.local" and admin role.
|
||||||
|
If that user doesn't exist, it falls back to the admin user from seeds
|
||||||
|
(identified by ADMIN_EMAIL environment variable or "admin@localhost").
|
||||||
|
|
||||||
|
## Caching
|
||||||
|
|
||||||
|
The system actor is cached in an Agent to avoid repeated database queries.
|
||||||
|
The cache is invalidated on application restart. For long-running applications,
|
||||||
|
consider implementing cache invalidation on role changes.
|
||||||
|
|
||||||
|
## Race Conditions
|
||||||
|
|
||||||
|
The system actor creation uses `upsert?: true` with `upsert_identity: :unique_email`
|
||||||
|
to prevent race conditions when multiple processes try to create the system user
|
||||||
|
simultaneously. This ensures idempotent creation and prevents database constraint errors.
|
||||||
|
|
||||||
|
## Security
|
||||||
|
|
||||||
|
The system actor should NEVER be used for user-initiated actions. It is
|
||||||
|
only for systemic operations that must bypass user permissions.
|
||||||
|
|
||||||
|
The system user is created without a password (`hashed_password = nil`) and
|
||||||
|
without an OIDC ID (`oidc_id = nil`) to prevent login. This ensures the
|
||||||
|
system user cannot be used for authentication, even if credentials are
|
||||||
|
somehow obtained.
|
||||||
|
"""
|
||||||
|
|
||||||
|
use Agent
|
||||||
|
|
||||||
|
require Ash.Query
|
||||||
|
|
||||||
|
alias Mv.Config
|
||||||
|
|
||||||
|
@doc """
|
||||||
|
Starts the SystemActor Agent.
|
||||||
|
|
||||||
|
This is called automatically by the application supervisor.
|
||||||
|
The agent starts with nil state and loads the system actor lazily on first access.
|
||||||
|
"""
|
||||||
|
def start_link(_opts) do
|
||||||
|
# Start with nil - lazy initialization on first get_system_actor call
|
||||||
|
# This prevents database access during application startup (important for tests)
|
||||||
|
Agent.start_link(fn -> nil end, name: __MODULE__)
|
||||||
|
end
|
||||||
|
|
||||||
|
@doc """
|
||||||
|
Returns the system actor (user with admin role).
|
||||||
|
|
||||||
|
The system actor is cached in an Agent for performance. On first access,
|
||||||
|
it loads the system user from the database or falls back to the admin user.
|
||||||
|
|
||||||
|
## Returns
|
||||||
|
|
||||||
|
- `%Mv.Accounts.User{}` - User with admin role loaded
|
||||||
|
- Raises if system actor cannot be found or loaded
|
||||||
|
|
||||||
|
## Examples
|
||||||
|
|
||||||
|
iex> system_actor = Mv.Helpers.SystemActor.get_system_actor()
|
||||||
|
iex> system_actor.role.permission_set_name
|
||||||
|
"admin"
|
||||||
|
|
||||||
|
"""
|
||||||
|
@spec get_system_actor() :: Mv.Accounts.User.t()
|
||||||
|
def get_system_actor do
|
||||||
|
case get_system_actor_result() do
|
||||||
|
{:ok, actor} -> actor
|
||||||
|
{:error, reason} -> raise "Failed to load system actor: #{inspect(reason)}"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
@doc """
|
||||||
|
Returns the system actor as a result tuple.
|
||||||
|
|
||||||
|
This variant returns `{:ok, actor}` or `{:error, reason}` instead of raising,
|
||||||
|
which is useful for error handling in pipes or when you want to handle errors explicitly.
|
||||||
|
|
||||||
|
## Returns
|
||||||
|
|
||||||
|
- `{:ok, %Mv.Accounts.User{}}` - Successfully loaded system actor
|
||||||
|
- `{:error, term()}` - Error loading system actor
|
||||||
|
|
||||||
|
## Examples
|
||||||
|
|
||||||
|
case SystemActor.get_system_actor_result() do
|
||||||
|
{:ok, actor} -> use_actor(actor)
|
||||||
|
{:error, reason} -> handle_error(reason)
|
||||||
|
end
|
||||||
|
|
||||||
|
"""
|
||||||
|
@spec get_system_actor_result() :: {:ok, Mv.Accounts.User.t()} | {:error, term()}
|
||||||
|
def get_system_actor_result do
|
||||||
|
# In test environment (SQL sandbox), always load directly to avoid Agent/Sandbox issues
|
||||||
|
if Config.sql_sandbox?() do
|
||||||
|
try do
|
||||||
|
{:ok, load_system_actor()}
|
||||||
|
rescue
|
||||||
|
e -> {:error, e}
|
||||||
|
end
|
||||||
|
else
|
||||||
|
try do
|
||||||
|
result =
|
||||||
|
Agent.get_and_update(__MODULE__, fn
|
||||||
|
nil ->
|
||||||
|
# Cache miss - load system actor
|
||||||
|
try do
|
||||||
|
actor = load_system_actor()
|
||||||
|
{actor, actor}
|
||||||
|
rescue
|
||||||
|
e -> {{:error, e}, nil}
|
||||||
|
end
|
||||||
|
|
||||||
|
cached_actor ->
|
||||||
|
# Cache hit - return cached actor
|
||||||
|
{cached_actor, cached_actor}
|
||||||
|
end)
|
||||||
|
|
||||||
|
case result do
|
||||||
|
{:error, reason} -> {:error, reason}
|
||||||
|
actor -> {:ok, actor}
|
||||||
|
end
|
||||||
|
catch
|
||||||
|
:exit, {:noproc, _} ->
|
||||||
|
# Agent not started - load directly without caching
|
||||||
|
try do
|
||||||
|
{:ok, load_system_actor()}
|
||||||
|
rescue
|
||||||
|
e -> {:error, e}
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
@doc """
|
||||||
|
Invalidates the system actor cache.
|
||||||
|
|
||||||
|
This forces a reload of the system actor on the next call to `get_system_actor/0`.
|
||||||
|
Useful when the system user's role might have changed.
|
||||||
|
|
||||||
|
## Examples
|
||||||
|
|
||||||
|
iex> Mv.Helpers.SystemActor.invalidate_cache()
|
||||||
|
:ok
|
||||||
|
|
||||||
|
"""
|
||||||
|
@spec invalidate_cache() :: :ok
|
||||||
|
def invalidate_cache do
|
||||||
|
case Process.whereis(__MODULE__) do
|
||||||
|
nil -> :ok
|
||||||
|
_pid -> Agent.update(__MODULE__, fn _state -> nil end)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
@doc """
|
||||||
|
Returns the email address of the system user.
|
||||||
|
|
||||||
|
This is useful for other modules that need to reference the system user
|
||||||
|
without loading the full user record.
|
||||||
|
|
||||||
|
## Returns
|
||||||
|
|
||||||
|
- `String.t()` - The system user email address ("system@mila.local")
|
||||||
|
|
||||||
|
## Examples
|
||||||
|
|
||||||
|
iex> Mv.Helpers.SystemActor.system_user_email()
|
||||||
|
"system@mila.local"
|
||||||
|
|
||||||
|
"""
|
||||||
|
@spec system_user_email() :: String.t()
|
||||||
|
def system_user_email, do: system_user_email_config()
|
||||||
|
|
||||||
|
# Returns the system user email from environment variable or default
|
||||||
|
# This allows configuration via SYSTEM_ACTOR_EMAIL env var
|
||||||
|
@spec system_user_email_config() :: String.t()
|
||||||
|
defp system_user_email_config do
|
||||||
|
System.get_env("SYSTEM_ACTOR_EMAIL") || "system@mila.local"
|
||||||
|
end
|
||||||
|
|
||||||
|
# Loads the system actor from the database
|
||||||
|
# First tries to find system@mila.local, then falls back to admin user
|
||||||
|
@spec load_system_actor() :: Mv.Accounts.User.t() | no_return()
|
||||||
|
defp load_system_actor do
|
||||||
|
case find_user_by_email(system_user_email_config()) do
|
||||||
|
{:ok, user} when not is_nil(user) ->
|
||||||
|
load_user_with_role(user)
|
||||||
|
|
||||||
|
{:ok, nil} ->
|
||||||
|
handle_system_user_not_found("no system user or admin user found")
|
||||||
|
|
||||||
|
{:error, _reason} = error ->
|
||||||
|
handle_system_user_error(error)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Handles case when system user doesn't exist
|
||||||
|
@spec handle_system_user_not_found(String.t()) :: Mv.Accounts.User.t() | no_return()
|
||||||
|
defp handle_system_user_not_found(message) do
|
||||||
|
case load_admin_user_fallback() do
|
||||||
|
{:ok, admin_user} ->
|
||||||
|
admin_user
|
||||||
|
|
||||||
|
{:error, _} ->
|
||||||
|
handle_fallback_error(message)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Handles database error when loading system user
|
||||||
|
@spec handle_system_user_error(term()) :: Mv.Accounts.User.t() | no_return()
|
||||||
|
defp handle_system_user_error(error) do
|
||||||
|
case load_admin_user_fallback() do
|
||||||
|
{:ok, admin_user} ->
|
||||||
|
admin_user
|
||||||
|
|
||||||
|
{:error, _} ->
|
||||||
|
handle_fallback_error("Failed to load system actor: #{inspect(error)}")
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Handles fallback error - creates test actor or raises
|
||||||
|
@spec handle_fallback_error(String.t()) :: Mv.Accounts.User.t() | no_return()
|
||||||
|
defp handle_fallback_error(message) do
|
||||||
|
if Config.sql_sandbox?() do
|
||||||
|
create_test_system_actor()
|
||||||
|
else
|
||||||
|
raise "Failed to load system actor: #{message}"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Creates a temporary admin user for tests when no system/admin user exists
|
||||||
|
@spec create_test_system_actor() :: Mv.Accounts.User.t() | no_return()
|
||||||
|
defp create_test_system_actor do
|
||||||
|
alias Mv.Accounts
|
||||||
|
alias Mv.Authorization
|
||||||
|
|
||||||
|
admin_role = ensure_admin_role_exists()
|
||||||
|
create_system_user_with_role(admin_role)
|
||||||
|
end
|
||||||
|
|
||||||
|
# Ensures admin role exists - finds or creates it
|
||||||
|
@spec ensure_admin_role_exists() :: Mv.Authorization.Role.t() | no_return()
|
||||||
|
defp ensure_admin_role_exists do
|
||||||
|
case find_admin_role() do
|
||||||
|
{:ok, role} ->
|
||||||
|
role
|
||||||
|
|
||||||
|
{:error, :not_found} ->
|
||||||
|
create_admin_role_with_retry()
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Finds admin role in existing roles
|
||||||
|
@spec find_admin_role() :: {:ok, Mv.Authorization.Role.t()} | {:error, :not_found}
|
||||||
|
defp find_admin_role do
|
||||||
|
alias Mv.Authorization
|
||||||
|
|
||||||
|
case Authorization.list_roles() do
|
||||||
|
{:ok, roles} ->
|
||||||
|
case Enum.find(roles, &(&1.permission_set_name == "admin")) do
|
||||||
|
nil -> {:error, :not_found}
|
||||||
|
role -> {:ok, role}
|
||||||
|
end
|
||||||
|
|
||||||
|
_ ->
|
||||||
|
{:error, :not_found}
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Creates admin role, handling race conditions
|
||||||
|
@spec create_admin_role_with_retry() :: Mv.Authorization.Role.t() | no_return()
|
||||||
|
defp create_admin_role_with_retry do
|
||||||
|
alias Mv.Authorization
|
||||||
|
|
||||||
|
case create_admin_role() do
|
||||||
|
{:ok, role} ->
|
||||||
|
role
|
||||||
|
|
||||||
|
{:error, :already_exists} ->
|
||||||
|
find_existing_admin_role()
|
||||||
|
|
||||||
|
{:error, error} ->
|
||||||
|
raise "Failed to create admin role: #{inspect(error)}"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Attempts to create admin role
|
||||||
|
@spec create_admin_role() ::
|
||||||
|
{:ok, Mv.Authorization.Role.t()} | {:error, :already_exists | term()}
|
||||||
|
defp create_admin_role do
|
||||||
|
alias Mv.Authorization
|
||||||
|
|
||||||
|
case Authorization.create_role(%{
|
||||||
|
name: "Admin",
|
||||||
|
description: "Administrator with full access",
|
||||||
|
permission_set_name: "admin"
|
||||||
|
}) do
|
||||||
|
{:ok, role} ->
|
||||||
|
{:ok, role}
|
||||||
|
|
||||||
|
{:error, %Ash.Error.Invalid{errors: [%{field: :name, message: "has already been taken"}]}} ->
|
||||||
|
{:error, :already_exists}
|
||||||
|
|
||||||
|
{:error, error} ->
|
||||||
|
{:error, error}
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Finds existing admin role after creation attempt failed due to race condition
|
||||||
|
@spec find_existing_admin_role() :: Mv.Authorization.Role.t() | no_return()
|
||||||
|
defp find_existing_admin_role do
|
||||||
|
alias Mv.Authorization
|
||||||
|
|
||||||
|
case Authorization.list_roles() do
|
||||||
|
{:ok, roles} ->
|
||||||
|
Enum.find(roles, &(&1.permission_set_name == "admin")) ||
|
||||||
|
raise "Admin role should exist but was not found"
|
||||||
|
|
||||||
|
_ ->
|
||||||
|
raise "Failed to find admin role after creation attempt"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Creates system user with admin role assigned
|
||||||
|
# SECURITY: System user is created without password (hashed_password = nil) and
|
||||||
|
# without OIDC ID (oidc_id = nil) to prevent login. This user is ONLY for
|
||||||
|
# internal system operations via SystemActor and should never be used for authentication.
|
||||||
|
@spec create_system_user_with_role(Mv.Authorization.Role.t()) ::
|
||||||
|
Mv.Accounts.User.t() | no_return()
|
||||||
|
defp create_system_user_with_role(admin_role) do
|
||||||
|
alias Mv.Accounts
|
||||||
|
|
||||||
|
Accounts.create_user!(%{email: system_user_email_config()},
|
||||||
|
upsert?: true,
|
||||||
|
upsert_identity: :unique_email
|
||||||
|
)
|
||||||
|
|> Ash.Changeset.for_update(:update, %{})
|
||||||
|
|> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove)
|
||||||
|
|> Ash.update!()
|
||||||
|
|> Ash.load!(:role, domain: Mv.Accounts)
|
||||||
|
end
|
||||||
|
|
||||||
|
# Finds a user by email address
|
||||||
|
# SECURITY: Uses authorize?: false for bootstrap lookup only.
|
||||||
|
# This is necessary because we need to find the system/admin user before
|
||||||
|
# we can load the system actor. If User policies require an actor, this
|
||||||
|
# would create a chicken-and-egg problem. This is safe because:
|
||||||
|
# 1. We only query by email (no sensitive data exposed)
|
||||||
|
# 2. This is only used during system actor initialization (bootstrap phase)
|
||||||
|
# 3. Once system actor is loaded, all subsequent operations use proper authorization
|
||||||
|
@spec find_user_by_email(String.t()) :: {:ok, Mv.Accounts.User.t() | nil} | {:error, term()}
|
||||||
|
defp find_user_by_email(email) do
|
||||||
|
Mv.Accounts.User
|
||||||
|
|> Ash.Query.filter(email == ^email)
|
||||||
|
|> Ash.read_one(domain: Mv.Accounts, authorize?: false)
|
||||||
|
end
|
||||||
|
|
||||||
|
# Loads a user with their role preloaded (required for authorization)
|
||||||
|
@spec load_user_with_role(Mv.Accounts.User.t()) :: Mv.Accounts.User.t() | no_return()
|
||||||
|
defp load_user_with_role(user) do
|
||||||
|
case Ash.load(user, :role, domain: Mv.Accounts) do
|
||||||
|
{:ok, user_with_role} ->
|
||||||
|
validate_admin_role(user_with_role)
|
||||||
|
|
||||||
|
{:error, reason} ->
|
||||||
|
raise "Failed to load role for system actor: #{inspect(reason)}"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Validates that the user has an admin role
|
||||||
|
@spec validate_admin_role(Mv.Accounts.User.t()) :: Mv.Accounts.User.t() | no_return()
|
||||||
|
defp validate_admin_role(%{role: %{permission_set_name: "admin"}} = user) do
|
||||||
|
user
|
||||||
|
end
|
||||||
|
|
||||||
|
@spec validate_admin_role(Mv.Accounts.User.t()) :: no_return()
|
||||||
|
defp validate_admin_role(%{role: %{permission_set_name: permission_set}}) do
|
||||||
|
raise """
|
||||||
|
System actor must have admin role, but has permission_set_name: #{permission_set}
|
||||||
|
Please assign the "Admin" role to the system user.
|
||||||
|
"""
|
||||||
|
end
|
||||||
|
|
||||||
|
@spec validate_admin_role(Mv.Accounts.User.t()) :: no_return()
|
||||||
|
defp validate_admin_role(%{role: nil}) do
|
||||||
|
raise """
|
||||||
|
System actor must have a role assigned, but role is nil.
|
||||||
|
Please assign the "Admin" role to the system user.
|
||||||
|
"""
|
||||||
|
end
|
||||||
|
|
||||||
|
@spec validate_admin_role(term()) :: no_return()
|
||||||
|
defp validate_admin_role(_user) do
|
||||||
|
raise """
|
||||||
|
System actor must have a role with admin permissions.
|
||||||
|
Please assign the "Admin" role to the system user.
|
||||||
|
"""
|
||||||
|
end
|
||||||
|
|
||||||
|
# Fallback: Loads admin user from seeds (ADMIN_EMAIL env var or default)
|
||||||
|
@spec load_admin_user_fallback() :: {:ok, Mv.Accounts.User.t()} | {:error, term()}
|
||||||
|
defp load_admin_user_fallback do
|
||||||
|
admin_email = System.get_env("ADMIN_EMAIL") || "admin@localhost"
|
||||||
|
|
||||||
|
case find_user_by_email(admin_email) do
|
||||||
|
{:ok, user} when not is_nil(user) ->
|
||||||
|
{:ok, load_user_with_role(user)}
|
||||||
|
|
||||||
|
{:ok, nil} ->
|
||||||
|
{:error, :admin_user_not_found}
|
||||||
|
|
||||||
|
{:error, _reason} = error ->
|
||||||
|
error
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
@ -10,6 +10,8 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do
|
||||||
use Ash.Resource.Validation
|
use Ash.Resource.Validation
|
||||||
alias Mv.Helpers
|
alias Mv.Helpers
|
||||||
|
|
||||||
|
require Logger
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
Validates email uniqueness across linked Member-User pairs.
|
Validates email uniqueness across linked Member-User pairs.
|
||||||
|
|
||||||
|
|
@ -30,8 +32,7 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do
|
||||||
def validate(changeset, _opts, _context) do
|
def validate(changeset, _opts, _context) do
|
||||||
email_changing? = Ash.Changeset.changing_attribute?(changeset, :email)
|
email_changing? = Ash.Changeset.changing_attribute?(changeset, :email)
|
||||||
|
|
||||||
actor = Map.get(changeset.context || %{}, :actor)
|
linked_user_id = get_linked_user_id(changeset.data)
|
||||||
linked_user_id = get_linked_user_id(changeset.data, actor)
|
|
||||||
is_linked? = not is_nil(linked_user_id)
|
is_linked? = not is_nil(linked_user_id)
|
||||||
|
|
||||||
# Only validate if member is already linked AND email is changing
|
# Only validate if member is already linked AND email is changing
|
||||||
|
|
@ -40,19 +41,22 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do
|
||||||
|
|
||||||
if should_validate? do
|
if should_validate? do
|
||||||
new_email = Ash.Changeset.get_attribute(changeset, :email)
|
new_email = Ash.Changeset.get_attribute(changeset, :email)
|
||||||
check_email_uniqueness(new_email, linked_user_id, actor)
|
check_email_uniqueness(new_email, linked_user_id)
|
||||||
else
|
else
|
||||||
:ok
|
:ok
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
defp check_email_uniqueness(email, exclude_user_id, actor) do
|
defp check_email_uniqueness(email, exclude_user_id) do
|
||||||
|
alias Mv.Helpers.SystemActor
|
||||||
|
|
||||||
query =
|
query =
|
||||||
Mv.Accounts.User
|
Mv.Accounts.User
|
||||||
|> Ash.Query.filter(email == ^email)
|
|> Ash.Query.filter(email == ^email)
|
||||||
|> maybe_exclude_id(exclude_user_id)
|
|> maybe_exclude_id(exclude_user_id)
|
||||||
|
|
||||||
opts = Helpers.ash_actor_opts(actor)
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
opts = Helpers.ash_actor_opts(system_actor)
|
||||||
|
|
||||||
case Ash.read(query, opts) do
|
case Ash.read(query, opts) do
|
||||||
{:ok, []} ->
|
{:ok, []} ->
|
||||||
|
|
@ -61,7 +65,11 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do
|
||||||
{:ok, _} ->
|
{:ok, _} ->
|
||||||
{:error, field: :email, message: "is already used by another user", value: email}
|
{:error, field: :email, message: "is already used by another user", value: email}
|
||||||
|
|
||||||
{:error, _} ->
|
{:error, reason} ->
|
||||||
|
Logger.warning(
|
||||||
|
"Email uniqueness validation query failed for member email '#{email}': #{inspect(reason)}. Allowing operation to proceed (fail-open)."
|
||||||
|
)
|
||||||
|
|
||||||
:ok
|
:ok
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
@ -69,8 +77,11 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do
|
||||||
defp maybe_exclude_id(query, nil), do: query
|
defp maybe_exclude_id(query, nil), do: query
|
||||||
defp maybe_exclude_id(query, id), do: Ash.Query.filter(query, id != ^id)
|
defp maybe_exclude_id(query, id), do: Ash.Query.filter(query, id != ^id)
|
||||||
|
|
||||||
defp get_linked_user_id(member_data, actor) do
|
defp get_linked_user_id(member_data) do
|
||||||
opts = Helpers.ash_actor_opts(actor)
|
alias Mv.Helpers.SystemActor
|
||||||
|
|
||||||
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
opts = Helpers.ash_actor_opts(system_actor)
|
||||||
|
|
||||||
case Ash.load(member_data, :user, opts) do
|
case Ash.load(member_data, :user, opts) do
|
||||||
{:ok, %{user: %{id: id}}} -> id
|
{:ok, %{user: %{id: id}}} -> id
|
||||||
|
|
|
||||||
|
|
@ -30,12 +30,11 @@ defmodule Mv.MembershipFees.CycleGenerator do
|
||||||
|
|
||||||
## Authorization
|
## Authorization
|
||||||
|
|
||||||
This module runs systemically and accepts optional actor parameters.
|
This module runs systemically and uses the system actor for all operations.
|
||||||
When called from hooks/changes, actor is extracted from changeset context.
|
This ensures that cycle generation always works, regardless of user permissions.
|
||||||
When called directly, actor should be provided for proper authorization.
|
|
||||||
|
|
||||||
All functions accept an optional `actor` parameter in the `opts` keyword list
|
All functions use `Mv.Helpers.SystemActor.get_system_actor/0` to bypass
|
||||||
that is passed to Ash operations to ensure proper authorization checks are performed.
|
user permission checks, as cycle generation is a mandatory side effect.
|
||||||
|
|
||||||
## Examples
|
## Examples
|
||||||
|
|
||||||
|
|
@ -47,6 +46,8 @@ defmodule Mv.MembershipFees.CycleGenerator do
|
||||||
|
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
alias Mv.Helpers
|
||||||
|
alias Mv.Helpers.SystemActor
|
||||||
alias Mv.Membership.Member
|
alias Mv.Membership.Member
|
||||||
alias Mv.MembershipFees.CalendarCycles
|
alias Mv.MembershipFees.CalendarCycles
|
||||||
alias Mv.MembershipFees.Changes.SetMembershipFeeStartDate
|
alias Mv.MembershipFees.Changes.SetMembershipFeeStartDate
|
||||||
|
|
@ -86,9 +87,7 @@ defmodule Mv.MembershipFees.CycleGenerator do
|
||||||
def generate_cycles_for_member(member_or_id, opts \\ [])
|
def generate_cycles_for_member(member_or_id, opts \\ [])
|
||||||
|
|
||||||
def generate_cycles_for_member(member_id, opts) when is_binary(member_id) do
|
def generate_cycles_for_member(member_id, opts) when is_binary(member_id) do
|
||||||
actor = Keyword.get(opts, :actor)
|
case load_member(member_id) do
|
||||||
|
|
||||||
case load_member(member_id, actor: actor) do
|
|
||||||
{:ok, member} -> generate_cycles_for_member(member, opts)
|
{:ok, member} -> generate_cycles_for_member(member, opts)
|
||||||
{:error, reason} -> {:error, reason}
|
{:error, reason} -> {:error, reason}
|
||||||
end
|
end
|
||||||
|
|
@ -98,27 +97,25 @@ defmodule Mv.MembershipFees.CycleGenerator do
|
||||||
today = Keyword.get(opts, :today, Date.utc_today())
|
today = Keyword.get(opts, :today, Date.utc_today())
|
||||||
skip_lock? = Keyword.get(opts, :skip_lock?, false)
|
skip_lock? = Keyword.get(opts, :skip_lock?, false)
|
||||||
|
|
||||||
do_generate_cycles_with_lock(member, today, skip_lock?, opts)
|
do_generate_cycles_with_lock(member, today, skip_lock?)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Generate cycles with lock handling
|
# Generate cycles with lock handling
|
||||||
# Returns {:ok, cycles, notifications} - notifications are never sent here,
|
# Returns {:ok, cycles, notifications} - notifications are never sent here,
|
||||||
# they should be returned to the caller (e.g., via after_action hook)
|
# they should be returned to the caller (e.g., via after_action hook)
|
||||||
defp do_generate_cycles_with_lock(member, today, true = _skip_lock?, opts) do
|
defp do_generate_cycles_with_lock(member, today, true = _skip_lock?) do
|
||||||
# Lock already set by caller (e.g., regenerate_cycles_on_type_change)
|
# Lock already set by caller (e.g., regenerate_cycles_on_type_change)
|
||||||
# Just generate cycles without additional locking
|
# Just generate cycles without additional locking
|
||||||
actor = Keyword.get(opts, :actor)
|
do_generate_cycles(member, today)
|
||||||
do_generate_cycles(member, today, actor: actor)
|
|
||||||
end
|
end
|
||||||
|
|
||||||
defp do_generate_cycles_with_lock(member, today, false, opts) do
|
defp do_generate_cycles_with_lock(member, today, false) do
|
||||||
lock_key = :erlang.phash2(member.id)
|
lock_key = :erlang.phash2(member.id)
|
||||||
actor = Keyword.get(opts, :actor)
|
|
||||||
|
|
||||||
Repo.transaction(fn ->
|
Repo.transaction(fn ->
|
||||||
Ecto.Adapters.SQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key])
|
Ecto.Adapters.SQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key])
|
||||||
|
|
||||||
case do_generate_cycles(member, today, actor: actor) do
|
case do_generate_cycles(member, today) do
|
||||||
{:ok, cycles, notifications} ->
|
{:ok, cycles, notifications} ->
|
||||||
# Return cycles and notifications - do NOT send notifications here
|
# Return cycles and notifications - do NOT send notifications here
|
||||||
# They will be sent by the caller (e.g., via after_action hook)
|
# They will be sent by the caller (e.g., via after_action hook)
|
||||||
|
|
@ -168,12 +165,15 @@ defmodule Mv.MembershipFees.CycleGenerator do
|
||||||
# Query ALL members with fee type assigned (including inactive/left members)
|
# Query ALL members with fee type assigned (including inactive/left members)
|
||||||
# The exit_date boundary is applied during cycle generation, not here.
|
# The exit_date boundary is applied during cycle generation, not here.
|
||||||
# This allows catch-up generation for members who left but are missing cycles.
|
# This allows catch-up generation for members who left but are missing cycles.
|
||||||
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
opts = Helpers.ash_actor_opts(system_actor)
|
||||||
|
|
||||||
query =
|
query =
|
||||||
Member
|
Member
|
||||||
|> Ash.Query.filter(not is_nil(membership_fee_type_id))
|
|> Ash.Query.filter(not is_nil(membership_fee_type_id))
|
||||||
|> Ash.Query.filter(not is_nil(join_date))
|
|> Ash.Query.filter(not is_nil(join_date))
|
||||||
|
|
||||||
case Ash.read(query) do
|
case Ash.read(query, opts) do
|
||||||
{:ok, members} ->
|
{:ok, members} ->
|
||||||
results = process_members_in_batches(members, batch_size, today)
|
results = process_members_in_batches(members, batch_size, today)
|
||||||
{:ok, build_results_summary(results)}
|
{:ok, build_results_summary(results)}
|
||||||
|
|
@ -235,33 +235,25 @@ defmodule Mv.MembershipFees.CycleGenerator do
|
||||||
|
|
||||||
# Private functions
|
# Private functions
|
||||||
|
|
||||||
defp load_member(member_id, opts) do
|
defp load_member(member_id) do
|
||||||
actor = Keyword.get(opts, :actor)
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
opts = Helpers.ash_actor_opts(system_actor)
|
||||||
|
|
||||||
query =
|
query =
|
||||||
Member
|
Member
|
||||||
|> Ash.Query.filter(id == ^member_id)
|
|> Ash.Query.filter(id == ^member_id)
|
||||||
|> Ash.Query.load([:membership_fee_type, :membership_fee_cycles])
|
|> Ash.Query.load([:membership_fee_type, :membership_fee_cycles])
|
||||||
|
|
||||||
result =
|
case Ash.read_one(query, opts) do
|
||||||
if actor do
|
|
||||||
Ash.read_one(query, actor: actor)
|
|
||||||
else
|
|
||||||
Ash.read_one(query)
|
|
||||||
end
|
|
||||||
|
|
||||||
case result do
|
|
||||||
{:ok, nil} -> {:error, :member_not_found}
|
{:ok, nil} -> {:error, :member_not_found}
|
||||||
{:ok, member} -> {:ok, member}
|
{:ok, member} -> {:ok, member}
|
||||||
{:error, reason} -> {:error, reason}
|
{:error, reason} -> {:error, reason}
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
defp do_generate_cycles(member, today, opts) do
|
defp do_generate_cycles(member, today) do
|
||||||
actor = Keyword.get(opts, :actor)
|
|
||||||
|
|
||||||
# Reload member with relationships to ensure fresh data
|
# Reload member with relationships to ensure fresh data
|
||||||
case load_member(member.id, actor: actor) do
|
case load_member(member.id) do
|
||||||
{:ok, member} ->
|
{:ok, member} ->
|
||||||
cond do
|
cond do
|
||||||
is_nil(member.membership_fee_type_id) ->
|
is_nil(member.membership_fee_type_id) ->
|
||||||
|
|
@ -271,7 +263,7 @@ defmodule Mv.MembershipFees.CycleGenerator do
|
||||||
{:error, :no_join_date}
|
{:error, :no_join_date}
|
||||||
|
|
||||||
true ->
|
true ->
|
||||||
generate_missing_cycles(member, today, actor: actor)
|
generate_missing_cycles(member, today)
|
||||||
end
|
end
|
||||||
|
|
||||||
{:error, reason} ->
|
{:error, reason} ->
|
||||||
|
|
@ -279,8 +271,7 @@ defmodule Mv.MembershipFees.CycleGenerator do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
defp generate_missing_cycles(member, today, opts) do
|
defp generate_missing_cycles(member, today) do
|
||||||
actor = Keyword.get(opts, :actor)
|
|
||||||
fee_type = member.membership_fee_type
|
fee_type = member.membership_fee_type
|
||||||
interval = fee_type.interval
|
interval = fee_type.interval
|
||||||
amount = fee_type.amount
|
amount = fee_type.amount
|
||||||
|
|
@ -296,7 +287,7 @@ defmodule Mv.MembershipFees.CycleGenerator do
|
||||||
# Only generate if start_date <= end_date
|
# Only generate if start_date <= end_date
|
||||||
if start_date && Date.compare(start_date, end_date) != :gt do
|
if start_date && Date.compare(start_date, end_date) != :gt do
|
||||||
cycle_starts = generate_cycle_starts(start_date, end_date, interval)
|
cycle_starts = generate_cycle_starts(start_date, end_date, interval)
|
||||||
create_cycles(cycle_starts, member.id, fee_type.id, amount, actor: actor)
|
create_cycles(cycle_starts, member.id, fee_type.id, amount)
|
||||||
else
|
else
|
||||||
{:ok, [], []}
|
{:ok, [], []}
|
||||||
end
|
end
|
||||||
|
|
@ -391,8 +382,10 @@ defmodule Mv.MembershipFees.CycleGenerator do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
defp create_cycles(cycle_starts, member_id, fee_type_id, amount, opts) do
|
defp create_cycles(cycle_starts, member_id, fee_type_id, amount) do
|
||||||
actor = Keyword.get(opts, :actor)
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
opts = Helpers.ash_actor_opts(system_actor)
|
||||||
|
|
||||||
# Always use return_notifications?: true to collect notifications
|
# Always use return_notifications?: true to collect notifications
|
||||||
# Notifications will be returned to the caller, who is responsible for
|
# Notifications will be returned to the caller, who is responsible for
|
||||||
# sending them (e.g., via after_action hook returning {:ok, result, notifications})
|
# sending them (e.g., via after_action hook returning {:ok, result, notifications})
|
||||||
|
|
@ -407,7 +400,7 @@ defmodule Mv.MembershipFees.CycleGenerator do
|
||||||
}
|
}
|
||||||
|
|
||||||
handle_cycle_creation_result(
|
handle_cycle_creation_result(
|
||||||
Ash.create(MembershipFeeCycle, attrs, return_notifications?: true, actor: actor),
|
Ash.create(MembershipFeeCycle, attrs, [return_notifications?: true] ++ opts),
|
||||||
cycle_start
|
cycle_start
|
||||||
)
|
)
|
||||||
end)
|
end)
|
||||||
|
|
|
||||||
|
|
@ -52,7 +52,8 @@ defmodule MvWeb do
|
||||||
quote do
|
quote do
|
||||||
use Phoenix.LiveView
|
use Phoenix.LiveView
|
||||||
|
|
||||||
on_mount MvWeb.LiveHelpers
|
on_mount {MvWeb.LiveHelpers, :default}
|
||||||
|
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
||||||
|
|
||||||
unquote(html_helpers())
|
unquote(html_helpers())
|
||||||
end
|
end
|
||||||
|
|
|
||||||
|
|
@ -20,10 +20,13 @@ defmodule MvWeb.LinkOidcAccountLive do
|
||||||
|
|
||||||
@impl true
|
@impl true
|
||||||
def mount(_params, session, socket) do
|
def mount(_params, session, socket) do
|
||||||
|
# Use SystemActor for authorization during OIDC linking (user is not yet logged in)
|
||||||
|
system_actor = Mv.Helpers.SystemActor.get_system_actor()
|
||||||
|
|
||||||
with user_id when not is_nil(user_id) <- Map.get(session, "oidc_linking_user_id"),
|
with user_id when not is_nil(user_id) <- Map.get(session, "oidc_linking_user_id"),
|
||||||
oidc_user_info when not is_nil(oidc_user_info) <-
|
oidc_user_info when not is_nil(oidc_user_info) <-
|
||||||
Map.get(session, "oidc_linking_user_info"),
|
Map.get(session, "oidc_linking_user_info"),
|
||||||
{:ok, user} <- Ash.get(Mv.Accounts.User, user_id) do
|
{:ok, user} <- Ash.get(Mv.Accounts.User, user_id, actor: system_actor) do
|
||||||
# Check if user is passwordless
|
# Check if user is passwordless
|
||||||
if passwordless?(user) do
|
if passwordless?(user) do
|
||||||
# Auto-link passwordless user immediately
|
# Auto-link passwordless user immediately
|
||||||
|
|
@ -46,9 +49,12 @@ defmodule MvWeb.LinkOidcAccountLive do
|
||||||
end
|
end
|
||||||
|
|
||||||
defp reload_user!(user_id) do
|
defp reload_user!(user_id) do
|
||||||
|
# Use SystemActor for authorization during OIDC linking (user is not yet logged in)
|
||||||
|
system_actor = Mv.Helpers.SystemActor.get_system_actor()
|
||||||
|
|
||||||
Mv.Accounts.User
|
Mv.Accounts.User
|
||||||
|> Ash.Query.filter(id == ^user_id)
|
|> Ash.Query.filter(id == ^user_id)
|
||||||
|> Ash.read_one!()
|
|> Ash.read_one!(actor: system_actor)
|
||||||
end
|
end
|
||||||
|
|
||||||
defp reset_password_form(socket) do
|
defp reset_password_form(socket) do
|
||||||
|
|
@ -58,13 +64,16 @@ defmodule MvWeb.LinkOidcAccountLive do
|
||||||
defp auto_link_passwordless_user(socket, user, oidc_user_info) do
|
defp auto_link_passwordless_user(socket, user, oidc_user_info) do
|
||||||
oidc_id = Map.get(oidc_user_info, "sub") || Map.get(oidc_user_info, "id")
|
oidc_id = Map.get(oidc_user_info, "sub") || Map.get(oidc_user_info, "id")
|
||||||
|
|
||||||
|
# Use SystemActor for authorization (passwordless user auto-linking)
|
||||||
|
system_actor = Mv.Helpers.SystemActor.get_system_actor()
|
||||||
|
|
||||||
case user.id
|
case user.id
|
||||||
|> reload_user!()
|
|> reload_user!()
|
||||||
|> Ash.Changeset.for_update(:link_oidc_id, %{
|
|> Ash.Changeset.for_update(:link_oidc_id, %{
|
||||||
oidc_id: oidc_id,
|
oidc_id: oidc_id,
|
||||||
oidc_user_info: oidc_user_info
|
oidc_user_info: oidc_user_info
|
||||||
})
|
})
|
||||||
|> Ash.update() do
|
|> Ash.update(actor: system_actor) do
|
||||||
{:ok, updated_user} ->
|
{:ok, updated_user} ->
|
||||||
Logger.info(
|
Logger.info(
|
||||||
"Passwordless account auto-linked to OIDC: user_id=#{updated_user.id}, oidc_id=#{oidc_id}"
|
"Passwordless account auto-linked to OIDC: user_id=#{updated_user.id}, oidc_id=#{oidc_id}"
|
||||||
|
|
@ -187,6 +196,9 @@ defmodule MvWeb.LinkOidcAccountLive do
|
||||||
defp link_oidc_account(socket, user, oidc_user_info) do
|
defp link_oidc_account(socket, user, oidc_user_info) do
|
||||||
oidc_id = Map.get(oidc_user_info, "sub") || Map.get(oidc_user_info, "id")
|
oidc_id = Map.get(oidc_user_info, "sub") || Map.get(oidc_user_info, "id")
|
||||||
|
|
||||||
|
# Use SystemActor for authorization (user just verified password but is not yet logged in)
|
||||||
|
system_actor = Mv.Helpers.SystemActor.get_system_actor()
|
||||||
|
|
||||||
# Update the user with the OIDC ID
|
# Update the user with the OIDC ID
|
||||||
case user.id
|
case user.id
|
||||||
|> reload_user!()
|
|> reload_user!()
|
||||||
|
|
@ -194,7 +206,7 @@ defmodule MvWeb.LinkOidcAccountLive do
|
||||||
oidc_id: oidc_id,
|
oidc_id: oidc_id,
|
||||||
oidc_user_info: oidc_user_info
|
oidc_user_info: oidc_user_info
|
||||||
})
|
})
|
||||||
|> Ash.update() do
|
|> Ash.update(actor: system_actor) do
|
||||||
{:ok, updated_user} ->
|
{:ok, updated_user} ->
|
||||||
# After successful linking, redirect to OIDC login
|
# After successful linking, redirect to OIDC login
|
||||||
# Since the user now has an oidc_id, the next OIDC login will succeed
|
# Since the user now has an oidc_id, the next OIDC login will succeed
|
||||||
|
|
|
||||||
|
|
@ -21,8 +21,6 @@ defmodule MvWeb.MemberLive.Form do
|
||||||
"""
|
"""
|
||||||
use MvWeb, :live_view
|
use MvWeb, :live_view
|
||||||
|
|
||||||
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
|
||||||
|
|
||||||
import MvWeb.LiveHelpers, only: [current_actor: 1, submit_form: 3]
|
import MvWeb.LiveHelpers, only: [current_actor: 1, submit_form: 3]
|
||||||
|
|
||||||
alias Mv.MembershipFees
|
alias Mv.MembershipFees
|
||||||
|
|
|
||||||
|
|
@ -27,8 +27,6 @@ defmodule MvWeb.MemberLive.Index do
|
||||||
"""
|
"""
|
||||||
use MvWeb, :live_view
|
use MvWeb, :live_view
|
||||||
|
|
||||||
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
|
||||||
|
|
||||||
require Ash.Query
|
require Ash.Query
|
||||||
require Logger
|
require Logger
|
||||||
import Ash.Expr
|
import Ash.Expr
|
||||||
|
|
|
||||||
|
|
@ -22,8 +22,6 @@ defmodule MvWeb.MemberLive.Show do
|
||||||
import Ash.Query
|
import Ash.Query
|
||||||
import MvWeb.LiveHelpers, only: [current_actor: 1]
|
import MvWeb.LiveHelpers, only: [current_actor: 1]
|
||||||
|
|
||||||
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
|
||||||
|
|
||||||
alias MvWeb.Helpers.MembershipFeeHelpers
|
alias MvWeb.Helpers.MembershipFeeHelpers
|
||||||
|
|
||||||
@impl true
|
@impl true
|
||||||
|
|
|
||||||
|
|
@ -554,11 +554,15 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
|
||||||
end
|
end
|
||||||
|
|
||||||
def handle_event("regenerate_cycles", _params, socket) do
|
def handle_event("regenerate_cycles", _params, socket) do
|
||||||
socket = assign(socket, :regenerating, true)
|
|
||||||
member = socket.assigns.member
|
|
||||||
actor = current_actor(socket)
|
actor = current_actor(socket)
|
||||||
|
|
||||||
case CycleGenerator.generate_cycles_for_member(member.id, actor: actor) do
|
# SECURITY: Only admins can manually regenerate cycles via UI
|
||||||
|
# Cycle generation itself uses system actor, but UI access should be restricted
|
||||||
|
if actor.role && actor.role.permission_set_name == "admin" do
|
||||||
|
socket = assign(socket, :regenerating, true)
|
||||||
|
member = socket.assigns.member
|
||||||
|
|
||||||
|
case CycleGenerator.generate_cycles_for_member(member.id) do
|
||||||
{:ok, _new_cycles, _notifications} ->
|
{:ok, _new_cycles, _notifications} ->
|
||||||
# Reload member with cycles
|
# Reload member with cycles
|
||||||
actor = current_actor(socket)
|
actor = current_actor(socket)
|
||||||
|
|
@ -595,6 +599,11 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
|
||||||
|> assign(:regenerating, false)
|
|> assign(:regenerating, false)
|
||||||
|> put_flash(:error, format_error(error))}
|
|> put_flash(:error, format_error(error))}
|
||||||
end
|
end
|
||||||
|
else
|
||||||
|
{:noreply,
|
||||||
|
socket
|
||||||
|
|> put_flash(:error, gettext("Only administrators can regenerate cycles"))}
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def handle_event("edit_cycle_amount", %{"cycle_id" => cycle_id}, socket) do
|
def handle_event("edit_cycle_amount", %{"cycle_id" => cycle_id}, socket) do
|
||||||
|
|
|
||||||
|
|
@ -13,7 +13,6 @@ defmodule MvWeb.MembershipFeeTypeLive.Form do
|
||||||
"""
|
"""
|
||||||
use MvWeb, :live_view
|
use MvWeb, :live_view
|
||||||
|
|
||||||
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
|
||||||
import MvWeb.LiveHelpers, only: [current_actor: 1, submit_form: 3]
|
import MvWeb.LiveHelpers, only: [current_actor: 1, submit_form: 3]
|
||||||
|
|
||||||
require Ash.Query
|
require Ash.Query
|
||||||
|
|
|
||||||
|
|
@ -14,7 +14,6 @@ defmodule MvWeb.MembershipFeeTypeLive.Index do
|
||||||
"""
|
"""
|
||||||
use MvWeb, :live_view
|
use MvWeb, :live_view
|
||||||
|
|
||||||
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
|
||||||
import MvWeb.LiveHelpers, only: [current_actor: 1]
|
import MvWeb.LiveHelpers, only: [current_actor: 1]
|
||||||
|
|
||||||
require Ash.Query
|
require Ash.Query
|
||||||
|
|
|
||||||
|
|
@ -17,8 +17,6 @@ defmodule MvWeb.RoleLive.Form do
|
||||||
|
|
||||||
import MvWeb.RoleLive.Helpers, only: [format_error: 1]
|
import MvWeb.RoleLive.Helpers, only: [format_error: 1]
|
||||||
|
|
||||||
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
|
||||||
|
|
||||||
@impl true
|
@impl true
|
||||||
def render(assigns) do
|
def render(assigns) do
|
||||||
~H"""
|
~H"""
|
||||||
|
|
|
||||||
|
|
@ -24,8 +24,6 @@ defmodule MvWeb.RoleLive.Index do
|
||||||
import MvWeb.RoleLive.Helpers,
|
import MvWeb.RoleLive.Helpers,
|
||||||
only: [format_error: 1, permission_set_badge_class: 1, opts_with_actor: 3]
|
only: [format_error: 1, permission_set_badge_class: 1, opts_with_actor: 3]
|
||||||
|
|
||||||
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
|
||||||
|
|
||||||
@impl true
|
@impl true
|
||||||
def mount(_params, _session, socket) do
|
def mount(_params, _session, socket) do
|
||||||
actor = socket.assigns[:current_user]
|
actor = socket.assigns[:current_user]
|
||||||
|
|
|
||||||
|
|
@ -19,8 +19,6 @@ defmodule MvWeb.RoleLive.Show do
|
||||||
import MvWeb.RoleLive.Helpers,
|
import MvWeb.RoleLive.Helpers,
|
||||||
only: [format_error: 1, permission_set_badge_class: 1, opts_with_actor: 3]
|
only: [format_error: 1, permission_set_badge_class: 1, opts_with_actor: 3]
|
||||||
|
|
||||||
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
|
||||||
|
|
||||||
@impl true
|
@impl true
|
||||||
def mount(%{"id" => id}, _session, socket) do
|
def mount(%{"id" => id}, _session, socket) do
|
||||||
try do
|
try do
|
||||||
|
|
|
||||||
|
|
@ -33,7 +33,6 @@ defmodule MvWeb.UserLive.Form do
|
||||||
"""
|
"""
|
||||||
use MvWeb, :live_view
|
use MvWeb, :live_view
|
||||||
|
|
||||||
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
|
||||||
import MvWeb.LiveHelpers, only: [current_actor: 1, submit_form: 3]
|
import MvWeb.LiveHelpers, only: [current_actor: 1, submit_form: 3]
|
||||||
|
|
||||||
@impl true
|
@impl true
|
||||||
|
|
|
||||||
|
|
@ -23,7 +23,6 @@ defmodule MvWeb.UserLive.Index do
|
||||||
use MvWeb, :live_view
|
use MvWeb, :live_view
|
||||||
import MvWeb.TableComponents
|
import MvWeb.TableComponents
|
||||||
|
|
||||||
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
|
||||||
import MvWeb.LiveHelpers, only: [current_actor: 1]
|
import MvWeb.LiveHelpers, only: [current_actor: 1]
|
||||||
|
|
||||||
@impl true
|
@impl true
|
||||||
|
|
|
||||||
|
|
@ -26,7 +26,6 @@ defmodule MvWeb.UserLive.Show do
|
||||||
"""
|
"""
|
||||||
use MvWeb, :live_view
|
use MvWeb, :live_view
|
||||||
|
|
||||||
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
|
|
||||||
import MvWeb.LiveHelpers, only: [current_actor: 1]
|
import MvWeb.LiveHelpers, only: [current_actor: 1]
|
||||||
|
|
||||||
@impl true
|
@impl true
|
||||||
|
|
|
||||||
|
|
@ -27,39 +27,17 @@ defmodule MvWeb.LiveHelpers do
|
||||||
end
|
end
|
||||||
|
|
||||||
defp ensure_user_role_loaded(socket) do
|
defp ensure_user_role_loaded(socket) do
|
||||||
if socket.assigns[:current_user] do
|
user = socket.assigns[:current_user]
|
||||||
user = socket.assigns.current_user
|
|
||||||
user_with_role = load_user_role(user)
|
if user do
|
||||||
|
# Use centralized Actor helper to ensure role is loaded
|
||||||
|
user_with_role = Mv.Authorization.Actor.ensure_loaded(user)
|
||||||
assign(socket, :current_user, user_with_role)
|
assign(socket, :current_user, user_with_role)
|
||||||
else
|
else
|
||||||
socket
|
socket
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
defp load_user_role(user) do
|
|
||||||
case Map.get(user, :role) do
|
|
||||||
%Ash.NotLoaded{} -> load_role_safely(user)
|
|
||||||
nil -> load_role_safely(user)
|
|
||||||
_role -> user
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
defp load_role_safely(user) do
|
|
||||||
# Use self as actor for loading own role relationship
|
|
||||||
opts = [domain: Mv.Accounts, actor: user]
|
|
||||||
|
|
||||||
case Ash.load(user, :role, opts) do
|
|
||||||
{:ok, loaded_user} ->
|
|
||||||
loaded_user
|
|
||||||
|
|
||||||
{:error, error} ->
|
|
||||||
# Log warning if role loading fails - this can cause authorization issues
|
|
||||||
require Logger
|
|
||||||
Logger.warning("Failed to load role for user #{user.id}: #{inspect(error)}")
|
|
||||||
user
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
Helper function to get the current actor (user) from socket assigns.
|
Helper function to get the current actor (user) from socket assigns.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1945,12 +1945,7 @@ msgstr "Bezahlstatus"
|
||||||
msgid "Reset"
|
msgid "Reset"
|
||||||
msgstr "Zurücksetzen"
|
msgstr "Zurücksetzen"
|
||||||
|
|
||||||
#~ #: lib/mv_web/live/components/member_filter_component.ex
|
#: lib/mv_web/live/member_live/show/membership_fees_component.ex
|
||||||
#~ #, elixir-autogen, elixir-format
|
#, elixir-autogen, elixir-format
|
||||||
#~ msgid "Filter by %{name}"
|
msgid "Only administrators can regenerate cycles"
|
||||||
#~ msgstr "Filtern nach %{name}"
|
msgstr "Nur Administrator*innen können Zyklen regenerieren"
|
||||||
|
|
||||||
#~ #: lib/mv_web/live/components/member_filter_component.ex
|
|
||||||
#~ #, elixir-autogen, elixir-format, fuzzy
|
|
||||||
#~ msgid "Payment status filter"
|
|
||||||
#~ msgstr "Bezahlstatusfilter"
|
|
||||||
|
|
|
||||||
|
|
@ -1945,3 +1945,8 @@ msgstr ""
|
||||||
#, elixir-autogen, elixir-format
|
#, elixir-autogen, elixir-format
|
||||||
msgid "Reset"
|
msgid "Reset"
|
||||||
msgstr ""
|
msgstr ""
|
||||||
|
|
||||||
|
#: lib/mv_web/live/member_live/show/membership_fees_component.ex
|
||||||
|
#, elixir-autogen, elixir-format
|
||||||
|
msgid "Only administrators can regenerate cycles"
|
||||||
|
msgstr ""
|
||||||
|
|
|
||||||
|
|
@ -1946,6 +1946,11 @@ msgstr ""
|
||||||
msgid "Reset"
|
msgid "Reset"
|
||||||
msgstr ""
|
msgstr ""
|
||||||
|
|
||||||
|
#: lib/mv_web/live/member_live/show/membership_fees_component.ex
|
||||||
|
#, elixir-autogen, elixir-format
|
||||||
|
msgid "Only administrators can regenerate cycles"
|
||||||
|
msgstr ""
|
||||||
|
|
||||||
#~ #: lib/mv_web/live/custom_field_value_live/form.ex
|
#~ #: lib/mv_web/live/custom_field_value_live/form.ex
|
||||||
#~ #, elixir-autogen, elixir-format, fuzzy
|
#~ #, elixir-autogen, elixir-format, fuzzy
|
||||||
#~ msgid "Use this form to manage Custom Field Value records in your database."
|
#~ msgid "Use this form to manage Custom Field Value records in your database."
|
||||||
|
|
|
||||||
|
|
@ -161,24 +161,30 @@ end
|
||||||
# This handles both existing users (e.g., from OIDC) and newly created users
|
# This handles both existing users (e.g., from OIDC) and newly created users
|
||||||
case Accounts.User
|
case Accounts.User
|
||||||
|> Ash.Query.filter(email == ^admin_email)
|
|> Ash.Query.filter(email == ^admin_email)
|
||||||
|> Ash.read_one(domain: Mv.Accounts) do
|
|> Ash.read_one(domain: Mv.Accounts, authorize?: false) do
|
||||||
{:ok, existing_admin_user} when not is_nil(existing_admin_user) ->
|
{:ok, existing_admin_user} when not is_nil(existing_admin_user) ->
|
||||||
# User already exists (e.g., via OIDC) - assign admin role
|
# User already exists (e.g., via OIDC) - assign admin role
|
||||||
|
# Use authorize?: false for bootstrap - this is initial setup
|
||||||
existing_admin_user
|
existing_admin_user
|
||||||
|> Ash.Changeset.for_update(:update, %{})
|
|> Ash.Changeset.for_update(:update, %{})
|
||||||
|> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove)
|
|> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove)
|
||||||
|> Ash.update!()
|
|> Ash.update!(authorize?: false)
|
||||||
|
|
||||||
{:ok, nil} ->
|
{:ok, nil} ->
|
||||||
# User doesn't exist - create admin user with password
|
# User doesn't exist - create admin user with password
|
||||||
Accounts.create_user!(%{email: admin_email}, upsert?: true, upsert_identity: :unique_email)
|
# Use authorize?: false for bootstrap - no admin user exists yet to use as actor
|
||||||
|
Accounts.create_user!(%{email: admin_email},
|
||||||
|
upsert?: true,
|
||||||
|
upsert_identity: :unique_email,
|
||||||
|
authorize?: false
|
||||||
|
)
|
||||||
|> Ash.Changeset.for_update(:admin_set_password, %{password: "testpassword"})
|
|> Ash.Changeset.for_update(:admin_set_password, %{password: "testpassword"})
|
||||||
|> Ash.update!()
|
|> Ash.update!(authorize?: false)
|
||||||
|> then(fn user ->
|
|> then(fn user ->
|
||||||
user
|
user
|
||||||
|> Ash.Changeset.for_update(:update, %{})
|
|> Ash.Changeset.for_update(:update, %{})
|
||||||
|> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove)
|
|> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove)
|
||||||
|> Ash.update!()
|
|> Ash.update!(authorize?: false)
|
||||||
end)
|
end)
|
||||||
|
|
||||||
{:error, error} ->
|
{:error, error} ->
|
||||||
|
|
@ -190,10 +196,10 @@ end
|
||||||
admin_user_with_role =
|
admin_user_with_role =
|
||||||
case Accounts.User
|
case Accounts.User
|
||||||
|> Ash.Query.filter(email == ^admin_email)
|
|> Ash.Query.filter(email == ^admin_email)
|
||||||
|> Ash.read_one(domain: Mv.Accounts) do
|
|> Ash.read_one(domain: Mv.Accounts, authorize?: false) do
|
||||||
{:ok, user} when not is_nil(user) ->
|
{:ok, user} when not is_nil(user) ->
|
||||||
user
|
user
|
||||||
|> Ash.load!(:role)
|
|> Ash.load!(:role, authorize?: false)
|
||||||
|
|
||||||
{:ok, nil} ->
|
{:ok, nil} ->
|
||||||
raise "Admin user not found after creation/assignment"
|
raise "Admin user not found after creation/assignment"
|
||||||
|
|
@ -202,6 +208,45 @@ admin_user_with_role =
|
||||||
raise "Failed to load admin user: #{inspect(error)}"
|
raise "Failed to load admin user: #{inspect(error)}"
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Create system user for systemic operations (email sync, validations, cycle generation)
|
||||||
|
# This user is used by Mv.Helpers.SystemActor for operations that must always run
|
||||||
|
# Email is configurable via SYSTEM_ACTOR_EMAIL environment variable
|
||||||
|
system_user_email = Mv.Helpers.SystemActor.system_user_email()
|
||||||
|
|
||||||
|
case Accounts.User
|
||||||
|
|> Ash.Query.filter(email == ^system_user_email)
|
||||||
|
|> Ash.read_one(domain: Mv.Accounts, authorize?: false) do
|
||||||
|
{:ok, existing_system_user} when not is_nil(existing_system_user) ->
|
||||||
|
# System user already exists - ensure it has admin role
|
||||||
|
# Use authorize?: false for bootstrap
|
||||||
|
existing_system_user
|
||||||
|
|> Ash.Changeset.for_update(:update, %{})
|
||||||
|
|> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove)
|
||||||
|
|> Ash.update!(authorize?: false)
|
||||||
|
|
||||||
|
{:ok, nil} ->
|
||||||
|
# System user doesn't exist - create it with admin role
|
||||||
|
# SECURITY: System user must NOT be able to log in:
|
||||||
|
# - No password (hashed_password = nil) - prevents password login
|
||||||
|
# - No OIDC ID (oidc_id = nil) - prevents OIDC login
|
||||||
|
# - This user is ONLY for internal system operations via SystemActor
|
||||||
|
# If either hashed_password or oidc_id is set, the user could potentially log in
|
||||||
|
# Use authorize?: false for bootstrap - system user creation happens before system actor exists
|
||||||
|
Accounts.create_user!(%{email: system_user_email},
|
||||||
|
upsert?: true,
|
||||||
|
upsert_identity: :unique_email,
|
||||||
|
authorize?: false
|
||||||
|
)
|
||||||
|
|> Ash.Changeset.for_update(:update, %{})
|
||||||
|
|> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove)
|
||||||
|
|> Ash.update!(authorize?: false)
|
||||||
|
|
||||||
|
{:error, error} ->
|
||||||
|
# Log error but don't fail seeds - SystemActor will fall back to admin user
|
||||||
|
IO.puts("Warning: Failed to create system user: #{inspect(error)}")
|
||||||
|
IO.puts("SystemActor will fall back to admin user (#{admin_email})")
|
||||||
|
end
|
||||||
|
|
||||||
# Load all membership fee types for assignment
|
# Load all membership fee types for assignment
|
||||||
# Sort by name to ensure deterministic order
|
# Sort by name to ensure deterministic order
|
||||||
all_fee_types =
|
all_fee_types =
|
||||||
|
|
@ -361,9 +406,20 @@ additional_users = [
|
||||||
|
|
||||||
created_users =
|
created_users =
|
||||||
Enum.map(additional_users, fn user_attrs ->
|
Enum.map(additional_users, fn user_attrs ->
|
||||||
Accounts.create_user!(user_attrs, upsert?: true, upsert_identity: :unique_email)
|
# Use admin user as actor for additional user creation (not bootstrap)
|
||||||
|
user =
|
||||||
|
Accounts.create_user!(user_attrs,
|
||||||
|
upsert?: true,
|
||||||
|
upsert_identity: :unique_email,
|
||||||
|
actor: admin_user_with_role
|
||||||
|
)
|
||||||
|> Ash.Changeset.for_update(:admin_set_password, %{password: "testpassword"})
|
|> Ash.Changeset.for_update(:admin_set_password, %{password: "testpassword"})
|
||||||
|> Ash.update!()
|
|> Ash.update!(actor: admin_user_with_role)
|
||||||
|
|
||||||
|
# Reload user to ensure all fields (including member_id) are loaded
|
||||||
|
Accounts.User
|
||||||
|
|> Ash.Query.filter(id == ^user.id)
|
||||||
|
|> Ash.read_one!(domain: Mv.Accounts, actor: admin_user_with_role)
|
||||||
end)
|
end)
|
||||||
|
|
||||||
# Create members with linked users to demonstrate the 1:1 relationship
|
# Create members with linked users to demonstrate the 1:1 relationship
|
||||||
|
|
@ -413,11 +469,13 @@ Enum.with_index(linked_members)
|
||||||
member =
|
member =
|
||||||
if user.member_id == nil do
|
if user.member_id == nil do
|
||||||
# User is free, create member and link - use upsert to prevent duplicates
|
# User is free, create member and link - use upsert to prevent duplicates
|
||||||
|
# Use authorize?: false for User lookup during relationship management (bootstrap phase)
|
||||||
Membership.create_member!(
|
Membership.create_member!(
|
||||||
Map.put(member_attrs_without_fee_type, :user, %{id: user.id}),
|
Map.put(member_attrs_without_fee_type, :user, %{id: user.id}),
|
||||||
upsert?: true,
|
upsert?: true,
|
||||||
upsert_identity: :unique_email,
|
upsert_identity: :unique_email,
|
||||||
actor: admin_user_with_role
|
actor: admin_user_with_role,
|
||||||
|
authorize?: false
|
||||||
)
|
)
|
||||||
else
|
else
|
||||||
# User already has a member, just create the member without linking - use upsert to prevent duplicates
|
# User already has a member, just create the member without linking - use upsert to prevent duplicates
|
||||||
|
|
|
||||||
424
test/mv/accounts/user_policies_test.exs
Normal file
424
test/mv/accounts/user_policies_test.exs
Normal file
|
|
@ -0,0 +1,424 @@
|
||||||
|
defmodule Mv.Accounts.UserPoliciesTest do
|
||||||
|
@moduledoc """
|
||||||
|
Tests for User resource authorization policies.
|
||||||
|
|
||||||
|
Tests all 4 permission sets (own_data, read_only, normal_user, admin)
|
||||||
|
and verifies that policies correctly enforce access control based on
|
||||||
|
user roles and permission sets.
|
||||||
|
"""
|
||||||
|
# async: false because we need database commits to be visible across queries
|
||||||
|
use Mv.DataCase, async: false
|
||||||
|
|
||||||
|
alias Mv.Accounts
|
||||||
|
alias Mv.Authorization
|
||||||
|
|
||||||
|
require Ash.Query
|
||||||
|
|
||||||
|
# Helper to create a role with a specific permission set
|
||||||
|
defp create_role_with_permission_set(permission_set_name) do
|
||||||
|
role_name = "Test Role #{permission_set_name} #{System.unique_integer([:positive])}"
|
||||||
|
|
||||||
|
case Authorization.create_role(%{
|
||||||
|
name: role_name,
|
||||||
|
description: "Test role for #{permission_set_name}",
|
||||||
|
permission_set_name: permission_set_name
|
||||||
|
}) do
|
||||||
|
{:ok, role} -> role
|
||||||
|
{:error, error} -> raise "Failed to create role: #{inspect(error)}"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Helper to create a user with a specific permission set
|
||||||
|
# Returns user with role preloaded (required for authorization)
|
||||||
|
defp create_user_with_permission_set(permission_set_name) do
|
||||||
|
# Create role with permission set
|
||||||
|
role = create_role_with_permission_set(permission_set_name)
|
||||||
|
|
||||||
|
# Create user
|
||||||
|
{:ok, user} =
|
||||||
|
Accounts.User
|
||||||
|
|> Ash.Changeset.for_create(:register_with_password, %{
|
||||||
|
email: "user#{System.unique_integer([:positive])}@example.com",
|
||||||
|
password: "testpassword123"
|
||||||
|
})
|
||||||
|
|> Ash.create()
|
||||||
|
|
||||||
|
# Assign role to user
|
||||||
|
{:ok, user} =
|
||||||
|
user
|
||||||
|
|> Ash.Changeset.for_update(:update, %{})
|
||||||
|
|> Ash.Changeset.manage_relationship(:role, role, type: :append_and_remove)
|
||||||
|
|> Ash.update()
|
||||||
|
|
||||||
|
# Reload user with role preloaded (critical for authorization!)
|
||||||
|
{:ok, user_with_role} = Ash.load(user, :role, domain: Mv.Accounts)
|
||||||
|
user_with_role
|
||||||
|
end
|
||||||
|
|
||||||
|
# Helper to create another user (for testing access to other users)
|
||||||
|
defp create_other_user do
|
||||||
|
create_user_with_permission_set("own_data")
|
||||||
|
end
|
||||||
|
|
||||||
|
# Shared test setup for permission sets with scope :own access
|
||||||
|
defp setup_user_with_own_access(permission_set) do
|
||||||
|
user = create_user_with_permission_set(permission_set)
|
||||||
|
other_user = create_other_user()
|
||||||
|
|
||||||
|
# Reload user to ensure role is preloaded
|
||||||
|
{:ok, user} = Ash.get(Accounts.User, user.id, domain: Mv.Accounts, load: [:role])
|
||||||
|
|
||||||
|
%{user: user, other_user: other_user}
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "own_data permission set (Mitglied)" do
|
||||||
|
setup do
|
||||||
|
setup_user_with_own_access("own_data")
|
||||||
|
end
|
||||||
|
|
||||||
|
test "can read own user record", %{user: user} do
|
||||||
|
{:ok, fetched_user} =
|
||||||
|
Ash.get(Accounts.User, user.id, actor: user, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
assert fetched_user.id == user.id
|
||||||
|
end
|
||||||
|
|
||||||
|
test "can update own email", %{user: user} do
|
||||||
|
new_email = "updated#{System.unique_integer([:positive])}@example.com"
|
||||||
|
|
||||||
|
{:ok, updated_user} =
|
||||||
|
user
|
||||||
|
|> Ash.Changeset.for_update(:update_user, %{email: new_email})
|
||||||
|
|> Ash.update(actor: user)
|
||||||
|
|
||||||
|
assert updated_user.email == Ash.CiString.new(new_email)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "cannot read other users (returns not found due to auto_filter)", %{
|
||||||
|
user: user,
|
||||||
|
other_user: other_user
|
||||||
|
} do
|
||||||
|
# Note: With auto_filter policies, when a user tries to read a user that doesn't
|
||||||
|
# match the filter (id == actor.id), Ash returns NotFound, not Forbidden.
|
||||||
|
# This is the expected behavior - the filter makes the record "invisible" to the user.
|
||||||
|
assert_raise Ash.Error.Invalid, fn ->
|
||||||
|
Ash.get!(Accounts.User, other_user.id, actor: user, domain: Mv.Accounts)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test "cannot update other users (returns forbidden)", %{user: user, other_user: other_user} do
|
||||||
|
assert_raise Ash.Error.Forbidden, fn ->
|
||||||
|
other_user
|
||||||
|
|> Ash.Changeset.for_update(:update_user, %{email: "hacked@example.com"})
|
||||||
|
|> Ash.update!(actor: user)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test "list users returns only own user", %{user: user} do
|
||||||
|
{:ok, users} = Ash.read(Accounts.User, actor: user, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
# Should only return the own user (scope :own filters)
|
||||||
|
assert length(users) == 1
|
||||||
|
assert hd(users).id == user.id
|
||||||
|
end
|
||||||
|
|
||||||
|
test "cannot create user (returns forbidden)", %{user: user} do
|
||||||
|
assert_raise Ash.Error.Forbidden, fn ->
|
||||||
|
Accounts.User
|
||||||
|
|> Ash.Changeset.for_create(:create_user, %{
|
||||||
|
email: "new#{System.unique_integer([:positive])}@example.com"
|
||||||
|
})
|
||||||
|
|> Ash.create!(actor: user)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test "cannot destroy user (returns forbidden)", %{user: user} do
|
||||||
|
assert_raise Ash.Error.Forbidden, fn ->
|
||||||
|
Ash.destroy!(user, actor: user)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "read_only permission set (Vorstand/Buchhaltung)" do
|
||||||
|
setup do
|
||||||
|
setup_user_with_own_access("read_only")
|
||||||
|
end
|
||||||
|
|
||||||
|
test "can read own user record", %{user: user} do
|
||||||
|
{:ok, fetched_user} =
|
||||||
|
Ash.get(Accounts.User, user.id, actor: user, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
assert fetched_user.id == user.id
|
||||||
|
end
|
||||||
|
|
||||||
|
test "can update own email", %{user: user} do
|
||||||
|
new_email = "updated#{System.unique_integer([:positive])}@example.com"
|
||||||
|
|
||||||
|
{:ok, updated_user} =
|
||||||
|
user
|
||||||
|
|> Ash.Changeset.for_update(:update_user, %{email: new_email})
|
||||||
|
|> Ash.update(actor: user)
|
||||||
|
|
||||||
|
assert updated_user.email == Ash.CiString.new(new_email)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "cannot read other users (returns not found due to auto_filter)", %{
|
||||||
|
user: user,
|
||||||
|
other_user: other_user
|
||||||
|
} do
|
||||||
|
# Note: With auto_filter policies, when a user tries to read a user that doesn't
|
||||||
|
# match the filter (id == actor.id), Ash returns NotFound, not Forbidden.
|
||||||
|
# This is the expected behavior - the filter makes the record "invisible" to the user.
|
||||||
|
assert_raise Ash.Error.Invalid, fn ->
|
||||||
|
Ash.get!(Accounts.User, other_user.id, actor: user, domain: Mv.Accounts)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test "cannot update other users (returns forbidden)", %{user: user, other_user: other_user} do
|
||||||
|
assert_raise Ash.Error.Forbidden, fn ->
|
||||||
|
other_user
|
||||||
|
|> Ash.Changeset.for_update(:update_user, %{email: "hacked@example.com"})
|
||||||
|
|> Ash.update!(actor: user)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test "list users returns only own user", %{user: user} do
|
||||||
|
{:ok, users} = Ash.read(Accounts.User, actor: user, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
# Should only return the own user (scope :own filters)
|
||||||
|
assert length(users) == 1
|
||||||
|
assert hd(users).id == user.id
|
||||||
|
end
|
||||||
|
|
||||||
|
test "cannot create user (returns forbidden)", %{user: user} do
|
||||||
|
assert_raise Ash.Error.Forbidden, fn ->
|
||||||
|
Accounts.User
|
||||||
|
|> Ash.Changeset.for_create(:create_user, %{
|
||||||
|
email: "new#{System.unique_integer([:positive])}@example.com"
|
||||||
|
})
|
||||||
|
|> Ash.create!(actor: user)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test "cannot destroy user (returns forbidden)", %{user: user} do
|
||||||
|
assert_raise Ash.Error.Forbidden, fn ->
|
||||||
|
Ash.destroy!(user, actor: user)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "normal_user permission set (Kassenwart)" do
|
||||||
|
setup do
|
||||||
|
setup_user_with_own_access("normal_user")
|
||||||
|
end
|
||||||
|
|
||||||
|
test "can read own user record", %{user: user} do
|
||||||
|
{:ok, fetched_user} =
|
||||||
|
Ash.get(Accounts.User, user.id, actor: user, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
assert fetched_user.id == user.id
|
||||||
|
end
|
||||||
|
|
||||||
|
test "can update own email", %{user: user} do
|
||||||
|
new_email = "updated#{System.unique_integer([:positive])}@example.com"
|
||||||
|
|
||||||
|
{:ok, updated_user} =
|
||||||
|
user
|
||||||
|
|> Ash.Changeset.for_update(:update_user, %{email: new_email})
|
||||||
|
|> Ash.update(actor: user)
|
||||||
|
|
||||||
|
assert updated_user.email == Ash.CiString.new(new_email)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "cannot read other users (returns not found due to auto_filter)", %{
|
||||||
|
user: user,
|
||||||
|
other_user: other_user
|
||||||
|
} do
|
||||||
|
# Note: With auto_filter policies, when a user tries to read a user that doesn't
|
||||||
|
# match the filter (id == actor.id), Ash returns NotFound, not Forbidden.
|
||||||
|
# This is the expected behavior - the filter makes the record "invisible" to the user.
|
||||||
|
assert_raise Ash.Error.Invalid, fn ->
|
||||||
|
Ash.get!(Accounts.User, other_user.id, actor: user, domain: Mv.Accounts)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test "cannot update other users (returns forbidden)", %{user: user, other_user: other_user} do
|
||||||
|
assert_raise Ash.Error.Forbidden, fn ->
|
||||||
|
other_user
|
||||||
|
|> Ash.Changeset.for_update(:update_user, %{email: "hacked@example.com"})
|
||||||
|
|> Ash.update!(actor: user)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test "list users returns only own user", %{user: user} do
|
||||||
|
{:ok, users} = Ash.read(Accounts.User, actor: user, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
# Should only return the own user (scope :own filters)
|
||||||
|
assert length(users) == 1
|
||||||
|
assert hd(users).id == user.id
|
||||||
|
end
|
||||||
|
|
||||||
|
test "cannot create user (returns forbidden)", %{user: user} do
|
||||||
|
assert_raise Ash.Error.Forbidden, fn ->
|
||||||
|
Accounts.User
|
||||||
|
|> Ash.Changeset.for_create(:create_user, %{
|
||||||
|
email: "new#{System.unique_integer([:positive])}@example.com"
|
||||||
|
})
|
||||||
|
|> Ash.create!(actor: user)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test "cannot destroy user (returns forbidden)", %{user: user} do
|
||||||
|
assert_raise Ash.Error.Forbidden, fn ->
|
||||||
|
Ash.destroy!(user, actor: user)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "admin permission set" do
|
||||||
|
setup do
|
||||||
|
user = create_user_with_permission_set("admin")
|
||||||
|
other_user = create_other_user()
|
||||||
|
|
||||||
|
# Reload user to ensure role is preloaded
|
||||||
|
{:ok, user} = Ash.get(Accounts.User, user.id, domain: Mv.Accounts, load: [:role])
|
||||||
|
|
||||||
|
%{user: user, other_user: other_user}
|
||||||
|
end
|
||||||
|
|
||||||
|
test "can read all users", %{user: user, other_user: other_user} do
|
||||||
|
{:ok, users} = Ash.read(Accounts.User, actor: user, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
# Should return all users (scope :all)
|
||||||
|
user_ids = Enum.map(users, & &1.id)
|
||||||
|
assert user.id in user_ids
|
||||||
|
assert other_user.id in user_ids
|
||||||
|
end
|
||||||
|
|
||||||
|
test "can read other users", %{user: user, other_user: other_user} do
|
||||||
|
{:ok, fetched_user} =
|
||||||
|
Ash.get(Accounts.User, other_user.id, actor: user, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
assert fetched_user.id == other_user.id
|
||||||
|
end
|
||||||
|
|
||||||
|
test "can update other users", %{user: user, other_user: other_user} do
|
||||||
|
new_email = "adminupdated#{System.unique_integer([:positive])}@example.com"
|
||||||
|
|
||||||
|
{:ok, updated_user} =
|
||||||
|
other_user
|
||||||
|
|> Ash.Changeset.for_update(:update_user, %{email: new_email})
|
||||||
|
|> Ash.update(actor: user)
|
||||||
|
|
||||||
|
assert updated_user.email == Ash.CiString.new(new_email)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "can create user", %{user: user} do
|
||||||
|
{:ok, new_user} =
|
||||||
|
Accounts.User
|
||||||
|
|> Ash.Changeset.for_create(:create_user, %{
|
||||||
|
email: "new#{System.unique_integer([:positive])}@example.com"
|
||||||
|
})
|
||||||
|
|> Ash.create(actor: user)
|
||||||
|
|
||||||
|
assert new_user.email
|
||||||
|
end
|
||||||
|
|
||||||
|
test "can destroy user", %{user: user, other_user: other_user} do
|
||||||
|
:ok = Ash.destroy(other_user, actor: user)
|
||||||
|
|
||||||
|
# Verify user is deleted
|
||||||
|
assert {:error, _} = Ash.get(Accounts.User, other_user.id, domain: Mv.Accounts)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "AshAuthentication bypass" do
|
||||||
|
test "register_with_password works without actor" do
|
||||||
|
# Registration should work without actor (AshAuthentication bypass)
|
||||||
|
{:ok, user} =
|
||||||
|
Accounts.User
|
||||||
|
|> Ash.Changeset.for_create(:register_with_password, %{
|
||||||
|
email: "register#{System.unique_integer([:positive])}@example.com",
|
||||||
|
password: "testpassword123"
|
||||||
|
})
|
||||||
|
|> Ash.create()
|
||||||
|
|
||||||
|
assert user.email
|
||||||
|
end
|
||||||
|
|
||||||
|
test "register_with_rauthy works with OIDC user_info" do
|
||||||
|
# OIDC registration should work (AshAuthentication bypass)
|
||||||
|
user_info = %{
|
||||||
|
"sub" => "oidc_sub_#{System.unique_integer([:positive])}",
|
||||||
|
"email" => "oidc#{System.unique_integer([:positive])}@example.com"
|
||||||
|
}
|
||||||
|
|
||||||
|
oauth_tokens = %{access_token: "token", refresh_token: "refresh"}
|
||||||
|
|
||||||
|
{:ok, user} =
|
||||||
|
Accounts.User
|
||||||
|
|> Ash.Changeset.for_create(:register_with_rauthy, %{
|
||||||
|
user_info: user_info,
|
||||||
|
oauth_tokens: oauth_tokens
|
||||||
|
})
|
||||||
|
|> Ash.create()
|
||||||
|
|
||||||
|
assert user.email
|
||||||
|
assert user.oidc_id == user_info["sub"]
|
||||||
|
end
|
||||||
|
|
||||||
|
test "sign_in_with_rauthy works with OIDC user_info" do
|
||||||
|
# First create a user with OIDC ID
|
||||||
|
user_info_create = %{
|
||||||
|
"sub" => "oidc_sub_#{System.unique_integer([:positive])}",
|
||||||
|
"email" => "oidc#{System.unique_integer([:positive])}@example.com"
|
||||||
|
}
|
||||||
|
|
||||||
|
oauth_tokens = %{access_token: "token", refresh_token: "refresh"}
|
||||||
|
|
||||||
|
{:ok, user} =
|
||||||
|
Accounts.User
|
||||||
|
|> Ash.Changeset.for_create(:register_with_rauthy, %{
|
||||||
|
user_info: user_info_create,
|
||||||
|
oauth_tokens: oauth_tokens
|
||||||
|
})
|
||||||
|
|> Ash.create()
|
||||||
|
|
||||||
|
# Now test sign_in_with_rauthy (should work via AshAuthentication bypass)
|
||||||
|
{:ok, signed_in_user} =
|
||||||
|
Accounts.User
|
||||||
|
|> Ash.Query.for_read(:sign_in_with_rauthy, %{
|
||||||
|
user_info: user_info_create,
|
||||||
|
oauth_tokens: oauth_tokens
|
||||||
|
})
|
||||||
|
|> Ash.read_one()
|
||||||
|
|
||||||
|
assert signed_in_user.id == user.id
|
||||||
|
end
|
||||||
|
|
||||||
|
# NOTE: get_by_subject is tested implicitly via AshAuthentication's JWT flow.
|
||||||
|
# Direct testing via Ash.Query.for_read(:get_by_subject) doesn't properly
|
||||||
|
# simulate the AshAuthentication context and would require mocking JWT tokens.
|
||||||
|
# The AshAuthentication bypass policy ensures this action works correctly
|
||||||
|
# when called through the proper authentication flow (sign_in, token refresh, etc.).
|
||||||
|
# Integration tests that use actual JWT tokens cover this functionality.
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "test environment bypass (NoActor)" do
|
||||||
|
test "operations without actor are allowed in test environment" do
|
||||||
|
# In test environment, NoActor check should allow operations
|
||||||
|
{:ok, user} =
|
||||||
|
Accounts.User
|
||||||
|
|> Ash.Changeset.for_create(:create_user, %{
|
||||||
|
email: "noactor#{System.unique_integer([:positive])}@example.com"
|
||||||
|
})
|
||||||
|
|> Ash.create()
|
||||||
|
|
||||||
|
assert user.email
|
||||||
|
|
||||||
|
# Read should also work
|
||||||
|
{:ok, fetched_user} = Ash.get(Accounts.User, user.id, domain: Mv.Accounts)
|
||||||
|
assert fetched_user.id == user.id
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
84
test/mv/authorization/actor_test.exs
Normal file
84
test/mv/authorization/actor_test.exs
Normal file
|
|
@ -0,0 +1,84 @@
|
||||||
|
defmodule Mv.Authorization.ActorTest do
|
||||||
|
@moduledoc """
|
||||||
|
Tests for the Actor helper module.
|
||||||
|
"""
|
||||||
|
use Mv.DataCase, async: false
|
||||||
|
|
||||||
|
alias Mv.Accounts
|
||||||
|
alias Mv.Authorization.Actor
|
||||||
|
|
||||||
|
describe "ensure_loaded/1" do
|
||||||
|
test "returns nil when actor is nil" do
|
||||||
|
assert Actor.ensure_loaded(nil) == nil
|
||||||
|
end
|
||||||
|
|
||||||
|
test "returns actor as-is when role is already loaded" do
|
||||||
|
# Create user with role
|
||||||
|
{:ok, user} =
|
||||||
|
Accounts.User
|
||||||
|
|> Ash.Changeset.for_create(:register_with_password, %{
|
||||||
|
email: "test#{System.unique_integer([:positive])}@example.com",
|
||||||
|
password: "testpassword123"
|
||||||
|
})
|
||||||
|
|> Ash.create()
|
||||||
|
|
||||||
|
# Load role
|
||||||
|
{:ok, user_with_role} = Ash.load(user, :role, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
# Should return as-is (no additional load)
|
||||||
|
result = Actor.ensure_loaded(user_with_role)
|
||||||
|
assert result.id == user.id
|
||||||
|
assert result.role != %Ash.NotLoaded{}
|
||||||
|
end
|
||||||
|
|
||||||
|
test "loads role when it's NotLoaded" do
|
||||||
|
# Create a role first
|
||||||
|
{:ok, role} =
|
||||||
|
Mv.Authorization.Role
|
||||||
|
|> Ash.Changeset.for_create(:create_role, %{
|
||||||
|
name: "Test Role #{System.unique_integer([:positive])}",
|
||||||
|
description: "Test role",
|
||||||
|
permission_set_name: "own_data"
|
||||||
|
})
|
||||||
|
|> Ash.create()
|
||||||
|
|
||||||
|
# Create user with role
|
||||||
|
{:ok, user} =
|
||||||
|
Accounts.User
|
||||||
|
|> Ash.Changeset.for_create(:register_with_password, %{
|
||||||
|
email: "test#{System.unique_integer([:positive])}@example.com",
|
||||||
|
password: "testpassword123"
|
||||||
|
})
|
||||||
|
|> Ash.create()
|
||||||
|
|
||||||
|
# Assign role to user
|
||||||
|
{:ok, user_with_role} =
|
||||||
|
user
|
||||||
|
|> Ash.Changeset.for_update(:update, %{})
|
||||||
|
|> Ash.Changeset.manage_relationship(:role, role, type: :append_and_remove)
|
||||||
|
|> Ash.update()
|
||||||
|
|
||||||
|
# Fetch user again WITHOUT loading role (simulates "role not preloaded" scenario)
|
||||||
|
{:ok, user_without_role_loaded} =
|
||||||
|
Ash.get(Accounts.User, user_with_role.id, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
# User has role as NotLoaded (relationship not preloaded)
|
||||||
|
assert match?(%Ash.NotLoaded{}, user_without_role_loaded.role)
|
||||||
|
|
||||||
|
# ensure_loaded should load it
|
||||||
|
result = Actor.ensure_loaded(user_without_role_loaded)
|
||||||
|
assert result.id == user.id
|
||||||
|
refute match?(%Ash.NotLoaded{}, result.role)
|
||||||
|
assert result.role.id == role.id
|
||||||
|
end
|
||||||
|
|
||||||
|
test "returns non-User actors as-is (no-op)" do
|
||||||
|
# Create a plain map (not Mv.Accounts.User)
|
||||||
|
other_actor = %{id: "fake", role: %Ash.NotLoaded{field: :role}}
|
||||||
|
|
||||||
|
# Should return as-is (pattern match doesn't apply to non-User)
|
||||||
|
result = Actor.ensure_loaded(other_actor)
|
||||||
|
assert result == other_actor
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
@ -76,8 +76,10 @@ defmodule Mv.Authorization.Checks.HasPermissionTest do
|
||||||
|
|
||||||
{:ok, result} = HasPermission.strict_check(own_data_user, authorizer, [])
|
{:ok, result} = HasPermission.strict_check(own_data_user, authorizer, [])
|
||||||
|
|
||||||
# Should return :unknown for :own scope (needs filter)
|
# Should return false for :own scope without record
|
||||||
assert result == :unknown
|
# This prevents bypassing expr-based filters in bypass policies
|
||||||
|
# The actual filtering is done via bypass policies with expr(id == ^actor(:id))
|
||||||
|
assert result == false
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
@ -104,14 +106,16 @@ defmodule Mv.Authorization.Checks.HasPermissionTest do
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "strict_check/3 - Scope :own" do
|
describe "strict_check/3 - Scope :own" do
|
||||||
test "actor with scope :own returns :unknown (needs filter)" do
|
test "actor with scope :own returns false (needs bypass policy with expr filter)" do
|
||||||
user = create_actor("user-123", "own_data")
|
user = create_actor("user-123", "own_data")
|
||||||
authorizer = create_authorizer(Mv.Accounts.User, :read)
|
authorizer = create_authorizer(Mv.Accounts.User, :read)
|
||||||
|
|
||||||
{:ok, result} = HasPermission.strict_check(user, authorizer, [])
|
{:ok, result} = HasPermission.strict_check(user, authorizer, [])
|
||||||
|
|
||||||
# Should return :unknown for :own scope (needs filter via auto_filter)
|
# Should return false for :own scope without record
|
||||||
assert result == :unknown
|
# This prevents bypassing expr-based filters in bypass policies
|
||||||
|
# The actual filtering is done via bypass policies with expr(id == ^actor(:id))
|
||||||
|
assert result == false
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
@ -270,4 +274,44 @@ defmodule Mv.Authorization.Checks.HasPermissionTest do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe "strict_check/3 - Role Loading Fallback" do
|
||||||
|
test "returns false if role is NotLoaded and cannot be loaded" do
|
||||||
|
# Create actor with NotLoaded role
|
||||||
|
# In real scenario, ensure_role_loaded would attempt to load via Ash.load
|
||||||
|
# For this test, we use a simple map to verify the pattern matching works
|
||||||
|
actor = %{
|
||||||
|
id: "user-123",
|
||||||
|
role: %Ash.NotLoaded{}
|
||||||
|
}
|
||||||
|
|
||||||
|
authorizer = create_authorizer(Mv.Accounts.User, :read)
|
||||||
|
|
||||||
|
# Should handle NotLoaded pattern and return false
|
||||||
|
# (In real scenario, ensure_role_loaded would attempt to load, but for this test
|
||||||
|
# we just verify the pattern matching works correctly)
|
||||||
|
{:ok, result} = HasPermission.strict_check(actor, authorizer, [])
|
||||||
|
assert result == false
|
||||||
|
end
|
||||||
|
|
||||||
|
test "returns false if role is nil" do
|
||||||
|
actor = %{
|
||||||
|
id: "user-123",
|
||||||
|
role: nil
|
||||||
|
}
|
||||||
|
|
||||||
|
authorizer = create_authorizer(Mv.Accounts.User, :read)
|
||||||
|
|
||||||
|
{:ok, result} = HasPermission.strict_check(actor, authorizer, [])
|
||||||
|
assert result == false
|
||||||
|
end
|
||||||
|
|
||||||
|
test "works correctly when role is already loaded" do
|
||||||
|
actor = create_actor("user-123", "admin")
|
||||||
|
authorizer = create_authorizer(Mv.Accounts.User, :read)
|
||||||
|
|
||||||
|
{:ok, result} = HasPermission.strict_check(actor, authorizer, [])
|
||||||
|
assert result == true
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
||||||
52
test/mv/authorization/checks/no_actor_test.exs
Normal file
52
test/mv/authorization/checks/no_actor_test.exs
Normal file
|
|
@ -0,0 +1,52 @@
|
||||||
|
defmodule Mv.Authorization.Checks.NoActorTest do
|
||||||
|
@moduledoc """
|
||||||
|
Tests for the NoActor Ash Policy Check.
|
||||||
|
|
||||||
|
This check allows actions without an actor ONLY in test environment.
|
||||||
|
In production/dev, all operations without an actor are denied.
|
||||||
|
"""
|
||||||
|
use ExUnit.Case, async: true
|
||||||
|
|
||||||
|
alias Mv.Authorization.Checks.NoActor
|
||||||
|
|
||||||
|
describe "match?/3" do
|
||||||
|
test "returns true when actor is nil in test environment" do
|
||||||
|
# In test environment (config :allow_no_actor_bypass = true), NoActor allows operations
|
||||||
|
result = NoActor.match?(nil, %{}, [])
|
||||||
|
assert result == true
|
||||||
|
end
|
||||||
|
|
||||||
|
test "returns false when actor is present" do
|
||||||
|
actor = %{id: "user-123"}
|
||||||
|
result = NoActor.match?(actor, %{}, [])
|
||||||
|
assert result == false
|
||||||
|
end
|
||||||
|
|
||||||
|
test "uses compile-time config (not runtime Mix.env)" do
|
||||||
|
# The @allow_no_actor_bypass is set via Application.compile_env at compile time
|
||||||
|
# In test.exs: config :mv, :allow_no_actor_bypass, true
|
||||||
|
# In prod/dev: not set (defaults to false)
|
||||||
|
# This ensures the check is release-safe (no runtime Mix.env dependency)
|
||||||
|
result = NoActor.match?(nil, %{}, [])
|
||||||
|
|
||||||
|
# In test environment (as compiled), should allow
|
||||||
|
assert result == true
|
||||||
|
|
||||||
|
# Note: We cannot test "production mode" here because the flag is compile-time.
|
||||||
|
# Production safety is guaranteed by:
|
||||||
|
# 1. Config only set in test.exs
|
||||||
|
# 2. Default is false (fail-closed)
|
||||||
|
# 3. No runtime environment checks
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "describe/1" do
|
||||||
|
test "returns description based on compile-time config" do
|
||||||
|
description = NoActor.describe([])
|
||||||
|
assert is_binary(description)
|
||||||
|
|
||||||
|
# In test environment (compiled with :allow_no_actor_bypass = true)
|
||||||
|
assert description =~ "test environment"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
362
test/mv/helpers/system_actor_test.exs
Normal file
362
test/mv/helpers/system_actor_test.exs
Normal file
|
|
@ -0,0 +1,362 @@
|
||||||
|
defmodule Mv.Helpers.SystemActorTest do
|
||||||
|
@moduledoc """
|
||||||
|
Tests for the SystemActor helper module.
|
||||||
|
"""
|
||||||
|
use Mv.DataCase, async: false
|
||||||
|
|
||||||
|
alias Mv.Helpers.SystemActor
|
||||||
|
alias Mv.Authorization
|
||||||
|
alias Mv.Accounts
|
||||||
|
|
||||||
|
require Ash.Query
|
||||||
|
|
||||||
|
# Helper function to ensure admin role exists
|
||||||
|
defp ensure_admin_role do
|
||||||
|
case Authorization.list_roles() do
|
||||||
|
{:ok, roles} ->
|
||||||
|
case Enum.find(roles, &(&1.permission_set_name == "admin")) do
|
||||||
|
nil ->
|
||||||
|
{:ok, role} =
|
||||||
|
Authorization.create_role(%{
|
||||||
|
name: "Admin",
|
||||||
|
description: "Administrator with full access",
|
||||||
|
permission_set_name: "admin"
|
||||||
|
})
|
||||||
|
|
||||||
|
role
|
||||||
|
|
||||||
|
role ->
|
||||||
|
role
|
||||||
|
end
|
||||||
|
|
||||||
|
_ ->
|
||||||
|
{:ok, role} =
|
||||||
|
Authorization.create_role(%{
|
||||||
|
name: "Admin",
|
||||||
|
description: "Administrator with full access",
|
||||||
|
permission_set_name: "admin"
|
||||||
|
})
|
||||||
|
|
||||||
|
role
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Helper function to ensure system user exists with admin role
|
||||||
|
defp ensure_system_user(admin_role) do
|
||||||
|
case Accounts.User
|
||||||
|
|> Ash.Query.filter(email == ^"system@mila.local")
|
||||||
|
|> Ash.read_one(domain: Mv.Accounts) do
|
||||||
|
{:ok, user} when not is_nil(user) ->
|
||||||
|
user
|
||||||
|
|> Ash.Changeset.for_update(:update, %{})
|
||||||
|
|> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove)
|
||||||
|
|> Ash.update!()
|
||||||
|
|> Ash.load!(:role, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
_ ->
|
||||||
|
Accounts.create_user!(%{email: "system@mila.local"},
|
||||||
|
upsert?: true,
|
||||||
|
upsert_identity: :unique_email
|
||||||
|
)
|
||||||
|
|> Ash.Changeset.for_update(:update, %{})
|
||||||
|
|> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove)
|
||||||
|
|> Ash.update!()
|
||||||
|
|> Ash.load!(:role, domain: Mv.Accounts)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Helper function to ensure admin user exists with admin role
|
||||||
|
defp ensure_admin_user(admin_role) do
|
||||||
|
admin_email = System.get_env("ADMIN_EMAIL") || "admin@localhost"
|
||||||
|
|
||||||
|
case Accounts.User
|
||||||
|
|> Ash.Query.filter(email == ^admin_email)
|
||||||
|
|> Ash.read_one(domain: Mv.Accounts) do
|
||||||
|
{:ok, user} when not is_nil(user) ->
|
||||||
|
user
|
||||||
|
|> Ash.Changeset.for_update(:update, %{})
|
||||||
|
|> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove)
|
||||||
|
|> Ash.update!()
|
||||||
|
|> Ash.load!(:role, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
_ ->
|
||||||
|
Accounts.create_user!(%{email: admin_email},
|
||||||
|
upsert?: true,
|
||||||
|
upsert_identity: :unique_email
|
||||||
|
)
|
||||||
|
|> Ash.Changeset.for_update(:update, %{})
|
||||||
|
|> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove)
|
||||||
|
|> Ash.update!()
|
||||||
|
|> Ash.load!(:role, domain: Mv.Accounts)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
setup do
|
||||||
|
admin_role = ensure_admin_role()
|
||||||
|
system_user = ensure_system_user(admin_role)
|
||||||
|
admin_user = ensure_admin_user(admin_role)
|
||||||
|
|
||||||
|
# Invalidate cache to ensure fresh load
|
||||||
|
SystemActor.invalidate_cache()
|
||||||
|
|
||||||
|
%{admin_role: admin_role, system_user: system_user, admin_user: admin_user}
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "get_system_actor/0" do
|
||||||
|
test "returns system user with admin role", %{system_user: _system_user} do
|
||||||
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
|
||||||
|
assert %Mv.Accounts.User{} = system_actor
|
||||||
|
assert to_string(system_actor.email) == "system@mila.local"
|
||||||
|
assert system_actor.role != nil
|
||||||
|
assert system_actor.role.permission_set_name == "admin"
|
||||||
|
end
|
||||||
|
|
||||||
|
test "falls back to admin user if system user doesn't exist", %{admin_user: _admin_user} do
|
||||||
|
# Delete system user if it exists
|
||||||
|
case Accounts.User
|
||||||
|
|> Ash.Query.filter(email == ^"system@mila.local")
|
||||||
|
|> Ash.read_one(domain: Mv.Accounts) do
|
||||||
|
{:ok, user} when not is_nil(user) ->
|
||||||
|
Ash.destroy!(user, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
_ ->
|
||||||
|
:ok
|
||||||
|
end
|
||||||
|
|
||||||
|
# Invalidate cache to force reload
|
||||||
|
SystemActor.invalidate_cache()
|
||||||
|
|
||||||
|
# Should fall back to admin user
|
||||||
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
|
||||||
|
assert %Mv.Accounts.User{} = system_actor
|
||||||
|
assert system_actor.role != nil
|
||||||
|
assert system_actor.role.permission_set_name == "admin"
|
||||||
|
# Should be admin user, not system user
|
||||||
|
assert to_string(system_actor.email) != "system@mila.local"
|
||||||
|
end
|
||||||
|
|
||||||
|
test "caches system actor for performance" do
|
||||||
|
# First call
|
||||||
|
actor1 = SystemActor.get_system_actor()
|
||||||
|
|
||||||
|
# Second call should return cached actor (same struct)
|
||||||
|
actor2 = SystemActor.get_system_actor()
|
||||||
|
|
||||||
|
# Should be the same struct (cached)
|
||||||
|
assert actor1.id == actor2.id
|
||||||
|
end
|
||||||
|
|
||||||
|
test "creates system user in test environment if none exists", %{admin_role: _admin_role} do
|
||||||
|
# In test environment, system actor should auto-create if missing
|
||||||
|
# Delete all users to test auto-creation
|
||||||
|
case Accounts.User
|
||||||
|
|> Ash.Query.filter(email == ^"system@mila.local")
|
||||||
|
|> Ash.read_one(domain: Mv.Accounts) do
|
||||||
|
{:ok, user} when not is_nil(user) ->
|
||||||
|
Ash.destroy!(user, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
_ ->
|
||||||
|
:ok
|
||||||
|
end
|
||||||
|
|
||||||
|
admin_email = System.get_env("ADMIN_EMAIL") || "admin@localhost"
|
||||||
|
|
||||||
|
case Accounts.User
|
||||||
|
|> Ash.Query.filter(email == ^admin_email)
|
||||||
|
|> Ash.read_one(domain: Mv.Accounts) do
|
||||||
|
{:ok, user} when not is_nil(user) ->
|
||||||
|
Ash.destroy!(user, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
_ ->
|
||||||
|
:ok
|
||||||
|
end
|
||||||
|
|
||||||
|
# Invalidate cache
|
||||||
|
SystemActor.invalidate_cache()
|
||||||
|
|
||||||
|
# Should auto-create system user in test environment
|
||||||
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
|
||||||
|
assert %Mv.Accounts.User{} = system_actor
|
||||||
|
assert to_string(system_actor.email) == "system@mila.local"
|
||||||
|
assert system_actor.role != nil
|
||||||
|
assert system_actor.role.permission_set_name == "admin"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "invalidate_cache/0" do
|
||||||
|
test "forces reload of system actor on next call" do
|
||||||
|
# Get initial actor
|
||||||
|
actor1 = SystemActor.get_system_actor()
|
||||||
|
|
||||||
|
# Invalidate cache
|
||||||
|
:ok = SystemActor.invalidate_cache()
|
||||||
|
|
||||||
|
# Next call should reload (but should be same user)
|
||||||
|
actor2 = SystemActor.get_system_actor()
|
||||||
|
|
||||||
|
# Should be same user (same ID)
|
||||||
|
assert actor1.id == actor2.id
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "get_system_actor_result/0" do
|
||||||
|
test "returns ok tuple with system actor" do
|
||||||
|
assert {:ok, actor} = SystemActor.get_system_actor_result()
|
||||||
|
assert %Mv.Accounts.User{} = actor
|
||||||
|
assert actor.role.permission_set_name == "admin"
|
||||||
|
end
|
||||||
|
|
||||||
|
test "returns error tuple when system actor cannot be loaded" do
|
||||||
|
# Delete all users to force error
|
||||||
|
case Accounts.User
|
||||||
|
|> Ash.Query.filter(email == ^"system@mila.local")
|
||||||
|
|> Ash.read_one(domain: Mv.Accounts) do
|
||||||
|
{:ok, user} when not is_nil(user) ->
|
||||||
|
Ash.destroy!(user, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
_ ->
|
||||||
|
:ok
|
||||||
|
end
|
||||||
|
|
||||||
|
admin_email = System.get_env("ADMIN_EMAIL") || "admin@localhost"
|
||||||
|
|
||||||
|
case Accounts.User
|
||||||
|
|> Ash.Query.filter(email == ^admin_email)
|
||||||
|
|> Ash.read_one(domain: Mv.Accounts) do
|
||||||
|
{:ok, user} when not is_nil(user) ->
|
||||||
|
Ash.destroy!(user, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
_ ->
|
||||||
|
:ok
|
||||||
|
end
|
||||||
|
|
||||||
|
SystemActor.invalidate_cache()
|
||||||
|
|
||||||
|
# In test environment, it should auto-create, so this should succeed
|
||||||
|
# But if it fails, we should get an error tuple
|
||||||
|
result = SystemActor.get_system_actor_result()
|
||||||
|
|
||||||
|
# Should either succeed (auto-created) or return error
|
||||||
|
assert match?({:ok, _}, result) or match?({:error, _}, result)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "system_user_email/0" do
|
||||||
|
test "returns the system user email address" do
|
||||||
|
assert SystemActor.system_user_email() == "system@mila.local"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "edge cases" do
|
||||||
|
test "raises error if admin user has no role", %{admin_user: admin_user} do
|
||||||
|
# Remove role from admin user
|
||||||
|
admin_user
|
||||||
|
|> Ash.Changeset.for_update(:update, %{})
|
||||||
|
|> Ash.Changeset.manage_relationship(:role, nil, type: :append_and_remove)
|
||||||
|
|> Ash.update!()
|
||||||
|
|
||||||
|
# Delete system user to force fallback
|
||||||
|
case Accounts.User
|
||||||
|
|> Ash.Query.filter(email == ^"system@mila.local")
|
||||||
|
|> Ash.read_one(domain: Mv.Accounts) do
|
||||||
|
{:ok, user} when not is_nil(user) ->
|
||||||
|
Ash.destroy!(user, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
_ ->
|
||||||
|
:ok
|
||||||
|
end
|
||||||
|
|
||||||
|
SystemActor.invalidate_cache()
|
||||||
|
|
||||||
|
# Should raise error because admin user has no role
|
||||||
|
assert_raise RuntimeError, ~r/System actor must have a role assigned/, fn ->
|
||||||
|
SystemActor.get_system_actor()
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test "handles concurrent calls without race conditions" do
|
||||||
|
# Delete system user and admin user to force creation
|
||||||
|
case Accounts.User
|
||||||
|
|> Ash.Query.filter(email == ^"system@mila.local")
|
||||||
|
|> Ash.read_one(domain: Mv.Accounts) do
|
||||||
|
{:ok, user} when not is_nil(user) ->
|
||||||
|
Ash.destroy!(user, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
_ ->
|
||||||
|
:ok
|
||||||
|
end
|
||||||
|
|
||||||
|
admin_email = System.get_env("ADMIN_EMAIL") || "admin@localhost"
|
||||||
|
|
||||||
|
case Accounts.User
|
||||||
|
|> Ash.Query.filter(email == ^admin_email)
|
||||||
|
|> Ash.read_one(domain: Mv.Accounts) do
|
||||||
|
{:ok, user} when not is_nil(user) ->
|
||||||
|
Ash.destroy!(user, domain: Mv.Accounts)
|
||||||
|
|
||||||
|
_ ->
|
||||||
|
:ok
|
||||||
|
end
|
||||||
|
|
||||||
|
SystemActor.invalidate_cache()
|
||||||
|
|
||||||
|
# Call get_system_actor concurrently from multiple processes
|
||||||
|
tasks =
|
||||||
|
for _ <- 1..10 do
|
||||||
|
Task.async(fn -> SystemActor.get_system_actor() end)
|
||||||
|
end
|
||||||
|
|
||||||
|
results = Task.await_many(tasks, :infinity)
|
||||||
|
|
||||||
|
# All should succeed and return the same actor
|
||||||
|
assert length(results) == 10
|
||||||
|
assert Enum.all?(results, &(&1.role.permission_set_name == "admin"))
|
||||||
|
assert Enum.all?(results, fn actor -> to_string(actor.email) == "system@mila.local" end)
|
||||||
|
|
||||||
|
# All should have the same ID (same user)
|
||||||
|
ids = Enum.map(results, & &1.id)
|
||||||
|
assert Enum.uniq(ids) |> length() == 1
|
||||||
|
end
|
||||||
|
|
||||||
|
test "raises error if system user has wrong role", %{system_user: system_user} do
|
||||||
|
# Create a non-admin role (using read_only as it's a valid permission set)
|
||||||
|
{:ok, read_only_role} =
|
||||||
|
Authorization.create_role(%{
|
||||||
|
name: "Read Only Role",
|
||||||
|
description: "Read-only access",
|
||||||
|
permission_set_name: "read_only"
|
||||||
|
})
|
||||||
|
|
||||||
|
# Assign wrong role to system user
|
||||||
|
system_user
|
||||||
|
|> Ash.Changeset.for_update(:update, %{})
|
||||||
|
|> Ash.Changeset.manage_relationship(:role, read_only_role, type: :append_and_remove)
|
||||||
|
|> Ash.update!()
|
||||||
|
|
||||||
|
SystemActor.invalidate_cache()
|
||||||
|
|
||||||
|
# Should raise error because system user doesn't have admin role
|
||||||
|
assert_raise RuntimeError, ~r/System actor must have admin role/, fn ->
|
||||||
|
SystemActor.get_system_actor()
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test "raises error if system user has no role", %{system_user: system_user} do
|
||||||
|
# Remove role from system user
|
||||||
|
system_user
|
||||||
|
|> Ash.Changeset.for_update(:update, %{})
|
||||||
|
|> Ash.Changeset.manage_relationship(:role, nil, type: :append_and_remove)
|
||||||
|
|> Ash.update!()
|
||||||
|
|
||||||
|
SystemActor.invalidate_cache()
|
||||||
|
|
||||||
|
# Should raise error because system user has no role
|
||||||
|
assert_raise RuntimeError, ~r/System actor must have a role assigned/, fn ->
|
||||||
|
SystemActor.get_system_actor()
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
Loading…
Add table
Add a link
Reference in a new issue