fix: make only the very first user admin, add REGISTRATION_ENABLED #38

Merged

jfro merged 5 commits from jtk/registration-first-admin into main 2026-10-18 18:02:34 +00:00

Conversation 0 Commits 5 Files changed 13 +371 -32

jfro commented 2026-10-18 17:22:25 +00:00

Owner

Copy link

Closes #12.

Problems (from the audit):

1. ensure_first_user_is_admin/0 ran after every registration and re-promoted the oldest user to admin, even if an admin had demoted them on purpose.
2. The check ran after the insert, outside any transaction, so two registrations racing on a fresh instance could both end up admin, or neither.
3. Registration was only controllable from the admin UI, and it's open by default.

Fix:

• New private Accounts.insert_user/1, used by register_user/1, register_user_with_password/1 and create_user_from_changeset/1 (OIDC). It takes a Postgres advisory transaction lock, and sets role: "admin" only if the users table is empty. The first user is logged (First user ID … made admin).
• ensure_first_user_is_admin/0 and its two calls in the registration controller are removed.
• REGISTRATION_ENABLED=true|false (also 1/0, yes/no, on/off; anything else fails at boot) fixes registration on or off. When it's unset, the admin setting decides, as before. On Admin → Users the toggle is disabled with a note when the env var is set, and the event handler refuses it too. Added to .env.sample.
• config/test.exs sets first_user_admin: false so fixtures stay plain users and async tests don't queue on the lock; the new first-user tests turn it back on.

Default when nothing is set (Jeremy's call, Oct 18): open only until the first user exists. registration_enabled?/0 now resolves: REGISTRATION_ENABLED set → env wins; admin setting saved → setting wins; neither → open only while the users table is empty. A new migration drops the registration_enabled = true row that the original settings migration seeded, but only when it's untouched (value = 'true' AND inserted_at = updated_at), so the default applies; rows an admin saved are kept. Admin → Users shows "Not set yet: by default sign-ups are open only until the first account exists" until the setting is saved. .env.sample and config/runtime.exs comments say the same.

Also (from Nye's re-review): OIDC sign-ups are always created as "user"; insert_user/1 is the only place that grants first-user admin, inside the lock, and forces "user" for everyone else. create_user_by_admin bypasses it so admin-chosen roles are kept.

Release note: Registration now defaults to open only until the first account exists. Existing instances that never saved the registration setting will close registration after upgrading; set REGISTRATION_ENABLED=true or turn it on in Admin → Users to keep it open. OIDC sign-ups still ignore this setting (tracked in #42).

Tests: first_user_admin_test.exs (first user admin / second not, email path, demoted first user stays demoted, OIDC changeset path, env override vs setting) and RegistrationEnvTest (no users → open; one user, nothing set → closed; saved true → open; saved false → closed even with no users; env true/false override both), admin_users_live_test.exs (default hint, toggle unlocked / locked), registration controller (redirects once a user exists). mix precommit: 531 passed.

Closes #12. **Problems (from the audit):** 1. `ensure_first_user_is_admin/0` ran after every registration and re-promoted the oldest user to admin, even if an admin had demoted them on purpose. 2. The check ran after the insert, outside any transaction, so two registrations racing on a fresh instance could both end up admin, or neither. 3. Registration was only controllable from the admin UI, and it's open by default. **Fix:** - New private `Accounts.insert_user/1`, used by `register_user/1`, `register_user_with_password/1` and `create_user_from_changeset/1` (OIDC). It takes a Postgres advisory transaction lock, and sets `role: "admin"` only if the users table is empty. The first user is logged (`First user ID … made admin`). - `ensure_first_user_is_admin/0` and its two calls in the registration controller are removed. - `REGISTRATION_ENABLED=true|false` (also 1/0, yes/no, on/off; anything else fails at boot) fixes registration on or off. When it's unset, the admin setting decides, as before. On Admin → Users the toggle is disabled with a note when the env var is set, and the event handler refuses it too. Added to `.env.sample`. - `config/test.exs` sets `first_user_admin: false` so fixtures stay plain users and async tests don't queue on the lock; the new first-user tests turn it back on. **Default when nothing is set (Jeremy's call, Oct 18): open only until the first user exists.** `registration_enabled?/0` now resolves: `REGISTRATION_ENABLED` set → env wins; admin setting saved → setting wins; neither → open only while the users table is empty. A new migration drops the `registration_enabled = true` row that the original settings migration seeded, but only when it's untouched (`value = 'true' AND inserted_at = updated_at`), so the default applies; rows an admin saved are kept. Admin → Users shows "Not set yet: by default sign-ups are open only until the first account exists" until the setting is saved. `.env.sample` and `config/runtime.exs` comments say the same. **Also (from Nye's re-review):** OIDC sign-ups are always created as `"user"`; `insert_user/1` is the only place that grants first-user admin, inside the lock, and forces `"user"` for everyone else. `create_user_by_admin` bypasses it so admin-chosen roles are kept. **Release note:** Registration now defaults to open only until the first account exists. Existing instances that never saved the registration setting will close registration after upgrading; set `REGISTRATION_ENABLED=true` or turn it on in Admin → Users to keep it open. OIDC sign-ups still ignore this setting (tracked in #42). **Tests:** `first_user_admin_test.exs` (first user admin / second not, email path, demoted first user stays demoted, OIDC changeset path, env override vs setting) and `RegistrationEnvTest` (no users → open; one user, nothing set → closed; saved true → open; saved false → closed even with no users; env true/false override both), `admin_users_live_test.exs` (default hint, toggle unlocked / locked), registration controller (redirects once a user exists). `mix precommit`: 531 passed.

jfro added 1 commit 2026-10-18 17:22:25 +00:00

fix: make only the very first user admin, add REGISTRATION_ENABLED ... 4e5f4c4da2

The first user is now made admin inside the insert, under an advisory lock,
only when the users table is empty. The old ensure_first_user_is_admin ran
after every registration and re-promoted the oldest user if they had been
demoted. REGISTRATION_ENABLED=true|false fixes registration on or off; when
unset, the admin setting decides (and the toggle shows when it's locked).

Refs #12

jfro added 2 commits 2026-10-18 17:46:28 +00:00

Merge remote-tracking branch 'origin/main' into jtk/registration-first-admin bbe12cd505

fix: decide first-user admin for OIDC sign-ups inside the lock ... 7ae90bc9d3

create_oidc_user no longer computes the role itself; insert_user is the
only place that grants first-user admin, and every other self-registration
is forced to role user.

jfro referenced this pull request 2026-10-18 17:46:55 +00:00

[low] OIDC: email_verified trusted for the mail fallback; raw email in logs; OIDC ignores REGISTRATION_ENABLED #42

jfro added 1 commit 2026-10-18 17:53:35 +00:00

Merge branch 'main' into jtk/registration-first-admin 70b4dfba89

jfro added 1 commit 2026-10-18 17:59:14 +00:00

fix: open registration only until the first user exists by default ... 0c8e3bb19c

With neither REGISTRATION_ENABLED nor a saved admin setting, registration is
open only while there are no users, so a fresh install can create its first
admin and then closes. Drop the seeded registration_enabled=true row when no
admin ever changed it, so existing instances get the new default.

jfro reviewed 2026-10-18 18:02:30 +00:00

jfro left a comment

Copy link

Review by Nye: APPROVED (at 0c8e3bb)

Precedence is right: env REGISTRATION_ENABLED → saved setting (parse_value) → Accounts.first_user?(). OIDC never calls registration_enabled?, so OIDC installs are unaffected (release note says so, #42).

Race: two concurrent sign-ups on an empty install both pass the open check, but insert_user's advisory lock makes only the first admin and forces 'user' on the second, so the worst case is an extra plain user, never two admins. Acceptable.

Migration: the 2025 seed inserted both timestamps with NOW() in one statement, so inserted_at = updated_at holds for untouched rows; any admin toggle changes value and so bumps updated_at, so saved rows survive; down re-inserts true with ON CONFLICT DO NOTHING.

Release note is accurate (existing instances that never saved the setting close after upgrade; set REGISTRATION_ENABLED=true or toggle in Admin → Users). Tests cover all 6 precedence cases plus UI hint and the /users/register redirect. CI green on 0c8e3bb, mergeable, no conflicts with main e473a95.

Non-blocking nit (no change needed for v0.6.0): with nothing saved and users present, the layout's registration_enabled? does a users COUNT per page render; trivial cost, could be cached later.

**Review by Nye: APPROVED** (at 0c8e3bb) Precedence is right: env REGISTRATION_ENABLED → saved setting (parse_value) → Accounts.first_user?(). OIDC never calls registration_enabled?, so OIDC installs are unaffected (release note says so, #42). Race: two concurrent sign-ups on an empty install both pass the open check, but insert_user's advisory lock makes only the first admin and forces 'user' on the second, so the worst case is an extra plain user, never two admins. Acceptable. Migration: the 2025 seed inserted both timestamps with NOW() in one statement, so inserted_at = updated_at holds for untouched rows; any admin toggle changes value and so bumps updated_at, so saved rows survive; down re-inserts true with ON CONFLICT DO NOTHING. Release note is accurate (existing instances that never saved the setting close after upgrade; set REGISTRATION_ENABLED=true or toggle in Admin → Users). Tests cover all 6 precedence cases plus UI hint and the /users/register redirect. CI green on 0c8e3bb, mergeable, no conflicts with main e473a95. Non-blocking nit (no change needed for v0.6.0): with nothing saved and users present, the layout's registration_enabled? does a users COUNT per page render; trivial cost, could be cached later.

jfro merged commit df180a7f6f into main 2026-10-18 18:02:34 +00:00

jfro deleted branch jtk/registration-first-admin 2026-10-18 18:02:34 +00:00

jfro referenced this pull request from a commit 2026-10-18 18:02:35 +00:00

fix: make only the very first user admin, add REGISTRATION_ENABLED (#38)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bug

  

critical

  

ebook-branch

  

high

  

low

  

main

  

medium

  

release-blocker

  

security

No labels

bug

critical

ebook-branch

high

low

main

medium

release-blocker

security

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

jfro/fuzzy_catalog!38

No description provided.