use hashed passwords in all commands (#4493)

* protocol changes

* server changes

* client changes for password reset and registration

* add hashed password to change password in client

* always use hashed password to log in

* add warning to client when using plain text password

* require real password for changing email on server

this is backwards compatible as users logged in with a real password on
older clients will not need this, only users logged in with a hashed
password

* implement password dialog when changing email

* require min password length

* use qstringlist to build query instead

* use clear instead of = ""

* add max to password dialog

* use proper const ness in abstractclient

* reject too long passwords instead of trimming

servatrice/src/servatrice_database_interface.cpp

@@ -198,6 +198,7 @@ bool Servatrice_DatabaseInterface::usernameIsValid(const QString &user, QString
 bool Servatrice_DatabaseInterface::registerUser(const QString &userName,
                                                 const QString &realName,
                                                 const QString &password,
+                                                bool passwordNeedsHash,
                                                 const QString &emailAddress,
                                                 const QString &country,
                                                 bool active)
@@ -205,7 +206,12 @@ bool Servatrice_DatabaseInterface::registerUser(const QString &userName,
     if (!checkSql())
         return false;
 
-    QString passwordSha512 = PasswordHasher::computeHash(password, PasswordHasher::generateRandomSalt());
+    QString passwordSha512;
+    if (passwordNeedsHash) {
+        passwordSha512 = PasswordHasher::computeHash(password, PasswordHasher::generateRandomSalt());
+    } else {
+        passwordSha512 = password;
+    }
     QString token = active ? QString() : PasswordHasher::generateActivationToken();
 
     QSqlQuery *query =
@@ -303,7 +309,7 @@ AuthenticationResult Servatrice_DatabaseInterface::checkUserPassword(Server_Prot
             }
 
             if (passwordQuery->next()) {
-                const QString correctPassword = passwordQuery->value(0).toString();
+                const QString correctPasswordSha512 = passwordQuery->value(0).toString();
                 const bool userIsActive = passwordQuery->value(1).toBool();
                 if (!userIsActive) {
                     qDebug("Login denied: user not active");
@@ -311,11 +317,11 @@ AuthenticationResult Servatrice_DatabaseInterface::checkUserPassword(Server_Prot
                 }
                 QString hashedPassword;
                 if (passwordNeedsHash) {
-                    hashedPassword = PasswordHasher::computeHash(password, correctPassword.left(16));
+                    hashedPassword = PasswordHasher::computeHash(password, correctPasswordSha512.left(16));
                 } else {
                     hashedPassword = password;
                 }
-                if (correctPassword == hashedPassword) {
+                if (correctPasswordSha512 == hashedPassword) {
                     qDebug("Login accepted: password right");
                     return PasswordRight;
                 } else {
@@ -935,10 +941,30 @@ void Servatrice_DatabaseInterface::logMessage(const int senderId,
     execSqlQuery(query);
 }
 
+bool Servatrice_DatabaseInterface::changeUserPassword(const QString &user,
+                                                      const QString &password,
+                                                      bool passwordNeedsHash)
+{
+    QString passwordSha512 = password;
+    if (passwordNeedsHash) {
+        passwordSha512 = PasswordHasher::computeHash(password, PasswordHasher::generateRandomSalt());
+    }
+
+    QSqlQuery *passwordQuery = prepareQuery("update {prefix}_users set password_sha512=:password, "
+                                            "passwordLastChangedDate = NOW() where name = :name");
+    passwordQuery->bindValue(":password", passwordSha512);
+    passwordQuery->bindValue(":name", user);
+    if (execSqlQuery(passwordQuery))
+        return true;
+
+    return false;
+}
+
 bool Servatrice_DatabaseInterface::changeUserPassword(const QString &user,
                                                       const QString &oldPassword,
+                                                      bool oldPasswordNeedsHash,
                                                       const QString &newPassword,
-                                                      const bool &force = false)
+                                                      bool newPasswordNeedsHash)
 {
     if (server->getAuthenticationMethod() != Servatrice::AuthenticationSql)
         return false;
@@ -953,30 +979,24 @@ bool Servatrice_DatabaseInterface::changeUserPassword(const QString &user,
     QSqlQuery *passwordQuery = prepareQuery("select password_sha512 from {prefix}_users where name = :name");
     passwordQuery->bindValue(":name", user);
 
-    if (!force) {
-        if (!execSqlQuery(passwordQuery)) {
-            qDebug("Change password denied: SQL error");
-            return false;
-        }
-
-        if (!passwordQuery->next())
-            return false;
-
-        const QString correctPassword = passwordQuery->value(0).toString();
-        if (correctPassword != PasswordHasher::computeHash(oldPassword, correctPassword.left(16)))
-            return false;
+    if (!execSqlQuery(passwordQuery)) {
+        qDebug("Change password denied: SQL error");
+        return false;
     }
 
-    QString passwordSha512 = PasswordHasher::computeHash(newPassword, PasswordHasher::generateRandomSalt());
+    if (!passwordQuery->next())
+        return false;
 
-    passwordQuery = prepareQuery("update {prefix}_users set password_sha512=:password, "
-                                 "passwordLastChangedDate = NOW() where name = :name");
-    passwordQuery->bindValue(":password", passwordSha512);
-    passwordQuery->bindValue(":name", user);
-    if (execSqlQuery(passwordQuery))
-        return true;
+    const QString correctPasswordSha512 = passwordQuery->value(0).toString();
+    QString oldPasswordSha512 = oldPassword;
+    if (oldPasswordNeedsHash) {
+        QString salt = correctPasswordSha512.left(16);
+        oldPasswordSha512 = PasswordHasher::computeHash(oldPassword, salt);
+    }
+    if (correctPasswordSha512 != oldPasswordSha512)
+        return false;
 
-    return false;
+    return changeUserPassword(user, newPassword, newPasswordNeedsHash);
 }
 
 int Servatrice_DatabaseInterface::getActiveUserCount(QString connectionType)

servatrice/src/servatrice_database_interface.h

@@ -98,6 +98,7 @@ public:
     bool registerUser(const QString &userName,
                       const QString &realName,
                       const QString &password,
+                      bool passwordNeedsHash,
                       const QString &emailAddress,
                       const QString &country,
                       bool active = false);
@@ -111,8 +112,12 @@ public:
                     LogMessage_TargetType targetType,
                     const int targetId,
                     const QString &targetName);
-    bool
-    changeUserPassword(const QString &user, const QString &oldPassword, const QString &newPassword, const bool &force);
+    bool changeUserPassword(const QString &user, const QString &password, bool passwordNeedsHash);
+    bool changeUserPassword(const QString &user,
+                            const QString &oldPassword,
+                            bool oldPasswordNeedsHash,
+                            const QString &newPassword,
+                            bool newPasswordNeedsHash);
     QList<ServerInfo_Ban> getUserBanHistory(const QString userName);
     bool
     addWarning(const QString userName, const QString adminName, const QString warningReason, const QString clientID);

servatrice/src/serversocketinterface.cpp

@@ -1164,19 +1164,29 @@ Response::ResponseCode AbstractServerSocketInterface::cmdRegisterAccount(const C
 
     QString realName = nameFromStdString(cmd.real_name());
     QString country = nameFromStdString(cmd.country());
-    QString password = nameFromStdString(cmd.password());
-
-    if (!isPasswordLongEnough(password.length())) {
-        if (servatrice->getEnableRegistrationAudit())
-            sqlInterface->addAuditRecord(userName.simplified(), this->getAddress(), clientId.simplified(),
-                                         "REGISTER_ACCOUNT", "Password is too short", false);
-
-        return Response::RespPasswordTooShort;
+    QString password;
+    bool passwordNeedsHash = false;
+    if (cmd.has_password()) {
+        if (cmd.password().length() > MAX_NAME_LENGTH)
+            return Response::RespRegistrationFailed;
+        password = QString::fromStdString(cmd.password());
+        passwordNeedsHash = true;
+        if (!isPasswordLongEnough(password.length())) {
+            if (servatrice->getEnableRegistrationAudit()) {
+                sqlInterface->addAuditRecord(userName.simplified(), this->getAddress(), clientId.simplified(),
+                                             "REGISTER_ACCOUNT", "Password is too short", false);
+            }
+            return Response::RespPasswordTooShort;
+        }
+    } else if (cmd.hashed_password().length() > MAX_NAME_LENGTH) {
+        return Response::RespRegistrationFailed;
+    } else {
+        password = QString::fromStdString(cmd.hashed_password());
     }
 
     bool requireEmailActivation = settingsCache->value("registration/requireemailactivation", true).toBool();
-    bool regSucceeded =
-        sqlInterface->registerUser(userName, realName, password, emailAddress, country, !requireEmailActivation);
+    bool regSucceeded = sqlInterface->registerUser(userName, realName, password, passwordNeedsHash, emailAddress,
+                                                   country, !requireEmailActivation);
 
     if (regSucceeded) {
         qDebug() << "Accepted register command for user: " << userName;
@@ -1256,20 +1266,72 @@ Response::ResponseCode AbstractServerSocketInterface::cmdAccountEdit(const Comma
     QString emailAddress = nameFromStdString(cmd.email());
     QString country = nameFromStdString(cmd.country());
 
+    bool checkedPassword = false;
     QString userName = QString::fromStdString(userInfo->name());
+    if (cmd.has_password_check()) {
+        if (cmd.password_check().length() > MAX_NAME_LENGTH)
+            return Response::RespWrongPassword;
+        QString password = QString::fromStdString(cmd.password_check());
+        QString clientId = QString::fromStdString(userInfo->clientid());
+        QString reasonStr{};
+        int secondsLeft{};
+        AuthenticationResult checkStatus =
+            databaseInterface->checkUserPassword(this, userName, password, clientId, reasonStr, secondsLeft, true);
+        if (checkStatus == PasswordRight) {
+            checkedPassword = true;
+        } else {
+            // the user already logged in with this info, the only change is their password
+            return Response::RespWrongPassword;
+        }
+    }
 
-    QSqlQuery *query = sqlInterface->prepareQuery("update {prefix}_users set realname=:realName, email=:email, "
-                                                  "country=:country where name=:userName");
-    query->bindValue(":realName", realName);
-    query->bindValue(":email", emailAddress);
-    query->bindValue(":country", country);
+    QStringList queryList({});
+    if (cmd.has_real_name()) {
+        queryList << "realname=:realName";
+    }
+    if (cmd.has_email()) {
+        // a real password is required in order to change the email address
+        if (usingRealPassword || checkedPassword) {
+            queryList << "email=:email";
+        } else {
+            return Response::RespFunctionNotAllowed;
+        }
+    }
+    if (cmd.has_country()) {
+        queryList << "country=:country";
+    }
+
+    if (queryList.isEmpty())
+        return Response::RespOk;
+
+    QString queryText = QString("update {prefix}_users set %1 where name=:userName").arg(queryList.join(", "));
+    QSqlQuery *query = sqlInterface->prepareQuery(queryText);
+    if (cmd.has_real_name()) {
+        QString realName = nameFromStdString(cmd.real_name());
+        query->bindValue(":realName", realName);
+    }
+    if (cmd.has_email()) {
+        QString emailAddress = nameFromStdString(cmd.email());
+        query->bindValue(":email", emailAddress);
+    }
+    if (cmd.has_country()) {
+        QString country = nameFromStdString(cmd.country());
+        query->bindValue(":country", country);
+    }
     query->bindValue(":userName", userName);
+
     if (!sqlInterface->execSqlQuery(query))
         return Response::RespInternalError;
 
-    userInfo->set_real_name(realName.toStdString());
-    userInfo->set_email(emailAddress.toStdString());
-    userInfo->set_country(country.toStdString());
+    if (cmd.has_real_name()) {
+        userInfo->set_real_name(realName.toStdString());
+    }
+    if (cmd.has_email()) {
+        userInfo->set_email(emailAddress.toStdString());
+    }
+    if (cmd.has_country()) {
+        userInfo->set_country(country.toStdString());
+    }
 
     return Response::RespOk;
 }
@@ -1300,15 +1362,26 @@ Response::ResponseCode AbstractServerSocketInterface::cmdAccountPassword(const C
     if (authState != PasswordRight)
         return Response::RespFunctionNotAllowed;
 
-    QString oldPassword = nameFromStdString(cmd.old_password());
-    QString newPassword = nameFromStdString(cmd.new_password());
-
-    if (!isPasswordLongEnough(newPassword.length()))
-        return Response::RespPasswordTooShort;
+    if (cmd.old_password().length() > MAX_NAME_LENGTH)
+        return Response::RespWrongPassword;
+    QString oldPassword = QString::fromStdString(cmd.old_password());
+    QString newPassword;
+    bool newPasswordNeedsHash = false;
+    if (cmd.has_new_password()) {
+        if (cmd.new_password().length() > MAX_NAME_LENGTH)
+            return Response::RespContextError;
+        newPassword = QString::fromStdString(cmd.new_password());
+        newPasswordNeedsHash = true;
+        if (!isPasswordLongEnough(newPassword.length()))
+            return Response::RespPasswordTooShort;
+    } else if (cmd.hashed_new_password().length() > MAX_NAME_LENGTH) {
+        return Response::RespContextError;
+    } else {
+        newPassword = QString::fromStdString(cmd.hashed_new_password());
+    }
 
     QString userName = QString::fromStdString(userInfo->name());
-
-    if (!databaseInterface->changeUserPassword(userName, oldPassword, newPassword, false))
+    if (!databaseInterface->changeUserPassword(userName, oldPassword, true, newPassword, newPasswordNeedsHash))
         return Response::RespWrongPassword;
 
     return Response::RespOk;
@@ -1421,7 +1494,20 @@ Response::ResponseCode AbstractServerSocketInterface::cmdForgotPasswordReset(con
         return Response::RespFunctionNotAllowed;
     }
 
-    if (sqlInterface->changeUserPassword(userName, "", nameFromStdString(cmd.new_password()), true)) {
+    QString password;
+    bool passwordNeedsHash = false;
+    if (cmd.has_new_password()) {
+        if (cmd.new_password().length() > MAX_NAME_LENGTH)
+            return Response::RespContextError;
+        password = QString::fromStdString(cmd.new_password());
+        passwordNeedsHash = true;
+    } else if (cmd.hashed_new_password().length() > MAX_NAME_LENGTH) {
+        return Response::RespContextError;
+    } else {
+        password = QString::fromStdString(cmd.hashed_new_password());
+    }
+
+    if (sqlInterface->changeUserPassword(nameFromStdString(cmd.user_name()), password, passwordNeedsHash)) {
         if (servatrice->getEnableForgotPasswordAudit())
             sqlInterface->addAuditRecord(userName.simplified(), this->getAddress(), clientId.simplified(),
                                          "PASSWORD_RESET", "", true);