From: Peter Amstutz <peter.amstutz@curii.com>
Date: Tue, 12 May 2020 16:52:50 +0000 (-0400)
Subject: 16007: Account for all the edges.  still wip.
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/26dc594d07456e81268740ee88865e166fde0d65

16007: Account for all the edges.  still wip.

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz@curii.com>
---

diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index f12f72520a..6dce3a42e0 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -78,6 +78,12 @@ class User < ArvadosModel
      {read: true, write: true},
      {read: true, write: true, manage: true}]
 
+  VAL_FOR_PERM =
+    {:read => 1,
+     :write => 2,
+     :manage => 3}
+
+
   def full_name
     "#{first_name} #{last_name}".strip
   end
@@ -89,7 +95,7 @@ class User < ArvadosModel
   end
 
   def groups_i_can(verb)
-    my_groups = self.group_permissions.select { |uuid, mask| mask[verb] }.keys
+    my_groups = self.group_permissions(VAL_FOR_PERM[verb]).keys
     if verb == :read
       my_groups << anonymous_group_uuid
     end
@@ -108,38 +114,25 @@ class User < ArvadosModel
         end
       end
       next if target_uuid == self.uuid
-      next if (group_permissions[target_uuid] and
-               group_permissions[target_uuid][action])
-      if target.respond_to? :owner_uuid
-        next if target.owner_uuid == self.uuid
-        next if (group_permissions[target.owner_uuid] and
-                 group_permissions[target.owner_uuid][action])
-      end
-      sufficient_perms = case action
-                         when :manage
-                           ['can_manage']
-                         when :write
-                           ['can_manage', 'can_write']
-                         when :read
-                           ['can_manage', 'can_write', 'can_read']
-                         else
-                           # (Skip this kind of permission opportunity
-                           # if action is an unknown permission type)
-                         end
-      if sufficient_perms
-        # Check permission links with head_uuid pointing directly at
-        # the target object. If target is a Group, this is redundant
-        # and will fail except [a] if permission caching is broken or
-        # [b] during a race condition, where a permission link has
-        # *just* been added.
-        if Link.where(link_class: 'permission',
-                      name: sufficient_perms,
-                      tail_uuid: groups_i_can(action) + [self.uuid],
-                      head_uuid: target_uuid).any?
-          next
-        end
+
+      target_owner_uuid = target.owner_uuid if target.respond_to? :owner_uuid
+
+      unless ActiveRecord::Base.connection.
+        exec_query(%{
+SELECT 1 FROM #{PERMISSION_VIEW}
+  WHERE user_uuid = $1 and
+        ((target_uuid = $2 and perm_level >= $3)
+         or (target_uuid = $4 and perm_level >= $3 and traverse_owned))
+},
+                  # "name" arg is a query label that appears in logs:
+                   "user_can_query",
+                   [[nil, self.uuid],
+                    [nil, target_uuid],
+                    [nil, VAL_FOR_PERM[action]],
+                    [nil, target_owner_uuid]]
+                  ).any?
+        return false
       end
-      return false
     end
     true
   end
@@ -194,16 +187,16 @@ from search_permission_graph('#{uuid}', 3) as g
     # 4. Upsert each permission in our subset (user, group, val)
 
     ## testinging
-#     puts "__ update_permissions __"
+    puts "\n__ update_permissions __"
 #     puts "What's in there now for #{starting_uuid}"
 #     printdump %{
 # select * from materialized_permissions where user_uuid='#{starting_uuid}'
 # }
 
-#     puts "search_permission_graph #{perm_origin_uuid} #{starting_uuid}, #{perm_level}"
-#     printdump %{
-# select '#{perm_origin_uuid}'::varchar as perm_origin_uuid, target_uuid, val, traverse_owned from search_permission_graph('#{starting_uuid}', #{perm_level})
-# }
+    puts "search_permission_graph #{perm_origin_uuid} #{starting_uuid}, #{perm_level}"
+    printdump %{
+select '#{perm_origin_uuid}'::varchar as perm_origin_uuid, target_uuid, val, traverse_owned from search_permission_graph('#{starting_uuid}', #{perm_level})
+}
 
 #     puts "other_links #{perm_origin_uuid} #{starting_uuid}, #{perm_level}"
 #     printdump %{
@@ -220,43 +213,53 @@ from search_permission_graph('#{uuid}', 3) as g
 #         links.head_uuid in (select target_uuid from perm_from_start)
 # }
 
-#     puts "additional_perms #{perm_origin_uuid} #{starting_uuid}, #{perm_level}"
-#     printdump %{
-# with
-# perm_from_start(perm_origin_uuid, target_uuid, val, traverse_owned) as (
-#   select '#{perm_origin_uuid}'::varchar, target_uuid, val, traverse_owned
-#     from search_permission_graph('#{starting_uuid}'::varchar, #{perm_level}))
-
-#     select links.tail_uuid as perm_origin_uuid, ps.target_uuid, ps.val, true
-#       from links, lateral search_permission_graph(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 ps
-#       where links.link_class='permission' and
-#         links.tail_uuid not in (select target_uuid from perm_from_start where traverse_owned) and
-#         links.tail_uuid != '#{perm_origin_uuid}' and
-#         links.head_uuid in (select target_uuid from perm_from_start)
-# }
+    puts "additional_perms #{perm_origin_uuid} #{starting_uuid}, #{perm_level}"
+    printdump %{
+with
+perm_from_start(perm_origin_uuid, target_uuid, val, traverse_owned) as (
+  select '#{perm_origin_uuid}'::varchar, target_uuid, val, traverse_owned
+    from search_permission_graph('#{starting_uuid}'::varchar, #{perm_level})),
+
+  edges(tail_uuid, head_uuid, val) as (
+        select * from permission_graph_edges()),
+
+  additional_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
+    select edges.tail_uuid as perm_origin_uuid, ps.target_uuid, ps.val,
+           should_traverse_owned(ps.target_uuid, ps.val)
+      from edges, lateral search_permission_graph(edges.head_uuid, edges.val) as ps
+      where (not (edges.tail_uuid = '#{perm_origin_uuid}' and
+                  edges.head_uuid = '#{starting_uuid}' and
+                  edges.val = #{perm_level})) and
+            edges.tail_uuid not in (select target_uuid from perm_from_start) and
+            edges.head_uuid in (select target_uuid from perm_from_start)),
+
+  partial_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
+      select * from perm_from_start
+    union
+      select * from additional_perms
+  ),
+
+  user_identity_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
+    select users.uuid as perm_origin_uuid, ps.target_uuid, ps.val, ps.traverse_owned
+      from users, lateral search_permission_graph(users.uuid, 3) as ps
+      where users.owner_uuid not in (select target_uuid from partial_perms where traverse_owned) and
+      users.uuid in (select target_uuid from partial_perms)
+  ),
+
+  all_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
+      select * from additional_perms
+    union
+      select * from user_identity_perms
+  )
+
+  select * from all_perms order by perm_origin_uuid, target_uuid
+}
 
-#     puts "Perms out"
-#     printdump %{
-# with
-# perm_from_start(perm_origin_uuid, target_uuid, val, traverse_owned) as (
-#   select  '#{perm_origin_uuid}'::varchar, target_uuid, val, traverse_owned
-#     from search_permission_graph('#{starting_uuid}', #{perm_level}))
-
-# (select materialized_permissions.user_uuid, u.target_uuid, max(least(materialized_permissions.perm_level, u.val)), bool_or(u.traverse_owned)
-#   from perm_from_start as u
-#   join materialized_permissions on (u.perm_origin_uuid = materialized_permissions.target_uuid)
-#   where materialized_permissions.traverse_owned
-#   group by materialized_permissions.user_uuid, u.target_uuid)
-# union
-#   select target_uuid as user_uuid, target_uuid, 3, true
-#     from perm_from_start where target_uuid like '_____-tpzed-_______________'
-# }
+    puts "Perms out"
+    printdump %{
+select * from compute_permission_subgraph('#{perm_origin_uuid}', '#{starting_uuid}', '#{perm_level}')
+order by user_uuid, target_uuid
+}
     ## end
 
     temptable_perms = "temp_perms_#{rand(2**64).to_s(10)}"
@@ -293,24 +296,50 @@ on conflict (user_uuid, target_uuid) do update set perm_level=EXCLUDED.perm_leve
 
     # for testing only - make a copy of the table and compare it to the one generated
     # using a full permission recompute
-#     temptable_compare = "compare_perms_#{rand(2**64).to_s(10)}"
-#     ActiveRecord::Base.connection.exec_query %{
-# create temporary table #{temptable_compare} on commit drop as select * from materialized_permissions
-# }
 
-    # Ensure a new group can be accessed by the appropriate users
-    # immediately after being created.
-    #User.invalidate_permissions_cache
+#      puts "dump--"
+#      User.printdump %{
+#  select * from users
+#      }
+#      User.printdump %{
+#  select * from groups
+#      }
+#      User.printdump %{
+# select * from search_permission_graph('zzzzz-tpzed-000000000000000', 3)
+#      }
+#      puts "--"
+
+
+    q1 = ActiveRecord::Base.connection.exec_query %{
+select user_uuid, target_uuid, perm_level, traverse_owned from materialized_permissions
+order by user_uuid, target_uuid
+}
 
-#     q1 = ActiveRecord::Base.connection.exec_query %{
-# select count(*) from materialized_permissions
-# }
-#     puts "correct version #{q1.first}"
+    q2 = ActiveRecord::Base.connection.exec_query %{
+select users.uuid as user_uuid, g.target_uuid, g.val as perm_level, g.traverse_owned
+from users, lateral search_permission_graph(users.uuid, 3) as g where g.val > 0
+order by users.uuid, target_uuid
+}
 
-#     q2 = ActiveRecord::Base.connection.exec_query %{
-# select count(*) from #{temptable_compare}
-# }
-#     puts "incremental update #{q2.first}"
+    if q1.count != q2.count
+      puts "Didn't match incremental+: #{q1.count} != full refresh-: #{q2.count}"
+    end
+
+    if q1.count > q2.count
+      q1.each_with_index do |r, i|
+        if r != q2[i]
+          puts "+#{r}\n-#{q2[i]}"
+          raise "Didn't match"
+        end
+      end
+    else
+      q2.each_with_index do |r, i|
+        if r != q1[i]
+          puts "+#{q1[i]}\n-#{r}"
+          raise "Didn't match"
+        end
+      end
+    end
   end
 
   # Return a hash of {user_uuid: group_perms}
@@ -321,8 +350,8 @@ on conflict (user_uuid, target_uuid) do update set perm_level=EXCLUDED.perm_leve
                   FROM #{PERMISSION_VIEW}
                   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|
+                 "all_group_permissions").
+      rows.each do |user_uuid, group_uuid, max_p_val|
       all_perms[user_uuid] ||= {}
       all_perms[user_uuid][group_uuid] = PERMS_FOR_VAL[max_p_val.to_i]
     end
@@ -332,18 +361,20 @@ on conflict (user_uuid, target_uuid) do update set perm_level=EXCLUDED.perm_leve
   # Return a hash of {group_uuid: perm_hash} where perm_hash[:read]
   # and perm_hash[:write] are true if this user can read and write
   # objects owned by group_uuid.
-  def group_permissions
-    group_perms = {self.uuid => {:read => true, :write => true, :manage => true}}
+  def group_permissions(level=1)
+    group_perms = {}
     ActiveRecord::Base.connection.
-      exec_query("SELECT target_uuid, perm_level
-                  FROM #{PERMISSION_VIEW}
-                  WHERE user_uuid = $1
-                  AND traverse_owned",
+      exec_query(%{
+SELECT target_uuid, perm_level
+  FROM #{PERMISSION_VIEW}
+  WHERE user_uuid = $1 and perm_level >= $2
+},
                   # "name" arg is a query label that appears in logs:
                   "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|
+                  [[nil, uuid],
+                   [nil, level]]).
+      rows.each do |group_uuid, max_p_val|
       group_perms[group_uuid] = PERMS_FOR_VAL[max_p_val.to_i]
     end
     group_perms
diff --git a/services/api/db/migrate/20200501150153_permission_table.rb b/services/api/db/migrate/20200501150153_permission_table.rb
index 438851afb2..d3afd4a749 100644
--- a/services/api/db/migrate/20200501150153_permission_table.rb
+++ b/services/api/db/migrate/20200501150153_permission_table.rb
@@ -61,6 +61,42 @@ $$;
 
         ActiveRecord::Base.connection.execute("INSERT INTO trashed_groups select * from compute_trashed()")
 
+        ActiveRecord::Base.connection.execute %{
+create or replace function should_traverse_owned (starting_uuid varchar(27),
+                                                  starting_perm integer)
+  returns bool
+STABLE
+language SQL
+as $$
+select starting_uuid like '_____-j7d0g-_______________' or
+       (starting_uuid like '_____-tpzed-_______________' and starting_perm >= 3);
+$$;
+}
+
+
+        ActiveRecord::Base.connection.execute %{
+create or replace function permission_graph_edges ()
+  returns table (tail_uuid varchar(27), head_uuid varchar(27), val integer)
+STABLE
+language SQL
+as $$
+           select groups.owner_uuid, groups.uuid, (3) from groups
+          union
+            select users.owner_uuid, users.uuid, (3) from users
+          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'
+$$;
+}
+
         # Get a set of permission by searching the graph and following
         # ownership and permission links.
         #
@@ -79,36 +115,42 @@ 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'
+          select * from permission_graph_edges()
         ),
         traverse_graph(target_uuid, val, traverse_owned) as (
             values (starting_uuid, starting_perm,
-                     (starting_uuid like '_____-j7d0g-_______________' or
-                      (starting_uuid like '_____-tpzed-_______________' and starting_perm >= 3)))
+                    should_traverse_owned(starting_uuid, starting_perm))
           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))
+                    least(edges.val, traverse_graph.val,
+                          case traverse_graph.traverse_owned
+                            when true then null
+                            else 0
+                          end),
+                    should_traverse_owned(edges.head_uuid, edges.val)
              from edges
-             join traverse_graph on (traverse_graph.target_uuid = edges.tail_uuid)
-             where traverse_graph.traverse_owned))
+             join traverse_graph on (traverse_graph.target_uuid = edges.tail_uuid)))
         select target_uuid, max(val), bool_or(traverse_owned) from traverse_graph
         group by (target_uuid) ;
 $$;
 }
 
+
+  # owned_by_user_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
+  #   select users.owner_uuid as perm_origin_uuid, u.target_uuid, u.val, u.traverse_owned
+  #     from users, lateral search_permission_graph(users.uuid, 3) as u
+  #     where users.owner_uuid not in (select target_uuid from perm_from_start) and
+  #           users.uuid in (select target_uuid from perm_from_start)
+  # ),
+
+  # owned_by_group_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
+  #   select groups.owner_uuid as perm_origin_uuid, groups.uuid, 3, true
+  #     from groups
+  #     where groups.owner_uuid not in (select target_uuid from perm_from_start) and
+  #           groups.uuid in (select target_uuid from perm_from_start)
+  # ),
+
+
         ActiveRecord::Base.connection.execute %{
 create or replace function compute_permission_subgraph (perm_origin_uuid varchar(27),
                                                         starting_uuid varchar(27),
@@ -122,33 +164,36 @@ perm_from_start(perm_origin_uuid, target_uuid, val, traverse_owned) as (
   select perm_origin_uuid, target_uuid, val, traverse_owned
     from search_permission_graph(starting_uuid, starting_perm)),
 
-  link_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
-    select links.tail_uuid as perm_origin_uuid, ps.target_uuid, ps.val, true
-      from links, lateral search_permission_graph(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 ps
-      where links.link_class='permission' and
-        (not (links.tail_uuid = perm_origin_uuid and links.head_uuid = starting_uuid)) and
-        links.tail_uuid not in (select target_uuid from perm_from_start where traverse_owned) and
-        links.head_uuid in (select target_uuid from perm_from_start)),
-
-  user_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
-    select x.target_uuid as perm_origin_uuid, y.target_uuid, y.val, y.traverse_owned
-      from (select * from perm_from_start union select * from link_perms) as x,
-      lateral search_permission_graph(x.target_uuid, 3) as y
-      where x.target_uuid like '_____-tpzed-_______________'
-  ),
+  edges(tail_uuid, head_uuid, val) as (
+        select * from permission_graph_edges()),
 
-  all_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
+  additional_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
+    select edges.tail_uuid as perm_origin_uuid, ps.target_uuid, ps.val,
+           should_traverse_owned(ps.target_uuid, ps.val)
+      from edges, lateral search_permission_graph(edges.head_uuid, edges.val) as ps
+      where (not (edges.tail_uuid = perm_origin_uuid and
+                 edges.head_uuid = starting_uuid and
+                 edges.val = starting_perm)) and
+            edges.tail_uuid not in (select target_uuid from perm_from_start) and
+            edges.head_uuid in (select target_uuid from perm_from_start)),
+
+  partial_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
       select * from perm_from_start
     union
-      select * from link_perms
+      select * from additional_perms
+  ),
+
+  user_identity_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
+    select users.uuid as perm_origin_uuid, ps.target_uuid, ps.val, ps.traverse_owned
+      from users, lateral search_permission_graph(users.uuid, 3) as ps
+      where users.owner_uuid not in (select target_uuid from partial_perms where traverse_owned) and
+      users.uuid in (select target_uuid from partial_perms)
+  ),
+
+  all_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
+      select * from partial_perms
     union
-      select * from user_perms
+      select * from user_identity_perms
   )
 
   select v.user_uuid, v.target_uuid, max(v.perm_level), bool_or(v.traverse_owned) from
@@ -179,5 +224,7 @@ $$;
     ActiveRecord::Base.connection.execute "DROP function compute_trashed ();"
     ActiveRecord::Base.connection.execute "DROP function search_permission_graph(varchar, integer);"
     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 function permission_graph_edges();"
   end
 end
diff --git a/services/api/lib/refresh_permission_view.rb b/services/api/lib/refresh_permission_view.rb
index 724e65dbe5..8160502b9a 100644
--- a/services/api/lib/refresh_permission_view.rb
+++ b/services/api/lib/refresh_permission_view.rb
@@ -12,7 +12,7 @@ def do_refresh_permission_view
     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
+from users, lateral search_permission_graph(users.uuid, 3) as g where g.val > 0
 }
   end
 end
diff --git a/services/api/test/fixtures/users.yml b/services/api/test/fixtures/users.yml
index 57633a3120..14630d9efa 100644
--- a/services/api/test/fixtures/users.yml
+++ b/services/api/test/fixtures/users.yml
@@ -418,3 +418,17 @@ double_redirects_to_active:
       organization: example.com
       role: Computational biologist
     getting_started_shown: 2015-03-26 12:34:56.789000000 Z
+
+has_can_login_permission:
+  owner_uuid: zzzzz-tpzed-000000000000000
+  uuid: zzzzz-tpzed-xabcdjxw79nv3jz
+  email: can-login-user@arvados.local
+  modified_by_client_uuid: zzzzz-ozdt8-teyxzyd8qllg11h
+  modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  first_name: Can_login
+  last_name: User
+  identity_url: https://can-login-user.openid.local
+  is_active: true
+  is_admin: false
+  modified_at: 2015-03-26 12:34:56.789000000 Z
+  username: can-login-user