diff --git a/README.md b/README.md index 2678c1ba6..d9ce2cfbc 100644 --- a/README.md +++ b/README.md @@ -587,21 +587,34 @@ receive automatically — see ### Public upload approval -New accounts start **without** permission to upload public files or media, and -verifying the email address does not grant it. The account carries -`meta.publicUploads: false` from registration; `POST /api/v1/attachments/uploads` -answers `403 public_uploads_not_approved` until an administrator flips it from -the **/admin → Users** tab (`POST /api/v1/admin/users/public-uploads`). - -The flag is deliberately tri-state, so nothing here needs a data migration: - -| `meta.publicUploads` | Meaning | +New accounts start **without** permission to upload files or media, and +verifying the email address does not grant it. The permission has two +independent scopes, so an administrator can approve the **public**, **private**, +or **all** variation per user: + +| Scope | Covers | Flag | +| --- | --- | --- | +| `public` | post, comment, and custom-emoji attachments | `meta.publicUploads` | +| `private` | message attachments + the user's own profile avatar/banner | `meta.privateUploads` | +| `all` | both of the above in one write | both flags | + +The account carries `meta.publicUploads: false` and `meta.privateUploads: +false` from registration; `POST /api/v1/attachments/uploads` answers +`403 public_uploads_not_approved` or `403 private_uploads_not_approved` +(depending on the requested purpose) until an administrator approves that +scope from the **/admin → Users** tab's per-row **Approve** menu +(`POST /api/v1/admin/users/public-uploads { userId, enabled, scope }`; scope +defaults to `public` for pre-scope callers). + +Each flag is deliberately tri-state, so nothing here needs a data migration: + +| flag value | Meaning | | --- | --- | | absent | account predates the change — uploads stay enabled | | `false` | withheld, awaiting admin approval (every new signup) | | `true` | granted by an administrator | -Administrators bypass the flag entirely, so the account that grants the +Administrators bypass the flags entirely, so the account that grants the permission can never be locked out of the surface that grants it. Use the SES **API** with an IAM key scoped to `ses:SendEmail` — do not create diff --git a/TESTING.md b/TESTING.md index 8986ed815..ab1131052 100644 --- a/TESTING.md +++ b/TESTING.md @@ -9,11 +9,21 @@ is fixed, and cite the checklist you ran in the PR description. ## Public upload approval (new-signup permissions) - [ ] Register a brand-new account. `POST /api/v1/auth/register` returns - `publicUploadsEnabled: false`, and `POST /api/v1/attachments/uploads` - answers `403` with `code: "public_uploads_not_approved"`. -- [ ] Open the verification link. `emailVerified` flips to `true` while - `publicUploadsEnabled` stays `false` — verifying an email must never be + `publicUploadsEnabled: false` AND `privateUploadsEnabled: false`; + `POST /api/v1/attachments/uploads` answers `403` with + `code: "public_uploads_not_approved"` for public purposes (`post`, + `comment`, `custom-emoji`, or no purpose) and + `code: "private_uploads_not_approved"` for private ones (`message`, + `profile-avatar`, `profile-banner`). +- [ ] Open the verification link. `emailVerified` flips to `true` while both + `*UploadsEnabled` flags stay `false` — verifying an email must never be what grants uploads. +- [ ] Scopes stay independent: approve only `private` + (`POST /api/v1/admin/users/public-uploads` with `scope: "private"`) and + confirm `profile-avatar`/`message` starts pass the gate while `post` + still 403s; approve only `public` on another account and confirm the + reverse; `scope: "all"` enables both, and a request without `scope` + keeps the legacy public-only behavior. - [ ] Confirm the `admin.new_user` message reaches `THINGTIME_ADMIN_NOTIFICATION_EMAIL` (default `admin@thingtime.com`) with the username, display name, email, user id, and signup time. In dev read @@ -22,15 +32,19 @@ is fixed, and cite the checklist you ran in the PR description. verification still succeeds and still redirects to `/login?verify=success` — a mail outage must never fail a committed verification, nor grant the permission. -- [ ] In **/admin → Users**, the account shows a `pending` Uploads badge and the - warning banner counts it. Click **Enable**: the badge flips optimistically, - a Lopu toast confirms, and the upload start no longer 403s. -- [ ] Click **Withhold** on the same row: uploads 403 again. An account that - predates the change (no `meta.publicUploads`) shows `enabled`, and an - admin row shows `enabled` with no toggle. +- [ ] In **/admin → Users**, the account shows a `pending` Uploads badge and + the warning banner counts it. Use the **Approve ▾** menu: "Enable public + uploads" / "Enable private uploads" flip only that scope (badge shows + `public` or `private`), "Enable all" turns the badge green `all` — each + optimistically with a Lopu toast — and the matching upload starts stop + 403ing. +- [ ] Withhold from the same menu: that scope 403s again ("Withhold all" + returns the badge to `pending`). An account that predates the change (no + `meta.publicUploads`/`meta.privateUploads`) shows `all`, and an admin row + shows `all` with no menu. - [ ] Non-admins calling `POST /api/v1/admin/users/public-uploads` get `403`; - a missing `userId` or non-boolean `enabled` gets `400`; an unknown user - gets `404`. + a missing `userId`, non-boolean `enabled`, or unknown `scope` gets `400`; + an unknown user gets `404`. - [ ] Run `npm run test:attachments` (it carries the public-upload permission unit tests alongside the upload-gate regression test). diff --git a/remix/CHANGELOG.md b/remix/CHANGELOG.md index 97f9ee78e..52f7758cd 100644 --- a/remix/CHANGELOG.md +++ b/remix/CHANGELOG.md @@ -19,6 +19,18 @@ assistant and manual changes attributed so future PR archaeology is less cursed. ### Security +- **Upload approval now has public / private / all scopes**: the + signup-permissions gate is split into two independent tri-state flags — + `meta.publicUploads` (post/comment/custom-emoji attachments) and the new + `meta.privateUploads` (message attachments + own profile media) — both + stamped `false` at registration and both privileged meta keys. The upload + start gate is purpose-aware (`403 public_uploads_not_approved` / + `private_uploads_not_approved`), `POST /api/v1/admin/users/public-uploads` + accepts `scope: 'public' | 'private' | 'all'` (default `public`, wire- + compatible), and the /admin Users tab's control becomes an Approve menu with + per-scope and enable/withhold-all actions plus per-scope pending flags. + Grandfathering and the admin bypass are unchanged. — Claude (AI), 2026-08-18 + - **New signups no longer receive public upload permissions**: accounts created from this change forward start with `meta.publicUploads: false`, and verifying the email address no longer grants uploads. `POST /api/v1/attachments/uploads` diff --git a/remix/app/api/utils/attachments/attachmentResponses.ts b/remix/app/api/utils/attachments/attachmentResponses.ts index 749f652f7..824783766 100644 --- a/remix/app/api/utils/attachments/attachmentResponses.ts +++ b/remix/app/api/utils/attachments/attachmentResponses.ts @@ -54,8 +54,16 @@ const defaultDependencies: AttachmentMutationDependencies = { readBody: readJsonBody }; +// Upload purposes by permission scope. Mirrors attachmentUploadIntent in +// attachments.ts (an absent purpose defaults to 'post'): public purposes land +// on publicly viewable surfaces, private ones only in the account's own DMs or +// profile. An unknown purpose is left to the service's own 400 — nothing is +// reserved either way. +const PUBLIC_UPLOAD_PURPOSES = new Set([undefined, 'post', 'comment', 'custom-emoji']); +const PRIVATE_UPLOAD_PURPOSES = new Set(['message', 'profile-avatar', 'profile-banner']); + export const createAttachmentMutationAction = ( - options: { rateKey: string; service: MutationService; requirePublicUploads?: boolean }, + options: { rateKey: string; service: MutationService; requireUploadPermission?: boolean }, overrides: Partial = {} ) => { const dependencies = { ...defaultDependencies, ...overrides }; @@ -76,23 +84,6 @@ export const createAttachmentMutationAction = ( if (user.accountKind !== 'user') { return json({ ok: false, error: 'Attachments require a user account' }, { status: 403 }); } - // Public file/media uploads are withheld from new accounts until an admin - // grants them (see auth/users.ts userPublicUploadsEnabled). Gate the - // START of an upload: nothing is reserved, no MPU is opened, and every - // downstream part/complete call has no upload id to act on. The already - // -uploaded lifecycle calls (parts/complete/abort/delete) stay ungated so - // a permission change mid-upload can't strand a paid-for reservation. - if (options.requirePublicUploads && !user.publicUploadsEnabled) { - return json( - { - ok: false, - error: 'Uploads are awaiting admin approval for this account', - code: 'public_uploads_not_approved' - }, - { status: 403 } - ); - } - const limit = await dependencies.enforceLimit(request, options.rateKey, `user:${user.id}`, { failClosed: true }); if (!limit.allowed) { if (limit.unavailable) { @@ -102,6 +93,34 @@ export const createAttachmentMutationAction = ( } const body = await dependencies.readBody(request, ATTACHMENT_JSON_BODY_BYTES); + // File/media uploads are withheld per SCOPE from new accounts until an + // admin grants them (auth/users.ts userPublicUploadsEnabled / + // userPrivateUploadsEnabled): the requested purpose decides which flag + // gates this start. Sits after the body read because the purpose lives in + // the body — a denied attempt still consumes rate budget, which only + // throttles retry spam. Gate the START of an upload: nothing is reserved, + // no MPU is opened, and every downstream part/complete call has no upload + // id to act on. The already-uploaded lifecycle calls + // (parts/complete/abort/delete) stay ungated so a permission change + // mid-upload can't strand a paid-for reservation. + if (options.requireUploadPermission) { + const purpose = body && typeof body === 'object' && !Array.isArray(body) ? (body as Record).purpose : undefined; + const needsPublic = PUBLIC_UPLOAD_PURPOSES.has(purpose) && !user.publicUploadsEnabled; + const needsPrivate = PRIVATE_UPLOAD_PURPOSES.has(purpose) && !user.privateUploadsEnabled; + if (needsPublic || needsPrivate) { + return json( + { + ok: false, + error: needsPublic + ? 'Public file and media uploads are awaiting admin approval for this account' + : 'Private file and media uploads are awaiting admin approval for this account', + code: needsPublic ? 'public_uploads_not_approved' : 'private_uploads_not_approved' + }, + { status: 403 } + ); + } + } + const result = await options.service(user.id, body); if (result.ok === false) { const { status, ...bodyResult } = result; diff --git a/remix/app/api/utils/auth/publicUploads.test.ts b/remix/app/api/utils/auth/publicUploads.test.ts index 205b66fee..d466d6596 100644 --- a/remix/app/api/utils/auth/publicUploads.test.ts +++ b/remix/app/api/utils/auth/publicUploads.test.ts @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import test from 'node:test'; -import { userPublicUploadsEnabled } from './users'; +import { userPrivateUploadsEnabled, userPublicUploadsEnabled } from './users'; // Signup-permissions hotfix. The permission is deliberately TRI-STATE, and each // state has to survive a refactor: @@ -43,3 +43,22 @@ test('admins are never gated by the flag', () => { // An admin locked out of uploads could not test or fix the approval flow. assert.equal(userPublicUploadsEnabled(doc({ publicUploads: false, admin: true })), true); }); + +test('private upload permission is an independent tri-state scope', () => { + // same tri-state contract as the public flag… + assert.equal(userPrivateUploadsEnabled(doc(undefined)), true); + assert.equal(userPrivateUploadsEnabled(doc({})), true); + assert.equal(userPrivateUploadsEnabled(doc({ privateUploads: false })), false); + assert.equal(userPrivateUploadsEnabled(doc({ privateUploads: true })), true); + for (const value of [0, '', null, 'false']) { + assert.equal(userPrivateUploadsEnabled(doc({ privateUploads: value })), true, `unexpected deny for ${JSON.stringify(value)}`); + } + assert.equal(userPrivateUploadsEnabled(doc({ privateUploads: false, admin: true })), true); + + // …and the scopes never bleed into each other: granting one variation must + // not grant the other ("all" is simply both flags true). + assert.equal(userPublicUploadsEnabled(doc({ publicUploads: false, privateUploads: true })), false); + assert.equal(userPrivateUploadsEnabled(doc({ publicUploads: true, privateUploads: false })), false); + assert.equal(userPublicUploadsEnabled(doc({ publicUploads: true, privateUploads: true })), true); + assert.equal(userPrivateUploadsEnabled(doc({ publicUploads: true, privateUploads: true })), true); +}); diff --git a/remix/app/api/utils/auth/registerUser.ts b/remix/app/api/utils/auth/registerUser.ts index 22d66d279..c272335a7 100644 --- a/remix/app/api/utils/auth/registerUser.ts +++ b/remix/app/api/utils/auth/registerUser.ts @@ -36,9 +36,10 @@ export type CreateUserAccountInput = { accountKind?: 'user' | 'service'; emailVerificationRequiredBy?: Date | null; storageAllowanceBytes?: number; - // Opt a creation path INTO public uploads (admin-provisioned accounts only). - // Public signup never sets it — see the meta assignment below. + // Opt a creation path INTO an upload scope (admin-provisioned accounts + // only). Public signup never sets them — see the meta assignment below. publicUploads?: boolean; + privateUploads?: boolean; meta?: Record; }; @@ -47,10 +48,10 @@ export type CreateUserAccountResult = { ok: false; status: number; error: string const isEmail = (s: string) => /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(s); // Privileged meta keys that must never be set at account creation (only via -// their own admin-gated / authenticated endpoints). `publicUploads` joins -// `admin` here: a public signup body must not be able to hand itself the -// upload permission this hotfix exists to withhold. -const PRIVILEGED_META_KEYS = ['admin', 'publicUploads']; +// their own admin-gated / authenticated endpoints). `publicUploads` and +// `privateUploads` join `admin` here: a public signup body must not be able to +// hand itself the upload permissions this hotfix exists to withhold. +const PRIVILEGED_META_KEYS = ['admin', 'publicUploads', 'privateUploads']; // Drop privileged keys from any caller-supplied meta before it's persisted. const sanitizeCreateMeta = (meta: unknown): Record => { @@ -103,13 +104,19 @@ export const createUserAccount = async (input: CreateUserAccountInput): Promise< // Defense-in-depth: privileged flags can never be set at creation time, // even if a caller sneaks them into meta. `admin` is granted only via the // admin-gated setUserAdmin (auth/admin.ts). - // Public file/media uploads start WITHHELD for every newly created account - // — verifying the email address no longer grants them. An admin turns them - // on per user from /admin (POST /api/v1/admin/users/public-uploads) after - // the "new user" notification lands. `publicUploads` is stripped from any - // caller-supplied meta above, so this is the only writer at creation time. - // Accounts that predate the hotfix have no flag at all and stay enabled. - meta: { ...sanitizeCreateMeta(input.meta), publicUploads: input.publicUploads === true } + // File/media uploads start WITHHELD for every newly created account, in + // BOTH scopes (public = post/comment/emoji, private = messages + own + // profile media) — verifying the email address no longer grants either. An + // admin turns them on per user, per scope or all at once, from /admin + // (POST /api/v1/admin/users/public-uploads) after the "new user" + // notification lands. Both keys are stripped from any caller-supplied meta + // above, so this is the only writer at creation time. Accounts that + // predate the hotfix have no flags at all and stay enabled. + meta: { + ...sanitizeCreateMeta(input.meta), + publicUploads: input.publicUploads === true, + privateUploads: input.privateUploads === true + } }; if (input.emailVerificationRequiredBy !== undefined) { diff --git a/remix/app/api/utils/auth/users.ts b/remix/app/api/utils/auth/users.ts index 627451c7f..c8ea12539 100644 --- a/remix/app/api/utils/auth/users.ts +++ b/remix/app/api/utils/auth/users.ts @@ -101,11 +101,14 @@ export type PublicUser = { }; activeThemeId: string | null; activeFeedAlgorithmId: string | null; - // Public file/media uploads are OFF for every account created after the + // Upload permissions are OFF for every account created after the // signup-permissions hotfix; an admin turns them on per user from /admin - // (see setUserPublicUploads). Accounts predating the flag have no - // meta.publicUploads and stay enabled — absence means "grandfathered". + // (see setUserUploadPermissions), per scope or all at once. Accounts + // predating the flags have no meta keys and stay enabled — absence means + // "grandfathered". Public = post/comment/custom-emoji attachments; private + // = message attachments + own profile avatar/banner. publicUploadsEnabled: boolean; + privateUploadsEnabled: boolean; // true when meta.admin OR the ADMIN_USERNAMES env allowlist — the client uses // it to reveal the admin panel; the server always re-checks server-side. isAdmin: boolean; @@ -124,15 +127,19 @@ export type PublicProfile = { temporary?: boolean; }; -// Public file/media upload permission. Tri-state on purpose: -// meta.publicUploads === false → withheld (every account created since the -// signup-permissions hotfix starts here, -// INCLUDING after the email is verified) -// meta.publicUploads === true → granted by an admin from /admin -// absent → grandfathered account, still allowed -// Admins are always allowed regardless of the flag, so a locked-out admin can -// never be unable to fix the account that grants the permission. +// Upload permissions. Each scope is tri-state on purpose: +// meta. === false → withheld (every account created since the +// signup-permissions hotfix starts here, +// INCLUDING after the email is verified) +// meta. === true → granted by an admin from /admin +// absent → grandfathered account, still allowed +// Scopes: `publicUploads` covers publicly viewable surfaces (post, comment, +// custom-emoji attachments); `privateUploads` covers media only the account's +// own circles see (message attachments, own profile avatar/banner). "All" is +// simply both flags. Admins are always allowed regardless of the flags, so a +// locked-out admin can never be unable to fix the account that grants them. export const userPublicUploadsEnabled = (user: any): boolean => isAdminDoc(user) || user?.meta?.publicUploads !== false; +export const userPrivateUploadsEnabled = (user: any): boolean => isAdminDoc(user) || user?.meta?.privateUploads !== false; export const toPublicUser = (user: any, subscription?: SubscriptionInfo | null): PublicUser => { const source = subscription?.subjectType === 'user' ? subscription.storage : null; @@ -176,6 +183,7 @@ export const toPublicUser = (user: any, subscription?: SubscriptionInfo | null): activeThemeId: typeof user.meta?.activeThemeId === 'string' ? user.meta.activeThemeId : null, activeFeedAlgorithmId: typeof user.meta?.activeFeedAlgorithmId === 'string' ? user.meta.activeFeedAlgorithmId : null, publicUploadsEnabled: userPublicUploadsEnabled(user), + privateUploadsEnabled: userPrivateUploadsEnabled(user), isAdmin: isAdminDoc(user) }; }; @@ -1073,11 +1081,13 @@ export type AdminUserRow = { isAdmin: boolean; envAdmin: boolean; // admin via ADMIN_USERNAMES — can't be demoted from the UI emailVerified: boolean; - // false while the account waits for an admin to grant public uploads + // false while the account waits for an admin to grant that upload scope publicUploadsEnabled: boolean; + privateUploadsEnabled: boolean; // true only when the flag was explicitly withheld (i.e. a post-hotfix // signup), so the UI can tell "awaiting approval" from "grandfathered". publicUploadsPending: boolean; + privateUploadsPending: boolean; }; // Escape user-supplied text before embedding it in a Mongo $regex — shared with @@ -1100,7 +1110,9 @@ const toAdminRow = (doc: any): AdminUserRow => ({ envAdmin: isEnvAdmin(doc.username), emailVerified: !!doc.emailVerified, publicUploadsEnabled: userPublicUploadsEnabled(doc), - publicUploadsPending: doc?.meta?.publicUploads === false && !isAdminDoc(doc) + privateUploadsEnabled: userPrivateUploadsEnabled(doc), + publicUploadsPending: doc?.meta?.publicUploads === false && !isAdminDoc(doc), + privateUploadsPending: doc?.meta?.privateUploads === false && !isAdminDoc(doc) }); // Set (or clear) a user's stored admin flag. Env-allowlist admins remain admin @@ -1129,28 +1141,34 @@ export const setUserAdmin = async (userId: string, admin: boolean): Promise => { +export type UploadPermissionUpdates = { publicUploads?: boolean; privateUploads?: boolean }; +export const setUserUploadPermissions = async (userId: string, updates: UploadPermissionUpdates): Promise => { + const keys = (['publicUploads', 'privateUploads'] as const).filter((key) => typeof updates[key] === 'boolean'); + if (!keys.length) return null; let applied = false; const result = await mutateUserThingSecure(userId, (secure) => { - secure.meta = { ...(secure.meta || {}), publicUploads: enabled === true }; + const next = { ...(secure.meta || {}) }; + for (const key of keys) next[key] = updates[key] === true; + secure.meta = next; }); if (result === 'contended') throw new SecureWriteContendedError(userId); if (result === 'mutated') applied = true; if (ObjectId.isValid(userId)) { - const legacy = await ( - await getUsersCollection() - ).updateOne({ _id: new ObjectId(userId) }, { $set: { 'meta.publicUploads': enabled === true, updatedAt: new Date() } }); + const $set: Record = { updatedAt: new Date() }; + for (const key of keys) $set[`meta.${key}`] = updates[key] === true; + const legacy = await (await getUsersCollection()).updateOne({ _id: new ObjectId(userId) }, { $set }); if (legacy.matchedCount) applied = true; } diff --git a/remix/app/api/utils/email/templates.ts b/remix/app/api/utils/email/templates.ts index 03b2bdbbc..601987da6 100644 --- a/remix/app/api/utils/email/templates.ts +++ b/remix/app/api/utils/email/templates.ts @@ -66,18 +66,18 @@ export const renderNewUserAdminNotificationTemplate = ({ return { subject: `New Thingtime user: @${username}`, text: [ - `A new user verified their email and is awaiting public upload approval.`, + `A new user verified their email and is awaiting file/media upload approval (public and private uploads are both withheld).`, '', ...rows.map(([label, value]) => `${label}: ${value}`), '', - `Approve or leave withheld in the admin Users tab: ${adminUrl}` + `Approve public, private, or all uploads in the admin Users tab: ${adminUrl}` ].join('\n'), html: - `

A new user verified their email and is awaiting public upload approval.

` + + `

A new user verified their email and is awaiting file/media upload approval (public and private uploads are both withheld).

` + `${rows .map(([label, value]) => ``) .join('')}
${htmlEscape(label)}${htmlEscape(value)}
` + - `

Open the admin Users tab to enable their file and media uploads.

` + `

Open the admin Users tab to enable their public, private, or all file and media uploads.

` }; }; diff --git a/remix/app/components/Admin/AdminDashboard.tsx b/remix/app/components/Admin/AdminDashboard.tsx index 67e93acc0..f63c61882 100644 --- a/remix/app/components/Admin/AdminDashboard.tsx +++ b/remix/app/components/Admin/AdminDashboard.tsx @@ -7,6 +7,11 @@ import { Button, Flex, Heading, + Menu, + MenuButton, + MenuDivider, + MenuItem, + MenuList, Spinner, Tab, TabList, @@ -57,7 +62,9 @@ type UserRow = { envAdmin: boolean; emailVerified: boolean; publicUploadsEnabled: boolean; + privateUploadsEnabled: boolean; publicUploadsPending: boolean; + privateUploadsPending: boolean; accountKind: 'user' | 'service'; storage: AdminStorageProjection; storageAllowanceBytes: number | null; @@ -206,7 +213,9 @@ const USER_QUERY_FIELDS: readonly AdminRowField[] = [ { id: 'isAdmin', label: 'Administrator', kind: 'boolean', sortable: true }, { id: 'emailVerified', label: 'Email verified', kind: 'boolean', sortable: true }, { id: 'publicUploadsEnabled', label: 'Public uploads enabled', kind: 'boolean', sortable: true }, + { id: 'privateUploadsEnabled', label: 'Private uploads enabled', kind: 'boolean', sortable: true }, { id: 'publicUploadsPending', label: 'Public uploads awaiting approval', kind: 'boolean', sortable: true }, + { id: 'privateUploadsPending', label: 'Private uploads awaiting approval', kind: 'boolean', sortable: true }, { id: 'envAdmin', label: 'Environment administrator', kind: 'boolean', sortable: true }, { id: 'storage.usedBytes', label: 'Account storage used (bytes)', kind: 'number', sortable: true }, { id: 'storage.allowanceBytes', label: 'Account storage allowance (bytes)', kind: 'number', sortable: true }, @@ -362,34 +371,41 @@ const SnapshotErrorNotice = ({ hasPreviousRows, onRetry }: { hasPreviousRows: bo ); -// Public file/media uploads are withheld from every account created since the -// signup-permissions hotfix — verifying an email address no longer grants them. -// This is the manual approval control: one row-scoped toggle that hits -// POST /api/v1/admin/users/public-uploads and refreshes the snapshot. +// File/media uploads are withheld from every account created since the +// signup-permissions hotfix — verifying an email address no longer grants +// them. This is the manual approval control, per scope: PUBLIC covers +// post/comment/custom-emoji attachments, PRIVATE covers message attachments + +// the user's own profile media, and "all" is both at once. Each action hits +// POST /api/v1/admin/users/public-uploads { userId, enabled, scope } and +// refreshes the snapshot. // // Optimistic per the UI house rule: the badge flips the moment the admin // clicks and reverts if the request fails, so approving never shows a spinner // where a known state already exists. -const PublicUploadsControl = ({ row, onChanged }: { row: UserRow; onChanged: () => void }) => { +const UploadApprovalsControl = ({ row, onChanged }: { row: UserRow; onChanged: () => void }) => { const api = useApi(); const lopu = useLopu(); - const [optimistic, setOptimistic] = React.useState(null); + const [optimistic, setOptimistic] = React.useState<{ pub: boolean; priv: boolean } | null>(null); const [saving, setSaving] = React.useState(false); - const enabled = optimistic ?? row.publicUploadsEnabled; + const pub = optimistic?.pub ?? row.publicUploadsEnabled; + const priv = optimistic?.priv ?? row.privateUploadsEnabled; // A fresh snapshot is authoritative again — drop the local override. React.useEffect(() => { setOptimistic(null); - }, [row.publicUploadsEnabled]); + }, [row.publicUploadsEnabled, row.privateUploadsEnabled]); - const toggle = async () => { - const next = !enabled; - setOptimistic(next); + const save = async (scope: 'public' | 'private' | 'all', enabled: boolean) => { + setOptimistic({ pub: scope === 'private' ? pub : enabled, priv: scope === 'public' ? priv : enabled }); setSaving(true); try { - const result = await api.v1.admin.setUserPublicUploads({ userId: row.id, enabled: next }); + const result = await api.v1.admin.setUserPublicUploads({ userId: row.id, enabled, scope }); if (result?.ok === false) throw new Error(result.error || 'Request failed'); - lopu({ title: next ? `Public uploads enabled for @${row.username} 🎉` : `Public uploads withheld for @${row.username}` }); + lopu({ + title: enabled + ? `${scope === 'all' ? 'All' : scope === 'public' ? 'Public' : 'Private'} uploads enabled for @${row.username} 🎉` + : `${scope === 'all' ? 'All' : scope === 'public' ? 'Public' : 'Private'} uploads withheld for @${row.username}` + }); onChanged(); } catch (error: any) { setOptimistic(null); @@ -399,19 +415,40 @@ const PublicUploadsControl = ({ row, onChanged }: { row: UserRow; onChanged: () } }; + const pending = row.publicUploadsPending || row.privateUploadsPending; + const summary = pub && priv ? 'all' : pub ? 'public' : priv ? 'private' : pending ? 'pending' : 'off'; + const summaryColor = pub && priv ? 'green' : pub || priv ? 'teal' : pending ? 'orange' : 'gray'; + return ( - - {enabled ? 'enabled' : row.publicUploadsPending ? 'pending' : 'off'} + + {summary} {row.isAdmin ? ( admin ) : ( - + + + Approve ▾ + + + save('public', !pub)}>{pub ? 'Withhold public uploads' : 'Enable public uploads'} + save('private', !priv)}>{priv ? 'Withhold private uploads' : 'Enable private uploads'} + + save('all', true)}> + Enable all + + save('all', false)}> + Withhold all + + + )} ); @@ -439,7 +476,10 @@ const UsersTab = () => { }); const [subscriptionFor, setSubscriptionFor] = React.useState(null); const [linksFor, setLinksFor] = React.useState(null); - const pendingUploadCount = React.useMemo(() => (rows ?? []).filter((row) => row.publicUploadsPending).length, [rows]); + const pendingUploadCount = React.useMemo( + () => (rows ?? []).filter((row) => row.publicUploadsPending || row.privateUploadsPending).length, + [rows] + ); return ( @@ -457,8 +497,8 @@ const UsersTab = () => { {pendingUploadCount > 0 && ( - {pendingUploadCount} new {pendingUploadCount === 1 ? 'account is' : 'accounts are'} awaiting public file & media - upload approval — sort or search by uploads below and use Enable. + {pendingUploadCount} new {pendingUploadCount === 1 ? 'account is' : 'accounts are'} awaiting file & media upload + approval — use Approve on a row to enable public, private, or all uploads. )} {error ? : null} @@ -508,7 +548,7 @@ const UsersTab = () => { - + {formatAdminDate(row.createdAt)} diff --git a/remix/app/docs/apiDocs.ts b/remix/app/docs/apiDocs.ts index 98c7f6f30..586fc3ba1 100644 --- a/remix/app/docs/apiDocs.ts +++ b/remix/app/docs/apiDocs.ts @@ -867,33 +867,42 @@ export const apiEndpointDocs: ApiEndpointDoc[] = [ endpoint({ id: 'admin-users-public-uploads', group: 'admin', - title: 'Approve public uploads', + title: 'Approve uploads (public / private / all)', endpoint: '/api/v1/admin/users/public-uploads', - summary: 'Grant or withhold a user’s public file and media upload permission (admin only).', + summary: 'Grant or withhold a user’s file and media upload permissions, per scope or all at once (admin only).', detail: - 'POST { userId, enabled } to set meta.publicUploads. Accounts created after the signup-permissions hotfix start ' + - 'withheld — verifying their email address does NOT grant uploads — so this endpoint is the manual approval step ' + - 'an admin performs after the “new user” notification email. While withheld, POST /api/v1/attachments/uploads ' + - 'returns 403 public_uploads_not_approved and no upload can start. Accounts that predate the flag have no ' + - 'meta.publicUploads and remain enabled; admins are always allowed regardless of the flag.', + 'POST { userId, enabled, scope } to set meta.publicUploads and/or meta.privateUploads. scope is ' + + "'public' (post/comment/custom-emoji attachments — the default when omitted), 'private' (message attachments + " + + "the user's own profile avatar/banner), or 'all' (both flags in one write). Accounts created after the " + + 'signup-permissions hotfix start with BOTH scopes withheld — verifying their email address does NOT grant ' + + 'uploads — so this endpoint is the manual approval step an admin performs after the “new user” notification ' + + 'email. While a scope is withheld, POST /api/v1/attachments/uploads returns 403 public_uploads_not_approved or ' + + 'private_uploads_not_approved for purposes in that scope and no upload can start. Accounts that predate the ' + + 'flags have no meta keys and remain enabled; admins are always allowed regardless of the flags.', auth: { mode: 'session', description: 'Requires an admin session (isAdmin).' }, methods: ['POST'], steps: [ - 'POST userId + enabled:true to approve uploads, enabled:false to withhold them again.', - 'Read the returned user row (publicUploadsEnabled, publicUploadsPending) to update the UI.', - 'The /admin Users tab lists pending accounts — publicUploadsPending is true while approval is outstanding.', - 'Non-admins receive 403; missing userId or a non-boolean enabled 400; unknown user 404.' + "POST userId + enabled:true + scope ('public' | 'private' | 'all') to approve that variation; enabled:false withholds it again.", + 'Read the returned user row (publicUploadsEnabled, privateUploadsEnabled, publicUploadsPending, privateUploadsPending) to update the UI.', + 'The /admin Users tab lists pending accounts — a *Pending flag is true while that scope’s approval is outstanding.', + "Non-admins receive 403; missing userId, a non-boolean enabled, or an unknown scope 400; unknown user 404." ], requestExamples: [ { - name: 'Approve uploads', - description: 'Enable public file and media uploads for a new user.', + name: 'Approve all uploads', + description: 'Enable public AND private file and media uploads for a vetted new user.', method: 'POST', - body: { userId: '64f000000000000000000002', enabled: true } + body: { userId: '64f000000000000000000002', enabled: true, scope: 'all' } }, { - name: 'Withhold uploads', - description: 'Revoke the permission again.', + name: 'Approve private only', + description: 'Let the user set profile media and attach in DMs while public uploads stay withheld.', + method: 'POST', + body: { userId: '64f000000000000000000002', enabled: true, scope: 'private' } + }, + { + name: 'Withhold public uploads', + description: 'Revoke the public variation again (scope defaults to public when omitted).', method: 'POST', body: { userId: '64f000000000000000000002', enabled: false } } @@ -909,7 +918,9 @@ export const apiEndpointDocs: ApiEndpointDoc[] = [ username: 'nik', emailVerified: true, publicUploadsEnabled: true, - publicUploadsPending: false + privateUploadsEnabled: true, + publicUploadsPending: false, + privateUploadsPending: false } } }, diff --git a/remix/app/hooks/useApi.tsx b/remix/app/hooks/useApi.tsx index 282606fb7..315528dfd 100644 --- a/remix/app/hooks/useApi.tsx +++ b/remix/app/hooks/useApi.tsx @@ -171,12 +171,12 @@ export function useApi() { [asyncFetcher] ), setUserPublicUploads: useCallback( - async (args: { userId: string; enabled: boolean }) => + async (args: { userId: string; enabled: boolean; scope?: 'public' | 'private' | 'all' }) => asyncFetcher.submit( - { userId: args?.userId, enabled: args?.enabled }, + { userId: args?.userId, enabled: args?.enabled, scope: args?.scope ?? 'public' }, { action: '/api/v1/admin/users/public-uploads', - errorContext: args?.enabled ? 'approve public uploads' : 'withhold public uploads' + errorContext: `${args?.enabled ? 'approve' : 'withhold'} ${args?.scope ?? 'public'} uploads` } ), [asyncFetcher] diff --git a/remix/app/routes/api/v1/admin/users/public-uploads/_public-uploads.tsx b/remix/app/routes/api/v1/admin/users/public-uploads/_public-uploads.tsx index addd98f4d..6cbcffeea 100644 --- a/remix/app/routes/api/v1/admin/users/public-uploads/_public-uploads.tsx +++ b/remix/app/routes/api/v1/admin/users/public-uploads/_public-uploads.tsx @@ -1,12 +1,16 @@ import { json, readJsonBody } from '~/api/http'; import { requireAdmin } from '~/api/utils/auth/requireAdmin'; -import { setUserPublicUploads } from '~/api/utils/auth/users'; +import { setUserUploadPermissions } from '~/api/utils/auth/users'; -// POST /api/v1/admin/users/public-uploads — { userId, enabled } — grant or -// withhold a user's public file/media upload permission (meta.publicUploads). -// New signups start withheld, so this is the manual approval step an admin -// performs after the "new user" notification email. Admin only. +// POST /api/v1/admin/users/public-uploads — { userId, enabled, scope? } — +// grant or withhold a user's file/media upload permissions +// (meta.publicUploads / meta.privateUploads). scope selects the variation: +// 'public' (post/comment/emoji — the default, preserving the pre-scope body +// shape), 'private' (messages + own profile media), or 'all' (both flags in +// one write). New signups start with both withheld, so this is the manual +// approval step an admin performs after the "new user" notification email. +// Admin only. export const action = async ({ request }: { request: Request }) => { const gate = await requireAdmin(request); if ('error' in gate) return json({ ok: false, error: gate.error.message }, { status: gate.error.status }); @@ -15,8 +19,15 @@ export const action = async ({ request }: { request: Request }) => { const userId = typeof body?.userId === 'string' ? body.userId : ''; if (!userId) return json({ ok: false, error: 'userId is required' }, { status: 400 }); if (typeof body?.enabled !== 'boolean') return json({ ok: false, error: 'enabled must be a boolean' }, { status: 400 }); + const scope = body?.scope === undefined ? 'public' : body.scope; + if (scope !== 'public' && scope !== 'private' && scope !== 'all') { + return json({ ok: false, error: "scope must be 'public', 'private', or 'all'" }, { status: 400 }); + } - const row = await setUserPublicUploads(userId, body.enabled); + const row = await setUserUploadPermissions(userId, { + ...(scope !== 'private' ? { publicUploads: body.enabled } : {}), + ...(scope !== 'public' ? { privateUploads: body.enabled } : {}) + }); if (!row) return json({ ok: false, error: 'User not found' }, { status: 404 }); return json({ ok: true, user: row }); }; diff --git a/remix/app/routes/api/v1/attachments/attachmentsRoutes.test.ts b/remix/app/routes/api/v1/attachments/attachmentsRoutes.test.ts index 028f9a865..50b7d092f 100644 --- a/remix/app/routes/api/v1/attachments/attachmentsRoutes.test.ts +++ b/remix/app/routes/api/v1/attachments/attachmentsRoutes.test.ts @@ -51,11 +51,14 @@ test('same-origin mutations honor the proxy-owned public origin and still fail c ); }); -// Signup-permissions hotfix: a brand-new account (publicUploadsEnabled false, -// even once its email is verified) must not be able to START an upload, while -// an approved account is unaffected — and routes that DON'T opt in stay open so -// an in-flight upload can still be completed or cancelled after a revoke. -test('upload starts require the account public-upload permission', async () => { +// Signup-permissions hotfix: a brand-new account (both upload scopes withheld, +// even once its email is verified) must not be able to START an upload, and +// the requested purpose decides WHICH scope gates it (public = +// post/comment/custom-emoji, private = message/profile media; "all" is both +// flags). Approved scopes are unaffected — and routes that DON'T opt in stay +// open so an in-flight upload can still be completed or cancelled after a +// revoke. +test('upload starts require the upload-permission scope matching the purpose', async () => { let serviceCalls = 0; const gated = (viewer: any) => createAttachmentMutationAction( @@ -65,22 +68,52 @@ test('upload starts require the account public-upload permission', async () => { serviceCalls += 1; return { ok: true }; }, - requirePublicUploads: true + requireUploadPermission: true }, { getUser: async () => viewer, enforceLimit: allowed as any } ); - const pending = { id: 'user-new', accountKind: 'user', emailVerified: true, publicUploadsEnabled: false } as any; + const pending = { + id: 'user-new', + accountKind: 'user', + emailVerified: true, + publicUploadsEnabled: false, + privateUploadsEnabled: false + } as any; + // no purpose defaults to 'post' — a public surface const denied = await gated(pending)({ request: post({}) }); assert.equal(denied.status, 403); assert.equal(serviceCalls, 0); const deniedBody = await denied.json(); assert.equal(deniedBody.code, 'public_uploads_not_approved'); assert.equal(denied.headers.get('Cache-Control'), 'private, no-store, max-age=0'); + for (const purpose of ['post', 'comment', 'custom-emoji']) { + const res = await gated(pending)({ request: post({ purpose }) }); + assert.equal(res.status, 403, `public purpose ${purpose} not gated`); + assert.equal((await res.json()).code, 'public_uploads_not_approved'); + } + for (const purpose of ['message', 'profile-avatar', 'profile-banner']) { + const res = await gated(pending)({ request: post({ purpose }) }); + assert.equal(res.status, 403, `private purpose ${purpose} not gated`); + assert.equal((await res.json()).code, 'private_uploads_not_approved'); + } + assert.equal(serviceCalls, 0); + + // each scope grants ONLY its own purposes — "all" is simply both flags + const publicOnly = { id: 'user-pub', accountKind: 'user', publicUploadsEnabled: true, privateUploadsEnabled: false } as any; + assert.equal((await gated(publicOnly)({ request: post({ purpose: 'post' }) })).status, 200); + assert.equal((await gated(publicOnly)({ request: post({ purpose: 'message' }) })).status, 403); + const privateOnly = { id: 'user-priv', accountKind: 'user', publicUploadsEnabled: false, privateUploadsEnabled: true } as any; + assert.equal((await gated(privateOnly)({ request: post({ purpose: 'profile-avatar' }) })).status, 200); + assert.equal((await gated(privateOnly)({ request: post({ purpose: 'comment' }) })).status, 403); + const approvedAll = { id: 'user-ok', accountKind: 'user', publicUploadsEnabled: true, privateUploadsEnabled: true } as any; + assert.equal((await gated(approvedAll)({ request: post({}) })).status, 200); + assert.equal((await gated(approvedAll)({ request: post({ purpose: 'message' }) })).status, 200); + assert.equal(serviceCalls, 4); - const approved = { id: 'user-ok', accountKind: 'user', publicUploadsEnabled: true } as any; - assert.equal((await gated(approved)({ request: post({}) })).status, 200); - assert.equal(serviceCalls, 1); + // an unknown purpose reaches the service's own validation (no scope gates it) + assert.equal((await gated(pending)({ request: post({ purpose: 'nonsense' }) })).status, 200); + assert.equal(serviceCalls, 5); // Lifecycle routes (parts/complete/abort/delete) never opt in, so a // permission flipped off mid-upload can't strand a reserved MPU. diff --git a/remix/app/routes/api/v1/attachments/uploads/_uploads.tsx b/remix/app/routes/api/v1/attachments/uploads/_uploads.tsx index d2851b533..5ab54b87c 100644 --- a/remix/app/routes/api/v1/attachments/uploads/_uploads.tsx +++ b/remix/app/routes/api/v1/attachments/uploads/_uploads.tsx @@ -2,13 +2,14 @@ import { createAttachmentMutationAction, attachmentPostOnlyLoader } from '~/api/ import { startAttachmentUpload } from '~/api/utils/attachments/attachments'; // POST /api/v1/attachments/uploads — reserve quota and create a private MPU. -// Requires the account's public file/media upload permission: new signups start -// without it (even once their email is verified) until an admin grants it from -// the /admin Users tab. +// Requires the upload permission scope matching the requested purpose (public +// = post/comment/custom-emoji, private = message/profile media): new signups +// start with both withheld (even once their email is verified) until an admin +// grants them — per scope or all — from the /admin Users tab. export const action = createAttachmentMutationAction({ rateKey: 'attachments.start', service: startAttachmentUpload, - requirePublicUploads: true + requireUploadPermission: true }); export const loader = attachmentPostOnlyLoader;