From ea640f3d4553393f84f48a257b23876c405067b5 Mon Sep 17 00:00:00 2001 From: Zaldaryon <273555259+Zaldaryon@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:48:30 -0300 Subject: [PATCH 1/3] feat(account): save multiple Vintage Story accounts and switch between them RiftLauncher supported one active account. On a shared device, a second player had to log out and enter their credentials again every time they wanted to play. This lets several accounts be saved and switched with no new login. Data model: config.account (a single AccountPublicType | null) becomes config.accounts (a list) and config.activeAccountId (the playerUid of the one the next launch writes into clientsettings.json, or null). playerUid is the key, not a new id field, since the launch path already trusts it as the account's identity. Switching needs no IPC channel or file write: clientsettings.json is written from exactly one place, EXECUTE_GAME, right before launch, so a config field change is the whole feature. The secret store (src/ipc/accountStore.ts) moves from one encrypted AccountSecrets blob to one encrypted map keyed by playerUid (saveAccountSecrets, getAccountSecrets, removeAccountSecrets, listStoredAccountIds), still one safeStorage-encrypted file, still never exposing a session key to the renderer. LOGOUT is replaced by REMOVE_ACCOUNT, since "log out" has no coherent meaning once there is a list; logging in twice with the same account refreshes its entry in place instead of duplicating it. Multi-account introduces one real new hazard: if the active account's secrets can't be read (a locked keyring), the settings file might still hold a HOUSEMATE's session, written there directly by the game on their own login. With one account that was harmless; with several it means launching already signed in as someone else. EXECUTE_GAME now clears a foreign session (a playeruid that isn't ours) before launching with none of our own, narrow enough to never touch a file that already shows nobody or shows us (domain/account/clientSettings.ts: removeSessionFromClientSettings, clearForeignClientSettingsSession). Migration (schema 3 to 4, domain/config/migrations.ts) moves the old single account into a one-entry list; a sibling impure step in configManager.ts (migrateAccountStore, next to the existing migrateLegacyAccount) re-keys the v1 secret store under that account's playerUid so the session survives the upgrade, not just the account record. Both config.json and the secret store back up their pre-migration bytes once, to a rolling .bak file, before being overwritten, on the industry-standard "never destroy the source until the new path is proven" principle: a bug in either migration then costs nothing, since the original is still sitting right next to it. UI: SessionButton becomes a Listbox (matching LanguagesMenu's existing pattern) when at least one account is saved, showing the active player's name, every saved account, "Add another account", and "Remove {name}" for the active one. The logged-out state (no accounts) is unchanged. Deferred, all additive later: importing an account from an existing installation's clientsettings.json, a stored-session-missing indicator, per-installation account binding, and the other locales (en-US is the only one the repo's own i18n-parity check requires staying in sync). --- src/config/configManager.ts | 93 +++++- src/domain/account/clientSettings.ts | 59 ++++ src/domain/account/credentials.ts | 37 +++ src/domain/config/defaults.ts | 3 +- src/domain/config/migrations.ts | 34 ++- src/global.d.ts | 16 +- src/ipc/accountStore.ts | 157 ++++++++-- src/ipc/handlers/accountHandlers.ts | 11 +- src/ipc/handlers/gameHandlers.ts | 45 ++- src/ipc/ipcChannels.ts | 2 +- src/preload/index.ts | 2 +- src/preload/preload.d.ts | 3 +- .../src/components/ui/SessionButton.tsx | 152 ++++++++-- .../src/features/account/adapters/account.ts | 6 +- .../config/contexts/ConfigContext.tsx | 44 ++- .../features/config/contexts/configReducer.ts | 52 +++- src/renderer/src/locales/en-US.json | 12 +- tests/domain/account/clientSettings.test.ts | 118 +++++++- tests/domain/account/credentials.test.ts | 51 +++- tests/domain/config/migrations.test.ts | 46 ++- tests/ipc/accountHandlers.test.ts | 43 ++- tests/ipc/accountStore.test.ts | 281 ++++++++++++++---- tests/ipc/configHandlers.test.ts | 3 +- tests/ipc/configManager.test.ts | 184 +++++++++++- tests/ipc/gameHandlers.test.ts | 131 +++++++- tests/ipc/modsHandlers.test.ts | 3 +- tests/ipc/pathsHandlers.test.ts | 3 +- tests/ipc/pathsHandlersWin32.test.ts | 10 +- tests/renderer-dom/helpers/windowApi.ts | 5 +- .../sessionButtonSwitchAccount.test.tsx | 105 +++++++ .../renderer-dom/shellSessionButton.test.tsx | 19 +- tests/renderer/configReducer.test.ts | 75 ++++- 32 files changed, 1586 insertions(+), 219 deletions(-) create mode 100644 tests/renderer-dom/sessionButtonSwitchAccount.test.tsx diff --git a/src/config/configManager.ts b/src/config/configManager.ts index 716c10b6..b218daae 100644 --- a/src/config/configManager.ts +++ b/src/config/configManager.ts @@ -3,7 +3,7 @@ import fse from "fs-extra" import { join } from "node:path" import { logMessage } from "@src/utils/logManager" import { parseLegacyAccount, toPublicAccount } from "@domain/account/credentials" -import { saveAccountSecrets } from "@src/ipc/accountStore" +import { adoptLegacySingleAccountSecrets, saveAccountSecrets } from "@src/ipc/accountStore" import { isRecord } from "@src/ipc/validation" import { clampConfigSchema, CURRENT_CONFIG_SCHEMA, migrateConfigDocument } from "@domain/config/migrations" import { normalizeBackgroundId } from "@domain/backgrounds" @@ -108,11 +108,13 @@ export async function getConfig(): Promise { if (configCache) return normalizeConfig(configCache) const config = await fse.readJSON(configPath, "utf-8") const hadLegacyAccountSecrets = await migrateLegacyAccount(config) + const reKeyedAccountStore = await migrateAccountStore(config) const migration = migrateConfigDocument(config) logConfigMigration(migration) + if (migration.applied.length > 0) await backupConfigBeforeMigration() const ensuredConfig = normalizeConfig(migration.doc) configCache = ensuredConfig - if (hadLegacyAccountSecrets || migration.applied.length > 0) await saveConfig(ensuredConfig) + if (hadLegacyAccountSecrets || reKeyedAccountStore || migration.applied.length > 0) await saveConfig(ensuredConfig) return ensuredConfig } catch (err) { logMessage("error", `[back] [config] [config/configManager.ts] [getConfig] Error getting config at ${configPath}. Using default config.`) @@ -181,7 +183,7 @@ async function migrateLegacyAccount(config: unknown): Promise { if (legacyAccount) { try { - await saveAccountSecrets(legacyAccount.secrets) + await saveAccountSecrets(legacyAccount.publicAccount.playerUid, legacyAccount.secrets) } catch { logMessage("warn", "[back] [config] [configManager.ts] Legacy account credentials were not migrated to secure storage.") } @@ -192,6 +194,63 @@ async function migrateLegacyAccount(config: unknown): Promise { return true } +/** + * Re-keys a single-account secret store under the account it belongs to. + * + * The v1 store held one `AccountSecrets` with no account attached to it; the + * v2 store (see `src/ipc/accountStore.ts`) holds entries keyed by + * `playerUid`. The only place that missing key exists is + * `config.account.playerUid`, which is why this runs here, reading the raw + * pre-pipeline document exactly as {@link migrateLegacyAccount} does, and not + * inside `accountStore.ts` itself: the store cannot name its own contents. + * Kept out of the schema pipeline for the same reason `migrateLegacyAccount` + * is: a side effect the pure domain layer is not allowed to have. + * + * A config with no readable account, or a v1 store that has already been + * re-keyed (or never existed), makes this a safe no-op: `false` either way, + * same as `migrateLegacyAccount` when there is nothing to do. + */ +async function migrateAccountStore(config: unknown): Promise { + if (!isRecord(config) || !isRecord(config.account)) return false + const uid = config.account.playerUid + if (typeof uid !== "string" || uid.length === 0) return false + + try { + return await adoptLegacySingleAccountSecrets(uid) + } catch { + logMessage("warn", "[back] [config] [configManager.ts] The stored account session was not carried into the multi-account store.") + return false + } +} + +function getConfigBackupPath(): string { + return join(app.getPath("userData"), "config.pre-migration.bak.json") +} + +/** + * Copies `config.json` exactly as it sits on disk, before a schema migration + * overwrites it with the reshaped document. + * + * One rolling backup, not one per migration: this is a local single-writer + * desktop file, not a fleet of servers, so "the config as it was right before + * the most recent migration" is the useful recovery point, not a full + * history. `overwrite: false` with `errorOnExist: false` makes the first + * migration's backup the one that survives; a later migration on an already- + * backed-up config leaves that earlier, more original snapshot alone. Best + * effort: a failed backup logs and does not stop the migration, since the + * live config is still correct either way and refusing to proceed over a + * backup that could not be written would trade a small safety net for a + * launcher that will not start. + */ +async function backupConfigBeforeMigration(): Promise { + try { + await fse.copy(configPath, getConfigBackupPath(), { overwrite: false, errorOnExist: false }) + } catch (err) { + logMessage("warn", "[back] [config] [configManager.ts] Could not back up config.json before migrating it.") + logMessage("debug", `[back] [config] [configManager.ts] ${err}`) + } +} + function asString(value: unknown, fallback: string, maxLength = 4_096): string { return typeof value === "string" && value.length <= maxLength && !value.includes("\0") ? value : fallback } @@ -267,6 +326,23 @@ function normalizeIcon(value: unknown): IconType | null { return icon.id && icon.name && icon.icon.toLowerCase().endsWith(".png") ? icon : null } +/** Ceiling on saved accounts, the same shape as the 1,000-entry caps above: generous for the real use case, not a promise to scale past it. */ +const MAX_STORED_ACCOUNTS = 50 + +/** Reads the accounts list, dropping anything unreadable and deduplicating by `playerUid`. */ +function normalizeAccounts(value: unknown): AccountPublicType[] { + const seen = new Set() + return (Array.isArray(value) ? value : []) + .map(toPublicAccount) + .filter((account): account is AccountPublicType => account !== null) + .filter((account) => { + if (seen.has(account.playerUid)) return false + seen.add(account.playerUid) + return true + }) + .slice(0, MAX_STORED_ACCOUNTS) +} + export function normalizeConfig(config: unknown): ConfigType { const rawConfig = (isRecord(config) ? config : {}) as Partial const rawWindow = (isRecord(rawConfig.window) ? rawConfig.window : {}) as Partial @@ -285,6 +361,8 @@ export function normalizeConfig(config: unknown): ConfigType { .filter((icon): icon is IconType => icon !== null) .slice(0, 1_000) + const accounts = normalizeAccounts(rawConfig.accounts) + const fixedConfig: ConfigType = { schemaVersion: clampConfigSchema(rawConfig.schemaVersion), lastUsedInstallation: rawConfig.lastUsedInstallation === null ? null : asString(rawConfig.lastUsedInstallation, defaultConfig.lastUsedInstallation ?? "", 128) || null, @@ -298,7 +376,14 @@ export function normalizeConfig(config: unknown): ConfigType { y: Math.trunc(asNumber(rawWindow.y, defaultConfig.window.y, -100_000, 100_000)), maximized: asBoolean(rawWindow.maximized, defaultConfig.window.maximized) }, - account: toPublicAccount(rawConfig.account), + accounts, + // An id naming nobody falls back to the first saved account rather than to null: a + // household that lost its choice (a dangling id, or a config hand-edited down to one + // fewer account) should land on someone, not on "no account selected". Every reader + // downstream (EXECUTE_GAME, the renderer contexts) can then do a plain lookup with no + // fallback branch of its own, because this is the one place the invariant is enforced: + // activeAccountId either names an entry in accounts, or is null when accounts is empty. + activeAccountId: accounts.some((account) => account.playerUid === rawConfig.activeAccountId) ? (rawConfig.activeAccountId as string) : (accounts[0]?.playerUid ?? null), installations, gameVersions, favMods: Array.isArray(rawConfig.favMods) ? rawConfig.favMods.filter((modId): modId is number => typeof modId === "number" && Number.isSafeInteger(modId)).slice(0, 10_000) : defaultConfig.favMods, diff --git a/src/domain/account/clientSettings.ts b/src/domain/account/clientSettings.ts index 385bcf06..330b895c 100644 --- a/src/domain/account/clientSettings.ts +++ b/src/domain/account/clientSettings.ts @@ -199,3 +199,62 @@ export async function writeClientSettingsSession(ports: WriteClientSettingsSessi return written.ok ? { outcome: "written" } : { outcome: "write-failed" } } + +/** + * The document with the eight session keys removed and everything else intact. + * + * The mirror image of {@link mergeSessionIntoClientSettings}'s read-modify-write + * discipline: every other setting the file carries survives untouched. + */ +export function removeSessionFromClientSettings(existingDocument: unknown): Record { + const document = existingDocument && typeof existingDocument === "object" && !Array.isArray(existingDocument) ? (existingDocument as Record) : {} + const stringSettings = { ...existingStringSettings(existingDocument) } + + for (const key of ["mptoken", "sessionkey", "sessionsignature", "useremail", "entitlements", "playeruid", "playername", "hostgameserver"]) delete stringSettings[key] + + return { ...document, [STRING_SETTINGS_SECTION]: stringSettings } +} + +export type ClearClientSettingsSessionResult = + | { outcome: "cleared" } + // Covers both a file with no session in it, and one that already holds our own: neither can + // launch anyone into the wrong identity, so neither is touched. + | { outcome: "not-foreign" } + | { outcome: "unreadable-settings" } + | { outcome: "write-failed" } + +export interface ClearForeignClientSettingsSessionInput { + /** Full path of the settings file. The caller resolves it, so path policy stays on the host side. */ + settingsPath: string + /** `playerUid` of the account the launcher is about to launch as. */ + playerUid: string +} + +/** + * Removes a session belonging to a DIFFERENT player from the settings file, + * and only that. Never writes our own account's session: that is + * {@link writeClientSettingsSession}'s job, and this function only runs when + * the caller has no secrets to write with (see gameHandlers.ts's EXECUTE_GAME). + * + * ## Why this exists + * + * With one saved account, a settings file the launcher could not update was + * harmless: whatever session it already held was that same account's own. + * With more than one, the file can hold a HOUSEMATE's session, since the game + * writes it there directly on its own successful login. Launching without + * clearing it would start the game already signed in as somebody else. This + * is narrow on purpose: it only fires when the file demonstrably holds a + * different player's `playeruid`, so it can never touch a file that already + * shows nobody, or shows us. + */ +export async function clearForeignClientSettingsSession(ports: WriteClientSettingsSessionPorts, input: ClearForeignClientSettingsSessionInput): Promise { + const existing = await ports.jsonFile.read(input.settingsPath) + if (!existing.ok) return { outcome: "unreadable-settings" } + + const stringSettings = existingStringSettings(existing.document) + const foreignUid = typeof stringSettings.playeruid === "string" ? stringSettings.playeruid : null + if (foreignUid === null || foreignUid === input.playerUid) return { outcome: "not-foreign" } + + const written = await ports.jsonFile.write(input.settingsPath, removeSessionFromClientSettings(existing.document)) + return written.ok ? { outcome: "cleared" } : { outcome: "write-failed" } +} diff --git a/src/domain/account/credentials.ts b/src/domain/account/credentials.ts index 7ff68a96..462e5872 100644 --- a/src/domain/account/credentials.ts +++ b/src/domain/account/credentials.ts @@ -195,6 +195,43 @@ export function parseStoredSecrets(value: unknown): AccountSecrets | null { } } +/** One id-keyed entry as the multi-account encrypted store holds it. */ +export type StoredAccountSecretsEntry = { id: string; secrets: AccountSecrets } + +/** + * Reads the multi-account payload out of the decrypted store: `{ accounts: [{ + * id, secrets }, ...] }`. + * + * An array of records rather than an object keyed by id on purpose: an object + * keyed by a service-supplied string invites a `__proto__`/`constructor` key, + * and an array has no such surface. An entry that does not parse is dropped + * rather than failing the whole read, the same reasoning `parseStoredSecrets` + * already gives for a single account: an unreadable entry logs that one + * account out, it does not cost every other saved account its session. On a + * duplicate id, the first entry wins. + */ +export function parseStoredSecretsById(value: unknown): Map { + const result = new Map() + if (!isRecord(value) || !Array.isArray(value.accounts)) return result + + for (const entry of value.accounts) { + if (!isRecord(entry)) continue + + let id: string + try { + id = accountString(entry.id, "account id", 256) + } catch { + continue + } + if (result.has(id)) continue + + const secrets = parseStoredSecrets(entry.secrets) + if (secrets) result.set(id, secrets) + } + + return result +} + /** * Reads the renderer-visible half out of stored config, dropping anything else * the object carried. A config file that still holds legacy session fields diff --git a/src/domain/config/defaults.ts b/src/domain/config/defaults.ts index 2d64824e..4f8f2236 100644 --- a/src/domain/config/defaults.ts +++ b/src/domain/config/defaults.ts @@ -19,7 +19,8 @@ export const DEFAULT_CONFIG_BASE: Omit 0 ? account.playerUid : null + migrated.accounts = uid ? [account] : [] + migrated.activeAccountId = uid + return migrated + } +} + /** Every migration the launcher knows, lowest schema first. */ -export const CONFIG_MIGRATIONS: readonly ConfigMigration[] = [floatMarkerToIntegerSchema, stampLinkedOnExternalVersions] +export const CONFIG_MIGRATIONS: readonly ConfigMigration[] = [floatMarkerToIntegerSchema, stampLinkedOnExternalVersions, singleAccountToAccountList] function byFromSchema(migrations: readonly ConfigMigration[]): Map { return new Map(migrations.map((migration) => [migration.fromSchema, migration])) diff --git a/src/global.d.ts b/src/global.d.ts index b554ebb4..62af5aab 100644 --- a/src/global.d.ts +++ b/src/global.d.ts @@ -126,7 +126,21 @@ declare global { type ConfigType = BasicConfigType & { window: WindowType - account: AccountPublicType | null + /** + * Every saved account, public half only. Keyed by `playerUid` (no separate + * `id` field: two identifiers that must always agree is a bug waiting to + * happen, and `playerUid` is already what clientSettings.ts trusts as an + * account's identity). The encrypted store holds the matching secrets + * under the same key. Deduplicated and capped by normalizeConfig. + */ + accounts: AccountPublicType[] + /** + * `playerUid` of the account the next game launch writes into + * `clientsettings.json`, or null when none is saved. normalizeConfig + * guarantees this either names an entry in `accounts` or is null, so + * every reader can look it up with a plain `find` and no fallback branch. + */ + activeAccountId: string | null installations: InstallationType[] gameVersions: GameVersionType[] customIcons: IconType[] diff --git a/src/ipc/accountStore.ts b/src/ipc/accountStore.ts index b0b1347c..ee32e853 100644 --- a/src/ipc/accountStore.ts +++ b/src/ipc/accountStore.ts @@ -2,29 +2,90 @@ import { app, safeStorage } from "electron" import fse from "fs-extra" import { join } from "node:path" -import { parseStoredSecrets, type AccountSecrets } from "@domain/account/credentials" +import { parseStoredSecrets, parseStoredSecretsById, type AccountSecrets, type StoredAccountSecretsEntry } from "@domain/account/credentials" type EncryptedAccountFile = { + version: 2 + ciphertext: string +} + +/** Decrypted shape of `EncryptedAccountFile.ciphertext`, one entry per saved account. */ +type StoredAccountsPayload = { + accounts: StoredAccountSecretsEntry[] +} + +/** Pre-multi-account store shape, kept only so a v1 file can be re-keyed once. */ +type LegacyEncryptedAccountFile = { version: 1 ciphertext: string } -const ACCOUNT_STORE_VERSION = 1 -let cachedSecrets: AccountSecrets | null | undefined +const ACCOUNT_STORE_VERSION = 2 +const LEGACY_ACCOUNT_STORE_VERSION = 1 + +/** + * `undefined` means the file has not been read this process. A read that + * cannot be decrypted, or finds nothing there, caches an empty map rather + * than `undefined`, so a later call answers from memory instead of hitting + * the disk again. A miss on one account cannot poison a later hit on another, + * because the cache holds the whole file's contents, not one account's. + */ +let cachedAccounts: Map | undefined function getAccountStorePath(): string { return join(app.getPath("userData"), "account-secrets.json") } +/** + * Where a pre-migration copy of the store is kept, so a re-key gone wrong or a + * downgrade to a build that only understands one account never destroys the + * original bytes. One rolling file, not one per migration: this is a local + * single-writer desktop store, not a fleet of servers, so a single "as it was + * right before the multi-account migration" snapshot is the useful recovery + * point, and it is only ever written once, by {@link adoptLegacySingleAccountSecrets}. + */ +function getAccountStoreBackupPath(): string { + return join(app.getPath("userData"), "account-secrets.pre-migration.bak.json") +} + function assertSecureStorage(): void { if (!safeStorage.isEncryptionAvailable()) throw new Error("Secure account storage is unavailable") if (process.platform === "linux" && safeStorage.getSelectedStorageBackend() === "basic_text") throw new Error("A system password store is required for account storage") } -export async function saveAccountSecrets(secrets: AccountSecrets): Promise { +async function readAccounts(): Promise> { + if (cachedAccounts !== undefined) return cachedAccounts + + try { + assertSecureStorage() + const stored = (await fse.readJSON(getAccountStorePath(), "utf8")) as Partial + if (stored.version !== ACCOUNT_STORE_VERSION || typeof stored.ciphertext !== "string") throw new Error("Invalid account store") + + const decrypted = safeStorage.decryptString(Buffer.from(stored.ciphertext, "base64")) + cachedAccounts = parseStoredSecretsById(JSON.parse(decrypted)) + } catch { + cachedAccounts = new Map() + } + + return cachedAccounts +} + +/** + * Encrypts and writes the whole account map, atomically. + * + * If the file on disk exists but cannot be decrypted, this still overwrites + * it: the old bytes are already unrecoverable, so refusing to write would + * only leave the launcher permanently unable to save any account. At + * multi-account scale this means one undecryptable file now costs every + * saved account's session instead of the one account the old single-account + * store held, which is a real change in blast radius worth this comment, + * even though it is not a new kind of risk. + */ +async function writeAccounts(accounts: Map): Promise { assertSecureStorage() - const encrypted = safeStorage.encryptString(JSON.stringify(secrets)).toString("base64") + const payload: StoredAccountsPayload = { accounts: Array.from(accounts, ([id, secrets]) => ({ id, secrets })) } + const encrypted = safeStorage.encryptString(JSON.stringify(payload)).toString("base64") const storePath = getAccountStorePath() const temporaryPath = `${storePath}.${process.pid}.${Date.now()}.tmp` const contents: EncryptedAccountFile = { version: ACCOUNT_STORE_VERSION, ciphertext: encrypted } @@ -34,35 +95,89 @@ export async function saveAccountSecrets(secrets: AccountSecrets): Promise await fse.chmod(temporaryPath, 0o600).catch(() => undefined) await fse.move(temporaryPath, storePath, { overwrite: true }) await fse.chmod(storePath, 0o600).catch(() => undefined) - cachedSecrets = secrets + cachedAccounts = accounts } finally { await fse.remove(temporaryPath).catch(() => undefined) } } -export async function getAccountSecrets(): Promise { - if (cachedSecrets !== undefined) return cachedSecrets +/** Saves or replaces one account's secrets. Logging into an already-saved account overwrites its entry: a session refresh, not a duplicate. */ +export async function saveAccountSecrets(accountId: string, secrets: AccountSecrets): Promise { + const accounts = new Map(await readAccounts()) + accounts.set(accountId, secrets) + await writeAccounts(accounts) +} - try { - assertSecureStorage() - const stored = (await fse.readJSON(getAccountStorePath(), "utf8")) as Partial - if (stored.version !== ACCOUNT_STORE_VERSION || typeof stored.ciphertext !== "string") throw new Error("Invalid account store") +/** Reads one account's secrets, or null when nothing is stored for it. */ +export async function getAccountSecrets(accountId: string): Promise { + return (await readAccounts()).get(accountId) ?? null +} - const decrypted = safeStorage.decryptString(Buffer.from(stored.ciphertext, "base64")) - cachedSecrets = parseStoredSecrets(JSON.parse(decrypted)) +/** + * Drops one account's secrets. `true` when there was nothing to remove, + * matching the old single-account store's `clearAccountSecrets`: a caller + * asking to remove an account that already has no stored session is not a + * failure. When the map empties, the file itself is removed rather than left + * behind holding an empty list, so a never-used store and a fully-logged-out + * one are byte-for-byte the same on disk. + */ +export async function removeAccountSecrets(accountId: string): Promise { + const accounts = new Map(await readAccounts()) + if (!accounts.delete(accountId)) return true + + try { + if (accounts.size === 0) { + await fse.remove(getAccountStorePath()) + cachedAccounts = new Map() + } else { + await writeAccounts(accounts) + } + return true } catch { - cachedSecrets = null + return false } - - return cachedSecrets } -export async function clearAccountSecrets(): Promise { +/** + * Re-keys a pre-multi-account store under the one account it used to hold. + * + * A no-op, returning `false`, on anything that is not exactly a v1 file: + * already migrated, never existed, or unreadable. That makes this safe to + * call on every launch, the same way the schema-3-to-4 config migration it + * runs alongside is: idempotent by construction, not by a separate "already + * ran" flag. + * + * Takes a backup of the original v1 file before overwriting it, once, so the + * pre-migration bytes are never destroyed. See {@link getAccountStoreBackupPath}. + */ +export async function adoptLegacySingleAccountSecrets(accountId: string): Promise { + const storePath = getAccountStorePath() + + let stored: Partial try { - await fse.remove(getAccountStorePath()) - cachedSecrets = null - return true + stored = (await fse.readJSON(storePath, "utf8")) as Partial } catch { return false } + if (stored.version !== LEGACY_ACCOUNT_STORE_VERSION || typeof stored.ciphertext !== "string") return false + + assertSecureStorage() + let secrets: AccountSecrets | null + try { + const decrypted = safeStorage.decryptString(Buffer.from(stored.ciphertext, "base64")) + secrets = parseStoredSecrets(JSON.parse(decrypted)) + } catch { + return false + } + if (!secrets) return false + + await fse.copy(storePath, getAccountStoreBackupPath(), { overwrite: false, errorOnExist: false }) + + await writeAccounts(new Map([[accountId, secrets]])) + return true +} + +/** Every account id the store currently holds secrets for. */ +export async function listStoredAccountIds(): Promise { + return Array.from((await readAccounts()).keys()) } diff --git a/src/ipc/handlers/accountHandlers.ts b/src/ipc/handlers/accountHandlers.ts index 67ffb3be..9298bb89 100644 --- a/src/ipc/handlers/accountHandlers.ts +++ b/src/ipc/handlers/accountHandlers.ts @@ -8,7 +8,7 @@ import { IPC_CHANNELS } from "@src/ipc/ipcChannels" import { assertTrustedIpcSender } from "@src/ipc/ipcSecurity" import { requestBoundedTextViaNode } from "@src/ipc/network" import { assertString, MAX_LOGIN_RESPONSE_BYTES } from "@src/ipc/validation" -import { clearAccountSecrets, saveAccountSecrets } from "@src/ipc/accountStore" +import { removeAccountSecrets, saveAccountSecrets } from "@src/ipc/accountStore" import { getErrorMessage, logMessage } from "@src/utils/logManager" const LOGIN_URL = new URL("https://auth3.vintagestory.at/v2/gamelogin") @@ -55,7 +55,10 @@ async function requestLoginPass(email: string, password: string, twoFactorCode?: async function settle(verdict: LoginVerdict): Promise { switch (verdict.status) { case "success": - await saveAccountSecrets(verdict.credentials.secrets) + // Keyed by the uid this same response just carried, so the store can never + // disagree with what the renderer is told. Logging into an account already + // saved overwrites its entry in place: a session refresh, not a duplicate. + await saveAccountSecrets(verdict.credentials.publicAccount.playerUid, verdict.credentials.secrets) return { status: "success", account: verdict.credentials.publicAccount } case "needs-two-factor": return needsTwoFactorResult() @@ -103,7 +106,7 @@ ipcMain.handle(IPC_CHANNELS.ACCOUNT_MANAGER.LOGIN, async (event, email: unknown, } }) -ipcMain.handle(IPC_CHANNELS.ACCOUNT_MANAGER.LOGOUT, async (event): Promise => { +ipcMain.handle(IPC_CHANNELS.ACCOUNT_MANAGER.REMOVE_ACCOUNT, async (event, accountId: unknown): Promise => { assertTrustedIpcSender(event) - return await clearAccountSecrets() + return await removeAccountSecrets(assertString(accountId, "account id", 256)) }) diff --git a/src/ipc/handlers/gameHandlers.ts b/src/ipc/handlers/gameHandlers.ts index 9fcdb53d..f4f86529 100644 --- a/src/ipc/handlers/gameHandlers.ts +++ b/src/ipc/handlers/gameHandlers.ts @@ -12,7 +12,7 @@ import { getAccountSecrets, saveAccountSecrets } from "@src/ipc/accountStore" import { getConfig } from "@src/config/configManager" import { detectInstalledGameVersion } from "@domain/versions/detect" import { buildGameLaunchPlan } from "@domain/versions/launch" -import { CLIENT_SETTINGS_FILE_NAME, writeClientSettingsSession } from "@domain/account/clientSettings" +import { CLIENT_SETTINGS_FILE_NAME, clearForeignClientSettingsSession, writeClientSettingsSession } from "@domain/account/clientSettings" import { gameProcessOutcomeToResult, invalidExecutableResult, @@ -86,9 +86,9 @@ function realJsonFile(): JsonFile { * Nothing here goes near the key itself. What gets logged is that an adoption * happened, never what was adopted. */ -async function adoptRefreshedSession(secrets: AccountSecrets): Promise { +async function adoptRefreshedSession(accountId: string, secrets: AccountSecrets): Promise { try { - await saveAccountSecrets(secrets) + await saveAccountSecrets(accountId, secrets) logMessage("info", `[back] [ipc] [ipc/handlers/gameHandlers.ts] [EXECUTE_GAME] The game had already refreshed this account's session. Adopted it instead of overwriting it.`) } catch (err) { logMessage("error", `[back] [ipc] [ipc/handlers/gameHandlers.ts] [EXECUTE_GAME] Could not store the session the game refreshed. Launching anyway.`) @@ -154,8 +154,9 @@ ipcMain.handle(IPC_CHANNELS.GAME_MANAGER.EXECUTE_GAME, async (event, version: un const safeInstallation = validateGameInstallation(installation) safeVersion.path = await assertManagedPath(safeVersion.path, "game version path") safeInstallation.path = await assertManagedPath(safeInstallation.path, "installation path") - const account = (await getConfig()).account - const accountSecrets = account ? await getAccountSecrets() : null + const config = await getConfig() + const account = config.accounts.find((candidate) => candidate.playerUid === config.activeAccountId) ?? null + const accountSecrets = account ? await getAccountSecrets(account.playerUid) : null logMessage("info", `[back] [ipc] [ipc/handlers/gameHandlers.ts] [EXECUTE_GAME] Trying to run Vintage Story ${safeVersion.version}.`) let processEnv: Record @@ -236,7 +237,7 @@ ipcMain.handle(IPC_CHANNELS.GAME_MANAGER.EXECUTE_GAME, async (event, version: un case "written": break case "adopted": - await adoptRefreshedSession(written.secrets) + await adoptRefreshedSession(account.playerUid, written.secrets) break case "unreadable-settings": case "write-failed": @@ -244,6 +245,38 @@ ipcMain.handle(IPC_CHANNELS.GAME_MANAGER.EXECUTE_GAME, async (event, version: un logMessage("debug", `[back] [ipc] [ipc/handlers/gameHandlers.ts] [EXECUTE_GAME] Error setting login session keys: ${written.outcome}.`) return sessionWriteFailedResult() } + } else if (account && !accountSecrets) { + // The active account has no usable session (a locked keyring, or a store this build cannot + // read). With one saved account that was always harmless: whatever stale session the file + // already held was that same account's own. With more than one it can be a housemate's, + // since the game writes their session there directly on their own successful login, and + // launching without checking would start the game already signed in as somebody else. + // Narrow on purpose: this only clears the file when it demonstrably holds a DIFFERENT + // player's session, never our own and never an empty one, and it never writes a session of + // its own; that stays writeClientSettingsSession's job above. + let settingsPath: string + try { + settingsPath = await assertManagedPath(join(safeInstallation.path, CLIENT_SETTINGS_FILE_NAME), "client settings", { allowMissing: true }) + } catch (err) { + logMessage("error", `[back] [ipc] [ipc/handlers/gameHandlers.ts] [EXECUTE_GAME] Error checking for another player's session keys.`) + logMessage("debug", `[back] [ipc] [ipc/handlers/gameHandlers.ts] [EXECUTE_GAME] Refused the client settings path: ${getErrorMessage(err)}`) + return sessionWriteFailedResult() + } + + const cleared = await clearForeignClientSettingsSession({ jsonFile: realJsonFile() }, { settingsPath, playerUid: account.playerUid }) + + switch (cleared.outcome) { + case "cleared": + logMessage("info", `[back] [ipc] [ipc/handlers/gameHandlers.ts] [EXECUTE_GAME] Cleared another player's session before launching without one of our own.`) + break + case "not-foreign": + break + case "unreadable-settings": + case "write-failed": + logMessage("error", `[back] [ipc] [ipc/handlers/gameHandlers.ts] [EXECUTE_GAME] Could not confirm this installation is not still signed in as another player.`) + logMessage("debug", `[back] [ipc] [ipc/handlers/gameHandlers.ts] [EXECUTE_GAME] ${cleared.outcome}.`) + return sessionWriteFailedResult() + } } logMessage("info", "[back] [ipc] [ipc/handlers/gameHandlers.ts] [EXECUTE_GAME] Running Vintagestory with a validated executable.") diff --git a/src/ipc/ipcChannels.ts b/src/ipc/ipcChannels.ts index c6fb08ad..e5eb4fa3 100644 --- a/src/ipc/ipcChannels.ts +++ b/src/ipc/ipcChannels.ts @@ -60,6 +60,6 @@ export const IPC_CHANNELS = { }, ACCOUNT_MANAGER: { LOGIN: "account-login", - LOGOUT: "account-logout" + REMOVE_ACCOUNT: "account-remove" } } diff --git a/src/preload/index.ts b/src/preload/index.ts index 309fa716..47993927 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -81,7 +81,7 @@ const api: BridgeAPI = { }, accountManager: { login: (email: string, password: string, twoFactorCode?: string): Promise => ipcRenderer.invoke(IPC_CHANNELS.ACCOUNT_MANAGER.LOGIN, email, password, twoFactorCode), - logout: (): Promise => ipcRenderer.invoke(IPC_CHANNELS.ACCOUNT_MANAGER.LOGOUT) + removeAccount: (accountId: string): Promise => ipcRenderer.invoke(IPC_CHANNELS.ACCOUNT_MANAGER.REMOVE_ACCOUNT, accountId) } } diff --git a/src/preload/preload.d.ts b/src/preload/preload.d.ts index 66a1b804..0b78b56b 100644 --- a/src/preload/preload.d.ts +++ b/src/preload/preload.d.ts @@ -86,7 +86,8 @@ declare global { } accountManager: { login: (email: string, password: string, twoFactorCode?: string) => Promise - logout: () => Promise + /** Drops one saved account's secrets, by its `playerUid`. */ + removeAccount: (accountId: string) => Promise } } diff --git a/src/renderer/src/components/ui/SessionButton.tsx b/src/renderer/src/components/ui/SessionButton.tsx index ff2040c4..ea700d65 100644 --- a/src/renderer/src/components/ui/SessionButton.tsx +++ b/src/renderer/src/components/ui/SessionButton.tsx @@ -1,10 +1,15 @@ import { useState } from "react" import { useTranslation } from "react-i18next" -import { PiFloppyDiskBackDuotone, PiTrashDuotone, PiUserDuotone, PiXCircleDuotone } from "react-icons/pi" +import { AnimatePresence, motion } from "motion/react" +import clsx from "clsx" +import { PiCaretDownDuotone, PiFloppyDiskBackDuotone, PiTrashDuotone, PiUserDuotone, PiUserPlusDuotone, PiXCircleDuotone } from "react-icons/pi" + +import { Listbox, ListboxButton, ListboxOptions, ListboxOption } from "@headlessui/react" import { useNotificationsContext } from "@renderer/contexts/NotificationsContext" -import { CONFIG_ACTIONS, useAccount, useConfigDispatch } from "@renderer/features/config/contexts/ConfigContext" -import { loginToAccount as login, logoutOfAccount as logout } from "@renderer/features/account/adapters/account" +import { CONFIG_ACTIONS, useAccountList, useConfigDispatch } from "@renderer/features/config/contexts/ConfigContext" +import { loginToAccount as login, removeAccount as removeAccountSecrets } from "@renderer/features/account/adapters/account" +import { DROPDOWN_MENU_ITEM_VARIANTS, DROPDOWN_MENU_WRAPPER_VARIANTS } from "@renderer/utils/animateVariants" import { ButtonsWrapper, @@ -23,9 +28,15 @@ import { } from "@renderer/components/ui/FormComponents" import PopupDialogPanel from "@renderer/components/ui/PopupDialogPanel" +// Sentinel option values a real playerUid cannot collide with in practice: `handleSelect` checks +// list membership first regardless, so even a collision would resolve to the switch, never these. +const ADD_ACCOUNT_OPTION = "__add-account__" +const REMOVE_ACCOUNT_OPTION = "__remove-account__" + function SessionButton(): JSX.Element { const { t } = useTranslation() - const account = useAccount() + const { accounts, activeAccountId } = useAccountList() + const activeAccount = accounts.find((account) => account.playerUid === activeAccountId) ?? null const configDispatch = useConfigDispatch() const { addNotification } = useNotificationsContext() @@ -36,7 +47,7 @@ function SessionButton(): JSX.Element { const [loggingIn, setLoggingIn] = useState(false) const [logInOpen, setLogInOpen] = useState(false) - const [logOutOpen, setLogOutOpen] = useState(false) + const [removeOpen, setRemoveOpen] = useState(false) async function handleLogin(): Promise { setLoggingIn(true) @@ -65,39 +76,114 @@ function SessionButton(): JSX.Element { } } - async function handleLogout(): Promise { - const loggedOut = await logout() - if (!loggedOut) return addNotification(t("features.config.logoutFailed"), "error") - configDispatch({ type: CONFIG_ACTIONS.SET_ACCOUNT, payload: null }) - addNotification(t("features.config.loggedout"), "success") - setLogOutOpen(false) + async function handleRemove(): Promise { + if (!activeAccount) return + const removed = await removeAccountSecrets(activeAccount.playerUid) + if (!removed) return addNotification(t("features.config.removeAccountFailed"), "error") + + // The IPC call lands first: dropping the config entry before the secrets are confirmed gone + // would leave a secret in the store with no account naming it and no way back to it. + configDispatch({ type: CONFIG_ACTIONS.REMOVE_ACCOUNT, payload: { playerUid: activeAccount.playerUid } }) + addNotification(t("features.config.accountRemoved", { user: activeAccount.playerName }), "success") + setRemoveOpen(false) } async function saveLogin(newAccount: AccountPublicType): Promise { - configDispatch({ type: CONFIG_ACTIONS.SET_ACCOUNT, payload: newAccount }) + configDispatch({ type: CONFIG_ACTIONS.ADD_ACCOUNT, payload: newAccount }) addNotification(t("features.config.loggedin", { user: newAccount.playerName }), "success") setLoggingIn(false) setLogInOpen(false) } + function handleSelect(value: string): void { + if (accounts.some((account) => account.playerUid === value)) { + configDispatch({ type: CONFIG_ACTIONS.SET_ACTIVE_ACCOUNT, payload: value }) + return + } + if (value === ADD_ACCOUNT_OPTION) return setLogInOpen(true) + if (value === REMOVE_ACCOUNT_OPTION) setRemoveOpen(true) + } + return ( <> - { - if (!account) { - setLogInOpen(true) - } else { - setLogOutOpen(true) - } - }} - title={!account ? t("features.config.loginTitle") : t("features.config.logoutTitle")} - className="w-full h-8" - > - - -

{!account ? t("features.config.loginTitle") : account.playerName}

-
+ {accounts.length < 1 ? ( + setLogInOpen(true)} title={t("features.config.loginTitle")} className="w-full h-8"> + +

{t("features.config.loginTitle")}

+
+ ) : ( + + {({ open }) => ( + <> + +

+ + {activeAccount?.playerName ?? t("features.config.loginTitle")} +

+ +
+ + + {open && ( + + + {accounts.map((account) => ( + + {account.playerName} + {account.email} + + ))} + +
+ + + + {t("features.config.addAnotherAccount")} + + + {activeAccount && ( + + + {t("features.config.removeAccountTitle", { user: activeAccount.playerName })} + + )} + + + )} + + + )} + + )} setLogInOpen(false)}> @@ -174,20 +260,20 @@ function SessionButton(): JSX.Element { - setLogOutOpen(false)}> + setRemoveOpen(false)}> <> -

{t("features.config.areYouSureLogout")}

-

{t("features.config.loginoutNotReversible")}

+

{t("features.config.areYouSureRemoveAccount", { user: activeAccount?.playerName ?? "" })}

+

{t("features.config.removeAccountNotReversible")}

- setLogOutOpen(false)} type="success"> + setRemoveOpen(false)} type="success"> { e.stopPropagation() - handleLogout() + handleRemove() }} type="error" > diff --git a/src/renderer/src/features/account/adapters/account.ts b/src/renderer/src/features/account/adapters/account.ts index 75790fc9..3f8cb872 100644 --- a/src/renderer/src/features/account/adapters/account.ts +++ b/src/renderer/src/features/account/adapters/account.ts @@ -1,5 +1,5 @@ /** - * Wraps the preload-bridge `accountManager` calls SessionButton's login/logout flow needs. + * Wraps the preload-bridge `accountManager` calls SessionButton's login/switch/remove flow needs. * * Lives outside components/ui, where SessionButton.tsx lives, because nothing under * src/renderer/src/components may touch the preload bridge directly. @@ -8,6 +8,6 @@ export function loginToAccount(email: string, password: string, twoFactorCode?: return window.api.accountManager.login(email, password, twoFactorCode) } -export function logoutOfAccount(): Promise { - return window.api.accountManager.logout() +export function removeAccount(accountId: string): Promise { + return window.api.accountManager.removeAccount(accountId) } diff --git a/src/renderer/src/features/config/contexts/ConfigContext.tsx b/src/renderer/src/features/config/contexts/ConfigContext.tsx index 3fe542bf..b94966fe 100644 --- a/src/renderer/src/features/config/contexts/ConfigContext.tsx +++ b/src/renderer/src/features/config/contexts/ConfigContext.tsx @@ -39,6 +39,9 @@ const GameVersionsContext = createContext(null) // The account is wrapped: `null` is a legitimate value (logged out), so it // cannot double as the "no provider above me" sentinel the other contexts use. const AccountContext = createContext<{ account: AccountType | null } | null>(null) +// Wrapped for the same reason: activeAccountId is legitimately null with a non-empty list, so +// the wrapper (not the field) is what tells a missing provider apart from a real logged-out state. +const AccountListContext = createContext<{ accounts: AccountPublicType[]; activeAccountId: string | null } | null>(null) const SettingsContext = createContext(null) const FavModsContext = createContext(null) const SuspendedModUpdatesContext = createContext(null) @@ -122,7 +125,9 @@ const ConfigProvider = ({ children }: { children: React.ReactNode }): JSX.Elemen // The list slices are handed out as-is: the reducer never rebuilds an array // it did not change, so their identity already tracks their content. Only the // composed slices need memoising to stay stable across unrelated actions. - const account = useMemo(() => ({ account: config.account }), [config.account]) + const activeAccount = useMemo(() => config.accounts.find((candidate) => candidate.playerUid === config.activeAccountId) ?? null, [config.accounts, config.activeAccountId]) + const account = useMemo(() => ({ account: activeAccount }), [activeAccount]) + const accountList = useMemo(() => ({ accounts: config.accounts, activeAccountId: config.activeAccountId }), [config.accounts, config.activeAccountId]) const settings = useMemo( () => ({ @@ -156,15 +161,17 @@ const ConfigProvider = ({ children }: { children: React.ReactNode }): JSX.Elemen - - - - - {children} - - - - + + + + + + {children} + + + + + @@ -186,6 +193,9 @@ const useGameVersions = (): GameVersionType[] => requireProvider(useContext(Game const useAccount = (): AccountType | null => requireProvider(useContext(AccountContext), "useAccount").account +/** Every saved account, and which one is active. For a switcher; a single logged-in read stays `useAccount`. */ +const useAccountList = (): { accounts: AccountPublicType[]; activeAccountId: string | null } => requireProvider(useContext(AccountListContext), "useAccountList") + /** Folders, window geometry, schema version and the last used installation. */ const useSettingsConfig = (): ConfigSettingsType => requireProvider(useContext(SettingsContext), "useSettingsConfig") @@ -199,4 +209,16 @@ const useCustomIcons = (): IconType[] => requireProvider(useContext(CustomIconsC /** Ids of installations the player has already been told about mod updates for, this session. */ const useNotifiedModUpdates = (): string[] => requireProvider(useContext(NotifiedModUpdatesContext), "useNotifiedModUpdates") -export { ConfigProvider, useConfigDispatch, useInstallations, useGameVersions, useAccount, useSettingsConfig, useFavMods, useSuspendedModUpdates, useCustomIcons, useNotifiedModUpdates } +export { + ConfigProvider, + useConfigDispatch, + useInstallations, + useGameVersions, + useAccount, + useAccountList, + useSettingsConfig, + useFavMods, + useSuspendedModUpdates, + useCustomIcons, + useNotifiedModUpdates +} diff --git a/src/renderer/src/features/config/contexts/configReducer.ts b/src/renderer/src/features/config/contexts/configReducer.ts index a255d95c..20a2756a 100644 --- a/src/renderer/src/features/config/contexts/configReducer.ts +++ b/src/renderer/src/features/config/contexts/configReducer.ts @@ -8,7 +8,9 @@ export enum CONFIG_ACTIONS { SET_DEFAULT_INSTALLATIONS_FOLDER = "SET_DEFAULT_INSTALLATIONS_FOLDER", SET_DEFAULT_VERSIONS_FOLDER = "SET_DEFAULT_VERSIONS_FOLDER", SET_DEFAULT_BACKUPS_FOLDER = "SET_DEFAULT_BACKUPS_FOLDER", - SET_ACCOUNT = "SET_ACCOUNT", + ADD_ACCOUNT = "ADD_ACCOUNT", + REMOVE_ACCOUNT = "REMOVE_ACCOUNT", + SET_ACTIVE_ACCOUNT = "SET_ACTIVE_ACCOUNT", SET_BACKGROUND = "SET_BACKGROUND", SET_MODDB_VISIBILITY_ANSWER = "SET_MODDB_VISIBILITY_ANSWER", SET_RECEIVE_BETA_UPDATES = "SET_RECEIVE_BETA_UPDATES", @@ -62,9 +64,31 @@ export interface SetDefaultBackupsFolder { payload: string } -export interface SetAccount { - type: CONFIG_ACTIONS.SET_ACCOUNT - payload: AccountType | null +/** + * Saves a fresh login, or refreshes an already-saved account's session. + * + * Also chooses it: an account just proven by a successful login is the one + * the player wants to play as. The same `playerUid` twice replaces the entry + * in place rather than duplicating it, which is what a session refresh is. + */ +export interface AddAccount { + type: CONFIG_ACTIONS.ADD_ACCOUNT + payload: AccountPublicType +} + +/** + * Drops one saved account. If it was the active one, the first remaining + * account is promoted; an empty list leaves `activeAccountId` null. + */ +export interface RemoveAccount { + type: CONFIG_ACTIONS.REMOVE_ACCOUNT + payload: { playerUid: string } +} + +/** Chooses which saved account the next game launch writes into clientsettings.json. An id naming nobody is a no-op. */ +export interface SetActiveAccount { + type: CONFIG_ACTIONS.SET_ACTIVE_ACCOUNT + payload: string | null } /** @@ -236,7 +260,9 @@ export type ConfigAction = | SetDefaultInstllationsFolder | SetDefaultVersionsFolder | SetDefaultBackupsFolder - | SetAccount + | AddAccount + | RemoveAccount + | SetActiveAccount | SetBackground | SetModDbVisibilityAnswer | SetReceiveBetaUpdates @@ -276,8 +302,20 @@ export const configReducer = (config: ConfigType, action: ConfigAction): ConfigT return { ...config, defaultVersionsFolder: action.payload } case CONFIG_ACTIONS.SET_DEFAULT_BACKUPS_FOLDER: return { ...config, backupsFolder: action.payload } - case CONFIG_ACTIONS.SET_ACCOUNT: - return { ...config, account: action.payload } + case CONFIG_ACTIONS.ADD_ACCOUNT: { + const others = config.accounts.filter((account) => account.playerUid !== action.payload.playerUid) + return { ...config, accounts: [...others, action.payload], activeAccountId: action.payload.playerUid } + } + case CONFIG_ACTIONS.REMOVE_ACCOUNT: { + const accounts = config.accounts.filter((account) => account.playerUid !== action.payload.playerUid) + const activeAccountId = config.activeAccountId === action.payload.playerUid ? (accounts[0]?.playerUid ?? null) : config.activeAccountId + return { ...config, accounts, activeAccountId } + } + case CONFIG_ACTIONS.SET_ACTIVE_ACCOUNT: + // An id naming nobody is a no-op returning the same object, so nothing re-renders for it, + // the same shape MOVE_INSTALLATION's guard above already uses. + if (action.payload !== null && !config.accounts.some((account) => account.playerUid === action.payload)) return config + return { ...config, activeAccountId: action.payload } case CONFIG_ACTIONS.SET_BACKGROUND: return { ...config, background: action.payload, _backgroundRevision: (config._backgroundRevision ?? 0) + 1 } case CONFIG_ACTIONS.SET_MODDB_VISIBILITY_ANSWER: diff --git a/src/renderer/src/locales/en-US.json b/src/renderer/src/locales/en-US.json index e76ea1eb..0be017fb 100644 --- a/src/renderer/src/locales/en-US.json +++ b/src/renderer/src/locales/en-US.json @@ -269,13 +269,10 @@ "defaultVersionsFolder": "VS Versions folder", "backupsFolder": "Backups folder", "loginTitle": "Log in", - "logoutTitle": "Log out", - "loggedout": "Logged out!", "loggingin": "Logging in!", "wrongtwofa": "Wrong 2FA code!", "invalidEmailPass": "Invalid email or password!", "loginUnreachable": "Couldn't reach the login service. Check your connection or firewall and try again.", - "logoutFailed": "Couldn't log out. Try again.", "unexpectedResponse": "The account service answered in a way this launcher couldn't read. Your password was not rejected. Try again after an update.", "loggedin": "Logged in as {{user}}!", "onlyIfEnabledTwoFA": "Fill this field only if you have 2FA enabled!", @@ -286,8 +283,13 @@ "ownImage": "Your own image", "backgroundsLoadFailed": "The background list couldn't be loaded. Check your connection and try again.", "language": "Language", - "areYouSureLogout": "Are you sure you want to log out?", - "loginoutNotReversible": "Logging out is not reversible but you can log back in again with the same Vintage Story account again!" + "switchAccountTitle": "Switch account", + "addAnotherAccount": "Add another account", + "removeAccountTitle": "Remove {{user}}", + "areYouSureRemoveAccount": "Are you sure you want to remove {{user}}?", + "removeAccountNotReversible": "Removing an account is not reversible but you can add it back again by logging in with the same Vintage Story account again!", + "accountRemoved": "Removed {{user}}.", + "removeAccountFailed": "Couldn't remove that account. Try again." }, "backups": { "backupInProgress": "There is a backup already in progress!", diff --git a/tests/domain/account/clientSettings.test.ts b/tests/domain/account/clientSettings.test.ts index 5758dd3c..5b5e3c15 100644 --- a/tests/domain/account/clientSettings.test.ts +++ b/tests/domain/account/clientSettings.test.ts @@ -1,7 +1,13 @@ import assert from "node:assert/strict" import { describe, it } from "vitest" -import { CLIENT_SETTINGS_FILE_NAME, mergeSessionIntoClientSettings, writeClientSettingsSession } from "../../../src/domain/account/clientSettings" +import { + CLIENT_SETTINGS_FILE_NAME, + clearForeignClientSettingsSession, + mergeSessionIntoClientSettings, + removeSessionFromClientSettings, + writeClientSettingsSession +} from "../../../src/domain/account/clientSettings" import type { AccountSessionFields } from "../../../src/domain/account/clientSettings" import type { JsonFile, JsonFileReadResult, JsonFileWriteResult } from "../../../src/domain/ports" @@ -279,3 +285,113 @@ describe("writeClientSettingsSession when the game refreshed the session itself" assert.deepEqual(writes[0]?.document, { intSettings: { maxFps: 60 }, stringSettings: { language: "es-es", ...WRITTEN_SESSION } }) }) }) + +describe("removeSessionFromClientSettings", () => { + it("removes exactly the eight session keys and nothing else", () => { + const result = removeSessionFromClientSettings({ stringSettings: { ...WRITTEN_SESSION, language: "es-es" }, intSettings: { maxFps: 60 } }) + + assert.deepEqual(stringSettingsOf(result), { language: "es-es" }) + assert.deepEqual(result.intSettings, { maxFps: 60 }) + for (const key of GAME_SESSION_KEYS) assert.equal(key in stringSettingsOf(result), false, key) + }) + + it("produces an empty stringSettings section when there was nothing but a session", () => { + const result = removeSessionFromClientSettings({ stringSettings: WRITTEN_SESSION }) + assert.deepEqual(stringSettingsOf(result), {}) + }) + + it("is a no-op shape-wise on a document with no session to begin with", () => { + const result = removeSessionFromClientSettings({ stringSettings: { language: "es-es" } }) + assert.deepEqual(stringSettingsOf(result), { language: "es-es" }) + }) + + it("treats a missing or unreadable document as empty rather than throwing", () => { + assert.deepEqual(removeSessionFromClientSettings(undefined), { stringSettings: {} }) + assert.deepEqual(removeSessionFromClientSettings(null), { stringSettings: {} }) + assert.deepEqual(removeSessionFromClientSettings("not an object"), { stringSettings: {} }) + }) + + it("leaves the document it was given alone", () => { + const original = { stringSettings: { ...WRITTEN_SESSION, language: "es-es" } } + removeSessionFromClientSettings(original) + assert.deepEqual(stringSettingsOf(original), { ...WRITTEN_SESSION, language: "es-es" }) + }) +}) + +/** + * With one saved account, a settings file the launcher could not update was + * harmless: whatever stale session it held was that same account's own. With + * more than one, it can be a housemate's, since the game writes their session + * there directly on their own successful login. This guards the launch path + * that keeps a locked or unreadable secret store from silently starting the + * game already signed in as somebody else. + */ +describe("clearForeignClientSettingsSession", () => { + const OUR_UID = "uid-1" + const FOREIGN_SESSION = { ...WRITTEN_SESSION, playeruid: "uid-housemate", playername: "Housemate" } + + it("clears a session that belongs to a different player", async () => { + const { jsonFile, writes } = fakeJsonFile({ ok: true, document: { stringSettings: { ...FOREIGN_SESSION, language: "es-es" }, intSettings: { maxFps: 60 } } }) + + const result = await clearForeignClientSettingsSession({ jsonFile }, { settingsPath: SETTINGS_PATH, playerUid: OUR_UID }) + + assert.deepEqual(result, { outcome: "cleared" }) + assert.equal(writes.length, 1) + const written = writes[0]?.document as Record + assert.deepEqual(stringSettingsOf(written), { language: "es-es" }) + assert.deepEqual(written.intSettings, { maxFps: 60 }, "everything else in the file survives") + }) + + it("leaves a file that already holds our own session alone", async () => { + const { jsonFile, writes } = fakeJsonFile({ ok: true, document: { stringSettings: WRITTEN_SESSION } }) + + const result = await clearForeignClientSettingsSession({ jsonFile }, { settingsPath: SETTINGS_PATH, playerUid: OUR_UID }) + + assert.deepEqual(result, { outcome: "not-foreign" }) + assert.deepEqual(writes, []) + }) + + it("leaves a file with no session in it alone", async () => { + const { jsonFile, writes } = fakeJsonFile({ ok: true, document: { stringSettings: { language: "es-es" } } }) + + const result = await clearForeignClientSettingsSession({ jsonFile }, { settingsPath: SETTINGS_PATH, playerUid: OUR_UID }) + + assert.deepEqual(result, { outcome: "not-foreign" }) + assert.deepEqual(writes, []) + }) + + it("leaves an entirely empty settings file alone", async () => { + const { jsonFile, writes } = fakeJsonFile({ ok: true, document: undefined }) + + const result = await clearForeignClientSettingsSession({ jsonFile }, { settingsPath: SETTINGS_PATH, playerUid: OUR_UID }) + + assert.deepEqual(result, { outcome: "not-foreign" }) + assert.deepEqual(writes, []) + }) + + it("reports the settings file as unreadable rather than guessing", async () => { + const { jsonFile, writes } = fakeJsonFile({ ok: false, error: "Unexpected token }" }) + + const result = await clearForeignClientSettingsSession({ jsonFile }, { settingsPath: SETTINGS_PATH, playerUid: OUR_UID }) + + assert.deepEqual(result, { outcome: "unreadable-settings" }) + assert.deepEqual(writes, []) + }) + + it("reports a clear that did not land", async () => { + const { jsonFile } = fakeJsonFile({ ok: true, document: { stringSettings: FOREIGN_SESSION } }, { ok: false, error: "EACCES" }) + + const result = await clearForeignClientSettingsSession({ jsonFile }, { settingsPath: SETTINGS_PATH, playerUid: OUR_UID }) + + assert.deepEqual(result, { outcome: "write-failed" }) + }) + + it("never writes a session of its own; that stays writeClientSettingsSession's job", async () => { + const { jsonFile, writes } = fakeJsonFile({ ok: true, document: { stringSettings: FOREIGN_SESSION } }) + + await clearForeignClientSettingsSession({ jsonFile }, { settingsPath: SETTINGS_PATH, playerUid: OUR_UID }) + + const written = writes[0]?.document as Record + for (const key of GAME_SESSION_KEYS) assert.equal(key in stringSettingsOf(written), false, key) + }) +}) diff --git a/tests/domain/account/credentials.test.ts b/tests/domain/account/credentials.test.ts index 4c60537a..51113204 100644 --- a/tests/domain/account/credentials.test.ts +++ b/tests/domain/account/credentials.test.ts @@ -1,7 +1,7 @@ import assert from "node:assert/strict" import { describe, it } from "vitest" -import { parseLegacyAccount, parseLoginAccount, parseStoredSecrets, toPublicAccount } from "../../../src/domain/account/credentials" +import { parseLegacyAccount, parseLoginAccount, parseStoredSecrets, parseStoredSecretsById, toPublicAccount } from "../../../src/domain/account/credentials" const EMAIL = "player@example.test" @@ -121,6 +121,55 @@ describe("parseStoredSecrets", () => { }) }) +describe("parseStoredSecretsById", () => { + const SECRETS_A = { sessionKey: "fake-key-a", sessionSignature: "fake-signature-a", mptoken: "fake-mptoken-a" } + const SECRETS_B = { sessionKey: "fake-key-b", sessionSignature: "fake-signature-b", mptoken: null } + + it("reads every account keyed by its own id", () => { + const result = parseStoredSecretsById({ + accounts: [ + { id: "uid-a", secrets: SECRETS_A }, + { id: "uid-b", secrets: SECRETS_B } + ] + }) + + assert.deepEqual(result.get("uid-a"), SECRETS_A) + assert.deepEqual(result.get("uid-b"), SECRETS_B) + assert.equal(result.size, 2) + }) + + it("returns an empty map for anything that is not the multi-account shape", () => { + for (const value of [null, "ciphertext", 7, [1, 2], {}, { accounts: "not an array" }, { accounts: null }]) { + assert.equal(parseStoredSecretsById(value).size, 0, String(value)) + } + }) + + it("drops one unreadable entry without losing the others", () => { + const result = parseStoredSecretsById({ + accounts: ["not a record", null, { id: "no-secrets" }, { id: "", secrets: SECRETS_A }, { id: "unreadable-secrets", secrets: { sessionKey: "only-a-key" } }, { id: "uid-b", secrets: SECRETS_B }] + }) + + assert.deepEqual(result, new Map([["uid-b", SECRETS_B]])) + }) + + it("keeps the first entry on a duplicate id", () => { + const result = parseStoredSecretsById({ + accounts: [ + { id: "uid-a", secrets: SECRETS_A }, + { id: "uid-a", secrets: SECRETS_B } + ] + }) + + assert.deepEqual(result.get("uid-a"), SECRETS_A) + assert.equal(result.size, 1) + }) + + it("refuses an id past the maximum account field length", () => { + const result = parseStoredSecretsById({ accounts: [{ id: "u".repeat(300), secrets: SECRETS_A }] }) + assert.equal(result.size, 0) + }) +}) + describe("toPublicAccount", () => { it("drops every field that is not part of the renderer-visible half", () => { const account = toPublicAccount({ diff --git a/tests/domain/config/migrations.test.ts b/tests/domain/config/migrations.test.ts index e0dae83d..625a0c14 100644 --- a/tests/domain/config/migrations.test.ts +++ b/tests/domain/config/migrations.test.ts @@ -11,6 +11,7 @@ import { floatMarkerToIntegerSchema, MAX_CONFIG_SCHEMA, migrateConfigDocument, + singleAccountToAccountList, stampLinkedOnExternalVersions } from "../../../src/domain/config/migrations" import type { ConfigMigration } from "../../../src/domain/config/migrations" @@ -143,7 +144,8 @@ describe("migrateConfigDocument on real configs", () => { assert.deepEqual(result.detected, { era: "float", schema: FLOAT_ERA_CONFIG_SCHEMA }) assert.deepEqual(result.applied, [ { fromSchema: 1, toSchema: 2 }, - { fromSchema: 2, toSchema: 3 } + { fromSchema: 2, toSchema: 3 }, + { fromSchema: 3, toSchema: 4 } ]) const doc = result.doc as Record @@ -151,6 +153,8 @@ describe("migrateConfigDocument on real configs", () => { assert.equal("version" in doc, false) assert.equal(doc.lastUsedInstallation, "abc") assert.deepEqual(doc.favMods, [12]) + assert.deepEqual(doc.accounts, [], "a config with no account reaches the current schema with an empty account list") + assert.equal(doc.activeAccountId, null) }) it("brings a versionless config to the current schema the same way", () => { @@ -205,7 +209,8 @@ describe("migrateConfigDocument on real configs", () => { CONFIG_MIGRATIONS.map((migration) => [migration.fromSchema, migration.toSchema]), [ [FLOAT_ERA_CONFIG_SCHEMA, FIRST_INTEGER_CONFIG_SCHEMA], - [2, 3] + [2, 3], + [3, 4] ] ) assert.equal(CONFIG_MIGRATIONS[CONFIG_MIGRATIONS.length - 1]?.toSchema, CURRENT_CONFIG_SCHEMA) @@ -384,3 +389,40 @@ describe("stampLinkedOnExternalVersions boundary checks", () => { assert.equal(versions[1]!.linked, true) }) }) + +describe("singleAccountToAccountList", () => { + it("steps from schema 3 to 4", () => { + assert.equal(singleAccountToAccountList.fromSchema, 3) + assert.equal(singleAccountToAccountList.toSchema, 4) + }) + + it("moves a valid legacy account into a one-entry list and makes it active", () => { + const account = { email: "a@b.c", playerName: "A", playerUid: "uid-a", playerEntitlements: null, hostGameServer: false } + const result = singleAccountToAccountList.migrate({ account, favMods: [1] }) as Record + + assert.deepEqual(result.accounts, [account]) + assert.equal(result.activeAccountId, "uid-a") + assert.equal("account" in result, false) + assert.deepEqual(result.favMods, [1], "fields the step does not own pass through untouched") + }) + + it("arrives at an empty list and no active account when there is nothing to migrate", () => { + for (const doc of [{}, { account: null }, { account: "not a record" }, { account: {} }, { account: { playerUid: "" } }]) { + const result = singleAccountToAccountList.migrate(doc) as Record + assert.deepEqual(result.accounts, [], JSON.stringify(doc)) + assert.equal(result.activeAccountId, null, JSON.stringify(doc)) + } + }) + + it("does not mutate the input document", () => { + const account = { email: "a@b.c", playerName: "A", playerUid: "uid-a", playerEntitlements: null, hostGameServer: false } + const doc = { account } + singleAccountToAccountList.migrate(doc) + assert.deepEqual(doc, { account }) + }) + + it("hands back non-objects untouched", () => { + assert.equal(singleAccountToAccountList.migrate(null), null) + assert.equal(singleAccountToAccountList.migrate("config"), "config") + }) +}) diff --git a/tests/ipc/accountHandlers.test.ts b/tests/ipc/accountHandlers.test.ts index 8bcdc967..92a45fd0 100644 --- a/tests/ipc/accountHandlers.test.ts +++ b/tests/ipc/accountHandlers.test.ts @@ -10,7 +10,7 @@ import "./helpers/electronMock" import { createTrustedEvent, createUntrustedEvent, getIpcHandler, setElectronUserDataPath } from "./helpers/electronMock" import { IPC_CHANNELS } from "@src/ipc/ipcChannels" -import { clearAccountSecrets, saveAccountSecrets } from "@src/ipc/accountStore" +import { removeAccountSecrets, saveAccountSecrets } from "@src/ipc/accountStore" import { requestBoundedTextViaNode } from "@src/ipc/network" /** @@ -42,7 +42,7 @@ vi.mock("@src/ipc/network", () => ({ vi.mock("@src/ipc/accountStore", () => ({ saveAccountSecrets: vi.fn(async () => undefined), - clearAccountSecrets: vi.fn(async () => true) + removeAccountSecrets: vi.fn(async () => true) })) import "@src/ipc/handlers/accountHandlers" @@ -64,7 +64,7 @@ const SUCCESS_BODY = JSON.stringify({ }) type LoginHandler = (event: IpcMainInvokeEvent, email: unknown, password: unknown, twoFactorCode?: unknown) => Promise -type LogoutHandler = (event: IpcMainInvokeEvent) => Promise +type RemoveAccountHandler = (event: IpcMainInvokeEvent, accountId: unknown) => Promise let userDataFolder: string let trustedEvent: IpcMainInvokeEvent @@ -73,8 +73,8 @@ function loginHandler(): LoginHandler { return getIpcHandler(IPC_CHANNELS.ACCOUNT_MANAGER.LOGIN) } -function logoutHandler(): LogoutHandler { - return getIpcHandler(IPC_CHANNELS.ACCOUNT_MANAGER.LOGOUT) +function removeAccountHandler(): RemoveAccountHandler { + return getIpcHandler(IPC_CHANNELS.ACCOUNT_MANAGER.REMOVE_ACCOUNT) } /** Queues one response body per pass, in order. */ @@ -95,7 +95,7 @@ beforeEach(async () => { setElectronUserDataPath(userDataFolder) vi.mocked(requestBoundedTextViaNode).mockReset() vi.mocked(saveAccountSecrets).mockReset().mockResolvedValue(undefined) - vi.mocked(clearAccountSecrets).mockReset().mockResolvedValue(true) + vi.mocked(removeAccountSecrets).mockReset().mockResolvedValue(true) trustedEvent = await createTrustedEvent() }) @@ -138,7 +138,7 @@ describe("LOGIN", () => { status: "success", account: { email: EMAIL, playerName: "Placeholder Player", playerUid: "placeholder-uid", playerEntitlements: "singleplayer", hostGameServer: false } }) - assert.deepEqual(vi.mocked(saveAccountSecrets).mock.calls, [[{ sessionKey: "placeholder-session-key", sessionSignature: "placeholder-session-signature", mptoken: null }]]) + assert.deepEqual(vi.mocked(saveAccountSecrets).mock.calls, [["placeholder-uid", { sessionKey: "placeholder-session-key", sessionSignature: "placeholder-session-signature", mptoken: null }]]) }) it("sends nothing back to the renderer that the store is meant to hold", async () => { @@ -275,23 +275,34 @@ describe("LOGIN", () => { }) }) -describe("LOGOUT", () => { +describe("REMOVE_ACCOUNT", () => { it("refuses a sender nothing registered as trusted", async () => { - await assert.rejects(logoutHandler()(createUntrustedEvent()), /sender|trusted|refused/i) + await assert.rejects(removeAccountHandler()(createUntrustedEvent(), "uid-a"), /sender|trusted|refused/i) - assert.equal(vi.mocked(clearAccountSecrets).mock.calls.length, 0) + assert.equal(vi.mocked(removeAccountSecrets).mock.calls.length, 0) }) - it("clears the stored secrets", async () => { - const result = await logoutHandler()(trustedEvent) + for (const [label, accountId] of [ + ["an id that is not a string", 42], + ["an empty id", ""] + ] as const) { + it(`refuses ${label} before touching the store`, async () => { + await assert.rejects(removeAccountHandler()(trustedEvent, accountId), TypeError) + + assert.equal(vi.mocked(removeAccountSecrets).mock.calls.length, 0) + }) + } + + it("removes the stored secrets for the given account", async () => { + const result = await removeAccountHandler()(trustedEvent, "uid-a") assert.equal(result, true) - assert.deepEqual(vi.mocked(clearAccountSecrets).mock.calls, [[]]) + assert.deepEqual(vi.mocked(removeAccountSecrets).mock.calls, [["uid-a"]]) }) - it("reports the failure when the secrets could not be cleared", async () => { - vi.mocked(clearAccountSecrets).mockResolvedValueOnce(false) + it("reports the failure when the secrets could not be removed", async () => { + vi.mocked(removeAccountSecrets).mockResolvedValueOnce(false) - assert.equal(await logoutHandler()(trustedEvent), false) + assert.equal(await removeAccountHandler()(trustedEvent, "uid-a"), false) }) }) diff --git a/tests/ipc/accountStore.test.ts b/tests/ipc/accountStore.test.ts index 691ab6ee..fd8e0c4a 100644 --- a/tests/ipc/accountStore.test.ts +++ b/tests/ipc/accountStore.test.ts @@ -7,7 +7,7 @@ import { afterEach, beforeEach, describe, it, vi } from "vitest" import type { AccountSecrets } from "@domain/account/credentials" /** - * The encrypted account store, against a fake `safeStorage`. + * The encrypted multi-account store, against a fake `safeStorage`. * * Electron's `safeStorage` talks to the OS keychain, so it is replaced here by * a reversible transform: enough to prove the store writes ciphertext and reads @@ -18,15 +18,21 @@ import type { AccountSecrets } from "@domain/account/credentials" * attacker would want out of this file, so no fixture here is shaped like a * real one. * - * `cachedSecrets` is module state, which is why almost every case re-imports + * `cachedAccounts` is module state, which is why almost every case re-imports * the store through `loadStore()` after `vi.resetModules()`: a test that shared * the cache with the one before it would be reading the previous test's answer. */ -const PLACEHOLDER_SECRETS: AccountSecrets = { - sessionKey: "placeholder-session-key", - sessionSignature: "placeholder-session-signature", - mptoken: "placeholder-multiplayer-token" +const ACCOUNT_A: AccountSecrets = { + sessionKey: "placeholder-session-key-a", + sessionSignature: "placeholder-session-signature-a", + mptoken: "placeholder-multiplayer-token-a" +} + +const ACCOUNT_B: AccountSecrets = { + sessionKey: "placeholder-session-key-b", + sessionSignature: "placeholder-session-signature-b", + mptoken: null } const mockState = vi.hoisted(() => ({ @@ -66,11 +72,20 @@ function storePath(): string { return join(mockState.userDataDir, "account-secrets.json") } +function backupPath(): string { + return join(mockState.userDataDir, "account-secrets.pre-migration.bak.json") +} + /** Writes the store file directly, standing in for whatever left it in that state. */ function writeStoreFile(contents: unknown): void { writeFileSync(storePath(), typeof contents === "string" ? contents : JSON.stringify(contents)) } +/** A legacy v1 single-account file, in the shape the pre-multi-account store wrote. */ +function writeLegacyStoreFile(secrets: AccountSecrets): void { + writeStoreFile({ version: 1, ciphertext: Buffer.from(`sealed:${JSON.stringify(secrets)}`, "utf8").toString("base64") }) +} + beforeEach(() => { mockState.userDataDir = mkdtempSync(join(tmpdir(), "rift-account-store-test-")) mockState.encryptionAvailable = true @@ -85,29 +100,40 @@ afterEach(() => { describe("saveAccountSecrets", () => { it("round-trips through a store the same process reads back cold", async () => { const writer = await loadStore() - await writer.saveAccountSecrets(PLACEHOLDER_SECRETS) + await writer.saveAccountSecrets("uid-a", ACCOUNT_A) const reader = await loadStore() - assert.deepEqual(await reader.getAccountSecrets(), PLACEHOLDER_SECRETS) + assert.deepEqual(await reader.getAccountSecrets("uid-a"), ACCOUNT_A) + }) + + it("keeps two accounts independent: saving one leaves the other exactly as it was", async () => { + const writer = await loadStore() + await writer.saveAccountSecrets("uid-a", ACCOUNT_A) + await writer.saveAccountSecrets("uid-b", ACCOUNT_B) + + const reader = await loadStore() + assert.deepEqual(await reader.getAccountSecrets("uid-a"), ACCOUNT_A) + assert.deepEqual(await reader.getAccountSecrets("uid-b"), ACCOUNT_B) }) it("leaves no plaintext secret in the file", async () => { const store = await loadStore() - await store.saveAccountSecrets(PLACEHOLDER_SECRETS) + await store.saveAccountSecrets("uid-a", ACCOUNT_A) const raw = readFileSync(storePath(), "utf8") - assert.equal(raw.includes(PLACEHOLDER_SECRETS.sessionKey), false, "the session key is readable in the store file") - assert.equal(raw.includes(PLACEHOLDER_SECRETS.sessionSignature), false, "the session signature is readable in the store file") + assert.equal(raw.includes(ACCOUNT_A.sessionKey), false, "the session key is readable in the store file") + assert.equal(raw.includes(ACCOUNT_A.sessionSignature), false, "the session signature is readable in the store file") + assert.equal(raw.includes("uid-a"), false, "the account id is readable outside the ciphertext") assert.deepEqual(Object.keys(JSON.parse(raw)).sort(), ["ciphertext", "version"]) - assert.equal(JSON.parse(raw).version, 1) + assert.equal(JSON.parse(raw).version, 2) }) it("keeps the file readable only by its owner", async () => { const store = await loadStore() - await store.saveAccountSecrets(PLACEHOLDER_SECRETS) + await store.saveAccountSecrets("uid-a", ACCOUNT_A) assert.equal(statSync(storePath()).mode & 0o777, 0o600) }) @@ -115,7 +141,7 @@ describe("saveAccountSecrets", () => { it("leaves no temporary file behind", async () => { const store = await loadStore() - await store.saveAccountSecrets(PLACEHOLDER_SECRETS) + await store.saveAccountSecrets("uid-a", ACCOUNT_A) assert.deepEqual( readdirSync(mockState.userDataDir).filter((entry) => entry.endsWith(".tmp")), @@ -123,22 +149,23 @@ describe("saveAccountSecrets", () => { ) }) - it("replaces the secrets a previous save wrote", async () => { + it("replaces the secrets a previous save wrote for the same account, a session refresh rather than a duplicate", async () => { const first = await loadStore() - await first.saveAccountSecrets(PLACEHOLDER_SECRETS) + await first.saveAccountSecrets("uid-a", ACCOUNT_A) const second = await loadStore() - await second.saveAccountSecrets({ ...PLACEHOLDER_SECRETS, sessionKey: "placeholder-session-key-two", mptoken: null }) + await second.saveAccountSecrets("uid-a", { ...ACCOUNT_A, sessionKey: "placeholder-session-key-a-refreshed" }) const reader = await loadStore() - assert.deepEqual(await reader.getAccountSecrets(), { ...PLACEHOLDER_SECRETS, sessionKey: "placeholder-session-key-two", mptoken: null }) + assert.deepEqual(await reader.getAccountSecrets("uid-a"), { ...ACCOUNT_A, sessionKey: "placeholder-session-key-a-refreshed" }) + assert.deepEqual(await reader.listStoredAccountIds(), ["uid-a"]) }) it("refuses to write when the platform offers no encryption", async () => { mockState.encryptionAvailable = false const store = await loadStore() - await assert.rejects(store.saveAccountSecrets(PLACEHOLDER_SECRETS), /Secure account storage is unavailable/) + await assert.rejects(store.saveAccountSecrets("uid-a", ACCOUNT_A), /Secure account storage is unavailable/) assert.equal(existsSync(storePath()), false) }) @@ -150,7 +177,7 @@ describe("saveAccountSecrets", () => { mockState.storageBackend = "basic_text" const store = await loadStore() - await assert.rejects(store.saveAccountSecrets(PLACEHOLDER_SECRETS), /A system password store is required/) + await assert.rejects(store.saveAccountSecrets("uid-a", ACCOUNT_A), /A system password store is required/) assert.equal(existsSync(storePath()), false) }) }) @@ -159,110 +186,258 @@ describe("getAccountSecrets", () => { it("answers null when nothing was ever stored", async () => { const store = await loadStore() - assert.equal(await store.getAccountSecrets(), null) + assert.equal(await store.getAccountSecrets("uid-a"), null) + }) + + it("answers null for an id nobody saved, even when other accounts exist", async () => { + const writer = await loadStore() + await writer.saveAccountSecrets("uid-a", ACCOUNT_A) + + const reader = await loadStore() + assert.equal(await reader.getAccountSecrets("uid-b"), null) }) it("answers null for a file that is not JSON", async () => { writeStoreFile("{ not json at all") const store = await loadStore() - assert.equal(await store.getAccountSecrets(), null) + assert.equal(await store.getAccountSecrets("uid-a"), null) }) it("answers null for a store written by a version this one does not know", async () => { - writeStoreFile({ version: 2, ciphertext: Buffer.from("sealed:{}", "utf8").toString("base64") }) + writeStoreFile({ version: 3, ciphertext: Buffer.from("sealed:{}", "utf8").toString("base64") }) const store = await loadStore() - assert.equal(await store.getAccountSecrets(), null) + assert.equal(await store.getAccountSecrets("uid-a"), null) + }) + + it("answers null for a legacy v1 store: reading it does not migrate it", async () => { + writeLegacyStoreFile(ACCOUNT_A) + const store = await loadStore() + + assert.equal(await store.getAccountSecrets("uid-a"), null, "only adoptLegacySingleAccountSecrets reads a v1 file") }) it("answers null when the ciphertext field is not a string", async () => { - writeStoreFile({ version: 1, ciphertext: 42 }) + writeStoreFile({ version: 2, ciphertext: 42 }) const store = await loadStore() - assert.equal(await store.getAccountSecrets(), null) + assert.equal(await store.getAccountSecrets("uid-a"), null) }) it("answers null when the ciphertext cannot be decrypted", async () => { - writeStoreFile({ version: 1, ciphertext: Buffer.from("someone else's bytes", "utf8").toString("base64") }) + writeStoreFile({ version: 2, ciphertext: Buffer.from("someone else's bytes", "utf8").toString("base64") }) const store = await loadStore() - assert.equal(await store.getAccountSecrets(), null) + assert.equal(await store.getAccountSecrets("uid-a"), null) }) it("answers null when the decrypted payload is not JSON", async () => { - writeStoreFile({ version: 1, ciphertext: Buffer.from("sealed:not json", "utf8").toString("base64") }) + writeStoreFile({ version: 2, ciphertext: Buffer.from("sealed:not json", "utf8").toString("base64") }) + const store = await loadStore() + + assert.equal(await store.getAccountSecrets("uid-a"), null) + }) + + it("drops one unreadable entry without losing the others in the same file", async () => { + writeStoreFile({ + version: 2, + ciphertext: Buffer.from( + `sealed:${JSON.stringify({ + accounts: [ + { id: "uid-a", secrets: { sessionKey: "only-a-key" } }, + { id: "uid-b", secrets: ACCOUNT_B } + ] + })}`, + "utf8" + ).toString("base64") + }) + const store = await loadStore() + + assert.equal(await store.getAccountSecrets("uid-a"), null, "missing sessionSignature makes this entry unreadable") + assert.deepEqual(await store.getAccountSecrets("uid-b"), ACCOUNT_B) + }) + + it("keeps the first entry on a duplicate id", async () => { + writeStoreFile({ + version: 2, + ciphertext: Buffer.from( + `sealed:${JSON.stringify({ + accounts: [ + { id: "uid-a", secrets: ACCOUNT_A }, + { id: "uid-a", secrets: ACCOUNT_B } + ] + })}`, + "utf8" + ).toString("base64") + }) const store = await loadStore() - assert.equal(await store.getAccountSecrets(), null) + assert.deepEqual(await store.getAccountSecrets("uid-a"), ACCOUNT_A) }) - it("answers null when the decrypted payload is missing a session field", async () => { - writeStoreFile({ version: 1, ciphertext: Buffer.from(`sealed:${JSON.stringify({ sessionKey: "placeholder-session-key" })}`, "utf8").toString("base64") }) + it("treats an id equal to __proto__ as ordinary data, since the store is a Map keyed by string, not an object", async () => { + writeStoreFile({ + version: 2, + ciphertext: Buffer.from(`sealed:${JSON.stringify({ accounts: [{ id: "__proto__", secrets: ACCOUNT_A }] })}`, "utf8").toString("base64") + }) const store = await loadStore() - assert.equal(await store.getAccountSecrets(), null) + assert.deepEqual(await store.getAccountSecrets("__proto__"), ACCOUNT_A) }) it("answers null when the platform offers no encryption, rather than reading the file", async () => { const writer = await loadStore() - await writer.saveAccountSecrets(PLACEHOLDER_SECRETS) + await writer.saveAccountSecrets("uid-a", ACCOUNT_A) mockState.encryptionAvailable = false const reader = await loadStore() - assert.equal(await reader.getAccountSecrets(), null) + assert.equal(await reader.getAccountSecrets("uid-a"), null) }) it("reads the file once and answers from memory after that", async () => { const writer = await loadStore() - await writer.saveAccountSecrets(PLACEHOLDER_SECRETS) + await writer.saveAccountSecrets("uid-a", ACCOUNT_A) const reader = await loadStore() - assert.deepEqual(await reader.getAccountSecrets(), PLACEHOLDER_SECRETS) + assert.deepEqual(await reader.getAccountSecrets("uid-a"), ACCOUNT_A) rmSync(storePath()) - assert.deepEqual(await reader.getAccountSecrets(), PLACEHOLDER_SECRETS) + assert.deepEqual(await reader.getAccountSecrets("uid-a"), ACCOUNT_A) }) it("remembers a miss too, rather than re-reading a file that is not there", async () => { const store = await loadStore() - assert.equal(await store.getAccountSecrets(), null) + assert.equal(await store.getAccountSecrets("uid-a"), null) // A save is what refreshes the cache; a file appearing underneath it is not. - writeStoreFile({ version: 1, ciphertext: Buffer.from(`sealed:${JSON.stringify(PLACEHOLDER_SECRETS)}`, "utf8").toString("base64") }) + writeStoreFile({ version: 2, ciphertext: Buffer.from(`sealed:${JSON.stringify({ accounts: [{ id: "uid-a", secrets: ACCOUNT_A }] })}`, "utf8").toString("base64") }) - assert.equal(await store.getAccountSecrets(), null) + assert.equal(await store.getAccountSecrets("uid-a"), null) }) }) -describe("clearAccountSecrets", () => { - it("removes the file and forgets what was cached", async () => { +describe("removeAccountSecrets", () => { + it("removes one account and leaves the file readable for the other", async () => { const store = await loadStore() - await store.saveAccountSecrets(PLACEHOLDER_SECRETS) - assert.deepEqual(await store.getAccountSecrets(), PLACEHOLDER_SECRETS) + await store.saveAccountSecrets("uid-a", ACCOUNT_A) + await store.saveAccountSecrets("uid-b", ACCOUNT_B) + + assert.equal(await store.removeAccountSecrets("uid-a"), true) - assert.equal(await store.clearAccountSecrets(), true) + assert.equal(await store.getAccountSecrets("uid-a"), null) + assert.deepEqual(await store.getAccountSecrets("uid-b"), ACCOUNT_B) + assert.equal(existsSync(storePath()), true, "the file itself survives while another account is still in it") + }) + + it("removes the file entirely once the last account is gone", async () => { + const store = await loadStore() + await store.saveAccountSecrets("uid-a", ACCOUNT_A) + + assert.equal(await store.removeAccountSecrets("uid-a"), true) assert.equal(existsSync(storePath()), false) - assert.equal(await store.getAccountSecrets(), null) + assert.equal(await store.getAccountSecrets("uid-a"), null) }) it("reports success when there was nothing to remove", async () => { const store = await loadStore() - assert.equal(await store.clearAccountSecrets(), true) + assert.equal(await store.removeAccountSecrets("uid-a"), true) }) - it.skipIf(process.platform !== "linux" || process.getuid?.() === 0)("reports failure when the file cannot be removed", async () => { + it.skipIf(process.platform !== "linux" || process.getuid?.() === 0)("reports failure when the file cannot be rewritten", async () => { const store = await loadStore() - await store.saveAccountSecrets(PLACEHOLDER_SECRETS) - // Removing a file needs write permission on its folder, not on the file. + await store.saveAccountSecrets("uid-a", ACCOUNT_A) + await store.saveAccountSecrets("uid-b", ACCOUNT_B) + // Removing uid-a still has to rewrite the file (uid-b remains), which needs + // write access to the directory the same way saveAccountSecrets does. chmodSync(mockState.userDataDir, 0o500) - assert.equal(await store.clearAccountSecrets(), false) + assert.equal(await store.removeAccountSecrets("uid-a"), false) chmodSync(mockState.userDataDir, 0o700) - assert.equal(existsSync(storePath()), true) + assert.deepEqual(await store.getAccountSecrets("uid-b"), ACCOUNT_B) + }) +}) + +describe("listStoredAccountIds", () => { + it("lists every id the store currently holds secrets for", async () => { + const store = await loadStore() + await store.saveAccountSecrets("uid-a", ACCOUNT_A) + await store.saveAccountSecrets("uid-b", ACCOUNT_B) + + assert.deepEqual(new Set(await store.listStoredAccountIds()), new Set(["uid-a", "uid-b"])) + }) + + it("is empty when nothing has ever been saved", async () => { + const store = await loadStore() + assert.deepEqual(await store.listStoredAccountIds(), []) + }) +}) + +describe("adoptLegacySingleAccountSecrets", () => { + it("re-keys a v1 store under the given account id", async () => { + writeLegacyStoreFile(ACCOUNT_A) + const store = await loadStore() + + assert.equal(await store.adoptLegacySingleAccountSecrets("uid-a"), true) + assert.deepEqual(await store.getAccountSecrets("uid-a"), ACCOUNT_A) + + const raw = JSON.parse(readFileSync(storePath(), "utf8")) + assert.equal(raw.version, 2) + }) + + it("backs up the original v1 file before overwriting it", async () => { + writeLegacyStoreFile(ACCOUNT_A) + const store = await loadStore() + + await store.adoptLegacySingleAccountSecrets("uid-a") + + assert.equal(existsSync(backupPath()), true) + const backed = JSON.parse(readFileSync(backupPath(), "utf8")) + assert.equal(backed.version, 1, "the backup holds the pre-migration v1 bytes, not the re-keyed result") + }) + + it("keeps the first backup rather than overwriting it on a later re-key attempt", async () => { + writeFileSync(backupPath(), JSON.stringify({ sentinel: "already there" })) + writeLegacyStoreFile(ACCOUNT_A) + const store = await loadStore() + + await store.adoptLegacySingleAccountSecrets("uid-a") + + const backed = JSON.parse(readFileSync(backupPath(), "utf8")) + assert.equal(backed.sentinel, "already there") + }) + + it("is a no-op on a store that has already been re-keyed", async () => { + const writer = await loadStore() + await writer.saveAccountSecrets("uid-a", ACCOUNT_A) + + const later = await loadStore() + assert.equal(await later.adoptLegacySingleAccountSecrets("uid-a"), false) + assert.deepEqual(await later.getAccountSecrets("uid-a"), ACCOUNT_A, "the already-current store is untouched") + }) + + it("is a no-op when there is no store file at all", async () => { + const store = await loadStore() + assert.equal(await store.adoptLegacySingleAccountSecrets("uid-a"), false) + }) + + it("is a no-op on a v1 file that cannot be decrypted", async () => { + writeStoreFile({ version: 1, ciphertext: Buffer.from("someone else's bytes", "utf8").toString("base64") }) + const store = await loadStore() + + assert.equal(await store.adoptLegacySingleAccountSecrets("uid-a"), false) + assert.equal(existsSync(backupPath()), false, "nothing worth backing up when the source was never readable") + }) + + it("is a no-op on a v1 file that decrypts but holds an incomplete secret", async () => { + writeStoreFile({ version: 1, ciphertext: Buffer.from(`sealed:${JSON.stringify({ sessionKey: "only-a-key" })}`, "utf8").toString("base64") }) + const store = await loadStore() + + assert.equal(await store.adoptLegacySingleAccountSecrets("uid-a"), false) + assert.equal(existsSync(backupPath()), false, "nothing worth backing up when the source was never usable") }) }) diff --git a/tests/ipc/configHandlers.test.ts b/tests/ipc/configHandlers.test.ts index 5aa6c91a..7d74031d 100644 --- a/tests/ipc/configHandlers.test.ts +++ b/tests/ipc/configHandlers.test.ts @@ -74,7 +74,8 @@ function minimalConfig(overrides: Partial = {}): ConfigType { defaultVersionsFolder: join(appDataFolder, "RiftLauncherGameVersions"), backupsFolder: join(appDataFolder, "RiftLauncherBackups"), window: { width: 1280, height: 720, x: 0, y: 0, maximized: false }, - account: null, + accounts: [], + activeAccountId: null, installations: [], gameVersions: [], favMods: [], diff --git a/tests/ipc/configManager.test.ts b/tests/ipc/configManager.test.ts index a3729c57..63d52c36 100644 --- a/tests/ipc/configManager.test.ts +++ b/tests/ipc/configManager.test.ts @@ -29,13 +29,15 @@ import { DEFAULT_COMPRESSION_LEVEL } from "@domain/config/defaults" * import, not after. */ vi.mock("@src/ipc/accountStore", () => ({ - saveAccountSecrets: vi.fn(async () => undefined) + saveAccountSecrets: vi.fn(async () => undefined), + adoptLegacySingleAccountSecrets: vi.fn(async () => false) })) -import { saveAccountSecrets } from "@src/ipc/accountStore" +import { adoptLegacySingleAccountSecrets, saveAccountSecrets } from "@src/ipc/accountStore" import { CUSTOM_BACKGROUND_ID, DEFAULT_BACKGROUND_ID } from "@domain/backgrounds" import { DEFAULT_MODDB_VISIBILITY_ANSWER, MODDB_VISIBILITY_ACCEPTED, MODDB_VISIBILITY_ALREADY_DONE, MODDB_VISIBILITY_DECLINED } from "@domain/moddbVisibility" import { DEFAULT_RECEIVE_BETA_UPDATES } from "@domain/appUpdate/betaUpdates" +import { CURRENT_CONFIG_SCHEMA } from "@domain/config/migrations" let temporaryRoot: string let userDataFolder: string @@ -59,6 +61,8 @@ beforeEach(() => { vi.mocked(saveAccountSecrets).mockReset() vi.mocked(saveAccountSecrets).mockResolvedValue(undefined) + vi.mocked(adoptLegacySingleAccountSecrets).mockReset() + vi.mocked(adoptLegacySingleAccountSecrets).mockResolvedValue(false) }) afterEach(() => { @@ -74,7 +78,8 @@ function minimalConfig(overrides: Partial = {}): ConfigType { defaultVersionsFolder: join(appDataFolder, "VSLGameVersions"), backupsFolder: join(appDataFolder, "VSLBackups"), window: { width: 1280, height: 720, x: 0, y: 0, maximized: false }, - account: null, + accounts: [], + activeAccountId: null, installations: [], gameVersions: [], favMods: [], @@ -475,7 +480,7 @@ describe("getConfig: schema migration logging", () => { const { getConfig } = await freshConfigManager() const config = await getConfig() - assert.equal(config.schemaVersion, 3) + assert.equal(config.schemaVersion, CURRENT_CONFIG_SCHEMA) }) }) @@ -501,15 +506,19 @@ describe("getConfig: legacy account secrets migration", () => { const config = await getConfig() assert.equal(vi.mocked(saveAccountSecrets).mock.calls.length, 1) - assert.deepEqual(config.account, { - email: "player@example.test", - playerName: "TestPlayer", - playerUid: "uid-0001", - playerEntitlements: null, - hostGameServer: false - }) + assert.deepEqual(vi.mocked(saveAccountSecrets).mock.calls[0]![0], "uid-0001", "keyed by the account's own playerUid") + assert.deepEqual(config.accounts, [ + { + email: "player@example.test", + playerName: "TestPlayer", + playerUid: "uid-0001", + playerEntitlements: null, + hostGameServer: false + } + ]) + assert.equal(config.activeAccountId, "uid-0001") // The legacy secret fields never reach the renderer-visible account. - assert.equal("sessionKey" in (config.account as object), false) + assert.equal("sessionKey" in config.accounts[0]!, false) }) it("discards an unparseable legacy account rather than migrate garbage", async () => { @@ -525,7 +534,8 @@ describe("getConfig: legacy account secrets migration", () => { const config = await getConfig() assert.equal(vi.mocked(saveAccountSecrets).mock.calls.length, 0) - assert.equal(config.account, null) + assert.deepEqual(config.accounts, []) + assert.equal(config.activeAccountId, null) }) it("logs a warning but still finishes when secure storage refuses the migrated secrets", async () => { @@ -550,7 +560,8 @@ describe("getConfig: legacy account secrets migration", () => { assert.equal(vi.mocked(saveAccountSecrets).mock.calls.length, 1) // The public half still comes through even though the secrets could not be stored. - assert.equal(config.account?.playerUid, "uid-0002") + assert.equal(config.activeAccountId, "uid-0002") + assert.equal(config.accounts[0]?.playerUid, "uid-0002") }) it("does not attempt a migration when the account carries none of the legacy secret fields", async () => { @@ -563,3 +574,148 @@ describe("getConfig: legacy account secrets migration", () => { assert.equal(vi.mocked(saveAccountSecrets).mock.calls.length, 0) }) }) + +describe("normalizeConfig: accounts", () => { + it("drops entries that are not readable accounts, and deduplicates by playerUid, first wins", async () => { + const { normalizeConfig } = await freshConfigManager() + const account = { email: "a@b.c", playerName: "A", playerUid: "uid-a", playerEntitlements: null, hostGameServer: false } + const duplicate = { ...account, playerName: "A (stale)" } + + const result = normalizeConfig({ accounts: ["not a record", null, { email: "no uid" }, account, duplicate] }) + + assert.deepEqual( + result.accounts.map((a) => a.playerName), + ["A"] + ) + }) + + it("caps the accounts array at 50 entries", async () => { + const { normalizeConfig } = await freshConfigManager() + const many = Array.from({ length: 55 }, (_, i) => ({ email: `p${i}@b.c`, playerName: `P${i}`, playerUid: `uid-${i}`, playerEntitlements: null, hostGameServer: false })) + const result = normalizeConfig({ accounts: many }) + assert.equal(result.accounts.length, 50) + }) + + it("falls back to [] when accounts is not an array", async () => { + const { normalizeConfig } = await freshConfigManager() + assert.deepEqual(normalizeConfig({ accounts: "nope" }).accounts, []) + }) + + it("strips session credentials from every entry, the same way a single account never carried them", async () => { + const { normalizeConfig } = await freshConfigManager() + const withSecrets = { + email: "a@b.c", + playerName: "A", + playerUid: "uid-a", + playerEntitlements: null, + hostGameServer: false, + sessionKey: "fake-key", + sessionSignature: "fake-signature", + mptoken: "fake-mptoken" + } + + const result = normalizeConfig({ accounts: [withSecrets] }) + + assert.equal(result.accounts.length, 1) + for (const field of ["sessionKey", "sessionSignature", "mptoken"]) assert.equal(field in result.accounts[0]!, false, field) + }) +}) + +describe("normalizeConfig: activeAccountId", () => { + const accountA = { email: "a@b.c", playerName: "A", playerUid: "uid-a", playerEntitlements: null, hostGameServer: false } + const accountB = { email: "b@b.c", playerName: "B", playerUid: "uid-b", playerEntitlements: null, hostGameServer: false } + + it("keeps an activeAccountId that names a saved account", async () => { + const { normalizeConfig } = await freshConfigManager() + const result = normalizeConfig({ accounts: [accountA, accountB], activeAccountId: "uid-b" }) + assert.equal(result.activeAccountId, "uid-b") + }) + + it("falls back to the first saved account when the id names nobody", async () => { + const { normalizeConfig } = await freshConfigManager() + const result = normalizeConfig({ accounts: [accountA, accountB], activeAccountId: "uid-gone" }) + assert.equal(result.activeAccountId, "uid-a") + }) + + it("is null when there are no saved accounts, whatever the stored id says", async () => { + const { normalizeConfig } = await freshConfigManager() + assert.equal(normalizeConfig({ accounts: [], activeAccountId: "uid-a" }).activeAccountId, null) + assert.equal(normalizeConfig({}).activeAccountId, null) + }) +}) + +describe("getConfig: account store re-key migration", () => { + it("re-keys the secret store under the legacy account's playerUid", async () => { + const legacyDoc = { ...minimalConfig(), account: { email: "a@b.c", playerName: "A", playerUid: "uid-rekey", playerEntitlements: null, hostGameServer: false } } + writeFileSync(join(userDataFolder, "config.json"), JSON.stringify(legacyDoc), "utf-8") + vi.mocked(adoptLegacySingleAccountSecrets).mockResolvedValueOnce(true) + + const { getConfig } = await freshConfigManager() + await getConfig() + + assert.equal(vi.mocked(adoptLegacySingleAccountSecrets).mock.calls.length, 1) + assert.equal(vi.mocked(adoptLegacySingleAccountSecrets).mock.calls[0]![0], "uid-rekey") + }) + + it("does nothing when there is no readable account to key the store by", async () => { + writeFileSync(join(userDataFolder, "config.json"), JSON.stringify(minimalConfig()), "utf-8") + + const { getConfig } = await freshConfigManager() + await getConfig() + + assert.equal(vi.mocked(adoptLegacySingleAccountSecrets).mock.calls.length, 0) + }) + + it("logs a warning but still finishes when re-keying throws", async () => { + const legacyDoc = { ...minimalConfig(), account: { email: "a@b.c", playerName: "A", playerUid: "uid-throws", playerEntitlements: null, hostGameServer: false } } + writeFileSync(join(userDataFolder, "config.json"), JSON.stringify(legacyDoc), "utf-8") + vi.mocked(adoptLegacySingleAccountSecrets).mockRejectedValueOnce(new Error("boom")) + + const { getConfig } = await freshConfigManager() + const config = await getConfig() + + assert.equal(config.activeAccountId, "uid-throws", "the document still migrates even though re-keying the store failed") + }) +}) + +describe("getConfig: config.json backup before a schema migration", () => { + function backupPath(): string { + return join(userDataFolder, "config.pre-migration.bak.json") + } + + it("copies the pre-migration document once a migration actually runs", async () => { + const legacyDoc = minimalConfig({ schemaVersion: 2 }) + writeFileSync(join(userDataFolder, "config.json"), JSON.stringify(legacyDoc), "utf-8") + + const { getConfig } = await freshConfigManager() + await getConfig() + + const fse = (await import("fs-extra")).default + assert.equal(await fse.pathExists(backupPath()), true) + const backed = await fse.readJSON(backupPath()) + assert.equal(backed.schemaVersion, 2, "the backup holds the document exactly as it was before migrating, not the migrated result") + }) + + it("takes no backup when the document is already at the current schema", async () => { + writeFileSync(join(userDataFolder, "config.json"), JSON.stringify(minimalConfig({ schemaVersion: CURRENT_CONFIG_SCHEMA })), "utf-8") + + const { getConfig } = await freshConfigManager() + await getConfig() + + const fse = (await import("fs-extra")).default + assert.equal(await fse.pathExists(backupPath()), false) + }) + + it("keeps the first backup rather than overwriting it on a later migration", async () => { + const fse = (await import("fs-extra")).default + await fse.ensureDir(userDataFolder) + await fse.writeJSON(backupPath(), { schemaVersion: 1, sentinel: "already there" }) + writeFileSync(join(userDataFolder, "config.json"), JSON.stringify(minimalConfig({ schemaVersion: 2 })), "utf-8") + + const { getConfig } = await freshConfigManager() + await getConfig() + + const backed = await fse.readJSON(backupPath()) + assert.equal(backed.sentinel, "already there") + }) +}) diff --git a/tests/ipc/gameHandlers.test.ts b/tests/ipc/gameHandlers.test.ts index 78f01a25..780ec773 100644 --- a/tests/ipc/gameHandlers.test.ts +++ b/tests/ipc/gameHandlers.test.ts @@ -10,6 +10,7 @@ import "./helpers/electronMock" import { createTrustedEvent, createUntrustedEvent, getIpcHandler, setElectronPath, setElectronUserDataPath } from "./helpers/electronMock" import { IPC_CHANNELS } from "@src/ipc/ipcChannels" +import { CURRENT_CONFIG_SCHEMA } from "@domain/config/migrations" /** * Branch coverage for src/ipc/handlers/gameHandlers.ts (EXECUTE_GAME, @@ -66,16 +67,24 @@ function baseInstallation( return { path: "", startParams: "", mesaGlThread: false, envVars: "", ...overrides } } -/** Writes a fake config.json this run's configManager reads back through getConfig(). */ +/** + * Writes a fake config.json this run's configManager reads back through getConfig(). + * + * Written already at the current schema, not an older one: the schema-3-to-4 migration + * unconditionally rebuilds `accounts`/`activeAccountId` from a legacy singular `account` + * field this fixture never has, so writing at an old schema would silently wipe whatever + * `accounts`/`activeAccountId` a test set here before the handler ever saw them. + */ function writeConfig(config: Partial): void { const fullConfig = { - schemaVersion: 2, + schemaVersion: CURRENT_CONFIG_SCHEMA, lastUsedInstallation: null, defaultInstallationsFolder: managedFolder, defaultVersionsFolder: versionsFolder, backupsFolder, window: { width: 1280, height: 720, x: 0, y: 0, maximized: false }, - account: null, + accounts: [], + activeAccountId: null, installations: [], gameVersions: [], favMods: [], @@ -225,7 +234,8 @@ describe("EXECUTE_GAME", () => { writeFileSync(executablePath, "not a real binary", { mode: 0o644 }) writeConfig({ gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], - account: { email: "player@example.com", playerName: "Player", playerUid: "1", playerEntitlements: null, hostGameServer: false } as unknown as ConfigType["account"] + accounts: [{ email: "player@example.com", playerName: "Player", playerUid: "1", playerEntitlements: null, hostGameServer: false }], + activeAccountId: "1" }) const event = await createTrustedEvent() @@ -248,7 +258,8 @@ describe("EXECUTE_GAME", () => { writeFileSync(join(gameVersionFolder, "Vintagestory"), "", "utf-8") writeConfig({ gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], - account: { email: "player@example.com", playerName: "Player", playerUid: "1", playerEntitlements: null, hostGameServer: false } as unknown as ConfigType["account"] + accounts: [{ email: "player@example.com", playerName: "Player", playerUid: "1", playerEntitlements: null, hostGameServer: false }], + activeAccountId: "1" }) // Read-only installation folder: clientsettings.json does not exist yet, @@ -264,6 +275,108 @@ describe("EXECUTE_GAME", () => { } }) + /** + * With one saved account, a session store the launcher could not read was + * harmless: whatever stale session the settings file held was that same + * account's own. With more than one account possible, it can be a + * housemate's, since the game writes their session there directly on their + * own successful login. These three pin the guard that keeps a launch from + * silently starting the game already signed in as somebody else. + */ + it("clears another player's session before launching with no session of our own", async () => { + const gameVersionFolder = join(versionsFolder, "1.20.0") + const installationFolder = join(managedFolder, "Main") + mkdirSync(gameVersionFolder, { recursive: true }) + mkdirSync(installationFolder, { recursive: true }) + writeFileSync(join(gameVersionFolder, "Vintagestory"), "not a real binary", { mode: 0o644 }) + writeConfig({ + gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], + accounts: [{ email: "player@example.com", playerName: "Player", playerUid: "1", playerEntitlements: null, hostGameServer: false }], + activeAccountId: "1" + }) + writeFileSync( + join(installationFolder, "clientsettings.json"), + JSON.stringify({ + stringSettings: { sessionkey: "housemate-session-key", sessionsignature: "housemate-session-signature", mptoken: null, playeruid: "housemate-uid", playername: "Housemate" }, + intSettings: { maxFps: 60 } + }), + "utf-8" + ) + + const { getAccountSecrets } = await import("@src/ipc/accountStore") + vi.mocked(getAccountSecrets).mockResolvedValueOnce(null) + + const event = await createTrustedEvent() + const result = await executeGameHandler()(event, { version: "1.20.0", path: gameVersionFolder }, baseInstallation({ path: installationFolder })) + assert.deepEqual(result, { ok: false, reason: "launch-failed" }, "the launch itself still proceeds; only the foreign session is cleared") + + const { readFileSync } = await import("node:fs") + const settings = JSON.parse(readFileSync(join(installationFolder, "clientsettings.json"), "utf-8")) + assert.equal(settings.stringSettings.playeruid, undefined) + assert.equal(settings.stringSettings.sessionkey, undefined) + assert.deepEqual(settings.intSettings, { maxFps: 60 }, "everything else in the file survives") + }) + + it("leaves a settings file with no foreign session alone when we have none of our own", async () => { + const gameVersionFolder = join(versionsFolder, "1.20.0") + const installationFolder = join(managedFolder, "Main") + mkdirSync(gameVersionFolder, { recursive: true }) + mkdirSync(installationFolder, { recursive: true }) + writeFileSync(join(gameVersionFolder, "Vintagestory"), "not a real binary", { mode: 0o644 }) + writeConfig({ + gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], + accounts: [{ email: "player@example.com", playerName: "Player", playerUid: "1", playerEntitlements: null, hostGameServer: false }], + activeAccountId: "1" + }) + // playeruid "1" is our own account: not foreign, so nothing should change here. + writeFileSync( + join(installationFolder, "clientsettings.json"), + JSON.stringify({ stringSettings: { sessionkey: "our-own-stale-key", sessionsignature: "our-own-signature", mptoken: null, playeruid: "1" } }), + "utf-8" + ) + + const { getAccountSecrets } = await import("@src/ipc/accountStore") + vi.mocked(getAccountSecrets).mockResolvedValueOnce(null) + + const event = await createTrustedEvent() + const result = await executeGameHandler()(event, { version: "1.20.0", path: gameVersionFolder }, baseInstallation({ path: installationFolder })) + assert.deepEqual(result, { ok: false, reason: "launch-failed" }) + + const { readFileSync } = await import("node:fs") + const settings = JSON.parse(readFileSync(join(installationFolder, "clientsettings.json"), "utf-8")) + assert.equal(settings.stringSettings.sessionkey, "our-own-stale-key", "our own session, even a stale one, is left exactly as it was") + }) + + it("resolves session-write-failed when a foreign session cannot be cleared", async () => { + const gameVersionFolder = join(versionsFolder, "1.20.0") + const installationFolder = join(managedFolder, "Main") + mkdirSync(gameVersionFolder, { recursive: true }) + mkdirSync(installationFolder, { recursive: true }) + writeFileSync(join(gameVersionFolder, "Vintagestory"), "", "utf-8") + writeConfig({ + gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], + accounts: [{ email: "player@example.com", playerName: "Player", playerUid: "1", playerEntitlements: null, hostGameServer: false }], + activeAccountId: "1" + }) + const settingsPath = join(installationFolder, "clientsettings.json") + writeFileSync(settingsPath, JSON.stringify({ stringSettings: { sessionkey: "housemate-key", playeruid: "housemate-uid" } }), "utf-8") + + const { getAccountSecrets } = await import("@src/ipc/accountStore") + vi.mocked(getAccountSecrets).mockResolvedValueOnce(null) + + // The file already exists, so it is the FILE's own permissions that gate the truncate-write + // the clear step needs, not the directory's: unlike the "cannot be written" test above, where + // the file has to be created from nothing and a read-only directory is what blocks that. + chmodSync(settingsPath, 0o400) + try { + const event = await createTrustedEvent() + const result = await executeGameHandler()(event, { version: "1.20.0", path: gameVersionFolder }, baseInstallation({ path: installationFolder })) + assert.deepEqual(result, { ok: false, reason: "session-write-failed" }) + } finally { + chmodSync(settingsPath, 0o600) + } + }) + /** * Issue #204: the launcher's stored session gets invalidated by a login * somewhere else, the game asks the player to log in and writes a working @@ -279,7 +392,8 @@ describe("EXECUTE_GAME", () => { writeFileSync(join(gameVersionFolder, "Vintagestory"), "not a real binary", { mode: 0o644 }) writeConfig({ gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], - account: { email: "player@example.com", playerName: "Player", playerUid: "1", playerEntitlements: null, hostGameServer: false } as unknown as ConfigType["account"] + accounts: [{ email: "player@example.com", playerName: "Player", playerUid: "1", playerEntitlements: null, hostGameServer: false }], + activeAccountId: "1" }) // What the game leaves behind after prompting: same account (playeruid "1" // is the one in the config above), a key the launcher has never seen. @@ -296,7 +410,7 @@ describe("EXECUTE_GAME", () => { const event = await createTrustedEvent() await executeGameHandler()(event, { version: "1.20.0", path: gameVersionFolder }, baseInstallation({ path: installationFolder })) - assert.deepEqual(vi.mocked(saveAccountSecrets).mock.calls, [[{ sessionKey: GAME_REFRESHED_KEY, sessionSignature: "game-session-signature", mptoken: "game-mp-token" }]]) + assert.deepEqual(vi.mocked(saveAccountSecrets).mock.calls, [["1", { sessionKey: GAME_REFRESHED_KEY, sessionSignature: "game-session-signature", mptoken: "game-mp-token" }]]) const { readFileSync } = await import("node:fs") const settings = JSON.parse(readFileSync(join(installationFolder, "clientsettings.json"), "utf-8")) @@ -312,7 +426,8 @@ describe("EXECUTE_GAME", () => { writeFileSync(join(gameVersionFolder, "Vintagestory"), "not a real binary", { mode: 0o644 }) writeConfig({ gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], - account: { email: "player@example.com", playerName: "Player", playerUid: "1", playerEntitlements: null, hostGameServer: false } as unknown as ConfigType["account"] + accounts: [{ email: "player@example.com", playerName: "Player", playerUid: "1", playerEntitlements: null, hostGameServer: false }], + activeAccountId: "1" }) writeFileSync( join(installationFolder, "clientsettings.json"), diff --git a/tests/ipc/modsHandlers.test.ts b/tests/ipc/modsHandlers.test.ts index 753cc063..b57ad98d 100644 --- a/tests/ipc/modsHandlers.test.ts +++ b/tests/ipc/modsHandlers.test.ts @@ -85,7 +85,8 @@ beforeEach(async () => { defaultVersionsFolder: join(temporaryRoot, "Versions"), backupsFolder: join(temporaryRoot, "Backups"), window: { width: 1280, height: 720, x: 0, y: 0, maximized: false }, - account: null, + accounts: [], + activeAccountId: null, installations: [{ name: "test", path: modsFolder, gameVersion: "", startParams: "", mesaGlThread: false, envVars: "", backups: [] }], gameVersions: [], favMods: [], diff --git a/tests/ipc/pathsHandlers.test.ts b/tests/ipc/pathsHandlers.test.ts index f35c4e15..bf0722d5 100644 --- a/tests/ipc/pathsHandlers.test.ts +++ b/tests/ipc/pathsHandlers.test.ts @@ -100,7 +100,8 @@ function writeConfig(config: Partial): void { defaultVersionsFolder: versionsFolder, backupsFolder, window: { width: 1280, height: 720, x: 0, y: 0, maximized: false }, - account: null, + accounts: [], + activeAccountId: null, installations: [], gameVersions: [], favMods: [], diff --git a/tests/ipc/pathsHandlersWin32.test.ts b/tests/ipc/pathsHandlersWin32.test.ts index 9cefe35e..68ff1467 100644 --- a/tests/ipc/pathsHandlersWin32.test.ts +++ b/tests/ipc/pathsHandlersWin32.test.ts @@ -14,6 +14,7 @@ import "./helpers/electronMock" import { createTrustedEvent, getIpcHandler, setElectronPath, setElectronUserDataPath } from "./helpers/electronMock" import { IPC_CHANNELS } from "@src/ipc/ipcChannels" +import { CURRENT_CONFIG_SCHEMA } from "@domain/config/migrations" /** * Branch coverage for RUN_INSTALLER's win32-only arms in @@ -95,15 +96,20 @@ let managedFolder: string let versionsFolder: string let userDataFolder: string +// Written already at the current schema: an older schema number here would make every +// getConfig() call run the schema pipeline for real, including its file-backup and +// account-store side effects, which is extra async work these tests' real-timer budgets +// were never sized to absorb. function writeConfig(config: Partial): void { const fullConfig = { - schemaVersion: 3, + schemaVersion: CURRENT_CONFIG_SCHEMA, lastUsedInstallation: null, defaultInstallationsFolder: managedFolder, defaultVersionsFolder: versionsFolder, backupsFolder: join(temporaryRoot, "Backups"), window: { width: 1280, height: 720, x: 0, y: 0, maximized: false }, - account: null, + accounts: [], + activeAccountId: null, installations: [], gameVersions: [], favMods: [], diff --git a/tests/renderer-dom/helpers/windowApi.ts b/tests/renderer-dom/helpers/windowApi.ts index 6e474871..0fb42339 100644 --- a/tests/renderer-dom/helpers/windowApi.ts +++ b/tests/renderer-dom/helpers/windowApi.ts @@ -36,7 +36,8 @@ export function createMockConfig(overrides: Partial = {}): ConfigTyp defaultVersionsFolder: "", backupsFolder: "", window: { width: 1280, height: 720, x: 0, y: 0, maximized: false }, - account: null, + accounts: [], + activeAccountId: null, installations: [], gameVersions: [], favMods: [], @@ -120,7 +121,7 @@ export function createMockWindowApi(overrides: WindowApiOverrides = {}): MockedB }, accountManager: { login: vi.fn(notMocked("accountManager.login")), - logout: vi.fn(notMocked("accountManager.logout")) + removeAccount: vi.fn(notMocked("accountManager.removeAccount")) } } diff --git a/tests/renderer-dom/sessionButtonSwitchAccount.test.tsx b/tests/renderer-dom/sessionButtonSwitchAccount.test.tsx new file mode 100644 index 00000000..d337e397 --- /dev/null +++ b/tests/renderer-dom/sessionButtonSwitchAccount.test.tsx @@ -0,0 +1,105 @@ +import { describe, expect, it, vi } from "vitest" +import { screen } from "@testing-library/react" +import userEvent from "@testing-library/user-event" + +import SessionButton from "@renderer/components/ui/SessionButton" +import NotificationsOverlay from "@renderer/components/layout/NotificationsOverlay" + +import { createMockConfig, installMockWindowApi } from "./helpers/windowApi" +import { renderWithProviders } from "./helpers/render" + +const ACCOUNT_A: AccountPublicType = { email: "a@example.com", playerName: "Alice", playerUid: "uid-a", playerEntitlements: null, hostGameServer: false } +const ACCOUNT_B: AccountPublicType = { email: "b@example.com", playerName: "Bob", playerUid: "uid-b", playerEntitlements: null, hostGameServer: false } + +describe("SessionButton with more than one saved account", () => { + it("shows the active account and lists every saved account once opened", async () => { + const user = userEvent.setup() + installMockWindowApi({ + configManager: { getConfig: async () => createMockConfig({ accounts: [ACCOUNT_A, ACCOUNT_B], activeAccountId: ACCOUNT_A.playerUid }) } + }) + + renderWithProviders() + + const trigger = await screen.findByRole("button", { name: "Alice" }) + await user.click(trigger) + + expect(await screen.findByRole("option", { name: /Bob/ })).toBeTruthy() + expect(screen.getByRole("option", { name: /Add another account/ })).toBeTruthy() + expect(screen.getByRole("option", { name: "Remove Alice" })).toBeTruthy() + }) + + it("switching to another account persists it as the new active one", async () => { + const user = userEvent.setup() + const saveConfig = vi.fn<(config: ConfigType) => Promise>(async () => ({ ok: true })) + installMockWindowApi({ + configManager: { getConfig: async () => createMockConfig({ accounts: [ACCOUNT_A, ACCOUNT_B], activeAccountId: ACCOUNT_A.playerUid }), saveConfig } + }) + + renderWithProviders() + + await user.click(await screen.findByRole("button", { name: "Alice" })) + await user.click(await screen.findByRole("option", { name: /Bob/ })) + + await screen.findByRole("button", { name: "Bob" }) + const lastSaved = saveConfig.mock.calls.at(-1)?.[0] + expect(lastSaved?.activeAccountId).toBe("uid-b") + }) + + it("opens the login form from the switcher's add option", async () => { + const user = userEvent.setup() + installMockWindowApi({ + configManager: { getConfig: async () => createMockConfig({ accounts: [ACCOUNT_A], activeAccountId: ACCOUNT_A.playerUid }) } + }) + + renderWithProviders() + + await user.click(await screen.findByRole("button", { name: "Alice" })) + await user.click(await screen.findByRole("option", { name: /Add another account/ })) + + expect(await screen.findByPlaceholderText("Email")).toBeTruthy() + }) + + it("removes the active account on confirm", async () => { + const user = userEvent.setup() + const removeAccount = vi.fn(async () => true) + installMockWindowApi({ + configManager: { getConfig: async () => createMockConfig({ accounts: [ACCOUNT_A, ACCOUNT_B], activeAccountId: ACCOUNT_A.playerUid }) }, + accountManager: { removeAccount } + }) + + renderWithProviders() + + await user.click(await screen.findByRole("button", { name: "Alice" })) + await user.click(await screen.findByRole("option", { name: "Remove Alice" })) + await user.click(await screen.findByRole("button", { name: "Remove Alice" })) + + expect(removeAccount).toHaveBeenCalledWith("uid-a") + // Alice is gone, Bob is promoted and shown on the trigger. + expect(await screen.findByRole("button", { name: "Bob" })).toBeTruthy() + }) + + it("leaves the account in the list and shows an error when removal fails", async () => { + const user = userEvent.setup() + const removeAccount = vi.fn(async () => false) + installMockWindowApi({ + configManager: { getConfig: async () => createMockConfig({ accounts: [ACCOUNT_A], activeAccountId: ACCOUNT_A.playerUid }) }, + accountManager: { removeAccount } + }) + + renderWithProviders( + <> + + + + ) + + await user.click(await screen.findByRole("button", { name: "Alice" })) + await user.click(await screen.findByRole("option", { name: "Remove Alice" })) + await user.click(await screen.findByRole("button", { name: "Remove Alice" })) + + expect(removeAccount).toHaveBeenCalledWith("uid-a") + expect(await screen.findByText("Couldn't remove that account. Try again.")).toBeTruthy() + // Still there: the trigger keeps showing Alice, not falling back to "Log in". + expect(screen.getByRole("button", { name: "Alice" })).toBeTruthy() + }) +}) diff --git a/tests/renderer-dom/shellSessionButton.test.tsx b/tests/renderer-dom/shellSessionButton.test.tsx index 7903b543..fbe735a6 100644 --- a/tests/renderer-dom/shellSessionButton.test.tsx +++ b/tests/renderer-dom/shellSessionButton.test.tsx @@ -8,16 +8,16 @@ import NotificationsOverlay from "@renderer/components/layout/NotificationsOverl import { createMockConfig, installMockWindowApi } from "./helpers/windowApi" import { renderWithProviders } from "./helpers/render" -const ACCOUNT: AccountPublicType = { playerName: "Steve" } as AccountPublicType +const ACCOUNT: AccountPublicType = { email: "steve@example.com", playerName: "Steve", playerUid: "steve-uid", playerEntitlements: null, hostGameServer: false } describe("SessionButton", () => { - it("opens the logged-in menu and calls the account hook's logout on confirm", async () => { + it("opens the account switcher and calls the account hook's remove on confirm", async () => { const user = userEvent.setup() - const logout = vi.fn(async () => true) + const removeAccount = vi.fn(async () => true) installMockWindowApi({ - configManager: { getConfig: async () => createMockConfig({ account: ACCOUNT }) }, - accountManager: { logout } + configManager: { getConfig: async () => createMockConfig({ accounts: [ACCOUNT], activeAccountId: ACCOUNT.playerUid }) }, + accountManager: { removeAccount } }) renderWithProviders() @@ -25,10 +25,13 @@ describe("SessionButton", () => { const button = await screen.findByRole("button", { name: "Steve" }) await user.click(button) - const logoutConfirm = await screen.findByRole("button", { name: "Log out" }) - await user.click(logoutConfirm) + const removeOption = await screen.findByRole("option", { name: "Remove Steve" }) + await user.click(removeOption) - expect(logout).toHaveBeenCalledTimes(1) + const removeConfirm = await screen.findByRole("button", { name: "Remove Steve" }) + await user.click(removeConfirm) + + expect(removeAccount).toHaveBeenCalledWith("steve-uid") }) it("reports the service as unreachable when the login call throws, not bad credentials", async () => { diff --git a/tests/renderer/configReducer.test.ts b/tests/renderer/configReducer.test.ts index a441535f..ff21782e 100644 --- a/tests/renderer/configReducer.test.ts +++ b/tests/renderer/configReducer.test.ts @@ -27,7 +27,8 @@ function baseConfig(overrides: Partial = {}): ConfigType { defaultVersionsFolder: "/versions", backupsFolder: "/backups", window: { width: 1280, height: 720, x: 0, y: 0, maximized: false }, - account: null, + accounts: [], + activeAccountId: null, installations: [], gameVersions: [], favMods: [], @@ -165,15 +166,73 @@ describe("configReducer: scalar setters", () => { assert.equal(optedOut.receiveBetaUpdates, false) assert.equal(optedOut.installations, config.installations) }) +}) - it("SET_ACCOUNT accepts an account and null alike", () => { - const config = baseConfig() - const account: AccountType = { email: "a@b.c", playerName: "A", playerUid: "1", playerEntitlements: null, hostGameServer: false } - const withAccount = configReducer(config, { type: CONFIG_ACTIONS.SET_ACCOUNT, payload: account }) - assert.deepEqual(withAccount.account, account) +describe("configReducer: accounts", () => { + const accountA: AccountType = { email: "a@b.c", playerName: "A", playerUid: "uid-a", playerEntitlements: null, hostGameServer: false } + const accountB: AccountType = { email: "b@b.c", playerName: "B", playerUid: "uid-b", playerEntitlements: null, hostGameServer: false } + + it("ADD_ACCOUNT appends a new account and makes it active", () => { + const config = baseConfig({ accounts: [accountA], activeAccountId: "uid-a" }) + const result = configReducer(config, { type: CONFIG_ACTIONS.ADD_ACCOUNT, payload: accountB }) + + assert.deepEqual(result.accounts, [accountA, accountB]) + assert.equal(result.activeAccountId, "uid-b") + }) + + it("ADD_ACCOUNT on the same playerUid twice replaces the entry in place rather than duplicating it", () => { + const config = baseConfig({ accounts: [accountA], activeAccountId: "uid-a" }) + const refreshed = { ...accountA, playerName: "A refreshed" } + + const result = configReducer(config, { type: CONFIG_ACTIONS.ADD_ACCOUNT, payload: refreshed }) + + assert.deepEqual(result.accounts, [refreshed]) + assert.equal(result.activeAccountId, "uid-a") + }) + + it("REMOVE_ACCOUNT drops the matching account and leaves the rest untouched", () => { + const config = baseConfig({ accounts: [accountA, accountB], activeAccountId: "uid-b" }) + const result = configReducer(config, { type: CONFIG_ACTIONS.REMOVE_ACCOUNT, payload: { playerUid: "uid-a" } }) + + assert.deepEqual(result.accounts, [accountB]) + assert.equal(result.activeAccountId, "uid-b", "removing a non-active account leaves the active choice alone") + }) + + it("REMOVE_ACCOUNT promotes the first remaining account when the active one is removed", () => { + const config = baseConfig({ accounts: [accountA, accountB], activeAccountId: "uid-a" }) + const result = configReducer(config, { type: CONFIG_ACTIONS.REMOVE_ACCOUNT, payload: { playerUid: "uid-a" } }) + + assert.deepEqual(result.accounts, [accountB]) + assert.equal(result.activeAccountId, "uid-b") + }) + + it("REMOVE_ACCOUNT on the last account leaves activeAccountId null", () => { + const config = baseConfig({ accounts: [accountA], activeAccountId: "uid-a" }) + const result = configReducer(config, { type: CONFIG_ACTIONS.REMOVE_ACCOUNT, payload: { playerUid: "uid-a" } }) + + assert.deepEqual(result.accounts, []) + assert.equal(result.activeAccountId, null) + }) + + it("SET_ACTIVE_ACCOUNT switches to a saved account", () => { + const config = baseConfig({ accounts: [accountA, accountB], activeAccountId: "uid-a" }) + const result = configReducer(config, { type: CONFIG_ACTIONS.SET_ACTIVE_ACCOUNT, payload: "uid-b" }) + + assert.equal(result.activeAccountId, "uid-b") + }) + + it("SET_ACTIVE_ACCOUNT accepts null, clearing the active choice", () => { + const config = baseConfig({ accounts: [accountA], activeAccountId: "uid-a" }) + const result = configReducer(config, { type: CONFIG_ACTIONS.SET_ACTIVE_ACCOUNT, payload: null }) + + assert.equal(result.activeAccountId, null) + }) + + it("SET_ACTIVE_ACCOUNT on an id naming nobody is a no-op, same state object back", () => { + const config = baseConfig({ accounts: [accountA], activeAccountId: "uid-a" }) + const result = configReducer(config, { type: CONFIG_ACTIONS.SET_ACTIVE_ACCOUNT, payload: "uid-nobody" }) - const loggedOut = configReducer(withAccount, { type: CONFIG_ACTIONS.SET_ACCOUNT, payload: null }) - assert.equal(loggedOut.account, null) + assert.equal(result, config, "nothing switched, so nothing downstream should see new state") }) }) From 4b9f41d8b544d4f9c419a3d347fc3cf4d5ed656f Mon Sep 17 00:00:00 2001 From: Zaldaryon <273555259+Zaldaryon@users.noreply.github.com> Date: Wed, 26 Aug 2026 21:22:03 -0300 Subject: [PATCH 2/3] fix(account): pin launch-time account choice, retry a burned re-key Responds to Pixnop's review on PR #253. Nothing pinned that EXECUTE_GAME picks the account to launch as from config.activeAccountId, not the first saved account: every fixture in gameHandlers.test.ts saved exactly one account, so a handler that ignored activeAccountId entirely would have stayed green. Two new tests close that: one asserts a launch with two saved accounts signs in as the active one and writes that uid into clientsettings.json, not the first account in the list; the other asserts a session the game wrote for a different saved account is overwritten rather than adopted into the active account's store entry, pinning the call site finding 1 was actually about (the domain guard itself was already covered by clientSettings.test.ts). The secret-store re-key (migrateAccountStore) got exactly one chance: it read the uid to re-key by from the raw document's legacy `account` field, which the schema 3 to 4 step deletes, so a locked keyring on that one launch stranded the account at schema 4 with an unreadable v1 store and no way to retry. The literal fix the review sketched, holding the config back at schema 3 until the re-key succeeds, does not work here: normalizeConfig drops any field it does not recognize, account included, and the renderer saves the whole config back within seconds of launch, so a config held at schema 3 loses the account record outright on that very first save, which is worse than the strand. Instead, migrateAccountStore now falls back to the already-migrated config's activeAccountId when the raw document has no legacy account field, so it retries on every later launch until the v1 store is gone or re-keyed. adoptLegacySingleAccountSecrets is already a no-op once that is true, so the retry costs one failed read per launch and nothing else. A new configManager.test.ts case pins the retry across two simulated launches, including that the on-disk config actually reaches schema 4 with no account field left after the first, burned attempt. Also, from the same review: - backupConfigBeforeMigration's gate now covers hadLegacyAccountSecrets and reKeyedAccountStore too, not only a schema-version migration: both paths save a real document and neither took a backup before. - Removed listStoredAccountIds (zero production callers) and AccountContext/useAccount/the activeAccount and account memos in ConfigContext.tsx: SessionButton already reads useAccountList, and nothing else in src/ still calls useAccount. Verified both fixes with a mutation check: reverting the account pick to config.accounts[0] turns the two new gameHandlers tests red, and reverting migrateAccountStore's activeAccountId fallback turns the new retry test red. Both reverted after confirming. --- src/config/configManager.ts | 44 +++++++--- src/ipc/accountStore.ts | 5 -- .../config/contexts/ConfigContext.tsx | 51 ++++------- tests/ipc/accountStore.test.ts | 20 +---- tests/ipc/configManager.test.ts | 22 +++++ tests/ipc/gameHandlers.test.ts | 85 ++++++++++++++++++- .../renderer-dom/configContextSlices.test.tsx | 4 +- 7 files changed, 157 insertions(+), 74 deletions(-) diff --git a/src/config/configManager.ts b/src/config/configManager.ts index b218daae..c5a0a651 100644 --- a/src/config/configManager.ts +++ b/src/config/configManager.ts @@ -108,13 +108,16 @@ export async function getConfig(): Promise { if (configCache) return normalizeConfig(configCache) const config = await fse.readJSON(configPath, "utf-8") const hadLegacyAccountSecrets = await migrateLegacyAccount(config) - const reKeyedAccountStore = await migrateAccountStore(config) const migration = migrateConfigDocument(config) logConfigMigration(migration) - if (migration.applied.length > 0) await backupConfigBeforeMigration() const ensuredConfig = normalizeConfig(migration.doc) + const reKeyedAccountStore = await migrateAccountStore(config, ensuredConfig) + // Every path that overwrites config.json gets the same backup, not just the schema pipeline: + // a re-key or a legacy-secrets migration writes just as real a document as a schema bump does. + const mustSave = hadLegacyAccountSecrets || reKeyedAccountStore || migration.applied.length > 0 + if (mustSave) await backupConfigBeforeMigration() configCache = ensuredConfig - if (hadLegacyAccountSecrets || reKeyedAccountStore || migration.applied.length > 0) await saveConfig(ensuredConfig) + if (mustSave) await saveConfig(ensuredConfig) return ensuredConfig } catch (err) { logMessage("error", `[back] [config] [config/configManager.ts] [getConfig] Error getting config at ${configPath}. Using default config.`) @@ -199,26 +202,39 @@ async function migrateLegacyAccount(config: unknown): Promise { * * The v1 store held one `AccountSecrets` with no account attached to it; the * v2 store (see `src/ipc/accountStore.ts`) holds entries keyed by - * `playerUid`. The only place that missing key exists is - * `config.account.playerUid`, which is why this runs here, reading the raw - * pre-pipeline document exactly as {@link migrateLegacyAccount} does, and not + * `playerUid`. The only place that missing key exists pre-migration is + * `config.account.playerUid`, which is why this reads the raw pre-pipeline + * document first, exactly as {@link migrateLegacyAccount} does, and not * inside `accountStore.ts` itself: the store cannot name its own contents. * Kept out of the schema pipeline for the same reason `migrateLegacyAccount` * is: a side effect the pure domain layer is not allowed to have. * - * A config with no readable account, or a v1 store that has already been - * re-keyed (or never existed), makes this a safe no-op: `false` either way, - * same as `migrateLegacyAccount` when there is nothing to do. + * Falls back to the already-migrated config's `activeAccountId` when the raw + * document has no legacy `account` field, which is what makes this retry + * automatically on every later launch rather than only the one that first + * saw the legacy field. `adoptLegacySingleAccountSecrets` is a no-op once the + * v1 file is gone or already re-keyed, so calling it again costs one failed + * read and nothing else; there is no separate "already migrated" flag to + * gate this on. Leaving the schema-4 commit itself ungated matters just as + * much as the retry: `normalizeConfig` drops any field it does not know, + * `account` included, so holding the document back at schema 3 would lose + * the account record outright on the very next save, which is worse than + * the stranded session this retry is fixing. + * + * A config with no readable account (raw or migrated), or a v1 store that + * has already been re-keyed (or never existed), makes this a safe no-op: + * `false` either way, same as `migrateLegacyAccount` when there is nothing + * to do. */ -async function migrateAccountStore(config: unknown): Promise { - if (!isRecord(config) || !isRecord(config.account)) return false - const uid = config.account.playerUid - if (typeof uid !== "string" || uid.length === 0) return false +async function migrateAccountStore(legacyDocument: unknown, config: ConfigType): Promise { + const legacyUid = isRecord(legacyDocument) && isRecord(legacyDocument.account) ? legacyDocument.account.playerUid : null + const uid = typeof legacyUid === "string" && legacyUid.length > 0 ? legacyUid : config.activeAccountId + if (uid === null || uid.length === 0) return false try { return await adoptLegacySingleAccountSecrets(uid) } catch { - logMessage("warn", "[back] [config] [configManager.ts] The stored account session was not carried into the multi-account store.") + logMessage("warn", "[back] [config] [configManager.ts] The stored account session was not carried into the multi-account store. Retrying on the next launch.") return false } } diff --git a/src/ipc/accountStore.ts b/src/ipc/accountStore.ts index ee32e853..2f7b30f4 100644 --- a/src/ipc/accountStore.ts +++ b/src/ipc/accountStore.ts @@ -176,8 +176,3 @@ export async function adoptLegacySingleAccountSecrets(accountId: string): Promis await writeAccounts(new Map([[accountId, secrets]])) return true } - -/** Every account id the store currently holds secrets for. */ -export async function listStoredAccountIds(): Promise { - return Array.from((await readAccounts()).keys()) -} diff --git a/src/renderer/src/features/config/contexts/ConfigContext.tsx b/src/renderer/src/features/config/contexts/ConfigContext.tsx index b94966fe..5e62f316 100644 --- a/src/renderer/src/features/config/contexts/ConfigContext.tsx +++ b/src/renderer/src/features/config/contexts/ConfigContext.tsx @@ -36,11 +36,8 @@ const EMPTY_NOTIFIED_MOD_UPDATES: string[] = [] const ConfigDispatchContext = createContext | null>(null) const InstallationsContext = createContext(null) const GameVersionsContext = createContext(null) -// The account is wrapped: `null` is a legitimate value (logged out), so it -// cannot double as the "no provider above me" sentinel the other contexts use. -const AccountContext = createContext<{ account: AccountType | null } | null>(null) -// Wrapped for the same reason: activeAccountId is legitimately null with a non-empty list, so -// the wrapper (not the field) is what tells a missing provider apart from a real logged-out state. +// Wrapped: activeAccountId is legitimately null with a non-empty list, so the wrapper (not the +// field) is what tells a missing provider apart from a real logged-out state. const AccountListContext = createContext<{ accounts: AccountPublicType[]; activeAccountId: string | null } | null>(null) const SettingsContext = createContext(null) const FavModsContext = createContext(null) @@ -125,8 +122,6 @@ const ConfigProvider = ({ children }: { children: React.ReactNode }): JSX.Elemen // The list slices are handed out as-is: the reducer never rebuilds an array // it did not change, so their identity already tracks their content. Only the // composed slices need memoising to stay stable across unrelated actions. - const activeAccount = useMemo(() => config.accounts.find((candidate) => candidate.playerUid === config.activeAccountId) ?? null, [config.accounts, config.activeAccountId]) - const account = useMemo(() => ({ account: activeAccount }), [activeAccount]) const accountList = useMemo(() => ({ accounts: config.accounts, activeAccountId: config.activeAccountId }), [config.accounts, config.activeAccountId]) const settings = useMemo( @@ -160,19 +155,17 @@ const ConfigProvider = ({ children }: { children: React.ReactNode }): JSX.Elemen - - - - - - - {children} - - - - - - + + + + + + {children} + + + + + @@ -191,9 +184,7 @@ const useInstallations = (): InstallationType[] => requireProvider(useContext(In const useGameVersions = (): GameVersionType[] => requireProvider(useContext(GameVersionsContext), "useGameVersions") -const useAccount = (): AccountType | null => requireProvider(useContext(AccountContext), "useAccount").account - -/** Every saved account, and which one is active. For a switcher; a single logged-in read stays `useAccount`. */ +/** Every saved account, and which one is active. */ const useAccountList = (): { accounts: AccountPublicType[]; activeAccountId: string | null } => requireProvider(useContext(AccountListContext), "useAccountList") /** Folders, window geometry, schema version and the last used installation. */ @@ -209,16 +200,4 @@ const useCustomIcons = (): IconType[] => requireProvider(useContext(CustomIconsC /** Ids of installations the player has already been told about mod updates for, this session. */ const useNotifiedModUpdates = (): string[] => requireProvider(useContext(NotifiedModUpdatesContext), "useNotifiedModUpdates") -export { - ConfigProvider, - useConfigDispatch, - useInstallations, - useGameVersions, - useAccount, - useAccountList, - useSettingsConfig, - useFavMods, - useSuspendedModUpdates, - useCustomIcons, - useNotifiedModUpdates -} +export { ConfigProvider, useConfigDispatch, useInstallations, useGameVersions, useAccountList, useSettingsConfig, useFavMods, useSuspendedModUpdates, useCustomIcons, useNotifiedModUpdates } diff --git a/tests/ipc/accountStore.test.ts b/tests/ipc/accountStore.test.ts index fd8e0c4a..9a8ac657 100644 --- a/tests/ipc/accountStore.test.ts +++ b/tests/ipc/accountStore.test.ts @@ -158,7 +158,10 @@ describe("saveAccountSecrets", () => { const reader = await loadStore() assert.deepEqual(await reader.getAccountSecrets("uid-a"), { ...ACCOUNT_A, sessionKey: "placeholder-session-key-a-refreshed" }) - assert.deepEqual(await reader.listStoredAccountIds(), ["uid-a"]) + + const stored = JSON.parse(readFileSync(storePath(), "utf8")) + const payload = JSON.parse(Buffer.from(stored.ciphertext, "base64").toString("utf8").slice("sealed:".length)) + assert.equal(payload.accounts.length, 1, "one entry replaced in place, not a second one appended") }) it("refuses to write when the platform offers no encryption", async () => { @@ -362,21 +365,6 @@ describe("removeAccountSecrets", () => { }) }) -describe("listStoredAccountIds", () => { - it("lists every id the store currently holds secrets for", async () => { - const store = await loadStore() - await store.saveAccountSecrets("uid-a", ACCOUNT_A) - await store.saveAccountSecrets("uid-b", ACCOUNT_B) - - assert.deepEqual(new Set(await store.listStoredAccountIds()), new Set(["uid-a", "uid-b"])) - }) - - it("is empty when nothing has ever been saved", async () => { - const store = await loadStore() - assert.deepEqual(await store.listStoredAccountIds(), []) - }) -}) - describe("adoptLegacySingleAccountSecrets", () => { it("re-keys a v1 store under the given account id", async () => { writeLegacyStoreFile(ACCOUNT_A) diff --git a/tests/ipc/configManager.test.ts b/tests/ipc/configManager.test.ts index 63d52c36..71fe7ee1 100644 --- a/tests/ipc/configManager.test.ts +++ b/tests/ipc/configManager.test.ts @@ -676,6 +676,28 @@ describe("getConfig: account store re-key migration", () => { assert.equal(config.activeAccountId, "uid-throws", "the document still migrates even though re-keying the store failed") }) + + it("asks again on the next launch when a locked keyring burned the first attempt", async () => { + const legacyDoc = { ...minimalConfig(), account: { email: "a@b.c", playerName: "A", playerUid: "uid-retry", playerEntitlements: null, hostGameServer: false } } + writeFileSync(join(userDataFolder, "config.json"), JSON.stringify(legacyDoc), "utf-8") + vi.mocked(adoptLegacySingleAccountSecrets).mockRejectedValueOnce(new Error("keyring locked")) + + const first = await freshConfigManager() + await first.getConfig() + await first.flushConfigWrites() + + // The commit the retry has to survive: schema 4 on disk, with no `account` field left to key the store by. + const fse = (await import("fs-extra")).default + const onDisk = await fse.readJSON(join(userDataFolder, "config.json")) + assert.equal(onDisk.schemaVersion, CURRENT_CONFIG_SCHEMA) + assert.equal("account" in onDisk, false) + + const second = await freshConfigManager() + const config = await second.getConfig() + + assert.deepEqual(vi.mocked(adoptLegacySingleAccountSecrets).mock.calls, [["uid-retry"], ["uid-retry"]], "a v1 store still on disk is retried, keyed by the same account") + assert.equal(config.activeAccountId, "uid-retry") + }) }) describe("getConfig: config.json backup before a schema migration", () => { diff --git a/tests/ipc/gameHandlers.test.ts b/tests/ipc/gameHandlers.test.ts index 780ec773..1a32c08b 100644 --- a/tests/ipc/gameHandlers.test.ts +++ b/tests/ipc/gameHandlers.test.ts @@ -38,7 +38,8 @@ import { CURRENT_CONFIG_SCHEMA } from "@domain/config/migrations" */ vi.mock("@src/ipc/accountStore", () => ({ getAccountSecrets: vi.fn(async () => ({ mptoken: null, sessionKey: "session-key", sessionSignature: "session-signature" })), - saveAccountSecrets: vi.fn(async () => undefined) + saveAccountSecrets: vi.fn(async () => undefined), + adoptLegacySingleAccountSecrets: vi.fn(async () => false) })) type ExecuteGameHandler = (event: IpcMainInvokeEvent, version: unknown, installation: unknown) => Promise @@ -455,6 +456,88 @@ describe("EXECUTE_GAME", () => { ) } }) + + /** + * The account a launch signs in as is the ACTIVE one, not the first saved one. + * Every other fixture in this file saves exactly one account, which makes those + * two indistinguishable: a handler that ignored activeAccountId entirely would + * stay green through all of them (PR #253 review, finding 1). Two accounts, and + * the uid that lands in clientsettings.json is what tells them apart. + */ + it("signs in as the active account, not the first one saved", async () => { + const gameVersionFolder = join(versionsFolder, "1.20.0") + const installationFolder = join(managedFolder, "Main") + mkdirSync(gameVersionFolder, { recursive: true }) + mkdirSync(installationFolder, { recursive: true }) + writeFileSync(join(gameVersionFolder, "Vintagestory"), "not a real binary", { mode: 0o644 }) + writeConfig({ + gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], + accounts: [ + { email: "alice@example.com", playerName: "Alice", playerUid: "uid-a", playerEntitlements: null, hostGameServer: false }, + { email: "bob@example.com", playerName: "Bob", playerUid: "uid-b", playerEntitlements: null, hostGameServer: false } + ], + activeAccountId: "uid-b" + }) + + const { getAccountSecrets } = await import("@src/ipc/accountStore") + vi.mocked(getAccountSecrets).mockClear() + + const event = await createTrustedEvent() + await executeGameHandler()(event, { version: "1.20.0", path: gameVersionFolder }, baseInstallation({ path: installationFolder })) + + assert.deepEqual(vi.mocked(getAccountSecrets).mock.calls, [["uid-b"]], "the session is read out of the active account's store entry") + + const { readFileSync } = await import("node:fs") + const settings = JSON.parse(readFileSync(join(installationFolder, "clientsettings.json"), "utf-8")) + assert.equal(settings.stringSettings.playeruid, "uid-b", "switch to Bob and the next launch signs Bob in") + assert.equal(settings.stringSettings.playername, "Bob") + assert.equal(settings.stringSettings.useremail, "bob@example.com") + }) + + /** + * Adoption is keyed on the account being launched, so a session the file holds + * for SOMEBODY ELSE is overwritten, never carried into our own store entry. The + * domain guard is pinned by clientSettings.test.ts (#209); this pins the call + * site, which a single-account fixture leaves free to be keyed on anything. + */ + it("overwrites another player's refreshed session instead of adopting it into the active account", async () => { + const gameVersionFolder = join(versionsFolder, "1.20.0") + const installationFolder = join(managedFolder, "Main") + mkdirSync(gameVersionFolder, { recursive: true }) + mkdirSync(installationFolder, { recursive: true }) + writeFileSync(join(gameVersionFolder, "Vintagestory"), "not a real binary", { mode: 0o644 }) + writeConfig({ + gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], + accounts: [ + { email: "alice@example.com", playerName: "Alice", playerUid: "uid-a", playerEntitlements: null, hostGameServer: false }, + { email: "bob@example.com", playerName: "Bob", playerUid: "uid-b", playerEntitlements: null, hostGameServer: false } + ], + activeAccountId: "uid-b" + }) + // Alice logged in through the game on this installation: a key the launcher has never seen, under her uid. + writeFileSync( + join(installationFolder, "clientsettings.json"), + JSON.stringify({ + stringSettings: { sessionkey: "alices-refreshed-key", sessionsignature: "alices-signature", mptoken: "alices-mp-token", playeruid: "uid-a", playername: "Alice" }, + intSettings: { maxFps: 60 } + }), + "utf-8" + ) + + const { saveAccountSecrets } = await import("@src/ipc/accountStore") + vi.mocked(saveAccountSecrets).mockClear() + + const event = await createTrustedEvent() + await executeGameHandler()(event, { version: "1.20.0", path: gameVersionFolder }, baseInstallation({ path: installationFolder })) + + const { readFileSync } = await import("node:fs") + const settings = JSON.parse(readFileSync(join(installationFolder, "clientsettings.json"), "utf-8")) + assert.equal(settings.stringSettings.playeruid, "uid-b", "a write, not an adoption") + assert.equal(settings.stringSettings.sessionkey, "session-key") + assert.equal(settings.stringSettings.playername, "Bob") + assert.deepEqual(settings.intSettings, { maxFps: 60 }, "everything else in the file survives") + assert.deepEqual(vi.mocked(saveAccountSecrets).mock.calls, [], "another player's key never reaches the active account's store entry") + }) }) describe("LOOK_FOR_A_GAME_VERSION", () => { diff --git a/tests/renderer-dom/configContextSlices.test.tsx b/tests/renderer-dom/configContextSlices.test.tsx index 91e23d89..c8144e7d 100644 --- a/tests/renderer-dom/configContextSlices.test.tsx +++ b/tests/renderer-dom/configContextSlices.test.tsx @@ -2,7 +2,7 @@ import { describe, expect, it, vi } from "vitest" import { render, screen } from "@testing-library/react" import userEvent from "@testing-library/user-event" -import { CONFIG_ACTIONS, useAccount, useConfigDispatch, useCustomIcons, useFavMods, useGameVersions, useInstallations, useSettingsConfig } from "@renderer/features/config/contexts/ConfigContext" +import { CONFIG_ACTIONS, useAccountList, useConfigDispatch, useCustomIcons, useFavMods, useGameVersions, useInstallations, useSettingsConfig } from "@renderer/features/config/contexts/ConfigContext" import { createMockConfig, installMockWindowApi } from "./helpers/windowApi" import { renderWithProviders } from "./helpers/render" @@ -68,7 +68,7 @@ function GameVersionsProbe({ log }: { log: RenderLog }): JSX.Element { function WholeConfigProbe({ log }: { log: RenderLog }): JSX.Element { useInstallations() useGameVersions() - useAccount() + useAccountList() useSettingsConfig() useFavMods() useCustomIcons() From 35c430434f65c2398e92ecc218dfbc533db87ddc Mon Sep 17 00:00:00 2001 From: Zaldaryon <273555259+Zaldaryon@users.noreply.github.com> Date: Thu, 27 Aug 2026 18:58:23 -0300 Subject: [PATCH 3/3] test(account): pin session adoption to the active account's uid Every other fixture in this file saves one account, which makes 'the active account' and 'the first saved account' the same uid, so a call site keyed on either stays green. Two accounts with the second active, and clientsettings.json holding a game-refreshed session under the active account's own uid, separates them: adoptRefreshedSession(config.accounts[0]!.playerUid, ...) turns this test red and nothing else in the file. PR #253 review, finding 1. --- tests/ipc/gameHandlers.test.ts | 47 ++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/ipc/gameHandlers.test.ts b/tests/ipc/gameHandlers.test.ts index 0ef502b2..5e290abf 100644 --- a/tests/ipc/gameHandlers.test.ts +++ b/tests/ipc/gameHandlers.test.ts @@ -562,6 +562,53 @@ describe("EXECUTE_GAME", () => { assert.deepEqual(settings.intSettings, { maxFps: 60 }, "everything else in the file survives") assert.deepEqual(vi.mocked(saveAccountSecrets).mock.calls, [], "another player's key never reaches the active account's store entry") }) + + /** + * The adoption is keyed on the account being launched, the same as the write above + * it. One saved account makes "the active account" and "the first saved account" the + * same uid, so a call site keyed on either stays green through every other fixture + * here; two accounts pull them apart. Keyed on the wrong one, the live session the + * game just refreshed for Bob lands in Alice's store entry, and the next launch as + * Alice signs the player in as Bob (PR #253 review, finding 1). + */ + it("adopts the refreshed session under the active account's uid, not the first saved account's", async () => { + const gameVersionFolder = join(versionsFolder, "1.20.0") + const installationFolder = join(managedFolder, "Main") + mkdirSync(gameVersionFolder, { recursive: true }) + mkdirSync(installationFolder, { recursive: true }) + writeFileSync(join(gameVersionFolder, "Vintagestory"), "not a real binary", { mode: 0o644 }) + writeConfig({ + gameVersions: [{ version: "1.20.0", path: gameVersionFolder }] as unknown as ConfigType["gameVersions"], + accounts: [ + { email: "alice@example.com", playerName: "Alice", playerUid: "uid-a", playerEntitlements: null, hostGameServer: false }, + { email: "bob@example.com", playerName: "Bob", playerUid: "uid-b", playerEntitlements: null, hostGameServer: false } + ], + activeAccountId: "uid-b" + }) + // Bob is the active account and the game refreshed HIS session on this installation: + // his own uid, and a key the launcher has never seen. The #204 adoption case exactly, + // only with a second account saved ahead of him. + writeFileSync( + join(installationFolder, "clientsettings.json"), + JSON.stringify({ + stringSettings: { sessionkey: GAME_REFRESHED_KEY, sessionsignature: "game-session-signature", mptoken: "game-mp-token", playeruid: "uid-b", playername: "Bob" }, + intSettings: { maxFps: 60 } + }), + "utf-8" + ) + + const { saveAccountSecrets } = await import("@src/ipc/accountStore") + vi.mocked(saveAccountSecrets).mockClear() + + const event = await createTrustedEvent() + await executeGameHandler()(event, { version: "1.20.0", path: gameVersionFolder }, baseInstallation({ path: installationFolder })) + + assert.deepEqual( + vi.mocked(saveAccountSecrets).mock.calls, + [["uid-b", { sessionKey: GAME_REFRESHED_KEY, sessionSignature: "game-session-signature", mptoken: "game-mp-token" }]], + "the adopted session is stored under the uid it was issued for, and under no other" + ) + }) }) describe("LOOK_FOR_A_GAME_VERSION", () => {