diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md index fb7bc23..5bee497 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -688,6 +688,44 @@ end - **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:** diff --git a/docs/roles-and-permissions-architecture.md b/docs/roles-and-permissions-architecture.md index d97145a..bc1b75c 100644 --- a/docs/roles-and-permissions-architecture.md +++ b/docs/roles-and-permissions-architecture.md @@ -2623,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) -**Last Updated:** 2026-01-13 +**Last Updated:** 2026-01-23 **Implementation Status:** ✅ Complete (2026-01-08) **Status:** Ready for Implementation @@ -2639,6 +2872,9 @@ iex> MvWeb.Authorization.can_access_page?(user, "/members/new") - Added comprehensive security section - Enhanced edge case documentation +**Changes from V2.0:** +- Added "Authorization Bootstrap Patterns" section explaining NoActor, system_actor, and authorize?: false + --- **End of Architecture Document**