Simpl-Resultat/spec-issue-66-oauth-keychain.md

# Spec — Migration des tokens OAuth vers le keychain OS (#66)

## Contexte

Simpl'Résultat utilise depuis la v0.7.0 un flux OAuth2 Authorization Code + PKCE pour authentifier les utilisateurs auprès de Logto (Compte Maximus). Les tokens résultants (`access_token`, `refresh_token`, `id_token`) sont actuellement persistés dans le filesystem utilisateur :

- **Chemin** : `<app_data_dir>/auth/tokens.json`
- **Protection Unix** : permissions `0600` (owner-only) via `OpenOptionsExt::mode(0o600)`
- **Protection Windows** : aucune (le fichier est écrit avec `fs::write` sans ACL particulière)
- **Commentaire de code source** (auth_commands.rs:7-11) : la solution actuelle est explicitement documentée comme transitoire, en attendant une migration vers le keychain OS.

**Objectif de la présente spec** : migrer le stockage des tokens OAuth de fichier plat vers le keychain OS natif (Credential Manager sur Windows, Secret Service sur Linux), avec migration transparente pour les utilisateurs existants et fallback gracieux si le keychain est indisponible.

**Référence** : CWE-312 (Cleartext Storage of Sensitive Information), issue Forgejo #66, liée à #51 (OAuth2 PKCE initial).

---

## Problème

### Ce qui est stocké en clair aujourd'hui

Le fichier `auth_commands.rs` écrit et lit 2 fichiers sensibles via la fonction `write_restricted()` (l.77-97) :

| Fichier | Contenu | Sensibilité | Usage |
|---|---|---|---|
| `tokens.json` | `access_token`, `refresh_token`, `id_token`, `expires_at` | **HAUTE** | Tous les appels API authentifiés, rotation via refresh |
| `account.json` | email, nom, picture, `subscription_status` | Moyenne | Affichage UI du compte, gating des features payantes |
| `last_check` | timestamp unix (dernier check abonnement) | Aucune | Throttling du polling Logto (24h) |

### Trou principal : Windows

La fonction `write_restricted()` utilise `OpenOptionsExt::mode(0o600)` uniquement sous `#[cfg(unix)]`. Sur Windows, le fallback est un `fs::write` brut — aucune ACL appliquée. Or Windows est une plateforme cible supportée. Un autre processus utilisateur peut lire le fichier.

### Trou secondaire : tous les OS

Même avec `0600` sur Linux, le contenu reste :
- Accessible à n'importe quel process tournant sous le même UID (malware utilisateur, extensions de navigateur local exfiltrant `$HOME`, outils de debug mal configurés, backups non chiffrés synchronisés sur le cloud)
- Inclus dans les backups de dossier home par défaut
- Lisible si l'attaquant obtient un shell non-root sur la machine

Le refresh token en particulier donne une session longue durée (rotation mais pas d'expiration courte) et représente le pire-cas d'exposition.

### Pourquoi c'est le bon moment

1. **Avant la monétisation** : la session Logto gatera l'accès aux features payantes (#53 machine activation, #50 Stripe). Un refresh token volé = contournement du gating de licence.
2. **La dette est connue** : le code lui-même documente l'intention de migrer (auth_commands.rs:7-8).
3. **Changement bien scopé** : le flux OAuth est stabilisé depuis v0.7.3, l'API de stockage est centralisée dans un seul fichier.

---

## Solution proposée

### Crate `keyring` (v3)

> **🟡 SECURITE** — Pas de pinning de version ni de revue supply-chain. `keyring` v3 tire `secret-service` qui pulls `zbus` et un large graphe D-Bus — surface d'attaque non négligeable pour une app privacy-first.
> **Resolution :** Pinner `keyring = "3.x"` explicitement dans Cargo.toml, ajouter `cargo audit` à check.yml après l'ajout de la dep, et documenter la chaîne de deps transitives dans l'ADR.
> *Ref : OWASP A06:2021*

Wrapper Rust maintenu au-dessus des keychains natifs :

| OS | Backend | Installé par défaut |
|---|---|---|
| Windows | Credential Manager (Win32 API) | Oui |
| Linux | Secret Service API via D-Bus (GNOME Keyring / KWallet) | **Non** — nécessite `libsecret-1-0` |
| macOS | Keychain Services (hors cible) | Oui |

Alternatives considérées :
- **`tauri-plugin-stronghold`** : chiffrement au repos avec une master password. Rejeté car demande à l'utilisateur de saisir une passphrase supplémentaire, ce qui casse l'UX de connexion silencieuse (refresh automatique au démarrage).
- **`tauri-plugin-store` + chiffrement custom** : il faudrait gérer une clé maître quelque part — on déplace le problème.
- **AES-256-GCM avec clé dérivée du PIN** : seulement viable pour les profils avec PIN, pas pour les tokens OAuth qui doivent être lus sans interaction.

`keyring` est le bon compromis : le système d'exploitation gère déjà la clé maître (session utilisateur).

### Scope

> **🟡 SECURITE** — L'exclusion de `account.json` ignore le tampering de `subscription_status`. Un malware local peut écrire `"active"` dedans pour bypass le gating de licence sans toucher au keychain.
> **Resolution :** Re-valider `subscription_status` depuis l'id_token/userinfo à chaque décision de gating, OU signer la valeur en cache, OU inclure account.json dans la migration keychain.
> *Ref : CWE-345*

> **🟢 ARCHITECTURE** — Scope limité aux tokens est le bon compromis : blast radius minimal, `write_restricted()` réutilisé tel quel, valeur sécurité ≈ coût du refactor. Garder tel quel et justifier l'asymétrie dans l'ADR.

**Migration uniquement de `tokens.json`**. Raisons :
- `account.json` contient de l'info d'affichage non-sensible (email déjà visible dans le menu, picture URL HTTP publique). Pas un secret opérationnel.
- `last_check` est un timestamp.
- Limiter le blast radius du changement, garder `write_restricted()` pour le reste.
- Permet un rollback ciblé si le keychain pose problème en production.

### Architecture

Nouveau module `src-tauri/src/auth/token_store.rs` exposant 3 fonctions :

```rust
pub struct StoredTokens { /* identique à aujourd'hui */ }

pub fn save(app: &AppHandle, tokens: &StoredTokens) -> Result<(), String>;
pub fn load(app: &AppHandle) -> Result<Option<StoredTokens>, String>;
pub fn delete(app: &AppHandle) -> Result<(), String>;
```

> **🔴 SECURITE + ARCHITECTURE + TECHNIQUE** — Service keychain ne correspond pas à l'identifiant bundle réel (`com.simpl.resultat` dans tauri.conf.json vs `com.lacompagniemaximus.simpl-resultat` dans la spec).
> **Resolution :** Utiliser `com.simpl.resultat` (l'identifiant canonique de l'app) dans la constante, les commandes `secret-tool` de test et l'ADR. Aligner sinon tauri.conf.json en premier.
> *Ref : CWE-1270*

> **🟡 ARCHITECTURE + TECHNIQUE** — Nouveau top-level module `auth/` casse la convention `commands/`.
> **Resolution :** Placer le module à `src-tauri/src/commands/token_store.rs` et l'enregistrer dans `commands/mod.rs` comme les autres. Pas de nouveau répertoire à créer.

**Conventions :**
- Une seule entrée keychain : `service = "com.lacompagniemaximus.simpl-resultat"`, `user = "oauth-tokens"`
- `value = serde_json::to_string(&StoredTokens)` (JSON compact, pas pretty)
- Le module `auth_commands.rs` ne touche plus jamais `tokens.json` directement — tous les accès passent par `token_store`.

### Implémentation — save()

> **🔴 SECURITE + ARCHITECTURE** — Fallback silencieux annule l'objectif de sécurité. Sur Windows le fallback n'applique aucune ACL, et un user qui installe le `.deb` sans libsecret continuera à stocker ses tokens en clair sans aucun signal visible.
> **Resolution :** Sur Windows, appliquer une DACL restrictive via `SetNamedSecurityInfoW` lors du fallback (ou fail-closed et forcer reauth). Exposer l'état `store_mode: keychain|file` à la frontend pour afficher une bannière de sécurité si le fallback est actif.
> *Ref : CWE-276*

```
1. Essayer keyring::Entry::new(SERVICE, USER).set_password(&json)
2. Si OK : supprimer tokens.json résiduel (migration nettoyage)
3. Si KO : fallback write_restricted() sur tokens.json + log warning
```

### Implémentation — load()

> **🔴 SECURITE** — Migration laisse des tokens en clair récupérables. `fs::remove_file` ne zéroifie pas les blocs disque, et le refresh token (long-lived) reste récupérable via unallocated sectors ou backups existants — précisément le threat model cité dans la spec.
> **Resolution :** Avant `remove_file`, overwrite le contenu avec des zéros et `fsync()`, puis delete. Documenter dans le changelog la recommandation de rotation de session post-migration pour les utilisateurs inquiets des backups passés.
> *Ref : CWE-212*

```
1. Essayer keyring::Entry::new(...).get_password()
2. Si OK et valeur présente : désérialiser et retourner Some(tokens)
3. Si KO ou vide : tenter de lire tokens.json
   3a. Si tokens.json présent : migrer (keyring write + file delete) et retourner Some(tokens)
   3b. Si tokens.json absent : retourner Ok(None)
```

C'est dans cette fonction que la **migration transparente** se fait : à la première lecture après upgrade, les tokens passent du fichier au keychain.

### Implémentation — delete()

```
1. Supprimer keychain entry (ignorer les erreurs "no entry")
2. Supprimer tokens.json (ignorer si absent)
```

Double-delete pour éviter les états "fantôme" où un reliquat traîne dans l'un des deux stores.

### Refactor d'`auth_commands.rs`

Tous les call sites qui manipulent actuellement `TOKENS_FILE` passent par `token_store` :

| Ligne actuelle | Opération | Nouveau code |
|---|---|---|
| 202 (handle_auth_callback) | write après exchange | `token_store::save(&app, &tokens)` |
| 219-227 (refresh_auth_token) | read | `token_store::load(&app)?` |
| 277 (refresh_auth_token) | write après rotation | `token_store::save(&app, &new_tokens)` |
| 251 (refresh_auth_token) | delete sur échec refresh | `token_store::delete(&app)` |
| 305 (logout) | delete | `token_store::delete(&app)` |
| 320 (check_subscription_status) | exists check | `token_store::load(&app)?.is_some()` |

La constante `TOKENS_FILE` est supprimée du fichier (reste `ACCOUNT_FILE` et `LAST_CHECK_FILE` gérés par `write_restricted` comme avant).

### Fallback gracieux

> **🟡 SECURITE** — Aucune distinction entre "keychain n'a jamais marché" et "keychain marchait hier et échoue aujourd'hui". Un process hostile local pourrait forcer la dégradation.
> **Resolution :** Persister un flag `store_mode` dans `app_data_dir/auth/`. Si le keychain a déjà fonctionné, un échec ultérieur doit refuser le plaintext et forcer reauth au lieu de dégrader silencieusement.
> *Ref : CWE-757*

Le fallback fichier s'active quand :
- L'appel `keyring::Entry::new()` retourne une erreur (keyring crate indisponible)
- `set_password()` / `get_password()` retourne `PlatformFailure` ou équivalent
- Spécifique Linux : D-Bus non démarré, libsecret absent, session sans keyring déverrouillé

Comportement attendu :
- `save()` et `load()` logguent un warning une seule fois par session (pas de spam)
- L'app reste fonctionnelle, juste avec le filet de sécurité `0600` (Unix) ou rien (Windows sans keychain — très improbable puisque Credential Manager est toujours présent)
- Aucune fenêtre d'erreur ne s'affiche à l'utilisateur

### Migration des utilisateurs existants

> **🟡 TECHNIQUE** — Timing incertain. `check_subscription_status` a un throttle 24h et un early-return sur `last_check` récent — un user qui relance souvent l'app peut voir `tokens.json` traîner indéfiniment.
> **Resolution :** Remplacer le check `dir.join(TOKENS_FILE).exists()` à auth_commands.rs:320 par `token_store::load(&app)?.is_some()` — ça force la migration dès la première lecture, sans attendre le throttle.

À la prochaine lecture (`refresh_auth_token` au démarrage via `check_subscription_status`), `token_store::load()` détecte `tokens.json` résiduel, le copie dans le keychain, et supprime le fichier. **Pas de reconnexion forcée, pas de notification.**

Si le keychain est indisponible, le fichier reste en place avec ses permissions `0600` — comportement équivalent à aujourd'hui.

---

## Impact CI/CD et packaging

### Linux — dépendance libsecret

> **🔴 SECURITE** — Cible AppImage oubliée. `tauri.conf.json` inclut `appimage` dans `bundle.targets` — ces builds n'héritent pas des deps apt et ne bundlent pas libsecret par défaut. Chaque user AppImage retombe silencieusement dans le fallback plaintext.
> **Resolution :** Soit bundler libsecret via `linuxdeploy` dans le build AppImage, soit retirer AppImage du scope v0.8, soit documenter `libsecret-1-0` comme pré-requis système dans les release notes AppImage.

> **🔴 TECHNIQUE** — `.forgejo/workflows/release.yml` n'est pas mentionné. Le build Linux de release échouera au linking si libsecret-1-dev n'est pas installé là aussi.
> **Resolution :** Ajouter `libsecret-1-dev` aux steps d'install système Linux de `release.yml` en plus de `check.yml`. Lister explicitement les deux workflows dans la spec.

Le crate `keyring` sous Linux utilise `libsecret` via D-Bus. Il faut :

1. **Dev** : `libsecret-1-dev` installé sur les machines de build (pop-os du dev + workers Forgejo Actions). À vérifier si déjà présent.
2. **Build .deb** : ajouter `libsecret-1-0` aux `depends` dans `tauri.conf.json` → `bundle.linux.deb.depends`.
3. **Build .rpm** : ajouter `libsecret` aux `depends` dans `bundle.linux.rpm.depends`.
4. **CI `check.yml`** : installer `libsecret-1-dev` avant `cargo check` / `cargo test`.

Sans ces ajouts, l'app **compile** mais au runtime le keychain échoue → fallback fichier → warning log. L'app reste fonctionnelle, mais la valeur de sécurité du changement est annulée pour les utilisateurs qui installent via `.deb` sans la dépendance.

### Windows

Credential Manager est un service Windows built-in, toujours disponible. Aucune dépendance à déclarer.

### Contrainte taille Credential Manager

> **🟢 SECURITE** — Plan B "access_token en RAM only" crée une race au cold-start offline : chaque démarrage doit hit Logto avant toute call authentifiée, cassant la promesse offline-first pour 24h.
> **Resolution :** Si la limite 2.5 KB est atteinte, stocker `access_token` et `refresh_token` dans **deux entrées keychain distinctes** (user=access, user=refresh) plutôt que sortir l'access token du store.

La limite théorique d'un credential Windows est ~2.5 KB (valeur stockée dans le `CRED_BLOB`). Un `StoredTokens` sérialisé pèse typiquement 1-1.8 KB (3 JWT + timestamp). À mesurer avec un vrai payload Logto avant le merge. Si on approche la limite, fallback possible : stocker uniquement le refresh token dans le keychain, garder l'access token en mémoire (il expire en 1h de toute façon).

---

## Tests

### Tests unitaires (impossibles pour la vraie partie keychain)

> **🟡 ARCHITECTURE + TECHNIQUE** — L'injection de trait `Backend` est YAGNI et techniquement infaisable : `keyring` v3 expose une struct `Entry` concrète sans trait public à swapper. Ajouter un wrapper trait juste pour les tests = abstraction sans contrepartie.
> **Resolution :** Drop le trait injection. Tester uniquement le round-trip serde sur `StoredTokens` et le chemin fallback fichier (qui ne touche pas keyring). Marquer `#[ignore]` tous les tests qui nécessitent un vrai keychain.

Le crate `keyring` ne fournit pas de mock officiel. On teste :
- La sérialisation / désérialisation `StoredTokens` (déjà couvert implicitement)
- La logique de migration via un fake backend (trait `Backend` injecté pour les tests)

### Tests manuels obligatoires avant merge

Sur **pop-os** (dev) :
1. **Fresh install** : supprimer `<app_data>/auth/`, lancer l'app, se connecter, vérifier qu'aucun fichier `tokens.json` n'est créé, vérifier via `secret-tool lookup service com.lacompagniemaximus.simpl-resultat user oauth-tokens`
2. **Migration** : créer un `tokens.json` artificiel (en repartant d'une ancienne version), lancer la nouvelle version, vérifier que le fichier est supprimé et le secret présent dans le keychain après le premier refresh
3. **Logout** : vérifier que le keychain entry ET le fichier résiduel sont effacés
4. **Fallback** : masquer D-Bus (`DBUS_SESSION_BUS_ADDRESS=/dev/null`), vérifier que l'app fonctionne et que le fallback fichier s'active

Sur **Windows** (VM ou machine dédiée) :
1. Fresh install + login → vérifier présence dans Credential Manager (`rundll32.exe keymgr.dll,KRShowKeyMgr`)
2. Migration depuis un `tokens.json` artificiel
3. Logout

### Tests CI

> **🔴 TECHNIQUE** — `check.yml` a deux jobs distincts (`rust` et `frontend`, conteneur ubuntu:22.04). La spec ne le précise pas et parle d'"installer avant cargo check" comme si c'était une seule étape.
> **Resolution :** Éditer uniquement le step **"Install system dependencies"** du job `rust` — append `libsecret-1-dev` à la liste `apt-get install` existante. Ne pas toucher le job frontend.

`check.yml` doit :
- Installer `libsecret-1-dev` avant `cargo check` / `cargo test`
- `cargo test` passe (les tests qui nécessitent un vrai keychain sont marqués `#[ignore]`, exécutés manuellement)

---

## Documentation à mettre à jour

> **🟡 ARCHITECTURE + TECHNIQUE** — Format de numérotation ADR incorrect. Les ADRs existants (0001-0005) utilisent un préfixe 4 chiffres sans `adr-`. De plus, `Security` n'est pas une catégorie du CHANGELOG du projet (voir `.claude/rules/changelog.md`).
> **Resolution :** Nommer le fichier `docs/adr/0006-oauth-tokens-keychain.md`. Classer l'entrée changelog sous `Changed` (ou `Fixed` si framed comme correction de vulnérabilité), pas `Security`.

- `docs/architecture.md` — section "Stockage" et "Commandes Tauri" : mentionner `token_store`, mettre à jour le diagramme de stockage auth
- `docs/adr/` — nouvel ADR `adr-006-oauth-tokens-keychain.md` décrivant la décision (contexte, options considérées, fallback)
- `CHANGELOG.md` / `CHANGELOG.fr.md` — section `Security` :
  - EN : `Migrated OAuth tokens storage from plaintext JSON file to OS keychain (Credential Manager on Windows, Secret Service on Linux). Existing users are migrated transparently on first token refresh.`
  - FR : `Migration du stockage des tokens OAuth d'un fichier JSON en clair vers le keychain du système (Credential Manager sous Windows, Secret Service sous Linux). Les utilisateurs existants sont migrés de façon transparente au premier rafraîchissement du token.`

---

## Critères d'acceptance (issue #66)

- [x] Tokens stockés dans le keychain OS → via `keyring` crate
- [x] Fallback gracieux si keychain indisponible → fallback `write_restricted()` avec warning logué
- [x] Migration automatique des fichiers existants → dans `token_store::load()` au premier appel
- [ ] Linux packaging : `libsecret-1-0` ajouté aux dépendances `.deb` / `.rpm`
- [ ] CI `check.yml` : `libsecret-1-dev` installé avant les tests
- [ ] ADR rédigé et mergé dans `docs/adr/`
- [ ] Tests manuels passés sur pop-os + Windows (3 scénarios chacun)

---

## Estimation

> **🟢 TECHNIQUE** — Estimation optimiste. N'inclut pas les debug cycles CI, ni les surprises de linking pkg-config dans le conteneur ubuntu:22.04 du job `rust`, ni le first-run libsecret de validation.
> **Resolution :** Monter à **4-5h** et prévoir un premier push CI dédié à valider que libsecret compile avant d'écrire les tests.

- Module `token_store` + Cargo.toml : 45 min
- Refactor `auth_commands.rs` : 20 min
- Mise à jour packaging (tauri.conf.json + check.yml) : 15 min
- Tests manuels pop-os : 30 min
- Tests manuels Windows (si VM dispo) : 30 min
- ADR + changelog + PR : 20 min

**Total : ~2h30 à 3h**

---

## Risques identifiés

| Risque | Probabilité | Sévérité | Mitigation |
|---|---|---|---|
| `libsecret` absent sur install `.deb` minimale → fallback silencieux annule le bénéfice | Moyenne | Moyenne | Ajouter aux deps `.deb`, documenter dans changelog |
| Credential Manager dépasse la limite 2.5 KB | Basse | Haute | Mesurer avant merge, plan B : refresh token only au keychain |
| Migration échoue silencieusement (fichier gardé) | Basse | Basse | Double-write acceptable, logué en warning |
| Régression du flux OAuth existant (utilisateurs v0.7.x) | Basse | Haute | Tests manuels exhaustifs des 3 scénarios sur 2 OS |
| Keyring Linux demande un déverrouillage GNOME Keyring au démarrage | Moyenne | Basse | C'est le comportement attendu — documenter dans le changelog |

---

## Questions ouvertes

1. **Scope account.json** — on laisse hors scope comme proposé, ou on migre aussi ? Recommandation : hors scope.
2. **libsecret dans `.deb`** — ajout immédiat ou follow-up ? Recommandation : immédiat, sinon la migration n'a aucune valeur pour la majorité des utilisateurs Linux.
3. **ADR format** — réutiliser le gabarit existant de `docs/adr/` (à vérifier s'il y en a un).

---

## Revision — Synthese

> Date: 2026-04-13 | Experts: Securite, Architecture, Technique

### Verdict
🔴 **CRITIQUES A CORRIGER** — La spec est solide sur les principes, mais 6 trous critiques sont à colmater avant implémentation : identifiant bundle incohérent, fallbacks silencieux qui annulent le gain de sécurité, plaintext récupérable post-migration, AppImage et release.yml oubliés, structure CI mal comprise.

### Resume

| Expert | 🔴 | 🟡 | 🟢 | Points cles |
|--------|-----|-----|-----|-------------|
| Securite | 3 | 3 | 1 | Fallback silencieux, plaintext résiduel, AppImage, subscription tampering, supply chain |
| Architecture | 1 | 3 | 1 | Bundle id, module path, YAGNI trait, ADR numbering, scope OK |
| Technique | 3 | 2 | 1 | release.yml oublié, check.yml mal lu, migration timing, estimation optimiste |

### Actions requises

1. 🔴 **Bundle identifier** — aligner service keychain sur `com.simpl.resultat` (tauri.conf.json)
2. 🔴 **Fallback save() non-silencieux** — DACL Windows OU fail-closed, exposer `store_mode` au frontend
3. 🔴 **Zéroification avant delete** — overwrite + fsync avant `fs::remove_file` des tokens migrés
4. 🔴 **AppImage libsecret** — bundler via linuxdeploy, retirer du scope, ou documenter le pré-requis
5. 🔴 **release.yml libsecret-1-dev** — ajouter aux steps Linux sinon le build release casse
6. 🔴 **check.yml job rust uniquement** — append à "Install system dependencies", pas de nouveau step
7. 🟡 **Module path** — `src-tauri/src/commands/token_store.rs`, pas de nouveau top-level `auth/`
8. 🟡 **Fallback integrity** — flag `store_mode` persisté, refus du downgrade si keychain a déjà marché
9. 🟡 **Migration timing** — remplacer `TOKENS_FILE.exists()` ligne 320 par `token_store::load()?.is_some()`
10. 🟡 **Pin keyring + cargo audit** — `keyring = "3.x"` explicite, `cargo audit` dans CI
11. 🟡 **subscription_status integrity** — re-valider ou signer la valeur en cache
12. 🟡 **Drop trait Backend** — tester uniquement round-trip serde + fallback fichier
13. 🟡 **ADR format** — `0006-oauth-tokens-keychain.md`, changelog sous `Changed` (pas `Security`)