[ARVADOS] updated: 1.3.0-2617-g1a599ec69

Git user git at public.arvados.org
Fri Jun 5 20:20:41 UTC 2020


Summary of changes:
 services/api/app/models/arvados_model.rb           |  12 +-
 services/api/app/models/user.rb                    |  15 +-
 .../db/migrate/20200501150153_permission_table.rb  | 158 +++++++++------------
 services/api/db/structure.sql                      | 122 ++++++----------
 .../20200501150153_permission_table_constants.rb   |  86 +++++++++++
 services/api/lib/update_permissions.rb             |  56 ++------
 6 files changed, 221 insertions(+), 228 deletions(-)
 create mode 100644 services/api/lib/20200501150153_permission_table_constants.rb

       via  1a599ec69ad8d533da1f12ad5d2c5789aa1c14e2 (commit)
      from  a3aee2781cfcd006fa1b7ce3cfeeb1dd2d53c270 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 1a599ec69ad8d533da1f12ad5d2c5789aa1c14e2
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Fri Jun 5 16:20:24 2020 -0400

    16007: Refactoring and update comments.
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index a27d6aba4..8afebfb79 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -312,10 +312,15 @@ class ArvadosModel < ApplicationRecord
       # The core of the permission check is a join against the
       # materialized_permissions table to determine if the user has at
       # least read permission to either the object itself or its
-      # direct owner.  See
+      # direct owner (if traverse_owned is true).  See
       # db/migrate/20200501150153_permission_table.rb for details on
       # how the permissions are computed.
 
+      # A user can have can_manage access to another user, this grants
+      # full access to all that user's stuff.  To implement that we
+      # need to include those other users in the permission query.
+      user_uuids_subquery = USER_UUIDS_SUBQUERY_TEMPLATE % {user: ":user_uuids", perm_level: 1}
+
       # Note: it is possible to combine the direct_check and
       # owner_check into a single EXISTS() clause, however it turns
       # out query optimizer doesn't like it and forces a sequential
@@ -324,11 +329,6 @@ class ArvadosModel < ApplicationRecord
       #
       # see issue 13208 for details.
 
-      user_uuids_subquery = %{
-select target_uuid from materialized_permissions where user_uuid in (:user_uuids)
-and target_uuid like '_____-tpzed-_______________' and traverse_owned=true and perm_level >= 1
-}
-
       # Match a direct read permission link from the user to the record uuid
       direct_check = "#{sql_table}.uuid IN (SELECT target_uuid FROM #{PERMISSION_VIEW} "+
                      "WHERE user_uuid IN (#{user_uuids_subquery}) AND perm_level >= 1 #{trashed_check})"
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 869ac88f4..747254f6c 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -121,10 +121,7 @@ class User < ArvadosModel
 
       target_owner_uuid = target.owner_uuid if target.respond_to? :owner_uuid
 
-      user_uuids_subquery = %{
-select target_uuid from materialized_permissions where user_uuid = $1
-and target_uuid like '_____-tpzed-_______________' and traverse_owned=true and perm_level >= #{VAL_FOR_PERM[action]}
-}
+      user_uuids_subquery = USER_UUIDS_SUBQUERY_TEMPLATE % {user: "$1", perm_level: VAL_FOR_PERM[action]}
 
       unless ActiveRecord::Base.connection.
         exec_query(%{
@@ -169,6 +166,9 @@ SELECT 1 FROM #{PERMISSION_VIEW}
   end
 
   # Return a hash of {user_uuid: group_perms}
+  #
+  # note: this does not account for permissions that a user gains by
+  # having can_manage on another user.
   def self.all_group_permissions
     all_perms = {}
     ActiveRecord::Base.connection.
@@ -189,14 +189,17 @@ SELECT 1 FROM #{PERMISSION_VIEW}
   # objects owned by group_uuid.
   def group_permissions(level=1)
     group_perms = {}
+
+    user_uuids_subquery = USER_UUIDS_SUBQUERY_TEMPLATE % {user: "$1", perm_level: VAL_FOR_PERM[action]}
+
     ActiveRecord::Base.connection.
       exec_query(%{
 SELECT target_uuid, perm_level
   FROM #{PERMISSION_VIEW}
-  WHERE user_uuid = $1 and perm_level >= $2
+  WHERE user_uuid = user_uuid in (#{user_uuids_subquery}) and perm_level >= $2
 },
                   # "name" arg is a query label that appears in logs:
-                  "group_permissions_for_user",
+                  "User.group_permissions",
                   # "binds" arg is an array of [col_id, value] for '$1' vars:
                   [[nil, uuid],
                    [nil, level]]).
diff --git a/services/api/db/migrate/20200501150153_permission_table.rb b/services/api/db/migrate/20200501150153_permission_table.rb
index 6dd2c29bd..7b9a99813 100644
--- a/services/api/db/migrate/20200501150153_permission_table.rb
+++ b/services/api/db/migrate/20200501150153_permission_table.rb
@@ -2,7 +2,7 @@
 #
 # SPDX-License-Identifier: AGPL-3.0
 
-require 'update_permissions'
+require '20200501150153_permission_table_constants'
 
 class PermissionTable < ActiveRecord::Migration[5.0]
   def up
@@ -56,28 +56,12 @@ WITH RECURSIVE
         )
         select uuid, trash_at from project_subtree;
 $$;
-}
-
-    ActiveRecord::Base.connection.execute %{
-create or replace function compute_trashed ()
-returns table (uuid varchar(27), trash_at timestamp)
-STABLE
-language SQL
-as $$
-/* Helper function to populate trashed_groups table. This starts with
-   each group owned by a user and computes the subtree under that
-   group to find any groups that are trashed.
-*/
-select ps.target_uuid as group_uuid, ps.trash_at from groups,
-  lateral project_subtree_with_trash_at(groups.uuid, groups.trash_at) ps
-  where groups.owner_uuid like '_____-tpzed-_______________'
-$$;
 }
 
     # Now populate the table.  For a non-test databse this is the only
     # time this ever happens, after this the trash table is updated
     # incrementally.  See app/models/group.rb#update_trash
-    ActiveRecord::Base.connection.execute("INSERT INTO trashed_groups select * from compute_trashed()")
+    refresh_trashed
 
     # The table to store the flattened permissions.  This is almost
     # exactly the same as the old materalized_permission_view except
@@ -116,12 +100,12 @@ $$;
 
     # Merge all permission relationships into a single view.  This
     # consists of: groups (projects) owning things, users owning
-    # things, and explicit permission links.
+    # things, users owning themselves, and explicit permission links.
     #
-    # Fun fact, a SQL view gets inlined into the query where it is
-    # used, this enables the query planner to inject constraints, so
-    # when using the view we only look up edges we plan to traverse
-    # and avoid a brute force computation of all edges.
+    # A SQL view gets inlined into the query where it is used as a
+    # subquery.  This enables the query planner to inject constraints,
+    # so we only look up edges we plan to traverse and avoid a brute
+    # force query of all edges.
     ActiveRecord::Base.connection.execute %{
 create view permission_graph_edges as
   select groups.owner_uuid as tail_uuid, groups.uuid as head_uuid, (3) as val from groups
@@ -143,6 +127,9 @@ union all
       where links.link_class='permission'
 }
 
+    # Code fragment that is used below.  This is used to ensure that
+    # the permission edge passed into compute_permission_subgraph
+    # takes precedence over an existing edge in the "edges" view.
     override = %{,
                             case (edges.tail_uuid = perm_origin_uuid AND
                                   edges.head_uuid = starting_uuid)
@@ -151,6 +138,14 @@ union all
                             end
 }
 
+    #
+    # The primary function to compute permissions for a subgraph.
+    # This originally was organized somewhat more cleanly, but this
+    # ran into performance issues due to the query optimizer not
+    # working across function and "with" expression boundaries.  So I
+    # had to fall back on using string templates for the repeated
+    # code.  I'm sorry.
+
     ActiveRecord::Base.connection.execute %{
 create or replace function compute_permission_subgraph (perm_origin_uuid varchar(27),
                                                         starting_uuid varchar(27),
@@ -159,7 +154,13 @@ returns table (user_uuid varchar(27), target_uuid varchar(27), val integer, trav
 STABLE
 language SQL
 as $$
-/* perm_origin_uuid: The object that 'gets' or 'has' the permission.
+
+/* The purpose of this function is to compute the permissions for a
+   subgraph of the database, starting from a given edge.  The newly
+   computed permissions are used to add and remove rows from the main
+   permissions table.
+
+   perm_origin_uuid: The object that 'gets' the permission.
 
    starting_uuid: The starting object the permission applies to.
 
@@ -167,40 +168,13 @@ as $$
                   starting_uuid One of 1, 2, 3 for can_read,
                   can_write, can_manage respectively, or 0 to revoke
                   permissions.
-
-   This function is broken up into a number of clauses, described
-   below.
-
-   Note on query optimization:
-
-   Each clause in a "with" statement is called a "common table
-   expression" or CTE.
-
-   In Postgres, they are evaluated in sequence and results of each CTE
-   is stored in a temporary table.  This means Postgres does not
-   propagate constraints from later subqueries to earlier subqueries
-   when they are CTEs.
-
-   This is a problem if, for example, a later subquery chooses 10
-   items out of a set of 1000000 defined by an earlier subquery,
-   because it will always compute all 1000000 rows even if the query
-   on the 1000000 rows could have been constrained.  This is why
-   permission_graph_edges is a view -- views are inlined so and can be
-   optimized using external constraints.
-
-   The query optimizer does sort the temporary tables for later use in
-   joins.
-
-   Final note, this query would have been almost impossible to write
-   (and certainly impossible to read) without splitting it up using
-   SQL "with" but unfortunately it also stumbles into a frustrating
-   Postgres optimizer bug, see
-   lib/refresh_permission_view.rb#update_permissions
-   for details and a partial workaround.
 */
 with
-  /* Gets the initial set of objects potentially affected by the
-     permission change, using search_permission_graph.
+  /* Starting from starting_uuid, determine the set of objects that
+     could be affected by this permission change.
+
+     Note: We don't traverse users unless it is an "identity"
+     permission (permission origin is self).
   */
   perm_from_start(perm_origin_uuid, target_uuid, val, traverse_owned) as (
     #{PERM_QUERY_TEMPLATE % {:base_case => %{
@@ -211,14 +185,21 @@ with
 :override => override
 } }),
 
-  /* Finds other inbound edges that grant permissions on the objects
-     in perm_from_start, and computes permissions that originate from
-     those.  This is required to handle the case where there is more
-     than one path through which a user gets permission to an object.
-     For example, a user owns a project and also shares it can_read
-     with a group the user belongs to, adding the can_read link must
-     not overwrite the existing can_manage permission granted by
-     ownership.
+  /* Find other inbound edges that grant permissions to 'targets' in
+     perm_from_start, and compute permissions that originate from
+     those.
+
+     This is necessary for two reasons:
+
+       1) Other users may have access to a subset of the objects
+       through other permission links than the one we started from.
+       If we don't recompute them, their permission will get dropped.
+
+       2) There may be more than one path through which a user gets
+       permission to an object.  For example, a user owns a project
+       and also shares it can_read with a group the user belongs
+       to. adding the can_read link must not overwrite the existing
+       can_manage permission granted by ownership.
   */
   additional_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
     #{PERM_QUERY_TEMPLATE % {:base_case => %{
@@ -234,7 +215,7 @@ with
 :override => override
 } }),
 
-  /* Combines the permissions computed in the first two phases. */
+  /* Combine the permissions computed in the first two phases. */
   all_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
       select * from perm_from_start
     union all
@@ -247,30 +228,27 @@ with
 
      Key insights:
 
-     * Permissions are transitive (with some special cases involving
-       users, this is controlled by the traverse_owned flag).
+     * For every group, the materialized_permissions lists all users
+       that can access to that group.
 
-     * A user object can only gain permissions via an inbound edge,
-       or appearing in the graph.
+     * The all_perms subquery has computed permissions on on a set of
+       objects for all inbound "origins", which are users or groups.
 
-     * The materialized_permissions table includes the permission
-       each user has on the tail end of each inbound edge.
+     * Permissions through groups are transitive.
 
-     * The all_perms subquery has permissions for each object in the
-       subgraph reachable from certain origin (tail end of an edge).
+     We can infer:
 
-     * Therefore, for each user, we can compute user permissions on
-       each object in subgraph by determining the permission the user
-       has on each origin (tail end of an edge), joining that with the
-       perm_origin_uuid column of all_perms, and taking the least() of
-       the origin edge or all_perms val (because of the "least
-       permission on the path" rule).  If an object was reachable by
-       more than one path (appears with more than one origin), we take
-       the max() of the computed permissions.
+     1) The materialized_permissions table declares that user X has permission N on group Y
+     2) The all_perms result has determined group Y has permission M on object Z
+     3) Therefore, user X has permission min(N, M) on object Z
 
-     * Finally, because users always have permission on themselves, the
-       query also makes sure those permission rows are always
-       returned.
+     This allows us to efficiently determine the set of users that
+     have permissions on the subset of objects, without having to
+     follow the chain of permission back up to find those users.
+
+     In addition, because users always have permission on themselves, this
+     query also makes sure those permission rows are always
+     returned.
   */
   select v.user_uuid, v.target_uuid, max(v.perm_level), bool_or(v.traverse_owned) from
     (select m.user_uuid,
@@ -289,17 +267,10 @@ $$;
      }
 
     #
-    # Populate the materialized_permissions by traversing permissions
+    # Populate materialized_permissions by traversing permissions
     # starting at each user.
     #
-    ActiveRecord::Base.connection.execute %{
-INSERT INTO materialized_permissions
-    #{PERM_QUERY_TEMPLATE % {:base_case => %{
-        select uuid, uuid, 3, true, true from users
-},
-:override => ''
-} }
-}
+    refresh_permissions
   end
 
   def down
@@ -307,7 +278,6 @@ INSERT INTO materialized_permissions
     drop_table :trashed_groups
 
     ActiveRecord::Base.connection.execute "DROP function project_subtree_with_trash_at (varchar, timestamp);"
-    ActiveRecord::Base.connection.execute "DROP function compute_trashed ();"
     ActiveRecord::Base.connection.execute "DROP function compute_permission_subgraph (varchar, varchar, integer);"
     ActiveRecord::Base.connection.execute "DROP function should_traverse_owned(varchar, integer);"
     ActiveRecord::Base.connection.execute "DROP view permission_graph_edges;"
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index aa3ffae62..a8885f584 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -45,7 +45,13 @@ CREATE EXTENSION IF NOT EXISTS pg_trgm WITH SCHEMA public;
 CREATE FUNCTION public.compute_permission_subgraph(perm_origin_uuid character varying, starting_uuid character varying, starting_perm integer) RETURNS TABLE(user_uuid character varying, target_uuid character varying, val integer, traverse_owned boolean)
     LANGUAGE sql STABLE
     AS $$
-/* perm_origin_uuid: The object that 'gets' or 'has' the permission.
+
+/* The purpose of this function is to compute the permissions for a
+   subgraph of the database, starting from a given edge.  The newly
+   computed permissions are used to add and remove rows from the main
+   permissions table.
+
+   perm_origin_uuid: The object that 'gets' the permission.
 
    starting_uuid: The starting object the permission applies to.
 
@@ -53,40 +59,13 @@ CREATE FUNCTION public.compute_permission_subgraph(perm_origin_uuid character va
                   starting_uuid One of 1, 2, 3 for can_read,
                   can_write, can_manage respectively, or 0 to revoke
                   permissions.
-
-   This function is broken up into a number of clauses, described
-   below.
-
-   Note on query optimization:
-
-   Each clause in a "with" statement is called a "common table
-   expression" or CTE.
-
-   In Postgres, they are evaluated in sequence and results of each CTE
-   is stored in a temporary table.  This means Postgres does not
-   propagate constraints from later subqueries to earlier subqueries
-   when they are CTEs.
-
-   This is a problem if, for example, a later subquery chooses 10
-   items out of a set of 1000000 defined by an earlier subquery,
-   because it will always compute all 1000000 rows even if the query
-   on the 1000000 rows could have been constrained.  This is why
-   permission_graph_edges is a view -- views are inlined so and can be
-   optimized using external constraints.
-
-   The query optimizer does sort the temporary tables for later use in
-   joins.
-
-   Final note, this query would have been almost impossible to write
-   (and certainly impossible to read) without splitting it up using
-   SQL "with" but unfortunately it also stumbles into a frustrating
-   Postgres optimizer bug, see
-   lib/refresh_permission_view.rb#update_permissions
-   for details and a partial workaround.
 */
 with
-  /* Gets the initial set of objects potentially affected by the
-     permission change, using search_permission_graph.
+  /* Starting from starting_uuid, determine the set of objects that
+     could be affected by this permission change.
+
+     Note: We don't traverse users unless it is an "identity"
+     permission (permission origin is self).
   */
   perm_from_start(perm_origin_uuid, target_uuid, val, traverse_owned) as (
     
@@ -119,14 +98,21 @@ WITH RECURSIVE
         group by (traverse_graph.origin_uuid, target_uuid)
 ),
 
-  /* Finds other inbound edges that grant permissions on the objects
-     in perm_from_start, and computes permissions that originate from
-     those.  This is required to handle the case where there is more
-     than one path through which a user gets permission to an object.
-     For example, a user owns a project and also shares it can_read
-     with a group the user belongs to, adding the can_read link must
-     not overwrite the existing can_manage permission granted by
-     ownership.
+  /* Find other inbound edges that grant permissions to 'targets' in
+     perm_from_start, and compute permissions that originate from
+     those.
+
+     This is necessary for two reasons:
+
+       1) Other users may have access to a subset of the objects
+       through other permission links than the one we started from.
+       If we don't recompute them, their permission will get dropped.
+
+       2) There may be more than one path through which a user gets
+       permission to an object.  For example, a user owns a project
+       and also shares it can_read with a group the user belongs
+       to. adding the can_read link must not overwrite the existing
+       can_manage permission granted by ownership.
   */
   additional_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
     
@@ -164,7 +150,7 @@ WITH RECURSIVE
         group by (traverse_graph.origin_uuid, target_uuid)
 ),
 
-  /* Combines the permissions computed in the first two phases. */
+  /* Combine the permissions computed in the first two phases. */
   all_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
       select * from perm_from_start
     union all
@@ -177,30 +163,27 @@ WITH RECURSIVE
 
      Key insights:
 
-     * Permissions are transitive (with some special cases involving
-       users, this is controlled by the traverse_owned flag).
+     * For every group, the materialized_permissions lists all users
+       that can access to that group.
+
+     * The all_perms subquery has computed permissions on on a set of
+       objects for all inbound "origins", which are users or groups.
 
-     * A user object can only gain permissions via an inbound edge,
-       or appearing in the graph.
+     * Permissions through groups are transitive.
 
-     * The materialized_permissions table includes the permission
-       each user has on the tail end of each inbound edge.
+     We can infer:
 
-     * The all_perms subquery has permissions for each object in the
-       subgraph reachable from certain origin (tail end of an edge).
+     1) The materialized_permissions table declares that user X has permission N on group Y
+     2) The all_perms result has determined group Y has permission M on object Z
+     3) Therefore, user X has permission min(N, M) on object Z
 
-     * Therefore, for each user, we can compute user permissions on
-       each object in subgraph by determining the permission the user
-       has on each origin (tail end of an edge), joining that with the
-       perm_origin_uuid column of all_perms, and taking the least() of
-       the origin edge or all_perms val (because of the "least
-       permission on the path" rule).  If an object was reachable by
-       more than one path (appears with more than one origin), we take
-       the max() of the computed permissions.
+     This allows us to efficiently determine the set of users that
+     have permissions on the subset of objects, without having to
+     follow the chain of permission back up to find those users.
 
-     * Finally, because users always have permission on themselves, the
-       query also makes sure those permission rows are always
-       returned.
+     In addition, because users always have permission on themselves, this
+     query also makes sure those permission rows are always
+     returned.
   */
   select v.user_uuid, v.target_uuid, max(v.perm_level), bool_or(v.traverse_owned) from
     (select m.user_uuid,
@@ -218,23 +201,6 @@ WITH RECURSIVE
 $$;
 
 
---
--- Name: compute_trashed(); Type: FUNCTION; Schema: public; Owner: -
---
-
-CREATE FUNCTION public.compute_trashed() RETURNS TABLE(uuid character varying, trash_at timestamp without time zone)
-    LANGUAGE sql STABLE
-    AS $$
-/* Helper function to populate trashed_groups table. This starts with
-   each group owned by a user and computes the subtree under that
-   group to find any groups that are trashed.
-*/
-select ps.target_uuid as group_uuid, ps.trash_at from groups,
-  lateral project_subtree_with_trash_at(groups.uuid, groups.trash_at) ps
-  where groups.owner_uuid like '_____-tpzed-_______________'
-$$;
-
-
 --
 -- Name: project_subtree_with_trash_at(character varying, timestamp without time zone); Type: FUNCTION; Schema: public; Owner: -
 --
diff --git a/services/api/lib/20200501150153_permission_table_constants.rb b/services/api/lib/20200501150153_permission_table_constants.rb
new file mode 100644
index 000000000..acf992432
--- /dev/null
+++ b/services/api/lib/20200501150153_permission_table_constants.rb
@@ -0,0 +1,86 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+# These constants are used in both
+# db/migrate/20200501150153_permission_table and update_permissions
+#
+# This file allows them to be easily imported by both to avoid duplication.
+#
+# Don't mess with this!  Any changes will affect both the current
+# update_permissions and the past migration.  If you are tinkering
+# with the permission system and need to change how
+# PERM_QUERY_TEMPLATE, refresh_trashed or refresh_permissions works,
+# you should make a new file with your modified functions and have
+# update_permissions reference that file instead.
+
+PERMISSION_VIEW = "materialized_permissions"
+
+TRASHED_GROUPS = "trashed_groups"
+
+# We need to use this parameterized query in a few different places,
+# including as a subquery in a larger query.
+#
+# There's basically two options, the way I did this originally was to
+# put this in a postgres function and do a lateral join over it.
+# However, postgres functions impose an optimization barrier, and
+# possibly have other overhead with temporary tables, so I ended up
+# going with the brute force approach of inlining the whole thing.
+#
+# The two substitutions are "base_case" which determines the initial
+# set of permission origins and "override" which is used to ensure
+# that the new permission takes precedence over the one in the edges
+# table (but some queries don't need that.)
+#
+PERM_QUERY_TEMPLATE = %{
+WITH RECURSIVE
+        traverse_graph(origin_uuid, target_uuid, val, traverse_owned, starting_set) as (
+            %{base_case}
+          union
+            (select traverse_graph.origin_uuid,
+                    edges.head_uuid,
+                      least(edges.val,
+                            traverse_graph.val
+                            %{override}),
+                    should_traverse_owned(edges.head_uuid, edges.val),
+                    false
+             from permission_graph_edges as edges, traverse_graph
+             where traverse_graph.target_uuid = edges.tail_uuid
+             and (edges.tail_uuid like '_____-j7d0g-_______________' or
+                  traverse_graph.starting_set)))
+        select traverse_graph.origin_uuid, target_uuid, max(val) as val, bool_or(traverse_owned) as traverse_owned from traverse_graph
+        group by (traverse_graph.origin_uuid, target_uuid)
+}
+
+def refresh_trashed
+  ActiveRecord::Base.transaction do
+    ActiveRecord::Base.connection.execute("LOCK TABLE #{TRASHED_GROUPS}")
+    ActiveRecord::Base.connection.execute("DELETE FROM #{TRASHED_GROUPS}")
+
+    # Helper populate trashed_groups table. This starts with
+    #   each group owned by a user and computes the subtree under that
+    #   group to find any groups that are trashed.
+    ActiveRecord::Base.connection.execute(%{
+INSERT INTO #{TRASHED_GROUPS}
+select ps.target_uuid as group_uuid, ps.trash_at from groups,
+  lateral project_subtree_with_trash_at(groups.uuid, groups.trash_at) ps
+  where groups.owner_uuid like '_____-tpzed-_______________'
+})
+  end
+end
+
+def refresh_permissions
+  ActiveRecord::Base.transaction do
+    ActiveRecord::Base.connection.execute("LOCK TABLE #{PERMISSION_VIEW}")
+    ActiveRecord::Base.connection.execute("DELETE FROM #{PERMISSION_VIEW}")
+
+    ActiveRecord::Base.connection.execute %{
+INSERT INTO materialized_permissions
+    #{PERM_QUERY_TEMPLATE % {:base_case => %{
+        select uuid, uuid, 3, true, true from users
+},
+:override => ''
+} }
+}, "refresh_permission_view.do"
+  end
+end
diff --git a/services/api/lib/update_permissions.rb b/services/api/lib/update_permissions.rb
index 33a8d9736..4d3986a4b 100644
--- a/services/api/lib/update_permissions.rb
+++ b/services/api/lib/update_permissions.rb
@@ -2,32 +2,7 @@
 #
 # SPDX-License-Identifier: AGPL-3.0
 
-PERMISSION_VIEW = "materialized_permissions"
-TRASHED_GROUPS = "trashed_groups"
-
-def refresh_permissions
-  ActiveRecord::Base.transaction do
-    ActiveRecord::Base.connection.execute("LOCK TABLE #{PERMISSION_VIEW}")
-    ActiveRecord::Base.connection.execute("DELETE FROM #{PERMISSION_VIEW}")
-
-    ActiveRecord::Base.connection.execute %{
-INSERT INTO materialized_permissions
-    #{PERM_QUERY_TEMPLATE % {:base_case => %{
-        select uuid, uuid, 3, true, true from users
-},
-:override => ''
-} }
-}, "refresh_permission_view.do"
-  end
-end
-
-def refresh_trashed
-  ActiveRecord::Base.transaction do
-    ActiveRecord::Base.connection.execute("LOCK TABLE #{TRASHED_GROUPS}")
-    ActiveRecord::Base.connection.execute("DELETE FROM #{TRASHED_GROUPS}")
-    ActiveRecord::Base.connection.execute("INSERT INTO #{TRASHED_GROUPS} select * from compute_trashed()")
-  end
-end
+require '20200501150153_permission_table_constants'
 
 def update_permissions perm_origin_uuid, starting_uuid, perm_level
   #
@@ -203,22 +178,15 @@ def skip_check_permissions_against_full_refresh
   end
 end
 
-PERM_QUERY_TEMPLATE = %{
-WITH RECURSIVE
-        traverse_graph(origin_uuid, target_uuid, val, traverse_owned, starting_set) as (
-            %{base_case}
-          union
-            (select traverse_graph.origin_uuid,
-                    edges.head_uuid,
-                      least(edges.val,
-                            traverse_graph.val
-                            %{override}),
-                    should_traverse_owned(edges.head_uuid, edges.val),
-                    false
-             from permission_graph_edges as edges, traverse_graph
-             where traverse_graph.target_uuid = edges.tail_uuid
-             and (edges.tail_uuid like '_____-j7d0g-_______________' or
-                  traverse_graph.starting_set)))
-        select traverse_graph.origin_uuid, target_uuid, max(val) as val, bool_or(traverse_owned) as traverse_owned from traverse_graph
-        group by (traverse_graph.origin_uuid, target_uuid)
+# Used to account for permissions that a user gains by having
+# can_manage on another user.
+#
+# note: in theory a user could have can_manage access to a user
+# through multiple levels, that isn't handled here (would require a
+# recursive query).  I think that's okay because users getting
+# transitive access through "can_manage" on a user is is rarely/never
+# used feature and something we probably want to deprecate and remove.
+USER_UUIDS_SUBQUERY_TEMPLATE = %{
+select target_uuid from materialized_permissions where user_uuid in (%{user})
+and target_uuid like '_____-tpzed-_______________' and traverse_owned=true and perm_level >= %{perm_level}
 }

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list