From ce043d3f68376f6658d1bf401a6bf5cf00e4a221 Mon Sep 17 00:00:00 2001 From: Peter Amstutz Date: Wed, 6 May 2020 23:01:18 -0400 Subject: [PATCH] 16007: initialize materialized_permissions from search_permission_graph Arvados-DCO-1.1-Signed-off-by: Peter Amstutz --- services/api/app/models/arvados_model.rb | 2 +- services/api/app/models/group.rb | 39 ++++++++ services/api/app/models/user.rb | 10 +-- .../20200501150153_permission_table.rb | 88 ++++++++++++++----- services/api/lib/refresh_permission_view.rb | 18 +++- services/api/test/test_helper.rb | 1 + 6 files changed, 127 insertions(+), 31 deletions(-) diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb index 83b9a6e104..150f462127 100644 --- a/services/api/app/models/arvados_model.rb +++ b/services/api/app/models/arvados_model.rb @@ -324,7 +324,7 @@ class ArvadosModel < ApplicationRecord owner_check = "" if sql_table != "api_client_authorizations" and sql_table != "groups" then owner_check = "OR #{sql_table}.owner_uuid IN (SELECT target_uuid FROM #{PERMISSION_VIEW} "+ - "WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 #{trashed_check} AND target_owner_uuid IS NOT NULL) " + "WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 #{trashed_check} AND traverse_owned) " end links_cond = "" diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb index 3292642d48..cc81b3fab9 100644 --- a/services/api/app/models/group.rb +++ b/services/api/app/models/group.rb @@ -70,6 +70,7 @@ insert into trashed_groups (group_uuid, trash_at) on conflict (group_uuid) do update set trash_at=EXCLUDED.trash_at; } end + if uuid_changed? or owner_uuid_changed? # This can change users' permissions on other groups as well as # this one. @@ -81,6 +82,44 @@ on conflict (group_uuid) do update set trash_at=EXCLUDED.trash_at; # Ensure a new group can be accessed by the appropriate users # immediately after being created. User.invalidate_permissions_cache self.async_permissions_update + + if new_record? or owner_uuid_changed? + # 1. Compute set (group, permission) implied by traversing + # graph starting at this group + # 2. Find links from outside the graph that point inside + # 3. We now have the full set of permissions for a subset of groups. + # 3. Upsert each permission in our subset (user, group, val) + # 4. Delete permissions from table not in our computed subset. + + temptable_perm_from_start = "perm_from_start_#{rand(2**64).to_s(10)}" + ActiveRecord::Base.connection.exec_query %{ +create temporary table #{temptable_perm_from_start} on commit drop +as select $1::varchar as starting_uuid, target_uuid, val +from search_permission_graph($1::varchar, 3::smallint) +}, + 'Group.search_permissions', + [[nil, self.uuid]] + + temptable_additional_perms = "additional_perms_#{rand(2**64).to_s(10)}" + ActiveRecord::Base.connection.exec_query %{ +create temporary table #{temptable_additional_perms} on commit drop +as select links.tail_uuid as starting_uuid, ps.target_uuid, ps.val + from links, lateral search_permission_graph(links.head_uuid::varchar, CASE +WHEN links.name = 'can_read' THEN 1::smallint +WHEN links.name = 'can_login' THEN 1::smallint +WHEN links.name = 'can_write' THEN 2::smallint +WHEN links.name = 'can_manage' THEN 3::smallint +END) as ps +where links.link_class='permission' and + links.tail_uuid not in (select target_uuid from #{temptable_perm_from_start}) and + links.head_uuid in (select target_uuid from #{temptable_perm_from_start}) +} + + q1 = ActiveRecord::Base.connection.exec_query "select * from #{temptable_perm_from_start}" + q1.each do |r| + #puts r + end + end end def assign_name diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb index a5fc12d0af..a344919667 100644 --- a/services/api/app/models/user.rb +++ b/services/api/app/models/user.rb @@ -155,9 +155,9 @@ class User < ArvadosModel def self.all_group_permissions all_perms = {} ActiveRecord::Base.connection. - exec_query("SELECT user_uuid, target_owner_uuid, perm_level + exec_query("SELECT user_uuid, target_uuid, perm_level FROM #{PERMISSION_VIEW} - WHERE target_owner_uuid IS NOT NULL", + WHERE traverse_owned", # "name" arg is a query label that appears in logs: "all_group_permissions", ).rows.each do |user_uuid, group_uuid, max_p_val| @@ -173,12 +173,12 @@ class User < ArvadosModel def group_permissions group_perms = {self.uuid => {:read => true, :write => true, :manage => true}} ActiveRecord::Base.connection. - exec_query("SELECT target_owner_uuid, perm_level + exec_query("SELECT target_uuid, perm_level FROM #{PERMISSION_VIEW} WHERE user_uuid = $1 - AND target_owner_uuid IS NOT NULL", + AND traverse_owned", # "name" arg is a query label that appears in logs: - "group_permissions for #{uuid}", + "group_permissions_for_user", # "binds" arg is an array of [col_id, value] for '$1' vars: [[nil, uuid]], ).rows.each do |group_uuid, max_p_val| diff --git a/services/api/db/migrate/20200501150153_permission_table.rb b/services/api/db/migrate/20200501150153_permission_table.rb index 20b715df4d..00f9bde912 100644 --- a/services/api/db/migrate/20200501150153_permission_table.rb +++ b/services/api/db/migrate/20200501150153_permission_table.rb @@ -4,7 +4,7 @@ class PermissionTable < ActiveRecord::Migration[5.0] t.string :user_uuid t.string :target_uuid t.integer :perm_level - t.string :target_owner_uuid + t.boolean :traverse_owned end ActiveRecord::Base.connection.execute %{ @@ -12,7 +12,7 @@ create or replace function compute_permission_table () returns table(user_uuid character varying (27), target_uuid character varying (27), perm_level smallint, - target_owner_uuid character varying(27)) + traverse_owned bool) VOLATILE language SQL as $$ @@ -31,7 +31,7 @@ as $$ SELECT groups.owner_uuid, groups.uuid, 3, - true AS bool + true FROM public.groups ), perm(val, follow, user_uuid, target_uuid) AS ( SELECT (3)::smallint AS val, @@ -50,16 +50,9 @@ as $$ SELECT perm.user_uuid, perm.target_uuid, max(perm.val) AS perm_level, - CASE perm.follow - WHEN true THEN perm.target_uuid - ELSE NULL::character varying - END AS target_owner_uuid + bool_or(perm.follow) as traverse_owned FROM perm - GROUP BY perm.user_uuid, perm.target_uuid, - CASE perm.follow - WHEN true THEN perm.target_uuid - ELSE NULL::character varying - END + GROUP BY perm.user_uuid, perm.target_uuid $$; } @@ -70,12 +63,12 @@ STABLE language SQL as $$ WITH RECURSIVE - project_subtree(uuid) as ( - values (starting_uuid) - union - select groups.uuid from groups join project_subtree on (groups.owner_uuid = project_subtree.uuid) - ) - select uuid from project_subtree; + project_subtree(uuid) as ( + values (starting_uuid) + union + select groups.uuid from groups join project_subtree on (groups.owner_uuid = project_subtree.uuid) + ) + select uuid from project_subtree; $$; } @@ -86,13 +79,13 @@ STABLE language SQL as $$ WITH RECURSIVE - project_subtree(uuid, trash_at) as ( - values (starting_uuid, starting_trash_at) - union - select groups.uuid, LEAST(project_subtree.trash_at, groups.trash_at) + project_subtree(uuid, trash_at) as ( + values (starting_uuid, starting_trash_at) + union + select groups.uuid, LEAST(project_subtree.trash_at, groups.trash_at) from groups join project_subtree on (groups.owner_uuid = project_subtree.uuid) - ) - select uuid, trash_at from project_subtree; + ) + select uuid, trash_at from project_subtree; $$; } @@ -112,6 +105,53 @@ 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-_______________' $$; +} + + ActiveRecord::Base.connection.execute("INSERT INTO trashed_groups select * from compute_trashed()") + + # Get a set of permission by searching the graph and following + # ownership and permission links. + # + # edges() - a subselect with the union of ownership and permission links + # + # traverse_graph() - recursive query, from the starting node, + # self-join with edges to find outgoing permissions. + # Re-runs the query on new rows until there are no more results. + # This accomplishes a breadth-first search of the permission graph. + # + ActiveRecord::Base.connection.execute %{ +create or replace function search_permission_graph (starting_uuid varchar(27), starting_perm integer) +returns table (target_uuid varchar(27), val integer, traverse_owned bool) +STABLE +language SQL +as $$ +WITH RECURSIVE edges(tail_uuid, head_uuid, val) as ( + select groups.owner_uuid, groups.uuid, (3) from groups + union + select links.tail_uuid, + links.head_uuid, + CASE + WHEN links.name = 'can_read' THEN 1 + WHEN links.name = 'can_login' THEN 1 + WHEN links.name = 'can_write' THEN 2 + WHEN links.name = 'can_manage' THEN 3 + END as val + from links + where links.link_class='permission' + ), + traverse_graph(target_uuid, val, traverse_owned) as ( + values (starting_uuid, starting_perm, true) + union + (select edges.head_uuid, + least(edges.val, traverse_graph.val), + (edges.head_uuid like '_____-j7d0g-_______________' or + (edges.head_uuid like '_____-tpzed-_______________' and edges.val >= 3)) + from edges + join traverse_graph on (traverse_graph.target_uuid = edges.tail_uuid) + where traverse_graph.traverse_owned)) + select target_uuid, max(val), bool_or(traverse_owned) from traverse_graph + group by (target_uuid) ; +$$; } ActiveRecord::Base.connection.execute "DROP MATERIALIZED VIEW IF EXISTS materialized_permission_view;" diff --git a/services/api/lib/refresh_permission_view.rb b/services/api/lib/refresh_permission_view.rb index 5ee8a7b449..f6642192a7 100644 --- a/services/api/lib/refresh_permission_view.rb +++ b/services/api/lib/refresh_permission_view.rb @@ -8,8 +8,24 @@ TRASHED_GROUPS = "trashed_groups" def do_refresh_permission_view ActiveRecord::Base.transaction do ActiveRecord::Base.connection.execute("LOCK TABLE permission_refresh_lock") + # ActiveRecord::Base.connection.execute("DELETE FROM #{PERMISSION_VIEW}") + # ActiveRecord::Base.connection.execute("INSERT INTO #{PERMISSION_VIEW} select * from compute_permission_table()") + + # puts "do_refresh_permission_view1" + # ActiveRecord::Base.connection.exec_query("select * from materialized_permissions order by user_uuid, target_uuid, perm_level").each do |r| + # puts "d: #{r}" + # end + ActiveRecord::Base.connection.execute("DELETE FROM #{PERMISSION_VIEW}") - ActiveRecord::Base.connection.execute("INSERT INTO #{PERMISSION_VIEW} select * from compute_permission_table()") + ActiveRecord::Base.connection.execute %{ +INSERT INTO #{PERMISSION_VIEW} +select users.uuid, g.target_uuid, g.val, g.traverse_owned +from users, lateral search_permission_graph(users.uuid, 3) as g +} + # puts "do_refresh_permission_view2" + # ActiveRecord::Base.connection.exec_query("select * from materialized_permissions order by user_uuid, target_uuid, perm_level").each do |r| + # puts "d: #{r}" + # end end end diff --git a/services/api/test/test_helper.rb b/services/api/test/test_helper.rb index a5bd142b90..cee618557e 100644 --- a/services/api/test/test_helper.rb +++ b/services/api/test/test_helper.rb @@ -61,6 +61,7 @@ class ActiveSupport::TestCase include CurrentApiClient setup do + do_refresh_permission_view ActiveRecord::Base.connection.execute("DELETE FROM #{TRASHED_GROUPS}") ActiveRecord::Base.connection.execute("INSERT INTO #{TRASHED_GROUPS} select * from compute_trashed()") end -- 2.30.2