feat: OAuth token storage via OS keychain (#78) #83

Merged

maximus merged 1 commit from issue-78-token-store into main

2026-04-14 00:17:42 +00:00

maximus commented

2026-04-13 23:49:06 +00:00

Owner

Fixes #78
Refs #66

Summary

New token_store module centralises OAuth token persistence.
Keychain (Credential Manager on Windows, Secret Service via sync-secret-service + crypto-rust on Linux) is the primary store; restricted file is a supervised fallback.
Transparent migration on first load for existing users: legacy tokens.json is copied into the keychain and then zero-overwritten + fsync'd before unlink.
store_mode flag persisted to refuse silent downgrades after a prior keychain success (forces re-auth instead of leaking plaintext).
New get_token_store_mode command for the upcoming settings banner (#81).
auth_commands.rs refactored: all tokens.json paths go through token_store. check_subscription_status uses token_store::load().is_some() so the migration runs even when the 24h throttle early-returns.

Test plan

cargo check clean
cargo test 17/17 pass (3 new tests for token_store)
npm run build clean
Manual: fresh login on pop-os, verify keychain entry exists via secret-tool lookup service com.simpl.resultat user oauth-tokens, verify no tokens.json in app data dir
Manual: place a legacy tokens.json on disk, start the app, verify migration + wipe
Manual: DBUS_SESSION_BUS_ADDRESS=/dev/null start, verify fallback path activates
Manual: Windows fresh login + migration (deferred to #82)

Fixes #78 Refs #66 ## Summary - New `token_store` module centralises OAuth token persistence. - Keychain (Credential Manager on Windows, Secret Service via sync-secret-service + crypto-rust on Linux) is the primary store; restricted file is a supervised fallback. - Transparent migration on first load for existing users: legacy `tokens.json` is copied into the keychain and then zero-overwritten + fsync'd before unlink. - `store_mode` flag persisted to refuse silent downgrades after a prior keychain success (forces re-auth instead of leaking plaintext). - New `get_token_store_mode` command for the upcoming settings banner (#81). - `auth_commands.rs` refactored: all tokens.json paths go through `token_store`. `check_subscription_status` uses `token_store::load().is_some()` so the migration runs even when the 24h throttle early-returns. ## Test plan - [x] `cargo check` clean - [x] `cargo test` 17/17 pass (3 new tests for token_store) - [x] `npm run build` clean - [ ] Manual: fresh login on pop-os, verify keychain entry exists via `secret-tool lookup service com.simpl.resultat user oauth-tokens`, verify no `tokens.json` in app data dir - [ ] Manual: place a legacy `tokens.json` on disk, start the app, verify migration + wipe - [ ] Manual: `DBUS_SESSION_BUS_ADDRESS=/dev/null` start, verify fallback path activates - [ ] Manual: Windows fresh login + migration (deferred to #82)

maximus added 1 commit 2026-04-13 23:49:07 +00:00

feat: migrate OAuth tokens to OS keychain via token_store (#78 )

PR Check / rust (push) Successful in 17m25s

Details

PR Check / frontend (push) Successful in 2m31s

Details

PR Check / rust (pull_request) Successful in 18m14s

Details

PR Check / frontend (pull_request) Successful in 2m14s

Details

feaed4058d

Introduce a new token_store module that persists OAuth tokens in the OS
keychain (Credential Manager on Windows, Secret Service on Linux through
sync-secret-service + crypto-rust, both pure-Rust backends).

- Keychain service name matches the Tauri bundle identifier
  (com.simpl.resultat) so credentials are scoped to the real app
  identity.
- Transparent migration on first load: a legacy tokens.json is copied
  into the keychain, then zeroed and unlinked before removal to reduce
  refresh-token recoverability from unallocated disk blocks.
- Store-mode flag (keychain|file) persisted next to the auth dir.
  After a successful keychain write the store refuses to silently
  downgrade to the file fallback, so a subsequent failure forces
  re-authentication instead of leaking plaintext.
- New get_token_store_mode command exposes the current mode to the
  frontend so a settings banner can warn users running on the file
  fallback.
- auth_commands.rs refactored: all tokens.json read/write/delete paths
  go through token_store; check_subscription_status now uses
  token_store::load().is_some() to trigger migration even when the
  24h throttle would early-return.

Refs #66

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

maximus commented

2026-04-14 00:11:21 +00:00

Author

Owner

Review — APPROVE ✓

Inspection de la PR #83 contre la checklist sprint.

Security

✓ Service keychain = com.simpl.resultat (canonique tauri.conf.json)
✓ Zero-overwrite + fsync avant remove_file dans zero_and_delete (CWE-212)
✓ Flag store_mode persistant refuse le downgrade silencieux après un succès keychain (CWE-757)
✓ zeroize appliqué au buffer local
✓ Aucun secret commit

Correctness

✓ Tous les call sites de l'ancien TOKENS_FILE passent par token_store
✓ check_subscription_status utilise token_store::load(&app)?.is_some() — déclenche la migration même quand le throttle 24h early-return
✓ refresh_auth_token appelle token_store::delete sur échec au lieu de fs::remove_file
✓ handle_auth_callback ordre correct : save tokens → fetch userinfo → save account
✓ Les variantes NoEntry de keyring sont traduites en Ok(None) / Ok(()) pour une API idempotente

Quality

✓ Module 300+ lignes mais la structure est claire (helpers privés groupés en haut, API publique en bas)
✓ 3 nouveaux tests unitaires (serde round-trip, parse store_mode, zero-and-delete)
✓ Commentaires expliquent le WHY (pas le what) pour les choix non-triviaux : scope keychain service, refus de downgrade, migration semantics

Observations mineures (non-bloquantes)

zero_and_delete utilise truncate(false) qui est la valeur par défaut d'OpenOptions — suppression possible, cosmétique.
Si write_store_mode échoue après un keychain_save réussi dans save(), on retourne Err mais les tokens sont déjà dans le keychain. Edge case très rare (disk full en plein entre deux fs ops). Pas bloquant.
keyring = "3.6" — version pinnée mineure comme demandé par la revue spec. cargo audit à ajouter dans #79.

Backend choice note

La revue spec envisageait libsecret-1-dev comme dep de build Linux. Finalement j'utilise sync-secret-service + crypto-rust qui sont pure Rust côté parseur de Secret Service, mais passent par dbus-secret-service → dbus crate → libdbus-1-dev au build time (au lieu de libsecret-1-dev). Net : impact CI/packaging identique, un seul apt package à ajouter dans #79, et libdbus-1-3 est quasi universellement présent sur Linux desktop au runtime donc pas besoin de l'ajouter aux .deb.depends.

Verdict : APPROVE — prêt à merger. Les points mineurs peuvent être traités en follow-up ou laissés tels quels.

## Review — APPROVE ✓ Inspection de la PR #83 contre la checklist sprint. ### Security - ✓ Service keychain = `com.simpl.resultat` (canonique tauri.conf.json) - ✓ Zero-overwrite + fsync avant `remove_file` dans `zero_and_delete` (CWE-212) - ✓ Flag `store_mode` persistant refuse le downgrade silencieux après un succès keychain (CWE-757) - ✓ `zeroize` appliqué au buffer local - ✓ Aucun secret commit ### Correctness - ✓ Tous les call sites de l'ancien `TOKENS_FILE` passent par `token_store` - ✓ `check_subscription_status` utilise `token_store::load(&app)?.is_some()` — déclenche la migration même quand le throttle 24h early-return - ✓ `refresh_auth_token` appelle `token_store::delete` sur échec au lieu de `fs::remove_file` - ✓ `handle_auth_callback` ordre correct : save tokens → fetch userinfo → save account - ✓ Les variantes `NoEntry` de keyring sont traduites en `Ok(None)` / `Ok(())` pour une API idempotente ### Quality - ✓ Module 300+ lignes mais la structure est claire (helpers privés groupés en haut, API publique en bas) - ✓ 3 nouveaux tests unitaires (serde round-trip, parse store_mode, zero-and-delete) - ✓ Commentaires expliquent le WHY (pas le what) pour les choix non-triviaux : scope keychain service, refus de downgrade, migration semantics ### Observations mineures (non-bloquantes) 1. `zero_and_delete` utilise `truncate(false)` qui est la valeur par défaut d'OpenOptions — suppression possible, cosmétique. 2. Si `write_store_mode` échoue *après* un `keychain_save` réussi dans `save()`, on retourne Err mais les tokens sont déjà dans le keychain. Edge case très rare (disk full en plein entre deux fs ops). Pas bloquant. 3. `keyring = "3.6"` — version pinnée mineure comme demandé par la revue spec. `cargo audit` à ajouter dans #79. ### Backend choice note La revue spec envisageait `libsecret-1-dev` comme dep de build Linux. Finalement j'utilise `sync-secret-service` + `crypto-rust` qui sont pure Rust côté parseur de Secret Service, mais passent par `dbus-secret-service` → `dbus` crate → **`libdbus-1-dev` au build time** (au lieu de libsecret-1-dev). Net : impact CI/packaging identique, un seul apt package à ajouter dans #79, et `libdbus-1-3` est quasi universellement présent sur Linux desktop au runtime donc pas besoin de l'ajouter aux `.deb.depends`. **Verdict : APPROVE** — prêt à merger. Les points mineurs peuvent être traités en follow-up ou laissés tels quels.