From 413d5a68fb4ec7421e9ddb760b537ac3c1b8ed5a Mon Sep 17 00:00:00 2001
From: mmd-osm <mmd.osm@gmail.com>
Date: Thu, 7 Dec 2017 19:04:16 +0100
Subject: [PATCH 1/2] Revoking administrator role on current user should fail
 Fixes #1697

---
 app/controllers/user_roles_controller.rb       | 11 +++++++++++
 config/locales/en.yml                          |  1 +
 test/controllers/user_roles_controller_test.rb |  4 ++++
 3 files changed, 16 insertions(+)

diff --git a/app/controllers/user_roles_controller.rb b/app/controllers/user_roles_controller.rb
index 536790dc5b..9db9019eb0 100644
--- a/app/controllers/user_roles_controller.rb
+++ b/app/controllers/user_roles_controller.rb
@@ -8,6 +8,7 @@ class UserRolesController < ApplicationController
   before_action :require_valid_role
   before_action :not_in_role, :only => [:grant]
   before_action :in_role, :only => [:revoke]
+  before_action :not_revoke_admin_current_user
 
   def grant
     @this_user.roles.create(:role => @role, :granter => current_user)
@@ -59,4 +60,14 @@ def in_role
       redirect_to :controller => "user", :action => "view", :display_name => @this_user.display_name
     end
   end
+
+  ##
+  # checks that administrator role is not revoked from current user
+  def not_revoke_admin_current_user
+    @role = params[:role]
+    if current_user == @this_user && @role == "administrator"
+      flash[:error] = t("user_role.filter.not_revoke_admin_current_user")
+      redirect_to :controller => "user", :action => "view", :display_name => @this_user.display_name
+    end
+  end
 end
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 062fd95d48..3b6eb87349 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -2035,6 +2035,7 @@ en:
       not_a_role: "The string `%{role}' is not a valid role."
       already_has_role: "The user already has role %{role}."
       doesnt_have_role: "The user does not have role %{role}."
+      not_revoke_admin_current_user: "Cannot revoke administrator role from current user."
     grant:
       title: Confirm role granting
       heading: Confirm role granting
diff --git a/test/controllers/user_roles_controller_test.rb b/test/controllers/user_roles_controller_test.rb
index f73fc90562..b982ff9d5d 100644
--- a/test/controllers/user_roles_controller_test.rb
+++ b/test/controllers/user_roles_controller_test.rb
@@ -134,5 +134,9 @@ def test_revoke
     end
     assert_redirected_to user_path(target_user.display_name)
     assert_equal "The string `no_such_role' is not a valid role.", flash[:error]
+
+    # Revoking administrator role from current user should fail
+    post :revoke, :params => { :display_name => administrator_user.display_name, :role => "administrator" }
+    assert_redirected_to user_path(administrator_user.display_name)
   end
 end

From 84ba71755714a9896feea3cf88d9a42c7326e4f1 Mon Sep 17 00:00:00 2001
From: mmd-osm <mmd.osm@gmail.com>
Date: Mon, 18 Dec 2017 22:56:05 +0100
Subject: [PATCH 2/2] Suggested code changes Getting rid of filter Asserting
 flash msg is shown

---
 app/controllers/user_roles_controller.rb       | 18 ++++++------------
 test/controllers/user_roles_controller_test.rb |  1 +
 2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/app/controllers/user_roles_controller.rb b/app/controllers/user_roles_controller.rb
index 9db9019eb0..5ef68216bf 100644
--- a/app/controllers/user_roles_controller.rb
+++ b/app/controllers/user_roles_controller.rb
@@ -8,7 +8,6 @@ class UserRolesController < ApplicationController
   before_action :require_valid_role
   before_action :not_in_role, :only => [:grant]
   before_action :in_role, :only => [:revoke]
-  before_action :not_revoke_admin_current_user
 
   def grant
     @this_user.roles.create(:role => @role, :granter => current_user)
@@ -16,7 +15,12 @@ def grant
   end
 
   def revoke
-    UserRole.where(:user_id => @this_user.id, :role => @role).delete_all
+    # checks that administrator role is not revoked from current user
+    if current_user == @this_user && @role == "administrator"
+      flash[:error] = t("user_role.filter.not_revoke_admin_current_user")
+    else
+      UserRole.where(:user_id => @this_user.id, :role => @role).delete_all
+    end
     redirect_to :controller => "user", :action => "view", :display_name => @this_user.display_name
   end
 
@@ -60,14 +64,4 @@ def in_role
       redirect_to :controller => "user", :action => "view", :display_name => @this_user.display_name
     end
   end
-
-  ##
-  # checks that administrator role is not revoked from current user
-  def not_revoke_admin_current_user
-    @role = params[:role]
-    if current_user == @this_user && @role == "administrator"
-      flash[:error] = t("user_role.filter.not_revoke_admin_current_user")
-      redirect_to :controller => "user", :action => "view", :display_name => @this_user.display_name
-    end
-  end
 end
diff --git a/test/controllers/user_roles_controller_test.rb b/test/controllers/user_roles_controller_test.rb
index b982ff9d5d..f9e3214062 100644
--- a/test/controllers/user_roles_controller_test.rb
+++ b/test/controllers/user_roles_controller_test.rb
@@ -138,5 +138,6 @@ def test_revoke
     # Revoking administrator role from current user should fail
     post :revoke, :params => { :display_name => administrator_user.display_name, :role => "administrator" }
     assert_redirected_to user_path(administrator_user.display_name)
+    assert_equal "Cannot revoke administrator role from current user.", flash[:error]
   end
 end