diff --git a/Gemfile b/Gemfile index 17e12a9..7798ecf 100644 --- a/Gemfile +++ b/Gemfile @@ -2,7 +2,9 @@ source 'http://rubygems.org' gem 'rails', '3.2.13' gem 'mongoid', '~> 2.7.1' -gem 'mongoid_rails_migrations' + +# Mongoid rails migration > 0.0.14 is not compatible to Mongoid 2.x +gem 'mongoid_rails_migrations', '~> 0.0.14' gem 'devise', '~> 1.5.4' gem 'haml' gem 'htmlentities' @@ -10,6 +12,8 @@ gem 'rack-ssl', :require => 'rack/ssl' # force SSL gem 'useragent' gem 'inherited_resources' +gem 'decent_exposure' +gem 'strong_parameters' gem 'SystemTimer', :platform => :ruby_18 gem 'actionmailer_inline_css', "~> 1.3.0" gem 'kaminari', '>= 0.14.1' diff --git a/Gemfile.lock b/Gemfile.lock index 5f2124f..1a4eba8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,6 +1,6 @@ GIT remote: https://github.com/NARKOZ/gitlab.git - revision: f2ba111dba70eca5346a880c541dafaf35d3332a + revision: 53d7a8a86dfed63e56eeb16ea496bb7a82de337e specs: gitlab (2.2.0) httparty @@ -91,9 +91,8 @@ GEM simplecov (>= 0.7) thor crack (0.3.2) - css_parser (1.2.6) + css_parser (1.3.4) addressable - rdoc daemons (1.1.9) database_cleaner (0.9.1) debug_inspector (0.0.2) @@ -103,6 +102,7 @@ GEM debugger-ruby_core_source (~> 1.2.1) debugger-linecache (1.2.0) debugger-ruby_core_source (1.2.2) + decent_exposure (2.2.0) devise (1.5.4) bcrypt-ruby (~> 3.0) orm_adapter (~> 0.0.3) @@ -135,6 +135,7 @@ GEM hike (1.2.2) hipchat (0.9.0) httparty + httparty hoi (0.0.6) httparty (> 0.6.0) json (> 1.4.0) @@ -151,9 +152,9 @@ GEM has_scope (~> 0.5.0) responders (~> 0.9) journey (1.0.4) - jquery-rails (2.1.3) - railties (>= 3.1.0, < 5.0) - thor (~> 0.14) + jquery-rails (3.0.0) + railties (>= 3.0, < 5.0) + thor (>= 0.14, < 2.0) json (1.8.0) jwt (0.1.8) multi_json (>= 1.5) @@ -229,12 +230,14 @@ GEM activeresource (>= 2.3.0) pivotal-tracker (0.5.10) builder + builder crack happymapper (>= 0.3.2) nokogiri (>= 1.4.3) nokogiri (>= 1.5.5) nokogiri-happymapper (>= 0.5.4) rest-client (~> 1.6.0) + rest-client (~> 1.6.0) pjax_rails (0.3.4) jquery-rails polyglot (0.3.3) @@ -279,7 +282,7 @@ GEM rbx-require-relative (0.0.9) rdoc (3.12.2) json (~> 1.4) - ref (1.0.4) + ref (1.0.5) responders (0.9.3) railties (~> 3.1) rest-client (1.6.7) @@ -323,6 +326,10 @@ GEM multi_json (~> 1.0) rack (~> 1.0) tilt (~> 1.1, != 1.3.0) + strong_parameters (0.2.1) + actionpack (~> 3.0) + activemodel (~> 3.0) + railties (~> 3.0) taskmapper (0.8.0) activeresource (~> 3.0) activesupport (~> 3.0) @@ -385,6 +392,7 @@ DEPENDENCIES coveralls database_cleaner (~> 0.9.0) debugger + decent_exposure devise (~> 1.5.4) email_spec execjs @@ -405,7 +413,7 @@ DEPENDENCIES meta_request mongo mongoid (~> 2.7.1) - mongoid_rails_migrations + mongoid_rails_migrations (~> 0.0.14) octokit omniauth-github oruen_redmine_client @@ -421,6 +429,7 @@ DEPENDENCIES ruby-debug ruby-fogbugz rushover + strong_parameters taskmapper (~> 0.8.0) taskmapper-unfuddle (~> 0.7.0) therubyracer diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 25939b7..c13ee52 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -17,6 +17,9 @@ class ApplicationController < ActionController::Base protected + ## + # Check if the current_user is admin or not and redirect to root url if not + # def require_admin! unless user_signed_in? && current_user.admin? flash[:error] = "Sorry, you don't have permission to do that" diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 54bd827..fbe4991 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -2,26 +2,23 @@ class UsersController < ApplicationController respond_to :html before_filter :require_admin!, :except => [:edit, :update] - before_filter :find_user, :only => [:show, :edit, :update, :destroy, :unlink_github] before_filter :require_user_edit_priviledges, :only => [:edit, :update] - def index - @users = User.all.page(params[:page]).per(current_user.per_page) - end + expose(:user) { + params[:id] ? User.find(params[:id]) : User.new(user_params) + } + expose(:users) { + User.all.page(params[:page]).per(current_user.per_page) + } - def new - @user = User.new - end + def index; end + def new; end + def show; end def create - @user = User.new(params[:user]) - - # Set protected attributes - @user.admin = params[:user].try(:[], :admin) if current_user.admin? - - if @user.save - flash[:success] = "#{@user.name} is now part of the team. Be sure to add them as a project watcher." - redirect_to user_path(@user) + if user.save + flash[:success] = "#{user.name} is now part of the team. Be sure to add them as a project watcher." + redirect_to user_path(user) else render :new end @@ -29,44 +26,47 @@ class UsersController < ApplicationController def update # Devise Hack - if params[:user][:password].blank? && params[:user][:password_confirmation].blank? - params[:user].delete(:password) - params[:user].delete(:password_confirmation) - end - - # Set protected attributes - @user.admin = params[:user][:admin] if current_user.admin? - - if @user.update_attributes(params[:user]) - flash[:success] = "#{@user.name}'s information was successfully updated" - redirect_to user_path(@user) + # if params[:user][:password].blank? && params[:user][:password_confirmation].blank? + # params[:user].delete(:password) + # params[:user].delete(:password_confirmation) + # end + + if user.update_attributes(user_params) + flash[:success] = "#{user.name}'s information was successfully updated" + redirect_to user_path(user) else render :edit end end def destroy - @user.destroy + user.destroy - flash[:success] = "That's sad. #{@user.name} is no longer part of your team." + flash[:success] = "That's sad. #{user.name} is no longer part of your team." redirect_to users_path end def unlink_github - @user.update_attributes :github_login => nil, :github_oauth_token => nil - redirect_to user_path(@user) + user.update_attributes :github_login => nil, :github_oauth_token => nil + redirect_to user_path(user) end protected - def find_user - @user = User.find(params[:id]) - end - def require_user_edit_priviledges - can_edit = current_user == @user || current_user.admin? + can_edit = current_user == user || current_user.admin? redirect_to(root_path) and return(false) unless can_edit end + def user_params + params[:user] ? params.require(:user).permit(*user_permit_params) : {} + end + + def user_permit_params + @user_permit_params ||= [:name, :username, :email, :github_login, :per_page, :time_zone, :password, :password_confirmation] + @user_permit_params << :admin if current_user.admin? + @user_permit_params + end + end diff --git a/app/models/user.rb b/app/models/user.rb index b93bd0c..3f8c5dc 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -19,8 +19,6 @@ class User validates_presence_of :name validates_uniqueness_of :github_login, :allow_nil => true - attr_protected :admin - has_many :apps, :foreign_key => 'watchers.user_id' if Errbit::Config.user_has_username diff --git a/app/views/users/_fields.html.haml b/app/views/users/_fields.html.haml index c1b3b35..45ec0c0 100644 --- a/app/views/users/_fields.html.haml +++ b/app/views/users/_fields.html.haml @@ -1,4 +1,4 @@ -= errors_for @user += errors_for user .required = f.label :name diff --git a/app/views/users/edit.html.haml b/app/views/users/edit.html.haml index 008d0a5..0e015c3 100644 --- a/app/views/users/edit.html.haml +++ b/app/views/users/edit.html.haml @@ -1,10 +1,10 @@ -- content_for :title, "Edit #{@user.name}" +- content_for :title, "Edit #{user.name}" - content_for :action_bar do - = render 'shared/link_github_account', :user => @user - = link_to('cancel', user_path(@user), :class => 'button') + = render 'shared/link_github_account', :user => user + = link_to('cancel', user_path(user), :class => 'button') -= form_for @user, :html => {:autocomplete => "off"} do |f| - = @user.errors.full_messages.to_sentence += form_for user, :html => {:autocomplete => "off"} do |f| + = user.errors.full_messages.to_sentence = render 'fields', :f => f %div.buttons= f.submit 'Update User' diff --git a/app/views/users/index.html.haml b/app/views/users/index.html.haml index 50102cd..119d54b 100644 --- a/app/views/users/index.html.haml +++ b/app/views/users/index.html.haml @@ -13,8 +13,8 @@ %th.main Email %th Admin? %tbody - - @users.each do |user| - %tr + - users.each do |user| + %tr.user_list - if Errbit::Config.use_gravatar %td= gravatar_tag user.email, :s => 24 %td.nowrap= link_to user.name, user_path(user) @@ -22,5 +22,5 @@ %td= user.username %td= user.email %td= user.admin? ? 'Y' : 'N' -= paginate @users += paginate users diff --git a/app/views/users/new.html.haml b/app/views/users/new.html.haml index ab1b16b..3db349b 100644 --- a/app/views/users/new.html.haml +++ b/app/views/users/new.html.haml @@ -1,8 +1,8 @@ - content_for :title, 'New User' - content_for :action_bar, link_to('cancel', users_path, :class => 'button') -= form_for @user do |f| - += form_for user do |f| + = render 'fields', :f => f - - %div.buttons= f.submit 'Add User' \ No newline at end of file + + %div.buttons= f.submit 'Add User' diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index 647e62b..1e18043 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -1,31 +1,32 @@ -- content_for :title, @user.name -- if Errbit::Config.use_gravatar && gravatar = gravatar_url(@user.email, :s => 86) +- content_for :title, user.name + +- if Errbit::Config.use_gravatar && gravatar = gravatar_url(user.email, :s => 86) - content_for :title_style do background: url('#{gravatar}') no-repeat; padding-left: 106px; - content_for :action_bar do - = render 'shared/link_github_account', :user => @user + = render 'shared/link_github_account' %span= link_to('Add a New User', new_user_path, :class => 'add') - = link_to 'edit', edit_user_path(@user), :class => 'button' - = link_to 'destroy', user_path(@user), :method => :delete, :data => { :confirm => 'Seriously?' }, :class => 'button' + = link_to 'edit', edit_user_path(user), :class => 'button' + = link_to 'destroy', user_path(user), :method => :delete, :data => { :confirm => 'Seriously?' }, :class => 'button' %table.single_user %tr %th Email - %td.main= @user.email + %td.main= user.email - if Errbit::Config.user_has_username %tr %th Username - %td.main= @user.username - - if Errbit::Config.github_authentication && @user.github_login.present? + %td.main= user.username + - if Errbit::Config.github_authentication && user.github_login.present? %tr %th GitHub Login - %td.main= link_to @user.github_login, "https://github.com/#{@user.github_login}" + %td.main= link_to user.github_login, "https://github.com/#{user.github_login}" %tr %th Admin? - %td= @user.admin? ? 'Y' : 'N' + %td= user.admin? ? 'Y' : 'N' %tr %th Created - %td= @user.created_at.to_s(:micro) + %td= user.created_at.to_s(:micro) diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index e73616f..7c4f251 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -1,7 +1,6 @@ require 'spec_helper' describe UsersController do - render_views it_requires_authentication it_requires_admin_privileges :for => { @@ -12,42 +11,39 @@ describe UsersController do :destroy => :delete } + let(:admin) { Fabricate(:admin) } + let(:user) { Fabricate(:user) } + let(:other_user) { Fabricate(:user) } + context 'Signed in as a regular user' do + before do - sign_in @user = Fabricate(:user) + sign_in user end it "should set a time zone" do - Time.zone.should.to_s == @user.time_zone + Time.zone.should.to_s == user.time_zone end context "GET /users/:other_id/edit" do it "redirects to the home page" do - get :edit, :id => Fabricate(:user).id + get :edit, :id => other_user.id response.should redirect_to(root_path) end end context "GET /users/:my_id/edit" do it 'finds the user' do - get :edit, :id => @user.id - assigns(:user).should == @user - end - - it "should have per_page option" do - get :edit, :id => @user.id - response.body.should match(/id="user_per_page"/) + get :edit, :id => user.id + controller.user.should == user + expect(response).to render_template 'edit' end - it "should have time_zone option" do - get :edit, :id => @user.id - response.body.should match(/id="user_time_zone"/) - end end context "PUT /users/:other_id" do it "redirects to the home page" do - put :update, :id => Fabricate(:user).id + put :update, :id => other_user.id response.should redirect_to(root_path) end end @@ -55,44 +51,47 @@ describe UsersController do context "PUT /users/:my_id/id" do context "when the update is successful" do it "sets a message to display" do - put :update, :id => @user.to_param, :user => {:name => 'Kermit'} + put :update, :id => user.to_param, :user => {:name => 'Kermit'} request.flash[:success].should include('updated') end it "redirects to the user's page" do - put :update, :id => @user.to_param, :user => {:name => 'Kermit'} - response.should redirect_to(user_path(@user)) + put :update, :id => user.to_param, :user => {:name => 'Kermit'} + response.should redirect_to(user_path(user)) end it "should not be able to become an admin" do - put :update, :id => @user.to_param, :user => {:admin => true} - @user.reload.admin.should be_false + expect { + put :update, :id => user.to_param, :user => {:admin => true} + }.to_not change { + user.reload.admin + }.from(false) end it "should be able to set per_page option" do - put :update, :id => @user.to_param, :user => {:per_page => 555} - @user.reload.per_page.should == 555 + put :update, :id => user.to_param, :user => {:per_page => 555} + user.reload.per_page.should == 555 end it "should be able to set time_zone option" do - put :update, :id => @user.to_param, :user => {:time_zone => "Warsaw"} - @user.reload.time_zone.should == "Warsaw" + put :update, :id => user.to_param, :user => {:time_zone => "Warsaw"} + user.reload.time_zone.should == "Warsaw" end it "should be able to not set github_login option" do - put :update, :id => @user.to_param, :user => {:github_login => " "} - @user.reload.github_login.should == nil + put :update, :id => user.to_param, :user => {:github_login => " "} + user.reload.github_login.should == nil end it "should be able to set github_login option" do - put :update, :id => @user.to_param, :user => {:github_login => "awesome_name"} - @user.reload.github_login.should == "awesome_name" + put :update, :id => user.to_param, :user => {:github_login => "awesome_name"} + user.reload.github_login.should == "awesome_name" end end context "when the update is unsuccessful" do it "renders the edit page" do - put :update, :id => @user.to_param, :user => {:name => nil} + put :update, :id => user.to_param, :user => {:name => nil} response.should render_template(:edit) end end @@ -101,81 +100,82 @@ describe UsersController do context 'Signed in as an admin' do before do - @user = Fabricate(:admin) - sign_in @user + sign_in admin end context "GET /users" do + it 'paginates all users' do - @user.update_attribute :per_page, 2 - users = 3.times { Fabricate(:user) } + admin.update_attribute :per_page, 2 + users = 3.times { + Fabricate(:user) + } get :index - assigns(:users).to_a.size.should == 2 + controller.users.to_a.size.should == 2 end + end context "GET /users/:id" do it 'finds the user' do - user = Fabricate(:user) get :show, :id => user.id - assigns(:user).should == user + controller.user.should == user end end context "GET /users/new" do it 'assigns a new user' do get :new - assigns(:user).should be_a(User) - assigns(:user).should be_new_record + controller.user.should be_a(User) + controller.user.should be_new_record end end context "GET /users/:id/edit" do it 'finds the user' do - user = Fabricate(:user) get :edit, :id => user.id - assigns(:user).should == user + controller.user.should == user end end context "POST /users" do context "when the create is successful" do - before do - @attrs = {:user => Fabricate.attributes_for(:user)} - end + let(:attrs) { {:user => Fabricate.attributes_for(:user)} } it "sets a message to display" do - post :create, @attrs + post :create, attrs request.flash[:success].should include('part of the team') end it "redirects to the user's page" do - post :create, @attrs - response.should redirect_to(user_path(assigns(:user))) + post :create, attrs + response.should redirect_to(user_path(controller.user)) end it "should be able to create admin" do - @attrs[:user][:admin] = true - post :create, @attrs + attrs[:user][:admin] = true + post :create, attrs response.should be_redirect - User.find(assigns(:user).to_param).admin.should be_true + User.find(controller.user.to_param).admin.should be_true end it "should has auth token" do - post :create, @attrs + post :create, attrs User.last.authentication_token.should_not be_blank end end context "when the create is unsuccessful" do + let(:user) { + Struct.new(:admin, :attributes).new(true, {}) + } before do - @user = Fabricate(:user) - User.should_receive(:new).and_return(@user) - @user.should_receive(:save).and_return(false) + User.should_receive(:new).and_return(user) + user.should_receive(:save).and_return(false) end it "renders the new page" do - post :create + post :create, :user => { :username => 'foo' } response.should render_template(:new) end end @@ -183,56 +183,47 @@ describe UsersController do context "PUT /users/:id" do context "when the update is successful" do - before do - @user = Fabricate(:user) - end it "sets a message to display" do - put :update, :id => @user.to_param, :user => {:name => 'Kermit'} + put :update, :id => user.to_param, :user => {:name => 'Kermit'} request.flash[:success].should include('updated') end it "redirects to the user's page" do - put :update, :id => @user.to_param, :user => {:name => 'Kermit'} - response.should redirect_to(user_path(@user)) + put :update, :id => user.to_param, :user => {:name => 'Kermit'} + response.should redirect_to(user_path(user)) end it "should be able to make user an admin" do - put :update, :id => @user.to_param, :user => {:admin => true} + put :update, :id => user.to_param, :user => {:admin => true} response.should be_redirect - User.find(assigns(:user).to_param).admin.should be_true + User.find(controller.user.to_param).admin.should be_true end end context "when the update is unsuccessful" do - before do - @user = Fabricate(:user) - end it "renders the edit page" do - put :update, :id => @user.to_param, :user => {:name => nil} + put :update, :id => user.to_param, :user => {:name => nil} response.should render_template(:edit) end end end context "DELETE /users/:id" do - before do - @user = Fabricate(:user) - end it "destroys the user" do - delete :destroy, :id => @user.id - User.where(:id => @user.id).first.should be_nil + delete :destroy, :id => user.id + User.where(:id => user.id).first.should be_nil end it "redirects to the users index page" do - delete :destroy, :id => @user.id + delete :destroy, :id => user.id response.should redirect_to(users_path) end it "sets a message to display" do - delete :destroy, :id => @user.id + delete :destroy, :id => user.id request.flash[:success].should include('no longer part of your team') end end diff --git a/spec/views/users/edit.html.haml_spec.rb b/spec/views/users/edit.html.haml_spec.rb new file mode 100644 index 0000000..4edf49a --- /dev/null +++ b/spec/views/users/edit.html.haml_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe 'users/edit.html.haml' do + let(:user) { stub_model(User, :name => 'shingara') } + before { + view.stub(:current_user).and_return(user) + view.stub(:user).and_return(user) + } + it 'should have per_page option' do + render + expect(rendered).to match(/id="user_per_page"/) + end + + it 'should have time_zone option' do + render + expect(rendered).to match(/id="user_time_zone"/) + end +end diff --git a/spec/views/users/index.html.haml_spec.rb b/spec/views/users/index.html.haml_spec.rb new file mode 100644 index 0000000..59ce22b --- /dev/null +++ b/spec/views/users/index.html.haml_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe 'users/index.html.haml' do + let(:user) { stub_model(User) } + before { + view.stub(:current_user).and_return(user) + view.stub(:users).and_return( + Kaminari.paginate_array([user], :total_count => 1).page(1) + ) + } + it 'should see users option' do + render + expect(rendered).to match(/class='user_list'/) + end + +end diff --git a/spec/views/users/new.html.haml_spec.rb b/spec/views/users/new.html.haml_spec.rb new file mode 100644 index 0000000..63c75e7 --- /dev/null +++ b/spec/views/users/new.html.haml_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe 'users/new.html.haml' do + let(:user) { stub_model(User) } + before { + view.stub(:current_user).and_return(user) + view.stub(:user).and_return(user) + } + it 'should have per_page option' do + render + expect(rendered).to match(/id="user_per_page"/) + end + + it 'should have time_zone option' do + render + expect(rendered).to match(/id="user_time_zone"/) + end +end diff --git a/spec/views/users/show.html.haml_spec.rb b/spec/views/users/show.html.haml_spec.rb index 882922e..e97cae9 100644 --- a/spec/views/users/show.html.haml_spec.rb +++ b/spec/views/users/show.html.haml_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' describe 'users/show.html.haml' do + let(:user) do stub_model(User, :created_at => Time.now, :email => "test@example.com") end @@ -8,12 +9,12 @@ describe 'users/show.html.haml' do before do Errbit::Config.stub(:github_authentication) { true } controller.stub(:current_user) { stub_model(User) } + view.stub(:user) { user } end context 'with GitHub authentication' do it 'shows github login' do user.github_login = 'test_user' - assign :user, user render rendered.should match(/GitHub/) rendered.should match(/test_user/) @@ -21,7 +22,6 @@ describe 'users/show.html.haml' do it 'does not show github if blank' do user.github_login = ' ' - assign :user, user render rendered.should_not match(/GitHub/) end @@ -30,7 +30,6 @@ describe 'users/show.html.haml' do context "Linking GitHub account" do context 'viewing another user page' do it "doesn't show and github linking buttons if user is not current user" do - assign :user, user render view.content_for(:action_bar).should_not include('Link GitHub account') view.content_for(:action_bar).should_not include('Unlink GitHub account') @@ -40,7 +39,6 @@ describe 'users/show.html.haml' do context 'viewing own user page' do before do controller.stub(:current_user) { user } - assign :user, user end it 'shows link github button when no login or token' do -- libgit2 0.21.2