Merge pull request #5763 from tigercl/feat/clear-certs

feat(clear certs): clear certs when deleting instance

apps/emqx/src/emqx_authentication.erl

@@ -199,12 +199,15 @@ pre_config_update(UpdateReq, OldConfig) ->
         {ok, NewConfig} -> {ok, may_to_map(NewConfig)}
     end.
 
-do_pre_config_update({create_authenticator, _ChainName, Config}, OldConfig) ->
-    try convert_certs(Config) of
-        NConfig ->
-            {ok, OldConfig ++ [NConfig]}
+do_pre_config_update({create_authenticator, ChainName, Config}, OldConfig) ->
+    try 
+        CertsDir = certs_dir([to_bin(ChainName), generate_id(Config)]),
+        NConfig = convert_certs(CertsDir, Config),
+        {ok, OldConfig ++ [NConfig]}
     catch
         error:{save_cert_to_file, _} = Reason ->
+            {error, Reason};
+        error:{missing_parameter, _} = Reason ->
             {error, Reason}
     end;
 do_pre_config_update({delete_authenticator, _ChainName, AuthenticatorID}, OldConfig) ->
@@ -212,17 +215,21 @@ do_pre_config_update({delete_authenticator, _ChainName, AuthenticatorID}, OldCon
                                 AuthenticatorID =/= generate_id(OldConfig0)
                              end, OldConfig),
     {ok, NewConfig};
-do_pre_config_update({update_authenticator, _ChainName, AuthenticatorID, Config}, OldConfig) ->
-    try lists:map(fun(OldConfig0) ->
-                              case AuthenticatorID =:= generate_id(OldConfig0) of
-                                  true -> convert_certs(Config, OldConfig0);
-                                  false -> OldConfig0
-                              end
-                          end, OldConfig) of
-        NewConfig ->
-            {ok, NewConfig}
+do_pre_config_update({update_authenticator, ChainName, AuthenticatorID, Config}, OldConfig) ->
+    try 
+        CertsDir = certs_dir([to_bin(ChainName), AuthenticatorID]),
+        NewConfig = lists:map(
+                        fun(OldConfig0) ->
+                            case AuthenticatorID =:= generate_id(OldConfig0) of
+                                true -> convert_certs(CertsDir, Config, OldConfig0);
+                                false -> OldConfig0
+                            end
+                        end, OldConfig),
+        {ok, NewConfig}
     catch
         error:{save_cert_to_file, _} = Reason ->
+            {error, Reason};
+         error:{missing_parameter, _} = Reason ->
             {error, Reason}
     end;
 do_pre_config_update({move_authenticator, _ChainName, AuthenticatorID, Position}, OldConfig) ->
@@ -254,13 +261,18 @@ do_post_config_update({create_authenticator, ChainName, Config}, _NewConfig, _Ol
     _ = create_chain(ChainName),
     create_authenticator(ChainName, NConfig);
 
-do_post_config_update({delete_authenticator, ChainName, AuthenticatorID}, _NewConfig, _OldConfig, _AppEnvs) ->
-    delete_authenticator(ChainName, AuthenticatorID);
+do_post_config_update({delete_authenticator, ChainName, AuthenticatorID}, _NewConfig, OldConfig, _AppEnvs) ->
+    case delete_authenticator(ChainName, AuthenticatorID) of
+        ok ->
+            [Config] = [Config0 || Config0 <- to_list(OldConfig), AuthenticatorID == generate_id(Config0)],
+            CertsDir = certs_dir([to_bin(ChainName), AuthenticatorID]),
+            clear_certs(CertsDir, Config),
+            ok;
+        {error, Reason} ->
+            {error, Reason}
+    end;
 
-do_post_config_update({update_authenticator, ChainName, AuthenticatorID, _Config}, NewConfig, _OldConfig, _AppEnvs) ->
-    [Config] = lists:filter(fun(NewConfig0) ->
-                                AuthenticatorID =:= generate_id(NewConfig0)
-                            end, NewConfig),
+do_post_config_update({update_authenticator, ChainName, AuthenticatorID, Config}, _NewConfig, _OldConfig, _AppEnvs) ->
     NConfig = check_config(Config),
     update_authenticator(ChainName, AuthenticatorID, NConfig);
 
@@ -441,15 +453,17 @@ 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),
+    Mechanism = to_bin(Mechanism0),
+    Backend = to_bin(Backend0),
     <<Mechanism/binary, ":", Backend/binary>>;
 generate_id(#{mechanism := Mechanism}) ->
-    atom_to_binary(Mechanism);
+    to_bin(Mechanism);
 generate_id(#{<<"mechanism">> := Mechanism, <<"backend">> := Backend}) ->
     <<Mechanism/binary, ":", Backend/binary>>;
 generate_id(#{<<"mechanism">> := Mechanism}) ->
-    Mechanism.
+    Mechanism;
+generate_id(_) ->
+    error({missing_parameter, mechanism}).
 
 %%--------------------------------------------------------------------
 %% gen_server callbacks
@@ -642,33 +656,54 @@ reply(Reply, State) ->
 %% Internal functions
 %%------------------------------------------------------------------------------
 
-convert_certs(#{<<"ssl">> := SSLOpts} = Config) ->
-    NSSLOPts = lists:foldl(fun(K, Acc) ->
+certs_dir(Dirs) when is_list(Dirs) ->
+    to_bin(filename:join([emqx:get_config([node, data_dir]), "certs/authn"] ++ Dirs)).
+
+convert_certs(CertsDir, Config) ->
+    case maps:get(<<"ssl">>, Config, undefined) of
+        undefined ->
+            Config;
+        SSLOpts ->
+            NSSLOPts = lists:foldl(fun(K, Acc) ->
                                case maps:get(K, Acc, undefined) of
                                    undefined -> Acc;
                                    PemBin ->
-                                       CertFile = generate_filename(K),
+                                       CertFile = generate_filename(CertsDir, K),
                                        ok = save_cert_to_file(CertFile, PemBin),
                                        Acc#{K => CertFile}
                                end
                            end, SSLOpts, [<<"certfile">>, <<"keyfile">>, <<"cacertfile">>]),
-    Config#{<<"ssl">> => NSSLOPts};
-convert_certs(Config) ->
-    Config.
-
-convert_certs(#{<<"ssl">> := NewSSLOpts} = NewConfig, OldConfig) ->
-    OldSSLOpts = maps:get(<<"ssl">>, OldConfig, #{}),
-    Diff = diff_certs(NewSSLOpts, OldSSLOpts),
-    NSSLOpts = lists:foldl(fun({identical, K}, Acc) ->
-                               Acc#{K => maps:get(K, OldSSLOpts)};
-                              ({_, K}, Acc) ->
-                               CertFile = generate_filename(K),
-                               ok = save_cert_to_file(CertFile, maps:get(K, NewSSLOpts)),
-                               Acc#{K => CertFile}
-                           end, NewSSLOpts, Diff),
-    NewConfig#{<<"ssl">> => NSSLOpts};
-convert_certs(NewConfig, _OldConfig) ->
-    NewConfig.
+            Config#{<<"ssl">> => NSSLOPts}
+    end.
+
+convert_certs(CertsDir, NewConfig, OldConfig) ->
+    case maps:get(<<"ssl">>, NewConfig, undefined) of
+        undefined ->
+            NewConfig;
+        NewSSLOpts ->
+            OldSSLOpts = maps:get(<<"ssl">>, OldConfig, #{}),
+            Diff = diff_certs(NewSSLOpts, OldSSLOpts),
+            NSSLOpts = lists:foldl(fun({identical, K}, Acc) ->
+                                    Acc#{K => maps:get(K, OldSSLOpts)};
+                                    ({_, K}, Acc) ->
+                                    CertFile = generate_filename(CertsDir, K),
+                                    ok = save_cert_to_file(CertFile, maps:get(K, NewSSLOpts)),
+                                    Acc#{K => CertFile}
+                                end, NewSSLOpts, Diff),
+            NewConfig#{<<"ssl">> => NSSLOpts}
+    end.
+
+clear_certs(CertsDir, Config) ->
+    case maps:get(<<"ssl">>, Config, undefined) of
+        undefined ->
+            ok;
+        SSLOpts ->
+            lists:foreach(
+                fun({_, Filename}) ->
+                    _ = file:delete(filename:join([CertsDir, Filename]))
+                end,
+                maps:to_list(maps:with([<<"certfile">>, <<"keyfile">>, <<"cacertfile">>], SSLOpts)))
+    end.
 
 save_cert_to_file(Filename, PemBin) ->
     case public_key:pem_decode(PemBin) =/= [] of
@@ -686,13 +721,13 @@ save_cert_to_file(Filename, PemBin) ->
             error({save_cert_to_file, invalid_certificate})
     end.
 
-generate_filename(Key) ->
+generate_filename(CertsDir, Key) ->
     Prefix = case Key of
                  <<"keyfile">> -> "key-";
                  <<"certfile">> -> "cert-";
                  <<"cacertfile">> -> "cacert-"
              end,
-    to_bin(filename:join([emqx:get_config([node, data_dir]), "certs/authn", Prefix ++ emqx_misc:gen_id() ++ ".pem"])).
+    to_bin(filename:join([CertsDir, Prefix ++ emqx_misc:gen_id() ++ ".pem"])).
 
 diff_certs(NewSSLOpts, OldSSLOpts) ->
     Keys = [<<"cacertfile">>, <<"certfile">>, <<"keyfile">>],
@@ -900,6 +935,7 @@ to_list(L) when is_list(L) ->
     L.
 
 to_bin(B) when is_binary(B) -> B;
-to_bin(L) when is_list(L) -> list_to_binary(L).
+to_bin(L) when is_list(L) -> list_to_binary(L);
+to_bin(A) when is_atom(A) -> atom_to_binary(A).
 
 call(Call) -> gen_server:call(?MODULE, Call, infinity).

apps/emqx/test/emqx_authentication_SUITE.erl

@@ -211,12 +211,12 @@ t_update_config(Config) when is_list(Config) ->
     ok = register_provider(?config("auth1"), ?MODULE),
     ok = register_provider(?config("auth2"), ?MODULE),
     Global = ?config(global),
-    AuthenticatorConfig1 = #{mechanism => 'password-based',
-                             backend => 'built-in-database',
-                             enable => true},
-    AuthenticatorConfig2 = #{mechanism => 'password-based',
-                             backend => mysql,
-                             enable => true},
+    AuthenticatorConfig1 = #{<<"mechanism">> => <<"password-based">>,
+                             <<"backend">> => <<"built-in-database">>,
+                             <<"enable">> => true},
+    AuthenticatorConfig2 = #{<<"mechanism">> => <<"password-based">>,
+                             <<"backend">> => <<"mysql">>,
+                             <<"enable">> => true},
     ID1 = <<"password-based:built-in-database">>,
     ID2 = <<"password-based:mysql">>,
 
@@ -227,7 +227,7 @@ t_update_config(Config) when is_list(Config) ->
     ?assertMatch({ok, _}, update_config([authentication], {create_authenticator, Global, AuthenticatorConfig2})),
     ?assertMatch({ok, #{id := ID2, state := #{mark := 1}}}, ?AUTHN:lookup_authenticator(Global, ID2)),
 
-    ?assertMatch({ok, _}, update_config([authentication], {update_authenticator, Global, ID1, AuthenticatorConfig1#{enable => false}})),
+    ?assertMatch({ok, _}, update_config([authentication], {update_authenticator, Global, ID1, AuthenticatorConfig1#{<<"enable">> => false}})),
     ?assertMatch({ok, #{id := ID1, state := #{mark := 2}}}, ?AUTHN:lookup_authenticator(Global, ID1)),
 
     ?assertMatch({ok, _}, update_config([authentication], {move_authenticator, Global, ID2, top})),
@@ -244,7 +244,7 @@ t_update_config(Config) when is_list(Config) ->
     ?assertMatch({ok, _}, update_config(ConfKeyPath, {create_authenticator, ListenerID, AuthenticatorConfig2})),
     ?assertMatch({ok, #{id := ID2, state := #{mark := 1}}}, ?AUTHN:lookup_authenticator(ListenerID, ID2)),
 
-    ?assertMatch({ok, _}, update_config(ConfKeyPath, {update_authenticator, ListenerID, ID1, AuthenticatorConfig1#{enable => false}})),
+    ?assertMatch({ok, _}, update_config(ConfKeyPath, {update_authenticator, ListenerID, ID1, AuthenticatorConfig1#{<<"enable">> => false}})),
     ?assertMatch({ok, #{id := ID1, state := #{mark := 2}}}, ?AUTHN:lookup_authenticator(ListenerID, ID1)),
 
     ?assertMatch({ok, _}, update_config(ConfKeyPath, {move_authenticator, ListenerID, ID2, top})),
@@ -257,19 +257,22 @@ t_update_config({'end', Config}) ->
     ?AUTHN:deregister_providers([?config("auth1"), ?config("auth2")]),
     ok.
 
-t_convert_cert_options({_, Config}) -> Config;
-t_convert_cert_options(Config) when is_list(Config) ->
+t_convert_certs({_, Config}) -> Config;
+t_convert_certs(Config) when is_list(Config) ->
+    Global = <<"mqtt:global">>,
     Certs = certs([ {<<"keyfile">>, "key.pem"}
                   , {<<"certfile">>, "cert.pem"}
                   , {<<"cacertfile">>, "cacert.pem"}
                   ]),
-    #{<<"ssl">> := NCerts} = ?AUTHN:convert_certs(#{<<"ssl">> => Certs}),
+
+    CertsDir = ?AUTHN:certs_dir([Global, <<"password-based:built-in-database">>]),
+    #{<<"ssl">> := NCerts} = ?AUTHN:convert_certs(CertsDir, #{<<"ssl">> => Certs}),
     ?assertEqual(false, diff_cert(maps:get(<<"keyfile">>, NCerts), maps:get(<<"keyfile">>, Certs))),
 
     Certs2 = certs([ {<<"keyfile">>, "key.pem"}
                    , {<<"certfile">>, "cert.pem"}
                    ]),
-    #{<<"ssl">> := NCerts2} = ?AUTHN:convert_certs(#{<<"ssl">> => Certs2}, #{<<"ssl">> => NCerts}),
+    #{<<"ssl">> := NCerts2} = ?AUTHN:convert_certs(CertsDir, #{<<"ssl">> => Certs2}, #{<<"ssl">> => NCerts}),
     ?assertEqual(false, diff_cert(maps:get(<<"keyfile">>, NCerts2), maps:get(<<"keyfile">>, Certs2))),
     ?assertEqual(maps:get(<<"keyfile">>, NCerts), maps:get(<<"keyfile">>, NCerts2)),
     ?assertEqual(maps:get(<<"certfile">>, NCerts), maps:get(<<"certfile">>, NCerts2)),
@@ -278,10 +281,14 @@ t_convert_cert_options(Config) when is_list(Config) ->
                    , {<<"certfile">>, "client-cert.pem"}
                    , {<<"cacertfile">>, "cacert.pem"}
                    ]),
-    #{<<"ssl">> := NCerts3} = ?AUTHN:convert_certs(#{<<"ssl">> => Certs3}, #{<<"ssl">> => NCerts2}),
+    #{<<"ssl">> := NCerts3} = ?AUTHN:convert_certs(CertsDir, #{<<"ssl">> => Certs3}, #{<<"ssl">> => NCerts2}),
     ?assertEqual(false, diff_cert(maps:get(<<"keyfile">>, NCerts3), maps:get(<<"keyfile">>, Certs3))),
     ?assertNotEqual(maps:get(<<"keyfile">>, NCerts2), maps:get(<<"keyfile">>, NCerts3)),
-    ?assertNotEqual(maps:get(<<"certfile">>, NCerts2), maps:get(<<"certfile">>, NCerts3)).
+    ?assertNotEqual(maps:get(<<"certfile">>, NCerts2), maps:get(<<"certfile">>, NCerts3)),
+
+    ?assertEqual(true, filelib:is_regular(maps:get(<<"keyfile">>, NCerts3))),
+    ?AUTHN:clear_certs(CertsDir, #{<<"ssl">> => NCerts3}),
+    ?assertEqual(false, filelib:is_regular(maps:get(<<"keyfile">>, NCerts3))).
 
 update_config(Path, ConfigRequest) ->
     emqx:update_config(Path, ConfigRequest, #{rawconf_with_defaults => true}).

apps/emqx_authn/src/emqx_authn_api.erl

@@ -1589,6 +1589,11 @@ definitions() ->
                 type => string,
                 example => <<"http://localhost:80">>
             },
+            refresh_interval => #{
+                type => integer,
+                default => 300,
+                example => 300
+            },
             verify_claims => #{
                 type => object,
                 additionalProperties => #{
@@ -1835,11 +1840,10 @@ find_listener(ListenerID) ->
             {ok, {Type, Name}}
     end.
 
-create_authenticator(ConfKeyPath, ChainName0, Config) ->
-    ChainName = to_atom(ChainName0),
-    case update_config(ConfKeyPath, {create_authenticator, ChainName, Config}) of
+create_authenticator(ConfKeyPath, ChainName, Config) ->
+    case update_config(ConfKeyPath, {create_authenticator, to_atom(ChainName), Config}) of
         {ok, #{post_config_update := #{?AUTHN := #{id := ID}},
-               raw_config := AuthenticatorsConfig}} ->
+            raw_config := AuthenticatorsConfig}} ->
             {ok, AuthenticatorConfig} = find_config(ID, AuthenticatorsConfig),
             {200, maps:put(id, ID, convert_certs(fill_defaults(AuthenticatorConfig)))};
         {error, {_, _, Reason}} ->
@@ -1861,9 +1865,8 @@ list_authenticator(ConfKeyPath, AuthenticatorID) ->
             serialize_error(Reason)
     end.
 
-update_authenticator(ConfKeyPath, ChainName0, AuthenticatorID, Config) ->
-    ChainName = to_atom(ChainName0),
-    case update_config(ConfKeyPath, {update_authenticator, ChainName, AuthenticatorID, Config}) of
+update_authenticator(ConfKeyPath, ChainName, AuthenticatorID, Config) ->
+    case update_config(ConfKeyPath, {update_authenticator, to_atom(ChainName), AuthenticatorID, Config}) of
         {ok, #{post_config_update := #{?AUTHN := #{id := ID}},
                raw_config := AuthenticatorsConfig}} ->
             {ok, AuthenticatorConfig} = find_config(ID, AuthenticatorsConfig),

apps/emqx_authn/src/simple_authn/emqx_authn_http.erl

@@ -78,7 +78,7 @@ validations() ->
 
 url(type) -> binary();
 url(nullable) -> false;
-url(validate) -> [fun check_url/1];
+url(validator) -> [fun check_url/1];
 url(_) -> undefined.
 
 headers(type) -> map();
@@ -99,7 +99,7 @@ headers_no_content_type(_) -> undefined.
 
 body(type) -> map();
 body(nullable) -> false;
-body(validate) -> [fun check_body/1];
+body(validator) -> [fun check_body/1];
 body(_) -> undefined.
 
 request_timeout(type) -> non_neg_integer();

apps/emqx_authn/src/simple_authn/emqx_authn_jwt.erl

@@ -123,7 +123,7 @@ server_name_indication(_) -> undefined.
 
 verify_claims(type) -> list();
 verify_claims(default) -> #{};
-verify_claims(validate) -> [fun check_verify_claims/1];
+verify_claims(validator) -> [fun do_check_verify_claims/1];
 verify_claims(converter) ->
     fun(VerifyClaims) ->
         maps:to_list(VerifyClaims)
@@ -298,12 +298,8 @@ do_verify_claims(Claims, [{Name, Value} | More]) ->
             {error, {claims, {Name, Value0}}}
     end.
 
-check_verify_claims(Conf) ->
-    Claims = hocon_schema:get_value("verify_claims", Conf),
-    do_check_verify_claims(Claims).
-
 do_check_verify_claims([]) ->
-    false;
+    true;
 do_check_verify_claims([{Name, Expected} | More]) ->
     check_claim_name(Name) andalso
     check_claim_expected(Expected) andalso

apps/emqx_connector/src/emqx_connector_http.erl

@@ -65,10 +65,10 @@ validations() ->
 
 base_url(type) -> url();
 base_url(nullable) -> false;
-base_url(validate) -> fun (#{query := _Query}) ->
-                              {error, "There must be no query in the base_url"};
+base_url(validator) -> fun(#{query := _Query}) ->
+                           {error, "There must be no query in the base_url"};
                           (_) -> ok
-                      end;
+                       end;
 base_url(_) -> undefined.
 
 connect_timeout(type) -> connect_timeout();