[ARVADOS] updated: c8fd30d6353b88061deee66516515bbdc7f8652d
Git user
git at public.curoverse.com
Fri Aug 26 23:43:00 EDT 2016
Summary of changes:
services/api/app/models/arvados_model.rb | 76 ++++++++++-------------
services/api/app/models/container.rb | 8 ++-
services/api/app/models/log.rb | 10 ++-
services/api/test/integration/permissions_test.rb | 10 +--
4 files changed, 50 insertions(+), 54 deletions(-)
via c8fd30d6353b88061deee66516515bbdc7f8652d (commit)
via c623dcdbc673cdb08bcc1460f0450a17bf412f69 (commit)
via 59a0fd6d150b06b64263052fc0a8ded14ea8f287 (commit)
via f7138c7c158383f35f54ec9cef0cb2ab580f75ea (commit)
from fbb6cc64c687a92ef3598c2cce9ef7a2d54e777f (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 c8fd30d6353b88061deee66516515bbdc7f8652d
Author: Tom Clegg <tom at curoverse.com>
Date: Fri Aug 26 23:34:14 2016 -0400
9799: Clean up permission code and comments.
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index a001677..b5ffe98 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -185,44 +185,42 @@ class ArvadosModel < ActiveRecord::Base
return self
end
- # Collect the uuids for each user and any groups readable by each user.
+ # 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) }
+
sql_conds = []
sql_table = kwargs.fetch(:table_name, table_name)
- or_object_uuid = ''
-
- # This row is owned by a member of users_list, or owned by a group
- # readable by a member of users_list
- # or
- # This row uuid is the uuid of a member of users_list
- # or
- # A permission link exists ('write' and 'manage' implicitly include
- # 'read') from a member of users_list, or a group readable by users_list,
- # to this row, or to the owner of this row (see join() below).
+
+ # Match any object (evidently a group or user) whose UUID is
+ # listed explicitly in owner_uuids.
sql_conds += ["#{sql_table}.uuid in (:owner_uuids)"]
- if owner_uuids.any?
- permitted = "(SELECT head_uuid FROM links WHERE link_class='permission' AND tail_uuid IN (:owner_uuids))"
- sql_conds += ["#{sql_table}.owner_uuid IN (:owner_uuids)",
- "#{sql_table}.uuid IN #{permitted}"]
- end
+ # Match any object whose owner is listed explicitly in
+ # owner_uuids.
+ sql_conds += ["#{sql_table}.owner_uuid IN (:owner_uuids)"]
+
+ # 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))"]
- if sql_table == "links" and user_uuids.any?
- # This row is a 'permission' or 'resources' link whose head or
- # tail _is_ the acting user.
- sql_conds += ["(#{sql_table}.link_class in (#{sanitize 'permission'}, #{sanitize 'resources'}) 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
- # Link head points to this row, or to the owner of this row (the
- # thing to be read)
- #
- # Link tail originates from this user, or a group that is readable
- # by this user (the identity with authorization to read)
- #
- # Link class is 'permission' ('write' and 'manage' implicitly
- # include 'read')
- where(sql_conds.join(' OR '), owner_uuids: owner_uuids, user_uuids: user_uuids)
+ where(sql_conds.join(' OR '),
+ owner_uuids: owner_uuids,
+ user_uuids: user_uuids,
+ permission_link_classes: ['permission', 'resources'])
end
def logged_attributes
commit c623dcdbc673cdb08bcc1460f0450a17bf412f69
Author: Tom Clegg <tom at curoverse.com>
Date: Fri Aug 26 23:16:15 2016 -0400
9799: Fix test order dependency.
diff --git a/services/api/test/integration/permissions_test.rb b/services/api/test/integration/permissions_test.rb
index 723aa1b..e4db862 100644
--- a/services/api/test/integration/permissions_test.rb
+++ b/services/api/test/integration/permissions_test.rb
@@ -1,9 +1,14 @@
require 'test_helper'
class PermissionsTest < ActionDispatch::IntegrationTest
+ include DbCurrentTime
include CurrentApiClient # for empty_collection
fixtures :users, :groups, :api_client_authorizations, :collections
+ teardown do
+ User.invalidate_permissions_cache db_current_time.to_i
+ end
+
test "adding and removing direct can_read links" do
# try to read collection as spectator
get "/arvados/v1/collections/#{collections(:foo_file).uuid}", {:format => :json}, auth(:spectator)
commit 59a0fd6d150b06b64263052fc0a8ded14ea8f287
Author: Tom Clegg <tom at curoverse.com>
Date: Fri Aug 26 23:06:11 2016 -0400
9799: Remove redundant test.
diff --git a/services/api/test/integration/permissions_test.rb b/services/api/test/integration/permissions_test.rb
index 44b5e6e..723aa1b 100644
--- a/services/api/test/integration/permissions_test.rb
+++ b/services/api/test/integration/permissions_test.rb
@@ -341,11 +341,6 @@ class PermissionsTest < ActionDispatch::IntegrationTest
assert_response 404
end
- test "get_permissions returns 404 for unreadable uuid" do
- get "/arvados/v1/permissions/#{groups(:public).uuid}", nil, auth(:active)
- assert_response 404
- end
-
test "get_permissions returns 403 if user can read but not manage" do
post "/arvados/v1/links", {
:link => {
commit f7138c7c158383f35f54ec9cef0cb2ab580f75ea
Author: Tom Clegg <tom at curoverse.com>
Date: Fri Aug 26 23:06:01 2016 -0400
9799: Dry up SQL statements using named bind parameters.
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 012d99b..a001677 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -187,9 +187,8 @@ class ArvadosModel < ActiveRecord::Base
# Collect the uuids for each user and any groups readable by each user.
user_uuids = users_list.map { |u| u.uuid }
- uuid_list = user_uuids + users_list.flat_map { |u| u.groups_i_can(:read) }
+ owner_uuids = user_uuids + users_list.flat_map { |u| u.groups_i_can(:read) }
sql_conds = []
- sql_params = []
sql_table = kwargs.fetch(:table_name, table_name)
or_object_uuid = ''
@@ -201,25 +200,18 @@ class ArvadosModel < ActiveRecord::Base
# A permission link exists ('write' and 'manage' implicitly include
# 'read') from a member of users_list, or a group readable by users_list,
# to this row, or to the owner of this row (see join() below).
- sql_conds += ["#{sql_table}.uuid in (?)"]
- sql_params += [user_uuids]
+ sql_conds += ["#{sql_table}.uuid in (:owner_uuids)"]
- if uuid_list.any?
- sql_conds += ["#{sql_table}.owner_uuid in (?)"]
- sql_params += [uuid_list]
-
- sanitized_uuid_list = uuid_list.
- collect { |uuid| sanitize(uuid) }.join(', ')
- permitted_uuids = "(SELECT head_uuid FROM links WHERE link_class='permission' AND tail_uuid IN (#{sanitized_uuid_list}))"
- sql_conds += ["#{sql_table}.uuid IN #{permitted_uuids}"]
+ if owner_uuids.any?
+ permitted = "(SELECT head_uuid FROM links WHERE link_class='permission' AND tail_uuid IN (:owner_uuids))"
+ sql_conds += ["#{sql_table}.owner_uuid IN (:owner_uuids)",
+ "#{sql_table}.uuid IN #{permitted}"]
end
- if sql_table == "links" and users_list.any?
- # This row is a 'permission' or 'resources' link class
- # The uuid for a member of users_list is referenced in either the head
- # or tail of the link
- sql_conds += ["(#{sql_table}.link_class in (#{sanitize 'permission'}, #{sanitize 'resources'}) AND (#{sql_table}.head_uuid IN (?) OR #{sql_table}.tail_uuid IN (?)))"]
- sql_params += [user_uuids, user_uuids]
+ if sql_table == "links" and user_uuids.any?
+ # This row is a 'permission' or 'resources' link whose head or
+ # tail _is_ the acting user.
+ sql_conds += ["(#{sql_table}.link_class in (#{sanitize 'permission'}, #{sanitize 'resources'}) AND (#{sql_table}.head_uuid IN (:user_uuids) OR #{sql_table}.tail_uuid IN (:user_uuids)))"]
end
# Link head points to this row, or to the owner of this row (the
@@ -230,7 +222,7 @@ class ArvadosModel < ActiveRecord::Base
#
# Link class is 'permission' ('write' and 'manage' implicitly
# include 'read')
- where(sql_conds.join(' OR '), *sql_params)
+ where(sql_conds.join(' OR '), owner_uuids: owner_uuids, user_uuids: user_uuids)
end
def logged_attributes
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index ae4d983..f73c8b7 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -82,9 +82,11 @@ class Container < ArvadosModel
end
user_uuids = users_list.map { |u| u.uuid }
uuid_list = user_uuids + users_list.flat_map { |u| u.groups_i_can(:read) }
- permitted_uuids = "(SELECT head_uuid FROM links WHERE link_class='permission' AND tail_uuid IN (?))"
- joins(:container_requests).where("container_requests.uuid IN #{permitted_uuids} OR container_requests.owner_uuid IN (?)",
- uuid_list, uuid_list)
+ permitted = "(SELECT head_uuid FROM links WHERE link_class='permission' AND tail_uuid IN (:uuids))"
+ joins(:container_requests).
+ where("container_requests.uuid IN #{permitted} OR "+
+ "container_requests.owner_uuid IN (:uuids)",
+ uuids: uuid_list)
end
protected
diff --git a/services/api/app/models/log.rb b/services/api/app/models/log.rb
index 7a8b50a..5a58a55 100644
--- a/services/api/app/models/log.rb
+++ b/services/api/app/models/log.rb
@@ -59,10 +59,14 @@ class Log < ArvadosModel
end
user_uuids = users_list.map { |u| u.uuid }
uuid_list = user_uuids + users_list.flat_map { |u| u.groups_i_can(:read) }
- permitted_uuids = "(SELECT head_uuid FROM links WHERE link_class='permission' AND tail_uuid IN (?))"
+ permitted = "(SELECT head_uuid FROM links WHERE link_class='permission' AND tail_uuid IN (:uuids))"
joins("LEFT JOIN container_requests ON container_requests.container_uuid=logs.object_uuid").
- where("logs.object_uuid IN #{permitted_uuids} OR container_requests.uuid IN (?) OR container_requests.owner_uuid IN (?) OR logs.object_uuid IN (?) OR logs.object_owner_uuid IN (?)",
- uuid_list, uuid_list, uuid_list, uuid_list, uuid_list)
+ where("logs.object_uuid IN #{permitted} OR "+
+ "container_requests.uuid IN (:uuids) OR "+
+ "container_requests.owner_uuid IN (:uuids) OR "+
+ "logs.object_uuid IN (:uuids) OR "+
+ "logs.object_owner_uuid IN (:uuids)",
+ uuids: uuid_list)
end
protected
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list