From 4c273040474eb455e1f1e7c2cd352611b50d71c4 Mon Sep 17 00:00:00 2001 From: Fabio Bas Date: Mon, 29 Jun 2015 23:35:27 +0200 Subject: [PATCH 1/5] publish username rules in registration failure --- cockatrice/src/window_main.cpp | 35 ++++++++++++++++++- common/server_database_interface.h | 2 +- .../src/servatrice_database_interface.cpp | 22 +++++++----- .../src/servatrice_database_interface.h | 2 +- servatrice/src/serversocketinterface.cpp | 8 ++++- 5 files changed, 57 insertions(+), 12 deletions(-) diff --git a/cockatrice/src/window_main.cpp b/cockatrice/src/window_main.cpp index 3a0d4044..de65d4d1 100644 --- a/cockatrice/src/window_main.cpp +++ b/cockatrice/src/window_main.cpp @@ -30,6 +30,10 @@ #include #include #include +#if QT_VERSION < 0x050000 + // for Qt::escape() + #include +#endif #include "main.h" #include "window_main.h" @@ -359,8 +363,37 @@ void MainWindow::registerError(Response::ResponseCode r, QString reasonStr, quin break; } case Response::RespUsernameInvalid: - QMessageBox::critical(this, tr("Error"), tr("Invalid username.\nYou may only use A-Z, a-z, 0-9, _, ., and - in your username.")); + { + QString error = tr("Invalid username.") + "
"; + QStringList rules = reasonStr.split(QChar('|')); + if (rules.size() == 7) + { + error += tr("The username must respect these rules:") + "
    " + + "
  • " + tr("length between %1 and %2 characters").arg(rules.at(0)).arg(rules.at(1)) + "
    • "; + if(rules.at(2).toInt() > 0) + error += "
  • " + tr("it can contain lowercase characters") + "
    • "; + if(rules.at(3).toInt() > 0) + error += "
  • " + tr("it can contain uppercase characters") + "
    • "; + if(rules.at(4).toInt() > 0) + error += "
  • " + tr("it can contain numeric characters") + "
    • "; + if(rules.at(6).size() > 0) + error += "
  • " + tr("it can contain the following punctuation: %1").arg( +#if QT_VERSION < 0x050000 + Qt::escape(rules.at(6)) +#else + rules.at(6).toHtmlEscaped() +#endif + ) + "
    • "; + if(rules.at(5).toInt() > 0) + error += "
  • " + tr("the first character can't be a punctuation") + "
    • "; + error += "
"; + } else { + error += tr("You may only use A-Z, a-z, 0-9, _, ., and - in your username."); + } + + QMessageBox::critical(this, tr("Error"), error); break; + } case Response::RespRegistrationFailed: QMessageBox::critical(this, tr("Error"), tr("Registration failed for a technical problem on the server.")); break; diff --git a/common/server_database_interface.h b/common/server_database_interface.h index d809b1ff..60d6c056 100644 --- a/common/server_database_interface.h +++ b/common/server_database_interface.h @@ -25,7 +25,7 @@ public: virtual DeckList *getDeckFromDatabase(int /* deckId */, int /* userId */) { return 0; } virtual qint64 startSession(const QString & /* userName */, const QString & /* address */) { return 0; } - virtual bool usernameIsValid(const QString & /*userName */) { return true; }; + virtual bool usernameIsValid(const QString & /*userName */, QString & /* error */) { return true; }; public slots: virtual void endSession(qint64 /* sessionId */ ) { } public: diff --git a/servatrice/src/servatrice_database_interface.cpp b/servatrice/src/servatrice_database_interface.cpp index 3a81c9ba..7aa5fa06 100644 --- a/servatrice/src/servatrice_database_interface.cpp +++ b/servatrice/src/servatrice_database_interface.cpp @@ -118,24 +118,29 @@ bool Servatrice_DatabaseInterface::execSqlQuery(QSqlQuery *query) return false; } -bool Servatrice_DatabaseInterface::usernameIsValid(const QString &user) +bool Servatrice_DatabaseInterface::usernameIsValid(const QString &user, QString & error) { - int maxNameLength = settingsCache->value("users/maxnamelength", 12).toInt(); int minNameLength = settingsCache->value("users/minnamelength", 6).toInt(); + int maxNameLength = settingsCache->value("users/maxnamelength", 12).toInt(); + bool allowLowercase = settingsCache->value("users/allowlowercase", true).toBool(); + bool allowUppercase = settingsCache->value("users/allowuppercase", true).toBool(); + bool allowNumerics = settingsCache->value("users/allownumerics", true).toBool(); + bool allowPunctuationPrefix = settingsCache->value("users/allowpunctuationprefix", false).toBool(); + QString allowedPunctuation = settingsCache->value("users/allowedpunctuation", "_").toString(); + error = QString("%1|%2|%3|%4|%5|%6|%7").arg(minNameLength).arg(maxNameLength).arg(allowLowercase).arg(allowUppercase).arg(allowNumerics).arg(allowPunctuationPrefix).arg(allowedPunctuation); + if (user.length() < minNameLength || user.length() > maxNameLength) return false; - bool allowPunctuationPrefix = settingsCache->value("users/allowpunctuationprefix", false).toBool(); - QString allowedPunctuation = settingsCache->value("users/allowedpunctuation", "_").toString(); if (!allowPunctuationPrefix && allowedPunctuation.contains(user.at(0))) return false; QString regEx("["); - if (settingsCache->value("users/allowlowercase", true).toBool()) + if (allowLowercase) regEx.append("a-z"); - if (settingsCache->value("users/allowuppercase", true).toBool()) + if (allowUppercase) regEx.append("A-Z"); - if(settingsCache->value("users/allownumerics", true).toBool()) + if(allowNumerics) regEx.append("0-9"); regEx.append(QRegExp::escape(allowedPunctuation)); regEx.append("]+"); @@ -242,7 +247,8 @@ AuthenticationResult Servatrice_DatabaseInterface::checkUserPassword(Server_Prot if (!checkSql()) return UnknownUser; - if (!usernameIsValid(user)) + QString error; + if (!usernameIsValid(user, error)) return UsernameInvalid; if (checkUserIsBanned(handler->getAddress(), user, reasonStr, banSecondsLeft)) diff --git a/servatrice/src/servatrice_database_interface.h b/servatrice/src/servatrice_database_interface.h index 10770691..e5aae344 100644 --- a/servatrice/src/servatrice_database_interface.h +++ b/servatrice/src/servatrice_database_interface.h @@ -62,7 +62,7 @@ public: void lockSessionTables(); void unlockSessionTables(); bool userSessionExists(const QString &userName); - bool usernameIsValid(const QString &user); + bool usernameIsValid(const QString &user, QString & error); bool checkUserIsBanned(const QString &ipAddress, const QString &userName, QString &banReason, int &banSecondsRemaining); bool getRequireRegistration(); diff --git a/servatrice/src/serversocketinterface.cpp b/servatrice/src/serversocketinterface.cpp index a2fa26b2..13198ef7 100644 --- a/servatrice/src/serversocketinterface.cpp +++ b/servatrice/src/serversocketinterface.cpp @@ -788,8 +788,14 @@ Response::ResponseCode ServerSocketInterface::cmdRegisterAccount(const Command_R } // TODO: Move this method outside of the db interface - if (!sqlInterface->usernameIsValid(userName)) + QString errorString; + if (!sqlInterface->usernameIsValid(userName, errorString)) + { + Response_Register *re = new Response_Register; + re->set_denied_reason_str(errorString.toStdString()); + rc.setResponseExtension(re); return Response::RespUsernameInvalid; + } if(sqlInterface->userExists(userName)) return Response::RespUserAlreadyExists; From 02dcaff35633c9d588114663d7f0f0b77e3494f2 Mon Sep 17 00:00:00 2001 From: Fabio Bas Date: Tue, 30 Jun 2015 18:44:40 +0200 Subject: [PATCH 2/5] Publish username rules in login failure, too --- cockatrice/src/window_main.cpp | 73 ++++++++++--------- cockatrice/src/window_main.h | 1 + common/server_protocolhandler.cpp | 7 +- .../src/servatrice_database_interface.cpp | 3 +- 4 files changed, 48 insertions(+), 36 deletions(-) diff --git a/cockatrice/src/window_main.cpp b/cockatrice/src/window_main.cpp index de65d4d1..ce85f37f 100644 --- a/cockatrice/src/window_main.cpp +++ b/cockatrice/src/window_main.cpp @@ -83,7 +83,7 @@ void MainWindow::processConnectionClosedEvent(const Event_ConnectionClosed &even break; } case Event_ConnectionClosed::SERVER_SHUTDOWN: reasonStr = tr("Scheduled server shutdown."); break; - case Event_ConnectionClosed::USERNAMEINVALID: reasonStr = tr("Invalid username.\nYou may only use A-Z, a-z, 0-9, _, ., and - in your username."); break; + case Event_ConnectionClosed::USERNAMEINVALID: reasonStr = tr("Invalid username."); break; default: reasonStr = QString::fromStdString(event.reason_str()); } QMessageBox::critical(this, tr("Connection closed"), tr("The server has terminated your connection.\nReason: %1").arg(reasonStr)); @@ -306,9 +306,12 @@ void MainWindow::loginError(Response::ResponseCode r, QString reasonStr, quint32 QMessageBox::critical(this, tr("Error"), bannedStr); break; } - case Response::RespUsernameInvalid: - QMessageBox::critical(this, tr("Error"), tr("Invalid username.\nYou may only use A-Z, a-z, 0-9, _, ., and - in your username.")); + case Response::RespUsernameInvalid: { + QString errorStr; + extractInvalidUsernameMessage(reasonStr, errorStr); + QMessageBox::critical(this, tr("Error"), errorStr); break; + } case Response::RespRegistrationRequired: if (QMessageBox::question(this, tr("Error"), tr("This server requires user registration. Do you want to register now?"), QMessageBox::Yes | QMessageBox::No) == QMessageBox::Yes) { actRegister(); @@ -332,6 +335,36 @@ void MainWindow::loginError(Response::ResponseCode r, QString reasonStr, quint32 actConnect(); } +void MainWindow::extractInvalidUsernameMessage(QString & in, QString & out) +{ + out = tr("Invalid username.") + "
"; + QStringList rules = in.split(QChar('|')); + if (rules.size() == 7) + { + out += tr("The username must respect these rules:") + "
    " + + "
  • " + tr("length between %1 and %2 characters").arg(rules.at(0)).arg(rules.at(1)) + "
    • "; + if(rules.at(2).toInt() > 0) + out += "
  • " + tr("it can contain lowercase characters") + "
    • "; + if(rules.at(3).toInt() > 0) + out += "
  • " + tr("it can contain uppercase characters") + "
    • "; + if(rules.at(4).toInt() > 0) + out += "
  • " + tr("it can contain numeric characters") + "
    • "; + if(rules.at(6).size() > 0) + out += "
  • " + tr("it can contain the following punctuation: %1").arg( +#if QT_VERSION < 0x050000 + Qt::escape(rules.at(6)) +#else + rules.at(6).toHtmlEscaped() +#endif + ) + "
    • "; + if(rules.at(5).toInt() > 0) + out += "
  • " + tr("the first character can't be a punctuation") + "
    • "; + out += "
"; + } else { + out += tr("You may only use A-Z, a-z, 0-9, _, ., and - in your username."); + } +} + void MainWindow::registerError(Response::ResponseCode r, QString reasonStr, quint32 endTime) { switch (r) { @@ -362,36 +395,10 @@ void MainWindow::registerError(Response::ResponseCode r, QString reasonStr, quin QMessageBox::critical(this, tr("Error"), bannedStr); break; } - case Response::RespUsernameInvalid: - { - QString error = tr("Invalid username.") + "
"; - QStringList rules = reasonStr.split(QChar('|')); - if (rules.size() == 7) - { - error += tr("The username must respect these rules:") + "
    " - + "
  • " + tr("length between %1 and %2 characters").arg(rules.at(0)).arg(rules.at(1)) + "
    • "; - if(rules.at(2).toInt() > 0) - error += "
  • " + tr("it can contain lowercase characters") + "
    • "; - if(rules.at(3).toInt() > 0) - error += "
  • " + tr("it can contain uppercase characters") + "
    • "; - if(rules.at(4).toInt() > 0) - error += "
  • " + tr("it can contain numeric characters") + "
    • "; - if(rules.at(6).size() > 0) - error += "
  • " + tr("it can contain the following punctuation: %1").arg( -#if QT_VERSION < 0x050000 - Qt::escape(rules.at(6)) -#else - rules.at(6).toHtmlEscaped() -#endif - ) + "
    • "; - if(rules.at(5).toInt() > 0) - error += "
  • " + tr("the first character can't be a punctuation") + "
    • "; - error += "
"; - } else { - error += tr("You may only use A-Z, a-z, 0-9, _, ., and - in your username."); - } - - QMessageBox::critical(this, tr("Error"), error); + case Response::RespUsernameInvalid: { + QString errorStr; + extractInvalidUsernameMessage(reasonStr, errorStr); + QMessageBox::critical(this, tr("Error"), errorStr); break; } case Response::RespRegistrationFailed: diff --git a/cockatrice/src/window_main.h b/cockatrice/src/window_main.h index 57b9e780..9bde26b1 100644 --- a/cockatrice/src/window_main.h +++ b/cockatrice/src/window_main.h @@ -109,6 +109,7 @@ public: protected: void closeEvent(QCloseEvent *event); void changeEvent(QEvent *event); + void extractInvalidUsernameMessage(QString & in, QString & out); }; #endif diff --git a/common/server_protocolhandler.cpp b/common/server_protocolhandler.cpp index 46e2bb83..7df87189 100644 --- a/common/server_protocolhandler.cpp +++ b/common/server_protocolhandler.cpp @@ -388,7 +388,12 @@ Response::ResponseCode Server_ProtocolHandler::cmdLogin(const Command_Login &cmd } case NotLoggedIn: return Response::RespWrongPassword; case WouldOverwriteOldSession: return Response::RespWouldOverwriteOldSession; - case UsernameInvalid: return Response::RespUsernameInvalid; + case UsernameInvalid: { + Response_Login *re = new Response_Login; + re->set_denied_reason_str(reasonStr.toStdString()); + rc.setResponseExtension(re); + return Response::RespUsernameInvalid; + } case RegistrationRequired: return Response::RespRegistrationRequired; case UserIsInactive: return Response::RespAccountNotActivated; default: authState = res; diff --git a/servatrice/src/servatrice_database_interface.cpp b/servatrice/src/servatrice_database_interface.cpp index 7aa5fa06..b48fb738 100644 --- a/servatrice/src/servatrice_database_interface.cpp +++ b/servatrice/src/servatrice_database_interface.cpp @@ -247,8 +247,7 @@ AuthenticationResult Servatrice_DatabaseInterface::checkUserPassword(Server_Prot if (!checkSql()) return UnknownUser; - QString error; - if (!usernameIsValid(user, error)) + if (!usernameIsValid(user, reasonStr)) return UsernameInvalid; if (checkUserIsBanned(handler->getAddress(), user, reasonStr, banSecondsLeft)) From 6b54d8cbfdec6d9667b6d82e0ac346e486163ee5 Mon Sep 17 00:00:00 2001 From: Fabio Bas Date: Tue, 30 Jun 2015 21:40:39 +0200 Subject: [PATCH 3/5] Fixed issues --- cockatrice/src/window_main.cpp | 16 +++++++--------- cockatrice/src/window_main.h | 2 +- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/cockatrice/src/window_main.cpp b/cockatrice/src/window_main.cpp index ce85f37f..6e64a06f 100644 --- a/cockatrice/src/window_main.cpp +++ b/cockatrice/src/window_main.cpp @@ -307,9 +307,7 @@ void MainWindow::loginError(Response::ResponseCode r, QString reasonStr, quint32 break; } case Response::RespUsernameInvalid: { - QString errorStr; - extractInvalidUsernameMessage(reasonStr, errorStr); - QMessageBox::critical(this, tr("Error"), errorStr); + QMessageBox::critical(this, tr("Error"), extractInvalidUsernameMessage(reasonStr)); break; } case Response::RespRegistrationRequired: @@ -335,9 +333,9 @@ void MainWindow::loginError(Response::ResponseCode r, QString reasonStr, quint32 actConnect(); } -void MainWindow::extractInvalidUsernameMessage(QString & in, QString & out) +QString MainWindow::extractInvalidUsernameMessage(QString & in) { - out = tr("Invalid username.") + "
"; + QString out = tr("Invalid username.") + "
"; QStringList rules = in.split(QChar('|')); if (rules.size() == 7) { @@ -357,12 +355,14 @@ void MainWindow::extractInvalidUsernameMessage(QString & in, QString & out) rules.at(6).toHtmlEscaped() #endif ) + ""; - if(rules.at(5).toInt() > 0) + if(rules.at(5).toInt() == 0) out += "
  • " + tr("the first character can't be a punctuation") + "
    • "; out += ""; } else { out += tr("You may only use A-Z, a-z, 0-9, _, ., and - in your username."); } + + return out; } void MainWindow::registerError(Response::ResponseCode r, QString reasonStr, quint32 endTime) @@ -396,9 +396,7 @@ void MainWindow::registerError(Response::ResponseCode r, QString reasonStr, quin break; } case Response::RespUsernameInvalid: { - QString errorStr; - extractInvalidUsernameMessage(reasonStr, errorStr); - QMessageBox::critical(this, tr("Error"), errorStr); + QMessageBox::critical(this, tr("Error"), extractInvalidUsernameMessage(reasonStr)); break; } case Response::RespRegistrationFailed: diff --git a/cockatrice/src/window_main.h b/cockatrice/src/window_main.h index 9bde26b1..5c319318 100644 --- a/cockatrice/src/window_main.h +++ b/cockatrice/src/window_main.h @@ -109,7 +109,7 @@ public: protected: void closeEvent(QCloseEvent *event); void changeEvent(QEvent *event); - void extractInvalidUsernameMessage(QString & in, QString & out); + QString extractInvalidUsernameMessage(QString & in); }; #endif From 1d210e57bd3707f01f54be512b042b45fc15d508 Mon Sep 17 00:00:00 2001 From: Zach H Date: Fri, 3 Jul 2015 23:54:14 -0400 Subject: [PATCH 4/5] show all rules --- cockatrice/src/window_main.cpp | 41 ++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/cockatrice/src/window_main.cpp b/cockatrice/src/window_main.cpp index 6e64a06f..9e91af04 100644 --- a/cockatrice/src/window_main.cpp +++ b/cockatrice/src/window_main.cpp @@ -339,26 +339,29 @@ QString MainWindow::extractInvalidUsernameMessage(QString & in) QStringList rules = in.split(QChar('|')); if (rules.size() == 7) { - out += tr("The username must respect these rules:") + "
      " - + "
    • " + tr("length between %1 and %2 characters").arg(rules.at(0)).arg(rules.at(1)) + "
      • "; - if(rules.at(2).toInt() > 0) - out += "
    • " + tr("it can contain lowercase characters") + "
      • "; - if(rules.at(3).toInt() > 0) - out += "
    • " + tr("it can contain uppercase characters") + "
      • "; - if(rules.at(4).toInt() > 0) - out += "
    • " + tr("it can contain numeric characters") + "
      • "; - if(rules.at(6).size() > 0) - out += "
    • " + tr("it can contain the following punctuation: %1").arg( -#if QT_VERSION < 0x050000 - Qt::escape(rules.at(6)) -#else - rules.at(6).toHtmlEscaped() -#endif - ) + "
      • "; - if(rules.at(5).toInt() == 0) - out += "
    • " + tr("the first character can't be a punctuation") + "
      • "; + out += tr("Your username must respect these rules:") + "
        "; + + out += "
      • " + tr("is %1 - %2 characters long").arg(rules.at(0)).arg(rules.at(1)) + "
        • "; + out += "
      • " + tr("can %1 contain lowercase characters").arg((rules.at(2).toInt() > 0) ? "" : tr("NOT")) + "
        • "; + out += "
      • " + tr("can %1 contain uppercase characters").arg((rules.at(3).toInt() > 0) ? "" : tr("NOT")) + "
        • "; + out += "
      • " + tr("can %1 contain numeric characters").arg((rules.at(4).toInt() > 0) ? "" : tr("NOT")) + "
        • "; + + if (rules.at(6).size() > 0) + { + out += "
      • " + tr("can contain the following punctuation: %1").arg( + #if QT_VERSION < 0x050000 + Qt::escape(rules.at(6)) + #else + rules.at(6).toHtmlEscaped() + #endif + ) + "
        • "; + } + + out += "
      • " + tr("first character can %1 be a punctuation mark").arg((rules.at(5).toInt() == 0) ? "" : tr("NOT")) + "
        • "; out += "
      "; - } else { + } + else + { out += tr("You may only use A-Z, a-z, 0-9, _, ., and - in your username."); } From 1f9b3ed28f72ab00993b739c2f391730f04a3d69 Mon Sep 17 00:00:00 2001 From: Zach H Date: Sat, 4 Jul 2015 18:52:50 -0400 Subject: [PATCH 5/5] minor fix --- cockatrice/src/window_main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cockatrice/src/window_main.cpp b/cockatrice/src/window_main.cpp index 9e91af04..8515d64b 100644 --- a/cockatrice/src/window_main.cpp +++ b/cockatrice/src/window_main.cpp @@ -357,7 +357,7 @@ QString MainWindow::extractInvalidUsernameMessage(QString & in) ) + ""; } - out += "
    • " + tr("first character can %1 be a punctuation mark").arg((rules.at(5).toInt() == 0) ? "" : tr("NOT")) + "
      • "; + out += "
    • " + tr("first character can %1 be a punctuation mark").arg((rules.at(5).toInt() > 0) ? "" : tr("NOT")) + "
      • "; out += "
    "; } else