PackageManager: Add checks on write rights

tmontaigu · tmontaigu · commit 1d6958c986ca · 2024-03-03T20:33:39.000+01:00
To be able to install a package, the package manager calls pip,
pip needs to have write permissions otherwise it will fail.

CloudCompare on windows being generally installed in C:\Programs
that requires admin rights, so a non admin cannot pip install things
in there.

There have been many user confused by this, so its easier to block
installation and have a message if we detect the folder is not writable

Also, the package manager should now automatically use the `--user` option
for pip install when it's using the system's Python
diff --git a/src/PackageManager.cpp b/src/PackageManager.cpp
@@ -34,6 +34,128 @@
 
 #include <ui_InstallDialog.h>
 
+#include "Utilities.h"
+
+#if defined(Q_OS_WIN32)
+#include <Windows.h>
+#endif
+
+#if defined(Q_OS_WIN32)
+static BOOL GetFolderRights(LPCTSTR folderName, DWORD genericAccessRights, DWORD *grantedRights)
+{
+    DWORD length = 0;
+
+    constexpr SECURITY_INFORMATION requestedInformation =
+        OWNER_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION;
+
+    /* Get the needed length */
+    GetFileSecurity(folderName, requestedInformation, nullptr, NULL, &length);
+
+    if (ERROR_INSUFFICIENT_BUFFER != GetLastError())
+    {
+        return FALSE;
+    }
+
+    PSECURITY_DESCRIPTOR security = LocalAlloc(LPTR, length);
+    if (GetFileSecurity(folderName, requestedInformation, security, length, &length) == FALSE)
+    {
+        LocalFree(security);
+        return FALSE;
+    }
+
+    DWORD desiredAccess = TOKEN_IMPERSONATE | TOKEN_QUERY | TOKEN_DUPLICATE | STANDARD_RIGHTS_READ;
+    HANDLE hToken = nullptr;
+    if (OpenProcessToken(GetCurrentProcess(), desiredAccess, &hToken) == FALSE)
+    {
+        LocalFree(security);
+        return FALSE;
+    }
+
+    HANDLE hImpersonatedToken = nullptr;
+    if (DuplicateToken(hToken, SecurityImpersonation, &hImpersonatedToken) == FALSE)
+    {
+        CloseHandle(hToken);
+        LocalFree(security);
+        return FALSE;
+    }
+
+    GENERIC_MAPPING mapping = {0xFFFFFFFF};
+    PRIVILEGE_SET privileges = {0};
+    DWORD grantedAccess = 0, privilegesLength = sizeof(privileges);
+    BOOL result = FALSE;
+
+    mapping.GenericRead = FILE_GENERIC_READ;
+    mapping.GenericWrite = FILE_GENERIC_WRITE;
+    mapping.GenericExecute = FILE_GENERIC_EXECUTE;
+    mapping.GenericAll = FILE_ALL_ACCESS;
+
+    MapGenericMask(&genericAccessRights, &mapping);
+    if (AccessCheck(security,
+                    hImpersonatedToken,
+                    genericAccessRights,
+                    &mapping,
+                    &privileges,
+                    &privilegesLength,
+                    &grantedAccess,
+                    &result) == FALSE)
+    {
+        CloseHandle(hImpersonatedToken);
+        CloseHandle(hToken);
+        LocalFree(security);
+        return FALSE;
+    }
+
+    *grantedRights = grantedAccess;
+
+    CloseHandle(hImpersonatedToken);
+    CloseHandle(hToken);
+    LocalFree(security);
+
+    return TRUE;
+}
+
+static bool HasReadWriteAccessToFolder(const QString &folderPath)
+{
+    constexpr DWORD access_mask = MAXIMUM_ALLOWED;
+    DWORD grant = 0;
+#if defined(UNICODE)
+    const std::wstring str = folderPath.toStdWString();
+    BOOL ret = GetFolderRights(str.c_str(), access_mask, &grant);
+#else
+    const std::string str = folderPath.toStdString();
+    BOOL ret = GetFolderRights(str.c_str(), access_mask, &grant);
+#endif
+
+    if (ret == FALSE)
+    {
+        plgWarning() << "Failed to get access rights for path '" << folderPath << '\n';
+        return false;
+    }
+
+    bool hasRead = false;
+    if (((grant & GENERIC_READ) == GENERIC_READ) ||
+        ((grant & FILE_GENERIC_READ) == FILE_GENERIC_READ))
+    {
+        hasRead = true;
+    }
+
+    bool hasWrite = false;
+    if (((grant & GENERIC_WRITE) == GENERIC_WRITE) ||
+        ((grant & FILE_GENERIC_WRITE) == FILE_GENERIC_WRITE))
+    {
+        hasWrite = true;
+    }
+
+    bool hasExecute = false;
+    if (((grant & GENERIC_EXECUTE) == GENERIC_EXECUTE) ||
+        ((grant & FILE_GENERIC_EXECUTE) == FILE_GENERIC_EXECUTE))
+    {
+        hasExecute = true;
+    }
+    return hasRead && hasWrite && hasExecute;
+}
+#endif // defined(Q_OS_WIN32)
+
 class InstallDialog : public QDialog
 {
     Q_OBJECT
@@ -75,12 +197,12 @@ class CommandOutputDialog : public QDialog
         resize(600, 300);
     }
 
-    void appendPlainText(const QString &text)
+    void appendPlainText(const QString &text) const
     {
         m_display->appendPlainText(text);
     }
 
-    void clear()
+    void clear() const
     {
         m_display->clear();
     }
@@ -93,7 +215,8 @@ PackageManager::PackageManager(const PythonConfig &config, QWidget *parent)
     : QWidget(parent),
       m_ui(new Ui_PackageManager),
       m_pythonProcess(new QProcess),
-      m_outputDialog(new CommandOutputDialog(this))
+      m_outputDialog(new CommandOutputDialog(this)),
+      m_shouldUseUserOption(false)
 {
     m_ui->setupUi(this);
     connect(m_pythonProcess, &QProcess::started, [this]() { setBusy(true); });
@@ -120,6 +243,45 @@ PackageManager::PackageManager(const PythonConfig &config, QWidget *parent)
     connect(m_ui->searchBar, &QLineEdit::returnPressed, this, &PackageManager::handleSearch);
     config.preparePythonProcess(*m_pythonProcess);
     refreshInstalledPackagesList();
+
+    m_ui->installBtn->setEnabled(true);
+    m_ui->messageFrame->hide();
+
+    switch (config.type())
+    {
+    // The main intent of checking access rights of venv is for the Windows bundled
+    // env, but checking for all venv won't hurt.
+    // on Windowsn the bundled env  is very likely to be installed in
+    // "C:\Programs\Cloud Compare\plugins\Python" and that requires admin rights to add/modify.
+    // It is better to notify user and prevent them from trying something that will faill.
+    case PythonConfig::Type::Venv:
+    case PythonConfig::Type::Conda:
+    case PythonConfig::Type::Unknown:
+    {
+#if defined Q_OS_WIN32
+        // On Windows we use a custom function because isWritable from Qt was not correct
+        // for our use case (its mentionned in their doc)
+        const bool hasEnoughRights = HasReadWriteAccessToFolder(config.pythonHome());
+#else
+        const QFileInfo dirInfo(config.pythonHome());
+        const bool hasEnoughRights = dirInfo.isWritable();
+#endif
+        if (!hasEnoughRights)
+        {
+            m_ui->installBtn->setEnabled(false);
+            m_ui->messageIconLabel->setPixmap(QApplication::style()
+                                                  ->standardIcon(QStyle::SP_MessageBoxCritical)
+                                                  .pixmap({64, 64}));
+            m_ui->messageTextLabel->setText("Admin rights are required to be able to install "
+                                            "packages in the current environment");
+            m_ui->messageFrame->show();
+        }
+    }
+    break;
+    case PythonConfig::Type::System:
+        m_shouldUseUserOption = true;
+        break;
+    }
 }
 
 void PackageManager::refreshInstalledPackagesList()
@@ -225,6 +387,10 @@ void PackageManager::handleInstallPackage()
     {
         arguments.push_back("--upgrade");
     }
+    if (m_shouldUseUserOption)
+    {
+        arguments.push_back("--user");
+    }
     executeCommand(arguments);
 
     if (m_pythonProcess->exitCode() != 0)
diff --git a/src/PackageManager.h b/src/PackageManager.h
@@ -51,6 +51,7 @@ class PackageManager final : public QWidget
     Ui_PackageManager *m_ui;
     QProcess *m_pythonProcess;
     CommandOutputDialog *m_outputDialog;
+    bool m_shouldUseUserOption;
 };
 
 #endif // PYTHON_PLUGIN_PACKAGE_MANAGER_H
diff --git a/src/PythonConfig.h b/src/PythonConfig.h
@@ -87,12 +87,6 @@ class PythonConfig final
         Unknown
     };
 
-    /// # On Windows:
-    /// Initialize python home and python path
-    /// corresponding to the environment to be used.
-    ///
-    /// # Other Platforms
-    /// Does nothing, as we rely on the system's python to be properly installed
     PythonConfig() = default;
 
     Type type() const
@@ -120,6 +114,11 @@ class PythonConfig final
         return o;
     }
 
+    const QString &pythonHome() const
+    {
+        return m_pythonHome;
+    }
+
     /// Sets the necessary settings of the QProcess so that
     /// it uses the correct Python exe.
     void preparePythonProcess(QProcess &pythonProcess) const;
@@ -148,6 +147,12 @@ class PythonConfig final
     static bool IsInsideEnvironment();
     static PythonConfig fromContainingEnvironment();
 
+    /// # On Windows:
+    /// Initialize python home and python path
+    /// corresponding to the environment to be used.
+    ///
+    /// # Other Platforms
+    /// Does nothing, as we rely on the system's python to be properly installed
     void initDefault();
 #ifdef Q_OS_WIN32
     /// Initialize the paths to point to where the Python
diff --git a/src/Ui/PackageManager.ui b/src/Ui/PackageManager.ui
@@ -6,16 +6,55 @@
    <rect>
     <x>0</x>
     <y>0</y>
-    <width>660</width>
-    <height>425</height>
+    <width>694</width>
+    <height>495</height>
    </rect>
   </property>
   <property name="windowTitle">
    <string>Packages</string>
   </property>
-  <layout class="QHBoxLayout" name="horizontalLayout">
+  <layout class="QHBoxLayout" name="horizontalLayout_2">
    <item>
     <layout class="QVBoxLayout" name="verticalLayout_2">
+     <item>
+      <widget class="QFrame" name="messageFrame">
+       <property name="frameShape">
+        <enum>QFrame::Box</enum>
+       </property>
+       <property name="frameShadow">
+        <enum>QFrame::Raised</enum>
+       </property>
+       <layout class="QHBoxLayout" name="horizontalLayout">
+        <item>
+         <widget class="QLabel" name="messageIconLabel">
+          <property name="text">
+           <string>TextLabel</string>
+          </property>
+         </widget>
+        </item>
+        <item>
+         <widget class="QLabel" name="messageTextLabel">
+          <property name="text">
+           <string>TextLabel</string>
+          </property>
+         </widget>
+        </item>
+        <item>
+         <spacer name="messageSpacer">
+          <property name="orientation">
+           <enum>Qt::Horizontal</enum>
+          </property>
+          <property name="sizeHint" stdset="0">
+           <size>
+            <width>456</width>
+            <height>20</height>
+           </size>
+          </property>
+         </spacer>
+        </item>
+       </layout>
+      </widget>
+     </item>
      <item>
       <widget class="QLineEdit" name="searchBar"/>
      </item>
