diff options
| author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2014-06-30 17:01:22 +0000 |
|---|---|---|
| committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2014-06-30 17:01:22 +0000 |
| commit | 4f0bfdb500b7f72a8e817d1224eaba6ed46204bc (patch) | |
| tree | 5b1daaaf1a15aa659190288bd3fc23cead46f591 | |
| parent | 76e36dd253229d580f2c6336a77e5fc403fe90c3 (diff) | |
| parent | 4967c087862e5c7c5009605000380d4451ce07ec (diff) | |
| download | gitlab-ce-4f0bfdb500b7f72a8e817d1224eaba6ed46204bc.tar.gz | |
Merge branch 'strong-parameters' into 'master'
Strong parameters
Replace protected_attributes with strong parameters.
Fixes #1340
90 files changed, 265 insertions, 419 deletions
@@ -10,8 +10,6 @@ end gem "rails", "~> 4.1.0" -gem "protected_attributes" - # Make links from text gem 'rails_autolink', '~> 1.1' diff --git a/Gemfile.lock b/Gemfile.lock index 382633c2246..987959d6805 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -331,8 +331,6 @@ GEM websocket-driver (>= 0.2.0) polyglot (0.3.4) posix-spawn (0.3.8) - protected_attributes (1.0.5) - activemodel (>= 4.0.1, < 5.0) pry (0.9.12.4) coderay (~> 1.0) method_source (~> 0.8) @@ -635,7 +633,6 @@ DEPENDENCIES org-ruby pg poltergeist (~> 1.5.1) - protected_attributes pry quiet_assets (~> 1.0.1) rack-attack diff --git a/app/controllers/admin/broadcast_messages_controller.rb b/app/controllers/admin/broadcast_messages_controller.rb index 9a70ef9d199..e1643bb34bf 100644 --- a/app/controllers/admin/broadcast_messages_controller.rb +++ b/app/controllers/admin/broadcast_messages_controller.rb @@ -6,7 +6,7 @@ class Admin::BroadcastMessagesController < Admin::ApplicationController end def create - @broadcast_message = BroadcastMessage.new(params[:broadcast_message]) + @broadcast_message = BroadcastMessage.new(broadcast_message_params) if @broadcast_message.save redirect_to admin_broadcast_messages_path, notice: 'Broadcast Message was successfully created.' @@ -29,4 +29,11 @@ class Admin::BroadcastMessagesController < Admin::ApplicationController def broadcast_messages @broadcast_messages ||= BroadcastMessage.order("starts_at DESC").page(params[:page]) end + + def broadcast_message_params + params.require(:broadcast_message).permit( + :alert_type, :color, :ends_at, :font, + :message, :starts_at + ) + end end diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index 1a523d081dd..0388997ec69 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -20,7 +20,7 @@ class Admin::GroupsController < Admin::ApplicationController end def create - @group = Group.new(params[:group]) + @group = Group.new(group_params) @group.path = @group.name.dup.parameterize if @group.name if @group.save @@ -32,7 +32,7 @@ class Admin::GroupsController < Admin::ApplicationController end def update - if @group.update_attributes(params[:group]) + if @group.update_attributes(group_params) redirect_to [:admin, @group], notice: 'Group was successfully updated.' else render "edit" @@ -56,4 +56,8 @@ class Admin::GroupsController < Admin::ApplicationController def group @group = Group.find_by(path: params[:id]) end + + def group_params + params.require(:group).permit(:name, :description, :path, :avatar) + end end diff --git a/app/controllers/admin/hooks_controller.rb b/app/controllers/admin/hooks_controller.rb index c5bf76f8c39..0a463239d74 100644 --- a/app/controllers/admin/hooks_controller.rb +++ b/app/controllers/admin/hooks_controller.rb @@ -5,7 +5,7 @@ class Admin::HooksController < Admin::ApplicationController end def create - @hook = SystemHook.new(params[:hook]) + @hook = SystemHook.new(hook_params) if @hook.save redirect_to admin_hooks_path, notice: 'Hook was successfully created.' @@ -37,4 +37,8 @@ class Admin::HooksController < Admin::ApplicationController redirect_to :back end + + def hook_params + params.require(:hook).permit(:url) + end end diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index f0040bf5e87..44c93471df4 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -13,7 +13,7 @@ class Admin::UsersController < Admin::ApplicationController end def new - @user = User.build_user + @user = User.new end def edit @@ -37,15 +37,12 @@ class Admin::UsersController < Admin::ApplicationController end def create - admin = params[:user].delete("admin") - opts = { force_random_password: true, password_expires_at: Time.now } - @user = User.build_user(params[:user].merge(opts), as: :admin) - @user.admin = (admin && admin.to_i > 0) + @user = User.new(user_params.merge(opts)) @user.created_by_id = current_user.id @user.generate_password @user.skip_confirmation! @@ -62,19 +59,15 @@ class Admin::UsersController < Admin::ApplicationController end def update - admin = params[:user].delete("admin") - - if params[:user][:password].blank? - params[:user].delete(:password) - params[:user].delete(:password_confirmation) - end - - if admin.present? - user.admin = !admin.to_i.zero? + if params[:user][:password].present? + user_params.merge( + password: params[:user][:password], + password_confirmation: params[:user][:password_confirmation], + ) end respond_to do |format| - if user.update_attributes(params[:user], as: :admin) + if user.update_attributes(user_params) user.confirm! format.html { redirect_to [:admin, user], notice: 'User was successfully updated.' } format.json { head :ok } @@ -115,4 +108,13 @@ class Admin::UsersController < Admin::ApplicationController def user @user ||= User.find_by!(username: params[:id]) end + + def user_params + params.require(:user).permit( + :email, :remember_me, :bio, :name, :username, + :skype, :linkedin, :twitter, :website_url, :color_scheme_id, :theme_id, :force_random_password, + :extern_uid, :provider, :password_expires_at, :avatar, :hide_no_ssh_key, + :projects_limit, :can_create_group, :admin + ) + end end diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index a2629c51384..ddde90d3ee0 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -22,7 +22,7 @@ class GroupsController < ApplicationController end def create - @group = Group.new(params[:group]) + @group = Group.new(group_params) @group.path = @group.name.dup.parameterize if @group.name if @group.save @@ -84,7 +84,7 @@ class GroupsController < ApplicationController end def update - if @group.update_attributes(params[:group]) + if @group.update_attributes(group_params) redirect_to edit_group_path(@group), notice: 'Group was successfully updated.' else render action: "edit" @@ -159,4 +159,8 @@ class GroupsController < ApplicationController params[:state] = 'opened' if params[:state].blank? params[:group_id] = @group.id end + + def group_params + params.require(:group).permit(:name, :description, :path, :avatar) + end end diff --git a/app/controllers/profiles/emails_controller.rb b/app/controllers/profiles/emails_controller.rb index 40c352dab0c..f3f0e69b83a 100644 --- a/app/controllers/profiles/emails_controller.rb +++ b/app/controllers/profiles/emails_controller.rb @@ -7,7 +7,7 @@ class Profiles::EmailsController < ApplicationController end def create - @email = current_user.emails.new(params[:email]) + @email = current_user.emails.new(email_params) flash[:alert] = @email.errors.full_messages.first unless @email.save @@ -23,4 +23,10 @@ class Profiles::EmailsController < ApplicationController format.js { render nothing: true } end end + + private + + def email_params + params.require(:email).permit(:email) + end end diff --git a/app/controllers/profiles/keys_controller.rb b/app/controllers/profiles/keys_controller.rb index 6713cd7c8c7..88414b13564 100644 --- a/app/controllers/profiles/keys_controller.rb +++ b/app/controllers/profiles/keys_controller.rb @@ -15,7 +15,7 @@ class Profiles::KeysController < ApplicationController end def create - @key = current_user.keys.new(params[:key]) + @key = current_user.keys.new(key_params) if @key.save redirect_to profile_key_path(@key) @@ -53,4 +53,9 @@ class Profiles::KeysController < ApplicationController end end + private + + def key_params + params.require(:key).permit(:title, :key) + end end diff --git a/app/controllers/profiles/passwords_controller.rb b/app/controllers/profiles/passwords_controller.rb index df6954554ea..0d93f5cbfdf 100644 --- a/app/controllers/profiles/passwords_controller.rb +++ b/app/controllers/profiles/passwords_controller.rb @@ -11,8 +11,8 @@ class Profiles::PasswordsController < ApplicationController end def create - new_password = params[:user][:password] - new_password_confirmation = params[:user][:password_confirmation] + new_password = user_params[:password] + new_password_confirmation = user_params[:password_confirmation] result = @user.update_attributes( password: new_password, @@ -31,11 +31,11 @@ class Profiles::PasswordsController < ApplicationController end def update - password_attributes = params[:user].select do |key, value| + password_attributes = user_params.select do |key, value| %w(password password_confirmation).include?(key.to_s) end - unless @user.valid_password?(params[:user][:current_password]) + unless @user.valid_password?(user_params[:current_password]) redirect_to edit_profile_password_path, alert: 'You must provide a valid current password' return end @@ -74,4 +74,8 @@ class Profiles::PasswordsController < ApplicationController def authorize_change_password! return render_404 if @user.ldap_user? end + + def user_params + params.require(:user).permit(:current_password, :password, :password_confirmation) + end end diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index 9c9a129b26b..e877f9b9049 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -14,9 +14,9 @@ class ProfilesController < ApplicationController end def update - params[:user].delete(:email) if @user.ldap_user? + user_params.except!(:email) if @user.ldap_user? - if @user.update_attributes(params[:user]) + if @user.update_attributes(user_params) flash[:notice] = "Profile was successfully updated" else flash[:alert] = "Failed to update profile" @@ -41,7 +41,7 @@ class ProfilesController < ApplicationController end def update_username - @user.update_attributes(username: params[:user][:username]) + @user.update_attributes(username: user_params[:username]) respond_to do |format| format.js @@ -57,4 +57,12 @@ class ProfilesController < ApplicationController def authorize_change_username! return render_404 unless @user.can_change_username? end + + def user_params + params.require(:user).permit( + :email, :password, :password_confirmation, :bio, :name, :username, + :skype, :linkedin, :twitter, :website_url, :color_scheme_id, :theme_id, + :avatar, :hide_no_ssh_key, + ) + end end diff --git a/app/controllers/projects/deploy_keys_controller.rb b/app/controllers/projects/deploy_keys_controller.rb index 6e1a76ff417..d20937ea8ea 100644 --- a/app/controllers/projects/deploy_keys_controller.rb +++ b/app/controllers/projects/deploy_keys_controller.rb @@ -22,7 +22,7 @@ class Projects::DeployKeysController < Projects::ApplicationController end def create - @key = DeployKey.new(params[:deploy_key]) + @key = DeployKey.new(deploy_key_params) if @key.valid? && @project.deploy_keys << @key redirect_to project_deploy_keys_path(@project) @@ -58,4 +58,8 @@ class Projects::DeployKeysController < Projects::ApplicationController def available_keys @available_keys ||= current_user.accessible_deploy_keys end + + def deploy_key_params + params.require(:deploy_key).permit(:key, :title) + end end diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb index c43d26385f7..268e19f26ee 100644 --- a/app/controllers/projects/hooks_controller.rb +++ b/app/controllers/projects/hooks_controller.rb @@ -12,7 +12,7 @@ class Projects::HooksController < Projects::ApplicationController end def create - @hook = @project.hooks.new(params[:hook]) + @hook = @project.hooks.new(hook_params) @hook.save if @hook.valid? @@ -40,4 +40,8 @@ class Projects::HooksController < Projects::ApplicationController def hook @hook ||= @project.hooks.find(params[:id]) end + + def hook_params + params.require(:hook).permit(:url, :push_events, :issues_events, :merge_requests_events, :tag_push_events) + end end diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index ffe65cb41c5..bf05845effe 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -42,7 +42,7 @@ class Projects::IssuesController < Projects::ApplicationController end def new - @issue = @project.issues.new(params[:issue]) + @issue = @project.issues.new(issue_params) respond_with(@issue) end @@ -59,7 +59,7 @@ class Projects::IssuesController < Projects::ApplicationController end def create - @issue = Issues::CreateService.new(project, current_user, params[:issue]).execute + @issue = Issues::CreateService.new(project, current_user, issue_params).execute respond_to do |format| format.html do @@ -76,7 +76,7 @@ class Projects::IssuesController < Projects::ApplicationController end def update - @issue = Issues::UpdateService.new(project, current_user, params[:issue]).execute(issue) + @issue = Issues::UpdateService.new(project, current_user, issue_params).execute(issue) respond_to do |format| format.js @@ -144,4 +144,11 @@ class Projects::IssuesController < Projects::ApplicationController raise ActiveRecord::RecordNotFound.new end end + + def issue_params + params.require(:issue).permit( + :title, :assignee_id, :position, :description, + :milestone_id, :label_list, :state_event + ) + end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 89f4ab01a3f..4d8429dd554 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -60,7 +60,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def new - @merge_request = MergeRequest.new(params[:merge_request]) + params[:merge_request] ||= ActionController::Parameters.new( + source_project: @project + ) + + @merge_request = MergeRequest.new(merge_request_params) @merge_request.source_project = @project unless @merge_request.source_project @merge_request.target_project ||= (@project.forked_from_project || @project) @target_branches = @merge_request.target_project.nil? ? [] : @merge_request.target_project.repository.branch_names @@ -110,7 +114,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController def create @target_branches ||= [] - @merge_request = MergeRequests::CreateService.new(project, current_user, params[:merge_request]).execute + @merge_request = MergeRequests::CreateService.new(project, current_user, merge_request_params).execute if @merge_request.valid? redirect_to project_merge_request_path(@merge_request.target_project, @merge_request), notice: 'Merge request was successfully created.' @@ -122,7 +126,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def update - @merge_request = MergeRequests::UpdateService.new(project, current_user, params[:merge_request]).execute(@merge_request) + @merge_request = MergeRequests::UpdateService.new(project, current_user, merge_request_params).execute(@merge_request) if @merge_request.valid? respond_to do |format| @@ -263,4 +267,12 @@ class Projects::MergeRequestsController < Projects::ApplicationController can?(current_user, action, project) end + + def merge_request_params + params.require(:merge_request).permit( + :title, :assignee_id, :source_project_id, :source_branch, + :target_project_id, :target_branch, :milestone_id, + :state_event, :description, :label_list + ) + end end diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb index c38c77d6b85..d338cdedfaf 100644 --- a/app/controllers/projects/milestones_controller.rb +++ b/app/controllers/projects/milestones_controller.rb @@ -37,7 +37,7 @@ class Projects::MilestonesController < Projects::ApplicationController end def create - @milestone = Milestones::CreateService.new(project, current_user, params[:milestone]).execute + @milestone = Milestones::CreateService.new(project, current_user, milestone_params).execute if @milestone.save redirect_to project_milestone_path(@project, @milestone) @@ -47,7 +47,7 @@ class Projects::MilestonesController < Projects::ApplicationController end def update - @milestone = Milestones::UpdateService.new(project, current_user, params[:milestone]).execute(milestone) + @milestone = Milestones::UpdateService.new(project, current_user, milestone_params).execute(milestone) respond_to do |format| format.js @@ -105,4 +105,8 @@ class Projects::MilestonesController < Projects::ApplicationController def module_enabled return render_404 unless @project.issues_enabled end + + def milestone_params + params.require(:milestone).permit(:title, :description, :due_date, :state_event) + end end diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 66cc1a3dec7..2154b6ed2eb 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -21,7 +21,7 @@ class Projects::NotesController < Projects::ApplicationController end def create - @note = Notes::CreateService.new(project, current_user, params[:note]).execute + @note = Notes::CreateService.new(project, current_user, note_params).execute respond_to do |format| format.json { render_note_json(@note) } @@ -30,7 +30,7 @@ class Projects::NotesController < Projects::ApplicationController end def update - note.update_attributes(params[:note]) + note.update_attributes(note_params) note.reset_events_cache respond_to do |format| @@ -109,4 +109,11 @@ class Projects::NotesController < Projects::ApplicationController def authorize_admin_note! return access_denied! unless can?(current_user, :admin_note, note) end + + def note_params + params.require(:note).permit( + :note, :noteable, :noteable_id, :noteable_type, :project_id, + :attachment, :line_code, :commit_id + ) + end end diff --git a/app/controllers/projects/protected_branches_controller.rb b/app/controllers/projects/protected_branches_controller.rb index e39e97af8dd..bd31b1d3c54 100644 --- a/app/controllers/projects/protected_branches_controller.rb +++ b/app/controllers/projects/protected_branches_controller.rb @@ -11,7 +11,7 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController end def create - @project.protected_branches.create(params[:protected_branch]) + @project.protected_branches.create(protected_branch_params) redirect_to project_protected_branches_path(@project) end @@ -23,4 +23,10 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController format.js { render nothing: true } end end + + private + + def protected_branch_params + params.require(:protected_branch).permit(:name) + end end diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb index 6db22186c14..b143dec3a93 100644 --- a/app/controllers/projects/services_controller.rb +++ b/app/controllers/projects/services_controller.rb @@ -16,7 +16,7 @@ class Projects::ServicesController < Projects::ApplicationController end def update - if @service.update_attributes(params[:service]) + if @service.update_attributes(service_params) redirect_to edit_project_service_path(@project, @service.to_param) else render 'edit' @@ -36,4 +36,11 @@ class Projects::ServicesController < Projects::ApplicationController def service @service ||= @project.services.find { |service| service.to_param == params[:id] } end + + def service_params + params.require(:service).permit( + :title, :token, :type, :active, :api_key, :subdomain, + :room, :recipients, :project_url + ) + end end diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb index f93f2d5f9bb..25026973118 100644 --- a/app/controllers/projects/snippets_controller.rb +++ b/app/controllers/projects/snippets_controller.rb @@ -25,7 +25,7 @@ class Projects::SnippetsController < Projects::ApplicationController end def create - @snippet = @project.snippets.build(params[:project_snippet]) + @snippet = @project.snippets.build(snippet_params) @snippet.author = current_user if @snippet.save @@ -39,7 +39,7 @@ class Projects::SnippetsController < Projects::ApplicationController end def update - if @snippet.update_attributes(params[:project_snippet]) + if @snippet.update_attributes(snippet_params) redirect_to project_snippet_path(@project, @snippet) else respond_with(@snippet) @@ -86,4 +86,8 @@ class Projects::SnippetsController < Projects::ApplicationController def module_enabled return render_404 unless @project.snippets_enabled end + + def snippet_params + params.require(:project_snippet).permit(:title, :content, :file_name, :private) + end end diff --git a/app/controllers/projects/team_members_controller.rb b/app/controllers/projects/team_members_controller.rb index 44068878cd1..1de5bac9ee8 100644 --- a/app/controllers/projects/team_members_controller.rb +++ b/app/controllers/projects/team_members_controller.rb @@ -27,7 +27,7 @@ class Projects::TeamMembersController < Projects::ApplicationController def update @user_project_relation = project.users_projects.find_by(user_id: member) - @user_project_relation.update_attributes(params[:team_member]) + @user_project_relation.update_attributes(member_params) unless @user_project_relation.valid? flash[:alert] = "User should have at least one role" @@ -67,4 +67,8 @@ class Projects::TeamMembersController < Projects::ApplicationController def member @member ||= User.find_by(username: params[:id]) end + + def member_params + params.require(:team_member).permit(:user_id, :project_access) + end end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 0d15b458b70..597efa40ded 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -20,7 +20,7 @@ class ProjectsController < ApplicationController end def create - @project = ::Projects::CreateService.new(current_user, params[:project]).execute + @project = ::Projects::CreateService.new(current_user, project_params).execute flash[:notice] = 'Project was successfully created.' if @project.saved? respond_to do |format| @@ -29,7 +29,7 @@ class ProjectsController < ApplicationController end def update - status = ::Projects::UpdateService.new(@project, current_user, params).execute + status = ::Projects::UpdateService.new(@project, current_user, project_params).execute respond_to do |format| if status @@ -44,7 +44,7 @@ class ProjectsController < ApplicationController end def transfer - ::Projects::TransferService.new(project, current_user, params[:project]).execute + ::Projects::TransferService.new(project, current_user, project_params).execute end def show @@ -85,7 +85,7 @@ class ProjectsController < ApplicationController redirect_to import_project_path(@project) end - @project.import_url = params[:project][:import_url] + @project.import_url = project_params[:import_url] if @project.save @project.reload @@ -185,4 +185,12 @@ class ProjectsController < ApplicationController def user_layout current_user ? "projects" : "public_projects" end + + def project_params + params.require(:project).permit( + :name, :path, :description, :issues_tracker, :label_list, + :issues_enabled, :merge_requests_enabled, :snippets_enabled, :issues_tracker_id, + :wiki_enabled, :visibility_level, :import_url, :last_activity_at, :namespace_id + ) + end end diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 5f18bac82ed..bf4c217fee1 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -13,7 +13,6 @@ class RegistrationsController < Devise::RegistrationsController def build_resource(hash=nil) super - self.resource.with_defaults end private diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index 4fe98f804dc..e75db61e680 100644 --- a/app/controllers/snippets_controller.rb +++ b/app/controllers/snippets_controller.rb @@ -51,7 +51,7 @@ class SnippetsController < ApplicationController end def create - @snippet = PersonalSnippet.new(params[:personal_snippet]) + @snippet = PersonalSnippet.new(snippet_params) @snippet.author = current_user if @snippet.save @@ -65,7 +65,7 @@ class SnippetsController < ApplicationController end def update - if @snippet.update_attributes(params[:personal_snippet]) + if @snippet.update_attributes(snippet_params) redirect_to snippet_path(@snippet) else respond_with @snippet @@ -109,4 +109,8 @@ class SnippetsController < ApplicationController def set_title @title = 'Snippets' end + + def snippet_params + params.require(:personal_snippet).permit(:title, :content, :file_name, :private) + end end diff --git a/app/controllers/users_groups_controller.rb b/app/controllers/users_groups_controller.rb index b9bdc189522..a35a12a866b 100644 --- a/app/controllers/users_groups_controller.rb +++ b/app/controllers/users_groups_controller.rb @@ -14,7 +14,7 @@ class UsersGroupsController < ApplicationController def update @member = @group.users_groups.find(params[:id]) - @member.update_attributes(params[:users_group]) + @member.update_attributes(member_params) end def destroy @@ -41,4 +41,8 @@ class UsersGroupsController < ApplicationController return render_404 end end + + def member_params + params.require(:users_group).permit(:group_access, :user_id) + end end diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb index ce8b7973cd9..4d0c04bcc3d 100644 --- a/app/models/broadcast_message.rb +++ b/app/models/broadcast_message.rb @@ -14,8 +14,6 @@ # class BroadcastMessage < ActiveRecord::Base - attr_accessible :alert_type, :color, :ends_at, :font, :message, :starts_at - validates :message, presence: true validates :starts_at, presence: true validates :ends_at, presence: true diff --git a/app/models/deploy_keys_project.rb b/app/models/deploy_keys_project.rb index 739d749830a..f23d8205ddc 100644 --- a/app/models/deploy_keys_project.rb +++ b/app/models/deploy_keys_project.rb @@ -10,13 +10,10 @@ # class DeployKeysProject < ActiveRecord::Base - attr_accessible :key_id, :project_id - belongs_to :project belongs_to :deploy_key validates :deploy_key_id, presence: true validates :deploy_key_id, uniqueness: { scope: [:project_id], message: "already exists in project" } - validates :project_id, presence: true end diff --git a/app/models/email.rb b/app/models/email.rb index 9068c2b87b6..57f476bd519 100644 --- a/app/models/email.rb +++ b/app/models/email.rb @@ -10,16 +10,8 @@ # class Email < ActiveRecord::Base - attr_accessible :email, :user_id - - # - # Relations - # belongs_to :user - # - # Validations - # validates :user_id, presence: true validates :email, presence: true, email: { strict_mode: true }, uniqueness: true validate :unique_email, if: ->(email) { email.email_changed? } diff --git a/app/models/event.rb b/app/models/event.rb index 1a8d55c54b4..c7e93825f97 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -15,9 +15,6 @@ # class Event < ActiveRecord::Base - attr_accessible :project, :action, :data, :author_id, :project_id, - :target_id, :target_type - default_scope { where.not(author_id: nil) } CREATED = 1 diff --git a/app/models/forked_project_link.rb b/app/models/forked_project_link.rb index 17add270f67..9b0c6263a96 100644 --- a/app/models/forked_project_link.rb +++ b/app/models/forked_project_link.rb @@ -10,10 +10,6 @@ # class ForkedProjectLink < ActiveRecord::Base - attr_accessible :forked_from_project_id, :forked_to_project_id - - # Relations belongs_to :forked_to_project, class_name: Project belongs_to :forked_from_project, class_name: Project - end diff --git a/app/models/group.rb b/app/models/group.rb index e51e19ab60c..3a5c5e11354 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -20,8 +20,6 @@ class Group < Namespace has_many :users_groups, dependent: :destroy has_many :users, through: :users_groups - attr_accessible :avatar - validate :avatar_type, if: ->(user) { user.avatar_changed? } validates :avatar, file_size: { maximum: 100.kilobytes.to_i } diff --git a/app/models/issue.rb b/app/models/issue.rb index f0c2e552273..a116a9354cb 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -33,9 +33,6 @@ class Issue < ActiveRecord::Base scope :of_group, ->(group) { where(project_id: group.project_ids) } scope :of_user_team, ->(team) { where(project_id: team.project_ids, assignee_id: team.member_ids) } - attr_accessible :title, :assignee_id, :position, :description, - :milestone_id, :label_list, :state_event - acts_as_taggable_on :labels scope :cared, ->(user) { where(assignee_id: user) } diff --git a/app/models/key.rb b/app/models/key.rb index 035c9efa016..d59993b1905 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -19,8 +19,6 @@ class Key < ActiveRecord::Base belongs_to :user - attr_accessible :key, :title - before_validation :strip_white_space, :generate_fingerpint validates :title, presence: true, length: { within: 0..255 } diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index a4b939f1140..676a57fa3d5 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -36,10 +36,6 @@ class MergeRequest < ActiveRecord::Base delegate :commits, :diffs, :last_commit, :last_commit_short_sha, to: :merge_request_diff, prefix: nil - attr_accessible :title, :assignee_id, :source_project_id, :source_branch, - :target_project_id, :target_branch, :milestone_id, - :state_event, :description, :label_list - attr_accessor :should_remove_source_branch # When this attribute is true some MR validation is ignored diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 7dce71a677b..d3c07555b0c 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -22,8 +22,6 @@ class MergeRequestDiff < ActiveRecord::Base belongs_to :merge_request - attr_accessible :state, :st_commits, :st_diffs - delegate :target_branch, :source_branch, to: :merge_request, prefix: nil state_machine :state, initial: :empty do diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 39ab0b536a3..8fd3e56d2ee 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -16,8 +16,6 @@ class Milestone < ActiveRecord::Base include InternalId - attr_accessible :title, :description, :due_date, :state_event - belongs_to :project has_many :issues has_many :merge_requests diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 446e5f04c63..cd58710825d 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -16,8 +16,6 @@ class Namespace < ActiveRecord::Base include Gitlab::ShellAdapter - attr_accessible :name, :description, :path - has_many :projects, dependent: :destroy belongs_to :owner, class_name: "User" diff --git a/app/models/note.rb b/app/models/note.rb index 94d45aa43db..ed4829b2b39 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -25,8 +25,6 @@ class Note < ActiveRecord::Base default_value_for :system, false - attr_accessible :note, :noteable, :noteable_id, :noteable_type, :project_id, - :attachment, :line_code, :commit_id attr_mentionable :note belongs_to :project @@ -63,13 +61,13 @@ class Note < ActiveRecord::Base def create_status_change_note(noteable, project, author, status, source) body = "_Status changed to #{status}#{' by ' + source.gfm_reference if source}_" - create({ + create( noteable: noteable, project: project, author: author, note: body, system: true - }, without_protection: true) + ) end # +noteable+ was referenced from +mentioner+, by including GFM in either +mentioner+'s description or an associated Note. @@ -88,7 +86,7 @@ class Note < ActiveRecord::Base note_options.merge!(noteable: noteable) end - create(note_options, without_protection: true) + create(note_options) end def create_milestone_change_note(noteable, project, author, milestone) @@ -98,13 +96,13 @@ class Note < ActiveRecord::Base "_Milestone changed to #{milestone.title}_" end - create({ + create( noteable: noteable, project: project, author: author, note: body, system: true - }, without_protection: true) + ) end def create_assignee_change_note(noteable, project, author, assignee) @@ -116,7 +114,7 @@ class Note < ActiveRecord::Base author: author, note: body, system: true - }, without_protection: true) + }) end def discussions_from_notes(notes) diff --git a/app/models/project.rb b/app/models/project.rb index 762b540b7a3..33aa4e72fbc 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -27,23 +27,20 @@ class Project < ActiveRecord::Base include Gitlab::ShellAdapter include Gitlab::VisibilityLevel + include Gitlab::ConfigHelper + extend Gitlab::ConfigHelper extend Enumerize default_value_for :archived, false - default_value_for :issues_enabled, true - default_value_for :merge_requests_enabled, true - default_value_for :wiki_enabled, true + default_value_for :visibility_level, gitlab_config_features.visibility_level + default_value_for :issues_enabled, gitlab_config_features.issues + default_value_for :merge_requests_enabled, gitlab_config_features.merge_requests + default_value_for :wiki_enabled, gitlab_config_features.wiki default_value_for :wall_enabled, false - default_value_for :snippets_enabled, true + default_value_for :snippets_enabled, gitlab_config_features.snippets ActsAsTaggableOn.strict_case_match = true - attr_accessible :name, :path, :description, :issues_tracker, :label_list, - :issues_enabled, :merge_requests_enabled, :snippets_enabled, :issues_tracker_id, - :wiki_enabled, :visibility_level, :import_url, :last_activity_at, as: [:default, :admin] - - attr_accessible :namespace_id, :creator_id, as: :admin - acts_as_taggable_on :labels, :issues_default_labels attr_accessor :new_default_branch @@ -100,6 +97,9 @@ class Project < ActiveRecord::Base message: "only letters, digits & '_' '-' '.' allowed. Letter or digit should be first" } validates :issues_enabled, :merge_requests_enabled, :wiki_enabled, inclusion: { in: [true, false] } + validates :visibility_level, + exclusion: { in: gitlab_config.restricted_visibility_levels }, + if: -> { gitlab_config.restricted_visibility_levels.any? } validates :issues_tracker_id, length: { maximum: 255 }, allow_blank: true validates :namespace, presence: true validates_uniqueness_of :name, scope: :namespace_id @@ -255,7 +255,7 @@ class Project < ActiveRecord::Base end def web_url - [Gitlab.config.gitlab.url, path_with_namespace].join("/") + [gitlab_config.url, path_with_namespace].join("/") end def web_url_without_protocol @@ -476,7 +476,7 @@ class Project < ActiveRecord::Base end def http_url_to_repo - [Gitlab.config.gitlab.url, "/", path_with_namespace, ".git"].join('') + [gitlab_config.url, "/", path_with_namespace, ".git"].join('') end # Check if current branch name is marked as protected in the system diff --git a/app/models/project_hook.rb b/app/models/project_hook.rb index 6db6767a88d..21867a9316c 100644 --- a/app/models/project_hook.rb +++ b/app/models/project_hook.rb @@ -18,8 +18,6 @@ class ProjectHook < WebHook belongs_to :project - attr_accessible :push_events, :issues_events, :merge_requests_events, :tag_push_events - scope :push_hooks, -> { where(push_events: true) } scope :tag_push_hooks, -> { where(tag_push_events: true) } scope :issue_hooks, -> { where(issues_events: true) } diff --git a/app/models/project_services/assembla_service.rb b/app/models/project_services/assembla_service.rb index 06e9d6118d2..9a8cbb32ac1 100644 --- a/app/models/project_services/assembla_service.rb +++ b/app/models/project_services/assembla_service.rb @@ -18,8 +18,6 @@ # class AssemblaService < Service - attr_accessible :subdomain - include HTTParty validates :token, presence: true, if: :activated? diff --git a/app/models/project_services/campfire_service.rb b/app/models/project_services/campfire_service.rb index 19030ecffa2..83e1bac1ef2 100644 --- a/app/models/project_services/campfire_service.rb +++ b/app/models/project_services/campfire_service.rb @@ -18,8 +18,6 @@ # class CampfireService < Service - attr_accessible :subdomain, :room - validates :token, presence: true, if: :activated? def title diff --git a/app/models/project_services/emails_on_push_service.rb b/app/models/project_services/emails_on_push_service.rb index 04775c4f2b2..be5bab4ec32 100644 --- a/app/models/project_services/emails_on_push_service.rb +++ b/app/models/project_services/emails_on_push_service.rb @@ -18,8 +18,6 @@ # class EmailsOnPushService < Service - attr_accessible :recipients - validates :recipients, presence: true, if: :activated? def title diff --git a/app/models/project_services/gitlab_ci_service.rb b/app/models/project_services/gitlab_ci_service.rb index ef395e0ec68..58ddce45288 100644 --- a/app/models/project_services/gitlab_ci_service.rb +++ b/app/models/project_services/gitlab_ci_service.rb @@ -18,8 +18,6 @@ # class GitlabCiService < CiService - attr_accessible :project_url - validates :project_url, presence: true, if: :activated? validates :token, presence: true, if: :activated? diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index d62f61856d1..9c6fe7dab21 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -18,8 +18,6 @@ # class HipchatService < Service - attr_accessible :room - validates :token, presence: true, if: :activated? def title diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index 50fd62def1d..7e54188abf7 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -18,9 +18,6 @@ # class SlackService < Service - attr_accessible :room - attr_accessible :subdomain - validates :room, presence: true, if: :activated? validates :subdomain, presence: true, if: :activated? validates :token, presence: true, if: :activated? diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb index d2b2b1218d1..1b06dd77523 100644 --- a/app/models/protected_branch.rb +++ b/app/models/protected_branch.rb @@ -12,8 +12,6 @@ class ProtectedBranch < ActiveRecord::Base include Gitlab::ShellAdapter - attr_accessible :name - belongs_to :project validates :name, presence: true validates :project, presence: true diff --git a/app/models/service.rb b/app/models/service.rb index d655937079d..0dc6d514b46 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -22,8 +22,6 @@ class Service < ActiveRecord::Base default_value_for :active, false - attr_accessible :title, :token, :type, :active, :api_key - belongs_to :project has_one :service_hook diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 9e4409daa1a..2c38e7939bd 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -18,8 +18,6 @@ class Snippet < ActiveRecord::Base include Linguist::BlobHelper - attr_accessible :title, :content, :file_name, :expires_at, :private - default_value_for :private, true belongs_to :author, class_name: "User" diff --git a/app/models/user.rb b/app/models/user.rb index 63d819a0f36..5fca392d350 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -50,31 +50,24 @@ require 'carrierwave/orm/activerecord' require 'file_size_validator' class User < ActiveRecord::Base + include Gitlab::ConfigHelper + extend Gitlab::ConfigHelper + default_value_for :admin, false - default_value_for :can_create_group, true + default_value_for :can_create_group, gitlab_config.default_can_create_group default_value_for :can_create_team, false default_value_for :hide_no_ssh_key, false + default_value_for :projects_limit, gitlab_config.default_projects_limit + default_value_for :theme_id, gitlab_config.default_theme devise :database_authenticatable, :token_authenticatable, :lockable, :async, :recoverable, :rememberable, :trackable, :validatable, :omniauthable, :confirmable, :registerable - attr_accessible :email, :password, :password_confirmation, :remember_me, :bio, :name, :username, - :skype, :linkedin, :twitter, :website_url, :color_scheme_id, :theme_id, :force_random_password, - :extern_uid, :provider, :password_expires_at, :avatar, :hide_no_ssh_key, - as: [:default, :admin] - - attr_accessible :projects_limit, :can_create_group, - as: :admin - attr_accessor :force_random_password # Virtual attribute for authenticating by either username or email attr_accessor :login - # Add login to attr_accessible - attr_accessible :login - - # # Relations # @@ -223,20 +216,8 @@ class User < ActiveRecord::Base where('users.username = ? OR users.id = ?', name_or_id.to_s, name_or_id.to_i).first end - def build_user(attrs = {}, options= {}) - if options[:as] == :admin - User.new(defaults.merge(attrs.symbolize_keys), options) - else - User.new(attrs, options).with_defaults - end - end - - def defaults - { - projects_limit: Gitlab.config.gitlab.default_projects_limit, - can_create_group: Gitlab.config.gitlab.default_can_create_group, - theme_id: Gitlab.config.gitlab.default_theme - } + def build_user(attrs = {}) + User.new(attrs) end end @@ -314,7 +295,7 @@ class User < ActiveRecord::Base end def can_change_username? - Gitlab.config.gitlab.username_changing_enabled + gitlab_config.username_changing_enabled end def can_create_project? @@ -489,7 +470,7 @@ class User < ActiveRecord::Base def avatar_url(size = nil) if avatar.present? - URI::join(Gitlab.config.gitlab.url, avatar.url).to_s + URI::join(gitlab_config.url, avatar.url).to_s else GravatarService.new.execute(email, size) end diff --git a/app/models/users_group.rb b/app/models/users_group.rb index 242c8abb3ca..270f968ef61 100644 --- a/app/models/users_group.rb +++ b/app/models/users_group.rb @@ -19,8 +19,6 @@ class UsersGroup < ActiveRecord::Base Gitlab::Access.options_with_owner end - attr_accessible :group_access, :user_id - belongs_to :user belongs_to :group diff --git a/app/models/users_project.rb b/app/models/users_project.rb index 6495bed4e61..69b2d71b436 100644 --- a/app/models/users_project.rb +++ b/app/models/users_project.rb @@ -16,8 +16,6 @@ class UsersProject < ActiveRecord::Base include Notifiable include Gitlab::Access - attr_accessible :user, :user_id, :project_access - belongs_to :user belongs_to :project diff --git a/app/models/web_hook.rb b/app/models/web_hook.rb index 76854da5c38..6cf0c1f683e 100644 --- a/app/models/web_hook.rb +++ b/app/models/web_hook.rb @@ -22,8 +22,6 @@ class WebHook < ActiveRecord::Base default_value_for :issues_events, false default_value_for :merge_requests_events, false - attr_accessible :url - # HTTParty timeout default_timeout 10 diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 169e1e95b4b..a0e57144435 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -1,7 +1,7 @@ module Issues class UpdateService < Issues::BaseService def execute(issue) - state = params.delete('state_event') || params.delete(:state_event) + state = params[:state_event] case state when 'reopen' @@ -10,7 +10,7 @@ module Issues Issues::CloseService.new(project, current_user, {}).execute(issue) end - if params.present? && issue.update_attributes(params) + if params.present? && issue.update_attributes(params.except(:state_event)) issue.reset_events_cache if issue.previous_changes.include?('milestone_id') diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index f1aa8b73930..6e416a0080c 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -7,10 +7,10 @@ module MergeRequests def execute(merge_request) # We dont allow change of source/target projects # after merge request was created - params.delete(:source_project_id) - params.delete(:target_project_id) + params.except!(:source_project_id) + params.except!(:target_project_id) - state = params.delete('state_event') || params.delete(:state_event) + state = params[:state_event] case state when 'reopen' @@ -19,7 +19,7 @@ module MergeRequests MergeRequests::CloseService.new(project, current_user, {}).execute(merge_request) end - if params.present? && merge_request.update_attributes(params) + if params.present? && merge_request.update_attributes(params.except(:state_event)) merge_request.reset_events_cache if merge_request.previous_changes.include?('milestone_id') diff --git a/app/services/milestones/update_service.rb b/app/services/milestones/update_service.rb index 307e96a2b36..ed64847f429 100644 --- a/app/services/milestones/update_service.rb +++ b/app/services/milestones/update_service.rb @@ -1,7 +1,7 @@ module Milestones class UpdateService < Milestones::BaseService def execute(milestone) - state = params.delete('state_event') || params.delete(:state_event) + state = params[:state_event] case state when 'activate' @@ -11,7 +11,7 @@ module Milestones end if params.present? - milestone.update_attributes(params) + milestone.update_attributes(params.except(:state_event)) end milestone diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index dfadcfd296a..3565e4e4f70 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -5,27 +5,13 @@ module Projects end def execute - # get namespace id - namespace_id = params.delete(:namespace_id) + @project = Project.new(params) - # check that user is allowed to set specified visibility_level + # Reset visibility levet if is not allowed to set it unless Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level]) - params.delete(:visibility_level) + @project.visibility_level = default_features.visibility_level end - # Load default feature settings - default_features = Gitlab.config.gitlab.default_projects_features - - default_opts = { - issues_enabled: default_features.issues, - wiki_enabled: default_features.wiki, - snippets_enabled: default_features.snippets, - merge_requests_enabled: default_features.merge_requests, - visibility_level: default_features.visibility_level - }.stringify_keys - - @project = Project.new(default_opts.merge(params)) - # Parametrize path for project # # Ex. @@ -33,13 +19,14 @@ module Projects # @project.path = @project.name.dup.parameterize unless @project.path.present? + # get namespace id + namespace_id = params[:namespace_id] if namespace_id # Find matching namespace and check if it allowed # for current user if namespace_id passed. - if allowed_namespace?(current_user, namespace_id) - @project.namespace_id = namespace_id - else + unless allowed_namespace?(current_user, namespace_id) + @project.namespace_id = nil deny_namespace return @project end diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index d115e92a105..e39fe882cb1 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -12,7 +12,7 @@ module Projects class TransferError < StandardError; end def execute - namespace_id = params.delete(:namespace_id) + namespace_id = params[:namespace_id] namespace = Namespace.find_by(id: namespace_id) if allowed_transfer?(current_user, project, namespace) diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 551a3653cad..36877a61679 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -1,23 +1,18 @@ module Projects class UpdateService < BaseService - def execute(role = :default) - params[:project].delete(:namespace_id) + def execute # check that user is allowed to set specified visibility_level - unless can?(current_user, :change_visibility_level, project) && Gitlab::VisibilityLevel.allowed_for?(current_user, params[:project][:visibility_level]) - params[:project].delete(:visibility_level) + unless can?(current_user, :change_visibility_level, project) && Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level]) + params[:visibility_level] = project.visibility_level end - new_branch = params[:project].delete(:default_branch) + new_branch = params[:default_branch] if project.repository.exists? && new_branch && new_branch != project.default_branch project.change_head(new_branch) end - if project.update_attributes(params[:project], as: role) - if project.previous_changes.include?('namespace_id') - project.send_move_instructions - end - + if project.update_attributes(params.except(:default_branch)) if project.previous_changes.include?('path') project.rename_repo end diff --git a/config/application.rb b/config/application.rb index 0a77f58f6d1..58a5949c653 100644 --- a/config/application.rb +++ b/config/application.rb @@ -41,12 +41,6 @@ module Gitlab # 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 = true - # Enable the asset pipeline config.assets.enabled = true config.assets.paths << Emoji.images_path diff --git a/config/environments/development.rb b/config/environments/development.rb index e4c7649fda0..356e26bd68c 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -19,9 +19,6 @@ Gitlab::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 3860dc5c74c..25b082b98da 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -26,9 +26,6 @@ Gitlab::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/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index f55e69c08f6..0480ec8ecfd 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -150,6 +150,6 @@ Settings['extra'] ||= Settingslogic.new({}) # if Rails.env.test? Settings.gitlab['default_projects_limit'] = 42 - Settings.gitlab['default_can_create_group'] = false + Settings.gitlab['default_can_create_group'] = true Settings.gitlab['default_can_create_team'] = false end diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb index 40362fee0bc..ddb87daeeb7 100644 --- a/features/steps/shared/project.rb +++ b/features/steps/shared/project.rb @@ -10,7 +10,7 @@ module SharedProject # Create a specific project called "Shop" And 'I own project "Shop"' do @project = Project.find_by(name: "Shop") - @project ||= create(:project, name: "Shop", namespace: @user.namespace) + @project ||= create(:project, name: "Shop", namespace: @user.namespace, snippets_enabled: true) @project.team << [@user, :master] end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index b6a5806d646..d7d209e16f7 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -98,10 +98,14 @@ module API def attributes_for_keys(keys) attrs = {} + keys.each do |key| - attrs[key] = params[key] if params[key].present? or (params.has_key?(key) and params[key] == false) + if params[key].present? or (params.has_key?(key) and params[key] == false) + attrs[key] = params[key] + end end - attrs + + ActionController::Parameters.new(attrs).permit! end # error helpers diff --git a/lib/api/users.rb b/lib/api/users.rb index 92dbe97f0a4..69553f16397 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -59,7 +59,7 @@ module API authenticated_as_admin! required_attributes! [:email, :password, :name, :username] attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :projects_limit, :username, :extern_uid, :provider, :bio, :can_create_group, :admin] - user = User.build_user(attrs, as: :admin) + user = User.build_user(attrs) admin = attrs.delete(:admin) user.admin = admin unless admin.nil? if user.save @@ -96,7 +96,7 @@ module API admin = attrs.delete(:admin) user.admin = admin unless admin.nil? - if user.update_attributes(attrs, as: :admin) + if user.update_attributes(attrs) present user, with: Entities::UserFull else not_found! diff --git a/lib/gitlab/config_helper.rb b/lib/gitlab/config_helper.rb new file mode 100644 index 00000000000..41880069e4c --- /dev/null +++ b/lib/gitlab/config_helper.rb @@ -0,0 +1,9 @@ +module Gitlab::ConfigHelper + def gitlab_config_features + Gitlab.config.gitlab.default_projects_features + end + + def gitlab_config + Gitlab.config.gitlab + end +end diff --git a/lib/gitlab/oauth/user.rb b/lib/gitlab/oauth/user.rb index 38e33c0eee5..94d59180e15 100644 --- a/lib/gitlab/oauth/user.rb +++ b/lib/gitlab/oauth/user.rb @@ -27,7 +27,7 @@ module Gitlab password_confirmation: password, } - user = model.build_user(opts, as: :admin) + user = model.build_user(opts) user.skip_confirmation! # Services like twitter and github does not return email via oauth diff --git a/spec/factories.rb b/spec/factories.rb index 41cc99cbcb9..ad4c56986c3 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -32,6 +32,7 @@ FactoryGirl.define do path { name.downcase.gsub(/\s/, '_') } namespace creator + snippets_enabled true trait :public do visibility_level Gitlab::VisibilityLevel::PUBLIC @@ -245,7 +246,7 @@ FactoryGirl.define do end end end - + factory :email do user email do diff --git a/spec/models/gitlab_ci_service_spec.rb b/spec/models/gitlab_ci_service_spec.rb index a0708f14236..439a30869bb 100644 --- a/spec/models/gitlab_ci_service_spec.rb +++ b/spec/models/gitlab_ci_service_spec.rb @@ -26,7 +26,6 @@ describe GitlabCiService do end describe "Mass assignment" do - it { should_not allow_mass_assignment_of(:project_id) } end describe 'commits methods' do diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index d53c4037c35..8b299cea67c 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -25,8 +25,6 @@ describe Issue do 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 diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 474067fe38a..95c0aed0ffe 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -20,8 +20,6 @@ describe Key do 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 diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 1148df87ab7..ec6d29de82b 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -28,8 +28,6 @@ describe MergeRequest do 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 "Respond to" do diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 8309ad3a724..a3071c3251a 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -22,7 +22,6 @@ describe Milestone do end describe "Mass assignment" do - it { should_not allow_mass_assignment_of(:project_id) } end describe "Validation" do diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index d2bf96979f9..3562ebed1ff 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -26,8 +26,6 @@ describe Namespace do it { should validate_presence_of :owner } describe "Mass assignment" do - it { should allow_mass_assignment_of(:name) } - it { should allow_mass_assignment_of(:path) } end describe "Respond to" do diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 43779e6bbfc..d06dee6ce92 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -27,8 +27,6 @@ describe Note do 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 diff --git a/spec/models/project_snippet_spec.rb b/spec/models/project_snippet_spec.rb index 42147179387..e4df934460b 100644 --- a/spec/models/project_snippet_spec.rb +++ b/spec/models/project_snippet_spec.rb @@ -23,7 +23,6 @@ describe ProjectSnippet do end describe "Mass assignment" do - it { should_not allow_mass_assignment_of(:project_id) } end describe "Validation" do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 93eae5a9ebd..c3263ed0fe7 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -48,8 +48,6 @@ describe Project do end describe "Mass assignment" do - it { should_not allow_mass_assignment_of(:namespace_id) } - it { should_not allow_mass_assignment_of(:creator_id) } end describe "Validation" do diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb index 35b929c2f3e..af48c2c6d9e 100644 --- a/spec/models/protected_branch_spec.rb +++ b/spec/models/protected_branch_spec.rb @@ -17,7 +17,6 @@ describe ProtectedBranch do end describe "Mass assignment" do - it { should_not allow_mass_assignment_of(:project_id) } end describe 'Validation' do diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index a4bed81c0f6..adeeac115c1 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -27,7 +27,6 @@ describe Service do end describe "Mass assignment" do - it { should_not allow_mass_assignment_of(:project_id) } end describe "Test Button" do diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index a77c594aaf1..d179e9516e2 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -24,7 +24,6 @@ describe Snippet do end describe "Mass assignment" do - it { should_not allow_mass_assignment_of(:author_id) } end describe "Validation" do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 0a665b7defb..a36b57a95de 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -65,8 +65,6 @@ describe User do 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 @@ -243,59 +241,23 @@ describe User do it { user.first_name.should == 'John' } end - describe 'without defaults' do + describe 'with defaults' do let(:user) { User.new } - it "should not apply defaults to user" do - user.projects_limit.should == 10 - user.can_create_group.should be_true - user.theme_id.should == Gitlab::Theme::BASIC - end - end - context 'as admin' do - describe 'with defaults' do - let(:user) { User.build_user({}, as: :admin) } - - it "should apply defaults to user" do - user.projects_limit.should == Gitlab.config.gitlab.default_projects_limit - user.can_create_group.should == Gitlab.config.gitlab.default_can_create_group - user.theme_id.should == Gitlab.config.gitlab.default_theme - end - end - - describe 'with default overrides' do - let(:user) { User.build_user({projects_limit: 123, can_create_group: true, can_create_team: true, theme_id: Gitlab::Theme::BASIC}, as: :admin) } - - it "should apply defaults to user" do - Gitlab.config.gitlab.default_projects_limit.should_not == 123 - Gitlab.config.gitlab.default_can_create_group.should_not be_true - Gitlab.config.gitlab.default_theme.should_not == Gitlab::Theme::BASIC - user.projects_limit.should == 123 - user.can_create_group.should be_true - user.theme_id.should == Gitlab::Theme::BASIC - end + it "should apply defaults to user" do + user.projects_limit.should == Gitlab.config.gitlab.default_projects_limit + user.can_create_group.should == Gitlab.config.gitlab.default_can_create_group + user.theme_id.should == Gitlab.config.gitlab.default_theme end end - context 'as user' do - describe 'with defaults' do - let(:user) { User.build_user } + describe 'with default overrides' do + let(:user) { User.new(projects_limit: 123, can_create_group: false, can_create_team: true, theme_id: Gitlab::Theme::BASIC) } - it "should apply defaults to user" do - user.projects_limit.should == Gitlab.config.gitlab.default_projects_limit - user.can_create_group.should == Gitlab.config.gitlab.default_can_create_group - user.theme_id.should == Gitlab.config.gitlab.default_theme - end - end - - describe 'with default overrides' do - let(:user) { User.build_user(projects_limit: 123, can_create_group: true, theme_id: Gitlab::Theme::BASIC) } - - it "should apply defaults to user" do - user.projects_limit.should == Gitlab.config.gitlab.default_projects_limit - user.can_create_group.should == Gitlab.config.gitlab.default_can_create_group - user.theme_id.should == Gitlab.config.gitlab.default_theme - end + it "should apply defaults to user" do + user.projects_limit.should == 123 + user.can_create_group.should be_false + user.theme_id.should == Gitlab::Theme::BASIC end end end diff --git a/spec/models/users_group_spec.rb b/spec/models/users_group_spec.rb index 05dd97d92d4..0b6f7a08198 100644 --- a/spec/models/users_group_spec.rb +++ b/spec/models/users_group_spec.rb @@ -20,7 +20,6 @@ describe UsersGroup do end describe "Mass assignment" do - it { should_not allow_mass_assignment_of(:group_id) } end describe "Validation" do diff --git a/spec/models/users_project_spec.rb b/spec/models/users_project_spec.rb index aa4b8cb449b..3f38164e964 100644 --- a/spec/models/users_project_spec.rb +++ b/spec/models/users_project_spec.rb @@ -20,7 +20,6 @@ describe UsersProject do end describe "Mass assignment" do - it { should_not allow_mass_assignment_of(:project_id) } end describe "Validation" do diff --git a/spec/models/web_hook_spec.rb b/spec/models/web_hook_spec.rb index 20ee1416125..e9c04ee89cb 100644 --- a/spec/models/web_hook_spec.rb +++ b/spec/models/web_hook_spec.rb @@ -23,7 +23,6 @@ describe ProjectHook do end describe "Mass assignment" do - it { should_not allow_mass_assignment_of(:project_id) } end describe "Validations" do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index c3eec56d133..8bbe9b5b736 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -97,19 +97,6 @@ describe API::API, api: true do response.status.should == 201 end - it "creating a user should respect default project limit" do - limit = 123456 - Gitlab.config.gitlab.stub(:default_projects_limit).and_return(limit) - attr = attributes_for(:user ) - expect { - post api("/users", admin), attr - }.to change { User.count }.by(1) - user = User.find_by(username: attr[:username]) - user.projects_limit.should == limit - user.theme_id.should == Gitlab::Theme::MARS - Gitlab.config.gitlab.unstub(:default_projects_limit) - end - it "should not create user with invalid email" do post api("/users", admin), { email: "invalid email", password: 'password' } response.status.should == 400 diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 106c14bc015..f59786efcf9 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -11,7 +11,6 @@ describe Notes::CreateService do project.team << [user, :master] opts = { note: 'Awesome comment', - description: 'please fix', noteable_type: 'Issue', noteable_id: issue.id } diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 74c23418a28..9c97dad2ff0 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -55,95 +55,6 @@ describe Projects::CreateService do it { File.exists?(@path).should be_false } end end - - context 'respect configured visibility setting' do - before(:each) do - @settings = double("settings") - @settings.stub(:issues) { true } - @settings.stub(:merge_requests) { true } - @settings.stub(:wiki) { true } - @settings.stub(:snippets) { true } - Gitlab.config.gitlab.stub(restricted_visibility_levels: []) - Gitlab.config.gitlab.stub(:default_projects_features).and_return(@settings) - end - - context 'should be public when setting is public' do - before do - @settings.stub(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC } - @project = create_project(@user, @opts) - end - - it { @project.public?.should be_true } - end - - context 'should be private when setting is private' do - before do - @settings.stub(:visibility_level) { Gitlab::VisibilityLevel::PRIVATE } - @project = create_project(@user, @opts) - end - - it { @project.private?.should be_true } - end - - context 'should be internal when setting is internal' do - before do - @settings.stub(:visibility_level) { Gitlab::VisibilityLevel::INTERNAL } - @project = create_project(@user, @opts) - end - - it { @project.internal?.should be_true } - end - end - - context 'respect configured visibility restrictions setting' do - before(:each) do - @settings = double("settings") - @settings.stub(:issues) { true } - @settings.stub(:merge_requests) { true } - @settings.stub(:wiki) { true } - @settings.stub(:snippets) { true } - @settings.stub(:visibility_level) { Gitlab::VisibilityLevel::PRIVATE } - @restrictions = [ Gitlab::VisibilityLevel::PUBLIC ] - Gitlab.config.gitlab.stub(restricted_visibility_levels: @restrictions) - Gitlab.config.gitlab.stub(:default_projects_features).and_return(@settings) - end - - context 'should be private when option is public' do - before do - @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) - @project = create_project(@user, @opts) - end - - it { @project.private?.should be_true } - end - - context 'should be public when option is public for admin' do - before do - @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) - @project = create_project(@admin, @opts) - end - - it { @project.public?.should be_true } - end - - context 'should be private when option is private' do - before do - @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) - @project = create_project(@user, @opts) - end - - it { @project.private?.should be_true } - end - - context 'should be internal when option is internal' do - before do - @opts.merge!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) - @project = create_project(@user, @opts) - end - - it { @project.internal?.should be_true } - end - end end def create_project(user, opts) diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index bb0470e3771..62357787521 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -6,14 +6,14 @@ describe Projects::UpdateService do @user = create :user @admin = create :user, admin: true @project = create :project, creator_id: @user.id, namespace: @user.namespace - @opts = { project: {} } + @opts = {} end context 'should be private when updated to private' do before do @created_private = @project.private? - @opts[:project].merge!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) update_project(@project, @user, @opts) end @@ -25,7 +25,7 @@ describe Projects::UpdateService do before do @created_private = @project.private? - @opts[:project].merge!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + @opts.merge!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) update_project(@project, @user, @opts) end @@ -37,7 +37,7 @@ describe Projects::UpdateService do before do @created_private = @project.private? - @opts[:project].merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) update_project(@project, @user, @opts) end @@ -56,7 +56,7 @@ describe Projects::UpdateService do before do @created_private = @project.private? - @opts[:project].merge!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) update_project(@project, @user, @opts) end @@ -68,7 +68,7 @@ describe Projects::UpdateService do before do @created_private = @project.private? - @opts[:project].merge!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + @opts.merge!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) update_project(@project, @user, @opts) end @@ -80,7 +80,7 @@ describe Projects::UpdateService do before do @created_private = @project.private? - @opts[:project].merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) update_project(@project, @user, @opts) end @@ -92,7 +92,7 @@ describe Projects::UpdateService do before do @created_private = @project.private? - @opts[:project].merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) update_project(@project, @admin, @opts) end diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb index 3802e94ecf0..0d67e7ee4e6 100644 --- a/spec/support/mentionable_shared_examples.rb +++ b/spec/support/mentionable_shared_examples.rb @@ -11,7 +11,7 @@ def common_mentionable_setup let(:mentioned_issue) { create :issue, project: mproject } let(:other_issue) { create :issue, project: mproject } - let(:mentioned_mr) { create :merge_request, source_project: mproject, source_branch: 'different' } + let(:mentioned_mr) { create :merge_request, :simple, source_project: mproject } let(:mentioned_commit) { double('commit', sha: '1234567890abcdef').as_null_object } # Override to add known commits to the repository stub. @@ -29,11 +29,7 @@ def common_mentionable_setup # unrecognized commits. commitmap = { '123456' => mentioned_commit } extra_commits.each { |c| commitmap[c.sha[0..5]] = c } - - repo = double('repository') - repo.stub(:commit) { |sha| commitmap[sha] } - mproject.stub(repository: repo) - + mproject.repository.stub(:commit) { |sha| commitmap[sha] } set_mentionable_text.call(ref_string) end end |
