diff --git a/backend/tests/test_telemetry.py b/backend/tests/test_telemetry.py index c7e4ddc..7856207 100644 --- a/backend/tests/test_telemetry.py +++ b/backend/tests/test_telemetry.py @@ -144,6 +144,10 @@ def test_telemetry_api_routes(client): token = login_r.json()["access_token"] headers = {"Authorization": f"Bearer {token}"} + now = datetime.now(timezone.utc) + started_at = (now - timedelta(minutes=5)).isoformat() + completed_at = now.isoformat() + # 2. Ingest telemetry batch payload = { "events": [ @@ -164,8 +168,8 @@ def test_telemetry_api_routes(client): "client_type": "desktop-macos", "client_version": "1.3.2", "device_name": "TestMac", - "started_at": "2026-09-16T21:00:00Z", - "completed_at": "2026-09-16T21:01:00Z", + "started_at": started_at, + "completed_at": completed_at, "duration_ms": 60000, "status": "failed", "files_scanned": 100, diff --git a/client-tauri/src-tauri/src/api.rs b/client-tauri/src-tauri/src/api.rs index e5166ff..6700386 100644 --- a/client-tauri/src-tauri/src/api.rs +++ b/client-tauri/src-tauri/src/api.rs @@ -789,8 +789,7 @@ impl ApiClient { }; let mut last_err = String::new(); for attempt in 0..4 { - let mut headers = self.headers(); - headers.insert("X-E2E-Return-Temp-Password", "1".parse().unwrap()); + let headers = self.headers(); match self.client().post(&url).headers(headers).json(&body).header("Content-Type", "application/json").send() { Ok(r) => { if r.status().is_success() { @@ -896,5 +895,33 @@ mod tests { let h_without = client.headers(); assert!(!h_without.contains_key("X-Sync-ID")); } + + #[test] + fn test_create_user_does_not_send_e2e_password_header() { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + + let handle = std::thread::spawn(move || { + let (mut stream, _) = listener.accept().unwrap(); + use std::io::{Read, Write}; + let mut buf = [0u8; 4096]; + let n = stream.read(&mut buf).unwrap(); + let req = String::from_utf8_lossy(&buf[..n]).to_string(); + + let resp = "HTTP/1.1 200 OK\r\nContent-Length: 2\r\nContent-Type: application/json\r\n\r\n{}"; + let _ = stream.write_all(resp.as_bytes()); + req + }); + + let client = ApiClient::new(format!("http://127.0.0.1:{}", port)); + let _ = client.create_user("test@example.com", "Test", "User"); + + let req = handle.join().unwrap(); + assert!( + !req.to_lowercase().contains("x-e2e-return-temp-password"), + "create_user must not send X-E2E-Return-Temp-Password header in production client calls, but got:\n{}", + req + ); + } } diff --git a/client-tauri/src/Settings.tsx b/client-tauri/src/Settings.tsx index eee5aa7..03f1c7c 100644 --- a/client-tauri/src/Settings.tsx +++ b/client-tauri/src/Settings.tsx @@ -32,6 +32,7 @@ import ExpandMore from "@mui/icons-material/ExpandMore"; import ExpandLess from "@mui/icons-material/ExpandLess"; import Refresh from "@mui/icons-material/Refresh"; import FolderOpen from "@mui/icons-material/FolderOpen"; +import ContentCopy from "@mui/icons-material/ContentCopy"; import { formatUserFacingError } from "./errors"; function formatBytes(n: number): string { @@ -71,6 +72,10 @@ export default function Settings({ email, onLogout }: SettingsProps) { const [createUserOpen, setCreateUserOpen] = useState(false); const [createUserError, setCreateUserError] = useState(""); const [adminActionError, setAdminActionError] = useState(null); + const [adminActionSuccess, setAdminActionSuccess] = useState(null); + const [creatingUser, setCreatingUser] = useState(false); + const [createdUserInfo, setCreatedUserInfo] = useState<{ email: string; temp_password?: string } | null>(null); + const [copiedPassword, setCopiedPassword] = useState(false); const [newUserEmail, setNewUserEmail] = useState(""); const [newUserFirst, setNewUserFirst] = useState(""); const [newUserLast, setNewUserLast] = useState(""); @@ -198,8 +203,16 @@ export default function Settings({ email, onLogout }: SettingsProps) { const handleCreateUser = async () => { setCreateUserError(""); + setAdminActionError(null); + setAdminActionSuccess(null); + setCreatingUser(true); try { - await invoke("api_create_user", { + const res = await invoke<{ + email: string; + first_name?: string; + last_name?: string; + temp_password?: string; + }>("api_create_user", { email: newUserEmail, first_name: newUserFirst, last_name: newUserLast, @@ -208,11 +221,20 @@ export default function Settings({ email, onLogout }: SettingsProps) { setNewUserEmail(""); setNewUserFirst(""); setNewUserLast(""); - setAdminActionError(null); loadUsers(); + if (res?.temp_password) { + setCreatedUserInfo({ + email: res.email, + temp_password: res.temp_password, + }); + } else { + setAdminActionSuccess("User created successfully. Welcome email sent with login credentials."); + } } catch (e) { setCreateUserError(formatUserFacingError(e)); console.error(e); + } finally { + setCreatingUser(false); } }; @@ -505,10 +527,16 @@ export default function Settings({ email, onLogout }: SettingsProps) { {adminActionError} )} + {adminActionSuccess && ( + setAdminActionSuccess(null)}> + {adminActionSuccess} + + )} - + + + + { + setCreatedUserInfo(null); + setCopiedPassword(false); + }} + maxWidth="xs" + fullWidth + > + User created + + + User {createdUserInfo?.email} was created successfully. + + + SMTP is not configured on the server, so no email was sent. Please securely share this temporary password with the user. It will not be shown again. + + + + + + + + + + ); } diff --git a/openspec/changes/fix-client-create-user-password-handling/.openspec.yaml b/openspec/changes/fix-client-create-user-password-handling/.openspec.yaml new file mode 100644 index 0000000..49ea85b --- /dev/null +++ b/openspec/changes/fix-client-create-user-password-handling/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-10-14 diff --git a/openspec/changes/fix-client-create-user-password-handling/design.md b/openspec/changes/fix-client-create-user-password-handling/design.md new file mode 100644 index 0000000..76f7e32 --- /dev/null +++ b/openspec/changes/fix-client-create-user-password-handling/design.md @@ -0,0 +1,47 @@ +## Context + +In `client-tauri/src-tauri/src/api.rs`, `ApiClient::create_user` currently injects: +```rust +headers.insert("X-E2E-Return-Temp-Password", "1".parse().unwrap()); +``` +This header was introduced for autonomous E2E testing scenarios to prevent sending real emails and return passwords directly in HTTP responses. However, because it was unconditionally hardcoded into `ApiClient::create_user`, all user creation actions from the desktop application trigger `skip_email = True` in the backend service (`backend/app/users/service.py`). + +In the client frontend (`client-tauri/src/Settings.tsx`), `handleCreateUser` calls `invoke("api_create_user", ...)` without inspecting the response: +```tsx +await invoke("api_create_user", { + email: newUserEmail, + first_name: newUserFirst, + last_name: newUserLast, +}); +setCreateUserOpen(false); +``` +When `temp_password` is returned in the response, it is dropped silently without being displayed. + +## Goals / Non-Goals + +**Goals:** +- Ensure the production desktop client does not transmit `X-E2E-Return-Temp-Password`, allowing the backend to dispatch welcome emails when SMTP is configured. +- Ensure the desktop client frontend captures any returned `temp_password` from `api_create_user` and displays it to the administrator in a clear modal dialog with a copy button. +- Provide a clear success message when a user is created and the password was delivered via email. + +**Non-Goals:** +- Changing backend user creation logic or endpoints (`POST /api/users` response schema already supports optional `temp_password`). +- Adding password reset workflows for existing users. + +## Decisions + +- **Decision 1: Remove `X-E2E-Return-Temp-Password` from `ApiClient::create_user`** + - Rationale: The desktop client is a production user-facing application and should not spoof test runner headers. Test runners (like `tests/e2e/e2e_setup.py`) already use the dedicated Python API client with `e2e_return_temp_password=True`. + - Alternative considered: Adding a checkbox in the UI to toggle email sending - rejected because admins shouldn't need to manually configure backend delivery modes. + +- **Decision 2: Modal dialog for temporary password display in Settings.tsx** + - Rationale: If the backend does not have SMTP configured (or returns a password), the temporary password is only available once in the API response. Closing the modal immediately causes permanent credential loss. Presenting a result modal with a copy-to-clipboard button ensures the administrator can safely copy and transfer credentials to the user. + - Alternative considered: Displaying the password in an inline alert banner - rejected because users often navigate away or close tabs/windows, whereas a dialog requires acknowledgment and prevents accidental dismissal before copying. + +- **Decision 3: Unit test using local mock TCP server** + - Rationale: `client-tauri/src-tauri/src/api.rs` unit tests can spin up a local TCP listener to verify that `ApiClient::create_user` does not include `X-E2E-Return-Temp-Password` in the transmitted HTTP request headers. + +## Risks / Trade-offs + +- [Risk] Server without SMTP won't send emails → + - Mitigation: The server already returns `temp_password` when SMTP is unconfigured; the new UI explicitly presents it to the administrator with instructions to share it. diff --git a/openspec/changes/fix-client-create-user-password-handling/proposal.md b/openspec/changes/fix-client-create-user-password-handling/proposal.md new file mode 100644 index 0000000..f151292 --- /dev/null +++ b/openspec/changes/fix-client-create-user-password-handling/proposal.md @@ -0,0 +1,28 @@ +## Why + +When an administrator creates a new user through the desktop client (Tauri UI), the desktop client unexpectedly includes the test header `X-E2E-Return-Temp-Password: 1` on the `POST /api/users` request. This causes the backend to bypass email delivery (`skip_email = True`), preventing the user from ever receiving their initial password email—even when SMTP is fully configured and operational. + +Simultaneously, the backend responds with the generated temporary password in the JSON payload (`temp_password`), but the desktop client's settings UI completely ignores the API response and immediately closes the dialog without displaying the temporary password or offering a way to copy it. As a result, the newly created user cannot receive an email, the administrator cannot view or copy the generated password, and the account is rendered completely inaccessible. Furthermore, in environments where SMTP is intentionally not configured (or email delivery is not used), the returned temporary password is also discarded and lost. + +## What Changes + +- In `client-tauri/src-tauri/src/api.rs`, remove the hardcoded `X-E2E-Return-Temp-Password: 1` request header from `ApiClient::create_user`. Normal client user creation requests will allow the backend to send welcome emails with temporary passwords when SMTP is configured. +- In `client-tauri/src-tauri/src/api.rs`, add unit test coverage verifying that `create_user` does not include `X-E2E-Return-Temp-Password` in request headers. +- In `client-tauri/src/Settings.tsx`, update `handleCreateUser` to inspect the response returned by `api_create_user`: + - When `temp_password` is present in the response (e.g. SMTP not configured on server or password returned), display a dedicated user creation success dialog showing the created user email, the generated temporary password, a button to copy the password to the clipboard, and an explanatory notice to securely share it with the user. + - When `temp_password` is absent (indicating the backend emailed the credentials), display an informative success banner confirming that the user was created and the invitation email was sent. + +## Capabilities + +### New Capabilities + + +### Modified Capabilities +- `desktop-client`: Require the desktop client to omit test password headers during administrative user creation, allowing the server to deliver password emails when configured, and require the client settings UI to capture and display generated temporary passwords when returned by the API. + +## Impact + +- **Affected code**: `client-tauri/src-tauri/src/api.rs`, `client-tauri/src/Settings.tsx` +- **APIs**: No backend API contract changes (`POST /api/users` already returns `UserCreateResponse` with optional `temp_password`). +- **Dependencies**: No new dependencies. +- **Behavior**: User accounts created from the desktop client will reliably receive email invitations when SMTP is configured, and administrators will be shown temporary passwords with copy-to-clipboard functionality when SMTP is not configured. diff --git a/openspec/changes/fix-client-create-user-password-handling/specs/desktop-client/spec.md b/openspec/changes/fix-client-create-user-password-handling/specs/desktop-client/spec.md new file mode 100644 index 0000000..216326f --- /dev/null +++ b/openspec/changes/fix-client-create-user-password-handling/specs/desktop-client/spec.md @@ -0,0 +1,12 @@ +## ADDED Requirements + +### Requirement: Admin User Creation Password Handling +The desktop client SHALL perform administrative user creation without bypassing server-side email notifications, and SHALL present any returned temporary password directly to the administrator with clipboard copy support. + +#### Scenario: User creation with server-configured SMTP +- **WHEN** an administrator creates a new user through the desktop client and the server has SMTP configured +- **THEN** the client does not send the `X-E2E-Return-Temp-Password` header, allowing the server to deliver the initial password email, and informs the administrator that credentials have been emailed + +#### Scenario: User creation without server SMTP or with returned temporary password +- **WHEN** an administrator creates a new user through the desktop client and the server returns a temporary password in the response payload +- **THEN** the client displays the temporary password in a completion dialog with an option to copy the password to the clipboard before dismissing diff --git a/openspec/changes/fix-client-create-user-password-handling/tasks.md b/openspec/changes/fix-client-create-user-password-handling/tasks.md new file mode 100644 index 0000000..0e7585f --- /dev/null +++ b/openspec/changes/fix-client-create-user-password-handling/tasks.md @@ -0,0 +1,14 @@ +## 1. Client API Header Correction + +- [x] 1.1 Remove `X-E2E-Return-Temp-Password` header insertion in `client-tauri/src-tauri/src/api.rs` and verify `cargo test` passes +- [x] 1.2 Add unit test in `client-tauri/src-tauri/src/api.rs` asserting `create_user` does not send `X-E2E-Return-Temp-Password` and verify with `cargo test` + +## 2. Desktop Settings UI Password Display + +- [x] 2.1 Update `handleCreateUser` and user creation dialog in `client-tauri/src/Settings.tsx` to handle `temp_password` and email confirmation +- [x] 2.2 Add temporary password modal dialog with copy-to-clipboard functionality in `client-tauri/src/Settings.tsx` +- [x] 2.3 Verify `npm run build` in `client-tauri` succeeds + +## 3. QA and Verification + +- [x] 3.1 Run `./scripts/run-qa.sh` to ensure all tests across backend, client-tauri, web, and docs pass