maximus/Simpl-Resultat
1
0
Fork 0
Code Issues 4 Pull requests Projects Releases 19 Packages Wiki Activity Actions

docs: ADR 0006 + changelog + architecture for OAuth keychain (#82) #87

Merged
maximus merged 1 commit from issue-82-wrap-up into main 2026-04-14 12:28:08 +00:00
Conversation 1 Commits 1 Files changed 4 +161 -6
maximus commented 2026-04-14 12:26:54 +00:00
Owner
Copy link

Fixes #82
Refs #66, #78, #79, #80, #81

Summary

Wrap-up of the spec-oauth-keychain milestone: documentation, ADR, and bilingual changelog entries. Closes the documentation gap before the next release tag.

  • ADR-0006 in docs/adr/0006-oauth-tokens-keychain.md — context, options considered, backend choice (sync-secret-service vs async, libdbus vs libsecret), migration semantics, anti-downgrade flag, and the subscription tampering fix via HMAC-signed account_cache.
  • architecture.md — new token_store + account_cache module entries, updated OAuth2 flow section, command count 34 → 35 (added get_token_store_mode).
  • CHANGELOG.md + CHANGELOG.fr.md — entries under [Unreleased] → Changed + Security, referencing #66, #78, #79, #80, #81.

Manual test matrix (deferred to release gatekeeper)

These cannot be automated in this conversation and are tracked on the issue for the release gatekeeper to run before the next tag:

Linux (pop-os):

  • Fresh install + login → secret-tool lookup service com.simpl.resultat user oauth-tokens returns a JSON blob, no tokens.json in <app_data>/auth/
  • Migration: create a legacy tokens.json artificially, start the app, verify the file is zero-overwritten + removed, keychain now has the entry
  • Logout: both keychain entry and file are cleared, HMAC key also gone (secret-tool lookup service com.simpl.resultat user account-hmac-key empty)
  • Fallback: DBUS_SESSION_BUS_ADDRESS=/dev/null start → app works, TokenStoreFallbackBanner appears in Settings, auth flow completes via file fallback
  • Downgrade refusal: run once with keychain, then block keychain — subsequent save should surface an error instead of silently writing a file
  • HMAC tampering: modify account.json after login (rewrite subscription_status manually), verify Premium features stay locked

Windows:

  • Fresh install + login → Credential Manager shows an entry under com.simpl.resultat
  • Migration from a legacy tokens.json artifact
  • Logout clears Credential Manager + HMAC entry
  • Measure credential blob size — must be well under the ~2.5 KB Win32 limit

Test plan (CI only)

  • ADR + changelog + architecture edits compile into a valid repo (no markdown syntax errors)
  • cargo check clean (no side effect on docs-only PR)
  • npm run build clean
Fixes #82 Refs #66, #78, #79, #80, #81 ## Summary Wrap-up of the `spec-oauth-keychain` milestone: documentation, ADR, and bilingual changelog entries. Closes the documentation gap before the next release tag. - **ADR-0006** in `docs/adr/0006-oauth-tokens-keychain.md` — context, options considered, backend choice (sync-secret-service vs async, libdbus vs libsecret), migration semantics, anti-downgrade flag, and the subscription tampering fix via HMAC-signed `account_cache`. - **architecture.md** — new `token_store` + `account_cache` module entries, updated OAuth2 flow section, command count 34 → 35 (added `get_token_store_mode`). - **CHANGELOG.md + CHANGELOG.fr.md** — entries under `[Unreleased]` → `Changed` + `Security`, referencing #66, #78, #79, #80, #81. ## Manual test matrix (deferred to release gatekeeper) These cannot be automated in this conversation and are tracked on the issue for the release gatekeeper to run before the next tag: **Linux (pop-os)**: - [ ] Fresh install + login → `secret-tool lookup service com.simpl.resultat user oauth-tokens` returns a JSON blob, no `tokens.json` in `<app_data>/auth/` - [ ] Migration: create a legacy `tokens.json` artificially, start the app, verify the file is zero-overwritten + removed, keychain now has the entry - [ ] Logout: both keychain entry and file are cleared, HMAC key also gone (`secret-tool lookup service com.simpl.resultat user account-hmac-key` empty) - [ ] Fallback: `DBUS_SESSION_BUS_ADDRESS=/dev/null` start → app works, `TokenStoreFallbackBanner` appears in Settings, auth flow completes via file fallback - [ ] Downgrade refusal: run once with keychain, then block keychain — subsequent save should surface an error instead of silently writing a file - [ ] HMAC tampering: modify `account.json` after login (rewrite `subscription_status` manually), verify Premium features stay locked **Windows**: - [ ] Fresh install + login → Credential Manager shows an entry under `com.simpl.resultat` - [ ] Migration from a legacy `tokens.json` artifact - [ ] Logout clears Credential Manager + HMAC entry - [ ] Measure credential blob size — must be well under the ~2.5 KB Win32 limit ## Test plan (CI only) - [x] ADR + changelog + architecture edits compile into a valid repo (no markdown syntax errors) - [x] `cargo check` clean (no side effect on docs-only PR) - [x] `npm run build` clean
maximus added 1 commit 2026-04-14 12:26:55 +00:00
docs: ADR 0006 + changelog + architecture for OAuth keychain (#82)
All checks were successful
PR Check / rust (push) Successful in 22m44s
Details
PR Check / frontend (push) Successful in 2m19s
Details
PR Check / rust (pull_request) Successful in 22m25s
Details
PR Check / frontend (pull_request) Successful in 2m19s
Details
65bc7f5130 
- New ADR-0006 documenting the OS keychain migration: context,
  options considered (keyring vs stronghold vs AES-from-PIN), the
  backend choice rationale (sync-secret-service vs async-secret-
  service), anti-downgrade design, migration semantics, and the
  subscription-tampering fix via account_cache.
- architecture.md updated: new token_store / account_cache module
  entries, auth_commands descriptions now point at the keychain-
  backed API, OAuth2 + deep-link flow diagram mentions the HMAC
  step, command count bumped to 35.
- CHANGELOG.md + CHANGELOG.fr.md under Unreleased:
  - Changed: tokens moved to keychain with transparent migration
    and Settings banner on fallback.
  - Changed: account cache is now HMAC-signed.
  - Security: CWE-312 and CWE-345 explicitly closed.

Manual test matrix (pop-os + Windows) is tracked in issue #82 and
will be run by the release gatekeeper before the next tag.

Refs #66, #78, #79, #80, #81

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
maximus commented 2026-04-14 12:27:38 +00:00
Author
Owner
Copy link

Review — APPROVE ✓

PR docs-only, revue rapide.

Correctness

  • ✓ ADR-0006 référence correctement les PRs #83-#87 et les issues #66, #78-#82
  • ✓ architecture.md : nouveau token_store + account_cache listés, OAuth flow updaté avec les étapes keychain + HMAC, count de commandes 34 → 35 (matche l'ajout de get_token_store_mode)
  • ✓ CHANGELOG.md et CHANGELOG.fr.md : entrées parallèles sous [Unreleased] → Changed + Security, conforme au format Keep a Changelog du projet
  • ✓ Références PR/issue propres, pas de lien cassé

Quality

  • ✓ ADR 0006 suit le template des ADRs existants (0001-0005) : Date, Statut, Contexte, Options considérées, Décision, Conséquences
  • ✓ Sections Options considérées et Conséquences négatives sont substantielles — utiles pour un relecteur futur qui voudrait revenir sur le choix
  • ✓ Matrice de tests manuels complète dans la PR body, tracée dans l'issue pour le release gatekeeper

Observations

  • Aucune — c'est propre.

Verdict : APPROVE

## Review — APPROVE ✓ PR docs-only, revue rapide. ### Correctness - ✓ ADR-0006 référence correctement les PRs #83-#87 et les issues #66, #78-#82 - ✓ architecture.md : nouveau `token_store` + `account_cache` listés, OAuth flow updaté avec les étapes keychain + HMAC, count de commandes 34 → 35 (matche l'ajout de `get_token_store_mode`) - ✓ CHANGELOG.md et CHANGELOG.fr.md : entrées parallèles sous `[Unreleased]` → `Changed` + `Security`, conforme au format Keep a Changelog du projet - ✓ Références PR/issue propres, pas de lien cassé ### Quality - ✓ ADR 0006 suit le template des ADRs existants (0001-0005) : Date, Statut, Contexte, Options considérées, Décision, Conséquences - ✓ Sections Options considérées et Conséquences négatives sont substantielles — utiles pour un relecteur futur qui voudrait revenir sur le choix - ✓ Matrice de tests manuels complète dans la PR body, tracée dans l'issue pour le release gatekeeper ### Observations - Aucune — c'est propre. **Verdict : APPROVE**
maximus merged commit 9ccfc7a9d9 into main 2026-04-14 12:28:08 +00:00
maximus deleted branch issue-82-wrap-up 2026-04-14 12:28:08 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
source:analyste

Créée par le Super-Analyste

  
source:defenseur

Créée par l Escouade Défenseur

  
source:human

Créée par un humain

  
source:medic

Créée par le Medic (auto-détection)

  
status:approved

PR approuvée, prête à merger

  
status:blocked

Bloquée (question, dépendance, clarification)

  
status:in-progress

Un agent (Medic/Dev) travaille dessus

  
status:needs-fix

Reviewer demande des corrections

  
status:ready

Analysée et documentée, prête à prendre

  
status:review

PR créée, en attente du Reviewer

  
status:triage

Nouvelle issue, pas encore analysée

  
type:bug

Correction de bug → Medic

  
type:feature

Nouvelle fonctionnalité → Dev

  
type:infra

Config, CI/CD, déploiement → Dev

  
type:refactor

Refactoring / amélioration technique → Dev

  
type:schema

Modèle de données / migrations → Dev

  
type:security

Issue de sécurité → Medic ou Dev

No labels
source:analyste
source:defenseur
source:human
source:medic
status:approved
status:blocked
status:in-progress
status:needs-fix
status:ready
status:review
status:triage
type:bug
type:feature
type:infra
type:refactor
type:schema
type:security
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: maximus/Simpl-Resultat#87
No description provided.
Delete branch "issue-82-wrap-up"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?