fix(account): adopt the session the game refreshed instead of overwriting it - #209
Conversation
…ting it The launcher wrote its stored session into an installation's clientsettings.json on every launch. Once that stored session was invalidated by a login somewhere else, the game prompted, the player logged in, the game wrote a working session into the same file, and the next launch put the dead one straight back. A prompt every launch, forever. writeClientSettingsSession now reads the file first, as it already did, and steps aside when it finds a session key that is not ours on an account whose playeruid is. That can only have come from the game storing what the auth service accepted, so it is newer and it works. The new "adopted" outcome carries the secrets back to EXECUTE_GAME, which puts them in the same encrypted store LOGIN writes to. A session belonging to a different playeruid is still overwritten: adopting one would leave the launcher showing one account while holding another account's credentials. Refs #204
Zaldaryon
left a comment
There was a problem hiding this comment.
Checked out c70c447 in an isolated worktree: typecheck, lint:ci, format:check clean, and the two targeted test files pass 46 of 46.
Traced the trust boundary this PR turns on, since adopting credentials from a file is exactly the kind of change worth not taking on faith. session.playerUid, the trusted half of the sessionToAdopt comparison, comes from account.playerUid at the EXECUTE_GAME call site, which is this launcher's own config state for the logged-in account, not anything read back from clientsettings.json. So the file (fully under the local player's own control anyway, same threat model as the encrypted store it feeds) can only ever trigger adoption when its playeruid already matches the account the launcher itself is logged into; a mismatched or missing uid falls through to the ordinary overwrite path unchanged. parseStoredSecrets still gates completeness on top of that, confirmed by the incomplete-signature test landing on the written branch instead of adopted. The switch on the new four-arm outcome type has no default case and the compiler is what enforces exhaustiveness, which is the point of dropping the ok boolean, and the one call site handles all four arms explicitly.
The log-redaction test is worth calling out on its own: it spies on logMessage ahead of the redactor rather than after it, so it is asserting what the handler chose to say, not what the redaction layer happened to catch.
No blocking findings. Approving.
A field tester who migrated over from MVL reported that the game asks them to log in every single time they start it, even though they are logged into RiftLauncher and never touch the login screen there. Not once after an update, not on version switches only. Every launch.
The mechanism turned out to be a loop the launcher was driving itself, and it is written up in issue 204. The auth service keeps one session per account, so logging in anywhere else, another launcher or the game on another profile, invalidates the key RiftLauncher has stored. From that moment on the launch path did this: write the stored (now dead) key into the installation's clientsettings.json, start the game, the game has the key rejected and prompts, the player logs in, the game stores the fresh working session in that same file, and then the next launch through RiftLauncher wrote the dead key straight back over it. The game repaired itself every time and the launcher undid the repair every time. One invalidation, a prompt forever.
What changes
writeClientSettingsSessionalready read the file before writing it, because that write has always been a read-modify-write that preserves the player's resolution, volumes and key bindings. So the document is already in hand at the moment the decision is made, and the fix fits inside it: when the file already carries a session key that is not ours, do not write, and hand the fields back to the caller to adopt.The argument for trusting the file over our own store is that the game only stores what the auth service handed it. A key in there that we did not put there is a key the service issued after our last launch, which makes it both newer and known good. Taking the game's side ends the loop after a single prompt.
Which fields decide that a session is "a different one"
sessionkeyalone tells you the session changed. It cannot tell you whose it is now, and that second question is the one that matters, because it separates a repair from a hijack. The launcher's encrypted store holds a session key, a signature and a multiplayer token with no account identity attached to them, and the account name shown in the UI lives somewhere else entirely, in config. Adopting a key issued for a different player would leave the launcher displaying one person while carrying another person's credentials, which is a worse bug than the one being fixed.So the check is the pair: adopt when
sessionkeydiffers from ours andplayeruidmatches ours.playeruidis the right partner for it rather thanuseremail, one of the other eight fields written into that section, because the launcher fillsuseremailin from whatever the user typed into the login form whileplayeruidcomes out of the login response itself, next to the key. A file whoseplayeruidis missing, or belongs to somebody else, gets overwritten exactly as before.Two more cases fall out of the same guard and are pinned by tests: a file with no session at all is written as it always was, and a file whose session is too incomplete to store (a key with no signature, say) is overwritten rather than adopted, since
parseStoredSecretsrefuses it.What the domain returns
The result type had
unreadable-settingsandwrite-failedhanging off anokboolean. Adoption is neither a success a caller may ignore nor a failure it may report, because it carries work: somebody has to persist those secrets or the next launch stomps them again. So the type became a four-arm discriminated union tagged on one field, in the styleLoginVerdictalready uses:Dropping the
okboolean is deliberate. It breaks compilation at the call site rather than letting the existingif (!written.ok)sail past a new arm it knows nothing about. The handler now switches, and the compiler will not let a future arm go unhandled either.Who stores the adopted session
EXECUTE_GAMEcallssaveAccountSecrets, the same functionLOGINcalls, on the same encrypted store. No second persistence route, no new module. If that store refuses the update, the launch still goes ahead: the game's own settings file already holds the working session, so a failure there costs the player one more prompt on the next launch, and failing the launch over a bookkeeping update would cost them the game they asked for.The key itself never appears in a log line or in a return value. Adoption logs that it happened and nothing about what was adopted.
A ceiling worth naming
The code carries a comment naming this limit. "Newer" is inferred, never verified, because there is no session-validation endpoint mapped in this launcher to ask. The whole argument is "the game wrote it, so the service accepted it". If the adopted key had itself been invalidated in the meantime, the player gets one more prompt and the game writes another one, which still terminates. Validating before adopting would need that endpoint found first.
Tests
Domain side, pure, no Electron: a file with no session writes ours as today, a file holding our own session writes unchanged with no adoption, a file holding a different session for the same player is left alone and the adopted outcome carries the game's three secret fields, a session belonging to another
playeruidis overwritten, an incomplete one is overwritten, and the unreadable and write-failed arms behave as they always did.Handler side, on the mocked-electron harness: the adopted outcome persists the new credentials through
saveAccountSecretsand the game's key survives in clientsettings.json, plus a test that spies onlogMessageat the source, ahead of the redactor, and asserts no session value ever reached it.Every one of them was mutation-checked before this went up. Reverting the adoption branch fails four tests. Persisting nothing fails the handler test. Logging the key fails the no-secrets test. Dropping the
playeruidguard fails the two cross-account tests.Gates all green locally: typecheck, lint with zero errors, prettier, and the full suite at 1320 passing with coverage above every floor.
Remedy for anyone already stuck in the loop before this ships: log out of RiftLauncher and back in, which refreshes the stored session.