From 1dcf56761f69022a9eebb232317e2420bb0267ce Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 17 Aug 2020 11:41:04 +0200 Subject: [PATCH] [users] Apply validation to the passwords config knows about - avoid update loops by checking values before emitting *Changed() - check validity of user and root passwords when asked - if root isn't going to be written, or re-uses the user password, defer to those status checks. --- src/modules/users/Config.cpp | 91 +++++++++++++++++++++++++++++---- src/modules/users/Config.h | 37 +++++++++++++- src/modules/users/UsersPage.cpp | 13 +++-- src/modules/users/UsersPage.h | 4 +- 4 files changed, 125 insertions(+), 20 deletions(-) diff --git a/src/modules/users/Config.cpp b/src/modules/users/Config.cpp index 69712a733..0a6d399bf 100644 --- a/src/modules/users/Config.cpp +++ b/src/modules/users/Config.cpp @@ -381,7 +381,7 @@ Config::setRequireStrongPasswords( bool strong ) } bool -Config::isPasswordAcceptable( const QString& password, QString& message ) +Config::isPasswordAcceptable( const QString& password, QString& message ) const { bool failureIsFatal = requireStrongPasswords(); @@ -402,23 +402,58 @@ Config::isPasswordAcceptable( const QString& password, QString& message ) void Config::setUserPassword( const QString& s ) { - m_userPassword = s; - // TODO: check new password status - emit userPasswordChanged( s ); + if ( s != m_userPassword ) + { + m_userPassword = s; + // TODO: check new password status + emit userPasswordChanged( s ); + } } void Config::setUserPasswordSecondary( const QString& s ) { - m_userPasswordSecondary = s; - // TODO: check new password status - emit userPasswordSecondaryChanged( s ); + if ( s != m_userPasswordSecondary ) + { + m_userPasswordSecondary = s; + // TODO: check new password status + emit userPasswordSecondaryChanged( s ); + } } +QPair< Config::PasswordValidity, QString > +Config::passwordStatus( const QString& pw1, const QString& pw2 ) const +{ + if ( pw1 != pw2 ) + { + return qMakePair( PasswordValidity::Invalid, tr( "Your passwords do not match!" ) ); + } + + QString message; + bool ok = isPasswordAcceptable( pw1, message ); + return qMakePair( ok ? PasswordValidity::Valid : PasswordValidity::Weak, message ); +} + + +int +Config::userPasswordValidity() const +{ + auto p = passwordStatus( m_userPassword, m_userPasswordSecondary ); + return p.first; +} + +QString +Config::userPasswordStatus() const +{ + auto p = passwordStatus( m_userPassword, m_userPasswordSecondary ); + return p.second; +} + + void Config::setRootPassword( const QString& s ) { - if ( writeRootPassword() ) + if ( writeRootPassword() && s != m_rootPassword ) { m_rootPassword = s; // TODO: check new password status @@ -429,7 +464,7 @@ Config::setRootPassword( const QString& s ) void Config::setRootPasswordSecondary( const QString& s ) { - if ( writeRootPassword() ) + if ( writeRootPassword() && s != m_rootPasswordSecondary ) { m_rootPasswordSecondary = s; // TODO: check new password status @@ -437,28 +472,62 @@ Config::setRootPasswordSecondary( const QString& s ) } } -QString Config::rootPassword() const +QString +Config::rootPassword() const { if ( writeRootPassword() ) { if ( reuseUserPasswordForRoot() ) + { return userPassword(); + } return m_rootPassword; } return QString(); } -QString Config::rootPasswordSecondary() const +QString +Config::rootPasswordSecondary() const { if ( writeRootPassword() ) { if ( reuseUserPasswordForRoot() ) + { return userPasswordSecondary(); + } return m_rootPasswordSecondary; } return QString(); } +int +Config::rootPasswordValidity() const +{ + if ( writeRootPassword() && !reuseUserPasswordForRoot() ) + { + auto p = passwordStatus( m_rootPassword, m_rootPasswordSecondary ); + return p.first; + } + else + { + return userPasswordValidity(); + } +} + +QString +Config::rootPasswordStatus() const +{ + if ( writeRootPassword() && !reuseUserPasswordForRoot() ) + { + auto p = passwordStatus( m_rootPassword, m_rootPasswordSecondary ); + return p.second; + } + else + { + return userPasswordStatus(); + } +} + STATICTEST void setConfigurationDefaultGroups( const QVariantMap& map, QStringList& defaultGroups ) diff --git a/src/modules/users/Config.h b/src/modules/users/Config.h index 334bfcdb3..97178f2ea 100644 --- a/src/modules/users/Config.h +++ b/src/modules/users/Config.h @@ -62,9 +62,14 @@ class Config : public QObject Q_PROPERTY( QString userPassword READ userPassword WRITE setUserPassword NOTIFY userPasswordChanged ) Q_PROPERTY( QString userPasswordSecondary READ userPasswordSecondary WRITE setUserPasswordSecondary NOTIFY userPasswordSecondaryChanged ) + Q_PROPERTY( int userPasswordValidity READ userPasswordValidity NOTIFY userPasswordStatusChanged STORED false ) + Q_PROPERTY( QString userPasswordStatus READ userPasswordStatus NOTIFY userPasswordStatusChanged STORED false ) + Q_PROPERTY( QString rootPassword READ rootPassword WRITE setRootPassword NOTIFY rootPasswordChanged ) Q_PROPERTY( QString rootPasswordSecondary READ rootPasswordSecondary WRITE setRootPasswordSecondary NOTIFY rootPasswordSecondaryChanged ) + Q_PROPERTY( int rootPasswordValidity READ rootPasswordValidity NOTIFY rootPasswordStatusChanged STORED false ) + Q_PROPERTY( QString rootPasswordStatus READ rootPasswordStatus NOTIFY rootPasswordStatusChanged STORED false ) Q_PROPERTY( bool writeRootPassword READ writeRootPassword CONSTANT ) Q_PROPERTY( bool reuseUserPasswordForRoot READ reuseUserPasswordForRoot WRITE setReuseUserPasswordForRoot NOTIFY @@ -75,6 +80,27 @@ class Config : public QObject requireStrongPasswordsChanged ) public: + /** @brief Validity (status) of a password + * + * Valid passwords are: + * - primary and secondary are equal **and** + * - all the password-strength checks pass + * Weak passwords: + * - primary and secondary are equal **and** + * - not all the checks pass **and** + * - permitWeakPasswords is @c true **and** + * - requireStrongPasswords is @c false + * Invalid passwords (all other cases): + * - the primary and secondary values are not equal **or** + * - not all the checks pass and weak passwords are not permitted + */ + enum PasswordValidity + { + Valid = 0, + Weak = 1, + Invalid = 2 + }; + Config( QObject* parent = nullptr ); ~Config(); @@ -128,16 +154,21 @@ public: * If the password is not acceptable, sets @p message to something * non-empty and returns @c false. */ - bool isPasswordAcceptable( const QString& password, QString& message ); + bool isPasswordAcceptable( const QString& password, QString& message ) const; // The user enters a password (and again in a separate UI element) QString userPassword() const { return m_userPassword; } QString userPasswordSecondary() const { return m_userPasswordSecondary; } + int userPasswordValidity() const; + QString userPasswordStatus() const; + // The root password **may** be entered in the UI, or may be suppressed // entirely when writeRootPassword is off, or may be equal to // the user password when reuseUserPasswordForRoot is on. QString rootPassword() const; QString rootPasswordSecondary() const; + int rootPasswordValidity() const; + QString rootPasswordStatus() const; static const QStringList& forbiddenLoginNames(); static const QStringList& forbiddenHostNames(); @@ -193,11 +224,15 @@ signals: void requireStrongPasswordsChanged( bool ); void userPasswordChanged( const QString& ); void userPasswordSecondaryChanged( const QString& ); + void userPasswordStatusChanged( int, QString& ); void rootPasswordChanged( const QString& ); void rootPasswordSecondaryChanged( const QString& ); + void rootPasswordStatusChanged( int, QString& ); private: + QPair< PasswordValidity, QString > passwordStatus( const QString&, const QString& ) const; + QStringList m_defaultGroups; QString m_userShell; QString m_autologinGroup; diff --git a/src/modules/users/UsersPage.cpp b/src/modules/users/UsersPage.cpp index 16a905d08..c8f561f78 100644 --- a/src/modules/users/UsersPage.cpp +++ b/src/modules/users/UsersPage.cpp @@ -248,8 +248,11 @@ UsersPage::reportHostNameStatus( const QString& status ) } bool -UsersPage::checkPasswordAcceptance( const QString& pw1, const QString& pw2, QLabel* badge, QLabel* message ) +UsersPage::checkPasswordAcceptance( Password p, QLabel* badge, QLabel* message ) { + QString pw1 = p == Password::User ? m_config->userPassword() : m_config->rootPassword(); + QString pw2 = p == Password::User ? m_config->userPasswordSecondary() : m_config->rootPasswordSecondary(); + if ( pw1 != pw2 ) { labelError( badge, message, tr( "Your passwords do not match!" ), Badness::Fatal ); @@ -278,9 +281,7 @@ UsersPage::checkPasswordAcceptance( const QString& pw1, const QString& pw2, QLab void UsersPage::onPasswordTextChanged( const QString& ) { - m_readyPassword = checkPasswordAcceptance( ui->textBoxUserPassword->text(), - ui->textBoxUserVerifiedPassword->text(), - ui->labelUserPassword, + m_readyPassword = checkPasswordAcceptance( Password::User, ui->labelUserPassword, ui->labelUserPasswordError ); emit checkReady( isReady() ); @@ -289,9 +290,7 @@ UsersPage::onPasswordTextChanged( const QString& ) void UsersPage::onRootPasswordTextChanged( const QString& ) { - m_readyRootPassword = checkPasswordAcceptance( ui->textBoxRootPassword->text(), - ui->textBoxVerifiedRootPassword->text(), - ui->labelRootPassword, + m_readyRootPassword = checkPasswordAcceptance( Password::Root, ui->labelRootPassword, ui->labelRootPasswordError ); emit checkReady( isReady() ); } diff --git a/src/modules/users/UsersPage.h b/src/modules/users/UsersPage.h index 46c4ed399..9046d1198 100644 --- a/src/modules/users/UsersPage.h +++ b/src/modules/users/UsersPage.h @@ -60,13 +60,15 @@ signals: void checkReady( bool ); private: + /// @brief Which password are we talking about? (for checkPasswordAcceptance()) + enum class Password { Root, User }; /** @brief Is the password acceptable? * * Checks the two copies of the password and places error messages in the * given QLabels. Returns true (and clears the error messages) if the * password is acceptable. */ - bool checkPasswordAcceptance( const QString& pw1, const QString& pw2, QLabel* badge, QLabel* message ); + bool checkPasswordAcceptance( Password p, QLabel* badge, QLabel* message ); void retranslate();