[ARVADOS] updated: 1.3.0-2616-ga3aee2781
Git user
git at public.arvados.org
Fri Jun 5 15:05:54 UTC 2020
Summary of changes:
services/api/app/models/arvados_model.rb | 11 +-
services/api/app/models/user.rb | 11 +-
.../db/migrate/20200501150153_permission_table.rb | 143 +++++------------
services/api/db/structure.sql | 176 +++++++++------------
services/api/lib/update_permissions.rb | 40 ++++-
services/api/test/unit/permission_test.rb | 77 ++++++---
services/api/test/unit/user_test.rb | 9 --
7 files changed, 212 insertions(+), 255 deletions(-)
via a3aee2781cfcd006fa1b7ce3cfeeb1dd2d53c270 (commit)
from b879b9cd18ddba6ba87b65f81eba676114478a06 (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 a3aee2781cfcd006fa1b7ce3cfeeb1dd2d53c270
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Thu Jun 4 16:58:18 2020 -0400
16007: Special handing for users with permissions on other users
Revise & simplify permission traversal. Don't traverse users except
when starting from the user (origin_uuid = starting_uuid).
This avoids disasterous queries where we re-traverse other users "just
in case" and end up recomputing the whole database. As a tradeoff,
our epic readable_by query gets a touch more epic, as it now has to go
to the permissions table to check if there are other user permissions
the current user also is allowed to use.
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 cc6f7816a..a27d6aba4 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -324,9 +324,14 @@ class ArvadosModel < ApplicationRecord
#
# see issue 13208 for details.
+ user_uuids_subquery = %{
+select target_uuid from materialized_permissions where user_uuid in (:user_uuids)
+and target_uuid like '_____-tpzed-_______________' and traverse_owned=true and perm_level >= 1
+}
+
# Match a direct read permission link from the user to the record uuid
direct_check = "#{sql_table}.uuid IN (SELECT target_uuid FROM #{PERMISSION_VIEW} "+
- "WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 #{trashed_check})"
+ "WHERE user_uuid IN (#{user_uuids_subquery}) AND perm_level >= 1 #{trashed_check})"
# Match a read permission for the user to the record's
# owner_uuid. This is so we can have a permissions table that
@@ -347,7 +352,7 @@ class ArvadosModel < ApplicationRecord
owner_check = ""
if sql_table != "api_client_authorizations" and sql_table != "groups" then
owner_check = "OR #{sql_table}.owner_uuid IN (SELECT target_uuid FROM #{PERMISSION_VIEW} "+
- "WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 #{trashed_check} AND traverse_owned) "
+ "WHERE user_uuid IN (#{user_uuids_subquery}) AND perm_level >= 1 #{trashed_check} AND traverse_owned) "
end
links_cond = ""
@@ -356,7 +361,7 @@ class ArvadosModel < ApplicationRecord
# users some permission _or_ gives anyone else permission to
# view one of the authorized users.
links_cond = "OR (#{sql_table}.link_class IN (:permission_link_classes) AND "+
- "(#{sql_table}.head_uuid IN (:user_uuids) OR #{sql_table}.tail_uuid IN (:user_uuids)))"
+ "(#{sql_table}.head_uuid IN (#{user_uuids_subquery}) OR #{sql_table}.tail_uuid IN (#{user_uuids_subquery})))"
end
sql_conds = "(#{direct_check} #{owner_check} #{links_cond}) #{exclude_trashed_records}"
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index cb7efe9cc..869ac88f4 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -121,10 +121,15 @@ class User < ArvadosModel
target_owner_uuid = target.owner_uuid if target.respond_to? :owner_uuid
+ user_uuids_subquery = %{
+select target_uuid from materialized_permissions where user_uuid = $1
+and target_uuid like '_____-tpzed-_______________' and traverse_owned=true and perm_level >= #{VAL_FOR_PERM[action]}
+}
+
unless ActiveRecord::Base.connection.
exec_query(%{
SELECT 1 FROM #{PERMISSION_VIEW}
- WHERE user_uuid = $1 and
+ WHERE user_uuid in (#{user_uuids_subquery}) and
((target_uuid = $2 and perm_level >= $3)
or (target_uuid = $4 and perm_level >= $3 and traverse_owned))
},
@@ -438,8 +443,10 @@ update #{PERMISSION_VIEW} set target_uuid=$1 where target_uuid = $2
end
skip_check_permissions_against_full_refresh do
update_permissions self.owner_uuid, self.uuid, 3
+ update_permissions self.uuid, self.uuid, 3
+ update_permissions new_user.owner_uuid, new_user.uuid, 3
end
- update_permissions new_user.owner_uuid, new_user.uuid, 3
+ update_permissions new_user.uuid, new_user.uuid, 3
end
end
diff --git a/services/api/db/migrate/20200501150153_permission_table.rb b/services/api/db/migrate/20200501150153_permission_table.rb
index 77944eb6e..6dd2c29bd 100644
--- a/services/api/db/migrate/20200501150153_permission_table.rb
+++ b/services/api/db/migrate/20200501150153_permission_table.rb
@@ -2,6 +2,8 @@
#
# SPDX-License-Identifier: AGPL-3.0
+require 'update_permissions'
+
class PermissionTable < ActiveRecord::Migration[5.0]
def up
# This is a major migration. We are replacing the
@@ -125,6 +127,8 @@ create view permission_graph_edges as
select groups.owner_uuid as tail_uuid, groups.uuid as head_uuid, (3) as val from groups
union all
select users.owner_uuid as tail_uuid, users.uuid as head_uuid, (3) as val from users
+union all
+ select users.uuid as tail_uuid, users.uuid as head_uuid, (3) as val from users
union all
select links.tail_uuid,
links.head_uuid,
@@ -133,69 +137,18 @@ union all
WHEN links.name = 'can_login' THEN 1
WHEN links.name = 'can_write' THEN 2
WHEN links.name = 'can_manage' THEN 3
+ ELSE 0
END as val
from links
where links.link_class='permission'
}
- ActiveRecord::Base.connection.execute %{
-create or replace function search_permission_graph (starting_uuid varchar(27),
- starting_perm integer,
- override_edge_tail varchar(27) default null,
- override_edge_head varchar(27) default null,
- override_edge_perm integer default null)
- returns table (target_uuid varchar(27), val integer, traverse_owned bool)
-STABLE
-language SQL
-as $$
-/*
- From starting_uuid, perform a recursive self-join on the edges
- to follow chains of permissions. This is a breadth-first search
- of the permission graph. Permission is propagated across edges,
- which may narrow the permission for subsequent links (eg I start
- at can_manage but when traversing a can_read link everything
- touched through that link will only be can_read).
-
- When revoking a permission, we follow the chain of permissions but
- with a permissions level of 0. The update on the permissions table
- has to happen _before_ the permission is actually removed, because
- we need to be able to traverse the edge before it goes away. When
- we do that, we also need to traverse it at the _new_ permission
- level - this is what override_edge_tail/head/perm are for.
-
- Yields the set of objects that are potentially affected, and
- their permission levels granted by having starting_perm on
- starting_uuid.
-
- If starting_uuid is a user, this computes the entire set of
- permissions for that user (because it returns everything that is
- reachable by that user).
-
- Used by the compute_permission_subgraph function.
-*/
-WITH RECURSIVE
- 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,
- case (edges.tail_uuid = override_edge_tail AND
- edges.head_uuid = override_edge_head)
- when true then override_edge_perm
+ override = %{,
+ case (edges.tail_uuid = perm_origin_uuid AND
+ edges.head_uuid = starting_uuid)
+ when true then starting_perm
else null
- end),
- should_traverse_owned(edges.head_uuid, edges.val)
- from permission_graph_edges as edges, traverse_graph
- where traverse_graph.target_uuid = edges.tail_uuid))
- select target_uuid, max(val), bool_or(traverse_owned) from traverse_graph
- group by (target_uuid);
-$$;
+ end
}
ActiveRecord::Base.connection.execute %{
@@ -250,12 +203,13 @@ with
permission change, using search_permission_graph.
*/
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,
- perm_origin_uuid,
- starting_uuid,
- starting_perm)),
+ #{PERM_QUERY_TEMPLATE % {:base_case => %{
+ values (perm_origin_uuid, starting_uuid, starting_perm,
+ should_traverse_owned(starting_uuid, starting_perm),
+ (perm_origin_uuid = starting_uuid or starting_uuid not like '_____-tpzed-_______________'))
+},
+:override => override
+} }),
/* Finds other inbound edges that grant permissions on the objects
in perm_from_start, and computes permissions that originate from
@@ -267,50 +221,24 @@ with
ownership.
*/
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 permission_graph_edges as edges,
- lateral search_permission_graph(edges.head_uuid,
- edges.val,
- perm_origin_uuid,
- starting_uuid,
- starting_perm) as ps
+ #{PERM_QUERY_TEMPLATE % {:base_case => %{
+ select edges.tail_uuid as origin_uuid, edges.head_uuid as target_uuid, edges.val,
+ should_traverse_owned(edges.head_uuid, edges.val),
+ edges.head_uuid like '_____-j7d0g-_______________'
+ from permission_graph_edges as edges
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)),
+ edges.head_uuid = starting_uuid)) and
+ edges.tail_uuid not in (select target_uuid from perm_from_start where target_uuid like '_____-j7d0g-_______________') and
+ edges.head_uuid in (select target_uuid from perm_from_start)
+},
+:override => override
+} }),
/* Combines the permissions computed in the first two phases. */
- partial_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
+ all_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
select * from perm_from_start
union all
select * from additional_perms
- ),
-
- /* If there are any users in the set of potentially affected objects
- and the user's owner was not traversed, recompute permissions for
- that user. This is required because users always have permission
- to themselves (identity property) which would be missing from the
- permission set if the user was traversed while computing
- permissions for another object.
- */
- 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,
- perm_origin_uuid,
- starting_uuid,
- starting_perm) 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)
- ),
-
- /* Combines all the computed permissions into one table. */
- all_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
- select * from partial_perms
- union
- select * from user_identity_perms
)
/* The actual query that produces rows to be added or removed
@@ -351,10 +279,11 @@ with
u.traverse_owned
from all_perms as u, materialized_permissions as m
where u.perm_origin_uuid = m.target_uuid AND m.traverse_owned
+ AND (m.user_uuid = m.target_uuid or m.target_uuid not like '_____-tpzed-_______________')
union all
- select perm_origin_uuid as user_uuid, target_uuid, val as perm_level, traverse_owned
+ select target_uuid as user_uuid, target_uuid, 3, true
from all_perms
- where all_perms.perm_origin_uuid like '_____-tpzed-_______________') as v
+ where all_perms.target_uuid like '_____-tpzed-_______________') as v
group by v.user_uuid, v.target_uuid
$$;
}
@@ -365,8 +294,11 @@ $$;
#
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
+ #{PERM_QUERY_TEMPLATE % {:base_case => %{
+ select uuid, uuid, 3, true, true from users
+},
+:override => ''
+} }
}
end
@@ -376,7 +308,6 @@ from users, lateral search_permission_graph(users.uuid, 3) as g where g.val > 0
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, varchar, 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 view permission_graph_edges;"
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index eaa3d6299..aa3ffae62 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -89,12 +89,35 @@ with
permission change, using search_permission_graph.
*/
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,
- perm_origin_uuid,
- starting_uuid,
- starting_perm)),
+
+WITH RECURSIVE
+ traverse_graph(origin_uuid, target_uuid, val, traverse_owned, starting_set) as (
+
+ values (perm_origin_uuid, starting_uuid, starting_perm,
+ should_traverse_owned(starting_uuid, starting_perm),
+ (perm_origin_uuid = starting_uuid or starting_uuid not like '_____-tpzed-_______________'))
+
+ union
+ (select traverse_graph.origin_uuid,
+ edges.head_uuid,
+ least(edges.val,
+ traverse_graph.val
+ ,
+ case (edges.tail_uuid = perm_origin_uuid AND
+ edges.head_uuid = starting_uuid)
+ when true then starting_perm
+ else null
+ end
+),
+ should_traverse_owned(edges.head_uuid, edges.val),
+ false
+ from permission_graph_edges as edges, traverse_graph
+ where traverse_graph.target_uuid = edges.tail_uuid
+ and (edges.tail_uuid like '_____-j7d0g-_______________' or
+ traverse_graph.starting_set)))
+ select traverse_graph.origin_uuid, target_uuid, max(val) as val, bool_or(traverse_owned) as traverse_owned from traverse_graph
+ group by (traverse_graph.origin_uuid, target_uuid)
+),
/* Finds other inbound edges that grant permissions on the objects
in perm_from_start, and computes permissions that originate from
@@ -106,50 +129,46 @@ with
ownership.
*/
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 permission_graph_edges as edges,
- lateral search_permission_graph(edges.head_uuid,
- edges.val,
- perm_origin_uuid,
- starting_uuid,
- starting_perm) as ps
+
+WITH RECURSIVE
+ traverse_graph(origin_uuid, target_uuid, val, traverse_owned, starting_set) as (
+
+ select edges.tail_uuid as origin_uuid, edges.head_uuid as target_uuid, edges.val,
+ should_traverse_owned(edges.head_uuid, edges.val),
+ edges.head_uuid like '_____-j7d0g-_______________'
+ from permission_graph_edges as edges
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)),
+ edges.head_uuid = starting_uuid)) and
+ edges.tail_uuid not in (select target_uuid from perm_from_start where target_uuid like '_____-j7d0g-_______________') and
+ edges.head_uuid in (select target_uuid from perm_from_start)
+
+ union
+ (select traverse_graph.origin_uuid,
+ edges.head_uuid,
+ least(edges.val,
+ traverse_graph.val
+ ,
+ case (edges.tail_uuid = perm_origin_uuid AND
+ edges.head_uuid = starting_uuid)
+ when true then starting_perm
+ else null
+ end
+),
+ should_traverse_owned(edges.head_uuid, edges.val),
+ false
+ from permission_graph_edges as edges, traverse_graph
+ where traverse_graph.target_uuid = edges.tail_uuid
+ and (edges.tail_uuid like '_____-j7d0g-_______________' or
+ traverse_graph.starting_set)))
+ select traverse_graph.origin_uuid, target_uuid, max(val) as val, bool_or(traverse_owned) as traverse_owned from traverse_graph
+ group by (traverse_graph.origin_uuid, target_uuid)
+),
/* Combines the permissions computed in the first two phases. */
- partial_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
+ all_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
select * from perm_from_start
union all
select * from additional_perms
- ),
-
- /* If there are any users in the set of potentially affected objects
- and the user's owner was not traversed, recompute permissions for
- that user. This is required because users always have permission
- to themselves (identity property) which would be missing from the
- permission set if the user was traversed while computing
- permissions for another object.
- */
- 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,
- perm_origin_uuid,
- starting_uuid,
- starting_perm) 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)
- ),
-
- /* Combines all the computed permissions into one table. */
- all_perms(perm_origin_uuid, target_uuid, val, traverse_owned) as (
- select * from partial_perms
- union
- select * from user_identity_perms
)
/* The actual query that produces rows to be added or removed
@@ -190,10 +209,11 @@ with
u.traverse_owned
from all_perms as u, materialized_permissions as m
where u.perm_origin_uuid = m.target_uuid AND m.traverse_owned
+ AND (m.user_uuid = m.target_uuid or m.target_uuid not like '_____-tpzed-_______________')
union all
- select perm_origin_uuid as user_uuid, target_uuid, val as perm_level, traverse_owned
+ select target_uuid as user_uuid, target_uuid, 3, true
from all_perms
- where all_perms.perm_origin_uuid like '_____-tpzed-_______________') as v
+ where all_perms.target_uuid like '_____-tpzed-_______________') as v
group by v.user_uuid, v.target_uuid
$$;
@@ -242,63 +262,6 @@ WITH RECURSIVE
$$;
---
--- Name: search_permission_graph(character varying, integer, character varying, character varying, integer); Type: FUNCTION; Schema: public; Owner: -
---
-
-CREATE FUNCTION public.search_permission_graph(starting_uuid character varying, starting_perm integer, override_edge_tail character varying DEFAULT NULL::character varying, override_edge_head character varying DEFAULT NULL::character varying, override_edge_perm integer DEFAULT NULL::integer) RETURNS TABLE(target_uuid character varying, val integer, traverse_owned boolean)
- LANGUAGE sql STABLE
- AS $$
-/*
- From starting_uuid, perform a recursive self-join on the edges
- to follow chains of permissions. This is a breadth-first search
- of the permission graph. Permission is propagated across edges,
- which may narrow the permission for subsequent links (eg I start
- at can_manage but when traversing a can_read link everything
- touched through that link will only be can_read).
-
- When revoking a permission, we follow the chain of permissions but
- with a permissions level of 0. The update on the permissions table
- has to happen _before_ the permission is actually removed, because
- we need to be able to traverse the edge before it goes away. When
- we do that, we also need to traverse it at the _new_ permission
- level - this is what override_edge_tail/head/perm are for.
-
- Yields the set of objects that are potentially affected, and
- their permission levels granted by having starting_perm on
- starting_uuid.
-
- If starting_uuid is a user, this computes the entire set of
- permissions for that user (because it returns everything that is
- reachable by that user).
-
- Used by the compute_permission_subgraph function.
-*/
-WITH RECURSIVE
- 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,
- case (edges.tail_uuid = override_edge_tail AND
- edges.head_uuid = override_edge_head)
- when true then override_edge_perm
- else null
- end),
- should_traverse_owned(edges.head_uuid, edges.val)
- from permission_graph_edges as edges, traverse_graph
- where 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: -
--
@@ -1092,6 +1055,11 @@ UNION ALL
users.uuid AS head_uuid,
3 AS val
FROM public.users
+UNION ALL
+ SELECT users.uuid AS tail_uuid,
+ users.uuid AS head_uuid,
+ 3 AS val
+ FROM public.users
UNION ALL
SELECT links.tail_uuid,
links.head_uuid,
@@ -1100,7 +1068,7 @@ UNION ALL
WHEN ((links.name)::text = 'can_login'::text) THEN 1
WHEN ((links.name)::text = 'can_write'::text) THEN 2
WHEN ((links.name)::text = 'can_manage'::text) THEN 3
- ELSE NULL::integer
+ ELSE 0
END AS val
FROM public.links
WHERE ((links.link_class)::text = 'permission'::text);
diff --git a/services/api/lib/update_permissions.rb b/services/api/lib/update_permissions.rb
index b2cf6595b..33a8d9736 100644
--- a/services/api/lib/update_permissions.rb
+++ b/services/api/lib/update_permissions.rb
@@ -9,12 +9,15 @@ def refresh_permissions
ActiveRecord::Base.transaction do
ActiveRecord::Base.connection.execute("LOCK TABLE #{PERMISSION_VIEW}")
ActiveRecord::Base.connection.execute("DELETE FROM #{PERMISSION_VIEW}")
+
ActiveRecord::Base.connection.execute %{
-INSERT INTO #{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
+INSERT INTO materialized_permissions
+ #{PERM_QUERY_TEMPLATE % {:base_case => %{
+ select uuid, uuid, 3, true, true from users
},
- "refresh_permission_view.do"
+:override => ''
+} }
+}, "refresh_permission_view.do"
end
end
@@ -161,9 +164,12 @@ order by user_uuid, target_uuid
}, "check_permissions_against_full_refresh.permission_table"
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
+ select pq.origin_uuid as user_uuid, target_uuid, pq.val as perm_level, pq.traverse_owned from (
+ #{PERM_QUERY_TEMPLATE % {:base_case => %{
+ select uuid, uuid, 3, true, true from users
+},
+:override => ''
+} }) as pq order by origin_uuid, target_uuid
}, "check_permissions_against_full_refresh.full_recompute"
if q1.count != q2.count
@@ -196,3 +202,23 @@ def skip_check_permissions_against_full_refresh
Thread.current[:no_check_permissions_against_full_refresh] = check_perm_was
end
end
+
+PERM_QUERY_TEMPLATE = %{
+WITH RECURSIVE
+ traverse_graph(origin_uuid, target_uuid, val, traverse_owned, starting_set) as (
+ %{base_case}
+ union
+ (select traverse_graph.origin_uuid,
+ edges.head_uuid,
+ least(edges.val,
+ traverse_graph.val
+ %{override}),
+ should_traverse_owned(edges.head_uuid, edges.val),
+ false
+ from permission_graph_edges as edges, traverse_graph
+ where traverse_graph.target_uuid = edges.tail_uuid
+ and (edges.tail_uuid like '_____-j7d0g-_______________' or
+ traverse_graph.starting_set)))
+ select traverse_graph.origin_uuid, target_uuid, max(val) as val, bool_or(traverse_owned) as traverse_owned from traverse_graph
+ group by (traverse_graph.origin_uuid, target_uuid)
+}
diff --git a/services/api/test/unit/permission_test.rb b/services/api/test/unit/permission_test.rb
index a0478ec54..d9383cf8e 100644
--- a/services/api/test/unit/permission_test.rb
+++ b/services/api/test/unit/permission_test.rb
@@ -10,7 +10,7 @@ class PermissionTest < ActiveSupport::TestCase
test "Grant permissions on an object I own" do
set_user_from_auth :active_trustedclient
- ob = Specimen.create
+ ob = Collection.create
assert ob.save
# Ensure I have permission to manage this group even when its owner changes
@@ -24,7 +24,7 @@ class PermissionTest < ActiveSupport::TestCase
test "Delete permission links when deleting an object" do
set_user_from_auth :active_trustedclient
- ob = Specimen.create!
+ ob = Collection.create!
Link.create!(tail_uuid: users(:active).uuid,
head_uuid: ob.uuid,
link_class: 'permission',
@@ -37,7 +37,7 @@ class PermissionTest < ActiveSupport::TestCase
test "permission links owned by root" do
set_user_from_auth :active_trustedclient
- ob = Specimen.create!
+ ob = Collection.create!
perm_link = Link.create!(tail_uuid: users(:active).uuid,
head_uuid: ob.uuid,
link_class: 'permission',
@@ -48,18 +48,18 @@ class PermissionTest < ActiveSupport::TestCase
test "readable_by" do
set_user_from_auth :active_trustedclient
- ob = Specimen.create!
+ ob = Collection.create!
Link.create!(tail_uuid: users(:active).uuid,
head_uuid: ob.uuid,
link_class: 'permission',
name: 'can_read')
- assert Specimen.readable_by(users(:active)).where(uuid: ob.uuid).any?, "user does not have read permission"
+ assert Collection.readable_by(users(:active)).where(uuid: ob.uuid).any?, "user does not have read permission"
end
test "writable_by" do
set_user_from_auth :active_trustedclient
- ob = Specimen.create!
+ ob = Collection.create!
Link.create!(tail_uuid: users(:active).uuid,
head_uuid: ob.uuid,
link_class: 'permission',
@@ -67,6 +67,34 @@ class PermissionTest < ActiveSupport::TestCase
assert ob.writable_by.include?(users(:active).uuid), "user does not have write permission"
end
+ test "update permission link" do
+ set_user_from_auth :admin
+
+ grp = Group.create! name: "blah project", group_class: "project"
+ ob = Collection.create! owner_uuid: grp.uuid
+
+ assert !users(:active).can?(write: ob)
+ assert !users(:active).can?(read: ob)
+
+ l1 = Link.create!(tail_uuid: users(:active).uuid,
+ head_uuid: grp.uuid,
+ link_class: 'permission',
+ name: 'can_write')
+
+ assert users(:active).can?(write: ob)
+ assert users(:active).can?(read: ob)
+
+ l1.update_attributes!(name: 'can_read')
+
+ assert !users(:active).can?(write: ob)
+ assert users(:active).can?(read: ob)
+
+ l1.destroy
+
+ assert !users(:active).can?(write: ob)
+ assert !users(:active).can?(read: ob)
+ end
+
test "writable_by reports requesting user's own uuid for a writable project" do
invited_to_write = users(:project_viewer)
group = groups(:asubproject)
@@ -124,16 +152,16 @@ class PermissionTest < ActiveSupport::TestCase
test "user owns group, group can_manage object's group, user can add permissions" do
set_user_from_auth :admin
- owner_grp = Group.create!(owner_uuid: users(:active).uuid)
-
- sp_grp = Group.create!
- sp = Specimen.create!(owner_uuid: sp_grp.uuid)
+ owner_grp = Group.create!(owner_uuid: users(:active).uuid, group_class: "role")
+ sp_grp = Group.create!(group_class: "project")
Link.create!(link_class: 'permission',
name: 'can_manage',
tail_uuid: owner_grp.uuid,
head_uuid: sp_grp.uuid)
+ sp = Collection.create!(owner_uuid: sp_grp.uuid)
+
# active user owns owner_grp, which has can_manage permission on sp_grp
# user should be able to add permissions on sp.
set_user_from_auth :active_trustedclient
@@ -149,7 +177,7 @@ class PermissionTest < ActiveSupport::TestCase
skip "can_manage permission on a non-group object" do
set_user_from_auth :admin
- ob = Specimen.create!
+ ob = Collection.create!
# grant can_manage permission to active
perm_link = Link.create!(tail_uuid: users(:active).uuid,
head_uuid: ob.uuid,
@@ -170,7 +198,7 @@ class PermissionTest < ActiveSupport::TestCase
test "user without can_manage permission may not modify permission link" do
set_user_from_auth :admin
- ob = Specimen.create!
+ ob = Collection.create!
# grant can_manage permission to active
perm_link = Link.create!(tail_uuid: users(:active).uuid,
head_uuid: ob.uuid,
@@ -192,7 +220,8 @@ class PermissionTest < ActiveSupport::TestCase
manager = create :active_user, first_name: "Manage", last_name: "Er"
minion = create :active_user, first_name: "Min", last_name: "Ion"
minions_specimen = act_as_user minion do
- Specimen.create!
+ g = Group.create! name: "minon project", group_class: "project"
+ Collection.create! owner_uuid: g.uuid
end
# Manager creates a group. (Make sure it doesn't magically give
# anyone any additional permissions.)
@@ -255,7 +284,7 @@ class PermissionTest < ActiveSupport::TestCase
create(:permission_link,
name: 'can_manage', tail_uuid: manager.uuid, head_uuid: minion.uuid)
end
- assert_empty(Specimen
+ assert_empty(Collection
.readable_by(manager)
.where(uuid: minions_specimen.uuid),
"manager saw the minion's private stuff")
@@ -273,7 +302,7 @@ class PermissionTest < ActiveSupport::TestCase
act_as_user manager do
# Now, manager can read and write Minion's stuff.
- assert_not_empty(Specimen
+ assert_not_empty(Collection
.readable_by(manager)
.where(uuid: minions_specimen.uuid),
"manager could not find minion's specimen by uuid")
@@ -309,12 +338,12 @@ class PermissionTest < ActiveSupport::TestCase
"#{a.first_name} should be able to see 'b' in the user list")
a_specimen = act_as_user a do
- Specimen.create!
+ Collection.create!
end
- assert_not_empty(Specimen.readable_by(a).where(uuid: a_specimen.uuid),
- "A cannot read own Specimen, following test probably useless.")
- assert_empty(Specimen.readable_by(b).where(uuid: a_specimen.uuid),
- "B can read A's Specimen")
+ assert_not_empty(Collection.readable_by(a).where(uuid: a_specimen.uuid),
+ "A cannot read own Collection, following test probably useless.")
+ assert_empty(Collection.readable_by(b).where(uuid: a_specimen.uuid),
+ "B can read A's Collection")
[a,b].each do |u|
assert_empty(User.readable_by(u).where(uuid: other.uuid),
"#{u.first_name} can see OTHER in the user list")
@@ -341,13 +370,13 @@ class PermissionTest < ActiveSupport::TestCase
test "cannot create with owner = unwritable user" do
set_user_from_auth :rominiadmin
assert_raises ArvadosModel::PermissionDeniedError, "created with owner = unwritable user" do
- Specimen.create!(owner_uuid: users(:active).uuid)
+ Collection.create!(owner_uuid: users(:active).uuid)
end
end
test "cannot change owner to unwritable user" do
set_user_from_auth :rominiadmin
- ob = Specimen.create!
+ ob = Collection.create!
assert_raises ArvadosModel::PermissionDeniedError, "changed owner to unwritable user" do
ob.update_attributes!(owner_uuid: users(:active).uuid)
end
@@ -356,13 +385,13 @@ class PermissionTest < ActiveSupport::TestCase
test "cannot create with owner = unwritable group" do
set_user_from_auth :rominiadmin
assert_raises ArvadosModel::PermissionDeniedError, "created with owner = unwritable group" do
- Specimen.create!(owner_uuid: groups(:aproject).uuid)
+ Collection.create!(owner_uuid: groups(:aproject).uuid)
end
end
test "cannot change owner to unwritable group" do
set_user_from_auth :rominiadmin
- ob = Specimen.create!
+ ob = Collection.create!
assert_raises ArvadosModel::PermissionDeniedError, "changed owner to unwritable group" do
ob.update_attributes!(owner_uuid: groups(:aproject).uuid)
end
diff --git a/services/api/test/unit/user_test.rb b/services/api/test/unit/user_test.rb
index 596cd415f..7fcd36d70 100644
--- a/services/api/test/unit/user_test.rb
+++ b/services/api/test/unit/user_test.rb
@@ -478,15 +478,6 @@ class UserTest < ActiveSupport::TestCase
vm = VirtualMachine.create
- # Set up the bogus Link
- bad_uuid = 'zzzzz-tpzed-xyzxyzxyzxyzxyz'
-
- resp_link = Link.create ({tail_uuid: email, link_class: 'permission',
- name: 'can_login', head_uuid: bad_uuid})
- resp_link.save(validate: false)
-
- verify_link resp_link, 'permission', 'can_login', email, bad_uuid
-
response = user.setup(repo_name: 'foo/testrepo',
vm_uuid: vm.uuid)
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list