From ff815f730d7d07323b0b6d64d526a97e5bee0abe Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Wed, 14 Oct 2026 06:56:44 -0700 Subject: [PATCH] fix: harden token delivery, session revokes, app identity and fs scoping (#4229) - relay sign-in popups take the opener origin from the referrer only - session revokes read the primary, batch per user, and tombstone caches in every region - wisp relay tokens bound to the minting session; verify rechecks the account - hosted origins keep app identity with the site owner; deleted app uids recorded in deleted_apps - subdomain cache tombstoned on delete, update, reclaim and app/account cascades - email lookups require an exact address match after the SQL lookup - workers deploy only from files the deployer owns - share_app_data:false enforced at ACL time, plus copy/move/rename/share/recursive listing - kv remove returns no value without read; kv:expire op grant usable - fs errors name the entry instead of the owner's path - shared-by-me drops rows a delegate no longer has authority over - private apps can't move index_url onto another user's origin - GUI: default apps saved and launched by uid; save picker accepts only copy/move; permission dialog waits for focus; Publish as Worker hidden on others' files --- .../database/SqliteDatabaseClient.test.ts | 2 +- .../clients/database/SqliteDatabaseClient.ts | 1 + .../migrations/mysql/mysql_mig_48.sql | 26 ++ .../migrations/postgres/postgres_mig_37.sql | 26 ++ .../migrations/sqlite/0094_deleted-apps.sql | 25 ++ src/backend/controllers/fs/FSController.ts | 16 +- src/backend/controllers/fs/legacyFsHelpers.ts | 2 +- .../fs/sharedFolderWrites.http.test.ts | 53 +++ .../share/ShareController.http.test.ts | 55 +++ .../controllers/wisp/WispController.test.ts | 120 ++++++- .../controllers/wisp/WispController.ts | 51 ++- src/backend/drivers/apps/AppDriver.js | 16 + src/backend/drivers/apps/AppDriver.test.ts | 72 ++++ src/backend/drivers/kv/KVStoreDriver.test.ts | 69 ++++ src/backend/drivers/kv/KVStoreDriver.ts | 12 + src/backend/drivers/util/fileInput.ts | 6 +- .../workers/WorkerDriver.cloudflare.test.ts | 99 +++++- src/backend/drivers/workers/WorkerDriver.ts | 46 ++- src/backend/services/acl/ACLService.test.ts | 125 ++++++- src/backend/services/acl/ACLService.ts | 16 +- .../auth/AuthService.appDeletion.test.ts | 192 ++++++++-- src/backend/services/auth/AuthService.test.ts | 266 ++++++++++++++ src/backend/services/auth/AuthService.ts | 234 ++++++++++--- src/backend/services/fs/FSService.test.ts | 104 +++++- src/backend/services/fs/FSService.ts | 87 +++-- .../localworker/LocalWorkerService.test.ts | 30 ++ .../localworker/LocalWorkerService.ts | 5 + .../services/permission/appDataScopes.ts | 93 +++++ .../services/share/ShareService.test.ts | 146 ++++++++ src/backend/services/share/ShareService.ts | 102 +++--- .../services/user/SignupService.test.ts | 27 ++ src/backend/services/user/SignupService.ts | 21 +- .../services/user/UserAccountService.test.ts | 47 +++ .../services/user/UserAccountService.ts | 9 + src/backend/stores/app/AppStore.js | 59 +++- src/backend/stores/app/AppStore.test.js | 84 +++++ src/backend/stores/fs/FSEntryStore.test.ts | 2 +- src/backend/stores/fs/FSEntryStore.ts | 52 +-- src/backend/stores/session/SessionStore.js | 215 ++++++++---- .../stores/session/SessionStore.test.ts | 252 ++++++++++++++ src/backend/stores/share/ShareStore.js | 6 +- src/backend/stores/share/ShareStore.test.js | 38 ++ .../stores/subdomain/SubdomainStore.test.ts | 328 +++++++++++++++++- .../stores/subdomain/SubdomainStore.ts | 223 +++++++++--- src/backend/stores/user/UserStore.test.ts | 109 ++++++ src/backend/stores/user/UserStore.ts | 44 ++- src/backend/util/email.test.ts | 23 +- src/backend/util/email.ts | 14 + src/docs/src/Email/sendTransactional.md | 6 +- src/docs/src/KV/remove.md | 2 +- src/gui/src/IPC.js | 15 +- src/gui/src/UI/UIDesktop.js | 10 +- src/gui/src/UI/UIItem.js | 8 +- src/gui/src/UI/UIPermissionDialog.js | 141 +++++--- src/gui/src/UI/UIPermissionDialog.test.js | 123 ++++++- src/gui/src/helpers/defaultApp.js | 56 +++ src/gui/src/helpers/defaultApp.test.js | 146 ++++++++ .../src/helpers/generateFileContextMenu.js | 8 +- src/gui/src/helpers/getAppByUid.js | 46 +++ src/gui/src/helpers/openItem.js | 32 +- src/gui/src/initgui.js | 35 +- src/gui/src/util/popupAuth.js | 37 ++ src/gui/src/util/popupAuth.test.js | 77 ++++ src/puter-js/src/modules/kv/remove.js | 3 +- 64 files changed, 3934 insertions(+), 461 deletions(-) create mode 100644 src/backend/clients/database/migrations/mysql/mysql_mig_48.sql create mode 100644 src/backend/clients/database/migrations/postgres/postgres_mig_37.sql create mode 100644 src/backend/clients/database/migrations/sqlite/0094_deleted-apps.sql create mode 100644 src/gui/src/helpers/defaultApp.js create mode 100644 src/gui/src/helpers/defaultApp.test.js create mode 100644 src/gui/src/helpers/getAppByUid.js diff --git a/src/backend/clients/database/SqliteDatabaseClient.test.ts b/src/backend/clients/database/SqliteDatabaseClient.test.ts index 8e3be0f58..175889866 100644 --- a/src/backend/clients/database/SqliteDatabaseClient.test.ts +++ b/src/backend/clients/database/SqliteDatabaseClient.test.ts @@ -27,7 +27,7 @@ import { DatabaseClientFactory } from './index.js'; import { SqliteDatabaseClient } from './SqliteDatabaseClient.js'; /** Highest schema version the migration table can reach. */ -const CURRENT_SCHEMA_VERSION = 89; +const CURRENT_SCHEMA_VERSION = 90; /** * These suites migrate real files on disk. Idle they finish in well under a diff --git a/src/backend/clients/database/SqliteDatabaseClient.ts b/src/backend/clients/database/SqliteDatabaseClient.ts index 383fce1a2..21ec4c26a 100644 --- a/src/backend/clients/database/SqliteDatabaseClient.ts +++ b/src/backend/clients/database/SqliteDatabaseClient.ts @@ -128,6 +128,7 @@ const AVAILABLE_MIGRATIONS: [number, string[]][] = [ [86, ['0091_icloud-dot-significance.sql']], [87, ['0092_share-entry-owner.sql']], [88, ['0093_fsentries-parent-uid-index.sql']], + [89, ['0094_deleted-apps.sql']], ]; export class SqliteDatabaseClient extends AbstractDatabaseClient { diff --git a/src/backend/clients/database/migrations/mysql/mysql_mig_48.sql b/src/backend/clients/database/migrations/mysql/mysql_mig_48.sql new file mode 100644 index 000000000..7cf68058f --- /dev/null +++ b/src/backend/clients/database/migrations/mysql/mysql_mig_48.sql @@ -0,0 +1,26 @@ +-- Copyright (C) 2024-present Puter Technologies Inc. +-- +-- This file is part of Puter. +-- +-- Puter is free software: you can redistribute it and/or modify +-- it under the terms of the GNU Affero General Public License as published +-- by the Free Software Foundation, either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU Affero General Public License for more details. +-- +-- You should have received a copy of the GNU Affero General Public License +-- along with this program. If not, see . + +-- Uid and owner of each deleted app. Mirrors SQLite migration 0094. +-- +-- Idempotent: `CREATE TABLE IF NOT EXISTS` lets the directory replay safely. + +CREATE TABLE IF NOT EXISTS `deleted_apps` ( + `uid` VARCHAR(40) NOT NULL, + `owner_user_id` INT UNSIGNED DEFAULT NULL, + PRIMARY KEY (`uid`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; diff --git a/src/backend/clients/database/migrations/postgres/postgres_mig_37.sql b/src/backend/clients/database/migrations/postgres/postgres_mig_37.sql new file mode 100644 index 000000000..b98f1b50e --- /dev/null +++ b/src/backend/clients/database/migrations/postgres/postgres_mig_37.sql @@ -0,0 +1,26 @@ +-- Copyright (C) 2024-present Puter Technologies Inc. +-- +-- This file is part of Puter. +-- +-- Puter is free software: you can redistribute it and/or modify +-- it under the terms of the GNU Affero General Public License as published +-- by the Free Software Foundation, either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU Affero General Public License for more details. +-- +-- You should have received a copy of the GNU Affero General Public License +-- along with this program. If not, see . + +-- Uid and owner of each deleted app. Mirrors SQLite migration 0094 / MySQL +-- mysql_mig_48. +-- +-- Idempotent via IF NOT EXISTS. + +CREATE TABLE IF NOT EXISTS deleted_apps ( + uid VARCHAR(40) PRIMARY KEY, + owner_user_id INTEGER DEFAULT NULL +); diff --git a/src/backend/clients/database/migrations/sqlite/0094_deleted-apps.sql b/src/backend/clients/database/migrations/sqlite/0094_deleted-apps.sql new file mode 100644 index 000000000..a63377d7f --- /dev/null +++ b/src/backend/clients/database/migrations/sqlite/0094_deleted-apps.sql @@ -0,0 +1,25 @@ +-- Copyright (C) 2024-present Puter Technologies Inc. +-- +-- This file is part of Puter. +-- +-- Puter is free software: you can redistribute it and/or modify +-- it under the terms of the GNU Affero General Public License as published +-- by the Free Software Foundation, either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU Affero General Public License for more details. +-- +-- You should have received a copy of the GNU Affero General Public License +-- along with this program. If not, see . + +-- Uid and owner of each deleted app. An origin-derived uid comes back when the +-- origin is used again; on a hosted subdomain now registered to someone else, +-- AuthService moves on to a fresh uid so the deleted app's data stays put. + +CREATE TABLE IF NOT EXISTS `deleted_apps` ( + "uid" TEXT PRIMARY KEY NOT NULL, + "owner_user_id" INTEGER DEFAULT NULL +); diff --git a/src/backend/controllers/fs/FSController.ts b/src/backend/controllers/fs/FSController.ts index c1072461e..873a24a7d 100644 --- a/src/backend/controllers/fs/FSController.ts +++ b/src/backend/controllers/fs/FSController.ts @@ -75,6 +75,7 @@ import { type GuiItemEventName, loadLegacyAssociatedApps, replacedByMove, + resolveOrNotFound, signEntryThumbnail, } from './legacyFsHelpers.js'; import type { @@ -1790,7 +1791,6 @@ export class FSController extends PuterController { } async #resolveEntryForRequest(source: Record) { - const mod = await import('../../services/fs/resolveNode.js'); const username = Context.get('actor')?.user?.username; const rawPath = typeof source.path === 'string' ? source.path : undefined; @@ -1810,15 +1810,8 @@ export class FSController extends PuterController { ? source.id : undefined, }; - const entry = await mod.resolveNode(this.stores.fsEntry, ref, { - required: true, - }); - if (!entry) { - throw new HttpError(404, 'Entry not found', { - legacyCode: 'not_found', - }); - } - return entry; + // The resolved path is the owner's real one, so a miss must not echo it. + return resolveOrNotFound(this.stores.fsEntry, ref); } async #assertCanDedupeCreate(actor: Actor, targetPath: string) { @@ -2240,7 +2233,8 @@ export class FSController extends PuterController { // the ActorUser type. Access via the escape hatch until a proper // storage-quota mechanism is in place. const actorUser = req.actor?.user as - Record | undefined; + | Record + | undefined; const candidates = [ this.#toStorageCapacityCandidate(actorUser?.free_storage), diff --git a/src/backend/controllers/fs/legacyFsHelpers.ts b/src/backend/controllers/fs/legacyFsHelpers.ts index 4d2a7c177..86c1f3768 100644 --- a/src/backend/controllers/fs/legacyFsHelpers.ts +++ b/src/backend/controllers/fs/legacyFsHelpers.ts @@ -129,7 +129,7 @@ const MAX_SELECTOR_DEPTH = 4; * Resolve `ref`, reporting any miss as the same 404 an unreadable entry gets, * so a lookup never echoes a resolved path or tells missing from hidden. */ -async function resolveOrNotFound( +export async function resolveOrNotFound( fsEntryStore: FSEntryStore, ref: NodeRef, ): Promise { diff --git a/src/backend/controllers/fs/sharedFolderWrites.http.test.ts b/src/backend/controllers/fs/sharedFolderWrites.http.test.ts index 63b3bc0a9..de4c1f037 100644 --- a/src/backend/controllers/fs/sharedFolderWrites.http.test.ts +++ b/src/backend/controllers/fs/sharedFolderWrites.http.test.ts @@ -194,4 +194,57 @@ describe('writes inside a folder shared with write', () => { (moved.json!.moved as { path: string }).path, ).toBe(`${s.maskedDir}/renamed.txt`); }, 180_000); + + // The recipient only knows the masked path, so refusals name entries the + // way the recipient addressed them. + it('keeps the owner’s real path out of error messages', async () => { + const s = await shareAFolder(); + await post('/touch', s.owner.token, { + path: `${s.realDir}/other.txt`, + }); + + const cases = [ + { + res: await post('/fs/stat', s.recipient.token, { + path: `${s.maskedDir}/missing.txt`, + }), + status: 404, + message: 'Subject does not exist', + }, + { + res: await post('/fs/mkdir', s.recipient.token, { + path: `${s.maskedDir}/note.txt`, + }), + status: 409, + message: 'An entry named `note.txt` already exists', + }, + { + res: await post('/fs/mkdir', s.recipient.token, { + path: `${s.maskedDir}/missing/child`, + }), + status: 404, + message: 'Parent path does not exist: missing', + }, + { + res: await post('/fs/rename', s.recipient.token, { + uid: s.noteUid, + new_name: 'other.txt', + }), + status: 409, + message: 'An entry named `other.txt` already exists', + }, + { + res: await post('/mkdir', s.recipient.token, { + path: `${s.maskedDir}/note.txt`, + }), + status: 409, + message: 'An entry named `note.txt` already exists', + }, + ]; + for (const { res, status, message } of cases) { + expect(res.status).toBe(status); + expect(res.json!.message).toBe(message); + expect(JSON.stringify(res.json)).not.toContain(s.realDir); + } + }, 180_000); }); diff --git a/src/backend/controllers/share/ShareController.http.test.ts b/src/backend/controllers/share/ShareController.http.test.ts index 047aacd91..d89b9bd6d 100644 --- a/src/backend/controllers/share/ShareController.http.test.ts +++ b/src/backend/controllers/share/ShareController.http.test.ts @@ -341,6 +341,61 @@ describe('share endpoints over HTTP', () => { expect(replayed.items).toEqual([]); }); + it('drops what a folder delegate re-shared once the item leaves the folder', async () => { + const owner = await makeUser(); + const delegate = await makeUser(); + const holder = await makeUser(); + + const dir = (await ( + await post('/mkdir', owner.token, { + path: `/${owner.username}/Proj`, + }) + ).json()) as { uid: string }; + await post('/touch', owner.token, { + path: `/${owner.username}/Proj/plan.txt`, + }); + const file = (await ( + await post('/stat', owner.token, { + path: `/${owner.username}/Proj/plan.txt`, + }) + ).json()) as { uid: string }; + + await post('/share', owner.token, { + recipients: [delegate.username], + items: [{ uid: dir.uid }], + mode: 'manage', + }); + await post('/share', delegate.token, { + recipients: [holder.username], + items: [{ uid: file.uid }], + mode: 'read', + }); + const listedTo = async (token: string) => + ( + (await ( + await get('/share/shared-by-me', token, {}) + ).json()) as { items: Array<{ uid_entry: string }> } + ).items.map((i) => i.uid_entry); + expect(await listedTo(delegate.token)).toContain(file.uid); + + const moved = await post('/move', owner.token, { + source: file.uid, + destination: `/${owner.username}`, + }); + expect(moved.status).toBe(200); + // A move doesn't bump readings; this stands in for the scan-cache TTL. + const delegateRow = await env.server.stores.user.getByUsername( + delegate.username, + ); + await env.server.stores.permission.bumpCacheGeneration( + `user:${delegateRow!.uuid}`, + ); + + expect(await listedTo(delegate.token)).not.toContain(file.uid); + // The holder's grant still stands, so its owner still sees it. + expect(await listedTo(owner.token)).toContain(file.uid); + }); + // A caller who believes they filtered must not silently receive // everything: a duplicated param arrives as an array, and an empty // string names nothing. diff --git a/src/backend/controllers/wisp/WispController.test.ts b/src/backend/controllers/wisp/WispController.test.ts index 1f84d1a7e..d37e2da19 100644 --- a/src/backend/controllers/wisp/WispController.test.ts +++ b/src/backend/controllers/wisp/WispController.test.ts @@ -4,7 +4,7 @@ import type { Request, Response } from 'express'; import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { PuterRouter } from '../../core/http/PuterRouter.js'; import { PuterServer } from '../../server.js'; -import { setupTestServer } from '../../testUtil.js'; +import { createTestUser, setupTestServer } from '../../testUtil.js'; import type { WispController } from './WispController.js'; let server: PuterServer; @@ -147,26 +147,114 @@ describe('WispController', () => { }); describe('verify', () => { - it('verifies a valid authenticated-user token', async () => { - const { res: createRes, captured: createCaptured } = makeRes(); - await createHandler( - makeReq({ - actor: { - user: { - uuid: '00000000-0000-0000-0000-000000000001', - }, - }, - }), - createRes, - ); - const token = (createCaptured.body as { token: string }).token; + const relayTokenFor = async (actor: unknown) => { + const { res, captured } = makeRes(); + await createHandler(makeReq({ actor }), res); + return (captured.body as { token: string }).token; + }; + + const verifyStatus = async (token: string) => { + const { res, captured } = makeRes(); + try { + await verifyHandler(makeReq({ body: { token } }), res); + return captured.statusCode; + } catch (e) { + return (e as { statusCode: number }).statusCode; + } + }; + + const freshUser = async () => { + const username = `wisp_${Math.random().toString(36).slice(2, 10)}`; + const creds = await createTestUser(server, { + username, + password: 'wisp-test-password', + }); + const row = (await server.stores.user.getByUsername(username))!; + const actorFor = async (token: string) => + (await server.services.auth.authenticateFromToken(token))!; + return { + row, + sessionActor: await actorFor(creds.token), + apiTokenActor: await actorFor(creds.apiToken), + apiToken: creds.apiToken, + }; + }; + + it('verifies a token minted by a live session', async () => { + const { sessionActor } = await freshUser(); + const token = await relayTokenFor(sessionActor); const { res, captured } = makeRes(); await verifyHandler(makeReq({ body: { token } }), res); expect(captured.statusCode).toBe(200); - const body = captured.body as { allow: boolean }; - expect(body.allow).toBe(true); + expect((captured.body as { allow: boolean }).allow).toBe(true); + }); + + it('rejects a token once the account is suspended', async () => { + const { row, sessionActor } = await freshUser(); + const token = await relayTokenFor(sessionActor); + + await server.stores.user.update(row.id, { suspended: 1 }); + + expect(await verifyStatus(token)).toBe(403); + }); + + it('rejects a token once the account is pending verification', async () => { + const { row, sessionActor } = await freshUser(); + const token = await relayTokenFor(sessionActor); + + await server.stores.user.update(row.id, { + requires_email_confirmation: 1, + email_confirmed: 0, + }); + + expect(await verifyStatus(token)).toBe(403); + }); + + it('rejects a token whose user no longer exists', async () => { + const token = await relayTokenFor({ + user: { uuid: '00000000-0000-0000-0000-000000000001' }, + }); + + expect(await verifyStatus(token)).toBe(403); + }); + + it('rejects a token once its session is revoked', async () => { + const { sessionActor } = await freshUser(); + const token = await relayTokenFor(sessionActor); + expect(await verifyStatus(token)).toBe(200); + + await server.services.auth.revokeSession(sessionActor.session!.uid); + + expect(await verifyStatus(token)).toBe(403); + }); + + it('binds a token minted by an access token to that token', async () => { + const { sessionActor, apiTokenActor, apiToken } = await freshUser(); + const token = await relayTokenFor(apiTokenActor); + expect(await verifyStatus(token)).toBe(200); + + await server.services.auth.revokeAccessToken( + sessionActor, + apiToken, + ); + + expect(await verifyStatus(token)).toBe(403); + }); + + it('applies the account checks to a token with no session claim', async () => { + const { row } = await freshUser(); + const token = server.services.token.sign( + 'wisp', + { $: 'token:wisp', $v: '0.0.0', user_uid: row.uuid }, + { expiresIn: '1d' }, + ); + expect(await verifyStatus(token)).toBe(200); + + await server.stores.user.update(row.id, { suspended: 1 }); + + expect(await verifyStatus(token)).toBe(403); }); it('verifies a valid guest token', async () => { diff --git a/src/backend/controllers/wisp/WispController.ts b/src/backend/controllers/wisp/WispController.ts index d9c7751d4..8d97e35da 100644 --- a/src/backend/controllers/wisp/WispController.ts +++ b/src/backend/controllers/wisp/WispController.ts @@ -18,7 +18,12 @@ */ import type { Request, Response } from 'express'; +import type { Actor } from '../../core/actor.js'; import { HttpError } from '../../core/http/HttpError.js'; +import { + assertNotSuspended, + assertVerifiedAccount, +} from '../../core/http/middleware/gates.js'; import type { PuterRouter } from '../../core/http/PuterRouter.js'; import { PuterController } from '../types.js'; import { @@ -71,12 +76,14 @@ export class WispController extends PuterController { const wispCfg = this.#wispConfig(); if (actor?.user?.uuid) { + const sessionUid = await this.#sessionUidFor(actor); const token = this.services.token.sign( 'wisp', { $: 'token:wisp', $v: '0.0.0', user_uid: actor.user.uuid, + ...(sessionUid ? { session_uid: sessionUid } : {}), }, { expiresIn: '1d' }, ); @@ -120,13 +127,10 @@ export class WispController extends PuterController { }); } - // Build policy event — extensions can deny via extension.on('wisp.get-policy') const isGuest = Boolean(decoded.guest); - let user: Record | null = null; - if (!isGuest && decoded.user_uid) { - user = await this.stores.user.getByUuid(String(decoded.user_uid)); - } + const user = isGuest ? null : await this.#liveUserFor(decoded); + // Build policy event — extensions can deny via extension.on('wisp.get-policy') const event: Record = { allow: true, policy: { allow: true }, @@ -145,6 +149,43 @@ export class WispController extends PuterController { res.json(event.policy); }; + /** + * The session a relay token lives no longer than. An access token's is its + * own session row; the actor doesn't carry it. + */ + async #sessionUidFor(actor: Actor): Promise { + if (actor.session?.uid) return actor.session.uid; + if (!actor.accessToken) return undefined; + const row = await this.stores.session.findActiveByAccessTokenUid( + actor.accessToken.uid, + ); + return row?.uuid; + } + + /** + * The token's user, held to the same account gates as minting. A token + * without `session_uid` (minted before the claim existed, or by a legacy + * access token with no session row) gets the account checks only. + */ + async #liveUserFor( + decoded: Record, + ): Promise> { + const forbidden = () => + new HttpError(403, 'Forbidden', { legacyCode: 'forbidden' }); + if (!decoded.user_uid) throw forbidden(); + const user = await this.stores.user.getByUuid(String(decoded.user_uid)); + if (!user) throw forbidden(); + assertNotSuspended(user); + assertVerifiedAccount(user); + if ( + decoded.session_uid && + !(await this.stores.session.getByUuid(String(decoded.session_uid))) + ) { + throw forbidden(); + } + return user; + } + #wispConfig(): NonNullable { return this.config.wisp ?? {}; } diff --git a/src/backend/drivers/apps/AppDriver.js b/src/backend/drivers/apps/AppDriver.js index 255d421c8..f954636c2 100644 --- a/src/backend/drivers/apps/AppDriver.js +++ b/src/backend/drivers/apps/AppDriver.js @@ -441,6 +441,22 @@ export class AppDriver extends PuterDriver { indexUrl: fields.index_url, excludeAppId: app.id, }); + if ( + app.is_private && + (await this.services.auth.originHeldByOtherUser( + fields.index_url, + app.owner_user_id, + )) + ) { + throw new HttpError( + 400, + "A private app can't use an index_url whose origin belongs to another user's app or site", + { + legacyCode: 'app_index_url_already_in_use', + fields: { index_url: fields.index_url }, + }, + ); + } } // Name conflict check (only if name is changing) diff --git a/src/backend/drivers/apps/AppDriver.test.ts b/src/backend/drivers/apps/AppDriver.test.ts index 41a18f340..6297f674e 100644 --- a/src/backend/drivers/apps/AppDriver.test.ts +++ b/src/backend/drivers/apps/AppDriver.test.ts @@ -1295,6 +1295,78 @@ describe('AppDriver.update additional branches', () => { expect.arrayContaining(['md', 'csv']), ); }); + + describe('a private app moving its index_url', () => { + const createApp = async (actor: Actor, indexUrl: string) => + withActor(actor, () => + driver.create({ + object: { + name: uniqueName('pv'), + title: 't', + index_url: indexUrl, + }, + }), + ); + const makePrivate = async (app: Record) => { + await server.clients.db.write( + 'UPDATE `apps` SET `is_private` = 1 WHERE `uid` = ?', + [app.uid], + ); + await server.stores.app.invalidateByUid(app.uid as string); + }; + + it('refuses an origin another user’s app already resolves to', async () => { + const other = await makeUser(); + const target = uniqueIndexUrl(); + const theirs = await createApp(other.actor, target); + const { actor } = await makeUser(); + const mine = await createApp(actor, uniqueIndexUrl()); + await makePrivate(mine); + + await expect( + withActor(actor, () => + driver.update({ + uid: mine.uid, + object: { index_url: target }, + }), + ), + ).rejects.toMatchObject({ statusCode: 400 }); + expect( + await server.services.auth.appUidFromOrigin( + new URL(target).origin, + ), + ).toBe(theirs.uid); + }); + + it('allows an unclaimed origin, a path below another user’s origin, and a public app', async () => { + const other = await makeUser(); + const target = uniqueIndexUrl(); + await createApp(other.actor, target); + const { actor } = await makeUser(); + const mine = await createApp(actor, uniqueIndexUrl()); + await makePrivate(mine); + + const fresh = uniqueIndexUrl(); + const moved = await withActor(actor, () => + driver.update({ uid: mine.uid, object: { index_url: fresh } }), + ); + expect(moved.index_url).toBe(fresh); + const below = `${target}app/`; + const nested = await withActor(actor, () => + driver.update({ uid: mine.uid, object: { index_url: below } }), + ); + expect(nested.index_url).toBe(below); + + const publicApp = await createApp(actor, uniqueIndexUrl()); + const copied = await withActor(actor, () => + driver.update({ + uid: publicApp.uid, + object: { index_url: target }, + }), + ); + expect(copied.index_url).toBe(target); + }); + }); }); // ── read: additional branches ────────────────────────────────────── diff --git a/src/backend/drivers/kv/KVStoreDriver.test.ts b/src/backend/drivers/kv/KVStoreDriver.test.ts index f2570992a..ba6069f97 100644 --- a/src/backend/drivers/kv/KVStoreDriver.test.ts +++ b/src/backend/drivers/kv/KVStoreDriver.test.ts @@ -1368,6 +1368,75 @@ describe('KVStoreDriver', () => { ); }); + it('lets the expire op set an expiry without the delete class', async () => { + const { owner, calendar, contacts, calendarActor } = await setup(); + await grant( + owner, + calendar.uid, + appDataPermission(contacts.uid, 'kv', 'expire'), + ); + expect( + await crossApp(calendarActor, contacts.uid, (optConfig) => + target.expire({ key: 'entry', ttl: 60, optConfig }), + ), + ).toBe(true); + }); + + it('returns no value from remove without read access', async () => { + const { owner, calendar, contacts, calendarActor, contactsActor } = + await setup(); + await inCtx( + () => + target.set({ + key: 'doc', + value: { token: 'secret', x: 1 }, + }), + contactsActor, + ); + await grant( + owner, + calendar.uid, + appDataPermission(contacts.uid, 'kv', 'remove'), + ); + const remove = (paths: string[]) => + crossApp(calendarActor, contacts.uid, (optConfig) => + target.remove({ key: 'doc', paths, optConfig }), + ); + + // A missing top-level path and a missing nested one both remove + // nothing; neither may hand back the stored value. + expect(await remove(['nope'])).toBeNull(); + expect(await remove(['also.missing'])).toBeNull(); + expect(await remove(['x'])).toBeNull(); + expect( + await inCtx(() => target.get({ key: 'doc' }), contactsActor), + ).toEqual({ token: 'secret' }); + }); + + it('returns the updated value from remove with read access too', async () => { + const { owner, calendar, contacts, calendarActor, contactsActor } = + await setup(); + await inCtx( + () => target.set({ key: 'doc', value: { a: 1, b: 2 } }), + contactsActor, + ); + await grant( + owner, + calendar.uid, + appDataPermission(contacts.uid, 'kv', 'remove'), + ); + await grant( + owner, + calendar.uid, + appDataPermission(contacts.uid, 'kv', 'get'), + ); + expect( + await crossApp(calendarActor, contacts.uid, (optConfig) => + target.remove({ key: 'doc', paths: ['a'], optConfig }), + ), + ).toEqual({ b: 2 }); + }); + // -- Compatibility ------------------------------------------------- it('does not consult permissions for an own-namespace call', async () => { diff --git a/src/backend/drivers/kv/KVStoreDriver.ts b/src/backend/drivers/kv/KVStoreDriver.ts index d8f744e1e..c1f6c1cb1 100644 --- a/src/backend/drivers/kv/KVStoreDriver.ts +++ b/src/backend/drivers/kv/KVStoreDriver.ts @@ -240,8 +240,10 @@ export class KVStoreDriver extends PuterDriver { }); } + // `expire` takes its TTL as `ttl`, and its own op is already a deletion. const raw = args as Record; if ( + op !== 'expire' && APP_DATA_KV_TTL_PARAMS.some( (p) => raw[p] !== undefined && raw[p] !== null, ) @@ -642,6 +644,16 @@ export class KVStoreDriver extends PuterDriver { opts, ); this.#meter(opts.actor, usage); + // The result is the entry's value, so a delete-only grant gets none. + if ( + opts.namespaceAppUuid && + !(await this.services.permission.check( + opts.actor!, + appDataPermission(opts.namespaceAppUuid, 'kv', 'get'), + )) + ) { + return null; + } return res; } } diff --git a/src/backend/drivers/util/fileInput.ts b/src/backend/drivers/util/fileInput.ts index 730bd1dac..a809823f6 100644 --- a/src/backend/drivers/util/fileInput.ts +++ b/src/backend/drivers/util/fileInput.ts @@ -59,6 +59,8 @@ export interface LoadedFile { bucketRegion: string | null; size: number | null; sqlId: number | null; // null in case of base64 URL or a future dynamodb FS. + /** The file's owner, who is not necessarily the actor that read it. */ + userId: number; } | null; } @@ -75,7 +77,8 @@ export interface ResolvedFileInput { * uuid? }`. */ export type FileInputRef = - string | { path?: string; uid?: string; uuid?: string }; + | string + | { path?: string; uid?: string; uuid?: string }; export interface OpenedFileInput { body: Readable; @@ -278,6 +281,7 @@ const fsEntryInfo = (entry: FSEntry): NonNullable => ({ bucketRegion: entry.bucketRegion, size: entry.size, sqlId: entry.id, + userId: entry.userId, }); /** {@link openFileInputStream} for an entry that is already resolved. */ diff --git a/src/backend/drivers/workers/WorkerDriver.cloudflare.test.ts b/src/backend/drivers/workers/WorkerDriver.cloudflare.test.ts index 48ad819e8..6ddc57c80 100644 --- a/src/backend/drivers/workers/WorkerDriver.cloudflare.test.ts +++ b/src/backend/drivers/workers/WorkerDriver.cloudflare.test.ts @@ -422,6 +422,102 @@ describe('WorkerDriver.create with a configured deploy backend', () => { expect(putCalls()).toHaveLength(0); }); + // The file's owner keeps editing rights at any share level, and later + // redeploys read the file as the worker's owner. + it.each(['read', 'write', 'manage'] as const)( + 'refuses a source file shared with the actor at %s', + async (mode) => { + const owner = await makeUser(); + const holder = await makeUser(); + const path = `/${owner.user.username}/worker.js`; + const entry = await writeSource( + owner.actor, + owner.user.id, + path, + 'v1', + ); + await server.services.acl.setUserUser( + owner.actor, + holder.actor, + { + path: entry.path, + resolveAncestors: () => + server.services.fs.getAncestorChain(entry.path), + }, + mode, + ); + const name = `borrowed-${mode}-${holder.user.username}`; + + await expect( + inCtx(holder.actor, () => + target.create({ + appId: '', + workerName: name, + filePath: path, + }), + ), + ).rejects.toMatchObject({ + statusCode: 403, + legacyCode: 'forbidden', + }); + expect(putCalls()).toHaveLength(0); + expect( + await server.stores.subdomain.getBySubdomain( + `workers.puter.${name}`, + ), + ).toBeNull(); + expect( + await server.stores.session.getWorker(holder.user.id, { + workerName: name, + }), + ).toBeNull(); + }, + ); + + it('refuses repointing an existing worker at another user’s file', async () => { + const { user, actor } = await makeUser(); + const other = await makeUser(); + const own = `/${user.username}/worker.js`; + const ownEntry = await writeSource(actor, user.id, own, 'v1'); + const foreign = `/${other.user.username}/worker.js`; + const foreignEntry = await writeSource( + other.actor, + other.user.id, + foreign, + 'v2', + ); + await server.services.acl.setUserUser( + other.actor, + actor, + { + path: foreignEntry.path, + resolveAncestors: () => + server.services.fs.getAncestorChain(foreignEntry.path), + }, + 'read', + ); + const name = `repoint-${user.username}`; + await inCtx(actor, () => + target.create({ appId: '', workerName: name, filePath: own }), + ); + + await expect( + inCtx(actor, () => + target.create({ + appId: '', + workerName: name, + filePath: foreign, + [INTERNAL_ADMISSION_BYPASS]: true, + }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + expect(putCalls()).toHaveLength(1); + const row = await server.stores.subdomain.getBySubdomain( + `workers.puter.${name}`, + ); + expect(Number(row!.root_dir_id)).toBe(ownEntry.id); + }); + it('surfaces an edge deploy failure with stack lines rebased past the preamble', async () => { const { user, actor } = await makeUser(); const path = `/${user.username}/worker.js`; @@ -472,7 +568,8 @@ describe('WorkerDriver.create with a configured deploy backend', () => { target.create({ appId: '', workerName: 'ghost-worker', - filePath: '/ghost/worker.js', + // Inline source: a path would 404 before the user lookup. + filePath: 'data:application/javascript,export default {}', }), ), ).rejects.toMatchObject({ diff --git a/src/backend/drivers/workers/WorkerDriver.ts b/src/backend/drivers/workers/WorkerDriver.ts index 361ba7bf3..eaca67413 100644 --- a/src/backend/drivers/workers/WorkerDriver.ts +++ b/src/backend/drivers/workers/WorkerDriver.ts @@ -407,6 +407,36 @@ export class WorkerDriver extends PuterDriver { ); } + // Read source file. loadFileInput runs the read-ACL check internally + // before pulling bytes from S3. Generated source skips all of it: + // there is no file, so there is nothing to authorize a read of. + const loaded = + deployTarget?.source === undefined + ? await loadFileInput( + { + fsEntry: this.stores.fsEntry, + s3Object: this.stores.s3Object, + }, + this.services.fs, + actor, + filePath, + { maxBytes: MAX_SOURCE_SIZE }, + ) + : null; + // A worker is redeployed from its file as the worker's owner, so a + // file someone else owns would let them swap the code it runs. + if ( + !deployTarget && + loaded?.fsEntry && + loaded.fsEntry.userId !== actor.user.id + ) { + throw new HttpError( + 403, + 'A worker can only be deployed from a file you own', + { legacyCode: 'forbidden' }, + ); + } + // If tied to an app, get an app-scoped worker token. Worker tokens // use `kind='worker'` so they don't collide with any interactive // `kind='app'` session for the same (user, app); the long expiry @@ -449,22 +479,6 @@ export class WorkerDriver extends PuterDriver { authorization = session.token; } - // Read source file. loadFileInput runs the read-ACL check internally - // before pulling bytes from S3. Generated source skips all of it: - // there is no file, so there is nothing to authorize a read of. - const loaded = - deployTarget?.source === undefined - ? await loadFileInput( - { - fsEntry: this.stores.fsEntry, - s3Object: this.stores.s3Object, - }, - this.services.fs, - actor, - filePath, - { maxBytes: MAX_SOURCE_SIZE }, - ) - : null; const sourceCode = deployTarget?.source ?? loaded!.buffer.toString('utf-8'); diff --git a/src/backend/services/acl/ACLService.test.ts b/src/backend/services/acl/ACLService.test.ts index d550ba5c4..506837035 100644 --- a/src/backend/services/acl/ACLService.test.ts +++ b/src/backend/services/acl/ACLService.test.ts @@ -92,6 +92,10 @@ function makeService() { // Default: no node on the chain is open to anyone with the link. listAnyoneReaching: vi.fn().mockResolvedValue([]), }, + app: { + // Default: no app row behind any AppData folder. + getByUids: vi.fn().mockResolvedValue(new Map()), + }, }; const services = { permission: { @@ -380,6 +384,56 @@ describe('ACLService.check — app-under-user', () => { ).toBe(true); }); + it("is refused an opted-out app's AppData whatever it was granted", async () => { + const { service, stores, services } = makeService(); + services.permission.scan.mockResolvedValue([{ $: 'option', key: 'k' }]); + const closed = `app-${uuidv4()}`; + stores.app.getByUids.mockImplementation(async (uids: string[]) => + uids.includes(closed) + ? new Map([ + [ + closed, + { uid: closed, metadata: { share_app_data: false } }, + ], + ]) + : new Map(), + ); + // Asked in another case; the stored chain names the real folder. + const asked: ResourceDescriptor = { + path: `/issuer/appdata/${closed.toUpperCase()}/state.json`, + resolveAncestors: async () => [ + { + uid: 'uid:file', + path: `/issuer/AppData/${closed}/state.json`, + }, + { uid: 'uid:root', path: `/issuer/AppData/${closed}` }, + { uid: 'uid:appdata', path: '/issuer/AppData' }, + { uid: 'uid:home', path: '/issuer' }, + ], + }; + expect(await service.check(appActor('issuer'), asked, 'read')).toBe( + false, + ); + expect(stores.app.getByUids).toHaveBeenCalledWith( + expect.arrayContaining([closed]), + ); + }); + + it('looks up no app for an AppData folder not named like one', async () => { + const { service, stores, services } = makeService(); + services.permission.scan.mockResolvedValue([{ $: 'option', key: 'k' }]); + for (const name of ['日本', 'notes', 'app-2']) { + expect( + await service.check( + appActor('issuer'), + resource(`/issuer/AppData/${name}/x`), + 'read', + ), + ).toBe(true); + } + expect(stores.app.getByUids).not.toHaveBeenCalled(); + }); + it("does not get its user's home short-circuit", async () => { const { service, services } = makeService(); // The app's user owns /issuer, so the recursive user check passes, @@ -1073,7 +1127,10 @@ describe('ACLService.statUserUser / setUserUser (integration)', () => { }); describe('app-under-user on shared paths', () => { - const makeApp = async (ownerUserId: number) => + const makeApp = async ( + ownerUserId: number, + fields: Record = {}, + ) => ( server.stores.app.create as unknown as ( fields: Record, @@ -1084,6 +1141,7 @@ describe('ACLService.statUserUser / setUserUser (integration)', () => { name: `acl-app-${uuidv4()}`, title: 'ACL app', index_url: `https://acl-${uuidv4()}.test/`, + ...fields, }, { ownerUserId }, ); @@ -1198,6 +1256,71 @@ describe('ACLService.statUserUser / setUserUser (integration)', () => { await acl.check(asApp(holder, mine), appDataOf(theirs.uid), 'write'), ).toBe(false); }); + + describe("another app's AppData", () => { + /** `//AppData//inner`, as the app would leave it. */ + const seedAppData = async (user: Actor, appUid: string) => { + const root = `/${user.user.username}/AppData/${appUid}`; + await server.services.fs.mkdir(user.user.id!, { + path: `${root}/inner`, + createMissingParents: true, + } as never); + return root; + }; + + const grantTo = ( + user: Actor, + app: { uid: string }, + permission: string, + ) => + runWithContext({ actor: user }, () => + server.services.permission.grantUserAppPermission( + user, + app.uid, + permission, + ), + ); + + it('stays closed to every grant once that app opts out of sharing', async () => { + const user = await makeUser(); + const closed = await makeApp(user.user.id!, { + metadata: JSON.stringify({ share_app_data: false }), + }); + const root = await seedAppData(user, closed.uid); + const inner = acl.fsDescriptor(`${root}/inner`); + const home = `/${user.user.username}`; + + // The folder by path, its parent, and the whole home. + for (const target of [root, `${home}/AppData`, home]) { + const reader = await makeApp(user.user.id!); + await grantTo(user, reader, `fs:${target}:read`); + expect( + await acl.check(asApp(user, reader), inner, 'read'), + ).toBe(false); + } + + // The user and the app that owns the folder keep their access. + expect(await acl.check(user, inner, 'write')).toBe(true); + expect( + await acl.check(asApp(user, closed), inner, 'write'), + ).toBe(true); + }); + + it('opens to an fs grant while that app shares', async () => { + const user = await makeUser(); + const open = await makeApp(user.user.id!); + const root = await seedAppData(user, open.uid); + const reader = await makeApp(user.user.id!); + await grantTo(user, reader, `fs:${root}:read`); + expect( + await acl.check( + asApp(user, reader), + acl.fsDescriptor(`${root}/inner`), + 'read', + ), + ).toBe(true); + }); + }); }); it('leaves one mode standing when two writes race on the same node', async () => { diff --git a/src/backend/services/acl/ACLService.ts b/src/backend/services/acl/ACLService.ts index 331654ff5..e30de7a8d 100644 --- a/src/backend/services/acl/ACLService.ts +++ b/src/backend/services/acl/ACLService.ts @@ -30,6 +30,7 @@ import { } from '../../core/actor'; import { PermissionUtil } from '../permission/permissionUtil'; import { MANAGE_PERM_PREFIX } from '../permission/consts'; +import { closedAppDataOwners } from '../permission/appDataScopes'; import { HttpError } from '../../core/http/HttpError.js'; import { actorHasSubscription } from '../metering/enforcement'; import { withRedisLock } from '../../util/redisLock.js'; @@ -49,7 +50,11 @@ export interface ResourceDescriptor { } export type AclMode = - 'see' | 'list' | 'read' | 'write' | typeof MANAGE_PERM_PREFIX; + | 'see' + | 'list' + | 'read' + | 'write' + | typeof MANAGE_PERM_PREFIX; /** Duck-typed error shape compatible with APIError consumers (fsv2). */ export interface AclError { @@ -203,6 +208,15 @@ export class ACLService extends PuterService { return true; } } + // Closed whatever the app was granted, on the folder or above it. + // The stored ancestor paths count too, so a case variant of the + // asked path still names the folder. + const closed = await closedAppDataOwners( + this.stores.app, + [resource.path, ...ancestors.map((a) => a.path)], + appUid, + ); + if (closed.size > 0) return false; } // Public folders: //Public with read-ish mode, owner must have diff --git a/src/backend/services/auth/AuthService.appDeletion.test.ts b/src/backend/services/auth/AuthService.appDeletion.test.ts index a763dba0b..d18f0b740 100644 --- a/src/backend/services/auth/AuthService.appDeletion.test.ts +++ b/src/backend/services/auth/AuthService.appDeletion.test.ts @@ -18,9 +18,10 @@ */ /** - * A hosted origin's app uid is derived from the origin, so once an app is - * deleted whoever registers the freed subdomain next is handed the same uid. - * Sessions made for the earlier app must not authenticate as the new one. + * A hosted origin's app uid is derived from the origin, so a deleted app's uid + * comes back when its owner uses the site again. Sessions made for the earlier + * app must not authenticate as the new one. Someone else who registers the + * freed subdomain gets a new uid, so users' KV and AppData stay behind. */ import { afterAll, beforeAll, describe, expect, it } from 'vitest'; @@ -168,10 +169,6 @@ describe('an app uid that returns after its app was deleted', () => { it("stops the deleted app's sessions authenticating as the next app on its origin", async () => { const devA = env.users.other; const visitor = env.users.user; - const devB = await createTestUser(env.server, { - username: `devb${uuidv4().slice(0, 8)}`, - password: 'dev-b-password-123', - }); const sub = `shop-${uuidv4().slice(0, 8)}`; const origin = `http://${sub}.site.puter.localhost`; @@ -203,32 +200,32 @@ describe('an app uid that returns after its app was deleted', () => { // The earlier app's session was made well before the next app exists. await backdateSession(sessionUid(tokenA), 3600); - const bCreate = await call(devB.token, 'puter-subdomains', 'create', { + // The same developer puts the site back up. + const again = await call(devA.token, 'puter-subdomains', 'create', { object: { subdomain: sub, - root_dir: await mkSiteDir(devB.username), + root_dir: await mkSiteDir(devA.username), }, }); - expect(bCreate.status, bCreate.text).toBe(200); + expect(again.status, again.text).toBe(200); - const { token: tokenB, app_uid: uidB } = await signIn( + const { token: tokenNext, app_uid: uidNext } = await signIn( visitor.token, origin, ); - // The uid itself is reused; only the old credentials are cut. - expect(uidB).toBe(uid); - expect(sessionUid(tokenB)).not.toBe(sessionUid(tokenA)); + // The uid and its data come back; only the old credentials are cut. + expect(uidNext).toBe(uid); + expect(sessionUid(tokenNext)).not.toBe(sessionUid(tokenA)); expect(await isRevoked(sessionUid(tokenA))).toBe(true); expect((await api('/whoami', tokenA)).status).toBe(401); - expect((await api('/whoami', tokenB)).status).toBe(200); - const setB = await call(tokenB, 'puter-kvstore', 'set', { - key: 'card', - value: 'b-era', + expect((await api('/whoami', tokenNext)).status).toBe(200); + const kept = await call(tokenNext, 'puter-kvstore', 'get', { + key: 'notes', }); - expect(setB.status, setB.text).toBe(200); + expect((kept.json as { result?: unknown }).result).toBe('a-era'); const getA = await call(tokenA, 'puter-kvstore', 'get', { - key: 'card', + key: 'notes', }); expect(getA.status).toBe(401); }); @@ -236,10 +233,6 @@ describe('an app uid that returns after its app was deleted', () => { it('stops an access token the deleted app minted from authenticating as the next app', async () => { const devA = env.users.other; const visitor = env.users.user; - const devB = await createTestUser(env.server, { - username: `devb${uuidv4().slice(0, 8)}`, - password: 'dev-b-password-123', - }); const sub = `shop-${uuidv4().slice(0, 8)}`; const origin = `http://${sub}.site.puter.localhost`; @@ -270,15 +263,15 @@ describe('an app uid that returns after its app was deleted', () => { await backdateSession(sessionUid(appTokenA), 3600); await backdateSession(sessionUid(scopedA), 3600); - // The next developer's sign-in brings the uid back. - const bCreate = await call(devB.token, 'puter-subdomains', 'create', { + // The developer's next sign-in on their own site brings the uid back. + const again = await call(devA.token, 'puter-subdomains', 'create', { object: { subdomain: sub, - root_dir: await mkSiteDir(devB.username), + root_dir: await mkSiteDir(devA.username), }, }); - expect(bCreate.status, bCreate.text).toBe(200); - expect((await signIn(devB.token, origin)).app_uid).toBe(uid); + expect(again.status, again.text).toBe(200); + expect((await signIn(devA.token, origin)).app_uid).toBe(uid); expect((await authenticate(scopedA)).reauth?.reason).toBe( 'session_revoked', @@ -371,3 +364,144 @@ describe('an app uid that returns after its app was deleted', () => { expect(body.auth_id).toBeUndefined(); }); }); + +describe("a deleted app's subdomain registered by another user", () => { + const writeUserFile = async ( + username: string, + path: string, + body: string, + ) => { + const row = await env.server.stores.user.getByUsername(username); + const buf = Buffer.from(body); + await env.server.services.fs.write(row!.id, { + fileMetadata: { + path, + size: buf.byteLength, + contentType: 'text/plain', + overwrite: true, + createMissingParents: true, + }, + fileContent: buf, + } as never); + }; + + /** + * A visitor stores KV and AppData on developer A's site, `release` takes + * A's app away, then developer B registers the same subdomain and the + * visitor signs in there. + */ + const reclaim = async ( + release: (ctx: { + devA: { id: number; token: string }; + sub: string; + uid: string; + }) => Promise, + ) => { + const visitor = env.users.user; + const devA = await createTestUser(env.server, { + username: `deva${uuidv4().slice(0, 8)}`, + password: 'dev-a-password-123', + }); + const devB = await createTestUser(env.server, { + username: `devb${uuidv4().slice(0, 8)}`, + password: 'dev-b-password-123', + }); + const sub = `notes-${uuidv4().slice(0, 8)}`; + const origin = `http://${sub}.site.puter.localhost`; + + const aCreate = await call(devA.token, 'puter-subdomains', 'create', { + object: { + subdomain: sub, + root_dir: await mkSiteDir(devA.username), + }, + }); + expect(aCreate.status, aCreate.text).toBe(200); + + const { token: tokenA, app_uid: uid } = await signIn( + visitor.token, + origin, + ); + const set = await call(tokenA, 'puter-kvstore', 'set', { + key: 'diary', + value: 'stored on A', + }); + expect(set.status, set.text).toBe(200); + const appDataPath = `/${visitor.username}/AppData/${uid}/private.txt`; + await writeUserFile(visitor.username, appDataPath, 'appdata from A'); + + const devARow = await env.server.stores.user.getByUsername( + devA.username, + ); + await release({ + devA: { id: devARow!.id, token: devA.token }, + sub, + uid, + }); + + const bCreate = await call(devB.token, 'puter-subdomains', 'create', { + object: { + subdomain: sub, + root_dir: await mkSiteDir(devB.username), + }, + }); + expect(bCreate.status, bCreate.text).toBe(200); + + const { token: tokenB, app_uid: uidB } = await signIn( + visitor.token, + origin, + ); + return { uid, uidB, tokenB, appDataPath }; + }; + + const expectDataStaysBehind = async ({ + uid, + uidB, + tokenB, + appDataPath, + }: Awaited>) => { + expect(uidB).not.toBe(uid); + + const kv = await call(tokenB, 'puter-kvstore', 'get', { key: 'diary' }); + expect(kv.status, kv.text).toBe(200); + expect((kv.json as { result?: unknown }).result).toBeNull(); + + const read = await api( + `/read?file=${encodeURIComponent(appDataPath)}`, + tokenB, + ); + expect(read.status).not.toBe(200); + }; + + it('gives the new site its own uid when A deletes the app and the site', async () => { + await expectDataStaysBehind( + await reclaim(async ({ devA, sub, uid }) => { + const delApp = await call(devA.token, 'puter-apps', 'delete', { + uid, + }); + expect(delApp.status, delApp.text).toBe(200); + const delSite = await call( + devA.token, + 'puter-subdomains', + 'delete', + { id: { subdomain: sub } }, + ); + expect(delSite.status, delSite.text).toBe(200); + }), + ); + }); + + it("gives the new site its own uid when A's account is deleted", async () => { + await expectDataStaysBehind( + await reclaim(async ({ devA }) => { + await env.server.services.userAccount.cascadeDelete(devA.id); + // MySQL and Postgres drop the user's apps by FK cascade; the + // test sqlite schema has no such FK, so apply it here. + await env.server.clients.db.write( + 'DELETE FROM `apps` WHERE `owner_user_id` = ?', + [devA.id], + ); + await env.server.clients.redis.flushall(); + }), + ); + }); +}); diff --git a/src/backend/services/auth/AuthService.test.ts b/src/backend/services/auth/AuthService.test.ts index b762cfaa9..16484ea0d 100644 --- a/src/backend/services/auth/AuthService.test.ts +++ b/src/backend/services/auth/AuthService.test.ts @@ -2129,6 +2129,136 @@ describe('AuthService (integration)', () => { ).toBe(app.uid); }); }); + + describe("when the app's site is registered to another user", () => { + it('gives the new site its own uid and restores the app once its owner holds the site again', async () => { + const owner = await makeUser(); + const other = await makeUser(); + const sub = `freed-${Math.random().toString(36).slice(2, 10)}`; + const origin = `https://${sub}.site.puter.localhost`; + const ownSite = await server.stores.subdomain.create({ + userId: owner.id, + subdomain: sub, + }); + const app = await server.stores.app.createFromOrigin( + await authService.appUidFromOrigin(origin), + authService.canonicalizeOrigin(origin), + { ownerUserId: owner.id }, + ); + // A path misses the canonical lookup, so only the derived + // uid ties this origin to the app. + await server.stores.app.update(app.id, { + index_url: `${origin}/play/`, + }); + expect(await authService.appUidFromOrigin(origin)).toBe( + app.uid, + ); + + await server.stores.subdomain.deleteByUuid(ownSite.uuid); + expect(await authService.appUidFromOrigin(origin)).toBe( + app.uid, + ); + + const otherSite = await server.stores.subdomain.create({ + userId: other.id, + subdomain: sub, + }); + const next = await authService.appUidFromOrigin(origin); + expect(next).not.toBe(app.uid); + expect( + (await authService.appUidsFromOrigins([origin])).get( + origin, + ), + ).toBe(next); + + await server.stores.subdomain.deleteByUuid(otherSite.uuid); + await server.stores.subdomain.create({ + userId: owner.id, + subdomain: sub, + }); + expect(await authService.appUidFromOrigin(origin)).toBe( + app.uid, + ); + expect( + (await authService.appUidsFromOrigins([origin])).get( + origin, + ), + ).toBe(app.uid); + }); + }); + + describe('after the origin-derived app is deleted', () => { + const uidsFor = async (origin: string) => [ + await authService.appUidFromOrigin(origin), + (await authService.appUidsFromOrigins([origin])).get(origin), + ]; + + const bootstrapAndDelete = async ( + origin: string, + ownerUserId: number | null, + ) => { + const app = await server.stores.app.createFromOrigin( + await authService.appUidFromOrigin(origin), + authService.canonicalizeOrigin(origin), + { ownerUserId }, + ); + await server.stores.app.delete(app.id); + return app.uid; + }; + + it("gives another user's site on the subdomain a new uid and returns the old one to the app's owner", async () => { + const owner = await makeUser(); + const other = await makeUser(); + const sub = `gone-${Math.random().toString(36).slice(2, 10)}`; + const origin = `https://${sub}.site.puter.localhost`; + const ownSite = await server.stores.subdomain.create({ + userId: owner.id, + subdomain: sub, + }); + const uid = await bootstrapAndDelete(origin, owner.id); + expect(await uidsFor(origin)).toEqual([uid, uid]); + + await server.stores.subdomain.deleteByUuid(ownSite.uuid); + const unregistered = await uidsFor(origin); + expect(unregistered[0]).not.toBe(uid); + expect(unregistered[1]).toBe(unregistered[0]); + + const otherSite = await server.stores.subdomain.create({ + userId: other.id, + subdomain: sub, + }); + const next = await uidsFor(origin); + expect(next[0]).not.toBe(uid); + expect(next[1]).toBe(next[0]); + + await server.stores.subdomain.deleteByUuid(otherSite.uuid); + await server.stores.subdomain.create({ + userId: owner.id, + subdomain: sub, + }); + expect(await uidsFor(origin)).toEqual([uid, uid]); + }); + + it('does not return an unowned deleted app uid to a hosted site', async () => { + const owner = await makeUser(); + const sub = `gone-${Math.random().toString(36).slice(2, 10)}`; + const origin = `https://${sub}.site.puter.localhost`; + const uid = await bootstrapAndDelete(origin, null); + await server.stores.subdomain.create({ + userId: owner.id, + subdomain: sub, + }); + expect(await authService.appUidFromOrigin(origin)).not.toBe( + uid, + ); + }); + + it('keeps the uid of an external origin', async () => { + const origin = `https://gone-${uuidv4()}.example.com`; + const uid = await bootstrapAndDelete(origin, null); + expect(await uidsFor(origin)).toEqual([uid, uid]); + }); + }); }); describe('appUidsFromOrigins', () => { @@ -3471,6 +3601,142 @@ describe('AuthService (integration)', () => { }); }); + describe('revoking every session of a user', () => { + const revokers: Array< + [string, (user: { id: number }) => Promise] + > = [ + [ + 'revokeAllSessions', + (user) => + authService.revokeAllSessions({ + user, + session: { uid: 'unrelated' }, + } as unknown as Actor), + ], + [ + 'revokeInteractiveSessionsForUserId', + (user) => + authService.revokeInteractiveSessionsForUserId(user.id), + ], + [ + 'revokeAllSessionsForUserId', + (user) => authService.revokeAllSessionsForUserId(user.id), + ], + ]; + + it.each(revokers)( + '%s revokes a session the replica has not seen yet', + async (_name, revoke) => { + const user = await makeUser(); + const web = await authService.createSessionToken(user, {}); + const webUuid = (web.session as { uuid: string }).uuid; + + const db = server.clients.db; + const original = db.read.bind(db); + const read = vi + .spyOn(db, 'read') + .mockImplementation( + async (sql: string, params?: unknown[]) => + sql.includes('FROM `sessions`') + ? [] + : original(sql, params), + ); + // sqlite's `pread` goes through `read`; keep it on the primary. + const pread = vi + .spyOn(db, 'pread') + .mockImplementation((sql: string, params?: unknown[]) => + original(sql, params), + ); + try { + await revoke(user); + } finally { + read.mockRestore(); + pread.mockRestore(); + } + + expect( + await server.stores.session.getByUuid(webUuid), + ).toBeNull(); + }, + ); + + it.each(revokers)( + '%s takes the same few primary round trips however many sessions there are', + async (name, revoke) => { + const user = await makeUser(); + const sessions = server.stores.session; + const uuids: string[] = []; + for (let i = 0; i < 15; i++) { + const web = await sessions.create(user.id); + const asset = await sessions.create(user.id, { + kind: 'asset', + parent_session_id: web.uuid, + }); + uuids.push(web.uuid, asset.uuid); + } + const app = await sessions.getOrCreateApp( + user.id, + `app-${uuidv4()}`, + ); + const appToken = await sessions.create(user.id, { + kind: 'access_token', + parent_session_id: app.uuid, + }); + const appUuids = [app.uuid, appToken.uuid]; + + const db = server.clients.db; + const pread = vi.spyOn(db, 'pread'); + const write = vi.spyOn(db, 'write'); + let primaryQueries: string[]; + try { + await revoke(user); + } finally { + primaryQueries = [ + ...pread.mock.calls, + ...write.mock.calls, + ].map(([sql]) => String(sql)); + pread.mockRestore(); + write.mockRestore(); + } + + // List, select, update, re-seed. + expect( + primaryQueries.filter((sql) => sql.includes('`sessions`')), + ).toHaveLength(4); + for (const uuid of uuids) { + expect(await sessions.getByUuid(uuid)).toBeNull(); + } + for (const uuid of appUuids) { + const revoked = (await sessions.getByUuid(uuid)) === null; + // Only revokeAllSessions keeps app authorizations. + expect(revoked).toBe(name !== 'revokeAllSessions'); + } + }, + ); + + it('a password reset leaves workers and standalone access tokens; a forced sign-out takes them', async () => { + const user = await makeUser(); + const sessions = server.stores.session; + const web = await sessions.create(user.id); + const worker = await sessions.getOrCreateWorker(user.id, { + appUid: null, + workerName: `wk-${uuidv4()}`, + }); + const token = await sessions.create(user.id, { + kind: 'access_token', + }); + + await authService.revokeInteractiveSessionsForUserId(user.id); + expect(await sessions.getByUuid(web.uuid)).toBeNull(); + expect(await sessions.getByUuid(worker.uuid)).toBeTruthy(); + expect(await sessions.getByUuid(token.uuid)).toBeTruthy(); + + await authService.revokeAllSessionsForUserId(user.id); + expect(await sessions.getByUuid(worker.uuid)).toBeNull(); + expect(await sessions.getByUuid(token.uuid)).toBeNull(); + }); + }); + describe('purgeEndedSessions', () => { const GATE_KEY = 'sessions:purge:gate'; const DAY = 24 * 60 * 60; diff --git a/src/backend/services/auth/AuthService.ts b/src/backend/services/auth/AuthService.ts index 00471dadc..71e713ab5 100644 --- a/src/backend/services/auth/AuthService.ts +++ b/src/backend/services/auth/AuthService.ts @@ -73,6 +73,11 @@ const SESSION_PURGE_PAUSE_MS = 50; const nowSeconds = (): number => Math.floor(Date.now() / 1000); +const positiveUserId = (value: unknown): number | null => { + const id = typeof value === 'string' ? Number(value) : value; + return typeof id === 'number' && Number.isInteger(id) && id > 0 ? id : null; +}; + /** * Hosted-asset cookie names by kind. `name` is what gets set; `legacyName` is * still read through the v2 deprecation window. @@ -892,18 +897,24 @@ export class AuthService extends PuterService { * user-driven UI flow and exempts workers + standalone access tokens by * design. * - * Iterates each top-level row through `revokeCascade` so derived rows - * (asset under web, app-issued access tokens under their app session) - * follow via the parent_session_id link. + * Cascades from every row so derived rows (asset under web, app-issued + * access tokens under their app session) follow via the parent_session_id + * link. */ async revokeAllSessionsForUserId(userId: number): Promise { if (!userId) return; - const rows = await this.stores.session.getByUserId(userId); - for (const row of rows) { - this.#announceRevocation( - await this.stores.session.revokeCascade(row.uuid as string), - ); - } + const rows = await this.stores.session.getByUserId(userId, { + primary: true, + }); + await this.#revokeCascades(rows.map((row) => row.uuid as string)); + } + + async #revokeCascades(rootUuids: string[]): Promise { + if (rootUuids.length === 0) return; + const revoked = (await this.stores.session.revokeCascades( + rootUuids, + )) as { userId: number; uuids: string[] }[]; + for (const entry of revoked) this.#announceRevocation(entry); } /** @@ -960,14 +971,14 @@ export class AuthService extends PuterService { */ async revokeInteractiveSessionsForUserId(userId: number): Promise { if (!userId) return; - const rows = await this.stores.session.getByUserId(userId); - for (const row of rows) { - if (row.kind === 'web' || row.kind === 'app') { - this.#announceRevocation( - await this.stores.session.revokeCascade(row.uuid as string), - ); - } - } + const rows = await this.stores.session.getByUserId(userId, { + primary: true, + }); + await this.#revokeCascades( + rows + .filter((row) => row.kind === 'web' || row.kind === 'app') + .map((row) => row.uuid as string), + ); } async revokeAllSessions( @@ -982,19 +993,17 @@ export class AuthService extends PuterService { const currentUuid = actor.session?.uid; const rows = await this.stores.session.getByUserId( actor.user.id as number, + { primary: true }, + ); + await this.#revokeCascades( + rows + .filter((row) => + row.kind === 'web' + ? opts.includeCurrent || row.uuid !== currentUuid + : row.kind === 'app' && opts.includeApps, + ) + .map((row) => row.uuid as string), ); - for (const row of rows) { - if (row.kind === 'web') { - if (!opts.includeCurrent && row.uuid === currentUuid) continue; - this.#announceRevocation( - await this.stores.session.revokeCascade(row.uuid as string), - ); - } else if (row.kind === 'app' && opts.includeApps) { - this.#announceRevocation( - await this.stores.session.revokeCascade(row.uuid as string), - ); - } - } } // -- App / origin resolution ------------------------------------- @@ -1115,15 +1124,64 @@ export class AuthService extends PuterService { const prefetched = gen0Uids.length > 0 ? await this.stores.app.getByUids(gen0Uids) - : new Map(); + : new Map< + string, + { index_url?: unknown; owner_user_id?: unknown } + >(); + + // Likewise the deleted app's owner for each hosted gen-0 uid with no + // row, and the site owner each owner gets compared against. + const deletedGen0Uids = missedAppOrigins + .filter( + (appOrigin) => + !prefetched.has(gen0ByAppOrigin.get(appOrigin)!) && + this.#hostedSubdomainForOrigin(appOrigin) !== null, + ) + .map((appOrigin) => gen0ByAppOrigin.get(appOrigin)!); + const deletedOwners = + deletedGen0Uids.length > 0 + ? await this.stores.app.getDeletedOwnersByUids(deletedGen0Uids) + : new Map(); + const subdomainByAppOrigin = new Map(); + for (const appOrigin of missedAppOrigins) { + const gen0Uid = gen0ByAppOrigin.get(appOrigin)!; + const row = prefetched.get(gen0Uid); + const ownerId = row + ? positiveUserId(row.owner_user_id) + : (deletedOwners.get(gen0Uid) ?? null); + const subdomain = this.#hostedSubdomainForOrigin(appOrigin); + if (ownerId !== null && subdomain) { + subdomainByAppOrigin.set(appOrigin, subdomain); + } + } + const siteRows = + subdomainByAppOrigin.size > 0 + ? await this.stores.subdomain.getBySubdomains([ + ...subdomainByAppOrigin.values(), + ]) + : new Map(); + const getSiteOwnerId = (appOrigin: string) => { + const subdomain = subdomainByAppOrigin.get(appOrigin); + const row = subdomain ? siteRows.get(subdomain) : undefined; + return row + ? Promise.resolve(positiveUserId(row.user_id)) + : this.#siteOwnerIdForOrigin(appOrigin); + }; const derivedByAppOrigin = new Map(); for (const appOrigin of missedAppOrigins) { const gen0Uid = gen0ByAppOrigin.get(appOrigin)!; - const uid = await this.#derivedAppUidForOrigin(appOrigin, (uid) => - uid === gen0Uid - ? Promise.resolve(prefetched.get(uid) ?? null) - : this.stores.app.getByUid(uid), + const uid = await this.#derivedAppUidForOrigin( + appOrigin, + (uid) => + uid === gen0Uid + ? Promise.resolve(prefetched.get(uid) ?? null) + : this.stores.app.getByUid(uid), + getSiteOwnerId, + (uid) => + uid === gen0Uid + ? Promise.resolve(deletedOwners.get(uid)) + : this.#deletedAppOwnerId(uid), ); derivedByAppOrigin.set(appOrigin, uid); } @@ -1169,26 +1227,51 @@ export class AuthService extends PuterService { * `app-`, unless that uid's row now lives at another * origin. A repointed row keeps its uid (and the data and grants keyed to * it), so the origin it left moves on to a successor uid instead of - * resolving to someone's app it no longer serves. `getByUid` is injectable - * so a batch caller can serve prefetched rows. + * resolving to someone's app it no longer serves. The same goes for a row + * whose hosted site is now registered to another user, and for a deleted + * app's uid on a hosted site its owner no longer holds. The lookups are + * injectable so a batch caller can serve prefetched rows. */ async #derivedAppUidForOrigin( origin: string, getByUid: ( uid: string, - ) => Promise<{ index_url?: unknown } | null | undefined> = (uid) => - this.stores.app.getByUid(uid), + ) => Promise< + { index_url?: unknown; owner_user_id?: unknown } | null | undefined + > = (uid) => this.stores.app.getByUid(uid), + getSiteOwnerId: (origin: string) => Promise = (o) => + this.#siteOwnerIdForOrigin(o), + getDeletedOwnerId: ( + uid: string, + ) => Promise = (uid) => + this.#deletedAppOwnerId(uid), ): Promise { + const hosted = this.#hostedSubdomainForOrigin(origin) !== null; for (let gen = 0; gen <= MAX_ORIGIN_UID_GENERATIONS; gen++) { const uid = this.#originUid(origin, gen); const app = await getByUid(uid); - if (!app) return uid; - if ( - typeof app.index_url === 'string' && - (await this.#appOriginFor(app.index_url)) === origin - ) { - return uid; + if (!app) { + if (!hosted) return uid; + const deletedOwnerId = await getDeletedOwnerId(uid); + if (deletedOwnerId === undefined) return uid; + if ( + deletedOwnerId !== null && + deletedOwnerId === (await getSiteOwnerId(origin)) + ) { + return uid; + } + continue; } + if ( + typeof app.index_url !== 'string' || + (await this.#appOriginFor(app.index_url)) !== origin + ) { + continue; + } + const appOwnerId = positiveUserId(app.owner_user_id); + if (appOwnerId === null) return uid; + const siteOwnerId = await getSiteOwnerId(origin); + if (siteOwnerId === null || siteOwnerId === appOwnerId) return uid; } // Past the cap the uid stops being derivable, but the row created for // it records this origin as its index_url, so the canonical lookup @@ -1196,6 +1279,11 @@ export class AuthService extends PuterService { return `app-${uuidv4()}`; } + /** Owner of the deleted app that held `uid`; undefined if none did. */ + async #deletedAppOwnerId(uid: string): Promise { + return (await this.stores.app.getDeletedOwnersByUids([uid])).get(uid); + } + /** * Resolve the user who owns the hosted subdomain `origin` points at. * Returns the `subdomains` row's `user_id` when the origin's host sits @@ -1206,25 +1294,65 @@ export class AuthService extends PuterService { * rows. */ async subdomainOwnerIdFromOrigin(origin: string): Promise { + const subdomain = this.#hostedSubdomainForOrigin(origin); + if (!subdomain) return null; + const row = await this.stores.subdomain.getBySubdomain(subdomain); + return positiveUserId(row?.user_id); + } + + /** + * Whether `indexUrl` is on a hosted site, or on an origin whose app + * identity resolves to an app, owned by someone other than `ownerUserId`. A + * private app wins its origin's identity, so an app shouldn't become or + * stay private where this is true. Index urls below the origin root never + * resolve from an origin, so only the site owner counts for them. + */ + async originHeldByOtherUser( + indexUrl: string, + ownerUserId: number, + ): Promise { + const origin = await this.#appOriginFor(indexUrl); + if (!origin) return false; + const siteOwnerId = await this.#siteOwnerIdForOrigin(origin); + if (siteOwnerId !== null) return siteOwnerId !== ownerUserId; + + const path = new URL(indexUrl).pathname.toLowerCase(); + if (path !== '/' && path !== '/index.html') return false; + const holderUid = await this.#findCanonicalAppUidForOrigin(origin); + const holder = holderUid + ? await this.stores.app.getByUid(holderUid) + : null; + const holderId = positiveUserId(holder?.owner_user_id); + return holderId !== null && holderId !== ownerUserId; + } + + /** + * {@link subdomainOwnerIdFromOrigin}, confirmed against the primary on a + * miss so a site registered a moment ago isn't read as unregistered. + */ + async #siteOwnerIdForOrigin(origin: string): Promise { + const subdomain = this.#hostedSubdomainForOrigin(origin); + if (!subdomain) return null; + const row = + (await this.stores.subdomain.getBySubdomain(subdomain)) ?? + (await this.stores.subdomain.getBySubdomain(subdomain, { + primary: true, + })); + return positiveUserId(row?.user_id); + } + + #hostedSubdomainForOrigin(origin: string): string | null { let parsed: URL; try { parsed = new URL(origin); } catch { return null; } - const subdomain = this.#hostedSubdomainForHost( + return this.#hostedSubdomainForHost( parsed.host.toLowerCase(), parsed.hostname.toLowerCase(), this.#getHostingDomains(), ); - if (!subdomain) return null; - const row = await this.stores.subdomain.getBySubdomain(subdomain); - const ownerId = row?.user_id; - return typeof ownerId === 'number' && - Number.isInteger(ownerId) && - ownerId > 0 - ? ownerId - : null; } /** diff --git a/src/backend/services/fs/FSService.test.ts b/src/backend/services/fs/FSService.test.ts index bc05200cd..c4cf10bed 100644 --- a/src/backend/services/fs/FSService.test.ts +++ b/src/backend/services/fs/FSService.test.ts @@ -3659,7 +3659,9 @@ describe('FSService mkdir, touch, rename and shortcuts', () => { try { const error = await caught(() => fs.mkdir(user.userId, { path })); expect(error.statusCode).toBe(409); - expect(error.message).toContain(path); + expect(error.message).toBe( + 'An entry named `racedfile` already exists', + ); } finally { await restore(); } @@ -6286,6 +6288,106 @@ describe('FSService — cross-app AppData access', () => { await server.stores.fsEntry.getEntryByPath(contactsFile.path), ).toBeFalsy(); }); + + // -- Carrying AppData out of AppData --------------------------------- + + describe('the AppData folder or home as a whole', () => { + let appData: FSEntry; + let desktop: FSEntry; + let closedUid: string; + let closedFile: FSEntry; + + beforeEach(async () => { + const closed = await makeRealApp(owner.userId, { + metadata: JSON.stringify({ share_app_data: false }), + }); + closedUid = closed.uid; + closedFile = await seedAppData(closed.uid); + appData = (await entryAt(owner, '/AppData'))!; + desktop = (await entryAt(owner, '/Desktop'))!; + }); + + it('refuses an app copying, moving or renaming it past a closed app', async () => { + const into = { source: appData, destinationParent: desktop }; + await expect( + asCalendar(() => + fs.copy(owner.userId, { ...into, newName: 'grab' }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + await expect( + asCalendar(() => + fs.move(owner.userId, { ...into, newName: 'grab' }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + await expect( + asCalendar(() => fs.rename(owner.userId, appData, 'grab')), + ).rejects.toMatchObject({ statusCode: 403 }); + expect(await entryAt(owner, '/Desktop/grab')).toBeFalsy(); + expect(await entryAt(owner, '/grab')).toBeFalsy(); + + // The home carries the same folder. + const home = (await entryAt(owner, ''))!; + const elsewhere = (await entryAt(await makeUser(), '/Desktop'))!; + await expect( + asCalendar(() => + fs.copy(owner.userId, { + source: home, + destinationParent: elsewhere, + }), + ), + ).rejects.toMatchObject({ statusCode: 403 }); + }); + + it('lets the user copy it', async () => { + await runWithContext({ actor: owner.actor }, () => + fs.copy(owner.userId, { + source: appData, + destinationParent: desktop, + newName: 'grab', + }), + ); + expect( + await entryAt(owner, `/Desktop/grab/${closedUid}/state.json`), + ).toBeTruthy(); + }); + + it('lets an app copy it while every other app shares', async () => { + await fs.remove(owner.userId, { + entry: (await entryAt(owner, `/AppData/${closedUid}`))!, + recursive: true, + }); + await asCalendar(() => + fs.copy(owner.userId, { + source: appData, + destinationParent: desktop, + newName: 'grab', + }), + ); + expect( + await entryAt( + owner, + `/Desktop/grab/${contacts.uid}/state.json`, + ), + ).toBeTruthy(); + }); + + it("leaves a closed app's entries out of an app's recursive listing", async () => { + const list = () => + fs.listDirectoryTreePage(owner.userId, owner.home, { + maxDepth: 10, + limit: 1000, + }); + const paths = (await asCalendar(list)).entries.map((e) => e.path); + expect(paths).toContain(contactsFile.path); + expect(paths).not.toContain(closedFile.path); + expect(paths).not.toContain(`${owner.home}/AppData/${closedUid}`); + + const asUser = await runWithContext({ actor: owner.actor }, list); + expect(asUser.entries.map((e) => e.path)).toContain( + closedFile.path, + ); + }); + }); }); describe('FSService AppData root guard', () => { diff --git a/src/backend/services/fs/FSService.ts b/src/backend/services/fs/FSService.ts index e9c3a7dd5..2c5742eb1 100644 --- a/src/backend/services/fs/FSService.ts +++ b/src/backend/services/fs/FSService.ts @@ -73,8 +73,11 @@ import { AclMode } from '../acl/ACLService.js'; import type { puterServices } from '../index.js'; import { APP_DATA_FS_MODE_CLASSES, + appDataOwnerOf, appDataPermission, appDataSharingAllowed, + carriesClosedAppData, + closedAppDataOwners, } from '../permission/appDataScopes.js'; import { MANAGE_PERM_PREFIX } from '../permission/consts.js'; import { PermissionUtil } from '../permission/permissionUtil.js'; @@ -142,11 +145,8 @@ const foreignAppDataOwner = ( username: string, ownAppUid: string, ): string | null => { - const segments = path.split('/'); - if (segments[1] !== username || segments[2]?.toLowerCase() !== 'appdata') { - return null; - } - const appUid = segments[3]; + if (path.split('/')[1] !== username) return null; + const appUid = appDataOwnerOf(path); if (!appUid || appUid === ownAppUid) return null; return appUid; }; @@ -829,7 +829,7 @@ export class FSService extends PuterService { if (pathReservedInBatch) { throw new HttpError( 409, - `Batch contains duplicate target path: ${normalizedInput.path}`, + `Batch contains duplicate target path: ${pathPosix.basename(normalizedInput.path)}`, { legacyCode: 'conflict' }, ); } @@ -850,7 +850,7 @@ export class FSService extends PuterService { } else if (pathReservedInBatch) { throw new HttpError( 409, - `Batch contains duplicate target path: ${normalizedInput.path}`, + `Batch contains duplicate target path: ${pathPosix.basename(normalizedInput.path)}`, { legacyCode: 'conflict' }, ); } @@ -2322,7 +2322,7 @@ export class FSService extends PuterService { if (directoryPathSet.has(targetPath)) { throw new HttpError( 409, - `Batch contains duplicate target path: ${targetPath}`, + `Batch contains duplicate target path: ${pathPosix.basename(targetPath)}`, { legacyCode: 'conflict' }, ); } @@ -3724,7 +3724,29 @@ export class FSService extends PuterService { sortOrder?: 'asc' | 'desc' | null; }, ): Promise<{ entries: FSEntry[]; cursor?: string }> { - return this.stores.fsEntry.listDescendantsPage(userId, path, options); + const page = await this.stores.fsEntry.listDescendantsPage( + userId, + path, + options, + ); + // Listing a home or AppData reaches into every app's AppData. + const app = (Context.get('actor') as Actor | undefined)?.effectiveApp; + if (!app) return page; + const closed = await closedAppDataOwners( + this.stores.app, + page.entries.map((entry) => entry.path), + app.uid, + ); + if (closed.size === 0) return page; + return { + ...page, + entries: page.entries.filter( + (entry) => + !closed.has( + appDataOwnerOf(entry.path)?.toLowerCase() ?? '', + ), + ), + }; } async countDirectoryTree( @@ -4001,7 +4023,7 @@ export class FSService extends PuterService { } else { throw new HttpError( 409, - `An entry already exists at ${targetPath}`, + `An entry named \`${name}\` already exists`, { legacyCode: 'conflict' }, ); } @@ -4035,7 +4057,7 @@ export class FSService extends PuterService { if (raced) { throw new HttpError( 409, - `An entry already exists at ${insertedPath}`, + `An entry named \`${name}\` already exists`, { legacyCode: 'conflict' }, ); } @@ -4098,6 +4120,7 @@ export class FSService extends PuterService { if (entry.name === newName) return entry; this.#assertAppDataRootsStay(entry.path); await this.#assertCrossAppDeleteAllowed(entry.path); + await this.#assertCarriesNoClosedAppData(entry, 'rename'); const parentPath = pathPosix.dirname(entry.path); const newPath = @@ -4106,9 +4129,11 @@ export class FSService extends PuterService { // Reject if another entry already owns the target path. const collision = await this.stores.fsEntry.getEntryByPath(newPath); if (collision && collision.uuid !== entry.uuid) { - throw new HttpError(409, `An entry already exists at ${newPath}`, { - legacyCode: 'conflict', - }); + throw new HttpError( + 409, + `An entry named \`${newName}\` already exists`, + { legacyCode: 'conflict' }, + ); } const updated = await this.stores.fsEntry.updateEntry(entry.uuid, { @@ -4163,7 +4188,7 @@ export class FSService extends PuterService { } else { throw new HttpError( 409, - `An entry already exists at ${childPath}`, + `An entry named \`${name}\` already exists`, { legacyCode: 'conflict' }, ); } @@ -4270,6 +4295,22 @@ export class FSService extends PuterService { ); } + /** An app may not carry another app's closed AppData out of AppData. */ + async #assertCarriesNoClosedAppData( + source: FSEntry, + verb: string, + ): Promise { + const app = (Context.get('actor') as Actor | undefined)?.effectiveApp; + if (!app) return; + if (await carriesClosedAppData(this.stores, source, app.uid)) { + throw new HttpError( + 403, + `Cannot ${verb} a folder holding another app's private data`, + { legacyCode: 'forbidden' }, + ); + } + } + /** * Only the account moving within its own tree (source _and_ destination * both its own) may set these freely; anyone else gets them stripped, or @@ -4840,6 +4881,7 @@ export class FSService extends PuterService { // The source only: moving *into* another app's AppData is a write, and // ACL plus the fs:write class already cover that. await this.#assertCrossAppDeleteAllowed(source.path); + await this.#assertCarriesNoClosedAppData(source, 'move'); await this.#assertCanRestructure(source, userId); // Write inside a shared folder reorganizes it, it does not empty it. if ( @@ -5031,14 +5073,10 @@ export class FSService extends PuterService { // v1 wire contract: clients (the GUI's move/paste flows among them) // key on `item_with_same_name_exists` + `entry_name` to offer a // replace/skip prompt. - throw new HttpError( - 409, - `An entry already exists at ${occupant.path}`, - { - legacyCode: 'item_with_same_name_exists', - fields: { entry_name: name }, - }, - ); + throw new HttpError(409, `An entry named \`${name}\` already exists`, { + legacyCode: 'item_with_same_name_exists', + fields: { entry_name: name }, + }); } /** @@ -5077,6 +5115,7 @@ export class FSService extends PuterService { { legacyCode: 'cannot_copy_directory_into_itself' }, ); } + await this.#assertCarriesNoClosedAppData(source, 'copy'); let name = input.newName ?? source.name; this.#assertUsableName(name); @@ -5119,7 +5158,7 @@ export class FSService extends PuterService { // v1 wire contract, as in move() above. throw new HttpError( 409, - `An entry already exists at ${targetPath}`, + `An entry named \`${name}\` already exists`, { legacyCode: 'item_with_same_name_exists', fields: { entry_name: name }, diff --git a/src/backend/services/localworker/LocalWorkerService.test.ts b/src/backend/services/localworker/LocalWorkerService.test.ts index c1d484c28..4610eefbc 100644 --- a/src/backend/services/localworker/LocalWorkerService.test.ts +++ b/src/backend/services/localworker/LocalWorkerService.test.ts @@ -192,6 +192,36 @@ describe('LocalWorkerService.reconstructDeployArgs', () => { ), ).rejects.toThrow(/source file not found/); }); + + it('refuses a worker whose source file belongs to another user', async () => { + const other = await createTestUser(server, { + username: 'lws-other', + password: 'local-worker-password', + }); + const otherRow = (await server.stores.user.getByUsername( + other.username, + ))!; + const path = `/${other.username}/worker-f.js`; + await server.services.fs.write(otherRow.id, { + fileMetadata: { + path, + size: 2, + contentType: 'text/javascript', + overwrite: true, + }, + fileContent: Readable.from(Buffer.from('//')), + }); + const foreign = await server.stores.fsEntry.getEntryByPath(path); + + await expect( + localWorkers.reconstructDeployArgs( + 'worker-f', + await seedWorkerRow('worker-f', { + root_dir_id: foreign!.id as number, + }), + ), + ).rejects.toThrow(/not owned by the worker's owner/); + }); }); describe('LocalWorkerService.cfCallLocal', () => { diff --git a/src/backend/services/localworker/LocalWorkerService.ts b/src/backend/services/localworker/LocalWorkerService.ts index 2dc475c39..0718d02e5 100644 --- a/src/backend/services/localworker/LocalWorkerService.ts +++ b/src/backend/services/localworker/LocalWorkerService.ts @@ -295,6 +295,11 @@ export class LocalWorkerService extends PuterService { `Local: worker ${workerName} source file not found (id=${row.root_dir_id})`, ); } + if (sourceEntry.userId !== ownerUser.id) { + throw new Error( + `Local: worker ${workerName} source file is not owned by the worker's owner`, + ); + } const loaded = await loadFileInput( { diff --git a/src/backend/services/permission/appDataScopes.ts b/src/backend/services/permission/appDataScopes.ts index a54da65d4..4a2690a80 100644 --- a/src/backend/services/permission/appDataScopes.ts +++ b/src/backend/services/permission/appDataScopes.ts @@ -17,6 +17,9 @@ * along with this program. If not, see . */ +import { validate as validateUuid } from 'uuid'; +import type { FSEntry } from '../../stores/fs/FSEntry'; +import type { FSEntryStore } from '../../stores/fs/FSEntryStore'; import { PermissionUtil } from './permissionUtil'; /** Root of the cross-app application-data permission namespace. */ @@ -145,6 +148,21 @@ export const parseAppDataPermission = ( return { targetAppUid, store, op }; }; +/** + * The app whose `//AppData/` subtree `path` sits in, under + * any user, or null. `AppData` matches case-insensitively, like the column. + * Only an `app-` name counts: any other folder there belongs to no app, + * and looking it up would only miss (or, outside latin1, fail the query). + */ +export const appDataOwnerOf = (path: string): string | null => { + const segments = path.split('/'); + if (segments[2]?.toLowerCase() !== 'appdata') return null; + const appUid = segments[3] ?? ''; + return /^app-/i.test(appUid) && validateUuid(appUid.slice(4)) + ? appUid + : null; +}; + /** * Whether `app` lets other apps reach its per-user data. * @@ -164,3 +182,78 @@ export const appDataSharingAllowed = (app: { metadata?: unknown }): boolean => { if (!metadata || typeof metadata !== 'object') return true; return (metadata as { share_app_data?: unknown }).share_app_data !== false; }; + +type AppLookup = { + getByUids( + uids: string[], + ): Promise>; +}; + +/** + * Lowercased uids of the apps, other than `ownAppUid`, whose AppData holds one + * of `paths` and that keep it closed. Reads nothing unless a path is in another + * app's AppData. + */ +export const closedAppDataOwners = async ( + apps: AppLookup, + paths: Iterable, + ownAppUid: string, +): Promise> => { + const own = ownAppUid.toLowerCase(); + const owners = new Set(); + for (const path of paths) { + const uid = appDataOwnerOf(path)?.toLowerCase(); + if (uid && uid !== own) owners.add(uid); + } + if (owners.size === 0) return owners; + const rows = await apps.getByUids([...owners]); + return new Set( + [...rows.values()] + .filter((app) => !appDataSharingAllowed(app)) + .map((app) => app.uid.toLowerCase()), + ); +}; + +const APP_DATA_PAGE_SIZE = 1000; + +/** + * Whether `folder` is a home or `AppData` folder holding the AppData of an app + * other than `ownAppUid` that keeps it closed. Copying, moving or sharing such + * a folder hands that data on past the opt-out. Anything else answers false + * without a read. + */ +export const carriesClosedAppData = async ( + stores: { + fsEntry: Pick; + app: AppLookup; + }, + folder: FSEntry, + ownAppUid: string, +): Promise => { + if (!folder.isDir) return false; + const segments = folder.path.split('/'); + const isHome = segments.length === 2 && segments[1] !== ''; + const isAppData = + segments.length === 3 && segments[2]!.toLowerCase() === 'appdata'; + if (!isHome && !isAppData) return false; + + const appData = isAppData + ? folder + : await stores.fsEntry.getEntryByPath(`${folder.path}/AppData`); + if (!appData) return false; + let after: FSEntry | undefined; + for (;;) { + const page = await stores.fsEntry.listChildrenOfParents([appData], { + limit: APP_DATA_PAGE_SIZE, + after, + }); + const closed = await closedAppDataOwners( + stores.app, + page.map((entry) => entry.path), + ownAppUid, + ); + if (closed.size > 0) return true; + if (page.length < APP_DATA_PAGE_SIZE) return false; + after = page[page.length - 1]; + } +}; diff --git a/src/backend/services/share/ShareService.test.ts b/src/backend/services/share/ShareService.test.ts index 2e295a550..967fe91e4 100644 --- a/src/backend/services/share/ShareService.test.ts +++ b/src/backend/services/share/ShareService.test.ts @@ -1591,6 +1591,97 @@ describe('ShareService', () => { ).toBe(false); }); + it('hides what a delegate shared once their own authority is withdrawn outside unshare', async () => { + const owner = await makeUser(); + const delegate = await makeUser(); + const third = await makeUser(); + const file = await makeFile(owner.user); + + await share(owner.actor, { + uid: file.uuid, + recipient: { email: delegate.email }, + mode: 'manage', + }); + await share(delegate.actor, { + uid: file.uuid, + recipient: { email: third.email }, + mode: 'read', + }); + expect( + (await listSharedByMe(delegate.actor)).items.map( + (i) => i.holder.username, + ), + ).toEqual([third.user.username]); + + await runWithContext({ actor: owner.actor }, () => + server.services.permission.revokeUserUserPermission( + owner.actor, + delegate.user.username!, + `manage:fs:${file.uuid}`, + ), + ); + + expect((await listSharedByMe(delegate.actor)).items).toEqual([]); + // The grant itself stands, so the owner still sees it to revoke. + const ownerView = await listSharedByMe(owner.actor); + expect( + ownerView.items.find( + (i) => i.holder.username === third.user.username, + )?.issuer.username, + ).toBe(delegate.user.username); + }); + + it('hides a team share once the delegate loses the team that made them one', async () => { + const owner = await makeUser(); + const delegate = await makeUser(); + const third = await makeUser(); + const file = await makeFile(owner.user); + + const viaTeam = await server.stores.team.create({ + ownerUserId: owner.user.id, + name: `t-${uuidv4().slice(0, 8)}`, + }); + const audience = await server.stores.team.create({ + ownerUserId: delegate.user.id, + name: `t-${uuidv4().slice(0, 8)}`, + }); + for (const [team, members] of [ + [viaTeam, [owner, delegate]], + [audience, [delegate, third]], + ] as const) { + for (const each of members) { + await server.stores.team.addMember(team.uid, each.user.id, { + orgOwned: false, + }); + } + } + + await share(owner.actor, { + uid: file.uuid, + recipient: { team: viaTeam.uid }, + mode: 'manage', + }); + await share(delegate.actor, { + uid: file.uuid, + recipient: { team: audience.uid }, + mode: 'read', + }); + const holderTeams = async (actor: Actor) => + (await listSharedByMe(actor)).items.map( + (i) => i.holderTeam?.uid, + ); + // Asked of the owner first: a delegate's own reading would sit in + // the permission cache past the team's deletion. + expect(await holderTeams(owner.actor)).toContain(audience.uid); + + await server.services.team.deleteTeam(viaTeam.uid, owner.user.id); + + expect(await holderTeams(delegate.actor)).not.toContain( + audience.uid, + ); + expect(await holderTeams(owner.actor)).toContain(audience.uid); + }); + it('reads the app off an invite recorded under the legacy key', async () => { const owner = await makeUser(); const file = await makeFile(owner.user); @@ -3515,6 +3606,61 @@ describe('ShareService', () => { ); expect(byOwner).toHaveLength(3); }); + + it("refuses its user's AppData while another app there keeps its data closed", async () => { + const owner = await makeUser(); + const recipient = await makeUser(); + const app = await makeApp(owner.user.id); + const closed = await server.stores.app.create( + { + name: `share-app-${uuidv4()}`, + title: 'Closed app', + index_url: `https://share-${uuidv4()}.test/`, + metadata: JSON.stringify({ share_app_data: false }), + }, + { ownerUserId: owner.user.id }, + ); + const file = await makeAppDataFile(owner, closed); + const appData = await server.stores.fsEntry.getEntryByPath( + `/${owner.user.username}/AppData`, + ); + await grantAppReach(owner, app, appData); + const input = { + uid: appData.uuid, + recipient: { username: recipient.user.username }, + mode: 'read', + }; + + await expect(share(asApp(owner, app), input)).rejects.toMatchObject( + { statusCode: 403 }, + ); + expect(await canRead(recipient.actor, file.path)).toBe(false); + + // The user can still share it. + await share(owner.actor, input); + expect(await canRead(recipient.actor, file.path)).toBe(true); + }); + + it("shares its user's AppData while every app there shares", async () => { + const owner = await makeUser(); + const recipient = await makeUser(); + const app = await makeApp(owner.user.id); + const file = await makeAppDataFile( + owner, + await makeApp(owner.user.id), + ); + const appData = await server.stores.fsEntry.getEntryByPath( + `/${owner.user.username}/AppData`, + ); + await grantAppReach(owner, app, appData); + + await share(asApp(owner, app), { + uid: appData.uuid, + recipient: { username: recipient.user.username }, + mode: 'read', + }); + expect(await canRead(recipient.actor, file.path)).toBe(true); + }); }); // The other derived actor: same reach bound, a different arm of the check. diff --git a/src/backend/services/share/ShareService.ts b/src/backend/services/share/ShareService.ts index 128234518..3c7f9626e 100644 --- a/src/backend/services/share/ShareService.ts +++ b/src/backend/services/share/ShareService.ts @@ -34,6 +34,7 @@ import { cleanEmail, isBlockedEmail, isProviderCanonicalized, + isSameAddress, isStorableEmail, } from '../../util/email.js'; import type { FSEntry } from '../../stores/fs/FSEntry'; @@ -51,6 +52,7 @@ import { actorHasSubscription, assertActorHasSubscription, } from '../metering/enforcement.js'; +import { carriesClosedAppData } from '../permission/appDataScopes'; import { MANAGE_PERM_PREFIX } from '../permission/consts'; import { PermissionUtil } from '../permission/permissionUtil.js'; import { SubscriptionCache } from '../events/subscriptionCache.js'; @@ -972,6 +974,15 @@ export class ShareService extends PuterService { // exist" may only be observed by someone entitled to share. const entry = await this.#resolveEntry(input, actor); await this.#assertCanManage(actor, entry, mode); + // The recipient reads as a user, past the per-app opt-out. + const app = actor.effectiveApp; + if (app && (await carriesClosedAppData(this.stores, entry, app.uid))) { + throw new HttpError( + 403, + "Cannot share a folder holding another app's private data", + { legacyCode: 'forbidden' }, + ); + } const resolved = await this.#resolveRecipient(input.recipient); if (resolved.kind === 'anyone') { @@ -2029,22 +2040,22 @@ export class ShareService extends PuterService { // Three bounds. A claimed row needs the grant *this issuer* made to // still be there — on the pair alone, a grant withdrawn outside // `unshare` stays listed while anyone else grants the same holder the - // same node. An invite needs its issuer to still hold the authority it - // would grant, or a revoked delegate keeps reading the entry's name - // and size out of invites that can never be claimed. And an app sees - // only the part of any of it the credential reaches in its own right. - const [stillReaches, pendingAllowed, reachable, liveGroupGrants] = + // same node. An invite, and any row a delegate issued on a node they + // don't own, needs its issuer to still hold the authority it granted, + // or a revoked delegate keeps reading the entry's name, size and + // thumbnail. And an app sees only the part of any of it the + // credential reaches in its own right. + const [stillReaches, issuerAllowed, reachable, liveGroupGrants] = await Promise.all([ this.#reachingGrants(rows, nodeById), - this.#pendingStillAuthorized( - // Group and link rows have no holder user but are not - // invites. - rows.filter( - (row) => - !row.holder_user_id && - !row.holder_group_id && - !row.anyone, - ), + this.#issuerStillAuthorized( + rows.filter((row) => { + if (row.anyone) return false; + const pending = + !row.holder_user_id && !row.holder_group_id; + const node = nodeById.get(Number(row.fsentry_id)); + return pending || node?.userId !== userId; + }), nodeById, users, ), @@ -2068,7 +2079,10 @@ export class ShareService extends PuterService { const anyone = Boolean(row.anyone); const pending = !anyone && !row.holder_user_id && !row.holder_group_id; - if (pending && !pendingAllowed.has(row.uid)) continue; + const delegated = !anyone && entry.userId !== userId; + if ((pending || delegated) && !issuerAllowed.has(row.uid)) { + continue; + } // Its liveness is the grant, not a holder's reach. if (anyone) { // Silent while the plan is lapsed, so not listed either. @@ -3346,15 +3360,6 @@ export class ShareService extends PuterService { }; } - /** - * Of `rows` (unclaimed invites), the uids whose issuer still holds the - * authority the invite would grant. A dead invite can never become access — - * claiming re-authorizes — but listed it would keep publishing the entry's - * name and size to an issuer whose own access was revoked. - * - * The owner's invites are theirs by definition; only a delegate's cost a - * check, and those are rare on any page. - */ /** * Group rows on this page whose grant is still there. The index row and the * grant are separate writes, so a row can outlive what it records. @@ -3403,29 +3408,47 @@ export class ShareService extends PuterService { return live; } - async #pendingStillAuthorized( + /** + * Of `rows`, the uids whose issuer still holds the authority the row + * grants. A dead invite can never become access, and a delegate's dead + * grant is not theirs to revoke, but listed either would keep publishing + * the entry to an issuer whose own access was withdrawn. + * + * The owner's rows are theirs by definition; a delegate's cost one check + * per issuer, node and mode. + */ + async #issuerStillAuthorized( rows: OutboundShareRow[], nodeById: Map, users: Map, ): Promise> { const allowed = new Set(); - const toCheck: OutboundShareRow[] = []; + const toCheck = new Map(); for (const row of rows) { const entry = nodeById.get(Number(row.fsentry_id)); const issuer = users.get(Number(row.issuer_user_id)); if (!entry || !issuer?.username) continue; - if (entry.userId === issuer.id) allowed.add(row.uid); - else toCheck.push(row); + if (entry.userId === issuer.id) { + allowed.add(row.uid); + continue; + } + const key = `${issuer.id}:${entry.id}:${row.mode}`; + toCheck.set(key, [...(toCheck.get(key) ?? []), row]); } - await runWithConcurrencyLimitSettled(toCheck, 8, async (row) => { - const entry = nodeById.get(Number(row.fsentry_id)) as FSEntry; - const issuer = users.get(Number(row.issuer_user_id)) as UserRow; - const ok = await this.services.permission.canManagePermission( - this.#actorFor(issuer), - entryPermissionForMode(entry.uuid, String(row.mode)), - ); - if (ok) allowed.add(row.uid); - }); + await runWithConcurrencyLimitSettled( + [...toCheck.values()], + 8, + async (group) => { + const row = group[0] as OutboundShareRow; + const entry = nodeById.get(Number(row.fsentry_id)) as FSEntry; + const issuer = users.get(Number(row.issuer_user_id)) as UserRow; + const ok = await this.services.permission.canManagePermission( + this.#actorFor(issuer), + entryPermissionForMode(entry.uuid, String(row.mode)), + ); + if (ok) for (const each of group) allowed.add(each.uid); + }, + ); return allowed; } @@ -3569,9 +3592,8 @@ export class ShareService extends PuterService { async #addressOwner(email: string): Promise { const owner = await this.stores.user.findEmailOwner(email); if (!owner?.email) return owner ?? null; - const sameAddress = - owner.email.trim().toLowerCase() === email.trim().toLowerCase(); - if (sameAddress || isProviderCanonicalized(email)) return owner; + if (isSameAddress(owner.email, email) || isProviderCanonicalized(email)) + return owner; return null; } diff --git a/src/backend/services/user/SignupService.test.ts b/src/backend/services/user/SignupService.test.ts index 0250aee02..ecaea89db 100644 --- a/src/backend/services/user/SignupService.test.ts +++ b/src/backend/services/user/SignupService.test.ts @@ -239,6 +239,33 @@ describe('SignupService.confirmEmail', () => { expect(after!.email_confirm_code).toBeNull(); }); + it('refuses with a 400 when the address index already has an owner', async () => { + const user = await makeUnconfirmed(); + // An owner row the address checks don't strip (no clean_email, a + // spelling sqlite compares unequal) whose index key still collides. + const rival = await server.stores.user.create({ + username: `ce_${slug()}`, + uuid: crypto.randomUUID(), + password: 'hashed', + email: `x${slug()}@example.com`, + }); + await server.clients.db.write( + 'UPDATE `user` SET `email` = ?, `clean_email` = NULL WHERE `id` = ?', + [user.email!.toUpperCase(), rival.id], + ); + + await expect( + server.services.signup.confirmEmail(user), + ).rejects.toMatchObject({ + statusCode: 400, + legacyCode: 'email_already_in_use', + }); + const after = await server.stores.user.getById(user.id, { + force: true, + }); + expect(after!.email_confirmed).toBeFalsy(); + }); + it('leaves another account no way to take the address through a pending change', async () => { const owner = await makeUnconfirmed(); const other = await makeUnconfirmed(); diff --git a/src/backend/services/user/SignupService.ts b/src/backend/services/user/SignupService.ts index 7f1c3af0c..40daac86b 100644 --- a/src/backend/services/user/SignupService.ts +++ b/src/backend/services/user/SignupService.ts @@ -650,12 +650,21 @@ export class SignupService extends PuterService { email, canonical, ); - await this.stores.user.update(user.id, { - email_confirmed: 1, - requires_email_confirmation: 0, - email_confirm_code: null, - email_confirm_token: null, - }); + try { + await this.stores.user.update(user.id, { + email_confirmed: 1, + requires_email_confirmation: 0, + email_confirm_code: null, + email_confirm_token: null, + }); + } catch (e) { + // An owner the checks above don't strip, such as a variant + // spelling the index's collation folds together. + if (!isOwnedEmailConflict(e)) throw e; + throw new HttpError(400, 'This email is already in use.', { + legacyCode: 'email_already_in_use' as never, + }); + } await promoteToVerifiedGroup(this.stores.group, this.config, user); // Best-effort side channels. Other tabs refresh; the one that sent diff --git a/src/backend/services/user/UserAccountService.test.ts b/src/backend/services/user/UserAccountService.test.ts index 1d1416513..6c5587b88 100644 --- a/src/backend/services/user/UserAccountService.test.ts +++ b/src/backend/services/user/UserAccountService.test.ts @@ -275,6 +275,33 @@ describe('UserAccountService', () => { ).toBeNull(); }); + it("drops the cached sites that go with the account's apps", async () => { + const developer = await seedUser(); + const siteOwner = await seedUser(); + const app = await server.stores.app.create( + { + name: `ua-app-${uuidv4()}`, + title: 'Hosting', + index_url: `https://${uuidv4()}.example.com/`, + }, + { ownerUserId: developer.id }, + ); + const site = await server.stores.subdomain.create({ + userId: siteOwner.id, + subdomain: `ua-app-site-${Math.random().toString(36).slice(2, 10)}`, + appOwner: app.id, + }); + + const cacheKey = `subdomains:name:${site.subdomain}`; + expect(await server.clients.redis.get(cacheKey)).not.toBeNull(); + + await server.services.userAccount.cascadeDelete(developer.id); + + // SQLite keeps the app and its site (MySQL and Postgres cascade + // both); either way the cached copy has to go. + expect(await server.clients.redis.get(cacheKey)).toBeNull(); + }); + it('finishes when dropping the seat caches fails', async () => { const user = await seedUser(); const capture = vi @@ -302,6 +329,26 @@ describe('UserAccountService', () => { expect(await server.stores.user.getById(user.id)).toBeNull(); }); + it("records the account's apps as deleted", async () => { + const user = await seedUser(); + const app = await server.stores.app.create( + { + name: `ua-app-${uuidv4()}`, + title: 'Owned', + index_url: `https://${uuidv4()}.example.com/`, + }, + { ownerUserId: user.id }, + ); + + await server.services.userAccount.cascadeDelete(user.id); + + expect( + (await server.stores.app.getDeletedOwnersByUids([app.uid])).get( + app.uid, + ), + ).toBe(user.id); + }); + it('frees the address for a new account', async () => { const user = await seedUser(); const email = user.email as string; diff --git a/src/backend/services/user/UserAccountService.ts b/src/backend/services/user/UserAccountService.ts index 6e50b1eae..3d7875e1c 100644 --- a/src/backend/services/user/UserAccountService.ts +++ b/src/backend/services/user/UserAccountService.ts @@ -100,6 +100,14 @@ export class UserAccountService extends PuterService { console.warn('[cascade-delete-user] subdomain cleanup failed:', e); } + // The user's apps cascade away with the row; record them first so a + // hosted origin doesn't hand their uids to the subdomain's next owner. + await this.stores.app.recordDeletedForOwner(userId); + // Other users' sites and workers tied to those apps cascade too. + const appSites = await this.stores.subdomain.listAppCascade({ + ownerUserId: userId, + }); + // Sessions FK is SET NULL, so delete explicitly to avoid dangling rows. await this.clients.db.write( 'DELETE FROM `sessions` WHERE `user_id` = ?', @@ -110,6 +118,7 @@ export class UserAccountService extends PuterService { ]); if (cachedIdentity) await this.stores.user.markDeleted(cachedIdentity); else await this.stores.user.invalidateById(userId); + await this.stores.subdomain.forgetCascadeDeleted(appSites); if (issuedRefs.length > 0) { try { diff --git a/src/backend/stores/app/AppStore.js b/src/backend/stores/app/AppStore.js index 8c2fca6a4..406411fd7 100644 --- a/src/backend/stores/app/AppStore.js +++ b/src/backend/stores/app/AppStore.js @@ -810,13 +810,21 @@ export class AppStore extends PuterStore { } /** - * Delete an app row. Also deletes its filetype associations. Invalidates - * cache. + * Delete an app row. Also deletes its filetype associations, records its + * uid and owner in `deleted_apps`, and invalidates cache, its own and that + * of the sites and workers that cascade away with it. */ async delete(appId) { const app = await this.getById(appId); if (!app) return false; + const sites = await this.stores.subdomain.listAppCascade({ appId }); + + const owner = app.owner_user_id ?? null; + await this.clients.db.write( + `INSERT INTO \`deleted_apps\` (\`uid\`, \`owner_user_id\`) VALUES (?, ?) ${this.#deletedAppsUpsert()}`, + [app.uid, owner, owner], + ); await this.clients.db.write( 'DELETE FROM `app_filetype_association` WHERE `app_id` = ?', [appId], @@ -826,9 +834,56 @@ export class AppStore extends PuterStore { ]); // Tombstone, so a lagging replica can't cache the row back for a day. await this.markDeleted(app); + await this.stores.subdomain.forgetCascadeDeleted(sites); return true; } + /** + * Record every app `userId` owns in `deleted_apps`, as {@link delete} does. + * Call before deleting the user: the `apps` rows go with it by FK cascade. + */ + async recordDeletedForOwner(userId) { + await this.clients.db.write( + `INSERT INTO \`deleted_apps\` (\`uid\`, \`owner_user_id\`) SELECT \`uid\`, \`owner_user_id\` FROM \`apps\` WHERE \`owner_user_id\` = ? ${this.#deletedAppsUpsert()}`, + [userId, userId], + ); + } + + /** + * Owner each of `uids` had when its app was deleted (null if it had none). + * Uids that never belonged to a deleted app are absent. + * + * @param {string[]} uids + * @returns {Promise>} + */ + async getDeletedOwnersByUids(uids) { + const unique = [...new Set(uids)]; + const out = new Map(); + for ( + let offset = 0; + offset < unique.length; + offset += BULK_QUERY_CHUNK_SIZE + ) { + const chunk = unique.slice(offset, offset + BULK_QUERY_CHUNK_SIZE); + const rows = await this.clients.db.read( + `SELECT \`uid\`, \`owner_user_id\` FROM \`deleted_apps\` WHERE \`uid\` IN (${chunk.map(() => '?').join(', ')})`, + chunk, + ); + for (const row of rows) { + const owner = Number(row.owner_user_id); + out.set( + String(row.uid), + row.owner_user_id != null && owner > 0 ? owner : null, + ); + } + } + return out; + } + + #deletedAppsUpsert() { + return this.clients.db.upsertClause(['uid'], ['owner_user_id']); + } + // -- Filetype associations ---------------------------------------- async getFiletypeAssociations(appId) { diff --git a/src/backend/stores/app/AppStore.test.js b/src/backend/stores/app/AppStore.test.js index 0ab3029bf..533be320d 100644 --- a/src/backend/stores/app/AppStore.test.js +++ b/src/backend/stores/app/AppStore.test.js @@ -773,10 +773,94 @@ describe('AppStore CRUD and cache invalidation', () => { expect(await appStore.getFiletypeAssociations(app.id)).toEqual([]); }); + it('drops the cached sites and workers that go with a deleted app', async () => { + const subdomains = server.stores.subdomain; + const app = await createApp(); + const slug = freshName(); + const site = await subdomains.create({ + userId: OTHER_OWNER, + subdomain: `${slug}-site`, + appOwner: app.id, + }); + const worker = await subdomains.create({ + userId: OTHER_OWNER, + subdomain: `workers.puter.${slug}`, + appOwner: app.id, + }); + // SQLite keeps this row (MySQL and Postgres cascade it); either way + // its cached copy has to go. + const associated = await subdomains.create({ + userId: OTHER_OWNER, + subdomain: `${slug}-associated`, + associatedAppId: app.id, + }); + const cacheKey = (row) => `subdomains:name:${row.subdomain}`; + expect(await redis.get(cacheKey(site))).not.toBeNull(); + + await appStore.delete(app.id); + + expect(await subdomains.getBySubdomain(site.subdomain)).toBeNull(); + expect(await subdomains.getBySubdomain(worker.subdomain)).toBeNull(); + expect(await redis.get(cacheKey(associated))).toBeNull(); + }); + it('reports false when deleting an app that is already gone', async () => { expect(await appStore.delete(99999999)).toBe(false); }); + it("records a deleted app's uid with its owner", async () => { + const owned = await createApp(); + const unowned = await appStore.createFromOrigin( + freshUid(), + `https://${freshName()}.example.com`, + ); + await appStore.delete(owned.id); + await appStore.delete(unowned.id); + + const live = await createApp(); + const deleted = await appStore.getDeletedOwnersByUids([ + owned.uid, + unowned.uid, + live.uid, + ]); + expect(deleted).toEqual( + new Map([ + [owned.uid, OWNER], + [unowned.uid, null], + ]), + ); + }); + + it("records every app an owner has, and a later owner's delete replaces it", async () => { + const first = await createApp({}, { ownerUserId: OTHER_OWNER }); + const second = await createApp({}, { ownerUserId: OTHER_OWNER }); + const notTheirs = await createApp(); + + await appStore.recordDeletedForOwner(OTHER_OWNER); + expect( + await appStore.getDeletedOwnersByUids([ + first.uid, + second.uid, + notTheirs.uid, + ]), + ).toEqual( + new Map([ + [first.uid, OTHER_OWNER], + [second.uid, OTHER_OWNER], + ]), + ); + + await db.write('UPDATE `apps` SET `owner_user_id` = ? WHERE `id` = ?', [ + OWNER, + first.id, + ]); + await appStore.invalidate(first); + await appStore.delete(first.id); + expect( + (await appStore.getDeletedOwnersByUids([first.uid])).get(first.uid), + ).toBe(OWNER); + }); + // -- ownership claiming -------------------------------------------- it('claims an unowned app row and refuses one that already has an owner', async () => { diff --git a/src/backend/stores/fs/FSEntryStore.test.ts b/src/backend/stores/fs/FSEntryStore.test.ts index ed6346a99..164f96ff5 100644 --- a/src/backend/stores/fs/FSEntryStore.test.ts +++ b/src/backend/stores/fs/FSEntryStore.test.ts @@ -2771,7 +2771,7 @@ describe('FSEntryStore lost-insert recovery', () => { ); expect(error.statusCode).toBe(409); - expect(error.message).toContain('Entry already exists at'); + expect(error.message).toContain('already exists'); spy.mockRestore(); }); diff --git a/src/backend/stores/fs/FSEntryStore.ts b/src/backend/stores/fs/FSEntryStore.ts index 06e9f8bf2..4a66b6af5 100644 --- a/src/backend/stores/fs/FSEntryStore.ts +++ b/src/backend/stores/fs/FSEntryStore.ts @@ -825,9 +825,11 @@ export class FSEntryStore extends PuterStore { for (const path of candidatePaths) { const entry = allEntries.get(path); if (entry && !entry.isDir) { - throw new HttpError(409, `Path is not a directory: ${path}`, { - legacyCode: 'conflict', - }); + throw new HttpError( + 409, + `Path is not a directory: ${pathPosix.basename(path)}`, + { legacyCode: 'conflict' }, + ); } } @@ -868,7 +870,7 @@ export class FSEntryStore extends PuterStore { if (parentEntry && !parentEntry.isDir) { throw new HttpError( 409, - `Path is not a directory: ${parentPath}`, + `Path is not a directory: ${pathPosix.basename(parentPath)}`, { legacyCode: 'conflict' }, ); } @@ -930,7 +932,7 @@ export class FSEntryStore extends PuterStore { if (!insertedEntry.isDir) { throw new HttpError( 409, - `Path is not a directory: ${path}`, + `Path is not a directory: ${pathPosix.basename(path)}`, { legacyCode: 'conflict' }, ); } @@ -949,14 +951,14 @@ export class FSEntryStore extends PuterStore { // path in #ensureDirectoryPath). Surface as 404. throw new HttpError( 404, - `Parent path does not exist: ${requiredPath}`, + `Parent path does not exist: ${pathPosix.basename(requiredPath)}`, { legacyCode: 'not_found' }, ); } if (!entry.isDir) { throw new HttpError( 409, - `Path is not a directory: ${requiredPath}`, + `Path is not a directory: ${pathPosix.basename(requiredPath)}`, { legacyCode: 'conflict' }, ); } @@ -981,7 +983,7 @@ export class FSEntryStore extends PuterStore { if (!existingEntry.isDir) { throw new HttpError( 409, - `Path is not a directory: ${normalizedPath}`, + `Path is not a directory: ${pathPosix.basename(normalizedPath)}`, { legacyCode: 'conflict' }, ); } @@ -991,7 +993,7 @@ export class FSEntryStore extends PuterStore { if (!createPaths) { throw new HttpError( 404, - `Parent path does not exist: ${normalizedPath}`, + `Parent path does not exist: ${pathPosix.basename(normalizedPath)}`, { legacyCode: 'not_found' }, ); } @@ -1072,14 +1074,14 @@ export class FSEntryStore extends PuterStore { // Error become a 500 + page on-call. throw new HttpError( 404, - `Parent path does not exist: ${normalizedPath}`, + `Parent path does not exist: ${pathPosix.basename(normalizedPath)}`, { legacyCode: 'not_found' }, ); } if (!resolvedEntry.isDir) { throw new HttpError( 409, - `Path is not a directory: ${normalizedPath}`, + `Path is not a directory: ${pathPosix.basename(normalizedPath)}`, { legacyCode: 'conflict' }, ); } @@ -1737,14 +1739,14 @@ export class FSEntryStore extends PuterStore { if (!parentEntry) { throw new HttpError( 404, - `Parent path does not exist: ${normalizedPath}`, + `Parent path does not exist: ${pathPosix.basename(normalizedPath)}`, { legacyCode: 'not_found' }, ); } if (!parentEntry.isDir) { throw new HttpError( 409, - `Path is not a directory: ${normalizedPath}`, + `Path is not a directory: ${pathPosix.basename(normalizedPath)}`, { legacyCode: 'conflict' }, ); } @@ -1803,14 +1805,14 @@ export class FSEntryStore extends PuterStore { if (!entry) { throw new HttpError( 404, - `Directory path does not exist: ${request.path}`, + `Directory path does not exist: ${pathPosix.basename(request.path)}`, { legacyCode: 'not_found' }, ); } if (!entry.isDir) { throw new HttpError( 409, - `Path is not a directory: ${request.path}`, + `Path is not a directory: ${pathPosix.basename(request.path)}`, { legacyCode: 'conflict' }, ); } @@ -1866,7 +1868,7 @@ export class FSEntryStore extends PuterStore { if (Number.isNaN(size) || size < 0) { throw new HttpError( 400, - `Invalid size for path ${targetPath}`, + `Invalid size for ${pathPosix.basename(targetPath)}`, { legacyCode: 'bad_request' }, ); } @@ -1893,7 +1895,7 @@ export class FSEntryStore extends PuterStore { if (duplicatePathSet.has(dedupeKey)) { throw new HttpError( 409, - `Batch contains duplicate target path: ${normalizedEntry.targetPath}`, + `Batch contains duplicate target path: ${pathPosix.basename(normalizedEntry.targetPath)}`, { legacyCode: 'conflict' }, ); } @@ -1944,7 +1946,7 @@ export class FSEntryStore extends PuterStore { if (!parentEntry) { throw new HttpError( 404, - `Parent directory not found for ${entry.targetPath}`, + `Parent directory not found for ${pathPosix.basename(entry.targetPath)}`, { legacyCode: 'dest_does_not_exist' }, ); } @@ -1956,14 +1958,14 @@ export class FSEntryStore extends PuterStore { if (!entry.input.overwrite) { throw new HttpError( 409, - `Entry already exists at ${entry.targetPath}`, + `An entry named \`${pathPosix.basename(entry.targetPath)}\` already exists`, { legacyCode: 'conflict' }, ); } if (existingEntry.isDir) { throw new HttpError( 409, - `Cannot overwrite a directory at ${entry.targetPath}`, + `Cannot overwrite a directory: ${pathPosix.basename(entry.targetPath)}`, { legacyCode: 'cannot_overwrite_a_directory' }, ); } @@ -2097,7 +2099,7 @@ export class FSEntryStore extends PuterStore { if (!parentEntry) { throw new HttpError( 404, - `Parent directory not found for ${entry.targetPath}`, + `Parent directory not found for ${pathPosix.basename(entry.targetPath)}`, { legacyCode: 'dest_does_not_exist' }, ); } @@ -2233,7 +2235,7 @@ export class FSEntryStore extends PuterStore { if (!parentEntry) { throw new HttpError( 404, - `Parent directory not found for ${entry.targetPath}`, + `Parent directory not found for ${pathPosix.basename(entry.targetPath)}`, { legacyCode: 'dest_does_not_exist' }, ); } @@ -2242,7 +2244,7 @@ export class FSEntryStore extends PuterStore { insertConflictErrors.push( new HttpError( 409, - `Entry already exists at ${entry.targetPath}`, + `An entry named \`${pathPosix.basename(entry.targetPath)}\` already exists`, { legacyCode: 'conflict' }, ), ); @@ -2252,7 +2254,7 @@ export class FSEntryStore extends PuterStore { insertConflictErrors.push( new HttpError( 409, - `Cannot overwrite a directory at ${entry.targetPath}`, + `Cannot overwrite a directory: ${pathPosix.basename(entry.targetPath)}`, { legacyCode: 'cannot_overwrite_a_directory' }, ), ); @@ -2262,7 +2264,7 @@ export class FSEntryStore extends PuterStore { insertConflictErrors.push( new HttpError( 409, - `Entry already exists at ${entry.targetPath}`, + `An entry named \`${pathPosix.basename(entry.targetPath)}\` already exists`, { legacyCode: 'conflict' }, ), ); diff --git a/src/backend/stores/session/SessionStore.js b/src/backend/stores/session/SessionStore.js index 81d5e1df9..7033047d4 100644 --- a/src/backend/stores/session/SessionStore.js +++ b/src/backend/stores/session/SessionStore.js @@ -45,6 +45,8 @@ const TOUCH_THROTTLE_MS = 60 * 1000; // long-lived processes. Clearing only loses throttling — at worst a // brief burst of redundant UPDATEs. const TOUCH_THROTTLE_MAX_ENTRIES = 10000; +// Roots per batched revoke statement; each is bound twice. +const REVOKE_CHUNK_SIZE = 500; // Per-kind sliding-expiry windows. The `touch` path bumps `expires_at` // to `now + window` for the session's kind on activity, so an active @@ -107,26 +109,41 @@ export class SessionStore extends PuterStore { const cached = await this.#readCache(uuid); if (cached) return cached; - const rows = await this.clients.db.read( + // Revoked moments ago: a replica may still show the row live. + const key = this.#cacheKey(uuid); + const revokedRecently = await this.isCacheKeyTombstoned([key]); + const rows = await this.clients.db[revokedRecently ? 'pread' : 'read']( 'SELECT * FROM `sessions` WHERE `uuid` = ? LIMIT 1', [uuid], ); const normalized = this.#normalizeRow(rows[0]); if (!normalized) return null; - this.#writeCache(normalized).catch(() => {}); + if (revokedRecently) { + void this.#writeCache(normalized); + } else { + void this.writeCacheUnlessDeleted([key], () => + this.#writeCache(normalized), + ); + } return normalized; } /** * Get sessions for a user. By default returns only active rows; pass `{ - * includeRevoked: true }` to include soft-revoked rows. + * includeRevoked: true }` to include soft-revoked rows. Revoke paths pass + * `{ primary: true }` so a session created moments ago isn't missed. */ - async getByUserId(userId, { includeRevoked = false } = {}) { + async getByUserId( + userId, + { includeRevoked = false, primary = false } = {}, + ) { const sql = includeRevoked ? 'SELECT * FROM `sessions` WHERE `user_id` = ?' : 'SELECT * FROM `sessions` WHERE `user_id` = ? AND `revoked_at` IS NULL'; - const rows = await this.clients.db.read(sql, [userId]); + const rows = await this.clients.db[primary ? 'pread' : 'read'](sql, [ + userId, + ]); return rows.map((r) => this.#normalizeRow(r)).filter(Boolean); } @@ -390,27 +407,56 @@ export class SessionStore extends PuterStore { 'UPDATE `sessions` SET `revoked_at` = ? WHERE `uuid` = ? AND `revoked_at` IS NULL', [now, uuid], ); - await this.publishCacheKeys({ keys, broadcast: true }); - await this.#cacheRevokedFromPrimary(uuid); + await this.#publishRevoked(rows); } /** - * Seed the cache with the revoked row, read from the primary. Dropping the - * key alone leaves the next miss to a replica that may still show the row - * active, which would then be cached for a full TTL. + * Drop every cache key of rows that just ended. A plain DEL leaves the next + * miss to a replica that may still show a row live, cached for a full TTL, + * so the uuid keys are tombstoned here and on peers (misses read the + * primary). */ - async #cacheRevokedFromPrimary(uuid) { + async #dropEndedRowKeys(rows) { + const uuidKeys = new Set(rows.map((r) => this.#cacheKey(r.uuid))); + await this.tombstoneCacheKeys([...uuidKeys]); + await this.publishCacheKeys({ + keys: rows + .flatMap((r) => this.#allCacheKeysForRow(r)) + .filter((key) => !uuidKeys.has(key)), + broadcast: true, + }); + } + + /** + * Post-UPDATE half of the double-delete: tombstone the keys, then cache the + * revoked rows on this node. + */ + async #publishRevoked(rows) { + await this.#dropEndedRowKeys(rows); + try { - const rows = await this.clients.db.pread( - 'SELECT * FROM `sessions` WHERE `uuid` = ? LIMIT 1', - [uuid], - ); - const normalized = this.#normalizeRow(rows[0]); - if (normalized?.revoked_at != null) { - await this.#writeCache(normalized); + const placeholders = rows.map(() => '?').join(', '); + const revoked = ( + await this.clients.db.pread( + `SELECT * FROM \`sessions\` WHERE \`uuid\` IN (${placeholders})`, + rows.map((r) => r.uuid), + ) + ) + .map((r) => this.#normalizeRow(r)) + .filter((r) => r?.revoked_at != null); + if (revoked.length === 0) return; + const pipeline = this.clients.redis.pipeline(); + for (const row of revoked) { + pipeline.set( + this.#cacheKey(row.uuid), + JSON.stringify(row), + 'EX', + CACHE_TTL_SECONDS, + ); } + await pipeline.exec(); } catch { - // No entry is the safe miss; the next read goes to the database. + // No entry is the safe miss; the tombstone sends it to the primary. } } @@ -426,34 +472,55 @@ export class SessionStore extends PuterStore { */ async revokeCascade(rootUuid) { if (!rootUuid) return null; + return (await this.revokeCascades([rootUuid]))[0] ?? null; + } - // Read each affected row's identity columns up-front — every - // composite cache mapping (app, legacy-token, legacy-web) must - // be invalidated alongside the uuid key, otherwise a follow-up - // `getOrCreateApp` would short-circuit to the freshly-revoked row. - const rows = await this.clients.db.read( - 'SELECT `uuid`, `user_id`, `kind`, `app_uid`, `parent_session_id`, `legacy_token_uid`, `meta`, `created_via`, `last_ip`, `last_user_agent` FROM `sessions` WHERE (`uuid` = ? OR `parent_session_id` = ?) AND `revoked_at` IS NULL', - [rootUuid, rootUuid], - ); - if (rows.length === 0) return null; + /** + * `revokeCascade` over many roots, batched so revoking all of a user's + * sessions costs a few primary round trips rather than a few per session. + * Returns `{ userId, uuids }` for each user that had rows revoked. + * + * @param {string[]} rootUuids + */ + async revokeCascades(rootUuids) { + const roots = [...new Set(rootUuids.filter(Boolean).map(String))]; + const uuidsByUser = new Map(); + for (let i = 0; i < roots.length; i += REVOKE_CHUNK_SIZE) { + const chunk = roots.slice(i, i + REVOKE_CHUNK_SIZE); + const placeholders = chunk.map(() => '?').join(', '); + const scope = `(\`uuid\` IN (${placeholders}) OR \`parent_session_id\` IN (${placeholders})) AND \`revoked_at\` IS NULL`; + const scopeParams = [...chunk, ...chunk]; - // Double-delete: see `removeByUuid` for rationale. - const keys = []; - for (const r of rows) keys.push(...this.#allCacheKeysForRow(r)); - await this.publishCacheKeys({ keys, broadcast: true }); + // Read each affected row's identity columns up-front — every + // composite cache mapping (app, legacy-token, legacy-web) must + // be invalidated alongside the uuid key, otherwise a follow-up + // `getOrCreateApp` would short-circuit to the freshly-revoked row. + // Primary read: see `removeByUuid`. + const rows = await this.clients.db.pread( + 'SELECT `uuid`, `user_id`, `kind`, `app_uid`, `parent_session_id`, `legacy_token_uid`, `meta`, `created_via`, `last_ip`, `last_user_agent` ' + + `FROM \`sessions\` WHERE ${scope}`, + scopeParams, + ); + if (rows.length === 0) continue; - const now = nowSeconds(); - await this.clients.db.write( - 'UPDATE `sessions` SET `revoked_at` = ? WHERE (`uuid` = ? OR `parent_session_id` = ?) AND `revoked_at` IS NULL', - [now, rootUuid, rootUuid], - ); + // Double-delete: see `removeByUuid` for rationale. + await this.publishCacheKeys({ + keys: rows.flatMap((r) => this.#allCacheKeysForRow(r)), + broadcast: true, + }); + await this.clients.db.write( + `UPDATE \`sessions\` SET \`revoked_at\` = ? WHERE ${scope}`, + [nowSeconds(), ...scopeParams], + ); + await this.#publishRevoked(rows); - await this.publishCacheKeys({ keys, broadcast: true }); - - return { - userId: Number(rows[0].user_id), - uuids: rows.map((r) => String(r.uuid)), - }; + for (const r of rows) { + const userId = Number(r.user_id); + if (!uuidsByUser.has(userId)) uuidsByUser.set(userId, []); + uuidsByUser.get(userId).push(String(r.uuid)); + } + } + return [...uuidsByUser].map(([userId, uuids]) => ({ userId, uuids })); } /** @@ -516,7 +583,7 @@ export class SessionStore extends PuterStore { `DELETE FROM \`sessions\` WHERE ${scope}`, scopeParams, ); - await this.publishCacheKeys({ keys, broadcast: true }); + await this.#dropEndedRowKeys(rows); return rows.map((r) => ({ uuid: String(r.uuid), @@ -539,7 +606,7 @@ export class SessionStore extends PuterStore { */ async accessTokenUidsForCascade(rootUuid) { if (!rootUuid) return []; - const rows = await this.clients.db.read( + const rows = await this.clients.db.pread( "SELECT `access_token_uid`, `legacy_token_uid` FROM `sessions` WHERE (`uuid` = ? OR `parent_session_id` = ?) AND `kind` = 'access_token' AND `revoked_at` IS NULL", [rootUuid, rootUuid], ); @@ -683,10 +750,10 @@ export class SessionStore extends PuterStore { return cached; } - const existing = await this.#selectAppRow(userId, appUid); + const selectRow = (opts) => this.#selectAppRow(userId, appUid, opts); + const existing = await this.#selectUnlessEnded(selectRow); if (existing && !predates(existing, notBefore)) { - await this.#writeCacheKey(cacheKey, existing); - this.#writeCache(existing).catch(() => {}); + await this.#cacheComposite(cacheKey, existing); return existing; } const stale = existing ?? (cachedLive ? cached : null); @@ -712,12 +779,11 @@ export class SessionStore extends PuterStore { ); // After a revoke the replica may still show the old row as live. - const winner = await this.#selectAppRow(userId, appUid, { + const winner = await this.#selectUnlessEnded(selectRow, { primary: stale !== null, }); const row = winner ?? created; - await this.#writeCacheKey(cacheKey, row); - this.#writeCache(row).catch(() => {}); + await this.#cacheComposite(cacheKey, row); return row; } @@ -779,12 +845,12 @@ export class SessionStore extends PuterStore { // re-SELECT under the partial unique index to find whichever // row actually won. // After a revoke the replica may still show the old row as live. - const winner = await this.#selectWorkerRow(userId, appUid, workerName, { - primary: existing !== null, - }); + const winner = await this.#selectUnlessEnded( + (o) => this.#selectWorkerRow(userId, appUid, workerName, o), + { primary: existing !== null }, + ); const row = winner ?? created; - await this.#writeCacheKey(cacheKey, row); - this.#writeCache(row).catch(() => {}); + await this.#cacheComposite(cacheKey, row); return row; } @@ -806,15 +872,10 @@ export class SessionStore extends PuterStore { return cached; } - const existing = await this.#selectWorkerRow( - userId, - appUid, - workerName, + const existing = await this.#selectUnlessEnded((o) => + this.#selectWorkerRow(userId, appUid, workerName, o), ); - if (existing) { - await this.#writeCacheKey(cacheKey, existing); - this.#writeCache(existing).catch(() => {}); - } + if (existing) await this.#cacheComposite(cacheKey, existing); return existing; } @@ -1169,6 +1230,32 @@ export class SessionStore extends PuterStore { } } + /** + * Run a row select on the replica, or on the primary when asked or when the + * replica's row ended moments ago (its uuid key is tombstoned) and the + * replica may not have the revoke or delete yet. + */ + async #selectUnlessEnded(select, { primary = false } = {}) { + if (primary) return select({ primary: true }); + const row = await select({ primary: false }); + if (!row) return row; + if (!(await this.isCacheKeyTombstoned([this.#cacheKey(row.uuid)]))) { + return row; + } + return select({ primary: true }); + } + + /** Cache a row under its composite key and its uuid key, unless it ended. */ + async #cacheComposite(compositeKey, row) { + await this.writeCacheUnlessDeleted( + [this.#cacheKey(row.uuid), compositeKey], + async () => { + await this.#writeCacheKey(compositeKey, row); + await this.#writeCache(row); + }, + ); + } + /** * Active app session for (userId, appUid). Matches the partial unique index * `idx_sessions_user_app_active`. Kept private — public callers should go diff --git a/src/backend/stores/session/SessionStore.test.ts b/src/backend/stores/session/SessionStore.test.ts index eb433aca5..80c213f0b 100644 --- a/src/backend/stores/session/SessionStore.test.ts +++ b/src/backend/stores/session/SessionStore.test.ts @@ -63,6 +63,47 @@ describe('SessionStore', () => { return rows[0] ?? null; }; + /** + * Answer replica reads of `sessions` the way a lagging replica would: each + * listed uuid as its given row, or absent when `null`. A given row also + * answers, ahead of any other row, the (user, app) lookups that would match + * it. Returns a restore. + */ + const lagReplica = (stale: Record | null>) => { + const db = server.clients.db; + const original = db.read.bind(db); + const spy = vi + .spyOn(db, 'read') + .mockImplementation(async (sql: string, params?: unknown[]) => { + const rows = await original(sql, params); + if (!sql.includes('FROM `sessions`')) return rows; + const result = rows.filter( + (row: Record) => !(row.uuid in stale), + ); + for (const [uuid, row] of Object.entries(stale)) { + if (!row) continue; + const byComposite = + sql.includes(`\`kind\` = '${row.kind}'`) && + params?.includes(row.user_id) && + params?.includes(row.app_uid); + if (params?.includes(uuid) || byComposite) { + result.unshift({ ...row }); + } + } + return result; + }); + // sqlite's `pread` goes through `read`; keep it on the primary. + const primary = vi + .spyOn(db, 'pread') + .mockImplementation((sql: string, params?: unknown[]) => + original(sql, params), + ); + return () => { + spy.mockRestore(); + primary.mockRestore(); + }; + }; + describe('create', () => { it('defaults to kind="web" and stores request metadata', async () => { const user = await makeUser(); @@ -179,6 +220,24 @@ describe('SessionStore', () => { expect(uuids).not.toContain(revoked.uuid); }); + it('with primary=true finds a session the replica has not seen yet', async () => { + const user = await makeUser(); + const session = await target.create(user.id); + + const restore = lagReplica({ [session.uuid]: null }); + try { + expect(await target.getByUserId(user.id)).toEqual([]); + const rows = await target.getByUserId(user.id, { + primary: true, + }); + expect(rows.map((r: { uuid: string }) => r.uuid)).toEqual([ + session.uuid, + ]); + } finally { + restore(); + } + }); + it('returns revoked sessions when includeRevoked is true', async () => { const user = await makeUser(); const active = await target.create(user.id); @@ -224,6 +283,23 @@ describe('SessionStore', () => { } }); + it('sends a peer region miss to the primary while its replica lags', async () => { + const user = await makeUser(); + const session = await target.create(user.id); + const live = await rawRow(session.uuid); + + await target.removeByUuid(session.uuid); + // A peer gets the tombstone and a dropped key, not the row. + await server.clients.redis.del(`sessions:v2:uuid:${session.uuid}`); + + const restore = lagReplica({ [session.uuid]: live }); + try { + expect(await target.getByUuid(session.uuid)).toBeNull(); + } finally { + restore(); + } + }); + it('is idempotent — second call does not overwrite revoked_at', async () => { const user = await makeUser(); const session = await target.create(user.id); @@ -337,6 +413,133 @@ describe('SessionStore', () => { await expect(target.revokeCascade('')).resolves.toBeNull(); await expect(target.revokeCascade(undefined)).resolves.toBeNull(); }); + + it('keeps the revoked rows revoked when a lagging replica still shows them live', async () => { + const user = await makeUser(); + const parent = await target.create(user.id); + const child = await target.create(user.id, { + kind: 'access_token', + parent_session_id: parent.uuid, + }); + const live = { + [parent.uuid]: await rawRow(parent.uuid), + [child.uuid]: await rawRow(child.uuid), + }; + + await target.revokeCascade(parent.uuid); + + const restore = lagReplica(live); + try { + expect(await target.getByUuid(parent.uuid)).toBeNull(); + expect(await target.getByUuid(child.uuid)).toBeNull(); + // A peer region gets the tombstone and a dropped key, so its + // next miss reads the primary rather than its replica. + await server.clients.redis.del( + `sessions:v2:uuid:${parent.uuid}`, + ); + expect(await target.getByUuid(parent.uuid)).toBeNull(); + } finally { + restore(); + } + // The primary read re-cached the revoked row. + expect( + JSON.parse( + (await server.clients.redis.get( + `sessions:v2:uuid:${parent.uuid}`, + )) as string, + ).revoked_at, + ).not.toBeNull(); + }); + + it('revokes a session the replica has not seen yet', async () => { + const user = await makeUser(); + const parent = await target.create(user.id); + const child = await target.create(user.id, { + kind: 'access_token', + parent_session_id: parent.uuid, + }); + + const restore = lagReplica({ + [parent.uuid]: null, + [child.uuid]: null, + }); + let revoked; + try { + revoked = await target.revokeCascade(parent.uuid); + } finally { + restore(); + } + + expect(new Set(revoked.uuids)).toEqual( + new Set([parent.uuid, child.uuid]), + ); + expect((await rawRow(child.uuid)).revoked_at).not.toBeNull(); + }); + }); + + describe('revokeCascades', () => { + it('revokes every root and its children, reported per user', async () => { + const alice = await makeUser(); + const bob = await makeUser(); + const aliceWeb = await target.create(alice.id); + const aliceAsset = await target.create(alice.id, { + kind: 'asset', + parent_session_id: aliceWeb.uuid, + }); + const aliceApp = await target.getOrCreateApp( + alice.id, + `app-${uuidv4()}`, + ); + const bobWeb = await target.create(bob.id); + const untouched = await target.create(alice.id); + + const revoked = await target.revokeCascades([ + aliceWeb.uuid, + // Already covered by its parent's cascade. + aliceAsset.uuid, + aliceApp.uuid, + bobWeb.uuid, + ]); + + const byUser = new Map( + revoked.map((r: { userId: number; uuids: string[] }) => [ + r.userId, + new Set(r.uuids), + ]), + ); + expect(byUser).toEqual( + new Map([ + [ + alice.id, + new Set([ + aliceWeb.uuid, + aliceAsset.uuid, + aliceApp.uuid, + ]), + ], + [bob.id, new Set([bobWeb.uuid])], + ]), + ); + for (const uuid of [ + aliceWeb.uuid, + aliceAsset.uuid, + aliceApp.uuid, + bobWeb.uuid, + ]) { + expect(await target.getByUuid(uuid)).toBeNull(); + } + expect(await target.getByUuid(untouched.uuid)).toBeTruthy(); + expect( + (await target.getOrCreateApp(alice.id, aliceApp.app_uid)).uuid, + ).not.toBe(aliceApp.uuid); + }); + + it('returns nothing when no root is active', async () => { + expect(await target.revokeCascades([])).toEqual([]); + expect(await target.revokeCascades(['nonexistent-uuid'])).toEqual( + [], + ); + }); }); // ── Composite-key lookups ────────────────────────────────────── @@ -398,6 +601,30 @@ describe('SessionStore', () => { const second = await target.getOrCreateApp(user.id, appUid); expect(second.uuid).not.toBe(first.uuid); }); + + it('mints a new row, and does not re-cache the revoked one, while a lagging replica shows it live', async () => { + const user = await makeUser(); + const appUid = `app-${uuidv4()}`; + const first = await target.getOrCreateApp(user.id, appUid); + const live = await rawRow(first.uuid); + + await target.revokeCascade(first.uuid); + // A peer region gets the tombstone and dropped keys, not the row. + await server.clients.redis.del(`sessions:v2:uuid:${first.uuid}`); + + const restore = lagReplica({ [first.uuid]: live }); + let second; + try { + second = await target.getOrCreateApp(user.id, appUid); + expect(await target.getByUuid(first.uuid)).toBeNull(); + } finally { + restore(); + } + expect(second.uuid).not.toBe(first.uuid); + expect(await target.getByUuid(second.uuid)).toMatchObject({ + uuid: second.uuid, + }); + }); }); describe('getOrCreateGodmodeToken', () => { @@ -911,6 +1138,31 @@ describe('SessionStore', () => { expect(second.uuid).not.toBe(first.uuid); }); + it('keeps a deleted session gone while a lagging replica still has it', async () => { + const user = await makeUser(); + const workerName = name(); + const worker = await target.getOrCreateWorker(user.id, { + appUid: null, + workerName, + }); + const live = await rawRow(worker.uuid); + + await target.deleteWorkerSessions({ workerName }); + + const restore = lagReplica({ [worker.uuid]: live }); + try { + expect(await target.getByUuidAny(worker.uuid)).toBeNull(); + expect( + await target.getWorker(user.id, { + appUid: null, + workerName, + }), + ).toBeNull(); + } finally { + restore(); + } + }); + it('refuses to run without a filter', async () => { const user = await makeUser(); const row = await target.getOrCreateWorker(user.id, { diff --git a/src/backend/stores/share/ShareStore.js b/src/backend/stores/share/ShareStore.js index d030d14c9..d02ffe855 100644 --- a/src/backend/stores/share/ShareStore.js +++ b/src/backend/stores/share/ShareStore.js @@ -19,6 +19,7 @@ import { v4 as uuidv4 } from 'uuid'; import { HttpError } from '../../core/http/HttpError.js'; +import { isSameAddress } from '../../util/email'; import { decodeCursor, keysetPage, openIdCursor } from '../../util/pagination'; import { escapeLike } from '../../util/sqlLike'; import { PuterStore } from '../types'; @@ -490,7 +491,10 @@ export class ShareStore extends PuterStore { 'ORDER BY `id`', [recipientEmail], ); - return rows.map((r) => this.#normalizeRow(r)); + // The collation also matches accent variants, which are other addresses. + return rows + .filter((r) => isSameAddress(r.recipient_email, recipientEmail)) + .map((r) => this.#normalizeRow(r)); } /** diff --git a/src/backend/stores/share/ShareStore.test.js b/src/backend/stores/share/ShareStore.test.js index 23754d91c..448fc1e31 100644 --- a/src/backend/stores/share/ShareStore.test.js +++ b/src/backend/stores/share/ShareStore.test.js @@ -467,6 +467,44 @@ describe('ShareStore', () => { ); }); + it('lists invites for an address, not for its accent variants', async () => { + const local = `inv-${uuidv4().slice(0, 8)}`; + const stored = `${local}@exámple.org`; + const { row: invite } = await store.upsertPending({ + issuerUserId: issuer.id, + recipientEmail: stored, + fsentryId: (await makeEntry(issuer)).id, + mode: 'read', + }); + + // MySQL's collation matches either typed form to the stored one; + // sqlite compares bytes, so fold the bound value the same way. + const db = server.clients.db; + const realRead = db.read.bind(db); + const listFolded = async (typed) => { + const read = vi + .spyOn(db, 'read') + .mockImplementation((q, p = []) => + realRead( + q, + p.map((v) => (v === typed ? stored : v)), + ), + ); + try { + return await store.listPendingByEmail(typed); + } finally { + read.mockRestore(); + } + }; + + expect(await listFolded(`${local}@example.org`)).toEqual([]); + expect( + (await listFolded(`${local.toUpperCase()}@EXÁMPLE.ORG`)).map( + (r) => r.uid, + ), + ).toEqual([invite.uid]); + }); + it('reaches ids past the first chunk', async () => { const entry = await makeEntry(issuer); await store.upsertActive({ diff --git a/src/backend/stores/subdomain/SubdomainStore.test.ts b/src/backend/stores/subdomain/SubdomainStore.test.ts index 99a7b425a..8c1994eb8 100644 --- a/src/backend/stores/subdomain/SubdomainStore.test.ts +++ b/src/backend/stores/subdomain/SubdomainStore.test.ts @@ -166,6 +166,249 @@ describe('SubdomainStore cache coherency', () => { }); }); +describe('SubdomainStore in a peer region behind a lagging replica', () => { + const newName = (tag: string) => + `sds-peer-${tag}-${Math.random().toString(36).slice(2, 8)}`; + + /** + * Run `write`, then replay its broadcasts as a peer region that held + * `cached` under `name` and has none of this node's cache writes. + */ + const applyOnPeer = async ( + name: string, + cached: unknown, + write: () => Promise, + ) => { + const updates: unknown[] = []; + const listener = (_key: string, data: unknown) => { + updates.push(data); + }; + server.clients.event.on('outer.cacheUpdate', listener); + try { + await write(); + } finally { + server.clients.event.off('outer.cacheUpdate', listener); + } + + const redis = server.clients.redis; + await redis.del(`${cacheKey(name)}:deleted`); + await redis.set(cacheKey(name), JSON.stringify(cached), 'EX', 3600); + for (const update of updates) { + await server.clients.event.emitAndWait( + 'outer.cacheUpdate', + update as never, + { from_outside: true } as never, + ); + } + await vi.waitFor(async () => + expect(await redis.get(cacheKey(name))).toBeNull(), + ); + }; + + /** Replica reads of `subdomains` return `staleRows`; the primary is real. */ + const withLaggingReplica = async ( + staleRows: unknown[], + fn: () => Promise, + ): Promise => { + const db = server.clients.db; + const realRead = db.read.bind(db); + const pread = vi.spyOn(db, 'pread').mockImplementation(realRead); + const read = vi + .spyOn(db, 'read') + .mockImplementation(async (q, p) => + /FROM `subdomains`/u.test(q) ? staleRows : realRead(q, p), + ); + try { + return await fn(); + } finally { + read.mockRestore(); + pread.mockRestore(); + } + }; + + /** The value the read path settles on in the cache for `name`. */ + const settledCache = async (name: string) => { + let raw: string | null = null; + await vi.waitFor(async () => { + raw = await server.clients.redis.get(cacheKey(name)); + expect(raw).not.toBeNull(); + }); + return raw; + }; + + it('does not cache a deleted site back from the replica', async () => { + const userId = await makeUser(); + const name = newName('delete'); + const row = await store.create({ + userId, + subdomain: name, + rootDirId: 1, + }); + const before = await store.getBySubdomain(name, { primary: true }); + + await applyOnPeer(name, before, () => + store.deleteByUuid(row.uuid, { userId }), + ); + + const found = await withLaggingReplica([before], () => + store.getBySubdomain(name), + ); + expect(found).toBeNull(); + expect(await settledCache(name)).toBe('__none__'); + }); + + it('serves the new root after an update, not the replica copy', async () => { + const userId = await makeUser(); + const name = newName('update'); + const row = await store.create({ + userId, + subdomain: name, + rootDirId: 1, + }); + const before = await store.getBySubdomain(name, { primary: true }); + + await applyOnPeer(name, before, () => + store.update(row.uuid, { root_dir_id: 2 }, { userId }), + ); + + const found = await withLaggingReplica([before], () => + store.getBySubdomain(name), + ); + expect(found?.root_dir_id).toBe(2); + expect(JSON.parse((await settledCache(name))!).root_dir_id).toBe(2); + }); + + it('serves the new holder after a reclaim, not the replica copy', async () => { + const firstUser = await makeUser(); + const nextUser = await makeUser(); + const name = newName('reclaim'); + const row = await store.create({ + userId: firstUser, + subdomain: name, + rootDirId: 1, + }); + const before = await store.getBySubdomain(name, { primary: true }); + + await applyOnPeer(name, before, () => + store.reclaimByUuid(row.uuid, { userId: nextUser, rootDirId: 2 }), + ); + + const found = await withLaggingReplica([before], () => + store.getBySubdomain(name), + ); + expect(found?.user_id).toBe(nextUser); + expect(found?.root_dir_id).toBe(2); + }); + + it('resolves a site re-created right after its delete', async () => { + const userId = await makeUser(); + const name = newName('recreate'); + const row = await store.create({ + userId, + subdomain: name, + rootDirId: 1, + }); + const before = await store.getBySubdomain(name, { primary: true }); + + await applyOnPeer(name, before, async () => { + await store.deleteByUuid(row.uuid, { userId }); + await store.create({ userId, subdomain: name, rootDirId: 2 }); + }); + + // The replica has applied neither the delete nor the re-create. + const found = await withLaggingReplica([before], () => + store.getBySubdomain(name), + ); + expect(found?.uuid).not.toBe(row.uuid); + expect(found?.root_dir_id).toBe(2); + }); + + it('sends a recently changed name in a batch to the primary', async () => { + const userId = await makeUser(); + const name = newName('batch'); + const other = newName('batch-other'); + const row = await store.create({ + userId, + subdomain: name, + rootDirId: 1, + }); + await store.create({ userId, subdomain: other, rootDirId: 5 }); + await server.clients.redis.del(cacheKey(other)); + const before = await store.getBySubdomain(name, { primary: true }); + + await applyOnPeer(name, before, () => + store.update(row.uuid, { root_dir_id: 2 }, { userId }), + ); + + const otherRow = await store.getBySubdomain(other, { primary: true }); + await server.clients.redis.del(cacheKey(other)); + const found = await withLaggingReplica([before, otherRow], () => + store.getBySubdomains([name, other]), + ); + expect(found.get(name)?.root_dir_id).toBe(2); + expect(found.get(other)?.root_dir_id).toBe(5); + }); + + it('caches a row read inside the change window only for the window', async () => { + const userId = await makeUser(); + const name = newName('window'); + const row = await store.create({ + userId, + subdomain: name, + rootDirId: 1, + }); + const before = await store.getBySubdomain(name, { primary: true }); + + await applyOnPeer(name, before, () => + store.update(row.uuid, { root_dir_id: 2 }, { userId }), + ); + + const redis = server.clients.redis; + const expectWindowTtl = async () => { + await settledCache(name); + const ttl = await redis.ttl(cacheKey(name)); + expect(ttl).toBeGreaterThan(0); + expect(ttl).toBeLessThanOrEqual(60); + }; + await withLaggingReplica([before], () => store.getBySubdomain(name)); + await expectWindowTtl(); + + await redis.del(cacheKey(name)); + await withLaggingReplica([before], () => store.getBySubdomains([name])); + await expectWindowTtl(); + }); + + it('drops a site deleted before the replica has seen it', async () => { + const userId = await makeUser(); + const name = newName('unseen'); + const row = await store.create({ + userId, + subdomain: name, + rootDirId: 1, + }); + + await withLaggingReplica([], () => + store.deleteByUuid(row.uuid, { userId }), + ); + + expect(await store.getBySubdomain(name)).toBeNull(); + }); + + it('serves a cache hit without checking for a recent change', async () => { + const userId = await makeUser(); + const name = newName('hit'); + await store.create({ userId, subdomain: name, rootDirId: 1 }); + + const pipeline = vi.spyOn(server.clients.redis, 'pipeline'); + try { + expect((await store.getBySubdomain(name))?.root_dir_id).toBe(1); + expect(pipeline).not.toHaveBeenCalled(); + } finally { + pipeline.mockRestore(); + } + }); +}); + describe('SubdomainStore app_owner filtering', () => { const makeApp = async (ownerUserId: number) => { const name = `sds-app-${Math.random().toString(36).slice(2, 10)}`; @@ -256,6 +499,76 @@ describe('SubdomainStore app_owner filtering', () => { ).toBe(0); }); + it('lists the rows an app delete cascades to, by app or by app owner', async () => { + const developer = await makeUser(); + const siteOwner = await makeUser(); + const app = await makeApp(developer); + const sibling = await makeApp(developer); + const unrelated = await makeApp(siteOwner); + const name = (tag: string) => + `sds-cascade-${tag}-${Math.random().toString(36).slice(2, 8)}`; + + const owned = await store.create({ + userId: siteOwner, + subdomain: name('owned'), + appOwner: app.id, + }); + const associated = await store.create({ + userId: siteOwner, + subdomain: name('associated'), + rootDirId: 7, + associatedAppId: app.id, + }); + const siblings = await store.create({ + userId: siteOwner, + subdomain: name('sibling'), + appOwner: sibling.id, + }); + await store.create({ + userId: siteOwner, + subdomain: name('unrelated'), + appOwner: unrelated.id, + }); + await store.create({ userId: siteOwner, subdomain: name('none') }); + + const names = (rows: Array<{ subdomain: string }>) => + rows.map((row) => row.subdomain).sort(); + expect(await store.listAppCascade({ appId: app.id })).toEqual( + expect.arrayContaining([ + { subdomain: owned.subdomain, root_dir_id: null }, + { subdomain: associated.subdomain, root_dir_id: 7 }, + ]), + ); + expect(names(await store.listAppCascade({ appId: app.id }))).toEqual( + names([owned, associated]), + ); + expect( + names(await store.listAppCascade({ ownerUserId: developer })), + ).toEqual(names([owned, associated, siblings])); + }); + + it('lists an app cascade past one page', async () => { + const userId = await makeUser(); + const app = await makeApp(userId); + const tag = Math.random().toString(36).slice(2, 8); + const total = 201; + const values = Array.from({ length: total }, (_, i) => [ + uuidv4(), + `sds-page-${tag}-${i}`, + userId, + app.id, + ]); + await server.clients.db.write( + `INSERT INTO \`subdomains\` (\`uuid\`, \`subdomain\`, \`user_id\`, \`app_owner\`) VALUES ${values + .map(() => '(?, ?, ?, ?)') + .join(', ')}`, + values.flat(), + ); + + const rows = await store.listAppCascade({ appId: app.id }); + expect(new Set(rows.map((row) => row.subdomain)).size).toBe(total); + }); + it('counts only rows older than `createdBefore`', async () => { const userId = await makeUser(); const prefix = `sds-aged-${Math.random().toString(36).slice(2, 6)}.`; @@ -450,13 +763,11 @@ describe('SubdomainStore listing and counting', () => { ).toEqual([`${p}a_b`]); expect(await store.countByUserIdAndPrefix(userId, `${p}a_`)).toBe(1); expect( - ( - await store.listByUserId(userId, { excludePrefix: `${p}a_` }) - ).map((r) => r.subdomain), + (await store.listByUserId(userId, { excludePrefix: `${p}a_` })).map( + (r) => r.subdomain, + ), ).toEqual([`${p}axb`]); - expect( - await store.count({ userId, excludePrefix: `${p}a_` }), - ).toBe(1); + expect(await store.count({ userId, excludePrefix: `${p}a_` })).toBe(1); }); it('narrows a prefix listing by a single app id and paginates it', async () => { @@ -658,6 +969,11 @@ describe('SubdomainStore.getBySubdomains', () => { const name = `gbs-neg-${Math.random().toString(36).slice(2, 8)}`; // Poison the negative-cache marker first. expect(await store.getBySubdomains([name])).toEqual(new Map()); + await vi.waitFor(async () => + expect(await server.clients.redis.get(cacheKey(name))).toBe( + '__none__', + ), + ); const read = vi.spyOn(server.clients.db, 'read'); try { diff --git a/src/backend/stores/subdomain/SubdomainStore.ts b/src/backend/stores/subdomain/SubdomainStore.ts index ae02828d7..395136179 100644 --- a/src/backend/stores/subdomain/SubdomainStore.ts +++ b/src/backend/stores/subdomain/SubdomainStore.ts @@ -19,7 +19,8 @@ import { v4 as uuidv4 } from 'uuid'; import { escapeLike } from '../../util/sqlLike'; -import { PuterStore } from '../types'; +import { runWithConcurrencyLimit } from '../../util/concurrency'; +import { CACHE_TOMBSTONE_TTL_SECONDS, PuterStore } from '../types'; /** * A row from the `subdomains` table (the shape `getBySubdomain` / `getByUuid` @@ -169,27 +170,28 @@ export class SubdomainStore extends PuterStore { } } + // Changed moments ago: a replica may still have the old row. + const tombstoned = + !primary && (await this.isCacheKeyTombstoned([cacheKey])); + const fromPrimary = primary || tombstoned; const sql = 'SELECT * FROM `subdomains` WHERE `subdomain` = ? LIMIT 1'; - const rows = primary + const rows = fromPrimary ? await this.clients.db.pread(sql, [subdomain]) : await this.clients.db.read(sql, [subdomain]); const row = (rows[0] as unknown as SubdomainRow | undefined) ?? null; - if (row) { - void this.#populateCache( + const populate = () => + this.#populateCache( cacheKey, - JSON.stringify(row), - CACHE_TTL_SECONDS, - primary, + row ? JSON.stringify(row) : NEGATIVE_CACHE_MARKER, + row + ? this.#rowTtlSeconds(tombstoned) + : NEGATIVE_CACHE_TTL_SECONDS, + fromPrimary, ); - } else { - void this.#populateCache( - cacheKey, - NEGATIVE_CACHE_MARKER, - NEGATIVE_CACHE_TTL_SECONDS, - primary, - ); - } + void (fromPrimary + ? populate() + : this.writeCacheUnlessDeleted([cacheKey], populate)); return row; } @@ -247,16 +249,56 @@ export class SubdomainStore extends PuterStore { } if (missingNames.length === 0) return result; + // Names changed moments ago go to the primary; see `getBySubdomain`. + const tombstoned = primary + ? new Set() + : await this.tombstonedCacheKeys( + missingNames.map((name) => this.#cacheKey(name)), + ); + const isChanged = (name: string) => + primary || tombstoned.has(this.#cacheKey(name)); + const primaryNames = missingNames.filter(isChanged); + const replicaNames = missingNames.filter((name) => !isChanged(name)); + for (const [names, fromPrimary] of [ + [primaryNames, true], + [replicaNames, false], + ] as const) { + if (names.length === 0) continue; + const rowsByName = await this.#readByNames(names, fromPrimary); + for (const name of names) { + const row = rowsByName.get(name.toLowerCase()); + if (row) result.set(name, row); + } + const populate = () => + this.#populateCacheForNames( + names, + result, + fromPrimary, + this.#rowTtlSeconds(fromPrimary && !primary), + ); + void (fromPrimary + ? populate() + : this.writeCacheUnlessDeleted( + names.map((name) => this.#cacheKey(name)), + populate, + )); + } + + return result; + } + + /** Rows for `names`, keyed by lowercased name. */ + async #readByNames( + names: string[], + primary: boolean, + ): Promise> { const rowsByName = new Map(); for ( let offset = 0; - offset < missingNames.length; + offset < names.length; offset += BULK_QUERY_CHUNK_SIZE ) { - const chunk = missingNames.slice( - offset, - offset + BULK_QUERY_CHUNK_SIZE, - ); + const chunk = names.slice(offset, offset + BULK_QUERY_CHUNK_SIZE); const placeholders = chunk.map(() => '?').join(', '); const sql = `SELECT * FROM \`subdomains\` WHERE \`subdomain\` IN (${placeholders})`; const rows = (primary @@ -273,15 +315,7 @@ export class SubdomainStore extends PuterStore { if (!rowsByName.has(key)) rowsByName.set(key, row); } } - - for (const name of missingNames) { - const row = rowsByName.get(name.toLowerCase()); - if (row) result.set(name, row); - } - - void this.#populateCacheForNames(missingNames, result, primary); - - return result; + return rowsByName; } async listByUserId( @@ -602,15 +636,9 @@ export class SubdomainStore extends PuterStore { // Not `{ ...before, ...allowed }`: `before` is the replica's, and a // column it has stale would go into the cache. const after = await this.getByUuid(uuid, { userId, primary: true }); - if (before?.subdomain && before.subdomain !== after?.subdomain) { - await this.publishCacheKeys({ - keys: [this.#cacheKey(before.subdomain)], - broadcast: true, - }); - } - if (after) { - await this.#refreshCache({ ...after, ...allowed }); - } + await this.#publishChanged(after ? [{ ...after, ...allowed }] : [], [ + before?.subdomain, + ]); // FSEntry rows embed a `subdomains_agg` JSON of associated subdomains, // so any rename / root_dir reassignment must drop the stale entry // caches on both the old and new root_dir_id. @@ -631,7 +659,9 @@ export class SubdomainStore extends PuterStore { userId?: number | undefined; } = {}, ) { - const row = await this.getByUuid(uuid, { userId }); + // Primary: a row created within replica lag would otherwise be missed + // and stay cached. + const row = await this.getByUuid(uuid, { userId, primary: true }); const where = userId !== undefined @@ -644,11 +674,12 @@ export class SubdomainStore extends PuterStore { params, ); if (row?.subdomain) { + const key = this.#cacheKey(row.subdomain); + await this.tombstoneCacheKeys([key]); await this.publishCacheKeys({ - keys: [this.#cacheKey(row.subdomain)], + keys: [key], serializedData: NEGATIVE_CACHE_MARKER, ttlSeconds: NEGATIVE_CACHE_TTL_SECONDS, - broadcast: true, }); await this.#invalidateRootDirEntry(row.root_dir_id); } @@ -685,7 +716,7 @@ export class SubdomainStore extends PuterStore { ); const after = await this.getByUuid(uuid, { primary: true }); - if (after) await this.#refreshCache(after); + await this.#publishChanged(after ? [after] : [], [before.subdomain]); const affectedRootDirIds = new Set( [before.root_dir_id, after?.root_dir_id].filter((v) => v != null), @@ -715,19 +746,80 @@ export class SubdomainStore extends PuterStore { [toAppId, fromAppId], ); - for (const row of rows) { - row.app_owner = toAppId; - await this.#refreshCache(row); - } + for (const row of rows) row.app_owner = toAppId; + await this.#publishChanged(rows); return rows; } + /** + * Rows the database deletes along with an app (`app_owner` and + * `associated_app_id` cascade), or with every app `ownerUserId` owns. Read + * from the primary before the delete; pass them to + * {@link forgetCascadeDeleted} after it. + */ + async listAppCascade( + apps: { appId: number } | { ownerUserId: number }, + ): Promise>> { + const [match, param] = + 'appId' in apps + ? ['= ?', apps.appId] + : [ + 'IN (SELECT `id` FROM `apps` WHERE `owner_user_id` = ?)', + apps.ownerUserId, + ]; + const out: Array> = []; + let afterId = 0; + for (;;) { + const rows = (await this.clients.db.pread( + `SELECT \`id\`, \`subdomain\`, \`root_dir_id\` FROM \`subdomains\` + WHERE (\`app_owner\` ${match} OR \`associated_app_id\` ${match}) + AND \`id\` > ? ORDER BY \`id\` LIMIT ?`, + [param, param, afterId, BULK_QUERY_CHUNK_SIZE], + )) as Array>; + for (const { subdomain, root_dir_id } of rows) { + out.push({ subdomain, root_dir_id }); + } + if (rows.length < BULK_QUERY_CHUNK_SIZE) return out; + afterId = Number(rows[rows.length - 1].id); + } + } + + /** + * Drop cached state, here and on peers, for rows deleted outside this + * store. + */ + async forgetCascadeDeleted( + rows: Array>, + ): Promise { + if (rows.length === 0) return; + await this.#publishChanged( + [], + rows.map((row) => row.subdomain), + ); + const rootDirIds = [ + ...new Set( + rows.map((row) => row.root_dir_id).filter((id) => id != null), + ), + ]; + await runWithConcurrencyLimit(rootDirIds, 8, (id) => + this.#invalidateRootDirEntry(id), + ); + } + // -- Internals ---------------------------------------------------- #cacheKey(subdomain: string) { return `${CACHE_KEY_PREFIX}:name:${subdomain}`; } + /** + * A row read inside a tombstone window lives only as long as the window: a + * second change can commit between that read and its cache write. + */ + #rowTtlSeconds(tombstoned: boolean) { + return tombstoned ? CACHE_TOMBSTONE_TTL_SECONDS : CACHE_TTL_SECONDS; + } + /** * Write-through on a primary read (heals a stale negative marker); `NX` on * a replica read so an in-flight fresher write can't be clobbered by a @@ -761,6 +853,7 @@ export class SubdomainStore extends PuterStore { names: string[], rowsByName: Map, primary: boolean, + rowTtlSeconds: number, ): Promise { try { const pipeline = this.clients.redis.pipeline(); @@ -768,9 +861,7 @@ export class SubdomainStore extends PuterStore { const row = rowsByName.get(name); const cacheKey = this.#cacheKey(name); const value = row ? JSON.stringify(row) : NEGATIVE_CACHE_MARKER; - const ttl = row - ? CACHE_TTL_SECONDS - : NEGATIVE_CACHE_TTL_SECONDS; + const ttl = row ? rowTtlSeconds : NEGATIVE_CACHE_TTL_SECONDS; if (primary) { pipeline.set(cacheKey, value, 'EX', ttl); } else { @@ -783,6 +874,40 @@ export class SubdomainStore extends PuterStore { } } + /** + * Cache rows just changed on the primary, here only. Their names (and + * `staleNames`) are tombstoned here and on peers, whose misses then read + * the primary until replicas catch up. + */ + async #publishChanged( + rows: SubdomainRow[], + staleNames: Array = [], + ) { + const names = new Set( + [...staleNames, ...rows.map((row) => row.subdomain)].filter( + (name): name is string => !!name, + ), + ); + await this.tombstoneCacheKeys( + [...names].map((name) => this.#cacheKey(name)), + ); + if (rows.length === 0) return; + try { + const pipeline = this.clients.redis.pipeline(); + for (const row of rows) { + pipeline.set( + this.#cacheKey(row.subdomain), + JSON.stringify(row), + 'EX', + CACHE_TTL_SECONDS, + ); + } + await pipeline.exec(); + } catch { + // The next read fills it from the primary. + } + } + async #refreshCache(row: { subdomain?: string }) { if (!row?.subdomain) return; await this.publishCacheKeys({ diff --git a/src/backend/stores/user/UserStore.test.ts b/src/backend/stores/user/UserStore.test.ts index 8075062ee..46ec48e40 100644 --- a/src/backend/stores/user/UserStore.test.ts +++ b/src/backend/stores/user/UserStore.test.ts @@ -640,6 +640,115 @@ describe('UserStore batched and uncached lookups', () => { }); }); +describe('UserStore address lookups under a folding collation', () => { + let server: PuterServer; + + beforeAll(async () => { + server = await setupTestServer(); + }); + + afterAll(async () => { + await server?.shutdown(); + }); + + /** + * Have the database match `typed` to rows holding `stored`, the way MySQL's + * accent- and case-insensitive collations do. sqlite compares bytes. + */ + const foldOnto = (typed: string, stored: string) => { + const db = server.clients.db; + const fold = (params: unknown[] = []) => + params.map((p) => (p === typed ? stored : p)); + const realRead = db.read.bind(db); + const realWrite = db.write.bind(db); + const read = vi + .spyOn(db, 'read') + .mockImplementation((q, p) => realRead(q, fold(p))); + const write = vi + .spyOn(db, 'write') + .mockImplementation((q, p) => realWrite(q, fold(p))); + return () => { + read.mockRestore(); + write.mockRestore(); + }; + }; + + const makeOwner = async (email: string) => + server.stores.user.create({ + username: `ua-${Math.random().toString(36).slice(2, 10)}`, + uuid: uuidv4(), + password: 'hashed', + email, + clean_email: cleanEmail(email), + email_confirmed: true, + }); + + it('does not resolve an address to an accent variant of it', async () => { + const local = `x${Math.random().toString(36).slice(2, 10)}`; + await makeOwner(`${local}@exámple.org`); + const typed = `${local}@example.org`; + + const restore = foldOnto(typed, `${local}@exámple.org`); + try { + expect( + await server.stores.user.getByEmail(typed, { cached: false }), + ).toBeNull(); + expect(await server.stores.user.findEmailOwner(typed)).toBeNull(); + expect(await server.stores.user.getByCleanEmail(typed)).toBeNull(); + } finally { + restore(); + } + }); + + it('still resolves an address typed in another case', async () => { + const local = `x${Math.random().toString(36).slice(2, 10)}`; + const owner = await makeOwner(`${local}@exámple.org`); + const typed = `${local.toUpperCase()}@EXÁMPLE.ORG`; + + const restore = foldOnto(typed, `${local}@exámple.org`); + try { + expect( + (await server.stores.user.getByEmail(typed, { cached: false })) + ?.id, + ).toBe(owner.id); + expect((await server.stores.user.findEmailOwner(typed))?.id).toBe( + owner.id, + ); + } finally { + restore(); + } + }); + + it('leaves an accent variant alone when an address is taken', async () => { + const local = `x${Math.random().toString(36).slice(2, 10)}`; + const twin = await makeOwner(`${local}@exámple.org`); + const taker = await server.stores.user.create({ + username: `ut-${Math.random().toString(36).slice(2, 10)}`, + uuid: uuidv4(), + password: null, + email: null, + }); + const typed = `${local}@example.org`; + + const restore = foldOnto(typed, `${local}@exámple.org`); + try { + await server.stores.user.unconfirmOthersByEmail( + taker.id, + typed, + typed, + ); + } finally { + restore(); + } + + const after = await server.stores.user.getById(twin.id, { + force: true, + }); + expect(after?.email).toBe(`${local}@exámple.org`); + expect(after?.email_confirmed).toBe(true); + }); +}); + describe('UserStore deleted-row tombstones', () => { let server: PuterServer; diff --git a/src/backend/stores/user/UserStore.ts b/src/backend/stores/user/UserStore.ts index 16d5227d4..f4fe1dc7a 100644 --- a/src/backend/stores/user/UserStore.ts +++ b/src/backend/stores/user/UserStore.ts @@ -18,7 +18,7 @@ */ import { isUniqueViolation } from '../../util/dbError.js'; -import { cleanEmail } from '../../util/email.js'; +import { cleanEmail, isSameAddress } from '../../util/email.js'; import { normalizeReferralCode } from '../../util/referralCode.js'; import { PuterStore } from '../types'; @@ -341,14 +341,16 @@ export class UserStore extends PuterStore { ): Promise { if (!cleanEmailValue) return null; if (!isStorableAsLatin1(cleanEmailValue)) return null; - const sql = `SELECT \`id\` FROM \`user\` WHERE \`clean_email\` = ? ${EMAIL_OWNER_ORDER} LIMIT 1`; + const sql = `SELECT \`id\`, \`clean_email\` FROM \`user\` WHERE \`clean_email\` = ? ${EMAIL_OWNER_ORDER} LIMIT 1`; const rows = (await (opts.force ? this.clients.db.pread(sql, [cleanEmailValue]) : this.clients.db.tryHardRead(sql, [cleanEmailValue]))) as Array<{ id: number; + clean_email: string | null; }>; const row = rows[0]; - if (!row) return null; + if (!row || !isSameAddress(row.clean_email, cleanEmailValue)) + return null; return this.getById(row.id as number, opts); } @@ -446,7 +448,14 @@ export class UserStore extends PuterStore { const rows = primary ? await this.clients.db.pread(sql, [value]) : await this.clients.db.tryHardRead(sql, [value]); - return rows[0] ? this.#normalizeRow(rows[0]) : null; + const row = rows[0]; + if (!row) return null; + if ( + prop === 'email' && + !isSameAddress(row.email as string, String(value)) + ) + return null; + return this.#normalizeRow(row); }, { skipRead: !cached, primary: force }, ); @@ -754,11 +763,12 @@ export class UserStore extends PuterStore { /** * The account other than `userId` that has already confirmed this address, - * if there is one. Matches raw + canonical, exactly like - * `unconfirmOthersByEmail`, and is meant to run immediately before it: a - * confirmed row proved access to the inbox, so it is refused rather than - * demoted. Everything that lookup leaves behind is an unconfirmed row, - * which is what `unconfirmOthersByEmail` is for. + * if there is one. Matches raw + canonical like `unconfirmOthersByEmail` + * (plus the accent variants it skips, which only refuses more), and is + * meant to run immediately before it: a confirmed row proved access to the + * inbox, so it is refused rather than demoted. Everything that lookup + * leaves behind is an unconfirmed row, which is what + * `unconfirmOthersByEmail` is for. * * Reads the primary — the confirmation it guards is about to write. */ @@ -787,10 +797,20 @@ export class UserStore extends PuterStore { // straight to SQL, so without their pre-image we don't know which // cache keys it retires — and a cached copy still carries the address // that was just revoked, which would keep answering lookups for it. - const stripped = (await this.clients.db.pread( + const matched = (await this.clients.db.pread( 'SELECT * FROM `user` WHERE `id` != ? AND (`email` = ? OR `clean_email` = ?)', [userId, email, cleanEmailValue], )) as Array>; + // The collation also matches accent variants, which are other addresses. + const stripped = matched.filter( + (row) => + isSameAddress(row.email as string | null, email) || + isSameAddress( + row.clean_email as string | null, + cleanEmailValue, + ), + ); + if (stripped.length === 0) return; await this.clients.db.write( `UPDATE \`user\` @@ -800,12 +820,12 @@ export class UserStore extends PuterStore { \`requires_email_confirmation\` = ?, \`email_confirm_code\` = NULL, \`email_confirm_token\` = NULL - WHERE \`id\` != ? + WHERE \`id\` IN (${stripped.map(() => '?').join(', ')}) AND (\`email\` = ? OR \`clean_email\` = ?)`, [ this.clients.db.booleanValue(false), this.clients.db.booleanValue(false), - userId, + ...stripped.map((row) => row.id), email, cleanEmailValue, ], diff --git a/src/backend/util/email.test.ts b/src/backend/util/email.test.ts index 23bc55474..62ff72d67 100644 --- a/src/backend/util/email.test.ts +++ b/src/backend/util/email.test.ts @@ -19,7 +19,12 @@ import { describe, expect, it } from 'vitest'; import validator from 'validator'; -import { cleanEmail, isBlockedEmail, isStorableEmail } from './email.ts'; +import { + cleanEmail, + isBlockedEmail, + isSameAddress, + isStorableEmail, +} from './email.ts'; describe('cleanEmail', () => { it('lowercases the whole address', () => { @@ -94,6 +99,22 @@ describe('cleanEmail', () => { }); }); +describe('isSameAddress', () => { + it('ignores case and surrounding space', () => { + expect(isSameAddress('Foo@Example.com ', 'foo@example.COM')).toBe(true); + }); + + it('tells accent variants apart', () => { + expect(isSameAddress('foo@gmáil.com', 'foo@gmail.com')).toBe(false); + expect(isSameAddress('fóo@gmail.com', 'foo@gmail.com')).toBe(false); + }); + + it('never matches a missing address', () => { + expect(isSameAddress(null, 'foo@example.com')).toBe(false); + expect(isSameAddress(undefined, '')).toBe(false); + }); +}); + describe('isBlockedEmail', () => { it('is false when no block list is configured', () => { expect(isBlockedEmail('a@mailinator.com', undefined)).toBe(false); diff --git a/src/backend/util/email.ts b/src/backend/util/email.ts index a67b19ea8..613f4374b 100644 --- a/src/backend/util/email.ts +++ b/src/backend/util/email.ts @@ -147,6 +147,20 @@ export function cleanEmail(email: string): string { return `${parts.local}@${parts.domain}`; } +/** + * Whether a stored address is the one looked up, ignoring case. Re-check SQL + * email matches with this: the MySQL collations also fold accents (`á` = `a`). + */ +export function isSameAddress( + stored: string | null | undefined, + lookedUp: string, +): boolean { + return ( + typeof stored === 'string' && + stored.trim().toLowerCase() === lookedUp.trim().toLowerCase() + ); +} + /** Storage gate; `isEmail` alone throws on non-strings and passes quoted parts. */ export function isStorableEmail(email: unknown): boolean { return ( diff --git a/src/docs/src/Email/sendTransactional.md b/src/docs/src/Email/sendTransactional.md index 8beb6a0f6..7c347b64f 100644 --- a/src/docs/src/Email/sendTransactional.md +++ b/src/docs/src/Email/sendTransactional.md @@ -37,7 +37,7 @@ A plain-text body. Shorthand for `text`; use the options form for anything else. - `html` (String) - HTML body. An inline image is shown with `` — see `cid` under `attachments`. - `cc` (String | Array) - Carbon-copy recipient(s). - `bcc` (String | Array) - Blind-copy recipient(s). At most **10** recipients across `to`, `cc` and `bcc`. -- `replyTo` (String) - Where replies go. Defaults to the confirmed account email of the app's owner (the worker's owner when the worker runs under no app). While that address is unconfirmed, the message goes out without a Reply-To and replies bounce. +- `replyTo` (String) - Where replies go. Defaults to the confirmed account email of the worker's owner. While that address is unconfirmed, the message goes out without a Reply-To and replies bounce. - `emailAccessToken` (String) - A worker's auth token — inside a worker, `me.puter.authToken`. Required when the caller is not itself a worker. The caller stays the billed and rate-limited identity. - `attachments` (Array) - Up to **10** attachments, **25 MB** in total. Each is an object: - `filename` (String) - Required with `content`; defaults to the file's name for a `path`/`uid` attachment. @@ -76,7 +76,9 @@ A recipient at a Puter address, `@puter.email`, is a Puter user, and t Mail goes out from a Puter-controlled address minted for your app — `-no-reply@apps.puter.email`, labelled with your app's title — so you never configure a sending domain, and a recipient's filters can tell your app's mail from every other app's. You cannot set `from`: the address is what proves the mail came through Puter, and the app is where the name and identity come from. -Recipients who hit reply reach `replyTo`. Left unset, that is the confirmed account email of the app's owner. Set it explicitly to route replies to a shared inbox instead. +The app is the one the worker runs under, or else the one the caller is signed in through. It's used only when the worker's owner also owns that app. Otherwise the mail goes out from `no-reply@apps.puter.email` with no app name. + +Recipients who hit reply reach `replyTo`. Left unset, that is the confirmed account email of the worker's owner. Set it explicitly to route replies to a shared inbox instead. ## Recipient protections diff --git a/src/docs/src/KV/remove.md b/src/docs/src/KV/remove.md index d6336da15..d1fddc8dc 100644 --- a/src/docs/src/KV/remove.md +++ b/src/docs/src/KV/remove.md @@ -26,7 +26,7 @@ Paths support dot notation, array indexes at any level (`[0]`, `items[0]`, or `s ## Return value -Returns a `Promise` that resolves to the updated value stored at `key`. An expired key resolves to `null`. +Returns a `Promise` that resolves to the updated value stored at `key`. An expired key resolves to `null`. A call on another app's data with `optConfig.appUuid` also resolves to `null` when your app may delete that app's entries but not read them. ## Errors diff --git a/src/gui/src/IPC.js b/src/gui/src/IPC.js index 703d39284..5f86399f5 100644 --- a/src/gui/src/IPC.js +++ b/src/gui/src/IPC.js @@ -1585,6 +1585,15 @@ const ipc_listener = async (event, handled) => { // showSaveFilePicker //-------------------------------------------------------- else if ( event.data.msg === 'showSaveFilePicker' ) { + // Only copy and move have a matching consent prompt; refuse anything else. + if ( event.data.source_path && !['copy', 'move'].includes(event.data.save_type) ) { + target_iframe.contentWindow.postMessage({ + msg: 'fileSaveCancelled', + original_msg_id: msg_id, + }, '*'); + return; + } + //auth if ( !window.is_auth() && !(await UIWindowSignup({ referrer: app_name })) ) { @@ -1785,14 +1794,14 @@ const ipc_listener = async (event, handled) => { await puter.fs.delete(target_path); } - if ( save_type === 'copy' ) { + if ( save_type === 'move' ) { + await puter.fs.move(source_path, target_path); + } else { const target_dir = path.dirname(target_path); const new_name = path.basename(target_path); await puter.fs.copy(source_path, target_dir, { newName: new_name, }); - } else { - await puter.fs.move(source_path, target_path); } node = await puter.fs.stat(target_path); }, diff --git a/src/gui/src/UI/UIDesktop.js b/src/gui/src/UI/UIDesktop.js index a5e7bd1f8..50fba4b45 100644 --- a/src/gui/src/UI/UIDesktop.js +++ b/src/gui/src/UI/UIDesktop.js @@ -49,6 +49,7 @@ import { notificationTarget } from './Dashboard/notificationCenter.js'; import apply_item_added_to_containers from '../helpers/applyItemAddedToContainers.js'; import parse_item_metadata from '../helpers/parseItemMetadata.js'; import UIWindowSearch from './UIWindowSearch.js'; +import { resolve_default_app } from '../helpers/defaultApp.js'; async function UIDesktop (options) { // start a transaction if we're not in embedded or fullpage mode @@ -1785,12 +1786,11 @@ async function UIDesktop (options) { return; } - const ext_pref = - window.user_preferences[`default_apps${path.extname(item_path).toLowerCase()}`]; - - if ( ext_pref ) { + const default_app = await resolve_default_app({ uid: stat.uid, path: item_path }); + if ( default_app ) { launch_app({ - name: ext_pref, + name: default_app.name, + app_obj: default_app, file_path: item_path, }); return; diff --git a/src/gui/src/UI/UIItem.js b/src/gui/src/UI/UIItem.js index bc0b6f9f2..139788e06 100644 --- a/src/gui/src/UI/UIItem.js +++ b/src/gui/src/UI/UIItem.js @@ -34,6 +34,7 @@ import mime from '../lib/mime.js'; import { isWeblinkName, weblinkChangeIconMenuItem } from '../helpers/weblink.js'; import { is_owned_by_me } from '../helpers/pathOwner.js'; import { can_rename, can_restructure, invalidate_shared_roots, shared_mode_for } from '../helpers/sharedAccess.js'; +import { is_default_app } from '../helpers/defaultApp.js'; const AI_APP_NAME = 'ai'; @@ -1228,7 +1229,7 @@ async function UIItem (options) { onClick: async function () { var extension = path.extname($(el_item).attr('data-path')).toLowerCase(); if ( - window.user_preferences[`default_apps${extension}`] !== suggested_app.name + ! is_default_app(window.user_preferences[`default_apps${extension}`], suggested_app) && ( (!window.user_preferences[`default_apps${extension}`] && index > 0) @@ -1251,7 +1252,7 @@ async function UIItem (options) { ], }); if ( (alert_resp) === 'yes' ) { - window.user_preferences[`default_apps${ extension}`] = suggested_app.name; + window.user_preferences[`default_apps${ extension}`] = suggested_app.uuid; window.mutate_user_preferences(window.user_preferences); } } @@ -1335,7 +1336,8 @@ async function UIItem (options) { //------------------------------------------- // Publish as Worker // ------------------------------------------- - if ( !is_trashed && !is_trash && !options.is_dir && $(el_item).attr('data-name').toLowerCase().endsWith('.js') ) { + // The backend refuses deploys from a file someone else owns. + if ( !is_trashed && !is_trash && !is_not_mine && !options.is_dir && $(el_item).attr('data-name').toLowerCase().endsWith('.js') ) { menu_items.push({ html: i18n('publish_as_serverless_worker'), onClick: async function () { diff --git a/src/gui/src/UI/UIPermissionDialog.js b/src/gui/src/UI/UIPermissionDialog.js index 40776875d..f707a3116 100644 --- a/src/gui/src/UI/UIPermissionDialog.js +++ b/src/gui/src/UI/UIPermissionDialog.js @@ -18,6 +18,7 @@ */ import { fsCreateKindFor } from '../helpers/fsCreateKind.js'; +import { get_app_by_uid } from '../helpers/getAppByUid.js'; // Keeps a second request for the same (entity, permission) from stacking a // duplicate dialog; both callers await the same user decision. @@ -29,8 +30,9 @@ const pending_dialogs = new Map(); // including the app that asked. One at a time, in arrival order. let dialog_queue = Promise.resolve(); -// Buttons stay disabled briefly after the dialog opens so a click aimed at -// the page underneath (or a clickjacking attempt) can't land on "Allow". +// Buttons stay disabled until the dialog has been visible and focused this long +// without a break, so a click aimed at whatever covered it can't land on +// "Allow". See `guard_input`. const INPUT_PROTECTION_MS = 350; // How long the grant request may take before it is aborted and the dialog @@ -181,9 +183,11 @@ async function show_permission_dialog (options) { let grant_may_have_committed = false; let settled = false; + let input_guard = null; const settle = (granted) => { if ( settled ) return; settled = true; + input_guard?.stop(); if ( ! granted && grant_may_have_committed ) { // Not awaited: the requester gets its answer now, and the // account is reconciled in the background. @@ -200,22 +204,21 @@ async function show_permission_dialog (options) { const $deny = $(el_dialog).find('.perm-dialog-deny'); const $error = $(el_dialog).find('.perm-dialog-error'); - // Input protection: ignore clicks for a moment after opening. - $allow.prop('disabled', true); - $deny.prop('disabled', true); - setTimeout(() => { - if ( settled ) return; - $allow.prop('disabled', false); - $deny.prop('disabled', false); - $deny.get(0)?.focus(); - }, INPUT_PROTECTION_MS); - // True while the grant POST is outstanding. Dismissing the dialog then // would answer "denied" for a permission the server may be in the // middle of committing, leaving the user believing they cancelled // something that is now granted. let granting = false; + // Buttons take clicks only once the input guard is ready and no grant + // is in flight. + const sync_buttons = () => { + const enabled = !!input_guard?.ready && ! granting; + $allow.prop('disabled', ! enabled); + $deny.prop('disabled', ! enabled); + }; + sync_buttons(); + // Hands the dialog back to the user after a failed attempt: re-enables // the buttons, restores dismissability, and shows a retryable error. const fail_grant = () => { @@ -234,15 +237,15 @@ async function show_permission_dialog (options) { // the default encoding would surface entities like `'` // verbatim in translations that contain an apostrophe. $error.text(i18n('perm_dialog_error', [], false)).show(); - $allow.prop('disabled', false).removeClass('perm-dialog-busy'); - $deny.prop('disabled', false); + $allow.removeClass('perm-dialog-busy'); + sync_buttons(); }; $allow.on('click', async () => { if ( granting ) return; granting = true; - $allow.prop('disabled', true).addClass('perm-dialog-busy'); - $deny.prop('disabled', true); + $allow.addClass('perm-dialog-busy'); + sync_buttons(); $error.hide(); // Time-box the request: while it is outstanding the dialog can't @@ -338,11 +341,85 @@ async function show_permission_dialog (options) { settle(false); return; } - // Focus the safe action by default; Enter must not grant by accident. - $deny.get(0)?.focus(); + input_guard = guard_input({ + win: window, + doc: document, + ms: INPUT_PROTECTION_MS, + on_change: (ready) => { + sync_buttons(); + // Focus the safe action by default; Enter must not grant by + // accident. A disabled button can't take focus, so this waits + // for the guard. + if ( ready && ! granting ) $deny.get(0)?.focus(); + }, + }); }); } +/** + * Reports `ready` only once the page has been visible and focused for `ms` + * without a break. Losing visibility or focus resets it, and the wait starts + * over when both come back or anything enters or leaves fullscreen. A delay + * counted from opening alone can run out while the dialog is covered by a + * popup, another tab or a fullscreen element, leaving it live for a click the + * user aimed at whatever covered it. + * + * @param {Object} opts + * @param {Window} opts.win + * @param {Document} opts.doc + * @param {number} opts.ms + * @param {(ready: boolean) => void} opts.on_change - Called when `ready` flips. + * @returns {{ readonly ready: boolean, stop: () => void }} + */ +export function guard_input ({ win, doc, ms, on_change }) { + let ready = false; + let timer = null; + let seen = false; + const set_ready = (value) => { + if ( ready === value ) return; + ready = value; + on_change(value); + }; + const restart = () => { + clearTimeout(timer); + timer = null; + set_ready(false); + seen = doc.visibilityState === 'visible' && doc.hasFocus(); + if ( ! seen ) return; + timer = setTimeout(() => { + timer = null; + set_ready(true); + }, ms); + }; + // Window `blur`/`focus` also fire when focus moves into or out of an app's + // iframe, while the page stays focused throughout. Only a real change counts, + // or the click that pulls focus out of an iframe would reset the wait and be + // swallowed. + const on_focus_change = () => { + if ( seen !== (doc.visibilityState === 'visible' && doc.hasFocus()) ) { + restart(); + } + }; + const listeners = [ + [win, 'focus', on_focus_change], + [win, 'blur', on_focus_change], + [doc, 'visibilitychange', on_focus_change], + [doc, 'fullscreenchange', restart], + ]; + for ( const [target, name, fn] of listeners ) target.addEventListener(name, fn); + restart(); + return { + get ready () { + return ready; + }, + stop () { + clearTimeout(timer); + timer = null; + for ( const [target, name, fn] of listeners ) target.removeEventListener(name, fn); + }, + }; +} + /** * Withdraws a grant whose POST may have committed even though the dialog never * saw a success response, so a "Don't Allow" can't leave a live permission the @@ -720,34 +797,6 @@ const APP_DATA_VERBS = { expire: 'delete', expireAt: 'delete', }; -/** - * Look up the app a cross-app request names. Returns null when it does not - * exist, so an unresolvable target never reaches the prompt. - */ -async function get_app_by_uid (uid) { - try { - const res = await fetch(`${window.api_origin}/drivers/call`, { - headers: { - 'Content-Type': 'application/json', - 'Authorization': `Bearer ${window.auth_token}`, - }, - body: JSON.stringify({ - interface: 'puter-apps', - driver: 'es:app', - method: 'read', - args: { uid }, - }), - method: 'POST', - }); - if ( ! res.ok ) return null; - const body = await res.json(); - return body?.result ?? null; - } catch (e) { - console.error('Failed to look up app', uid, e); - return null; - } -} - /** * The uid of the app making the request, for the describers that compare it * against a uid the permission itself names. diff --git a/src/gui/src/UI/UIPermissionDialog.test.js b/src/gui/src/UI/UIPermissionDialog.test.js index 984894197..7ad3d7887 100644 --- a/src/gui/src/UI/UIPermissionDialog.test.js +++ b/src/gui/src/UI/UIPermissionDialog.test.js @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; // The dialog module reads `i18n` and other GUI plumbing as globals rather than // importing them, so they are defined before it loads. `i18n` echoes the key @@ -10,7 +10,7 @@ window.auth_token = 'tok'; globalThis.i18n = (key, params = {}) => `${key}(${Object.entries(params).map(([k, v]) => `${k}=${v}`).join(',')})`; -const { get_app_data_description, get_kv_share_description } = +const { get_app_data_description, get_kv_share_description, guard_input } = await import('./UIPermissionDialog.js'); const CONTACTS = 'app-contacts'; @@ -245,3 +245,122 @@ describe('UIPermissionDialog key-value delegation descriptions', () => { ).toBeNull(); }); }); + +describe('UIPermissionDialog input guard', () => { + const MS = 350; + let win; + let doc; + let guard; + let changes; + + /** A page the user is looking at, or not, as plain event targets. */ + const page = ({ visible = true, focused = true } = {}) => { + win = new EventTarget(); + doc = new EventTarget(); + doc.visibilityState = visible ? 'visible' : 'hidden'; + doc.hasFocus = () => focused; + return { win, doc }; + }; + + const start = (state) => { + page(state); + changes = []; + guard = guard_input({ + win, + doc, + ms: MS, + on_change: (ready) => changes.push(ready), + }); + }; + + beforeEach(() => vi.useFakeTimers()); + afterEach(() => { + guard?.stop(); + vi.useRealTimers(); + }); + + it('becomes ready after the delay on a visible, focused page', () => { + start(); + expect(guard.ready).toBe(false); + vi.advanceTimersByTime(MS - 1); + expect(guard.ready).toBe(false); + vi.advanceTimersByTime(1); + expect(guard.ready).toBe(true); + expect(changes).toEqual([true]); + }); + + it('does not count down while the page is unfocused', () => { + // Opened behind a popup: the delay must not run out unseen. + start({ focused: false }); + vi.advanceTimersByTime(MS * 10); + expect(guard.ready).toBe(false); + + doc.hasFocus = () => true; + win.dispatchEvent(new Event('focus')); + vi.advanceTimersByTime(MS - 1); + expect(guard.ready).toBe(false); + vi.advanceTimersByTime(1); + expect(guard.ready).toBe(true); + }); + + it('does not count down while the page is hidden', () => { + start({ visible: false }); + vi.advanceTimersByTime(MS * 10); + expect(guard.ready).toBe(false); + + doc.visibilityState = 'visible'; + doc.dispatchEvent(new Event('visibilitychange')); + vi.advanceTimersByTime(MS); + expect(guard.ready).toBe(true); + }); + + it('re-disables on blur and waits again after focus returns', () => { + start(); + vi.advanceTimersByTime(MS); + expect(guard.ready).toBe(true); + + doc.hasFocus = () => false; + win.dispatchEvent(new Event('blur')); + expect(guard.ready).toBe(false); + + doc.hasFocus = () => true; + win.dispatchEvent(new Event('focus')); + expect(guard.ready).toBe(false); + vi.advanceTimersByTime(MS); + expect(guard.ready).toBe(true); + expect(changes).toEqual([true, false, true]); + }); + + it('keeps the wait when focus only moves between the page and an iframe', () => { + // The page keeps focus throughout, so the user's first click on the + // dialog must not be swallowed by a restart. + start(); + vi.advanceTimersByTime(MS - 100); + win.dispatchEvent(new Event('blur')); + win.dispatchEvent(new Event('focus')); + vi.advanceTimersByTime(100); + expect(guard.ready).toBe(true); + win.dispatchEvent(new Event('focus')); + expect(guard.ready).toBe(true); + expect(changes).toEqual([true]); + }); + + it('restarts the wait when something leaves fullscreen', () => { + start(); + vi.advanceTimersByTime(MS); + doc.dispatchEvent(new Event('fullscreenchange')); + expect(guard.ready).toBe(false); + vi.advanceTimersByTime(MS); + expect(guard.ready).toBe(true); + }); + + it('stops listening once stopped', () => { + start({ focused: false }); + guard.stop(); + doc.hasFocus = () => true; + win.dispatchEvent(new Event('focus')); + vi.advanceTimersByTime(MS * 10); + expect(guard.ready).toBe(false); + expect(changes).toEqual([]); + }); +}); diff --git a/src/gui/src/helpers/defaultApp.js b/src/gui/src/helpers/defaultApp.js new file mode 100644 index 000000000..b79f2f461 --- /dev/null +++ b/src/gui/src/helpers/defaultApp.js @@ -0,0 +1,56 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import path from '../lib/path.js'; +import { get_app_by_uid } from './getAppByUid.js'; + +/** The `user_preferences` key holding the "always open with" app for a file's extension. */ +const default_app_key = (file_path) => `default_apps${path.extname(file_path).toLowerCase()}`; + +/** + * Whether a stored "always open with" value names `app`. New values are the + * app's uid; older ones are its name. + */ +export const is_default_app = (pref, app) => !!pref && !!app && (pref === app.uuid || pref === app.name); + +/** + * The app the user chose to always open this file's type with, or null. Pass + * it to `launch_app` as `app_obj`. + * + * The choice only counts while it names one of the file's suggested apps + * (built-ins and apps approved for opening files): an app name is free for + * anyone to claim once the app is renamed or deleted, and older choices are + * stored by name. Suggestions are cached server-side and can carry a name + * that has since moved to another app, so the match is re-read by uid, which + * also brings the launch gates `puter.apps.get` applies. + */ +export const resolve_default_app = async ({ uid, path: file_path }) => { + const pref = window.user_preferences?.[default_app_key(file_path)]; + if ( ! pref ) return null; + let match; + try { + const suggested_apps = await window.suggest_apps_for_fsentry({ uid, path: file_path }); + match = suggested_apps?.find(app => is_default_app(pref, app)); + } catch (e) { + console.warn('Could not resolve default app', e); + return null; + } + if ( ! match?.uuid ) return null; + return await get_app_by_uid(match.uuid, { icon_size: 64 }); +}; diff --git a/src/gui/src/helpers/defaultApp.test.js b/src/gui/src/helpers/defaultApp.test.js new file mode 100644 index 000000000..76ff083e0 --- /dev/null +++ b/src/gui/src/helpers/defaultApp.test.js @@ -0,0 +1,146 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { is_default_app, resolve_default_app } from './defaultApp.js'; + +const EDITOR = { uuid: 'app-11111111-1111-4111-8111-111111111111', name: 'editor', title: 'Editor' }; +const MARKDOWN = { uuid: 'app-22222222-2222-4222-8222-222222222222', name: 'markdown-pro', title: 'Markdown Pro' }; +const FILE = { uid: 'f0a1b2c3-d4e5-4f60-8172-839405162738', path: '/alice/Documents/notes.md' }; + +let suggested; +// What `puter-apps` read returns, by uid. +let apps; + +/** The uid each `puter-apps` read asked for, with its params. */ +const reads = () => globalThis.fetch.mock.calls.map(([, init]) => JSON.parse(init.body).args); + +beforeEach(() => { + suggested = [EDITOR, MARKDOWN]; + apps = { + [EDITOR.uuid]: { uid: EDITOR.uuid, name: 'editor', title: 'Editor', index_url: 'https://editor.test/' }, + [MARKDOWN.uuid]: { uid: MARKDOWN.uuid, name: 'markdown-pro', title: 'Markdown Pro', index_url: 'https://md.test/' }, + }; + globalThis.window = { + api_origin: 'https://api.test', + auth_token: 'tok', + user_preferences: {}, + suggest_apps_for_fsentry: vi.fn(async () => suggested), + }; + globalThis.fetch = vi.fn(async (_url, init) => { + const app = apps[JSON.parse(init.body).args.uid] ?? null; + return { ok: app !== null, json: async () => ({ success: true, result: app }) }; + }); +}); + +describe('is_default_app', () => { + it('matches a stored uid or a stored name', () => { + expect(is_default_app(MARKDOWN.uuid, MARKDOWN)).toBe(true); + expect(is_default_app('markdown-pro', MARKDOWN)).toBe(true); + expect(is_default_app(EDITOR.uuid, MARKDOWN)).toBe(false); + }); + + it('never matches an unset preference', () => { + expect(is_default_app(undefined, MARKDOWN)).toBe(false); + expect(is_default_app('', { uuid: '', name: '' })).toBe(false); + }); +}); + +describe('resolve_default_app', () => { + it('returns null without asking the server when no app is set for the extension', async () => { + window.user_preferences['default_apps.txt'] = EDITOR.uuid; + + expect(await resolve_default_app(FILE)).toBeNull(); + expect(window.suggest_apps_for_fsentry).not.toHaveBeenCalled(); + expect(globalThis.fetch).not.toHaveBeenCalled(); + }); + + it('resolves a stored uid to that suggested app, read by uid', async () => { + window.user_preferences['default_apps.md'] = MARKDOWN.uuid; + + expect(await resolve_default_app(FILE)).toBe(apps[MARKDOWN.uuid]); + expect(window.suggest_apps_for_fsentry).toHaveBeenCalledWith(FILE); + expect(reads()).toEqual([{ uid: MARKDOWN.uuid, params: { icon_size: 64 } }]); + }); + + it('reads the preference for the lowercased extension', async () => { + window.user_preferences['default_apps.md'] = MARKDOWN.uuid; + + expect(await resolve_default_app({ ...FILE, path: '/alice/NOTES.MD' })).toBe(apps[MARKDOWN.uuid]); + }); + + it('resolves a stored name while that app is still suggested for the file', async () => { + window.user_preferences['default_apps.md'] = 'markdown-pro'; + + expect(await resolve_default_app(FILE)).toBe(apps[MARKDOWN.uuid]); + expect(reads()).toEqual([{ uid: MARKDOWN.uuid, params: { icon_size: 64 } }]); + }); + + it('returns the app the uid names now, not a stale suggested name', async () => { + // Cached suggestions still carry the old name, which another app has + // since claimed. Launching by that name would open the other app. + window.user_preferences['default_apps.md'] = MARKDOWN.uuid; + apps[MARKDOWN.uuid] = { ...apps[MARKDOWN.uuid], name: 'markdown-pro-2' }; + + const app = await resolve_default_app(FILE); + expect(app.uid).toBe(MARKDOWN.uuid); + expect(app.name).toBe('markdown-pro-2'); + }); + + it('keeps the launch gates from the read', async () => { + window.user_preferences['default_apps.md'] = MARKDOWN.uuid; + const denied = { hasAccess: false, fallbackAppName: 'app-center' }; + apps[MARKDOWN.uuid] = { uid: MARKDOWN.uuid, name: 'markdown-pro', privateAccess: denied }; + + expect((await resolve_default_app(FILE)).privateAccess).toBe(denied); + }); + + it('returns null when the suggested app can no longer be read', async () => { + window.user_preferences['default_apps.md'] = MARKDOWN.uuid; + delete apps[MARKDOWN.uuid]; + + expect(await resolve_default_app(FILE)).toBeNull(); + }); + + it('ignores a stored name once no suggested app carries it', async () => { + // The app was renamed or deleted, and whoever holds the name now is + // not an app suggested for this file. + window.user_preferences['default_apps.md'] = 'markdown-pro'; + suggested = [EDITOR, { ...MARKDOWN, name: 'markdown-pro-2' }]; + + expect(await resolve_default_app(FILE)).toBeNull(); + }); + + it('ignores a stored uid whose app is no longer suggested', async () => { + window.user_preferences['default_apps.md'] = MARKDOWN.uuid; + suggested = [EDITOR]; + + expect(await resolve_default_app(FILE)).toBeNull(); + }); + + it('returns null when suggestions cannot be loaded', async () => { + window.user_preferences['default_apps.md'] = MARKDOWN.uuid; + window.suggest_apps_for_fsentry = vi.fn(async () => { + throw new Error('network'); + }); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + + expect(await resolve_default_app(FILE)).toBeNull(); + }); +}); diff --git a/src/gui/src/helpers/generateFileContextMenu.js b/src/gui/src/helpers/generateFileContextMenu.js index 5f2509a99..da14c6609 100644 --- a/src/gui/src/helpers/generateFileContextMenu.js +++ b/src/gui/src/helpers/generateFileContextMenu.js @@ -31,6 +31,7 @@ import path from '../lib/path.js'; import { isWeblinkName, weblinkChangeIconMenuItem } from './weblink.js'; import { is_owned_by_me } from './pathOwner.js'; import { can_rename, can_restructure, can_share, invalidate_shared_roots } from './sharedAccess.js'; +import { is_default_app } from './defaultApp.js'; /** * Generates context menu items for file/folder operations @@ -140,7 +141,8 @@ const generate_file_context_menu = async function (options) { // ------------------------------------------- // Publish as Worker // ------------------------------------------- - if ( !is_trashed && !is_trash && !fsentry.is_dir && $(el_item).attr('data-name').toLowerCase().endsWith('.js') ) { + // The backend refuses deploys from a file someone else owns. + if ( !is_trashed && !is_trash && !is_not_mine && !fsentry.is_dir && $(el_item).attr('data-name').toLowerCase().endsWith('.js') ) { menu_items.push({ html: i18n('publish_as_serverless_worker'), disabled: is_worker, @@ -506,7 +508,7 @@ async function generateOpenWithItems (el_item, fsentry, suggested_apps) { onClick: async function () { var extension = path.extname($(el_item).attr('data-path')).toLowerCase(); if ( - window.user_preferences[`default_apps${extension}`] !== suggested_app.name + ! is_default_app(window.user_preferences[`default_apps${extension}`], suggested_app) && ( (!window.user_preferences[`default_apps${extension}`] && index > 0) @@ -529,7 +531,7 @@ async function generateOpenWithItems (el_item, fsentry, suggested_apps) { ], }); if ( (alert_resp) === 'yes' ) { - window.user_preferences[`default_apps${extension}`] = suggested_app.name; + window.user_preferences[`default_apps${extension}`] = suggested_app.uuid; window.mutate_user_preferences(window.user_preferences); } } diff --git a/src/gui/src/helpers/getAppByUid.js b/src/gui/src/helpers/getAppByUid.js new file mode 100644 index 000000000..a6a011620 --- /dev/null +++ b/src/gui/src/helpers/getAppByUid.js @@ -0,0 +1,46 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +/** + * Read an app by uid, as `puter.apps.get` reads one by name (`params` takes + * the same options). Returns null when it does not exist or the lookup fails. + */ +export async function get_app_by_uid (uid, params) { + try { + const res = await fetch(`${window.api_origin}/drivers/call`, { + headers: { + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${window.auth_token}`, + }, + body: JSON.stringify({ + interface: 'puter-apps', + driver: 'es:app', + method: 'read', + args: { uid, params }, + }), + method: 'POST', + }); + if ( ! res.ok ) return null; + const body = await res.json(); + return body?.result ?? null; + } catch (e) { + console.error('Failed to look up app', uid, e); + return null; + } +} diff --git a/src/gui/src/helpers/openItem.js b/src/gui/src/helpers/openItem.js index c06ea2987..98763c97c 100644 --- a/src/gui/src/helpers/openItem.js +++ b/src/gui/src/helpers/openItem.js @@ -23,6 +23,7 @@ import i18n from '../i18n/i18n.js'; import launch_app from './launchApp.js'; import path from '../lib/path.js'; import item_icon from './itemIcon.js'; +import { resolve_default_app } from './defaultApp.js'; import { is_window_on_screen, user_facing_windows } from './windowVisibility.js'; // Files whose app launch is still in flight, by file uid. Between the click @@ -283,22 +284,6 @@ Please try recreating the link.`); } } //---------------------------------------------------------------- - // Does the user have a preference for this file type? - //---------------------------------------------------------------- - else if ( !associated_app_name && !is_dir && window.user_preferences[`default_apps${path.extname(item_path).toLowerCase()}`] ) { - const launch_promise = launch_app({ - name: window.user_preferences[`default_apps${path.extname(item_path).toLowerCase()}`], - file_path: item_path, - window_title: path.basename(item_path), - maximized: options.maximized ?? window.is_dashboard_mode, - file_uid: file_uid, - }); - if ( target_file_uid ) { - launching_file_uids.set(target_file_uid, Date.now()); - launch_promise.finally(() => launching_file_uids.delete(target_file_uid)); - } - } - //---------------------------------------------------------------- // Is there an app associated with this item? //---------------------------------------------------------------- else if ( associated_app_name !== '' ) { @@ -347,6 +332,21 @@ Please try recreating the link.`); // the dashboard reuse branch above swallows re-clicks meanwhile. if ( target_file_uid ) launching_file_uids.set(target_file_uid, Date.now()); + // Does the user have a preference for this file type? + const default_app = await resolve_default_app({ uid: fsuid, path: item_path }); + if ( default_app ) { + const launch_promise = launch_app({ + name: default_app.name, + app_obj: default_app, + file_path: item_path, + window_title: path.basename(item_path), + maximized: options.maximized ?? window.is_dashboard_mode, + file_uid: file_uid, + }); + if ( target_file_uid ) launch_promise.finally(() => launching_file_uids.delete(target_file_uid)); + return; + } + // get all info needed to open an item try { open_item_meta = await $.ajax({ diff --git a/src/gui/src/initgui.js b/src/gui/src/initgui.js index b998d22bc..d440ffd71 100644 --- a/src/gui/src/initgui.js +++ b/src/gui/src/initgui.js @@ -85,7 +85,13 @@ import { ThemeService } from './services/ThemeService.js'; // silently resolve to the factory — use `window.privacy_aware_path` instead. import { privacy_aware_path as privacy_aware_path_factory } from './util/desktop.js'; import { resolveAPIOrigin } from './util/apiOrigin.js'; -import { deliversTokenAtBoot, relationshipIsConsent, runsUserAppTokenExchange } from './util/popupAuth.js'; +import { + deliversTokenAtBoot, + relationshipIsConsent, + relaysTokenAtBoot, + resolveOpenerOrigin, + runsUserAppTokenExchange, +} from './util/popupAuth.js'; import { verifyOidcPopupReturn } from './util/popupOidcReturn.js'; /** @@ -351,8 +357,10 @@ const postAuthActions = async (action) => { // rule as the postMessage hand-off below — otherwise adding one query // parameter to a `request-permission` URL turns the permission prompt // into a token grant, and skips the prompt entirely. - let isolated = window.url_query_params.get("cross_origin_isolated") === 'true' - && deliversTokenAtBoot(action); + let isolated = relaysTokenAtBoot( + action, + window.url_query_params.get('cross_origin_isolated') === 'true', + ); let session = window.url_query_params.get('signin_session'); // Signing the opener in is something the user has to have asked for. @@ -1487,14 +1495,19 @@ window.initgui = async function (options) { window.url_query_params.get('msg_id'), action, ); - window.openerOrigin = - window.oidcPopupReturn?.opener_origin || document.referrer; - if (!window.openerOrigin) { - try { - window.openerOrigin = await requestOpenerOrigin(); - } catch (e) { - throw new Error('No referrer found'); - } + try { + window.openerOrigin = await resolveOpenerOrigin({ + relaysToken: relaysTokenAtBoot( + action, + window.url_query_params.get('cross_origin_isolated') === + 'true', + ), + oidcOpenerOrigin: window.oidcPopupReturn?.opener_origin, + referrer: document.referrer, + askOpener: requestOpenerOrigin, + }); + } catch (e) { + throw new Error('No referrer found'); } // Everything below needs an origin to identify the opener by. diff --git a/src/gui/src/util/popupAuth.js b/src/gui/src/util/popupAuth.js index 77da4d7ad..7753cc138 100644 --- a/src/gui/src/util/popupAuth.js +++ b/src/gui/src/util/popupAuth.js @@ -88,6 +88,43 @@ export const defersTokenToOpener = (action) => export const deliversTokenAtBoot = (action) => deliversTokenToOpener(action) && !defersTokenToOpener(action); +/** + * Whether a popup hands its boot-time token to the `/login/set` relay instead + * of posting it to the opener. + * + * @param {string|null|undefined} action + * @param {boolean} crossOriginIsolated - The popup's `cross_origin_isolated` flag. + * @returns {boolean} + */ +export const relaysTokenAtBoot = (action, crossOriginIsolated) => + crossOriginIsolated && deliversTokenAtBoot(action); + +/** + * The opener origin a popup acts for. + * + * A relay popup takes it only from `document.referrer`, the origin of the page + * that opened it with this session id. Whoever holds the session id collects + * the relayed token, and the opener can navigate to another app before it + * answers the `requestOrigin` handshake. + * + * @param {{ + * relaysToken: boolean; + * oidcOpenerOrigin: string|null|undefined; + * referrer: string; + * askOpener: () => Promise; + * }} sources + * @returns {Promise} `''` for a relay popup with no referrer. + */ +export const resolveOpenerOrigin = async ({ + relaysToken, + oidcOpenerOrigin, + referrer, + askOpener, +}) => { + if ( relaysToken ) return referrer; + return oidcOpenerOrigin || referrer || await askOpener(); +}; + /** * Whether the opener's existing relationship with the user stands in for * consent. Only when no account picker was shown: a dismissed picker is a no. diff --git a/src/gui/src/util/popupAuth.test.js b/src/gui/src/util/popupAuth.test.js index 2b69b8c3a..ab9e08063 100644 --- a/src/gui/src/util/popupAuth.test.js +++ b/src/gui/src/util/popupAuth.test.js @@ -25,6 +25,8 @@ import { deliversTokenToOpener, offersFederatedSignInInPopup, relationshipIsConsent, + relaysTokenAtBoot, + resolveOpenerOrigin, runsUserAppTokenExchange, } from './popupAuth.js'; @@ -201,6 +203,81 @@ describe('relationshipIsConsent', () => { }); }); +describe('relaysTokenAtBoot', () => { + it('relays only for an isolated popup that hands over a token at boot', () => { + expect(relaysTokenAtBoot('sign-in', true)).toBe(true); + expect(relaysTokenAtBoot(undefined, true)).toBe(true); + expect(relaysTokenAtBoot('sign-in', false)).toBe(false); + expect(relaysTokenAtBoot('request-permission', true)).toBe(false); + expect(relaysTokenAtBoot('show-open-file-picker', true)).toBe(false); + }); +}); + +describe('resolveOpenerOrigin', () => { + const OPENER = 'https://opener.example'; + const OTHER_APP = 'https://other-app.example'; + // What a page running puter.js answers the handshake with: its own origin, + // whichever app the opener window has navigated to by then. + const answeredBy = (origin) => async () => origin; + + it('never asks the opener when the token goes to the relay', async () => { + let asked = false; + const origin = await resolveOpenerOrigin({ + relaysToken: true, + oidcOpenerOrigin: undefined, + referrer: '', + askOpener: async () => { + asked = true; + return OTHER_APP; + }, + }); + expect(asked).toBe(false); + expect(origin).toBe(''); + }); + + it('uses the referrer for a relay popup', async () => { + const origin = await resolveOpenerOrigin({ + relaysToken: true, + oidcOpenerOrigin: undefined, + referrer: OPENER, + askOpener: answeredBy(OTHER_APP), + }); + expect(origin).toBe(OPENER); + }); + + it('ignores an OIDC proof for a relay popup', async () => { + // The proof's origin may itself have come from the handshake. + const origin = await resolveOpenerOrigin({ + relaysToken: true, + oidcOpenerOrigin: OTHER_APP, + referrer: OPENER, + askOpener: answeredBy(OTHER_APP), + }); + expect(origin).toBe(OPENER); + }); + + it('prefers the OIDC proof, then the referrer, then the handshake otherwise', async () => { + const base = { relaysToken: false, askOpener: answeredBy(OTHER_APP) }; + expect(await resolveOpenerOrigin({ ...base, oidcOpenerOrigin: OPENER, referrer: 'https://idp.example' })) + .toBe(OPENER); + expect(await resolveOpenerOrigin({ ...base, oidcOpenerOrigin: null, referrer: OPENER })) + .toBe(OPENER); + expect(await resolveOpenerOrigin({ ...base, oidcOpenerOrigin: null, referrer: '' })) + .toBe(OTHER_APP); + }); + + it('surfaces a failed handshake', async () => { + await expect(resolveOpenerOrigin({ + relaysToken: false, + oidcOpenerOrigin: null, + referrer: '', + askOpener: async () => { + throw new Error('No window.opener available'); + }, + })).rejects.toThrow('No window.opener available'); + }); +}); + describe('the retired opener_origin gate', () => { it('is gone, because no action believes the raw parameter now', () => { // It used to allow `opener_origin` for every action but diff --git a/src/puter-js/src/modules/kv/remove.js b/src/puter-js/src/modules/kv/remove.js index d87cd98b6..308d671aa 100644 --- a/src/puter-js/src/modules/kv/remove.js +++ b/src/puter-js/src/modules/kv/remove.js @@ -9,7 +9,8 @@ import { assertKeyPresent, assertKeySize } from './lib/validate.js'; * Removes values from a key by one or more dot-separated paths (e.g. * `"profile.bio"`), returning the updated value. An `optConfig` object may * trail the paths. Removing the root path (`''`) deletes the key and - * resolves to `null`. + * resolves to `null`, as does a call on another app's data the caller may + * delete from but not read. * * @this {import('./index.js').KVModule} * @param {string} key