maximus/Simpl-Resultat
1
0
Fork 0
Code Issues 6 Pull requests Projects Releases 20 Packages Wiki Activity Actions
Simpl-Resultat/spec-issue-66-oauth-keychain.md
le king fu 4912ae39b0 docs: add WIP specs for OAuth keychain, monetisation, reports, and web 
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-20 20:41:00 -04:00

22 KiB
Raw Blame History

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 :

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)

  • Tokens stockés dans le keychain OS → via keyring crate
  • Fallback gracieux si keychain indisponible → fallback write_restricted() avec warning logué
  • 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)