diff --git a/README.md b/README.md index 499495a..33a5f99 100644 --- a/README.md +++ b/README.md @@ -229,6 +229,8 @@ Outside `development` and `test`, the backend requires: - `AI_ENCRYPTION_KEY` containing a valid Fernet key; - independent network egress controls for AI providers. +Registration is gated by an admin-managed email allowlist, in every environment. On a genuinely fresh instance — an empty database, nobody pre-approved — the first account anyone registers (password or OAuth) is auto-approved automatically and becomes that instance's owner; this is what lets a self-hoster actually use their own deployment. Every registration after that first one needs an existing account holder to approve the email first, with `apps/backend/scripts/approve_email.py`. + Do not expose the development configuration directly to the public internet. Review [SECURITY.md](SECURITY.md) and the [AI provider egress policy](docs/security/AI_PROVIDER_EGRESS.md) before any shared deployment. Report vulnerabilities privately—never in a public issue. ## Documentation and contributing diff --git a/apps/backend/app/auth/service.py b/apps/backend/app/auth/service.py index 4394967..8cdf00f 100644 --- a/apps/backend/app/auth/service.py +++ b/apps/backend/app/auth/service.py @@ -2,7 +2,7 @@ from datetime import UTC, datetime, timedelta from uuid import uuid4 -from sqlalchemy import select, update +from sqlalchemy import func, select, update from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session @@ -18,7 +18,7 @@ from app.core.exceptions import ConflictServiceError, UnauthorizedError, ValidationServiceError from app.models.approved_email import ApprovedEmail from app.models.refresh_token import RefreshToken -from app.models.user import User +from app.models.user import SEED_USER_ID, User logger = logging.getLogger(__name__) @@ -73,9 +73,101 @@ def _ensure_email_available(self, normalized_email: str) -> None: def _require_approval(self, normalized_email: str) -> ApprovedEmail: approval = self.db.scalars(select(ApprovedEmail).where(ApprovedEmail.email == normalized_email)).first() - if approval is None: - raise ValidationServiceError(EMAIL_NOT_APPROVED) - return approval + if approval is not None: + return approval + + # Checked before the #388 bootstrap below, not after: in development + # every registration is already frictionless regardless of ordinal + # position, so the dev-bypass reason is the more specific and + # accurate one to attribute here. Bootstrap is the fallback for + # every environment this local-only rule doesn't cover. + if self.settings.app_env == "development": + # #384: local development must stay exactly as frictionless as it + # was before the allowlist existed -- restoring that means an + # email that was never explicitly approved is auto-approved here + # instead of rejected, so the rest of this method's caller + # (_create_approved_user) still has a real, persisted + # ApprovedEmail row to stamp used_at/used_by_user_id onto. Every + # other behavior (uniqueness, the audit trail, the OAuth path) + # stays identical to the real approved case -- this only ever + # changes what happens when no admin has approved the address + # yet, and only in development. + # + # Deliberately `development` only, not the broader dev/test + # leniency pairing used elsewhere in Settings + # (AUTH_SECRET_KEY/AI_ENCRYPTION_KEY): the backend test suite + # runs under this same default app_env with nothing overriding + # it, and its own allowlist-rejection tests need this bypass to + # NOT apply to them (see tests/conftest.py's `client` fixture, + # which sets APP_ENV=test specifically so this distinction is + # real rather than accidental). `test` is intentionally excluded. + auto_approval = ApprovedEmail( + id=str(uuid4()), + email=normalized_email, + note="Auto-approved: local development (#384). Never happens outside APP_ENV=development.", + added_by="dev-bypass", + ) + self.db.add(auto_approval) + return auto_approval + + bootstrap_approval = self._first_user_bootstrap(normalized_email) + if bootstrap_approval is not None: + return bootstrap_approval + + raise ValidationServiceError(EMAIL_NOT_APPROVED) + + def _first_user_bootstrap(self, normalized_email: str) -> ApprovedEmail | None: + """#388: the first real account ever registered on a fresh instance + becomes its owner. Checked as the fallback for every environment the + #384 dev-only bypass above doesn't already cover unconditionally -- + so in practice this is what makes registration possible at all in + `staging`/`production`/any other real deployment. + + Without this, a genuine self-hoster running their own copy of PARTHA + in production mode has no way to ever register at all: nobody is + pre-approved on a fresh database except the hardcoded product-owner + row seeded by the #374 migration, which is this project's own owner, + not theirs. This is the self-hoster claiming their own instance, the + same bootstrap pattern used by most self-hosted software (the first + person to reach the setup wizard becomes the admin). + + "First" is measured by the `users` table being otherwise empty, + excluding the permanent system placeholder row every database gets + from the 0002 migration (SEED_USER_ID) -- that row is not a real + account and must never itself count as "already have an owner". + + Every registration after the first real one goes through the normal + allowlist exactly as before; this only ever changes what happens + once, the very first time. + + Known, accepted limitation: this check and the eventual `User` + insert are not atomic with each other (the insert happens later, in + _create_approved_user). Two concurrent *first-ever* registrations on + the same fresh database could both observe zero real users and both + be auto-approved as owner. The window only exists for the single + moment between a fresh instance's first boot and its first + successful registration, is closed permanently the instant one + registration commits, and does not reopen or weaken the allowlist + for anyone after that. Closing it completely would need a + dedicated, atomically-claimed mutex (e.g. a single-row table claimed + via a unique-constraint insert, the same pattern _create_approved_user + already uses for email-uniqueness races) -- deliberately not added + here; flagged instead of guessed at, since it's a real design + tradeoff between full correctness and a new migration/table for a + narrow, single-operator bootstrap scenario. + """ + real_user_count = self.db.scalar(select(func.count()).select_from(User).where(User.id != SEED_USER_ID)) + if real_user_count: + return None + + bootstrap_approval = ApprovedEmail( + id=str(uuid4()), + email=normalized_email, + note="Auto-approved: first user on this instance becomes its owner (#388).", + added_by="first-user-bootstrap", + ) + self.db.add(bootstrap_approval) + return bootstrap_approval def _create_approved_user(self, user: User, approval: ApprovedEmail) -> tuple[User, str, str]: self.db.add(user) diff --git a/apps/backend/tests/conftest.py b/apps/backend/tests/conftest.py index 7044b97..6021fe2 100644 --- a/apps/backend/tests/conftest.py +++ b/apps/backend/tests/conftest.py @@ -79,6 +79,12 @@ def client(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Generator[TestCli monkeypatch.setenv("STORAGE_PATH", str(storage_path)) monkeypatch.setenv("AUTO_CREATE_TABLES", "true") monkeypatch.setenv("CORS_ORIGINS", "http://testserver") + # Deliberately "test", not the default "development": AuthService's + # dev-only allowlist bypass (#384) is scoped to app_env == "development" + # specifically so it never applies to the test suite -- without this, + # every test asserting an allowlist rejection would silently start + # passing for the wrong reason (auto-approval, not a real check). + monkeypatch.setenv("APP_ENV", "test") # Tests drive AnalysisWorker.run_once() deterministically; the background # daemon thread would otherwise race the queue non-deterministically (#93). monkeypatch.setenv("ANALYSIS_WORKER_AUTOSTART", "false") diff --git a/apps/backend/tests/test_approved_emails_migration.py b/apps/backend/tests/test_approved_emails_migration.py index 778b30e..4909ab9 100644 --- a/apps/backend/tests/test_approved_emails_migration.py +++ b/apps/backend/tests/test_approved_emails_migration.py @@ -58,6 +58,10 @@ def approved_emails_migration_db(tmp_path, monkeypatch): database_url = _database_url(tmp_path) monkeypatch.setenv("DATABASE_URL", database_url) monkeypatch.setenv("CORS_ORIGINS", "http://testserver") + # See tests/conftest.py's `client` fixture: "test", not the default + # "development", so AuthService's dev-only allowlist bypass (#384) never + # applies here either. + monkeypatch.setenv("APP_ENV", "test") from app.core import config config.get_settings.cache_clear() diff --git a/apps/backend/tests/test_auth.py b/apps/backend/tests/test_auth.py index f6fadb2..42695d3 100644 --- a/apps/backend/tests/test_auth.py +++ b/apps/backend/tests/test_auth.py @@ -73,13 +73,159 @@ def test_register_with_an_approved_email_succeeds(client): def test_register_rejects_an_email_that_was_never_approved(client): - response = _register(client, skip_approval=True) + # A baseline first user is registered here so #388's first-user + # bootstrap (see below) has already closed before this assertion runs -- + # otherwise this registration would BE the first-ever one on a fresh + # database and succeed via bootstrap instead of exercising the plain + # rejection this test is actually about. + assert _register(client).status_code == 201 + + response = _register(client, email="bob@example.com", skip_approval=True) error = assert_error_response(response, 422, "validation_error") assert "hasn't been approved" in error.message assert "waitlist" in error.message.lower() +def test_first_ever_registration_becomes_the_owner_without_approval(client): + """#388: the very first account on a fresh instance is auto-approved + without needing anyone to have pre-approved it -- otherwise a genuine + self-hoster running their own copy of PARTHA has no way to ever + register at all, since nobody is pre-approved on a fresh database + except this project's own owner (seeded by the #374 migration, not + relevant to a self-hoster's own instance). + + The `client` fixture itself runs as APP_ENV=test (see conftest.py) -- + a non-development environment -- so this exercises the real + cross-environment bootstrap, not #384's dev-only bypass.""" + response = _register(client, skip_approval=True) + + assert response.status_code == 201 + assert response.json()["user"]["email"] == "alice@example.com" + + +def test_second_registration_after_the_first_owner_still_needs_approval(client): + """The bootstrap window closes the instant the first registration + commits: a second, different unapproved email on the same instance is + rejected exactly as it would be without #388 at all.""" + assert _register(client, skip_approval=True).status_code == 201 + + response = _register(client, email="bob@example.com", skip_approval=True) + + error = assert_error_response(response, 422, "validation_error") + assert "hasn't been approved" in error.message + + +def test_first_ever_registration_works_in_production_too(client): + """Not #384's dev-only bypass, and not specific to the test fixture's + own APP_ENV=test -- #388's bootstrap has no environment check at all. + Constructed the same way test_development_bypasses_the_allowlist_for_an_ + unapproved_email builds a non-development AuthService directly against + the same (still-empty) database the `client` fixture just created.""" + from sqlalchemy import select + + from app.auth.service import AuthService + from app.core.config import get_settings + from app.core.database import SessionLocal + from app.models.approved_email import ApprovedEmail + + prod_settings = get_settings().model_copy(update={"app_env": "production"}) + + with SessionLocal() as db: + user, access_token, refresh_token = AuthService(db, prod_settings).register( + "self-hoster@example.com", "correct-horse-battery" + ) + assert user.email == "self-hoster@example.com" + assert access_token + assert refresh_token + + bootstrap_approval = db.scalars( + select(ApprovedEmail).where(ApprovedEmail.email == "self-hoster@example.com") + ).one() + assert bootstrap_approval.added_by == "first-user-bootstrap" + assert bootstrap_approval.used_at is not None + assert bootstrap_approval.used_by_user_id == user.id + + +def test_first_user_bootstrap_ignores_the_seed_placeholder_row(client): + """A real, Alembic-migrated deployment always has the credential-less + SEED_USER_ID placeholder row (app/models/user.py) in `users` before + anyone has ever registered -- migration 0002 seeds it on every fresh + database, unlike this test's own `create_all`-based fixture DB. #388's + "first user" check must not mistake that permanent system row for an + already-claimed instance, or bootstrap would never fire on a real + deployment at all.""" + from app.core.database import SessionLocal + from app.models.user import SEED_USER_EMAIL, SEED_USER_ID, User + + with SessionLocal() as db: + db.add(User(id=SEED_USER_ID, email=SEED_USER_EMAIL, password_hash=None)) + db.commit() + + response = _register(client, skip_approval=True) + + assert response.status_code == 201 + + +def test_development_bypasses_the_allowlist_for_an_unapproved_email(client): + """#384: local development must stay exactly as frictionless as it was + before the allowlist existed. The `client` fixture itself runs as + APP_ENV=test (see conftest.py) specifically so this doesn't leak into + the rest of the suite -- this test builds its own AuthService against + an app_env="development" copy of the real settings to exercise the + bypass directly, the same construction + test_register_commit_time_collision_is_reported_as_conflict uses.""" + from sqlalchemy import select + + from app.auth.service import AuthService + from app.core.config import get_settings + from app.core.database import SessionLocal + from app.models.approved_email import ApprovedEmail + + dev_settings = get_settings().model_copy(update={"app_env": "development"}) + + with SessionLocal() as db: + user, access_token, refresh_token = AuthService(db, dev_settings).register( + "never-approved@example.com", "correct-horse-battery" + ) + assert user.email == "never-approved@example.com" + assert access_token + assert refresh_token + + # A real, persisted ApprovedEmail row was created -- not a special + # in-memory-only path -- so the rest of register()'s audit trail + # (used_at/used_by_user_id) behaves identically to a real approval. + auto_approval = db.scalars( + select(ApprovedEmail).where(ApprovedEmail.email == "never-approved@example.com") + ).one() + assert auto_approval.added_by == "dev-bypass" + assert auto_approval.used_at is not None + assert auto_approval.used_by_user_id == user.id + + +def test_development_bypass_does_not_apply_outside_development(client): + """The same unapproved email that succeeds under app_env="development" + (previous test) must still be rejected under every other environment + value -- this is a narrowly-scoped dev convenience, not a relaxation of + the check itself. A baseline user is registered first so #388's + first-user bootstrap (which, unlike the dev-only bypass, applies in + every one of these environments) has already closed before the loop + runs -- otherwise the first iteration would succeed via bootstrap + rather than exercising the dev-bypass boundary this test is about.""" + from app.auth.service import AuthService + from app.core.config import get_settings + from app.core.database import SessionLocal + from app.core.exceptions import ValidationServiceError + + assert _register(client).status_code == 201 + + for env in ("test", "staging", "production"): + settings = get_settings().model_copy(update={"app_env": env}) + with SessionLocal() as db: + with pytest.raises(ValidationServiceError, match="hasn't been approved"): + AuthService(db, settings).register(f"never-approved-{env}@example.com", "correct-horse-battery") + + def test_register_still_succeeds_after_the_approved_email_is_used_once(client): """Approval is not single-use (#374): re-registering the SAME email a second time is rejected by the ordinary email-uniqueness conflict, not diff --git a/apps/backend/tests/test_oauth_routes.py b/apps/backend/tests/test_oauth_routes.py index 17993b3..d8ce0d5 100644 --- a/apps/backend/tests/test_oauth_routes.py +++ b/apps/backend/tests/test_oauth_routes.py @@ -168,6 +168,14 @@ def test_successful_login_for_an_already_linked_identity_sets_refresh_cookie(sel _clear_overrides(anonymous) def test_brand_new_unapproved_identity_redirects_to_signup_required(self, client): + # A baseline user is registered first so #388's first-user bootstrap + # (AuthService._require_approval, applies to this OAuth path exactly + # the same as password registration) has already closed -- otherwise + # this callback would BE the first-ever registration on a fresh + # database and succeed via bootstrap instead of exercising the + # rejection this test is about. + register_user(client, "existing-owner@example.com") + identity = OAuthIdentityInfo( subject="never-seen-sub", email="brandnew@example.com", email_verified=True, display_name="Brand New" ) diff --git a/apps/backend/tests/test_oauth_service.py b/apps/backend/tests/test_oauth_service.py index d1cacf8..2fb302e 100644 --- a/apps/backend/tests/test_oauth_service.py +++ b/apps/backend/tests/test_oauth_service.py @@ -206,9 +206,19 @@ def test_brand_new_unapproved_identity_is_rejected_never_bypassing_the_allowlist (AuthService.register), and an OAuth-created account with no equivalent check would be a silent bypass of that gate, not a feature. An unapproved visitor is sent back to the allowlist-gated - registration form instead.""" + registration form instead. + + A baseline user is created first so #388's first-user bootstrap + (which applies to this OAuth path exactly the same as it does to + password registration, since both go through + AuthService._require_approval) has already closed before this + identity is attempted -- otherwise this OAuth sign-in would BE the + first-ever registration on a fresh database and succeed via + bootstrap instead of exercising the rejection this test is about.""" import asyncio + _create_user(db, "existing-owner@example.com") + identity = OAuthIdentityInfo( subject="sub-1", email="newperson@example.com", email_verified=True, display_name="New Person" ) @@ -222,6 +232,29 @@ def test_brand_new_unapproved_identity_is_rejected_never_bypassing_the_allowlist assert db.query(User).filter(User.email == "newperson@example.com").count() == 0 assert db.query(OAuthIdentity).count() == 0 + def test_first_ever_oauth_identity_becomes_the_owner_via_bootstrap(self, db): + """#388's first-user bootstrap applies to the OAuth path exactly the + same way it applies to password registration, since both call + AuthService._require_approval -- a verified provider identity that + happens to be the very first account on a fresh, otherwise-empty + instance is auto-approved and signed in, not rejected.""" + import asyncio + + identity = OAuthIdentityInfo( + subject="sub-1", email="first-owner@example.com", email_verified=True, display_name="First Owner" + ) + service = _make_service(db, {"google": FakeProviderClient(identity=identity)}) + url = service.start("google", intent="login", frontend_redirect_base="http://localhost:5173") + state = url.split("state=")[1].split("&")[0] + + base, result = asyncio.run(service.complete_callback("google", state=state, code="c", provider_error=None)) + assert result.kind == "session" + assert result.user is not None + assert result.user.email == "first-owner@example.com" + + approval = db.query(ApprovedEmail).filter(ApprovedEmail.email == "first-owner@example.com").one() + assert approval.added_by == "first-user-bootstrap" + def test_brand_new_approved_identity_creates_an_account_via_oauth(self, db): """The one exception to 'OAuth never creates an account': a verified provider email that's already on the SAME allowlist password diff --git a/apps/backend/tests/test_system.py b/apps/backend/tests/test_system.py index c36121c..8a328ac 100644 --- a/apps/backend/tests/test_system.py +++ b/apps/backend/tests/test_system.py @@ -9,7 +9,7 @@ def test_health_endpoint(client): assert response.status_code == 200 assert response.json()["status"] == "ok" - assert response.json()["environment"] == "development" + assert response.json()["environment"] == "test" def test_readiness_endpoint(client): @@ -18,7 +18,7 @@ def test_readiness_endpoint(client): assert response.status_code == 200 assert response.json() == { "status": "ready", - "environment": "development", + "environment": "test", "checks": {"database": "ok", "storage": "ok"}, } @@ -51,7 +51,7 @@ def test_readiness_endpoint_reports_database_failure(client, monkeypatch): assert response.status_code == 503 assert response.json() == { "status": "not_ready", - "environment": "development", + "environment": "test", "checks": {"database": "error", "storage": "ok"}, } @@ -66,7 +66,7 @@ def test_readiness_endpoint_reports_storage_failure(client, monkeypatch): assert response.status_code == 503 assert response.json() == { "status": "not_ready", - "environment": "development", + "environment": "test", "checks": {"database": "ok", "storage": "error"}, }