Merge pull request #1406 from timopollmeier/fix-iterator-with-20.08

Also create owner WITH clause for single resources
greenbone · Feb 4, 2021 · ac619bd · ac619bd
2 parents 7eb002d + 8e746ef
commit ac619bd
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 63 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ### Changed
 
 ### Fixed
+- Also create owner WITH clause for single resources [#1406](https://github.com/greenbone/gvmd/pull/1406)
 
 ### Removed
 

diff --git a/src/manage_acl.c b/src/manage_acl.c
@@ -1011,50 +1011,7 @@ acl_where_owned_user (const char *user_id, const char *user_sql,
   guint index;
 
   if (with)
-    *with = NULL;
-
-  if (owned == 0)
-   return g_strdup (" t ()");
-
-  permission_or = g_string_new ("");
-  index = 0;
-  if (permissions == NULL || permissions->len == 0)
     {
-      /* Treat filters with no permissions keyword as "any". */
-      permission_or = g_string_new ("t ()");
-      index = 1;
-    }
-  else if (permissions)
-    for (; index < permissions->len; index++)
-      {
-        gchar *permission, *quoted;
-        permission = (gchar*) g_ptr_array_index (permissions, index);
-        if (strcasecmp (permission, "any") == 0)
-          {
-            g_string_free (permission_or, TRUE);
-            permission_or = g_string_new ("t ()");
-            index = 1;
-            break;
-          }
-        quoted = sql_quote (permission);
-        if (index == 0)
-          g_string_append_printf (permission_or, "name = '%s'", quoted);
-        else
-          g_string_append_printf (permission_or, " OR name = '%s'",
-                                  quoted);
-        g_free (quoted);
-      }
-
-  table_trash = get->trash && strcasecmp (type, "task");
-  if (resource || (user_id == NULL))
-    owned_clause
-     = g_strdup (" (t ())");
-  else if (with)
-    {
-      gchar *permission_clause;
-
-      /* Caller supports WITH clause. */
-
       *with = g_strdup_printf
                ("WITH permissions_subject"
                 "     AS (SELECT * FROM permissions"
@@ -1098,6 +1055,49 @@ acl_where_owned_user (const char *user_id, const char *user_sql,
                 user_sql,
                 user_sql,
                 user_sql);
+    }
+
+  if (owned == 0)
+   return g_strdup (" t ()");
+
+  permission_or = g_string_new ("");
+  index = 0;
+  if (permissions == NULL || permissions->len == 0)
+    {
+      /* Treat filters with no permissions keyword as "any". */
+      permission_or = g_string_new ("t ()");
+      index = 1;
+    }
+  else if (permissions)
+    for (; index < permissions->len; index++)
+      {
+        gchar *permission, *quoted;
+        permission = (gchar*) g_ptr_array_index (permissions, index);
+        if (strcasecmp (permission, "any") == 0)
+          {
+            g_string_free (permission_or, TRUE);
+            permission_or = g_string_new ("t ()");
+            index = 1;
+            break;
+          }
+        quoted = sql_quote (permission);
+        if (index == 0)
+          g_string_append_printf (permission_or, "name = '%s'", quoted);
+        else
+          g_string_append_printf (permission_or, " OR name = '%s'",
+                                  quoted);
+        g_free (quoted);
+      }
+
+  table_trash = get->trash && strcasecmp (type, "task");
+  if (resource || (user_id == NULL))
+    owned_clause
+     = g_strdup (" (t ())");
+  else if (with)
+    {
+      gchar *permission_clause;
+
+      /* Caller supports WITH clause. */
 
       permission_clause = NULL;
       if (user_id && index)

diff --git a/src/manage_sql.c b/src/manage_sql.c
@@ -4857,11 +4857,13 @@ init_get_iterator2_with (iterator_t* iterator, const char *type,
 
   with_clause = NULL;
 
-  if (resource)
-    /* Ownership test is done above by find function. */
-    owned_clause = g_strdup (" t ()");
-  else if (assume_permitted)
-    owned_clause = g_strdup (" t ()");
+  if (resource || assume_permitted)
+    /* Ownership test of single resources is done above by find function
+     * but acl_where_owned has to be called to generate WITH clause
+     * in case subqueries depend on it.
+     */
+    owned_clause = acl_where_owned (type, get, 0, owner_filter, resource,
+                                    permissions, &with_clause);
   else
     owned_clause = acl_where_owned (type, get, owned, owner_filter, resource,
                                     permissions, &with_clause);
@@ -22202,7 +22204,7 @@ init_result_get_iterator_severity (iterator_t* iterator, const get_data_t *get,
   gchar *filter;
   int autofp, apply_overrides, dynamic_severity;
   gchar *extra_tables, *extra_where, *extra_where_single;
-  gchar *owned_clause, *with_clause, *with_clauses;
+  gchar *owned_clause, *with_clause;
   char *user_id;
 
   assert (report);
@@ -22360,11 +22362,12 @@ init_result_get_iterator_severity (iterator_t* iterator, const get_data_t *get,
 
   user_id = sql_string ("SELECT id FROM users WHERE uuid = '%s';",
                         current_credentials.uuid);
-  owned_clause = acl_where_owned_for_get ("override", user_id, &with_clause);
+  // Do not get ACL with_clause as it will be added by init_get_iterator2_with.
+  owned_clause = acl_where_owned_for_get ("override", user_id, NULL);
   free (user_id);
-  with_clauses = g_strdup_printf
-                  ("%s%s"
-                   " valid_overrides"
+
+  with_clause = g_strdup_printf
+                  (" valid_overrides"
                    " AS (SELECT result_nvt, hosts, new_severity, port,"
                    "            severity, result"
                    "     FROM overrides"
@@ -22382,17 +22385,9 @@ init_result_get_iterator_severity (iterator_t* iterator, const get_data_t *get,
                    "     ORDER BY result DESC, task DESC, port DESC, severity ASC,"
                    "           creation_time DESC)"
                    " ",
-                   with_clause
-                    /* Skip the leading "WITH" because init_get..
-                     * below will add it.  A bit of a hack, but
-                     * it's the only place that needs this. */
-                    ? with_clause + 4
-                    : "",
-                   with_clause ? "," : "",
                    owned_clause,
                    report,
                    report);
-  g_free (with_clause);
   g_free (owned_clause);
 
   table_order_if_sort_not_specified = 1;
@@ -22413,11 +22408,11 @@ init_result_get_iterator_severity (iterator_t* iterator, const get_data_t *get,
                                  TRUE,
                                  report ? TRUE : FALSE,
                                  extra_order,
-                                 with_clauses,
+                                 with_clause,
                                  1);
   table_order_if_sort_not_specified = 0;
   column_array_free (filterable_columns);
-  g_free (with_clauses);
+  g_free (with_clause);
   g_free (extra_tables);
   g_free (extra_where);
   g_free (extra_where_single);