Explorar o código

fix: exhook client.authorize never be execauted

see: https://github.com/emqx/emqx/issues/8779
JianBo He %!s(int64=3) %!d(string=hai) anos
pai
achega
168f44e45b

+ 21 - 3
apps/emqx/src/emqx_access_control.erl

@@ -17,6 +17,7 @@
 -module(emqx_access_control).
 
 -include("emqx.hrl").
+-include("logger.hrl").
 
 -export([
     authenticate/1,
@@ -70,9 +71,26 @@ check_authorization_cache(ClientInfo, PubSub, Topic) ->
 
 do_authorize(ClientInfo, PubSub, Topic) ->
     NoMatch = emqx:get_config([authorization, no_match], allow),
-    case run_hooks('client.authorize', [ClientInfo, PubSub, Topic], NoMatch) of
-        allow -> allow;
-        _Other -> deny
+    Default = #{result => NoMatch, from => default},
+    case run_hooks('client.authorize', [ClientInfo, PubSub, Topic], Default) of
+        AuthzResult = #{result := Result} when Result == allow; Result == deny ->
+            From = maps:get(from, AuthzResult, unknown),
+            emqx:run_hook(
+                'client.check_authz_complete',
+                [ClientInfo, PubSub, Topic, Result, From]
+            ),
+            Result;
+        Other ->
+            ?SLOG(error, #{
+                msg => "unknown_authorization_return_format",
+                expected_example => "#{result => allow, from => default}",
+                got => Other
+            }),
+            emqx:run_hook(
+                'client.check_authz_complete',
+                [ClientInfo, PubSub, Topic, deny, unknown_return_format]
+            ),
+            deny
     end.
 
 -compile({inline, [run_hooks/3]}).

+ 5 - 17
apps/emqx_authz/src/emqx_authz.erl

@@ -319,7 +319,7 @@ authorize(
                 is_superuser => true
             }),
             emqx_metrics:inc(?METRIC_SUPERUSER),
-            {stop, allow};
+            {stop, #{result => allow, from => superuser}};
         false ->
             authorize_non_superuser(Client, PubSub, Topic, DefaultResult, Sources)
     end.
@@ -331,15 +331,11 @@ authorize_non_superuser(
     } = Client,
     PubSub,
     Topic,
-    DefaultResult,
+    _DefaultResult,
     Sources
 ) ->
     case do_authorize(Client, PubSub, Topic, sources_with_defaults(Sources)) of
         {{matched, allow}, AuthzSource} ->
-            emqx:run_hook(
-                'client.check_authz_complete',
-                [Client, PubSub, Topic, allow, AuthzSource]
-            ),
             log_allowed(#{
                 username => Username,
                 ipaddr => IpAddress,
@@ -348,12 +344,8 @@ authorize_non_superuser(
             }),
             emqx_metrics_worker:inc(authz_metrics, AuthzSource, allow),
             emqx_metrics:inc(?METRIC_ALLOW),
-            {stop, allow};
+            {stop, #{result => allow, from => AuthzSource}};
         {{matched, deny}, AuthzSource} ->
-            emqx:run_hook(
-                'client.check_authz_complete',
-                [Client, PubSub, Topic, deny, AuthzSource]
-            ),
             ?SLOG(warning, #{
                 msg => "authorization_permission_denied",
                 username => Username,
@@ -363,12 +355,8 @@ authorize_non_superuser(
             }),
             emqx_metrics_worker:inc(authz_metrics, AuthzSource, deny),
             emqx_metrics:inc(?METRIC_DENY),
-            {stop, deny};
+            {stop, #{result => deny, from => AuthzSource}};
         nomatch ->
-            emqx:run_hook(
-                'client.check_authz_complete',
-                [Client, PubSub, Topic, DefaultResult, default]
-            ),
             ?SLOG(info, #{
                 msg => "authorization_failed_nomatch",
                 username => Username,
@@ -377,7 +365,7 @@ authorize_non_superuser(
                 reason => "no-match rule"
             }),
             emqx_metrics:inc(?METRIC_NOMATCH),
-            {stop, DefaultResult}
+            ignore
     end.
 
 log_allowed(Meta) ->

+ 2 - 2
apps/emqx_exhook/src/emqx_exhook_handler.erl

@@ -133,7 +133,7 @@ on_client_authenticate(ClientInfo, AuthResult) ->
     end.
 
 on_client_authorize(ClientInfo, PubSub, Topic, Result) ->
-    Bool = Result == allow,
+    Bool = maps:get(result, Result, deny) == allow,
     Type =
         case PubSub of
             publish -> 'PUBLISH';
@@ -158,7 +158,7 @@ on_client_authorize(ClientInfo, PubSub, Topic, Result) ->
                     true -> allow;
                     _ -> deny
                 end,
-            {StopOrOk, NResult};
+            {StopOrOk, #{result => NResult, from => exhook}};
         _ ->
             {ok, Result}
     end.