feat: Maximus Account OAuth2 + machine activation (#51, #53) #65

Merged

maximus merged 7 commits from issue-51-compte-maximus-oauth into main 2026-04-10 19:38:28 +00:00

Conversation 6 Commits 7 Files changed 18 +1728 -29

maximus commented 2026-04-10 18:19:54 +00:00

Owner

Copy link

Summary

• Compte Maximus (OAuth2 PKCE via Logto): sign-in/sign-out in Settings, deep-link callback handler, token storage, daily subscription check
• Machine activation: activate/deactivate machines against the license server, machine list UI in the license card
• Premium detection: get_edition() now checks account subscription status — Premium overrides Base
• CSP + deep-link: restrictive Content Security Policy, tauri-plugin-deep-link configured for simpl-resultat:// scheme
• i18n FR/EN, changelogs, architecture docs updated

Issues

Closes #51, partially addresses #53 (client-side ready, needs server API #49)

Test plan

• App compiles (npm run build + cargo check)
• Settings page shows License card + Account card
• Account card shows sign-in button when not authenticated
• License card shows machine activation section when a license is active
• Edition detection returns Premium when account.json has active subscription

🤖 Generated with Claude Code

## Summary - **Compte Maximus** (OAuth2 PKCE via Logto): sign-in/sign-out in Settings, deep-link callback handler, token storage, daily subscription check - **Machine activation**: activate/deactivate machines against the license server, machine list UI in the license card - **Premium detection**: `get_edition()` now checks account subscription status — Premium overrides Base - **CSP + deep-link**: restrictive Content Security Policy, `tauri-plugin-deep-link` configured for `simpl-resultat://` scheme - i18n FR/EN, changelogs, architecture docs updated ## Issues Closes #51, partially addresses #53 (client-side ready, needs server API #49) ## Test plan - [ ] App compiles (`npm run build` + `cargo check`) - [ ] Settings page shows License card + Account card - [ ] Account card shows sign-in button when not authenticated - [ ] License card shows machine activation section when a license is active - [ ] Edition detection returns Premium when account.json has active subscription 🤖 Generated with [Claude Code](https://claude.com/claude-code)

maximus added 1 commit 2026-04-10 18:19:54 +00:00

feat: Maximus Account OAuth2 PKCE + machine activation + subscription check (#51, #53) ... b53a902f11

- Add auth_commands.rs: OAuth2 PKCE flow (start_oauth, handle_auth_callback,
  refresh_auth_token, get_account_info, check_subscription_status, logout)
- Add deep-link handler in lib.rs for simpl-resultat://auth/callback
- Add AccountCard.tsx + useAuth hook + authService.ts
- Add machine activation commands (activate, deactivate, list, get_activation_status)
- Extend LicenseCard with machine management UI
- get_edition() now checks account subscription for Premium detection
- Daily subscription status check (refresh token if last check > 24h)
- Configure CSP for API/auth endpoints
- Configure tauri-plugin-deep-link for desktop
- Update i18n (FR/EN), changelogs, and architecture docs

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

maximus commented 2026-04-10 18:23:37 +00:00

Author

Owner

Copy link

🔍 PR Review — feat: Maximus Account OAuth2 + machine activation (#51, #53)

Verdict: REQUEST_CHANGES ⛔

Bonne PR dans l'ensemble — l'architecture OAuth2 PKCE est solide, l'intégration deep-link est propre, et la discipline i18n/changelog est respectée. Cependant, deux problèmes bloquants (sécurité + correctness) doivent être corrigés avant le merge.

---

Problèmes bloquants

1. extract_auth_code ne décode pas l'URL du code d'autorisation

src-tauri/src/lib.rs:181 — Le paramètre code est extrait brut du query string sans URL-decoding. Si le provider OIDC (Logto) percent-encode des caractères dans le code (courant avec +, =, /), l'échange de token échouera silencieusement avec une erreur "invalid grant".

Fix: URL-décoder la valeur. Le crate urlencoding est déjà en dépendance :

urlencoding::decode(v).unwrap_or(std::borrow::Cow::Borrowed(v)).to_string()

2. Mutex::lock().unwrap() peut paniquer et crasher l'app

src-tauri/src/commands/auth_commands.rs:139,160 — Si un code path panique en tenant le lock (improbable aujourd'hui mais fragile), le Mutex est empoisonné et ces .unwrap() crashent l'app entière.

Fix: Remplacer par .lock().map_err(|_| "Internal OAuth state error".to_string())? aux deux endroits.

---

Suggestions (non-bloquantes)

• Tokens en JSON plain sur disque (auth_commands.rs:12-13) — Le commentaire reconnaît que c'est acceptable pour v1. Tracker l'intégration OS keychain comme issue de suivi.
• base64_url_encode réimplémente base64 manuellement (auth_commands.rs:97-128) — Le crate base64 est déjà en dépendance (via jsonwebtoken). Utiliser base64::engine::general_purpose::URL_SAFE_NO_PAD.encode() serait plus simple.
• CSP img-src https: trop large (tauri.conf.json:21) — Restreindre au domaine Logto plutôt que tout HTTPS.
• check_subscription_status appelle refresh_auth_token qui est aussi un #[tauri::command] — Extraire la logique dans un helper privé refresh_token_internal().

---

🤖 Review by Claude Code

## 🔍 PR Review — `feat: Maximus Account OAuth2 + machine activation (#51, #53)` **Verdict: REQUEST_CHANGES** ⛔ Bonne PR dans l'ensemble — l'architecture OAuth2 PKCE est solide, l'intégration deep-link est propre, et la discipline i18n/changelog est respectée. Cependant, deux problèmes bloquants (sécurité + correctness) doivent être corrigés avant le merge. --- ### Problèmes bloquants #### 1. `extract_auth_code` ne décode pas l'URL du code d'autorisation **`src-tauri/src/lib.rs:181`** — Le paramètre `code` est extrait brut du query string sans URL-decoding. Si le provider OIDC (Logto) percent-encode des caractères dans le code (courant avec `+`, `=`, `/`), l'échange de token échouera silencieusement avec une erreur "invalid grant". **Fix:** URL-décoder la valeur. Le crate `urlencoding` est déjà en dépendance : ```rust urlencoding::decode(v).unwrap_or(std::borrow::Cow::Borrowed(v)).to_string() ``` #### 2. `Mutex::lock().unwrap()` peut paniquer et crasher l'app **`src-tauri/src/commands/auth_commands.rs:139,160`** — Si un code path panique en tenant le lock (improbable aujourd'hui mais fragile), le Mutex est empoisonné et ces `.unwrap()` crashent l'app entière. **Fix:** Remplacer par `.lock().map_err(|_| "Internal OAuth state error".to_string())?` aux deux endroits. --- ### Suggestions (non-bloquantes) - **Tokens en JSON plain sur disque** (`auth_commands.rs:12-13`) — Le commentaire reconnaît que c'est acceptable pour v1. Tracker l'intégration OS keychain comme issue de suivi. - **`base64_url_encode` réimplémente base64 manuellement** (`auth_commands.rs:97-128`) — Le crate `base64` est déjà en dépendance (via jsonwebtoken). Utiliser `base64::engine::general_purpose::URL_SAFE_NO_PAD.encode()` serait plus simple. - **CSP `img-src https:` trop large** (`tauri.conf.json:21`) — Restreindre au domaine Logto plutôt que tout HTTPS. - **`check_subscription_status` appelle `refresh_auth_token`** qui est aussi un `#[tauri::command]` — Extraire la logique dans un helper privé `refresh_token_internal()`. --- 🤖 Review by Claude Code

maximus added 1 commit 2026-04-10 18:43:28 +00:00

fix: URL-decode auth code + replace Mutex unwrap with map_err ... be5f6a55c5

- extract_auth_code now URL-decodes the code parameter to handle
  percent-encoded characters from the OAuth provider
- Replace Mutex::lock().unwrap() with .lock().map_err() in start_oauth
  and handle_auth_callback to avoid panics on poisoned mutex

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

maximus commented 2026-04-10 18:48:04 +00:00

Author

Owner

Copy link

🔍 PR Review — REQUEST_CHANGES

Summary

Solid OAuth2 PKCE flow and machine activation implementation with good architectural decisions (PKCE verifier in memory, graceful degradation, daily check throttling, CSP). Three blocking issues need to be addressed before merge.

Blocking Issues

1. auth_commands.rs:147-182 — Hand-rolled base64 encoder is fragile and unnecessary. The base64 crate is already available as a transitive dependency (via jsonwebtoken which pulls base64 0.22.1). The custom base64_encode module is unnecessary complexity and a potential source of subtle bugs in the PKCE challenge computation. If the challenge does not match exactly, the token exchange will silently fail with an opaque error from Logto. Replace with base64::engine::general_purpose::URL_SAFE_NO_PAD.

2. auth_commands.rs:265 — Token file written without restrictive permissions. The file is written via fs::write() which inherits the process umask. On Linux with a permissive umask, tokens.json (containing refresh tokens) could be world-readable. The code comment says "acceptable because app data dir has user-only permissions" but Tauri's app_data_dir does NOT guarantee 0700 permissions on all platforms. At minimum, on Unix, set the file to 0600 after writing, or use std::os::unix::fs::OpenOptionsExt to create it with restricted permissions.

3. auth_commands.rs:453 — chrono_now() uses .unwrap() on SystemTime. While SystemTime::now() succeeding is virtually guaranteed, the duration_since(UNIX_EPOCH) can technically panic if the system clock is set before epoch. Use .unwrap_or_default() or return an error to stay consistent with the "fail closed" philosophy used elsewhere (e.g. current_edition).

Non-blocking Suggestions

• Logto App ID (simpl-resultat-desktop): Ensure this matches the actual Logto application configuration. Consider adding a comment/TODO.
• AccountInfo.email defaults to empty string if OIDC provider doesn't return one — consider Option<String> or returning an error.
• Deep-link handler: Consider logging unrecognized deep-link URLs for debugging.
• LicenseCard.tsx — loadMachines only loads on first expand: If user activates/deactivates from another device, they need to close and re-open. Consider loading each time the section is expanded.
• CSP img-src https: is quite broad — tighten to the specific domain serving profile pictures if possible.
• check_account_edition reads from disk on every current_edition() call — since check_entitlement can be called frequently, consider caching in memory with a TTL.

---

🤖 Reviewed by Claude Code (/pr-review)

## 🔍 PR Review — REQUEST_CHANGES ### Summary Solid OAuth2 PKCE flow and machine activation implementation with good architectural decisions (PKCE verifier in memory, graceful degradation, daily check throttling, CSP). Three blocking issues need to be addressed before merge. ### Blocking Issues 1. **`auth_commands.rs:147-182` — Hand-rolled base64 encoder is fragile and unnecessary.** The `base64` crate is already available as a transitive dependency (via `jsonwebtoken` which pulls `base64 0.22.1`). The custom `base64_encode` module is unnecessary complexity and a potential source of subtle bugs in the PKCE challenge computation. If the challenge does not match exactly, the token exchange will silently fail with an opaque error from Logto. Replace with `base64::engine::general_purpose::URL_SAFE_NO_PAD`. 2. **`auth_commands.rs:265` — Token file written without restrictive permissions.** The file is written via `fs::write()` which inherits the process umask. On Linux with a permissive umask, `tokens.json` (containing refresh tokens) could be world-readable. The code comment says "acceptable because app data dir has user-only permissions" but Tauri's `app_data_dir` does NOT guarantee `0700` permissions on all platforms. At minimum, on Unix, set the file to `0600` after writing, or use `std::os::unix::fs::OpenOptionsExt` to create it with restricted permissions. 3. **`auth_commands.rs:453` — `chrono_now()` uses `.unwrap()` on `SystemTime`.** While `SystemTime::now()` succeeding is virtually guaranteed, the `duration_since(UNIX_EPOCH)` can technically panic if the system clock is set before epoch. Use `.unwrap_or_default()` or return an error to stay consistent with the "fail closed" philosophy used elsewhere (e.g. `current_edition`). ### Non-blocking Suggestions - **Logto App ID** (`simpl-resultat-desktop`): Ensure this matches the actual Logto application configuration. Consider adding a comment/TODO. - **`AccountInfo.email` defaults to empty string** if OIDC provider doesn't return one — consider `Option<String>` or returning an error. - **Deep-link handler**: Consider logging unrecognized deep-link URLs for debugging. - **`LicenseCard.tsx` — `loadMachines` only loads on first expand**: If user activates/deactivates from another device, they need to close and re-open. Consider loading each time the section is expanded. - **CSP `img-src https:`** is quite broad — tighten to the specific domain serving profile pictures if possible. - **`check_account_edition` reads from disk on every `current_edition()` call** — since `check_entitlement` can be called frequently, consider caching in memory with a TTL. --- 🤖 Reviewed by Claude Code (`/pr-review`)

maximus added 1 commit 2026-04-10 18:58:21 +00:00

fix: use base64 crate, restrict token file perms, safer chrono_now ... 9e26ad58d1

- Replace hand-rolled base64 encoder with base64::URL_SAFE_NO_PAD crate
- Set 0600 permissions on tokens.json via write_restricted() helper (Unix)
- Replace chrono_now() .unwrap() with .unwrap_or_default()

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

maximus commented 2026-04-10 18:59:59 +00:00

Author

Owner

Copy link

🔍 PR Review — REQUEST_CHANGES

Summary

The OAuth2 PKCE flow and machine activation features are well-structured overall, with good separation of concerns (Rust commands, TS services, React hooks/components). However, there is one security issue that must be fixed before merge.

Blocking Issues

1. src-tauri/src/commands/auth_commands.rs (lines ~217, ~290) — account.json is written with fs::write() (world-readable by default) instead of write_restricted(). This file contains PII (email, name, picture URL) and the subscription_status field that directly gates Premium features. An unprivileged local process could read or overwrite it to bypass the paywall. Both write sites (handle_auth_callback and refresh_auth_token) should use write_restricted() — consistent with the treatment of tokens.json.

Non-blocking Suggestions

1. CSP img-src overly broad (tauri.conf.json): img-src 'self' data: https: allows loading images from any HTTPS origin. Consider restricting to the Logto avatar domain.

2. logout does not clear the PKCE verifier (auth_commands.rs): If a user starts OAuth then logs out, the stale verifier remains in memory. Harmless but could cause a confusing error if the old callback arrives later.

3. Missing useAuth hook in architecture doc (docs/architecture.md): The hooks count was bumped to 14 but the useAuth row is not in the table.

---

Review by pr-review skill

## 🔍 PR Review — REQUEST_CHANGES ### Summary The OAuth2 PKCE flow and machine activation features are well-structured overall, with good separation of concerns (Rust commands, TS services, React hooks/components). However, there is one security issue that must be fixed before merge. ### Blocking Issues 1. **`src-tauri/src/commands/auth_commands.rs` (lines ~217, ~290)** — `account.json` is written with `fs::write()` (world-readable by default) instead of `write_restricted()`. This file contains PII (email, name, picture URL) and the `subscription_status` field that directly gates Premium features. An unprivileged local process could read or overwrite it to bypass the paywall. Both write sites (`handle_auth_callback` and `refresh_auth_token`) should use `write_restricted()` — consistent with the treatment of `tokens.json`. ### Non-blocking Suggestions 1. **CSP `img-src` overly broad** (`tauri.conf.json`): `img-src 'self' data: https:` allows loading images from any HTTPS origin. Consider restricting to the Logto avatar domain. 2. **`logout` does not clear the PKCE verifier** (`auth_commands.rs`): If a user starts OAuth then logs out, the stale verifier remains in memory. Harmless but could cause a confusing error if the old callback arrives later. 3. **Missing `useAuth` hook in architecture doc** (`docs/architecture.md`): The hooks count was bumped to 14 but the `useAuth` row is not in the table. --- *Review by `pr-review` skill*

maximus added 1 commit 2026-04-10 19:04:08 +00:00

fix: use write_restricted for account.json (0600 perms) ... ca3005bc0e

account.json contains PII and subscription_status — apply the same
restricted file permissions as tokens.json.

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

maximus commented 2026-04-10 19:08:30 +00:00

Author

Owner

Copy link

PR Review — feat: Maximus Account OAuth2 + machine activation (#51, #53)

Verdict: REQUEST_CHANGES

Solid implementation overall — clean PKCE flow, proper verifier lifetime management via Tauri managed state, good CSP, well-structured i18n, and graceful degradation on network errors. Two issues to address before merge.

---

Blocking Issues

Issue 1 — last_check file not using restricted permissions
src-tauri/src/commands/auth_commands.rs line ~399: The last_check timestamp file is written via fs::write() (default permissions), while tokens.json and account.json correctly use write_restricted() with 0o600. For consistency across the auth/ directory, use write_restricted() for this file as well — or set directory-level 0o700 permissions on auth/ creation so all files inherit restricted access.

Issue 2 — useAuth missing from architecture docs hook table
docs/architecture.md: The hook count was bumped to 14, but useAuth is not listed in the hook table. Should be added for documentation accuracy.

---

Suggestions (non-blocking)

S1 — .keys-temp/ in .gitignore
Confirmed no private key material was committed in git history. Good practice to add the gitignore entry, but consider removing the directory from the repo entirely if it still exists locally, and document the key generation process in a separate secure location.

S2 — subscription_status from custom_data is fragile
auth_commands.rs reads info["custom_data"]["subscription_status"] from the Logto userinfo endpoint. This depends on Logto being configured to include custom_data in the userinfo response. If the field is absent, Premium will silently never activate. Consider adding a log::debug! or eprintln! warning when custom_data is missing to aid debugging.

S3 — auth-callback-error payload typing
useAuth.ts: listen<string>("auth-callback-error", ...) assumes the Rust side emits a plain String. This works today, but if the error is ever serialized as a JSON-wrapped string, the UI would display spurious quotes. Consider documenting this contract.

---

Review by Claude Code (/pr-review)

## PR Review — `feat: Maximus Account OAuth2 + machine activation (#51, #53)` **Verdict: REQUEST_CHANGES** Solid implementation overall — clean PKCE flow, proper verifier lifetime management via Tauri managed state, good CSP, well-structured i18n, and graceful degradation on network errors. Two issues to address before merge. --- ### Blocking Issues **Issue 1 — `last_check` file not using restricted permissions** `src-tauri/src/commands/auth_commands.rs` line ~399: The `last_check` timestamp file is written via `fs::write()` (default permissions), while `tokens.json` and `account.json` correctly use `write_restricted()` with `0o600`. For consistency across the `auth/` directory, use `write_restricted()` for this file as well — or set directory-level `0o700` permissions on `auth/` creation so all files inherit restricted access. **Issue 2 — `useAuth` missing from architecture docs hook table** `docs/architecture.md`: The hook count was bumped to 14, but `useAuth` is not listed in the hook table. Should be added for documentation accuracy. --- ### Suggestions (non-blocking) **S1 — `.keys-temp/` in `.gitignore`** Confirmed no private key material was committed in git history. Good practice to add the gitignore entry, but consider removing the directory from the repo entirely if it still exists locally, and document the key generation process in a separate secure location. **S2 — `subscription_status` from `custom_data` is fragile** `auth_commands.rs` reads `info["custom_data"]["subscription_status"]` from the Logto userinfo endpoint. This depends on Logto being configured to include `custom_data` in the userinfo response. If the field is absent, Premium will silently never activate. Consider adding a `log::debug!` or `eprintln!` warning when `custom_data` is missing to aid debugging. **S3 — `auth-callback-error` payload typing** `useAuth.ts`: `listen<string>("auth-callback-error", ...)` assumes the Rust side emits a plain `String`. This works today, but if the error is ever serialized as a JSON-wrapped string, the UI would display spurious quotes. Consider documenting this contract. --- _Review by Claude Code (`/pr-review`)_

maximus added 1 commit 2026-04-10 19:14:34 +00:00

fix: restrict last_check file perms + add useAuth to architecture docs ... 4e92882724

- Use write_restricted() for auth/last_check file (consistent 0600)
- Add useAuth hook to the hooks table in docs/architecture.md

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

maximus commented 2026-04-10 19:19:28 +00:00

Author

Owner

Copy link

PR Review — #65 feat: Maximus Account OAuth2 + machine activation

Verdict: REQUEST_CHANGES

Solid implementation of OAuth2 PKCE and machine activation. Architecture is clean, i18n is complete, PKCE flow is well-designed with in-memory verifier. Two blocking issues need attention before merge.

---

Blocking Issues

1. CSP img-src https: leaks user IP to third-party servers
📄 src-tauri/tauri.conf.json

The directive img-src 'self' data: https: allows loading images from any HTTPS server. When AccountCard.tsx renders state.account.picture (e.g. a Google/Gravatar avatar URL from the OIDC provider), the app will fetch the image directly from a third-party server — leaking the user's IP address and the fact that they use Simpl'Résultat.

For a privacy-first app, this should be restricted:

img-src 'self' data: https://auth.lacompagniemaximus.com

Or better: proxy/cache profile pictures locally on first fetch.

2. .keys-temp/ in .gitignore raises key-material concerns
📄 .gitignore

Adding .keys-temp/ to .gitignore implies cryptographic key material was generated in the repo directory. The production Ed25519 public key was swapped in (license_commands.rs), so a private key was generated somewhere. Please:

• Confirm no private key material exists in git history
• Remove the .keys-temp/ gitignore entry from this PR (it's noise and a red flag)
• Document where the production private key lives (the comment says Issue #49)

---

Suggestions (non-blocking)

• Token storage (plain JSON): The comment in auth_commands.rs acknowledges this and plans OS keychain for a future iteration. Acceptable for now — track as a follow-up issue.

• Offline Premium persistence: In check_subscription_status, when refresh fails, the last_check timestamp is still updated. A user could be offline for weeks and retain Premium from the cached account.json. Consider degrading to Base after N consecutive failed checks.

• Event listener cleanup in useAuth.ts: The listen() calls use .then() to push unlisten functions. If the component unmounts before the promises resolve, the cleanup runs with an empty array and listeners leak. Consider an async IIFE pattern inside the effect.

• Logto app ID default: logto_app_id() defaults to "simpl-resultat-desktop" — verify this matches the actual Logto app configuration (Logto usually generates random IDs).

---

🤖 Review by Claude Code

## PR Review — `#65` feat: Maximus Account OAuth2 + machine activation ### Verdict: **REQUEST_CHANGES** Solid implementation of OAuth2 PKCE and machine activation. Architecture is clean, i18n is complete, PKCE flow is well-designed with in-memory verifier. Two blocking issues need attention before merge. --- ### Blocking Issues **1. CSP `img-src https:` leaks user IP to third-party servers** 📄 `src-tauri/tauri.conf.json` The directive `img-src 'self' data: https:` allows loading images from **any** HTTPS server. When `AccountCard.tsx` renders `state.account.picture` (e.g. a Google/Gravatar avatar URL from the OIDC provider), the app will fetch the image directly from a third-party server — leaking the user's IP address and the fact that they use Simpl'Résultat. For a **privacy-first** app, this should be restricted: ``` img-src 'self' data: https://auth.lacompagniemaximus.com ``` Or better: proxy/cache profile pictures locally on first fetch. **2. `.keys-temp/` in `.gitignore` raises key-material concerns** 📄 `.gitignore` Adding `.keys-temp/` to `.gitignore` implies cryptographic key material was generated in the repo directory. The production Ed25519 public key was swapped in (`license_commands.rs`), so a private key was generated somewhere. Please: - Confirm no private key material exists in git history - Remove the `.keys-temp/` gitignore entry from this PR (it's noise and a red flag) - Document where the production private key lives (the comment says Issue #49) --- ### Suggestions (non-blocking) - **Token storage (plain JSON)**: The comment in `auth_commands.rs` acknowledges this and plans OS keychain for a future iteration. Acceptable for now — track as a follow-up issue. - **Offline Premium persistence**: In `check_subscription_status`, when refresh fails, the `last_check` timestamp is still updated. A user could be offline for weeks and retain Premium from the cached `account.json`. Consider degrading to Base after N consecutive failed checks. - **Event listener cleanup in `useAuth.ts`**: The `listen()` calls use `.then()` to push unlisten functions. If the component unmounts before the promises resolve, the cleanup runs with an empty array and listeners leak. Consider an async IIFE pattern inside the effect. - **Logto app ID default**: `logto_app_id()` defaults to `"simpl-resultat-desktop"` — verify this matches the actual Logto app configuration (Logto usually generates random IDs). --- 🤖 Review by Claude Code

maximus added 1 commit 2026-04-10 19:24:55 +00:00

fix: tighten CSP img-src, show initials instead of external avatar ... 60b995394e

Privacy-first: remove 'https:' from img-src CSP directive to prevent
IP leaks via external avatar URLs (Google/Gravatar). AccountCard now
shows user initials instead of loading a remote image.

Also remove .keys-temp/ from .gitignore (not relevant to this PR).

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

maximus commented 2026-04-10 19:26:56 +00:00

Author

Owner

Copy link

🔍 PR Review — REQUEST_CHANGES

Reviewer: Claude Code (adversarial review)

La PR est bien structurée et couvre OAuth2 PKCE, activation de machines, et détection Premium. Cependant, il y a des problèmes bloquants.

---

Issues bloquantes

1. src-tauri/src/commands/license_commands.rs:25-26 — La clé publique de production casse les tests existants.

La clé PEM a été changée de la clé de test RFC 8410 vers une clé de production. Les tests license_commands.rs (#[cfg(test)]) signent des JWTs avec la clé privée correspondant à l'ancienne clé publique. Avec la nouvelle clé, cargo test devrait échouer. Il faut soit conditionner la clé en #[cfg(test)], soit adapter les tests.

2. src-tauri/src/commands/auth_commands.rs:55-60 — Tokens OAuth stockés en clair sur disque.

Le commentaire mentionne "encrypted at rest in a future iteration via OS keychain". Le refresh token en particulier permet un accès persistant. Action requise : documenter cette limitation dans la PR body et créer une issue de suivi pour l'intégration keychain.

3. src-tauri/src/lib.rs:152 — handle_auth_callback est exposé comme commande Tauri invocable depuis JS ET appelé depuis le deep-link handler.

Le deep-link handler appelle déjà commands::handle_auth_callback directement côté Rust. L'exposer aussi comme commande Tauri dans invoke_handler crée une surface d'attaque inutile. Retirer commands::handle_auth_callback de la liste invoke_handler.

---

Suggestions (non bloquantes)

• logto_app_id() retourne "simpl-resultat-desktop" — vérifier que c'est bien le client ID configuré dans Logto (souvent un UUID).
• Le paramètre OAuth2 state n'est pas utilisé. PKCE mitigue le risque principal, mais state protège contre le login CSRF.
• useAuth.ts:107-115 — les listen() retournent des promises sans .catch().
• LicenseCard.tsx gère 6 useState — envisager d'extraire un hook useMachineActivation.
• fetch_userinfo utilise /oidc/me — vérifier que c'est le bon endpoint Logto (standard OIDC = /oidc/userinfo).

## 🔍 PR Review — REQUEST_CHANGES **Reviewer:** Claude Code (adversarial review) La PR est bien structurée et couvre OAuth2 PKCE, activation de machines, et détection Premium. Cependant, il y a des problèmes bloquants. --- ### Issues bloquantes **1. `src-tauri/src/commands/license_commands.rs:25-26` — La clé publique de production casse les tests existants.** La clé PEM a été changée de la clé de test RFC 8410 vers une clé de production. Les tests `license_commands.rs` (`#[cfg(test)]`) signent des JWTs avec la clé privée correspondant à l'ancienne clé publique. Avec la nouvelle clé, `cargo test` devrait échouer. Il faut soit conditionner la clé en `#[cfg(test)]`, soit adapter les tests. **2. `src-tauri/src/commands/auth_commands.rs:55-60` — Tokens OAuth stockés en clair sur disque.** Le commentaire mentionne "encrypted at rest in a future iteration via OS keychain". Le refresh token en particulier permet un accès persistant. **Action requise :** documenter cette limitation dans la PR body et créer une issue de suivi pour l'intégration keychain. **3. `src-tauri/src/lib.rs:152` — `handle_auth_callback` est exposé comme commande Tauri invocable depuis JS ET appelé depuis le deep-link handler.** Le deep-link handler appelle déjà `commands::handle_auth_callback` directement côté Rust. L'exposer aussi comme commande Tauri dans `invoke_handler` crée une surface d'attaque inutile. Retirer `commands::handle_auth_callback` de la liste `invoke_handler`. --- ### Suggestions (non bloquantes) - `logto_app_id()` retourne `"simpl-resultat-desktop"` — vérifier que c'est bien le client ID configuré dans Logto (souvent un UUID). - Le paramètre OAuth2 `state` n'est pas utilisé. PKCE mitigue le risque principal, mais `state` protège contre le login CSRF. - `useAuth.ts:107-115` — les `listen()` retournent des promises sans `.catch()`. - `LicenseCard.tsx` gère 6 `useState` — envisager d'extraire un hook `useMachineActivation`. - `fetch_userinfo` utilise `/oidc/me` — vérifier que c'est le bon endpoint Logto (standard OIDC = `/oidc/userinfo`).

maximus added 1 commit 2026-04-10 19:35:14 +00:00

fix: remove handle_auth_callback from invoke_handler ... e314bbe1e3

The auth callback is handled exclusively via the deep-link handler in
lib.rs — exposing it as a JS-invocable command is unnecessary attack
surface. The frontend listens for auth-callback-success/error events
instead.

Plaintext token storage documented as known limitation (see #66).

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

maximus merged commit 4b42d53659 into main 2026-04-10 19:38:28 +00:00

maximus referenced this pull request from a commit 2026-04-10 19:38:29 +00:00

Merge pull request 'feat: Maximus Account OAuth2 + machine activation (#51, #53)' (#65) from issue-51-compte-maximus-oauth into main

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

Subscribe

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#65

No description provided.