[ARVADOS] created: 975b1912434cdd158abcf2d8b882d90c57c4299e
git at public.curoverse.com
git at public.curoverse.com
Fri Jun 20 17:11:59 EDT 2014
at 975b1912434cdd158abcf2d8b882d90c57c4299e (commit)
commit 975b1912434cdd158abcf2d8b882d90c57c4299e
Author: Tim Pierce <twp at curoverse.com>
Date: Fri Jun 13 17:11:22 2014 -0400
2873: permission links are owned by root
Permission links are themselves "owned" by root. (Technically, the
owner_uuid on permission links will be ignored when determining a user's
permission to modify an object -- ensuring that it is always root
provides consistency, and helps to flush out any permission edge cases
early in testing.)
ArvadosModel.ensure_owner_uuid_is_permitted and
Link.permission_to_attach_to_objects grant permission to modify
permission links based on the user's relationship to the link's head.
New methods:
* User.owns? determines ownership of an object.
* User.can_manage? determines whether a user has permission to manage an object.
* ArvadosModel.has_permission? determines whether an object has been
granted a particular permission on a specified target.
Refs #2873.
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index 2df6686..a6f2e41 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -160,6 +160,13 @@ class ArvadosModel < ActiveRecord::Base
attributes
end
+ def has_permission? perm_type, target_uuid
+ Link.where(link_class: "permission",
+ name: perm_type,
+ tail_uuid: uuid,
+ head_uuid: target_uuid).any?
+ end
+
protected
def ensure_ownership_path_leads_to_user
@@ -212,6 +219,14 @@ class ArvadosModel < ActiveRecord::Base
end
if new_record?
return true
+ elsif respond_to? :link_class and link_class == 'permission'
+ # Users are permitted to modify permission links themselves
+ # if they have "manage" permission on the destination object.
+ if current_user.can_manage? head_uuid
+ return true
+ else
+ raise PermissionDeniedError
+ end
elsif current_user.uuid == self.owner_uuid_was or
current_user.uuid == self.uuid or
current_user.can? write: self.owner_uuid_was
diff --git a/services/api/app/models/link.rb b/services/api/app/models/link.rb
index 1b3fc34..575f32c 100644
--- a/services/api/app/models/link.rb
+++ b/services/api/app/models/link.rb
@@ -5,6 +5,7 @@ class Link < ArvadosModel
serialize :properties, Hash
before_create :permission_to_attach_to_objects
before_update :permission_to_attach_to_objects
+ before_save :permission_link_ownership, if: "link_class == 'permission'"
after_update :maybe_invalidate_permissions_cache
after_create :maybe_invalidate_permissions_cache
after_destroy :maybe_invalidate_permissions_cache
@@ -51,14 +52,8 @@ class Link < ArvadosModel
# Administrators can grant permissions
return true if current_user.is_admin
- # All users can grant permissions on objects they own
- head_obj = self.class.
- resource_class_for_uuid(self.head_uuid).
- where('uuid=?',head_uuid).
- first
- if head_obj
- return true if head_obj.owner_uuid == current_user.uuid
- end
+ # All users can grant permissions on objects they own or can manage
+ return true if current_user.can_manage? head_uuid
# Users with "can_grant" permission on an object can grant
# permissions on that object
@@ -100,4 +95,10 @@ class Link < ArvadosModel
ensure_owner_uuid_is_permitted
end
end
+
+ # permission_link_ownership: ensure that permission links are
+ # owned by root.
+ def permission_link_ownership
+ self.owner_uuid = system_user_uuid
+ end
end
diff --git a/services/api/app/models/user.rb b/services/api/app/models/user.rb
index 52dd8d7..f1b5c44 100644
--- a/services/api/app/models/user.rb
+++ b/services/api/app/models/user.rb
@@ -179,8 +179,55 @@ class User < ArvadosModel
self.save!
end
+ def owns? object_uuid
+ return User.find_user_owning(object_uuid).andand.uuid == uuid
+ end
+
+ def can_manage? object_uuid
+ is_admin or
+ owns?(object_uuid) or
+ has_permission?(:can_manage, object_uuid)
+ end
+
protected
+ # Returns the first User found in the ownership path for obj_uuid.
+ # If obj_uuid is not owned by any user, returns nil.
+ #
+ # TODO(twp): this code largely stolen from
+ # ArvadosModel::ensure_ownership_path_leads_to_user. See if we can
+ # refactor these methods to share more code.
+ #
+ def self.find_user_owning obj_uuid
+ uuid_in_path = {obj_uuid => true}
+ # Walk up the owner_uuid chain for obj_uuid until one of these
+ # conditions is met:
+ # - the owner_uuid belongs to the User class
+ # - no owner_uuid is found (no User owns this object)
+ # - we discover an ownership cycle (a fatal consistency error)
+ #
+ x = obj_uuid
+ while (owner_class = ArvadosModel.resource_class_for_uuid(x)) != User
+ begin
+ if !owner_class.respond_to? :find_by_uuid
+ raise ActiveRecord::RecordNotFound.new
+ else
+ x = owner_class.find_by_uuid(x).owner_uuid
+ end
+ rescue ActiveRecord::RecordNotFound => e
+ # errors.add :owner_uuid, "is not owned by any user: #{e}"
+ return nil
+ end
+ # If there is an ownership cycle, we can conclude that
+ # no User owns this object.
+ if uuid_in_path[x]
+ return nil
+ end
+ uuid_in_path[x] = true
+ end
+ return owner_class.find_by_uuid(x)
+ end
+
def ensure_ownership_path_leads_to_user
true
end
diff --git a/services/api/test/unit/permission_test.rb b/services/api/test/unit/permission_test.rb
index 6e96dcc..23f5f75 100644
--- a/services/api/test/unit/permission_test.rb
+++ b/services/api/test/unit/permission_test.rb
@@ -1,6 +1,8 @@
require 'test_helper'
class PermissionTest < ActiveSupport::TestCase
+ include CurrentApiClient
+
test "Grant permissions on an object I own" do
set_user_from_auth :active_trustedclient
@@ -28,4 +30,78 @@ class PermissionTest < ActiveSupport::TestCase
assert_empty(Link.where(head_uuid: ob_uuid),
"Permission link was not deleted when object was deleted")
end
+
+ test "permission links owned by root" do
+ set_user_from_auth :active_trustedclient
+ ob = Specimen.create!
+ perm_link = Link.create!(tail_uuid: users(:active).uuid,
+ head_uuid: ob.uuid,
+ link_class: 'permission',
+ name: 'can_read')
+ assert_equal system_user_uuid, perm_link.owner_uuid
+ end
+
+ test "readable_by" do
+ set_user_from_auth :active_trustedclient
+
+ ob = Specimen.create!
+ Link.create!(tail_uuid: users(:active).uuid,
+ head_uuid: ob.uuid,
+ link_class: 'permission',
+ name: 'can_read')
+ assert Specimen.readable_by(users(:active)).where(uuid: ob.uuid).any?, "user does not have read permission"
+ end
+
+ test "writable_by" do
+ set_user_from_auth :active_trustedclient
+
+ ob = Specimen.create!
+ Link.create!(tail_uuid: users(:active).uuid,
+ head_uuid: ob.uuid,
+ link_class: 'permission',
+ name: 'can_write')
+ assert ob.writable_by.include?(users(:active).uuid), "user does not have write permission"
+ end
+
+ test "user with can_manage permission allowed to modify permission link" do
+ set_user_from_auth :admin
+
+ ob = Specimen.create!
+ # grant can_manage permission to active
+ perm_link = Link.create!(tail_uuid: users(:active).uuid,
+ head_uuid: ob.uuid,
+ link_class: 'permission',
+ name: 'can_manage')
+ # ob is owned by :admin, the link is owned by root
+ assert_equal users(:admin).uuid, ob.owner_uuid
+ assert_equal system_user_uuid, perm_link.owner_uuid
+
+ # user "active" can modify the permission link
+ set_user_from_auth :active_trustedclient
+ perm_link.properties["foo"] = 'bar'
+ assert perm_link.save, "could not save modified link"
+
+ assert_equal 'bar', perm_link.properties['foo'], "link properties do not include foo = bar"
+ end
+
+ test "user without can_manage permission may not modify permission link" do
+ set_user_from_auth :admin
+
+ ob = Specimen.create!
+ # grant can_manage permission to active
+ perm_link = Link.create!(tail_uuid: users(:active).uuid,
+ head_uuid: ob.uuid,
+ link_class: 'permission',
+ name: 'can_read')
+ # ob is owned by :admin, the link is owned by root
+ assert_equal ob.owner_uuid, users(:admin).uuid
+ assert_equal perm_link.owner_uuid, system_user_uuid
+
+ # user "active" may not modify the permission link
+ set_user_from_auth :active_trustedclient
+ perm_link.name = 'can_manage'
+ assert_raises ArvadosModel::PermissionDeniedError do
+ perm_link.save
+ end
+ end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list