fix: show an appropriate error when a provided password fails its challenge

With this change the user now gets appropriate error feedback when trying to unlock their accounts using an incorrect password.

Additionally password entry is temporarily suspended in the UI while key derivation (etc.) is in progress.
This should help convey the idea that "something is happening", or rather avoid the impression that "nothing happens" or "this is broken" when in fact key derivation may simply be slow depending on the machine.
master
Johan Ouwerkerk 2020-11-22 16:24:47 +01:00 committed by Bhushan Shah
parent cbd069085e
commit ad4308793b
5 changed files with 41 additions and 14 deletions

View File

@ -39,12 +39,14 @@ Kirigami.ScrollablePage {
Kirigami.PasswordField {
id: newPassword
text: ""
enabled: !passwordRequest.passwordProvided
Kirigami.FormData.label: i18nc("@label:textbox", "New password:")
onAccepted: newPasswordCopy.forceActiveFocus()
}
Kirigami.PasswordField {
id: newPasswordCopy
text: ""
enabled: !passwordRequest.passwordProvided
Kirigami.FormData.label: i18nc("@label:textbox", "Verify password:")
onAccepted: applyAction.trigger()
}
@ -54,13 +56,11 @@ Kirigami.ScrollablePage {
id: applyAction
text: i18n("Apply")
iconName: "answer-correct"
enabled: newPassword.text === newPasswordCopy.text && newPassword.text && newPassword.text.length > 0
enabled: !passwordRequest.passwordProvided && newPassword.text === newPasswordCopy.text && newPassword.text && newPassword.text.length > 0
onTriggered: {
// TODO convert to C++ helper, have proper logging?
if (passwordRequest) {
if (!passwordRequest.provideBothPasswords(newPassword.text, newPasswordCopy.text)) {
bannerTextError = true;
}
bannerTextError = !passwordRequest.provideBothPasswords(newPassword.text, newPasswordCopy.text);
}
// TODO warn if not
}
@ -76,4 +76,11 @@ Kirigami.ScrollablePage {
showCloseButton: true
}
}
Connections {
target: passwordRequest
onPasswordRejected: {
bannerTextError = true
}
}
}

View File

@ -51,6 +51,7 @@ Kirigami.ScrollablePage {
id: existingPassword
text: ""
Kirigami.FormData.label: i18nc("@label:textbox", "Password:")
enabled: !passwordRequest.passwordProvided
onAccepted: {
if (unlockAction.enabled) {
unlockAction.trigger()
@ -63,15 +64,20 @@ Kirigami.ScrollablePage {
id: unlockAction
text: i18n("Unlock")
iconName: "unlock"
enabled: existingPassword.text && existingPassword.text.length > 0
enabled: !passwordRequest.passwordProvided && existingPassword.text && existingPassword.text.length > 0
onTriggered: {
// TODO convert to C++ helper, have proper logging?
if (passwordRequest) {
if (!passwordRequest.providePassword(existingPassword.text)) {
bannerTextError = true;
}
bannerTextError = !passwordRequest.providePassword(existingPassword.text);
}
// TODO warn if not
}
}
Connections {
target: passwordRequest
onPasswordRejected: {
bannerTextError = true
}
}
}

View File

@ -170,7 +170,7 @@ Kirigami.ApplicationWindow {
*/
Connections {
target: passwordRequest
onDerivedKey : {
onPasswordAccepted : {
// TODO convert to C++ helper, have proper logging?
if (!passwordRequest.keyAvailable) {
return; // TODO warn if not

View File

@ -18,6 +18,7 @@ namespace model
QObject::connect(m_secret, &accounts::AccountSecret::newPasswordNeeded, this, &PasswordRequest::setNewPasswordNeeded);
QObject::connect(m_secret, &accounts::AccountSecret::passwordAvailable, this, &PasswordRequest::setPasswordAvailable);
QObject::connect(m_secret, &accounts::AccountSecret::keyAvailable, this, &PasswordRequest::setKeyAvailable);
QObject::connect(m_secret, &accounts::AccountSecret::keyFailed, this, &PasswordRequest::setPasswordRejected);
m_previous = secret->isExistingPasswordRequested();
m_firstRun = secret->isNewPasswordRequested();
m_haveKey = secret->isKeyAvailable();
@ -96,7 +97,7 @@ namespace model
{
if (!m_haveKey) {
m_haveKey = true;
Q_EMIT derivedKey();
Q_EMIT passwordAccepted();
} else {
qCDebug(logger) << "Ignored signal: already marked key as available";
}
@ -106,12 +107,23 @@ namespace model
{
if (!m_havePassword) {
m_havePassword = true;
Q_EMIT passwordAccepted();
Q_EMIT passwordStateChanged();
} else {
qCDebug(logger) << "Ignored signal: already marked password as available";
}
}
void PasswordRequest::setPasswordRejected(void)
{
if (m_havePassword) {
m_havePassword = false;
Q_EMIT passwordStateChanged();
Q_EMIT passwordRejected();
} else {
qCDebug(logger) << "Ignored signal: already marked password as rejected";
}
}
void PasswordRequest::setPreviouslyDefined(void)
{
if (!m_previous) {

View File

@ -14,8 +14,8 @@ namespace model
Q_OBJECT
Q_PROPERTY(bool firstRun READ firstRun NOTIFY passwordRequestChanged)
Q_PROPERTY(bool previouslyDefined READ previouslyDefined NOTIFY passwordRequestChanged)
Q_PROPERTY(bool keyAvailable READ keyAvailable NOTIFY derivedKey)
Q_PROPERTY(bool passwordProvided READ passwordProvided NOTIFY passwordAccepted)
Q_PROPERTY(bool keyAvailable READ keyAvailable NOTIFY passwordAccepted)
Q_PROPERTY(bool passwordProvided READ passwordProvided NOTIFY passwordStateChanged)
public:
explicit PasswordRequest(accounts::AccountSecret *secret, QObject *parent = nullptr);
bool firstRun(void) const;
@ -30,10 +30,12 @@ namespace model
void passwordExists(void);
void newPasswordNeeded(void);
void passwordAccepted(void);
void derivedKey(void);
void passwordRejected(void);
void passwordStateChanged(void);
private Q_SLOTS:
void setKeyAvailable(void);
void setPasswordAvailable(void);
void setPasswordRejected(void);
void setPreviouslyDefined(void);
void setNewPasswordNeeded(void);
private: