diff --git a/app/models/event.rb b/app/models/event.rb index dc76b6fd022e04169c624e7e4f24813b4ce630fd..76e428adc76967d79b7b6e744fefb2b8de6b64ac 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 96a54907ca3cbd2cc2b4445880f53a8e9674d84f..e1181b97018943c511b29647b8aeef182bd33be3 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 a39a4a16c22dd60edbf6644b86ed1b1be5f4ddc7..e23447e6aeac9bbdcad1ece566beb43329d0bf7d 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 184ac5fce19cbd28c10a23f92e11e9f9b1583dec..bb7b53face03a327aca51912a1a698af36fe6222 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 d416fb630c5db5e345e35cea1baaeada78349ffe..65fa461f2e0791751c9cc2c0f9b52640bb6ecdd3 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 34edb94edca5cf64fafd764fb5587a25b99792fd..9ac77ef78233fb910eb27a80b0bcc6b538a1153e 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 56d5d7910b9827cd1efd43fc13c75c6600d153f8..7470bd95c88f441e77b5c7d0059fa92b98bcb89a 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 # @@ -25,11 +28,6 @@ class Project < ActiveRecord::Base attr_accessor :error_code - # - # Protected attributes - # - attr_protected :private_flag, :owner_id - # # Scopes # diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb index 7c30f7a0b6d4fe9d90a9b5f20e268d55107d83ef..4ea083c17e3c81e457bcc8aa5823e431fe6652fa 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 2c941499f1f2475388fb8eec2a136e1d4cd49355..bfd28684f20862b762a0ca3e4fd391347ab302e0 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 ce64a10f3f07fd8b85b105152d140458e1fe1bf4..c42cc86593ce743bb9fcfb1b0365441332c61925 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 76efa50198b719c3c03f9320910bc85c34fc5646..5d826d2fb6d09d3468b2778129e2bc85658da8a0 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 ebb1ff49c7a07e8f68d625beaf7c76433520c7c2..b053f5ad4128c1aed09b9709bb0f43988a8a649f 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 ac972a70df2ba6204783f1231dd528602fb11729..55b46ec0c898e1fe245643a4321482b708a3ce32 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 ad41f19657f52f737b85650e484fb5896c1d6153..27de3fa2436b3f0e2b865ce2399d74d9826fbd9c 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 87b095e27a1cffa65f674d8eb4cb8d39988906ff..38400d17c8b8eacae26e935cad5c2b846be4a318 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 1e7765d9719cb71f8d22357bd5183798ee673759..f5816e42b7fb68b7f807b0b4f13b64f813544a11 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/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 90bd5d740811ea01851949d39f39a24e5767e680..500cb64df48016a905711a49d02ddb84810c1ba7 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -30,7 +30,7 @@ module Gitlab log.info "#{ldap_prefix}Creating user from #{provider} login"\ " {uid => #{uid}, name => #{name}, email => #{email}}" password = Devise.friendly_token[0, 8].downcase - @user = User.new( + @user = User.new({ extern_uid: uid, provider: provider, name: name, @@ -38,7 +38,7 @@ module Gitlab password: password, password_confirmation: password, projects_limit: Gitlab.config.default_projects_limit, - ) + }, as: :admin) if Gitlab.config.omniauth['block_auto_created_users'] && !ldap @user.blocked = true end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 34192da94ad0bb0d1d1a2ae3cd6da12644daca70..099c41985cbae17f4bb36549caa03a90d7e60096 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 85cd291d6818e425986e0b37b13756e9e5b4cfa6..3ccfdf034de61becc46dbf16d125c054a1a908f9 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 523e823de34358add83f11fa88805e0ceff725fe..a54849240aef7af1409347bc385d26f615210911 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 f0f0f88303f7ca424b3fd3c39fd1f8bf97fa3fce..9c11a7b104383af1f2c2dc0d6e7bb71a977d4b4b 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 7809953f5b32fd8d386e8aa5779f7351df0593a5..34493a1117dc7736ea7b58eeba8734174d2ee479 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 756f69ded56425e01e40de3d8461623cc552aea6..c313b58ecd6aab8a23255e45aabec0c207c8a22a 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 9180bc3bca63d2f0e281c7f6bd21e4353aadf9dd..4b2923624dded4e065ce7ed7ef417fbcb2cdc4e5 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 ffb861c4910c1c59d5b79964e0fa2a8a8b89325f..66c36e51ec762f0c2b80376cc6ac5d455a24ee34 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 14a373e10a529c691d93ab4d908fb6424c330483..b77d88783f458cf7d7dd2d1d4373d95b47940a9e 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 33cb358e7bd5111b428eea5d00236bb6abab57cc..a13a08db17ad47f7815ea457d3b181d49a48ef3c 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 3cba5b64ff0ff4b235885b14fb5b76783d174b2c..422d67cf016ff1b995f90bbec25156a8cf52820c 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 de6ce42633192c0bd0302084175cdee0cd95ff62..1e27954cb84c1d32142b7c1da9a8918598d38f95 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) }