From 44bc61d4be71a3d9bd965378d0a22d9725d75420 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 4 Aug 2020 22:35:04 +0200 Subject: [PATCH 01/11] [users] set up Config object before widget --- src/modules/users/UsersViewStep.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/modules/users/UsersViewStep.cpp b/src/modules/users/UsersViewStep.cpp index b9ea3ae17..39d06b64a 100644 --- a/src/modules/users/UsersViewStep.cpp +++ b/src/modules/users/UsersViewStep.cpp @@ -168,6 +168,8 @@ UsersViewStep::onLeave() 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; @@ -204,6 +206,4 @@ UsersViewStep::setConfigurationMap( const QVariantMap& configurationMap ) Action hostsfileAction = getBool( configurationMap, "writeHostsFile", true ) ? Action::WriteEtcHosts : Action::None; m_actions = hostsfileAction | hostnameAction; - - m_config->setConfigurationMap( configurationMap ); } From 35dff4d12c0aa39324c15761541b74e129004b4d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 4 Aug 2020 22:16:48 +0200 Subject: [PATCH 02/11] [users] Migrate reuse-password and password-strength to Config - add the "reuse user password for root" setting to Config, make the UI page follow that setting. - add the require-strong-password default and toggle settings to Config; this is not well-checked yet. On the widget / UI side, connect checkboxes only if they are visible; refactor reuse-user-password-for-root settings. --- src/modules/users/Config.cpp | 27 ++++++++++ src/modules/users/Config.h | 26 ++++++++++ src/modules/users/UsersPage.cpp | 77 +++++++++++++++-------------- src/modules/users/UsersPage.h | 5 +- src/modules/users/UsersViewStep.cpp | 5 -- 5 files changed, 95 insertions(+), 45 deletions(-) diff --git a/src/modules/users/Config.cpp b/src/modules/users/Config.cpp index e0174e74e..823b9fb99 100644 --- a/src/modules/users/Config.cpp +++ b/src/modules/users/Config.cpp @@ -343,6 +343,27 @@ Config::setAutoLogin( bool b ) } } +void +Config::setReuseUserPasswordForRoot( bool reuse ) +{ + if ( reuse != m_reuseUserPasswordForRoot ) + { + m_reuseUserPasswordForRoot = reuse; + emit reuseUserPasswordForRootChanged( reuse ); + } +} + +void +Config::setRequireStrongPasswords( bool strong ) +{ + if ( strong != m_requireStrongPasswords ) + { + m_requireStrongPasswords = strong; + emit requireStrongPasswordsChanged( strong ); + } +} + + STATICTEST void setConfigurationDefaultGroups( const QVariantMap& map, QStringList& defaultGroups ) { @@ -377,4 +398,10 @@ Config::setConfigurationMap( const QVariantMap& configurationMap ) m_writeRootPassword = CalamaresUtils::getBool( configurationMap, "setRootPassword", true ); Calamares::JobQueue::instance()->globalStorage()->insert( "setRootPassword", m_writeRootPassword ); + + m_reuseUserPasswordForRoot = CalamaresUtils::getBool( configurationMap, "doReusePassword", false ); + + m_permitWeakPasswords = CalamaresUtils::getBool( configurationMap, "allowWeakPasswords", false ); + m_requireStrongPasswords + = !m_permitWeakPasswords || !CalamaresUtils::getBool( configurationMap, "allowWeakPasswordsDefault", false ); } diff --git a/src/modules/users/Config.h b/src/modules/users/Config.h index b84dc5aaf..8c30dad4c 100644 --- a/src/modules/users/Config.h +++ b/src/modules/users/Config.h @@ -42,6 +42,14 @@ class Config : public QObject Q_PROPERTY( QString hostName READ hostName WRITE setHostName NOTIFY hostNameChanged ) Q_PROPERTY( QString hostNameStatus READ hostNameStatus NOTIFY hostNameStatusChanged ) + Q_PROPERTY( bool writeRootPassword READ writeRootPassword CONSTANT ) + Q_PROPERTY( bool reuseUserPasswordForRoot READ reuseUserPasswordForRoot WRITE setReuseUserPasswordForRoot NOTIFY + reuseUserPasswordForRootChanged ) + + Q_PROPERTY( bool permitWeakPasswords READ permitWeakPasswords CONSTANT ) + Q_PROPERTY( bool requireStrongPasswords READ requireStrongPasswords WRITE setRequireStrongPasswords NOTIFY + requireStrongPasswordsChanged ) + public: Config( QObject* parent = nullptr ); ~Config(); @@ -76,6 +84,12 @@ public: bool doAutoLogin() const { return m_doAutoLogin; } /// Should the root password be written (if false, no password is set and the root account is disabled for login) bool writeRootPassword() const { return m_writeRootPassword; } + /// Should the user's password be used for root, too? (if root is written at all) + bool reuseUserPasswordForRoot() const { return m_reuseUserPasswordForRoot; } + /// Show UI to change the "require strong password" setting? + bool permitWeakPasswords() const { return m_permitWeakPasswords; } + /// Current setting for "require strong password"? + bool requireStrongPasswords() const { return m_requireStrongPasswords; } const QStringList& defaultGroups() const { return m_defaultGroups; } @@ -109,6 +123,11 @@ public Q_SLOTS: /// Sets the autologin flag void setAutoLogin( bool b ); + /// Set to true to use the user password, unchanged, for root too + void setReuseUserPasswordForRoot( bool reuse ); + /// Change setting for "require strong password" + void setRequireStrongPasswords( bool strong ); + signals: void userShellChanged( const QString& ); void autologinGroupChanged( const QString& ); @@ -119,6 +138,8 @@ signals: void hostNameChanged( const QString& ); void hostNameStatusChanged( const QString& ); void autoLoginChanged( bool ); + void reuseUserPasswordForRootChanged( bool ); + void requireStrongPasswordsChanged( bool ); private: QStringList m_defaultGroups; @@ -129,7 +150,12 @@ private: QString m_loginName; QString m_hostName; bool m_doAutoLogin = false; + bool m_writeRootPassword = true; + bool m_reuseUserPasswordForRoot = false; + + bool m_permitWeakPasswords = false; + bool m_requireStrongPasswords = true; bool m_customLoginName = false; bool m_customHostName = false; diff --git a/src/modules/users/UsersPage.cpp b/src/modules/users/UsersPage.cpp index c4502256a..9054d60e7 100644 --- a/src/modules/users/UsersPage.cpp +++ b/src/modules/users/UsersPage.cpp @@ -105,6 +105,12 @@ UsersPage::UsersPage( Config* config, QWidget* parent ) { ui->setupUi( this ); + ui->checkBoxReusePassword->setVisible( m_config->writeRootPassword() ); + ui->checkBoxReusePassword->setChecked( m_config->reuseUserPasswordForRoot() ); + + ui->checkBoxValidatePassword->setVisible( m_config->permitWeakPasswords() ); + ui->checkBoxValidatePassword->setChecked( m_config->requireStrongPasswords() ); + // Connect signals and slots connect( ui->textBoxUserPassword, &QLineEdit::textChanged, this, &UsersPage::onPasswordTextChanged ); connect( ui->textBoxUserVerifiedPassword, &QLineEdit::textChanged, this, &UsersPage::onPasswordTextChanged ); @@ -115,21 +121,7 @@ UsersPage::UsersPage( Config* config, QWidget* parent ) onRootPasswordTextChanged( ui->textBoxRootPassword->text() ); checkReady( isReady() ); } ); - connect( ui->checkBoxReusePassword, &QCheckBox::stateChanged, this, [this]( const int checked ) { - /* When "reuse" is checked, hide the fields for explicitly - * entering the root password. However, if we're going to - * disable the root password anyway, hide them all regardless of - * the checkbox -- so when writeRoot is false, checked needs - * to be true, to hide them all. - */ - const bool visible = m_config->writeRootPassword() ? !checked : false; - ui->labelChooseRootPassword->setVisible( visible ); - ui->labelRootPassword->setVisible( visible ); - ui->labelRootPasswordError->setVisible( visible ); - ui->textBoxRootPassword->setVisible( visible ); - ui->textBoxVerifiedRootPassword->setVisible( visible ); - checkReady( isReady() ); - } ); + connect( ui->checkBoxReusePassword, &QCheckBox::stateChanged, this, &UsersPage::onReuseUserPasswordChanged ); connect( ui->textBoxFullName, &QLineEdit::textEdited, config, &Config::setFullName ); connect( config, &Config::fullNameChanged, this, &UsersPage::onFullNameTextEdited ); @@ -147,13 +139,25 @@ UsersPage::UsersPage( Config* config, QWidget* parent ) } ); connect( config, &Config::autoLoginChanged, ui->checkBoxDoAutoLogin, &QCheckBox::setChecked ); - ui->checkBoxReusePassword->setVisible( m_config->writeRootPassword() ); - ui->checkBoxReusePassword->setChecked( true ); - ui->checkBoxValidatePassword->setChecked( true ); + if ( m_config->writeRootPassword() ) + { + connect( ui->checkBoxReusePassword, &QCheckBox::stateChanged, this, [this]( int checked ) { + m_config->setReuseUserPasswordForRoot( checked != Qt::Unchecked ); + } ); + connect( config, &Config::reuseUserPasswordForRootChanged, ui->checkBoxReusePassword, &QCheckBox::setChecked ); + } - setPasswordCheckboxVisible( false ); + if ( m_config->permitWeakPasswords() ) + { + connect( ui->checkBoxValidatePassword, &QCheckBox::stateChanged, this, [this]( int checked ) { + m_config->setRequireStrongPasswords( checked != Qt::Unchecked ); + } ); + connect( config, &Config::requireStrongPasswordsChanged, ui->checkBoxValidatePassword, &QCheckBox::setChecked ); + } CALAMARES_RETRANSLATE_SLOT( &UsersPage::retranslate ); + + onReuseUserPasswordChanged( m_config->reuseUserPasswordForRoot() ); } UsersPage::~UsersPage() @@ -335,25 +339,26 @@ UsersPage::onRootPasswordTextChanged( const QString& ) emit checkReady( isReady() ); } - -void -UsersPage::setPasswordCheckboxVisible( bool visible ) -{ - ui->checkBoxValidatePassword->setVisible( visible ); -} - -void -UsersPage::setValidatePasswordDefault( bool checked ) -{ - ui->checkBoxValidatePassword->setChecked( checked ); - emit checkReady( isReady() ); -} - void -UsersPage::setReusePasswordDefault( bool checked ) +UsersPage::onReuseUserPasswordChanged( const int checked ) { - ui->checkBoxReusePassword->setChecked( checked ); - emit checkReady( isReady() ); + /* When "reuse" is checked, hide the fields for explicitly + * entering the root password. However, if we're going to + * disable the root password anyway, hide them all regardless of + * the checkbox -- so when writeRoot is false, visible needs + * to be false, to hide them all. + * + * In principle this is only connected when writeRootPassword is @c true, + * but it is **always** called at least once in the constructor + * to set up initial visibility. + */ + const bool visible = m_config->writeRootPassword() ? !checked : false; + ui->labelChooseRootPassword->setVisible( visible ); + ui->labelRootPassword->setVisible( visible ); + ui->labelRootPasswordError->setVisible( visible ); + ui->textBoxRootPassword->setVisible( visible ); + ui->textBoxVerifiedRootPassword->setVisible( visible ); + checkReady( isReady() ); } void diff --git a/src/modules/users/UsersPage.h b/src/modules/users/UsersPage.h index b8cb0f06a..5e0909b54 100644 --- a/src/modules/users/UsersPage.h +++ b/src/modules/users/UsersPage.h @@ -50,10 +50,6 @@ public: void onActivate(); - void setPasswordCheckboxVisible( bool visible ); - void setValidatePasswordDefault( bool checked ); - void setReusePasswordDefault( bool checked ); - /** @brief Process entries in the passwordRequirements config entry * * Called once for each item in the config entry, which should @@ -73,6 +69,7 @@ protected slots: void reportHostNameStatus( const QString& ); void onPasswordTextChanged( const QString& ); void onRootPasswordTextChanged( const QString& ); + void onReuseUserPasswordChanged( const int ); signals: void checkReady( bool ); diff --git a/src/modules/users/UsersViewStep.cpp b/src/modules/users/UsersViewStep.cpp index 39d06b64a..092826d24 100644 --- a/src/modules/users/UsersViewStep.cpp +++ b/src/modules/users/UsersViewStep.cpp @@ -174,8 +174,6 @@ UsersViewStep::setConfigurationMap( const QVariantMap& configurationMap ) (void)this->widget(); using CalamaresUtils::getBool; - m_widget->setReusePasswordDefault( getBool( configurationMap, "doReusePassword", false ) ); - if ( configurationMap.contains( "passwordRequirements" ) && configurationMap.value( "passwordRequirements" ).type() == QVariant::Map ) { @@ -187,9 +185,6 @@ UsersViewStep::setConfigurationMap( const QVariantMap& configurationMap ) } } - m_widget->setPasswordCheckboxVisible( getBool( configurationMap, "allowWeakPasswords", false ) ); - m_widget->setValidatePasswordDefault( !getBool( configurationMap, "allowWeakPasswordsDefault", false ) ); - using Action = SetHostNameJob::Action; QString hostnameActionString = CalamaresUtils::getString( configurationMap, "setHostname" ); From 32e3933355e501a75bdc8229503b98f4ecdec2c4 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 5 Aug 2020 00:51:08 +0200 Subject: [PATCH 03/11] CMake: stop clobbering config files When CMake runs, configure_file() will clobber the config files in the build/ directory, which is annoying during testing: you need to keep making the same edits, or edit the source. - Introduce new behavior: the config file is **not** overwritten unless the source file is newer. This means that edits to config files in the build directory are preserved. - If INSTALL_CONFIG is **on** then the files are clobbered anyway (the source is considered new regardless). --- CMakeModules/CalamaresAddPlugin.cmake | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/CMakeModules/CalamaresAddPlugin.cmake b/CMakeModules/CalamaresAddPlugin.cmake index 66536eda7..ee3c63acb 100644 --- a/CMakeModules/CalamaresAddPlugin.cmake +++ b/CMakeModules/CalamaresAddPlugin.cmake @@ -187,13 +187,22 @@ function( calamares_add_plugin ) install( FILES ${CMAKE_CURRENT_BINARY_DIR}/${PLUGIN_DESC_FILE} DESTINATION ${PLUGIN_DESTINATION} ) + set( _warned_config OFF ) foreach( PLUGIN_CONFIG_FILE ${PLUGIN_CONFIG_FILES} ) - configure_file( ${PLUGIN_CONFIG_FILE} ${PLUGIN_CONFIG_FILE} COPYONLY ) + if( ${CMAKE_CURRENT_SOURCE_DIR}/${PLUGIN_CONFIG_FILE} IS_NEWER_THAN ${CMAKE_CURRENT_BINARY_DIR}/${PLUGIN_CONFIG_FILE} OR INSTALL_CONFIG ) + configure_file( ${PLUGIN_CONFIG_FILE} ${PLUGIN_CONFIG_FILE} COPYONLY ) + else() + message( " ${BoldYellow}Not updating${ColorReset} ${PLUGIN_CONFIG_FILE}" ) + set( _warned_config ON ) + endif() if ( INSTALL_CONFIG ) install( FILES ${CMAKE_CURRENT_BINARY_DIR}/${PLUGIN_CONFIG_FILE} DESTINATION ${PLUGIN_DATA_DESTINATION} ) endif() endforeach() + if ( _warned_config ) + message( "" ) + endif() endif() endfunction() From 2efce1ac7a0f4ef05405fe1373db86cd9213e5ac Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 5 Aug 2020 10:29:13 +0200 Subject: [PATCH 04/11] [users] Move the hostname-setting config - The configuration for writing the hostname (to /etc/hostname, to /etc/hosts and possibly to systemd-hostname) is read-only, because it comes from the config file and won't change after. --- src/modules/users/Config.cpp | 37 ++++++++++++++++++++++++++ src/modules/users/Config.h | 19 ++++++++++++++ src/modules/users/SetHostNameJob.cpp | 14 +++++----- src/modules/users/SetHostNameJob.h | 18 +++---------- src/modules/users/UsersViewStep.cpp | 39 ++-------------------------- src/modules/users/UsersViewStep.h | 4 --- 6 files changed, 69 insertions(+), 62 deletions(-) diff --git a/src/modules/users/Config.cpp b/src/modules/users/Config.cpp index 823b9fb99..b043691f8 100644 --- a/src/modules/users/Config.cpp +++ b/src/modules/users/Config.cpp @@ -36,6 +36,22 @@ static const QRegExp HOSTNAME_RX( "^[a-zA-Z0-9][-a-zA-Z0-9_]*$" ); static constexpr const int HOSTNAME_MIN_LENGTH = 2; static constexpr const int HOSTNAME_MAX_LENGTH = 63; +const NamedEnumTable< HostNameAction >& +hostNameActionNames() +{ + // *INDENT-OFF* + // clang-format off + static const NamedEnumTable< HostNameAction > names { + { QStringLiteral( "none" ), HostNameAction::None }, + { QStringLiteral( "etcfile" ), HostNameAction::EtcHostname }, + { QStringLiteral( "hostnamed" ), HostNameAction::SystemdHostname } + }; + // clang-format on + // *INDENT-ON* + + return names; +} + Config::Config( QObject* parent ) : QObject( parent ) { @@ -378,6 +394,25 @@ setConfigurationDefaultGroups( const QVariantMap& map, QStringList& defaultGroup } } +STATICTEST HostNameActions +getHostNameActions( const QVariantMap& configurationMap ) +{ + HostNameAction setHostName = HostNameAction::EtcHostname; + QString hostnameActionString = CalamaresUtils::getString( configurationMap, "setHostname" ); + if ( !hostnameActionString.isEmpty() ) + { + bool ok = false; + setHostName = hostNameActionNames().find( hostnameActionString, ok ); + if ( !ok ) + { + setHostName = HostNameAction::EtcHostname; // Rather than none + } + } + + HostNameAction writeHosts = CalamaresUtils::getBool( configurationMap, "writeHostsFile", true ) ? HostNameAction::WriteEtcHosts : HostNameAction::None; + return setHostName | writeHosts; +} + void Config::setConfigurationMap( const QVariantMap& configurationMap ) @@ -393,6 +428,8 @@ Config::setConfigurationMap( const QVariantMap& configurationMap ) setAutologinGroup( CalamaresUtils::getString( configurationMap, "autologinGroup" ) ); setSudoersGroup( CalamaresUtils::getString( configurationMap, "sudoersGroup" ) ); + m_hostNameActions = getHostNameActions( configurationMap ); + setConfigurationDefaultGroups( configurationMap, m_defaultGroups ); m_doAutoLogin = CalamaresUtils::getBool( configurationMap, "doAutologin", false ); diff --git a/src/modules/users/Config.h b/src/modules/users/Config.h index 8c30dad4c..e61564897 100644 --- a/src/modules/users/Config.h +++ b/src/modules/users/Config.h @@ -21,9 +21,23 @@ #ifndef USERS_CONFIG_H #define USERS_CONFIG_H +#include "utils/NamedEnum.h" + #include #include +enum HostNameAction +{ + None = 0x0, + EtcHostname = 0x1, // Write to /etc/hostname directly + SystemdHostname = 0x2, // Set via hostnamed(1) + WriteEtcHosts = 0x4 // Write /etc/hosts (127.0.1.1 is this host) +}; +Q_DECLARE_FLAGS( HostNameActions, HostNameAction ) +Q_DECLARE_OPERATORS_FOR_FLAGS( HostNameActions ) + +const NamedEnumTable< HostNameAction >& hostNameActionNames(); + class Config : public QObject { Q_OBJECT @@ -41,6 +55,7 @@ class Config : public QObject Q_PROPERTY( QString hostName READ hostName WRITE setHostName NOTIFY hostNameChanged ) Q_PROPERTY( QString hostNameStatus READ hostNameStatus NOTIFY hostNameStatusChanged ) + Q_PROPERTY( HostNameActions hostNameActions READ hostNameActions CONSTANT ) Q_PROPERTY( bool writeRootPassword READ writeRootPassword CONSTANT ) Q_PROPERTY( bool reuseUserPasswordForRoot READ reuseUserPasswordForRoot WRITE setReuseUserPasswordForRoot NOTIFY @@ -79,6 +94,8 @@ public: QString hostName() const { return m_hostName; } /// Status message about hostname -- empty for "ok" QString hostNameStatus() const; + /// How to write the hostname + HostNameActions hostNameActions() const { return m_hostNameActions; } /// Should the user be automatically logged-in? bool doAutoLogin() const { return m_doAutoLogin; } @@ -159,6 +176,8 @@ private: bool m_customLoginName = false; bool m_customHostName = false; + + HostNameActions m_hostNameActions; }; #endif diff --git a/src/modules/users/SetHostNameJob.cpp b/src/modules/users/SetHostNameJob.cpp index 555fcdc7d..a4cfa0268 100644 --- a/src/modules/users/SetHostNameJob.cpp +++ b/src/modules/users/SetHostNameJob.cpp @@ -27,13 +27,13 @@ #include #include -#include -#include -#include +#include +#include +#include using WriteMode = CalamaresUtils::System::WriteMode; -SetHostNameJob::SetHostNameJob( const QString& hostname, Actions a ) +SetHostNameJob::SetHostNameJob( const QString& hostname, HostNameActions a ) : Calamares::Job() , m_hostname( hostname ) , m_actions( a ) @@ -138,7 +138,7 @@ SetHostNameJob::exec() return Calamares::JobResult::error( tr( "Internal Error" ) ); } - if ( m_actions & Action::EtcHostname ) + if ( m_actions & HostNameAction::EtcHostname ) { if ( !setFileHostname( m_hostname ) ) { @@ -147,7 +147,7 @@ SetHostNameJob::exec() } } - if ( m_actions & Action::WriteEtcHosts ) + if ( m_actions & HostNameAction::WriteEtcHosts ) { if ( !writeFileEtcHosts( m_hostname ) ) { @@ -156,7 +156,7 @@ SetHostNameJob::exec() } } - if ( m_actions & Action::SystemdHostname ) + if ( m_actions & HostNameAction::SystemdHostname ) { // Does its own logging setSystemdHostname( m_hostname ); diff --git a/src/modules/users/SetHostNameJob.h b/src/modules/users/SetHostNameJob.h index e3867b9d0..5d52b545b 100644 --- a/src/modules/users/SetHostNameJob.h +++ b/src/modules/users/SetHostNameJob.h @@ -21,23 +21,15 @@ #ifndef SETHOSTNAMEJOB_CPP_H #define SETHOSTNAMEJOB_CPP_H +#include "Config.h" + #include "Job.h" class SetHostNameJob : public Calamares::Job { Q_OBJECT public: - enum Action - { - None = 0x0, - EtcHostname = 0x1, // Write to /etc/hostname directly - SystemdHostname = 0x2, // Set via hostnamed(1) - WriteEtcHosts = 0x4 // Write /etc/hosts (127.0.1.1 is this host) - }; - Q_DECLARE_FLAGS( Actions, Action ) - - - SetHostNameJob( const QString& hostname, Actions a ); + SetHostNameJob( const QString& hostname, HostNameActions a ); QString prettyName() const override; QString prettyDescription() const override; QString prettyStatusMessage() const override; @@ -45,9 +37,7 @@ public: private: const QString m_hostname; - const Actions m_actions; + const HostNameActions m_actions; }; -Q_DECLARE_OPERATORS_FOR_FLAGS( SetHostNameJob::Actions ) - #endif // SETHOSTNAMEJOB_CPP_H diff --git a/src/modules/users/UsersViewStep.cpp b/src/modules/users/UsersViewStep.cpp index 092826d24..300bdf072 100644 --- a/src/modules/users/UsersViewStep.cpp +++ b/src/modules/users/UsersViewStep.cpp @@ -34,28 +34,9 @@ CALAMARES_PLUGIN_FACTORY_DEFINITION( UsersViewStepFactory, registerPlugin< UsersViewStep >(); ) -static const NamedEnumTable< SetHostNameJob::Action >& -hostnameActions() -{ - using Action = SetHostNameJob::Action; - - // *INDENT-OFF* - // clang-format off - static const NamedEnumTable< Action > names { - { QStringLiteral( "none" ), Action::None }, - { QStringLiteral( "etcfile" ), Action::EtcHostname }, - { QStringLiteral( "hostnamed" ), Action::SystemdHostname } - }; - // clang-format on - // *INDENT-ON* - - return names; -} - UsersViewStep::UsersViewStep( QObject* parent ) : Calamares::ViewStep( parent ) , m_widget( nullptr ) - , m_actions( SetHostNameJob::Action::None ) , m_config( new Config( this ) ) { emit nextStatusChanged( true ); @@ -158,7 +139,8 @@ UsersViewStep::onLeave() j = new SetPasswordJob( "root", m_widget->getRootPassword() ); m_jobs.append( Calamares::job_ptr( j ) ); - j = new SetHostNameJob( m_config->hostName(), m_actions ); + // TODO: Config object should create jobs + j = new SetHostNameJob( m_config->hostName(), m_config->hostNameActions() ); m_jobs.append( Calamares::job_ptr( j ) ); m_widget->fillGlobalStorage(); @@ -184,21 +166,4 @@ UsersViewStep::setConfigurationMap( const QVariantMap& configurationMap ) m_widget->addPasswordCheck( i.key(), i.value() ); } } - - using Action = SetHostNameJob::Action; - - QString hostnameActionString = CalamaresUtils::getString( configurationMap, "setHostname" ); - if ( hostnameActionString.isEmpty() ) - { - hostnameActionString = QStringLiteral( "EtcFile" ); - } - bool ok = false; - auto hostnameAction = hostnameActions().find( hostnameActionString, ok ); - if ( !ok ) - { - hostnameAction = Action::EtcHostname; - } - - Action hostsfileAction = getBool( configurationMap, "writeHostsFile", true ) ? Action::WriteEtcHosts : Action::None; - m_actions = hostsfileAction | hostnameAction; } diff --git a/src/modules/users/UsersViewStep.h b/src/modules/users/UsersViewStep.h index 5c3674571..bfc43d1cd 100644 --- a/src/modules/users/UsersViewStep.h +++ b/src/modules/users/UsersViewStep.h @@ -20,8 +20,6 @@ #ifndef USERSPAGEPLUGIN_H #define USERSPAGEPLUGIN_H -#include "SetHostNameJob.h" - #include "DllMacro.h" #include "utils/PluginFactory.h" #include "viewpages/ViewStep.h" @@ -61,8 +59,6 @@ private: UsersPage* m_widget; QList< Calamares::job_ptr > m_jobs; - SetHostNameJob::Actions m_actions; - Config* m_config; }; From fcafe5db8f7434c5efbfadf09e5932f5a136f3e1 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 5 Aug 2020 10:50:38 +0200 Subject: [PATCH 05/11] [users] Test the moved setHostname Config - document that the default for writeHostsFile is *true* --- src/modules/users/Tests.cpp | 36 ++++++++++++++++++++++++++++++++++++ src/modules/users/users.conf | 1 + 2 files changed, 37 insertions(+) diff --git a/src/modules/users/Tests.cpp b/src/modules/users/Tests.cpp index a4ee45fe2..41cd96842 100644 --- a/src/modules/users/Tests.cpp +++ b/src/modules/users/Tests.cpp @@ -25,6 +25,7 @@ // Implementation details extern void setConfigurationDefaultGroups( const QVariantMap& map, QStringList& defaultGroups ); +extern HostNameActions getHostNameActions( const QVariantMap& configurationMap ); /** @brief Test Config object methods and internals * @@ -40,6 +41,8 @@ private Q_SLOTS: void initTestCase(); void testDefaultGroups(); + void testHostActions_data(); + void testHostActions(); }; UserTests::UserTests() {} @@ -105,6 +108,39 @@ UserTests::testDefaultGroups() } } +void +UserTests::testHostActions_data() +{ + QTest::addColumn< bool >( "set" ); + QTest::addColumn< QString >( "string" ); + QTest::addColumn< int >( "result" ); + + QTest::newRow( "unset " ) << false << QString() << int( HostNameAction::EtcHostname ); + QTest::newRow( "empty " ) << true << QString() << int( HostNameAction::EtcHostname ); + QTest::newRow( "bad " ) << true << QString( "derp" ) << int( HostNameAction::EtcHostname ); + QTest::newRow( "none " ) << true << QString( "none" ) << int( HostNameAction::None ); + QTest::newRow( "systemd" ) << true << QString( "Hostnamed" ) << int( HostNameAction::SystemdHostname ); +} + +void +UserTests::testHostActions() +{ + QFETCH( bool, set ); + QFETCH( QString, string ); + QFETCH( int, result ); + + QVariantMap m; + if ( set ) + { + m.insert( "setHostname", string ); + } + QCOMPARE( getHostNameActions( m ), HostNameActions( result ) | HostNameAction::WriteEtcHosts ); // write bits default to true + m.insert( "writeHostsFile", false ); + QCOMPARE( getHostNameActions( m ), HostNameActions( result ) ); + m.insert( "writeHostsFile", true ); + QCOMPARE( getHostNameActions( m ), HostNameActions( result ) | HostNameAction::WriteEtcHosts ); +} + QTEST_GUILESS_MAIN( UserTests ) diff --git a/src/modules/users/users.conf b/src/modules/users/users.conf index 32f74978a..259723df2 100644 --- a/src/modules/users/users.conf +++ b/src/modules/users/users.conf @@ -138,4 +138,5 @@ setHostname: EtcFile # Should /etc/hosts be written with a hostname for this machine # (also adds localhost and some ipv6 standard entries). +# Defaults to *true*. writeHostsFile: true From 0ecf1e1cc131d04a0d042e2478ff73b69cbff1e2 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 5 Aug 2020 12:24:39 +0200 Subject: [PATCH 06/11] [users] Drop default parameter for badness --- src/modules/users/UsersPage.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/modules/users/UsersPage.cpp b/src/modules/users/UsersPage.cpp index 9054d60e7..9509592ba 100644 --- a/src/modules/users/UsersPage.cpp +++ b/src/modules/users/UsersPage.cpp @@ -50,7 +50,7 @@ enum class Badness /** Add an error message and pixmap to a label. */ static inline void -labelError( QLabel* pix, QLabel* label, const QString& message, Badness bad = Badness::Fatal ) +labelError( QLabel* pix, QLabel* label, const QString& message, Badness bad ) { label->setText( message ); pix->setPixmap( CalamaresUtils::defaultPixmap( ( bad == Badness::Fatal ) ? CalamaresUtils::StatusError @@ -88,7 +88,7 @@ labelStatus( QLabel* pix, QLabel* label, const QString& value, const QString& st } else { - labelError( pix, label, status ); + labelError( pix, label, status, Badness::Fatal ); ok = false; } } @@ -278,7 +278,7 @@ UsersPage::checkPasswordAcceptance( const QString& pw1, const QString& pw2, QLab { if ( pw1 != pw2 ) { - labelError( badge, message, tr( "Your passwords do not match!" ) ); + labelError( badge, message, tr( "Your passwords do not match!" ), Badness::Fatal ); return false; } else From 7b87242107f90388f401ab4650d281d5fd26ba32 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 5 Aug 2020 12:56:09 +0200 Subject: [PATCH 07/11] [users] PW checking does not need widgets --- src/modules/users/CheckPWQuality.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/modules/users/CheckPWQuality.cpp b/src/modules/users/CheckPWQuality.cpp index f06137c9b..33309df6a 100644 --- a/src/modules/users/CheckPWQuality.cpp +++ b/src/modules/users/CheckPWQuality.cpp @@ -22,7 +22,6 @@ #include #include -#include #ifdef HAVE_LIBPWQUALITY #include From 900deb5dc86042a447770a929e377a636d7dcd79 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 5 Aug 2020 13:03:18 +0200 Subject: [PATCH 08/11] [users] Move the configuration of password checks to Config - the Widget (Page) does not need to know the password checks, that's business logic that belongs to Config. --- src/modules/users/Config.cpp | 81 ++++++++++++++++++++++++++++- src/modules/users/Config.h | 14 +++++ src/modules/users/UsersPage.cpp | 68 +++--------------------- src/modules/users/UsersPage.h | 13 ----- src/modules/users/UsersViewStep.cpp | 15 ------ 5 files changed, 102 insertions(+), 89 deletions(-) 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() ); - } - } } From b2b9ae779932643afbb7ff3e7e80a89bfe76d759 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 5 Aug 2020 13:03:56 +0200 Subject: [PATCH 09/11] [users] Add tests for moved password-check configuration - link the PW checks to the test, and libpwquality if needed - test only does very basic config-mungeing --- src/modules/users/CMakeLists.txt | 3 +++ src/modules/users/Tests.cpp | 15 +++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/src/modules/users/CMakeLists.txt b/src/modules/users/CMakeLists.txt index 5fafcd4f3..95b3a9697 100644 --- a/src/modules/users/CMakeLists.txt +++ b/src/modules/users/CMakeLists.txt @@ -71,4 +71,7 @@ calamares_add_test( SOURCES Tests.cpp Config.cpp + CheckPWQuality.cpp + LIBRARIES + ${USER_EXTRA_LIB} ) diff --git a/src/modules/users/Tests.cpp b/src/modules/users/Tests.cpp index 41cd96842..ff435803d 100644 --- a/src/modules/users/Tests.cpp +++ b/src/modules/users/Tests.cpp @@ -26,6 +26,7 @@ // Implementation details extern void setConfigurationDefaultGroups( const QVariantMap& map, QStringList& defaultGroups ); extern HostNameActions getHostNameActions( const QVariantMap& configurationMap ); +extern bool addPasswordCheck( const QString& key, const QVariant& value, PasswordCheckList& passwordChecks ); /** @brief Test Config object methods and internals * @@ -43,6 +44,7 @@ private Q_SLOTS: void testDefaultGroups(); void testHostActions_data(); void testHostActions(); + void testPasswordChecks(); }; UserTests::UserTests() {} @@ -141,6 +143,19 @@ UserTests::testHostActions() QCOMPARE( getHostNameActions( m ), HostNameActions( result ) | HostNameAction::WriteEtcHosts ); } +void +UserTests::testPasswordChecks() +{ + { + PasswordCheckList l; + QCOMPARE( l.length(), 0 ); + QVERIFY( !addPasswordCheck( "nonempty", QVariant(false), l ) ); // a silly setting + QCOMPARE( l.length(), 0 ); + QVERIFY( addPasswordCheck( "nonempty", QVariant(true), l ) ); + QCOMPARE( l.length(), 1 ); + } +} + QTEST_GUILESS_MAIN( UserTests ) From eb72d662d10b6b94b3ce89f9e2eeb0cd968849ba Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 5 Aug 2020 13:29:06 +0200 Subject: [PATCH 10/11] [users] Add password fields to Config - no checking is done for validity, and there is no password-status --- src/modules/users/Config.cpp | 38 +++++++++++++++++++++++++++++++++++- src/modules/users/Config.h | 28 ++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/src/modules/users/Config.cpp b/src/modules/users/Config.cpp index 90c301997..ea3aa5221 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 ) { bool failureIsFatal = requireStrongPasswords(); @@ -399,7 +399,43 @@ Config::isPasswordAcceptable(const QString& password, QString& message) return true; } +void +Config::setUserPassword( const QString& s ) +{ + 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 ); +} + +void +Config::setRootPassword( const QString& s ) +{ + if ( writeRootPassword() ) + { + m_rootPassword = s; + // TODO: check new password status + emit rootPasswordChanged( s ); + } +} +void +Config::setRootPasswordSecondary( const QString& s ) +{ + if ( writeRootPassword() ) + { + m_rootPasswordSecondary = s; + // TODO: check new password status + emit rootPasswordSecondaryChanged( s ); + } +} STATICTEST void diff --git a/src/modules/users/Config.h b/src/modules/users/Config.h index c89ae9bbf..5177542c2 100644 --- a/src/modules/users/Config.h +++ b/src/modules/users/Config.h @@ -59,6 +59,13 @@ class Config : public QObject Q_PROPERTY( QString hostNameStatus READ hostNameStatus NOTIFY hostNameStatusChanged ) Q_PROPERTY( HostNameActions hostNameActions READ hostNameActions CONSTANT ) + Q_PROPERTY( QString userPassword READ userPassword WRITE setUserPassword NOTIFY userPasswordChanged ) + Q_PROPERTY( QString userPasswordSecondary READ userPasswordSecondary WRITE setUserPasswordSecondary NOTIFY + userPasswordSecondaryChanged ) + Q_PROPERTY( QString rootPassword READ rootPassword WRITE setRootPassword NOTIFY rootPasswordChanged ) + Q_PROPERTY( QString rootPasswordSecondary READ rootPasswordSecondary WRITE setRootPasswordSecondary NOTIFY + rootPasswordSecondaryChanged ) + Q_PROPERTY( bool writeRootPassword READ writeRootPassword CONSTANT ) Q_PROPERTY( bool reuseUserPasswordForRoot READ reuseUserPasswordForRoot WRITE setReuseUserPasswordForRoot NOTIFY reuseUserPasswordForRootChanged ) @@ -123,6 +130,11 @@ public: */ bool isPasswordAcceptable( const QString& password, QString& message ); + QString userPassword() const { return m_userPassword; } + QString userPasswordSecondary() const { return m_userPasswordSecondary; } + QString rootPassword() const { return m_rootPassword; } + QString rootPasswordSecondary() const { return m_rootPasswordSecondary; } + static const QStringList& forbiddenLoginNames(); static const QStringList& forbiddenHostNames(); @@ -158,6 +170,11 @@ public Q_SLOTS: /// Change setting for "require strong password" void setRequireStrongPasswords( bool strong ); + void setUserPassword( const QString& ); + void setUserPasswordSecondary( const QString& ); + void setRootPassword( const QString& ); + void setRootPasswordSecondary( const QString& ); + signals: void userShellChanged( const QString& ); void autologinGroupChanged( const QString& ); @@ -170,6 +187,11 @@ signals: void autoLoginChanged( bool ); void reuseUserPasswordForRootChanged( bool ); void requireStrongPasswordsChanged( bool ); + void userPasswordChanged( const QString& ); + void userPasswordSecondaryChanged( const QString& ); + void rootPasswordChanged( const QString& ); + void rootPasswordSecondaryChanged( const QString& ); + private: QStringList m_defaultGroups; @@ -179,6 +201,12 @@ private: QString m_fullName; QString m_loginName; QString m_hostName; + + QString m_userPassword; + QString m_userPasswordSecondary; // enter again to be sure + QString m_rootPassword; + QString m_rootPasswordSecondary; + bool m_doAutoLogin = false; bool m_writeRootPassword = true; From b49b9a66e6af900f0102ccddd2d2102c648009b8 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 5 Aug 2020 13:42:18 +0200 Subject: [PATCH 11/11] [users] Drop data-access from the Page - get username, password etc. from the config object, not the page - jobs now depend entirely on config - handle logic of "what's the root password" in Config --- src/modules/users/Config.cpp | 22 ++++++++++++++++++++++ src/modules/users/Config.h | 8 ++++++-- src/modules/users/UsersPage.cpp | 26 -------------------------- src/modules/users/UsersPage.h | 5 ----- src/modules/users/UsersViewStep.cpp | 5 ++--- 5 files changed, 30 insertions(+), 36 deletions(-) diff --git a/src/modules/users/Config.cpp b/src/modules/users/Config.cpp index ea3aa5221..69712a733 100644 --- a/src/modules/users/Config.cpp +++ b/src/modules/users/Config.cpp @@ -437,6 +437,28 @@ Config::setRootPasswordSecondary( const QString& s ) } } +QString Config::rootPassword() const +{ + if ( writeRootPassword() ) + { + if ( reuseUserPasswordForRoot() ) + return userPassword(); + return m_rootPassword; + } + return QString(); +} + +QString Config::rootPasswordSecondary() const +{ + if ( writeRootPassword() ) + { + if ( reuseUserPasswordForRoot() ) + return userPasswordSecondary(); + return m_rootPasswordSecondary; + } + return QString(); +} + STATICTEST void setConfigurationDefaultGroups( const QVariantMap& map, QStringList& defaultGroups ) diff --git a/src/modules/users/Config.h b/src/modules/users/Config.h index 5177542c2..334bfcdb3 100644 --- a/src/modules/users/Config.h +++ b/src/modules/users/Config.h @@ -130,10 +130,14 @@ public: */ bool isPasswordAcceptable( const QString& password, QString& message ); + // The user enters a password (and again in a separate UI element) QString userPassword() const { return m_userPassword; } QString userPasswordSecondary() const { return m_userPasswordSecondary; } - QString rootPassword() const { return m_rootPassword; } - QString rootPasswordSecondary() const { return m_rootPasswordSecondary; } + // 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; static const QStringList& forbiddenLoginNames(); static const QStringList& forbiddenHostNames(); diff --git a/src/modules/users/UsersPage.cpp b/src/modules/users/UsersPage.cpp index aca20fd58..5a96af4e4 100644 --- a/src/modules/users/UsersPage.cpp +++ b/src/modules/users/UsersPage.cpp @@ -199,32 +199,6 @@ UsersPage::isReady() const return readyFields; } -QString -UsersPage::getRootPassword() const -{ - if ( m_config->writeRootPassword() ) - { - if ( ui->checkBoxReusePassword->isChecked() ) - { - return ui->textBoxUserPassword->text(); - } - else - { - return ui->textBoxRootPassword->text(); - } - } - else - { - return QString(); - } -} - -QPair< QString, QString > -UsersPage::getUserPassword() const -{ - return QPair< QString, QString >( m_config->loginName(), ui->textBoxUserPassword->text() ); -} - void UsersPage::fillGlobalStorage() const { diff --git a/src/modules/users/UsersPage.h b/src/modules/users/UsersPage.h index 2f446b563..46c4ed399 100644 --- a/src/modules/users/UsersPage.h +++ b/src/modules/users/UsersPage.h @@ -48,11 +48,6 @@ public: void onActivate(); - ///@brief Root password, depends on settings, may be empty - QString getRootPassword() const; - ///@brief User name and password - QPair< QString, QString > getUserPassword() const; - protected slots: void onFullNameTextEdited( const QString& ); void reportLoginNameStatus( const QString& ); diff --git a/src/modules/users/UsersViewStep.cpp b/src/modules/users/UsersViewStep.cpp index 75c76bd21..2733539b6 100644 --- a/src/modules/users/UsersViewStep.cpp +++ b/src/modules/users/UsersViewStep.cpp @@ -132,11 +132,10 @@ UsersViewStep::onLeave() m_config->doAutoLogin(), m_config->defaultGroups() ); - auto userPW = m_widget->getUserPassword(); - j = new SetPasswordJob( userPW.first, userPW.second ); + j = new SetPasswordJob( m_config->loginName(), m_config->userPassword() ); m_jobs.append( Calamares::job_ptr( j ) ); - j = new SetPasswordJob( "root", m_widget->getRootPassword() ); + j = new SetPasswordJob( "root", m_config->rootPassword() ); m_jobs.append( Calamares::job_ptr( j ) ); // TODO: Config object should create jobs