Registration closes itself once the instance has an owner
CI & Build / Build now, or wait for Android? (push) Successful in 2s
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 6s
CI & Build / Python tests (push) Successful in 11s
CI & Build / integration (push) Successful in 17s
CI & Build / Build & push image (push) Successful in 28s
CI & Build / Build now, or wait for Android? (push) Successful in 2s
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 6s
CI & Build / Python tests (push) Successful in 11s
CI & Build / integration (push) Successful in 17s
CI & Build / Build & push image (push) Successful in 28s
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.
This commit is contained in:
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user