remove protected attributes

16 files changed, 17 insertions, 53 deletions

diff --git a/Gemfile b/Gemfile
index 26ecb3e..61e9b98 100644
--- a/Gemfile
+++ b/Gemfile
@@ -9,7 +9,6 @@ def linux_only(require_as)
 end
 
 gem 'rails', '4.1.9'
-gem 'protected_attributes'
 gem 'activerecord-deprecated_finders'
 gem 'activerecord-session_store'
 gem "nested_form"
diff --git a/Gemfile.lock b/Gemfile.lock
index 99d833e..8d66913 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -210,8 +210,6 @@ GEM
       multi_json (~> 1.0)
       websocket-driver (>= 0.2.0)
     powerpack (0.0.9)
-    protected_attributes (1.0.8)
-      activemodel (>= 4.0.1, < 5.0)
     pry (0.9.12.4)
       coderay (~> 1.0)
       method_source (~> 0.8)
@@ -400,7 +398,6 @@ DEPENDENCIES
   oauth2 (= 1.0.0)
   pg
   poltergeist (~> 1.5.1)
-  protected_attributes
   pry
   quiet_assets
   rack-mini-profiler
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 06b01ca..f254c8b 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -49,7 +49,7 @@ class ProjectsController < ApplicationController
   end
 
   def create
-    @project = CreateProjectService.new.execute(current_user, params[:project], project_url(":project_id"))
+    @project = CreateProjectService.new.execute(current_user, project_params, project_url(":project_id"))
 
     if @project.persisted?
       redirect_to project_path(@project, show_guide: true), notice: 'Project was successfully created.'
@@ -62,7 +62,7 @@ class ProjectsController < ApplicationController
   end
 
   def update
-    if project.update_attributes(params[:project])
+    if project.update_attributes(project_params)
       
       EventService.new.change_project_settings(current_user, project)
       
@@ -110,4 +110,12 @@ class ProjectsController < ApplicationController
     response.headers["Pragma"] = "no-cache"
     response.headers["Expires"] = "Fri, 01 Jan 1990 00:00:00 GMT"
   end
+
+  def project_params
+    params.require(:project).permit(:name, :path, :timeout, :token, :timeout_in_minutes,
+    :default_ref, :gitlab_url, :always_build, :polling_interval,
+    :public, :ssh_url_to_repo, :gitlab_id, :allow_git_fetch, :skip_refs,
+    :email_recipients, :email_add_pusher, :email_only_broken_builds, :coverage_regex,
+    :jobs_attributes, :shared_runners_enabled)
+  end
 end
diff --git a/app/controllers/web_hooks_controller.rb b/app/controllers/web_hooks_controller.rb
index 03b89cd..8e72609 100644
--- a/app/controllers/web_hooks_controller.rb
+++ b/app/controllers/web_hooks_controller.rb
@@ -12,7 +12,7 @@ class WebHooksController < ApplicationController
   end
 
   def create
-    @web_hook = @project.web_hooks.new(params[:web_hook])
+    @web_hook = @project.web_hooks.new(web_hook_params)
     @web_hook.save
 
     if @web_hook.valid?
@@ -44,4 +44,8 @@ class WebHooksController < ApplicationController
   def project
     @project = Project.find(params[:project_id])
   end
+
+  def web_hook_params
+    params.require(:web_hook).permit(:url)
+  end
 end
diff --git a/app/models/build.rb b/app/models/build.rb
index fa0cb8f..de44a1c 100644
--- a/app/models/build.rb
+++ b/app/models/build.rb
@@ -30,9 +30,6 @@ class Build < ActiveRecord::Base
   belongs_to :runner
   belongs_to :job
 
-  attr_accessible :status, :finished_at, :trace, :started_at, :runner_id,
-    :commit_id, :coverage, :commands, :job_id
-
   validates :commit, presence: true
   validates :status, presence: true
   validates :coverage, numericality: true, allow_blank: true
diff --git a/app/models/project.rb b/app/models/project.rb
index 4351da1..10916ab 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -27,12 +27,6 @@
 class Project < ActiveRecord::Base
   include ProjectStatus
 
-  attr_accessible :name, :path, :timeout, :token, :timeout_in_minutes,
-    :default_ref, :gitlab_url, :always_build, :polling_interval,
-    :public, :ssh_url_to_repo, :gitlab_id, :allow_git_fetch, :skip_refs,
-    :email_recipients, :email_add_pusher, :email_only_broken_builds, :coverage_regex,
-    :jobs_attributes, :shared_runners_enabled
-
   has_many :commits, dependent: :destroy
   has_many :builds, through: :commits, dependent: :destroy
   has_many :runner_projects, dependent: :destroy
diff --git a/app/models/runner_project.rb b/app/models/runner_project.rb
index 118ce07..6907677 100644
--- a/app/models/runner_project.rb
+++ b/app/models/runner_project.rb
@@ -10,8 +10,6 @@
 #
 
 class RunnerProject < ActiveRecord::Base
-  attr_accessible :project_id, :runner_id
-
   belongs_to :runner
   belongs_to :project
 
diff --git a/app/models/web_hook.rb b/app/models/web_hook.rb
index 6b27d5a..9a284d8 100644
--- a/app/models/web_hook.rb
+++ b/app/models/web_hook.rb
@@ -14,8 +14,6 @@ class WebHook < ActiveRecord::Base
 
   belongs_to :project
 
-  attr_accessible :url
-
   # HTTParty timeout
   default_timeout 10
 
diff --git a/app/services/create_project_service.rb b/app/services/create_project_service.rb
index e5f9440..ddbdc8b 100644
--- a/app/services/create_project_service.rb
+++ b/app/services/create_project_service.rb
@@ -21,7 +21,7 @@ class CreateProjectService
     if forked_project
       # Copy jobs
       @project.jobs = forked_project.jobs.map do |job|
-        Job.new(job.attributes)
+        Job.new(job.attributes.except("id"))
       end
 
       # Copy settings
diff --git a/config/application.rb b/config/application.rb
index e2cea6b..c101033 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -41,12 +41,6 @@ module GitlabCi
     # like if you have constraints or database-specific column types
     # config.active_record.schema_format = :sql
 
-    # Enforce whitelist mode for mass assignment.
-    # This will create an empty whitelist of attributes available for mass-assignment for all models
-    # in your app. As such, your models will need to explicitly whitelist or blacklist accessible
-    # parameters by using an attr_accessible or attr_protected declaration.
-    config.active_record.whitelist_attributes = false
-
     # Enable the asset pipeline
     config.assets.enabled = true
 
diff --git a/config/environments/development.rb b/config/environments/development.rb
index 6828857..419aaed 100644
--- a/config/environments/development.rb
+++ b/config/environments/development.rb
@@ -19,9 +19,6 @@ GitlabCi::Application.configure do
   # Only use best-standards-support built into browsers
   config.action_dispatch.best_standards_support = :builtin
 
-  # Raise exception on mass assignment protection for Active Record models
-  config.active_record.mass_assignment_sanitizer = :strict
-
   # Do not compress assets
   config.assets.compress = false
 
diff --git a/config/environments/test.rb b/config/environments/test.rb
index 4ba8e77..70607d3 100644
--- a/config/environments/test.rb
+++ b/config/environments/test.rb
@@ -26,9 +26,6 @@ GitlabCi::Application.configure do
   # ActionMailer::Base.deliveries array.
   config.action_mailer.delivery_method = :test
 
-  # Raise exception on mass assignment protection for Active Record models
-  config.active_record.mass_assignment_sanitizer = :strict
-
   # Print deprecation notices to the stderr
   config.active_support.deprecation = :stderr
 
diff --git a/lib/tasks/brakeman.rake b/lib/tasks/brakeman.rake
index c97efff..3646831 100644
--- a/lib/tasks/brakeman.rake
+++ b/lib/tasks/brakeman.rake
@@ -1,6 +1,6 @@
 desc 'Security check via brakeman'
  task :brakeman do
-   if system("brakeman --skip-files lib/upgrader.rb -w3 -z -x ModelAttributes")
+   if system("brakeman --skip-files lib/upgrader.rb -w3 -z")
      exit 0
    else
      puts 'Security check failed'
diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index f8db78d..de0e0b9 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -38,13 +38,6 @@ describe Build do
   it { should respond_to :pending? }
   it { should respond_to :trace_html }
 
-  it { should allow_mass_assignment_of(:commit_id) }
-  it { should allow_mass_assignment_of(:status) }
-  it { should allow_mass_assignment_of(:started_at) }
-  it { should allow_mass_assignment_of(:finished_at) }
-  it { should allow_mass_assignment_of(:trace) }
-  it { should allow_mass_assignment_of(:runner_id) }
-
   describe :first_pending do
     let(:first) { FactoryGirl.create :build, commit: commit, status: 'pending', created_at: Date.yesterday }
     let(:second) { FactoryGirl.create :build, commit: commit, status: 'pending' }
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index d4a516b..03bc1e8 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -30,14 +30,6 @@ describe Commit do
   it { should respond_to :git_author_email }
   it { should respond_to :short_sha }
 
-  it { should allow_mass_assignment_of(:project_id) }
-  it { should allow_mass_assignment_of(:ref) }
-  it { should allow_mass_assignment_of(:sha) }
-  it { should allow_mass_assignment_of(:before_sha) }
-  it { should allow_mass_assignment_of(:push_data) }
-  it { should allow_mass_assignment_of(:status) }
-  it { should allow_mass_assignment_of(:project_name) }
-
   describe :last_build do
     subject { commit.last_build }
     before do
diff --git a/spec/models/web_hook_spec.rb b/spec/models/web_hook_spec.rb
index 2c3c1c3..0f0f175 100644
--- a/spec/models/web_hook_spec.rb
+++ b/spec/models/web_hook_spec.rb
@@ -16,10 +16,6 @@ describe WebHook do
     it { should belong_to :project }
   end
 
-  describe "Mass assignment" do
-    it { should_not allow_mass_assignment_of(:project_id) }
-  end
-
   describe "Validations" do
     it { should validate_presence_of(:url) }