attr_protected :modified_by_client_uuid
attr_protected :modified_at
after_initialize :log_start_state
- before_create :ensure_permission_to_create
- before_update :ensure_permission_to_update
+ before_save :ensure_permission_to_save
+ before_save :ensure_owner_uuid_is_permitted
+ before_save :ensure_ownership_path_leads_to_user
+ before_destroy :ensure_owner_uuid_is_permitted
before_destroy :ensure_permission_to_destroy
before_create :update_modified_by_fields
protected
- def ensure_permission_to_create
- raise PermissionDeniedError unless permission_to_create
+ def ensure_ownership_path_leads_to_user
+ if new_record? or owner_uuid_changed?
+ uuid_in_path = {owner_uuid => true, uuid => true}
+ x = owner_uuid
+ while (owner_class = self.class.resource_class_for_uuid(x)) != User
+ begin
+ if x == uuid
+ # Test for cycles with the new version, not the DB contents
+ x = owner_uuid
+ 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 false
+ end
+ if uuid_in_path[x]
+ if x == owner_uuid
+ errors.add :owner_uuid, "would create an ownership cycle"
+ else
+ errors.add :owner_uuid, "has an ownership cycle"
+ end
+ return false
+ end
+ uuid_in_path[x] = true
+ end
+ end
+ true
end
- def permission_to_create
- current_user.andand.is_active
+ def ensure_owner_uuid_is_permitted
+ return false if !current_user
+ self.owner_uuid ||= current_user.uuid
+ if self.owner_uuid_changed?
+ if current_user.uuid == self.owner_uuid or
+ current_user.can? write: self.owner_uuid
+ # current_user is, or has :write permission on, the new owner
+ else
+ logger.warn "User #{current_user.uuid} tried to change owner_uuid of #{self.class.to_s} #{self.uuid} to #{self.owner_uuid} but does not have permission to write to #{self.owner_uuid}"
+ return false
+ end
+ end
+ if new_record?
+ return true
+ elsif current_user.uuid == self.owner_uuid_was or
+ current_user.uuid == self.uuid or
+ current_user.can? write: self.owner_uuid_was
+ # current user is, or has :write permission on, the previous owner
+ return true
+ else
+ logger.warn "User #{current_user.uuid} tried to modify #{self.class.to_s} #{self.uuid} but does not have permission to write #{self.owner_uuid_was}"
+ raise PermissionDeniedError
+ end
+ end
+
+ def ensure_permission_to_save
+ unless (new_record? ? permission_to_create : permission_to_update)
+ raise PermissionDeniedError
+ end
end
- def ensure_permission_to_update
- raise PermissionDeniedError unless permission_to_update
+ def permission_to_create
+ current_user.andand.is_active
end
def permission_to_update
logger.warn "User #{current_user.uuid} tried to change uuid of #{self.class.to_s} #{self.uuid_was} to #{self.uuid}"
return false
end
- if self.owner_uuid_changed?
- if current_user.uuid == self.owner_uuid or
- current_user.can? write: self.owner_uuid
- # current_user is, or has :write permission on, the new owner
- else
- logger.warn "User #{current_user.uuid} tried to change owner_uuid of #{self.class.to_s} #{self.uuid} to #{self.owner_uuid} but does not have permission to write to #{self.owner_uuid}"
- return false
- end
- end
- if current_user.uuid == self.owner_uuid_was or
- current_user.uuid == self.uuid or
- current_user.can? write: self.owner_uuid_was
- # current user is, or has :write permission on, the previous owner
- return true
- else
- logger.warn "User #{current_user.uuid} tried to modify #{self.class.to_s} #{self.uuid} but does not have permission to write #{self.owner_uuid_was}"
- return false
- end
+ return true
end
def ensure_permission_to_destroy
end
def start!(ping_url_method)
- ensure_permission_to_update
+ ensure_permission_to_save
ping_url = ping_url_method.call({ id: self.uuid, ping_secret: self.info[:ping_secret] })
if (Rails.configuration.compute_node_ec2run_args and
Rails.configuration.compute_node_ami)
def system_user
if not $system_user
real_current_user = Thread.current[:user]
- Thread.current[:user] = User.new(is_admin: true, is_active: true)
+ Thread.current[:user] = User.new(is_admin: true,
+ is_active: true,
+ uuid: system_user_uuid)
$system_user = User.where('uuid=?', system_user_uuid).first
if !$system_user
$system_user = User.new(uuid: system_user_uuid,
trusted_workbench:
uuid: zzzzz-ozdt8-teyxzyd8qllg11h
+ owner_uuid: zzzzz-tpzed-000000000000000
name: Official Workbench
url_prefix: https://official-workbench.local/
is_trusted: true
untrusted:
uuid: zzzzz-ozdt8-obw7foaks3qjyej
+ owner_uuid: zzzzz-tpzed-000000000000000
name: Untrusted
url_prefix: https://untrusted.local/
is_trusted: false
name: A Subfolder
description: "Test folder belonging to active user's first test folder"
group_class: folder
+
+bad_group_has_ownership_cycle_a:
+ uuid: zzzzz-j7d0g-cx2al9cqkmsf1hs
+ owner_uuid: zzzzz-j7d0g-0077nzts8c178lw
+ modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+ modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ created_at: 2014-05-03 18:50:08 -0400
+ modified_at: 2014-05-03 18:50:08 -0400
+ updated_at: 2014-05-03 18:50:08 -0400
+ name: Owned by bad group b
+
+bad_group_has_ownership_cycle_b:
+ uuid: zzzzz-j7d0g-0077nzts8c178lw
+ owner_uuid: zzzzz-j7d0g-cx2al9cqkmsf1hs
+ modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+ modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ created_at: 2014-05-03 18:50:08 -0400
+ modified_at: 2014-05-03 18:50:08 -0400
+ updated_at: 2014-05-03 18:50:08 -0400
+ name: Owned by bad group a
link_class: tag
name: foo_tag
properties: {}
+
+active_user_can_manage_bad_group_cx2al9cqkmsf1hs:
+ uuid: zzzzz-o0j2j-ezv55ahzc9lvjwe
+ owner_uuid: zzzzz-tpzed-000000000000000
+ created_at: 2014-05-03 18:50:08 -0400
+ modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+ modified_by_user_uuid: zzzzz-tpzed-000000000000000
+ modified_at: 2014-05-03 18:50:08 -0400
+ updated_at: 2014-05-03 18:50:08 -0400
+ tail_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+ link_class: permission
+ name: can_manage
+ head_uuid: zzzzz-j7d0g-cx2al9cqkmsf1hs
+ properties: {}
Thread.current[:user] = nil
end
+ def set_user_from_auth(auth_name)
+ client_auth = api_client_authorizations(auth_name)
+ Thread.current[:api_client_authorization] = client_auth
+ Thread.current[:api_client] = client_auth.api_client
+ Thread.current[:user] = client_auth.user
+ end
+
def expect_json
self.request.headers["Accept"] = "text/json"
end
require 'test_helper'
class GroupTest < ActiveSupport::TestCase
- # test "the truth" do
- # assert true
- # end
+
+ test "cannot set owner_uuid to object with existing ownership cycle" do
+ set_user_from_auth :active_trustedclient
+
+ # First make sure we have lots of permission on the bad group
+ g = groups(:bad_group_has_ownership_cycle_b)
+ g.name += " xyz"
+ assert g.save, "active user should be able to modify group #{g.uuid}"
+
+ # Use the group as the owner of a new object
+ s = Specimen.
+ create(owner_uuid: groups(:bad_group_has_ownership_cycle_b).uuid)
+ assert s.valid?, "ownership should pass validation"
+ assert_equal false, s.save, "should not save object with #{g.uuid} as owner"
+
+ # Use the group as the new owner of an existing object
+ s = specimens(:in_afolder)
+ s.owner_uuid = groups(:bad_group_has_ownership_cycle_b).uuid
+ assert s.valid?, "ownership should pass validation"
+ assert_equal false, s.save, "should not save object with #{g.uuid} as owner"
+ end
+
+ test "cannot create a new ownership cycle" do
+ set_user_from_auth :active_trustedclient
+
+ g_foo = Group.create(name: "foo")
+ g_foo.save!
+
+ g_bar = Group.create(name: "bar")
+ g_bar.save!
+
+ g_foo.owner_uuid = g_bar.uuid
+ assert g_foo.save, lambda { g_foo.errors.messages }
+ g_bar.owner_uuid = g_foo.uuid
+ assert g_bar.valid?, "ownership cycle should not prevent validation"
+ assert_equal false, g_bar.save, "should not create an ownership loop"
+ assert g_bar.errors.messages[:owner_uuid].join(" ").match(/ownership cycle/)
+ end
+
+ test "cannot create a single-object ownership cycle" do
+ set_user_from_auth :active_trustedclient
+
+ g_foo = Group.create(name: "foo")
+ assert g_foo.save
+
+ # Ensure I have permission to manage this group even when its owner changes
+ perm_link = Link.create(tail_uuid: users(:active).uuid,
+ head_uuid: g_foo.uuid,
+ link_class: 'permission',
+ name: 'can_manage')
+ assert perm_link.save
+
+ g_foo.owner_uuid = g_foo.uuid
+ assert_equal false, g_foo.save, "should not create an ownership loop"
+ assert g_foo.errors.messages[:owner_uuid].join(" ").match(/ownership cycle/)
+ end
+
end
end
end
- def set_user_from_auth(auth_name)
- client_auth = api_client_authorizations(auth_name)
- Thread.current[:api_client_authorization] = client_auth
- Thread.current[:api_client] = client_auth.api_client
- Thread.current[:user] = client_auth.user
- end
-
test "creating a user makes a log" do
set_user_from_auth :admin_trustedclient
u = User.new(first_name: "Log", last_name: "Test")
projects / arvados.git / commitdiff