local-it/mitgliederverwaltung
local-it/mitgliederverwaltung
2
3
Fork 0
Code Issues 57 Pull requests 3 Projects 2 Releases 5 Packages 1 Activity Actions

Cleanup of the docs closes #507 #530

Merged
moritz merged 6 commits from issue/mitgliederverwaltung-507 into main 2026-06-15 22:00:43 +02:00
Conversation 0 Commits 6 Files changed 29 +348 -3836
8 changed files with 348 additions and 3836 deletions
Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 8d783276d0 - Show all commits

2
docs/admin-bootstrap-and-oidc-role-sync.md
View file

@ -26,7 +26,7 @@
### Seeds (Dev/Test)
- priv/repo/seeds.exs – Uses ADMIN_PASSWORD or ADMIN_PASSWORD_FILE when set; otherwise fallback "testpassword" only in dev/test.
- priv/repo/seeds_bootstrap.exs – Uses ADMIN_PASSWORD or ADMIN_PASSWORD_FILE when set; otherwise fallback "testpassword" only in dev/test.
## OIDC Role Sync (Part B)

45
docs/oidc-account-linking.md
View file

@ -102,12 +102,12 @@ Interactive UI for password verification and account linking.
**Changes**:
- `lib/mv_web/locale_controller.ex`: Sets locale cookie with `http_only` and `secure` flags
- `MvWeb.LocaleController`: Sets locale cookie with `http_only` and a config-driven `secure` flag
- `lib/mv_web/router.ex`: Reads locale from cookie if session empty
**Security Features**:
- `http_only: true` - Cookie not accessible via JavaScript (XSS protection)
- `secure: true` - Cookie only transmitted over HTTPS in production
- `secure: Application.get_env(:mv, :use_secure_cookies, false)` - the `secure` flag is config-driven (defaults to `false`; enabled in production) so the cookie is only transmitted over HTTPS in production
- `same_site: "Lax"` - CSRF protection
## Security Considerations
@ -139,47 +139,6 @@ Interactive UI for password verification and account linking.
- `Logger.warning` for failed authentication attempts
- `Logger.error` for system errors
## Usage Examples
### Scenario 1: New OIDC User
```elixir
# User signs in with OIDC for the first time
# → New user created with oidc_id
```
### Scenario 2: Existing OIDC User
```elixir
# User with oidc_id signs in via OIDC
# → Matched by oidc_id, email updated if changed
```
### Scenario 3: Password User + OIDC Login
```elixir
# User with password account tries OIDC login
# → PasswordVerificationRequired raised
# → Redirected to /auth/link-oidc-account
# → User enters password
# → Password verified and logged
# → oidc_id linked to account
# → Successful linking logged
# → Redirected to complete OIDC login
```
### Scenario 4: Passwordless User + OIDC Login
```elixir
# User without password (invited user) tries OIDC login
# → PasswordVerificationRequired raised
# → Redirected to /auth/link-oidc-account
# → System detects passwordless user
# → oidc_id automatically linked (no password prompt)
# → Auto-linking logged
# → Redirected to complete OIDC login
```
## API
### Custom Actions

65
docs/page-permission-route-coverage.md
View file

@ -19,9 +19,8 @@ This document lists all protected routes, which permission set may access them,
| `/users/:id/show/edit` | ✓ (own only) | ✓ (own only) | ✓ (own only) | ✓ |
| `/settings` | ✗ | ✗ | ✗ | ✓ |
| `/membership_fee_settings` | ✗ | ✗ | ✗ | ✓ |
| `/membership_fee_types` | ✗ | ✗ | ✗ | ✓ |
| `/membership_fee_types/new` | ✗ | ✗ | ✗ | ✓ |
| `/membership_fee_types/:id/edit` | ✗ | ✗ | ✗ | ✓ |
| `/membership_fee_settings/new_fee_type` | ✗ | ✗ | ✗ | ✓ |
| `/membership_fee_settings/:id/edit_fee_type` | ✗ | ✗ | ✗ | ✓ |
| `/groups` | ✗ | ✓ | ✓ | ✓ |
| `/groups/new` | ✗ | ✗ | ✗ | ✓ |
| `/groups/:slug` | ✗ | ✓ | ✓ | ✓ |
@ -31,10 +30,18 @@ This document lists all protected routes, which permission set may access them,
| `/admin/roles/new` | ✗ | ✗ | ✗ | ✓ |
| `/admin/roles/:id` | ✗ | ✗ | ✗ | ✓ |
| `/admin/roles/:id/edit` | ✗ | ✗ | ✗ | ✓ |
| `/join_requests` (Step 2) | ✗ | ✗ | ✓ | ✓ |
| `/join_requests/:id` (Step 2) | ✗ | ✗ | ✓ | ✓ |
| `/join_requests` | ✗ | ✗ | ✓ | ✓ |
| `/join_requests/:id` | ✗ | ✗ | ✓ | ✓ |
| `/admin/datafields` | ✗ | ✗ | ✗ | ✓ |
| `/admin/import` | ✗ | ✗ | ✗ | ✓ |
| `/admin/import/template/en` | ✗ | ✗ | ✗ | ✓ |
| `/admin/import/template/de` | ✗ | ✗ | ✗ | ✓ |
| `/members/export.csv` | ✗ | ✓ | ✓ | ✓ |
| `/members/export.pdf` | ✗ | ✗ | ✗ | ✓ |
**Note:** Permission sets define `/custom_field_values` and related paths, but there are no such routes in the router; those entries are for future use. Step 2 (Approval UI) adds `/join_requests` and `/join_requests/:id` for normal_user and admin; routes and permission set entries are not yet implemented; tests exist in `check_page_permission_test.exs` (describe "join_requests routes" and integration blocks).
**Note:** Permission sets define `/custom_field_values` and related paths, but there are no such routes in the router; those entries are for future use. The Approval UI routes `/join_requests` and `/join_requests/:id` are implemented and routed: `normal_user` lists them explicitly in its permission set, and `admin` reaches them through the `*` wildcard.
**Note on admin-only routes:** `/admin/datafields`, `/admin/import`, `/admin/import/template/en`, `/admin/import/template/de`, and `/members/export.pdf` are not listed explicitly in any permission set; only `admin` can reach them, via the `*` wildcard. `/members/export.csv` is additionally granted explicitly to `read_only` and `normal_user`.
## Public Paths (no permission check)
@ -46,50 +53,12 @@ The join confirmation route `GET /confirm_join/:token` is public (matched by `/c
## Test Coverage
**File:** `test/mv_web/plugs/check_page_permission_test.exs`
**File:** `test/mv_web/plugs/check_page_permission_test.exs` covers both unit tests (plug called directly with a mock conn) and full-router integration tests. The route→permission-set matrix above is the source of truth; each permission set (own_data/Mitglied, read_only, normal_user/Kassenwart, admin) is exercised there. Allowed routes return 200; denied routes return 302 → `/users/:id`. `GET /` redirects own_data to its profile. Unauthenticated access is denied and redirected to `/sign-in`; public paths (`/auth/sign-in`, `/register`) are allowed. Error cases (no role, invalid permission_set_name) deny.
### Unit tests (plug called directly with mock conn)
Two coverage notes:
- Static: own_data denied `/members`; read_only allowed `/members`; flash on denial.
- Dynamic: read_only allowed `/members/123`; normal_user allowed `/members/456/edit`; read_only denied `/members/123/edit`.
- read_only / normal_user: denied `/admin/roles`; read_only denied `/members/new`.
- Wildcard: admin allowed `/admin/roles`, `/members/999/edit`.
- Unauthenticated: nil user denied, redirect `/sign-in`.
- Public: unauthenticated allowed `/auth/sign-in`, `/register`.
- Error: no role, invalid permission_set_name → denied.
- **Join requests (Step 2):** normal_user and admin allowed `/join_requests`, `/join_requests/:id`; read_only and own_data denied. Tests fail (red) until routes and permission set are added.
### Integration tests (full router, Mitglied = own_data)
**Denied (Mitglied gets 302 → `/users/:id`):**
- `/members`, `/members/new`, `/users`, `/users/new`, `/settings`, `/membership_fee_settings`, `/membership_fee_types`, `/membership_fee_types/new`, `/groups`, `/groups/new`, `/admin/roles`, `/admin/roles/new`
- `/members/:id/edit`, `/members/:id/show/edit`, `/users/:id` (other user), `/users/:id/edit` (other), `/users/:id/show/edit` (other), `/membership_fee_types/:id/edit`, `/groups/:slug`, `/admin/roles/:id`, `/admin/roles/:id/edit`
**Allowed (Mitglied gets 200):**
- `/users/:id` (own profile), `/users/:id/edit`, `/users/:id/show/edit`
- `/members/:id`, `/members/:id/edit`, `/members/:id/show/edit` for linked member (plug unit tests; full-router tests for linked member skipped: session/LiveView constraints)
**Root:** `GET /` redirects Mitglied to profile (root not allowed for own_data).
All protected routes above are either covered by integration “denied” tests for Mitglied or by unit tests for the relevant permission set.
### Integration tests (full router, read_only = Vorstand/Buchhaltung)
**Allowed (200):** `/`, `/members`, `/members/:id`, `/users/:id` (own profile), `/users/:id/edit`, `/users/:id/show/edit`, `/groups`, `/groups/:slug`.
**Denied (302 → `/users/:id`):** `/members/new`, `/members/:id/edit`, `/members/:id/show/edit`, `/users`, `/users/new`, `/users/:id` (other user), `/settings`, `/membership_fee_settings`, `/membership_fee_types`, `/groups/new`, `/groups/:slug/edit`, `/admin/roles`, `/admin/roles/:id`.
### Integration tests (full router, normal_user = Kassenwart)
**Allowed (200):** `/`, `/members`, `/members/new`, `/members/:id`, `/members/:id/edit`, `/members/:id/show/edit`, `/users/:id` (own profile), `/users/:id/edit`, `/users/:id/show/edit`, `/groups`, `/groups/:slug`.
**Denied (302 → `/users/:id`):** `/users`, `/users/new`, `/users/:id` (other user), `/settings`, `/membership_fee_settings`, `/membership_fee_types`, `/groups/new`, `/groups/:slug/edit`, `/admin/roles`, `/admin/roles/:id`.
### Integration tests (full router, admin)
**Allowed (200):** All protected routes (sample covered: `/`, `/members`, `/users`, `/settings`, `/membership_fee_settings`, `/admin/roles`, `/members/:id`, `/admin/roles/:id`, `/groups/:slug`).
- **Linked-member routes** (`/members/:id*` for own_data) are covered by plug unit tests; full-router integration tests for the linked member are skipped due to session/LiveView constraints.
- **Join requests:** normal_user and admin are allowed `/join_requests` and `/join_requests/:id` (normal_user via its explicit permission-set pages, admin via the `*` wildcard); read_only and own_data are denied.
## Plug behaviour: reserved segments

287
docs/policy-bypass-vs-haspermission.md
View file

@ -1,69 +1,56 @@
# Policy Pattern: Bypass vs. HasPermission
**Date:** 2026-01-22
**Status:** Implemented and Tested
**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**:
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
1. **Bypass with `expr()` for READ** — handles list queries via `auto_filter`.
2. **HasPermission for UPDATE/CREATE/DESTROY** — uses scope from PermissionSets when a record is present.
This pattern ensures that the scope concept in PermissionSets is actually used and not redundant.
---
This ensures the scope concept in PermissionSets is actually used and not redundant.
## The Problem
### Initial Assumption (INCORRECT)
The initial assumption was that `HasPermission` returning `{:filter, expr(...)}` would automatically trigger Ash's `auto_filter` for list queries. It does not:
> "No separate Own Credentials Bypass needed, as all permission sets already have User read/update :own. HasPermission with scope :own handles this correctly."
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. List queries fail with empty results.
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)
# Ash.read(User, actor: user)
# Expected: [user] (filtered to own record)
# Actual: [] (empty list)
```
---
## The Solution
### Pattern: Bypass for READ, HasPermission for UPDATE
**User Resource Example:**
Bypass for READ, HasPermission for everything else:
```elixir
policies do
# Bypass for READ (handles list queries via auto_filter)
# AshAuthentication (registration/login)
bypass AshAuthentication.Checks.AshAuthenticationInteraction do
authorize_if always()
end
# 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)
# HasPermission — scope from PermissionSets, used when a record is present
policy action_type([:read, :create, :update, :destroy]) do
description "Check permissions from user's role and permission set"
authorize_if Mv.Authorization.Checks.HasPermission
@ -71,260 +58,100 @@ policies do
end
```
**Why This Works:**
Why it 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 |
| Operation | Record? | Method | Result |
|-----------|---------|--------|--------|
| READ (list) | No | `bypass` + `expr()` | Ash compiles expr to SQL WHERE → filtered list |
| READ (single) | Yes | `bypass` + `expr()` | Ash evaluates expr → true/false |
| UPDATE / CREATE / DESTROY | Yes (changeset) | `HasPermission` + scope | `strict_check` evaluates record → authorized |
**Important: UPDATE Strategy**
### UPDATE is controlled by PermissionSets, not hardcoded
UPDATE is **NOT** a hardcoded bypass. It is controlled by **PermissionSets**:
UPDATE is **not** a hardcoded bypass. All permission sets (`:own_data`, `:read_only`, `:normal_user`, `:admin`) explicitly grant `User.update :own`; `HasPermission` evaluates `scope :own` when a changeset with a record is present. Removing `User.update :own` from a set would remove credential-update ability for that set — intentional.
- 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
**Decision: `read_only` grants `User.update :own`** even though it is "read-only" for member data, so password changes work while member data stays read-only.
**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.
### No explicit `forbid_if always()`
---
We do **not** add a trailing `forbid_if always()`. Ash fails closed implicitly — it forbids when no policy authorizes. An explicit terminal forbid breaks tests because it forbids valid operations that earlier policies should authorize.
## 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:**
`scope :own` is used for operations where a record is present (UPDATE/CREATE/DESTROY), even though the bypass handles READ:
```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 ✅
%{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!
```
---
Proven by `test/mv/accounts/user_policies_test.exs` ("can update own email"): the update succeeds via `HasPermission` with `scope :own` (not via bypass).
## Consistency Across Resources
Both User and Member follow the same shape — bypass for READ, HasPermission for UPDATE/CREATE/DESTROY — differing only in the actor key and scope:
### 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
PermissionSets: `own_data` / `read_only` / `normal_user` use `scope :own` for read/update; `admin` uses `scope :all`.
### 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
---
PermissionSets: `own_data` uses `scope :linked` for read/update; `read_only` uses `scope :all` for read (no update); `normal_user` and `admin` use `scope :all`.
## Technical Deep Dive
### Why Does `expr()` in Bypass Work?
### Why `expr()` in bypass works
**Ash treats `expr()` natively in two contexts:**
Ash treats `expr()` natively in both contexts:
1. **strict_check** (single record):
- Ash evaluates the expression against the record
- Returns true/false based on match
- **strict_check** (single record): evaluates the expression against the record → true/false.
- **auto_filter** (list queries): compiles the expression to a SQL WHERE clause applied in the DB query.
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] ✅
# Ash.read(User, actor: user)
# Compiled SQL: SELECT * FROM users WHERE id = $1 → [user]
```
### Why Doesn't HasPermission Trigger auto_filter?
**HasPermission.strict_check logic:**
### Why HasPermission doesn't trigger auto_filter
```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
# No record (list query) → return false. Ash STOPS, 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)
- ✅ Tests use system_actor for authorization
**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
---
**Why return `false`, not `:unknown`?** We tested returning `:unknown`; Ash's policy evaluation still did **not** reliably call `auto_filter`. The `bypass` with `expr()` is the only consistent solution. (`has_permission_test.exs` accordingly expects `false`, not `:unknown`.)
## 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.**
---
If a future Ash version reliably calls `auto_filter` when `strict_check` returns `:unknown` or `{:filter, expr}`, the READ bypass could be removed and a single HasPermission policy kept for all operations (with tests updated). **This workaround was first identified under Ash 3.13.x and is still required as of the Ash version pinned in `mix.lock`; the bypass pattern remains 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`
- Ash policies: <https://hexdocs.pm/ash/policies.html>
- Implementation: see the `policies do` block in `Mv.Accounts.User` (`lib/accounts/user.ex`)
- Tests: `test/mv/accounts/user_policies_test.exs`, `test/mv/authorization/checks/has_permission_test.exs`
- Architecture: `docs/roles-and-permissions-architecture.md`
- Permission sets: `lib/mv/authorization/permission_sets.ex`

1664
docs/roles-and-permissions-architecture.md
View file

File diff suppressed because it is too large Load diff

1684
docs/roles-and-permissions-implementation-plan.md
View file

File diff suppressed because it is too large Load diff

168
docs/roles-and-permissions-overview.md
View file

@ -63,20 +63,7 @@ During the design phase, we evaluated multiple implementation approaches to find
### Approach 1: JSONB in Roles Table
Store all permissions as a single JSONB column directly in the roles table.
**Advantages:**
- Simplest database schema (single table)
- Very flexible structure
- No additional tables needed
- Fast to implement
**Disadvantages:**
- Poor queryability (can't efficiently filter by specific permissions)
- No referential integrity
- Difficult to validate structure
- Hard to audit permission changes
- Can't leverage database indexes effectively
Store all permissions as a single JSONB column directly in the roles table. Simplest schema (single table), flexible, fast to implement — but poor queryability (can't filter by specific permissions), no referential integrity, hard to validate/audit, can't use indexes.
**Verdict:** Rejected - Poor queryability makes it unsuitable for complex permission logic.
@ -84,22 +71,7 @@ Store all permissions as a single JSONB column directly in the roles table.
### Approach 2: Normalized Database Tables
Separate tables for `permission_sets`, `permission_set_resources`, `permission_set_pages` with full normalization.
**Advantages:**
- Fully queryable with SQL
- Runtime configurable permissions
- Strong referential integrity
- Easy to audit changes
- Can index for performance
**Disadvantages:**
- Complex database schema (4+ tables)
- DB queries required for every permission check
- Requires ETS cache for performance
- Needs admin UI for permission management
- Longer implementation time (4-5 weeks)
- Overkill for fixed set of 4 permission sets
Separate tables for `permission_sets`, `permission_set_resources`, `permission_set_pages` with full normalization. Fully queryable, runtime-configurable, strong referential integrity, auditable, indexable — but complex schema (4+ tables), a DB query per check, needs ETS cache + admin UI, 4-5 weeks, overkill for 4 fixed sets.
**Verdict:** Deferred to Phase 3 - Excellent for runtime configuration but too complex for MVP.
@ -107,20 +79,7 @@ Separate tables for `permission_sets`, `permission_set_resources`, `permission_s
### Approach 3: Custom Authorizer
Implement a custom Ash Authorizer from scratch instead of using Ash Policies.
**Advantages:**
- Complete control over authorization logic
- Can implement any custom behavior
- Not constrained by Ash Policy DSL
**Disadvantages:**
- Significantly more code to write and maintain
- Loses benefits of Ash's declarative policies
- Harder to test than built-in policy system
- Mixes declarative and imperative approaches
- Must reimplement filter generation for queries
- Higher bug risk
Implement a custom Ash Authorizer from scratch instead of using Ash Policies. Full control over logic — but significantly more code, loses Ash's declarative policies (must reimplement query filter generation), harder to test, mixes declarative/imperative, higher bug risk.
**Verdict:** Rejected - Too much custom code, reduces maintainability and loses Ash ecosystem benefits.
@ -128,21 +87,7 @@ Implement a custom Ash Authorizer from scratch instead of using Ash Policies.
### Approach 4: Simple Role Enum
Add a simple `:role` enum field directly on User resource with hardcoded checks in each policy.
**Advantages:**
- Very simple to implement (< 1 week)
- No extra tables needed
- Fast performance
- Easy to understand
**Disadvantages:**
- No separation between roles and permissions
- Can't add new roles without code changes
- No dynamic permission configuration
- Not extensible to field-level permissions
- Violates separation of concerns (role = job function, not permission set)
- Difficult to maintain as requirements grow
Add a `:role` enum field directly on User with hardcoded checks in each policy. Very simple (< 1 week), no extra tables, fast — but no separation of role (job function) from permission set, can't add roles without code changes, no dynamic config, not extensible to field-level, hard to maintain as requirements grow.
**Verdict:** Rejected - Too inflexible, doesn't meet requirement for configurable permissions and role separation.
@ -150,33 +95,11 @@ Add a simple `:role` enum field directly on User resource with hardcoded checks
### Approach 5: Hardcoded Permissions with Migration Path (SELECTED for MVP)
Permission Sets hardcoded in Elixir module, only Roles table in database.
Permission Sets hardcoded in Elixir module, only Roles table in database. Fast (2-3 weeks vs 4-5), maximum performance (zero DB queries, < 1μs), pure-function testing, Git-reviewable permissions, no data migration, keeps role/permission-set separation, clear Phase 3 upgrade path. Trade-offs: permissions not editable at runtime (only role assignment), new permissions need a code deploy, unsuitable if permissions change > 1x/week, limited to the 4 predefined sets.
**Advantages:**
- Fast implementation (2-3 weeks vs 4-5 weeks)
- Maximum performance (zero DB queries, < 1 microsecond)
- Simple to test (pure functions)
- Code-reviewable permissions (visible in Git)
- No migration needed for existing data
- Clearly defined 4 permission sets as required
- Clear migration path to database-backed solution (Phase 3)
- Maintains separation of roles and permission sets
**Why Selected:** MVP requires 4 fixed sets (not custom ones), no stated need for runtime permission editing, performance is critical, fast time-to-market, and a clear upgrade path exists when runtime config becomes necessary.
**Disadvantages:**
- Permissions not editable at runtime (only role assignment possible)
- New permissions require code deployment
- Not suitable if permissions change frequently (> 1x/week)
- Limited to the 4 predefined permission sets
**Why Selected:**
- MVP requirement is for 4 fixed permission sets (not custom ones)
- No stated requirement for runtime permission editing
- Performance is critical for authorization checks
- Fast time-to-market (2-3 weeks)
- Clear upgrade path when runtime configuration becomes necessary
**Migration Path:**
When runtime permission editing becomes a business requirement, migrate to Approach 2 (normalized DB tables) without changing the public API of the PermissionSets module.
**Migration Path:** When runtime permission editing becomes a business requirement, migrate to Approach 2 (normalized DB tables) without changing the public API of the PermissionSets module.
---
@ -201,7 +124,7 @@ When runtime permission editing becomes a business requirement, migrate to Appro
**Resource Level (MVP):**
- Controls create, read, update, destroy actions on resources
- Resources: Member, User, CustomFieldValue, CustomField, Role
- Resources: Member, User, CustomFieldValue, CustomField, Role, Group, MemberGroup, MembershipFeeType, MembershipFeeCycle, JoinRequest
**Page Level (MVP):**
- Controls access to LiveView pages
@ -214,7 +137,7 @@ When runtime permission editing becomes a business requirement, migrate to Appro
### Special Cases
1. **Own Credentials:** Users can always edit their own email and password
2. **Linked Member Email:** Only admins can edit email of members linked to users
2. **Linked Member Email:** Only administrators or the linked user themselves can change the email of a member linked to a user
3. **User-Member Linking:** Only admins can link/unlink users to members (except self-service creation)
---
@ -331,46 +254,39 @@ Users need to create member profiles for themselves (self-service), but only adm
- Unlink members from users
- Create members pre-linked to arbitrary users
### Selected Approach: Separate Ash Actions
### Selected Approach: Admin-Only `:user` Argument
Instead of complex field-level validation, we use action-based authorization.
Linking is **not** modelled as separate per-operation actions. The Member resource has a single
`create_member` and a single `update_member` action; linking and unlinking happen through an
optional **`:user` argument** on those actions. `user_id` is deliberately not accepted, so the
foreign key cannot be set directly.
### Actions on Member Resource
### How Linking Works on the Member Resource
**1. create_member_for_self** (All authenticated users)
- Automatically sets user_id = actor.id
- User cannot specify different user_id
- UI: "Create My Profile" button
**`create_member` / `update_member`** (the only Member write actions)
- The optional `:user` argument drives the relationship via `manage_relationship`.
- On update, `on_missing: :ignore` means omitting `:user` leaves the link unchanged
(no "unlink by omission"); unlink is explicit (`user: nil`).
- The policy check `ForbidMemberUserLinkUnlessAdmin` forbids the action for non-admins whenever the
`:user` argument is present (any value), so only admins may set or change the link.
- Non-admins can still create/update members as long as they do not pass `:user`.
**2. create_member** (Admin only)
- Can set user_id to any user or leave unlinked
- Full flexibility for admin
- UI: Admin member management form
**Self-service** ("a user creates a member linked to themselves") is handled on the **User** side:
the admin-only `update_user` action takes a `:member` argument for link/unlink, and the UI exposes
the linking controls only to admins.
**3. link_member_to_user** (Admin only)
- Updates existing member to set user_id
- Connects unlinked member to user account
### Why This Design?
**4. unlink_member_from_user** (Admin only)
- Sets user_id to nil
- Disconnects member from user account
**Single write path:** one create and one update action to reason about, instead of a fan-out of
`link_*`/`unlink_*` actions.
**5. update** (Permission-based)
- Normal updates (name, address, etc.)
- user_id NOT in accept list (prevents manipulation)
- Available to users with Member.update permission
**Centralized rule:** the admin-only constraint lives in one reusable policy check
(`ForbidMemberUserLinkUnlessAdmin`).
### Why Separate Actions?
**Server-Side Security:** `user_id` is never accepted directly, so it cannot be mass-assigned —
only argument-driven relationship management can change it.
**Explicit Semantics:** Each action has clear, single purpose
**Server-Side Security:** user_id set by server, not client input
**Better UX:** Different UI flows for different use cases
**Simple Policies:** Authorization at action level, not field level
**Easy Testing:** Each action independently testable
**Better UX:** distinct UI flows for self-service vs. admin linking.
---
@ -486,23 +402,7 @@ Use Custom Validations
**[roles-and-permissions-architecture.md](./roles-and-permissions-architecture.md):** Complete technical specification with code examples
**[roles-and-permissions-implementation-plan.md](./roles-and-permissions-implementation-plan.md):** Detailed implementation plan with TDD approach
**[roles-and-permissions-implementation-plan.md](./roles-and-permissions-implementation-plan.md):** Historical record of how the MVP was built (PR #346/#345)
**[CODE_GUIDELINES.md](../CODE_GUIDELINES.md):** Project coding standards
---
## Summary
The selected architecture uses **hardcoded Permission Sets in Elixir** for the MVP, providing:
- **Speed:** 2-3 weeks implementation vs 4-5 weeks
- **Performance:** Zero database queries for authorization
- **Clarity:** Permissions in Git, reviewable and testable
- **Flexibility:** Clear migration path to database-backed system
**User-Member linking** uses **separate Ash Actions** for clarity and security.
**Field-level permissions** have a **defined strategy** (Calculations + Validations) for Phase 2 implementation.
The approach balances pragmatism for MVP delivery with extensibility for future requirements.

269
docs/user-resource-policies-implementation-summary.md
View file

@ -1,269 +0,0 @@
# 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. 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
# 3. HasPermission for all operations (uses scope from PermissionSets)
policy action_type([:read, :create, :update, :destroy]) do
description "Check permissions from user's role and permission set"
authorize_if Mv.Authorization.Checks.HasPermission
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)
- ✅ Tests use system_actor for authorization
---
## 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
- ✅ All tests explicitly use system_actor for authorization
---
## 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** 🎉