Merge pull request #11771 from savonarola/1015-validate-bcrypt-schema-in-api

feat(authn): allow authn providers to define a separate schama for API

apps/emqx_auth/src/emqx_authn/emqx_authn_api.erl

@@ -147,7 +147,7 @@ schema("/authentication") ->
             description => ?DESC(authentication_get),
             responses => #{
                 200 => emqx_dashboard_swagger:schema_with_example(
-                    hoconsc:array(emqx_authn_schema:authenticator_type()),
+                    hoconsc:array(authenticator_type(config)),
                     authenticator_array_example()
                 )
             }
@@ -156,12 +156,12 @@ schema("/authentication") ->
             tags => ?API_TAGS_GLOBAL,
             description => ?DESC(authentication_post),
             'requestBody' => emqx_dashboard_swagger:schema_with_examples(
-                emqx_authn_schema:authenticator_type(),
+                authenticator_type(api_write),
                 authenticator_examples()
             ),
             responses => #{
                 200 => emqx_dashboard_swagger:schema_with_examples(
-                    emqx_authn_schema:authenticator_type(),
+                    authenticator_type(config),
                     authenticator_examples()
                 ),
                 400 => error_codes([?BAD_REQUEST], <<"Bad Request">>),
@@ -178,7 +178,7 @@ schema("/authentication/:id") ->
             parameters => [param_auth_id()],
             responses => #{
                 200 => emqx_dashboard_swagger:schema_with_examples(
-                    emqx_authn_schema:authenticator_type(),
+                    authenticator_type(config),
                     authenticator_examples()
                 ),
                 404 => error_codes([?NOT_FOUND], <<"Not Found">>)
@@ -189,7 +189,7 @@ schema("/authentication/:id") ->
             description => ?DESC(authentication_id_put),
             parameters => [param_auth_id()],
             'requestBody' => emqx_dashboard_swagger:schema_with_examples(
-                emqx_authn_schema:authenticator_type(),
+                authenticator_type(api_write),
                 authenticator_examples()
             ),
             responses => #{
@@ -236,7 +236,7 @@ schema("/listeners/:listener_id/authentication") ->
             parameters => [param_listener_id()],
             responses => #{
                 200 => emqx_dashboard_swagger:schema_with_example(
-                    hoconsc:array(emqx_authn_schema:authenticator_type()),
+                    hoconsc:array(authenticator_type(config)),
                     authenticator_array_example()
                 )
             }
@@ -247,12 +247,12 @@ schema("/listeners/:listener_id/authentication") ->
             description => ?DESC(listeners_listener_id_authentication_post),
             parameters => [param_listener_id()],
             'requestBody' => emqx_dashboard_swagger:schema_with_examples(
-                emqx_authn_schema:authenticator_type(),
+                authenticator_type(api_write),
                 authenticator_examples()
             ),
             responses => #{
                 200 => emqx_dashboard_swagger:schema_with_examples(
-                    emqx_authn_schema:authenticator_type(),
+                    authenticator_type(config),
                     authenticator_examples()
                 ),
                 400 => error_codes([?BAD_REQUEST], <<"Bad Request">>),
@@ -270,7 +270,7 @@ schema("/listeners/:listener_id/authentication/:id") ->
             parameters => [param_listener_id(), param_auth_id()],
             responses => #{
                 200 => emqx_dashboard_swagger:schema_with_examples(
-                    emqx_authn_schema:authenticator_type(),
+                    authenticator_type(config),
                     authenticator_examples()
                 ),
                 404 => error_codes([?NOT_FOUND], <<"Not Found">>)
@@ -282,7 +282,7 @@ schema("/listeners/:listener_id/authentication/:id") ->
             description => ?DESC(listeners_listener_id_authentication_id_put),
             parameters => [param_listener_id(), param_auth_id()],
             'requestBody' => emqx_dashboard_swagger:schema_with_examples(
-                emqx_authn_schema:authenticator_type(),
+                authenticator_type(api_write),
                 authenticator_examples()
             ),
             responses => #{
@@ -1278,6 +1278,9 @@ paginated_list_type(Type) ->
         {meta, ref(emqx_dashboard_swagger, meta)}
     ].
 
+authenticator_type(Kind) ->
+    emqx_authn_schema:authenticator_type(Kind).
+
 authenticator_array_example() ->
     [Config || #{value := Config} <- maps:values(authenticator_examples())].
 

apps/emqx_auth/src/emqx_authn/emqx_authn_password_hashing.erl

@@ -53,7 +53,8 @@
 
 -export([
     type_ro/1,
-    type_rw/1
+    type_rw/1,
+    type_rw_api/1
 ]).
 
 -export([
@@ -67,21 +68,17 @@
 -define(SALT_ROUNDS_MAX, 10).
 
 namespace() -> "authn-hash".
-roots() -> [pbkdf2, bcrypt, bcrypt_rw, simple].
+roots() -> [pbkdf2, bcrypt, bcrypt_rw, bcrypt_rw_api, simple].
 
 fields(bcrypt_rw) ->
     fields(bcrypt) ++
         [
-            {salt_rounds,
-                sc(
-                    range(?SALT_ROUNDS_MIN, ?SALT_ROUNDS_MAX),
-                    #{
-                        default => ?SALT_ROUNDS_MAX,
-                        example => ?SALT_ROUNDS_MAX,
-                        desc => "Work factor for BCRYPT password generation.",
-                        converter => fun salt_rounds_converter/2
-                    }
-                )}
+            {salt_rounds, fun bcrypt_salt_rounds/1}
+        ];
+fields(bcrypt_rw_api) ->
+    fields(bcrypt) ++
+        [
+            {salt_rounds, fun bcrypt_salt_rounds_api/1}
         ];
 fields(bcrypt) ->
     [{name, sc(bcrypt, #{required => true, desc => "BCRYPT password hashing."})}];
@@ -110,6 +107,15 @@ fields(simple) ->
         {salt_position, fun salt_position/1}
     ].
 
+bcrypt_salt_rounds(converter) -> fun salt_rounds_converter/2;
+bcrypt_salt_rounds(Option) -> bcrypt_salt_rounds_api(Option).
+
+bcrypt_salt_rounds_api(type) -> range(?SALT_ROUNDS_MIN, ?SALT_ROUNDS_MAX);
+bcrypt_salt_rounds_api(default) -> ?SALT_ROUNDS_MAX;
+bcrypt_salt_rounds_api(example) -> ?SALT_ROUNDS_MAX;
+bcrypt_salt_rounds_api(desc) -> "Work factor for BCRYPT password generation.";
+bcrypt_salt_rounds_api(_) -> undefined.
+
 salt_rounds_converter(undefined, _) ->
     undefined;
 salt_rounds_converter(I, _) when is_integer(I) ->
@@ -119,6 +125,8 @@ salt_rounds_converter(X, _) ->
 
 desc(bcrypt_rw) ->
     "Settings for bcrypt password hashing algorithm (for DB backends with write capability).";
+desc(bcrypt_rw_api) ->
+    desc(bcrypt_rw);
 desc(bcrypt) ->
     "Settings for bcrypt password hashing algorithm.";
 desc(pbkdf2) ->
@@ -143,14 +151,20 @@ dk_length(desc) ->
 dk_length(_) ->
     undefined.
 
-%% for simple_authn/emqx_authn_mnesia
+%% for emqx_authn_mnesia
 type_rw(type) ->
     hoconsc:union(rw_refs());
-type_rw(default) ->
-    #{<<"name">> => sha256, <<"salt_position">> => prefix};
 type_rw(desc) ->
     "Options for password hash creation and verification.";
-type_rw(_) ->
+type_rw(Option) ->
+    type_ro(Option).
+
+%% for emqx_authn_mnesia API
+type_rw_api(type) ->
+    hoconsc:union(api_refs());
+type_rw_api(desc) ->
+    "Options for password hash creation and verification through API.";
+type_rw_api(_) ->
     undefined.
 
 %% for other authn resources
@@ -242,31 +256,41 @@ check_password(#{name := Other, salt_position := SaltPosition}, Salt, PasswordHa
 %%------------------------------------------------------------------------------
 
 rw_refs() ->
-    All = [
-        hoconsc:ref(?MODULE, bcrypt_rw),
-        hoconsc:ref(?MODULE, pbkdf2),
-        hoconsc:ref(?MODULE, simple)
-    ],
-    fun
-        (all_union_members) -> All;
-        ({value, #{<<"name">> := <<"bcrypt">>}}) -> [hoconsc:ref(?MODULE, bcrypt_rw)];
-        ({value, #{<<"name">> := <<"pbkdf2">>}}) -> [hoconsc:ref(?MODULE, pbkdf2)];
-        ({value, #{<<"name">> := _}}) -> [hoconsc:ref(?MODULE, simple)];
-        ({value, _}) -> throw(#{reason => "algorithm_name_missing"})
-    end.
+    union_selector(rw).
 
 ro_refs() ->
-    All = [
-        hoconsc:ref(?MODULE, bcrypt),
-        hoconsc:ref(?MODULE, pbkdf2),
-        hoconsc:ref(?MODULE, simple)
-    ],
+    union_selector(ro).
+
+api_refs() ->
+    union_selector(api).
+
+sc(Type, Meta) -> hoconsc:mk(Type, Meta).
+
+union_selector(Kind) ->
     fun
-        (all_union_members) -> All;
-        ({value, #{<<"name">> := <<"bcrypt">>}}) -> [hoconsc:ref(?MODULE, bcrypt)];
-        ({value, #{<<"name">> := <<"pbkdf2">>}}) -> [hoconsc:ref(?MODULE, pbkdf2)];
-        ({value, #{<<"name">> := _}}) -> [hoconsc:ref(?MODULE, simple)];
+        (all_union_members) -> refs(Kind);
+        ({value, #{<<"name">> := <<"bcrypt">>}}) -> [bcrypt_ref(Kind)];
+        ({value, #{<<"name">> := <<"pbkdf2">>}}) -> [pbkdf2_ref(Kind)];
+        ({value, #{<<"name">> := _}}) -> [simple_ref(Kind)];
         ({value, _}) -> throw(#{reason => "algorithm_name_missing"})
     end.
 
-sc(Type, Meta) -> hoconsc:mk(Type, Meta).
+refs(Kind) ->
+    [
+        bcrypt_ref(Kind),
+        pbkdf2_ref(Kind),
+        simple_ref(Kind)
+    ].
+
+pbkdf2_ref(_) ->
+    hoconsc:ref(?MODULE, pbkdf2).
+
+bcrypt_ref(rw) ->
+    hoconsc:ref(?MODULE, bcrypt_rw);
+bcrypt_ref(api) ->
+    hoconsc:ref(?MODULE, bcrypt_rw_api);
+bcrypt_ref(_) ->
+    hoconsc:ref(?MODULE, bcrypt).
+
+simple_ref(_) ->
+    hoconsc:ref(?MODULE, simple).

apps/emqx_auth/src/emqx_authn/emqx_authn_schema.erl

@@ -34,7 +34,9 @@
     tags/0,
     fields/1,
     authenticator_type/0,
+    authenticator_type/1,
     authenticator_type_without/1,
+    authenticator_type_without/2,
     mechanism/1,
     backend/1
 ]).
@@ -43,17 +45,35 @@
     global_auth_fields/0
 ]).
 
+-export_type([shema_kind/0]).
+
 -define(AUTHN_MODS_PT_KEY, {?MODULE, authn_schema_mods}).
+-define(DEFAULT_SCHEMA_KIND, config).
 
 %%--------------------------------------------------------------------
 %% Authn Source Schema Behaviour
 %%--------------------------------------------------------------------
 
 -type schema_ref() :: ?R_REF(module(), hocon_schema:name()).
+-type shema_kind() ::
+    %% api_write: schema for mutating API request validation
+    api_write
+    %% config: schema for config validation
+    | config.
 -callback refs() -> [schema_ref()].
--callback select_union_member(emqx_config:raw_config()) -> schema_ref() | undefined | no_return().
+-callback refs(shema_kind()) -> [schema_ref()].
+-callback select_union_member(emqx_config:raw_config()) -> [schema_ref()] | undefined | no_return().
+-callback select_union_member(shema_kind(), emqx_config:raw_config()) ->
+    [schema_ref()] | undefined | no_return().
 -callback fields(hocon_schema:name()) -> [hocon_schema:field()].
 
+-optional_callbacks([
+    select_union_member/1,
+    select_union_member/2,
+    refs/0,
+    refs/1
+]).
+
 roots() -> [].
 
 injected_fields(AuthnSchemaMods) ->
@@ -67,45 +87,63 @@ tags() ->
     [<<"Authentication">>].
 
 authenticator_type() ->
-    hoconsc:union(union_member_selector(provider_schema_mods())).
+    authenticator_type(?DEFAULT_SCHEMA_KIND).
+
+authenticator_type(Kind) ->
+    hoconsc:union(union_member_selector(Kind, provider_schema_mods())).
 
 authenticator_type_without(ProviderSchemaMods) ->
+    authenticator_type_without(?DEFAULT_SCHEMA_KIND, ProviderSchemaMods).
+
+authenticator_type_without(Kind, ProviderSchemaMods) ->
     hoconsc:union(
-        union_member_selector(provider_schema_mods() -- ProviderSchemaMods)
+        union_member_selector(Kind, provider_schema_mods() -- ProviderSchemaMods)
     ).
 
-union_member_selector(Mods) ->
-    AllTypes = config_refs(Mods),
+union_member_selector(Kind, Mods) ->
+    AllTypes = config_refs(Kind, Mods),
     fun
         (all_union_members) -> AllTypes;
-        ({value, Value}) -> select_union_member(Value, Mods)
+        ({value, Value}) -> select_union_member(Kind, Value, Mods)
     end.
 
-select_union_member(#{<<"mechanism">> := Mechanism, <<"backend">> := Backend}, []) ->
+select_union_member(_Kind, #{<<"mechanism">> := Mechanism, <<"backend">> := Backend}, []) ->
     throw(#{
         reason => "unsupported_mechanism",
         mechanism => Mechanism,
         backend => Backend
     });
-select_union_member(#{<<"mechanism">> := Mechanism}, []) ->
+select_union_member(_Kind, #{<<"mechanism">> := Mechanism}, []) ->
     throw(#{
         reason => "unsupported_mechanism",
         mechanism => Mechanism
     });
-select_union_member(#{<<"mechanism">> := _} = Value, [Mod | Mods]) ->
-    case Mod:select_union_member(Value) of
+select_union_member(Kind, #{<<"mechanism">> := _} = Value, [Mod | Mods]) ->
+    case mod_select_union_member(Kind, Value, Mod) of
         undefined ->
-            select_union_member(Value, Mods);
+            select_union_member(Kind, Value, Mods);
         Member ->
             Member
     end;
-select_union_member(#{} = _Value, _Mods) ->
+select_union_member(_Kind, #{} = _Value, _Mods) ->
     throw(#{reason => "missing_mechanism_field"});
-select_union_member(Value, _Mods) ->
+select_union_member(_Kind, Value, _Mods) ->
     throw(#{reason => "not_a_struct", value => Value}).
 
-config_refs(Mods) ->
-    lists:append([Mod:refs() || Mod <- Mods]).
+mod_select_union_member(Kind, Value, Mod) ->
+    emqx_utils:call_first_defined([
+        {Mod, select_union_member, [Kind, Value]},
+        {Mod, select_union_member, [Value]}
+    ]).
+
+config_refs(Kind, Mods) ->
+    lists:append([mod_refs(Kind, Mod) || Mod <- Mods]).
+
+mod_refs(Kind, Mod) ->
+    emqx_utils:call_first_defined([
+        {Mod, refs, [Kind]},
+        {Mod, refs, []}
+    ]).
 
 root_type() ->
     hoconsc:array(authenticator_type()).

apps/emqx_auth/test/emqx_authn/emqx_authn_api_SUITE.erl

@@ -63,14 +63,16 @@ end_per_testcase(_, Config) ->
 init_per_suite(Config) ->
     Apps = emqx_cth_suite:start(
         [
-            emqx,
             emqx_conf,
+            emqx,
             emqx_auth,
+            %% to load schema
+            {emqx_auth_mnesia, #{start => false}},
             emqx_management,
             {emqx_dashboard, "dashboard.listeners.http { enable = true, bind = 18083 }"}
         ],
         #{
-            work_dir => ?config(priv_dir, Config)
+            work_dir => filename:join(?config(priv_dir, Config), ?MODULE)
         }
     ),
     _ = emqx_common_test_http:create_default_app(),
@@ -535,6 +537,36 @@ ignore_switch_to_global_chain(_) ->
     ),
     ok = emqtt:disconnect(Client4).
 
+t_bcrypt_validation(_Config) ->
+    BaseConf = #{
+        mechanism => <<"password_based">>,
+        backend => <<"built_in_database">>,
+        user_id_type => <<"username">>
+    },
+    BcryptValid = #{
+        name => <<"bcrypt">>,
+        salt_rounds => 10
+    },
+    BcryptInvalid = #{
+        name => <<"bcrypt">>,
+        salt_rounds => 15
+    },
+
+    ConfValid = BaseConf#{password_hash_algorithm => BcryptValid},
+    ConfInvalid = BaseConf#{password_hash_algorithm => BcryptInvalid},
+
+    {ok, 400, _} = request(
+        post,
+        uri([?CONF_NS]),
+        ConfInvalid
+    ),
+
+    {ok, 200, _} = request(
+        post,
+        uri([?CONF_NS]),
+        ConfValid
+    ).
+
 %%------------------------------------------------------------------------------
 %% Helpers
 %%------------------------------------------------------------------------------

apps/emqx_auth/test/emqx_authz/emqx_authz_SUITE.erl

@@ -70,6 +70,7 @@ init_per_testcase(TestCase, Config) when
     {ok, _} = emqx:update_config([authorization, deny_action], disconnect),
     Config;
 init_per_testcase(_TestCase, Config) ->
+    _ = file:delete(emqx_authz_file:acl_conf_file()),
     {ok, _} = emqx_authz:update(?CMD_REPLACE, []),
     Config.
 

apps/emqx_auth_mnesia/src/emqx_authn_mnesia_schema.erl

@@ -24,27 +24,30 @@
 -export([
     fields/1,
     desc/1,
-    refs/0,
-    select_union_member/1
+    refs/1,
+    select_union_member/2
 ]).
 
-refs() ->
+refs(api_write) ->
+    [?R_REF(builtin_db_api)];
+refs(_) ->
     [?R_REF(builtin_db)].
 
-select_union_member(#{
+select_union_member(Kind, #{
     <<"mechanism">> := ?AUTHN_MECHANISM_SIMPLE_BIN, <<"backend">> := ?AUTHN_BACKEND_BIN
 }) ->
-    refs();
-select_union_member(_) ->
+    refs(Kind);
+select_union_member(_Kind, _Value) ->
     undefined.
 
 fields(builtin_db) ->
     [
-        {mechanism, emqx_authn_schema:mechanism(?AUTHN_MECHANISM_SIMPLE)},
-        {backend, emqx_authn_schema:backend(?AUTHN_BACKEND)},
-        {user_id_type, fun user_id_type/1},
         {password_hash_algorithm, fun emqx_authn_password_hashing:type_rw/1}
-    ] ++ emqx_authn_schema:common_fields().
+    ] ++ common_fields();
+fields(builtin_db_api) ->
+    [
+        {password_hash_algorithm, fun emqx_authn_password_hashing:type_rw_api/1}
+    ] ++ common_fields().
 
 desc(builtin_db) ->
     ?DESC(builtin_db);
@@ -56,3 +59,10 @@ user_id_type(desc) -> ?DESC(?FUNCTION_NAME);
 user_id_type(default) -> <<"username">>;
 user_id_type(required) -> true;
 user_id_type(_) -> undefined.
+
+common_fields() ->
+    [
+        {mechanism, emqx_authn_schema:mechanism(?AUTHN_MECHANISM_SIMPLE)},
+        {backend, emqx_authn_schema:backend(?AUTHN_BACKEND)},
+        {user_id_type, fun user_id_type/1}
+    ] ++ emqx_authn_schema:common_fields().

apps/emqx_utils/src/emqx_utils.erl

@@ -62,7 +62,8 @@
     merge_lists/3,
     tcp_keepalive_opts/4,
     format/1,
-    format_mfal/1
+    format_mfal/1,
+    call_first_defined/1
 ]).
 
 -export([
@@ -554,6 +555,22 @@ format_mfal(Data) ->
             undefined
     end.
 
+-spec call_first_defined(list({module(), atom(), list()})) -> term() | no_return().
+call_first_defined([{Module, Function, Args} | Rest]) ->
+    try
+        apply(Module, Function, Args)
+    catch
+        error:undef:Stacktrace ->
+            case Stacktrace of
+                [{Module, Function, _, _} | _] ->
+                    call_first_defined(Rest);
+                _ ->
+                    erlang:raise(error, undef, Stacktrace)
+            end
+    end;
+call_first_defined([]) ->
+    error(none_fun_is_defined).
+
 %%------------------------------------------------------------------------------
 %% Internal Functions
 %%------------------------------------------------------------------------------

changes/ce/fix-11771.en.md

@@ -0,0 +1 @@
+Fixed validation of Bcrypt salt rounds in authentification management through the API/Dashboard.