[ARVADOS] created: 59d6af850acadece8512e701188f111d82e03497
Git user
git at public.curoverse.com
Thu Aug 24 12:04:43 EDT 2017
at 59d6af850acadece8512e701188f111d82e03497 (commit)
commit 59d6af850acadece8512e701188f111d82e03497
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Thu Aug 24 12:03:09 2017 -0400
12032: Use permission_view in subquery to filter objects readable by user.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/app/controllers/arvados/v1/repositories_controller.rb b/services/api/app/controllers/arvados/v1/repositories_controller.rb
index a28cee2..b88e10c 100644
--- a/services/api/app/controllers/arvados/v1/repositories_controller.rb
+++ b/services/api/app/controllers/arvados/v1/repositories_controller.rb
@@ -46,7 +46,7 @@ class Arvados::V1::RepositoriesController < ApplicationController
# group also has access to the repository. Access level is
# min(group-to-repo permission, user-to-group permission).
user_aks.each do |user_uuid, _|
- perm_mask = all_group_permissions[user_uuid][perm.tail_uuid]
+ perm_mask = all_group_permissions[user_uuid].andand[perm.tail_uuid]
if not perm_mask
next
elsif perm_mask[:manage] and perm.name == 'can_manage'
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index ea69735..457b1bf 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -252,49 +252,68 @@ class ArvadosModel < ActiveRecord::Base
kwargs = {}
end
- # Check if any of the users are admin. If so, we're done.
- if users_list.select { |u| u.is_admin }.any?
- # Return existing relation with no new filters.
- return where({})
- end
-
# Collect the UUIDs of the authorized users.
- user_uuids = users_list.map { |u| u.uuid }
-
- # Collect the UUIDs of all groups readable by any of the
- # authorized users. If one of these (or the UUID of one of the
- # authorized users themselves) is an object's owner_uuid, that
- # object is readable.
- owner_uuids = user_uuids + users_list.flat_map { |u| u.groups_i_can(:read) }
- owner_uuids.uniq!
-
- sql_conds = []
sql_table = kwargs.fetch(:table_name, table_name)
+ include_trashed = kwargs.fetch(:include_trashed, 0)
- # Match any object (evidently a group or user) whose UUID is
- # listed explicitly in owner_uuids.
- sql_conds += ["#{sql_table}.uuid in (:owner_uuids)"]
+ sql_conds = []
+ user_uuids = users_list.map { |u| u.uuid }
- # Match any object whose owner is listed explicitly in
- # owner_uuids.
- sql_conds += ["#{sql_table}.owner_uuid IN (:owner_uuids)"]
+ User.install_view('permission')
- # Match the head of any permission link whose tail is listed
- # explicitly in owner_uuids.
- sql_conds += ["#{sql_table}.uuid IN (SELECT head_uuid FROM links WHERE link_class='permission' AND tail_uuid IN (:owner_uuids))"]
+ # Check if any of the users are admin.
+ if users_list.select { |u| u.is_admin }.any?
+ # For admins, only filter on "trashed"
+ # sql_conds += ["#{sql_table}.uuid in (SELECT target_uuid
+ # FROM permission_view
+ # WHERE trashed in (:include_trashed)
+ # GROUP BY user_uuid, target_uuid)"]
+
+ # if self.column_names.include? 'owner_uuid'
+ # sql_conds[0] += "AND #{sql_table}.owner_uuid in (SELECT target_uuid
+ # FROM permission_view
+ # WHERE trashed in (:include_trashed)
+ # GROUP BY user_uuid, target_uuid)"
+ # end
+ return where({})
+ else
+ # Match any object (evidently a group or user) whose UUID is
+ # listed explicitly in user_uuids.
+ sql_conds += ["#{sql_table}.uuid in (:user_uuids)"]
+
+ # Match any object whose owner is listed explicitly in
+ # user_uuids.
+ sql_conds += ["#{sql_table}.owner_uuid IN (:user_uuids)"]
+
+ # At least read permission from user_uuid to target_uuid of object
+ sql_conds += ["#{sql_table}.uuid in (SELECT target_uuid
+ FROM permission_view
+ WHERE user_uuid in (:user_uuids) and perm_level >= 1 and trashed = (:include_trashed)
+ GROUP BY user_uuid, target_uuid)"]
+
+ if self.column_names.include? 'owner_uuid'
+ # At least read permission from user_uuid to target_uuid that owns object
+ sql_conds += ["#{sql_table}.owner_uuid in (SELECT target_uuid
+ FROM permission_view
+ WHERE user_uuid in (:user_uuids) and
+ target_owner_uuid IS NOT NULL and
+ perm_level >= 1 and trashed = (:include_trashed)
+ GROUP BY user_uuid, target_uuid)"]
+ end
- if sql_table == "links"
- # Match any permission link that gives one of the authorized
- # users some permission _or_ gives anyone else permission to
- # view one of the authorized users.
- sql_conds += ["(#{sql_table}.link_class in (:permission_link_classes) AND "+
- "(#{sql_table}.head_uuid IN (:user_uuids) OR #{sql_table}.tail_uuid IN (:user_uuids)))"]
+ if sql_table == "links"
+ # Match any permission link that gives one of the authorized
+ # users some permission _or_ gives anyone else permission to
+ # view one of the authorized users.
+ sql_conds += ["(#{sql_table}.link_class in (:permission_link_classes) AND "+
+ "(#{sql_table}.head_uuid IN (:user_uuids) OR #{sql_table}.tail_uuid IN (:user_uuids)))"]
+ end
end
where(sql_conds.join(' OR '),
- owner_uuids: owner_uuids,
user_uuids: user_uuids,
- permission_link_classes: ['permission', 'resources'])
+ permission_link_classes: ['permission', 'resources'],
+ include_trashed: include_trashed)
end
def save_with_unique_name!
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index f093410..c8c9bfb 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -155,14 +155,14 @@ class User < ArvadosModel
install_view('permission')
all_perms = {}
ActiveRecord::Base.connection.
- exec_query('SELECT user_uuid, target_owner_uuid, max(perm_level)
+ exec_query('SELECT user_uuid, target_owner_uuid, max(perm_level), max(trashed)
FROM permission_view
WHERE target_owner_uuid IS NOT NULL
GROUP BY user_uuid, target_owner_uuid',
# "name" arg is a query label that appears in logs:
"all_group_permissions",
- ).rows.each do |user_uuid, group_uuid, max_p_val|
- all_perms[user_uuid] ||= {}
+ ).rows.each do |user_uuid, group_uuid, max_p_val, trashed|
+ all_perms[user_uuid] ||= {user_uuid => {:read => true, :write => true, :manage => true}}
all_perms[user_uuid][group_uuid] = PERMS_FOR_VAL[max_p_val.to_i]
end
all_perms
@@ -174,9 +174,9 @@ class User < ArvadosModel
def calculate_group_permissions
self.class.install_view('permission')
- group_perms = {}
+ group_perms = {self.uuid => {:read => true, :write => true, :manage => true}}
ActiveRecord::Base.connection.
- exec_query('SELECT target_owner_uuid, max(perm_level)
+ exec_query('SELECT target_owner_uuid, max(perm_level), max(trashed)
FROM permission_view
WHERE user_uuid = $1
AND target_owner_uuid IS NOT NULL
@@ -185,7 +185,7 @@ class User < ArvadosModel
"group_permissions for #{uuid}",
# "binds" arg is an array of [col_id, value] for '$1' vars:
[[nil, uuid]],
- ).rows.each do |group_uuid, max_p_val|
+ ).rows.each do |group_uuid, max_p_val, trashed|
group_perms[group_uuid] = PERMS_FOR_VAL[max_p_val.to_i]
end
Rails.cache.write "#{PERM_CACHE_PREFIX}#{self.uuid}", group_perms, expires_in: PERM_CACHE_TTL
diff --git a/services/api/lib/create_permission_view.sql b/services/api/lib/create_permission_view.sql
index 6e24d30..10c4b27 100644
--- a/services/api/lib/create_permission_view.sql
+++ b/services/api/lib/create_permission_view.sql
@@ -2,6 +2,21 @@
--
-- SPDX-License-Identifier: AGPL-3.0
+-- constructing perm_edges
+-- 1. get the list of all permission links,
+-- 2. any can_manage link or permission link to a group means permission should "follow through"
+-- (as a special case, can_manage links to a user grant access to everything owned by the user,
+-- unlike can_read or can_write which only grant access to the user record)
+-- 3. add all owner->owned relationships between groups as can_manage edges
+--
+-- constructing permissions
+-- 1. base case: start with set of all users as the working set
+-- 2. recursive case:
+-- join with edges where the tail is in the working set and "follow" is true
+-- produce a new working set with the head (target) of each edge
+-- set permission to the least permission encountered on the path
+-- propagate trashed flag down
+
CREATE TEMPORARY VIEW permission_view AS
WITH RECURSIVE
perm_value (name, val) AS (
@@ -11,35 +26,41 @@ perm_value (name, val) AS (
('can_write', 2),
('can_manage', 3)
),
-perm_edges (tail_uuid, head_uuid, val, follow) AS (
+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
+ (pv.val = 3 OR groups.uuid IS NOT NULL) AS follow,
+ 0::smallint AS trashed
FROM links
LEFT JOIN perm_value pv ON pv.name = links.name
LEFT JOIN groups ON pv.val<3 AND groups.uuid = links.head_uuid
WHERE links.link_class = 'permission'
UNION ALL
- SELECT owner_uuid, uuid, 3, true FROM groups
+ SELECT owner_uuid, uuid, 3, true, 0::smallint FROM groups
),
-perm (val, follow, user_uuid, target_uuid) AS (
+perm (val, follow, user_uuid, target_uuid, trashed, startnode) AS (
SELECT 3::smallint AS val,
- true AS follow,
+ false AS follow,
users.uuid::varchar(32) AS user_uuid,
- users.uuid::varchar(32) AS target_uuid
+ users.uuid::varchar(32) AS target_uuid,
+ 0::smallint AS trashed,
+ true AS startnode
FROM users
UNION
- SELECT LEAST(perm.val, edges.val)::smallint AS val,
- edges.follow AS follow,
- perm.user_uuid::varchar(32) AS user_uuid,
- edges.head_uuid::varchar(32) AS target_uuid
+ SELECT LEAST(perm.val, edges.val)::smallint AS val,
+ edges.follow AS follow,
+ perm.user_uuid::varchar(32) AS user_uuid,
+ edges.head_uuid::varchar(32) AS target_uuid,
+ GREATEST(perm.trashed, edges.trashed)::smallint AS trashed,
+ false AS startnode
FROM perm
INNER JOIN perm_edges edges
- ON perm.follow AND edges.tail_uuid = perm.target_uuid
+ ON (perm.startnode or perm.follow) AND edges.tail_uuid = perm.target_uuid
)
SELECT user_uuid,
target_uuid,
val AS perm_level,
- CASE follow WHEN true THEN target_uuid ELSE NULL END AS target_owner_uuid
+ CASE follow WHEN true THEN target_uuid ELSE NULL END AS target_owner_uuid,
+ trashed
FROM perm;
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list