before_action :load_limit_offset_order_params, only: [:index, :contents]
before_action :load_select_param
before_action(:find_object_by_uuid,
- except: [:index, :create] + ERROR_ACTIONS)
+ except: [:index, :create, :update] + ERROR_ACTIONS)
+ before_action :find_object_for_update, only: [:update]
before_action :load_where_param, only: [:index, :contents]
before_action :load_filters_param, only: [:index, :contents]
before_action :find_objects_for_index, :only => :index
controller_name
end
- def find_object_by_uuid
+ def find_object_for_update
+ find_object_by_uuid(with_lock: true)
+ end
+
+ def find_object_by_uuid(with_lock: false)
if params[:id] and params[:id].match(/\D/)
params[:uuid] = params.delete :id
end
@filters = []
@objects = nil
find_objects_for_index
- @object = @objects.first
+ if with_lock
+ @object = @objects.lock.first
+ else
+ @object = @objects.first
+ end
end
def nullable_attributes
super
end
- def find_object_by_uuid
+ def find_object_by_uuid(with_lock: false)
uuid_param = params[:uuid] || params[:id]
if (uuid_param != current_api_client_authorization.andand.uuid &&
!Thread.current[:api_client].andand.is_trusted)
@where = {}
@filters = [['uuid', '=', uuid_param]]
find_objects_for_index
- @object = @objects.first
+ query = @objects
+ if with_lock
+ query = query.lock
+ end
+ @object = query.first
end
def current_api_client_is_trusted
super
end
- def find_object_by_uuid
+ def find_object_by_uuid(with_lock: false)
if loc = Keep::Locator.parse(params[:id])
loc.strip_hints!
# available lifetime.
select_attrs = (@select || ["manifest_text"]) | ["portable_data_hash", "trash_at"]
- if c = Collection.
+ model = Collection
+ if with_lock
+ model = model.lock
+ end
+ if c = model.
readable_by(*@read_users, opts).
where({ portable_data_hash: loc.to_s }).
order("trash_at desc").
end
end
else
- super
+ super(with_lock: with_lock)
end
end
resource_attrs.delete :tail_kind
if resource_attrs[:link_class] == 'permission' && Link::PermLevel[resource_attrs[:name]]
- existing = Link.where(link_class: 'permission',
- tail_uuid: resource_attrs[:tail_uuid],
- head_uuid: resource_attrs[:head_uuid],
- name: Link::PermLevel.keys).first
+ existing = Link.
+ lock. # select ... for update
+ where(link_class: 'permission',
+ tail_uuid: resource_attrs[:tail_uuid],
+ head_uuid: resource_attrs[:head_uuid],
+ name: Link::PermLevel.keys).first
if existing
@object = existing
if Link::PermLevel[resource_attrs[:name]] > Link::PermLevel[existing.name]
resource_attrs[:name] == 'can_login' &&
resource_attrs[:properties].respond_to?(:has_key?) &&
resource_attrs[:properties].has_key?(:username)
- existing = Link.where(link_class: 'permission',
- tail_uuid: resource_attrs[:tail_uuid],
- head_uuid: resource_attrs[:head_uuid]).
+ existing = Link.
+ lock. # select ... for update
+ where(link_class: 'permission',
+ tail_uuid: resource_attrs[:tail_uuid],
+ head_uuid: resource_attrs[:head_uuid]).
where('properties @> ?', SafeJSON.dump({'username' => resource_attrs[:properties][:username]})).
first
if existing
protected
- def find_object_by_uuid
+ def find_object_by_uuid(with_lock: false)
if params[:id] && params[:id].match(/\D/)
params[:uuid] = params.delete :id
end
.where(uuid: params[:uuid])
.first
elsif !current_user
- super
+ super(with_lock: with_lock)
else
# The usual permission-filtering index query is unnecessarily
# inefficient, and doesn't match all permission links that
# by UUID, then check whether (a) its tail_uuid is the current
# user or (b) its head_uuid is an object the current_user
# can_manage.
- link = Link.unscoped.where(uuid: params[:uuid]).first
+ model = Link
+ if with_lock
+ model = model.lock
+ end
+ link = model.unscoped.where(uuid: params[:uuid]).first
if link && link.link_class != 'permission'
# Not a permission link. Re-fetch using generic
# permission-filtering query.
- super
+ super(with_lock: with_lock)
elsif link && (current_user.uuid == link.tail_uuid ||
current_user.can?(manage: link.head_uuid))
# Permission granted.
validate :permission_to_attach_to_objects
before_update :restrict_alter_permissions
before_update :apply_max_overlapping_permissions
+ before_create :apply_max_overlapping_permissions
after_update :delete_overlapping_permissions
after_update :call_update_permissions
after_create :call_update_permissions
def apply_max_overlapping_permissions
return if self.link_class != 'permission' || !PermLevel[self.name]
- Link.where(link_class: 'permission',
- tail_uuid: self.tail_uuid,
- head_uuid: self.head_uuid,
- name: PermLevel.keys).
+ Link.
+ lock. # select ... for update
+ where(link_class: 'permission',
+ tail_uuid: self.tail_uuid,
+ head_uuid: self.head_uuid,
+ name: PermLevel.keys).
where('uuid <> ?', self.uuid).each do |other|
if PermLevel[other.name] > PermLevel[self.name]
self.name = other.name
def delete_overlapping_permissions
return if self.link_class != 'permission'
+ redundant = nil
if PermLevel[self.name]
- Link.where(link_class: 'permission',
- tail_uuid: self.tail_uuid,
- head_uuid: self.head_uuid,
- name: PermLevel.keys).
- where('uuid <> ?', self.uuid).
- delete_all
+ redundant = Link.
+ lock. # select ... for update
+ where(link_class: 'permission',
+ tail_uuid: self.tail_uuid,
+ head_uuid: self.head_uuid,
+ name: PermLevel.keys).
+ where('uuid <> ?', self.uuid)
elsif self.name == 'can_login' &&
self.properties.respond_to?(:has_key?) &&
self.properties.has_key?('username')
- Link.where(link_class: 'permission',
- tail_uuid: self.tail_uuid,
- head_uuid: self.head_uuid,
- name: 'can_login').
- where('properties @> ?', SafeJSON.dump({'username' => self.properties['username']})).
- where('uuid <> ?', self.uuid).
- delete_all
+ redundant = Link.
+ lock. # select ... for update
+ where(link_class: 'permission',
+ tail_uuid: self.tail_uuid,
+ head_uuid: self.head_uuid,
+ name: 'can_login').
+ where('properties @> ?', SafeJSON.dump({'username' => self.properties['username']})).
+ where('uuid <> ?', self.uuid)
+ end
+ if redundant
+ redundant.each do |link|
+ link.clear_permissions
+ end
+ redundant.delete_all
end
end
assert_response 403
end
- test "job readable after updating other attributes" do
- authorize_with :admin
- post :update, params: {
- id: nodes(:busy).uuid,
- node: {last_ping_at: 1.second.ago},
- }
- assert_response :success
- assert_equal(jobs(:nearly_finished_job).uuid, json_response["job_uuid"],
- "mismatched job UUID after ping update")
- end
-
test "node should fail ping with invalid hostname config format" do
Rails.configuration.Containers.SLURM.Managed.AssignNodeHostname = 'compute%<slot_number>04' # should end with "04d"
post :ping, params: {
assert users(:active).can?(write: col.uuid)
assert users(:active).can?(manage: col.uuid)
- l3.destroy!
+ # Creating l3 should have automatically deleted l1 and upgraded to
+ # the max permission of {l1, l3}, i.e., can_manage (see #18693) so
+ # there should be no can_read link now.
+ refute Link.where(tail_uuid: l3.tail_uuid,
+ head_uuid: l3.head_uuid,
+ link_class: 'permission',
+ name: 'can_read').any?
assert Collection.readable_by(users(:active)).where(uuid: col.uuid).first
assert users(:active).can?(read: col.uuid)
assert users(:active).can?(write: prj.uuid)
assert users(:active).can?(manage: prj.uuid)
- l3.destroy!
+ # Creating l3 should have automatically deleted l0 and upgraded to
+ # the max permission of {l0, l3}, i.e., can_manage (see #18693) so
+ # there should be no can_read link now.
+ refute Link.where(tail_uuid: l3.tail_uuid,
+ head_uuid: l3.head_uuid,
+ link_class: 'permission',
+ name: 'can_read').any?
assert Group.readable_by(users(:active)).where(uuid: prj.uuid).first
assert users(:active).can?(read: prj.uuid)
projects / arvados.git / commitdiff