Add schema and store for named access lists (#125) #136

Merged

qwc merged 1 commit from feature/named-access-lists into main

2026-08-30 18:26:15 +02:00

qwc commented

2026-08-30 18:11:17 +02:00

Owner

First of three PRs for #125. This one is inert by design — it adds the storage layer without changing any behaviour.

The shape

Per your note that a list must be able to be an LDAP group, or an LDAP group plus extra users, membership deliberately mirrors global_access rules:

access_lists          id, name (unique), description, created_at
access_list_members   list_id, subject_type, subject_identifier, role
                      UNIQUE(list_id, subject_type, subject_identifier)
projects              + access_list_id -> access_lists(id)

A list is therefore any mix of ldap_group, oauth2_group and user subjects, each with its own viewer/editor role. Same shape as a global access rule, so the resolution path #135 just introduced extends to lists rather than needing a second mechanism.

The project points at a list by id rather than by name, so renaming a list can't orphan projects.

Two decisions worth your eye

ON DELETE RESTRICT on projects.access_list_id. Deleting a list that projects still use would silently change who can reach them — the failure mode this whole issue family keeps producing. The database refuses it instead, and CountProjectsUsing lets the admin UI say "used by 3 projects" rather than surfacing a constraint error. FK enforcement is on in production (database.go:63), so this actually bites.

The down migration resets affected projects to custom, not private. Rolling back removes the pointer, and those projects have to land somewhere; custom is the narrower choice, so a rollback can only ever reduce access.

Tests

internal/store/sql/access_list_test.go covers the mixed-membership case from the issue (LDAP group + two named users), re-adding a member updating its role in place, unique list names, and that deleting an in-use list is refused and then succeeds once nothing points at it.

internal/database/access_lists_schema_test.go round-trips migration 012 down to 011 and back, asserting the list project falls back to custom on the way down.

I also had to add access_list_id to the explicit column lists in project.go — GetBySlug and friends select columns by name, so the round-trip test caught the pointer coming back nil until they were updated.

What's next

This PR — schema + store.
Checker + service — CanView / CanUpload / FilterAccessible resolve visibility = 'list' through list membership, reusing globalRole's grant-or-rule pattern from #135; projects.Service accepts the new visibility and requires a list id with it.
Admin UI + docs — manage lists and members, the visibility picker gains the named lists, and roles-permissions.md / manage-global-access.md document them.

Nothing between here and step 2 can set visibility = 'list' through the UI — admin.go:236 still coerces unknown values to custom — so merging this on its own is safe.

Assisted by Claude Opus 5.

First of three PRs for #125. This one is inert by design — it adds the storage layer without changing any behaviour. ## The shape Per your note that a list must be able to be an LDAP group, or an LDAP group plus extra users, membership deliberately mirrors `global_access` rules: ``` access_lists id, name (unique), description, created_at access_list_members list_id, subject_type, subject_identifier, role UNIQUE(list_id, subject_type, subject_identifier) projects + access_list_id -> access_lists(id) ``` A list is therefore any mix of `ldap_group`, `oauth2_group` and `user` subjects, each with its own viewer/editor role. Same shape as a global access rule, so the resolution path #135 just introduced extends to lists rather than needing a second mechanism. The project points at a list by id rather than by name, so renaming a list can't orphan projects. ## Two decisions worth your eye **`ON DELETE RESTRICT` on `projects.access_list_id`.** Deleting a list that projects still use would silently change who can reach them — the failure mode this whole issue family keeps producing. The database refuses it instead, and `CountProjectsUsing` lets the admin UI say "used by 3 projects" rather than surfacing a constraint error. FK enforcement is on in production (`database.go:63`), so this actually bites. **The down migration resets affected projects to `custom`, not `private`.** Rolling back removes the pointer, and those projects have to land somewhere; `custom` is the narrower choice, so a rollback can only ever reduce access. ## Tests `internal/store/sql/access_list_test.go` covers the mixed-membership case from the issue (LDAP group + two named users), re-adding a member updating its role in place, unique list names, and that deleting an in-use list is refused and then succeeds once nothing points at it. `internal/database/access_lists_schema_test.go` round-trips migration 012 down to 011 and back, asserting the `list` project falls back to `custom` on the way down. I also had to add `access_list_id` to the explicit column lists in `project.go` — `GetBySlug` and friends select columns by name, so the round-trip test caught the pointer coming back nil until they were updated. ## What's next 1. **This PR** — schema + store. 2. **Checker + service** — `CanView` / `CanUpload` / `FilterAccessible` resolve `visibility = 'list'` through list membership, reusing `globalRole`'s grant-or-rule pattern from #135; `projects.Service` accepts the new visibility and requires a list id with it. 3. **Admin UI + docs** — manage lists and members, the visibility picker gains the named lists, and `roles-permissions.md` / `manage-global-access.md` document them. Nothing between here and step 2 can set `visibility = 'list'` through the UI — `admin.go:236` still coerces unknown values to `custom` — so merging this on its own is safe. --- Assisted by Claude Opus 5.

qwc added 1 commit

2026-08-30 18:11:17 +02:00

Add schema and store for named access lists (#125 )

CI / test (pull_request) Successful in 1m17s

Details

CI / build (pull_request) Successful in 47s

Details

CI / docker (pull_request) Has been skipped

Details

76654d2100

First step of named access lists: a reusable set of subjects that a
project can point at instead of repeating per-project grants. Membership
mirrors global_access rules, so a list can be one LDAP group, or a group
plus individually named users.

Migration 012 adds access_lists and access_list_members across all three
dialects, plus projects.access_list_id. That FK is ON DELETE RESTRICT: a
list a project still points at cannot be deleted, since that would
silently change who can reach the project. Going back down resets such
projects to 'custom' rather than to a broader rule, so they fail closed.

Member subject types and roles are validated at store entry instead of
relying on the column default, and the visibility value 'list' exists as
a constant. Nothing serves it yet — the handler still coerces unknown
visibilities to 'custom' — so this change is inert until the checker
learns to resolve lists.

Assisted-by: Claude Opus 5

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Marcel M. Otte <marcel.otte@mmo.to>

qwc referenced this pull request

2026-08-30 18:16:11 +02:00

Resolve named access lists in the access checker (#125) #137

qwc merged commit 212d379ed7 into main

2026-08-30 18:26:15 +02:00

qwc deleted branch feature/named-access-lists

2026-08-30 18:26:16 +02:00

qwc referenced this pull request from a commit

2026-08-30 18:26:16 +02:00

Merge pull request 'Add schema and store for named access lists (#125)' (#136) from feature/named-access-lists into main

qwc referenced this pull request

2026-08-30 18:30:32 +02:00