From f24094442a753a86f58d203627e8894a9603f4b9 Mon Sep 17 00:00:00 2001 From: Cyril Mougel Date: Thu, 6 Jun 2013 00:46:16 +0200 Subject: [PATCH] Avoid user delete admin access of current user logged --- CHANGELOG.md | 3 +++ app/controllers/users_controller.rb | 26 +++++++++++++++----------- config/locales/en.yml | 2 ++ spec/controllers/users_controller_spec.rb | 54 +++++++++++++++++++++++++++++++++++++++--------------- 4 files changed, 59 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cd6030..aef66b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ - Update some gems ([@shingara][]) - [#492][] Improve some Pjax call ([@nfedyashev][]) - [#428][] Add the support of Unfuddle Tracker ([@parallel588][]) +- Avoid to delete his own user ([@shingara][]) +- [#456] Avoid to delete admin access of current user logged ([@shingara][]) ### Bug Fixes @@ -48,6 +50,7 @@ [#428]: https://github.com/errbit/errbit/issues/428 [#453]: https://github.com/errbit/errbit/issues/453 [#455]: https://github.com/errbit/errbit/issues/455 +[#456]: https://github.com/errbit/errbit/issues/456 [#457]: https://github.com/errbit/errbit/issues/457 [#460]: https://github.com/errbit/errbit/issues/460 [#466]: https://github.com/errbit/errbit/issues/466 diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index c8e31de..f06e30a 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -25,26 +25,25 @@ class UsersController < ApplicationController end def update - # Devise Hack - # 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" + flash[:success] = I18n.t('controllers.users.flash.update.success', :name => user.name) redirect_to user_path(user) else render :edit end end + ## + # Destroy the user pass in args + # + # @param [ String ] id the id of user we want delete + # def destroy if user == current_user flash[:error] = I18n.t('controllers.users.flash.destroy.error') else UserDestroy.new(user).destroy - flash[:success] = "That's sad. #{user.name} is no longer part of your team." + flash[:success] = I18n.t('controllers.users.flash.destroy.success', :name => user.name) end redirect_to users_path end @@ -62,14 +61,19 @@ class UsersController < ApplicationController end def user_params - params[:user] ? params.require(:user).permit(*user_permit_params) : {} + @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 ||= [:name,:username, :email, :github_login, :per_page, :time_zone] + @user_permit_params << :admin if current_user.admin? && current_user.id != params[:id] + @user_permit_params |= [:password, :password_confirmation] if user_password_params.values.all?{|pa| !pa.blank? } @user_permit_params end + def user_password_params + @user_password_params ||= params[:user] ? params.require(:user).permit(:password, :password_confirmation) : {} + end + end diff --git a/config/locales/en.yml b/config/locales/en.yml index 12d010f..79a017c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -22,3 +22,5 @@ en: destroy: success: "That's sad. %{name} is no longer part of your team." error: "You can't delete yourself" + update: + success: "%{name}'s information was successfully updated." diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 5d4ebdc..57f3e16 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -183,24 +183,18 @@ describe UsersController do context "PUT /users/:id" do context "when the update is successful" do + before { + put :update, :id => user.to_param, :user => user_params + } - it "sets a message to display" do - 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)) - end - - it "should be able to make user an admin" do - put :update, :id => user.to_param, :user => {:admin => true} - response.should be_redirect - User.find(controller.user.to_param).admin.should be_true + context "with normal params" do + let(:user_params) { {:name => 'Kermit'} } + it "sets a message to display" do + expect(request.flash[:success]).to eq I18n.t('controllers.users.flash.update.success', :name => user.name) + expect(response).to redirect_to(user_path(user)) + end end end - context "when the update is unsuccessful" do it "renders the edit page" do @@ -238,6 +232,36 @@ describe UsersController do end end end + + describe "#user_params" do + context "with current user not admin" do + before { + controller.stub(:current_user).and_return(user) + controller.stub(:params).and_return(ActionController::Parameters.new(user_param)) + } + let(:user_param) { {'user' => { :name => 'foo', :admin => true }} } + it 'not have admin field' do + expect(controller.send(:user_params)).to eq ({'name' => 'foo'}) + end + context "with password and password_confirmation empty?" do + let(:user_param) { {'user' => { :name => 'foo', 'password' => '', 'password_confirmation' => '' }} } + it 'not have password and password_confirmation field' do + expect(controller.send(:user_params)).to eq ({'name' => 'foo'}) + end + end + end + + context "with current user admin" do + it 'have admin field' + context "with password and password_confirmation empty?" do + it 'not have password and password_confirmation field' + end + context "on his own user" do + it 'not have admin field' + end + end + end + end end -- libgit2 0.21.2