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/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 5f2bd066e..09d02f510 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -14,7 +14,7 @@ class Admin::UsersController < ApplicationController def create role = params[:user].delete(:role) - @user = User.new params[:user] + @user = User.new params[:user], :as => :create @user.set_role role if @user.save flash[:notice] = t('flash.user.saved') diff --git a/app/models/user.rb b/app/models/user.rb index a37276179..04e19ec5b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -40,11 +40,13 @@ class User < ActiveRecord::Base validates :uname, :presence => true, :uniqueness => {:case_sensitive => false}, :format => { :with => /^[a-z0-9_]+$/ } validate { errors.add(:uname, :taken) if Group.where('uname LIKE ?', uname).present? } - validates :role, :inclusion => {:in => ROLES - ['banned']} + validates :role, :inclusion => {:in => ROLES - ['banned']}, :allow_blank => true validates :language, :inclusion => {:in => LANGUAGES}, :allow_blank => true - attr_accessible :email, :password, :password_confirmation, :current_password, :remember_me, :login, :name, :ssh_key, :uname, :language, + attr_accessible :email, :password, :password_confirmation, :current_password, :remember_me, :login, :name, :ssh_key, :language, :site, :company, :professional_experience, :location, :avatar + attr_accessible :uname, :name, :email, :password, :password_confirmation, :language, :site, :company, + :professional_experience, :location, :avatar, :as => :create attr_readonly :uname, :own_projects_count attr_readonly :uname attr_accessor :login diff --git a/app/views/admin/users/new.html.haml b/app/views/admin/users/new.html.haml index 67d4d3803..b167b5da1 100644 --- a/app/views/admin/users/new.html.haml +++ b/app/views/admin/users/new.html.haml @@ -3,16 +3,6 @@ %h2.title= t("layout.users.new_header") .inner = form_for @user, :url => users_path, :html => { :class => :form } do |f| - .leftlist - = f.label :password, t("activerecord.attributes.user.password") - .rightlist - = f.password_field :password - .both - .leftlist - = f.label :password_confirmation, t("activerecord.attributes.user.password_confirmation") - .rightlist - = f.password_field :password_confirmation - .both = render :partial => "users/form", :locals => {:f => f} - content_for :sidebar do diff --git a/app/views/users/_form.html.haml b/app/views/users/_form.html.haml index 1decda3b3..60b1b564e 100644 --- a/app/views/users/_form.html.haml +++ b/app/views/users/_form.html.haml @@ -3,10 +3,21 @@ = f.label :role, t("activerecord.attributes.user.role"), :class => :label .rightlist = f.select :role, User::ROLES, :selected => @user.get_role - .leftlist - = f.label :uname, t("activerecord.attributes.user.uname") - .rightlist - = f.text_field :uname + - if @user.new_record? + .leftlist + = f.label :uname, t("activerecord.attributes.user.uname") + .rightlist + = f.text_field :uname + .leftlist + = f.label :password, t("activerecord.attributes.user.password") + .rightlist + = f.password_field :password + .both + .leftlist + = f.label :password_confirmation, t("activerecord.attributes.user.password_confirmation") + .rightlist + = f.password_field :password_confirmation + .both .leftlist = f.label :name, t("activerecord.attributes.user.name") .rightlist diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index a5a714873..7a15ae171 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -1,52 +1,30 @@ # -*- encoding : utf-8 -*- require 'spec_helper' -shared_examples_for 'user with users list viewer rights' do - it 'should be able to perform index action' do - get :index - response.should render_template(:index) - end - - it 'should assigns 5 users without filter params' do - get :index - assigns[:users].count.should == 5 - end - - it 'should find one user' do - get :index, :filter => {:email => "user1@nonexistanceserver.com"} - assigns[:users].size == 1 - end - - it 'should find user with searchable email' do - get :index, :filter => {:email => "user1@nonexistanceserver.com"} - assigns[:users].first.email.should == "user1@nonexistanceserver.com" - end -end - describe UsersController do before(:each) do stub_rsync_methods @simple_user = Factory(:user) + @other_user = Factory(:user) @admin = Factory(:admin) %w[user1 user2 user3].each do |uname| Factory(:user, :uname => uname, :email => "#{ uname }@nonexistanceserver.com") end - end - - context 'for global admin' do - before(:each) do - set_session_for(@admin) - end - - it_should_behave_like 'user with users list viewer rights' + @update_params = {:email => 'new_email@test.com'} end context 'for guest' do - it 'should not be able to perform index action' do - get :index + it 'should not be able to view profile' do + get :profile response.should redirect_to(new_user_session_path) end + + it 'should not be able to update other profile' do + get :update, {:id => @other_user.id}.merge(@update_params) + response.should redirect_to(new_user_session_path) + @other_user.reload.email.should_not == @update_params[:email] + end end context 'for simple user' do @@ -54,9 +32,23 @@ describe UsersController do set_session_for(@simple_user) end - it 'should not be able to perform index action' do - get :index - response.should redirect_to(forbidden_path) + it 'should be able to view profile' do + get :profile + response.code.should eq('200') + end + + context 'with mass assignment' do + it 'should not be able to update uname' do + @simple_user.should_not allow_mass_assignment_of :uname + end + + it 'should not be able to update role' do + @simple_user.should_not allow_mass_assignment_of :role + end + + it 'should not be able to update other user' do + @simple_user.should_not allow_mass_assignment_of :id + end end end end