Merge pull request #5684 from tigercl/chore/authn

chore(authn): provide easy-to-read hints for more errors

apps/emqx/src/emqx_authentication.erl

@@ -78,6 +78,25 @@
 -define(VER_1, <<"1">>).
 -define(VER_2, <<"2">>).
 
+-type chain_name() :: atom().
+-type authenticator_id() :: binary().
+-type position() :: top | bottom | {before, authenticator_id()}.
+-type update_request() :: {create_authenticator, chain_name(), map()}
+                        | {delete_authenticator, chain_name(), authenticator_id()}
+                        | {update_authenticator, chain_name(), authenticator_id(), map()}
+                        | {move_authenticator, chain_name(), authenticator_id(), position()}.
+-type authn_type() :: atom() | {atom(), atom()}.
+-type provider() :: module().
+
+-type chain() :: #{name := chain_name(),
+                   authenticators := [authenticator()]}.
+
+-type authenticator() :: #{id := authenticator_id(),
+                           provider := provider(),
+                           enable := boolean(),
+                           state := map()}.
+
+
 -type config() :: #{atom() => term()}.
 -type state() :: #{atom() => term()}.
 -type extra() :: #{is_superuser := boolean(),
@@ -128,7 +147,12 @@
 -callback update_user(UserID, UserInfo, State)
     -> {ok, User}
      | {error, term()}
-    when UserID::binary, UserInfo::map(), State::state(), User::user_info().
+    when UserID::binary(), UserInfo::map(), State::state(), User::user_info().
+
+-callback lookup_user(UserID, UserInfo, State)
+    -> {ok, User}
+     | {error, term()}
+    when UserID::binary(), UserInfo::map(), State::state(), User::user_info().
 
 -callback list_users(State)
     -> {ok, Users}
@@ -138,6 +162,7 @@
                     , add_user/2
                     , delete_user/2
                     , update_user/3
+                    , lookup_user/3
                     , list_users/1
                     ]).
 
@@ -159,6 +184,8 @@ authentication(_) -> undefined.
 %% Callbacks of config handler
 %%------------------------------------------------------------------------------
 
+-spec pre_config_update(update_request(), emqx_config:raw_config())
+    -> {ok, map() | list()} | {error, term()}.
 pre_config_update(UpdateReq, OldConfig) ->
     case do_pre_config_update(UpdateReq, to_list(OldConfig)) of
         {error, Reason} -> {error, Reason};
@@ -185,22 +212,22 @@ do_pre_config_update({move_authenticator, _ChainName, AuthenticatorID, Position}
         {error, Reason} -> {error, Reason};
         {ok, Part1, [Found | Part2]} ->
             case Position of
-                <<"top">> ->
+                top ->
                     {ok, [Found | Part1] ++ Part2};
-                <<"bottom">> ->
+                bottom ->
                     {ok, Part1 ++ Part2 ++ [Found]};
-                <<"before:", Before/binary>> ->
+                {before, Before} ->
                     case split_by_id(Before, Part1 ++ Part2) of
                         {error, Reason} ->
                             {error, Reason};
                         {ok, NPart1, [NFound | NPart2]} ->
                             {ok, NPart1 ++ [Found, NFound | NPart2]}
-                    end;
-                _ ->
-                    {error, {invalid_parameter, position}}
+                    end
             end
     end.
 
+-spec post_config_update(update_request(), map() | list(), emqx_config:raw_config(), emqx_config:app_envs())
+    -> ok | {ok, map()} | {error, term()}.
 post_config_update(UpdateReq, NewConfig, OldConfig, AppEnvs) ->
     do_post_config_update(UpdateReq, check_config(to_list(NewConfig)), OldConfig, AppEnvs).
 
@@ -220,13 +247,7 @@ do_post_config_update({update_authenticator, ChainName, AuthenticatorID, _Config
     update_authenticator(ChainName, AuthenticatorID, NConfig);
 
 do_post_config_update({move_authenticator, ChainName, AuthenticatorID, Position}, _NewConfig, _OldConfig, _AppEnvs) ->
-    NPosition = case Position of
-                    <<"top">> -> top;
-                    <<"bottom">> -> bottom;
-                    <<"before:", Before/binary>> ->
-                        {before, Before}
-                end,
-    move_authenticator(ChainName, AuthenticatorID, NPosition).
+    move_authenticator(ChainName, AuthenticatorID, Position).
 
 check_config(Config) ->
     #{authentication := CheckedConfig} = hocon_schema:check_plain(emqx_authentication,
@@ -269,6 +290,7 @@ do_authenticate([#authenticator{provider = Provider, state = State} | More], Cre
 %% APIs
 %%------------------------------------------------------------------------------
 
+-spec initialize_authentication(chain_name(), [#{binary() => term()}]) -> ok.
 initialize_authentication(_, []) ->
     ok;
 initialize_authentication(ChainName, AuthenticatorsConfig) ->
@@ -283,43 +305,56 @@ initialize_authentication(ChainName, AuthenticatorsConfig) ->
         end
     end, CheckedConfig).
 
+-spec start_link() -> {ok, pid()} | ignore | {error, term()}.
 start_link() ->
     gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
 
+-spec stop() -> ok.
 stop() ->
     gen_server:stop(?MODULE).
 
+-spec get_refs() -> {ok, Refs} when Refs :: [{authn_type(), module()}].
 get_refs() ->
     gen_server:call(?MODULE, get_refs).
 
+-spec add_provider(authn_type(), module()) -> ok.
 add_provider(AuthNType, Provider) ->
     gen_server:call(?MODULE, {add_provider, AuthNType, Provider}).
 
+-spec remove_provider(authn_type()) -> ok.
 remove_provider(AuthNType) ->
     gen_server:call(?MODULE, {remove_provider, AuthNType}).
 
+-spec create_chain(chain_name()) -> {ok, chain()} | {error, term()}.
 create_chain(Name) ->
     gen_server:call(?MODULE, {create_chain, Name}).
 
+-spec delete_chain(chain_name()) -> ok | {error, term()}.
 delete_chain(Name) ->
     gen_server:call(?MODULE, {delete_chain, Name}).
 
+-spec lookup_chain(chain_name()) -> {ok, chain()} | {error, term()}.
 lookup_chain(Name) ->
     gen_server:call(?MODULE, {lookup_chain, Name}).
 
+-spec list_chains() -> {ok, [chain()]}.
 list_chains() ->
     Chains = ets:tab2list(?CHAINS_TAB),
     {ok, [serialize_chain(Chain) || Chain <- Chains]}.
 
+-spec create_authenticator(chain_name(), config()) -> {ok, authenticator()} | {error, term()}.
 create_authenticator(ChainName, Config) ->
     gen_server:call(?MODULE, {create_authenticator, ChainName, Config}).
 
+-spec delete_authenticator(chain_name(), authenticator_id()) -> ok | {error, term()}.
 delete_authenticator(ChainName, AuthenticatorID) ->
     gen_server:call(?MODULE, {delete_authenticator, ChainName, AuthenticatorID}).
 
+-spec update_authenticator(chain_name(), authenticator_id(), config()) -> {ok, authenticator()} | {error, term()}.
 update_authenticator(ChainName, AuthenticatorID, Config) ->
     gen_server:call(?MODULE, {update_authenticator, ChainName, AuthenticatorID, Config}).
 
+-spec lookup_authenticator(chain_name(), authenticator_id()) -> {ok, authenticator()} | {error, term()}.
 lookup_authenticator(ChainName, AuthenticatorID) ->
     case ets:lookup(?CHAINS_TAB, ChainName) of
         [] ->
@@ -333,6 +368,7 @@ lookup_authenticator(ChainName, AuthenticatorID) ->
             end
     end.
 
+-spec list_authenticators(chain_name()) -> {ok, [authenticator()]} | {error, term()}.
 list_authenticators(ChainName) ->
     case ets:lookup(?CHAINS_TAB, ChainName) of
         [] ->
@@ -341,28 +377,36 @@ list_authenticators(ChainName) ->
             {ok, serialize_authenticators(Authenticators)}
     end.
 
+-spec move_authenticator(chain_name(), authenticator_id(), position()) -> ok | {error, term()}.
 move_authenticator(ChainName, AuthenticatorID, Position) ->
     gen_server:call(?MODULE, {move_authenticator, ChainName, AuthenticatorID, Position}).
 
+-spec import_users(chain_name(), authenticator_id(), binary()) -> ok | {error, term()}.
 import_users(ChainName, AuthenticatorID, Filename) ->
     gen_server:call(?MODULE, {import_users, ChainName, AuthenticatorID, Filename}).
 
+-spec add_user(chain_name(), authenticator_id(), user_info()) -> {ok, user_info()} | {error, term()}.
 add_user(ChainName, AuthenticatorID, UserInfo) ->
     gen_server:call(?MODULE, {add_user, ChainName, AuthenticatorID, UserInfo}).
 
+-spec delete_user(chain_name(), authenticator_id(), binary()) -> ok | {error, term()}.
 delete_user(ChainName, AuthenticatorID, UserID) ->
     gen_server:call(?MODULE, {delete_user, ChainName, AuthenticatorID, UserID}).
 
+-spec update_user(chain_name(), authenticator_id(), binary(), map()) -> {ok, user_info()} | {error, term()}.
 update_user(ChainName, AuthenticatorID, UserID, NewUserInfo) ->
     gen_server:call(?MODULE, {update_user, ChainName, AuthenticatorID, UserID, NewUserInfo}).
 
+-spec lookup_user(chain_name(), authenticator_id(), binary()) -> {ok, user_info()} | {error, term()}.
 lookup_user(ChainName, AuthenticatorID, UserID) ->
     gen_server:call(?MODULE, {lookup_user, ChainName, AuthenticatorID, UserID}).
 
 %% TODO: Support pagination
+-spec list_users(chain_name(), authenticator_id()) -> {ok, [user_info()]} | {error, term()}.
 list_users(ChainName, AuthenticatorID) ->
     gen_server:call(?MODULE, {list_users, ChainName, AuthenticatorID}).
 
+-spec generate_id(config()) -> authenticator_id().
 generate_id(#{mechanism := Mechanism0, backend := Backend0}) ->
     Mechanism = atom_to_binary(Mechanism0),
     Backend = atom_to_binary(Backend0),
@@ -484,7 +528,7 @@ handle_call({update_authenticator, ChainName, AuthenticatorID, Config}, _From, S
                                     {error, Reason}
                             end;
                         false ->
-                            {error, mechanism_or_backend_change_is_not_alloed}
+                            {error, change_of_authentication_type_is_not_allowed}
                     end
             end
         end,
@@ -679,7 +723,7 @@ call_authenticator(ChainName, AuthenticatorID, Func, Args) ->
                         true ->
                             erlang:apply(Provider, Func, Args ++ [State]);
                         false ->
-                            {error, unsupported_feature}
+                            {error, unsupported_operation}
                     end
             end
         end,

apps/emqx/src/emqx_listeners.erl

@@ -253,7 +253,7 @@ do_start_listener(quic, ListenerName, #{bind := ListenOn} = Opts) ->
     end.
 
 delete_authentication(Type, ListenerName, _Conf) ->
-    emqx_authentication:delete_chain(atom_to_binary(listener_id(Type, ListenerName))).
+    emqx_authentication:delete_chain(listener_id(Type, ListenerName)).
 
 %% Update the listeners at runtime
 post_config_update(_Req, NewListeners, OldListeners, _AppEnvs) ->

apps/emqx/test/emqx_authentication_SUITE.erl

@@ -206,7 +206,7 @@ t_update_config(_) ->
     ?assertMatch({ok, _}, update_config([authentication], {update_authenticator, Global, ID1, #{}})),
     ?assertMatch({ok, #{id := ID1, state := #{mark := 2}}}, ?AUTHN:lookup_authenticator(Global, ID1)),
 
-    ?assertMatch({ok, _}, update_config([authentication], {move_authenticator, Global, ID2, <<"top">>})),
+    ?assertMatch({ok, _}, update_config([authentication], {move_authenticator, Global, ID2, top})),
     ?assertMatch({ok, [#{id := ID2}, #{id := ID1}]}, ?AUTHN:list_authenticators(Global)),
 
     ?assertMatch({ok, _}, update_config([authentication], {delete_authenticator, Global, ID1})),
@@ -223,7 +223,7 @@ t_update_config(_) ->
     ?assertMatch({ok, _}, update_config(ConfKeyPath, {update_authenticator, ListenerID, ID1, #{}})),
     ?assertMatch({ok, #{id := ID1, state := #{mark := 2}}}, ?AUTHN:lookup_authenticator(ListenerID, ID1)),
 
-    ?assertMatch({ok, _}, update_config(ConfKeyPath, {move_authenticator, ListenerID, ID2, <<"top">>})),
+    ?assertMatch({ok, _}, update_config(ConfKeyPath, {move_authenticator, ListenerID, ID2, top})),
     ?assertMatch({ok, [#{id := ID2}, #{id := ID1}]}, ?AUTHN:list_authenticators(ListenerID)),
 
     ?assertMatch({ok, _}, update_config(ConfKeyPath, {delete_authenticator, ListenerID, ID1})),

apps/emqx_authn/src/emqx_authn_api.erl

@@ -37,7 +37,7 @@
 
 -define(EXAMPLE_1, #{mechanism => <<"password-based">>,
                      backend => <<"built-in-database">>,
-                     query => <<"SELECT password_hash from built-in-database WHERE username = ${username}">>,
+                     user_id_type => <<"username">>,
                      password_hash_algorithm => #{
                          name => <<"sha256">>
                      }}).
@@ -1193,10 +1193,10 @@ definitions() ->
                 enum => [<<"built-in-database">>],
                 example => <<"built-in-database">>
             },
-            query => #{
+            user_id_type => #{
                 type => string,
-                default => <<"SELECT password_hash from built-in-database WHERE username = ${username}">>,
-                example => <<"SELECT password_hash from built-in-database WHERE username = ${username}">>
+                enum => [<<"username">>, <<"clientid">>],
+                example => <<"username">>
             },
             password_hash_algorithm => minirest:ref(<<"PasswordHashAlgorithm">>)
         }
@@ -1550,7 +1550,8 @@ definitions() ->
             enable_pipelining => #{
                 type => boolean,
                 default => true
-            }
+            },
+            ssl => minirest:ref(<<"SSL">>)
         }
     },
 
@@ -1584,6 +1585,10 @@ definitions() ->
             certificate => #{
                 type => string
             },
+            endpoint => #{
+                type => string,
+                example => <<"http://localhost:80">>
+            },
             verify_claims => #{
                 type => object,
                 additionalProperties => #{
@@ -1631,6 +1636,7 @@ definitions() ->
             },
             salt_rounds => #{
                 type => integer,
+                description => <<"Only valid when the name field is set to bcrypt">>,
                 default => 10
             }
         }
@@ -1857,8 +1863,7 @@ list_authenticator(ConfKeyPath, AuthenticatorID) ->
 
 update_authenticator(ConfKeyPath, ChainName0, AuthenticatorID, Config) ->
     ChainName = to_atom(ChainName0),
-    case update_config(ConfKeyPath,
-                                  {update_authenticator, ChainName, AuthenticatorID, Config}) of
+    case update_config(ConfKeyPath, {update_authenticator, ChainName, AuthenticatorID, Config}) of
         {ok, #{post_config_update := #{?AUTHN := #{id := ID}},
                raw_config := AuthenticatorsConfig}} ->
             {ok, AuthenticatorConfig} = find_config(ID, AuthenticatorsConfig),
@@ -1878,10 +1883,15 @@ delete_authenticator(ConfKeyPath, ChainName0, AuthenticatorID) ->
 
 move_authenitcator(ConfKeyPath, ChainName0, AuthenticatorID, Position) ->
     ChainName = to_atom(ChainName0),
-    case update_config(ConfKeyPath, {move_authenticator, ChainName, AuthenticatorID, Position}) of
-        {ok, _} ->
-            {204};
-        {error, {_, _, Reason}} ->
+    case parse_position(Position) of
+        {ok, NPosition} ->
+            case update_config(ConfKeyPath, {move_authenticator, ChainName, AuthenticatorID, NPosition}) of
+                {ok, _} ->
+                    {204};
+                {error, {_, _, Reason}} ->
+                    serialize_error(Reason)
+            end;
+        {error, Reason} ->
             serialize_error(Reason)
     end.
 
@@ -1963,29 +1973,69 @@ fill_defaults(Config) ->
 
 serialize_error({not_found, {authenticator, ID}}) ->
     {404, #{code => <<"NOT_FOUND">>,
-            message => list_to_binary(io_lib:format("Authenticator '~s' does not exist", [ID]))}};
+            message => list_to_binary(
+                io_lib:format("Authenticator '~s' does not exist", [ID])
+            )}};
+
 serialize_error({not_found, {listener, ID}}) ->
     {404, #{code => <<"NOT_FOUND">>,
-            message => list_to_binary(io_lib:format("Listener '~s' does not exist", [ID]))}};
+            message => list_to_binary(
+                io_lib:format("Listener '~s' does not exist", [ID])
+            )}};
+
+serialize_error({not_found, {chain, ?GLOBAL}}) ->
+    {500, #{code => <<"INTERNAL_SERVER_ERROR">>,
+            message => <<"Authentication status is abnormal">>}};
+
+serialize_error({not_found, {chain, Name}}) ->
+    {400, #{code => <<"BAD_REQUEST">>,
+            message => list_to_binary(
+                io_lib:format("No authentication has been create for listener '~s'", [Name])
+            )}};
+
 serialize_error({already_exists, {authenticator, ID}}) ->
     {409, #{code => <<"ALREADY_EXISTS">>,
             message => list_to_binary(
                 io_lib:format("Authenticator '~s' already exist", [ID])
             )}};
+
+serialize_error(no_available_provider) ->
+    {400, #{code => <<"BAD_REQUEST">>,
+            message => <<"Unsupported authentication type">>}};
+
+serialize_error(change_of_authentication_type_is_not_allowed) ->
+    {400, #{code => <<"BAD_REQUEST">>,
+            message => <<"Change of authentication type is not allowed">>}};
+
+serialize_error(unsupported_operation) ->
+    {400, #{code => <<"BAD_REQUEST">>,
+            message => <<"Operation not supported in this authentication type">>}};
+
 serialize_error({missing_parameter, Name}) ->
     {400, #{code => <<"MISSING_PARAMETER">>,
             message => list_to_binary(
                 io_lib:format("The input parameter '~p' that is mandatory for processing this request is not supplied", [Name])
             )}};
+
 serialize_error({invalid_parameter, Name}) ->
     {400, #{code => <<"INVALID_PARAMETER">>,
             message => list_to_binary(
                 io_lib:format("The value of input parameter '~p' is invalid", [Name])
             )}};
+
 serialize_error(Reason) ->
     {400, #{code => <<"BAD_REQUEST">>,
             message => list_to_binary(io_lib:format("~p", [Reason]))}}.
 
+parse_position(<<"top">>) ->
+    {ok, top};
+parse_position(<<"bottom">>) ->
+    {ok, bottom};
+parse_position(<<"before:", Before/binary>>) ->
+    {ok, {before, Before}};
+parse_position(_) ->
+    {error, {invalid_parameter, position}}.
+
 to_list(M) when is_map(M) ->
     [M];
 to_list(L) when is_list(L) ->

apps/emqx_authn/src/simple_authn/emqx_authn_http.erl

@@ -214,7 +214,7 @@ default_headers_no_content_type() ->
 
 transform_header_name(Headers) ->
     maps:fold(fun(K0, V, Acc) ->
-                  K = list_to_binary(string:to_lower(binary_to_list(K0))),
+                  K = list_to_binary(string:to_lower(to_list(K0))),
                   maps:put(K, V, Acc)
               end, #{}, Headers).
 
@@ -301,3 +301,8 @@ parse_body(<<"application/x-www-form-urlencoded">>, Body) ->
     {ok, maps:from_list(cow_qs:parse_qs(Body))};
 parse_body(ContentType, _) ->
     {error, {unsupported_content_type, ContentType}}.
+
+to_list(A) when is_atom(A) ->
+    atom_to_list(A);
+to_list(B) when is_binary(B) ->
+    binary_to_list(B).

apps/emqx_authz/src/emqx_authz_schema.erl

@@ -66,7 +66,7 @@ fields(http_get) ->
                               },
                   converter => fun (Headers0) ->
                                     Headers1 = maps:fold(fun(K0, V, AccIn) ->
-                                                           K1 = iolist_to_binary(string:to_lower(binary_to_list(K0))),
+                                                           K1 = iolist_to_binary(string:to_lower(to_list(K0))),
                                                            maps:put(K1, V, AccIn)
                                                         end, #{}, Headers0),
                                     maps:merge(#{ <<"accept">> => <<"application/json">>
@@ -177,3 +177,8 @@ connector_fields(DB, Fields) ->
     , {enable, #{type => boolean(),
                  default => true}}
     ] ++ Mod:fields(Fields).
+
+to_list(A) when is_atom(A) ->
+    atom_to_list(A);
+to_list(B) when is_binary(B) ->
+    binary_to_list(B).

apps/emqx_connector/src/emqx_connector_http.erl

@@ -58,14 +58,7 @@ fields(config) ->
     , {pool_type,         fun pool_type/1}
     , {pool_size,         fun pool_size/1}
     , {enable_pipelining, fun enable_pipelining/1}
-    ] ++ emqx_connector_schema_lib:ssl_fields();
-
-fields(ssl_opts) ->
-    [ {cacertfile, fun cacertfile/1}
-    , {keyfile,    fun keyfile/1}
-    , {certfile,   fun certfile/1}
-    , {verify,     fun verify/1}
-    ].
+    ] ++ emqx_connector_schema_lib:ssl_fields().
 
 validations() ->
     [ {check_ssl_opts, fun check_ssl_opts/1} ].
@@ -102,23 +95,6 @@ enable_pipelining(type) -> boolean();
 enable_pipelining(default) -> true;
 enable_pipelining(_) -> undefined.
 
-cacertfile(type) -> string();
-cacertfile(nullable) -> true;
-cacertfile(_) -> undefined.
-
-keyfile(type) -> string();
-keyfile(nullable) -> true;
-keyfile(_) -> undefined.
-
-%% TODO: certfile is required
-certfile(type) -> string();
-certfile(nullable) -> true;
-certfile(_) -> undefined.
-
-verify(type) -> boolean();
-verify(default) -> false;
-verify(_) -> undefined.
-
 %% ===================================================================
 on_start(InstId, #{base_url := #{scheme := Scheme,
                                  host := Host,