X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/231a86fd3f7e30e9f66d71d92ad7c26578637e37..7499f61a2912cfdb1a316808fafa6e6ee77ee2e0:/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 8c8ad8e254..3966b7c393 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -16,6 +16,7 @@ class ArvadosModel < ApplicationRecord
   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
@@ -137,6 +138,7 @@ class ArvadosModel < ApplicationRecord
   def reload(*args)
     super
     log_start_state
+    self
   end
 
   def self.create raw_params={}, *args
@@ -285,10 +287,13 @@ class ArvadosModel < ApplicationRecord
     sql_conds = nil
     user_uuids = users_list.map { |u| u.uuid }
 
+    # 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 explicitly trashed
-      exclude_trashed_records = "AND #{sql_table}.is_trashed = false"
+      # 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
 
     if users_list.select { |u| u.is_admin }.any?
@@ -296,16 +301,28 @@ class ArvadosModel < ApplicationRecord
       if !include_trash
         if sql_table != "api_client_authorizations"
           # Only include records where the owner is not trashed
-          sql_conds = "#{sql_table}.owner_uuid NOT IN (SELECT target_uuid FROM #{PERMISSION_VIEW} "+
-                      "WHERE trashed = 1) #{exclude_trashed_records}"
+          sql_conds = "#{sql_table}.owner_uuid NOT IN (SELECT group_uuid FROM #{TRASHED_GROUPS} "+
+                      "where trash_at <= statement_timestamp()) #{exclude_trashed_records}"
         end
       end
     else
       trashed_check = ""
       if !include_trash then
-        trashed_check = "AND trashed = 0"
+        trashed_check = "AND target_uuid NOT IN (SELECT group_uuid FROM #{TRASHED_GROUPS} where trash_at <= statement_timestamp())"
       end
 
+      # 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.
+      user_uuids_subquery = USER_UUIDS_SUBQUERY_TEMPLATE % {user: ":user_uuids", perm_level: 1}
+
       # Note: it is possible to combine the direct_check and
       # owner_check into a single EXISTS() clause, however it turns
       # out query optimizer doesn't like it and forces a sequential
@@ -316,13 +333,28 @@ class ArvadosModel < ApplicationRecord
 
       # 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) AND perm_level >= 1 #{trashed_check})"
+                     "WHERE user_uuid IN (#{user_uuids_subquery}) AND perm_level >= 1 #{trashed_check})"
 
-      # Match a read permission link from the user to the record's owner_uuid
+      # 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 = "OR #{sql_table}.owner_uuid IN (SELECT target_uuid FROM #{PERMISSION_VIEW} "+
-          "WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 #{trashed_check} AND target_owner_uuid IS NOT NULL) "
+          "WHERE user_uuid IN (#{user_uuids_subquery}) AND perm_level >= 1 #{trashed_check} AND traverse_owned) "
       end
 
       links_cond = ""
@@ -331,7 +363,7 @@ class ArvadosModel < ApplicationRecord
         # users some permission _or_ gives anyone else permission to
         # view one of the authorized users.
         links_cond = "OR (#{sql_table}.link_class IN (:permission_link_classes) AND "+
-                       "(#{sql_table}.head_uuid IN (:user_uuids) OR #{sql_table}.tail_uuid IN (:user_uuids)))"
+                       "(#{sql_table}.head_uuid IN (#{user_uuids_subquery}) OR #{sql_table}.tail_uuid IN (#{user_uuids_subquery})))"
       end
 
       sql_conds = "(#{direct_check} #{owner_check} #{links_cond}) #{exclude_trashed_records}"
@@ -422,7 +454,7 @@ class ArvadosModel < ApplicationRecord
   end
 
   def logged_attributes
-    attributes.except(*Rails.configuration.AuditLogs.UnloggedAttributes)
+    attributes.except(*Rails.configuration.AuditLogs.UnloggedAttributes.stringify_keys.keys)
   end
 
   def self.full_text_searchable_columns
@@ -457,6 +489,9 @@ class ArvadosModel < ApplicationRecord
     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
@@ -541,6 +576,9 @@ class ArvadosModel < ApplicationRecord
           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
@@ -549,7 +587,7 @@ class ArvadosModel < ApplicationRecord
       # 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
@@ -589,7 +627,12 @@ class ArvadosModel < ApplicationRecord
   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
@@ -713,6 +756,20 @@ class ArvadosModel < ApplicationRecord
     %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]
 
@@ -721,20 +778,19 @@ class ArvadosModel < ApplicationRecord
         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"
@@ -789,10 +845,24 @@ class ArvadosModel < ApplicationRecord
              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
     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