Fix editability of tables with column-limited insert privilege

Closes GH-28835 Includes testcase
qgis · Jul 15, 2024 · d3474e4 · d3474e4
1 parent ad03718
commit d3474e4
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 20 deletions.
diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp
@@ -1492,18 +1492,10 @@ bool QgsPostgresProvider::hasSufficientPermsAndCapabilities()
 
   mEnabledCapabilities = QgsVectorDataProvider::Capability::ReloadData;
 
-  QString sql;
   QgsPostgresResult testAccess;
 
   bool forceReadOnly = ( mReadFlags & QgsDataProvider::ForceReadOnly );
   bool inRecovery = false;
-  sql = QStringLiteral( "SELECT "
-                        "has_table_privilege(%1,'SELECT'),"   // 0
-                        "pg_is_in_recovery(),"                // 1
-                        "current_schema(), "                  // 2
-                        "has_table_privilege(%1,'INSERT'),"   // 3
-                        "has_table_privilege(%1,'DELETE')" ) // 4
-        .arg( quotedValue( mQuery ) );
 
   if ( !mIsQuery )
   {
@@ -1515,23 +1507,37 @@ bool QgsPostgresProvider::hasSufficientPermsAndCapabilities()
       mEnabledCapabilities |= QgsVectorDataProvider::SelectAtId;
     }
 
+    QString sql = QStringLiteral(
+                    "SELECT "
+                    "has_table_privilege(%1,'SELECT')," // 0 (select priv)
+                    "pg_is_in_recovery(),"              // 1 (in recovery)
+                    "current_schema() "                 // 2 (current schema)
+                  ).arg( quotedValue( mQuery ) );
+
+
     if ( connectionRO()->pgVersion() >= 80400 )
     {
-      sql += QString( ",has_any_column_privilege(%1,'UPDATE')" // 5
-                      ",%2" ) // 6
-             .arg( quotedValue( mQuery ),
-                   mGeometryColumn.isNull()
-                   ? QStringLiteral( "'f'" )
-                   : QStringLiteral( "has_column_privilege(%1,%2,'UPDATE')" )
-                   .arg( quotedValue( mQuery ),
-                         quotedValue( mGeometryColumn ) )
-                 );
+      sql += QString(
+               ",has_any_column_privilege(%1,'INSERT')" // 3 (insert priv)
+               ",has_table_privilege(%1,'DELETE')"      // 4 (delete priv)
+               ",has_any_column_privilege(%1,'UPDATE')" // 5 (update priv)
+               ",%2"                                    // 6 (geom upd priv)
+             ).arg( quotedValue( mQuery ),
+                    mGeometryColumn.isNull()
+                    ? QStringLiteral( "'f'" )
+                    : QStringLiteral( "has_column_privilege(%1,%2,'UPDATE')" )
+                    .arg( quotedValue( mQuery ),
+                          quotedValue( mGeometryColumn ) )
+                  );
     }
     else
     {
-      sql += QString( ",has_table_privilege(%1,'UPDATE')" // 5
-                      ",has_table_privilege(%1,'UPDATE')" ) // 6
-             .arg( quotedValue( mQuery ) );
+      sql += QString(
+               ",has_table_privilege(%1,'INSERT')" // 3 (insert priv)
+               ",has_table_privilege(%1,'DELETE')" // 4 (delete priv)
+               ",has_table_privilege(%1,'UPDATE')" // 5 (update priv)
+               ",has_table_privilege(%1,'UPDATE')" // 6 (geom col priv)
+             ).arg( quotedValue( mQuery ) );
     }
 
     testAccess = connectionRO()->LoggedPQexec( "QgsPostgresProvider", sql );

diff --git a/tests/src/python/test_provider_postgres.py b/tests/src/python/test_provider_postgres.py
@@ -3475,6 +3475,38 @@ def testPktLowerCase(self):
         cur.execute(sql_pk)
         self.assertEqual(cur.fetchall(), [('dep', 'character varying')])
 
+    def testColumnRestrictedLayerIsEditable(self):
+        """
+            Test editability of table with partial column insert privs
+            See https://github.com/qgis/QGIS/issues/28835
+        """
+
+        md = QgsProviderRegistry.instance().providerMetadata("postgres")
+        conn = md.createConnection(self.dbconn, {})
+
+        conn.executeSql('''
+DROP TABLE IF EXISTS public.qgis_issue_gh_28835;
+CREATE UNLOGGED TABLE public.qgis_issue_gh_28835 (
+  id INT PRIMARY KEY,
+  restricted_column TEXT,
+  geom GEOMETRY(point, 4326)
+)
+            ''')
+
+        uri = QgsDataSourceUri(self.dbconn + ' table="public"."qgis_issue_gh_28835" (geom)')
+        uri.setUsername('qgis_test_unprivileged_user')
+        uri.setPassword('qgis_test_unprivileged_user_password')
+
+        conn.executeSql('GRANT SELECT ON public.qgis_issue_gh_28835 TO qgis_test_unprivileged_user')
+        vl = QgsVectorLayer(uri.uri(), 'test', 'postgres')
+        self.assertTrue(vl.isValid(), "qgis_issue_gh_28835 is an invalid layer")
+        self.assertFalse(vl.startEditing(), "qgis_issue_gh_28835 is unexpectedly editable by qgis_test_unprivileged_user before grants")
+
+        conn.executeSql('GRANT INSERT(geom) ON public.qgis_issue_gh_28835 TO qgis_test_unprivileged_user')
+        vl = QgsVectorLayer(uri.uri(), 'test', 'postgres')
+        self.assertTrue(vl.isValid(), "qgis_issue_gh_28835 is an invalid layer")
+        self.assertTrue(vl.startEditing(), "qgis_issue_gh_28835 is not editable by qgis_test_unprivileged_user after restricted-column insert grant")
+
 
 class TestPyQgsPostgresProviderCompoundKey(QgisTestCase, ProviderTestCase):