diff --git a/src/modules/users/Config.cpp b/src/modules/users/Config.cpp index b043691f8..90c301997 100644 --- a/src/modules/users/Config.cpp +++ b/src/modules/users/Config.cpp @@ -26,6 +26,7 @@ #include "utils/String.h" #include "utils/Variant.h" +#include #include #include @@ -379,6 +380,27 @@ Config::setRequireStrongPasswords( bool strong ) } } +bool +Config::isPasswordAcceptable(const QString& password, QString& message) +{ + bool failureIsFatal = requireStrongPasswords(); + + for ( auto pc : m_passwordChecks ) + { + QString s = pc.filter( password ); + + if ( !s.isEmpty() ) + { + message = s; + return !failureIsFatal; + } + } + + return true; +} + + + STATICTEST void setConfigurationDefaultGroups( const QVariantMap& map, QStringList& defaultGroups ) @@ -409,10 +431,59 @@ getHostNameActions( const QVariantMap& configurationMap ) } } - HostNameAction writeHosts = CalamaresUtils::getBool( configurationMap, "writeHostsFile", true ) ? HostNameAction::WriteEtcHosts : HostNameAction::None; + HostNameAction writeHosts = CalamaresUtils::getBool( configurationMap, "writeHostsFile", true ) + ? HostNameAction::WriteEtcHosts + : HostNameAction::None; return setHostName | writeHosts; } +/** @brief Process entries in the passwordRequirements config entry + * + * Called once for each item in the config entry, which should + * be a key-value pair. What makes sense as a value depends on + * the key. Supported keys are documented in users.conf. + * + * @return if the check was added, returns @c true + */ +STATICTEST bool +addPasswordCheck( const QString& key, const QVariant& value, PasswordCheckList& passwordChecks ) +{ + if ( key == "minLength" ) + { + add_check_minLength( passwordChecks, value ); + } + else if ( key == "maxLength" ) + { + add_check_maxLength( passwordChecks, value ); + } + else if ( key == "nonempty" ) + { + if ( value.toBool() ) + { + passwordChecks.push_back( + PasswordCheck( []() { return QCoreApplication::translate( "PWQ", "Password is empty" ); }, + []( const QString& s ) { return !s.isEmpty(); }, + PasswordCheck::Weight( 1 ) ) ); + } + else + { + cDebug() << "nonempty check is mentioned but set to false"; + return false; + } + } +#ifdef CHECK_PWQUALITY + else if ( key == "libpwquality" ) + { + add_check_libpwquality( passwordChecks, value ); + } +#endif // CHECK_PWQUALITY + else + { + cWarning() << "Unknown password-check key" << key; + return false; + } + return true; +} void Config::setConfigurationMap( const QVariantMap& configurationMap ) @@ -441,4 +512,12 @@ Config::setConfigurationMap( const QVariantMap& configurationMap ) m_permitWeakPasswords = CalamaresUtils::getBool( configurationMap, "allowWeakPasswords", false ); m_requireStrongPasswords = !m_permitWeakPasswords || !CalamaresUtils::getBool( configurationMap, "allowWeakPasswordsDefault", false ); + + // If the value doesn't exist, or isn't a map, this gives an empty map -- no problem + auto pr_checks( configurationMap.value( "passwordRequirements" ).toMap() ); + for ( decltype( pr_checks )::const_iterator i = pr_checks.constBegin(); i != pr_checks.constEnd(); ++i ) + { + addPasswordCheck( i.key(), i.value(), m_passwordChecks ); + } + std::sort( m_passwordChecks.begin(), m_passwordChecks.end() ); } diff --git a/src/modules/users/Config.h b/src/modules/users/Config.h index e61564897..c89ae9bbf 100644 --- a/src/modules/users/Config.h +++ b/src/modules/users/Config.h @@ -21,6 +21,8 @@ #ifndef USERS_CONFIG_H #define USERS_CONFIG_H +#include "CheckPWQuality.h" + #include "utils/NamedEnum.h" #include @@ -110,6 +112,17 @@ public: const QStringList& defaultGroups() const { return m_defaultGroups; } + /** @brief Checks if the password is acceptable. + * + * If all is well, sets @p message to empty and returns @c true. + * If there are warnings, but acceptable, sets @p message to something + * non-empty and returns @c true. This happens if requireStrongPasswords + * is turned off (by config or user). + * If the password is not acceptable, sets @p message to something + * non-empty and returns @c false. + */ + bool isPasswordAcceptable( const QString& password, QString& message ); + static const QStringList& forbiddenLoginNames(); static const QStringList& forbiddenHostNames(); @@ -178,6 +191,7 @@ private: bool m_customHostName = false; HostNameActions m_hostNameActions; + PasswordCheckList m_passwordChecks; }; #endif diff --git a/src/modules/users/UsersPage.cpp b/src/modules/users/UsersPage.cpp index 9509592ba..aca20fd58 100644 --- a/src/modules/users/UsersPage.cpp +++ b/src/modules/users/UsersPage.cpp @@ -283,38 +283,21 @@ UsersPage::checkPasswordAcceptance( const QString& pw1, const QString& pw2, QLab } else { - bool failureIsFatal = ui->checkBoxValidatePassword->isChecked(); - bool failureFound = false; - - if ( m_passwordChecksChanged ) + QString s; + bool ok = m_config->isPasswordAcceptable( pw1, s ); + if ( !ok ) { - std::sort( m_passwordChecks.begin(), m_passwordChecks.end() ); - m_passwordChecksChanged = false; + labelError( badge, message, s, Badness::Fatal ); } - - for ( auto pc : m_passwordChecks ) + else if ( !s.isEmpty() ) { - QString s = pc.filter( pw1 ); - - if ( !s.isEmpty() ) - { - labelError( badge, message, s, failureIsFatal ? Badness::Fatal : Badness::Warning ); - failureFound = true; - if ( failureIsFatal ) - { - return false; - } - } + labelError( badge, message, s, Badness::Warning ); } - - if ( !failureFound ) + else { labelOk( badge, message ); } - - // Here, if failureFound is true then we've found **warnings**, - // which is ok to continue but the user should know. - return true; + return ok; } } @@ -360,38 +343,3 @@ UsersPage::onReuseUserPasswordChanged( const int checked ) ui->textBoxVerifiedRootPassword->setVisible( visible ); checkReady( isReady() ); } - -void -UsersPage::addPasswordCheck( const QString& key, const QVariant& value ) -{ - m_passwordChecksChanged = true; - - if ( key == "minLength" ) - { - add_check_minLength( m_passwordChecks, value ); - } - else if ( key == "maxLength" ) - { - add_check_maxLength( m_passwordChecks, value ); - } - else if ( key == "nonempty" ) - { - if ( value.toBool() ) - { - m_passwordChecks.push_back( - PasswordCheck( []() { return QCoreApplication::translate( "PWQ", "Password is empty" ); }, - []( const QString& s ) { return !s.isEmpty(); }, - PasswordCheck::Weight( 1 ) ) ); - } - } -#ifdef CHECK_PWQUALITY - else if ( key == "libpwquality" ) - { - add_check_libpwquality( m_passwordChecks, value ); - } -#endif // CHECK_PWQUALITY - else - { - cWarning() << "Unknown password-check key" << key; - } -} diff --git a/src/modules/users/UsersPage.h b/src/modules/users/UsersPage.h index 5e0909b54..2f446b563 100644 --- a/src/modules/users/UsersPage.h +++ b/src/modules/users/UsersPage.h @@ -24,8 +24,6 @@ #ifndef USERSPAGE_H #define USERSPAGE_H -#include "CheckPWQuality.h" - #include class Config; @@ -50,14 +48,6 @@ public: void onActivate(); - /** @brief Process entries in the passwordRequirements config entry - * - * Called once for each item in the config entry, which should - * be a key-value pair. What makes sense as a value depends on - * the key. Supported keys are documented in users.conf. - */ - void addPasswordCheck( const QString& key, const QVariant& value ); - ///@brief Root password, depends on settings, may be empty QString getRootPassword() const; ///@brief User name and password @@ -88,9 +78,6 @@ private: Ui::Page_UserSetup* ui; Config* m_config; - PasswordCheckList m_passwordChecks; - bool m_passwordChecksChanged = false; - bool m_readyFullName; bool m_readyUsername; bool m_readyHostname; diff --git a/src/modules/users/UsersViewStep.cpp b/src/modules/users/UsersViewStep.cpp index 300bdf072..75c76bd21 100644 --- a/src/modules/users/UsersViewStep.cpp +++ b/src/modules/users/UsersViewStep.cpp @@ -151,19 +151,4 @@ void UsersViewStep::setConfigurationMap( const QVariantMap& configurationMap ) { m_config->setConfigurationMap( configurationMap ); - - // Create the widget, after all .. as long as writing configuration to the UI is needed - (void)this->widget(); - using CalamaresUtils::getBool; - - if ( configurationMap.contains( "passwordRequirements" ) - && configurationMap.value( "passwordRequirements" ).type() == QVariant::Map ) - { - auto pr_checks( configurationMap.value( "passwordRequirements" ).toMap() ); - - for ( decltype( pr_checks )::const_iterator i = pr_checks.constBegin(); i != pr_checks.constEnd(); ++i ) - { - m_widget->addPasswordCheck( i.key(), i.value() ); - } - } }