From 6373af3300b850c1d4c74ca8e5f8453c3da3eb18 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Tue, 25 Nov 2014 19:21:27 +0100 Subject: Add more properties to the user_authentication_* notifications Until now, the only property was {name, Username}. The added properties are: o {connection_type, network | direct} o {error, Message} (only if the authentication failed) For network connections, the following informations are added as returned by rabbit_reader:infos/2: o auth_mechanism o host o name (the property is renamed to connection_name to avoid conflict with the username) o peer_cert_issuer o peer_cert_subject o peer_cert_validity o peer_host o peer_port o protocol o ssl o ssl_cipher o ssl_protocol o vhost The notification is sent by rabbit_reader:notify_auth_result/5 and rabbit_direct:notify_auth_result/4, not by rabbit_access_control:check_user_login/2 anymore. This fixes a bug where a "user_authentication_success" event would be sent by rabbit_access_control:check_user_login/2, even if rabbit_reader:auth_phase/2 rejects the user later because the connection isn't on the loopback interface. --- src/rabbit_access_control.erl | 31 ++++++++++++------------ src/rabbit_direct.erl | 20 ++++++++++++++-- src/rabbit_reader.erl | 55 ++++++++++++++++++++++++++++++++++++++----- 3 files changed, 83 insertions(+), 23 deletions(-) diff --git a/src/rabbit_access_control.erl b/src/rabbit_access_control.erl index d1577432..41c54b07 100644 --- a/src/rabbit_access_control.erl +++ b/src/rabbit_access_control.erl @@ -31,10 +31,12 @@ -spec(check_user_pass_login/2 :: (rabbit_types:username(), rabbit_types:password()) - -> {'ok', rabbit_types:user()} | {'refused', string(), [any()]}). + -> {'ok', rabbit_types:user()} | + {'refused', rabbit_types:username(), string(), [any()]}). -spec(check_user_login/2 :: (rabbit_types:username(), [{atom(), any()}]) - -> {'ok', rabbit_types:user()} | {'refused', string(), [any()]}). + -> {'ok', rabbit_types:user()} | + {'refused', rabbit_types:username(), string(), [any()]}). -spec(check_user_loopback/2 :: (rabbit_types:username(), rabbit_net:socket() | inet:ip_address()) -> 'ok' | 'not_allowed'). @@ -55,7 +57,7 @@ check_user_pass_login(Username, Password) -> check_user_login(Username, AuthProps) -> {ok, Modules} = application:get_env(rabbit, auth_backends), R = lists:foldl( - fun ({ModN, ModZs0}, {refused, _, _}) -> + fun ({ModN, ModZs0}, {refused, _, _, _}) -> ModZs = case ModZs0 of A when is_atom(A) -> [A]; L when is_list(L) -> L @@ -69,7 +71,7 @@ check_user_login(Username, AuthProps) -> Else -> Else end; - (Mod, {refused, _, _}) -> + (Mod, {refused, _, _, _}) -> %% Same module for authN and authZ. Just take the result %% it gives us case try_authenticate(Mod, Username, AuthProps) of @@ -81,19 +83,17 @@ check_user_login(Username, AuthProps) -> (_, {ok, User}) -> %% We've successfully authenticated. Skip to the end... {ok, User} - end, {refused, "No modules checked '~s'", [Username]}, Modules), - rabbit_event:notify(case R of - {ok, _User} -> user_authentication_success; - _ -> user_authentication_failure - end, [{name, Username}]), + end, + {refused, Username, "No modules checked '~s'", [Username]}, Modules), R. try_authenticate(Module, Username, AuthProps) -> case Module:user_login_authentication(Username, AuthProps) of {ok, AuthUser} -> {ok, AuthUser}; - {error, E} -> {refused, "~s failed authenticating ~s: ~p~n", + {error, E} -> {refused, Username, + "~s failed authenticating ~s: ~p~n", [Module, Username, E]}; - {refused, F, A} -> {refused, F, A} + {refused, F, A} -> {refused, Username, F, A} end. try_authorize(Modules, Username) -> @@ -101,12 +101,13 @@ try_authorize(Modules, Username) -> fun (Module, {ok, ModsImpls}) -> case Module:user_login_authorization(Username) of {ok, Impl} -> {ok, [{Module, Impl} | ModsImpls]}; - {error, E} -> {refused, "~s failed authorizing ~s: ~p~n", + {error, E} -> {refused, Username, + "~s failed authorizing ~s: ~p~n", [Module, Username, E]}; - {refused, F, A} -> {refused, F, A} + {refused, F, A} -> {refused, Username, F, A} end; - (_, {refused, _, _} = Error) -> - Error + (_, {refused, F, A}) -> + {refused, Username, F, A} end, {ok, []}, Modules). user(#auth_user{username = Username, tags = Tags}, {ok, ModZImpls}) -> diff --git a/src/rabbit_direct.erl b/src/rabbit_direct.erl index f6140f09..9756dd49 100644 --- a/src/rabbit_direct.erl +++ b/src/rabbit_direct.erl @@ -83,14 +83,30 @@ connect({Username, Password}, VHost, Protocol, Pid, Infos) -> connect0(AuthFun, VHost, Protocol, Pid, Infos) -> case rabbit:is_running() of true -> case AuthFun() of - {ok, User} -> + {ok, User = #user{username = Username}} -> + notify_auth_result(Username, + user_authentication_success, "", []), connect1(User, VHost, Protocol, Pid, Infos); - {refused, _M, _A} -> + {refused, Username, Msg, Args} -> + notify_auth_result(Username, + user_authentication_failure, Msg, Args), {error, {auth_failure, "Refused"}} end; false -> {error, broker_not_found_on_node} end. +notify_auth_result(Username, AuthResult, Msg, Args) -> + EventProps0 = [{connection_type, direct}], + EventProps1 = case Username of + none -> [{name, ''} | EventProps0]; + _ -> [{name, Username} | EventProps0] + end, + EventProps = case Msg of + "" -> EventProps1; + _ -> [{error, lists:flatten(io_lib:format(Msg, Args))} | EventProps1] + end, + rabbit_event:notify(AuthResult, EventProps). + connect1(User, VHost, Protocol, Pid, Infos) -> try rabbit_access_control:check_vhost_access(User, VHost, undefined) of ok -> ok = pg_local:join(rabbit_direct, Pid), diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 2033dd14..a18d75d7 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1046,9 +1046,15 @@ auth_phase(Response, auth_state = AuthState}, sock = Sock}) -> case AuthMechanism:handle_response(Response, AuthState) of + {refused, Username, Msg, Args} -> + auth_fail(Username, Msg, Args, Name, State); {refused, Msg, Args} -> - auth_fail(Msg, Args, Name, State); + %% Older auth mechanisms didn't return the username, even if + %% they reach a stage where they know it. + auth_fail(none, Msg, Args, Name, State); {protocol_error, Msg, Args} -> + notify_auth_result(none, user_authentication_failure, + Msg, Args, State), rabbit_misc:protocol_error(syntax_error, Msg, Args); {challenge, Challenge, AuthState1} -> Secure = #'connection.secure'{challenge = Challenge}, @@ -1057,9 +1063,12 @@ auth_phase(Response, auth_state = AuthState1}}; {ok, User = #user{username = Username}} -> case rabbit_access_control:check_user_loopback(Username, Sock) of - ok -> ok; - not_allowed -> auth_fail("user '~s' can only connect via " - "localhost", [Username], Name, State) + ok -> + notify_auth_result(Username, user_authentication_success, + "", [], State); + not_allowed -> + auth_fail(Username, "user '~s' can only connect via " + "localhost", [Username], Name, State) end, Tune = #'connection.tune'{frame_max = get_env(frame_max), channel_max = get_env(channel_max), @@ -1071,11 +1080,14 @@ auth_phase(Response, end. -ifdef(use_specs). --spec(auth_fail/4 :: (string(), [any()], binary(), #v1{}) -> no_return()). +-spec(auth_fail/5 :: + (rabbit_types:username() | none, string(), [any()], binary(), #v1{}) -> + no_return()). -endif. -auth_fail(Msg, Args, AuthName, +auth_fail(Username, Msg, Args, AuthName, State = #v1{connection = #connection{protocol = Protocol, capabilities = Capabilities}}) -> + notify_auth_result(Username, user_authentication_failure, Msg, Args, State), AmqpError = rabbit_misc:amqp_error( access_refused, "~s login refused: ~s", [AuthName, io_lib:format(Msg, Args)], none), @@ -1094,6 +1106,37 @@ auth_fail(Msg, Args, AuthName, end, rabbit_misc:protocol_error(AmqpError). +notify_auth_result(Username, AuthResult, Msg, Args, State) -> + EventProps0 = [{connection_type, network}], + EventProps1 = lists:foldl( + fun + (name, Acc) -> [{connection_name, i(name, State)} | Acc]; + (Item, Acc) -> [{Item, i(Item, State)} | Acc] + end, EventProps0, [ + peer_cert_validity, + peer_cert_subject, + peer_cert_issuer, + ssl_cipher, + ssl_protocol, + ssl, + auth_mechanism, + protocol, + peer_port, + peer_host, + name, + vhost, + host + ]), + EventProps2 = case Username of + none -> [{name, ''} | EventProps1]; + _ -> [{name, Username} | EventProps1] + end, + EventProps = case Msg of + "" -> EventProps2; + _ -> [{error, lists:flatten(io_lib:format(Msg, Args))} | EventProps2] + end, + rabbit_event:notify(AuthResult, EventProps). + %%-------------------------------------------------------------------------- infos(Items, State) -> [{Item, i(Item, State)} || Item <- Items]. -- cgit v1.2.1 From 77651e4c3fce5a622b3bb74d006fe53bc86b5c01 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Wed, 26 Nov 2014 16:51:47 +0100 Subject: Update rabbit_auth_mechanism:handle_response() spec to match the change --- src/rabbit_auth_mechanism.erl | 5 ++++- src/rabbit_reader.erl | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/rabbit_auth_mechanism.erl b/src/rabbit_auth_mechanism.erl index d11af095..4f771db2 100644 --- a/src/rabbit_auth_mechanism.erl +++ b/src/rabbit_auth_mechanism.erl @@ -37,12 +37,15 @@ %% {protocol_error, Msg, Args} %% Client got the protocol wrong. Log and die. %% {refused, Msg, Args} +%% (deprecated) Client failed authentication. Log and die. +%% {refused, Username, Msg, Args} %% Client failed authentication. Log and die. -callback handle_response(binary(), any()) -> {'ok', rabbit_types:user()} | {'challenge', binary(), any()} | {'protocol_error', string(), [any()]} | - {'refused', string(), [any()]}. + {'refused', string(), [any()]} | + {'refused', rabbit_types:username() | none, string(), [any()]}. -else. diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index a18d75d7..05ed3eda 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1049,8 +1049,8 @@ auth_phase(Response, {refused, Username, Msg, Args} -> auth_fail(Username, Msg, Args, Name, State); {refused, Msg, Args} -> - %% Older auth mechanisms didn't return the username, even if - %% they reach a stage where they know it. + %% Deprecated: older auth mechanisms didn't return the + %% username, even if they reach a stage where they know it. auth_fail(none, Msg, Args, Name, State); {protocol_error, Msg, Args} -> notify_auth_result(none, user_authentication_failure, -- cgit v1.2.1 From 62dc4e1df7d259f9bee6059a8735c1551a9361be Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Wed, 26 Nov 2014 17:01:45 +0100 Subject: Rephrase a comment to use the present tense --- src/rabbit_reader.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 05ed3eda..364dfc05 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1049,7 +1049,7 @@ auth_phase(Response, {refused, Username, Msg, Args} -> auth_fail(Username, Msg, Args, Name, State); {refused, Msg, Args} -> - %% Deprecated: older auth mechanisms didn't return the + %% Deprecated: older auth mechanisms don't return the %% username, even if they reach a stage where they know it. auth_fail(none, Msg, Args, Name, State); {protocol_error, Msg, Args} -> -- cgit v1.2.1 From 8a25a1aa86eb707bb0ed68d3f78f18f4a25c0bc5 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Wed, 26 Nov 2014 17:53:45 +0100 Subject: Drop {refuse, _, _} from rabbit_auth_mechanism:handle_response/2 return values --- src/rabbit_auth_mechanism.erl | 3 --- src/rabbit_reader.erl | 4 ---- 2 files changed, 7 deletions(-) diff --git a/src/rabbit_auth_mechanism.erl b/src/rabbit_auth_mechanism.erl index 4f771db2..c8e23a75 100644 --- a/src/rabbit_auth_mechanism.erl +++ b/src/rabbit_auth_mechanism.erl @@ -36,15 +36,12 @@ %% Another round is needed. Here's the state I want next time. %% {protocol_error, Msg, Args} %% Client got the protocol wrong. Log and die. -%% {refused, Msg, Args} -%% (deprecated) Client failed authentication. Log and die. %% {refused, Username, Msg, Args} %% Client failed authentication. Log and die. -callback handle_response(binary(), any()) -> {'ok', rabbit_types:user()} | {'challenge', binary(), any()} | {'protocol_error', string(), [any()]} | - {'refused', string(), [any()]} | {'refused', rabbit_types:username() | none, string(), [any()]}. -else. diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 364dfc05..713b1844 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1048,10 +1048,6 @@ auth_phase(Response, case AuthMechanism:handle_response(Response, AuthState) of {refused, Username, Msg, Args} -> auth_fail(Username, Msg, Args, Name, State); - {refused, Msg, Args} -> - %% Deprecated: older auth mechanisms don't return the - %% username, even if they reach a stage where they know it. - auth_fail(none, Msg, Args, Name, State); {protocol_error, Msg, Args} -> notify_auth_result(none, user_authentication_failure, Msg, Args, State), -- cgit v1.2.1 From 86e2af9c031b7c2791461f78a9924e104129b2af Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Tue, 2 Dec 2014 17:14:30 +0100 Subject: Move the info keys list added to user_authentication_* to a -define() --- src/rabbit_reader.erl | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 713b1844..260fdad7 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -58,6 +58,11 @@ -define(INFO_KEYS, ?CREATION_EVENT_KEYS ++ ?STATISTICS_KEYS -- [pid]). +-define(AUTH_NOTIFICATION_INFO_KEYS, + [peer_cert_validity, peer_cert_subject, peer_cert_issuer, + ssl_cipher, ssl_protocol, ssl, auth_mechanism, protocol, + peer_port, peer_host, name, vhost, host]). + -define(IS_RUNNING(State), (State#v1.connection_state =:= running orelse State#v1.connection_state =:= blocking orelse @@ -1108,21 +1113,7 @@ notify_auth_result(Username, AuthResult, Msg, Args, State) -> fun (name, Acc) -> [{connection_name, i(name, State)} | Acc]; (Item, Acc) -> [{Item, i(Item, State)} | Acc] - end, EventProps0, [ - peer_cert_validity, - peer_cert_subject, - peer_cert_issuer, - ssl_cipher, - ssl_protocol, - ssl, - auth_mechanism, - protocol, - peer_port, - peer_host, - name, - vhost, - host - ]), + end, EventProps0, ?AUTH_NOTIFICATION_INFO_KEYS), EventProps2 = case Username of none -> [{name, ''} | EventProps1]; _ -> [{name, Username} | EventProps1] -- cgit v1.2.1 From 5d3858e788d1114608541dc1e830206534fd89ab Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Tue, 2 Dec 2014 17:17:07 +0100 Subject: Use rabbit_misc:format/2 instead of lists:flatten/1 + io_lib:format/2 --- src/rabbit_direct.erl | 2 +- src/rabbit_reader.erl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rabbit_direct.erl b/src/rabbit_direct.erl index 9756dd49..ddd8d4e5 100644 --- a/src/rabbit_direct.erl +++ b/src/rabbit_direct.erl @@ -103,7 +103,7 @@ notify_auth_result(Username, AuthResult, Msg, Args) -> end, EventProps = case Msg of "" -> EventProps1; - _ -> [{error, lists:flatten(io_lib:format(Msg, Args))} | EventProps1] + _ -> [{error, rabbit_misc:format(Msg, Args)} | EventProps1] end, rabbit_event:notify(AuthResult, EventProps). diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 260fdad7..88bbc367 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1120,7 +1120,7 @@ notify_auth_result(Username, AuthResult, Msg, Args, State) -> end, EventProps = case Msg of "" -> EventProps2; - _ -> [{error, lists:flatten(io_lib:format(Msg, Args))} | EventProps2] + _ -> [{error, rabbit_misc:format(Msg, Args)} | EventProps2] end, rabbit_event:notify(AuthResult, EventProps). -- cgit v1.2.1 From 2c4bdf1d191d97d55461209f9f185a044ab84433 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Tue, 2 Dec 2014 17:38:12 +0100 Subject: Use list comprehension instead of lists:foldl/3 To keep a somewhat logical order in the list of user_authentication_* properties, reverse the order of the AUTH_NOTIFICATION_INFO_KEYS list. This list was previously reversed by lists:foldl/3. --- src/rabbit_reader.erl | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 88bbc367..60246fbe 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -59,9 +59,9 @@ -define(INFO_KEYS, ?CREATION_EVENT_KEYS ++ ?STATISTICS_KEYS -- [pid]). -define(AUTH_NOTIFICATION_INFO_KEYS, - [peer_cert_validity, peer_cert_subject, peer_cert_issuer, - ssl_cipher, ssl_protocol, ssl, auth_mechanism, protocol, - peer_port, peer_host, name, vhost, host]). + [host, vhost, name, peer_host, peer_port, protocol, auth_mechanism, + ssl, ssl_protocol, ssl_cipher, peer_cert_issuer, peer_cert_subject, + peer_cert_validity]). -define(IS_RUNNING(State), (State#v1.connection_state =:= running orelse @@ -1109,11 +1109,11 @@ auth_fail(Username, Msg, Args, AuthName, notify_auth_result(Username, AuthResult, Msg, Args, State) -> EventProps0 = [{connection_type, network}], - EventProps1 = lists:foldl( - fun - (name, Acc) -> [{connection_name, i(name, State)} | Acc]; - (Item, Acc) -> [{Item, i(Item, State)} | Acc] - end, EventProps0, ?AUTH_NOTIFICATION_INFO_KEYS), + EventProps1 = EventProps0 ++ [ + case Item of + name -> {connection_name, i(name, State)}; + _ -> {Item, i(Item, State)} + end || Item <- ?AUTH_NOTIFICATION_INFO_KEYS], EventProps2 = case Username of none -> [{name, ''} | EventProps1]; _ -> [{name, Username} | EventProps1] -- cgit v1.2.1 From 6aa27b4694b043423d2d992602643420f955f91d Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Tue, 2 Dec 2014 17:45:27 +0100 Subject: Only include ssl/certificate informations when the connection is over SSL This gives lighter notifications for plain TCP connections. --- src/rabbit_reader.erl | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 60246fbe..1ec8a150 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -60,7 +60,10 @@ -define(AUTH_NOTIFICATION_INFO_KEYS, [host, vhost, name, peer_host, peer_port, protocol, auth_mechanism, - ssl, ssl_protocol, ssl_cipher, peer_cert_issuer, peer_cert_subject, + ssl]). + +-define(AUTH_NOTIFICATION_SSL_INFO_KEYS, + [ssl_protocol, ssl_cipher, peer_cert_issuer, peer_cert_subject, peer_cert_validity]). -define(IS_RUNNING(State), @@ -1114,13 +1117,21 @@ notify_auth_result(Username, AuthResult, Msg, Args, State) -> name -> {connection_name, i(name, State)}; _ -> {Item, i(Item, State)} end || Item <- ?AUTH_NOTIFICATION_INFO_KEYS], - EventProps2 = case Username of - none -> [{name, ''} | EventProps1]; - _ -> [{name, Username} | EventProps1] + EventProps2 = case i(ssl, State) of + false -> EventProps1; + true -> EventProps1 ++ [ + case Item of + name -> {connection_name, i(name, State)}; + _ -> {Item, i(Item, State)} + end || Item <- ?AUTH_NOTIFICATION_SSL_INFO_KEYS] + end, + EventProps3 = case Username of + none -> [{name, ''} | EventProps2]; + _ -> [{name, Username} | EventProps2] end, EventProps = case Msg of - "" -> EventProps2; - _ -> [{error, rabbit_misc:format(Msg, Args)} | EventProps2] + "" -> EventProps3; + _ -> [{error, rabbit_misc:format(Msg, Args)} | EventProps3] end, rabbit_event:notify(AuthResult, EventProps). -- cgit v1.2.1 From e74bd8fe71260fe669793bcdad245591a61c56ab Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Tue, 2 Dec 2014 17:55:23 +0100 Subject: Pass "extra properties" to notify_auth_result/{3,4} instead of a message This avoids to passs an empty message in the case of successful authentication. --- src/rabbit_direct.erl | 12 +++++------- src/rabbit_reader.erl | 15 +++++++-------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/rabbit_direct.erl b/src/rabbit_direct.erl index ddd8d4e5..34eff61f 100644 --- a/src/rabbit_direct.erl +++ b/src/rabbit_direct.erl @@ -85,26 +85,24 @@ connect0(AuthFun, VHost, Protocol, Pid, Infos) -> true -> case AuthFun() of {ok, User = #user{username = Username}} -> notify_auth_result(Username, - user_authentication_success, "", []), + user_authentication_success, []), connect1(User, VHost, Protocol, Pid, Infos); {refused, Username, Msg, Args} -> notify_auth_result(Username, - user_authentication_failure, Msg, Args), + user_authentication_failure, + [{error, rabbit_misc:format(Msg, Args)}]), {error, {auth_failure, "Refused"}} end; false -> {error, broker_not_found_on_node} end. -notify_auth_result(Username, AuthResult, Msg, Args) -> +notify_auth_result(Username, AuthResult, ExtraProps) -> EventProps0 = [{connection_type, direct}], EventProps1 = case Username of none -> [{name, ''} | EventProps0]; _ -> [{name, Username} | EventProps0] end, - EventProps = case Msg of - "" -> EventProps1; - _ -> [{error, rabbit_misc:format(Msg, Args)} | EventProps1] - end, + EventProps = EventProps1 ++ ExtraProps, rabbit_event:notify(AuthResult, EventProps). connect1(User, VHost, Protocol, Pid, Infos) -> diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 1ec8a150..968e6a4d 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1058,7 +1058,8 @@ auth_phase(Response, auth_fail(Username, Msg, Args, Name, State); {protocol_error, Msg, Args} -> notify_auth_result(none, user_authentication_failure, - Msg, Args, State), + [{error, rabbit_misc:format(Msg, Args)}], + State), rabbit_misc:protocol_error(syntax_error, Msg, Args); {challenge, Challenge, AuthState1} -> Secure = #'connection.secure'{challenge = Challenge}, @@ -1069,7 +1070,7 @@ auth_phase(Response, case rabbit_access_control:check_user_loopback(Username, Sock) of ok -> notify_auth_result(Username, user_authentication_success, - "", [], State); + [], State); not_allowed -> auth_fail(Username, "user '~s' can only connect via " "localhost", [Username], Name, State) @@ -1091,7 +1092,8 @@ auth_phase(Response, auth_fail(Username, Msg, Args, AuthName, State = #v1{connection = #connection{protocol = Protocol, capabilities = Capabilities}}) -> - notify_auth_result(Username, user_authentication_failure, Msg, Args, State), + notify_auth_result(Username, user_authentication_failure, + [{error, rabbit_misc:format(Msg, Args)}], State), AmqpError = rabbit_misc:amqp_error( access_refused, "~s login refused: ~s", [AuthName, io_lib:format(Msg, Args)], none), @@ -1110,7 +1112,7 @@ auth_fail(Username, Msg, Args, AuthName, end, rabbit_misc:protocol_error(AmqpError). -notify_auth_result(Username, AuthResult, Msg, Args, State) -> +notify_auth_result(Username, AuthResult, ExtraProps, State) -> EventProps0 = [{connection_type, network}], EventProps1 = EventProps0 ++ [ case Item of @@ -1129,10 +1131,7 @@ notify_auth_result(Username, AuthResult, Msg, Args, State) -> none -> [{name, ''} | EventProps2]; _ -> [{name, Username} | EventProps2] end, - EventProps = case Msg of - "" -> EventProps3; - _ -> [{error, rabbit_misc:format(Msg, Args)} | EventProps3] - end, + EventProps = EventProps3 ++ ExtraProps, rabbit_event:notify(AuthResult, EventProps). %%-------------------------------------------------------------------------- -- cgit v1.2.1 From 7378afd5fed81839813307fc9080a60af61aa91d Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Wed, 3 Dec 2014 14:05:03 +0100 Subject: Style fix: Use ++ to construct EventProps, not multiple temporary variables --- src/rabbit_direct.erl | 9 +++------ src/rabbit_reader.erl | 33 ++++++++++++++------------------- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/src/rabbit_direct.erl b/src/rabbit_direct.erl index 34eff61f..931e1e4d 100644 --- a/src/rabbit_direct.erl +++ b/src/rabbit_direct.erl @@ -97,12 +97,9 @@ connect0(AuthFun, VHost, Protocol, Pid, Infos) -> end. notify_auth_result(Username, AuthResult, ExtraProps) -> - EventProps0 = [{connection_type, direct}], - EventProps1 = case Username of - none -> [{name, ''} | EventProps0]; - _ -> [{name, Username} | EventProps0] - end, - EventProps = EventProps1 ++ ExtraProps, + EventProps = [{connection_type, direct}] ++ + [{name, case Username of none -> ''; _ -> Username end}] ++ + ExtraProps, rabbit_event:notify(AuthResult, EventProps). connect1(User, VHost, Protocol, Pid, Infos) -> diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 968e6a4d..31e60e9a 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1113,25 +1113,20 @@ auth_fail(Username, Msg, Args, AuthName, rabbit_misc:protocol_error(AmqpError). notify_auth_result(Username, AuthResult, ExtraProps, State) -> - EventProps0 = [{connection_type, network}], - EventProps1 = EventProps0 ++ [ - case Item of - name -> {connection_name, i(name, State)}; - _ -> {Item, i(Item, State)} - end || Item <- ?AUTH_NOTIFICATION_INFO_KEYS], - EventProps2 = case i(ssl, State) of - false -> EventProps1; - true -> EventProps1 ++ [ - case Item of - name -> {connection_name, i(name, State)}; - _ -> {Item, i(Item, State)} - end || Item <- ?AUTH_NOTIFICATION_SSL_INFO_KEYS] - end, - EventProps3 = case Username of - none -> [{name, ''} | EventProps2]; - _ -> [{name, Username} | EventProps2] - end, - EventProps = EventProps3 ++ ExtraProps, + EventProps = [{connection_type, network}] ++ + [case Item of + name -> {connection_name, i(name, State)}; + _ -> {Item, i(Item, State)} + end || Item <- ?AUTH_NOTIFICATION_INFO_KEYS] ++ + case i(ssl, State) of + false -> []; + true -> [case Item of + name -> {connection_name, i(name, State)}; + _ -> {Item, i(Item, State)} + end || Item <- ?AUTH_NOTIFICATION_SSL_INFO_KEYS] + end ++ + [{name, case Username of none -> ''; _ -> Username end}] ++ + ExtraProps, rabbit_event:notify(AuthResult, EventProps). %%-------------------------------------------------------------------------- -- cgit v1.2.1 From f59084f398c8d7f73209c5ea60dac28352c027b6 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Wed, 3 Dec 2014 14:10:35 +0100 Subject: Filter out auth notification properties with no value --- src/rabbit_direct.erl | 2 +- src/rabbit_reader.erl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rabbit_direct.erl b/src/rabbit_direct.erl index 931e1e4d..79c7a195 100644 --- a/src/rabbit_direct.erl +++ b/src/rabbit_direct.erl @@ -100,7 +100,7 @@ notify_auth_result(Username, AuthResult, ExtraProps) -> EventProps = [{connection_type, direct}] ++ [{name, case Username of none -> ''; _ -> Username end}] ++ ExtraProps, - rabbit_event:notify(AuthResult, EventProps). + rabbit_event:notify(AuthResult, [P || {_, V} = P <- EventProps, V =/= '']). connect1(User, VHost, Protocol, Pid, Infos) -> try rabbit_access_control:check_vhost_access(User, VHost, undefined) of diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 31e60e9a..0c5f0232 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1127,7 +1127,7 @@ notify_auth_result(Username, AuthResult, ExtraProps, State) -> end ++ [{name, case Username of none -> ''; _ -> Username end}] ++ ExtraProps, - rabbit_event:notify(AuthResult, EventProps). + rabbit_event:notify(AuthResult, [P || {_, V} = P <- EventProps, V =/= '']). %%-------------------------------------------------------------------------- -- cgit v1.2.1 From 3e4d1f7b0d46ca99f7ecc05e6ec29c1d17ff8d2c Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Wed, 10 Dec 2014 11:25:59 +0100 Subject: Now that empty keys are dropped, we can always add ssl_* keys If the connection isn't over SSL, ssl_* keys will be empty and dropped anyway. --- src/rabbit_reader.erl | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index 0c5f0232..e1ffb704 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -60,10 +60,7 @@ -define(AUTH_NOTIFICATION_INFO_KEYS, [host, vhost, name, peer_host, peer_port, protocol, auth_mechanism, - ssl]). - --define(AUTH_NOTIFICATION_SSL_INFO_KEYS, - [ssl_protocol, ssl_cipher, peer_cert_issuer, peer_cert_subject, + ssl, ssl_protocol, ssl_cipher, peer_cert_issuer, peer_cert_subject, peer_cert_validity]). -define(IS_RUNNING(State), @@ -1118,13 +1115,6 @@ notify_auth_result(Username, AuthResult, ExtraProps, State) -> name -> {connection_name, i(name, State)}; _ -> {Item, i(Item, State)} end || Item <- ?AUTH_NOTIFICATION_INFO_KEYS] ++ - case i(ssl, State) of - false -> []; - true -> [case Item of - name -> {connection_name, i(name, State)}; - _ -> {Item, i(Item, State)} - end || Item <- ?AUTH_NOTIFICATION_SSL_INFO_KEYS] - end ++ [{name, case Username of none -> ''; _ -> Username end}] ++ ExtraProps, rabbit_event:notify(AuthResult, [P || {_, V} = P <- EventProps, V =/= '']). -- cgit v1.2.1 From 643d598dfc6452b301a9ceeceb20f68d6a820336 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Wed, 10 Dec 2014 11:42:13 +0100 Subject: No need to concatenate hard-coded lists... --- src/rabbit_direct.erl | 4 ++-- src/rabbit_reader.erl | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/rabbit_direct.erl b/src/rabbit_direct.erl index 79c7a195..11233e7e 100644 --- a/src/rabbit_direct.erl +++ b/src/rabbit_direct.erl @@ -97,8 +97,8 @@ connect0(AuthFun, VHost, Protocol, Pid, Infos) -> end. notify_auth_result(Username, AuthResult, ExtraProps) -> - EventProps = [{connection_type, direct}] ++ - [{name, case Username of none -> ''; _ -> Username end}] ++ + EventProps = [{connection_type, direct}, + {name, case Username of none -> ''; _ -> Username end}] ++ ExtraProps, rabbit_event:notify(AuthResult, [P || {_, V} = P <- EventProps, V =/= '']). diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl index e1ffb704..c92eaf7f 100644 --- a/src/rabbit_reader.erl +++ b/src/rabbit_reader.erl @@ -1110,12 +1110,12 @@ auth_fail(Username, Msg, Args, AuthName, rabbit_misc:protocol_error(AmqpError). notify_auth_result(Username, AuthResult, ExtraProps, State) -> - EventProps = [{connection_type, network}] ++ + EventProps = [{connection_type, network}, + {name, case Username of none -> ''; _ -> Username end}] ++ [case Item of name -> {connection_name, i(name, State)}; _ -> {Item, i(Item, State)} end || Item <- ?AUTH_NOTIFICATION_INFO_KEYS] ++ - [{name, case Username of none -> ''; _ -> Username end}] ++ ExtraProps, rabbit_event:notify(AuthResult, [P || {_, V} = P <- EventProps, V =/= '']). -- cgit v1.2.1