SSH Agent: Show error messages if there are problems with agent communication #1614
Conversation
src/sshagent/SSHAgent.h
Outdated
| @@ -68,6 +70,8 @@ public slots: | |||
| #endif | |||
|
|
|||
| QMap<QString, QSet<OpenSSHKey>> m_keys; | |||
| QString m_error; | |||
| MessageWidget* m_messageWidget; | |||
src/sshagent/SSHAgent.cpp
Outdated
| { | ||
| m_instance = new SSHAgent(parent); | ||
| m_instance = new SSHAgent(messageWidget); |
There was a problem hiding this comment.
It don't like parenting the SSH agent to a single GUI widget. You should continue using the MainWindow or, even better, the application itself. Either pass in the MessageWidget as a second mandatory parameter or, even better, remove tight coupling altogether and only emit signals which you can connect to the MessageWidget from the outside.
|
Connected to global MessageWidget with signals now. Still keeping the in-class error string because the same instance is shared between EditEntryWidget and it should be able to feed the error strings to its own MessageWidget. |
src/sshagent/SSHAgent.h
Outdated
|
|
||
| const QString errorString() const; | ||
| bool isAgentRunning() const; | ||
| bool addIdentity(OpenSSHKey& key, quint32 lifetime = 0, bool confirm = false); | ||
| bool removeIdentity(OpenSSHKey& key); | ||
| void removeIdentityAtLock(const OpenSSHKey& key, const Uuid& uuid); | ||
|
|
||
| signals: | ||
| void showMessage(const QString& message, MessageWidget::MessageType); |
There was a problem hiding this comment.
Shouldn't this signal rather be called error or connectionFailed or something?
There was a problem hiding this comment.
Can't directly map it to MessageWidget slot if it doesn't include the second argument so I made it compatible. Can I somehow create a glue between argument incompatible signal/slot easily?
There was a problem hiding this comment.
I would simply use a separate signal for each type of event (if you have multiple) and then create a function that consumes this event and shows a message with the correct MessageType.
hifi
force-pushed
the
fix/agent-error-messages
branch
2 times, most recently
from
9e9bc5a to
a30f33a
Compare
hifi
force-pushed
the
fix/agent-error-messages
branch
from
a30f33a to
8d37f49
Compare
- Fix unnecessary automatic upgrade to KDBX 4.0 and prevent challenge-response key being stripped [#1568] - Abort saving and show an error message when challenge-response fails [#1659] - Support inner stream protection on all string attributes [#1646] - Fix favicon downloads not finishing on some websites [#1657] - Fix freeze due to invalid STDIN data [#1628] - Correct issue with encrypted RSA SSH keys [#1587] - Fix crash on macOS due to QTBUG-54832 [#1607] - Show error message if ssh-agent communication fails [#1614] - Fix --pw-stdin and filename parameters being ignored [#1608] - Fix Auto-Type syntax check not allowing spaces and special characters [#1626] - Fix reference placeholders in combination with Auto-Type [#1649] - Fix qtbase translations not being loaded [#1611] - Fix startup crash on Windows due to missing SVG libraries [#1662] - Correct database tab order regression [#1610] - Fix GCC 8 compilation error [#1612] - Fix copying of advanced attributes on KDE [#1640] - Fix member initialization of CategoryListWidgetDelegate [#1613] - Fix inconsistent toolbar icon sizes and provide higher-quality icons [#1616] - Improve preview panel geometry [#1609]
Implements in-entry messages when using manual add/removal and messages on lock/unlock.
Motivation and context
Error handling is currently silent and is a blocker for a user to figure out what's wrong. Considering this a UX bug.
How has this been tested?
Manually.
Types of changes
Checklist: