From 72c02282aa2f7cb00ba0724dd4cee5595931eab4 Mon Sep 17 00:00:00 2001 From: "konstantin.grabar" Date: Wed, 21 Mar 2012 17:52:35 +0400 Subject: [PATCH] [refs #232] Add mass-assignment fixes. Add create and mass-assig tests. --- Gemfile | 1 + Gemfile.lock | 6 ++++++ app/controllers/groups_controller.rb | 6 ++++-- app/models/group.rb | 1 + db/schema.rb | 9 ++++----- spec/controllers/groups_controller_spec.rb | 15 +++++---------- spec/models/group_spec.rb | 11 +++++++++++ 7 files changed, 32 insertions(+), 17 deletions(-) diff --git a/Gemfile b/Gemfile index b422fa0b4..60f30b6e7 100644 --- a/Gemfile +++ b/Gemfile @@ -70,4 +70,5 @@ group :test do gem 'rspec-rails', '~> 2.8.1' gem 'factory_girl_rails', '~> 1.7.0' gem 'rr', '~> 1.0.4' + gem 'shoulda' end diff --git a/Gemfile.lock b/Gemfile.lock index e94e27f5a..85277fbef 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -251,6 +251,11 @@ GEM tilt (~> 1.3) shotgun (0.9) rack (>= 1.0) + shoulda (3.0.1) + shoulda-context (~> 1.0.0) + shoulda-matchers (~> 1.0.0) + shoulda-context (1.0.0) + shoulda-matchers (1.0.0) sinatra (1.3.2) rack (~> 1.3, >= 1.3.6) rack-protection (~> 1.2) @@ -338,6 +343,7 @@ DEPENDENCIES russian (~> 0.6.0) sass-rails (~> 3.2.4) shotgun + shoulda therubyracer (~> 0.9.10) uglifier (~> 1.2.1) unicorn (~> 4.2.0) diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index c1effe3dd..204962c53 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -7,7 +7,8 @@ class GroupsController < ApplicationController before_filter :authenticate_user! before_filter :find_group, :only => [:show, :edit, :update, :destroy] - load_and_authorize_resource + load_and_authorize_resource :except => :create + authorize_resource :only => :create autocomplete :group, :uname def index @@ -34,8 +35,9 @@ class GroupsController < ApplicationController end def create - @group = Group.new params[:group] + @group = Group.new(:description => params[:group][:description]) @group.owner = current_user + @group.uname = params[:group][:uname] if @group.save flash[:notice] = t('flash.group.saved') diff --git a/app/models/group.rb b/app/models/group.rb index ada4880a1..69d839a49 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -22,6 +22,7 @@ class Group < ActiveRecord::Base scope :by_owner, lambda {|owner| where(:owner_id => owner.id)} scope :by_admin, lambda {|admin| joins(:relations).where(:'relations.role' => 'admin', :'relations.target_id' => admin.id, :'relations.target_type' => 'User')} + attr_accessible :description attr_readonly :own_projects_count delegate :ssh_key, :email, :to => :owner diff --git a/db/schema.rb b/db/schema.rb index bab2a9d99..37ffee4b7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -285,17 +285,17 @@ ActiveRecord::Schema.define(:version => 20120314223151) do t.text "description" t.string "ancestry" t.boolean "has_issues", :default => true + t.boolean "has_wiki", :default => false t.string "srpm_file_name" t.string "srpm_content_type" t.integer "srpm_file_size" t.datetime "srpm_updated_at" - t.boolean "has_wiki", :default => false t.string "default_branch", :default => "master" t.boolean "is_rpm", :default => true end add_index "projects", ["category_id"], :name => "index_projects_on_category_id" - add_index "projects", ["owner_id"], :name => "index_projects_on_name_and_owner_id_and_owner_type", :unique => true + add_index "projects", ["owner_id"], :name => "index_projects_on_name_and_owner_id_and_owner_type", :unique => true, :case_sensitive => false create_table "register_requests", :force => true do |t| t.string "name" @@ -303,14 +303,13 @@ ActiveRecord::Schema.define(:version => 20120314223151) do t.string "token" t.boolean "approved", :default => false t.boolean "rejected", :default => false - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false + t.datetime "created_at" + t.datetime "updated_at" t.string "interest" t.text "more" end add_index "register_requests", ["email"], :name => "index_register_requests_on_email", :unique => true, :case_sensitive => false - add_index "register_requests", ["token"], :name => "index_register_requests_on_token", :unique => true, :case_sensitive => false create_table "relations", :force => true do |t| t.integer "object_id" diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 77effc1a3..4dcb8d44f 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -11,11 +11,6 @@ shared_examples_for 'group user without update rights' do put :update, :id => @group.id, :group => {:description => 'new description'} @group.reload.description.should_not == 'new description' end - - pending 'should be able to manage_members group' do - get :manage_members, :id => @group.id - response.should render_template("") - end end shared_examples_for 'group user without destroy rights' do @@ -41,11 +36,6 @@ shared_examples_for 'group admin' do put :update, {:id => @group.id}.merge(@update_params) response.should redirect_to(group_path(@group)) end - - pending 'should be able to manage_members group' do - get :manage_members, :id => @group.id - response.should render_template("") - end end shared_examples_for 'no group user' do @@ -96,6 +86,11 @@ describe GroupsController do put :update, {:id => @group.id}.merge(@update_params) response.should redirect_to(new_user_session_path) end + + it 'should not be able to perform create action' do + post :create, @create_params + response.should redirect_to(new_user_session_path) + end end context 'for global admin' do diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 7cc5beea9..605695ef3 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -33,6 +33,7 @@ describe Group do context 'for group admin' do before(:each) do @user = Factory(:user) + @another_user = Factory(:user) @group.objects.create(:object_type => 'User', :object_id => @user.id, :role => 'admin') @ability = Ability.new(@user) end @@ -46,6 +47,16 @@ describe Group do it "should not be able to destroy group" do @ability.should_not be_able_to(:destroy, @group) end + + context 'with mass assignment' do + it 'should not be able to update uname' do + @group.should_not allow_mass_assignment_of :uname => 'new_uname' + end + + it 'should not be able to update owner' do + @group.should_not allow_mass_assignment_of :owner_type => 'User', :owner_id => @another_user.id + end + end end context 'for group owner' do