From 99f810d0435f3ef80442ea54c896f40bd8f803c3 Mon Sep 17 00:00:00 2001 From: Alec Perkins Date: Tue, 27 Oct 2026 08:42:19 -0400 Subject: [PATCH] Harden auth, invites and deployment after security review - Signed-out visitors get the same login redirect for missing and private projects, so the redirect no longer reveals private ones. - Production refuses the console email backend; sign-in codes must never reach the logs. - Database cache shared by all workers for allauth rate limits; invite sending throttled per sender and per recipient. - Email invite links are claimed on first use and the token rotated, so a token copied from request logs is worthless. - allauth password URLs removed; adding an email requires a fresh code and notifies existing addresses. - Markdown drops remote images until uploads exist. - Project and org slugs lowercased; only superusers can grant staff status or permissions in the admin. - Container runs as a non-root user; CI actions pinned to commits. Co-Authored-By: Claude Fable 5.1 --- .forgejo/workflows/ci.yml | 9 +-- Dockerfile | 4 ++ bin/start | 8 +++ config/settings/base.py | 13 +++++ config/settings/dev.py | 2 + config/settings/prod.py | 3 + config/settings/test.py | 1 + config/urls.py | 21 ++++++- docs/plan.md | 13 +++-- intent/accounts/admin.py | 7 +++ intent/markdown.py | 5 +- intent/orgs/admin.py | 3 + .../migrations/0002_orginvite_claimed_by.py | 21 +++++++ intent/orgs/models.py | 9 +++ intent/orgs/services.py | 55 ++++++++++++++++++- intent/orgs/urls.py | 1 + intent/orgs/views.py | 39 +++++++++---- intent/projects/decorators.py | 16 +++--- intent/projects/forms.py | 3 + intent/projects/services.py | 3 +- templates/orgs/invite.html | 2 +- tests/test_accounts.py | 27 ++++++++- tests/test_markdown.py | 4 ++ tests/test_org_views.py | 37 +++++++++++-- tests/test_projects.py | 17 +++++- 25 files changed, 281 insertions(+), 42 deletions(-) create mode 100644 intent/orgs/migrations/0002_orginvite_claimed_by.py diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index c6de86b..941a354 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -9,8 +9,9 @@ jobs: # Adjust to match the labels of your Forgejo runner. runs-on: docker steps: - - uses: https://data.forgejo.org/actions/checkout@v4 - - uses: https://github.com/astral-sh/setup-uv@v6 + # Actions are pinned to commits so a moved tag can't change what runs here. + - uses: https://data.forgejo.org/actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + - uses: https://github.com/astral-sh/setup-uv@d0cc045d04ccac9d8b7881df0226f9e82c39688e # v6.8.0 with: version: "0.13.6" - run: uv sync --locked @@ -23,8 +24,8 @@ jobs: if: github.event_name == 'push' && github.ref == 'refs/heads/main' runs-on: docker steps: - - uses: https://data.forgejo.org/actions/checkout@v4 - - uses: https://github.com/superfly/flyctl-actions/setup-flyctl@master + - uses: https://data.forgejo.org/actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + - uses: https://github.com/superfly/flyctl-actions/setup-flyctl@ed8efb33836e8b2096c7fd3ba1c8afe303ebbff1 # master, 2026-10 - run: flyctl deploy --remote-only env: FLY_API_TOKEN: ${{ secrets.FLY_API_TOKEN }} diff --git a/Dockerfile b/Dockerfile index ae52bc7..5a91643 100644 --- a/Dockerfile +++ b/Dockerfile @@ -23,13 +23,17 @@ CMD ["sh", "-c", "python manage.py migrate --noinput && python manage.py runserv FROM base AS prod COPY --from=litestream/litestream:0.5.17 /usr/local/bin/litestream /usr/local/bin/litestream +# The app runs as this user; bin/start drops to it after making the data volume writable. +RUN useradd --system --uid 1000 --create-home app RUN --mount=type=cache,target=/root/.cache/uv \ --mount=type=bind,source=uv.lock,target=uv.lock \ --mount=type=bind,source=pyproject.toml,target=pyproject.toml \ uv sync --locked --no-install-project --no-dev COPY litestream.yml /etc/litestream.yml COPY . . +# Placeholder settings: collectstatic needs the prod settings to import, nothing more. RUN DJANGO_SETTINGS_MODULE=config.settings.prod DJANGO_SECRET_KEY=collectstatic \ + EMAIL_BACKEND=django.core.mail.backends.dummy.EmailBackend \ python manage.py collectstatic --noinput ENV DJANGO_SETTINGS_MODULE=config.settings.prod EXPOSE 8000 diff --git a/bin/start b/bin/start index 5de1914..036db3f 100755 --- a/bin/start +++ b/bin/start @@ -3,16 +3,24 @@ # because release machines don't mount the volume that holds the SQLite database. set -eu +# The volume is mounted owned by root. Take ownership for the app user, then rerun as it. +if [ "$(id -u)" = 0 ]; then + chown -R app:app "${INTENT_DATA_DIR:?}" + exec setpriv --reuid=app --regid=app --init-groups env HOME=/home/app "$0" "$@" +fi + DB="${INTENT_DATA_DIR:?}/db.sqlite3" SERVER="gunicorn config.wsgi --bind 0.0.0.0:8000 --workers 3 --access-logfile -" if [ -z "${LITESTREAM_BUCKET:-}" ]; then echo "LITESTREAM_BUCKET not set: running without backups" >&2 python manage.py migrate --noinput + python manage.py createcachetable exec $SERVER fi # On a fresh volume, pull the latest backup before anything touches the database. litestream restore -if-db-not-exists -if-replica-exists "$DB" python manage.py migrate --noinput +python manage.py createcachetable exec litestream replicate -exec "$SERVER" diff --git a/config/settings/base.py b/config/settings/base.py index d12a8e0..2068ffc 100644 --- a/config/settings/base.py +++ b/config/settings/base.py @@ -76,6 +76,15 @@ DATABASES = { } } +# Rate limits (allauth's and ours) live here, so the cache must be shared by every worker +# and survive restarts. The database is the only shared store on a one-machine deploy. +CACHES = { + "default": { + "BACKEND": "django.core.cache.backends.db.DatabaseCache", + "LOCATION": "cache", + } +} + AUTH_USER_MODEL = "accounts.User" DEFAULT_AUTO_FIELD = "django.db.models.BigAutoField" @@ -100,6 +109,10 @@ ACCOUNT_LOGIN_BY_CODE_ENABLED = True ACCOUNT_LOGIN_BY_CODE_SUPPORTS_RESEND = True ACCOUNT_LOGIN_BY_CODE_TIMEOUT = 600 ACCOUNT_LOGOUT_ON_GET = False +# Adding an email address adds a way to sign in, so ask for a fresh code first, and tell +# the existing addresses about it. +ACCOUNT_REAUTHENTICATION_REQUIRED = True +ACCOUNT_EMAIL_NOTIFICATIONS = True SOCIALACCOUNT_PROVIDERS = { "github": {"SCOPE": ["read:user", "user:email"]}, } diff --git a/config/settings/dev.py b/config/settings/dev.py index 4cdc7b7..380d0bc 100644 --- a/config/settings/dev.py +++ b/config/settings/dev.py @@ -5,3 +5,5 @@ SECRET_KEY = SECRET_KEY or "dev-insecure-secret-key" # noqa: F405 ALLOWED_HOSTS = ["localhost", "127.0.0.1", "[::1]"] DATA_DIR.mkdir(exist_ok=True) # noqa: F405 +# Avoid needing `createcachetable` for local development. +CACHES = {"default": {"BACKEND": "django.core.cache.backends.locmem.LocMemCache"}} diff --git a/config/settings/prod.py b/config/settings/prod.py index 535ee6b..b7655f2 100644 --- a/config/settings/prod.py +++ b/config/settings/prod.py @@ -7,6 +7,9 @@ from .base import * # noqa: F403 if not SECRET_KEY: # noqa: F405 raise ImproperlyConfigured("DJANGO_SECRET_KEY must be set") +# The console backend would print sign-in codes and invite links into the logs. +if EMAIL_BACKEND == "django.core.mail.backends.console.EmailBackend": # noqa: F405 + raise ImproperlyConfigured("EMAIL_BACKEND must be set to a real email backend in production") ALLOWED_HOSTS = [h for h in os.environ.get("DJANGO_ALLOWED_HOSTS", "").split(",") if h] CSRF_TRUSTED_ORIGINS = [f"https://{h}" for h in ALLOWED_HOSTS] diff --git a/config/settings/test.py b/config/settings/test.py index 9567e0f..b908025 100644 --- a/config/settings/test.py +++ b/config/settings/test.py @@ -2,6 +2,7 @@ from .base import * # noqa: F403 SECRET_KEY = "test-secret-key" PASSWORD_HASHERS = ["django.contrib.auth.hashers.MD5PasswordHasher"] +CACHES = {"default": {"BACKEND": "django.core.cache.backends.locmem.LocMemCache"}} STORAGES = { **STORAGES, # noqa: F405 "default": {"BACKEND": "django.core.files.storage.InMemoryStorage"}, diff --git a/config/urls.py b/config/urls.py index a1093ea..7d0712e 100644 --- a/config/urls.py +++ b/config/urls.py @@ -1,14 +1,31 @@ +import allauth.urls +from allauth.account import urls as allauth_account_urls from django.contrib import admin -from django.urls import include, path +from django.urls import URLResolver, include, path from intent.core import views from intent.orgs.urls import invite_urlpatterns from intent.projects.views import project_create + +def _without_passwords(patterns): + """allauth's URLs minus the password views: regular users sign in with a code only.""" + return [p for p in patterns if not str(p.pattern).startswith("password/")] + + +account_urlpatterns = [ + ( + path("", include(_without_passwords(allauth_account_urls.urlpatterns))) + if isinstance(p, URLResolver) and p.urlconf_name is allauth_account_urls + else p + ) + for p in allauth.urls.urlpatterns +] + urlpatterns = [ path("", views.home, name="home"), path("admin/", admin.site.urls), - path("accounts/", include("allauth.urls")), + path("accounts/", include(account_urlpatterns)), path("invites/", include(invite_urlpatterns)), # Organizations own the rest of the URL space (see RESERVED_SLUGS in orgs and projects). path("/new/", project_create, name="project_create"), diff --git a/docs/plan.md b/docs/plan.md index 3ec88e2..4f17149 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -34,10 +34,10 @@ Decisions made in the concept are not repeated. These fill the gaps. | Thread → issue | Original commenter or any team member. Thread gets `promoted_to` issue; issue body is prefilled with the thread text and links back. | | Deleted users | Content remains, attributed to a "ghost" user. | | Moderation | Project owners can delete comments and close issues in their project. Sysadmin can deactivate a user site-wide. | -| Invites | By email or existing username. Username invites appear on the invitee's Invitations page (no notifications in V1). Email invites send a link carrying a random token (only its hash is stored); the link requires sign-in and whoever holds it may accept with any account. Email invites also appear on the Invitations page of a user with that verified email. Re-inviting replaces the pending invite. Expire after 14 days. The last org owner cannot be removed, demoted or leave. | +| Invites | By email or existing username. Username invites appear on the invitee's Invitations page (no notifications in V1). Email invites send a link carrying a random token (only its hash is stored); the link requires sign-in and the first signed-in visitor claims the invite with whatever account they used. Claiming rotates the token, so the link (which appears in request logs) is dead after one use; the claimant then accepts or declines from their Invitations page. Unclaimed email invites also appear on the Invitations page of a user with that verified email. Re-inviting replaces the pending invite. Sending is throttled (30 per sender per hour, 3 per recipient per org per hour). Expire after 14 days. The last org owner cannot be removed, demoted or leave. | | URLs | Organizations live at the root: `//`, `///`, `///features/INT-12/`. App routes are reserved slugs at both levels. | | Project slug and prefix | Fixed after creation, since they appear in URLs and feature IDs. Name, visibility and open contribution stay editable. | -| Hidden projects | A project the user can't see returns 404 (not 403), so private projects don't reveal they exist. Signed-out visitors are sent to sign in instead. | +| Hidden projects | A project the user can't see returns 404 (not 403), so private projects don't reveal they exist. Signed-out visitors are sent to sign in instead, for missing and hidden projects alike, so the redirect reveals nothing either. Markdown drops remote images (``) until uploads exist, so an author can't log readers' IP addresses. | | Project owners | No last-owner guard: org owners can always administer every project. Leaving the org removes a user's project roles in it. | | Feature editing | One form edits title, description, parent, status and replacement, with a required change note. Saving with no changes is refused. A "Preview description" button renders the Markdown server-side, so preview works without JS. Django admin shows features read-only so every change goes through revisions. | @@ -46,8 +46,9 @@ Decisions made in the concept are not repeated. These fill the gaps. | Topic | Default | |---|---| | Runtime | Python 3.13, Django 5.2 LTS, `uv` for dependencies, `ruff` for lint/format. | -| Auth | `django-allauth`: GitHub, Discord, OpenID Connect provider per Forgejo instance (Forgejo supports OIDC). Sysadmins register each provider as a SocialApp in Django admin, so adding a Forgejo instance needs no deploy. Email sign-in is allauth's login-by-code: the "magic link" is an emailed one-time code, not a clickable link. Signup asks for email and username only; regular users never get a password. Multiple social accounts per user via allauth's connect flow (no automatic linking by email). Usernames: letters, digits, `-`, `_`, max 39. | -| Sysadmins | Django `is_staff` users with passwords, Django admin at `/admin/`. A model constraint rejects staff users as org members. Regular users have no password (unusable). | +| Auth | `django-allauth`: GitHub, Discord, OpenID Connect provider per Forgejo instance (Forgejo supports OIDC). Sysadmins register each provider as a SocialApp in Django admin, so adding a Forgejo instance needs no deploy. Email sign-in is allauth's login-by-code: the "magic link" is an emailed one-time code, not a clickable link. Signup asks for email and username only; regular users never get a password, and allauth's password URLs are not routed at all. Adding an email address requires re-entering a fresh code, and existing addresses are notified. Multiple social accounts per user via allauth's connect flow (no automatic linking by email). Usernames: letters, digits, `-`, `_`, max 39. | +| Sysadmins | Django `is_staff` users with passwords, Django admin at `/admin/`. A model constraint rejects staff users as org members. Regular users have no password (unusable). Only superusers can grant staff or superuser status or permissions in the admin. | +| Cache | Django's database cache (`createcachetable` runs at start). It holds allauth's rate limits and the invite throttle, so it must be shared by all gunicorn workers and survive restarts; the database is the only shared store on a one-machine deploy. Dev and tests use the in-memory cache. | | Database | SQLite on a Fly volume. `transaction_mode = "IMMEDIATE"`, WAL mode, `busy_timeout` 5 s. Litestream replicates to B2 continuously. | | Files | Tigris via `django-storages` S3 backend, private bucket, time-limited signed URLs. HTML prototypes are shown in a sandboxed `