[ARVADOS] updated: 1.3.0-2538-g34481779f
Git user
git at public.arvados.org
Sat May 9 04:29:03 UTC 2020
Summary of changes:
services/api/app/models/arvados_model.rb | 12 +-
services/api/app/models/group.rb | 71 +++--------
services/api/app/models/link.rb | 31 +++--
services/api/app/models/user.rb | 142 ++++++++++++++++++++-
.../db/migrate/20200501150153_permission_table.rb | 115 +++++++++--------
services/api/lib/refresh_permission_view.rb | 12 --
services/api/test/performance/permission_test.rb | 3 +-
services/api/test/test_helper.rb | 12 +-
services/api/test/unit/collection_test.rb | 13 ++
9 files changed, 257 insertions(+), 154 deletions(-)
via 34481779fc2a8df4112d20afaf1f8a0d8fa0d4a3 (commit)
via e821bb285c3a004c6500ea5ff75582795d06189c (commit)
via 100c0763a0d94e944eb24b7a2900b6e58e2cd516 (commit)
via 6f8d1535cb6b1c12b816a47e5ec7d8f8afa9011e (commit)
from ce043d3f68376f6658d1bf401a6bf5cf00e4a221 (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 34481779fc2a8df4112d20afaf1f8a0d8fa0d4a3
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Sat May 9 00:28:28 2020 -0400
16007: Still WIP trying to figure out what to do with links to users
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index 3ba7c4b96..d9ef342b3 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -21,7 +21,7 @@ class Group < ArvadosModel
after_create :update_permissions
after_create :update_trash
- after_update :update_permissions
+ after_update :update_permissions, :if => :owner_uuid_changed?
after_update :update_trash
after_destroy :clear_permissions_and_trash
@@ -45,7 +45,7 @@ class Group < ArvadosModel
end
def update_trash
- if trash_at_changed? or owner_uuid_changed? or (new_record? and !trash_at.nil?)
+ if trash_at_changed? or owner_uuid_changed?
# The group was added or removed from the trash.
#
# Strategy:
@@ -76,9 +76,7 @@ on conflict (group_uuid) do update set trash_at=EXCLUDED.trash_at;
end
def update_permissions
- if new_record? or owner_uuid_changed?
- User.update_permissions self.owner_uuid, self.uuid, 3
- end
+ User.update_permissions self.owner_uuid, self.uuid, 3
end
def clear_permissions_and_trash
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index be130a99d..d2ca8102e 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -40,7 +40,7 @@ class User < ArvadosModel
(user.uuid != anonymous_user_uuid)
}
after_create :send_admin_notifications
- after_update :update_permissions
+ after_update :update_permissions, :if => :owner_uuid_changed?
after_update :send_profile_created_notification
after_update :sync_repository_names, :if => Proc.new { |user|
(user.uuid != system_user_uuid) and
@@ -145,17 +145,19 @@ class User < ArvadosModel
end
def update_permissions
- if owner_uuid_changed?
-# puts "Update permissions for #{uuid} #{new_record?}"
-# User.printdump %{
-# select * from materialized_permissions where user_uuid='#{uuid}'
-# }
-# puts "---"
+
+ puts "Update permissions for #{uuid}"
+ User.printdump %{
+select * from materialized_permissions where user_uuid='#{uuid}'
+}
+ puts "---"
User.update_permissions self.owner_uuid, self.uuid, 3
-# User.printdump %{
-#select * from materialized_permissions where user_uuid='#{uuid}'
-#}
- end
+
+ puts "post-update"
+ User.printdump %{
+select * from materialized_permissions where user_uuid='#{uuid}'
+}
+ puts "<<<"
end
def self.printdump qr
@@ -192,15 +194,16 @@ from search_permission_graph('#{uuid}', 3) as g
# 4. Upsert each permission in our subset (user, group, val)
## testinging
-# puts "What's in there now for #{starting_uuid}"
-# printdump %{
-# select * from materialized_permissions where user_uuid='#{starting_uuid}'
-# }
+ puts "__ 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 "Perms out"
# printdump %{
@@ -233,10 +236,11 @@ as select * from compute_permission_subgraph($1, $2, $3)
q1 = ActiveRecord::Base.connection.exec_query %{
select * from #{temptable_perms}
}
- # puts "recomputed perms was #{perm_origin_uuid} #{starting_uuid}, #{perm_level}"
- # q1.each do |r|
- # puts r
- # end
+ puts "recomputed perms was #{perm_origin_uuid} #{starting_uuid}, #{perm_level}"
+ q1.each do |r|
+ puts r
+ end
+ puts "<<<<"
ActiveRecord::Base.connection.exec_query %{
delete from materialized_permissions where
diff --git a/services/api/db/migrate/20200501150153_permission_table.rb b/services/api/db/migrate/20200501150153_permission_table.rb
index aa36df176..07c155517 100644
--- a/services/api/db/migrate/20200501150153_permission_table.rb
+++ b/services/api/db/migrate/20200501150153_permission_table.rb
@@ -93,7 +93,9 @@ WITH RECURSIVE edges(tail_uuid, head_uuid, val) as (
where links.link_class='permission'
),
traverse_graph(target_uuid, val, traverse_owned) as (
- values (starting_uuid, starting_perm, true)
+ values (starting_uuid, starting_perm,
+ (starting_uuid like '_____-j7d0g-_______________' or
+ (starting_uuid like '_____-tpzed-_______________' and starting_perm >= 3)))
union
(select edges.head_uuid,
least(edges.val, traverse_graph.val),
@@ -131,19 +133,28 @@ perm_from_start(perm_origin_uuid, target_uuid, val, traverse_owned) as (
END) as ps
where links.link_class='permission' and
links.tail_uuid not in (select target_uuid from perm_from_start) and
- links.head_uuid in (select target_uuid from perm_from_start))
+ links.head_uuid in (select target_uuid from perm_from_start)),
-select materialized_permissions.user_uuid,
- u.target_uuid,
- max(least(u.val, materialized_permissions.perm_level)),
- bool_or(u.traverse_owned)
- from ((select * from perm_from_start) union (select * from additional_perms)) 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-_______________'
+ identity_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
+ select target_uuid as perm_origin_uuid, target_uuid, 3, true
+ from perm_from_start where target_uuid like '_____-tpzed-_______________'),
+
+ all_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
+ select * from perm_from_start
+ union
+ select * from additional_perms
+ union
+ select * from identity_perms
+ )
+
+ select materialized_permissions.user_uuid,
+ u.target_uuid,
+ max(least(u.val, materialized_permissions.perm_level)),
+ bool_or(u.traverse_owned)
+ from all_perms as u
+ join materialized_permissions on (u.perm_origin_uuid = materialized_permissions.target_uuid)
+ where materialized_permissions.traverse_owned
+ group by user_uuid, u.target_uuid
$$;
}
commit e821bb285c3a004c6500ea5ff75582795d06189c
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Fri May 8 22:23:54 2020 -0400
16007: Getting closer to a thing that works
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 7edad3ecb..be130a99d 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -146,16 +146,15 @@ class User < ArvadosModel
def update_permissions
if owner_uuid_changed?
- puts "Update permissions for #{uuid} #{new_record?}"
- User.printdump %{
-select * from materialized_permissions where user_uuid='#{uuid}'
-}
- puts "---"
+# puts "Update permissions for #{uuid} #{new_record?}"
+# User.printdump %{
+# select * from materialized_permissions where user_uuid='#{uuid}'
+# }
+# puts "---"
User.update_permissions self.owner_uuid, self.uuid, 3
- User.printdump %{
-select * from materialized_permissions where user_uuid='#{uuid}'
-}
-
+# User.printdump %{
+#select * from materialized_permissions where user_uuid='#{uuid}'
+#}
end
end
@@ -166,6 +165,15 @@ select * from materialized_permissions where user_uuid='#{uuid}'
end
end
+ def recompute_permissions
+ ActiveRecord::Base.connection.execute("DELETE FROM #{PERMISSION_VIEW} where user_uuid='#{uuid}'")
+ ActiveRecord::Base.connection.execute %{
+INSERT INTO #{PERMISSION_VIEW}
+select '#{uuid}', g.target_uuid, g.val, g.traverse_owned
+from search_permission_graph('#{uuid}', 3) as g
+}
+ end
+
def self.update_permissions perm_origin_uuid, starting_uuid, perm_level
# Update a subset of the permission graph
# perm_level is the inherited permission
@@ -184,32 +192,32 @@ select * from materialized_permissions where user_uuid='#{uuid}'
# 4. Upsert each permission in our subset (user, group, val)
## testinging
- puts "What's in there now for #{starting_uuid}"
- printdump %{
-select * from materialized_permissions where user_uuid='#{starting_uuid}'
-}
+# 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 "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 %{
+# 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-_______________'
+# }
## end
temptable_perms = "temp_perms_#{rand(2**64).to_s(10)}"
@@ -225,10 +233,10 @@ as select * from compute_permission_subgraph($1, $2, $3)
q1 = ActiveRecord::Base.connection.exec_query %{
select * from #{temptable_perms}
}
- puts "recomputed perms was #{perm_origin_uuid} #{starting_uuid}, #{perm_level}"
- q1.each do |r|
- puts r
- end
+ # puts "recomputed perms was #{perm_origin_uuid} #{starting_uuid}, #{perm_level}"
+ # q1.each do |r|
+ # puts r
+ # end
ActiveRecord::Base.connection.exec_query %{
delete from materialized_permissions where
@@ -442,8 +450,6 @@ on conflict (user_uuid, target_uuid) do update set perm_level=EXCLUDED.perm_leve
raise "user does not exist" if !new_user
raise "cannot merge to an already merged user" if new_user.redirect_to_user_uuid
- User.update_permissions self.owner_uuid, self.uuid, 0
-
# If 'self' is a remote user, don't transfer authorizations
# (i.e. ability to access the account) to the new user, because
# that gives the remote site the ability to access the 'new'
@@ -518,8 +524,8 @@ on conflict (user_uuid, target_uuid) do update set perm_level=EXCLUDED.perm_leve
if redirect_to_new_user
update_attributes!(redirect_to_user_uuid: new_user.uuid, username: nil)
end
- User.update_permissions self.owner_uuid, self.uuid, 3
- User.update_permissions new_user.owner_uuid, new_user.uuid, 3
+ self.recompute_permissions
+ new_user.recompute_permissions
end
end
@@ -777,7 +783,7 @@ on conflict (user_uuid, target_uuid) do update set perm_level=EXCLUDED.perm_leve
# add the user to the 'All users' group
def create_user_group_link
- puts "In create_user_group_link"
+ #puts "In create_user_group_link"
return (Link.where(tail_uuid: self.uuid,
head_uuid: all_users_group[:uuid],
link_class: 'permission',
diff --git a/services/api/db/migrate/20200501150153_permission_table.rb b/services/api/db/migrate/20200501150153_permission_table.rb
index 425a9505f..aa36df176 100644
--- a/services/api/db/migrate/20200501150153_permission_table.rb
+++ b/services/api/db/migrate/20200501150153_permission_table.rb
@@ -133,7 +133,10 @@ perm_from_start(perm_origin_uuid, target_uuid, val, traverse_owned) as (
links.tail_uuid not in (select target_uuid from perm_from_start) and
links.head_uuid in (select target_uuid from perm_from_start))
-select materialized_permissions.user_uuid, u.target_uuid, max(least(u.val, materialized_permissions.perm_level)), bool_or(u.traverse_owned)
+select materialized_permissions.user_uuid,
+ u.target_uuid,
+ max(least(u.val, materialized_permissions.perm_level)),
+ bool_or(u.traverse_owned)
from ((select * from perm_from_start) union (select * from additional_perms)) as u
join materialized_permissions on (u.perm_origin_uuid = materialized_permissions.target_uuid)
where materialized_permissions.traverse_owned
commit 100c0763a0d94e944eb24b7a2900b6e58e2cd516
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Fri May 8 22:08:36 2020 -0400
16007: Now switch over to incremental permissions (WIP)
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index e5760938a..3ba7c4b96 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -17,9 +17,15 @@ class Group < ArvadosModel
attribute :properties, :jsonbHash, default: {}
validate :ensure_filesystem_compatible_name
- after_create :invalidate_permissions_cache
- after_update :maybe_invalidate_permissions_cache
before_create :assign_name
+ after_create :update_permissions
+ after_create :update_trash
+
+ after_update :update_permissions
+ after_update :update_trash
+
+ after_destroy :clear_permissions_and_trash
+
api_accessible :user, extend: :common do |t|
t.add :name
@@ -38,8 +44,8 @@ class Group < ArvadosModel
super if group_class == 'project'
end
- def maybe_invalidate_permissions_cache
- if trash_at_changed? or owner_uuid_changed?
+ def update_trash
+ if trash_at_changed? or owner_uuid_changed? or (new_record? and !trash_at.nil?)
# The group was added or removed from the trash.
#
# Strategy:
@@ -67,56 +73,20 @@ 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.
- invalidate_permissions_cache
- end
end
- def invalidate_permissions_cache
- # Ensure a new group can be accessed by the appropriate users
- # immediately after being created.
- User.invalidate_permissions_cache self.async_permissions_update
-
+ def update_permissions
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]]
+ User.update_permissions self.owner_uuid, self.uuid, 3
+ end
+ end
- 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})
-}
+ def clear_permissions_and_trash
+ User.update_permissions self.owner_uuid, self.uuid, 0
+ ActiveRecord::Base.connection.exec_query %{
+delete from trashed_groups where group_uuid=$1
+}, "Group.clear_trash", [[nil, self.uuid]]
- 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/link.rb b/services/api/app/models/link.rb
index ad7800fe6..9e52c05e6 100644
--- a/services/api/app/models/link.rb
+++ b/services/api/app/models/link.rb
@@ -11,12 +11,12 @@ class Link < ArvadosModel
# already know how to properly treat them.
attribute :properties, :jsonbHash, default: {}
+ validate :name_links_are_obsolete
before_create :permission_to_attach_to_objects
before_update :permission_to_attach_to_objects
- after_update :maybe_invalidate_permissions_cache
- after_create :maybe_invalidate_permissions_cache
- after_destroy :maybe_invalidate_permissions_cache
- validate :name_links_are_obsolete
+ after_update :update_permissions
+ after_create :update_permissions
+ after_destroy :clear_permissions
api_accessible :user, extend: :common do |t|
t.add :tail_uuid
@@ -64,15 +64,22 @@ class Link < ArvadosModel
false
end
- def maybe_invalidate_permissions_cache
+ PERM_LEVEL = {
+ 'can_read' => 1,
+ 'can_login' => 1,
+ 'can_write' => 2,
+ 'can_manage' => 3,
+ }
+
+ def update_permissions
+ if self.link_class == 'permission'
+ User.update_permissions tail_uuid, head_uuid, PERM_LEVEL[name]
+ end
+ end
+
+ def clear_permissions
if self.link_class == 'permission'
- # Clearing the entire permissions cache can generate many
- # unnecessary queries if many active users are not affected by
- # this change. In such cases it would be better to search cached
- # permissions for head_uuid and tail_uuid, and invalidate the
- # cache for only those users. (This would require a browseable
- # cache.)
- User.invalidate_permissions_cache
+ User.update_permissions tail_uuid, head_uuid, 0
end
end
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index a34491966..7edad3ecb 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -3,7 +3,6 @@
# SPDX-License-Identifier: AGPL-3.0
require 'can_be_an_owner'
-require 'refresh_permission_view'
class User < ArvadosModel
include HasUuid
@@ -32,15 +31,16 @@ class User < ArvadosModel
before_create :set_initial_username, :if => Proc.new { |user|
user.username.nil? and user.email
}
+ after_create :update_permissions
after_create :setup_on_activate
after_create :add_system_group_permission_link
- after_create :invalidate_permissions_cache
after_create :auto_setup_new_user, :if => Proc.new { |user|
Rails.configuration.Users.AutoSetupNewUsers and
(user.uuid != system_user_uuid) and
(user.uuid != anonymous_user_uuid)
}
after_create :send_admin_notifications
+ after_update :update_permissions
after_update :send_profile_created_notification
after_update :sync_repository_names, :if => Proc.new { |user|
(user.uuid != system_user_uuid) and
@@ -48,6 +48,7 @@ class User < ArvadosModel
(not user.username_was.nil?)
}
+
has_many :authorized_keys, :foreign_key => :authorized_user_uuid, :primary_key => :uuid
has_many :repositories, foreign_key: :owner_uuid, primary_key: :uuid
@@ -143,12 +144,125 @@ class User < ArvadosModel
true
end
- def self.invalidate_permissions_cache(async=false)
- refresh_permission_view(async)
+ def update_permissions
+ if owner_uuid_changed?
+ puts "Update permissions for #{uuid} #{new_record?}"
+ User.printdump %{
+select * from materialized_permissions where user_uuid='#{uuid}'
+}
+ puts "---"
+ User.update_permissions self.owner_uuid, self.uuid, 3
+ User.printdump %{
+select * from materialized_permissions where user_uuid='#{uuid}'
+}
+
+ end
+ end
+
+ def self.printdump qr
+ q1 = ActiveRecord::Base.connection.exec_query qr
+ q1.each do |r|
+ puts r
+ end
end
- def invalidate_permissions_cache
- User.invalidate_permissions_cache
+ def self.update_permissions perm_origin_uuid, starting_uuid, perm_level
+ # Update a subset of the permission graph
+ # perm_level is the inherited permission
+ # perm_level is a number from 0-3
+ # can_read=1
+ # can_write=2
+ # can_manage=3
+ # call with perm_level=0 to revoke permissions
+ #
+ # 1. Compute set (group, permission) implied by traversing
+ # graph starting at this group
+ # 2. Find links from outside the graph that point inside
+ # 3. For each starting uuid, get the set of permissions from the
+ # materialized permission table
+ # 3. Delete permissions from table not in our computed subset.
+ # 4. Upsert each permission in our subset (user, group, val)
+
+ ## testinging
+ 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 "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-_______________'
+}
+ ## end
+
+ temptable_perms = "temp_perms_#{rand(2**64).to_s(10)}"
+ ActiveRecord::Base.connection.exec_query %{
+create temporary table #{temptable_perms} on commit drop
+as select * from compute_permission_subgraph($1, $2, $3)
+},
+ 'Group.search_permissions',
+ [[nil, perm_origin_uuid],
+ [nil, starting_uuid],
+ [nil, perm_level]]
+
+ q1 = ActiveRecord::Base.connection.exec_query %{
+select * from #{temptable_perms}
+}
+ puts "recomputed perms was #{perm_origin_uuid} #{starting_uuid}, #{perm_level}"
+ q1.each do |r|
+ puts r
+ end
+
+ ActiveRecord::Base.connection.exec_query %{
+delete from materialized_permissions where
+ target_uuid in (select target_uuid from #{temptable_perms}) and
+ (user_uuid not in (select user_uuid from #{temptable_perms} where target_uuid=materialized_permissions.target_uuid)
+ or user_uuid in (select user_uuid from #{temptable_perms} where target_uuid=materialized_permissions.target_uuid and perm_level=0))
+}
+
+ ActiveRecord::Base.connection.exec_query %{
+insert into materialized_permissions (user_uuid, target_uuid, perm_level, traverse_owned)
+ select user_uuid, target_uuid, val as perm_level, traverse_owned from #{temptable_perms}
+on conflict (user_uuid, target_uuid) do update set perm_level=EXCLUDED.perm_level, traverse_owned=EXCLUDED.traverse_owned;
+}
+
+ # 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
+
+# q1 = ActiveRecord::Base.connection.exec_query %{
+# select count(*) from materialized_permissions
+# }
+# puts "correct version #{q1.first}"
+
+# q2 = ActiveRecord::Base.connection.exec_query %{
+# select count(*) from #{temptable_compare}
+# }
+# puts "incremental update #{q2.first}"
end
# Return a hash of {user_uuid: group_perms}
@@ -328,6 +442,8 @@ class User < ArvadosModel
raise "user does not exist" if !new_user
raise "cannot merge to an already merged user" if new_user.redirect_to_user_uuid
+ User.update_permissions self.owner_uuid, self.uuid, 0
+
# If 'self' is a remote user, don't transfer authorizations
# (i.e. ability to access the account) to the new user, because
# that gives the remote site the ability to access the 'new'
@@ -402,7 +518,8 @@ class User < ArvadosModel
if redirect_to_new_user
update_attributes!(redirect_to_user_uuid: new_user.uuid, username: nil)
end
- invalidate_permissions_cache
+ User.update_permissions self.owner_uuid, self.uuid, 3
+ User.update_permissions new_user.owner_uuid, new_user.uuid, 3
end
end
@@ -660,6 +777,7 @@ class User < ArvadosModel
# add the user to the 'All users' group
def create_user_group_link
+ puts "In create_user_group_link"
return (Link.where(tail_uuid: self.uuid,
head_uuid: all_users_group[:uuid],
link_class: 'permission',
diff --git a/services/api/db/migrate/20200501150153_permission_table.rb b/services/api/db/migrate/20200501150153_permission_table.rb
index 296549bb7..425a9505f 100644
--- a/services/api/db/migrate/20200501150153_permission_table.rb
+++ b/services/api/db/migrate/20200501150153_permission_table.rb
@@ -6,6 +6,7 @@ class PermissionTable < ActiveRecord::Migration[5.0]
t.integer :perm_level
t.boolean :traverse_owned
end
+ add_index :materialized_permissions, [:user_uuid, :target_uuid], unique: true, name: 'permission_user_target'
ActiveRecord::Base.connection.execute %{
create or replace function project_subtree (starting_uuid varchar(27))
@@ -71,8 +72,9 @@ $$;
# 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)
+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 $$
@@ -105,6 +107,43 @@ WITH RECURSIVE edges(tail_uuid, head_uuid, val) as (
$$;
}
+ ActiveRecord::Base.connection.execute %{
+create or replace function compute_permission_subgraph (perm_origin_uuid varchar(27),
+ starting_uuid varchar(27),
+ starting_perm integer)
+returns table (user_uuid varchar(27), target_uuid varchar(27), val integer, traverse_owned bool)
+STABLE
+language SQL
+as $$
+with
+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)),
+
+ additional_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
+ links.tail_uuid not in (select target_uuid from perm_from_start) and
+ links.head_uuid in (select target_uuid from perm_from_start))
+
+select materialized_permissions.user_uuid, u.target_uuid, max(least(u.val, materialized_permissions.perm_level)), bool_or(u.traverse_owned)
+ from ((select * from perm_from_start) union (select * from additional_perms)) 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-_______________'
+$$;
+ }
+
ActiveRecord::Base.connection.execute "DROP MATERIALIZED VIEW IF EXISTS materialized_permission_view;"
end
@@ -116,5 +155,6 @@ $$;
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 search_permission_graph(varchar, integer);"
+ ActiveRecord::Base.connection.execute "DROP function compute_permission_subgraph (varchar, varchar, integer);"
end
end
diff --git a/services/api/test/performance/permission_test.rb b/services/api/test/performance/permission_test.rb
index a0605f97e..81edf488a 100644
--- a/services/api/test/performance/permission_test.rb
+++ b/services/api/test/performance/permission_test.rb
@@ -40,7 +40,8 @@ class PermissionPerfTest < ActionDispatch::IntegrationTest
end
end
end
- User.invalidate_permissions_cache
+ #User.invalidate_permissions_cache
+ do_refresh_permission_view
end
end)
end
diff --git a/services/api/test/test_helper.rb b/services/api/test/test_helper.rb
index cee618557..c6ea0b320 100644
--- a/services/api/test/test_helper.rb
+++ b/services/api/test/test_helper.rb
@@ -2,6 +2,8 @@
#
# SPDX-License-Identifier: AGPL-3.0
+require 'refresh_permission_view'
+
ENV["RAILS_ENV"] = "test"
unless ENV["NO_COVERAGE_TEST"]
begin
@@ -60,12 +62,6 @@ class ActiveSupport::TestCase
include ArvadosTestSupport
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
-
teardown do
Thread.current[:api_client_ip_address] = nil
Thread.current[:api_client_authorization] = nil
@@ -213,4 +209,6 @@ class ActionDispatch::IntegrationTest
end
# Ensure permissions are computed from the test fixtures.
-User.invalidate_permissions_cache
+do_refresh_permission_view
+ActiveRecord::Base.connection.execute("DELETE FROM #{TRASHED_GROUPS}")
+ActiveRecord::Base.connection.execute("INSERT INTO #{TRASHED_GROUPS} select * from compute_trashed()")
commit 6f8d1535cb6b1c12b816a47e5ec7d8f8afa9011e
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed May 6 23:38:47 2020 -0400
16007: use statement_timestamp for checking trash_at, fix test.
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 150f46212..00f9254b2 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -287,8 +287,8 @@ class ArvadosModel < ApplicationRecord
exclude_trashed_records = ""
if !include_trash and (sql_table == "groups" or sql_table == "collections") then
- # Only include records that are not explicitly trashed
- exclude_trashed_records = "AND #{sql_table}.is_trashed = false"
+ # Only include records that are not trashed
+ exclude_trashed_records = "AND (#{sql_table}.trash_at is NULL or #{sql_table}.trash_at > statement_timestamp())"
end
if users_list.select { |u| u.is_admin }.any?
@@ -296,16 +296,14 @@ class ArvadosModel < ApplicationRecord
if !include_trash
if sql_table != "api_client_authorizations"
# Only include records where the owner is not trashed
- #sql_conds = "#{sql_table}.owner_uuid NOT IN (SELECT target_uuid FROM #{PERMISSION_VIEW} "+
- # "WHERE trashed = 1) #{exclude_trashed_records}"
- sql_conds = "#{sql_table}.owner_uuid NOT IN (SELECT group_uuid FROM trashed_groups where trash_at < clock_timestamp()) #{exclude_trashed_records}"
+ sql_conds = "#{sql_table}.owner_uuid NOT IN (SELECT group_uuid FROM trashed_groups "+
+ "where trash_at <= statement_timestamp()) #{exclude_trashed_records}"
end
end
else
trashed_check = ""
if !include_trash then
- #trashed_check = "AND trashed = 0"
- trashed_check = "AND target_uuid NOT IN (SELECT group_uuid FROM trashed_groups where trash_at < clock_timestamp())"
+ trashed_check = "AND target_uuid NOT IN (SELECT group_uuid FROM trashed_groups where trash_at <= statement_timestamp())"
end
# Note: it is possible to combine the direct_check and
diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index cc81b3fab..e5760938a 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -43,10 +43,7 @@ class Group < ArvadosModel
# The group was added or removed from the trash.
#
# Strategy:
- # Determine the time this goes in the trash
- # (or null, if it does not belong in the trash)
- # Compute subtree to determine the time each subproject goes
- # in the trash
+ # Compute project subtree, propagating trash_at to subprojects
# Remove groups that don't belong from trash
# Add/update groups that do belong in the trash
diff --git a/services/api/db/migrate/20200501150153_permission_table.rb b/services/api/db/migrate/20200501150153_permission_table.rb
index 00f9bde91..296549bb7 100644
--- a/services/api/db/migrate/20200501150153_permission_table.rb
+++ b/services/api/db/migrate/20200501150153_permission_table.rb
@@ -7,55 +7,6 @@ class PermissionTable < ActiveRecord::Migration[5.0]
t.boolean :traverse_owned
end
- ActiveRecord::Base.connection.execute %{
-create or replace function compute_permission_table ()
-returns table(user_uuid character varying (27),
- target_uuid character varying (27),
- perm_level smallint,
- traverse_owned bool)
-VOLATILE
-language SQL
-as $$
- WITH RECURSIVE perm_value(name, val) AS (
- VALUES ('can_read'::text,(1)::smallint), ('can_login'::text,1), ('can_write'::text,2), ('can_manage'::text,3)
- ), perm_edges(tail_uuid, head_uuid, val, follow) AS (
- SELECT links.tail_uuid,
- links.head_uuid,
- pv.val,
- ((pv.val = 3) OR (groups.uuid IS NOT NULL)) AS follow
- FROM ((public.links
- LEFT JOIN perm_value pv ON ((pv.name = (links.name)::text)))
- LEFT JOIN public.groups ON (((pv.val < 3) AND ((groups.uuid)::text = (links.head_uuid)::text))))
- WHERE ((links.link_class)::text = 'permission'::text)
- UNION ALL
- SELECT groups.owner_uuid,
- groups.uuid,
- 3,
- true
- FROM public.groups
- ), perm(val, follow, user_uuid, target_uuid) AS (
- SELECT (3)::smallint AS val,
- true AS follow,
- (users.uuid)::character varying(32) AS user_uuid,
- (users.uuid)::character varying(32) AS target_uuid
- FROM public.users
- UNION
- SELECT (LEAST((perm_1.val)::integer, edges.val))::smallint AS val,
- edges.follow,
- perm_1.user_uuid,
- (edges.head_uuid)::character varying(32) AS target_uuid
- FROM (perm perm_1
- JOIN perm_edges edges ON ((perm_1.follow AND ((edges.tail_uuid)::text = (perm_1.target_uuid)::text))))
- )
- SELECT perm.user_uuid,
- perm.target_uuid,
- max(perm.val) AS perm_level,
- bool_or(perm.follow) as traverse_owned
- FROM perm
- GROUP BY perm.user_uuid, perm.target_uuid
-$$;
-}
-
ActiveRecord::Base.connection.execute %{
create or replace function project_subtree (starting_uuid varchar(27))
returns table (target_uuid varchar(27))
@@ -161,9 +112,9 @@ $$;
drop_table :materialized_permissions
drop_table :trashed_groups
- ActiveRecord::Base.connection.execute "DROP function compute_permission_table ();"
- ActiveRecord::Base.connection.execute "DROP function project_subtree (varchar(27));"
- ActiveRecord::Base.connection.execute "DROP function project_subtree_with_trash_at (varchar(27), timestamp);"
+ ActiveRecord::Base.connection.execute "DROP function project_subtree (varchar);"
+ 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 search_permission_graph(varchar, integer);"
end
end
diff --git a/services/api/lib/refresh_permission_view.rb b/services/api/lib/refresh_permission_view.rb
index f6642192a..724e65dbe 100644
--- a/services/api/lib/refresh_permission_view.rb
+++ b/services/api/lib/refresh_permission_view.rb
@@ -8,24 +8,12 @@ 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 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/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index bf1ba517e..addea8306 100644
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -1000,6 +1000,19 @@ class CollectionTest < ActiveSupport::TestCase
test "delete referring links in SweepTrashedObjects" do
uuid = collections(:trashed_on_next_sweep).uuid
act_as_system_user do
+ assert_raises ActiveRecord::RecordInvalid do
+ # Cannot create because :trashed_on_next_sweep is already trashed
+ Link.create!(head_uuid: uuid,
+ tail_uuid: system_user_uuid,
+ link_class: 'whatever',
+ name: 'something')
+ end
+
+ # Bump trash_at to now + 1 minute
+ Collection.where(uuid: uuid).
+ update(trash_at: db_current_time + (1).minute)
+
+ # Not considered trashed now
Link.create!(head_uuid: uuid,
tail_uuid: system_user_uuid,
link_class: 'whatever',
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list