[ARVADOS] updated: 1.3.0-2544-gd87627e76
Git user
git at public.arvados.org
Wed May 13 20:42:44 UTC 2020
Summary of changes:
services/api/app/models/group.rb | 39 ++-
services/api/app/models/link.rb | 17 +-
services/api/app/models/user.rb | 243 +++++----------
.../db/migrate/20200501150153_permission_table.rb | 226 ++++++++++----
services/api/db/structure.sql | 343 +++++++++++++--------
services/api/lib/refresh_permission_view.rb | 94 +++++-
services/api/test/fixtures/groups.yml | 4 +-
services/api/test/fixtures/users.yml | 14 +
services/api/test/integration/groups_test.rb | 10 +-
services/api/test/unit/owner_test.rb | 12 +-
services/api/test/unit/user_test.rb | 7 +-
11 files changed, 603 insertions(+), 406 deletions(-)
via d87627e76bf804359fc5b7eccb6f4b5ced1ae820 (commit)
via 53b63c565818eff87788a71e8a98ef81a865109a (commit)
via 136f55507cabf20f5e91fa6fd8b74793e07793fb (commit)
via bc35acd3025ff29f1b2246d470505eea4e457cb0 (commit)
via 26dc594d07456e81268740ee88865e166fde0d65 (commit)
via d4ba39653ebc4c067592d4eaf9733505e0b860c2 (commit)
from 34481779fc2a8df4112d20afaf1f8a0d8fa0d4a3 (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 d87627e76bf804359fc5b7eccb6f4b5ced1ae820
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed May 13 16:42:19 2020 -0400
16007: Remove redundant User#recompute_permissions
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 9afe44582..c27b633ac 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -157,19 +157,6 @@ SELECT 1 FROM #{PERMISSION_VIEW}
MaterializedPermission.where("user_uuid = ? or target_uuid = ?", uuid, uuid).delete_all
end
- def recompute_permissions
- ActiveRecord::Base.connection.exec_delete("DELETE FROM #{PERMISSION_VIEW} where user_uuid=$1",
- "User.recompute_permissions.delete_user_uuid",
- [[nil, uuid]])
- ActiveRecord::Base.connection.exec_insert %{
-INSERT INTO #{PERMISSION_VIEW}
-select $1::varchar, g.target_uuid, g.val, g.traverse_owned
-from search_permission_graph($1::varchar, 3) as g
-},
- "User.recompute_permissions.insert",
- [[nil, uuid]]
- end
-
# Return a hash of {user_uuid: group_perms}
def self.all_group_permissions
all_perms = {}
@@ -361,6 +348,8 @@ update #{PERMISSION_VIEW} set target_uuid=$1 where target_uuid = $2
raise "user does not exist" if !new_user
raise "cannot merge to an already merged user" if new_user.redirect_to_user_uuid
+ 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'
@@ -435,8 +424,8 @@ update #{PERMISSION_VIEW} set target_uuid=$1 where target_uuid = $2
if redirect_to_new_user
update_attributes!(redirect_to_user_uuid: new_user.uuid, username: nil)
end
- self.recompute_permissions
- new_user.recompute_permissions
+ update_permissions self.owner_uuid, self.uuid, 3
+ update_permissions new_user.owner_uuid, new_user.uuid, 3
end
end
commit 53b63c565818eff87788a71e8a98ef81a865109a
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed May 13 16:31:34 2020 -0400
16007: Name our custom queries consistently
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 1d69300b6..9b8a9877e 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -58,41 +58,43 @@ class Group < ArvadosModel
create temporary table #{temptable} on commit drop
as select * from project_subtree_with_trash_at($1, LEAST($2, $3)::timestamp)
},
- 'Group.get_subtree',
+ 'Group.update_trash.select',
[[nil, self.uuid],
[nil, TrashedGroup.find_by_group_uuid(self.owner_uuid).andand.trash_at],
[nil, self.trash_at]]
- ActiveRecord::Base.connection.exec_query %{
+ ActiveRecord::Base.connection.exec_delete %{
delete from trashed_groups where group_uuid in (select target_uuid from #{temptable} where trash_at is NULL);
-}
+},
+ "Group.update_trash.delete"
ActiveRecord::Base.connection.exec_query %{
insert into trashed_groups (group_uuid, trash_at)
select target_uuid as group_uuid, trash_at from #{temptable} where trash_at is not NULL
on conflict (group_uuid) do update set trash_at=EXCLUDED.trash_at;
-}
+},
+ "Group.update_trash.insert"
end
end
def before_ownership_change
if owner_uuid_changed? and !self.owner_uuid_was.nil?
MaterializedPermission.where(user_uuid: owner_uuid_was, target_uuid: uuid).delete_all
- User.update_permissions self.owner_uuid, self.uuid, 0
+ update_permissions self.owner_uuid, self.uuid, 0
end
end
def after_ownership_change
if owner_uuid_changed?
- User.update_permissions self.owner_uuid, self.uuid, 3
+ update_permissions self.owner_uuid, self.uuid, 3
end
end
def clear_permissions_and_trash
MaterializedPermission.where(target_uuid: uuid).delete_all
- ActiveRecord::Base.connection.exec_query %{
+ ActiveRecord::Base.connection.exec_delete %{
delete from trashed_groups where group_uuid=$1
-}, "Group.clear_trash", [[nil, self.uuid]]
+}, "Group.clear_permissions_and_trash", [[nil, self.uuid]]
end
diff --git a/services/api/app/models/link.rb b/services/api/app/models/link.rb
index 2aadb374d..f6210fbb5 100644
--- a/services/api/app/models/link.rb
+++ b/services/api/app/models/link.rb
@@ -14,8 +14,8 @@ class Link < ArvadosModel
validate :name_links_are_obsolete
before_create :permission_to_attach_to_objects
before_update :permission_to_attach_to_objects
- after_update :update_permissions
- after_create :update_permissions
+ after_update :call_update_permissions
+ after_create :call_update_permissions
before_destroy :clear_permissions
after_destroy :clear_permissions
@@ -72,21 +72,21 @@ class Link < ArvadosModel
'can_manage' => 3,
}
- def update_permissions
+ def call_update_permissions
if self.link_class == 'permission'
- User.update_permissions tail_uuid, head_uuid, PERM_LEVEL[name]
+ update_permissions tail_uuid, head_uuid, PERM_LEVEL[name]
end
end
def clear_permissions
if self.link_class == 'permission'
- User.update_permissions tail_uuid, head_uuid, 0, false
+ update_permissions tail_uuid, head_uuid, 0, false
end
end
def check_permissions
if self.link_class == 'permission'
- User.update_permissions tail_uuid, head_uuid, 0, true
+ update_permissions tail_uuid, head_uuid, 0, true
end
end
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 4df2039d8..9afe44582 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -143,242 +143,31 @@ SELECT 1 FROM #{PERMISSION_VIEW}
def before_ownership_change
if owner_uuid_changed? and !self.owner_uuid_was.nil?
MaterializedPermission.where(user_uuid: owner_uuid_was, target_uuid: uuid).delete_all
- User.update_permissions self.owner_uuid_was, self.uuid, 0, false
+ update_permissions self.owner_uuid_was, self.uuid, 0, false
end
end
def after_ownership_change
-
-# puts "Update permissions for #{uuid}"
-# User.printdump %{
-# select * from materialized_permissions where user_uuid='#{uuid}'
-# }
- puts "-1- #{uuid_changed?} and #{uuid} and #{uuid_was}"
- puts "-2- #{owner_uuid_changed?} and #{owner_uuid} and #{owner_uuid_was}"
-
- unless owner_uuid_changed?
- return
+ if owner_uuid_changed?
+ update_permissions self.owner_uuid, self.uuid, 3
end
-
-# if !uuid_was.nil? and uuid_changed?
-# puts "Cleaning house #{uuid_was}"
-# ActiveRecord::Base.connection.exec_query %{
-# update #{PERMISSION_VIEW} set user_uuid=$1 where user_uuid = $2
-# },
-# 'Change user uuid',
-# [[nil, uuid],
-# [nil, uuid_was]]
-# ActiveRecord::Base.connection.exec_query %{
-# update #{PERMISSION_VIEW} set target_uuid=$1 where target_uuid = $2
-# },
-# 'Change user uuid',
-# [[nil, uuid],
-# [nil, uuid_was]]
-# end
-
- User.update_permissions self.owner_uuid, self.uuid, 3
-
-# puts "post-update"
-# User.printdump %{
-# select * from materialized_permissions where user_uuid='#{uuid}'
-# }
-# puts "<<<"
end
def clear_permissions
MaterializedPermission.where("user_uuid = ? or target_uuid = ?", uuid, uuid).delete_all
end
- def self.printdump qr
- q1 = ActiveRecord::Base.connection.exec_query qr
- q1.each do |r|
- puts r
- end
- end
-
def recompute_permissions
- ActiveRecord::Base.connection.execute("DELETE FROM #{PERMISSION_VIEW} where user_uuid='#{uuid}'")
- ActiveRecord::Base.connection.execute %{
+ ActiveRecord::Base.connection.exec_delete("DELETE FROM #{PERMISSION_VIEW} where user_uuid=$1",
+ "User.recompute_permissions.delete_user_uuid",
+ [[nil, uuid]])
+ ActiveRecord::Base.connection.exec_insert %{
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, check=true
- # 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 "\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 "other_links #{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, links.head_uuid, links.name
-# from links
-# 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.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) or
- users.owner_uuid = users.uuid) 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 %{
-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)}"
- ActiveRecord::Base.connection.exec_query %{
-create temporary table #{temptable_perms} on commit drop
-as select * from compute_permission_subgraph($1, $2, $3)
+select $1::varchar, g.target_uuid, g.val, g.traverse_owned
+from search_permission_graph($1::varchar, 3) as g
},
- 'Group.search_permissions',
- [[nil, perm_origin_uuid],
- [nil, starting_uuid],
- [nil, perm_level]]
-
-# q1 = ActiveRecord::Base.connection.exec_query %{
-# select * from #{temptable_perms} order by user_uuid, target_uuid
-# }
-# 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
- target_uuid in (select target_uuid from #{temptable_perms}) and
- not exists (select 1 from #{temptable_perms}
- where target_uuid=materialized_permissions.target_uuid and
- user_uuid=materialized_permissions.user_uuid and
- val>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} where val>0
-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
-
-# puts "dump--"
-# User.printdump %{
-# select owner_uuid, uuid from users order by uuid
-# }
-# User.printdump %{
-# select * from groups
-# }
-# User.printdump %{
-#select * from search_permission_graph('zzzzz-tpzed-000000000000000', 3)
-# }
-# puts "--"
-
- if check
- q1 = ActiveRecord::Base.connection.exec_query %{
-select user_uuid, target_uuid, perm_level, traverse_owned from materialized_permissions
-order by user_uuid, target_uuid
-}
-
- 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
-}
-
- 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
+ "User.recompute_permissions.insert",
+ [[nil, uuid]]
end
# Return a hash of {user_uuid: group_perms}
@@ -535,16 +324,16 @@ SELECT target_uuid, perm_level
self.uuid = new_uuid
save!(validate: false)
change_all_uuid_refs(old_uuid: old_uuid, new_uuid: new_uuid)
- ActiveRecord::Base.connection.exec_query %{
+ ActiveRecord::Base.connection.exec_update %{
update #{PERMISSION_VIEW} set user_uuid=$1 where user_uuid = $2
},
- 'Change user uuid',
+ 'User.update_uuid.update_permissions_user_uuid',
[[nil, new_uuid],
[nil, old_uuid]]
- ActiveRecord::Base.connection.exec_query %{
+ ActiveRecord::Base.connection.exec_update %{
update #{PERMISSION_VIEW} set target_uuid=$1 where target_uuid = $2
},
- 'Change target uuid',
+ 'User.update_uuid.update_permissions_target_uuid',
[[nil, new_uuid],
[nil, old_uuid]]
end
@@ -905,7 +694,6 @@ update #{PERMISSION_VIEW} set target_uuid=$1 where target_uuid = $2
# 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 e0d1c495f..be01e7e50 100644
--- a/services/api/db/migrate/20200501150153_permission_table.rb
+++ b/services/api/db/migrate/20200501150153_permission_table.rb
@@ -1,5 +1,8 @@
class PermissionTable < ActiveRecord::Migration[5.0]
def up
+ ActiveRecord::Base.connection.execute "DROP MATERIALIZED VIEW IF EXISTS materialized_permission_view;"
+ drop_table :permission_refresh_lock
+
create_table :materialized_permissions, :id => false do |t|
t.string :user_uuid
t.string :target_uuid
@@ -31,7 +34,7 @@ $$;
end
add_index :trashed_groups, :group_uuid, :unique => true
- ActiveRecord::Base.connection.execute %{
+ ActiveRecord::Base.connection.execute %{
create or replace function compute_trashed ()
returns table (uuid varchar(27), trash_at timestamp)
STABLE
@@ -43,9 +46,9 @@ select ps.target_uuid as group_uuid, ps.trash_at from groups,
$$;
}
- ActiveRecord::Base.connection.execute("INSERT INTO trashed_groups select * from compute_trashed()")
+ ActiveRecord::Base.connection.execute("INSERT INTO trashed_groups select * from compute_trashed()")
- ActiveRecord::Base.connection.execute %{
+ ActiveRecord::Base.connection.execute %{
create or replace function should_traverse_owned (starting_uuid varchar(27),
starting_perm integer)
returns bool
@@ -58,7 +61,7 @@ $$;
}
- ActiveRecord::Base.connection.execute %{
+ ActiveRecord::Base.connection.execute %{
create or replace function permission_graph_edges ()
returns table (tail_uuid varchar(27), head_uuid varchar(27), val integer)
STABLE
@@ -91,7 +94,7 @@ $$;
# 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 %{
+ 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)
@@ -119,23 +122,7 @@ WITH RECURSIVE edges(tail_uuid, head_uuid, val) as (
$$;
}
-
- # 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 %{
+ ActiveRecord::Base.connection.execute %{
create or replace function compute_permission_subgraph (perm_origin_uuid varchar(27),
starting_uuid varchar(27),
starting_perm integer)
@@ -196,12 +183,80 @@ perm_from_start(perm_origin_uuid, target_uuid, val, traverse_owned) as (
$$;
}
- ActiveRecord::Base.connection.execute "DROP MATERIALIZED VIEW IF EXISTS materialized_permission_view;"
-
+ ActiveRecord::Base.connection.execute %{
+INSERT INTO materialized_permissions
+select users.uuid, g.target_uuid, g.val, g.traverse_owned
+from users, lateral search_permission_graph(users.uuid, 3) as g where g.val > 0
+}
end
+
def down
+ ActiveRecord::Base.connection.execute(%{
+CREATE MATERIALIZED VIEW materialized_permission_view 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, trashed) AS (
+ SELECT links.tail_uuid,
+ links.head_uuid,
+ pv.val,
+ ((pv.val = 3) OR (groups.uuid IS NOT NULL)) AS follow,
+ (0)::smallint AS trashed,
+ (0)::smallint AS followtrash
+ 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 AS bool,
+ CASE
+ WHEN ((groups.trash_at IS NOT NULL) AND (groups.trash_at < clock_timestamp())) THEN 1
+ ELSE 0
+ END AS "case",
+ 1
+ FROM public.groups
+ ), perm(val, follow, user_uuid, target_uuid, trashed) 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,
+ (0)::smallint AS trashed
+ 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,
+ ((GREATEST((perm_1.trashed)::integer, edges.trashed) * edges.followtrash))::smallint AS trashed
+ 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,
+ CASE perm.follow
+ WHEN true THEN perm.target_uuid
+ ELSE NULL::character varying
+ END AS target_owner_uuid,
+ max(perm.trashed) AS trashed
+ FROM perm
+ GROUP BY perm.user_uuid, perm.target_uuid,
+ CASE perm.follow
+ WHEN true THEN perm.target_uuid
+ ELSE NULL::character varying
+ END
+ WITH NO DATA;
+}
+ )
+
+ add_index :materialized_permission_view, [:trashed, :target_uuid], name: 'permission_target_trashed'
+ add_index :materialized_permission_view, [:user_uuid, :trashed, :perm_level], name: 'permission_target_user_trashed_level'
+
drop_table :materialized_permissions
drop_table :trashed_groups
+ create_table :permission_refresh_lock
ActiveRecord::Base.connection.execute "DROP function project_subtree_with_trash_at (varchar, timestamp);"
ActiveRecord::Base.connection.execute "DROP function compute_trashed ();"
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index 71e482083..c34d69e28 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -929,34 +929,6 @@ CREATE SEQUENCE public.nodes_id_seq
ALTER SEQUENCE public.nodes_id_seq OWNED BY public.nodes.id;
---
--- Name: permission_refresh_lock; Type: TABLE; Schema: public; Owner: -
---
-
-CREATE TABLE public.permission_refresh_lock (
- id integer NOT NULL
-);
-
-
---
--- Name: permission_refresh_lock_id_seq; Type: SEQUENCE; Schema: public; Owner: -
---
-
-CREATE SEQUENCE public.permission_refresh_lock_id_seq
- START WITH 1
- INCREMENT BY 1
- NO MINVALUE
- NO MAXVALUE
- CACHE 1;
-
-
---
--- Name: permission_refresh_lock_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: -
---
-
-ALTER SEQUENCE public.permission_refresh_lock_id_seq OWNED BY public.permission_refresh_lock.id;
-
-
--
-- Name: pipeline_instances; Type: TABLE; Schema: public; Owner: -
--
@@ -1392,13 +1364,6 @@ ALTER TABLE ONLY public.logs ALTER COLUMN id SET DEFAULT nextval('public.logs_id
ALTER TABLE ONLY public.nodes ALTER COLUMN id SET DEFAULT nextval('public.nodes_id_seq'::regclass);
---
--- Name: permission_refresh_lock id; Type: DEFAULT; Schema: public; Owner: -
---
-
-ALTER TABLE ONLY public.permission_refresh_lock ALTER COLUMN id SET DEFAULT nextval('public.permission_refresh_lock_id_seq'::regclass);
-
-
--
-- Name: pipeline_instances id; Type: DEFAULT; Schema: public; Owner: -
--
@@ -1583,14 +1548,6 @@ ALTER TABLE ONLY public.nodes
ADD CONSTRAINT nodes_pkey PRIMARY KEY (id);
---
--- Name: permission_refresh_lock permission_refresh_lock_pkey; Type: CONSTRAINT; Schema: public; Owner: -
---
-
-ALTER TABLE ONLY public.permission_refresh_lock
- ADD CONSTRAINT permission_refresh_lock_pkey PRIMARY KEY (id);
-
-
--
-- Name: pipeline_instances pipeline_instances_pkey; Type: CONSTRAINT; Schema: public; Owner: -
--
diff --git a/services/api/lib/refresh_permission_view.rb b/services/api/lib/refresh_permission_view.rb
index 8160502b9..e89978d7b 100644
--- a/services/api/lib/refresh_permission_view.rb
+++ b/services/api/lib/refresh_permission_view.rb
@@ -7,13 +7,14 @@ 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("LOCK TABLE #{PERMISSION_VIEW}")
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 where g.val > 0
-}
+},
+ "refresh_permission_view.do"
end
end
@@ -44,3 +45,90 @@ def refresh_permission_view(async=false)
do_refresh_permission_view
end
end
+
+
+def update_permissions perm_origin_uuid, starting_uuid, perm_level, check=false
+ # 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)
+
+ ActiveRecord::Base.connection.execute "LOCK TABLE #{PERMISSION_VIEW} in SHARE MODE"
+
+ 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)
+},
+ 'update_permissions.select',
+ [[nil, perm_origin_uuid],
+ [nil, starting_uuid],
+ [nil, perm_level]]
+
+ ActiveRecord::Base.connection.exec_delete %{
+delete from #{PERMISSION_VIEW} where
+ target_uuid in (select target_uuid from #{temptable_perms}) and
+ not exists (select 1 from #{temptable_perms}
+ where target_uuid=#{PERMISSION_VIEW}.target_uuid and
+ user_uuid=#{PERMISSION_VIEW}.user_uuid and
+ val>0)
+},
+ "update_permissions.delete"
+
+ ActiveRecord::Base.connection.exec_query %{
+insert into #{PERMISSION_VIEW} (user_uuid, target_uuid, perm_level, traverse_owned)
+ select user_uuid, target_uuid, val as perm_level, traverse_owned from #{temptable_perms} where val>0
+on conflict (user_uuid, target_uuid) do update set perm_level=EXCLUDED.perm_level, traverse_owned=EXCLUDED.traverse_owned;
+},
+ "update_permissions.insert"
+
+ if check
+ #
+ # For validation/debugging, this checks contents of the
+ # incrementally-updated 'materialized_permission' against a
+ # from-scratch permission refresh.
+ #
+
+ q1 = ActiveRecord::Base.connection.exec_query %{
+select user_uuid, target_uuid, perm_level, traverse_owned from #{PERMISSION_VIEW}
+order by user_uuid, target_uuid
+}
+
+ 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
+}
+
+ 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
+end
diff --git a/services/api/test/integration/groups_test.rb b/services/api/test/integration/groups_test.rb
index eb97fc1f4..445670a3d 100644
--- a/services/api/test/integration/groups_test.rb
+++ b/services/api/test/integration/groups_test.rb
@@ -193,11 +193,15 @@ class NonTransactionalGroupsTest < ActionDispatch::IntegrationTest
assert_response :success
end
- test "create request with async=true defers permissions update" do
+ test "create request with async=true does not defer permissions update" do
Rails.configuration.API.AsyncPermissionsUpdateInterval = 1 # second
name = "Random group #{rand(1000)}"
assert_equal nil, Group.find_by_name(name)
+ # Following the implementation of incremental permission updates
+ # (#16007) the async flag is now a no-op. Permission changes are
+ # visible immediately.
+
# Trigger the asynchronous permission update by using async=true parameter.
post "/arvados/v1/groups",
params: {
@@ -209,7 +213,7 @@ class NonTransactionalGroupsTest < ActionDispatch::IntegrationTest
headers: auth(:active)
assert_response 202
- # The group exists on the database, but it's not accessible yet.
+ # The group exists in the database
assert_not_nil Group.find_by_name(name)
get "/arvados/v1/groups",
params: {
@@ -218,7 +222,7 @@ class NonTransactionalGroupsTest < ActionDispatch::IntegrationTest
},
headers: auth(:active)
assert_response 200
- assert_equal 0, json_response['items_available']
+ assert_equal 1, json_response['items_available']
# Wait a bit and try again.
sleep(1)
diff --git a/services/api/test/unit/owner_test.rb b/services/api/test/unit/owner_test.rb
index 3280b1fc3..2bdc198a3 100644
--- a/services/api/test/unit/owner_test.rb
+++ b/services/api/test/unit/owner_test.rb
@@ -70,7 +70,6 @@ class OwnerTest < ActiveSupport::TestCase
"new #{o_class} should really be in DB")
old_uuid = o.uuid
new_uuid = o.uuid.sub(/..........$/, rand(2**256).to_s(36)[0..9])
- puts "//Changing//"
if o.respond_to? :update_uuid
o.update_uuid(new_uuid: new_uuid)
else
commit 136f55507cabf20f5e91fa6fd8b74793e07793fb
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed May 13 15:04:20 2020 -0400
16007: Can finally commit structure.sql
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/api/db/migrate/20200501150153_permission_table.rb b/services/api/db/migrate/20200501150153_permission_table.rb
index 36d618ea2..e0d1c495f 100644
--- a/services/api/db/migrate/20200501150153_permission_table.rb
+++ b/services/api/db/migrate/20200501150153_permission_table.rb
@@ -8,22 +8,6 @@ class PermissionTable < ActiveRecord::Migration[5.0]
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))
-returns table (target_uuid varchar(27))
-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;
-$$;
-}
-
ActiveRecord::Base.connection.execute %{
create or replace function project_subtree_with_trash_at (starting_uuid varchar(27), starting_trash_at timestamp)
returns table (target_uuid varchar(27), trash_at timestamp)
@@ -219,7 +203,6 @@ $$;
drop_table :materialized_permissions
drop_table :trashed_groups
- 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);"
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index 88cd0baa2..71e482083 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -38,6 +38,161 @@ CREATE EXTENSION IF NOT EXISTS pg_trgm WITH SCHEMA public;
-- COMMENT ON EXTENSION pg_trgm IS 'text similarity measurement and index searching based on trigrams';
+--
+-- Name: compute_permission_subgraph(character varying, character varying, integer); Type: FUNCTION; Schema: public; Owner: -
+--
+
+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 $$
+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)),
+
+ 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.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) or
+ users.owner_uuid = users.uuid) 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_identity_perms
+ )
+
+ select v.user_uuid, v.target_uuid, max(v.perm_level), bool_or(v.traverse_owned) from
+ (select materialized_permissions.user_uuid,
+ u.target_uuid,
+ least(u.val, materialized_permissions.perm_level) as perm_level,
+ 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
+ union
+ select perm_origin_uuid as user_uuid, target_uuid, val as perm_level, traverse_owned
+ from all_perms
+ where perm_origin_uuid like '_____-tpzed-_______________') as v
+ group by v.user_uuid, v.target_uuid
+$$;
+
+
+--
+-- 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 $$
+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: permission_graph_edges(); Type: FUNCTION; Schema: public; Owner: -
+--
+
+CREATE FUNCTION public.permission_graph_edges() RETURNS TABLE(tail_uuid character varying, head_uuid character varying, val integer)
+ LANGUAGE sql STABLE
+ 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'
+$$;
+
+
+--
+-- Name: project_subtree_with_trash_at(character varying, timestamp without time zone); Type: FUNCTION; Schema: public; Owner: -
+--
+
+CREATE FUNCTION public.project_subtree_with_trash_at(starting_uuid character varying, starting_trash_at timestamp without time zone) RETURNS TABLE(target_uuid character varying, trash_at timestamp without time zone)
+ LANGUAGE sql STABLE
+ 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)
+ from groups join project_subtree on (groups.owner_uuid = project_subtree.uuid)
+ )
+ select uuid, trash_at from project_subtree;
+$$;
+
+
+--
+-- Name: search_permission_graph(character varying, integer); Type: FUNCTION; Schema: public; Owner: -
+--
+
+CREATE FUNCTION public.search_permission_graph(starting_uuid character varying, starting_perm integer) RETURNS TABLE(target_uuid character varying, val integer, traverse_owned boolean)
+ LANGUAGE sql STABLE
+ AS $$
+WITH RECURSIVE edges(tail_uuid, head_uuid, val) as (
+ select * from permission_graph_edges()
+ ),
+ traverse_graph(target_uuid, val, traverse_owned) as (
+ values (starting_uuid, starting_perm,
+ should_traverse_owned(starting_uuid, starting_perm))
+ union
+ (select edges.head_uuid,
+ 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)))
+ select target_uuid, max(val), bool_or(traverse_owned) from traverse_graph
+ group by (target_uuid) ;
+$$;
+
+
+--
+-- Name: should_traverse_owned(character varying, integer); Type: FUNCTION; Schema: public; Owner: -
+--
+
+CREATE FUNCTION public.should_traverse_owned(starting_uuid character varying, starting_perm integer) RETURNS boolean
+ LANGUAGE sql STABLE
+ AS $$
+select starting_uuid like '_____-j7d0g-_______________' or
+ (starting_uuid like '_____-tpzed-_______________' and starting_perm >= 3);
+$$;
+
+
SET default_tablespace = '';
SET default_with_oids = false;
@@ -719,93 +874,17 @@ ALTER SEQUENCE public.logs_id_seq OWNED BY public.logs.id;
--
--- Name: users; Type: TABLE; Schema: public; Owner: -
+-- Name: materialized_permissions; Type: TABLE; Schema: public; Owner: -
--
-CREATE TABLE public.users (
- id integer NOT NULL,
- uuid character varying(255),
- owner_uuid character varying(255) NOT NULL,
- created_at timestamp without time zone NOT NULL,
- modified_by_client_uuid character varying(255),
- modified_by_user_uuid character varying(255),
- modified_at timestamp without time zone,
- email character varying(255),
- first_name character varying(255),
- last_name character varying(255),
- identity_url character varying(255),
- is_admin boolean,
- prefs text,
- updated_at timestamp without time zone NOT NULL,
- default_owner_uuid character varying(255),
- is_active boolean DEFAULT false,
- username character varying(255),
- redirect_to_user_uuid character varying
+CREATE TABLE public.materialized_permissions (
+ user_uuid character varying,
+ target_uuid character varying,
+ perm_level integer,
+ traverse_owned boolean
);
---
--- Name: materialized_permission_view; Type: MATERIALIZED VIEW; Schema: public; Owner: -
---
-
-CREATE MATERIALIZED VIEW public.materialized_permission_view 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, trashed) AS (
- SELECT links.tail_uuid,
- links.head_uuid,
- pv.val,
- ((pv.val = 3) OR (groups.uuid IS NOT NULL)) AS follow,
- (0)::smallint AS trashed,
- (0)::smallint AS followtrash
- 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 AS bool,
- CASE
- WHEN ((groups.trash_at IS NOT NULL) AND (groups.trash_at < clock_timestamp())) THEN 1
- ELSE 0
- END AS "case",
- 1
- FROM public.groups
- ), perm(val, follow, user_uuid, target_uuid, trashed) 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,
- (0)::smallint AS trashed
- 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,
- ((GREATEST((perm_1.trashed)::integer, edges.trashed) * edges.followtrash))::smallint AS trashed
- 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,
- CASE perm.follow
- WHEN true THEN perm.target_uuid
- ELSE NULL::character varying
- END AS target_owner_uuid,
- max(perm.trashed) AS trashed
- FROM perm
- GROUP BY perm.user_uuid, perm.target_uuid,
- CASE perm.follow
- WHEN true THEN perm.target_uuid
- ELSE NULL::character varying
- END
- WITH NO DATA;
-
-
--
-- Name: nodes; Type: TABLE; Schema: public; Owner: -
--
@@ -1079,6 +1158,42 @@ CREATE SEQUENCE public.traits_id_seq
ALTER SEQUENCE public.traits_id_seq OWNED BY public.traits.id;
+--
+-- Name: trashed_groups; Type: TABLE; Schema: public; Owner: -
+--
+
+CREATE TABLE public.trashed_groups (
+ group_uuid character varying,
+ trash_at timestamp without time zone
+);
+
+
+--
+-- Name: users; Type: TABLE; Schema: public; Owner: -
+--
+
+CREATE TABLE public.users (
+ id integer NOT NULL,
+ uuid character varying(255),
+ owner_uuid character varying(255) NOT NULL,
+ created_at timestamp without time zone NOT NULL,
+ modified_by_client_uuid character varying(255),
+ modified_by_user_uuid character varying(255),
+ modified_at timestamp without time zone,
+ email character varying(255),
+ first_name character varying(255),
+ last_name character varying(255),
+ identity_url character varying(255),
+ is_admin boolean,
+ prefs text,
+ updated_at timestamp without time zone NOT NULL,
+ default_owner_uuid character varying(255),
+ is_active boolean DEFAULT false,
+ username character varying(255),
+ redirect_to_user_uuid character varying
+);
+
+
--
-- Name: users_id_seq; Type: SEQUENCE; Schema: public; Owner: -
--
@@ -2513,6 +2628,13 @@ CREATE INDEX index_traits_on_owner_uuid ON public.traits USING btree (owner_uuid
CREATE UNIQUE INDEX index_traits_on_uuid ON public.traits USING btree (uuid);
+--
+-- Name: index_trashed_groups_on_group_uuid; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE UNIQUE INDEX index_trashed_groups_on_group_uuid ON public.trashed_groups USING btree (group_uuid);
+
+
--
-- Name: index_users_on_created_at; Type: INDEX; Schema: public; Owner: -
--
@@ -2703,17 +2825,10 @@ CREATE INDEX nodes_search_index ON public.nodes USING btree (uuid, owner_uuid, m
--
--- Name: permission_target_trashed; Type: INDEX; Schema: public; Owner: -
---
-
-CREATE INDEX permission_target_trashed ON public.materialized_permission_view USING btree (trashed, target_uuid);
-
-
---
--- Name: permission_target_user_trashed_level; Type: INDEX; Schema: public; Owner: -
+-- Name: permission_user_target; Type: INDEX; Schema: public; Owner: -
--
-CREATE INDEX permission_target_user_trashed_level ON public.materialized_permission_view USING btree (user_uuid, trashed, perm_level);
+CREATE UNIQUE INDEX permission_user_target ON public.materialized_permissions USING btree (user_uuid, target_uuid);
--
@@ -3024,6 +3139,7 @@ INSERT INTO "schema_migrations" (version) VALUES
('20190523180148'),
('20190808145904'),
('20190809135453'),
-('20190905151603');
+('20190905151603'),
+('20200501150153');
commit bc35acd3025ff29f1b2246d470505eea4e457cb0
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed May 13 14:52:37 2020 -0400
16007: All the API tests pass!!!
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 d9ef342b3..1d69300b6 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -18,14 +18,14 @@ class Group < ArvadosModel
validate :ensure_filesystem_compatible_name
before_create :assign_name
- after_create :update_permissions
+ after_create :after_ownership_change
after_create :update_trash
- after_update :update_permissions, :if => :owner_uuid_changed?
- after_update :update_trash
-
- after_destroy :clear_permissions_and_trash
+ before_update :before_ownership_change
+ after_update :after_ownership_change
+ after_update :update_trash
+ before_destroy :clear_permissions_and_trash
api_accessible :user, extend: :common do |t|
t.add :name
@@ -75,12 +75,21 @@ on conflict (group_uuid) do update set trash_at=EXCLUDED.trash_at;
end
end
- def update_permissions
- User.update_permissions self.owner_uuid, self.uuid, 3
+ def before_ownership_change
+ if owner_uuid_changed? and !self.owner_uuid_was.nil?
+ MaterializedPermission.where(user_uuid: owner_uuid_was, target_uuid: uuid).delete_all
+ User.update_permissions self.owner_uuid, self.uuid, 0
+ end
+ end
+
+ def after_ownership_change
+ if owner_uuid_changed?
+ User.update_permissions self.owner_uuid, self.uuid, 3
+ end
end
def clear_permissions_and_trash
- User.update_permissions self.owner_uuid, self.uuid, 0
+ MaterializedPermission.where(target_uuid: uuid).delete_all
ActiveRecord::Base.connection.exec_query %{
delete from trashed_groups where group_uuid=$1
}, "Group.clear_trash", [[nil, self.uuid]]
diff --git a/services/api/app/models/link.rb b/services/api/app/models/link.rb
index 9e52c05e6..2aadb374d 100644
--- a/services/api/app/models/link.rb
+++ b/services/api/app/models/link.rb
@@ -16,6 +16,7 @@ class Link < ArvadosModel
before_update :permission_to_attach_to_objects
after_update :update_permissions
after_create :update_permissions
+ before_destroy :clear_permissions
after_destroy :clear_permissions
api_accessible :user, extend: :common do |t|
@@ -79,7 +80,13 @@ class Link < ArvadosModel
def clear_permissions
if self.link_class == 'permission'
- User.update_permissions tail_uuid, head_uuid, 0
+ User.update_permissions tail_uuid, head_uuid, 0, false
+ end
+ end
+
+ def check_permissions
+ if self.link_class == 'permission'
+ User.update_permissions tail_uuid, head_uuid, 0, true
end
end
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 6dce3a42e..4df2039d8 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -27,11 +27,12 @@ class User < ArvadosModel
user.username.nil? and user.username_changed?
}
before_update :setup_on_activate
+
before_create :check_auto_admin
before_create :set_initial_username, :if => Proc.new { |user|
user.username.nil? and user.email
}
- after_create :update_permissions
+ after_create :after_ownership_change
after_create :setup_on_activate
after_create :add_system_group_permission_link
after_create :auto_setup_new_user, :if => Proc.new { |user|
@@ -40,14 +41,16 @@ class User < ArvadosModel
(user.uuid != anonymous_user_uuid)
}
after_create :send_admin_notifications
- after_update :update_permissions, :if => :owner_uuid_changed?
+
+ before_update :before_ownership_change
+ after_update :after_ownership_change
after_update :send_profile_created_notification
after_update :sync_repository_names, :if => Proc.new { |user|
(user.uuid != system_user_uuid) and
user.username_changed? and
(not user.username_was.nil?)
}
-
+ after_destroy :clear_permissions
has_many :authorized_keys, :foreign_key => :authorized_user_uuid, :primary_key => :uuid
has_many :repositories, foreign_key: :owner_uuid, primary_key: :uuid
@@ -137,13 +140,42 @@ SELECT 1 FROM #{PERMISSION_VIEW}
true
end
- def update_permissions
+ def before_ownership_change
+ if owner_uuid_changed? and !self.owner_uuid_was.nil?
+ MaterializedPermission.where(user_uuid: owner_uuid_was, target_uuid: uuid).delete_all
+ User.update_permissions self.owner_uuid_was, self.uuid, 0, false
+ end
+ end
+
+ def after_ownership_change
# puts "Update permissions for #{uuid}"
# User.printdump %{
# select * from materialized_permissions where user_uuid='#{uuid}'
# }
-# puts "---"
+ puts "-1- #{uuid_changed?} and #{uuid} and #{uuid_was}"
+ puts "-2- #{owner_uuid_changed?} and #{owner_uuid} and #{owner_uuid_was}"
+
+ unless owner_uuid_changed?
+ return
+ end
+
+# if !uuid_was.nil? and uuid_changed?
+# puts "Cleaning house #{uuid_was}"
+# ActiveRecord::Base.connection.exec_query %{
+# update #{PERMISSION_VIEW} set user_uuid=$1 where user_uuid = $2
+# },
+# 'Change user uuid',
+# [[nil, uuid],
+# [nil, uuid_was]]
+# ActiveRecord::Base.connection.exec_query %{
+# update #{PERMISSION_VIEW} set target_uuid=$1 where target_uuid = $2
+# },
+# 'Change user uuid',
+# [[nil, uuid],
+# [nil, uuid_was]]
+# end
+
User.update_permissions self.owner_uuid, self.uuid, 3
# puts "post-update"
@@ -153,6 +185,10 @@ SELECT 1 FROM #{PERMISSION_VIEW}
# puts "<<<"
end
+ def clear_permissions
+ MaterializedPermission.where("user_uuid = ? or target_uuid = ?", uuid, uuid).delete_all
+ end
+
def self.printdump qr
q1 = ActiveRecord::Base.connection.exec_query qr
q1.each do |r|
@@ -169,7 +205,7 @@ from search_permission_graph('#{uuid}', 3) as g
}
end
- def self.update_permissions perm_origin_uuid, starting_uuid, perm_level
+ def self.update_permissions perm_origin_uuid, starting_uuid, perm_level, check=true
# Update a subset of the permission graph
# perm_level is the inherited permission
# perm_level is a number from 0-3
@@ -228,8 +264,7 @@ perm_from_start(perm_origin_uuid, target_uuid, val, traverse_owned) as (
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.head_uuid = '#{starting_uuid}')) 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)),
@@ -242,7 +277,8 @@ perm_from_start(perm_origin_uuid, target_uuid, val, traverse_owned) as (
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
+ where (users.owner_uuid not in (select target_uuid from partial_perms) or
+ users.owner_uuid = users.uuid) and
users.uuid in (select target_uuid from partial_perms)
),
@@ -284,59 +320,62 @@ as select * from compute_permission_subgraph($1, $2, $3)
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))
+ not exists (select 1 from #{temptable_perms}
+ where target_uuid=materialized_permissions.target_uuid and
+ user_uuid=materialized_permissions.user_uuid and
+ val>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}
+ select user_uuid, target_uuid, val as perm_level, traverse_owned from #{temptable_perms} where val>0
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
-# puts "dump--"
-# User.printdump %{
-# select * from users
-# }
-# User.printdump %{
-# select * from groups
-# }
-# User.printdump %{
-# select * from search_permission_graph('zzzzz-tpzed-000000000000000', 3)
-# }
-# puts "--"
-
+# puts "dump--"
+# User.printdump %{
+# select owner_uuid, uuid from users order by uuid
+# }
+# User.printdump %{
+# select * from groups
+# }
+# User.printdump %{
+#select * from search_permission_graph('zzzzz-tpzed-000000000000000', 3)
+# }
+# puts "--"
- q1 = ActiveRecord::Base.connection.exec_query %{
+ if check
+ q1 = ActiveRecord::Base.connection.exec_query %{
select user_uuid, target_uuid, perm_level, traverse_owned from materialized_permissions
order by user_uuid, target_uuid
}
- q2 = ActiveRecord::Base.connection.exec_query %{
+ 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
}
- if q1.count != q2.count
- puts "Didn't match incremental+: #{q1.count} != full refresh-: #{q2.count}"
- end
+ 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"
+ 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
- end
- else
- q2.each_with_index do |r, i|
- if r != q1[i]
- puts "+#{q1[i]}\n-#{r}"
- raise "Didn't match"
+ 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
@@ -496,6 +535,18 @@ SELECT target_uuid, perm_level
self.uuid = new_uuid
save!(validate: false)
change_all_uuid_refs(old_uuid: old_uuid, new_uuid: new_uuid)
+ ActiveRecord::Base.connection.exec_query %{
+update #{PERMISSION_VIEW} set user_uuid=$1 where user_uuid = $2
+},
+ 'Change user uuid',
+ [[nil, new_uuid],
+ [nil, old_uuid]]
+ ActiveRecord::Base.connection.exec_query %{
+update #{PERMISSION_VIEW} set target_uuid=$1 where target_uuid = $2
+},
+ 'Change target uuid',
+ [[nil, new_uuid],
+ [nil, old_uuid]]
end
end
diff --git a/services/api/db/migrate/20200501150153_permission_table.rb b/services/api/db/migrate/20200501150153_permission_table.rb
index d3afd4a74..36d618ea2 100644
--- a/services/api/db/migrate/20200501150153_permission_table.rb
+++ b/services/api/db/migrate/20200501150153_permission_table.rb
@@ -172,8 +172,7 @@ perm_from_start(perm_origin_uuid, target_uuid, val, traverse_owned) as (
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.head_uuid = starting_uuid)) 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)),
@@ -186,7 +185,8 @@ perm_from_start(perm_origin_uuid, target_uuid, val, traverse_owned) as (
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
+ where (users.owner_uuid not in (select target_uuid from partial_perms) or
+ users.owner_uuid = users.uuid) and
users.uuid in (select target_uuid from partial_perms)
),
diff --git a/services/api/test/fixtures/groups.yml b/services/api/test/fixtures/groups.yml
index 92a1ced52..22c999ecd 100644
--- a/services/api/test/fixtures/groups.yml
+++ b/services/api/test/fixtures/groups.yml
@@ -60,6 +60,7 @@ testusergroup_admins:
uuid: zzzzz-j7d0g-48foin4vonvc2at
owner_uuid: zzzzz-tpzed-000000000000000
name: Administrators of a subset of users
+ group_class: role
aproject:
uuid: zzzzz-j7d0g-v955i6s2oi1cbso
@@ -143,6 +144,7 @@ active_user_has_can_manage:
uuid: zzzzz-j7d0g-ptt1ou6a9lxrv07
owner_uuid: zzzzz-tpzed-d9tiejq69daie8f
name: Active user has can_manage
+ group_class: project
# Group for testing granting permission between users who share a group.
group_for_sharing_tests:
@@ -343,4 +345,4 @@ trashed_on_next_sweep:
trash_at: 2001-01-01T00:00:00Z
delete_at: 2038-03-01T00:00:00Z
is_trashed: false
- modified_at: 2001-01-01T00:00:00Z
\ No newline at end of file
+ modified_at: 2001-01-01T00:00:00Z
diff --git a/services/api/test/unit/owner_test.rb b/services/api/test/unit/owner_test.rb
index 528c6d253..3280b1fc3 100644
--- a/services/api/test/unit/owner_test.rb
+++ b/services/api/test/unit/owner_test.rb
@@ -70,8 +70,13 @@ class OwnerTest < ActiveSupport::TestCase
"new #{o_class} should really be in DB")
old_uuid = o.uuid
new_uuid = o.uuid.sub(/..........$/, rand(2**256).to_s(36)[0..9])
- assert(o.update_attributes(uuid: new_uuid),
- "should change #{o_class} uuid from #{old_uuid} to #{new_uuid}")
+ puts "//Changing//"
+ if o.respond_to? :update_uuid
+ o.update_uuid(new_uuid: new_uuid)
+ else
+ assert(o.update_attributes(uuid: new_uuid),
+ "should change #{o_class} uuid from #{old_uuid} to #{new_uuid}")
+ end
assert_equal(false, o_class.where(uuid: old_uuid).any?,
"#{old_uuid} should disappear when renamed to #{new_uuid}")
end
@@ -116,8 +121,8 @@ class OwnerTest < ActiveSupport::TestCase
"setting owner to self should work")
old_uuid = o.uuid
new_uuid = o.uuid.sub(/..........$/, rand(2**256).to_s(36)[0..9])
- assert(o.update_attributes(uuid: new_uuid),
- "should change uuid of User that owns self")
+ o.update_uuid(new_uuid: new_uuid)
+ o = User.find_by_uuid(new_uuid)
assert_equal(false, User.where(uuid: old_uuid).any?,
"#{old_uuid} should not be in DB after deleting")
assert_equal(true, User.where(uuid: new_uuid).any?,
diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb
index 260795c12..5d25361ed 100644
--- a/services/api/test/unit/user_test.rb
+++ b/services/api/test/unit/user_test.rb
@@ -165,7 +165,12 @@ class UserTest < ActiveSupport::TestCase
if auto_admin_first_user_config
# This test requires no admin users exist (except for the system user)
- users(:admin).delete
+ act_as_system_user do
+ users(:admin).update_attributes!(is_admin: false)
+ end
+ # need to manually call clear_permissions because we used 'delete' instead of 'destory'
+ # we use delete here because 'destroy' will throw an error
+ #users(:admin).clear_permissions
@all_users = User.where("uuid not like '%-000000000000000'").where(:is_admin => true)
assert_equal 0, @all_users.count, "No admin users should exist (except for the system user)"
end
commit 26dc594d07456e81268740ee88865e166fde0d65
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Tue May 12 12:52:50 2020 -0400
16007: Account for all the edges. still wip.
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 f12f72520..6dce3a42e 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 438851afb..d3afd4a74 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 724e65dbe..8160502b9 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 57633a312..14630d9ef 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 at 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
commit d4ba39653ebc4c067592d4eaf9733505e0b860c2
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon May 11 17:41:35 2020 -0400
16007: Almost everything passes
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 d2ca8102e..f12f72520 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -146,18 +146,18 @@ class User < ArvadosModel
def update_permissions
- puts "Update permissions for #{uuid}"
- 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
- puts "post-update"
- User.printdump %{
-select * from materialized_permissions where user_uuid='#{uuid}'
-}
- puts "<<<"
+# puts "post-update"
+# User.printdump %{
+# select * from materialized_permissions where user_uuid='#{uuid}'
+# }
+# puts "<<<"
end
def self.printdump qr
@@ -194,16 +194,52 @@ from search_permission_graph('#{uuid}', 3) as g
# 4. Upsert each permission in our subset (user, group, val)
## testinging
- puts "__ update_permissions __"
- 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 "other_links #{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, links.head_uuid, links.name
+# from links
+# 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}))
+
+# 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 "Perms out"
# printdump %{
@@ -233,14 +269,14 @@ as select * from compute_permission_subgraph($1, $2, $3)
[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
- puts "<<<<"
+# q1 = ActiveRecord::Base.connection.exec_query %{
+# select * from #{temptable_perms} order by user_uuid, target_uuid
+# }
+# 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 07c155517..438851afb 100644
--- a/services/api/db/migrate/20200501150153_permission_table.rb
+++ b/services/api/db/migrate/20200501150153_permission_table.rb
@@ -122,7 +122,7 @@ 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 (
+ 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
@@ -132,29 +132,38 @@ perm_from_start(perm_origin_uuid, target_uuid, val, traverse_owned) as (
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
+ (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)),
- 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-_______________'),
+ 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-_______________'
+ ),
all_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
select * from perm_from_start
union
- select * from additional_perms
+ select * from link_perms
union
- select * from identity_perms
+ select * from user_perms
)
- select materialized_permissions.user_uuid,
+ select v.user_uuid, v.target_uuid, max(v.perm_level), bool_or(v.traverse_owned) from
+ (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
+ least(u.val, materialized_permissions.perm_level) as perm_level,
+ 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
+ union
+ select perm_origin_uuid as user_uuid, target_uuid, val as perm_level, traverse_owned
+ from all_perms
+ where perm_origin_uuid like '_____-tpzed-_______________') as v
+ group by v.user_uuid, v.target_uuid
$$;
}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list