X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/91dc5f1d7f5ad9eb2640f6089e2d0476cbf87c8e..0eb72b526bf8bbb011551ecf019f604e17a534f1:/services/api/app/models/arvados_model.rb diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb index 672374bc6c..ea69735502 100644 --- a/services/api/app/models/arvados_model.rb +++ b/services/api/app/models/arvados_model.rb @@ -1,5 +1,10 @@ +# Copyright (C) The Arvados Authors. All rights reserved. +# +# SPDX-License-Identifier: AGPL-3.0 + require 'has_uuid' require 'record_filters' +require 'serializers' class ArvadosModel < ActiveRecord::Base self.abstract_class = true @@ -8,10 +13,6 @@ class ArvadosModel < ActiveRecord::Base include DbCurrentTime extend RecordFilters - attr_protected :created_at - attr_protected :modified_by_user_uuid - attr_protected :modified_by_client_uuid - attr_protected :modified_at after_initialize :log_start_state before_save :ensure_permission_to_save before_save :ensure_owner_uuid_is_permitted @@ -26,13 +27,16 @@ class ArvadosModel < ActiveRecord::Base after_find :convert_serialized_symbols_to_strings before_validation :normalize_collection_uuids before_validation :set_default_owner - validate :ensure_serialized_attribute_type validate :ensure_valid_uuids # Note: This only returns permission links. It does not account for # permissions obtained via user.is_admin or # user.uuid==object.owner_uuid. - has_many :permissions, :foreign_key => :head_uuid, :class_name => 'Link', :primary_key => :uuid, :conditions => "link_class = 'permission'" + has_many(:permissions, + ->{where(link_class: 'permission')}, + foreign_key: :head_uuid, + class_name: 'Link', + primary_key: :uuid) class PermissionDeniedError < StandardError def http_status @@ -46,6 +50,12 @@ class ArvadosModel < ActiveRecord::Base end end + class LockFailedError < StandardError + def http_status + 422 + end + end + class InvalidStateTransitionError < StandardError def http_status 422 @@ -72,6 +82,46 @@ class ArvadosModel < ActiveRecord::Base "#{current_api_base}/#{self.class.to_s.pluralize.underscore}/#{self.uuid}" end + def self.permit_attribute_params raw_params + # strong_parameters does not provide security: permissions are + # implemented with before_save hooks. + # + # The following permit! is necessary even with + # "ActionController::Parameters.permit_all_parameters = true", + # because permit_all does not permit nested attributes. + if raw_params + serialized_attributes.each do |colname, coder| + param = raw_params[colname.to_sym] + if param.nil? + # ok + elsif !param.is_a?(coder.object_class) + raise ArgumentError.new("#{colname} parameter must be #{coder.object_class}, not #{param.class}") + elsif has_nonstring_keys?(param) + raise ArgumentError.new("#{colname} parameter cannot have non-string hash keys") + end + end + end + ActionController::Parameters.new(raw_params).permit! + end + + def initialize raw_params={}, *args + super(self.class.permit_attribute_params(raw_params), *args) + end + + # Reload "old attributes" for logging, too. + def reload(*args) + super + log_start_state + end + + def self.create raw_params={}, *args + super(permit_attribute_params(raw_params), *args) + end + + def update_attributes raw_params={}, *args + super(self.class.permit_attribute_params(raw_params), *args) + end + def self.selectable_attributes(template=:user) # Return an array of attribute name strings that can be selected # in the given template. @@ -150,6 +200,14 @@ class ArvadosModel < ActiveRecord::Base ["id", "uuid"] end + def self.limit_index_columns_read + # This method returns a list of column names. + # If an index request reads that column from the database, + # APIs that return lists will only fetch objects until reaching + # max_index_database_read bytes of data from those columns. + [] + end + # If current user can manage the object, return an array of uuids of # users and groups that have permission to write the object. The # first two elements are always [self.owner_uuid, current user's @@ -196,7 +254,8 @@ class ArvadosModel < ActiveRecord::Base # Check if any of the users are admin. If so, we're done. if users_list.select { |u| u.is_admin }.any? - return self + # Return existing relation with no new filters. + return where({}) end # Collect the UUIDs of the authorized users. @@ -238,8 +297,59 @@ class ArvadosModel < ActiveRecord::Base permission_link_classes: ['permission', 'resources']) end + def save_with_unique_name! + uuid_was = uuid + name_was = name + max_retries = 2 + transaction do + conn = ActiveRecord::Base.connection + conn.exec_query 'SAVEPOINT save_with_unique_name' + begin + save! + rescue ActiveRecord::RecordNotUnique => rn + raise if max_retries == 0 + max_retries -= 1 + + conn.exec_query 'ROLLBACK TO SAVEPOINT save_with_unique_name' + + # Dig into the error to determine if it is specifically calling out a + # (owner_uuid, name) uniqueness violation. In this specific case, and + # the client requested a unique name with ensure_unique_name==true, + # update the name field and try to save again. Loop as necessary to + # discover a unique name. It is necessary to handle name choosing at + # this level (as opposed to the client) to ensure that record creation + # never fails due to a race condition. + err = rn.original_exception + raise unless err.is_a?(PG::UniqueViolation) + + # Unfortunately ActiveRecord doesn't abstract out any of the + # necessary information to figure out if this the error is actually + # the specific case where we want to apply the ensure_unique_name + # behavior, so the following code is specialized to Postgres. + detail = err.result.error_field(PG::Result::PG_DIAG_MESSAGE_DETAIL) + raise unless /^Key \(owner_uuid, name\)=\([a-z0-9]{5}-[a-z0-9]{5}-[a-z0-9]{15}, .*?\) already exists\./.match detail + + new_name = "#{name_was} (#{db_current_time.utc.iso8601(3)})" + if new_name == name + # If the database is fast enough to do two attempts in the + # same millisecond, we need to wait to ensure we try a + # different timestamp on each attempt. + sleep 0.002 + new_name = "#{name_was} (#{db_current_time.utc.iso8601(3)})" + end + + self[:name] = new_name + self[:uuid] = nil if uuid_was.nil? && !uuid.nil? + conn.exec_query 'SAVEPOINT save_with_unique_name' + retry + ensure + conn.exec_query 'RELEASE SAVEPOINT save_with_unique_name' + end + end + end + def logged_attributes - attributes.except *Rails.configuration.unlogged_attributes + attributes.except(*Rails.configuration.unlogged_attributes) end def self.full_text_searchable_columns @@ -252,12 +362,7 @@ class ArvadosModel < ActiveRecord::Base parts = full_text_searchable_columns.collect do |column| "coalesce(#{column},'')" end - # We prepend a space to the tsvector() argument here. Otherwise, - # it might start with a column that has its own (non-full-text) - # index, which causes Postgres to use the column index instead of - # the tsvector index, which causes full text queries to be just as - # slow as if we had no index at all. - "to_tsvector('english', ' ' || #{parts.join(" || ' ' || ")})" + "to_tsvector('english', #{parts.join(" || ' ' || ")})" end def self.apply_filters query, filters @@ -335,36 +440,31 @@ class ArvadosModel < ActiveRecord::Base raise PermissionDeniedError end - # Verify "write" permission on old owner - # default fail unless one of: - # owner_uuid did not change - # previous owner_uuid is nil - # current user is the old owner - # current user is this object - # current user can_write old owner - unless !owner_uuid_changed? or - owner_uuid_was.nil? or - current_user.uuid == self.owner_uuid_was or - current_user.uuid == self.uuid or - current_user.can? write: self.owner_uuid_was - logger.warn "User #{current_user.uuid} tried to modify #{self.class.to_s} #{uuid} but does not have permission to write old owner_uuid #{owner_uuid_was}" - errors.add :owner_uuid, "cannot be changed without write permission on old owner" - raise PermissionDeniedError - end - - # Verify "write" permission on new owner - # default fail unless one of: - # current_user is this object - # current user can_write new owner, or this object if owner unchanged - if new_record? or owner_uuid_changed? or is_a?(ApiClientAuthorization) - write_target = owner_uuid + if new_record? || owner_uuid_changed? + # Permission on owner_uuid_was is needed to move an existing + # object away from its previous owner (which implies permission + # to modify this object itself, so we don't need to check that + # separately). Permission on the new owner_uuid is also needed. + [['old', owner_uuid_was], + ['new', owner_uuid] + ].each do |which, check_uuid| + if check_uuid.nil? + # old_owner_uuid is nil? New record, no need to check. + elsif !current_user.can?(write: check_uuid) + logger.warn "User #{current_user.uuid} tried to set ownership of #{self.class.to_s} #{self.uuid} but does not have permission to write #{which} owner_uuid #{check_uuid}" + errors.add :owner_uuid, "cannot be set or changed without write permission on #{which} owner" + raise PermissionDeniedError + end + end else - write_target = uuid - end - unless current_user == self or current_user.can? write: write_target - logger.warn "User #{current_user.uuid} tried to modify #{self.class.to_s} #{uuid} but does not have permission to write new owner_uuid #{owner_uuid}" - errors.add :owner_uuid, "cannot be changed without write permission on new owner" - raise PermissionDeniedError + # If the object already existed and we're not changing + # owner_uuid, we only need write permission on the object + # itself. + if !current_user.can?(write: self.uuid) + logger.warn "User #{current_user.uuid} tried to modify #{self.class.to_s} #{self.uuid} without write permission" + errors.add :uuid, "is not writable" + raise PermissionDeniedError + end end true @@ -412,6 +512,7 @@ class ArvadosModel < ActiveRecord::Base def update_modified_by_fields current_time = db_current_time + self.created_at = created_at_was || current_time self.updated_at = current_time self.owner_uuid ||= current_default_owner if self.respond_to? :owner_uuid= self.modified_at = current_time @@ -420,6 +521,19 @@ class ArvadosModel < ActiveRecord::Base true end + def self.has_nonstring_keys? x + if x.is_a? Hash + x.each do |k,v| + return true if !(k.is_a?(String) || k.is_a?(Symbol)) || has_nonstring_keys?(v) + end + elsif x.is_a? Array + x.each do |v| + return true if has_nonstring_keys?(v) + end + end + false + end + def self.has_symbols? x if x.is_a? Hash x.each do |k,v| @@ -455,22 +569,36 @@ class ArvadosModel < ActiveRecord::Base end end - def ensure_serialized_attribute_type - # Specifying a type in the "serialize" declaration causes rails to - # raise an exception if a different data type is retrieved from - # the database during load(). The validation preventing such - # crash-inducing records from being inserted in the database in - # the first place seems to have been left as an exercise to the - # developer. - self.class.serialized_attributes.each do |colname, attr| - if attr.object_class - if self.attributes[colname].class != attr.object_class - self.errors.add colname.to_sym, "must be a #{attr.object_class.to_s}, not a #{self.attributes[colname].class.to_s}" - elsif self.class.has_symbols? attributes[colname] - self.errors.add colname.to_sym, "must not contain symbols: #{attributes[colname].inspect}" - end - end + def self.where_serialized(colname, value) + if value.empty? + # rails4 stores as null, rails3 stored as serialized [] or {} + sql = "#{colname.to_s} is null or #{colname.to_s} IN (?)" + sorted = value + else + sql = "#{colname.to_s} IN (?)" + sorted = deep_sort_hash(value) end + where(sql, [sorted.to_yaml, SafeJSON.dump(sorted)]) + end + + Serializer = { + Hash => HashSerializer, + Array => ArraySerializer, + } + + def self.serialize(colname, type) + coder = Serializer[type] + @serialized_attributes ||= {} + @serialized_attributes[colname.to_s] = coder + super(colname, coder) + end + + def self.serialized_attributes + @serialized_attributes ||= {} + end + + def serialized_attributes + self.class.serialized_attributes end def convert_serialized_symbols_to_strings @@ -483,14 +611,14 @@ class ArvadosModel < ActiveRecord::Base self.class.serialized_attributes.each do |colname, attr| if self.class.has_symbols? attributes[colname] attributes[colname] = self.class.recursive_stringify attributes[colname] - self.send(colname + '=', - self.class.recursive_stringify(attributes[colname])) + send(colname + '=', + self.class.recursive_stringify(attributes[colname])) end end end def foreign_key_attributes - attributes.keys.select { |a| a.match /_uuid$/ } + attributes.keys.select { |a| a.match(/_uuid$/) } end def skip_uuid_read_permission_check @@ -505,7 +633,7 @@ class ArvadosModel < ActiveRecord::Base foreign_key_attributes.each do |attr| attr_value = send attr if attr_value.is_a? String and - attr_value.match /^[0-9a-f]{32,}(\+[@\w]+)*$/ + attr_value.match(/^[0-9a-f]{32,}(\+[@\w]+)*$/) begin send "#{attr}=", Collection.normalize_uuid(attr_value) rescue @@ -584,13 +712,12 @@ class ArvadosModel < ActiveRecord::Base unless uuid.is_a? String return nil end - resource_class = nil uuid.match HasUuid::UUID_REGEX do |re| return uuid_prefixes[re[1]] if uuid_prefixes[re[1]] end - if uuid.match /.+@.+/ + if uuid.match(/.+@.+/) return Email end @@ -603,7 +730,7 @@ class ArvadosModel < ActiveRecord::Base if self == ArvadosModel # If called directly as ArvadosModel.find_by_uuid rather than via subclass, # delegate to the appropriate subclass based on the given uuid. - self.resource_class_for_uuid(uuid).find_by_uuid(uuid) + self.resource_class_for_uuid(uuid).unscoped.find_by_uuid(uuid) else super end @@ -636,7 +763,7 @@ class ArvadosModel < ActiveRecord::Base end def log_destroy - log_change('destroy') do |log| + log_change('delete') do |log| log.fill_properties('old', etag(@old_attributes), @old_logged_attributes) log.update_to nil end