diff options
28 files changed, 100 insertions, 44 deletions
diff --git a/app/models/event.rb b/app/models/event.rb index dc76b6fd022..76e428adc76 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -1,6 +1,9 @@ class Event < ActiveRecord::Base include PushEvent + attr_accessible :project, :action, :data, :author_id, :project_id, + :target_id, :target_type + default_scope where("author_id IS NOT NULL") Created = 1 diff --git a/app/models/issue.rb b/app/models/issue.rb index 96a54907ca3..e1181b97018 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -2,6 +2,9 @@ class Issue < ActiveRecord::Base include IssueCommonality include Votes + attr_accessible :title, :assignee_id, :closed, :position, :description, + :milestone_id, :label_list, :author_id_of_changes + acts_as_taggable_on :labels belongs_to :milestone diff --git a/app/models/key.rb b/app/models/key.rb index a39a4a16c22..e23447e6aea 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -4,7 +4,7 @@ class Key < ActiveRecord::Base belongs_to :user belongs_to :project - attr_protected :user_id + attr_accessible :key, :title validates :title, presence: true, diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 184ac5fce19..bb7b53face0 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -4,6 +4,9 @@ class MergeRequest < ActiveRecord::Base include IssueCommonality include Votes + attr_accessible :title, :assignee_id, :closed, :target_branch, :source_branch, + :author_id_of_changes + BROKEN_DIFF = "--broken-diff" UNCHECKED = 1 @@ -48,7 +51,8 @@ class MergeRequest < ActiveRecord::Base end def mark_as_unchecked - self.update_attributes(state: UNCHECKED) + self.state = UNCHECKED + self.save end def can_be_merged? diff --git a/app/models/milestone.rb b/app/models/milestone.rb index d416fb630c5..65fa461f2e0 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -13,6 +13,8 @@ # class Milestone < ActiveRecord::Base + attr_accessible :title, :description, :due_date, :closed + belongs_to :project has_many :issues diff --git a/app/models/note.rb b/app/models/note.rb index 34edb94edca..9ac77ef7823 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -2,6 +2,9 @@ require 'carrierwave/orm/activerecord' require 'file_size_validator' class Note < ActiveRecord::Base + attr_accessible :note, :noteable, :noteable_id, :noteable_type, :project_id, + :attachment, :line_code + belongs_to :project belongs_to :noteable, polymorphic: true belongs_to :author, @@ -16,7 +19,6 @@ class Note < ActiveRecord::Base to: :author, prefix: true - attr_protected :author, :author_id attr_accessor :notify attr_accessor :notify_author diff --git a/app/models/project.rb b/app/models/project.rb index 56d5d7910b9..7470bd95c88 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -6,6 +6,9 @@ class Project < ActiveRecord::Base include Authority include Team + attr_accessible :name, :path, :description, :code, :default_branch, :issues_enabled, + :wall_enabled, :merge_requests_enabled, :wiki_enabled + # # Relations # @@ -26,11 +29,6 @@ class Project < ActiveRecord::Base attr_accessor :error_code # - # Protected attributes - # - attr_protected :private_flag, :owner_id - - # # Scopes # scope :public_only, where(private_flag: false) diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb index 7c30f7a0b6d..4ea083c17e3 100644 --- a/app/models/protected_branch.rb +++ b/app/models/protected_branch.rb @@ -1,6 +1,8 @@ class ProtectedBranch < ActiveRecord::Base include GitHost + attr_accessible :name + belongs_to :project validates_presence_of :project_id validates_presence_of :name diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 2c941499f1f..bfd28684f20 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -1,6 +1,8 @@ class Snippet < ActiveRecord::Base include Linguist::BlobHelper + attr_accessible :title, :content, :file_name, :expires_at + belongs_to :project belongs_to :author, class_name: "User" has_many :notes, as: :noteable, dependent: :destroy @@ -9,7 +11,6 @@ class Snippet < ActiveRecord::Base :email, to: :author, prefix: true - attr_protected :author, :author_id, :project, :project_id validates_presence_of :project_id validates_presence_of :author_id @@ -46,11 +47,11 @@ class Snippet < ActiveRecord::Base 0 end - def name + def name file_name end - def mode + def mode nil end diff --git a/app/models/users_project.rb b/app/models/users_project.rb index ce64a10f3f0..c42cc86593c 100644 --- a/app/models/users_project.rb +++ b/app/models/users_project.rb @@ -6,11 +6,11 @@ class UsersProject < ActiveRecord::Base DEVELOPER = 30 MASTER = 40 + attr_accessible :user, :user_id, :project_access + belongs_to :user belongs_to :project - attr_protected :project_id, :project - after_save :update_repository after_destroy :update_repository diff --git a/app/models/web_hook.rb b/app/models/web_hook.rb index 76efa50198b..5d826d2fb6d 100644 --- a/app/models/web_hook.rb +++ b/app/models/web_hook.rb @@ -1,6 +1,8 @@ class WebHook < ActiveRecord::Base include HTTParty + attr_accessible :url + # HTTParty timeout default_timeout 10 @@ -18,11 +20,11 @@ class WebHook < ActiveRecord::Base post_url = url.gsub(parsed_url.userinfo+"@", "") WebHook.post(post_url, body: data.to_json, - headers: { "Content-Type" => "application/json" }, + headers: { "Content-Type" => "application/json" }, basic_auth: {username: parsed_url.user, password: parsed_url.password}) end end - + end # == Schema Information # diff --git a/app/models/wiki.rb b/app/models/wiki.rb index ebb1ff49c7a..b053f5ad412 100644 --- a/app/models/wiki.rb +++ b/app/models/wiki.rb @@ -1,4 +1,6 @@ class Wiki < ActiveRecord::Base + attr_accessible :title, :content, :slug + belongs_to :project belongs_to :user has_many :notes, as: :noteable, dependent: :destroy diff --git a/app/roles/issue_commonality.rb b/app/roles/issue_commonality.rb index ac972a70df2..55b46ec0c89 100644 --- a/app/roles/issue_commonality.rb +++ b/app/roles/issue_commonality.rb @@ -3,8 +3,6 @@ module IssueCommonality extend ActiveSupport::Concern included do - attr_protected :author, :author_id, :project, :project_id - belongs_to :project belongs_to :author, class_name: "User" belongs_to :assignee, class_name: "User" diff --git a/config/application.rb b/config/application.rb index ad41f19657f..27de3fa2436 100644 --- a/config/application.rb +++ b/config/application.rb @@ -39,6 +39,12 @@ module Gitlab # Configure sensitive parameters which will be filtered from the log file. config.filter_parameters += [:password] + # 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 = true + # Enable the asset pipeline config.assets.enabled = true diff --git a/config/environments/development.rb b/config/environments/development.rb index 87b095e27a1..38400d17c8b 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -33,7 +33,7 @@ Gitlab::Application.configure do # Raise exception on mass assignment protection for Active Record models config.active_record.mass_assignment_sanitizer = :strict - + # Log the query plan for queries taking more than this (works # with SQLite, MySQL, and PostgreSQL) config.active_record.auto_explain_threshold_in_seconds = 0.5 diff --git a/config/environments/test.rb b/config/environments/test.rb index 1e7765d9719..f5816e42b7f 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -34,6 +34,9 @@ Gitlab::Application.configure do # like if you have constraints or database-specific column types # config.active_record.schema_format = :sql + # 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/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 34192da94ad..099c41985cb 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -5,6 +5,11 @@ describe Issue do it { should belong_to(:milestone) } end + describe "Mass assignment" do + it { should_not allow_mass_assignment_of(:author_id) } + it { should_not allow_mass_assignment_of(:project_id) } + end + describe "Validation" do it { should ensure_length_of(:description).is_within(0..2000) } it { should ensure_inclusion_of(:closed).in_array([true, false]) } diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 85cd291d681..3ccfdf034de 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -6,6 +6,11 @@ describe Key do it { should belong_to(:project) } end + describe "Mass assignment" do + it { should_not allow_mass_assignment_of(:project_id) } + it { should_not allow_mass_assignment_of(:user_id) } + end + describe "Validation" do it { should validate_presence_of(:title) } it { should validate_presence_of(:key) } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 523e823de34..a54849240ae 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -6,6 +6,11 @@ describe MergeRequest do it { should validate_presence_of(:source_branch) } end + describe "Mass assignment" do + it { should_not allow_mass_assignment_of(:author_id) } + it { should_not allow_mass_assignment_of(:project_id) } + end + describe 'modules' do it { should include_module(IssueCommonality) } it { should include_module(Votes) } diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index f0f0f88303f..9c11a7b1043 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -6,6 +6,10 @@ describe Milestone do it { should have_many(:issues) } end + describe "Mass assignment" do + it { should_not allow_mass_assignment_of(:project_id) } + end + describe "Validation" do it { should validate_presence_of(:title) } it { should validate_presence_of(:project_id) } diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 7809953f5b3..34493a1117d 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -7,6 +7,11 @@ describe Note do it { should belong_to(:author).class_name('User') } end + describe "Mass assignment" do + it { should_not allow_mass_assignment_of(:author) } + it { should_not allow_mass_assignment_of(:author_id) } + end + describe "Validation" do it { should validate_presence_of(:note) } it { should validate_presence_of(:project) } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 756f69ded56..c313b58ecd6 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -17,6 +17,11 @@ describe Project do it { should have_many(:protected_branches).dependent(:destroy) } end + describe "Mass assignment" do + it { should_not allow_mass_assignment_of(:owner_id) } + it { should_not allow_mass_assignment_of(:private_flag) } + end + describe "Validation" do let!(:project) { create(:project) } diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb index 9180bc3bca6..4b2923624dd 100644 --- a/spec/models/protected_branch_spec.rb +++ b/spec/models/protected_branch_spec.rb @@ -5,6 +5,10 @@ describe ProtectedBranch do it { should belong_to(:project) } end + describe "Mass assignment" do + it { should_not allow_mass_assignment_of(:project_id) } + end + describe 'Validation' do it { should validate_presence_of(:project_id) } it { should validate_presence_of(:name) } diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index ffb861c4910..66c36e51ec7 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -7,6 +7,11 @@ describe Snippet do it { should have_many(:notes).dependent(:destroy) } end + describe "Mass assignment" do + it { should_not allow_mass_assignment_of(:author_id) } + it { should_not allow_mass_assignment_of(:project_id) } + end + describe "Validation" do it { should validate_presence_of(:author_id) } it { should validate_presence_of(:project_id) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 14a373e10a5..b77d88783f4 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -15,6 +15,11 @@ describe User do it { should have_many(:assigned_merge_requests).dependent(:destroy) } end + describe "Mass assignment" do + it { should_not allow_mass_assignment_of(:projects_limit) } + it { should allow_mass_assignment_of(:projects_limit).as(:admin) } + end + describe 'validations' do it { should validate_presence_of(:projects_limit) } it { should validate_numericality_of(:projects_limit) } @@ -73,30 +78,4 @@ describe User do user.authentication_token.should_not be_blank end end - - describe "attributes can be changed by a regular user" do - before do - @user = Factory :user - @user.update_attributes(skype: "testskype", linkedin: "testlinkedin") - end - it { @user.skype.should == 'testskype' } - it { @user.linkedin.should == 'testlinkedin' } - end - - describe "attributes that shouldn't be changed by a regular user" do - before do - @user = Factory :user - @user.update_attributes(projects_limit: 50) - end - it { @user.projects_limit.should_not == 50 } - end - - describe "attributes can be changed by an admin user" do - before do - @admin_user = Factory :admin - @admin_user.update_attributes({ skype: "testskype", projects_limit: 50 }, as: :admin) - end - it { @admin_user.skype.should == 'testskype' } - it { @admin_user.projects_limit.should == 50 } - end end diff --git a/spec/models/users_project_spec.rb b/spec/models/users_project_spec.rb index 33cb358e7bd..a13a08db17a 100644 --- a/spec/models/users_project_spec.rb +++ b/spec/models/users_project_spec.rb @@ -6,6 +6,10 @@ describe UsersProject do it { should belong_to(:user) } end + describe "Mass assignment" do + it { should_not allow_mass_assignment_of(:project_id) } + end + describe "Validation" do let!(:users_project) { create(:users_project) } diff --git a/spec/models/web_hook_spec.rb b/spec/models/web_hook_spec.rb index 3cba5b64ff0..422d67cf016 100644 --- a/spec/models/web_hook_spec.rb +++ b/spec/models/web_hook_spec.rb @@ -5,6 +5,10 @@ describe ProjectHook 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) } diff --git a/spec/models/wiki_spec.rb b/spec/models/wiki_spec.rb index de6ce426331..1e27954cb84 100644 --- a/spec/models/wiki_spec.rb +++ b/spec/models/wiki_spec.rb @@ -7,6 +7,11 @@ describe Wiki do it { should have_many(:notes).dependent(:destroy) } end + describe "Mass assignment" do + it { should_not allow_mass_assignment_of(:project_id) } + it { should_not allow_mass_assignment_of(:user_id) } + end + describe "Validation" do it { should validate_presence_of(:title) } it { should ensure_length_of(:title).is_within(1..250) } |