#472: Remove protected_attributes gem, use strong_parameters for Api::V1::AdvisoriesController

Gemfile

@@ -6,8 +6,6 @@ gem 'activeadmin',                      github: 'activeadmin'
 gem 'pg'
 gem 'schema_plus', '~> 1.5'
 ########
-gem 'protected_attributes'
-########
 gem 'devise'
 gem 'omniauth'
 gem 'omniauth-facebook'

Gemfile.lock

@@ -342,8 +342,6 @@ GEM
     polyamorous (1.1.0)
       activerecord (>= 3.0)
     posix-spawn (0.3.10)
-    protected_attributes (1.0.9)
-      activemodel (>= 4.0.1, < 5.0)
     puma (2.11.1)
       rack (>= 1.1, < 2.0)
     pundit (0.3.0)
@@ -634,7 +632,6 @@ DEPENDENCIES
   paperclip
   perform_later!
   pg
-  protected_attributes
   puma
   pundit
   rack-throttle (~> 0.3.0)

app/controllers/api/v1/advisories_controller.rb

@@ -16,7 +16,7 @@ class Api::V1::AdvisoriesController < Api::V1::BaseController
   def create
     authorize :advisory
     if @build_list.can_attach_to_advisory? &&
-        @build_list.associate_and_create_advisory(params[:advisory]) &&
+        @build_list.associate_and_create_advisory(advisory_params) &&
         @build_list.save
       render_json_response @build_list.advisory, 'Advisory has been created successfully'
     else
@@ -35,6 +35,10 @@ class Api::V1::AdvisoriesController < Api::V1::BaseController
 
   protected
 
+  def advisory_params
+    permit_params(:advisory, *policy(Advisory).permitted_attributes)
+  end
+
   def load_build_list
     @build_list = BuildList.find params[:build_list_id]
     authorize @build_list.save_to_platform, :local_admin_manage?

app/controllers/concerns/strong_params.rb

@@ -4,6 +4,8 @@ module StrongParams
   protected
 
   def permit_params(param_name, *accessible)
-    (params[param_name] || ActionController::Parameters.new).permit(*accessible.flatten)
+    [param_name].flatten.inject(params.dup) do |pp, name|
+      pp = pp[name] || ActionController::Parameters.new
+    end.permit(*accessible.flatten)
   end
 end

app/controllers/projects/build_lists_controller.rb

@@ -105,7 +105,7 @@ class Projects::BuildListsController < Projects::BaseController
 
       if params[:attach_advisory] == 'new'
         # create new advisory
-        unless @build_list.associate_and_create_advisory(params[:build_list][:advisory])
+        unless @build_list.associate_and_create_advisory(advisory_params)
           redirect_to :back, notice: t('layout.build_lists.publish_fail') and return
         end
       else
@@ -206,6 +206,10 @@ class Projects::BuildListsController < Projects::BaseController
 
   protected
 
+  def advisory_params
+    permit_params(%i(build_list advisory), *policy(Advisory).permitted_attributes)
+  end
+
   # Private: before_action hook which loads BuidList.
   def load_build_list
     authorize @build_list =

app/models/activity_feed.rb

@@ -9,7 +9,7 @@ class ActivityFeed < ActiveRecord::Base
   belongs_to :creator, class_name: 'User'
   serialize  :data
 
-  attr_accessible :user, :kind, :data, :project_owner, :project_name, :creator_id
+  # attr_accessible :user, :kind, :data, :project_owner, :project_name, :creator_id
 
   default_scope { order created_at: :desc }
   scope :outdated,        -> { offset(1000) }

app/models/advisory.rb

@@ -12,8 +12,6 @@ class Advisory < ActiveRecord::Base
   after_create :generate_advisory_id
   before_save  :normalize_references, if: :references_changed?
 
-  attr_accessible :description, :references
-
   ID_TEMPLATE        = 'ROSA-%<type>s-%<year>d:%<id>04d'
   ID_STRING_TEMPLATE = 'ROSA-%<type>s-%<year>04s:%<id>04s'
   TYPES = {'security' => 'SA', 'bugfix' => 'A'}

app/models/avatar.rb

@@ -16,5 +16,5 @@ class Avatar < ActiveRecord::Base
   validates_attachment_content_type :avatar, content_type: /\Aimage/
   validates_attachment_file_name :avatar, matches: [ /(png|jpe?g|gif|bmp|tif?f)\z/i ]
 
-  attr_accessible :avatar
+  # attr_accessible :avatar
 end

app/models/build_list.rb

@@ -88,12 +88,12 @@ class BuildList < ActiveRecord::Base
   before_validation :prepare_extra_params,        on: :create
   before_validation :prepare_auto_publish_status, on: :create
 
-  attr_accessible :include_repos, :auto_publish, :build_for_platform_id, :commit_hash,
-                  :arch_id, :project_id, :save_to_repository_id, :update_type,
-                  :save_to_platform_id, :project_version, :auto_create_container,
-                  :extra_repositories, :extra_build_lists, :extra_params,
-                  :include_testing_subrepository, :auto_publish_status,
-                  :use_cached_chroot, :use_extra_tests, :save_buildroot
+  # attr_accessible :include_repos, :auto_publish, :build_for_platform_id, :commit_hash,
+  #                 :arch_id, :project_id, :save_to_repository_id, :update_type,
+  #                 :save_to_platform_id, :project_version, :auto_create_container,
+  #                 :extra_repositories, :extra_build_lists, :extra_params,
+  #                 :include_testing_subrepository, :auto_publish_status,
+  #                 :use_cached_chroot, :use_extra_tests, :save_buildroot
 
   LIVE_TIME     = 4.week  # for unpublished
   MAX_LIVE_TIME = 3.month # for published

app/models/build_list/package.rb

@@ -7,7 +7,7 @@ class BuildList::Package < ActiveRecord::Base
 
   serialize :dependent_packages, Array
 
-  attr_accessible :fullname, :name, :release, :version, :sha1, :epoch, :dependent_packages
+  # attr_accessible :fullname, :name, :release, :version, :sha1, :epoch, :dependent_packages
 
   validates :build_list, :build_list_id, :project, :project_id,
             :platform, :platform_id, :fullname,

app/models/build_script.rb

@@ -18,7 +18,7 @@ class BuildScript < ActiveRecord::Base
 
   before_validation :attach_project
   attr_writer       :project_name
-  attr_accessible   :project_name, :treeish, :commit, :sha1, :status
+  # attr_accessible   :project_name, :treeish, :commit, :sha1, :status
 
   state_machine :status, initial: :active do
     event(:disable) { transition active: :blocked }

app/models/collaborator.rb

@@ -8,7 +8,7 @@ class Collaborator
   attr_accessor :role, :actor, :project, :relation
   attr_reader :id, :actor_id, :actor_type, :actor_name, :project_id
 
-  attr_accessible :role
+  # attr_accessible :role
 
   delegate :new_record?, to: :relation
 

app/models/comment.rb

@@ -22,7 +22,7 @@ class Comment < ActiveRecord::Base
   after_create :subscribe_on_reply, unless: ->(c) { c.commit_comment? }
   after_create :subscribe_users
 
-  attr_accessible :body, :data
+  # attr_accessible :body, :data
 
   def commentable
     commit_comment? ? project.repo.commit(Comment.hex_to_commit_hash commentable_id) : super

app/models/concerns/autostart.rb

@@ -16,7 +16,7 @@ module Autostart
     validates :autostart_status, numericality: true,
       inclusion: {in: AUTOSTART_STATUSES}, allow_blank: true
 
-    attr_accessible :autostart_status
+    # attr_accessible :autostart_status
   end
 
   def human_autostart_status

app/models/concerns/default_branchable.rb

@@ -5,7 +5,7 @@ module DefaultBranchable
     validates :default_branch,
               length:     { maximum: 100 }
 
-    attr_accessible :default_branch
+    # attr_accessible :default_branch
   end
 
 end

app/models/concerns/external_nodable.rb

@@ -9,7 +9,7 @@ module ExternalNodable
               allow_blank:            true
 
 
-    attr_accessible :external_nodes
+    # attr_accessible :external_nodes
   end
 
 end

app/models/concerns/product_build_lists/statusable.rb

@@ -41,7 +41,7 @@ module ProductBuildLists::Statusable
       presence:       true,
       inclusion:      { in: STATUSES }
 
-    attr_accessible :status
+    # attr_accessible :status
 
     before_destroy :can_destroy?
 

app/models/concerns/time_living.rb

@@ -18,7 +18,7 @@ module TimeLiving
     }
 
     before_validation :convert_time_living
-    attr_accessible :time_living
+    # attr_accessible :time_living
   end
 
   protected

app/models/event_log.rb

@@ -12,7 +12,7 @@ class EventLog < ActiveRecord::Base
     self.eventable_name ||= eventable.name if eventable.respond_to?(:name)
   end
   # after_create { self.class.current_controller = nil }
-  attr_accessible :kind, :message, :eventable, :eventable_name
+  # attr_accessible :kind, :message, :eventable, :eventable_name
 
   class << self
     def create_with_current_controller(attributes)

app/models/feedback.rb

@@ -12,7 +12,7 @@ class Feedback
 
   attr_accessor :name, :email, :subject, :message
 
-  attr_accessible :name, :email, :subject, :message
+  # attr_accessible :name, :email, :subject, :message
 
   validates :name, :subject, :message, presence: true
   validates :email, presence: true,

app/models/flash_notify.rb

@@ -8,7 +8,7 @@ class FlashNotify < ActiveRecord::Base
   validates :status, inclusion: {in: STATUSES}
   validates :body_ru, :body_en, :status, presence: true
 
-  attr_accessible :body_ru, :body_en, :status, :published
+  # attr_accessible :body_ru, :body_en, :status, :published
 
   def hash_id
     @digest ||= Digest::MD5.hexdigest("#{self.id}-#{self.updated_at}")

app/models/group.rb

@@ -32,7 +32,7 @@ class Group < Avatar
     joins(:actors).where('relations.role' => ['admin', 'writer'], 'relations.actor_id' => actor.id, 'relations.actor_type' => 'User')
   }
 
-  attr_accessible :uname, :description, :delete_avatar
+  # attr_accessible :uname, :description, :delete_avatar
   attr_readonly :uname
 
   attr_accessor :delete_avatar

app/models/hook.rb

@@ -9,7 +9,7 @@ class Hook < ActiveRecord::Base
   validates :project, :data, presence: true
   validates :name, presence: true, inclusion: {in: NAMES}
 
-  attr_accessible :data, :name
+  # attr_accessible :data, :name
 
   serialize :data,  Hash
 

app/models/issue.rb

@@ -47,7 +47,7 @@ class Issue < ActiveRecord::Base
   before_create :update_statistic
   before_update :update_statistic
 
-  attr_accessible :labelings_attributes, :title, :body, :assignee_id
+  # attr_accessible :labelings_attributes, :title, :body, :assignee_id
   accepts_nested_attributes_for :labelings,
     reject_if: lambda {|attributes| attributes['label_id'].blank?},
     allow_destroy: true

app/models/key_pair.rb

@@ -4,7 +4,7 @@ class KeyPair < ActiveRecord::Base
   belongs_to :user
 
   attr_accessor :fingerprint
-  attr_accessible :public, :secret, :repository_id
+  # attr_accessible :public, :secret, :repository_id
   attr_encrypted :secret, key: APP_CONFIG['keys']['key_pair_secret_key']
 
   validates :repository, :user, presence: true

app/models/label.rb

@@ -9,5 +9,5 @@ class Label < ActiveRecord::Base
   validates :color, presence: true
   validates :color, format: { with: /\A([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})\z/, message: I18n.t('layout.issues.invalid_labels') }
 
-  attr_accessible :name, :color
+  # attr_accessible :name, :color
 end

app/models/labeling.rb

@@ -2,5 +2,5 @@ class Labeling < ActiveRecord::Base
   belongs_to :issue
   belongs_to :label
 
-  attr_accessible :id, :label_id
+  # attr_accessible :id, :label_id
 end

app/models/mass_build.rb

@@ -45,10 +45,10 @@ class MassBuild < ActiveRecord::Base
   scope :search,      -> (q) { where("#{table_name}.description ILIKE ?", "%#{q}%") if q.present? }
 
   attr_accessor :arches, :repositories
-  attr_accessible :arches, :auto_publish_status, :projects_list, :build_for_platform_id,
-                  :extra_repositories, :extra_build_lists, :increase_release_tag,
-                  :use_cached_chroot, :use_extra_tests, :description, :extra_mass_builds,
-                  :include_testing_subrepository, :auto_create_container, :repositories
+  # attr_accessible :arches, :auto_publish_status, :projects_list, :build_for_platform_id,
+  #                 :extra_repositories, :extra_build_lists, :increase_release_tag,
+  #                 :use_cached_chroot, :use_extra_tests, :description, :extra_mass_builds,
+  #                 :include_testing_subrepository, :auto_create_container, :repositories
 
   validates :save_to_platform_id,
             :build_for_platform_id,

app/models/node_instruction.rb

@@ -23,7 +23,7 @@ class NodeInstruction < ActiveRecord::Base
     errors.add(:status, 'Can be only single active instruction for each node') if !disabled? && NodeInstruction.duplicate(id.to_i, user_id).exists?
   }
 
-  attr_accessible :instruction, :user_id, :output, :status
+  # attr_accessible :instruction, :user_id, :output, :status
 
   state_machine :status, initial: :ready do
 

app/models/platform.rb

@@ -98,18 +98,18 @@ class Platform < ActiveRecord::Base
   after_destroy -> { remove_symlink_directory unless hidden? }
 
   accepts_nested_attributes_for :platform_arch_settings, allow_destroy: true
-  attr_accessible :name,
-                  :distrib_type,
-                  :parent_platform_id,
-                  :platform_type,
-                  :owner,
-                  :visibility,
-                  :description,
-                  :released,
-                  :platform_arch_settings_attributes,
-                  :automatic_metadata_regeneration,
-                  :admin_id,
-                  :term
+  # attr_accessible :name,
+  #                 :distrib_type,
+  #                 :parent_platform_id,
+  #                 :platform_type,
+  #                 :owner,
+  #                 :visibility,
+  #                 :description,
+  #                 :released,
+  #                 :platform_arch_settings_attributes,
+  #                 :automatic_metadata_regeneration,
+  #                 :admin_id,
+  #                 :term
 
   attr_accessor :admin_id, :term
 

app/models/platform_arch_setting.rb

@@ -16,5 +16,5 @@ class PlatformArchSetting < ActiveRecord::Base
   scope :by_arch,    ->(arch) { where(arch_id: arch) if arch.present? }
   scope :by_default, -> { where(default: true) }
 
-  attr_accessible :arch_id, :platform_id, :default
+  # attr_accessible :arch_id, :platform_id, :default
 end

app/models/product.rb

@@ -16,13 +16,13 @@ class Product < ActiveRecord::Base
 
   scope :recent, -> { order(:name) }
 
-  attr_accessible :name,
-                  :description,
-                  :project_id,
-                  :main_script,
-                  :params,
-                  :platform_id,
-                  :project_version
+  # attr_accessible :name,
+  #                 :description,
+  #                 :project_id,
+  #                 :main_script,
+  #                 :params,
+  #                 :platform_id,
+  #                 :project_version
   attr_readonly :platform_id
 
   def full_clone(attrs = {})

app/models/product_build_list.rb

@@ -28,16 +28,16 @@ class ProductBuildList < ActiveRecord::Base
   validates :main_script, :params, length: { maximum: 255 }
 
   attr_accessor :base_url, :product_name
-  attr_accessible :base_url,
-                  :branch,
-                  :project_id,
-                  :main_script,
-                  :params,
-                  :project_version,
-                  :commit_hash,
-                  :product_id,
-                  :not_delete,
-                  :product_name
+  # attr_accessible :base_url,
+  #                 :branch,
+  #                 :project_id,
+  #                 :main_script,
+  #                 :params,
+  #                 :project_version,
+  #                 :commit_hash,
+  #                 :product_id,
+  #                 :not_delete,
+  #                 :product_name
 
   attr_readonly :product_id
   serialize :results, Array

app/models/project.rb

@@ -62,10 +62,10 @@ class Project < ActiveRecord::Base
     errors.delete :project_to_repositories
   end
 
-  attr_accessible :name, :description, :visibility, :srpm, :is_package,
-                  :has_issues, :has_wiki, :maintainer_id, :publish_i686_into_x86_64,
-                  :url, :srpms_list, :mass_import, :add_to_repository_id, :architecture_dependent,
-                  :autostart_status
+  # attr_accessible :name, :description, :visibility, :srpm, :is_package,
+  #                 :has_issues, :has_wiki, :maintainer_id, :publish_i686_into_x86_64,
+  #                 :url, :srpms_list, :mass_import, :add_to_repository_id, :architecture_dependent,
+  #                 :autostart_status
   attr_readonly :owner_id, :owner_type
 
   before_validation :truncate_name, on: :create

app/models/project_statistic.rb

@@ -6,5 +6,5 @@ class ProjectStatistic < ActiveRecord::Base
   validates :arch, :project, :average_build_time, :build_count, presence: true
   validates :project_id, uniqueness: { scope: :arch_id }
 
-  attr_accessible :average_build_time, :build_count
+  # attr_accessible :average_build_time, :build_count
 end

app/models/project_tag.rb

@@ -11,7 +11,7 @@ class ProjectTag < ActiveRecord::Base
   validates :project, :commit_id, :sha1, :tag_name, :format_id, presence: true
   validates :project_id, uniqueness: { scope: [:tag_name, :format_id] }
 
-  attr_accessible :project_id, :commit_id, :sha1, :tag_name, :format_id
+  # attr_accessible :project_id, :commit_id, :sha1, :tag_name, :format_id
 
   def sha1_of_file_store_files
     [sha1]

app/models/project_to_repository.rb

@@ -12,7 +12,7 @@ class ProjectToRepository < ActiveRecord::Base
 
   validate :one_project_in_platform_repositories, on: :create
 
-  attr_accessible :project, :project_id
+  # attr_accessible :project, :project_id
 
   AUTOSTART_OPTIONS.each do |field|
     store_accessor :autostart_options, field

app/models/pull_request.rb

@@ -49,7 +49,7 @@ class PullRequest < ActiveRecord::Base
   after_destroy :clean_dir
 
   accepts_nested_attributes_for :issue
-  attr_accessible :issue_attributes, :to_ref, :from_ref
+  # attr_accessible :issue_attributes, :to_ref, :from_ref
 
   scope :needed_checking,       -> { includes(:issue).where(issues: { status: [STATUS_OPEN, STATUS_BLOCKED, STATUS_READY] }) }
   scope :not_closed_or_merged,  -> { needed_checking }

app/models/relation.rb

@@ -15,7 +15,7 @@ class Relation < ActiveRecord::Base
 #  validate { errors.add(:actor, :taken) if Relation.where(actor_type: self.actor_type, actor_id: self.actor_id).present? }
   before_validation :add_default_role
 
-  attr_accessible :actor_id, :actor_type, :target_id, :target_type, :actor, :target, :role
+  # attr_accessible :actor_id, :actor_type, :target_id, :target_type, :actor, :target, :role
 
   scope :by_user_through_groups, ->(u) {
     where("actor_type = 'User' AND actor_id = ? OR actor_type = 'Group' AND actor_id IN (?)", u.id, u.group_ids)

app/models/repository.rb

@@ -36,12 +36,12 @@ class Repository < ActiveRecord::Base
 
   before_destroy  :detele_directory
 
-  attr_accessible :name,
-                  :description,
-                  :publish_without_qa,
-                  :synchronizing_publications,
-                  :publish_builds_only_from_branch,
-                  :build_for_platform_id
+  # attr_accessible :name,
+  #                 :description,
+  #                 :publish_without_qa,
+  #                 :synchronizing_publications,
+  #                 :publish_builds_only_from_branch,
+  #                 :build_for_platform_id
 
   attr_readonly :name, :platform_id
   attr_accessor :projects_list, :build_for_platform_id

app/models/repository_status.rb

@@ -31,7 +31,7 @@ class RepositoryStatus < ActiveRecord::Base
   validates :repository, :platform, presence: true
   validates :repository_id, uniqueness: { scope: :platform_id }
 
-  attr_accessible :platform_id, :repository_id
+  # attr_accessible :platform_id, :repository_id
 
   scope :platform_ready, -> { where(platforms: {status: READY}).joins(:platform) }
   scope :for_regeneration, -> { where(status: WAITING_FOR_REGENERATION) }

app/models/settings_notifier.rb

@@ -3,16 +3,16 @@ class SettingsNotifier < ActiveRecord::Base
 
   validates :user, presence: true
 
-  attr_accessible :can_notify,
-                  :update_code,
-                  :new_comment_commit_owner,
-                  :new_comment_commit_repo_owner,
-                  :new_comment_commit_commentor,
-                  :new_comment,
-                  :new_comment_reply,
-                  :new_issue,
-                  :issue_assign,
-                  :new_build,
-                  :new_associated_build
+  # attr_accessible :can_notify,
+  #                 :update_code,
+  #                 :new_comment_commit_owner,
+  #                 :new_comment_commit_repo_owner,
+  #                 :new_comment_commit_commentor,
+  #                 :new_comment,
+  #                 :new_comment_reply,
+  #                 :new_issue,
+  #                 :issue_assign,
+  #                 :new_build,
+  #                 :new_associated_build
 
 end

app/models/ssh_key.rb

@@ -5,7 +5,7 @@ class SshKey < ActiveRecord::Base
   SHELL_KEY_COMMAND = "sudo -i -u #{APP_CONFIG['shell_user']} ~#{APP_CONFIG['shell_user']}/gitlab-shell/bin/gitlab-keys"
 
   belongs_to :user
-  attr_accessible :key, :name
+  # attr_accessible :key, :name
 
   before_validation -> { self.key = key.strip if key.present? }
   before_validation :set_fingerprint

app/models/statistic.rb

@@ -41,13 +41,13 @@ class Statistic < ActiveRecord::Base
   validates :activity_at,
     presence: true
 
-  attr_accessible :user_id,
-                  :email,
-                  :project_id,
-                  :project_name_with_owner,
-                  :key,
-                  :counter,
-                  :activity_at
+  # attr_accessible :user_id,
+  #                 :email,
+  #                 :project_id,
+  #                 :project_name_with_owner,
+  #                 :key,
+  #                 :counter,
+  #                 :activity_at
 
   scope :for_period,            -> (start_date, end_date) {
     where(activity_at: (start_date..end_date))

app/models/subscribe.rb

@@ -3,7 +3,7 @@ class Subscribe < ActiveRecord::Base
   belongs_to :user
   belongs_to :project
 
-  attr_accessible :status, :user_id
+  # attr_accessible :status, :user_id
   validates :user, presence: true
 
   def commit_subscribe?

app/models/token.rb

@@ -12,7 +12,7 @@ class Token < ActiveRecord::Base
 
   before_validation :generate_token, on: :create
 
-  attr_accessible :description
+  # attr_accessible :description
 
   state_machine :status, initial: :active do
     event :block do

app/models/user.rb

@@ -56,8 +56,8 @@ class User < Avatar
   validates :role, inclusion: { in: EXTENDED_ROLES }, allow_blank: true
   validates :language, inclusion: { in: LANGUAGES }, allow_blank: true
 
-  attr_accessible :email, :password, :password_confirmation, :current_password, :remember_me, :login, :name, :uname, :language,
-                  :site, :company, :professional_experience, :location, :sound_notifications, :hide_email, :delete_avatar
+  # attr_accessible :email, :password, :password_confirmation, :current_password, :remember_me, :login, :name, :uname, :language,
+  #                 :site, :company, :professional_experience, :location, :sound_notifications, :hide_email, :delete_avatar
   attr_readonly :uname
   attr_accessor :login, :delete_avatar
 

app/models/user_builds_setting.rb

@@ -5,6 +5,6 @@ class UserBuildsSetting < ActiveRecord::Base
 
   validates :user, presence: true
 
-  attr_accessible :platforms
+  # attr_accessible :platforms
 
 end

app/policies/advisory_policy.rb

@@ -11,4 +11,14 @@ class AdvisoryPolicy < ApplicationPolicy
   end
   alias_method :update?, :create?
 
+  # Public: Get list of parameters that the user is allowed to alter.
+  #
+  # Returns Array
+  def permitted_attributes
+    %i(
+      description
+      references
+    )
+  end
+
 end

config/environments/development.rb

@@ -52,9 +52,6 @@ Rosa::Application.configure do
   # Expands the lines which load the assets
   config.assets.debug = true
 
-  # Raise exception on mass assignment protection for Active Record models
-  config.active_record.mass_assignment_sanitizer = :strict
-
   config.middleware.insert_before Rails::Rack::Logger, DisableAssetsLogger
 
   config.eager_load = false