Merge pull request #14607 from rabbitmq/mergify/bp/v4.2.x/pr-14545

Check for protected tag during user update and deletion via management API (backport #14545)

Author: michaelklishin

deps/rabbitmq_management/src/rabbit_mgmt_util.erl

@@ -53,6 +53,8 @@
 
 -export([set_resp_not_found/2]).
 
+-export([is_protected_user/1]).
+
 -import(rabbit_misc, [pget/2]).
 
 -include("rabbit_mgmt.hrl").
@@ -208,6 +210,14 @@ is_authorized_user(ReqData, Context, Username, Password, ReplyWhenFailed) ->
                                                 ReplyWhenFailed,
                                                 auth_config()).
 
+is_protected_user(Username) ->
+    case rabbit_auth_backend_internal:lookup_user(Username) of
+        {ok, User} ->
+            Tags = internal_user:get_tags(User),
+            rabbit_web_dispatch_access_control:is_protected_user(Tags);
+        {error, _} -> false
+    end.
+
 vhost_from_headers(ReqData) ->
     rabbit_web_dispatch_access_control:vhost_from_headers(ReqData).
 

deps/rabbitmq_management/src/rabbit_mgmt_wm_user.erl

@@ -46,17 +46,31 @@ to_json(ReqData, Context) ->
 
 accept_content(ReqData0, Context = #context{user = #user{username = ActingUser}}) ->
     Username = rabbit_mgmt_util:id(user, ReqData0),
-    rabbit_mgmt_util:with_decode(
-      [], ReqData0, Context,
-      fun(_, User, ReqData) ->
-              _ = put_user(User#{name => Username}, ActingUser),
-              {true, ReqData, Context}
-      end).
+    case rabbit_mgmt_util:is_protected_user(Username) of
+        true ->
+            rabbit_mgmt_util:bad_request(
+              <<"User updates via API are disabled for this user">>,
+              ReqData0, Context);
+        false ->
+            rabbit_mgmt_util:with_decode(
+              [], ReqData0, Context,
+              fun(_, User, ReqData) ->
+                      _ = put_user(User#{name => Username}, ActingUser),
+                      {true, ReqData, Context}
+              end)
+    end.
 
 delete_resource(ReqData, Context = #context{user = #user{username = ActingUser}}) ->
     User = rabbit_mgmt_util:id(user, ReqData),
-    rabbit_auth_backend_internal:delete_user(User, ActingUser),
-    {true, ReqData, Context}.
+    case rabbit_mgmt_util:is_protected_user(User) of
+        true ->
+            rabbit_mgmt_util:bad_request(
+              <<"User deletion via API is disabled for this user">>,
+              ReqData, Context);
+        false ->
+            rabbit_auth_backend_internal:delete_user(User, ActingUser),
+            {true, ReqData, Context}
+    end.
 
 is_authorized(ReqData, Context) ->
     rabbit_mgmt_util:is_authorized_admin(ReqData, Context).

deps/rabbitmq_management/test/rabbit_mgmt_http_SUITE.erl

@@ -77,6 +77,7 @@ groups() ->
 some_tests() ->
     [
         users_test,
+        users_protected_test,
         exchanges_test,
         queues_test,
         bindings_test,
@@ -657,6 +658,21 @@ users_test(Config) ->
     http_get(Config, "/users/users_test", ?NOT_FOUND),
     passed.
 
+users_protected_test(Config) ->
+    ProtectedUser = <<"protected_user">>,
+    rabbit_ct_broker_helpers:add_user(Config, ProtectedUser),
+    rabbit_ct_broker_helpers:set_user_tags(Config, 0, ProtectedUser, [management, protected]),
+
+    %% Verify protected user cannot be updated via API
+    http_put(Config, "/users/protected_user", [{password, <<"new_password">>},
+                                               {tags, <<"management,protected">>}], ?BAD_REQUEST),
+
+    %% Verify protected user cannot be deleted via API
+    http_delete(Config, "/users/protected_user", ?BAD_REQUEST),
+
+    rabbit_ct_broker_helpers:delete_user(Config, ProtectedUser),
+    passed.
+
 without_permissions_users_test(Config) ->
     assert_item(#{name => <<"guest">>, tags => [<<"administrator">>]},
                 http_get(Config, "/whoami")),

deps/rabbitmq_web_dispatch/src/rabbit_web_dispatch_access_control.erl

@@ -24,7 +24,7 @@
 -export([id/2]).
 -export([not_authorised/3, halt_response/5]).
 
--export([is_admin/1, is_policymaker/1, is_monitor/1, is_mgmt_user/1]).
+-export([is_admin/1, is_policymaker/1, is_monitor/1, is_mgmt_user/1, is_protected_user/1]).
 
 -import(rabbit_misc, [pget/2]).
 
@@ -243,8 +243,12 @@ is_policymaker(T) -> intersects(T, [administrator, policymaker]).
 is_monitor(T)     -> intersects(T, [administrator, monitoring]).
 is_mgmt_user(T)   -> intersects(T, [administrator, monitoring, policymaker,
                                     management]).
+is_protected_user(T) -> intersects(T, [protected]).
 
-intersects(A, B) -> lists:any(fun(I) -> lists:member(I, B) end, A).
+intersects(A, [B]) ->
+    lists:member(B, A);
+intersects(A, B) ->
+    lists:any(fun(I) -> lists:member(I, B) end, A).
 
 user_matches_vhost(ReqData, User) ->
     case vhost(ReqData) of