enable QtKeychain with Wasm - #515
Merged
Merged
Conversation
Reviewer's GuideEnables QtKeychain-based password storage for the Wasm build and refactors the account/password UI and password management flow to use a new ManagePasswordDialog and async, non-blocking dialogs, while extending PasswordConfig to support per-account keys and deletion notifications and updating build/Docker config to fetch and build a QtKeychain fork with Wasm support. Sequence diagram for non-Wasm password set and auto-login flowsequenceDiagram
actor User
participant GeneralPage
participant ManagePasswordDialog
participant PasswordConfig
participant WriteJob as QKeychain::WritePasswordJob
User->>GeneralPage: clicks setPassword button
GeneralPage->>GeneralPage: slot_setPasswordClicked()
alt password already saved
GeneralPage->>PasswordConfig: getPassword()
PasswordConfig->>WriteJob: m_readJob.start()
Note over PasswordConfig,WriteJob: Read job emits sig_incomingPassword when finished
else no password saved
GeneralPage->>ManagePasswordDialog: create dialog and open()
end
rect rgb(230,230,250)
PasswordConfig-->>GeneralPage: sig_incomingPassword(password)
GeneralPage->>ManagePasswordDialog: create dialog
GeneralPage->>ManagePasswordDialog: setAccountName(accountName)
GeneralPage->>ManagePasswordDialog: setPassword(password)
ManagePasswordDialog->>ManagePasswordDialog: user edits account and password
User-->>ManagePasswordDialog: clicks OK
ManagePasswordDialog-->>GeneralPage: accepted()
GeneralPage->>GeneralPage: update config.account.accountName
GeneralPage->>PasswordConfig: setPassword(newPassword)
PasswordConfig->>WriteJob: m_writeJob.start()
WriteJob-->>PasswordConfig: finished()
PasswordConfig-->>GeneralPage: sig_passwordSaved()
GeneralPage->>GeneralPage: setConfig().account.accountPassword = true
GeneralPage->>GeneralPage: updateAutoLoginEnabled()
alt autoLogin not checked
GeneralPage->>GeneralPage: show QMessageBox Question
User-->>GeneralPage: chooses Yes
GeneralPage->>GeneralPage: ui->autoLogin->setChecked(true)
GeneralPage->>GeneralPage: setConfig().account.rememberLogin = true
end
end
rect rgb(240,255,240)
ManagePasswordDialog-->>GeneralPage: sig_deleteRequested()
GeneralPage->>PasswordConfig: deletePassword()
PasswordConfig->>WriteJob: m_deleteJob.start()
WriteJob-->>PasswordConfig: finished()
PasswordConfig-->>GeneralPage: sig_passwordDeleted()
GeneralPage->>GeneralPage: clear accountPassword and rememberLogin
GeneralPage->>GeneralPage: ui->autoLogin->setChecked(false)
GeneralPage->>GeneralPage: updateAutoLoginEnabled()
end
Sequence diagram for Wasm account management with QtKeychainsequenceDiagram
actor User
participant GeneralPage
participant ManagePasswordDialog
participant PasswordConfig
participant WriteJob as QKeychain::WritePasswordJob
User->>GeneralPage: clicks setPassword button
GeneralPage->>GeneralPage: slot_setPasswordClicked()
Note over GeneralPage: CURRENT_PLATFORM == Wasm
GeneralPage->>ManagePasswordDialog: create dialog
GeneralPage->>ManagePasswordDialog: setAccountName(accountName)
ManagePasswordDialog->>ManagePasswordDialog: user edits account name
User-->>ManagePasswordDialog: clicks OK
ManagePasswordDialog-->>GeneralPage: accepted()
GeneralPage->>GeneralPage: update config.account.accountName
GeneralPage->>PasswordConfig: setPassword(emptyString)
PasswordConfig->>WriteJob: m_writeJob.setKey(accountName)
PasswordConfig->>WriteJob: m_writeJob.setTextData(emptyString)
WriteJob->>WriteJob: start()
WriteJob-->>PasswordConfig: finished()
alt success
PasswordConfig-->>GeneralPage: sig_passwordSaved()
GeneralPage->>GeneralPage: setConfig().account.accountPassword = true
GeneralPage->>GeneralPage: updateAutoLoginEnabled()
else error or user denied
PasswordConfig-->>GeneralPage: sig_error(msg) or suppressed on AccessDeniedByUser
GeneralPage->>GeneralPage: log warning and show nonblocking QMessageBox
end
Class diagram for updated password management componentsclassDiagram
class GeneralPage {
+slot_loadConfig()
+slot_setPasswordClicked()
+slot_themeComboBoxChanged(index int)
+slot_displayMumeClockStateChanged(state int)
+slot_displayXPStatusStateChanged(state int)
+updateAutoLoginEnabled()
+sig_reloadConfig()
}
class PasswordConfig {
+PasswordConfig(parent QObject)
+~PasswordConfig()
+setPassword(password QString)
+getPassword()
+deletePassword()
+sig_error(msg QString)
+sig_incomingPassword(password QString)
+sig_passwordSaved()
+sig_passwordDeleted()
-m_readJob QKeychain::ReadPasswordJob
-m_writeJob QKeychain::WritePasswordJob
-m_deleteJob QKeychain::DeletePasswordJob
}
class ManagePasswordDialog {
+ManagePasswordDialog(parent QWidget)
+~ManagePasswordDialog()
+setAccountName(name QString)
+accountName() QString
+setPassword(password QString)
+password() QString
+sig_deleteRequested()
-ui Ui::ManagePasswordDialog*
}
class QKeychainReadPasswordJob {
+error() int
+errorString() QString
+textData() QString
+setKey(key QString)
+start()
}
class QKeychainWritePasswordJob {
+error() int
+errorString() QString
+setKey(key QString)
+setTextData(password QString)
+start()
}
class QKeychainDeletePasswordJob {
+error() int
+errorString() QString
+setKey(key QString)
+start()
}
GeneralPage --> PasswordConfig : uses
GeneralPage --> ManagePasswordDialog : creates
PasswordConfig --> QKeychainReadPasswordJob : wraps
PasswordConfig --> QKeychainWritePasswordJob : wraps
PasswordConfig --> QKeychainDeletePasswordJob : wraps
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
PasswordConfig::setPassword,getPassword, anddeletePassword, the#else(MMAPPER_NO_QTKEYCHAIN) branches referenceaccountName, which is not a parameter or member and will fail to compile; either remove thosestd::ignorelines or add an explicit parameter if you intend to support that value there. - When using the account name as the QtKeychain key on Wasm, consider what happens if
accountNameis empty or later changed (e.g. user renames the account): you may want to guard against empty keys and/or add a migration/cleanup path for passwords stored under the previous name.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `PasswordConfig::setPassword`, `getPassword`, and `deletePassword`, the `#else` (MMAPPER_NO_QTKEYCHAIN) branches reference `accountName`, which is not a parameter or member and will fail to compile; either remove those `std::ignore` lines or add an explicit parameter if you intend to support that value there.
- When using the account name as the QtKeychain key on Wasm, consider what happens if `accountName` is empty or later changed (e.g. user renames the account): you may want to guard against empty keys and/or add a migration/cleanup path for passwords stored under the previous name.
## Individual Comments
### Comment 1
<location path="src/configuration/PasswordConfig.cpp" line_range="77" />
<code_context>
m_writeJob.setTextData(password);
m_writeJob.start();
#else
+ std::ignore = accountName;
std::ignore = password;
emit sig_error("Password setting is not available.");
</code_context>
<issue_to_address>
**issue (bug_risk):** The non-QtKeychain branches reference `accountName`, which is not defined in this scope and will not compile.
In the `#else` branches of `setPassword`, `getPassword`, and `deletePassword`, `accountName` is assigned to `std::ignore` but is neither a parameter nor a local variable, so this will not compile when `MMAPPER_NO_QTKEYCHAIN` is defined. Either remove the `std::ignore = accountName;` lines if the functions are not meant to take `accountName`, or change the signatures/overloads so that a valid `accountName` is available here.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| m_writeJob.setTextData(password); | ||
| m_writeJob.start(); | ||
| #else | ||
| std::ignore = accountName; |
There was a problem hiding this comment.
issue (bug_risk): The non-QtKeychain branches reference accountName, which is not defined in this scope and will not compile.
In the #else branches of setPassword, getPassword, and deletePassword, accountName is assigned to std::ignore but is neither a parameter nor a local variable, so this will not compile when MMAPPER_NO_QTKEYCHAIN is defined. Either remove the std::ignore = accountName; lines if the functions are not meant to take accountName, or change the signatures/overloads so that a valid accountName is available here.
nschimme
force-pushed
the
wasm-qtkeychain
branch
from
April 14, 2026 17:38
fcbaa15 to
908b29a
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #515 +/- ##
==========================================
- Coverage 25.26% 25.21% -0.06%
==========================================
Files 513 514 +1
Lines 42372 42458 +86
Branches 4581 4585 +4
==========================================
Hits 10704 10704
- Misses 31668 31754 +86 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Enable QtKeychain-based password storage and account auto-login for the Wasm build and refactor account credential management into a dedicated dialog and configuration flow.
New Features:
Bug Fixes:
Enhancements:
Build: