[ARVADOS] created: d901415a244f16562fdcb87b2f2ded40cc369c45
Git user
git at public.curoverse.com
Thu Aug 31 10:42:38 EDT 2017
at d901415a244f16562fdcb87b2f2ded40cc369c45 (commit)
commit d901415a244f16562fdcb87b2f2ded40cc369c45
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Thu Aug 31 10:37:13 2017 -0400
12032: Refactor readable_by to minimize subqueries.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 46b96a9..d09283d 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -180,7 +180,7 @@ class ApplicationController < ActionController::Base
end
def find_objects_for_index
- @objects ||= model_class.readable_by(*@read_users, {:include_trash => (params[:include_trash] || ['destroy', 'trash', 'untrash'].include?(action_name))})
+ @objects ||= model_class.readable_by(*@read_users, {:include_trash => (params[:include_trash] || 'untrash' == action_name)})
apply_where_limit_order_params
end
diff --git a/services/api/app/controllers/arvados/v1/groups_controller.rb b/services/api/app/controllers/arvados/v1/groups_controller.rb
index 9e0cdf8..8e195ff 100644
--- a/services/api/app/controllers/arvados/v1/groups_controller.rb
+++ b/services/api/app/controllers/arvados/v1/groups_controller.rb
@@ -163,13 +163,14 @@ class Arvados::V1::GroupsController < ApplicationController
end
end.compact
- if klass == Collection and params[:include_trash]
- @objects = klass.unscoped.readable_by(*@read_users, {:include_trash => params[:include_trash]}).
- order(request_order).where(where_conds)
- else
- @objects = klass.readable_by(*@read_users, {:include_trash => params[:include_trash]}).
- order(request_order).where(where_conds)
- end
+ query_on = if klass == Collection and params[:include_trash]
+ klass.unscoped
+ else
+ klass
+ end
+ @objects = query_on.readable_by(*@read_users, {:include_trash => params[:include_trash]}).
+ order(request_order).where(where_conds)
+
klass_limit = limit_all - all_objects.count
@limit = klass_limit
apply_where_limit_order_params klass
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 518d4c8..4a4f7ec 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -265,52 +265,46 @@ class ArvadosModel < ActiveRecord::Base
# Check if any of the users are admin.
if users_list.select { |u| u.is_admin }.any?
if !include_trash
- # exclude rows that are trashed.
+ # exclude rows that are explicitly trashed.
if self.column_names.include? "owner_uuid"
- sql_conds += ["NOT EXISTS(SELECT target_uuid
+ sql_conds += ["NOT EXISTS(SELECT 1
FROM permission_view
WHERE trashed = 1 AND
(#{sql_table}.uuid = target_uuid OR #{sql_table}.owner_uuid = target_uuid))"]
else
- sql_conds += ["NOT EXISTS(SELECT target_uuid
+ sql_conds += ["NOT EXISTS(SELECT 1
FROM permission_view
WHERE trashed = 1 AND
(#{sql_table}.uuid = target_uuid))"]
end
end
else
- trash_clause = if !include_trash then "trashed = 0 AND" else "" end
-
# Can read object (evidently a group or user) whose UUID is listed
# explicitly in user_uuids.
sql_conds += ["#{sql_table}.uuid IN (:user_uuids)"]
-
- direct_permission_check = "EXISTS(SELECT 1 FROM permission_view
- WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 AND #{trash_clause}
- (#{sql_table}.uuid = target_uuid))"
+ trashed_check = if include_trash then "EXISTS" else "0 IN " end
+ perm_check = "perm_level >= 1"
if self.column_names.include? "owner_uuid"
- # if an explicit permission row exists for the uuid in question, apply
- # the "direct_permission_check"
- # if not, check for permission to read the owner instead
- sql_conds += ["CASE
- WHEN EXISTS(select 1 FROM permission_view where target_uuid = #{sql_table}.uuid)
- THEN #{direct_permission_check}
- ELSE EXISTS(SELECT 1 FROM permission_view
- WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 AND #{trash_clause}
- (#{sql_table}.owner_uuid = target_uuid AND target_owner_uuid is NOT NULL))
- END"]
- # Can also read if one of the users is the owner of the object.
- trash_clause = if !include_trash
- "1 NOT IN (SELECT trashed
- FROM permission_view
- WHERE #{sql_table}.uuid = target_uuid) AND"
- else
- ""
- end
- sql_conds += ["(#{trash_clause} #{sql_table}.owner_uuid IN (:user_uuids))"]
+ # to be readable:
+ # row(s) exists granting readable permission to uuid or owner, and none are trashed
+ sql_conds += ["#{trashed_check}(SELECT MAX(trashed) FROM permission_view "+
+ "WHERE user_uuid IN (:user_uuids) AND #{perm_check} AND "+
+ "(#{sql_table}.uuid = target_uuid OR (#{sql_table}.owner_uuid = target_uuid AND target_owner_uuid is NOT NULL)))"]
+ # reader is owner, and item is not trashed
+ not_trashed_check = if include_trash then
+ ""
+ else
+ "AND 1 NOT IN (SELECT MAX(trashed) FROM permission_view "+
+ "WHERE #{sql_table}.uuid = target_uuid OR #{sql_table}.owner_uuid = target_uuid)"
+ end
+ sql_conds += ["(#{sql_table}.owner_uuid IN (:user_uuids) #{not_trashed_check})"]
else
- sql_conds += [direct_permission_check]
+ # to be readable:
+ # * a non-trash row exists with readable permission uuid
+ sql_conds += ["#{trashed_check}(SELECT MAX(trashed) FROM permission_view "+
+ "WHERE user_uuid IN (:user_uuids) AND #{perm_check} AND "+
+ "#{sql_table}.uuid = target_uuid) "]
end
if sql_table == "links"
diff --git a/services/api/test/integration/groups_test.rb b/services/api/test/integration/groups_test.rb
index 6226ffd..c4ab3cf 100644
--- a/services/api/test/integration/groups_test.rb
+++ b/services/api/test/integration/groups_test.rb
@@ -99,7 +99,8 @@ class GroupsTest < ActionDispatch::IntegrationTest
test "group contents with include trash collections" do
get "/arvados/v1/groups/contents", {
include_trash: "true",
- filters: [["uuid", "is_a", "arvados#collection"]].to_json
+ filters: [["uuid", "is_a", "arvados#collection"]].to_json,
+ limit: 1000
}, auth(:active)
assert_response 200
@@ -108,4 +109,17 @@ class GroupsTest < ActionDispatch::IntegrationTest
assert_includes coll_uuids, collections(:foo_collection_in_aproject).uuid
assert_includes coll_uuids, collections(:expired_collection).uuid
end
+
+ test "group contents without trash collections" do
+ get "/arvados/v1/groups/contents", {
+ filters: [["uuid", "is_a", "arvados#collection"]].to_json,
+ limit: 1000
+ }, auth(:active)
+ assert_response 200
+
+ coll_uuids = []
+ json_response['items'].each { |c| coll_uuids << c['uuid'] }
+ assert_includes coll_uuids, collections(:foo_collection_in_aproject).uuid
+ assert_not_includes coll_uuids, collections(:expired_collection).uuid
+ end
end
commit 5b0057c61154b1f46ed83d6f392eae21c2cfa8f7
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Wed Aug 30 13:13:29 2017 -0400
12032: Log.readable_by uses permission_view
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/app/models/log.rb b/services/api/app/models/log.rb
index 7f2d3ef..99d0e28 100644
--- a/services/api/app/models/log.rb
+++ b/services/api/app/models/log.rb
@@ -67,17 +67,14 @@ class Log < ArvadosModel
return self
end
user_uuids = users_list.map { |u| u.uuid }
- uuid_list = user_uuids + users_list.flat_map { |u| u.groups_i_can(:read) }
- uuid_list.uniq!
- permitted = "(SELECT head_uuid FROM links WHERE link_class='permission' AND tail_uuid IN (:uuids))"
+
+ User.install_view('permission')
+
joins("LEFT JOIN container_requests ON container_requests.container_uuid=logs.object_uuid").
- 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.owner_uuid IN (:uuids) OR "+
- "logs.object_owner_uuid IN (:uuids)",
- uuids: uuid_list)
+ where("EXISTS(SELECT target_uuid FROM permission_view "+
+ "WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 AND "+
+ "target_uuid IN (container_requests.uuid, container_requests.owner_uuid, logs.object_uuid, logs.owner_uuid, logs.object_owner_uuid))",
+ user_uuids: user_uuids)
end
protected
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 0e2db76..9f053c0 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -152,8 +152,8 @@ class User < ArvadosModel
# Return a hash of {user_uuid: group_perms}
def self.all_group_permissions
- install_view('permission')
all_perms = {}
+ User.install_view('permission')
ActiveRecord::Base.connection.
exec_query('SELECT user_uuid, target_owner_uuid, perm_level, trashed
FROM permission_view
@@ -171,9 +171,8 @@ class User < ArvadosModel
# and perm_hash[:write] are true if this user can read and write
# objects owned by group_uuid.
def calculate_group_permissions
- self.class.install_view('permission')
-
group_perms = {self.uuid => {:read => true, :write => true, :manage => true}}
+ User.install_view('permission')
ActiveRecord::Base.connection.
exec_query('SELECT target_owner_uuid, perm_level, trashed
FROM permission_view
commit 5e161d86721d036761f77c45a25b9d25bacc7be3
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Tue Aug 29 09:08:19 2017 -0400
12032: Controller support for group trash.
Conttroller support and testing for contents, index, show, trash, untrash,
containers.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index a3d8f08..46b96a9 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -180,7 +180,7 @@ class ApplicationController < ActionController::Base
end
def find_objects_for_index
- @objects ||= model_class.readable_by(*@read_users)
+ @objects ||= model_class.readable_by(*@read_users, {:include_trash => (params[:include_trash] || ['destroy', 'trash', 'untrash'].include?(action_name))})
apply_where_limit_order_params
end
diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index 87d88fe..0312d95 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -3,9 +3,11 @@
# SPDX-License-Identifier: AGPL-3.0
require "arvados/keep"
+require "trashable"
class Arvados::V1::CollectionsController < ApplicationController
include DbCurrentTime
+ include TrashableController
def self._index_requires_parameters
(super rescue {}).
@@ -16,7 +18,6 @@ class Arvados::V1::CollectionsController < ApplicationController
})
end
-
def create
if resource_attrs[:uuid] and (loc = Keep::Locator.parse(resource_attrs[:uuid]))
resource_attrs[:portable_data_hash] = loc.to_s
@@ -58,39 +59,6 @@ class Arvados::V1::CollectionsController < ApplicationController
end
end
- def destroy
- if !@object.is_trashed
- @object.update_attributes!(trash_at: db_current_time)
- end
- earliest_delete = (@object.trash_at +
- Rails.configuration.blob_signature_ttl.seconds)
- if @object.delete_at > earliest_delete
- @object.update_attributes!(delete_at: earliest_delete)
- end
- show
- end
-
- def trash
- if !@object.is_trashed
- @object.update_attributes!(trash_at: db_current_time)
- end
- show
- end
-
- def untrash
- if @object.is_trashed
- @object.trash_at = nil
-
- if params[:ensure_unique_name]
- @object.save_with_unique_name!
- else
- @object.save!
- end
- else
- raise InvalidStateTransitionError
- end
- show
- end
def find_collections(visited, sp, &b)
case sp
diff --git a/services/api/app/controllers/arvados/v1/groups_controller.rb b/services/api/app/controllers/arvados/v1/groups_controller.rb
index 75ef095..9e0cdf8 100644
--- a/services/api/app/controllers/arvados/v1/groups_controller.rb
+++ b/services/api/app/controllers/arvados/v1/groups_controller.rb
@@ -2,7 +2,19 @@
#
# SPDX-License-Identifier: AGPL-3.0
+require "trashable"
+
class Arvados::V1::GroupsController < ApplicationController
+ include TrashableController
+
+ def self._index_requires_parameters
+ (super rescue {}).
+ merge({
+ include_trash: {
+ type: 'boolean', required: false, description: "Include items whose is_trashed attribute is true."
+ },
+ })
+ end
def self._contents_requires_parameters
params = _index_requires_parameters.
@@ -152,10 +164,10 @@ class Arvados::V1::GroupsController < ApplicationController
end.compact
if klass == Collection and params[:include_trash]
- @objects = klass.unscoped.readable_by(*@read_users).
+ @objects = klass.unscoped.readable_by(*@read_users, {:include_trash => params[:include_trash]}).
order(request_order).where(where_conds)
else
- @objects = klass.readable_by(*@read_users).
+ @objects = klass.readable_by(*@read_users, {:include_trash => params[:include_trash]}).
order(request_order).where(where_conds)
end
klass_limit = limit_all - all_objects.count
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index ccd8f82..518d4c8 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -254,7 +254,8 @@ class ArvadosModel < ActiveRecord::Base
# Collect the UUIDs of the authorized users.
sql_table = kwargs.fetch(:table_name, table_name)
- include_trashed = kwargs.fetch(:include_trashed, false)
+ include_trash = kwargs.fetch(:include_trash, false)
+ query_on = kwargs.fetch(:query_on, self)
sql_conds = []
user_uuids = users_list.map { |u| u.uuid }
@@ -263,49 +264,53 @@ class ArvadosModel < ActiveRecord::Base
# Check if any of the users are admin.
if users_list.select { |u| u.is_admin }.any?
- if !include_trashed
+ if !include_trash
# exclude rows that are trashed.
if self.column_names.include? "owner_uuid"
sql_conds += ["NOT EXISTS(SELECT target_uuid
- FROM permission_view pv
+ FROM permission_view
WHERE trashed = 1 AND
- (#{sql_table}.uuid = pv.target_uuid OR #{sql_table}.owner_uuid = pv.target_uuid))"]
+ (#{sql_table}.uuid = target_uuid OR #{sql_table}.owner_uuid = target_uuid))"]
else
sql_conds += ["NOT EXISTS(SELECT target_uuid
- FROM permission_view pv
+ FROM permission_view
WHERE trashed = 1 AND
- (#{sql_table}.uuid = pv.target_uuid))"]
+ (#{sql_table}.uuid = target_uuid))"]
end
end
else
- # Match objects which appear in the permission view
- trash_clause = if !include_trashed then "trashed = 0 AND" else "" end
+ trash_clause = if !include_trash then "trashed = 0 AND" else "" end
- # Match any object (evidently a group or user) whose UUID is
- # listed explicitly in user_uuids.
+ # Can read object (evidently a group or user) whose UUID is listed
+ # explicitly in user_uuids.
sql_conds += ["#{sql_table}.uuid IN (:user_uuids)"]
+ direct_permission_check = "EXISTS(SELECT 1 FROM permission_view
+ WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 AND #{trash_clause}
+ (#{sql_table}.uuid = target_uuid))"
+
if self.column_names.include? "owner_uuid"
- sql_conds += ["EXISTS(SELECT target_uuid
- FROM permission_view pv
+ # if an explicit permission row exists for the uuid in question, apply
+ # the "direct_permission_check"
+ # if not, check for permission to read the owner instead
+ sql_conds += ["CASE
+ WHEN EXISTS(select 1 FROM permission_view where target_uuid = #{sql_table}.uuid)
+ THEN #{direct_permission_check}
+ ELSE EXISTS(SELECT 1 FROM permission_view
WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 AND #{trash_clause}
- (#{sql_table}.uuid = pv.target_uuid OR
- (#{sql_table}.owner_uuid = pv.target_uuid AND pv.target_owner_uuid is NOT NULL)))"]
- # Match any object whose owner is listed explicitly in
- # user_uuids.
- trash_clause = if !include_trashed
+ (#{sql_table}.owner_uuid = target_uuid AND target_owner_uuid is NOT NULL))
+ END"]
+ # Can also read if one of the users is the owner of the object.
+ trash_clause = if !include_trash
"1 NOT IN (SELECT trashed
- FROM permission_view pv
- WHERE #{sql_table}.uuid = pv.target_uuid) AND"
+ FROM permission_view
+ WHERE #{sql_table}.uuid = target_uuid) AND"
else
""
end
sql_conds += ["(#{trash_clause} #{sql_table}.owner_uuid IN (:user_uuids))"]
else
- sql_conds += ["EXISTS(SELECT target_uuid
- FROM permission_view pv
- WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 AND #{trash_clause}
- (#{sql_table}.uuid = pv.target_uuid))"]
+ sql_conds += [direct_permission_check]
end
if sql_table == "links"
@@ -317,9 +322,9 @@ class ArvadosModel < ActiveRecord::Base
end
end
- where(sql_conds.join(' OR '),
- user_uuids: user_uuids,
- permission_link_classes: ['permission', 'resources'])
+ query_on.where(sql_conds.join(' OR '),
+ user_uuids: user_uuids,
+ permission_link_classes: ['permission', 'resources'])
end
def save_with_unique_name!
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 36c87af..d61ba75 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -297,17 +297,15 @@ class Container < ArvadosModel
end
def self.readable_by(*users_list)
- if users_list.select { |u| u.is_admin }.any?
- return self
+ # Load optional keyword arguments, if they exist.
+ if users_list.last.is_a? Hash
+ kwargs = users_list.pop
+ else
+ kwargs = {}
end
- user_uuids = users_list.map { |u| u.uuid }
- uuid_list = user_uuids + users_list.flat_map { |u| u.groups_i_can(:read) }
- uuid_list.uniq!
- 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)
+ kwargs[:query_on] = joins(:container_requests)
+ users_list.push kwargs
+ ContainerRequest.readable_by(*users_list)
end
def final?
diff --git a/services/api/app/models/log.rb b/services/api/app/models/log.rb
index 73f143e..7f2d3ef 100644
--- a/services/api/app/models/log.rb
+++ b/services/api/app/models/log.rb
@@ -60,6 +60,9 @@ class Log < ArvadosModel
end
def self.readable_by(*users_list)
+ if users_list.last.is_a? Hash
+ users_list.pop
+ end
if users_list.select { |u| u.is_admin }.any?
return self
end
diff --git a/services/api/config/routes.rb b/services/api/config/routes.rb
index 2e9d618..5b6fe80 100644
--- a/services/api/config/routes.rb
+++ b/services/api/config/routes.rb
@@ -30,6 +30,8 @@ Server::Application.routes.draw do
resources :groups do
get 'contents', on: :collection
get 'contents', on: :member
+ post 'trash', on: :member
+ post 'untrash', on: :member
end
resources :humans
resources :job_tasks
diff --git a/services/api/lib/create_permission_view.sql b/services/api/lib/create_permission_view.sql
index 639610a..71ac8c4 100644
--- a/services/api/lib/create_permission_view.sql
+++ b/services/api/lib/create_permission_view.sql
@@ -37,9 +37,8 @@ perm_edges (tail_uuid, head_uuid, val, follow, trashed) AS (
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, CASE
- WHEN trash_at IS NOT NULL and trash_at < clock_timestamp() THEN 1
- ELSE 0 END
+ SELECT owner_uuid, uuid, 3, true,
+ CASE WHEN trash_at IS NOT NULL and trash_at < clock_timestamp() THEN 1 ELSE 0 END
FROM groups
),
perm (val, follow, user_uuid, target_uuid, trashed, startnode) AS (
diff --git a/services/api/lib/trashable.rb b/services/api/lib/trashable.rb
index 1e2f466..38ebaf7 100644
--- a/services/api/lib/trashable.rb
+++ b/services/api/lib/trashable.rb
@@ -89,3 +89,40 @@ module Trashable
true
end
end
+
+module TrashableController
+ def destroy
+ if !@object.is_trashed
+ @object.update_attributes!(trash_at: db_current_time)
+ end
+ earliest_delete = (@object.trash_at +
+ Rails.configuration.blob_signature_ttl.seconds)
+ if @object.delete_at > earliest_delete
+ @object.update_attributes!(delete_at: earliest_delete)
+ end
+ show
+ end
+
+ def trash
+ if !@object.is_trashed
+ @object.update_attributes!(trash_at: db_current_time)
+ end
+ show
+ end
+
+ def untrash
+ if @object.is_trashed
+ @object.trash_at = nil
+
+ if params[:ensure_unique_name]
+ @object.save_with_unique_name!
+ else
+ @object.save!
+ end
+ else
+ raise InvalidStateTransitionError
+ end
+ show
+ end
+
+end
diff --git a/services/api/test/fixtures/groups.yml b/services/api/test/fixtures/groups.yml
index 1cb5817..2411831 100644
--- a/services/api/test/fixtures/groups.yml
+++ b/services/api/test/fixtures/groups.yml
@@ -313,6 +313,13 @@ trashed_subproject:
owner_uuid: zzzzz-j7d0g-trashedproject1
name: trashed subproject
group_class: project
+ is_trashed: false
+
+trashed_subproject3:
+ uuid: zzzzz-j7d0g-trashedproject3
+ owner_uuid: zzzzz-j7d0g-trashedproject1
+ name: trashed subproject 3
+ group_class: project
trash_at: 2001-01-01T00:00:00Z
delete_at: 2038-03-01T00:00:00Z
is_trashed: true
diff --git a/services/api/test/functional/arvados/v1/groups_controller_test.rb b/services/api/test/functional/arvados/v1/groups_controller_test.rb
index 043d665..b80fcb1 100644
--- a/services/api/test/functional/arvados/v1/groups_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb
@@ -533,111 +533,145 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
### trashed project tests ###
[:active, :admin].each do |auth|
- test "trashed project is hidden in contents (#{auth})" do
- authorize_with auth
- get :contents, {
- id: users(:active).uuid,
- format: :json,
- }
- item_uuids = json_response['items'].map do |item|
- item['uuid']
+ # project: to query, to untrash, is visible, parent contents listing success
+ [[:trashed_project, [], false, true],
+ [:trashed_project, [:trashed_project], true, true],
+ [:trashed_subproject, [], false, false],
+ [:trashed_subproject, [:trashed_project], true, true],
+ [:trashed_subproject3, [:trashed_project], false, true],
+ [:trashed_subproject3, [:trashed_subproject3], false, false],
+ [:trashed_subproject3, [:trashed_project, :trashed_subproject3], true, true],
+ ].each do |project, untrash, visible, success|
+
+ test "contents listing #{project} #{untrash} as #{auth}" do
+ authorize_with auth
+ untrash.each do |pr|
+ Group.find_by_uuid(groups(pr).uuid).update! is_trashed: false
+ end
+ get :contents, {
+ id: groups(project).owner_uuid,
+ format: :json
+ }
+ if success
+ assert_response :success
+ item_uuids = json_response['items'].map do |item|
+ item['uuid']
+ end
+ if visible
+ assert_includes(item_uuids, groups(project).uuid)
+ else
+ assert_not_includes(item_uuids, groups(project).uuid)
+ end
+ else
+ assert_response 404
+ end
end
- assert_not_includes(item_uuids, groups(:trashed_project).uuid)
- end
- test "untrashed project is visible in contents (#{auth})" do
- authorize_with auth
- Group.find_by_uuid(groups(:trashed_project).uuid).update! is_trashed: false
- get :contents, {
- id: users(:active).uuid,
- format: :json,
- }
- item_uuids = json_response['items'].map do |item|
- item['uuid']
+ test "contents of #{project} #{untrash} as #{auth}" do
+ authorize_with auth
+ untrash.each do |pr|
+ Group.find_by_uuid(groups(pr).uuid).update! is_trashed: false
+ end
+ get :contents, {
+ id: groups(project).uuid,
+ format: :json
+ }
+ if visible
+ assert_response :success
+ else
+ assert_response 404
+ end
end
- assert_includes(item_uuids, groups(:trashed_project).uuid)
- end
- test "trashed project is hidden in index (#{auth})" do
- authorize_with :active
- get :index, {
- format: :json,
- }
- item_uuids = json_response['items'].map do |item|
- item['uuid']
+ test "index #{project} #{untrash} as #{auth}" do
+ authorize_with auth
+ untrash.each do |pr|
+ Group.find_by_uuid(groups(pr).uuid).update! is_trashed: false
+ end
+ get :index, {
+ format: :json,
+ }
+ assert_response :success
+ item_uuids = json_response['items'].map do |item|
+ item['uuid']
+ end
+ if visible
+ assert_includes(item_uuids, groups(project).uuid)
+ else
+ assert_not_includes(item_uuids, groups(project).uuid)
+ end
end
- assert_not_includes(item_uuids, groups(:trashed_project).uuid)
- assert_not_includes(item_uuids, groups(:trashed_subproject).uuid)
- end
- test "untrashed project is visible in index (#{auth})" do
- authorize_with :active
- Group.find_by_uuid(groups(:trashed_project).uuid).update! is_trashed: false
- get :index, {
- format: :json,
- }
- item_uuids = json_response['items'].map do |item|
- item['uuid']
+ test "show #{project} #{untrash} as #{auth}" do
+ authorize_with auth
+ untrash.each do |pr|
+ Group.find_by_uuid(groups(pr).uuid).update! is_trashed: false
+ end
+ get :show, {
+ id: groups(project).uuid,
+ format: :json
+ }
+ if visible
+ assert_response :success
+ else
+ assert_response 404
+ end
end
- assert_includes(item_uuids, groups(:trashed_project).uuid)
- assert_includes(item_uuids, groups(:trashed_subproject).uuid)
- end
- test "cannot get contents of trashed project (#{auth})" do
- authorize_with :active
- get :contents, {
- id: groups(:trashed_project).uuid,
- format: :json,
- }
- assert_response 404
- end
+ test "show include_trash #{project} #{untrash} as #{auth}" do
+ authorize_with auth
+ untrash.each do |pr|
+ Group.find_by_uuid(groups(pr).uuid).update! is_trashed: false
+ end
+ get :show, {
+ id: groups(project).uuid,
+ format: :json,
+ include_trash: true
+ }
+ assert_response :success
+ end
- test "cannot get contents of trashed subproject (#{auth})" do
- authorize_with :active
- get :contents, {
- id: groups(:trashed_subproject).uuid,
- format: :json,
- }
- assert_response 404
+ test "index include_trash #{project} #{untrash} as #{auth}" do
+ authorize_with auth
+ untrash.each do |pr|
+ Group.find_by_uuid(groups(pr).uuid).update! is_trashed: false
+ end
+ get :index, {
+ format: :json,
+ include_trash: true
+ }
+ assert_response :success
+ item_uuids = json_response['items'].map do |item|
+ item['uuid']
+ end
+ assert_includes(item_uuids, groups(project).uuid)
+ end
end
- test "can get contents of untrashed project (#{auth})" do
- authorize_with :active
- Group.find_by_uuid(groups(:trashed_project).uuid).update! is_trashed: false
- get :contents, {
+ test "delete project #{auth}" do
+ authorize_with auth
+ [:trashed_project].each do |pr|
+ Group.find_by_uuid(groups(pr).uuid).update! is_trashed: false
+ end
+ assert !Group.find_by_uuid(groups(:trashed_project).uuid).is_trashed
+ post :destroy, {
id: groups(:trashed_project).uuid,
format: :json,
}
- assert_response 200
- end
-
- test "can get contents of untrashed subproject (#{auth})" do
- authorize_with :active
- Group.find_by_uuid(groups(:trashed_project).uuid).update! is_trashed: false
- get :contents, {
- id: groups(:trashed_subproject).uuid,
- format: :json,
- }
- assert_response 200
+ assert_response :success
+ assert Group.find_by_uuid(groups(:trashed_project).uuid).is_trashed
end
- test "cannot show trashed project (#{auth})" do
- authorize_with :active
- get :show, {
+ test "untrash project #{auth}" do
+ authorize_with auth
+ assert Group.find_by_uuid(groups(:trashed_project).uuid).is_trashed
+ post :untrash, {
id: groups(:trashed_project).uuid,
format: :json,
}
- assert_response 404
+ assert_response :success
+ assert !Group.find_by_uuid(groups(:trashed_project).uuid).is_trashed
end
- test "cannot show trashed subproject (#{auth})" do
- authorize_with :active
- get :show, {
- id: groups(:trashed_subproject).uuid,
- format: :json,
- }
- assert_response 404
- end
end
-
end
diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb
index e2c8829..4672acd 100644
--- a/services/api/test/unit/group_test.rb
+++ b/services/api/test/unit/group_test.rb
@@ -69,30 +69,87 @@ class GroupTest < ActiveSupport::TestCase
assert Collection.readable_by(users(:active)).where(uuid: col.uuid).any?
g_foo.update! is_trashed: true
assert Collection.readable_by(users(:active)).where(uuid: col.uuid).empty?
+ assert Collection.readable_by(users(:active), {:include_trash => true}).where(uuid: col.uuid).any?
g_foo.update! is_trashed: false
assert Collection.readable_by(users(:active)).where(uuid: col.uuid).any?
end
+ test "delete group" do
+ set_user_from_auth :active_trustedclient
- test "delete group propagates to subgroups" do
+ g_foo = Group.create!(name: "foo")
+ g_bar = Group.create!(name: "bar", owner_uuid: g_foo.uuid)
+ g_baz = Group.create!(name: "baz", owner_uuid: g_bar.uuid)
+
+ assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
+ assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).any?
+ assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).any?
+ g_foo.update! is_trashed: true
+ assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).empty?
+ assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).empty?
+ assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).empty?
+
+ assert Group.readable_by(users(:active), {:include_trash => true}).where(uuid: g_foo.uuid).any?
+ assert Group.readable_by(users(:active), {:include_trash => true}).where(uuid: g_bar.uuid).any?
+ assert Group.readable_by(users(:active), {:include_trash => true}).where(uuid: g_baz.uuid).any?
+ end
+
+
+ test "delete subgroup" do
set_user_from_auth :active_trustedclient
g_foo = Group.create!(name: "foo")
g_bar = Group.create!(name: "bar", owner_uuid: g_foo.uuid)
- col = Collection.create!(owner_uuid: g_bar.uuid)
+ g_baz = Group.create!(name: "baz", owner_uuid: g_bar.uuid)
assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).any?
- assert Collection.readable_by(users(:active)).where(uuid: col.uuid).any?
+ assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).any?
+ g_bar.update! is_trashed: true
+
+ assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
+ assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).empty?
+ assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).empty?
+
+ assert Group.readable_by(users(:active), {:include_trash => true}).where(uuid: g_bar.uuid).any?
+ assert Group.readable_by(users(:active), {:include_trash => true}).where(uuid: g_baz.uuid).any?
+ end
+
+ test "delete subsubgroup" do
+ set_user_from_auth :active_trustedclient
+
+ g_foo = Group.create!(name: "foo")
+ g_bar = Group.create!(name: "bar", owner_uuid: g_foo.uuid)
+ g_baz = Group.create!(name: "baz", owner_uuid: g_bar.uuid)
+
+ assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
+ assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).any?
+ assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).any?
+ g_baz.update! is_trashed: true
+ assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
+ assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).any?
+ assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).empty?
+ assert Group.readable_by(users(:active), {:include_trash => true}).where(uuid: g_baz.uuid).any?
+ end
+
+
+ test "delete group propagates to subgroups" do
+ set_user_from_auth :active_trustedclient
+
+ g_foo = groups(:trashed_project)
+ g_bar = groups(:trashed_subproject)
+ g_baz = groups(:trashed_subproject3)
+ col = collections(:collection_in_trashed_subproject)
- g_foo.update! is_trashed: true
assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).empty?
assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).empty?
+ assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).empty?
assert Collection.readable_by(users(:active)).where(uuid: col.uuid).empty?
set_user_from_auth :admin
assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).empty?
assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).empty?
+ assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).empty?
assert Collection.readable_by(users(:active)).where(uuid: col.uuid).empty?
set_user_from_auth :active_trustedclient
@@ -100,6 +157,12 @@ class GroupTest < ActiveSupport::TestCase
assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).any?
assert Collection.readable_by(users(:active)).where(uuid: col.uuid).any?
+
+ # this one should still be deleted.
+ assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).empty?
+
+ g_baz.update! is_trashed: false
+ assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).any?
end
end
commit 549a1cc8d7854d58b1182adbee64c771222023c7
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Mon Aug 28 09:57:05 2017 -0400
12032: Adding controller tests
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/test/fixtures/groups.yml b/services/api/test/fixtures/groups.yml
index c9783d0..1cb5817 100644
--- a/services/api/test/fixtures/groups.yml
+++ b/services/api/test/fixtures/groups.yml
@@ -310,7 +310,7 @@ trashed_project:
trashed_subproject:
uuid: zzzzz-j7d0g-trashedproject2
- owner_uuid:
+ owner_uuid: zzzzz-j7d0g-trashedproject1
name: trashed subproject
group_class: project
trash_at: 2001-01-01T00:00:00Z
diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index 56b0790..4cf8778 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -1090,4 +1090,44 @@ EOS
assert_nil json_response['delete_at']
assert_match /^same name for trashed and persisted collections \(\d{4}-\d\d-\d\d.*?Z\)$/, json_response['name']
end
+
+ test 'cannot show collection in trashed subproject' do
+ authorize_with :active
+ get :show, {
+ id: collections(:collection_in_trashed_subproject).uuid,
+ format: :json
+ }
+ assert_response 404
+ end
+
+ test 'can show collection in untrashed subproject' do
+ authorize_with :active
+ Group.find_by_uuid(groups(:trashed_project).uuid).update! is_trashed: false
+ get :show, {
+ id: collections(:collection_in_trashed_subproject).uuid,
+ format: :json,
+ }
+ assert_response :success
+ end
+
+ test 'cannot index collection in trashed subproject' do
+ authorize_with :active
+ get :index
+ assert_response :success
+ item_uuids = json_response['items'].map do |item|
+ item['uuid']
+ end
+ assert_not_includes(item_uuids, collections(:collection_in_trashed_subproject).uuid)
+ end
+
+ test 'can index collection in untrashed subproject' do
+ authorize_with :active
+ Group.find_by_uuid(groups(:trashed_project).uuid).update! is_trashed: false
+ get :index
+ assert_response :success
+ item_uuids = json_response['items'].map do |item|
+ item['uuid']
+ end
+ assert_includes(item_uuids, collections(:collection_in_trashed_subproject).uuid)
+ end
end
diff --git a/services/api/test/functional/arvados/v1/groups_controller_test.rb b/services/api/test/functional/arvados/v1/groups_controller_test.rb
index 5ae1e9a..043d665 100644
--- a/services/api/test/functional/arvados/v1/groups_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb
@@ -529,4 +529,115 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
assert_includes(owners, groups(:aproject).uuid)
assert_includes(owners, groups(:asubproject).uuid)
end
+
+ ### trashed project tests ###
+
+ [:active, :admin].each do |auth|
+ test "trashed project is hidden in contents (#{auth})" do
+ authorize_with auth
+ get :contents, {
+ id: users(:active).uuid,
+ format: :json,
+ }
+ item_uuids = json_response['items'].map do |item|
+ item['uuid']
+ end
+ assert_not_includes(item_uuids, groups(:trashed_project).uuid)
+ end
+
+ test "untrashed project is visible in contents (#{auth})" do
+ authorize_with auth
+ Group.find_by_uuid(groups(:trashed_project).uuid).update! is_trashed: false
+ get :contents, {
+ id: users(:active).uuid,
+ format: :json,
+ }
+ item_uuids = json_response['items'].map do |item|
+ item['uuid']
+ end
+ assert_includes(item_uuids, groups(:trashed_project).uuid)
+ end
+
+ test "trashed project is hidden in index (#{auth})" do
+ authorize_with :active
+ get :index, {
+ format: :json,
+ }
+ item_uuids = json_response['items'].map do |item|
+ item['uuid']
+ end
+ assert_not_includes(item_uuids, groups(:trashed_project).uuid)
+ assert_not_includes(item_uuids, groups(:trashed_subproject).uuid)
+ end
+
+ test "untrashed project is visible in index (#{auth})" do
+ authorize_with :active
+ Group.find_by_uuid(groups(:trashed_project).uuid).update! is_trashed: false
+ get :index, {
+ format: :json,
+ }
+ item_uuids = json_response['items'].map do |item|
+ item['uuid']
+ end
+ assert_includes(item_uuids, groups(:trashed_project).uuid)
+ assert_includes(item_uuids, groups(:trashed_subproject).uuid)
+ end
+
+ test "cannot get contents of trashed project (#{auth})" do
+ authorize_with :active
+ get :contents, {
+ id: groups(:trashed_project).uuid,
+ format: :json,
+ }
+ assert_response 404
+ end
+
+ test "cannot get contents of trashed subproject (#{auth})" do
+ authorize_with :active
+ get :contents, {
+ id: groups(:trashed_subproject).uuid,
+ format: :json,
+ }
+ assert_response 404
+ end
+
+ test "can get contents of untrashed project (#{auth})" do
+ authorize_with :active
+ Group.find_by_uuid(groups(:trashed_project).uuid).update! is_trashed: false
+ get :contents, {
+ id: groups(:trashed_project).uuid,
+ format: :json,
+ }
+ assert_response 200
+ end
+
+ test "can get contents of untrashed subproject (#{auth})" do
+ authorize_with :active
+ Group.find_by_uuid(groups(:trashed_project).uuid).update! is_trashed: false
+ get :contents, {
+ id: groups(:trashed_subproject).uuid,
+ format: :json,
+ }
+ assert_response 200
+ end
+
+ test "cannot show trashed project (#{auth})" do
+ authorize_with :active
+ get :show, {
+ id: groups(:trashed_project).uuid,
+ format: :json,
+ }
+ assert_response 404
+ end
+
+ test "cannot show trashed subproject (#{auth})" do
+ authorize_with :active
+ get :show, {
+ id: groups(:trashed_subproject).uuid,
+ format: :json,
+ }
+ assert_response 404
+ end
+ end
+
end
commit 61ba951ab92487e8123d83b1d77a16e24575ddcd
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Fri Aug 25 14:23:30 2017 -0400
12032: Add test fixtures
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/lib/create_permission_view.sql b/services/api/lib/create_permission_view.sql
index e6195d7..639610a 100644
--- a/services/api/lib/create_permission_view.sql
+++ b/services/api/lib/create_permission_view.sql
@@ -37,7 +37,10 @@ perm_edges (tail_uuid, head_uuid, val, follow, trashed) AS (
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, CASE is_trashed WHEN true THEN 1 ELSE 0 END FROM groups
+ SELECT owner_uuid, uuid, 3, true, CASE
+ WHEN trash_at IS NOT NULL and trash_at < clock_timestamp() THEN 1
+ ELSE 0 END
+ FROM groups
),
perm (val, follow, user_uuid, target_uuid, trashed, startnode) AS (
SELECT 3::smallint AS val,
diff --git a/services/api/test/fixtures/collections.yml b/services/api/test/fixtures/collections.yml
index 7777f74..8023503 100644
--- a/services/api/test/fixtures/collections.yml
+++ b/services/api/test/fixtures/collections.yml
@@ -703,6 +703,17 @@ same_name_as_trashed_coll_to_test_name_conflict_on_untrash:
manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:file1 0:0:file2\n"
name: same name for trashed and persisted collections
+collection_in_trashed_subproject:
+ uuid: zzzzz-4zz18-trashedproj2col
+ portable_data_hash: 80cf6dd2cf079dd13f272ec4245cb4a8+48
+ owner_uuid: zzzzz-j7d0g-trashedproject2
+ created_at: 2014-02-03T17:22:54Z
+ modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+ modified_by_user_uuid: zzzzz-tpzed-d9tiejq69daie8f
+ modified_at: 2014-02-03T17:22:54Z
+ updated_at: 2014-02-03T17:22:54Z
+ manifest_text: ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:file1 0:0:file2\n"
+ name: collection in trashed subproject
# Test Helper trims the rest of the file
diff --git a/services/api/test/fixtures/groups.yml b/services/api/test/fixtures/groups.yml
index 90d087f..c9783d0 100644
--- a/services/api/test/fixtures/groups.yml
+++ b/services/api/test/fixtures/groups.yml
@@ -298,3 +298,21 @@ starred_and_shared_active_user_project:
name: Starred and shared active user project
description: Starred and shared active user project
group_class: project
+
+trashed_project:
+ uuid: zzzzz-j7d0g-trashedproject1
+ owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ name: trashed project
+ group_class: project
+ trash_at: 2001-01-01T00:00:00Z
+ delete_at: 2038-03-01T00:00:00Z
+ is_trashed: true
+
+trashed_subproject:
+ uuid: zzzzz-j7d0g-trashedproject2
+ owner_uuid:
+ name: trashed subproject
+ group_class: project
+ trash_at: 2001-01-01T00:00:00Z
+ delete_at: 2038-03-01T00:00:00Z
+ is_trashed: true
commit 724da26ed62a59b4879a8a02eaaab99bf681ee10
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Thu Aug 24 21:58:38 2017 -0400
12032: Fix hiding trashed groups directly owned by user.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 5a9d43f..ccd8f82 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -293,7 +293,14 @@ class ArvadosModel < ActiveRecord::Base
(#{sql_table}.owner_uuid = pv.target_uuid AND pv.target_owner_uuid is NOT NULL)))"]
# Match any object whose owner is listed explicitly in
# user_uuids.
- sql_conds += ["#{sql_table}.owner_uuid IN (:user_uuids)"]
+ trash_clause = if !include_trashed
+ "1 NOT IN (SELECT trashed
+ FROM permission_view pv
+ WHERE #{sql_table}.uuid = pv.target_uuid) AND"
+ else
+ ""
+ end
+ sql_conds += ["(#{trash_clause} #{sql_table}.owner_uuid IN (:user_uuids))"]
else
sql_conds += ["EXISTS(SELECT target_uuid
FROM permission_view pv
diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb
index 11f546b..e2c8829 100644
--- a/services/api/test/unit/group_test.rb
+++ b/services/api/test/unit/group_test.rb
@@ -90,10 +90,12 @@ class GroupTest < ActiveSupport::TestCase
assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).empty?
assert Collection.readable_by(users(:active)).where(uuid: col.uuid).empty?
- assert Group.readable_by(users(:active), {:include_trashed => true}).where(uuid: g_foo.uuid).any?
- assert Group.readable_by(users(:active), {:include_trashed => true}).where(uuid: g_bar.uuid).any?
- assert Collection.readable_by(users(:active), {:include_trashed => true}).where(uuid: col.uuid).any?
+ set_user_from_auth :admin
+ assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).empty?
+ assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).empty?
+ assert Collection.readable_by(users(:active)).where(uuid: col.uuid).empty?
+ set_user_from_auth :active_trustedclient
g_foo.update! is_trashed: false
assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).any?
commit 9620f75a4d257bb5b6381f6b9671ed4179db1d12
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Thu Aug 24 17:09:49 2017 -0400
12032: Tests for group delete behavior.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index 0dd2a73..861800d 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -3,12 +3,15 @@
# SPDX-License-Identifier: AGPL-3.0
require 'can_be_an_owner'
+require 'trashable'
class Group < ArvadosModel
include HasUuid
include KindAndEtag
include CommonApiTemplate
include CanBeAnOwner
+ include Trashable
+
after_create :invalidate_permissions_cache
after_update :maybe_invalidate_permissions_cache
before_create :assign_name
@@ -18,6 +21,9 @@ class Group < ArvadosModel
t.add :group_class
t.add :description
t.add :writable_by
+ t.add :delete_at
+ t.add :trash_at
+ t.add :is_trashed
end
def maybe_invalidate_permissions_cache
diff --git a/services/api/db/migrate/20170824202826_trashable_groups.rb b/services/api/db/migrate/20170824202826_trashable_groups.rb
new file mode 100644
index 0000000..b7e3373
--- /dev/null
+++ b/services/api/db/migrate/20170824202826_trashable_groups.rb
@@ -0,0 +1,7 @@
+class TrashableGroups < ActiveRecord::Migration
+ def change
+ add_column :groups, :trash_at, :datetime
+ add_column :groups, :delete_at, :datetime
+ add_column :groups, :is_trashed, :boolean, null: false, default: false
+ end
+end
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index 7e6bb1d..90aa0a3 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -391,7 +391,10 @@ CREATE TABLE groups (
name character varying(255) NOT NULL,
description character varying(524288),
updated_at timestamp without time zone NOT NULL,
- group_class character varying(255)
+ group_class character varying(255),
+ trash_at timestamp without time zone,
+ delete_at timestamp without time zone,
+ is_trashed boolean DEFAULT false NOT NULL
);
@@ -2791,3 +2794,5 @@ INSERT INTO schema_migrations (version) VALUES ('20170419175801');
INSERT INTO schema_migrations (version) VALUES ('20170628185847');
+INSERT INTO schema_migrations (version) VALUES ('20170824202826');
+
diff --git a/services/api/lib/create_permission_view.sql b/services/api/lib/create_permission_view.sql
index 82839e8..e6195d7 100644
--- a/services/api/lib/create_permission_view.sql
+++ b/services/api/lib/create_permission_view.sql
@@ -37,7 +37,7 @@ perm_edges (tail_uuid, head_uuid, val, follow, trashed) AS (
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, 0::smallint FROM groups
+ SELECT owner_uuid, uuid, 3, true, CASE is_trashed WHEN true THEN 1 ELSE 0 END FROM groups
),
perm (val, follow, user_uuid, target_uuid, trashed, startnode) AS (
SELECT 3::smallint AS val,
diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb
index a1123b1..11f546b 100644
--- a/services/api/test/unit/group_test.rb
+++ b/services/api/test/unit/group_test.rb
@@ -60,4 +60,44 @@ class GroupTest < ActiveSupport::TestCase
assert g_foo.errors.messages[:owner_uuid].join(" ").match(/ownership cycle/)
end
+ test "delete group hides contents" do
+ set_user_from_auth :active_trustedclient
+
+ g_foo = Group.create!(name: "foo")
+ col = Collection.create!(owner_uuid: g_foo.uuid)
+
+ assert Collection.readable_by(users(:active)).where(uuid: col.uuid).any?
+ g_foo.update! is_trashed: true
+ assert Collection.readable_by(users(:active)).where(uuid: col.uuid).empty?
+ g_foo.update! is_trashed: false
+ assert Collection.readable_by(users(:active)).where(uuid: col.uuid).any?
+ end
+
+
+ test "delete group propagates to subgroups" do
+ set_user_from_auth :active_trustedclient
+
+ g_foo = Group.create!(name: "foo")
+ g_bar = Group.create!(name: "bar", owner_uuid: g_foo.uuid)
+ col = Collection.create!(owner_uuid: g_bar.uuid)
+
+ assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
+ assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).any?
+ assert Collection.readable_by(users(:active)).where(uuid: col.uuid).any?
+
+ g_foo.update! is_trashed: true
+ assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).empty?
+ assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).empty?
+ assert Collection.readable_by(users(:active)).where(uuid: col.uuid).empty?
+
+ assert Group.readable_by(users(:active), {:include_trashed => true}).where(uuid: g_foo.uuid).any?
+ assert Group.readable_by(users(:active), {:include_trashed => true}).where(uuid: g_bar.uuid).any?
+ assert Collection.readable_by(users(:active), {:include_trashed => true}).where(uuid: col.uuid).any?
+
+ g_foo.update! is_trashed: false
+ assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
+ assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).any?
+ assert Collection.readable_by(users(:active)).where(uuid: col.uuid).any?
+ end
+
end
commit 0365e9f395deab27e1d310b1bf06d5b4f1e76eac
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Thu Aug 24 16:22:12 2017 -0400
12032: Additional refactoring of readable_by. Refactor "trashable" into module.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 457b1bf..5a9d43f 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -254,7 +254,7 @@ class ArvadosModel < ActiveRecord::Base
# Collect the UUIDs of the authorized users.
sql_table = kwargs.fetch(:table_name, table_name)
- include_trashed = kwargs.fetch(:include_trashed, 0)
+ include_trashed = kwargs.fetch(:include_trashed, false)
sql_conds = []
user_uuids = users_list.map { |u| u.uuid }
@@ -263,57 +263,56 @@ class ArvadosModel < ActiveRecord::Base
# 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({})
+ if !include_trashed
+ # exclude rows that are trashed.
+ if self.column_names.include? "owner_uuid"
+ sql_conds += ["NOT EXISTS(SELECT target_uuid
+ FROM permission_view pv
+ WHERE trashed = 1 AND
+ (#{sql_table}.uuid = pv.target_uuid OR #{sql_table}.owner_uuid = pv.target_uuid))"]
+ else
+ sql_conds += ["NOT EXISTS(SELECT target_uuid
+ FROM permission_view pv
+ WHERE trashed = 1 AND
+ (#{sql_table}.uuid = pv.target_uuid))"]
+ end
+ end
else
+ # Match objects which appear in the permission view
+ trash_clause = if !include_trashed then "trashed = 0 AND" else "" end
+
# 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)"]
+ sql_conds += ["#{sql_table}.uuid IN (:user_uuids)"]
+
+ if self.column_names.include? "owner_uuid"
+ sql_conds += ["EXISTS(SELECT target_uuid
+ FROM permission_view pv
+ WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 AND #{trash_clause}
+ (#{sql_table}.uuid = pv.target_uuid OR
+ (#{sql_table}.owner_uuid = pv.target_uuid AND pv.target_owner_uuid is NOT NULL)))"]
+ # Match any object whose owner is listed explicitly in
+ # user_uuids.
+ sql_conds += ["#{sql_table}.owner_uuid IN (:user_uuids)"]
+ else
+ sql_conds += ["EXISTS(SELECT target_uuid
+ FROM permission_view pv
+ WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 AND #{trash_clause}
+ (#{sql_table}.uuid = pv.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_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 '),
user_uuids: user_uuids,
- permission_link_classes: ['permission', 'resources'],
- include_trashed: include_trashed)
+ permission_link_classes: ['permission', 'resources'])
end
def save_with_unique_name!
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 6f13a63..ce7e9fe 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -4,6 +4,7 @@
require 'arvados/keep'
require 'sweep_trashed_collections'
+require 'trashable'
class Collection < ArvadosModel
extend CurrentApiClient
@@ -11,20 +12,16 @@ class Collection < ArvadosModel
include HasUuid
include KindAndEtag
include CommonApiTemplate
+ include Trashable
serialize :properties, Hash
- before_validation :set_validation_timestamp
before_validation :default_empty_manifest
before_validation :check_encoding
before_validation :check_manifest_validity
before_validation :check_signatures
before_validation :strip_signatures_and_update_replication_confirmed
- before_validation :ensure_trash_at_not_in_past
- before_validation :sync_trash_state
- before_validation :default_trash_interval
validate :ensure_pdh_matches_manifest_text
- validate :validate_trash_and_delete_timing
before_save :set_file_names
# Query only untrashed collections by default.
@@ -486,81 +483,4 @@ class Collection < ArvadosModel
super
end
- # Use a single timestamp for all validations, even though each
- # validation runs at a different time.
- def set_validation_timestamp
- @validation_timestamp = db_current_time
- end
-
- # If trash_at is being changed to a time in the past, change it to
- # now. This allows clients to say "expires {client-current-time}"
- # without failing due to clock skew, while avoiding odd log entries
- # like "expiry date changed to {1 year ago}".
- def ensure_trash_at_not_in_past
- if trash_at_changed? && trash_at
- self.trash_at = [@validation_timestamp, trash_at].max
- end
- end
-
- # Caller can move into/out of trash by setting/clearing is_trashed
- # -- however, if the caller also changes trash_at, then any changes
- # to is_trashed are ignored.
- def sync_trash_state
- if is_trashed_changed? && !trash_at_changed?
- if is_trashed
- self.trash_at = @validation_timestamp
- else
- self.trash_at = nil
- self.delete_at = nil
- end
- end
- self.is_trashed = trash_at && trash_at <= @validation_timestamp || false
- true
- end
-
- def default_trash_interval
- if trash_at_changed? && !delete_at_changed?
- # If trash_at is updated without touching delete_at,
- # automatically update delete_at to a sensible value.
- if trash_at.nil?
- self.delete_at = nil
- else
- self.delete_at = trash_at + Rails.configuration.default_trash_lifetime.seconds
- end
- elsif !trash_at || !delete_at || trash_at > delete_at
- # Not trash, or bogus arguments? Just validate in
- # validate_trash_and_delete_timing.
- elsif delete_at_changed? && delete_at >= trash_at
- # Fix delete_at if needed, so it's not earlier than the expiry
- # time on any permission tokens that might have been given out.
-
- # In any case there are no signatures expiring after now+TTL.
- # Also, if the existing trash_at time has already passed, we
- # know we haven't given out any signatures since then.
- earliest_delete = [
- @validation_timestamp,
- trash_at_was,
- ].compact.min + Rails.configuration.blob_signature_ttl.seconds
-
- # The previous value of delete_at is also an upper bound on the
- # longest-lived permission token. For example, if TTL=14,
- # trash_at_was=now-7, delete_at_was=now+7, then it is safe to
- # set trash_at=now+6, delete_at=now+8.
- earliest_delete = [earliest_delete, delete_at_was].compact.min
-
- # If delete_at is too soon, use the earliest possible time.
- if delete_at < earliest_delete
- self.delete_at = earliest_delete
- end
- end
- end
-
- def validate_trash_and_delete_timing
- if trash_at.nil? != delete_at.nil?
- errors.add :delete_at, "must be set if trash_at is set, and must be nil otherwise"
- elsif delete_at && delete_at < trash_at
- errors.add :delete_at, "must not be earlier than trash_at"
- end
- true
- end
end
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index c8c9bfb..0e2db76 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -155,10 +155,9 @@ class User < ArvadosModel
install_view('permission')
all_perms = {}
ActiveRecord::Base.connection.
- exec_query('SELECT user_uuid, target_owner_uuid, max(perm_level), max(trashed)
+ exec_query('SELECT user_uuid, target_owner_uuid, perm_level, trashed
FROM permission_view
- WHERE target_owner_uuid IS NOT NULL
- GROUP BY user_uuid, target_owner_uuid',
+ WHERE target_owner_uuid IS NOT NULL',
# "name" arg is a query label that appears in logs:
"all_group_permissions",
).rows.each do |user_uuid, group_uuid, max_p_val, trashed|
@@ -176,11 +175,10 @@ class User < ArvadosModel
group_perms = {self.uuid => {:read => true, :write => true, :manage => true}}
ActiveRecord::Base.connection.
- exec_query('SELECT target_owner_uuid, max(perm_level), max(trashed)
+ exec_query('SELECT target_owner_uuid, perm_level, trashed
FROM permission_view
WHERE user_uuid = $1
- AND target_owner_uuid IS NOT NULL
- GROUP BY target_owner_uuid',
+ AND target_owner_uuid IS NOT NULL',
# "name" arg is a query label that appears in logs:
"group_permissions for #{uuid}",
# "binds" arg is an array of [col_id, value] for '$1' vars:
diff --git a/services/api/lib/create_permission_view.sql b/services/api/lib/create_permission_view.sql
index 10c4b27..82839e8 100644
--- a/services/api/lib/create_permission_view.sql
+++ b/services/api/lib/create_permission_view.sql
@@ -60,7 +60,8 @@ perm (val, follow, user_uuid, target_uuid, trashed, startnode) AS (
)
SELECT user_uuid,
target_uuid,
- val AS perm_level,
+ MAX(val) AS perm_level,
CASE follow WHEN true THEN target_uuid ELSE NULL END AS target_owner_uuid,
- trashed
- FROM perm;
+ MAX(trashed) AS trashed
+ FROM perm
+ GROUP BY user_uuid, target_uuid, target_owner_uuid;
diff --git a/services/api/lib/trashable.rb b/services/api/lib/trashable.rb
new file mode 100644
index 0000000..1e2f466
--- /dev/null
+++ b/services/api/lib/trashable.rb
@@ -0,0 +1,91 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+module Trashable
+ def self.included(base)
+ base.before_validation :set_validation_timestamp
+ base.before_validation :ensure_trash_at_not_in_past
+ base.before_validation :sync_trash_state
+ base.before_validation :default_trash_interval
+ base.validate :validate_trash_and_delete_timing
+ end
+
+ # Use a single timestamp for all validations, even though each
+ # validation runs at a different time.
+ def set_validation_timestamp
+ @validation_timestamp = db_current_time
+ end
+
+ # If trash_at is being changed to a time in the past, change it to
+ # now. This allows clients to say "expires {client-current-time}"
+ # without failing due to clock skew, while avoiding odd log entries
+ # like "expiry date changed to {1 year ago}".
+ def ensure_trash_at_not_in_past
+ if trash_at_changed? && trash_at
+ self.trash_at = [@validation_timestamp, trash_at].max
+ end
+ end
+
+ # Caller can move into/out of trash by setting/clearing is_trashed
+ # -- however, if the caller also changes trash_at, then any changes
+ # to is_trashed are ignored.
+ def sync_trash_state
+ if is_trashed_changed? && !trash_at_changed?
+ if is_trashed
+ self.trash_at = @validation_timestamp
+ else
+ self.trash_at = nil
+ self.delete_at = nil
+ end
+ end
+ self.is_trashed = trash_at && trash_at <= @validation_timestamp || false
+ true
+ end
+
+ def default_trash_interval
+ if trash_at_changed? && !delete_at_changed?
+ # If trash_at is updated without touching delete_at,
+ # automatically update delete_at to a sensible value.
+ if trash_at.nil?
+ self.delete_at = nil
+ else
+ self.delete_at = trash_at + Rails.configuration.default_trash_lifetime.seconds
+ end
+ elsif !trash_at || !delete_at || trash_at > delete_at
+ # Not trash, or bogus arguments? Just validate in
+ # validate_trash_and_delete_timing.
+ elsif delete_at_changed? && delete_at >= trash_at
+ # Fix delete_at if needed, so it's not earlier than the expiry
+ # time on any permission tokens that might have been given out.
+
+ # In any case there are no signatures expiring after now+TTL.
+ # Also, if the existing trash_at time has already passed, we
+ # know we haven't given out any signatures since then.
+ earliest_delete = [
+ @validation_timestamp,
+ trash_at_was,
+ ].compact.min + Rails.configuration.blob_signature_ttl.seconds
+
+ # The previous value of delete_at is also an upper bound on the
+ # longest-lived permission token. For example, if TTL=14,
+ # trash_at_was=now-7, delete_at_was=now+7, then it is safe to
+ # set trash_at=now+6, delete_at=now+8.
+ earliest_delete = [earliest_delete, delete_at_was].compact.min
+
+ # If delete_at is too soon, use the earliest possible time.
+ if delete_at < earliest_delete
+ self.delete_at = earliest_delete
+ end
+ end
+ end
+
+ def validate_trash_and_delete_timing
+ if trash_at.nil? != delete_at.nil?
+ errors.add :delete_at, "must be set if trash_at is set, and must be nil otherwise"
+ elsif delete_at && delete_at < trash_at
+ errors.add :delete_at, "must not be earlier than trash_at"
+ end
+ true
+ end
+end
commit 4e40ff2ac4198765de3865de375e3a3f93e9aa65
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