Skip to content

Commit

Permalink
Skip access check on absent will queue
Browse files Browse the repository at this point in the history
Resolves rabbitmq#11021

Prior to this commit, an MQTT client that connects to RabbitMQ needed
configure access to its will queue even if the will queue has never
existed. This breaks client apps connecting with either v3 or v4 or with
v5 without making use of the Will-Delay-Interval.

Specifically, in 3.13.0 and 3.13.1 an MQTT client that connects to
RabbitMQ needs unnecessarily configure access to queue
`mqtt-will-<MQTT client ID>`.

This commit only check for configure access, if the queue actually gets
deleted, i.e. if it existed.
  • Loading branch information
ansd committed Apr 17, 2024
1 parent 2ff5944 commit bb106ff
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 26 deletions.
50 changes: 26 additions & 24 deletions deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl
Original file line number Diff line number Diff line change
Expand Up @@ -1846,30 +1846,32 @@ delete_queue(QName,
user = User = #user{username = Username},
authz_ctx = AuthzCtx}}) ->
%% configure access to queue required for queue.delete
case check_resource_access(User, QName, configure, AuthzCtx) of
ok ->
case rabbit_amqqueue:with(
QName,
fun (Q) ->
rabbit_queue_type:delete(Q, false, false, Username)
end,
fun (not_found) ->
ok;
({absent, Q, crashed}) ->
rabbit_classic_queue:delete_crashed(Q, Username);
({absent, Q, stopped}) ->
rabbit_classic_queue:delete_crashed(Q, Username);
({absent, _Q, _Reason}) ->
ok
end) of
{ok, _N} ->
ok;
ok ->
ok
end;
{error, access_refused} = E ->
E
end.
%% We only check access if the queue actually exists.
rabbit_amqqueue:with(
QName,
fun (Q) ->
case check_resource_access(User, QName, configure, AuthzCtx) of
ok ->
{ok, _N} = rabbit_queue_type:delete(Q, false, false, Username),
ok;
Err ->
Err
end
end,
fun (not_found) ->
ok;
({absent, Q, State})
when State =:= crashed orelse
State =:= stopped ->
case check_resource_access(User, QName, configure, AuthzCtx) of
ok ->
rabbit_classic_queue:delete_crashed(Q, Username);
Err ->
Err
end;
({absent, _Q, _State}) ->
ok
end).

-spec handle_pre_hibernate() -> ok.
handle_pre_hibernate() ->
Expand Down
12 changes: 10 additions & 2 deletions deps/rabbitmq_mqtt/test/auth_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ sub_groups() ->
topic_read_permission,
topic_write_permission,
topic_write_permission_variable_expansion,
loopback_user_connects_from_remote_host
loopback_user_connects_from_remote_host,
connect_permission
]
},
{limit, [shuffle],
Expand Down Expand Up @@ -284,7 +285,8 @@ init_per_testcase(T, Config)
when T =:= will_queue_create_permission_queue_read;
T =:= will_queue_create_permission_exchange_write;
T =:= will_queue_publish_permission_exchange_write;
T =:= will_queue_publish_permission_topic_write ->
T =:= will_queue_publish_permission_topic_write;
T =:= will_queue_delete_permission ->
case ?config(mqtt_version, Config) of
v4 -> {skip, "Will Delay Interval is an MQTT 5.0 feature"};
v5 -> testcase_started(Config, T)
Expand Down Expand Up @@ -982,6 +984,12 @@ loopback_user_connects_from_remote_host(Config) ->
true = rpc(Config, 0, meck, validate, [Mod]),
ok = rpc(Config, 0, meck, unload, [Mod]).

%% No specific configure, write, or read permissions should be required for only connecting.
connect_permission(Config) ->
set_permissions("", "", "", Config),
C = open_mqtt_connection(Config),
ok = emqtt:disconnect(C).

set_topic_permissions(WritePat, ReadPat, Config) ->
rpc(Config, 0,
rabbit_auth_backend_internal, set_topic_permissions,
Expand Down

0 comments on commit bb106ff

Please sign in to comment.