X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/667a7121e08d4fffc24cafdc3ed474374782b959..864c3b0afd16c77e046f0072d8517d34c5a44792:/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 72db306f3a..f2bae3a4b5 100644 --- a/services/api/app/models/arvados_model.rb +++ b/services/api/app/models/arvados_model.rb @@ -2,17 +2,21 @@ # # SPDX-License-Identifier: AGPL-3.0 +require 'arvados_model_updates' require 'has_uuid' require 'record_filters' require 'serializers' +require 'request_error' -class ArvadosModel < ActiveRecord::Base +class ArvadosModel < ApplicationRecord self.abstract_class = true + include ArvadosModelUpdates include CurrentApiClient # current_user, current_api_client, etc. include DbCurrentTime extend RecordFilters + after_find :schedule_restoring_changes after_initialize :log_start_state before_save :ensure_permission_to_save before_save :ensure_owner_uuid_is_permitted @@ -24,7 +28,6 @@ class ArvadosModel < ActiveRecord::Base after_create :log_create after_update :log_update after_destroy :log_destroy - after_find :convert_serialized_symbols_to_strings before_validation :normalize_collection_uuids before_validation :set_default_owner validate :ensure_valid_uuids @@ -38,37 +41,47 @@ class ArvadosModel < ActiveRecord::Base class_name: 'Link', primary_key: :uuid) - class PermissionDeniedError < StandardError + # If async is true at create or update, permission graph + # update is deferred allowing making multiple calls without the performance + # penalty. + attr_accessor :async_permissions_update + + # Ignore listed attributes on mass assignments + def self.protected_attributes + [] + end + + class PermissionDeniedError < RequestError def http_status 403 end end - class AlreadyLockedError < StandardError + class AlreadyLockedError < RequestError def http_status 422 end end - class LockFailedError < StandardError + class LockFailedError < RequestError def http_status 422 end end - class InvalidStateTransitionError < StandardError + class InvalidStateTransitionError < RequestError def http_status 422 end end - class UnauthorizedError < StandardError + class UnauthorizedError < RequestError def http_status 401 end end - class UnresolvableContainerError < StandardError + class UnresolvableContainerError < RequestError def http_status 422 end @@ -89,7 +102,11 @@ class ArvadosModel < ActiveRecord::Base # The following permit! is necessary even with # "ActionController::Parameters.permit_all_parameters = true", # because permit_all does not permit nested attributes. + raw_params ||= {} + if raw_params + raw_params = raw_params.to_hash + raw_params.delete_if { |k, _| self.protected_attributes.include? k } serialized_attributes.each do |colname, coder| param = raw_params[colname.to_sym] if param.nil? @@ -100,6 +117,15 @@ class ArvadosModel < ActiveRecord::Base raise ArgumentError.new("#{colname} parameter cannot have non-string hash keys") end end + # Check JSONB columns that aren't listed on serialized_attributes + columns.select{|c| c.type == :jsonb}.collect{|j| j.name}.each do |colname| + if serialized_attributes.include? colname || raw_params[colname.to_sym].nil? + next + end + if has_nonstring_keys?(raw_params[colname.to_sym]) + raise ArgumentError.new("#{colname} parameter cannot have non-string hash keys") + end + end end ActionController::Parameters.new(raw_params).permit! end @@ -112,6 +138,7 @@ class ArvadosModel < ActiveRecord::Base def reload(*args) super log_start_state + self end def self.create raw_params={}, *args @@ -132,7 +159,7 @@ class ArvadosModel < ActiveRecord::Base textonly_operator = !operator.match(/[<=>]/) self.columns.select do |col| case col.type - when :string, :text, :jsonb + when :string, :text true when :datetime, :integer, :boolean !textonly_operator @@ -237,7 +264,7 @@ class ArvadosModel < ActiveRecord::Base end.compact.uniq end - # Return a query with read permissions restricted to the union of of the + # Return a query with read permissions restricted to the union of the # permissions of the members of users_list, i.e. if something is readable by # any user in users_list, it will be readable in the query returned by this # function. @@ -255,49 +282,133 @@ class ArvadosModel < ActiveRecord::Base # Collect the UUIDs of the authorized users. sql_table = kwargs.fetch(:table_name, table_name) include_trash = kwargs.fetch(:include_trash, false) - query_on = kwargs.fetch(:query_on, self) + include_old_versions = kwargs.fetch(:include_old_versions, false) - sql_conds = [] + sql_conds = nil user_uuids = users_list.map { |u| u.uuid } + all_user_uuids = [] + + # For details on how the trashed_groups table is constructed, see + # see db/migrate/20200501150153_permission_table.rb + + exclude_trashed_records = "" + if !include_trash and (sql_table == "groups" or sql_table == "collections") then + # Only include records that are not trashed + exclude_trashed_records = "AND (#{sql_table}.trash_at is NULL or #{sql_table}.trash_at > statement_timestamp())" + end + + trashed_check = "" + if !include_trash && sql_table != "api_client_authorizations" + trashed_check = "#{sql_table}.owner_uuid NOT IN (SELECT group_uuid FROM #{TRASHED_GROUPS} " + + "where trash_at <= statement_timestamp()) #{exclude_trashed_records}" + end if users_list.select { |u| u.is_admin }.any? - if !include_trash - # exclude rows that are explicitly trashed. - if sql_table != "api_client_authorizations" - sql_conds.push "NOT EXISTS(SELECT 1 - FROM #{PERMISSION_VIEW} - WHERE trashed = 1 AND - (#{sql_table}.uuid = target_uuid OR #{sql_table}.owner_uuid = target_uuid))" - end + # Admin skips most permission checks, but still want to filter on trashed items. + if !include_trash && sql_table != "api_client_authorizations" + # Only include records where the owner is not trashed + sql_conds = trashed_check end else - if include_trash - trashed_check = "" - else - trashed_check = "AND trashed = 0" + # The core of the permission check is a join against the + # materialized_permissions table to determine if the user has at + # least read permission to either the object itself or its + # direct owner (if traverse_owned is true). See + # db/migrate/20200501150153_permission_table.rb for details on + # how the permissions are computed. + + # A user can have can_manage access to another user, this grants + # full access to all that user's stuff. To implement that we + # need to include those other users in the permission query. + + # This was previously implemented by embedding the subquery + # directly into the query, but it was discovered later that this + # causes the Postgres query planner to do silly things because + # the query heuristics assumed the subquery would have a lot + # more rows that it does, and choose a bad merge strategy. By + # doing the query here and embedding the result as a constant, + # Postgres also knows exactly how many items there are and can + # choose the right query strategy. + # + # (note: you could also do this with a temporary table, but that + # would require all every request be wrapped in a transaction, + # which is not currently the case). + + all_user_uuids = ActiveRecord::Base.connection.exec_query %{ +#{USER_UUIDS_SUBQUERY_TEMPLATE % {user: "'#{user_uuids.join "', '"}'", perm_level: 1}} +}, + 'readable_by.user_uuids' + + user_uuids_subquery = ":user_uuids" + + # Note: it is possible to combine the direct_check and + # owner_check into a single IN (SELECT) clause, however it turns + # out query optimizer doesn't like it and forces a sequential + # table scan. Constructing the query with separate IN (SELECT) + # clauses enables it to use the index. + # + # see issue 13208 for details. + + # Match a direct read permission link from the user to the record uuid + direct_check = "#{sql_table}.uuid IN (SELECT target_uuid FROM #{PERMISSION_VIEW} "+ + "WHERE user_uuid IN (#{user_uuids_subquery}) AND perm_level >= 1)" + + # Match a read permission for the user to the record's + # owner_uuid. This is so we can have a permissions table that + # mostly consists of users and groups (projects are a type of + # group) and not have to compute and list user permission to + # every single object in the system. + # + # Don't do this for API keys (special behavior) or groups + # (already covered by direct_check). + # + # The traverse_owned flag indicates whether the permission to + # read an object also implies transitive permission to read + # things the object owns. The situation where this is important + # are determining if we can read an object owned by another + # user. This makes it possible to have permission to read the + # user record without granting permission to read things the + # other user owns. + owner_check = "" + if sql_table != "api_client_authorizations" and sql_table != "groups" then + owner_check = "#{sql_table}.owner_uuid IN (SELECT target_uuid FROM #{PERMISSION_VIEW} "+ + "WHERE user_uuid IN (#{user_uuids_subquery}) AND perm_level >= 1 AND traverse_owned) " + + # We want to do owner_check before direct_check in the OR + # clause. The order of the OR clause isn't supposed to + # matter, but in practice, it does -- apparently in the + # absence of other hints, it uses the ordering from the query. + # For certain types of queries (like filtering on owner_uuid), + # every item will match the owner_check clause, so then + # Postgres will optimize out the direct_check entirely. + direct_check = " OR " + direct_check end - if sql_table != "api_client_authorizations" and sql_table != "groups" - owner_check = "OR (target_uuid = #{sql_table}.owner_uuid AND target_owner_uuid IS NOT NULL)" - else - owner_check = "" - end - - sql_conds.push "EXISTS(SELECT 1 FROM #{PERMISSION_VIEW} "+ - "WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 #{trashed_check} AND (target_uuid = #{sql_table}.uuid #{owner_check}))" - + links_cond = "" if sql_table == "links" # Match any permission link that gives one of the authorized # users some permission _or_ gives anyone else permission to # view one of the authorized users. - sql_conds.push "(#{sql_table}.link_class IN (:permission_link_classes) AND "+ - "(#{sql_table}.head_uuid IN (:user_uuids) OR #{sql_table}.tail_uuid IN (:user_uuids)))" + links_cond = "OR (#{sql_table}.link_class IN (:permission_link_classes) AND "+ + "(#{sql_table}.head_uuid IN (#{user_uuids_subquery}) OR #{sql_table}.tail_uuid IN (#{user_uuids_subquery})))" + end + + sql_conds = "(#{owner_check} #{direct_check} #{links_cond}) #{trashed_check.empty? ? "" : "AND"} #{trashed_check}" + + end + + if !include_old_versions && sql_table == "collections" + exclude_old_versions = "#{sql_table}.uuid = #{sql_table}.current_version_uuid" + if sql_conds.nil? + sql_conds = exclude_old_versions + else + sql_conds += " AND #{exclude_old_versions}" end end - query_on.where(sql_conds.join(' OR '), - user_uuids: user_uuids, - permission_link_classes: ['permission', 'resources']) + self.where(sql_conds, + user_uuids: all_user_uuids.collect{|c| c["target_uuid"]}, + permission_link_classes: ['permission', 'resources']) end def save_with_unique_name! @@ -322,7 +433,7 @@ class ArvadosModel < ActiveRecord::Base # 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 + err = rn.cause raise unless err.is_a?(PG::UniqueViolation) # Unfortunately ActiveRecord doesn't abstract out any of the @@ -342,7 +453,13 @@ class ArvadosModel < ActiveRecord::Base end self[:name] = new_name - self[:uuid] = nil if uuid_was.nil? && !uuid.nil? + if uuid_was.nil? && !uuid.nil? + self[:uuid] = nil + if self.is_a? Collection + # Reset so that is assigned to the new UUID + self[:current_version_uuid] = nil + end + end conn.exec_query 'SAVEPOINT save_with_unique_name' retry ensure @@ -351,8 +468,20 @@ class ArvadosModel < ActiveRecord::Base end end + def user_owner_uuid + if self.owner_uuid.nil? + return current_user.uuid + end + owner_class = ArvadosModel.resource_class_for_uuid(self.owner_uuid) + if owner_class == User + self.owner_uuid + else + owner_class.find_by_uuid(self.owner_uuid).user_owner_uuid + end + end + def logged_attributes - attributes.except(*Rails.configuration.unlogged_attributes) + attributes.except(*Rails.configuration.AuditLogs.UnloggedAttributes.stringify_keys.keys) end def self.full_text_searchable_columns @@ -361,12 +490,25 @@ class ArvadosModel < ActiveRecord::Base end.map(&:name) end + def self.full_text_coalesce + full_text_searchable_columns.collect do |column| + is_jsonb = self.columns.select{|x|x.name == column}[0].type == :jsonb + cast = (is_jsonb || serialized_attributes[column]) ? '::text' : '' + "coalesce(#{column}#{cast},'')" + end + end + + def self.full_text_trgm + "(#{full_text_coalesce.join(" || ' ' || ")})" + end + def self.full_text_tsvector parts = full_text_searchable_columns.collect do |column| - cast = serialized_attributes[column] ? '::text' : '' + is_jsonb = self.columns.select{|x|x.name == column}[0].type == :jsonb + cast = (is_jsonb || serialized_attributes[column]) ? '::text' : '' "coalesce(#{column}#{cast},'')" end - "to_tsvector('english', #{parts.join(" || ' ' || ")})" + "to_tsvector('english', substr(#{parts.join(" || ' ' || ")}, 0, 8000))" end def self.apply_filters query, filters @@ -374,6 +516,9 @@ class ArvadosModel < ActiveRecord::Base if not ft[:cond_out].any? return query end + ft[:joins].each do |t| + query = query.joins(t) + end query.where('(' + ft[:cond_out].join(') AND (') + ')', *ft[:param_out]) end @@ -408,7 +553,7 @@ class ArvadosModel < ActiveRecord::Base end rescue ActiveRecord::RecordNotFound => e errors.add :owner_uuid, "is not owned by any user: #{e}" - return false + throw(:abort) end if uuid_in_path[x] if x == owner_uuid @@ -416,7 +561,7 @@ class ArvadosModel < ActiveRecord::Base else errors.add :owner_uuid, "has an ownership cycle" end - return false + throw(:abort) end uuid_in_path[x] = true end @@ -458,6 +603,9 @@ class ArvadosModel < ActiveRecord::Base 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 + elsif rsc_class == Group && Group.find_by_uuid(owner_uuid).group_class != "project" + errors.add :owner_uuid, "must be a project" + raise PermissionDeniedError end end else @@ -466,7 +614,7 @@ class ArvadosModel < ActiveRecord::Base # 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" + errors.add :uuid, " #{uuid} is not writable by #{current_user.uuid}" raise PermissionDeniedError end end @@ -506,7 +654,12 @@ class ArvadosModel < ActiveRecord::Base end def permission_to_destroy - permission_to_update + if [system_user_uuid, system_group_uuid, anonymous_group_uuid, + anonymous_user_uuid, public_project_uuid].include? uuid + false + else + permission_to_update + end end def maybe_update_modified_by_fields @@ -516,11 +669,15 @@ class ArvadosModel < ActiveRecord::Base def update_modified_by_fields current_time = db_current_time - self.created_at = created_at_was || 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 - self.modified_by_user_uuid = current_user ? current_user.uuid : nil + if !anonymous_updater + self.modified_by_user_uuid = current_user ? current_user.uuid : nil + end + if !timeless_updater + self.modified_at = current_time + end self.modified_by_client_uuid = current_api_client ? current_api_client.uuid : nil true end @@ -538,51 +695,24 @@ class ArvadosModel < ActiveRecord::Base false end - def self.has_symbols? x - if x.is_a? Hash - x.each do |k,v| - return true if has_symbols?(k) or has_symbols?(v) - end - elsif x.is_a? Array - x.each do |k| - return true if has_symbols?(k) - end - elsif x.is_a? Symbol - return true - elsif x.is_a? String - return true if x.start_with?(':') && !x.start_with?('::') + def self.where_serialized(colname, value, md5: false) + colsql = colname.to_s + if md5 + colsql = "md5(#{colsql})" end - false - end - - def self.recursive_stringify x - if x.is_a? Hash - Hash[x.collect do |k,v| - [recursive_stringify(k), recursive_stringify(v)] - end] - elsif x.is_a? Array - x.collect do |k| - recursive_stringify k - end - elsif x.is_a? Symbol - x.to_s - elsif x.is_a? String and x.start_with?(':') and !x.start_with?('::') - x[1..-1] - else - x - 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 (?)" + sql = "#{colsql} is null or #{colsql} IN (?)" sorted = value else - sql = "#{colname.to_s} IN (?)" + sql = "#{colsql} IN (?)" sorted = deep_sort_hash(value) end - where(sql, [sorted.to_yaml, SafeJSON.dump(sorted)]) + params = [sorted.to_yaml, SafeJSON.dump(sorted)] + if md5 + params = params.map { |x| Digest::MD5.hexdigest(x) } + end + where(sql, params) end Serializer = { @@ -605,22 +735,6 @@ class ArvadosModel < ActiveRecord::Base self.class.serialized_attributes end - def convert_serialized_symbols_to_strings - # ensure_serialized_attribute_type should prevent symbols from - # getting into the database in the first place. If someone managed - # to get them into the database (perhaps using an older version) - # we'll convert symbols to strings when loading from the - # database. (Otherwise, loading and saving an object with existing - # symbols in a serialized field will crash.) - self.class.serialized_attributes.each do |colname, attr| - if self.class.has_symbols? attributes[colname] - attributes[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$/) } end @@ -662,13 +776,27 @@ class ArvadosModel < ActiveRecord::Base end def self.uuid_like_pattern - "#{Rails.configuration.uuid_prefix}-#{uuid_prefix}-_______________" + "#{Rails.configuration.ClusterID}-#{uuid_prefix}-_______________" end def self.uuid_regex %r/[a-z0-9]{5}-#{uuid_prefix}-[a-z0-9]{15}/ end + def check_readable_uuid attr, attr_value + return if attr_value.nil? + if (r = ArvadosModel::resource_class_for_uuid attr_value) + unless skip_uuid_read_permission_check.include? attr + r = r.readable_by(current_user) + end + if r.where(uuid: attr_value).count == 0 + errors.add(attr, "'#{attr_value}' not found") + end + else + # Not a valid uuid or PDH, but that (currently) is not an error. + end + end + def ensure_valid_uuids specials = [system_user_uuid] @@ -677,20 +805,19 @@ class ArvadosModel < ActiveRecord::Base next if skip_uuid_existence_check.include? attr attr_value = send attr next if specials.include? attr_value - if attr_value - if (r = ArvadosModel::resource_class_for_uuid attr_value) - unless skip_uuid_read_permission_check.include? attr - r = r.readable_by(current_user) - end - if r.where(uuid: attr_value).count == 0 - errors.add(attr, "'#{attr_value}' not found") - end - end - end + check_readable_uuid attr, attr_value end end end + def ensure_filesystem_compatible_name + if name == "." || name == ".." + errors.add(:name, "cannot be '.' or '..'") + elsif Rails.configuration.Collections.ForwardSlashNameSubstitution == "" && !name.nil? && name.index('/') + errors.add(:name, "cannot contain a '/' character") + end + end + class Email def self.kind "email" @@ -728,48 +855,111 @@ class ArvadosModel < ActiveRecord::Base nil end + # Fill in implied zero/false values in database records that were + # created before #17014 made them explicit, and reset the Rails + # "changed" state so the record doesn't appear to have been modified + # after loading. + # + # Invoked by Container and ContainerRequest models as an after_find + # hook. + def fill_container_defaults_after_find + fill_container_defaults + set_attribute_was('runtime_constraints', runtime_constraints) + set_attribute_was('scheduling_parameters', scheduling_parameters) + clear_changes_information + end + + # Fill in implied zero/false values. Invoked by ContainerRequest as + # a before_validation hook in order to (a) ensure every key has a + # value in the updated database record and (b) ensure the attribute + # whitelist doesn't reject a change from an explicit zero/false + # value in the database to an implicit zero/false value in an update + # request. + def fill_container_defaults + self.runtime_constraints = { + 'API' => false, + 'keep_cache_ram' => 0, + 'ram' => 0, + 'vcpus' => 0, + }.merge(attributes['runtime_constraints'] || {}) + self.scheduling_parameters = { + 'max_run_time' => 0, + 'partitions' => [], + 'preemptible' => false, + }.merge(attributes['scheduling_parameters'] || {}) + end + # ArvadosModel.find_by_uuid needs extra magic to allow it to return # an object in any class. def self.find_by_uuid uuid 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).unscoped.find_by_uuid(uuid) + self.resource_class_for_uuid(uuid).find_by_uuid(uuid) else super end end + def is_audit_logging_enabled? + return !(Rails.configuration.AuditLogs.MaxAge.to_i == 0 && + Rails.configuration.AuditLogs.MaxDeleteBatch.to_i > 0) + end + + def schedule_restoring_changes + # This will be checked at log_start_state, to reset any (virtual) changes + # produced by the act of reading a serialized attribute. + @fresh_from_database = true + end + def log_start_state - @old_attributes = Marshal.load(Marshal.dump(attributes)) - @old_logged_attributes = Marshal.load(Marshal.dump(logged_attributes)) + if is_audit_logging_enabled? + @old_attributes = Marshal.load(Marshal.dump(attributes)) + @old_logged_attributes = Marshal.load(Marshal.dump(logged_attributes)) + if @fresh_from_database + # This instance was created from reading a database record. Attributes + # haven't been changed, but those serialized attributes will be reported + # as unpersisted, so we restore them to avoid issues with lock!() and + # with_lock(). + restore_attributes + @fresh_from_database = nil + end + end end def log_change(event_type) - log = Log.new(event_type: event_type).fill_object(self) - yield log - log.save! - log_start_state + if is_audit_logging_enabled? + log = Log.new(event_type: event_type).fill_object(self) + yield log + log.save! + log_start_state + end end def log_create - log_change('create') do |log| - log.fill_properties('old', nil, nil) - log.update_to self + if is_audit_logging_enabled? + log_change('create') do |log| + log.fill_properties('old', nil, nil) + log.update_to self + end end end def log_update - log_change('update') do |log| - log.fill_properties('old', etag(@old_attributes), @old_logged_attributes) - log.update_to self + if is_audit_logging_enabled? + log_change('update') do |log| + log.fill_properties('old', etag(@old_attributes), @old_logged_attributes) + log.update_to self + end end end def log_destroy - log_change('delete') do |log| - log.fill_properties('old', etag(@old_attributes), @old_logged_attributes) - log.update_to nil + if is_audit_logging_enabled? + log_change('delete') do |log| + log.fill_properties('old', etag(@old_attributes), @old_logged_attributes) + log.update_to nil + end end end end