Avoid user delete admin access of current user logged

Fix errbit#456

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

app/controllers/users_controller.rb

@@ -25,26 +25,25 @@ def create
   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 @@ def require_user_edit_priviledges
     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
 

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."

spec/controllers/users_controller_spec.rb

@@ -183,24 +183,18 @@
 
     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 @@
         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