From ce2b159022c164673a700ab8fec4db1334613845 Mon Sep 17 00:00:00 2001
From: Ilya Averyanov <av@rubybox.dev>
Date: Tue, 1 Aug 2023 16:48:45 +0300
Subject: [PATCH] chore(auth): make pre_hook_authenticate be part of
 emqx_access_control domain

---
 apps/emqx/include/emqx_access_control.hrl |  3 ++
 apps/emqx/include/emqx_authentication.hrl |  2 +-
 apps/emqx/src/emqx_access_control.erl     | 39 ++++++++++++++++++++++-
 apps/emqx/src/emqx_authentication.erl     | 17 ----------
 4 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/apps/emqx/include/emqx_access_control.hrl b/apps/emqx/include/emqx_access_control.hrl
index e840d2b4a..08146339b 100644
--- a/apps/emqx/include/emqx_access_control.hrl
+++ b/apps/emqx/include/emqx_access_control.hrl
@@ -32,3 +32,6 @@
 -define(authz_action(PUBSUB, QOS), #{action_type := PUBSUB, qos := QOS}).
 -define(authz_action(PUBSUB), ?authz_action(PUBSUB, _)).
 -define(authz_action, ?authz_action(_)).
+
+-define(AUTHN_TRACE_TAG, "AUTHN").
+
diff --git a/apps/emqx/include/emqx_authentication.hrl b/apps/emqx/include/emqx_authentication.hrl
index 70b35a474..d668e9a54 100644
--- a/apps/emqx/include/emqx_authentication.hrl
+++ b/apps/emqx/include/emqx_authentication.hrl
@@ -18,8 +18,8 @@
 -define(EMQX_AUTHENTICATION_HRL, true).
 
 -include_lib("emqx/include/logger.hrl").
+-include_lib("emqx/include/emqx_access_control.hrl").
 
--define(AUTHN_TRACE_TAG, "AUTHN").
 -define(GLOBAL, 'mqtt:global').
 
 -define(TRACE_AUTHN_PROVIDER(Msg), ?TRACE_AUTHN_PROVIDER(Msg, #{})).
diff --git a/apps/emqx/src/emqx_access_control.erl b/apps/emqx/src/emqx_access_control.erl
index 43669bf6c..a8ffadb44 100644
--- a/apps/emqx/src/emqx_access_control.erl
+++ b/apps/emqx/src/emqx_access_control.erl
@@ -17,6 +17,7 @@
 -module(emqx_access_control).
 
 -include("emqx.hrl").
+-include("emqx_access_control.hrl").
 -include("logger.hrl").
 
 -export([
@@ -29,6 +30,14 @@
 -compile(nowarn_export_all).
 -endif.
 
+-define(TRACE_RESULT(Label, Tag, Result, Reason), begin
+    ?TRACE(Label, Tag, #{
+        result => (Result),
+        reason => (Reason)
+    }),
+    Result
+end).
+
 %%--------------------------------------------------------------------
 %% APIs
 %%--------------------------------------------------------------------
@@ -44,7 +53,7 @@ authenticate(Credential) ->
     %% if auth backend returning nothing but just 'ok'
     %% it means it's not a superuser, or there is no way to tell.
     NotSuperUser = #{is_superuser => false},
-    case emqx_authentication:pre_hook_authenticate(Credential) of
+    case pre_hook_authenticate(Credential) of
         ok ->
             inc_authn_metrics(anonymous),
             {ok, NotSuperUser};
@@ -99,6 +108,34 @@ authorize(ClientInfo, Action, Topic) ->
     inc_authz_metrics(Result),
     Result.
 
+%%--------------------------------------------------------------------
+%% Internal Functions
+%%--------------------------------------------------------------------
+
+-spec pre_hook_authenticate(emqx_types:clientinfo()) ->
+    ok | continue | {error, not_authorized}.
+pre_hook_authenticate(#{enable_authn := false}) ->
+    ?TRACE_RESULT("pre_hook_authenticate", ?AUTHN_TRACE_TAG, ok, enable_authn_false);
+pre_hook_authenticate(#{enable_authn := quick_deny_anonymous} = Credential) ->
+    case is_username_defined(Credential) of
+        true ->
+            continue;
+        false ->
+            ?TRACE_RESULT(
+                "pre_hook_authenticate",
+                ?AUTHN_TRACE_TAG,
+                {error, not_authorized},
+                enable_authn_false
+            )
+    end;
+pre_hook_authenticate(_) ->
+    continue.
+
+is_username_defined(#{username := undefined}) -> false;
+is_username_defined(#{username := <<>>}) -> false;
+is_username_defined(#{username := _Username}) -> true;
+is_username_defined(_) -> false.
+
 check_authorization_cache(ClientInfo, Action, Topic) ->
     case emqx_authz_cache:get_authz_cache(Action, Topic) of
         not_found ->
diff --git a/apps/emqx/src/emqx_authentication.erl b/apps/emqx/src/emqx_authentication.erl
index cce789f24..92182f93a 100644
--- a/apps/emqx/src/emqx_authentication.erl
+++ b/apps/emqx/src/emqx_authentication.erl
@@ -29,11 +29,9 @@
 -include_lib("stdlib/include/ms_transform.hrl").
 
 -define(CONF_ROOT, ?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_ATOM).
--define(IS_UNDEFINED(X), (X =:= undefined orelse X =:= <<>>)).
 
 %% The authentication entrypoint.
 -export([
-    pre_hook_authenticate/1,
     authenticate/2
 ]).
 
@@ -220,21 +218,6 @@ when
 %%------------------------------------------------------------------------------
 %% Authenticate
 %%------------------------------------------------------------------------------
--spec pre_hook_authenticate(emqx_types:clientinfo()) ->
-    ok | continue | {error, not_authorized}.
-pre_hook_authenticate(#{enable_authn := false}) ->
-    ?TRACE_RESULT("authentication_result", ok, enable_authn_false);
-pre_hook_authenticate(#{enable_authn := quick_deny_anonymous} = Credential) ->
-    case maps:get(username, Credential, undefined) of
-        U when ?IS_UNDEFINED(U) ->
-            ?TRACE_RESULT(
-                "authentication_result", {error, not_authorized}, enable_authn_false
-            );
-        _ ->
-            continue
-    end;
-pre_hook_authenticate(_) ->
-    continue.
 
 authenticate(#{listener := Listener, protocol := Protocol} = Credential, AuthResult) ->
     case get_authenticators(Listener, global_chain(Protocol)) of