Add scope verification and tests

libnymea-core/usermanager/usermanager.cpp

@@ -96,7 +96,7 @@ UserManager::UserManager(const QString &dbName, QObject *parent):
 
     if (!initDB()) {
         qCWarning(dcUserManager()) << "Error initializing user database. Trying to correct it.";
-        if (QFileInfo(m_db.databaseName()).exists()) {
+        if (QFileInfo::exists(m_db.databaseName())) {
             rotate(m_db.databaseName());
             if (!initDB()) {
                 qCWarning(dcUserManager()) << "Error fixing user database. Giving up. Users can't be stored.";
@@ -141,6 +141,7 @@ UserInfoList UserManager::users() const
         qCWarning(dcUserManager()) << "Unable to execute SQL query" << userQuery << m_db.lastError().databaseText() << m_db.lastError().driverText();
         return users;
     }
+
     while (resultQuery.next()) {
         UserInfo info = UserInfo(resultQuery.value("username").toString());
         info.setEmail(resultQuery.value("email").toString());
@@ -164,6 +165,11 @@ UserManager::UserError UserManager::createUser(const QString &username, const QS
         return UserErrorBadPassword;
     }
 
+    if (!validateScopes(scopes)) {
+        // The method warns about he specific validation
+        return UserErrorInconsistantScopes;
+    }
+
     QSqlQuery checkForDuplicateUserQuery(m_db);
     checkForDuplicateUserQuery.prepare("SELECT * FROM users WHERE lower(username) = ?;");
     // Note: We're using toLower() on the username mainly for the reason that in old versions the username used to be an email address
@@ -223,9 +229,9 @@ UserManager::UserError UserManager::changePassword(const QString &username, cons
     QByteArray salt = QUuid::createUuid().toString().remove(QRegularExpression("[{}]")).toUtf8();
     QByteArray hashedPassword = QCryptographicHash::hash(QString(newPassword + salt).toUtf8(), QCryptographicHash::Sha512).toBase64();
     QString updatePasswordQueryString = QString("UPDATE users SET password = \"%1\", salt = \"%2\" WHERE lower(username) = \"%3\";")
-            .arg(QString::fromUtf8(hashedPassword))
+                                            .arg(QString::fromUtf8(hashedPassword))
-            .arg(QString::fromUtf8(salt))
+                                            .arg(QString::fromUtf8(salt))
-            .arg(username.toLower());
+                                            .arg(username.toLower());
 
     QSqlQuery updatePasswordQuery(m_db);
     if (!updatePasswordQuery.exec(updatePasswordQueryString)) {
@@ -267,6 +273,12 @@ UserManager::UserError UserManager::removeUser(const QString &username)
 
 UserManager::UserError UserManager::setUserScopes(const QString &username, Types::PermissionScopes scopes)
 {
+    if (!validateScopes(scopes)) {
+        // The method warns about he specific validation
+        return UserErrorInconsistantScopes;
+    }
+
+
     QString scopesString = Types::scopesToStringList(scopes).join(',');
     QSqlQuery setScopesQuery(m_db);
     setScopesQuery.prepare("UPDATE users SET scopes = ? WHERE username = ?");
@@ -332,11 +344,11 @@ QByteArray UserManager::authenticate(const QString &username, const QString &pas
 
     QByteArray token = QCryptographicHash::hash(QUuid::createUuid().toByteArray(), QCryptographicHash::Sha256).toBase64();
     QString storeTokenQueryString = QString("INSERT INTO tokens(id, username, token, creationdate, devicename) VALUES(\"%1\", \"%2\", \"%3\", \"%4\", \"%5\");")
-            .arg(QUuid::createUuid().toString())
+                                        .arg(QUuid::createUuid().toString())
-            .arg(username.toLower())
+                                        .arg(username.toLower())
-            .arg(QString::fromUtf8(token))
+                                        .arg(QString::fromUtf8(token))
-            .arg(NymeaCore::instance()->timeManager()->currentDateTime().toString("yyyy-MM-dd hh:mm:ss"))
+                                        .arg(NymeaCore::instance()->timeManager()->currentDateTime().toString("yyyy-MM-dd hh:mm:ss"))
-            .arg(deviceName);
+                                        .arg(deviceName);
 
     QSqlQuery storeTokenQuery(m_db);
     if (!storeTokenQuery.exec(storeTokenQueryString)) {
@@ -391,9 +403,8 @@ void UserManager::cancelPushButtonAuth(int transactionId)
 */
 UserInfo UserManager::userInfo(const QString &username) const
 {
-
     QString getUserQueryString = QString("SELECT * FROM users WHERE lower(username) = \"%1\";")
-            .arg(username);
+    .arg(username);
 
     QSqlQuery getUserQuery(m_db);
     if (!getUserQuery.exec(getUserQueryString)) {
@@ -444,7 +455,7 @@ TokenInfo UserManager::tokenInfo(const QByteArray &token) const
     }
 
     QString getTokenQueryString = QString("SELECT id, username, creationdate, deviceName FROM tokens WHERE token = \"%1\";")
-            .arg(QString::fromUtf8(token));
+                                      .arg(QString::fromUtf8(token));
 
     QSqlQuery getTokenQuery(m_db);
     if (!getTokenQuery.exec(getTokenQueryString)) {
@@ -467,7 +478,7 @@ TokenInfo UserManager::tokenInfo(const QUuid &tokenId) const
 {
 
     QString getTokenQueryString = QString("SELECT id, username, creationdate, deviceName FROM tokens WHERE id = \"%1\";")
-            .arg(tokenId.toString());
+    .arg(tokenId.toString());
 
     QSqlQuery getTokenQuery(m_db);
     if (!getTokenQuery.exec(getTokenQueryString)) {
@@ -490,7 +501,7 @@ TokenInfo UserManager::tokenInfo(const QUuid &tokenId) const
 UserManager::UserError UserManager::removeToken(const QUuid &tokenId)
 {
     QString removeTokenQueryString = QString("DELETE FROM tokens WHERE id = \"%1\";")
-            .arg(tokenId.toString());
+    .arg(tokenId.toString());
 
     QSqlQuery removeTokenQuery(m_db);
     if (!removeTokenQuery.exec(removeTokenQueryString)) {
@@ -519,7 +530,7 @@ bool UserManager::verifyToken(const QByteArray &token)
         return false;
     }
     QString getTokenQueryString = QString("SELECT * FROM tokens WHERE token = \"%1\";")
-            .arg(QString::fromUtf8(token));
+                                      .arg(QString::fromUtf8(token));
 
     QSqlQuery getTokenQuery(m_db);
     if (!getTokenQuery.exec(getTokenQueryString)) {
@@ -738,6 +749,47 @@ bool UserManager::validateToken(const QByteArray &token) const
     return validator.match(token).hasMatch();
 }
 
+bool UserManager::validateScopes(Types::PermissionScopes scopes) const
+{
+    if (scopes.testFlag(Types::PermissionScopeAdmin) || scopes == Types::PermissionScopeNone || scopes == Types::PermissionScopeControlThings)
+        return true;
+
+    if (scopes.testFlag(Types::PermissionScopeConfigureThings)) {
+        if (!scopes.testFlag(Types::PermissionScopeControlThings) ||
+            !scopes.testFlag(Types::PermissionScopeAccessAllThings)) {
+            qCWarning(dcUserManager()) << "Invalid scopes combination. If a user can configure things he must have access to all things and must be able to control them.";
+            return false;
+        }
+    }
+
+    // Note: if access to all things, there are no restrictions
+    if (!scopes.testFlag(Types::PermissionScopeAccessAllThings)) {
+        if (scopes.testFlag(Types::PermissionScopeControlThings) ||
+            scopes.testFlag(Types::PermissionScopeConfigureRules)||
+            scopes.testFlag(Types::PermissionScopeExecuteRules)) {
+            qCWarning(dcUserManager()) << "Invalid scopes combination. If a user has not access to all things, he can not configure them or create/execute rules.";
+            return false;
+        }
+    }
+
+    if (scopes.testFlag(Types::PermissionScopeExecuteRules)) {
+        if (!scopes.testFlag(Types::PermissionScopeAccessAllThings)) {
+            qCWarning(dcUserManager()) << "Invalid scopes combination. If a user can execute rules, he must have access to all things.";
+            return false;
+        }
+    }
+
+    if (scopes.testFlag(Types::PermissionScopeConfigureRules)) {
+        if (!scopes.testFlag(Types::PermissionScopeAccessAllThings) ||
+            !scopes.testFlag(Types::PermissionScopeExecuteRules)) {
+            qCWarning(dcUserManager()) << "Invalid scopes combination. If a user can create rules, he must have access to all things and be able to execute them.";
+            return false;
+        }
+    }
+
+    return true;
+}
+
 void UserManager::dumpDBError(const QString &message)
 {
     qCCritical(dcUserManager) << message << "Driver error:" << m_db.lastError().driverText() << "Database error:" << m_db.lastError().databaseText();
@@ -770,11 +822,11 @@ void UserManager::onPushButtonPressed()
 
     QByteArray token = QCryptographicHash::hash(QUuid::createUuid().toByteArray(), QCryptographicHash::Sha256).toBase64();
     QString storeTokenQueryString = QString("INSERT INTO tokens(id, username, token, creationdate, devicename) VALUES(\"%1\", \"%2\", \"%3\", \"%4\", \"%5\");")
-            .arg(QUuid::createUuid().toString())
+                                        .arg(QUuid::createUuid().toString())
-            .arg("")
+                                        .arg("")
-            .arg(QString::fromUtf8(token))
+                                        .arg(QString::fromUtf8(token))
-            .arg(NymeaCore::instance()->timeManager()->currentDateTime().toString("yyyy-MM-dd hh:mm:ss"))
+                                        .arg(NymeaCore::instance()->timeManager()->currentDateTime().toString("yyyy-MM-dd hh:mm:ss"))
-            .arg(m_pushButtonTransaction.second);
+                                        .arg(m_pushButtonTransaction.second);
 
     QSqlQuery storeTokenQuery(m_db);
     if (!storeTokenQuery.exec(storeTokenQueryString) || m_db.lastError().type() != QSqlError::NoError) {

libnymea-core/usermanager/usermanager.h

@@ -46,7 +46,8 @@ public:
         UserErrorDuplicateUserId,
         UserErrorBadPassword,
         UserErrorTokenNotFound,
-        UserErrorPermissionDenied
+        UserErrorPermissionDenied,
+        UserErrorInconsistantScopes
     };
     Q_ENUM(UserError)
 
@@ -91,6 +92,7 @@ private:
     bool validateUsername(const QString &username) const;
     bool validatePassword(const QString &password) const;
     bool validateToken(const QByteArray &token) const;
+    bool validateScopes(Types::PermissionScopes scopes) const;
 
     void dumpDBError(const QString &message);
 

tests/auto/usermanager/testusermanager.cpp

@@ -102,6 +102,9 @@ private slots:
 
     void getUserInfo();
 
+    void testScopeConsitancy_data();
+    void testScopeConsitancy();
+
 private:
     // m_apiToken is in testBase
     QUuid m_tokenId;
@@ -116,11 +119,11 @@ void TestUsermanager::initTestCase()
 {
     NymeaDBusService::setBusType(QDBusConnection::SessionBus);
     NymeaTestBase::initTestCase("*.debug=false\n"
-                                     "Application.debug=true\n"
+                                "Application.debug=true\n"
-                                     "Tests.debug=true\n"
+                                "Tests.debug=true\n"
-                                     "UserManager.debug=true\n"
+                                "UserManager.debug=true\n"
-                                     "PushButtonAgent.debug=true\n"
+                                "PushButtonAgent.debug=true\n"
-                                     "MockDevice.debug=true");
+                                "MockDevice.debug=true");
 }
 
 void TestUsermanager::init()
@@ -357,7 +360,7 @@ void TestUsermanager::authenticatePushButtonAuthConnectionDrop()
 
     // Create a new clientId for alice and connect it to the server
     QUuid aliceId = QUuid::createUuid();
-    m_mockTcpServer->clientConnected(aliceId);
+    emit m_mockTcpServer->clientConnected(aliceId);
     m_mockTcpServer->injectData(aliceId, "{\"id\": 0, \"method\": \"JSONRPC.Hello\"}");
     if (clientSpy.count() == 0) clientSpy.wait();
 
@@ -368,12 +371,12 @@ void TestUsermanager::authenticatePushButtonAuthConnectionDrop()
     QCOMPARE(response.toMap().value("params").toMap().value("success").toBool(), true);
 
     // Disconnect alice
-    m_mockTcpServer->clientDisconnected(aliceId);
+    emit m_mockTcpServer->clientDisconnected(aliceId);
 
     // Now try with bob
     // Create a new clientId for bob and connect it to the server
     QUuid bobId = QUuid::createUuid();
-    m_mockTcpServer->clientConnected(bobId);
+    emit m_mockTcpServer->clientConnected(bobId);
     clientSpy.clear();
     m_mockTcpServer->injectData(bobId, "{\"id\": 0, \"method\": \"JSONRPC.Hello\"}");
     if (clientSpy.count() == 0) clientSpy.wait();
@@ -495,9 +498,8 @@ void TestUsermanager::authenticateAfterPasswordChangeFail()
     QVERIFY2(disconnectedSpy.count() == 1, "Connection should have dropped");
 
     QTest::qWait(3200);
-    m_mockTcpServer->clientConnected(m_clientId);
+    emit m_mockTcpServer->clientConnected(m_clientId);
     injectAndWait("JSONRPC.Hello");
-
 }
 
 void TestUsermanager::getUserInfo()
@@ -505,14 +507,9 @@ void TestUsermanager::getUserInfo()
     authenticate();
 
     QVariant response = injectAndWait("Users.GetUserInfo");
-
     QCOMPARE(response.toMap().value("status").toString(), QString("success"));
-
     QVariantMap userInfoMap = response.toMap().value("params").toMap().value("userInfo").toMap();
-
-
     QCOMPARE(userInfoMap.value("username").toString(), QString("valid@user.test"));
-
 }
 
 void TestUsermanager::unauthenticatedCallAfterTokenRemove()
@@ -530,9 +527,117 @@ void TestUsermanager::unauthenticatedCallAfterTokenRemove()
     QVERIFY2(spy.count() == 1, "Connection should be terminated!");
 
     QTest::qWait(3200);
-    m_mockTcpServer->clientConnected(m_clientId);
+    emit m_mockTcpServer->clientConnected(m_clientId);
     injectAndWait("JSONRPC.Hello");
 }
 
+void TestUsermanager::testScopeConsitancy_data()
+{
+    QTest::addColumn<QList<Types::PermissionScope>>("scopes");
+    QTest::addColumn<QString>("error");
+
+    QTest::newRow("valid: admin")
+        << (QList<Types::PermissionScope>()
+            << Types::PermissionScopeAdmin)
+        << "UserErrorNoError";
+
+    QTest::newRow("valid: none")
+        << (QList<Types::PermissionScope>()
+            << Types::PermissionScopeNone)
+        << "UserErrorNoError";
+
+    QTest::newRow("valid: only control, not all things")
+        << (QList<Types::PermissionScope>()
+            << Types::PermissionScopeControlThings
+            << Types::PermissionScopeAccessAllThings)
+        << "UserErrorNoError";
+
+    QTest::newRow("valid: only control, not all things")
+        << (QList<Types::PermissionScope>()
+            << Types::PermissionScopeControlThings
+            << Types::PermissionScopeConfigureThings
+            << Types::PermissionScopeAccessAllThings)
+        << "UserErrorNoError";
+
+    QTest::newRow("valid: only control, all things")
+        << (QList<Types::PermissionScope>()
+            << Types::PermissionScopeControlThings
+            << Types::PermissionScopeAccessAllThings)
+        << "UserErrorNoError";
+
+    QTest::newRow("valid: control things/rules, all things")
+        << (QList<Types::PermissionScope>()
+            << Types::PermissionScopeControlThings
+            << Types::PermissionScopeAccessAllThings
+            << Types::PermissionScopeExecuteRules)
+        << "UserErrorNoError";
+
+    QTest::newRow("valid: only execute rules")
+        << (QList<Types::PermissionScope>()
+            << Types::PermissionScopeAccessAllThings
+            << Types::PermissionScopeExecuteRules)
+        << "UserErrorNoError";
+
+
+    QTest::newRow("invalid: missing control and all things")
+        << (QList<Types::PermissionScope>()
+            << Types::PermissionScopeConfigureThings)
+        << "UserErrorInconsistantScopes";
+
+    QTest::newRow("invalid: control/configure things. not all things")
+        << (QList<Types::PermissionScope>()
+            << Types::PermissionScopeControlThings
+            << Types::PermissionScopeConfigureThings)
+        << "UserErrorInconsistantScopes";
+
+    QTest::newRow("invalid: only execute rules, not all things")
+        << (QList<Types::PermissionScope>()
+            << Types::PermissionScopeExecuteRules)
+        << "UserErrorInconsistantScopes";
+
+    QTest::newRow("invalid: only configure rules")
+        << (QList<Types::PermissionScope>()
+            << Types::PermissionScopeConfigureRules)
+        << "UserErrorInconsistantScopes";
+
+    QTest::newRow("invalid: configure and execute rules, not all things")
+        << (QList<Types::PermissionScope>()
+            << Types::PermissionScopeExecuteRules
+            << Types::PermissionScopeConfigureRules)
+        << "UserErrorInconsistantScopes";
+
+    QTest::newRow("invalid: control things/rules, not all things")
+        << (QList<Types::PermissionScope>()
+            << Types::PermissionScopeControlThings
+            << Types::PermissionScopeExecuteRules)
+        << "UserErrorInconsistantScopes";
+}
+
+void TestUsermanager::testScopeConsitancy()
+{
+    QFETCH(QList<Types::PermissionScope>, scopes);
+    QFETCH(QString, error);
+
+    authenticate();
+
+    QVariant response = injectAndWait("Users.GetUserInfo");
+    QCOMPARE(response.toMap().value("status").toString(), QString("success"));
+    QVariantMap userInfoMap = response.toMap().value("params").toMap().value("userInfo").toMap();
+    QCOMPARE(userInfoMap.value("username").toString(), QString("valid@user.test"));
+
+    QMetaEnum metaEnum = QMetaEnum::fromType<Types::PermissionScope>();
+    QStringList scopesList;
+    foreach (Types::PermissionScope scope, scopes)
+        scopesList.append(metaEnum.valueToKey(scope));
+
+    // Now try to edit with the given scopes
+    QVariantMap params;
+    params.insert("username", userInfoMap.value("username").toString());
+    params.insert("scopes", scopesList);
+    response = injectAndWait("Users.SetUserScopes", params);
+    QCOMPARE(response.toMap().value("status").toString(), QString("success"));
+    QCOMPARE(response.toMap().value("params").toMap().value("error").toString(), error);
+}
+
 #include "testusermanager.moc"
 QTEST_MAIN(TestUsermanager)