[ARVADOS] updated: 1f9df3c7140d4eacf58d4e19a7ddc81260296229
git at public.curoverse.com
git at public.curoverse.com
Thu Apr 24 09:14:06 EDT 2014
Summary of changes:
services/api/app/models/arvados_model.rb | 32 ++++++++++++++++----------
services/api/test/fixtures/logs.yml | 31 +++++++++++++++++++++++++-
services/api/test/fixtures/repositories.yml | 7 +++++-
services/api/test/unit/log_test.rb | 21 +++++++++++++++++
4 files changed, 77 insertions(+), 14 deletions(-)
via 1f9df3c7140d4eacf58d4e19a7ddc81260296229 (commit)
from aaa65e9d351284278ccb6d3e803fd3dd4f748c31 (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 1f9df3c7140d4eacf58d4e19a7ddc81260296229
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Thu Apr 24 09:14:01 2014 -0400
Fixed bug whereby readable_by could return duplicate rows if there is more than
one permission granting the ability to read the row.
Reorganized readable_by query a bit to make it easier to follow.
Added test for ownership and permission links when reading entries in the logs table.
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 449a061..c87a909 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -80,22 +80,31 @@ class ArvadosModel < ActiveRecord::Base
collect { |uuid| sanitize(uuid) }.join(', ')
or_references_me = ''
- if self == Link and user
+ if self == User
+ or_row_is_me = "OR (#{table_name}.uuid=#{sanitize user.uuid})"
+ end
+
+ if self == Link
or_references_me = "OR (#{table_name}.link_class in (#{sanitize 'permission'}, #{sanitize 'resources'}) AND #{sanitize user.uuid} IN (#{table_name}.head_uuid, #{table_name}.tail_uuid))"
end
- if self == Log and user
- object_owner = ", #{table_name}.object_owner_uuid"
+ if self == Log
+ or_object_uuid = ", #{table_name}.object_uuid"
or_object_owner = "OR (#{table_name}.object_owner_uuid in (#{sanitized_uuid_list}))"
end
- # Link head points to this row, or to the owner of this row
- # (or owner of object described by this row, for logs table only)
- # Link tail originates from this user, or a group that is readable by this user
- # Link is any permission link ('write' and 'manage' implicitly grant 'read')
+ # Link head points to this row, or to the owner of this row (the thing to be read)
+ # (or the object described by this row, for logs table only)
+ # Link tail originates from this user, or a group that is readable by this
+ # user (the identity with authorization to read)
+ # Link is any permission link ('write' and 'manage' implicitly include 'read')
+ # The existence of such a link is tested in the where clause as permissions.head_uuid IS NOT NULL.
+ # or
+ # User is admin
# or
# This row is owned by this user, or owned by a group readable by this user
# or
+ # This is the users table
# This row uuid is equal this user uuid
# or
# This is the links table
@@ -104,11 +113,10 @@ class ArvadosModel < ActiveRecord::Base
# or
# This is the logs table
# This object described by this row is owned by this user, or owned by a group readable by this user
- joins("LEFT JOIN links permissions ON permissions.head_uuid in (#{table_name}.owner_uuid, #{table_name}.uuid #{object_owner}) AND permissions.tail_uuid in (#{sanitized_uuid_list}) AND permissions.link_class='permission'").
- where("?=? OR #{table_name}.owner_uuid in (?) OR #{table_name}.uuid=? OR permissions.head_uuid IS NOT NULL #{or_references_me} #{or_object_owner}",
- true, user.is_admin,
- uuid_list,
- user.uuid)
+
+ joins("LEFT JOIN links permissions ON permissions.head_uuid in (#{table_name}.owner_uuid, #{table_name}.uuid #{or_object_uuid}) AND permissions.tail_uuid in (#{sanitized_uuid_list}) AND permissions.link_class='permission'").
+ where("permissions.head_uuid IS NOT NULL OR ?=? OR #{table_name}.owner_uuid in (?) #{or_row_is_me} #{or_references_me} #{or_object_owner}",
+ user.is_admin, true, uuid_list).uniq
end
def logged_attributes
diff --git a/services/api/test/fixtures/logs.yml b/services/api/test/fixtures/logs.yml
index d805439..f1ba81d 100644
--- a/services/api/test/fixtures/logs.yml
+++ b/services/api/test/fixtures/logs.yml
@@ -1,3 +1,32 @@
log1:
+ id: 1
uuid: zzzzz-xxxxx-pshmckwoma9plh7
- object_uuid: zzzzz-tpzed-l1s2piq4t4mps8r
\ No newline at end of file
+ object_uuid: zzzzz-tpzed-l1s2piq4t4mps8r
+
+log2: # admin changes repository2, which is owned by active user
+ id: 2
+ uuid: zzzzz-xxxxx-pshmckwoma00002
+ owner_uuid: zzzzz-tpzed-d9tiejq69daie8f # admin user
+ object_uuid: zzzzz-2x53u-382brsig8rp3667 # repository foo
+ object_owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz # active user
+
+log3: # admin changes specimen owned_by_spectator
+ id: 3
+ uuid: zzzzz-xxxxx-pshmckwoma00003
+ owner_uuid: zzzzz-tpzed-d9tiejq69daie8f # admin user
+ object_uuid: zzzzz-2x53u-3b0xxwzlbzxq5yr # specimen owned_by_spectator
+ object_owner_uuid: zzzzz-tpzed-l1s2piq4t4mps8r # spectator user
+
+log4: # foo collection added, readable by active through link
+ id: 4
+ uuid: zzzzz-xxxxx-pshmckwoma00004
+ owner_uuid: zzzzz-tpzed-000000000000000 # system user
+ object_uuid: 1f4b0bc7583c2a7f9102c395f4ffc5e3+45 # foo file
+ object_owner_uuid: zzzzz-tpzed-000000000000000 # system user
+
+log5: # baz collection added, readable by active and spectator through group 'all users' group membership
+ id: 5
+ uuid: zzzzz-xxxxx-pshmckwoma00005
+ owner_uuid: zzzzz-tpzed-000000000000000 # system user
+ object_uuid: ea10d51bcf88862dbcc36eb292017dfd+45 # baz file
+ object_owner_uuid: zzzzz-tpzed-000000000000000 # system user
diff --git a/services/api/test/fixtures/repositories.yml b/services/api/test/fixtures/repositories.yml
index ec3755d..62a153d 100644
--- a/services/api/test/fixtures/repositories.yml
+++ b/services/api/test/fixtures/repositories.yml
@@ -1,4 +1,9 @@
foo:
uuid: zzzzz-2x53u-382brsig8rp3666
- owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz # active user
name: foo
+
+repository2:
+ uuid: zzzzz-2x53u-382brsig8rp3667
+ owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz # active user
+ name: foo2
diff --git a/services/api/test/unit/log_test.rb b/services/api/test/unit/log_test.rb
index 3876775..be8498d 100644
--- a/services/api/test/unit/log_test.rb
+++ b/services/api/test/unit/log_test.rb
@@ -224,4 +224,25 @@ class LogTest < ActiveSupport::TestCase
auth.destroy
assert_auth_logged_with_clean_properties(auth, :destroy)
end
+
+ test "use ownership and permission links to determine which logs a user can see" do
+ c = Log.readable_by(users(:admin)).order("id asc").each.to_a
+ assert_equal 5, c.size
+ assert_equal 1, c[0].id # no-op
+ assert_equal 2, c[1].id # admin changes repository foo, which is owned by active user
+ assert_equal 3, c[2].id # admin changes specimen owned_by_spectator
+ assert_equal 4, c[3].id # foo collection added, readable by active through link
+ assert_equal 5, c[4].id # baz collection added, readable by active and spectator through group 'all users' group membership
+
+ c = Log.readable_by(users(:active)).order("id asc").each.to_a
+ assert_equal 3, c.size
+ assert_equal 2, c[0].id # admin changes repository foo, which is owned by active user
+ assert_equal 4, c[1].id # foo collection added, readable by active through link
+ assert_equal 5, c[2].id # baz collection added, readable by active and spectator through group 'all users' group membership
+
+ c = Log.readable_by(users(:spectator)).order("id asc").each.to_a
+ assert_equal 2, c.size
+ assert_equal 3, c[0].id # admin changes specimen owned_by_spectator
+ assert_equal 5, c[1].id # baz collection added, readable by active and spectator through group 'all users' group membership
+ end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list