From 2141a0ac45554c24c63e0e689df2d1a5c94e19cc Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 23 Aug 2026 14:18:58 -0400 Subject: [PATCH] Registration closes itself once the instance has an owner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Operator: *"registration should be open only for the first user and they get granted admin privileges. then registration is closed."* The old shape had a window in it. The first account was always allowed and became admin; every account after that was gated by `allow_registration` — which defaulted to ON. So the door stayed open between "my account exists" and "I remembered to turn it off in Settings", and on a public host that gap is the entire exposure: it starts the moment DNS resolves and lasts until someone remembers. Now the door shuts as a CONSEQUENCE of the admin account existing, in the same transaction that creates it. Not "defaults closed" — that would still need the first person to get in somehow. There is no window to remember, because there is no window. Re-opening it is a deliberate act in Settings → Access: turn it on, have the person register, turn it off. Crude, and it is the only mechanism there is — **there is no invite system**, not even a stub. That is real work (a token table, admin create/revoke, a redemption flow, expiry) and is filed as later work rather than smuggled into a release. An integration test covers it, because it is the interaction between two writes in one transaction: first register → 201 and `is_admin: true`; the setting is then false; a second register → 403; re-open deliberately and a third → 201, not admin. **This does not retroactively close an instance that already has users.** The close fires on first-account creation, so a server whose admin predates this keeps whatever the setting was — which was on. `docs/public-hosting.md` now says so explicitly, and step 1 of the checklist is "check" rather than "do" for exactly that reason. --- docs/public-hosting.md | 22 ++++++++++----- src/thoughtsync/auth.py | 16 ++++++++++- src/thoughtsync/settings.py | 3 +- tests/test_integration.py | 56 +++++++++++++++++++++++++++++++++++++ 4 files changed, 88 insertions(+), 9 deletions(-) diff --git a/docs/public-hosting.md b/docs/public-hosting.md index dcf6ea8..dda5973 100644 --- a/docs/public-hosting.md +++ b/docs/public-hosting.md @@ -9,14 +9,19 @@ you. ## Do these four things first -**1. Close registration.** `allow_registration` defaults to **on**, because the first -run of a fresh instance has to be able to create the admin account. It stays on -afterwards. Once your own account exists, turn it off in **Settings → Access → Allow -new registrations**, or the first stranger to find the hostname can open an account on -your server. +**1. Check registration is closed.** On a fresh instance this now takes care of +itself: the first account created becomes the admin *and* closes registration behind +it, so there is no window between "my account exists" and "I remembered to turn it +off". A brand-new instance is never locked out of itself, and never left open either. -The first account created is always the admin, regardless of this setting — so a -brand-new instance is never locked out of itself. +**Instances that predate this still need one manual flip.** The close fires when the +first account is created, so a server whose admin already existed keeps whatever +`allow_registration` was set to — which was **on** by default. Check **Settings → +Access → Allow new registrations** before exposing an instance you have been running +on a LAN. + +To let someone else in, turn it back on, have them register, turn it off. There is no +invite system yet, so that is the mechanism. **2. Terminate TLS in front of it, and forward the scheme.** The app marks the session cookie `Secure` and sends HSTS only when it can tell the request arrived over @@ -88,6 +93,9 @@ Know these before you decide who gets an account. - **No per-user storage quota.** Any account can upload attachments until the volume is full. `max_attachment_mb` caps a single file, not a total. - **No audit log.** Device tokens record `last_used_at`; sign-ins are not recorded. +- **No invites.** Adding a second person means re-opening registration while they + sign up, then closing it again. There is no per-person token, no expiry, and no + record of who invited whom. None of these are hard blockers for an instance whose accounts are you and people you know. They are the reason not to hand out open registration to strangers. diff --git a/src/thoughtsync/auth.py b/src/thoughtsync/auth.py index 7960d40..4018950 100644 --- a/src/thoughtsync/auth.py +++ b/src/thoughtsync/auth.py @@ -18,7 +18,7 @@ from .ratelimit import ( sign_in_by_address, ) from .security import dummy_verify, generate_token, hash_password, hash_token, verify_password -from .settings import get_setting +from .settings import get_setting, set_settings bp = Blueprint("auth", __name__, url_prefix="/api/auth") @@ -197,6 +197,20 @@ async def register(): is_admin=is_first, ) db.add(user) + if is_first: + # Registration CLOSES the moment the instance has an owner. + # + # Not "defaults closed" — that would still need the first person to get in + # somehow. Closed as a CONSEQUENCE of the admin account existing, which is + # the only formulation with no open window in it. Leaving the setting on + # meant the gap between "my account exists" and "I remembered to turn it + # off in Settings" was wide open, and on a public host that gap is the + # entire exposure — it starts the moment DNS resolves. + # + # An admin who wants a second person turns it back on in Settings → Access, + # adds them, and turns it off. Crude until invites exist, but it is a + # deliberate act rather than a default nobody chose. + await set_settings(db, {"allow_registration": False}) await db.commit() await db.refresh(user) session[SESSION_KEY] = str(user.id) diff --git a/src/thoughtsync/settings.py b/src/thoughtsync/settings.py index a39823d..439223b 100644 --- a/src/thoughtsync/settings.py +++ b/src/thoughtsync/settings.py @@ -32,7 +32,8 @@ REGISTRY: list[SettingDef] = [ "bool", True, "Allow new registrations", - "When off, only existing users can sign in. The first account is always allowed.", + "When off, only existing users can sign in. Closes itself once the first " + "account exists — turn it back on only while you're adding someone.", "Access", ), SettingDef( diff --git a/tests/test_integration.py b/tests/test_integration.py index 0a2db6e..2916a87 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -21,10 +21,13 @@ import pytest import pytest_asyncio from sqlalchemy import select, text +from thoughtsync import ratelimit +from thoughtsync.app import create_app from thoughtsync.db import dispose_engine, session_scope from thoughtsync.models.note import Note from thoughtsync.models.note_item import NoteItem from thoughtsync.models.user import User +from thoughtsync.settings import get_setting, set_settings from thoughtsync.notes.helpers import derive_display_title from thoughtsync.models.note_link_preview import NoteLinkPreview from thoughtsync.sync import _apply_note_items @@ -50,6 +53,19 @@ async def db(): await dispose_engine() +@pytest_asyncio.fixture +async def app_client(db): + """A test client against the real app, over the migrated database. + + The credential throttle is process-global and its counters outlive a single + test, so they are cleared here — otherwise a suite that registers a few times + starts handing out 429s for reasons that have nothing to do with the test. + """ + ratelimit.reset_all() + yield create_app().test_client() + ratelimit.reset_all() + + @pytest_asyncio.fixture async def owner(db): """A user to hang notes off — `notes.owner_id` is a real foreign key.""" @@ -281,3 +297,43 @@ async def test_detection_agrees_with_what_gets_stored(db, owner, monkeypatch): r for r in (await db.scalars(select(NoteLinkPreview.url).where(NoteLinkPreview.note_id == note.id))).all() } assert stored == set(detect_urls(body)) + + +async def test_registration_closes_itself_once_an_admin_exists(app_client, db): + """The gap this removes: registration was open between "my account exists" and + "I remembered to turn it off", and on a public host that gap starts at DNS. + + Runs against a real database because it is the interaction between two writes — + the user row and the settings row — inside one transaction. + """ + # The instance is empty (the fixture truncated it), so this is the first account: + # allowed unconditionally, and it becomes the admin. + first = await app_client.post( + "/api/auth/register", + json={"email": "owner@example.test", "password": "a-long-enough-password"}, + ) + assert first.status_code == 201 + assert (await first.get_json())["is_admin"] is True + + # …and the door shut behind it. + async with session_scope() as fresh: + assert await get_setting(fresh, "allow_registration") is False + + second = await app_client.post( + "/api/auth/register", + json={"email": "stranger@example.test", "password": "a-long-enough-password"}, + ) + assert second.status_code == 403 + + # Re-opening it deliberately still works — that is how a second person gets in + # until invites exist. + async with session_scope() as fresh: + await set_settings(fresh, {"allow_registration": True}) + await fresh.commit() + + third = await app_client.post( + "/api/auth/register", + json={"email": "invited@example.test", "password": "a-long-enough-password"}, + ) + assert third.status_code == 201 + assert (await third.get_json())["is_admin"] is False