diff --git a/src/config/configManager.ts b/src/config/configManager.ts index e98431db..b554daad 100644 --- a/src/config/configManager.ts +++ b/src/config/configManager.ts @@ -4,7 +4,7 @@ import { join } from "node:path" import { writeJsonAtomic } from "@src/ipc/atomicJsonFile" 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" @@ -106,8 +106,13 @@ export async function getConfig(): Promise { const migration = migrateConfigDocument(config) logConfigMigration(migration) 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 || 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.`) @@ -176,7 +181,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.") } @@ -187,6 +192,76 @@ 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 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. + * + * 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(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. Retrying on the next launch.") + 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 } @@ -262,6 +337,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 @@ -280,6 +372,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, @@ -293,7 +387,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 c58bffca..4eac0817 100644 --- a/src/ipc/accountStore.ts +++ b/src/ipc/accountStore.ts @@ -2,30 +2,91 @@ 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" import { writeJsonAtomic } from "@src/ipc/atomicJsonFile" 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 contents: EncryptedAccountFile = { version: ACCOUNT_STORE_VERSION, ciphertext: encrypted } @@ -34,32 +95,81 @@ export async function saveAccountSecrets(secrets: AccountSecrets): Promise // matching what this store did before. await writeJsonAtomic(storePath, contents, { mode: 0o600 }) await fse.chmod(storePath, 0o600).catch(() => undefined) - cachedSecrets = secrets + cachedAccounts = accounts } -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 } 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 6a51d704..3a05e555 100644 --- a/src/ipc/handlers/gameHandlers.ts +++ b/src/ipc/handlers/gameHandlers.ts @@ -13,7 +13,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, @@ -87,9 +87,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.`) @@ -155,8 +155,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 @@ -237,7 +238,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": @@ -245,6 +246,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..5e62f316 100644 --- a/src/renderer/src/features/config/contexts/ConfigContext.tsx +++ b/src/renderer/src/features/config/contexts/ConfigContext.tsx @@ -36,9 +36,9 @@ 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: 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 +122,7 @@ 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 accountList = useMemo(() => ({ accounts: config.accounts, activeAccountId: config.activeAccountId }), [config.accounts, config.activeAccountId]) const settings = useMemo( () => ({ @@ -155,7 +155,7 @@ const ConfigProvider = ({ children }: { children: React.ReactNode }): JSX.Elemen - + @@ -165,7 +165,7 @@ const ConfigProvider = ({ children }: { children: React.ReactNode }): JSX.Elemen - + @@ -184,7 +184,8 @@ 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. */ +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 +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, useSettingsConfig, useFavMods, useSuspendedModUpdates, useCustomIcons, useNotifiedModUpdates } +export { ConfigProvider, useConfigDispatch, useInstallations, useGameVersions, 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 b1999e6c..7b3f4199 100644 --- a/src/renderer/src/locales/en-US.json +++ b/src/renderer/src/locales/en-US.json @@ -270,13 +270,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!", @@ -287,8 +284,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..3fd65ec4 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,30 +141,36 @@ 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) + // write-file-atomic names its temp sibling `.`, never `.tmp`, so a + // suffix filter would pass on anything: assert the store file is the only entry left. assert.deepEqual( - readdirSync(mockState.userDataDir).filter((entry) => entry.endsWith(".tmp")), + readdirSync(mockState.userDataDir).filter((entry) => entry !== "account-secrets.json"), [] ) }) - 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" }) + + 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 () => { 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 +182,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 +191,243 @@ 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(), null) + assert.equal(await store.getAccountSecrets("uid-a"), null, "missing sessionSignature makes this entry unreadable") + assert.deepEqual(await store.getAccountSecrets("uid-b"), ACCOUNT_B) }) - 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("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("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.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.clearAccountSecrets(), true) + assert.equal(await store.removeAccountSecrets("uid-a"), 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("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..71fe7ee1 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,170 @@ 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") + }) + + 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", () => { + 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 79ea7023..5e290abf 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" import { writeJsonAtomic } from "@src/ipc/atomicJsonFile" /** @@ -38,7 +39,8 @@ import { writeJsonAtomic } from "@src/ipc/atomicJsonFile" */ 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) })) // Real implementation, wrapped, so the crash-safety guarantee stays covered by @@ -75,16 +77,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: [], @@ -235,7 +245,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() @@ -263,7 +274,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, @@ -279,6 +291,111 @@ 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 foreign session has to already be in the file for there to be anything to clear, so + // unlike the "cannot be written" test above this one cannot start from an empty folder. What + // blocks the write is still the DIRECTORY: the clear goes out through writeJsonAtomic, which + // creates a sibling temp file and renames it over the destination, so the destination file's + // own mode never gates it and only a directory nothing may create in does. 0o500 still allows + // the read that finds the foreign uid in the first place. + chmodSync(installationFolder, 0o500) + 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(installationFolder, 0o700) + } + }) + /** * 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 @@ -294,7 +411,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. @@ -311,7 +429,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")) @@ -333,7 +451,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"), @@ -361,6 +480,135 @@ 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") + }) + + /** + * 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", () => { diff --git a/tests/ipc/modsHandlers.test.ts b/tests/ipc/modsHandlers.test.ts index c64e3dbd..f7d7f111 100644 --- a/tests/ipc/modsHandlers.test.ts +++ b/tests/ipc/modsHandlers.test.ts @@ -95,7 +95,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/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() 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") }) })