Skip to content

A follow-up to #9550 #10062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion deps/rabbitmq_management/src/rabbit_mgmt_util.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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]).
Expand Down Expand Up @@ -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),
Expand Down
15 changes: 14 additions & 1 deletion deps/rabbitmq_management/src/rabbit_mgmt_wm_queue.erl
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,14 @@ delete_resource(ReqData, Context = #context{user = #user{username = ActingUser}}
end.

is_authorized(ReqData, Context) ->
rabbit_mgmt_util:is_authorized_vhost(ReqData, Context).
case cowboy_req:method(ReqData) of
<<"DELETE">> ->
has_resource_permissions(configure, ReqData, Context);
<<"PUT">> ->
has_resource_permissions(configure, ReqData, Context);
_ ->
rabbit_mgmt_util:is_authorized_vhost(ReqData, Context)
end.

%%--------------------------------------------------------------------

Expand Down Expand Up @@ -128,3 +135,9 @@ queue(VHost, QName) ->
{ok, Q} -> rabbit_mgmt_format:queue(Q);
{error, not_found} -> not_found
end.

has_resource_permissions(Permission, 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, Permission).
17 changes: 16 additions & 1 deletion deps/rabbitmq_management/test/rabbit_mgmt_http_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]).
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down