From 665f52dbe2c7b092d3ecd3784e22650d85f48fee Mon Sep 17 00:00:00 2001 From: ssavutu Date: Fri, 31 Jul 2026 02:16:58 -0400 Subject: [PATCH 1/6] fix(media): log why an upload failed, and document the write grant The CephFS corpus arrives owned by whoever ran the rsync, mode 755, while the backend container runs as uid 10001. Nothing in the read path notices, so uploads fail long after media serving looks healthy -- and the failure surfaces as "failed to store upload" rather than "failed to create upload directory", because MkdirAll returns nil for a directory that already exists and every migrated YYYY/MM directory does. The handler discarded the underlying error, so the log said only "status 500". It now logs it: a full volume, an unmounted tree and a permission problem all reach the client as the same message and need very different fixes. deploy/README.md gains the write check and the setfacl grant. The existing media section only ever verified that Nginx could read. Co-Authored-By: Claude Opus 5 --- deploy/README.md | 29 +++++++++++++++++++++++++++++ server/internal/handlers/media.go | 8 ++++++++ 2 files changed, 37 insertions(+) diff --git a/deploy/README.md b/deploy/README.md index e3f6676..f23fb2d 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -148,6 +148,35 @@ curl -I http://localhost/wp-content/uploads/YYYY/MM/name.jpg Expect `200` with `Cache-Control: public, max-age=2592000, immutable`. +### Making the media tree writable (uploads) + +The checks above only prove Nginx can *read*. `POST /v1/media` also has to +**write**, and the rsynced corpus arrives owned by whoever ran the rsync +(`tadmin`), mode 755, while the backend container runs as uid **10001**. Nothing +in the read path notices, so uploads fail long after media serving looks healthy: + +```bash +docker exec triangle-cms-backend-blue-1 \ + sh -c 'touch /mnt/cephfs/media/wp-content/uploads/.wtest && echo ok' +``` + +If that says `Permission denied`, grant the container uid write on upload +directories. CephFS is mounted with `acl`, so this is additive -- ownership and +the migrated files are untouched, and Nginx keeps reading as before: + +```bash +sudo find /mnt/cephfs/media/wp-content/uploads -type d \ + -exec setfacl -m u:10001:rwx -m d:u:10001:rwx {} + +``` + +The `d:` (default) entry is what makes each new `YYYY/MM` directory inherit the +grant, so this does not need repeating every month. + +The failure is easy to misread. `MkdirAll` returns nil for a directory that +already exists, and every migrated `YYYY/MM` directory does exist, so a +permission problem surfaces as `failed to store upload` rather than `failed to +create upload directory`. Check the backend log for the underlying `error=`. + ### Media library Serving the files is independent of *listing* them. The CMS media page reads a diff --git a/server/internal/handlers/media.go b/server/internal/handlers/media.go index 5326cb4..6321eb5 100644 --- a/server/internal/handlers/media.go +++ b/server/internal/handlers/media.go @@ -157,6 +157,7 @@ func PostMedia(conn *sql.DB) http.Handler { relDir := path.Join(uploadsSubdir, now.Format("2006"), now.Format("01")) absDir := filepath.Join(root, filepath.FromSlash(relDir)) if err := os.MkdirAll(absDir, 0o775); err != nil { + slog.Error("media upload: create directory", "dir", absDir, "error", err) writeError(w, http.StatusInternalServerError, "failed to create upload directory") return } @@ -164,6 +165,13 @@ func PostMedia(conn *sql.DB) http.Handler { base := sanitizeBaseName(header.Filename) name, written, err := storeUpload(absDir, base, ext, file) if err != nil { + // Worth a log line rather than just a 500: the message the client + // gets cannot say whether the media volume is full, unmounted, or + // simply not writable by this container's uid, and those need very + // different fixes. A permission error here is easy to mistake for a + // mkdir failure, since MkdirAll returns nil for a directory that + // already exists -- which every migrated YYYY/MM directory does. + slog.Error("media upload: store file", "dir", absDir, "error", err) writeError(w, http.StatusInternalServerError, "failed to store upload") return } From b2730d87df80a834cfb652784a898e13ac9f25be Mon Sep 17 00:00:00 2001 From: ssavutu Date: Fri, 31 Jul 2026 02:21:00 -0400 Subject: [PATCH 2/6] docs(deploy): note that setfacl is not installed by default The CephFS mount carries `acl`, which makes it look like the tooling is present; Ubuntu server images do not ship setfacl. Records the package and a setgid fallback for hosts where installing it is unwelcome. Co-Authored-By: Claude Opus 5 --- deploy/README.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/deploy/README.md b/deploy/README.md index f23fb2d..143da0f 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -162,9 +162,12 @@ docker exec triangle-cms-backend-blue-1 \ If that says `Permission denied`, grant the container uid write on upload directories. CephFS is mounted with `acl`, so this is additive -- ownership and -the migrated files are untouched, and Nginx keeps reading as before: +the migrated files are untouched, and Nginx keeps reading as before. The mount +supporting ACLs does not mean the tools are installed; Ubuntu server images +generally lack them: ```bash +sudo apt-get install -y acl # setfacl is not installed by default sudo find /mnt/cephfs/media/wp-content/uploads -type d \ -exec setfacl -m u:10001:rwx -m d:u:10001:rwx {} + ``` @@ -172,6 +175,16 @@ sudo find /mnt/cephfs/media/wp-content/uploads -type d \ The `d:` (default) entry is what makes each new `YYYY/MM` directory inherit the grant, so this does not need repeating every month. +Without the `acl` package, setgid does the same job in plain POSIX, at the cost +of changing group ownership rather than adding a grant beside it: + +```bash +sudo find /mnt/cephfs/media/wp-content/uploads -type d -exec chgrp 101 {} + +sudo find /mnt/cephfs/media/wp-content/uploads -type d -exec chmod 2775 {} + +``` + +`101` is the container's gid; the setgid bit is what new directories inherit. + The failure is easy to misread. `MkdirAll` returns nil for a directory that already exists, and every migrated `YYYY/MM` directory does exist, so a permission problem surfaces as `failed to store upload` rather than `failed to From fa9da448c50b905bba92280879d4afdd1da24e87 Mon Sep 17 00:00:00 2001 From: ssavutu Date: Fri, 31 Jul 2026 01:10:02 -0400 Subject: [PATCH 3/6] fix(comments): hide pingbacks and trackbacks from the public endpoint adminCommentConditions has always excluded them, which left GetApprovedCommentsByArticleSlug as the one place they still surfaced. They are automated link notifications rather than anything a reader wrote, and in the migrated WordPress data they are almost entirely SEO spam: 155 rows sourced from domains like weddingdressesideas.com and garciniacambogia-reviews.org, every one of them approved by WordPress. Same predicate as the admin path, so the two agree. Rows stay in the table. Co-Authored-By: Claude Opus 5 --- server/internal/database/comments.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/server/internal/database/comments.go b/server/internal/database/comments.go index 5ebf826..2e9a9ea 100644 --- a/server/internal/database/comments.go +++ b/server/internal/database/comments.go @@ -110,6 +110,14 @@ func ScanComment(rows *sql.Rows) (Comment, error) { return comment, nil } +// GetApprovedCommentsByArticleSlug returns the reader comments shown under an +// article. Pingbacks and trackbacks are excluded: WordPress stored them in the +// same table, but they are automated link notifications rather than anything a +// reader wrote, and on the imported data they are almost entirely SEO spam. +// This matches the predicate adminCommentConditions already applies, which is +// what left the public endpoint as the one place they still surfaced. Rows stay +// in the table; nothing renders them. A NULL or empty type means a comment -- +// that is what WordPress wrote for ordinary rows. func GetApprovedCommentsByArticleSlug(ctx context.Context, conn *sql.DB, slug string) ([]Comment, error) { rows, err := conn.QueryContext(ctx, ` SELECT @@ -126,6 +134,7 @@ func GetApprovedCommentsByArticleSlug(ctx context.Context, conn *sql.DB, slug st FROM comments c JOIN articles a ON a.id = c.article_id WHERE a.slug = ? AND c.status = 'approved' + AND (c.`+"`type`"+` IS NULL OR c.`+"`type`"+` = '' OR c.`+"`type`"+` = 'comment') ORDER BY COALESCE(c.created_at_gmt, c.created_at), c.id `, slug) if err != nil { From a69bf1114d4882a2441237928d805a171a316607 Mon Sep 17 00:00:00 2001 From: ssavutu Date: Fri, 31 Jul 2026 18:05:16 -0400 Subject: [PATCH 4/6] feat(media): raise the upload cap to 90 MiB and stop buffering uploads in RAM The migrated WordPress corpus contains unresized camera originals up to ~77 MiB (largest: 2025/07/BZ9A5771.jpg), so the 25 MiB cap rejected files the newsroom demonstrably produces. Raise the default to 90 MiB, chosen to sit under Cloudflare's 100 MB request-body limit on the tunnel fronting Delta -- past that a body clears Nginx and the backend but dies at the edge with an error the CMS never sees. ParseMultipartForm was being handed the size limit as its argument, but that argument is the in-memory buffer threshold, not the cap (MaxBytesReader has always been what enforces the cap). At 90 MiB that would let a single upload pin its full size in RAM, plus the ~10 MiB of slack Go adds on top. Give it a fixed 8 MiB instead and let the rest spill to temp files; the write path already streams and only reads image headers for dimensions, so a large upload now costs container disk rather than memory. Nginx's client_max_body_size moves to 91m to stay just above the backend, and the body/proxy timeouts go to 300s so a 90 MiB upload over a slow uplink is not killed mid-flight. MEDIA_MAX_UPLOAD_BYTES is also plumbed through Compose and the env template -- it was read by the backend but never passed in, so the cap could previously only be changed by editing Go. Deploying this needs both halves: reinstall the Nginx site and reload, and recreate the backend slots. The smaller of the two limits silently wins. Co-Authored-By: Claude Opus 5 --- deploy/README.md | 17 +++++++- deploy/cms.env.example | 1 + deploy/compose.cms.yml | 4 ++ deploy/nginx/triangle-cms.conf | 13 ++++-- server/internal/handlers/media.go | 18 +++++++-- server/internal/handlers/media_test.go | 56 ++++++++++++++++++++++++++ 6 files changed, 101 insertions(+), 8 deletions(-) diff --git a/deploy/README.md b/deploy/README.md index 143da0f..758de62 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -114,12 +114,23 @@ Nginx will not start without `active-upstreams.conf`, since the site `include`s it unconditionally. A passing `nginx -t` *before* the site is enabled only validates the stock config and proves nothing. -The site sets `client_max_body_size 26m` on `/v1/` to sit just above the -backend's `MEDIA_MAX_UPLOAD_BYTES` (25 MiB default). Nginx's stock limit is 1m, +The site sets `client_max_body_size 91m` on `/v1/` to sit just above the +backend's `MEDIA_MAX_UPLOAD_BYTES` (90 MiB default). Nginx's stock limit is 1m, which rejects an ordinary phone photo with a 413 before the CMS ever sees it, so a deploy that skips re-copying this file leaves media uploads broken while the app looks correctly configured. Raise both together, never just one. +The 90 MiB figure is sized to the migrated corpus, which contains unresized +camera originals up to ~77 MiB (largest: `2025/07/BZ9A5771.jpg`). The hard +ceiling above it is Cloudflare's **100 MB** request-body limit on the tunnel +fronting Delta; a body that passes Nginx and the backend but exceeds that dies +at the edge with an error the CMS never sees, so do not raise the pair past +~95 MiB without moving media uploads off the tunnel. + +The backend streams uploads to a temp file rather than buffering them in RAM +(only the first 8 MiB stays in memory), so a large upload costs container disk, +not memory. + ### Media serving `location /wp-content/` reads the migrated WordPress corpus straight off CephFS. @@ -260,6 +271,8 @@ The file must contain the exact immutable image tag for the active deployment: `/mnt/cephfs/media` unless the bind-mount target changes. - `MEDIA_BASE_URL` - public origin that serves `/wp-content/`, used to build media URLs returned by the upload endpoint. Empty yields relative URLs. +- `MEDIA_MAX_UPLOAD_BYTES` - per-file upload cap in bytes. Empty uses the + 90 MiB default. Must stay at or below Nginx's `client_max_body_size`. Keep `CMS_AUTO_PROMOTE_ALL_ADMINS=false` and `CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP=false` in production. Rebuild taxonomy diff --git a/deploy/cms.env.example b/deploy/cms.env.example index bb8e654..eabf8e6 100644 --- a/deploy/cms.env.example +++ b/deploy/cms.env.example @@ -19,3 +19,4 @@ AKISMET_BLOG_URL= MEDIA_HOST_PATH= MEDIA_ROOT= MEDIA_BASE_URL= +MEDIA_MAX_UPLOAD_BYTES= diff --git a/deploy/compose.cms.yml b/deploy/compose.cms.yml index a079034..b8e470e 100644 --- a/deploy/compose.cms.yml +++ b/deploy/compose.cms.yml @@ -35,6 +35,10 @@ x-backend-base: &backend-base # assets under MEDIA_ROOT; MEDIA_BASE_URL is the public host that serves them. MEDIA_ROOT: ${MEDIA_ROOT:-/mnt/cephfs/media} MEDIA_BASE_URL: ${MEDIA_BASE_URL:-} + # Per-file upload cap. Must stay at or below Nginx's client_max_body_size + # (91m) and below Cloudflare's 100 MB tunnel limit; raise all of them + # together or the smallest one silently wins. + MEDIA_MAX_UPLOAD_BYTES: ${MEDIA_MAX_UPLOAD_BYTES:-94371840} volumes: # CephFS media tree (host). rw so the upload endpoint can store new files; # host Nginx serves the same tree read-only (see deploy/nginx/triangle-cms.conf). diff --git a/deploy/nginx/triangle-cms.conf b/deploy/nginx/triangle-cms.conf index 67f4a4a..4bd4bcd 100644 --- a/deploy/nginx/triangle-cms.conf +++ b/deploy/nginx/triangle-cms.conf @@ -39,12 +39,19 @@ server { } location /v1/ { - # POST /v1/media accepts up to MEDIA_MAX_UPLOAD_BYTES (25 MiB by - # default). Nginx's own default is 1m, so without this it rejects any + # POST /v1/media accepts up to MEDIA_MAX_UPLOAD_BYTES (90 MiB by + # default -- the legacy corpus has full-res camera originals near 77 + # MiB). Nginx's own default is 1m, so without this it rejects any # ordinary phone photo with a 413 before the request reaches the CMS, # and the app-side limit never gets a say. Keep this at or above the # backend's limit so the backend is the one that decides. - client_max_body_size 26m; + client_max_body_size 91m; + # A 90 MiB body over a slow uplink takes minutes; the stock 60s applies + # per read, but raise the ceilings so a stalled-but-alive upload is not + # killed mid-flight. + client_body_timeout 300s; + proxy_read_timeout 300s; + proxy_send_timeout 300s; proxy_pass $triangle_cms_backend; } diff --git a/server/internal/handlers/media.go b/server/internal/handlers/media.go index 6321eb5..a60739a 100644 --- a/server/internal/handlers/media.go +++ b/server/internal/handlers/media.go @@ -33,8 +33,18 @@ import ( const ( mediaFormField = "file" - // defaultMaxUploadBytes caps a single upload when MEDIA_MAX_UPLOAD_BYTES is unset. - defaultMaxUploadBytes int64 = 25 << 20 // 25 MiB + // defaultMaxUploadBytes caps a single upload when MEDIA_MAX_UPLOAD_BYTES is + // unset. The migrated WordPress corpus contains full-resolution camera + // originals up to ~77 MiB, so anything smaller than this rejects files the + // newsroom demonstrably produces. The ceiling is Cloudflare's 100 MB request + // limit on the tunnel in front of Delta -- stay below it, since a body that + // clears the backend but not the tunnel fails with an opaque edge error. + defaultMaxUploadBytes int64 = 90 << 20 // 90 MiB + // multipartMemoryBytes is how much of a multipart body ParseMultipartForm may + // hold in RAM before spilling the rest to temp files. It is deliberately + // decoupled from the size limit: passing the limit itself would let a single + // upload pin its full size in memory (and Go adds ~10 MiB of slack on top). + multipartMemoryBytes int64 = 8 << 20 // 8 MiB // uploadsSubdir is the WordPress-compatible prefix (under MEDIA_ROOT) that all // uploads live under, so new files line up with the rsynced legacy corpus and // the Nginx `location /wp-content/` root that serves them. @@ -111,7 +121,9 @@ func PostMedia(conn *sql.DB) http.Handler { // Cap the request body. A little slack over maxBytes covers multipart // framing so a file exactly at the limit still succeeds. r.Body = http.MaxBytesReader(w, r.Body, maxBytes+4096) - if err := r.ParseMultipartForm(maxBytes); err != nil { + // MaxBytesReader, not this argument, is what enforces the limit; this only + // decides where the bytes land on the way in (see multipartMemoryBytes). + if err := r.ParseMultipartForm(multipartMemoryBytes); err != nil { var tooLarge *http.MaxBytesError if errors.As(err, &tooLarge) { writeError(w, http.StatusRequestEntityTooLarge, diff --git a/server/internal/handlers/media_test.go b/server/internal/handlers/media_test.go index 266885f..ac99ab5 100644 --- a/server/internal/handlers/media_test.go +++ b/server/internal/handlers/media_test.go @@ -12,6 +12,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strconv" "strings" "testing" @@ -69,6 +70,61 @@ func TestPostMedia_NotConfigured(t *testing.T) { } } +func TestPostMedia_RejectsOversizeUpload(t *testing.T) { + t.Setenv("MEDIA_ROOT", t.TempDir()) + t.Setenv("MEDIA_MAX_UPLOAD_BYTES", "1024") + rec := httptest.NewRecorder() + // Comfortably past the limit plus the multipart-framing slack, so this is + // the size check firing and not a boundary-arithmetic accident. + PostMedia(nil).ServeHTTP(rec, uploadRequest(t, "big.png", bytes.Repeat([]byte("a"), 64<<10))) + if rec.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("status = %d, want 413; body = %s", rec.Code, rec.Body.String()) + } +} + +// The size limit is enforced by MaxBytesReader, not by ParseMultipartForm's +// argument -- that one only decides how much stays in RAM before spilling to a +// temp file. A body well past multipartMemoryBytes but under the limit must +// therefore parse normally; reaching the 415 content-type check proves it did. +func TestPostMedia_AcceptsBodyLargerThanMultipartMemory(t *testing.T) { + t.Setenv("MEDIA_ROOT", t.TempDir()) + t.Setenv("MEDIA_MAX_UPLOAD_BYTES", strconv.FormatInt(multipartMemoryBytes*4, 10)) + rec := httptest.NewRecorder() + PostMedia(nil).ServeHTTP(rec, uploadRequest(t, "notes.txt", bytes.Repeat([]byte("a"), int(multipartMemoryBytes)+1<<20))) + if rec.Code != http.StatusUnsupportedMediaType { + t.Fatalf("status = %d, want 415 (413 means the size cap fired early); body = %s", rec.Code, rec.Body.String()) + } +} + +func TestMaxUploadBytes(t *testing.T) { + // The migrated WordPress corpus holds camera originals near 77 MiB, so a + // default below that rejects files the newsroom already has. + if defaultMaxUploadBytes < 80<<20 { + t.Fatalf("defaultMaxUploadBytes = %d, too small for the legacy corpus", defaultMaxUploadBytes) + } + + t.Run("default when unset", func(t *testing.T) { + t.Setenv("MEDIA_MAX_UPLOAD_BYTES", "") + if got := maxUploadBytes(); got != defaultMaxUploadBytes { + t.Fatalf("maxUploadBytes = %d, want %d", got, defaultMaxUploadBytes) + } + }) + + t.Run("env override", func(t *testing.T) { + t.Setenv("MEDIA_MAX_UPLOAD_BYTES", "4096") + if got := maxUploadBytes(); got != 4096 { + t.Fatalf("maxUploadBytes = %d, want 4096", got) + } + }) + + t.Run("falls back on garbage", func(t *testing.T) { + t.Setenv("MEDIA_MAX_UPLOAD_BYTES", "90MiB") + if got := maxUploadBytes(); got != defaultMaxUploadBytes { + t.Fatalf("maxUploadBytes = %d, want the default %d", got, defaultMaxUploadBytes) + } + }) +} + func TestPostMediaIndex_NotConfigured(t *testing.T) { t.Setenv("MEDIA_ROOT", "") rec := httptest.NewRecorder() From f05302e79971b30e736fccc9a305175317cc1ac6 Mon Sep 17 00:00:00 2001 From: ssavutu Date: Fri, 31 Jul 2026 18:19:51 -0400 Subject: [PATCH 5/6] feat(sidebar): temporarily hide the Newsletter nav item Comment out the Newsletter entry and its Mail import, leaving a note on how to restore both. Co-Authored-By: Claude Opus 5 --- frontend/src/components/Sidebar.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/Sidebar.tsx b/frontend/src/components/Sidebar.tsx index 121841f..5f4eb1a 100644 --- a/frontend/src/components/Sidebar.tsx +++ b/frontend/src/components/Sidebar.tsx @@ -4,7 +4,7 @@ import { LayoutDashboard, FileText, TrendingUp, - Mail, + // Mail, // used by the disabled Newsletter nav item Image, Users, Layers, @@ -52,7 +52,9 @@ const navGroups: NavGroup[] = [ { icon: TrendingUp, label: "Developing Stories", path: "/developing-stories" }, { icon: BarChart3, label: "Poll", path: "/poll" }, { icon: Image, label: "Media", path: "/media" }, - { icon: Mail, label: "Newsletter", path: "/newsletter" }, + // Newsletter is temporarily disabled; re-enable this entry (and the Mail + // import above) to bring it back. + // { icon: Mail, label: "Newsletter", path: "/newsletter" }, ], }, { From e726fde09e49ecfedf4e07cca6d321196d913adb Mon Sep 17 00:00:00 2001 From: ssavutu Date: Fri, 31 Jul 2026 18:27:53 -0400 Subject: [PATCH 6/6] feat(auth): bootstrap the first user as admin, default the rest to editor New CMS users were promoted to admin whenever CMS_AUTO_PROMOTE_ALL_ADMINS was set, and existing users were re-promoted on every login. Drop the flag and give new accounts the editor role instead. The first user to log in against an empty cms_users table is still bootstrapped as an admin so a fresh install has someone who can manage roles. The count and the insert share a transaction with a locking read so two simultaneous first logins can't both come out as admin. Existing admins keep their role; this only governs account creation. Co-Authored-By: Claude Opus 5 --- .env.example | 1 - deploy/README.md | 10 +++--- deploy/cms.env.example | 1 - deploy/compose.cms.yml | 1 - docker-compose.yml | 1 - server/internal/database/users.go | 60 ++++++++++++++++++++----------- 6 files changed, 46 insertions(+), 28 deletions(-) diff --git a/.env.example b/.env.example index 1b0533b..0d50d7c 100644 --- a/.env.example +++ b/.env.example @@ -7,7 +7,6 @@ GRAFANA_ADMIN_PASSWORD=change-this-admin-password OIDC_ISSUER_URL= OIDC_CLIENT_ID= OIDC_CLIENT_SECRET= -CMS_AUTO_PROMOTE_ALL_ADMINS=false CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP=false # Delta production variables live in deploy/cms.env.example. Do not put diff --git a/deploy/README.md b/deploy/README.md index 758de62..823da02 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -260,7 +260,6 @@ The file must contain the exact immutable image tag for the active deployment: - `FRONTEND_ORIGIN` - `OIDC_REDIRECT_URI` - `CMS_SESSION_TTL_SECONDS` -- `CMS_AUTO_PROMOTE_ALL_ADMINS` - `CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP` - `AKISMET_API_KEY` - optional; leave empty to disable comment spam filtering. - `AKISMET_BLOG_URL` - full public site URL Akismet should associate with @@ -274,9 +273,12 @@ The file must contain the exact immutable image tag for the active deployment: - `MEDIA_MAX_UPLOAD_BYTES` - per-file upload cap in bytes. Empty uses the 90 MiB default. Must stay at or below Nginx's `client_max_body_size`. -Keep `CMS_AUTO_PROMOTE_ALL_ADMINS=false` and -`CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP=false` in production. Rebuild taxonomy -through the admin endpoint after deploys when needed. +Keep `CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP=false` in production. Rebuild +taxonomy through the admin endpoint after deploys when needed. + +New users are created as editors. The very first user to log in to an empty +`cms_users` table is bootstrapped as an admin; promote anyone else from the +users screen. ## Required GitHub Production Environment Variables diff --git a/deploy/cms.env.example b/deploy/cms.env.example index eabf8e6..ab242d9 100644 --- a/deploy/cms.env.example +++ b/deploy/cms.env.example @@ -12,7 +12,6 @@ OIDC_CLIENT_SECRET= FRONTEND_ORIGIN= OIDC_REDIRECT_URI= CMS_SESSION_TTL_SECONDS= -CMS_AUTO_PROMOTE_ALL_ADMINS= CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP= AKISMET_API_KEY= AKISMET_BLOG_URL= diff --git a/deploy/compose.cms.yml b/deploy/compose.cms.yml index b8e470e..c707afa 100644 --- a/deploy/compose.cms.yml +++ b/deploy/compose.cms.yml @@ -27,7 +27,6 @@ x-backend-base: &backend-base FRONTEND_ORIGIN: ${FRONTEND_ORIGIN:?FRONTEND_ORIGIN is required} OIDC_REDIRECT_URI: ${OIDC_REDIRECT_URI:?OIDC_REDIRECT_URI is required} CMS_SESSION_TTL_SECONDS: ${CMS_SESSION_TTL_SECONDS:-604800} - CMS_AUTO_PROMOTE_ALL_ADMINS: ${CMS_AUTO_PROMOTE_ALL_ADMINS:-false} CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP: ${CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP:-false} AKISMET_API_KEY: ${AKISMET_API_KEY:-} AKISMET_BLOG_URL: ${AKISMET_BLOG_URL:-} diff --git a/docker-compose.yml b/docker-compose.yml index 39713b6..73b2dbf 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -43,7 +43,6 @@ services: TLS_KEY_FILE: /app/certs/localhost.key OIDC_ISSUER_URL: ${OIDC_ISSUER_URL:-} OIDC_CLIENT_ID: ${OIDC_CLIENT_ID:-} - CMS_AUTO_PROMOTE_ALL_ADMINS: ${CMS_AUTO_PROMOTE_ALL_ADMINS:-false} CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP: ${CMS_REBUILD_TAXONOMY_COUNTS_ON_STARTUP:-false} depends_on: mariadb: diff --git a/server/internal/database/users.go b/server/internal/database/users.go index 02c166e..5560925 100644 --- a/server/internal/database/users.go +++ b/server/internal/database/users.go @@ -3,7 +3,6 @@ package database import ( "context" "database/sql" - "os" "strings" "time" @@ -96,14 +95,8 @@ func EnsureUsersTable(ctx context.Context, conn *sql.DB) error { } func FindOrCreateUser(ctx context.Context, conn *sql.DB, sub, email, name string) (*models.User, error) { - autoPromoteAllAdmins := strings.EqualFold(strings.TrimSpace(os.Getenv("CMS_AUTO_PROMOTE_ALL_ADMINS")), "true") user, err := findUserBySub(ctx, conn, sub) if err == nil { - if autoPromoteAllAdmins && user.Role != models.RoleAdmin { - if err := UpdateUserRole(ctx, conn, user.ID, models.RoleAdmin); err == nil { - user.Role = models.RoleAdmin - } - } _ = updateLastLogin(ctx, conn, user.ID) return user, nil } @@ -113,19 +106,7 @@ func FindOrCreateUser(ctx context.Context, conn *sql.DB, sub, email, name string authorID := findAuthorIDByEmail(ctx, conn, email) - role := models.RoleEditor - if autoPromoteAllAdmins { - role = models.RoleAdmin - } - - res, err := conn.ExecContext(ctx, - "INSERT INTO cms_users (sub, email, name, role, author_id) VALUES (?, ?, ?, ?, ?)", - sub, email, name, role, authorID, - ) - if err != nil { - return nil, err - } - id, err := res.LastInsertId() + id, role, err := insertUser(ctx, conn, sub, email, name, authorID) if err != nil { return nil, err } @@ -143,6 +124,45 @@ func FindOrCreateUser(ctx context.Context, conn *sql.DB, sub, email, name string }, nil } +// insertUser creates a CMS user, bootstrapping the very first one as an admin so +// a fresh install has someone who can manage roles. Everyone after that starts as +// an editor and has to be promoted from the users screen. The count and the +// insert share a transaction (and a locking read) so two simultaneous first +// logins can't both come out as admin. +func insertUser(ctx context.Context, conn *sql.DB, sub, email, name string, authorID *int64) (int64, models.Role, error) { + tx, err := conn.BeginTx(ctx, nil) + if err != nil { + return 0, "", err + } + defer func() { _ = tx.Rollback() }() + + var existing int64 + if err := tx.QueryRowContext(ctx, "SELECT COUNT(*) FROM cms_users FOR UPDATE").Scan(&existing); err != nil { + return 0, "", err + } + + role := models.RoleEditor + if existing == 0 { + role = models.RoleAdmin + } + + res, err := tx.ExecContext(ctx, + "INSERT INTO cms_users (sub, email, name, role, author_id) VALUES (?, ?, ?, ?, ?)", + sub, email, name, role, authorID, + ) + if err != nil { + return 0, "", err + } + id, err := res.LastInsertId() + if err != nil { + return 0, "", err + } + if err := tx.Commit(); err != nil { + return 0, "", err + } + return id, role, nil +} + func findUserBySub(ctx context.Context, conn *sql.DB, sub string) (*models.User, error) { row := conn.QueryRowContext(ctx, "SELECT id, sub, email, name, role, author_id, created_at, last_login_at FROM cms_users WHERE sub = ?",