A follow-up to #9550

Author: michaelklishin

deps/rabbitmq_management/src/rabbit_mgmt_util.erl

@@ -16,7 +16,9 @@
          is_authorized_monitor/2, is_authorized_policies/2,
          is_authorized_vhost_visible/2,
          is_authorized_vhost_visible_for_monitoring/2,
-         is_authorized_global_parameters/2]).
+         is_authorized_global_parameters/2,
+         is_authorized_vhost_and_has_resource_permission/4
+]).
 -export([user/1]).
 -export([bad_request/3, service_unavailable/3, bad_request_exception/4, internal_server_error/4,
          id/2, parse_bool/1, parse_int/1, redirect_to_home/3]).
@@ -120,6 +122,10 @@ is_authorized_vhost_visible_for_monitoring(ReqData, Context) ->
                                                                         Context,
                                                                         auth_config()).
 
+is_authorized_vhost_and_has_resource_permission(ReqData, Context, Resource, Permission) ->
+    rabbit_web_dispatch_access_control:is_authorized_vhost_and_has_resource_permission(
+        ReqData, Context, Resource, Permission, auth_config()).
+
 auth_config() ->
     BasicAuthEnabled = not get_bool_env(rabbitmq_management, disable_basic_auth, false),
     OauthEnabled = get_bool_env(rabbitmq_management, oauth_enabled, false),

deps/rabbitmq_management/src/rabbit_mgmt_wm_queue.erl

@@ -96,7 +96,10 @@ delete_resource(ReqData, Context = #context{user = #user{username = ActingUser}}
    end.
 
 is_authorized(ReqData, Context) ->
-    rabbit_mgmt_util:is_authorized_vhost(ReqData, Context).
+    VHost = rabbit_mgmt_util:id(vhost, ReqData),
+    QName = rabbit_mgmt_util:id(queue, ReqData),
+    QRes  = rabbit_misc:r(VHost, queue, QName),
+    rabbit_mgmt_util:is_authorized_vhost_and_has_resource_permission(ReqData, Context, QRes, configure).
 
 %%--------------------------------------------------------------------
 

deps/rabbitmq_management/test/rabbit_mgmt_http_SUITE.erl

@@ -93,6 +93,7 @@ all_tests() -> [
     permissions_administrator_test,
     permissions_vhost_test,
     permissions_amqp_test,
+    permissions_queue_delete_test,
     permissions_connection_channel_consumer_test,
     consumers_cq_test,
     consumers_qq_test,
@@ -148,7 +149,9 @@ all_tests() -> [
     rates_test,
     single_active_consumer_cq_test,
     single_active_consumer_qq_test,
-%%    oauth_test,  %% disabled until we are able to enable oauth2 plugin
+    %% This needs OAuth 2 plugin to be enabled on the node. How to best do that with Bazel and CT
+    %% remains an open question right now.
+    %% oauth_test,
     disable_basic_auth_test,
     login_test,
     csp_headers_test,
@@ -1427,6 +1430,18 @@ permissions_amqp_test(Config) ->
     http_delete(Config, "/users/myuser", {group, '2xx'}),
     passed.
 
+permissions_queue_delete_test(Config) ->
+    QArgs = #{},
+    PermArgs = [{configure, <<"foo.*">>}, {write, <<".*">>}, {read, <<".*">>}],
+    http_put(Config, "/users/myuser", [{password, <<"myuser">>},
+                                       {tags, <<"management">>}], {group, '2xx'}),
+    http_put(Config, "/permissions/%2F/myuser", PermArgs, {group, '2xx'}),
+    http_put(Config, "/queues/%2F/bar-queue", QArgs, {group, '2xx'}),
+    http_delete(Config, "/queues/%2F/bar-queue", "myuser", "myuser", ?NOT_AUTHORISED),
+    http_delete(Config, "/queues/%2F/bar-queue", {group, '2xx'}),
+    http_delete(Config, "/users/myuser", {group, '2xx'}),
+    passed.
+
 %% Opens a new connection and a channel on it.
 %% The channel is not managed by rabbit_ct_client_helpers and
 %% should be explicitly closed by the caller.

deps/rabbitmq_web_dispatch/src/rabbit_web_dispatch_access_control.erl

@@ -17,7 +17,9 @@
          is_authorized_monitor/3, is_authorized_policies/3,
          is_authorized_vhost_visible/3,
          is_authorized_vhost_visible_for_monitoring/3,
-         is_authorized_global_parameters/3]).
+         is_authorized_global_parameters/3,
+         is_authorized_vhost_and_has_resource_permission/5
+]).
 
 -export([list_visible_vhosts/1, list_visible_vhosts_names/1, list_login_vhosts/2]).
 -export([id/2]).
@@ -176,12 +178,12 @@ is_authorized(ReqData, Context, Username, Password, ErrorMsg, Fun, AuthConfig, R
     end.
 
 
-%% Used for connections / channels. A normal user can only see / delete
+%% Used for connections and channels. A normal user can only see / delete
 %% their own stuff. Monitors can see other users' and delete their
 %% own. Admins can do it all.
 is_authorized_user(ReqData, Context, Item, AuthConfig) ->
     is_authorized(ReqData, Context,
-                  <<"User not authorised to access object">>,
+                  <<"User not authorised to access the resource(s)">>,
                   fun(#user{username = Username, tags = Tags}) ->
                           case cowboy_req:method(ReqData) of
                               <<"DELETE">> -> is_admin(Tags);
@@ -190,11 +192,11 @@ is_authorized_user(ReqData, Context, Item, AuthConfig) ->
                   end,
                   AuthConfig).
 
-%% For policies / parameters. Like is_authorized_vhost but you have to
+%% For policies and parameters. Like is_authorized_vhost but you have to
 %% be a policymaker.
 is_authorized_policies(ReqData, Context, AuthConfig) ->
     is_authorized(ReqData, Context,
-                  <<"User not authorised to access object">>,
+                  <<"User not authorised to access the resource(s)">>,
                   fun(User = #user{tags = Tags}) ->
                           is_admin(Tags) orelse
                                            (is_policymaker(Tags) andalso
@@ -211,6 +213,23 @@ is_authorized_global_parameters(ReqData, Context, AuthConfig) ->
                   end,
                   AuthConfig).
 
+is_authorized_vhost_and_has_resource_permission(ReqData, Context, Resource, Permission, AuthConfig) ->
+    is_authorized(ReqData, Context,
+                  <<"User not authorised to access this resource">>,
+                  fun(User) ->
+                      try
+                          AuthzData = get_authz_data_as_map(ReqData),
+                          ok =:= rabbit_access_control:check_resource_access(User, Resource, Permission, AuthzData)
+                      catch
+                          exit:Err:_Stacktrace ->
+                              #amqp_error{explanation = Msg} = Err,
+                              _ = rabbit_log:error(Msg),
+                              false
+                      end
+                  end,
+                  AuthConfig).
+
+
 vhost_from_headers(ReqData) ->
     case cowboy_req:header(<<"x-vhost">>, ReqData) of
         undefined -> none;
@@ -263,6 +282,9 @@ get_authz_data(ReqData) ->
     {PeerAddress, _PeerPort} = cowboy_req:peer(ReqData),
     {ip, PeerAddress}.
 
+get_authz_data_as_map(ReqData) ->
+    {PeerAddress, _PeerPort} = cowboy_req:peer(ReqData),
+    #{ip => PeerAddress}.
 
 not_authorised(Reason, ReqData, Context) ->
     %% TODO: consider changing to 403 in 4.0