diff --git a/.dockerignore b/.dockerignore index 66b12ad..3e72258 100644 --- a/.dockerignore +++ b/.dockerignore @@ -7,3 +7,4 @@ __pycache__ .pytest_cache .ruff_cache docs +tests diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index 941a354..b5c4e8d 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -17,6 +17,8 @@ jobs: - run: uv sync --locked - run: uv run ruff check . - run: uv run ruff format --check . + # Browser for the accessibility, CSP and keyboard tests in tests/browser. + - run: uv run playwright install --with-deps chromium - run: uv run pytest deploy: diff --git a/README.md b/README.md index f7b2ad9..6eb1be9 100644 --- a/README.md +++ b/README.md @@ -23,20 +23,12 @@ Then open http://localhost:8000. Data lives in `data/` (uv) or the `data` volume Checks: ```sh +uv run playwright install chromium # once, for the real-browser tests uv run ruff check . && uv run ruff format --check . && uv run pytest ``` +`uv run pytest -m "not browser"` skips the browser tests for a quicker run. + ## Production -Fly.io, one machine with a volume at `/data`. `bin/start` restores the database from B2 if the volume is empty, runs migrations, and starts gunicorn under Litestream. - -Secrets (`fly secrets set`): - -| Name | Purpose | -|---|---| -| `DJANGO_SECRET_KEY` | Django secret key | -| `LITESTREAM_BUCKET`, `LITESTREAM_ENDPOINT`, `LITESTREAM_ACCESS_KEY_ID`, `LITESTREAM_SECRET_ACCESS_KEY` | B2 backup bucket | -| `BUCKET_NAME`, `AWS_*` | Tigris uploads; set by `fly storage create` | -| `SENTRY_DSN` | Optional error reporting | - -Restore drill: create a fresh volume, deploy, and confirm `bin/start` logs a restore before migrating. +Fly.io, one machine with a volume at `/data`; Litestream backs the database up to B2 and uploads go to Tigris. Deploying, secrets, administrators, email, moderation and the restore drill are in [docs/operations.md](docs/operations.md). diff --git a/bin/start b/bin/start index 036db3f..16cdc8d 100755 --- a/bin/start +++ b/bin/start @@ -5,7 +5,8 @@ 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:?}" + mkdir -p "${INTENT_DATA_DIR:?}" + chown -R app:app "$INTENT_DATA_DIR" exec setpriv --reuid=app --regid=app --init-groups env HOME=/home/app "$0" "$@" fi diff --git a/config/settings/base.py b/config/settings/base.py index 8a681f1..2a61f46 100644 --- a/config/settings/base.py +++ b/config/settings/base.py @@ -17,6 +17,8 @@ INSTALLED_APPS = [ "django.contrib.sessions", "django.contrib.messages", "django.contrib.staticfiles", + "django_otp", + "django_otp.plugins.otp_totp", "allauth", "allauth.account", "allauth.socialaccount", @@ -39,10 +41,12 @@ MIDDLEWARE = [ # First, so health checks skip host validation and the HTTPS redirect. "intent.core.middleware.HealthCheckMiddleware", "django.middleware.security.SecurityMiddleware", + "intent.core.middleware.SecurityHeadersMiddleware", "django.contrib.sessions.middleware.SessionMiddleware", "django.middleware.common.CommonMiddleware", "django.middleware.csrf.CsrfViewMiddleware", "django.contrib.auth.middleware.AuthenticationMiddleware", + "django_otp.middleware.OTPMiddleware", "allauth.account.middleware.AccountMiddleware", "django.contrib.messages.middleware.MessageMiddleware", "django.middleware.clickjacking.XFrameOptionsMiddleware", @@ -101,6 +105,11 @@ AUTHENTICATION_BACKENDS = [ "allauth.account.auth_backends.AuthenticationBackend", ] +# The admin asks for a code from an authenticator app as well as the password. +# Add a device with `manage.py add_admin_totp `. Off only in local development. +ADMIN_REQUIRE_OTP = True +OTP_TOTP_ISSUER = "Intent admin" + LOGIN_URL = "account_login" LOGIN_REDIRECT_URL = "home" @@ -124,8 +133,16 @@ SOCIALACCOUNT_PROVIDERS = { "github": {"SCOPE": ["read:user", "user:email"]}, } -# Provider is TBD (see concept.md); console output until then. +# Provider is TBD (see concept.md); console output until then. Any Anymail provider works +# without code changes: set EMAIL_BACKEND to its backend and pass its settings as ANYMAIL_* +# variables, e.g. EMAIL_BACKEND=anymail.backends.postmark.EmailBackend with +# ANYMAIL_POSTMARK_SERVER_TOKEN=... (see docs/operations.md). EMAIL_BACKEND = os.environ.get("EMAIL_BACKEND", "django.core.mail.backends.console.EmailBackend") +ANYMAIL = { + key.removeprefix("ANYMAIL_"): value + for key, value in os.environ.items() + if key.startswith("ANYMAIL_") +} DEFAULT_FROM_EMAIL = os.environ.get("DEFAULT_FROM_EMAIL", "Intent ") AUTH_PASSWORD_VALIDATORS = [ @@ -153,6 +170,8 @@ if os.environ.get("BUCKET_NAME"): "bucket_name": os.environ["BUCKET_NAME"], "endpoint_url": os.environ.get("AWS_ENDPOINT_URL_S3"), "region_name": os.environ.get("AWS_REGION", "auto"), + # Bucket in the hostname, matching CSP_BUCKET_ORIGIN below. + "addressing_style": "virtual", "default_acl": None, "querystring_auth": True, "querystring_expire": 300, @@ -165,6 +184,14 @@ else: "OPTIONS": {"location": DATA_DIR / "media"}, } +# The bucket's origin, so pages may show images and frame prototypes from signed URLs. +# Tigris signed URLs are virtual-hosted: https://.fly.storage.tigris.dev. +CSP_BUCKET_ORIGIN = os.environ.get("CSP_BUCKET_ORIGIN", "") +if not CSP_BUCKET_ORIGIN and os.environ.get("BUCKET_NAME"): + _endpoint = os.environ.get("AWS_ENDPOINT_URL_S3", "https://fly.storage.tigris.dev") + _scheme, _, _host = _endpoint.partition("://") + CSP_BUCKET_ORIGIN = f"{_scheme}://{os.environ['BUCKET_NAME']}.{_host.rstrip('/')}" + STORAGES = { "default": _default_storage, "staticfiles": {"BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage"}, diff --git a/config/settings/dev.py b/config/settings/dev.py index 380d0bc..7bc3f73 100644 --- a/config/settings/dev.py +++ b/config/settings/dev.py @@ -1,3 +1,5 @@ +import os + from .base import * # noqa: F403 DEBUG = True @@ -7,3 +9,6 @@ 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"}} + +# Plain password admin locally, unless you want to try the real thing. +ADMIN_REQUIRE_OTP = os.environ.get("ADMIN_REQUIRE_OTP") == "1" diff --git a/config/urls.py b/config/urls.py index c50df45..4b1bfd9 100644 --- a/config/urls.py +++ b/config/urls.py @@ -1,5 +1,6 @@ import allauth.urls from allauth.account import urls as allauth_account_urls +from django.conf import settings from django.contrib import admin from django.urls import URLResolver, include, path @@ -22,6 +23,13 @@ account_urlpatterns = [ for p in allauth.urls.urlpatterns ] +if settings.ADMIN_REQUIRE_OTP: + from django_otp.admin import OTPAdminSite + + # Same site instance and URL names; it now also requires a verified OTP device. + admin.site.__class__ = OTPAdminSite + admin.site.login_template = OTPAdminSite.login_template + urlpatterns = [ path("", views.home, name="home"), path("admin/", admin.site.urls), diff --git a/docs/operations.md b/docs/operations.md new file mode 100644 index 0000000..feacff6 --- /dev/null +++ b/docs/operations.md @@ -0,0 +1,129 @@ +# Operating Intent + +How to deploy, configure and look after a production Intent. For what the app does, see +[concept.md](concept.md); for how it's built, [plan.md](plan.md). + +## Shape + +One Fly.io machine with a volume at `/data` holding the SQLite database. Litestream streams +the database to a Backblaze B2 bucket continuously. Uploaded files go to a private Tigris +bucket. `bin/start` runs as root only long enough to take ownership of the volume, then as +the `app` user it: + +1. restores the database from B2 if the volume has none (a fresh volume), +2. runs migrations and creates the cache table, +3. starts gunicorn under `litestream replicate`. + +## Secrets and settings + +Set with `fly secrets set NAME=value`. Non-secret settings live in `fly.toml`. + +| Name | Required | Purpose | +|---|---|---| +| `DJANGO_SECRET_KEY` | yes | Signs sessions and tokens. Long and random. | +| `DJANGO_ALLOWED_HOSTS` | yes (`fly.toml`) | Comma-separated hostnames. Also sets the trusted CSRF origins. | +| `EMAIL_BACKEND` | yes | Production refuses to start with the console backend. See Email. | +| `ANYMAIL_*` | with Anymail | The provider's settings, e.g. `ANYMAIL_POSTMARK_SERVER_TOKEN`. | +| `DEFAULT_FROM_EMAIL` | yes | e.g. `Intent `. Must be a sender the provider allows. | +| `LITESTREAM_BUCKET`, `LITESTREAM_ENDPOINT`, `LITESTREAM_ACCESS_KEY_ID`, `LITESTREAM_SECRET_ACCESS_KEY` | yes | B2 backup bucket and an application key limited to it. | +| `BUCKET_NAME`, `AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, `AWS_ENDPOINT_URL_S3`, `AWS_REGION` | yes | Tigris uploads. `fly storage create` sets these. | +| `CSP_BUCKET_ORIGIN` | no | Only if signed URLs aren't `https://.fly.storage.tigris.dev`. | +| `SENTRY_DSN` | no | Error reporting. | + +## First deploy + +1. Replace the `CHANGEME` values in `fly.toml` (app name, region, host). +2. `fly launch --no-deploy`, then `fly volumes create intent_data --size 1`. +3. `fly storage create` for the Tigris bucket. +4. Set the secrets above. +5. `fly deploy`. Check `fly logs` for "running without backups" (Litestream not configured) or a restore message. +6. `fly ssh console`, then create the first administrator (next section). + +## Administrators + +Administrators are separate from regular users: they sign in only at `/admin/`, with a +password and a code from an authenticator app, and can't join organizations. + +```sh +fly ssh console +cd /app +python manage.py createsuperuser +python manage.py add_admin_totp +``` + +The second command prints an `otpauth://` link. Add it to an authenticator app (most can scan +it if you paste it into any QR generator you trust, or enter the secret by hand). If someone +loses their device, run it again with `--replace`. + +Only superusers can make other accounts staff or give out permissions. + +In local development the admin uses a password only. To try the real flow, run with +`ADMIN_REQUIRE_OTP=1`. + +## Organizations and sign-in providers + +- **Organizations** are created in the admin (Organizations → Add), naming the first owner. They invite everyone else. +- **GitHub and Discord sign-in:** in the admin, Social applications → Add, with the provider's client ID and secret. +- **A Forgejo instance:** register an OAuth2 application on the instance with the redirect URL `https:///accounts/oidc//login/callback/`, then add a Social application with provider "OpenID Connect", the same `provider_id`, the client ID and secret, and under Settings `{"server_url": "https://"}`. No deploy is needed. + +## Email + +Sign-in codes and email invitations need working email. Any provider +[Anymail](https://anymail.dev) supports works without code changes. For example, Postmark: + +```sh +fly secrets set \ + EMAIL_BACKEND=anymail.backends.postmark.EmailBackend \ + ANYMAIL_POSTMARK_SERVER_TOKEN=... \ + DEFAULT_FROM_EMAIL="Intent " +``` + +Then send a test and sign in with an emailed code: + +```sh +fly ssh console -C "python /app/manage.py sendtestemail you@example.org" +``` + +## Moderation + +- **Delete a comment:** the author, or an owner of the project, from the comment itself. It leaves a "Comment deleted" marker. +- **Close an issue:** any project collaborator or owner. +- **Block a person:** admin → Users → select → "Deactivate". They're signed out everywhere and can't sign in; what they wrote stays under their name. "Reactivate" undoes it. +- **Delete an account:** admin → the user → Delete. What they wrote stays, shown as written by "ghost". The admin won't delete an organization's last owner; make someone else an owner first. + +## Limits + +Per person: 30 comments or new threads per 10 minutes, 20 new issues per hour, 30 file uploads +per hour, 30 invitations per hour (and 3 per address per organization). allauth limits +sign-in attempts and code requests itself. Limits live in the database cache and reset when +their window ends. Change them in `intent/core/throttle.py`. + +## Backups and the restore drill + +Litestream keeps B2 within about a second of the database. Fly volume snapshots (daily by +default; check with `fly volumes snapshots list `) are a second, independent copy. + +Run the restore drill before launch and after any change to `bin/start` or `litestream.yml`: + +1. Note a recent change in the app, e.g. a comment you just posted. +2. Create a new volume and a new machine with it, from the same image: + `fly volumes create intent_data --size 1 --region ` then + `fly machine clone --attach-volume :/data`. +3. In `fly logs` for the new machine, confirm a Litestream restore happens before "Applying migrations" (or "No migrations to apply"). +4. `fly ssh console --machine ` and check the change from step 1 is there: + `python manage.py shell -c "from intent.comments.models import Comment; print(Comment.objects.latest('created_at').body)"`. +5. Destroy the clone and its volume. Two machines must never write to the same B2 path for long. + +To recover for real, stop the machine, destroy its volume, create a new one, and start the +machine: `bin/start` restores from B2 on an empty volume. + +## Tests + +```sh +uv run playwright install chromium # once per machine +uv run pytest # everything, including real-browser tests +uv run pytest -m "not browser" # fast run without the browser +``` + +The browser tests run axe (WCAG 2.1 A and AA) on every page in light and dark mode, check +there are no CSP violations or script errors, and walk the main flow by keyboard. diff --git a/docs/plan.md b/docs/plan.md index f662b42..2099268 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -201,6 +201,16 @@ Each phase ends deployable. Rough sizes assume one person, part-time; adjust fre - Backup restore drill repeated against the real volume. - **Exit:** launch checklist (below) complete. +Done in code; what remains needs production (see the checklist and `docs/operations.md`): + +- **Email:** `django-anymail` installed. The provider is chosen entirely by environment (`EMAIL_BACKEND` plus `ANYMAIL_*` variables), so picking one needs no code change. Still TBD. +- **Moderation:** deactivating a user in the admin blocks sign-in and ends their sessions; their content stays. Deleted accounts' content shows as "ghost" (authors are `SET_NULL`; no placeholder user row), and `ghost` and other official-looking usernames are reserved. The admin refuses to delete an organization's last owner, and bulk delete is off for users so that check always applies. +- **Rate limits:** one fixed-window helper (`intent/core/throttle.py`) in the shared database cache: comments and new threads 30 per 10 minutes, issues 20 per hour, uploads 30 per hour, invites as before. A limited HTMX reply keeps the draft. +- **CSP and headers:** every page gets `default-src 'self'`, no inline script or style, `object-src 'none'`, `base-uri 'none'`, `frame-ancestors 'self'`, and the bucket origin for images and frames when one is configured. No `form-action`, because sign-in with an external provider posts a form that redirects off-site. htmx is configured not to inject styles or eval. Also `Permissions-Policy`. File responses keep their own stricter sandbox CSP. +- **Admin second factor:** `django-otp` TOTP on the admin site, with `manage.py add_admin_totp ` to enrol. Off by default only in local development. +- **Browser tests** (`tests/browser`, marked `browser`): axe WCAG 2.1 A/AA on every page in light and dark, signed in and out; no CSP violations or script errors on any page including the admin; a keyboard-only walkthrough (skip link, line comment, HTMX reply with focus); the theme toggle. axe found that the page had no explicit background, so tools assumed white in dark mode; `:root` now sets `Canvas`/`CanvasText`. CI installs Chromium. +- **Restore drill:** written up in `docs/operations.md`; needs the real volume and B2. + ## 6. Launch checklist - [ ] Email provider configured; magic link and invite emails tested in production @@ -208,11 +218,13 @@ Each phase ends deployable. Rough sizes assume one person, part-time; adjust fre - [ ] Fly volume snapshots enabled as a second backup - [ ] Sentry receiving errors from production - [ ] At least one Forgejo OIDC provider registered and tested -- [ ] axe passes on feature, issue, project and org pages, light and dark -- [ ] Keyboard-only walkthrough of the example flow +- [x] axe passes on feature, issue, project and org pages, light and dark (`tests/browser`; recheck against production once deployed) +- [ ] Keyboard-only walkthrough of the example flow (the comment and reply path is automated; do the full flow by hand once) - [ ] Privacy check: private project content not reachable via file URLs, feeds, or admin leaks - [ ] `EMAIL_BACKEND` set in production (prod settings refuse the console backend) - [ ] Container confirmed running as the `app` user on the Fly volume +- [ ] First administrator created with `add_admin_totp`, and admin sign-in tested with a code +- [ ] HSTS raised to a year once the domain is settled (`SECURE_HSTS_SECONDS` in prod settings) ## 7. Hooks for V1.1 diff --git a/intent/accounts/admin.py b/intent/accounts/admin.py index 38e6d53..a975ebd 100644 --- a/intent/accounts/admin.py +++ b/intent/accounts/admin.py @@ -1,12 +1,47 @@ from django.contrib import admin from django.contrib.auth.admin import UserAdmin +from django.contrib.sessions.models import Session +from django.db.models import Count, F, Q + +from intent.orgs.models import Organization, Role from .models import User +def orgs_left_without_owner(users) -> list[Organization]: + """Organizations whose only owners are among `users`.""" + ids = [u.pk for u in users] + return list( + Organization.objects.annotate( + owners=Count("memberships", filter=Q(memberships__role=Role.OWNER)), + leaving=Count( + "memberships", + filter=Q(memberships__role=Role.OWNER, memberships__user__in=ids), + ), + ).filter(owners__gt=0, owners=F("leaving")) + ) + + +def end_sessions(users) -> None: + """Sign users out everywhere. Sessions aren't indexed by user, so scan them; there are few. + + Django already treats inactive users as signed out; this also clears their session data. + """ + ids = {str(u.pk) for u in users} + for session in Session.objects.all(): + if session.get_decoded().get("_auth_user_id") in ids: + session.delete() + + @admin.register(User) class IntentUserAdmin(UserAdmin): + """Moderation lives here: deactivate to block someone and keep their content; delete to + remove the account, after which their content shows as written by "ghost".""" + search_fields = ["username", "email"] + list_display = ["username", "email", "is_active", "is_staff", "date_joined", "last_login"] + list_filter = ["is_active", "is_staff", "is_superuser"] + actions = ["deactivate", "reactivate"] def get_readonly_fields(self, request, obj=None): # Only superusers may hand out staff or superuser status, or permissions. @@ -14,3 +49,32 @@ class IntentUserAdmin(UserAdmin): if not request.user.is_superuser: fields += ["is_staff", "is_superuser", "groups", "user_permissions"] return fields + + def save_model(self, request, obj, form, change): + super().save_model(request, obj, form, change) + if change and "is_active" in form.changed_data and not obj.is_active: + end_sessions([obj]) + + @admin.action(description="Deactivate: block sign-in and end sessions, keep content") + def deactivate(self, request, queryset): + users = list(queryset.exclude(pk=request.user.pk)) + User.objects.filter(pk__in=[u.pk for u in users]).update(is_active=False) + end_sessions(users) + self.message_user(request, f"Deactivated {len(users)} account(s).") + + @admin.action(description="Reactivate") + def reactivate(self, request, queryset): + count = queryset.update(is_active=True) + self.message_user(request, f"Reactivated {count} account(s).") + + def has_delete_permission(self, request, obj=None): + # An org's last owner can't be deleted; there would be no one left to run it. + if obj is not None and orgs_left_without_owner([obj]): + return False + return super().has_delete_permission(request, obj) + + def get_actions(self, request): + # Delete one account at a time, so the last-owner check above always applies. + actions = super().get_actions(request) + actions.pop("delete_selected", None) + return actions diff --git a/intent/accounts/management/__init__.py b/intent/accounts/management/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/intent/accounts/management/commands/__init__.py b/intent/accounts/management/commands/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/intent/accounts/management/commands/add_admin_totp.py b/intent/accounts/management/commands/add_admin_totp.py new file mode 100644 index 0000000..356c19a --- /dev/null +++ b/intent/accounts/management/commands/add_admin_totp.py @@ -0,0 +1,31 @@ +from django.contrib.auth import get_user_model +from django.core.management.base import BaseCommand, CommandError +from django_otp.plugins.otp_totp.models import TOTPDevice + + +class Command(BaseCommand): + help = ( + "Give a staff account an authenticator-app (TOTP) device for the admin's second " + "factor, and print the otpauth:// link to add to the app." + ) + + def add_arguments(self, parser): + parser.add_argument("username") + parser.add_argument( + "--replace", action="store_true", help="Remove the account's existing devices first." + ) + + def handle(self, username, replace, **options): + user = get_user_model().objects.filter(username=username).first() + if user is None: + raise CommandError(f"No user called {username}.") + if not user.is_staff: + raise CommandError(f"{username} isn't staff; only the admin uses this.") + devices = TOTPDevice.objects.filter(user=user) + if devices.exists() and not replace: + raise CommandError(f"{username} already has a device. Use --replace to reset it.") + devices.delete() + device = TOTPDevice.objects.create(user=user, name="Authenticator app", confirmed=True) + self.stdout.write("Add this to an authenticator app (most can scan it as a QR code):") + self.stdout.write(device.config_url) + self.stdout.write("Then sign in at /admin/ with the password and the current code.") diff --git a/intent/accounts/validators.py b/intent/accounts/validators.py index f0c733d..82cc186 100644 --- a/intent/accounts/validators.py +++ b/intent/accounts/validators.py @@ -1,5 +1,17 @@ +from django.core.exceptions import ValidationError from django.core.validators import RegexValidator +# "ghost" stands in for deleted accounts; the rest would look official. +RESERVED_USERNAMES = { + "admin", "administrator", "ghost", "intent", "root", "staff", "support", "system", +} # fmt: skip + + +def validate_not_reserved(value): + if value.lower() in RESERVED_USERNAMES: + raise ValidationError("That username is reserved.") + + # Handles are used in URLs and mentions, and must never look like an email address, # so a single "username or email" field can tell them apart. username_validators = [ @@ -7,4 +19,5 @@ username_validators = [ r"^[A-Za-z0-9][A-Za-z0-9_-]{0,38}$", "Use 1–39 letters, numbers, hyphens or underscores, starting with a letter or number.", ), + validate_not_reserved, ] diff --git a/intent/comments/services.py b/intent/comments/services.py index 3d790ea..7736c34 100644 --- a/intent/comments/services.py +++ b/intent/comments/services.py @@ -10,6 +10,8 @@ from intent.access import ( can_resolve_thread, project_access, ) +from intent.core import throttle as limits +from intent.core.throttle import throttle from .models import Comment, CommentEdit, CommentThread, ThreadKind @@ -32,6 +34,7 @@ def clean_body(body: str) -> str: def _require_contribute(by, project): if not project_access(by, project).can_contribute: raise PermissionDenied + throttle(f"comments:{by.pk}", *limits.COMMENTS) def start_feature_thread( diff --git a/intent/core/middleware.py b/intent/core/middleware.py index a9ee6c1..55e420e 100644 --- a/intent/core/middleware.py +++ b/intent/core/middleware.py @@ -14,3 +14,49 @@ class HealthCheckMiddleware: cursor.execute("SELECT 1") return HttpResponse("ok", content_type="text/plain") return self.get_response(request) + + +# Everything is served from our own origin; nothing inline. Uploaded images are shown from +# the bucket (signed URLs) and HTML prototypes are framed from it, so those origins are added +# when a bucket is configured. No form-action: sign-in with GitHub, Discord or Forgejo posts a +# form that redirects to the provider, and browsers apply form-action to that redirect. +def _content_security_policy() -> str: + from django.conf import settings + + bucket = settings.CSP_BUCKET_ORIGIN + extra = f" {bucket}" if bucket else "" + return "; ".join( + [ + "default-src 'self'", + "script-src 'self'", + "style-src 'self'", + f"img-src 'self' data:{extra}", + f"frame-src 'self'{extra}", + "object-src 'none'", + "base-uri 'none'", + "frame-ancestors 'self'", + ] + ) + + +PERMISSIONS_POLICY = ( + "camera=(), microphone=(), geolocation=(), payment=(), usb=(), interest-cohort=()" +) + + +class SecurityHeadersMiddleware: + """CSP and Permissions-Policy on every response that doesn't set its own. + + Django's SecurityMiddleware covers HSTS, nosniff, Referrer-Policy and COOP; the file view + sets a stricter sandbox CSP of its own, which this leaves alone. + """ + + def __init__(self, get_response): + self.get_response = get_response + self.csp = _content_security_policy() + + def __call__(self, request): + response = self.get_response(request) + response.headers.setdefault("Content-Security-Policy", self.csp) + response.headers.setdefault("Permissions-Policy", PERMISSIONS_POLICY) + return response diff --git a/intent/core/throttle.py b/intent/core/throttle.py new file mode 100644 index 0000000..3ea2130 --- /dev/null +++ b/intent/core/throttle.py @@ -0,0 +1,31 @@ +"""Fixed-window rate limits kept in the shared cache (the database cache in production). + +Services call `throttle()` before doing work; going over a limit raises a ValidationError +with a user-facing message, which views already show like any other rule violation. +""" + +from django.core.cache import cache +from django.core.exceptions import ValidationError + +TOO_MANY_MESSAGE = "You're doing that too often. Wait a few minutes and try again." + +# (events, seconds). Generous for people; tight enough to stop a script flooding a project. +COMMENTS = (30, 10 * 60) # Comments and new threads, per user. +ISSUES = (20, 60 * 60) # New issues, per user. +UPLOADS = (30, 60 * 60) # File uploads and new versions, per user. +INVITES_PER_SENDER = (30, 60 * 60) +INVITES_PER_RECIPIENT = (3, 60 * 60) # Per org, so one address isn't flooded with email. + + +def throttle(key: str, limit: int, seconds: int, message: str = TOO_MANY_MESSAGE) -> None: + """Count an event under `key`; raise once more than `limit` happen within `seconds`.""" + key = f"intent:throttle:{key}" + if cache.add(key, 1, timeout=seconds): + return + try: + count = cache.incr(key) + except ValueError: # Expired between add and incr. + cache.add(key, 1, timeout=seconds) + return + if count > limit: + raise ValidationError(message) diff --git a/intent/files/services.py b/intent/files/services.py index 35f1630..c398e59 100644 --- a/intent/files/services.py +++ b/intent/files/services.py @@ -5,6 +5,8 @@ from pathlib import PurePath from django.core.exceptions import ValidationError from django.db import transaction +from intent.core import throttle as limits +from intent.core.throttle import throttle from intent.features.services import commit_file_change from .models import ALLOWED_TYPES, MAX_BYTES, FeatureFile, FeatureFileVersion @@ -12,6 +14,10 @@ from .models import ALLOWED_TYPES, MAX_BYTES, FeatureFile, FeatureFileVersion ALLOWED_LIST = ", ".join(sorted({ext.lstrip(".") for ext in ALLOWED_TYPES})) +def _throttle_uploads(by) -> None: + throttle(f"uploads:{by.pk}", *limits.UPLOADS) + + def check_upload(upload) -> tuple[str, str]: """Validate an upload. Returns its display name and content type.""" name = PurePath(upload.name or "").name.strip()[:200] @@ -48,6 +54,7 @@ def add_file(feature, by, *, kind, upload, note, issues=()) -> FeatureFile: files_change=f"Added {kind} “{name}”", issues=issues, ) + _throttle_uploads(by) file = FeatureFile.objects.create( feature=feature, kind=kind, name=name, added_in_revision=revision.revision ) @@ -68,6 +75,7 @@ def replace_file(file, by, *, upload, note, issues=()) -> FeatureFileVersion: files_change=f"Replaced “{file.name}” with version {file.current_version + 1}", issues=issues, ) + _throttle_uploads(by) file.current_version += 1 file.name = name file.save(update_fields=["current_version", "name", "updated_at"]) diff --git a/intent/issues/services.py b/intent/issues/services.py index 4f7ded1..6feab39 100644 --- a/intent/issues/services.py +++ b/intent/issues/services.py @@ -12,12 +12,15 @@ from intent.access import ( project_access, ) from intent.comments.models import CommentThread, ThreadKind +from intent.core import throttle as limits +from intent.core.throttle import throttle from intent.markdown import source_lines from intent.projects.models import Project from .models import EventKind, Issue, IssueEvent, IssueStatus MAX_BODY = 50_000 +GHOST = "ghost" # Shown for content whose author's account was deleted. def _clean_text(title, body): @@ -40,6 +43,7 @@ def create_issue(project, by, *, title, body="", features=()) -> Issue: raise PermissionDenied title, body = _clean_text(title, body) _check_features(project, features) + throttle(f"issues:{by.pk}", *limits.ISSUES) with transaction.atomic(): Project.objects.filter(pk=project.pk).update(next_issue_number=F("next_issue_number") + 1) project.refresh_from_db(fields=["next_issue_number"]) @@ -176,7 +180,7 @@ def promotion_text(thread) -> tuple[str, str]: quoted = source_lines(snapshot.description, thread.line_start, thread.line_end) parts.append("\n".join(f"> {line}".rstrip() for line in quoted.splitlines())) for c in comments: - name = c.author.username if c.author else "deleted user" + name = c.author.username if c.author else GHOST quoted = "\n".join(f"> {line}".rstrip() for line in c.body.splitlines()) parts.append(f"**{name}** wrote:\n\n{quoted}") return title, "\n\n".join(parts) diff --git a/intent/orgs/services.py b/intent/orgs/services.py index 964d222..2b767cd 100644 --- a/intent/orgs/services.py +++ b/intent/orgs/services.py @@ -5,44 +5,26 @@ Missing permissions raise PermissionDenied. """ from allauth.account.models import EmailAddress -from django.core.cache import cache from django.core.exceptions import PermissionDenied, ValidationError from django.db import transaction from django.db.models import Q from django.utils import timezone from intent.access import org_access +from intent.core import throttle as limits +from intent.core.throttle import throttle from .models import InviteStatus, Organization, OrgInvite, OrgMembership, Role LAST_OWNER_MESSAGE = "An organization needs at least one owner. Make someone else an owner first." TOO_MANY_INVITES_MESSAGE = "Too many invitations sent recently. Try again later." -# Each invite can send an email, so cap how many one person sends and how often one -# address is hit, with room for genuine use. -INVITES_PER_SENDER = (30, 60 * 60) -INVITES_PER_RECIPIENT = (3, 60 * 60) - def _require_owner(by, org): if not org_access(by, org).can_manage_members: raise PermissionDenied -def _throttle(key: str, limit: int, seconds: int) -> None: - """Count an event under `key`; raise once more than `limit` happen within `seconds`.""" - key = f"intent:throttle:{key}" - if cache.add(key, 1, timeout=seconds): - return - try: - count = cache.incr(key) - except ValueError: # Expired between add and incr. - cache.add(key, 1, timeout=seconds) - return - if count > limit: - raise ValidationError(TOO_MANY_INVITES_MESSAGE) - - def _is_last_owner(membership) -> bool: return ( membership.role == Role.OWNER @@ -77,8 +59,11 @@ def invite(org, by, *, user=None, email="", role=Role.COLLABORATOR) -> tuple[Org email = email.strip().lower() if bool(user) == bool(email): raise ValueError("Invite exactly one of user or email") - _throttle(f"invites-by:{by.pk}", *INVITES_PER_SENDER) - _throttle(f"invites-to:{org.pk}:{user.pk if user else email}", *INVITES_PER_RECIPIENT) + recipient = user.pk if user else email + throttle(f"invites-by:{by.pk}", *limits.INVITES_PER_SENDER, TOO_MANY_INVITES_MESSAGE) + throttle( + f"invites-to:{org.pk}:{recipient}", *limits.INVITES_PER_RECIPIENT, TOO_MANY_INVITES_MESSAGE + ) with transaction.atomic(): if user: diff --git a/pyproject.toml b/pyproject.toml index 0b77a62..4c4553f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,6 +6,8 @@ requires-python = ">=3.13" dependencies = [ "django>=5.2,<5.3", "django-allauth[socialaccount]>=65.21", + "django-anymail==15.2", + "django-otp==1.7.5", "django-storages[s3]>=1.14.6", "gunicorn>=26.2.0", "markdown-it-py>=4.2.0", @@ -17,6 +19,7 @@ dependencies = [ [dependency-groups] dev = [ + "playwright==1.64.1", "pytest>=9.1.1", "pytest-django>=4.14.0", "ruff>=0.17.3", @@ -33,3 +36,6 @@ select = ["E", "F", "I", "UP", "B", "DJ"] [tool.pytest.ini_options] DJANGO_SETTINGS_MODULE = "config.settings.test" testpaths = ["tests"] +markers = [ + "browser: real-browser tests (Playwright + Chromium); skip with -m 'not browser'", +] diff --git a/static/css/intent.css b/static/css/intent.css index 2ed6209..dbb06a4 100644 --- a/static/css/intent.css +++ b/static/css/intent.css @@ -6,6 +6,10 @@ :root { color-scheme: light dark; + /* The same system colours the browser would use, made explicit so tools (and embeds) + that can't see the canvas don't assume black on white. */ + background-color: Canvas; + color: CanvasText; } :root[data-theme="light"] { diff --git a/templates/base.html b/templates/base.html index 2c93df8..e98ec7b 100644 --- a/templates/base.html +++ b/templates/base.html @@ -8,6 +8,8 @@ {# Blocking on purpose: applies the saved theme before first paint. #} + {# No injected