chore(authn): improve log and fix disabled authentication still working

apps/emqx/src/emqx_authentication.erl

@@ -289,16 +289,10 @@ check_config(Config) ->
 %%------------------------------------------------------------------------------
 
 authenticate(#{listener := Listener, protocol := Protocol} = Credential, _AuthResult) ->
-    case ets:lookup(?CHAINS_TAB, Listener) of
+    Authenticators = get_authenticators(Listener, global_chain(Protocol)),
-        [#chain{authenticators = Authenticators}] when Authenticators =/= [] ->
+    case get_enabled(Authenticators) of
-            do_authenticate(Authenticators, Credential);
+        [] -> ignore;
-        _ ->
+        NAuthenticators -> do_authenticate(NAuthenticators, Credential)
-            case ets:lookup(?CHAINS_TAB, global_chain(Protocol)) of
-                [#chain{authenticators = Authenticators}] when Authenticators =/= [] ->
-                    do_authenticate(Authenticators, Credential);
-                _ ->
-                    ignore
-            end
     end.
 
 do_authenticate([], _) ->
@@ -315,11 +309,31 @@ do_authenticate([#authenticator{id = ID, provider = Provider, state = State} | M
             %% {error, Reason}
             {stop, Result}
     catch
-        error:Reason:Stacktrace ->
+        Class:Reason:Stacktrace ->
-            ?LOG(warning, "The following error occurred in '~s' during authentication: ~p", [ID, {Reason, Stacktrace}]),
+            ?SLOG(warning, #{msg => "an unexpected error in authentication",
+                             class => Class,
+                             reason => Reason,
+                             stacktrace => Stacktrace,
+                             authenticator => ID}),
             do_authenticate(More, Credential)
     end.
 
+get_authenticators(Listener, Global) ->
+    case ets:lookup(?CHAINS_TAB, Listener) of
+        [#chain{authenticators = Authenticators}] ->
+            Authenticators;
+        _ ->
+            case ets:lookup(?CHAINS_TAB, Global) of
+                [#chain{authenticators = Authenticators}] ->
+                    Authenticators;
+                _ ->
+                    []
+            end
+    end.
+
+get_enabled(Authenticators) ->
+    [Authenticator || Authenticator <- Authenticators, Authenticator#authenticator.enable =:= true].
+
 %%------------------------------------------------------------------------------
 %% APIs
 %%------------------------------------------------------------------------------
@@ -335,7 +349,9 @@ initialize_authentication(ChainName, AuthenticatorsConfig) ->
             {ok, _} ->
                 ok;
             {error, Reason} ->
-                ?LOG(error, "Failed to create authenticator '~s': ~p", [generate_id(AuthenticatorConfig), Reason])
+                ?SLOG(error, #{msg => "failed to create authenticator",
+                               reason => Reason,
+                               authenticator => generate_id(AuthenticatorConfig)})
         end
     end, CheckedConfig).
 
@@ -540,7 +556,7 @@ handle_call({create_authenticator, ChainName, Config}, _From, #{providers := Pro
                 false ->
                     case do_create_authenticator(ChainName, AuthenticatorID, Config, Providers) of
                         {ok, Authenticator} ->
-                            NAuthenticators = Authenticators ++ [Authenticator],
+                            NAuthenticators = Authenticators ++ [Authenticator#authenticator{enable = maps:get(enable, Config)}],
                             true = ets:insert(?CHAINS_TAB, Chain#chain{authenticators = NAuthenticators}),
                             {ok, serialize_authenticator(Authenticator)};
                         {error, Reason} ->
@@ -579,7 +595,8 @@ handle_call({update_authenticator, ChainName, AuthenticatorID, Config}, _From, S
                             Unique = unique(ChainName, AuthenticatorID, Version),
                             case Provider:update(Config#{'_unique' => Unique}, ST) of
                                 {ok, NewST} ->
-                                    NewAuthenticator = Authenticator#authenticator{state = switch_version(NewST)},
+                                    NewAuthenticator = Authenticator#authenticator{state = switch_version(NewST),
+                                                                                   enable = maps:get(enable, Config)},
                                     NewAuthenticators = replace_authenticator(AuthenticatorID, NewAuthenticator, Authenticators),
                                     true = ets:insert(?CHAINS_TAB, Chain#chain{authenticators = NewAuthenticators}),
                                     {ok, serialize_authenticator(NewAuthenticator)};
@@ -633,15 +650,15 @@ handle_call({list_users, ChainName, AuthenticatorID}, _From, State) ->
     reply(Reply, State);
 
 handle_call(Req, _From, State) ->
-    ?LOG(error, "Unexpected call: ~p", [Req]),
+    ?SLOG(error, #{msg => "unexpected call", req => Req}),
     {reply, ignored, State}.
 
 handle_cast(Req, State) ->
-    ?LOG(error, "Unexpected case: ~p", [Req]),
+    ?SLOG(error, #{msg => "unexpected cast", req => Req}),
     {noreply, State}.
 
 handle_info(Info, State) ->
-    ?LOG(error, "Unexpected info: ~p", [Info]),
+    ?SLOG(error, #{msg => "unexpected info", info => Info}),
     {noreply, State}.
 
 terminate(_Reason, _State) ->

apps/emqx_authn/src/simple_authn/emqx_authn_http.erl

@@ -188,9 +188,9 @@ check_url(URL) ->
     end.
 
 check_body(Body) ->
-    lists:all(fun({_, V}) ->
+    maps:fold(fun(_K, _V, false) -> false;
-                  is_binary(V)
+                 (_K, V, true) -> is_binary(V)
-              end, maps:to_list(Body)).
+              end, true, Body).
 
 default_headers() ->
     maps:put(<<"content-type">>,

apps/emqx_authn/src/simple_authn/emqx_authn_jwks_connector.erl

@@ -94,7 +94,9 @@ handle_info({http, {RequestID, Result}},
     State1 = State0#{request_id := undefined},
     case Result of
         {error, Reason} ->
-            ?LOG(error, "Failed to request jwks endpoint(~s): ~p", [Endpoint, Reason]),
+            ?SLOG(warning, #{msg => "failed to request jwks endpoint",
+                             endpoint => Endpoint,
+                             reason => Reason}),
             State1;
         {_StatusLine, _Headers, Body} ->
             try
@@ -102,7 +104,9 @@ handle_info({http, {RequestID, Result}},
                 {_, JWKs} = JWKS#jose_jwk.keys,
                 State1#{jwks := JWKs}
             catch _:_ ->
-                ?LOG(error, "Invalid jwks returned from jwks endpoint(~s): ~p~n", [Endpoint, Body]),
+                ?SLOG(warning, #{msg => "invalid jwks returned from jwks endpoint",
+                                 endpoint => Endpoint,
+                                 body => Body}),
                 State1
             end
     end;
@@ -140,7 +144,9 @@ refresh_jwks(#{endpoint := Endpoint,
     NState = case httpc:request(get, {Endpoint, [{"Accept", "application/json"}]}, HTTPOpts,
                                 [{body_format, binary}, {sync, false}, {receiver, self()}]) of
                  {error, Reason} ->
-                     ?LOG(error, "Failed to request jwks endpoint(~s): ~p", [Endpoint, Reason]),
+                     ?SLOG(warning, #{msg => "failed to request jwks endpoint",
+                                      endpoint => Endpoint,
+                                      reason => Reason}),
                      State;
                  {ok, RequestID} ->
                      State#{request_id := RequestID}

apps/emqx_authn/src/simple_authn/emqx_authn_mongodb.erl

@@ -146,14 +146,18 @@ authenticate(#{password := Password} = Credential,
     case emqx_resource:query(Unique, {find_one, Collection, Selector2, #{}}) of
         undefined -> ignore;
         {error, Reason} ->
-            ?LOG(error, "['~s'] Query failed: ~p", [Unique, Reason]),
+            ?SLOG(error, #{msg => "query failed",
+                           unique => Unique,
+                           reason => Reason}),
             ignore;
         Doc ->
             case check_password(Password, Doc, State) of
                 ok ->
                     {ok, #{is_superuser => is_superuser(Doc, State)}};
                 {error, {cannot_find_password_hash_field, PasswordHashField}} ->
-                    ?LOG(error, "['~s'] Can't find password hash field: ~s", [Unique, PasswordHashField]),
+                    ?SLOG(error, #{msg => "can't find password hash field",
+                                   unique => Unique,
+                                   password_hash_field => PasswordHashField}),
                     {error, bad_username_or_password};
                 {error, Reason} ->
                     {error, Reason}

apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl

@@ -138,7 +138,9 @@ authenticate(#{password := Password} = Credential,
                     {error, Reason}
             end;
         {error, Reason} ->
-            ?LOG(error, "['~s'] Query failed: ~p", [Unique, Reason]),
+            ?SLOG(error, #{msg => "query failed",
+                           unique => Unique,
+                           reason => Reason}),
             ignore
     end.