diff --git a/cockatrice/src/abstractclient.cpp b/cockatrice/src/abstractclient.cpp index abd1ec0d..ab0f3d25 100644 --- a/cockatrice/src/abstractclient.cpp +++ b/cockatrice/src/abstractclient.cpp @@ -22,7 +22,8 @@ #include -AbstractClient::AbstractClient(QObject *parent) : QObject(parent), nextCmdId(0), status(StatusDisconnected) +AbstractClient::AbstractClient(QObject *parent) + : QObject(parent), nextCmdId(0), status(StatusDisconnected), serverSupportsPasswordHash(false) { qRegisterMetaType("QVariant"); qRegisterMetaType("CommandContainer"); diff --git a/cockatrice/src/abstractclient.h b/cockatrice/src/abstractclient.h index 30c99823..97b9c0f2 100644 --- a/cockatrice/src/abstractclient.h +++ b/cockatrice/src/abstractclient.h @@ -88,6 +88,7 @@ protected slots: protected: QMap pendingCommands; QString userName, password, email, country, realName, token; + bool serverSupportsPasswordHash; void setStatus(ClientStatus _status); int getNewCmdId() { @@ -107,7 +108,11 @@ public: void sendCommand(const CommandContainer &cont); void sendCommand(PendingCommand *pend); - const QString getUserName() + bool getServerSupportsPasswordHash() const + { + return serverSupportsPasswordHash; + } + const QString &getUserName() const { return userName; } diff --git a/cockatrice/src/dlg_edit_password.cpp b/cockatrice/src/dlg_edit_password.cpp index abec2cf9..c942c0c9 100644 --- a/cockatrice/src/dlg_edit_password.cpp +++ b/cockatrice/src/dlg_edit_password.cpp @@ -59,7 +59,11 @@ DlgEditPassword::DlgEditPassword(QWidget *parent) : QDialog(parent) void DlgEditPassword::actOk() { - if (newPasswordEdit->text() != newPasswordEdit2->text()) { + // TODO this stuff should be using qvalidators + if (newPasswordEdit->text().length() < 8) { + QMessageBox::critical(this, tr("Error"), tr("Your password is too short.")); + return; + } else if (newPasswordEdit->text() != newPasswordEdit2->text()) { QMessageBox::warning(this, tr("Error"), tr("The new passwords don't match.")); return; } diff --git a/cockatrice/src/dlg_forgotpasswordreset.cpp b/cockatrice/src/dlg_forgotpasswordreset.cpp index bf9ea5a8..9669a21e 100644 --- a/cockatrice/src/dlg_forgotpasswordreset.cpp +++ b/cockatrice/src/dlg_forgotpasswordreset.cpp @@ -123,7 +123,11 @@ void DlgForgotPasswordReset::actOk() return; } - if (newpasswordEdit->text() != newpasswordverifyEdit->text()) { + // TODO this stuff should be using qvalidators + if (newpasswordEdit->text().length() < 8) { + QMessageBox::critical(this, tr("Error"), tr("Your password is too short.")); + return; + } else if (newpasswordEdit->text() != newpasswordverifyEdit->text()) { QMessageBox::critical(this, tr("Reset Password Error"), tr("The passwords do not match.")); return; } diff --git a/cockatrice/src/dlg_register.cpp b/cockatrice/src/dlg_register.cpp index adf6ba6e..fc3c97c2 100644 --- a/cockatrice/src/dlg_register.cpp +++ b/cockatrice/src/dlg_register.cpp @@ -356,7 +356,11 @@ DlgRegister::DlgRegister(QWidget *parent) : QDialog(parent) void DlgRegister::actOk() { - if (passwordEdit->text() != passwordConfirmationEdit->text()) { + // TODO this stuff should be using qvalidators + if (passwordEdit->text().length() < 8) { + QMessageBox::critical(this, tr("Registration Warning"), tr("Your password is too short.")); + return; + } else if (passwordEdit->text() != passwordConfirmationEdit->text()) { QMessageBox::critical(this, tr("Registration Warning"), tr("Your passwords do not match, please try again.")); return; } else if (emailConfirmationEdit->text() != emailEdit->text()) { diff --git a/cockatrice/src/remoteclient.cpp b/cockatrice/src/remoteclient.cpp index 75d0ab61..6b9a0940 100644 --- a/cockatrice/src/remoteclient.cpp +++ b/cockatrice/src/remoteclient.cpp @@ -27,7 +27,7 @@ static const unsigned int protocolVersion = 14; RemoteClient::RemoteClient(QObject *parent) : AbstractClient(parent), timeRunning(0), lastDataReceived(0), messageInProgress(false), handshakeStarted(false), - usingWebSocket(false), messageLength(0) + usingWebSocket(false), messageLength(0), hashedPassword() { clearNewClientFeatures(); @@ -110,6 +110,7 @@ void RemoteClient::processServerIdentificationEvent(const Event_ServerIdentifica setStatus(StatusDisconnecting); return; } + serverSupportsPasswordHash = event.server_options() & Event_ServerIdentification::SupportsPasswordHash; if (getStatus() == StatusRequestingForgotPassword) { Command_ForgotPasswordRequest cmdForgotPasswordRequest; @@ -127,7 +128,14 @@ void RemoteClient::processServerIdentificationEvent(const Event_ServerIdentifica cmdForgotPasswordReset.set_user_name(userName.toStdString()); cmdForgotPasswordReset.set_clientid(getSrvClientID(lastHostname).toStdString()); cmdForgotPasswordReset.set_token(token.toStdString()); - cmdForgotPasswordReset.set_new_password(password.toStdString()); + if (!password.isEmpty() && serverSupportsPasswordHash) { + auto passwordSalt = PasswordHasher::generateRandomSalt(); + hashedPassword = PasswordHasher::computeHash(password, passwordSalt); + cmdForgotPasswordReset.set_hashed_new_password(hashedPassword.toStdString()); + } else if (!password.isEmpty()) { + qWarning() << "using plain text password to reset password"; + cmdForgotPasswordReset.set_new_password(password.toStdString()); + } PendingCommand *pend = prepareSessionCommand(cmdForgotPasswordReset); connect(pend, SIGNAL(finished(Response, CommandContainer, QVariant)), this, SLOT(submitForgotPasswordResetResponse(Response))); @@ -150,7 +158,14 @@ void RemoteClient::processServerIdentificationEvent(const Event_ServerIdentifica if (getStatus() == StatusRegistering) { Command_Register cmdRegister; cmdRegister.set_user_name(userName.toStdString()); - cmdRegister.set_password(password.toStdString()); + if (!password.isEmpty() && serverSupportsPasswordHash) { + auto passwordSalt = PasswordHasher::generateRandomSalt(); + hashedPassword = PasswordHasher::computeHash(password, passwordSalt); + cmdRegister.set_hashed_password(hashedPassword.toStdString()); + } else if (!password.isEmpty()) { + qWarning() << "using plain text password to register"; + cmdRegister.set_password(password.toStdString()); + } cmdRegister.set_email(email.toStdString()); cmdRegister.set_country(country.toStdString()); cmdRegister.set_real_name(realName.toStdString()); @@ -175,13 +190,7 @@ void RemoteClient::processServerIdentificationEvent(const Event_ServerIdentifica return; } - if (!password.isEmpty() && event.server_options() & Event_ServerIdentification::SupportsPasswordHash) { - // TODO store and log in using stored hashed password - doRequestPasswordSalt(); // log in using password salt - } else { - // TODO add setting for client to reject unhashed logins - doLogin(); - } + doLogin(); } void RemoteClient::doRequestPasswordSalt() @@ -213,21 +222,33 @@ Command_Login RemoteClient::generateCommandLogin() void RemoteClient::doLogin() { - setStatus(StatusLoggingIn); - Command_Login cmdLogin = generateCommandLogin(); - cmdLogin.set_password(password.toStdString()); + if (!password.isEmpty() && serverSupportsPasswordHash) { + // TODO store and log in using stored hashed password + if (hashedPassword.isEmpty()) { + doRequestPasswordSalt(); // ask salt to create hashedPassword, then log in + } else { + doHashedLogin(); // log in using hashed password instead + } + } else { + // TODO add setting for client to reject unhashed logins + setStatus(StatusLoggingIn); + Command_Login cmdLogin = generateCommandLogin(); + if (!password.isEmpty()) { + qWarning() << "using plain text password to log in"; + cmdLogin.set_password(password.toStdString()); + } - PendingCommand *pend = prepareSessionCommand(cmdLogin); - connect(pend, SIGNAL(finished(Response, CommandContainer, QVariant)), this, SLOT(loginResponse(Response))); - sendCommand(pend); + PendingCommand *pend = prepareSessionCommand(cmdLogin); + connect(pend, SIGNAL(finished(Response, CommandContainer, QVariant)), this, SLOT(loginResponse(Response))); + sendCommand(pend); + } } -void RemoteClient::doLogin(const QString &passwordSalt) +void RemoteClient::doHashedLogin() { setStatus(StatusLoggingIn); Command_Login cmdLogin = generateCommandLogin(); - const auto hashedPassword = PasswordHasher::computeHash(password, passwordSalt); cmdLogin.set_hashed_password(hashedPassword.toStdString()); PendingCommand *pend = prepareSessionCommand(cmdLogin); @@ -244,12 +265,13 @@ void RemoteClient::passwordSaltResponse(const Response &response) { if (response.response_code() == Response::RespOk) { const Response_PasswordSalt &resp = response.GetExtension(Response_PasswordSalt::ext); - QString salt = QString::fromStdString(resp.password_salt()); - if (salt.isEmpty()) { // the server does not recognize the user but allows them to enter unregistered - password = ""; // the password will not be used + auto passwordSalt = QString::fromStdString(resp.password_salt()); + if (passwordSalt.isEmpty()) { // the server does not recognize the user but allows them to enter unregistered + password.clear(); // the password will not be used doLogin(); } else { - doLogin(salt); + hashedPassword = PasswordHasher::computeHash(password, passwordSalt); + doHashedLogin(); } } else if (response.response_code() != Response::RespNotConnected) { emit loginError(response.response_code(), {}, 0, {}); @@ -438,6 +460,7 @@ void RemoteClient::doConnectToServer(const QString &hostname, password = _password; lastHostname = hostname; lastPort = port; + hashedPassword.clear(); connectToHost(hostname, port); setStatus(StatusConnecting); @@ -460,6 +483,7 @@ void RemoteClient::doRegisterToServer(const QString &hostname, realName = _realname; lastHostname = hostname; lastPort = port; + hashedPassword.clear(); connectToHost(hostname, port); setStatus(StatusRegistering); @@ -561,7 +585,7 @@ QString RemoteClient::getSrvClientID(const QString &_hostname) QHostAddress hostAddress = hostInfo.addresses().first(); srvClientID += hostAddress.toString(); } else { - qDebug() << "Warning: ClientID generation host lookup failure [" << hostInfo.errorString() << "]"; + qWarning() << "ClientID generation host lookup failure [" << hostInfo.errorString() << "]"; srvClientID += _hostname; } QString uniqueServerClientID = @@ -648,6 +672,7 @@ void RemoteClient::doSubmitForgotPasswordResetToServer(const QString &hostname, lastPort = port; token = _token.trimmed(); password = _newpassword; + hashedPassword.clear(); connectToHost(lastHostname, lastPort); setStatus(StatusSubmitForgotPasswordReset); diff --git a/cockatrice/src/remoteclient.h b/cockatrice/src/remoteclient.h index 5e001fb5..e87f306b 100644 --- a/cockatrice/src/remoteclient.h +++ b/cockatrice/src/remoteclient.h @@ -70,7 +70,7 @@ private slots: const QString &_realname); void doRequestPasswordSalt(); void doLogin(); - void doLogin(const QString &passwordSalt); + void doHashedLogin(); Command_Login generateCommandLogin(); void doDisconnectFromServer(); void doActivateToServer(const QString &_token); @@ -101,6 +101,7 @@ private: QWebSocket *websocket; QString lastHostname; unsigned int lastPort; + QString hashedPassword; QString getSrvClientID(const QString &_hostname); bool newMissingFeatureFound(const QString &_serversMissingFeatures); diff --git a/cockatrice/src/userinfobox.cpp b/cockatrice/src/userinfobox.cpp index 0a7d451b..4b2796f3 100644 --- a/cockatrice/src/userinfobox.cpp +++ b/cockatrice/src/userinfobox.cpp @@ -4,6 +4,8 @@ #include "dlg_edit_avatar.h" #include "dlg_edit_password.h" #include "dlg_edit_user.h" +#include "gettextwithmax.h" +#include "passwordhasher.h" #include "pb/response_get_user_info.pb.h" #include "pb/session_commands.pb.h" #include "pending_command.h" @@ -11,6 +13,8 @@ #include #include +#include +#include #include #include @@ -200,7 +204,22 @@ void UserInfoBox::actEditInternal(const Response &r) Command_AccountEdit cmd; cmd.set_real_name(dlg.getRealName().toStdString()); - cmd.set_email(dlg.getEmail().toStdString()); + if (client->getServerSupportsPasswordHash()) { + if (email != dlg.getEmail()) { + // real password is required to change email + bool ok = false; + QString password = + getTextWithMax(this, tr("Enter Password"), + tr("Password verification is required in order to change your email address"), + QLineEdit::Password, "", &ok); + if (!ok) + return; + cmd.set_password_check(password.toStdString()); + cmd.set_email(dlg.getEmail().toStdString()); + } // servers that support password hash do not require all fields to be filled anymore + } else { + cmd.set_email(dlg.getEmail().toStdString()); + } cmd.set_country(dlg.getCountry().toStdString()); PendingCommand *pend = client->prepareSessionCommand(cmd); @@ -216,9 +235,42 @@ void UserInfoBox::actPassword() if (!dlg.exec()) return; + auto oldPassword = dlg.getOldPassword(); + auto newPassword = dlg.getNewPassword(); + + if (client->getServerSupportsPasswordHash()) { + Command_RequestPasswordSalt cmd; + cmd.set_user_name(client->getUserName().toStdString()); + + PendingCommand *pend = client->prepareSessionCommand(cmd); + connect(pend, + // we need qoverload here in order to select the right version of this function + QOverload::of(&PendingCommand::finished), + this, [=](const Response &response, const CommandContainer &, const QVariant &) { + if (response.response_code() == Response::RespOk) { + changePassword(oldPassword, newPassword); + } else { + QMessageBox::critical(this, tr("Error"), + tr("An error occurred while trying to update your user information.")); + } + }); + client->sendCommand(pend); + } else { + changePassword(oldPassword, newPassword); + } +} + +void UserInfoBox::changePassword(const QString &oldPassword, const QString &newPassword) +{ Command_AccountPassword cmd; - cmd.set_old_password(dlg.getOldPassword().toStdString()); - cmd.set_new_password(dlg.getNewPassword().toStdString()); + cmd.set_old_password(oldPassword.toStdString()); + if (client->getServerSupportsPasswordHash()) { + auto passwordSalt = PasswordHasher::generateRandomSalt(); + QString hashedPassword = PasswordHasher::computeHash(newPassword, passwordSalt); + cmd.set_hashed_new_password(hashedPassword.toStdString()); + } else { + cmd.set_new_password(newPassword.toStdString()); + } PendingCommand *pend = client->prepareSessionCommand(cmd); connect(pend, SIGNAL(finished(Response, CommandContainer, QVariant)), this, @@ -254,6 +306,9 @@ void UserInfoBox::processEditResponse(const Response &r) QMessageBox::critical(this, tr("Error"), tr("This server does not permit you to update your user informations.")); break; + case Response::RespWrongPassword: + QMessageBox::critical(this, tr("Error"), tr("The entered password does not match your account.")); + break; case Response::RespInternalError: default: QMessageBox::critical(this, tr("Error"), @@ -280,7 +335,7 @@ void UserInfoBox::processPasswordResponse(const Response &r) case Response::RespInternalError: default: QMessageBox::critical(this, tr("Error"), - tr("An error occured while trying to update your user informations.")); + tr("An error occurred while trying to update your user information.")); break; } } diff --git a/cockatrice/src/userinfobox.h b/cockatrice/src/userinfobox.h index 3f662c64..937f1dda 100644 --- a/cockatrice/src/userinfobox.h +++ b/cockatrice/src/userinfobox.h @@ -49,6 +49,7 @@ public slots: private: void resizeEvent(QResizeEvent *event) override; + void changePassword(const QString &oldPassword, const QString &newPassword); }; #endif diff --git a/common/pb/session_commands.proto b/common/pb/session_commands.proto index c6a6dcbd..07bf744b 100644 --- a/common/pb/session_commands.proto +++ b/common/pb/session_commands.proto @@ -119,7 +119,7 @@ message Command_Register { // User name client wants to register required string user_name = 1; // Hashed password to be inserted into database - required string password = 2; + optional string password = 2; // Email address of the client for user validation optional string email = 3; // gender = 4; // obsolete @@ -127,6 +127,7 @@ message Command_Register { optional string country = 5; optional string real_name = 6; optional string clientid = 7; + optional string hashed_password = 8; } // User wants to activate an account @@ -149,6 +150,7 @@ message Command_AccountEdit { optional string email = 2; // gender = 3; // obsolete optional string country = 4; + optional string password_check = 100; // password is required to change sensitive information } message Command_AccountImage { @@ -164,6 +166,8 @@ message Command_AccountPassword { } optional string old_password = 1; optional string new_password = 2; + // optional string hashed_old_password = 3; // we don't want users to steal hashed passwords and change them + optional string hashed_new_password = 4; } message Command_ForgotPasswordRequest { @@ -182,6 +186,7 @@ message Command_ForgotPasswordReset { optional string clientid = 2; optional string token = 3; optional string new_password = 4; + optional string hashed_new_password = 5; } message Command_ForgotPasswordChallenge { diff --git a/common/server_database_interface.h b/common/server_database_interface.h index bac1e520..4f404f7f 100644 --- a/common/server_database_interface.h +++ b/common/server_database_interface.h @@ -115,6 +115,7 @@ public: virtual bool registerUser(const QString & /* userName */, const QString & /* realName */, const QString & /* password */, + bool /* passwordNeedsHash */, const QString & /* emailAddress */, const QString & /* country */, bool /* active = false */) @@ -151,10 +152,16 @@ public: { return 0; }; + virtual bool + changeUserPassword(const QString & /* user */, const QString & /* password */, bool /* passwordNeedsHash */) + { + return false; + }; virtual bool changeUserPassword(const QString & /* user */, const QString & /* oldPassword */, + bool /* oldPasswordNeedsHash */, const QString & /* newPassword */, - const bool & /* force */) + bool /* newPasswordNeedsHash */) { return false; }; diff --git a/common/server_protocolhandler.cpp b/common/server_protocolhandler.cpp index 1ca934ac..2daa1118 100644 --- a/common/server_protocolhandler.cpp +++ b/common/server_protocolhandler.cpp @@ -31,7 +31,7 @@ Server_ProtocolHandler::Server_ProtocolHandler(Server *_server, Server_DatabaseInterface *_databaseInterface, QObject *parent) : QObject(parent), Server_AbstractUserInterface(_server), deleted(false), databaseInterface(_databaseInterface), - authState(NotLoggedIn), acceptsUserListChanges(false), acceptsRoomListChanges(false), + authState(NotLoggedIn), usingRealPassword(false), acceptsUserListChanges(false), acceptsRoomListChanges(false), idleClientWarningSent(false), timeRunning(0), lastDataReceived(0), lastActionReceived(0) { @@ -447,8 +447,12 @@ Response::ResponseCode Server_ProtocolHandler::cmdLogin(const Command_Login &cmd QString password; bool needsHash = false; if (cmd.has_password()) { - password = nameFromStdString(cmd.password()); + if (cmd.password().length() > MAX_NAME_LENGTH) + return Response::RespWrongPassword; + password = QString::fromStdString(cmd.password()); needsHash = true; + } else if (cmd.hashed_password().length() > MAX_NAME_LENGTH) { + return Response::RespContextError; } else { password = nameFromStdString(cmd.hashed_password()); } @@ -515,6 +519,7 @@ Response::ResponseCode Server_ProtocolHandler::cmdLogin(const Command_Login &cmd return Response::RespAccountNotActivated; default: authState = res; + usingRealPassword = needsHash; } // limit the number of non-privileged users that can connect to the server based on configuration settings @@ -791,6 +796,8 @@ Server_ProtocolHandler::cmdCreateGame(const Command_CreateGame &cmd, Server_Room const int gameId = databaseInterface->getNextGameId(); if (gameId == -1) return Response::RespInternalError; + if (cmd.password().length() > MAX_NAME_LENGTH) + return Response::RespContextError; auto level = userInfo->user_level(); bool isJudge = level & ServerInfo_User::IsJudge; @@ -818,10 +825,10 @@ Server_ProtocolHandler::cmdCreateGame(const Command_CreateGame &cmd, Server_Room // When server doesn't permit registered users to exist, do not honor only-reg setting bool onlyRegisteredUsers = cmd.only_registered() && (server->permitUnregisteredUsers()); - Server_Game *game = new Server_Game(copyUserInfo(false), gameId, description, nameFromStdString(cmd.password()), - cmd.max_players(), gameTypes, cmd.only_buddies(), onlyRegisteredUsers, - cmd.spectators_allowed(), cmd.spectators_need_password(), - cmd.spectators_can_talk(), cmd.spectators_see_everything(), room); + Server_Game *game = new Server_Game( + copyUserInfo(false), gameId, description, QString::fromStdString(cmd.password()), cmd.max_players(), gameTypes, + cmd.only_buddies(), onlyRegisteredUsers, cmd.spectators_allowed(), cmd.spectators_need_password(), + cmd.spectators_can_talk(), cmd.spectators_see_everything(), room); game->addPlayer(this, rc, asSpectator, asJudge, false); room->addGame(game); diff --git a/common/server_protocolhandler.h b/common/server_protocolhandler.h index 78f45594..760ca273 100644 --- a/common/server_protocolhandler.h +++ b/common/server_protocolhandler.h @@ -52,6 +52,7 @@ protected: bool deleted; Server_DatabaseInterface *databaseInterface; AuthenticationResult authState; + bool usingRealPassword; bool acceptsUserListChanges; bool acceptsRoomListChanges; bool idleClientWarningSent; diff --git a/common/server_room.cpp b/common/server_room.cpp index 84428690..e9f5c20f 100644 --- a/common/server_room.cpp +++ b/common/server_room.cpp @@ -241,6 +241,8 @@ Response::ResponseCode Server_Room::processJoinGameCommand(const Command_JoinGam ResponseContainer &rc, Server_AbstractUserInterface *userInterface) { + if (cmd.password().length() > MAX_NAME_LENGTH) + return Response::RespWrongPassword; // This function is called from the Server thread and from the S_PH thread. // server->roomsMutex is always locked. @@ -265,8 +267,9 @@ Response::ResponseCode Server_Room::processJoinGameCommand(const Command_JoinGam QMutexLocker gameLocker(&game->gameMutex); - Response::ResponseCode result = game->checkJoin(userInterface->getUserInfo(), nameFromStdString(cmd.password()), - cmd.spectator(), cmd.override_restrictions(), cmd.join_as_judge()); + Response::ResponseCode result = + game->checkJoin(userInterface->getUserInfo(), QString::fromStdString(cmd.password()), cmd.spectator(), + cmd.override_restrictions(), cmd.join_as_judge()); if (result == Response::RespOk) game->addPlayer(userInterface, rc, cmd.spectator(), cmd.join_as_judge()); diff --git a/servatrice/src/servatrice_database_interface.cpp b/servatrice/src/servatrice_database_interface.cpp index 7e591bcc..7c602c39 100644 --- a/servatrice/src/servatrice_database_interface.cpp +++ b/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) diff --git a/servatrice/src/servatrice_database_interface.h b/servatrice/src/servatrice_database_interface.h index 7428fd55..2a4e778a 100644 --- a/servatrice/src/servatrice_database_interface.h +++ b/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 getUserBanHistory(const QString userName); bool addWarning(const QString userName, const QString adminName, const QString warningReason, const QString clientID); diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index bdfe6d99..4fd68416 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/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);