From 2bc014db69d2809c0c7ac9fb5ba26b269739d660 Mon Sep 17 00:00:00 2001 From: firest Date: Thu, 9 May 2024 18:15:32 +0800 Subject: [PATCH] fix(events): call `client.check_authn_complete` even if authentication fails --- apps/emqx/src/emqx_access_control.erl | 27 +++++++++++++++---- .../src/emqx_rule_api_schema.erl | 5 ++-- .../emqx_rule_engine/src/emqx_rule_events.erl | 18 +++++++++++-- .../test/emqx_rule_engine_api_2_SUITE.erl | 1 + .../emqx_rule_engine_api_rule_test_SUITE.erl | 1 + rel/i18n/emqx_rule_api_schema.hocon | 3 +++ 6 files changed, 46 insertions(+), 9 deletions(-) diff --git a/apps/emqx/src/emqx_access_control.erl b/apps/emqx/src/emqx_access_control.erl index 9d2cbfe90..e3c730cd5 100644 --- a/apps/emqx/src/emqx_access_control.erl +++ b/apps/emqx/src/emqx_access_control.erl @@ -72,15 +72,15 @@ authenticate(Credential) -> {ok, AuthResult, _AuthData} = OkResult -> on_authentication_complete(Credential, AuthResult, ok), OkResult; - {error, _Reason} = Error -> - inc_authn_metrics(error), + {error, Reason} = Error -> + on_authentication_complete(Credential, Reason, error), Error; %% {continue, AuthCache} | {continue, AuthData, AuthCache} Other -> Other end; - {error, _Reason} = Error -> - inc_authn_metrics(error), + {error, Reason} = Error -> + on_authentication_complete(Credential, Reason, error), Error end. @@ -241,9 +241,26 @@ inc_authn_metrics(anonymous) -> emqx_metrics:inc('authentication.success.anonymous'), emqx_metrics:inc('authentication.success'). +on_authentication_complete(Credential, Reason, error) -> + emqx_hooks:run( + 'client.check_authn_complete', + [ + Credential, + #{ + reason_code => Reason + } + ] + ), + inc_authn_metrics(error); on_authentication_complete(Credential, Result, Type) -> emqx_hooks:run( 'client.check_authn_complete', - [Credential, Result#{is_anonymous => (Type =:= anonymous)}] + [ + Credential, + Result#{ + reason_code => success, + is_anonymous => (Type =:= anonymous) + } + ] ), inc_authn_metrics(Type). diff --git a/apps/emqx_rule_engine/src/emqx_rule_api_schema.erl b/apps/emqx_rule_engine/src/emqx_rule_api_schema.erl index 1db1251b5..b94e436d6 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_api_schema.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_api_schema.erl @@ -289,9 +289,10 @@ fields("ctx_check_authn_complete") -> {"event", event_sc(Event)}, {"clientid", sc(binary(), #{desc => ?DESC("event_clientid")})}, {"username", sc(binary(), #{desc => ?DESC("event_username")})}, + {"reason_code", sc(binary(), #{desc => ?DESC("event_ctx_authn_reason_code")})}, {"peername", sc(binary(), #{desc => ?DESC("event_peername")})}, - {"is_anonymous", sc(boolean(), #{desc => ?DESC("event_is_anonymous")})}, - {"is_superuser", sc(boolean(), #{desc => ?DESC("event_is_superuser")})} + {"is_anonymous", sc(boolean(), #{desc => ?DESC("event_is_anonymous"), required => false})}, + {"is_superuser", sc(boolean(), #{desc => ?DESC("event_is_superuser"), required => false})} ]; fields("ctx_bridge_mqtt") -> Event = '$bridges/mqtt:*', diff --git a/apps/emqx_rule_engine/src/emqx_rule_events.erl b/apps/emqx_rule_engine/src/emqx_rule_events.erl index 3b242089f..ebcde724a 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_events.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_events.erl @@ -458,15 +458,22 @@ eventmsg_check_authn_complete( peerhost := PeerHost, peerport := PeerPort }, - #{is_anonymous := IsAnonymous} = Result + Result ) -> - IsSuperuser = maps:get(is_superuser, Result, false), + #{ + reason_code := Reason, + is_superuser := IsSuperuser, + is_anonymous := IsAnonymous + } = maps:merge( + #{is_anonymous => false, is_superuser => false}, Result + ), with_basic_columns( 'client.check_authn_complete', #{ clientid => ClientId, username => Username, peername => ntoa({PeerHost, PeerPort}), + reason_code => force_to_bin(Reason), is_anonymous => IsAnonymous, is_superuser => IsSuperuser }, @@ -901,6 +908,7 @@ test_columns('client.check_authn_complete') -> [ {<<"clientid">>, [<<"c_emqx">>, <<"the clientid if the client">>]}, {<<"username">>, [<<"u_emqx">>, <<"the username if the client">>]}, + {<<"reason_code">>, [<<"sucess">>, <<"the reason code">>]}, {<<"is_superuser">>, [true, <<"Whether this is a superuser">>]}, {<<"is_anonymous">>, [false, <<"Whether this is a superuser">>]} ]; @@ -1079,6 +1087,7 @@ columns_with_exam('client.check_authn_complete') -> {<<"clientid">>, <<"c_emqx">>}, {<<"username">>, <<"u_emqx">>}, {<<"peername">>, <<"192.168.0.10:56431">>}, + {<<"reason_code">>, <<"sucess">>}, {<<"is_superuser">>, true}, {<<"is_anonymous">>, false}, {<<"timestamp">>, erlang:system_time(millisecond)}, @@ -1201,6 +1210,11 @@ reason({shutdown, Reason}) when is_atom(Reason) -> Reason; reason({Error, _}) when is_atom(Error) -> Error; reason(_) -> internal_error. +force_to_bin(Bin) when is_binary(Bin) -> + Bin; +force_to_bin(Term) -> + emqx_utils_conv:bin(io_lib:format("~p", [Term])). + ntoa(undefined) -> undefined; ntoa(IpOrIpPort) -> diff --git a/apps/emqx_rule_engine/test/emqx_rule_engine_api_2_SUITE.erl b/apps/emqx_rule_engine/test/emqx_rule_engine_api_2_SUITE.erl index 0b12ac263..abee97a72 100644 --- a/apps/emqx_rule_engine/test/emqx_rule_engine_api_2_SUITE.erl +++ b/apps/emqx_rule_engine/test/emqx_rule_engine_api_2_SUITE.erl @@ -273,6 +273,7 @@ t_rule_test_smoke(_Config) -> #{ <<"clientid">> => <<"c_emqx">>, <<"event_type">> => <<"client_check_authn_complete">>, + <<"reason_code">> => <<"sucess">>, <<"is_superuser">> => true, <<"is_anonymous">> => false, <<"username">> => <<"u_emqx">> diff --git a/apps/emqx_rule_engine/test/emqx_rule_engine_api_rule_test_SUITE.erl b/apps/emqx_rule_engine/test/emqx_rule_engine_api_rule_test_SUITE.erl index b3be5ccdc..640318c9a 100644 --- a/apps/emqx_rule_engine/test/emqx_rule_engine_api_rule_test_SUITE.erl +++ b/apps/emqx_rule_engine/test/emqx_rule_engine_api_rule_test_SUITE.erl @@ -206,6 +206,7 @@ t_ctx_check_authn_complete(_) -> #{ clientid => <<"c_emqx">>, event_type => client_check_authn_complete, + reason_code => <<"sucess">>, is_superuser => true, is_anonymous => false }, diff --git a/rel/i18n/emqx_rule_api_schema.hocon b/rel/i18n/emqx_rule_api_schema.hocon index 192290728..d59a501c6 100644 --- a/rel/i18n/emqx_rule_api_schema.hocon +++ b/rel/i18n/emqx_rule_api_schema.hocon @@ -390,4 +390,7 @@ event_is_anonymous.desc: event_is_superuser.desc: """True if this is a super user.""" +event_ctx_authn_reason_code.desc: +"""The reason code""" + }