fix(federation): close detached-account gaps from final review — presence/hydrate guards, ack re-detect clear, self-delete password (detach spec §4.3/§4.4/§4.6)

2026-07-02 19:34:32 +02:00
parent 172398171a
commit 13d050c1bb
10 changed files with 190 additions and 8 deletions
@@ -380,7 +380,7 @@ When a user changes their password on their home instance, `authStore.changePass

 **Pre-checks:**
 1. `username` must match stored username (confirmation safeguard)
-2. Local users must provide and verify `password`; federated users rely on JWT auth
+2. Native local users **and detached accounts** (`federation_home_orphaned = 1`) must provide and verify `password` against the local hash; non-detached federated users rely on JWT auth (their home instance already vouches for them). A detached account is a sovereign local account with no home verifying anything, so it follows the LOCAL rule — the same self-destruct protection as a native account, and mirroring the change-password rule (§5, detach spec §4.4). Condition: `!user.homeInstance || user.federationHomeOrphaned === 1`. Missing password → 400; wrong password → 403.
 3. Must not own any spaces (returns 400 with `ownedSpaces` list)

 **Client-side flow** (`authStore.deleteAccount()`):
@@ -366,7 +366,7 @@ Detection (`markPeerReset`) only snapshots + journals + notifies; it destroys no
 - For every flagged real account (`federation_heal_pending = 1`, non-stub, `isDeleted = 0`, `homeInstanceMatch`): set `federation_home_orphaned = 1` and clear `federation_heal_pending`. **That is all** — this is a flag-only **detach**, not a freeze.
 - **`federation_home_orphaned = 1` means "DETACHED / sovereign local account,"** not "frozen." The account was cut loose from its (now-reset) home instance and operates as a purely local account from here on: it logs in with its **local password** (`auth.ts` no longer blocks a detached account before password verify — only the self-heal branch is permanently disabled for it, see `auth.md` §4), and it gains local profile edit + local change-password (`users.ts`). There is **no login freeze**.
 - **No rename.** The username is preserved — first-come-first-served on this instance. There is **no `!orphaned:{uid}@{domain}` handle-freeing** and **no space-owner special case**: all real accounts are treated uniformly and owners simply keep managing their spaces.
- **What closes the post-re-peer hijack** is NOT a login freeze. It is the combination of (a) the login self-heal being **permanently disabled** for detached accounts (`auth.ts` returns 401 in the failed-local-password branch without contacting the home domain — a new incarnation can never re-hash its way in) and (b) the S2S identity-binding guards, which **exclude detached rows** on every domain-keyed surface: `findFederatedUser` tier-2 (`federation_home_orphaned = 0` predicate, ~line 3522), the S2S `profile_update` handler (accept-and-skip, ~line 6162), and the S2S `DELETE /api/federation/identity` guard (idempotent 200, no deletion, ~line 2357). See "S2S Identity Deletion" above and design §4.3.
+- **What closes the post-re-peer hijack** is NOT a login freeze. It is the combination of (a) the login self-heal being **permanently disabled** for detached accounts (`auth.ts` returns 401 in the failed-local-password branch without contacting the home domain — a new incarnation can never re-hash its way in) and (b) the S2S identity-binding guards, which **exclude detached rows** on every domain-keyed surface: `findFederatedUser` tier-2 (`federation_home_orphaned = 0` predicate, ~line 3522), the S2S `profile_update` handler (accept-and-skip, ~line 6162), the S2S `presence_update` handler (accept-and-skip, after the domain-collision check), the `hydrateReplicatedUserProfile` fill-empty path (no-op early return alongside the native-user skip), and the S2S `DELETE /api/federation/identity` guard (idempotent 200, no deletion, ~line 2357). Every domain-keyed **mutation** that a tier-1 (`homeUserId`) hit can reach is guarded at its own site — a tier-1 hit on a detached row is a legitimate historical read, but no write is applied. See "S2S Identity Deletion" above and design §4.3.
 - Content (space messages, memberships, reactions) and usernames are preserved in all cases. No `user_updated` broadcast is emitted (nothing visible changes). The returned count refreshes the journal's `orphaned_account_count`.

 **Login self-heal epoch guard (design §6.3a).** The federated password self-heal (`auth.ts` §4) gates re-hashing on the home instance's current epoch, read via the authenticated `fetchPeerEpoch(peer)` (HMAC-signed both ways): no baseline on record → allow (legacy); baseline differs from the fetched epoch → refuse; epoch can't be determined (`fetchPeerEpoch` null — 404/unreachable/bad-sig/desynced secret) → **fail closed/refuse**; match → allow. This closes the *pre*-re-peer hijack (a reset home accepting a new same-name user's password during the undetected-reset window). The *post*-re-peer window is closed by detach: once an account is detached, its self-heal path is permanently disabled and the S2S binding guards exclude it (above), so the new incarnation has zero influence over it. Full three-way in `auth.md` §4.
@@ -505,7 +505,7 @@ Two layers of replay protection:
 - Three-tier lookup: homeUserId match → domain + username hint match → not found
 - Tier 1: delegates to `resolveLocalUser` (fast path)
 - Tier 2: uses `extractDomain(homeInstance)` + `hints.username` to match stubs created by the auth registration path (which may have a different homeUserId)
- **Tier 2 excludes detached accounts** (`federation_home_orphaned = 1`): a detached account is sovereign and must never be re-bound to the reset domain's new incarnation via username heuristics — that is exactly how a new same-name user would capture the established account. **Tier 1 (`homeUserId` match) is deliberately NOT excluded:** the new incarnation mints fresh `homeUserId`s, so a tier-1 hit on a detached row is a legitimate historical reference (e.g. an old group-DM attribution relayed by a third instance), not the new incarnation. Mutations are blocked at their own sites (profile_update handler, S2S identity delete).
+- **Tier 2 excludes detached accounts** (`federation_home_orphaned = 1`): a detached account is sovereign and must never be re-bound to the reset domain's new incarnation via username heuristics — that is exactly how a new same-name user would capture the established account. **Tier 1 (`homeUserId` match) is deliberately NOT excluded:** the new incarnation mints fresh `homeUserId`s, so a tier-1 hit on a detached row is a legitimate historical reference (e.g. an old group-DM attribution relayed by a third instance), not the new incarnation. Mutations are blocked at their own sites (profile_update handler, presence_update handler, `hydrateReplicatedUserProfile` fill-empty, S2S identity delete).
 - Side-effect-free — does not modify any records
 - When multiple candidates match in tier 2, prefers real accounts over stubs, then most profile data
 - **Use when:** Read-only lookup that needs to find users created by either auth or relay path
@@ -1294,6 +1294,8 @@ When relay events carry `FederationRelayProfileSnapshot` data:

 `hydrateReplicatedUserProfile` is **best-effort fill-empty only**: it never overwrites an existing avatar/banner/displayName/bio. This protects locally-downloaded bare filenames produced by `processProfileUpdateEvent` (which carries the monotonic `profileUpdatedAt` version) from being clobbered back to absolute URLs on the next DM/friend relay. Authoritative updates flow exclusively through the version-checked `profile_update` event.

+**Detached-account guard:** Alongside the native-user skip (`!user.homeInstance → return`), the function also **no-ops on detached rows** (`federation_home_orphaned = 1 → return user`). A detached account retains its `homeInstance` for provenance (design §7), so the native-user skip alone would not catch it; without this guard a DM/friend relay from the reset domain's new incarnation, resolved via an old `homeUserId` tier-1 hit, could fill the sovereign account's empty fields. This is the same domain-keyed mutation class as `profile_update`/`presence_update`, guarded at its own site (design §4.3).
+
 When the function does fill an empty avatar/banner, it calls `downloadProfileAsset` against the user's home instance and stores the resulting **local bare filename**. Only on download failure does it fall back to the absolute URL — matching the behavior of `processProfileUpdateEvent`.

 #### Profile Sync (S2S)
@@ -1370,6 +1372,8 @@ No-op for replicated users (we don't own their presence).

 **Receiver:** `processPresenceUpdateEvent` (`routes/federation.ts`). Strict attribution — `payload.homeInstance` domain MUST equal source peer's domain. Resolves the local stub by `homeUserId`, validates the stub's `homeInstance` matches the payload's domain, updates the stub's `status`, and broadcasts a WS `presence_update` to local users via `collectProfileBroadcastTargetIds(stub.id)` — friends, DM members, and space co-members.

+**Detached-account guard:** After the domain-collision check and before the status write, if the resolved stub has `federation_home_orphaned = 1` (detached — home domain was reset, now a sovereign local account), the event is **acked (messageId pushed to `accepted`) and skipped without applying**. The reset domain's new incarnation must never flip an established account's presence by replaying its old `homeUserId`. Ack (not reject) mirrors the `profile_update` guard rationale — the sender considers the identity theirs, so we no-op rather than trigger a retry loop.
+
 **Peer lifecycle hooks** (`utils/federationPresence.ts`):
 - **`onPeerActivated`** invokes `snapshotPresenceForPeer(origin)` — emits a `presence_update` only for online natives that have an S2S relationship with the peer (friend/DM with a peer-stub, or `replicatedInstances` opt-in for the peer origin). Snapshot work scales with relationship count, not native count.
 - **`onPeerDeactivated`** invokes `markPeerStubsOffline(origin)` — flips every stub from that peer to `offline` and broadcasts a local `presence_update` so users see them go offline immediately.
@@ -145,6 +145,66 @@ describe('S2S identity delete — detached account guard', () => {
  });
 });

+describe('S2S presence_update — detached account guard', () => {
+  it('skips a detached account (acked, status unchanged)', async () => {
+    const fed = await import('./federation.js');
+    const event: FederationRelayEvent = {
+      eventType: 'presence_update',
+      contextType: 'profile',
+      messageId: 'm-presence-hijack',
+      encryptionVersion: 0,
+      timestamp: Date.now(),
+      presenceUpdate: {
+        homeUserId: DETACHED_HOME_UID,
+        homeInstance: PEER_DOMAIN,
+        status: 'online', // would flip the sovereign account's presence if not guarded
+        ts: Date.now(),
+      },
+    };
+    const accepted: string[] = [];
+    const rejected: Array<{ messageId: string; reason: string }> = [];
+    fed.processPresenceUpdateEvent(event, PEER_DOMAIN, testDb, accepted, rejected);
+
+    // Acked (not rejected) — avoid a sender retry loop.
+    expect(rejected).toEqual([]);
+    expect(accepted).toEqual(['m-presence-hijack']);
+
+    // The detached row's status is untouched (stays offline).
+    const row = testDb.select().from(schema.users).where(eq(schema.users.id, DETACHED_ID)).get();
+    expect(row?.status).toBe('offline');
+  });
+});
+
+describe('hydrateReplicatedUserProfile — detached account guard', () => {
+  it('leaves a detached row untouched even when fields are empty', async () => {
+    const fed = await import('./federation.js');
+    const before = testDb.select().from(schema.users).where(eq(schema.users.id, DETACHED_ID)).get()!;
+    expect(before.bio).toBeNull();
+    expect(before.avatarColor).toBeNull();
+
+    // A replayed old-homeUserId snapshot from the new incarnation. Without the
+    // detached guard, hydrate would fill the empty bio / avatarColor fields.
+    const result = await fed.hydrateReplicatedUserProfile(before, {
+      username: 'alice',
+      displayName: 'Hijacked',
+      bio: 'hijacked bio',
+      avatarColor: '#ffffff',
+      avatar: null,
+      banner: null,
+    }, testDb);
+
+    // No-op return: the row object is returned unchanged.
+    expect(result.bio).toBeNull();
+    expect(result.avatarColor).toBeNull();
+
+    // And the DB row is untouched.
+    const after = testDb.select().from(schema.users).where(eq(schema.users.id, DETACHED_ID)).get()!;
+    expect(after.bio).toBeNull();
+    expect(after.avatarColor).toBeNull();
+    expect(after.displayName).toBe('Alice');
+  });
+});
+
 describe('S2S profile_update — detached account guard', () => {
  it('skips a detached account (acked, profile unchanged)', async () => {
    const fed = await import('./federation.js');
@@ -4962,6 +4962,12 @@ export async function hydrateReplicatedUserProfile(
 ): Promise<typeof schema.users.$inferSelect> {
  if (!profile) return user;
  if (!user.homeInstance) return user; // Don't update native users
+  // Detached accounts are sovereign local accounts: the home domain now belongs
+  // to a different incarnation, so a relayed snapshot resolved via an old
+  // homeUserId (tier-1 historical hit) must never fill this row's fields. No-op
+  // return, mirroring the profile_update / presence_update / identity-delete
+  // guards (detach spec §4.3).
+  if (user.federationHomeOrphaned === 1) return user;

  const baseUrl = user.homeInstance.startsWith('http') ? user.homeInstance : `https://${user.homeInstance}`;
  const buildAbsoluteUrl = (value: string): string => {
@@ -6572,6 +6578,16 @@ export function processPresenceUpdateEvent(
    return;
  }

+  // Detached accounts are sovereign: the domain now belongs to a different
+  // incarnation, which must never flip the established account's presence by
+  // replaying its old homeUserId. Ack (not reject) — the sender considers this
+  // identity theirs to update; from our side the update simply no-ops.
+  if (localUser.federationHomeOrphaned === 1) {
+    console.log(`[federation] Skipping presence_update for detached account ${localUser.id} (home-orphaned)`);
+    accepted.push(event.messageId);
+    return;
+  }
+
  db.update(schema.users)
    .set({ status: payload.status })
    .where(eq(schema.users.id, localUser.id))
@@ -198,6 +198,51 @@ describe('POST /api/users/@me/change-password — local rule for detached accoun
  });
 });

+describe('DELETE /api/users/@me — local rule for detached accounts', () => {
+  it('detached self-delete REQUIRES the local password (local rule)', async () => {
+    // No password → 400
+    const missing = await app.inject({
+      method: 'DELETE',
+      url: '/api/users/@me',
+      headers: { Authorization: `Bearer ${detachedToken()}` },
+      payload: { username: DETACHED_USERNAME },
+    });
+    expect(missing.statusCode).toBe(400);
+    expect(testDb.select().from(schema.users).where(eq(schema.users.id, DETACHED_ID)).get()!.isDeleted).toBe(0);
+
+    // Wrong password → 403
+    const wrong = await app.inject({
+      method: 'DELETE',
+      url: '/api/users/@me',
+      headers: { Authorization: `Bearer ${detachedToken()}` },
+      payload: { username: DETACHED_USERNAME, password: 'not-the-password' },
+    });
+    expect(wrong.statusCode).toBe(403);
+    expect(testDb.select().from(schema.users).where(eq(schema.users.id, DETACHED_ID)).get()!.isDeleted).toBe(0);
+
+    // Correct password → 200 and the account is tombstoned.
+    const ok = await app.inject({
+      method: 'DELETE',
+      url: '/api/users/@me',
+      headers: { Authorization: `Bearer ${detachedToken()}` },
+      payload: { username: DETACHED_USERNAME, password: DETACHED_PASSWORD },
+    });
+    expect(ok.statusCode).toBe(200);
+    expect(testDb.select().from(schema.users).where(eq(schema.users.id, DETACHED_ID)).get()!.isDeleted).toBe(1);
+  });
+
+  it('non-detached federated self-delete still works JWT-only (no password required)', async () => {
+    const res = await app.inject({
+      method: 'DELETE',
+      url: '/api/users/@me',
+      headers: { Authorization: `Bearer ${federatedToken()}` },
+      payload: { username: FEDERATED_USERNAME },
+    });
+    expect(res.statusCode).toBe(200);
+    expect(testDb.select().from(schema.users).where(eq(schema.users.id, FEDERATED_ID)).get()!.isDeleted).toBe(1);
+  });
+});
+
 describe('sanitizeUser — federationHomeOrphaned is self-view only', () => {
  it('exposes federationHomeOrphaned only on self-view', () => {
    const detachedRow = testDb.select().from(schema.users).where(eq(schema.users.id, DETACHED_ID)).get()!;
@@ -121,8 +121,13 @@ export async function userRoutes(app: FastifyInstance): Promise<void> {
      return reply.code(400).send({ error: 'Username does not match', statusCode: 400 });
    }

-    // Native users must verify password; federated users rely on JWT auth
-    if (!user.homeInstance) {
+    // Native users must verify password; non-detached federated users rely on
+    // JWT auth (their home instance already vouches for them). EXCEPTION:
+    // detached accounts (federation_home_orphaned = 1) are sovereign local
+    // accounts with no home verifying anything — they follow the LOCAL rule and
+    // must supply their local password to self-destruct, mirroring
+    // change-password (detach spec §4.4).
+    if (!user.homeInstance || user.federationHomeOrphaned === 1) {
      if (!password || typeof password !== 'string') {
        return reply.code(400).send({ error: 'Password is required', statusCode: 400 });
      }
@@ -149,6 +149,47 @@ describe('markPeerReset — detection-only reset routing', () => {
    expect(row.resolvedAt).toBeNull();
  });

+  it('re-detected reset clears a stale acknowledgedAt (dismissed card re-surfaces)', async () => {
+    seedPeer();
+    seedUser('stub-1', { passwordHash: STUB });
+    seedUser('real-1', { passwordHash: '$2b$10$realbcrypthash' });
+
+    const { markPeerReset } = await import('./federationReset.js');
+
+    // First reset detected, then the admin dismisses (acknowledges) the card.
+    markPeerReset('peer-1', ORIGIN, 'E0', 'E1');
+    testDb.update(schema.federationResetEvents)
+      .set({ acknowledgedAt: Date.now() })
+      .where(eq(schema.federationResetEvents.origin, ORIGIN)).run();
+    expect(testDb.select().from(schema.federationResetEvents)
+      .where(eq(schema.federationResetEvents.origin, ORIGIN)).get()!.acknowledgedAt).not.toBeNull();
+
+    // The peer resets AGAIN before the first was resolved — a fresh batch is
+    // detached and needs fresh admin attention, so the dismissal must clear.
+    markPeerReset('peer-1', ORIGIN, 'E0', 'E2');
+    expect(testDb.select().from(schema.federationResetEvents)
+      .where(eq(schema.federationResetEvents.origin, ORIGIN)).get()!.acknowledgedAt).toBeNull();
+  });
+
+  it('a resolved+acknowledged prior reset is re-armed (acknowledgedAt cleared) on a new reset', async () => {
+    seedPeer();
+    seedUser('stub-1', { passwordHash: STUB });
+
+    const { markPeerReset } = await import('./federationReset.js');
+    markPeerReset('peer-1', ORIGIN, 'E0', 'E1');
+    // Simulate the heal resolving the first reset AND the admin dismissing it.
+    testDb.update(schema.federationResetEvents)
+      .set({ resolvedAt: Date.now(), newEpoch: 'E1', acknowledgedAt: Date.now() })
+      .where(eq(schema.federationResetEvents.origin, ORIGIN)).run();
+
+    // Brand-new reset lands (fresh-journal / onConflictDoUpdate branch).
+    markPeerReset('peer-1', ORIGIN, 'E1', 'E2');
+    const row = testDb.select().from(schema.federationResetEvents)
+      .where(eq(schema.federationResetEvents.origin, ORIGIN)).get()!;
+    expect(row.resolvedAt).toBeNull();
+    expect(row.acknowledgedAt).toBeNull();
+  });
+
  it('matches home_instance stored as a full URL (defensive format match)', async () => {
    seedPeer();
    // Legacy straggler stored with the https:// prefix rather than bare domain.
@@ -113,9 +113,12 @@ export function markPeerReset(peerId: string, origin: string, deadEpoch: string,
    if (existing && existing.resolvedAt === null) {
      // Double-reset: keep the ORIGINAL dead_epoch + detected_at (the
      // incarnation already snapshotted), refresh counts only. Never overwrite
-      // dead_epoch on an unresolved row.
+      // dead_epoch on an unresolved row. Clear `acknowledged_at`: a re-detected
+      // reset is a fresh event that detached a new batch and needs fresh admin
+      // attention — a stale dismissal must not keep the disposition card hidden
+      // (detach spec §4.6).
      tx.update(schema.federationResetEvents)
-        .set({ stubCount, orphanedAccountCount })
+        .set({ stubCount, orphanedAccountCount, acknowledgedAt: null })
        .where(eq(schema.federationResetEvents.origin, origin))
        .run();
    } else {
@@ -140,6 +143,10 @@ export function markPeerReset(peerId: string, origin: string, deadEpoch: string,
            resolvedAt: null,
            stubCount,
            orphanedAccountCount,
+            // Re-arm the admin surface: a fresh reset on a previously
+            // resolved+dismissed origin must clear the old dismissal so the
+            // new detached batch's disposition card re-surfaces (detach §4.6).
+            acknowledgedAt: null,
          },
        })
        .run();
@@ -218,6 +218,10 @@ describe('FederationPanel — Reset cleanup', () => {
    const removeBtn = await screen.findByRole('button', { name: 'Remove' });
    fireEvent.click(removeBtn);

+    // The confirm dialog title uses the current "detached" vocabulary, not the
+    // legacy "Orphaned Account" wording.
+    expect(await screen.findByText('Remove detached account')).toBeInTheDocument();
+
    const confirmBtn = await screen.findByRole('button', { name: 'Delete permanently' });
    fireEvent.click(confirmBtn);

@@ -1095,7 +1095,7 @@ function ResetCleanup() {
          isOpen={true}
          onClose={() => { if (!actionLoading) setConfirmAction(null); }}
          onConfirm={handleConfirm}
-          title={confirmAction.kind === 'repeer' ? 'Re-establish Federation' : 'Remove Orphaned Account'}
+          title={confirmAction.kind === 'repeer' ? 'Re-establish Federation' : 'Remove detached account'}
          description={
            confirmAction.kind === 'repeer'
              ? `This deletes the local peer record and starts a fresh authenticated handshake with ${confirmAction.peer.origin}. The remote must be reachable and (if it does not auto-accept) approve the request.`