Repository navigation
fix: allow safe self-service account deletion - #387
DonovanMontoya wants to merge 2 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07b33170af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| user_id: &str, | ||
| self_deletion: Option<(&str, Option<&str>)>, | ||
| ) -> Result<(), ApiError> { | ||
| let _account_guard = state.database.account_gate.write().await; |
There was a problem hiding this comment.
If another listener is preparing a large book download or uploading a book, deletion queues an exclusive lock behind that entire operation: auth_middleware holds the shared lock through next.run, including the explicitly unbounded routes in apps/server/src/app.rs:395-429. Tokio's write-preferring lock then blocks every subsequent authenticated request, including other listeners' progress saves, media requests, and sign-ins, until deletion acquires the lock or hits its 90-second timeout (app.rs:363-365). Thus deleting one account can make the whole server unresponsive while an unrelated transfer runs. Scope request draining to the account being deleted, or otherwise avoid holding this global gate across long-running operations.
Useful? React with 👍 / 👎.
|
Closing for now. OperaLibre accounts live on each owner's own server, administrators can already remove accounts in Administration, and iOS 1.3.1 passed App Review without self-service deletion. The new account gate also makes every authenticated request wait behind a pending deletion, including long uploads, which is a lot of risk for a feature nobody has asked for. The branch stays in place if App Review ever asks for this. |
What changed
OperaLibre accounts could be created through remote owner setup and Administration, but readers could not initiate deletion of their own account. Settings now provides password-confirmed permanent deletion, cancellation, and sign-out after server confirmation. This addresses the missing in-app initiation described in Apple's account deletion guidance.
The authenticated-account endpoint accepts no target user ID. Ownership transfer and personal-record cleanup commit in one database transaction, active account requests drain before deletion, and all session/media/access tokens are invalidated. Cleanup includes progress and its backups, settings, reading history, activity, avatars, download requests and server-stored Libro.fm credentials. Administrator deletion shares the same cleanup.
The final owner must explicitly transfer control to another existing account; the form explains how to create a replacement first. Shared library files, server configuration and other listeners remain. Local/demo mode has no server account, and Jellyfin deletion is explicitly unsupported. User/API documentation explains backup and external-copy limitations.
Screenshots
After: mobile-width browser fixture rendering the actual owner deletion component. Baseline screenshot unavailable; the previous version had no self-deletion control. This is fixture evidence, not a native-device capture.
Testing
Local macOS checks:
--locked --all-targets -- -D warnings) passed.TMPDIR=/tmp cargo test --locked --manifest-path apps/server/Cargo.toml: 655 passed, 7 ignored. The default macOS temporary path triggers the existing Libation fast-exit fixture failure on both this branch and a separately rebuilt, untouched a6e51c0 baseline; the short temporary path passes the full suite.No simulator or physical-device testing. Native clients share this React flow, but native runtime behavior and App Store acceptance are not established by these checks. Remote CI will run on the PR. No live library or server data was used. PR #384 is unchanged.