diff --git a/deps/rabbit/Makefile b/deps/rabbit/Makefile index 8904826192..ff47efe0a8 100644 --- a/deps/rabbit/Makefile +++ b/deps/rabbit/Makefile @@ -115,6 +115,7 @@ define PROJECT_ENV {tracking_execution_timeout, 15000}, {stream_messages_soft_limit, 256}, {track_auth_attempt_source, false}, + {authorization_failure_disclosure, false}, {credentials_obfuscation_fallback_secret, <<"nocookie">>}, {dead_letter_worker_consumer_prefetch, 32}, {dead_letter_worker_publisher_confirm_timeout, 180000}, diff --git a/deps/rabbit/src/rabbit_access_control.erl b/deps/rabbit/src/rabbit_access_control.erl index b0b2ecf0f8..080b8869d9 100644 --- a/deps/rabbit/src/rabbit_access_control.erl +++ b/deps/rabbit/src/rabbit_access_control.erl @@ -361,6 +361,15 @@ check_access(Fun, Module, ErrStr, ErrArgs, ErrName) -> ok; false -> rabbit_misc:protocol_error(ErrName, ErrStr, ErrArgs); + {false, Reason} -> + case application:get_env(rabbit, authorization_failure_disclosure) of + {ok, true} -> + FullErrStr = ErrStr ++ " by backend ~ts: ~ts", + FullErrArgs = ErrArgs ++ [Module, Reason], + rabbit_misc:protocol_error(ErrName, FullErrStr, FullErrArgs); + _ -> + rabbit_misc:protocol_error(ErrName, ErrStr, ErrArgs) + end; {error, E} -> FullErrStr = ErrStr ++ ", backend ~ts returned an error: ~tp", FullErrArgs = ErrArgs ++ [Module, E], diff --git a/deps/rabbit/src/rabbit_auth_backend_internal.erl b/deps/rabbit/src/rabbit_auth_backend_internal.erl index 09bf96f6a1..d651845439 100644 --- a/deps/rabbit/src/rabbit_auth_backend_internal.erl +++ b/deps/rabbit/src/rabbit_auth_backend_internal.erl @@ -143,7 +143,8 @@ check_resource_access(#auth_user{username = Username}, _AuthContext) -> case rabbit_db_user:get_user_permissions(Username, VHostPath) of undefined -> - false; + {false, rabbit_misc:format("user '~ts' has no permissions for vhost '~ts'", + [Username, VHostPath])}; #user_permission{permission = P} -> PermRegexp = case element(permission_index(Permission), P) of %% <<"^$">> breaks Emacs' erlang mode @@ -151,8 +152,12 @@ check_resource_access(#auth_user{username = Username}, RE -> RE end, case re:run(Name, PermRegexp, [{capture, none}]) of - match -> true; - nomatch -> false + match -> + true; + nomatch -> + {false, rabbit_misc:format( + "'~ts' does not match the permission regex '~ts'", + [Name, PermRegexp])} end end. @@ -170,12 +175,17 @@ check_topic_access(#auth_user{username = Username}, RE -> RE end, PermRegexpExpanded = expand_topic_permission( - PermRegexp, - maps:get(variable_map, Context, undefined) - ), - case re:run(maps:get(routing_key, Context), PermRegexpExpanded, [{capture, none}]) of - match -> true; - nomatch -> false + PermRegexp, + maps:get(variable_map, Context, undefined) + ), + Topic = maps:get(routing_key, Context), + case re:run(Topic, PermRegexpExpanded, [{capture, none}]) of + match -> + true; + nomatch -> + {false, rabbit_misc:format( + "topic '~ts' does not match the regex '~ts'", + [Topic, PermRegexpExpanded])} end end. diff --git a/deps/rabbit/src/rabbit_authz_backend.erl b/deps/rabbit/src/rabbit_authz_backend.erl index fdf9c6c561..eb8c0c41ad 100644 --- a/deps/rabbit/src/rabbit_authz_backend.erl +++ b/deps/rabbit/src/rabbit_authz_backend.erl @@ -32,25 +32,27 @@ %% Possible responses: %% true %% false +%% {false, Reason} %% {error, Error} %% Something went wrong. Log and die. -callback check_vhost_access(AuthUser :: rabbit_types:auth_user(), VHost :: rabbit_types:vhost(), AuthzData :: rabbit_types:authz_data()) -> - boolean() | {'error', any()}. + boolean() | {false, Reason :: string()} | {'error', any()}. %% Given #auth_user, resource and permission, can a user access a resource? %% %% Possible responses: %% true %% false +%% {false, Reason} %% {error, Error} %% Something went wrong. Log and die. -callback check_resource_access(rabbit_types:auth_user(), rabbit_types:r(atom()), rabbit_types:permission_atom(), rabbit_types:authz_context()) -> - boolean() | {'error', any()}. + boolean() | {false, Reason :: string()} | {'error', any()}. %% Given #auth_user, topic as resource, permission, and context, can a user access the topic? %% @@ -63,7 +65,7 @@ rabbit_types:r(atom()), rabbit_types:permission_atom(), rabbit_types:topic_access_context()) -> - boolean() | {'error', any()}. + boolean() | {false, Reason :: string()} | {'error', any()}. %% Updates backend state that has expired. %% diff --git a/deps/rabbit/test/topic_permission_SUITE.erl b/deps/rabbit/test/topic_permission_SUITE.erl index 6b3ff76f44..9e7a350159 100644 --- a/deps/rabbit/test/topic_permission_SUITE.erl +++ b/deps/rabbit/test/topic_permission_SUITE.erl @@ -36,8 +36,10 @@ init_per_suite(Config) -> Config1 = rabbit_ct_helpers:set_config( Config, [{rmq_nodename_suffix, ?MODULE}]), + Config2 = rabbit_ct_helpers:merge_app_env( + Config1, {rabbit, [{authorization_failure_disclosure, true}]}), rabbit_ct_helpers:run_setup_steps( - Config1, + Config2, rabbit_ct_broker_helpers:setup_steps() ++ rabbit_ct_client_helpers:setup_steps()). @@ -109,10 +111,12 @@ amqp_x_cc_annotation(Config) -> condition = ?V_1_0_AMQP_ERROR_UNAUTHORIZED_ACCESS, description = {utf8, Description1}}}}} -> ?assertEqual( - <<"write access to topic 'x.1' in exchange 'amq.topic' in vhost '/' refused for user 'guest'">>, + <<"write access to topic 'x.1' in exchange 'amq.topic' in vhost '/' " + "refused for user 'guest' by backend rabbit_auth_backend_internal: " + "topic 'x.1' does not match the regex '^a'">>, Description1) after 30_000 -> amqp_utils:flush(missing_ended), - ct:fail({missing_event, ?LINE}) + ct:fail({missing_event, ?LINE}) end, {ok, Session3} = amqp10_client:begin_session_sync(Connection), @@ -132,10 +136,12 @@ amqp_x_cc_annotation(Config) -> condition = ?V_1_0_AMQP_ERROR_UNAUTHORIZED_ACCESS, description = {utf8, Description2}}}}} -> ?assertEqual( - <<"write access to topic 'x.2' in exchange 'amq.topic' in vhost '/' refused for user 'guest'">>, + <<"write access to topic 'x.2' in exchange 'amq.topic' in vhost '/' " + "refused for user 'guest' by backend rabbit_auth_backend_internal: " + "topic 'x.2' does not match the regex '^a'">>, Description2) after 30_000 -> amqp_utils:flush(missing_ended), - ct:fail({missing_event, ?LINE}) + ct:fail({missing_event, ?LINE}) end, {ok, #{message_count := 0}} = rabbitmq_amqp_client:delete_queue(LinkPair, QName1), @@ -190,8 +196,9 @@ amqpl_headers(Header, Config) -> props = #'P_basic'{headers = [{Header, array, [{longstr, <<"a.2">>}]}]}}), ok = assert_channel_down( Ch1, - <<"ACCESS_REFUSED - write access to topic 'x.1' in exchange " - "'amq.topic' in vhost '/' refused for user 'guest'">>), + <<"ACCESS_REFUSED - write access to topic 'x.1' in exchange 'amq.topic' " + "in vhost '/' refused for user 'guest' by backend rabbit_auth_backend_internal: " + "topic 'x.1' does not match the regex '^a'">>), Ch2 = rabbit_ct_client_helpers:open_channel(Config), monitor(process, Ch2), @@ -203,8 +210,9 @@ amqpl_headers(Header, Config) -> props = #'P_basic'{headers = [{Header, array, [{longstr, <<"x.2">>}]}]}}), ok = assert_channel_down( Ch2, - <<"ACCESS_REFUSED - write access to topic 'x.2' in exchange " - "'amq.topic' in vhost '/' refused for user 'guest'">>), + <<"ACCESS_REFUSED - write access to topic 'x.2' in exchange 'amq.topic' " + "in vhost '/' refused for user 'guest' by backend rabbit_auth_backend_internal: " + "topic 'x.2' does not match the regex '^a'">>), Ch3 = rabbit_ct_client_helpers:open_channel(Config), ?assertEqual(#'queue.delete_ok'{message_count = 1}, @@ -337,12 +345,14 @@ topic_permission_checks1(_Config) -> Context ) || Perm <- Permissions], %% user has access to exchange, routing key does not match - [false = rabbit_auth_backend_internal:check_topic_access( - User, - Topic, - Perm, - #{routing_key => <<"x.y.z">>} - ) || Perm <- Permissions], + [?assertEqual( + {false, "topic 'x.y.z' does not match the regex '^a'"}, + rabbit_auth_backend_internal:check_topic_access( + User, + Topic, + Perm, + #{routing_key => <<"x.y.z">>} + )) || Perm <- Permissions], %% user has access to exchange but not on this vhost %% let pass when there's no match [true = rabbit_auth_backend_internal:check_topic_access( @@ -379,17 +389,20 @@ topic_permission_checks1(_Config) -> } ) || Perm <- Permissions], %% routing key KO - [false = rabbit_auth_backend_internal:check_topic_access( - User, - Topic#resource{virtual_host = <<"other-vhost">>}, - Perm, - #{routing_key => <<"services.default.accounts.dummy.notifications">>, - variable_map => #{ - <<"username">> => <<"guest">>, - <<"vhost">> => <<"other-vhost">> - } - } - ) || Perm <- Permissions], + [?assertEqual( + {false, "topic 'services.default.accounts.dummy.notifications' does not " + "match the regex 'services.other-vhost.accounts.guest.notifications'"}, + rabbit_auth_backend_internal:check_topic_access( + User, + Topic#resource{virtual_host = <<"other-vhost">>}, + Perm, + #{routing_key => <<"services.default.accounts.dummy.notifications">>, + variable_map => #{ + <<"username">> => <<"guest">>, + <<"vhost">> => <<"other-vhost">> + } + } + )) || Perm <- Permissions], ok. @@ -407,11 +420,10 @@ clear_topic_permissions(Config) -> Config, 0, rabbit_auth_backend_internal, clear_topic_permissions, [<<"guest">>, <<"/">>, <<"acting-user">>]). -assert_channel_down(Ch, Reason) -> +assert_channel_down(Ch, ExpectedReason) -> receive {'DOWN', _MonitorRef, process, Ch, - {shutdown, - {server_initiated_close, 403, Reason}}} -> - ok + {shutdown, {server_initiated_close, 403, ActualReason}}} -> + ?assertEqual(ExpectedReason, ActualReason) after 30_000 -> - ct:fail({did_not_receive, Reason}) + ct:fail({missing_down, ExpectedReason}) end. diff --git a/deps/rabbitmq_auth_backend_cache/src/rabbit_auth_backend_cache.erl b/deps/rabbitmq_auth_backend_cache/src/rabbit_auth_backend_cache.erl index a952ccb047..748cc37d19 100644 --- a/deps/rabbitmq_auth_backend_cache/src/rabbit_auth_backend_cache.erl +++ b/deps/rabbitmq_auth_backend_cache/src/rabbit_auth_backend_cache.erl @@ -36,30 +36,21 @@ user_login_authorization(Username, AuthProps) -> end). check_vhost_access(#auth_user{} = AuthUser, VHostPath, AuthzData) -> - with_cache(authz, {check_vhost_access, [AuthUser, VHostPath, AuthzData]}, - fun(true) -> success; - (false) -> refusal; - ({error, _} = Err) -> Err; - (_) -> unknown - end). + with_cache(authz, + {check_vhost_access, [AuthUser, VHostPath, AuthzData]}, + fun convert_backend_result/1). check_resource_access(#auth_user{} = AuthUser, #resource{} = Resource, Permission, AuthzContext) -> - with_cache(authz, {check_resource_access, [AuthUser, Resource, Permission, AuthzContext]}, - fun(true) -> success; - (false) -> refusal; - ({error, _} = Err) -> Err; - (_) -> unknown - end). + with_cache(authz, + {check_resource_access, [AuthUser, Resource, Permission, AuthzContext]}, + fun convert_backend_result/1). check_topic_access(#auth_user{} = AuthUser, #resource{} = Resource, Permission, Context) -> - with_cache(authz, {check_topic_access, [AuthUser, Resource, Permission, Context]}, - fun(true) -> success; - (false) -> refusal; - ({error, _} = Err) -> Err; - (_) -> unknown - end). + with_cache(authz, + {check_topic_access, [AuthUser, Resource, Permission, Context]}, + fun convert_backend_result/1). expiry_timestamp(_) -> never. @@ -67,6 +58,12 @@ expiry_timestamp(_) -> never. %% Implementation %% +convert_backend_result(true) -> success; +convert_backend_result(false) -> refusal; +convert_backend_result({false, _}) -> refusal; +convert_backend_result({error, _} = Err) -> Err; +convert_backend_result(_) -> unknown. + clear_cache_cluster_wide() -> Nodes = rabbit_nodes:list_running(), ?LOG_WARNING("Clearing auth_backend_cache in all nodes : ~p", [Nodes]), diff --git a/deps/rabbitmq_auth_backend_cache/src/rabbit_auth_cache.erl b/deps/rabbitmq_auth_backend_cache/src/rabbit_auth_cache.erl index a8171133e9..3de66c4e36 100644 --- a/deps/rabbitmq_auth_backend_cache/src/rabbit_auth_cache.erl +++ b/deps/rabbitmq_auth_backend_cache/src/rabbit_auth_cache.erl @@ -18,7 +18,7 @@ -callback clear() -> ok. expiration(TTL) -> - erlang:system_time(milli_seconds) + TTL. + erlang:system_time(millisecond) + TTL. expired(Exp) -> - erlang:system_time(milli_seconds) > Exp. + erlang:system_time(millisecond) > Exp. diff --git a/deps/rabbitmq_auth_backend_cache/test/rabbit_auth_backend_cache_SUITE.erl b/deps/rabbitmq_auth_backend_cache/test/rabbit_auth_backend_cache_SUITE.erl index 2c88b32860..98a512c18e 100644 --- a/deps/rabbitmq_auth_backend_cache/test/rabbit_auth_backend_cache_SUITE.erl +++ b/deps/rabbitmq_auth_backend_cache/test/rabbit_auth_backend_cache_SUITE.erl @@ -6,6 +6,7 @@ %% -module(rabbit_auth_backend_cache_SUITE). +-include_lib("eunit/include/eunit.hrl"). -include_lib("rabbit_common/include/rabbit.hrl"). -compile(export_all). @@ -89,14 +90,24 @@ access_response(Config) -> true = rpc(Config,rabbit_auth_backend_internal, check_resource_access, [Auth, AvailableResource, configure, #{}]), true = rpc(Config,rabbit_auth_backend_cache, check_resource_access, [Auth, AvailableResource, configure, #{}]), - false = rpc(Config,rabbit_auth_backend_internal, check_resource_access, [Auth, RestrictedResource, configure, #{}]), - false = rpc(Config,rabbit_auth_backend_cache, check_resource_access, [Auth, RestrictedResource, configure, #{}]), + ExpectedResourceAccess = {false, "user 'guest' has no permissions for vhost 'restricted'"}, + ?assertEqual( + ExpectedResourceAccess, + rpc(Config,rabbit_auth_backend_internal, check_resource_access, [Auth, RestrictedResource, configure, #{}])), + ?assertEqual( + ExpectedResourceAccess, + rpc(Config,rabbit_auth_backend_cache, check_resource_access, [Auth, RestrictedResource, configure, #{}])), true = rpc(Config,rabbit_auth_backend_internal, check_topic_access, [Auth, TopicResource, write, AuthorisedTopicContext]), true = rpc(Config,rabbit_auth_backend_cache, check_topic_access, [Auth, TopicResource, write, AuthorisedTopicContext]), - false = rpc(Config,rabbit_auth_backend_internal, check_topic_access, [Auth, TopicResource, write, RestrictedTopicContext]), - false = rpc(Config,rabbit_auth_backend_cache, check_topic_access, [Auth, TopicResource, write, RestrictedTopicContext]). + ExpectedTopicAccess = {false, "topic 'b.b' does not match the regex '^a'"}, + ?assertEqual( + ExpectedTopicAccess, + rpc(Config, rabbit_auth_backend_internal, check_topic_access, [Auth, TopicResource, write, RestrictedTopicContext])), + ?assertEqual( + ExpectedTopicAccess, + rpc(Config,rabbit_auth_backend_cache, check_topic_access, [Auth, TopicResource, write, RestrictedTopicContext])). cache_expiration(Config) -> AvailableVhost = <<"/">>, @@ -124,7 +135,7 @@ cache_expiration(Config) -> false = rpc(Config,rabbit_auth_backend_internal, check_vhost_access, [Auth, AvailableVhost, undefined]), true = rpc(Config,rabbit_auth_backend_cache, check_vhost_access, [Auth, AvailableVhost, undefined]), - false = rpc(Config,rabbit_auth_backend_internal, check_resource_access, [Auth, AvailableResource, configure, #{}]), + {false, _} = rpc(Config,rabbit_auth_backend_internal, check_resource_access, [Auth, AvailableResource, configure, #{}]), true = rpc(Config,rabbit_auth_backend_cache, check_resource_access, [Auth, AvailableResource, configure, #{}]), {ok, TTL} = rpc(Config, application, get_env, [rabbitmq_auth_backend_cache, cache_ttl]), @@ -135,8 +146,8 @@ cache_expiration(Config) -> false = rpc(Config,rabbit_auth_backend_internal, check_vhost_access, [Auth, AvailableVhost, undefined]), false = rpc(Config,rabbit_auth_backend_cache, check_vhost_access, [Auth, AvailableVhost, undefined]), - false = rpc(Config,rabbit_auth_backend_internal, check_resource_access, [Auth, AvailableResource, configure, #{}]), - false = rpc(Config,rabbit_auth_backend_cache, check_resource_access, [Auth, AvailableResource, configure, #{}]). + {false, _} = rpc(Config,rabbit_auth_backend_internal, check_resource_access, [Auth, AvailableResource, configure, #{}]), + {false, _} = rpc(Config,rabbit_auth_backend_cache, check_resource_access, [Auth, AvailableResource, configure, #{}]). cache_expiration_topic(Config) -> AvailableVhost = <<"/">>, @@ -153,14 +164,14 @@ cache_expiration_topic(Config) -> <<"guest">>, <<"/">>, <<"amq.topic">>, <<"^a">>, <<"^b">>, <<"acting-user">> ]), - false = rpc(Config,rabbit_auth_backend_internal, check_topic_access, [Auth, TopicResource, write, RestrictedTopicContext]), + {false, _} = rpc(Config,rabbit_auth_backend_internal, check_topic_access, [Auth, TopicResource, write, RestrictedTopicContext]), true = rpc(Config,rabbit_auth_backend_cache, check_topic_access, [Auth, TopicResource, write, RestrictedTopicContext]), {ok, TTL} = rpc(Config, application, get_env, [rabbitmq_auth_backend_cache, cache_ttl]), timer:sleep(TTL), - false = rpc(Config,rabbit_auth_backend_internal, check_topic_access, [Auth, TopicResource, write, RestrictedTopicContext]), - false = rpc(Config,rabbit_auth_backend_cache, check_topic_access, [Auth, TopicResource, write, RestrictedTopicContext]). + {false, _} = rpc(Config,rabbit_auth_backend_internal, check_topic_access, [Auth, TopicResource, write, RestrictedTopicContext]), + {false, _} = rpc(Config,rabbit_auth_backend_cache, check_topic_access, [Auth, TopicResource, write, RestrictedTopicContext]). rpc(Config, M, F, A) -> rabbit_ct_broker_helpers:rpc(Config, 0, M, F, A). diff --git a/deps/rabbitmq_auth_backend_http/src/rabbit_auth_backend_http.erl b/deps/rabbitmq_auth_backend_http/src/rabbit_auth_backend_http.erl index 119b943fab..13845a88ed 100644 --- a/deps/rabbitmq_auth_backend_http/src/rabbit_auth_backend_http.erl +++ b/deps/rabbitmq_auth_backend_http/src/rabbit_auth_backend_http.erl @@ -120,34 +120,34 @@ check_vhost_access(#auth_user{username = Username, tags = Tags}, VHost, do_check_vhost_access(Username, Tags, VHost, Ip, AuthzData) -> OptionsParameters = context_as_parameters(AuthzData), - bool_req(vhost_path, [{username, Username}, - {vhost, VHost}, - {ip, Ip}, - {tags, join_tags(Tags)}] ++ OptionsParameters). + req(vhost_path, [{username, Username}, + {vhost, VHost}, + {ip, Ip}, + {tags, join_tags(Tags)}] ++ OptionsParameters). check_resource_access(#auth_user{username = Username, tags = Tags}, #resource{virtual_host = VHost, kind = Type, name = Name}, Permission, AuthzContext) -> OptionsParameters = context_as_parameters(AuthzContext), - bool_req(resource_path, [{username, Username}, - {vhost, VHost}, - {resource, Type}, - {name, Name}, - {permission, Permission}, - {tags, join_tags(Tags)}] ++ OptionsParameters). + req(resource_path, [{username, Username}, + {vhost, VHost}, + {resource, Type}, + {name, Name}, + {permission, Permission}, + {tags, join_tags(Tags)}] ++ OptionsParameters). check_topic_access(#auth_user{username = Username, tags = Tags}, #resource{virtual_host = VHost, kind = topic = Type, name = Name}, Permission, Context) -> OptionsParameters = context_as_parameters(Context), - bool_req(topic_path, [{username, Username}, - {vhost, VHost}, - {resource, Type}, - {name, Name}, - {permission, Permission}, - {tags, join_tags(Tags)}] ++ OptionsParameters). + req(topic_path, [{username, Username}, + {vhost, VHost}, + {resource, Type}, + {name, Name}, + {permission, Permission}, + {tags, join_tags(Tags)}] ++ OptionsParameters). expiry_timestamp(_) -> never. @@ -163,7 +163,7 @@ context_as_parameters(Options) when is_map(Options) -> context_as_parameters(_) -> []. -bool_req(PathName, Props) -> +req(PathName, Props) -> Path = p(PathName), Query = q(Props), case http_req(Path, Query) of @@ -172,7 +172,7 @@ bool_req(PathName, Props) -> "deny " ++ Reason -> ?LOG_INFO("HTTP authorisation denied for path ~ts with query ~ts: ~ts", [Path, Query, Reason]), - false; + {false, Reason}; Body -> case string:lowercase(Body) of "deny" -> diff --git a/deps/rabbitmq_auth_backend_http/test/auth_SUITE.erl b/deps/rabbitmq_auth_backend_http/test/auth_unit_SUITE.erl similarity index 99% rename from deps/rabbitmq_auth_backend_http/test/auth_SUITE.erl rename to deps/rabbitmq_auth_backend_http/test/auth_unit_SUITE.erl index e7bddd59f0..7158d4e9c7 100644 --- a/deps/rabbitmq_auth_backend_http/test/auth_SUITE.erl +++ b/deps/rabbitmq_auth_backend_http/test/auth_unit_SUITE.erl @@ -4,7 +4,7 @@ %% %% Copyright (c) 2007-2025 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved. --module(auth_SUITE). +-module(auth_unit_SUITE). -include_lib("common_test/include/ct.hrl"). -include_lib("eunit/include/eunit.hrl"). diff --git a/deps/rabbitmq_auth_backend_oauth2/src/rabbit_oauth2_scope.erl b/deps/rabbitmq_auth_backend_oauth2/src/rabbit_oauth2_scope.erl index 75e4c1f78f..0ca5964e1a 100644 --- a/deps/rabbitmq_auth_backend_oauth2/src/rabbit_oauth2_scope.erl +++ b/deps/rabbitmq_auth_backend_oauth2/src/rabbit_oauth2_scope.erl @@ -24,37 +24,56 @@ -spec vhost_access(binary(), [binary()]) -> boolean(). vhost_access(VHost, Scopes) -> PermissionScopes = get_scope_permissions(Scopes), - lists:any( - fun({VHostPattern, _, _, _}) -> - wildcard:match(VHost, VHostPattern) - end, - PermissionScopes). + case lists:any( + fun({VHostPattern, _, _, _}) -> + wildcard:match(VHost, VHostPattern) + end, + PermissionScopes) of + true -> + true; + false -> + {false, rabbit_misc:format("no scope in ~tp matches vhost '~ts'", + [Scopes, VHost])} + end. -spec resource_access(rabbit_types:r(atom()), permission(), [binary()]) -> boolean(). -resource_access(#resource{virtual_host = VHost, name = Name}, +resource_access(#resource{virtual_host = VHost, name = Name} = Resource, Permission, Scopes) -> - lists:any( - fun({VHostPattern, NamePattern, _, ScopeGrantedPermission}) -> - wildcard:match(VHost, VHostPattern) andalso - wildcard:match(Name, NamePattern) andalso - Permission =:= ScopeGrantedPermission - end, - get_scope_permissions(Scopes)). + case lists:any( + fun({VHostPattern, NamePattern, _, ScopeGrantedPermission}) -> + wildcard:match(VHost, VHostPattern) andalso + wildcard:match(Name, NamePattern) andalso + Permission =:= ScopeGrantedPermission + end, + get_scope_permissions(Scopes)) of + true -> + true; + false -> + {false, rabbit_misc:format("no scope in ~tp has '~s' permission for ~ts", + [Scopes, Permission, rabbit_misc:rs(Resource)])} + end. -spec topic_access(rabbit_types:r(atom()), permission(), map(), [binary()]) -> boolean(). -topic_access(#resource{virtual_host = VHost, name = ExchangeName}, +topic_access(#resource{virtual_host = VHost, name = ExchangeName} = Resource, Permission, #{routing_key := RoutingKey}, Scopes) -> - lists:any( - fun({VHostPattern, ExchangeNamePattern, RoutingKeyPattern, ScopeGrantedPermission}) -> - is_binary(RoutingKeyPattern) andalso - wildcard:match(VHost, VHostPattern) andalso - wildcard:match(ExchangeName, ExchangeNamePattern) andalso - wildcard:match(RoutingKey, RoutingKeyPattern) andalso - Permission =:= ScopeGrantedPermission - end, - get_scope_permissions(Scopes)). + case lists:any( + fun({VHostPattern, ExchangeNamePattern, RoutingKeyPattern, ScopeGrantedPermission}) -> + is_binary(RoutingKeyPattern) andalso + wildcard:match(VHost, VHostPattern) andalso + wildcard:match(ExchangeName, ExchangeNamePattern) andalso + wildcard:match(RoutingKey, RoutingKeyPattern) andalso + Permission =:= ScopeGrantedPermission + end, + get_scope_permissions(Scopes)) of + true -> + true; + false -> + {false, rabbit_misc:format( + "no scope in ~tp has '~s' permission for exchange ~ts and topic '~ts'", + [Scopes, Permission, rabbit_misc:rs(Resource), RoutingKey])} + end. %% Internal ------------------------------------------------------------------- diff --git a/deps/rabbitmq_auth_backend_oauth2/test/scope_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/scope_SUITE.erl index 7f83c1adab..410b5c98f9 100644 --- a/deps/rabbitmq_auth_backend_oauth2/test/scope_SUITE.erl +++ b/deps/rabbitmq_auth_backend_oauth2/test/scope_SUITE.erl @@ -378,14 +378,15 @@ configure_refused(Vhost, Resource, Scope) -> resource_perm(Vhost, Resource, Scope, configure, false). -resource_perm(Vhost, Resource, Scopes, Permission, Result) when is_list(Scopes) -> - [ ?assertEqual(Result, rabbit_oauth2_scope:resource_access( - #resource{virtual_host = Vhost, - kind = Kind, - name = Resource}, - Permission, - Scopes)) || Kind <- [queue, exchange] ]; - +resource_perm(Vhost, Name, Scopes, Permission, Expected) when is_list(Scopes) -> + [begin + Resource = #resource{virtual_host = Vhost, + kind = Kind, + name = Name}, + Actual = rabbit_oauth2_scope:resource_access(Resource, Permission, Scopes), + assert(Expected, Actual) + end + || Kind <- [queue, exchange]]; resource_perm(Vhost, Resource, Scope, Permission, Result) -> resource_perm(Vhost, Resource, [Scope], Permission, Result). @@ -401,14 +402,19 @@ topic_read_refused(Vhost, Resource, RoutingKey, Scopes) when is_list(Scopes) -> topic_read_refused(Vhost, Resource, RoutingKey, Scope) -> topic_perm(Vhost, Resource, RoutingKey, Scope, read, false). -topic_perm(Vhost, Resource, RoutingKey, Scopes, Permission, Result) when is_list(Scopes) -> - ?assertEqual(Result, rabbit_oauth2_scope:topic_access( - #resource{virtual_host = Vhost, - kind = topic, - name = Resource}, - Permission, - #{routing_key => RoutingKey}, - Scopes)); - +topic_perm(Vhost, Name, RoutingKey, Scopes, Permission, Expected) when is_list(Scopes) -> + Resource = #resource{virtual_host = Vhost, + kind = topic, + name = Name}, + Actual = rabbit_oauth2_scope:topic_access(Resource, + Permission, + #{routing_key => RoutingKey}, + Scopes), + assert(Expected, Actual); topic_perm(Vhost, Resource, RoutingKey, Scope, Permission, Result) -> topic_perm(Vhost, Resource, RoutingKey, [Scope], Permission, Result). + +assert(true, Actual) -> + ?assert(Actual); +assert(false, Actual) -> + ?assertMatch({false, _Reason}, Actual). diff --git a/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl b/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl index 3cfb5c10f3..1a03845465 100644 --- a/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl +++ b/deps/rabbitmq_auth_backend_oauth2/test/unit_SUITE.erl @@ -1131,16 +1131,21 @@ test_restricted_vhost_access_with_a_valid_token(_) -> Jwk = ?UTIL_MOD:fixture_jwk(), Token = ?UTIL_MOD:sign_token_hs(?UTIL_MOD:token_with_sub( - ?UTIL_MOD:fixture_token(), Username), Jwk), + ?UTIL_MOD:fixture_token(), Username), Jwk), UaaEnv = [{signing_keys, #{<<"token-key">> => {map, Jwk}}}], set_env(key_config, UaaEnv), %% this user can authenticate successfully and access certain vhosts {ok, #auth_user{username = Username, tags = []} = User} = - user_login_authentication(Username, [{password, Token}]), + user_login_authentication(Username, [{password, Token}]), %% access to a different vhost - ?assertEqual(false, check_vhost_access(User, <<"different vhost">>, none)). + ?assertEqual({false, + "no scope in [<<\"configure:vhost/foo\">>,<<\"read:vhost/bar\">>,\n" + " <<\"read:vhost/bar/%23%2Ffoo\">>,<<\"read:vhost/foo\">>,\n" + " <<\"write:vhost/foo\">>]" + " matches vhost 'different vhost'"}, + check_vhost_access(User, <<"different vhost">>, none)). test_insufficient_permissions_in_a_valid_token(_) -> VHost = <<"vhost">>, @@ -1158,9 +1163,19 @@ test_insufficient_permissions_in_a_valid_token(_) -> %% access to these resources is not granted assert_resource_access_denied(User, VHost, <<"foo1">>, configure), - assert_resource_access_denied(User, VHost, <<"bar">>, write), - assert_topic_access_refused(User, VHost, <<"bar">>, read, - #{routing_key => <<"foo/#">>}). + + ExpectedReason0 = "no scope in [<<\"configure:vhost/foo\">>,<<\"read:vhost/bar\">>,\n" + " <<\"read:vhost/bar/%23%2Ffoo\">>,<<\"read:vhost/foo\">>,\n" + " <<\"write:vhost/foo\">>]" + " has 'write' permission for queue 'bar' in vhost 'vhost'", + assert_resource_access_response({false, ExpectedReason0}, User, VHost, <<"bar">>, write), + + ExpectedReason1 = "no scope in [<<\"configure:vhost/foo\">>,<<\"read:vhost/bar\">>,\n" + " <<\"read:vhost/bar/%23%2Ffoo\">>,<<\"read:vhost/foo\">>,\n" + " <<\"write:vhost/foo\">>]" + " has 'read' permission for exchange 'bar' in vhost 'vhost' and topic 'foo/#'", + assert_topic_access_response({false, ExpectedReason1}, User, VHost, <<"bar">>, read, + #{routing_key => <<"foo/#">>}). test_invalid_signature(_) -> Username = <<"username">>, @@ -1470,6 +1485,7 @@ test_extract_scope_from_path_expression(_) -> set_env(Par, Var) -> application:set_env(rabbitmq_auth_backend_oauth2, Par, Var). + unset_env(Par) -> application:unset_env(rabbitmq_auth_backend_oauth2, Par). @@ -1479,9 +1495,8 @@ assert_vhost_access_granted(AuthUser, VHost) -> assert_vhost_access_denied(AuthUser, VHost) -> assert_vhost_access_response(false, AuthUser, VHost). -assert_vhost_access_response(ExpectedResult, AuthUser, VHost) -> - ?assertEqual(ExpectedResult, - check_vhost_access(AuthUser, VHost, none)). +assert_vhost_access_response(Expected, AuthUser, VHost) -> + assert(Expected, check_vhost_access(AuthUser, VHost, none)). assert_resource_access_granted(AuthUser, VHost, ResourceName, PermissionKind) -> assert_resource_access_response(true, AuthUser, VHost, ResourceName, @@ -1496,13 +1511,10 @@ assert_resource_access_errors(ExpectedError, AuthUser, VHost, ResourceName, assert_resource_access_response({error, ExpectedError}, AuthUser, VHost, ResourceName, PermissionKind). -assert_resource_access_response(ExpectedResult, AuthUser, VHost, ResourceName, - PermissionKind) -> - ?assertEqual(ExpectedResult, - rabbit_auth_backend_oauth2:check_resource_access( - AuthUser, - rabbit_misc:r(VHost, queue, ResourceName), - PermissionKind, #{})). +assert_resource_access_response(ExpectedResult, AuthUser, VHost, + ResourceName, PermissionKind) -> + assert_resource_access_response(ExpectedResult, AuthUser, VHost, queue, + ResourceName, PermissionKind). assert_resource_access_granted(AuthUser, VHost, ResourceKind, ResourceName, PermissionKind) -> @@ -1519,13 +1531,13 @@ assert_resource_access_errors(ExpectedError, AuthUser, VHost, ResourceKind, assert_resource_access_response({error, ExpectedError}, AuthUser, VHost, ResourceKind, ResourceName, PermissionKind). -assert_resource_access_response(ExpectedResult, AuthUser, VHost, ResourceKind, - ResourceName, PermissionKind) -> - ?assertEqual(ExpectedResult, - rabbit_auth_backend_oauth2:check_resource_access( - AuthUser, - rabbit_misc:r(VHost, ResourceKind, ResourceName), - PermissionKind, #{})). +assert_resource_access_response(Expected, AuthUser, VHost, ResourceKind, + ResourceName, PermissionKind) -> + Actual = rabbit_auth_backend_oauth2:check_resource_access( + AuthUser, + rabbit_misc:r(VHost, ResourceKind, ResourceName), + PermissionKind, #{}), + assert(Expected, Actual). assert_topic_access_granted(AuthUser, VHost, ResourceName, PermissionKind, AuthContext) -> @@ -1537,13 +1549,18 @@ assert_topic_access_refused(AuthUser, VHost, ResourceName, PermissionKind, assert_topic_access_response(false, AuthUser, VHost, ResourceName, PermissionKind, AuthContext). -assert_topic_access_response(ExpectedResult, AuthUser, VHost, ResourceName, - PermissionKind, AuthContext) -> - ?assertEqual(ExpectedResult, - rabbit_auth_backend_oauth2:check_topic_access( - AuthUser, - #resource{virtual_host = VHost, - kind = topic, - name = ResourceName}, - PermissionKind, - AuthContext)). +assert_topic_access_response(Expected, AuthUser, VHost, ResourceName, + PermissionKind, AuthContext) -> + Actual = rabbit_auth_backend_oauth2:check_topic_access( + AuthUser, + #resource{virtual_host = VHost, + kind = topic, + name = ResourceName}, + PermissionKind, + AuthContext), + assert(Expected, Actual). + +assert(false, Actual) -> + ?assertMatch({false, _Reason}, Actual); +assert(Expected, Actual) -> + ?assertEqual(Expected, Actual). diff --git a/deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl b/deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl index 48ab2a8697..e20c0b3267 100644 --- a/deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl +++ b/deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl @@ -1609,8 +1609,9 @@ binding_action_with_checks(QName, TopicFilter, BindingArgs, Action, fun rabbit_binding:Action/2, AuthState) else {error, Reason} = Err -> - ?LOG_ERROR("Failed to ~s binding between ~s and ~s for topic filter ~s: ~p", - [Action, rabbit_misc:rs(ExchangeName), rabbit_misc:rs(QName), TopicFilter, Reason]), + ?LOG_ERROR( + "Failed to ~s binding between ~ts and ~ts for topic filter ~ts: ~tp", + [Action, rabbit_misc:rs(ExchangeName), rabbit_misc:rs(QName), TopicFilter, Reason]), Err end. @@ -2292,7 +2293,7 @@ check_resource_access(User, Resource, Perm, Context) -> catch exit:#amqp_error{name = access_refused, explanation = Msg} -> - ?LOG_ERROR("MQTT resource access refused: ~s", [Msg]), + ?LOG_ERROR("MQTT resource access refused: ~ts", [Msg]), {error, access_refused} end end. @@ -2326,7 +2327,7 @@ check_topic_access( catch exit:#amqp_error{name = access_refused, explanation = Msg} -> - ?LOG_ERROR("MQTT topic access refused: ~s", [Msg]), + ?LOG_ERROR("MQTT topic access refused: ~ts", [Msg]), {error, access_refused} end end.