fix: always check authn_http's header and ssl_option

apps/emqx_authn/src/emqx_authn.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_authn, [
     {description, "EMQX Authentication"},
-    {vsn, "0.1.17"},
+    {vsn, "0.1.18"},
     {modules, []},
     {registered, [emqx_authn_sup, emqx_authn_registry]},
     {applications, [kernel, stdlib, emqx_resource, emqx_connector, ehttpc, epgsql, mysql, jose]},

apps/emqx_authn/src/emqx_authn_schema.erl

@@ -18,10 +18,12 @@
 
 -elvis([{elvis_style, invalid_dynamic_call, disable}]).
 -include_lib("hocon/include/hoconsc.hrl").
+-include("emqx_authn.hrl").
 
 -export([
     common_fields/0,
     roots/0,
+    validations/0,
     tags/0,
     fields/1,
     authenticator_type/0,
@@ -207,3 +209,27 @@ array(Name) ->
 
 array(Name, DescId) ->
     {Name, ?HOCON(?R_REF(Name), #{desc => ?DESC(DescId)})}.
+
+validations() ->
+    [
+        {check_http_ssl_opts, fun(Conf) ->
+            CheckFun = fun emqx_authn_http:check_ssl_opts/1,
+            validation(Conf, CheckFun)
+        end},
+        {check_http_headers, fun(Conf) ->
+            CheckFun = fun emqx_authn_http:check_headers/1,
+            validation(Conf, CheckFun)
+        end}
+    ].
+
+validation(Conf, CheckFun) when is_map(Conf) ->
+    validation(hocon_maps:get(?CONF_NS, Conf), CheckFun);
+validation(undefined, _) ->
+    ok;
+validation([], _) ->
+    ok;
+validation([AuthN | Tail], CheckFun) ->
+    case CheckFun(#{?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_BINARY => AuthN}) of
+        ok -> validation(Tail, CheckFun);
+        Error -> Error
+    end.

apps/emqx_authn/src/simple_authn/emqx_authn_http.erl

@@ -38,6 +38,8 @@
     headers/1
 ]).
 
+-export([check_headers/1, check_ssl_opts/1]).
+
 -export([
     refs/0,
     union_member_selector/1,
@@ -107,8 +109,8 @@ common_fields() ->
 
 validations() ->
     [
-        {check_ssl_opts, fun check_ssl_opts/1},
-        {check_headers, fun check_headers/1}
+        {check_ssl_opts, fun ?MODULE:check_ssl_opts/1},
+        {check_headers, fun ?MODULE:check_headers/1}
     ].
 
 url(type) -> binary();
@@ -262,21 +264,39 @@ transform_header_name(Headers) ->
     ).
 
 check_ssl_opts(Conf) ->
-    {BaseUrl, _Path, _Query} = parse_url(get_conf_val("url", Conf)),
-    case BaseUrl of
-        <<"https://", _/binary>> ->
-            case get_conf_val("ssl.enable", Conf) of
-                true -> ok;
-                false -> false
-            end;
-        <<"http://", _/binary>> ->
-            ok
+    case get_conf_val("url", Conf) of
+        undefined ->
+            ok;
+        Url ->
+            {BaseUrl, _Path, _Query} = parse_url(Url),
+            case BaseUrl of
+                <<"https://", _/binary>> ->
+                    case get_conf_val("ssl.enable", Conf) of
+                        true ->
+                            ok;
+                        false ->
+                            <<"it's required to enable the TLS option to establish a https connection">>
+                    end;
+                <<"http://", _/binary>> ->
+                    ok
+            end
     end.
 
 check_headers(Conf) ->
-    Method = to_bin(get_conf_val("method", Conf)),
-    Headers = get_conf_val("headers", Conf),
-    Method =:= <<"post">> orelse (not maps:is_key(<<"content-type">>, Headers)).
+    case get_conf_val("headers", Conf) of
+        undefined ->
+            ok;
+        Headers ->
+            case to_bin(get_conf_val("method", Conf)) of
+                <<"post">> ->
+                    ok;
+                <<"get">> ->
+                    case maps:is_key(<<"content-type">>, Headers) of
+                        false -> ok;
+                        true -> <<"HTTP GET requests cannot include content-type header.">>
+                    end
+            end
+    end.
 
 parse_url(Url) ->
     case string:split(Url, "//", leading) of
@@ -311,7 +331,7 @@ parse_config(
         method => Method,
         path => Path,
         headers => ensure_header_name_type(Headers),
-        base_path_templete => emqx_authn_utils:parse_str(Path),
+        base_path_template => emqx_authn_utils:parse_str(Path),
         base_query_template => emqx_authn_utils:parse_deep(
             cow_qs:parse_qs(to_bin(Query))
         ),
@@ -324,7 +344,7 @@ parse_config(
 generate_request(Credential, #{
     method := Method,
     headers := Headers0,
-    base_path_templete := BasePathTemplate,
+    base_path_template := BasePathTemplate,
     base_query_template := BaseQueryTemplate,
     body_template := BodyTemplate
 }) ->

apps/emqx_authn/test/emqx_authn_https_SUITE.erl

@@ -114,6 +114,22 @@ t_create_invalid_version(_Config) ->
         emqx_access_control:authenticate(?CREDENTIALS)
     ).
 
+t_create_disable_ssl_opts_when_https(_Config) ->
+    {ok, _} = create_https_auth_with_ssl_opts(
+        #{
+            <<"server_name_indication">> => <<"authn-server">>,
+            <<"verify">> => <<"verify_peer">>,
+            <<"versions">> => [<<"tlsv1.2">>],
+            <<"ciphers">> => [<<"ECDHE-RSA-AES256-GCM-SHA384">>],
+            <<"enable">> => <<"false">>
+        }
+    ),
+
+    ?assertEqual(
+        {error, not_authorized},
+        emqx_access_control:authenticate(?CREDENTIALS)
+    ).
+
 t_create_invalid_ciphers(_Config) ->
     {ok, _} = create_https_auth_with_ssl_opts(
         #{
@@ -135,6 +151,7 @@ t_create_invalid_ciphers(_Config) ->
 
 create_https_auth_with_ssl_opts(SpecificSSLOpts) ->
     AuthConfig = raw_https_auth_config(SpecificSSLOpts),
+    ct:pal("111:~p~n", [AuthConfig]),
     emqx:update_config(?PATH, {create_authenticator, ?GLOBAL, AuthConfig}).
 
 raw_https_auth_config(SpecificSSLOpts) ->

apps/emqx_conf/test/emqx_conf_schema_tests.erl

@@ -5,27 +5,27 @@
 -module(emqx_conf_schema_tests).
 
 -include_lib("eunit/include/eunit.hrl").
+-define(BASE_CONF,
+    ""
+    "\n"
+    "     node {\n"
+    "        name = \"emqx1@127.0.0.1\"\n"
+    "        cookie = \"emqxsecretcookie\"\n"
+    "        data_dir = \"data\"\n"
+    "     }\n"
+    "     cluster {\n"
+    "        name = emqxcl\n"
+    "        discovery_strategy = static\n"
+    "        static.seeds = ~p\n"
+    "        core_nodes = ~p\n"
+    "     }\n"
+    ""
+).
 array_nodes_test() ->
     ExpectNodes = ['emqx1@127.0.0.1', 'emqx2@127.0.0.1'],
-    BaseConf =
-        ""
-        "\n"
-        "     node {\n"
-        "        name = \"emqx1@127.0.0.1\"\n"
-        "        cookie = \"emqxsecretcookie\"\n"
-        "        data_dir = \"data\"\n"
-        "     }\n"
-        "     cluster {\n"
-        "        name = emqxcl\n"
-        "        discovery_strategy = static\n"
-        "        static.seeds = ~p\n"
-        "        core_nodes = ~p\n"
-        "     }\n"
-        "   "
-        "",
     lists:foreach(
         fun(Nodes) ->
-            ConfFile = iolist_to_binary(io_lib:format(BaseConf, [Nodes, Nodes])),
+            ConfFile = iolist_to_binary(io_lib:format(?BASE_CONF, [Nodes, Nodes])),
             {ok, Conf} = hocon:binary(ConfFile, #{format => richmap}),
             ConfList = hocon_tconf:generate(emqx_conf_schema, Conf),
             ClusterDiscovery = proplists:get_value(
@@ -46,6 +46,72 @@ array_nodes_test() ->
     ),
     ok.
 
+authn_validations_test() ->
+    BaseConf = iolist_to_binary(io_lib:format(?BASE_CONF, ["emqx1@127.0.0.1", "emqx1@127.0.0.1"])),
+    DisableSSLWithHttps =
+        ""
+        "\n"
+        "authentication = [\n"
+        "{backend = \"http\"\n"
+        "body {password = \"${password}\", username = \"${username}\"}\n"
+        "connect_timeout = \"15s\"\n"
+        "enable_pipelining = 100\n"
+        "headers {\"content-type\" = \"application/json\"}\n"
+        "mechanism = \"password_based\"\n"
+        "method = \"post\"\n"
+        "pool_size = 8\n"
+        "request_timeout = \"5s\"\n"
+        "ssl {enable = false, verify = \"verify_peer\"}\n"
+        "url = \"https://127.0.0.1:8080\"\n"
+        "}\n"
+        "]\n"
+        "",
+    Conf = <<BaseConf/binary, (list_to_binary(DisableSSLWithHttps))/binary>>,
+    {ok, ConfMap} = hocon:binary(Conf, #{format => richmap}),
+    ?assertThrow(
+        {emqx_conf_schema, [
+            #{
+                kind := validation_error,
+                reason := integrity_validation_failure,
+                result := _,
+                validation_name := check_http_ssl_opts
+            }
+        ]},
+        hocon_tconf:generate(emqx_conf_schema, ConfMap)
+    ),
+    BadHeader =
+        ""
+        "\n"
+        "authentication = [\n"
+        "{backend = \"http\"\n"
+        "body {password = \"${password}\", username = \"${username}\"}\n"
+        "connect_timeout = \"15s\"\n"
+        "enable_pipelining = 100\n"
+        "headers {\"content-type\" = \"application/json\"}\n"
+        "mechanism = \"password_based\"\n"
+        "method = \"get\"\n"
+        "pool_size = 8\n"
+        "request_timeout = \"5s\"\n"
+        "ssl {enable = false, verify = \"verify_peer\"}\n"
+        "url = \"http://127.0.0.1:8080\"\n"
+        "}\n"
+        "]\n"
+        "",
+    Conf1 = <<BaseConf/binary, (list_to_binary(BadHeader))/binary>>,
+    {ok, ConfMap1} = hocon:binary(Conf1, #{format => richmap}),
+    ?assertThrow(
+        {emqx_conf_schema, [
+            #{
+                kind := validation_error,
+                reason := integrity_validation_failure,
+                result := _,
+                validation_name := check_http_headers
+            }
+        ]},
+        hocon_tconf:generate(emqx_conf_schema, ConfMap1)
+    ),
+    ok.
+
 doc_gen_test() ->
     %% the json file too large to encode.
     {