diff --git a/Cargo.lock b/Cargo.lock index 84defcb..9ed7e4e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -987,6 +987,7 @@ dependencies = [ "nucleo", "prost", "prost-types", + "quick-xml", "ratatui", "regex", "rstest", diff --git a/client b/client index fb149f1..31ed0b2 160000 --- a/client +++ b/client @@ -1 +1 @@ -Subproject commit fb149f196bd47f316ede6371ec848822397fd4a3 +Subproject commit 31ed0b28f8c0e6495e72a69e43a1b09d5dbd0332 diff --git a/crossplatform_storage.md b/crossplatform_storage.md new file mode 100644 index 0000000..a3bdcd4 --- /dev/null +++ b/crossplatform_storage.md @@ -0,0 +1,171 @@ +# Cross-platform storage for the client (Windows / macOS / Linux) + +This document covers two things: + +1. Where to store **app-internal state** (the auto-login token, config, caches). +2. Where to store **user-facing output files** (exports). + +And which crate to use so you never hand-roll platform paths. + +--- + +## 1. The shortcut crate: `directories` + +Yes, there is a crate for exactly this: [`directories`](https://crates.io/crates/directories). +You already depend on its little sibling `dirs = "6.0.0"`, which gives you the *base* +folders (`home_dir`, `data_dir`, `download_dir`, …). `directories` builds on the same +logic but adds `ProjectDirs`, which computes the **per-application** subdirectory +following each OS's conventions in one call: + +```toml +# client/Cargo.toml +[dependencies] +directories = "6" +``` + +```rust +use directories::ProjectDirs; + +// (qualifier, organization, application) +let proj = ProjectDirs::from("com", "komp-ac", "komp_ac_client") + .ok_or_else(|| anyhow::anyhow!("no home directory available"))?; + +proj.config_dir(); // settings the user may edit +proj.data_dir(); // app-owned data (safe default for the token too) +proj.cache_dir(); // re-creatable stuff, OK to delete +proj.state_dir(); // Option<&Path> — logs, session state; Linux-only concept +``` + +What that resolves to: + +| Call | Linux | macOS | Windows | +|----------------|------------------------------------|---------------------------------------------------|--------------------------------------------------| +| `config_dir()` | `~/.config/komp_ac_client` | `~/Library/Application Support/com.komp-ac.komp_ac_client` | `%APPDATA%\komp-ac\komp_ac_client\config` | +| `data_dir()` | `~/.local/share/komp_ac_client` | `~/Library/Application Support/com.komp-ac.komp_ac_client` | `%APPDATA%\komp-ac\komp_ac_client\data` | +| `cache_dir()` | `~/.cache/komp_ac_client` | `~/Library/Caches/com.komp-ac.komp_ac_client` | `%LOCALAPPDATA%\komp-ac\komp_ac_client\cache` | +| `state_dir()` | `~/.local/state/komp_ac_client` | `None` | `None` | + +> Alternative: keep only `dirs` and compose paths yourself (`dirs::data_dir()?.join(APP_NAME)`). +> That works, but `ProjectDirs` removes the per-OS naming decisions entirely, so prefer it. + +--- + +## 2. Fixing the auto-login token path (the current bug) + +`client/src/config/storage/storage.rs:34` currently does: + +```rust +let state_dir = dirs::state_dir() + .or_else(|| dirs::home_dir().map(|home| home.join(".local").join("state"))) +``` + +`dirs::state_dir()` returns `Some` **only on Linux**. On Windows and macOS it is +`None`, so the fallback kicks in and you get `C:\Users\x\.local\state\...` and +`/Users/x/.local/state/...` — it *works*, but it's a hidden Unix-style litter +directory that no Windows/macOS user or uninstaller will ever find. + +Cross-platform fix — fall back to the **data dir**, which exists everywhere: + +```rust +pub fn get_token_storage_path() -> Result { + let proj = directories::ProjectDirs::from("com", "komp-ac", APP_NAME) + .ok_or_else(|| anyhow::anyhow!("Could not determine app directories"))?; + + // state_dir is the right place on Linux; data_dir is the right place elsewhere. + let dir = proj.state_dir().unwrap_or_else(|| proj.data_dir()).to_path_buf(); + + fs::create_dir_all(&dir) + .with_context(|| format!("Failed to create app state directory at {dir:?}"))?; + + Ok(dir.join(TOKEN_FILE_NAME)) +} +``` + +**Migration:** existing Linux users keep the same path (`~/.local/state/komp_ac_client`), +so nothing breaks there. If you care about existing Windows/macOS installs that already +wrote to `~/.local/state`, add a one-time check: if the new path doesn't exist but the +old one does, move the file. + +### Token file security per platform + +- **Unix:** you already `chmod 600` — keep the `#[cfg(unix)]` block as is. +- **Windows:** there is no mode-bit equivalent, but `%APPDATA%` is already + ACL-restricted to the current user, so a plain file there is the accepted baseline. + No extra code needed. +- **Optional upgrade:** the [`keyring`](https://crates.io/crates/keyring) crate stores + the token in the OS credential store (Windows Credential Manager, macOS Keychain, + Linux Secret Service). More secure, but adds a D-Bus/secret-service requirement on + Linux and Keychain prompts on macOS. For a TUI client, the ACL'd file is a fine + default; consider `keyring` only for the `access_token` field if security review + demands it. + +--- + +## 3. Where output/export files go + +Rule of thumb: **files the user asked for go in user-visible folders; files the app +needs go in `ProjectDirs`.** Never write outputs next to the executable (unwritable +under `Program Files` / `/usr/bin`) and never default to the current working directory +(unpredictable for a GUI/TUI launched from a launcher). + +Your export logic (`client/src/pages/import_export/export/logic.rs:70`) already does +the right first step: + +```rust +dirs::download_dir().unwrap_or_else(std::env::temp_dir) +``` + +Two improvements: + +1. `download_dir()` can be `None` on minimal Linux setups (no `xdg-user-dirs` + installed → no `~/Downloads` mapping). Falling all the way to `temp_dir()` means + exports silently land in `/tmp` and get wiped on reboot. Prefer a chain: + + ```rust + let out_dir = dirs::download_dir() + .or_else(dirs::document_dir) + .or_else(dirs::home_dir) + .unwrap_or_else(std::env::temp_dir) + .join(APP_NAME); // keep exports grouped in one folder + ``` + +2. Always **show the resolved absolute path in the status line** after export (you + already do this via `last_export_path`) — on three platforms the default location + differs, so telling the user where the file went matters more than which folder + you picked. + +For program outputs that are *not* user-requested (logs, generated caches, import +scratch files): use `proj.state_dir()/data_dir()` for logs, `proj.cache_dir()` for +scratch. Same `ProjectDirs` instance, no new decisions. + +--- + +## 4. General cross-platform hygiene checklist + +- Build every path with `PathBuf::join`, never string concatenation with `/` or `\\`. + (The codebase already does this — keep it that way.) +- Always `fs::create_dir_all` before the first write; none of these directories are + guaranteed to exist, especially on a fresh Windows profile. +- Don't read `$HOME` / `%USERPROFILE%` env vars directly — that's exactly what + `dirs`/`directories` abstracts (including Windows Known Folders redirection). +- Handle the `None` case from every `dirs::*` call (headless CI, service accounts, + containers have no home). Return an error or fall back to `temp_dir` explicitly. +- Filenames typed by the user: reject `/`, `\`, and reserved Windows names + (`CON`, `NUL`, `COM1`, trailing dots/spaces) before creating export files, or the + export will fail only on Windows. +- Test matrix: the cheap smoke test is `cargo check --target x86_64-pc-windows-gnu` + plus running the path-resolution unit tests with the `dirs` values mocked; the real + test is running the client once per OS and checking where `auth.token` and one + export actually land. + +--- + +## 5. Summary of concrete steps + +1. Add `directories = "6"` to `client/Cargo.toml` (keep `dirs` for `download_dir` etc.). +2. Create one `ProjectDirs::from("com", "komp-ac", "komp_ac_client")` helper (e.g. in + `client/src/config/storage/`) and route all internal paths through it. +3. Rewrite `get_token_storage_path()` as in §2 → auto-login token lands in the + idiomatic per-OS location; Linux path unchanged. +4. Improve the export default-directory chain as in §3 (`download → documents → home → temp`). +5. (Optional) move the raw `access_token` into `keyring` later. diff --git a/tui-pages b/tui-pages index 981a77c..750d357 160000 --- a/tui-pages +++ b/tui-pages @@ -1 +1 @@ -Subproject commit 981a77c86377b28f9297551f22ed99004e9e4a99 +Subproject commit 750d357d9ce86615d3a401b6f9068cc2f03a2e27