Merge pull request #13040 from savonarola/0513-fix-http-authn-error-handling

fix(auth_http): fix query encoding

apps/emqx/src/emqx_schema.erl

@@ -93,7 +93,6 @@
     validate_heap_size/1,
     validate_packet_size/1,
     user_lookup_fun_tr/2,
-    validate_alarm_actions/1,
     validate_keepalive_multiplier/1,
     non_empty_string/1,
     validations/0,
@@ -1617,10 +1616,9 @@ fields("alarm") ->
     [
         {"actions",
             sc(
-                hoconsc:array(atom()),
+                hoconsc:array(hoconsc:enum([log, publish])),
                 #{
                     default => [log, publish],
-                    validator => fun ?MODULE:validate_alarm_actions/1,
                     example => [log, publish],
                     desc => ?DESC(alarm_actions)
                 }
@@ -2777,15 +2775,6 @@ validate_keepalive_multiplier(Multiplier) when
 validate_keepalive_multiplier(_Multiplier) ->
     {error, #{reason => keepalive_multiplier_out_of_range, min => 1, max => 65535}}.
 
-validate_alarm_actions(Actions) ->
-    UnSupported = lists:filter(
-        fun(Action) -> Action =/= log andalso Action =/= publish end, Actions
-    ),
-    case UnSupported of
-        [] -> ok;
-        Error -> {error, Error}
-    end.
-
 validate_tcp_keepalive(Value) ->
     case iolist_to_binary(Value) of
         <<"none">> ->

apps/emqx_auth/include/emqx_authn.hrl

@@ -21,8 +21,6 @@
 
 -define(AUTHN, emqx_authn_chains).
 
--define(RE_PLACEHOLDER, "\\$\\{[a-z0-9\\-]+\\}").
-
 %% has to be the same as the root field name defined in emqx_schema
 -define(CONF_NS, ?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME).
 -define(CONF_NS_ATOM, ?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_ATOM).
@@ -32,4 +30,16 @@
 
 -define(AUTHN_RESOURCE_GROUP, <<"emqx_authn">>).
 
+%% VAR_NS_CLIENT_ATTRS is added here because it can be initialized before authn.
+%% NOTE: authn return may add more to (or even overwrite) client_attrs.
+-define(AUTHN_DEFAULT_ALLOWED_VARS, [
+    ?VAR_USERNAME,
+    ?VAR_CLIENTID,
+    ?VAR_PASSWORD,
+    ?VAR_PEERHOST,
+    ?VAR_CERT_SUBJECT,
+    ?VAR_CERT_CN_NAME,
+    ?VAR_NS_CLIENT_ATTRS
+]).
+
 -endif.

apps/emqx_auth/include/emqx_authz.hrl

@@ -38,8 +38,6 @@
 -define(ROOT_KEY, [authorization]).
 -define(CONF_KEY_PATH, [authorization, sources]).
 
--define(RE_PLACEHOLDER, "\\$\\{[a-z0-9_]+\\}").
-
 %% has to be the same as the root field name defined in emqx_schema
 -define(CONF_NS, ?EMQX_AUTHORIZATION_CONFIG_ROOT_NAME).
 -define(CONF_NS_ATOM, ?EMQX_AUTHORIZATION_CONFIG_ROOT_NAME_ATOM).

apps/emqx_auth/src/emqx_auth_utils.erl

@@ -16,15 +16,221 @@
 
 -module(emqx_auth_utils).
 
-%% TODO
+-include_lib("emqx/include/emqx_placeholder.hrl").
-%% Move more identical authn and authz helpers here
+-include_lib("snabbkaffe/include/trace.hrl").
 
+%% Template parsing/rendering
+-export([
+    parse_deep/2,
+    parse_str/2,
+    parse_sql/3,
+    render_deep_for_json/2,
+    render_deep_for_url/2,
+    render_deep_for_raw/2,
+    render_str/2,
+    render_urlencoded_str/2,
+    render_sql_params/2
+]).
+
+%% URL parsing
 -export([parse_url/1]).
 
+%% HTTP request/response helpers
+-export([generate_request/2]).
+
+-define(DEFAULT_HTTP_REQUEST_CONTENT_TYPE, <<"application/json">>).
+
 %%--------------------------------------------------------------------
 %% API
 %%--------------------------------------------------------------------
 
+%%--------------------------------------------------------------------
+%% Template parsing/rendering
+
+parse_deep(Template, AllowedVars) ->
+    Result = emqx_template:parse_deep(Template),
+    handle_disallowed_placeholders(Result, AllowedVars, {deep, Template}).
+
+parse_str(Template, AllowedVars) ->
+    Result = emqx_template:parse(Template),
+    handle_disallowed_placeholders(Result, AllowedVars, {string, Template}).
+
+parse_sql(Template, ReplaceWith, AllowedVars) ->
+    {Statement, Result} = emqx_template_sql:parse_prepstmt(
+        Template,
+        #{parameters => ReplaceWith, strip_double_quote => true}
+    ),
+    {Statement, handle_disallowed_placeholders(Result, AllowedVars, {string, Template})}.
+
+handle_disallowed_placeholders(Template, AllowedVars, Source) ->
+    case emqx_template:validate(AllowedVars, Template) of
+        ok ->
+            Template;
+        {error, Disallowed} ->
+            ?tp(warning, "auth_template_invalid", #{
+                template => Source,
+                reason => Disallowed,
+                allowed => #{placeholders => AllowedVars},
+                notice =>
+                    "Disallowed placeholders will be rendered as is."
+                    " However, consider using `${$}` escaping for literal `$` where"
+                    " needed to avoid unexpected results."
+            }),
+            Result = prerender_disallowed_placeholders(Template, AllowedVars),
+            case Source of
+                {string, _} ->
+                    emqx_template:parse(Result);
+                {deep, _} ->
+                    emqx_template:parse_deep(Result)
+            end
+    end.
+
+prerender_disallowed_placeholders(Template, AllowedVars) ->
+    {Result, _} = emqx_template:render(Template, #{}, #{
+        var_trans => fun(Name, _) ->
+            % NOTE
+            % Rendering disallowed placeholders in escaped form, which will then
+            % parse as a literal string.
+            case lists:member(Name, AllowedVars) of
+                true -> "${" ++ Name ++ "}";
+                false -> "${$}{" ++ Name ++ "}"
+            end
+        end
+    }),
+    Result.
+
+render_deep_for_json(Template, Credential) ->
+    % NOTE
+    % Ignoring errors here, undefined bindings will be replaced with empty string.
+    {Term, _Errors} = emqx_template:render(
+        Template,
+        rename_client_info_vars(Credential),
+        #{var_trans => fun to_string_for_json/2}
+    ),
+    Term.
+
+render_deep_for_raw(Template, Credential) ->
+    % NOTE
+    % Ignoring errors here, undefined bindings will be replaced with empty string.
+    {Term, _Errors} = emqx_template:render(
+        Template,
+        rename_client_info_vars(Credential),
+        #{var_trans => fun to_string_for_raw/2}
+    ),
+    Term.
+
+render_deep_for_url(Template, Credential) ->
+    render_deep_for_raw(Template, Credential).
+
+render_str(Template, Credential) ->
+    % NOTE
+    % Ignoring errors here, undefined bindings will be replaced with empty string.
+    {String, _Errors} = emqx_template:render(
+        Template,
+        rename_client_info_vars(Credential),
+        #{var_trans => fun to_string/2}
+    ),
+    unicode:characters_to_binary(String).
+
+render_urlencoded_str(Template, Credential) ->
+    % NOTE
+    % Ignoring errors here, undefined bindings will be replaced with empty string.
+    {String, _Errors} = emqx_template:render(
+        Template,
+        rename_client_info_vars(Credential),
+        #{var_trans => fun to_urlencoded_string/2}
+    ),
+    unicode:characters_to_binary(String).
+
+render_sql_params(ParamList, Credential) ->
+    % NOTE
+    % Ignoring errors here, undefined bindings will be replaced with empty string.
+    {Row, _Errors} = emqx_template:render(
+        ParamList,
+        rename_client_info_vars(Credential),
+        #{var_trans => fun to_sql_value/2}
+    ),
+    Row.
+
+to_urlencoded_string(Name, Value) ->
+    case uri_string:compose_query([{<<"q">>, to_string(Name, Value)}]) of
+        <<"q=", EncodedBin/binary>> ->
+            EncodedBin;
+        "q=" ++ EncodedStr ->
+            list_to_binary(EncodedStr)
+    end.
+
+to_string(Name, Value) ->
+    emqx_template:to_string(render_var(Name, Value)).
+
+%% This converter is to generate data structure possibly with non-utf8 strings.
+%% It converts to unicode only strings (character lists).
+
+to_string_for_raw(Name, Value) ->
+    strings_to_unicode(Name, render_var(Name, Value)).
+
+%% This converter is to generate data structure suitable for JSON serialization.
+%% JSON strings are sequences of unicode characters, not bytes.
+%% So we force all rendered data to be unicode, not only character lists.
+
+to_string_for_json(Name, Value) ->
+    all_to_unicode(Name, render_var(Name, Value)).
+
+strings_to_unicode(_Name, Value) when is_binary(Value) ->
+    Value;
+strings_to_unicode(Name, Value) when is_list(Value) ->
+    to_unicode_binary(Name, Value);
+strings_to_unicode(_Name, Value) ->
+    emqx_template:to_string(Value).
+
+all_to_unicode(Name, Value) when is_list(Value) orelse is_binary(Value) ->
+    to_unicode_binary(Name, Value);
+all_to_unicode(_Name, Value) ->
+    emqx_template:to_string(Value).
+
+to_unicode_binary(Name, Value) when is_list(Value) orelse is_binary(Value) ->
+    try unicode:characters_to_binary(Value) of
+        Encoded when is_binary(Encoded) ->
+            Encoded;
+        _ ->
+            error({encode_error, {non_unicode_data, Name}})
+    catch
+        error:badarg ->
+            error({encode_error, {non_unicode_data, Name}})
+    end.
+
+to_sql_value(Name, Value) ->
+    emqx_utils_sql:to_sql_value(render_var(Name, Value)).
+
+render_var(_, undefined) ->
+    % NOTE
+    % Any allowed but undefined binding will be replaced with empty string, even when
+    % rendering SQL values.
+    <<>>;
+render_var(?VAR_PEERHOST, Value) ->
+    inet:ntoa(Value);
+render_var(?VAR_PASSWORD, Value) ->
+    iolist_to_binary(Value);
+render_var(_Name, Value) ->
+    Value.
+
+rename_client_info_vars(ClientInfo) ->
+    Renames = [
+        {cn, cert_common_name},
+        {dn, cert_subject},
+        {protocol, proto_name}
+    ],
+    lists:foldl(
+        fun({Old, New}, Acc) ->
+            emqx_utils_maps:rename(Old, New, Acc)
+        end,
+        ClientInfo,
+        Renames
+    ).
+
+%%--------------------------------------------------------------------
+%% URL parsing
+
 -spec parse_url(binary()) ->
     {_Base :: emqx_utils_uri:request_base(), _Path :: binary(), _Query :: binary()}.
 parse_url(Url) ->
@@ -48,6 +254,59 @@ parse_url(Url) ->
             end
     end.
 
+%%--------------------------------------------------------------------
+%% HTTP request/response helpers
+
+generate_request(
+    #{
+        method := Method,
+        headers := Headers,
+        base_path_template := BasePathTemplate,
+        base_query_template := BaseQueryTemplate,
+        body_template := BodyTemplate
+    },
+    Values
+) ->
+    Path = render_urlencoded_str(BasePathTemplate, Values),
+    Query = render_deep_for_url(BaseQueryTemplate, Values),
+    case Method of
+        get ->
+            Body = render_deep_for_url(BodyTemplate, Values),
+            NPath = append_query(Path, Query, Body),
+            {ok, {NPath, Headers}};
+        _ ->
+            try
+                ContentType = post_request_content_type(Headers),
+                Body = serialize_body(ContentType, BodyTemplate, Values),
+                NPathQuery = append_query(Path, Query),
+                {ok, {NPathQuery, Headers, Body}}
+            catch
+                error:{encode_error, _} = Reason ->
+                    {error, Reason}
+            end
+    end.
+
+post_request_content_type(Headers) ->
+    proplists:get_value(<<"content-type">>, Headers, ?DEFAULT_HTTP_REQUEST_CONTENT_TYPE).
+
+append_query(Path, []) ->
+    Path;
+append_query(Path, Query) ->
+    [Path, $?, uri_string:compose_query(Query)].
+append_query(Path, Query, Body) ->
+    append_query(Path, Query ++ maps:to_list(Body)).
+
+serialize_body(<<"application/json">>, BodyTemplate, ClientInfo) ->
+    Body = emqx_auth_utils:render_deep_for_json(BodyTemplate, ClientInfo),
+    emqx_utils_json:encode(Body);
+serialize_body(<<"application/x-www-form-urlencoded">>, BodyTemplate, ClientInfo) ->
+    Body = emqx_auth_utils:render_deep_for_url(BodyTemplate, ClientInfo),
+    uri_string:compose_query(maps:to_list(Body));
+serialize_body(undefined, _BodyTemplate, _ClientInfo) ->
+    throw(missing_content_type_header);
+serialize_body(ContentType, _BodyTemplate, _ClientInfo) ->
+    throw({unknown_content_type_header_value, ContentType}).
+
 -ifdef(TEST).
 -include_lib("eunit/include/eunit.hrl").
 

apps/emqx_auth/src/emqx_authn/emqx_authn_chains.erl

@@ -678,16 +678,28 @@ do_authenticate(
             {stop, Result}
     catch
         Class:Reason:Stacktrace ->
-            ?TRACE_AUTHN(warning, "authenticator_error", #{
+            ?TRACE_AUTHN(
-                exception => Class,
+                warning,
-                reason => Reason,
+                "authenticator_error",
-                stacktrace => Stacktrace,
+                maybe_add_stacktrace(
-                authenticator => ID
+                    Class,
-            }),
+                    #{
+                        exception => Class,
+                        reason => Reason,
+                        authenticator => ID
+                    },
+                    Stacktrace
+                )
+            ),
             emqx_metrics_worker:inc(authn_metrics, MetricsID, nomatch),
             do_authenticate(ChainName, More, Credential)
     end.
 
+maybe_add_stacktrace('throw', Data, _Stacktrace) ->
+    Data;
+maybe_add_stacktrace(_, Data, Stacktrace) ->
+    Data#{stacktrace => Stacktrace}.
+
 authenticate_with_provider(#authenticator{id = ID, provider = Provider, state = State}, Credential) ->
     AuthnResult = Provider:authenticate(Credential, State),
     ?TRACE_AUTHN("authenticator_result", #{

apps/emqx_auth/src/emqx_authn/emqx_authn_utils.erl

@@ -16,8 +16,8 @@
 
 -module(emqx_authn_utils).
 
--include_lib("emqx/include/emqx_placeholder.hrl").
 -include_lib("emqx_authn.hrl").
+-include_lib("emqx/include/emqx_placeholder.hrl").
 -include_lib("snabbkaffe/include/trace.hrl").
 
 -export([
@@ -26,12 +26,7 @@
     check_password_from_selected_map/3,
     parse_deep/1,
     parse_str/1,
-    parse_str/2,
     parse_sql/2,
-    render_deep/2,
-    render_str/2,
-    render_urlencoded_str/2,
-    render_sql_params/2,
     is_superuser/1,
     client_attrs/1,
     bin/1,
@@ -46,18 +41,6 @@
     default_headers_no_content_type/0
 ]).
 
-%% VAR_NS_CLIENT_ATTRS is added here because it can be initialized before authn.
-%% NOTE: authn return may add more to (or even overwrite) client_attrs.
--define(ALLOWED_VARS, [
-    ?VAR_USERNAME,
-    ?VAR_CLIENTID,
-    ?VAR_PASSWORD,
-    ?VAR_PEERHOST,
-    ?VAR_CERT_SUBJECT,
-    ?VAR_CERT_CN_NAME,
-    ?VAR_NS_CLIENT_ATTRS
-]).
-
 -define(DEFAULT_RESOURCE_OPTS, #{
     start_after_created => false
 }).
@@ -88,6 +71,13 @@ start_resource_if_enabled({ok, _} = Result, ResourceId, #{enable := true}) ->
 start_resource_if_enabled(Result, _ResourceId, _Config) ->
     Result.
 
+parse_deep(Template) -> emqx_auth_utils:parse_deep(Template, ?AUTHN_DEFAULT_ALLOWED_VARS).
+
+parse_str(Template) -> emqx_auth_utils:parse_str(Template, ?AUTHN_DEFAULT_ALLOWED_VARS).
+
+parse_sql(Template, ReplaceWith) ->
+    emqx_auth_utils:parse_sql(Template, ReplaceWith, ?AUTHN_DEFAULT_ALLOWED_VARS).
+
 check_password_from_selected_map(_Algorithm, _Selected, undefined) ->
     {error, bad_username_or_password};
 check_password_from_selected_map(Algorithm, Selected, Password) ->
@@ -111,101 +101,6 @@ check_password_from_selected_map(Algorithm, Selected, Password) ->
             end
     end.
 
-parse_deep(Template) ->
-    Result = emqx_template:parse_deep(Template),
-    handle_disallowed_placeholders(Result, ?ALLOWED_VARS, {deep, Template}).
-
-parse_str(Template, AllowedVars) ->
-    Result = emqx_template:parse(Template),
-    handle_disallowed_placeholders(Result, AllowedVars, {string, Template}).
-
-parse_str(Template) ->
-    parse_str(Template, ?ALLOWED_VARS).
-
-parse_sql(Template, ReplaceWith) ->
-    {Statement, Result} = emqx_template_sql:parse_prepstmt(
-        Template,
-        #{parameters => ReplaceWith, strip_double_quote => true}
-    ),
-    {Statement, handle_disallowed_placeholders(Result, ?ALLOWED_VARS, {string, Template})}.
-
-handle_disallowed_placeholders(Template, AllowedVars, Source) ->
-    case emqx_template:validate(AllowedVars, Template) of
-        ok ->
-            Template;
-        {error, Disallowed} ->
-            ?tp(warning, "authn_template_invalid", #{
-                template => Source,
-                reason => Disallowed,
-                allowed => #{placeholders => AllowedVars},
-                notice =>
-                    "Disallowed placeholders will be rendered as is."
-                    " However, consider using `${$}` escaping for literal `$` where"
-                    " needed to avoid unexpected results."
-            }),
-            Result = prerender_disallowed_placeholders(Template),
-            case Source of
-                {string, _} ->
-                    emqx_template:parse(Result);
-                {deep, _} ->
-                    emqx_template:parse_deep(Result)
-            end
-    end.
-
-prerender_disallowed_placeholders(Template) ->
-    {Result, _} = emqx_template:render(Template, #{}, #{
-        var_trans => fun(Name, _) ->
-            % NOTE
-            % Rendering disallowed placeholders in escaped form, which will then
-            % parse as a literal string.
-            case lists:member(Name, ?ALLOWED_VARS) of
-                true -> "${" ++ Name ++ "}";
-                false -> "${$}{" ++ Name ++ "}"
-            end
-        end
-    }),
-    Result.
-
-render_deep(Template, Credential) ->
-    % NOTE
-    % Ignoring errors here, undefined bindings will be replaced with empty string.
-    {Term, _Errors} = emqx_template:render(
-        Template,
-        mapping_credential(Credential),
-        #{var_trans => fun to_string/2}
-    ),
-    Term.
-
-render_str(Template, Credential) ->
-    % NOTE
-    % Ignoring errors here, undefined bindings will be replaced with empty string.
-    {String, _Errors} = emqx_template:render(
-        Template,
-        mapping_credential(Credential),
-        #{var_trans => fun to_string/2}
-    ),
-    unicode:characters_to_binary(String).
-
-render_urlencoded_str(Template, Credential) ->
-    % NOTE
-    % Ignoring errors here, undefined bindings will be replaced with empty string.
-    {String, _Errors} = emqx_template:render(
-        Template,
-        mapping_credential(Credential),
-        #{var_trans => fun to_urlencoded_string/2}
-    ),
-    unicode:characters_to_binary(String).
-
-render_sql_params(ParamList, Credential) ->
-    % NOTE
-    % Ignoring errors here, undefined bindings will be replaced with empty string.
-    {Row, _Errors} = emqx_template:render(
-        ParamList,
-        mapping_credential(Credential),
-        #{var_trans => fun to_sql_valaue/2}
-    ),
-    Row.
-
 is_superuser(#{<<"is_superuser">> := Value}) ->
     #{is_superuser => to_bool(Value)};
 is_superuser(#{}) ->
@@ -327,30 +222,6 @@ without_password(Credential, [Name | Rest]) ->
             without_password(Credential, Rest)
     end.
 
-to_urlencoded_string(Name, Value) ->
-    emqx_http_lib:uri_encode(to_string(Name, Value)).
-
-to_string(Name, Value) ->
-    emqx_template:to_string(render_var(Name, Value)).
-
-to_sql_valaue(Name, Value) ->
-    emqx_utils_sql:to_sql_value(render_var(Name, Value)).
-
-render_var(_, undefined) ->
-    % NOTE
-    % Any allowed but undefined binding will be replaced with empty string, even when
-    % rendering SQL values.
-    <<>>;
-render_var(?VAR_PEERHOST, Value) ->
-    inet:ntoa(Value);
-render_var(_Name, Value) ->
-    Value.
-
-mapping_credential(C = #{cn := CN, dn := DN}) ->
-    C#{cert_common_name => CN, cert_subject => DN};
-mapping_credential(C) ->
-    C.
-
 transform_header_name(Headers) ->
     maps:fold(
         fun(K0, V, Acc) ->

apps/emqx_auth/src/emqx_authz/emqx_authz_rule.erl

@@ -259,7 +259,7 @@ compile_topic(<<"eq ", Topic/binary>>) ->
 compile_topic({eq, Topic}) ->
     {eq, emqx_topic:words(bin(Topic))};
 compile_topic(Topic) ->
-    Template = emqx_authz_utils:parse_str(Topic, ?ALLOWED_VARS),
+    Template = emqx_auth_utils:parse_str(Topic, ?ALLOWED_VARS),
     case emqx_template:is_const(Template) of
         true -> emqx_topic:words(bin(Topic));
         false -> {pattern, Template}

apps/emqx_auth/src/emqx_authz/emqx_authz_utils.erl

@@ -16,7 +16,6 @@
 
 -module(emqx_authz_utils).
 
--include_lib("emqx/include/emqx_placeholder.hrl").
 -include_lib("emqx_authz.hrl").
 -include_lib("snabbkaffe/include/trace.hrl").
 
@@ -28,14 +27,6 @@
     update_resource/2,
     remove_resource/1,
     update_config/2,
-    parse_deep/2,
-    parse_str/2,
-    render_urlencoded_str/2,
-    parse_sql/3,
-    render_deep/2,
-    render_str/2,
-    render_sql_params/2,
-    client_vars/1,
     vars_for_rule_query/2,
     parse_rule_from_row/2
 ]).
@@ -100,7 +91,7 @@ cleanup_resources() ->
     ).
 
 make_resource_id(Name) ->
-    NameBin = bin(Name),
+    NameBin = emqx_utils_conv:bin(Name),
     emqx_resource:generate_id(NameBin).
 
 update_config(Path, ConfigRequest) ->
@@ -109,85 +100,6 @@ update_config(Path, ConfigRequest) ->
         override_to => cluster
     }).
 
-parse_deep(Template, PlaceHolders) ->
-    Result = emqx_template:parse_deep(Template),
-    handle_disallowed_placeholders(Result, {deep, Template}, PlaceHolders).
-
-parse_str(Template, PlaceHolders) ->
-    Result = emqx_template:parse(Template),
-    handle_disallowed_placeholders(Result, {string, Template}, PlaceHolders).
-
-parse_sql(Template, ReplaceWith, PlaceHolders) ->
-    {Statement, Result} = emqx_template_sql:parse_prepstmt(
-        Template,
-        #{parameters => ReplaceWith, strip_double_quote => true}
-    ),
-    FResult = handle_disallowed_placeholders(Result, {string, Template}, PlaceHolders),
-    {Statement, FResult}.
-
-handle_disallowed_placeholders(Template, Source, Allowed) ->
-    case emqx_template:validate(Allowed, Template) of
-        ok ->
-            Template;
-        {error, Disallowed} ->
-            ?tp(warning, "authz_template_invalid", #{
-                template => Source,
-                reason => Disallowed,
-                allowed => #{placeholders => Allowed},
-                notice =>
-                    "Disallowed placeholders will be rendered as is."
-                    " However, consider using `${$}` escaping for literal `$` where"
-                    " needed to avoid unexpected results."
-            }),
-            Result = emqx_template:escape_disallowed(Template, Allowed),
-            case Source of
-                {string, _} ->
-                    emqx_template:parse(Result);
-                {deep, _} ->
-                    emqx_template:parse_deep(Result)
-            end
-    end.
-
-render_deep(Template, Values) ->
-    % NOTE
-    % Ignoring errors here, undefined bindings will be replaced with empty string.
-    {Term, _Errors} = emqx_template:render(
-        Template,
-        client_vars(Values),
-        #{var_trans => fun to_string/2}
-    ),
-    Term.
-
-render_str(Template, Values) ->
-    % NOTE
-    % Ignoring errors here, undefined bindings will be replaced with empty string.
-    {String, _Errors} = emqx_template:render(
-        Template,
-        client_vars(Values),
-        #{var_trans => fun to_string/2}
-    ),
-    unicode:characters_to_binary(String).
-
-render_urlencoded_str(Template, Values) ->
-    % NOTE
-    % Ignoring errors here, undefined bindings will be replaced with empty string.
-    {String, _Errors} = emqx_template:render(
-        Template,
-        client_vars(Values),
-        #{var_trans => fun to_urlencoded_string/2}
-    ),
-    unicode:characters_to_binary(String).
-
-render_sql_params(ParamList, Values) ->
-    % NOTE
-    % Ignoring errors here, undefined bindings will be replaced with empty string.
-    {Row, _Errors} = emqx_template:render(
-        ParamList,
-        client_vars(Values),
-        #{var_trans => fun to_sql_value/2}
-    ),
-    Row.
-
 -spec parse_http_resp_body(binary(), binary()) -> allow | deny | ignore | error.
 parse_http_resp_body(<<"application/x-www-form-urlencoded", _/binary>>, Body) ->
     try
@@ -239,42 +151,6 @@ vars_for_rule_query(Client, ?authz_action(PubSub, Qos) = Action) ->
 %% Internal functions
 %%--------------------------------------------------------------------
 
-client_vars(ClientInfo) ->
-    maps:from_list(
-        lists:map(
-            fun convert_client_var/1,
-            maps:to_list(ClientInfo)
-        )
-    ).
-
-convert_client_var({cn, CN}) -> {cert_common_name, CN};
-convert_client_var({dn, DN}) -> {cert_subject, DN};
-convert_client_var({protocol, Proto}) -> {proto_name, Proto};
-convert_client_var(Other) -> Other.
-
-to_urlencoded_string(Name, Value) ->
-    emqx_http_lib:uri_encode(to_string(Name, Value)).
-
-to_string(Name, Value) ->
-    emqx_template:to_string(render_var(Name, Value)).
-
-to_sql_value(Name, Value) ->
-    emqx_utils_sql:to_sql_value(render_var(Name, Value)).
-
-render_var(_, undefined) ->
-    % NOTE
-    % Any allowed but undefined binding will be replaced with empty string, even when
-    % rendering SQL values.
-    <<>>;
-render_var(?VAR_PEERHOST, Value) ->
-    inet:ntoa(Value);
-render_var(_Name, Value) ->
-    Value.
-
-bin(A) when is_atom(A) -> atom_to_binary(A, utf8);
-bin(L) when is_list(L) -> list_to_binary(L);
-bin(X) -> X.
-
 to_list(Tuple) when is_tuple(Tuple) ->
     tuple_to_list(Tuple);
 to_list(List) when is_list(List) ->

apps/emqx_auth_http/src/emqx_authn_http.erl

@@ -28,6 +28,8 @@
     destroy/1
 ]).
 
+-define(DEFAULT_CONTENT_TYPE, <<"application/json">>).
+
 %%------------------------------------------------------------------------------
 %% APIs
 %%------------------------------------------------------------------------------
@@ -68,23 +70,34 @@ authenticate(
         request_timeout := RequestTimeout
     } = State
 ) ->
-    Request = generate_request(Credential, State),
+    case generate_request(Credential, State) of
-    Response = emqx_resource:simple_sync_query(ResourceId, {Method, Request, RequestTimeout}),
+        {ok, Request} ->
-    ?TRACE_AUTHN_PROVIDER("http_response", #{
+            Response = emqx_resource:simple_sync_query(
-        request => request_for_log(Credential, State),
+                ResourceId, {Method, Request, RequestTimeout}
-        response => response_for_log(Response),
+            ),
-        resource => ResourceId
+            ?TRACE_AUTHN_PROVIDER("http_response", #{
-    }),
+                request => request_for_log(Credential, State),
-    case Response of
+                response => response_for_log(Response),
-        {ok, 204, _Headers} ->
+                resource => ResourceId
-            {ok, #{is_superuser => false}};
+            }),
-        {ok, 200, Headers, Body} ->
+            case Response of
-            handle_response(Headers, Body);
+                {ok, 204, _Headers} ->
-        {ok, _StatusCode, _Headers} = Response ->
+                    {ok, #{is_superuser => false}};
-            ignore;
+                {ok, 200, Headers, Body} ->
-        {ok, _StatusCode, _Headers, _Body} = Response ->
+                    handle_response(Headers, Body);
-            ignore;
+                {ok, _StatusCode, _Headers} = Response ->
-        {error, _Reason} ->
+                    ignore;
+                {ok, _StatusCode, _Headers, _Body} = Response ->
+                    ignore;
+                {error, _Reason} ->
+                    ignore
+            end;
+        {error, Reason} ->
+            ?TRACE_AUTHN_PROVIDER(
+                error,
+                "generate_http_request_failed",
+                #{reason => Reason, credential => emqx_authn_utils:without_password(Credential)}
+            ),
             ignore
     end.
 
@@ -99,7 +112,8 @@ destroy(#{resource_id := ResourceId}) ->
 with_validated_config(Config, Fun) ->
     Pipeline = [
         fun check_ssl_opts/1,
-        fun check_headers/1,
+        fun normalize_headers/1,
+        fun check_method_headers/1,
         fun parse_config/1
     ],
     case emqx_utils:pipeline(Pipeline, Config, undefined) of
@@ -116,15 +130,23 @@ check_ssl_opts(#{url := <<"https://", _/binary>>, ssl := #{enable := false}}) ->
 check_ssl_opts(_) ->
     ok.
 
-check_headers(#{headers := Headers, method := get}) ->
+normalize_headers(#{headers := Headers} = Config) ->
+    {ok, Config#{headers => ensure_binary_names(Headers)}, undefined}.
+
+check_method_headers(#{headers := Headers, method := get}) ->
     case maps:is_key(<<"content-type">>, Headers) of
         false ->
             ok;
         true ->
             {error, {invalid_headers, <<"HTTP GET requests cannot include content-type header.">>}}
     end;
-check_headers(_) ->
+check_method_headers(#{headers := Headers, method := post} = Config) ->
-    ok.
+    {ok,
+        Config#{
+            headers =>
+                maps:merge(#{<<"content-type">> => ?DEFAULT_CONTENT_TYPE}, Headers)
+        },
+        undefined}.
 
 parse_config(
     #{
@@ -138,12 +160,15 @@ parse_config(
     State = #{
         method => Method,
         path => Path,
-        headers => ensure_header_name_type(Headers),
+        headers => maps:to_list(Headers),
         base_path_template => emqx_authn_utils:parse_str(Path),
         base_query_template => emqx_authn_utils:parse_deep(
             cow_qs:parse_qs(Query)
         ),
-        body_template => emqx_authn_utils:parse_deep(maps:get(body, Config, #{})),
+        body_template =>
+            emqx_authn_utils:parse_deep(
+                emqx_utils_maps:binary_key_map(maps:get(body, Config, #{}))
+            ),
         request_timeout => RequestTimeout,
         url => RawUrl
     },
@@ -154,46 +179,8 @@ parse_config(
         },
         State}.
 
-generate_request(Credential, #{
+generate_request(Credential, State) ->
-    method := Method,
+    emqx_auth_utils:generate_request(State, Credential).
-    headers := Headers0,
-    base_path_template := BasePathTemplate,
-    base_query_template := BaseQueryTemplate,
-    body_template := BodyTemplate
-}) ->
-    Headers = maps:to_list(Headers0),
-    Path = emqx_authn_utils:render_urlencoded_str(BasePathTemplate, Credential),
-    Query = emqx_authn_utils:render_deep(BaseQueryTemplate, Credential),
-    Body = emqx_authn_utils:render_deep(BodyTemplate, Credential),
-    case Method of
-        get ->
-            NPathQuery = append_query(to_list(Path), to_list(Query) ++ maps:to_list(Body)),
-            {NPathQuery, Headers};
-        post ->
-            NPathQuery = append_query(to_list(Path), to_list(Query)),
-            ContentType = proplists:get_value(<<"content-type">>, Headers),
-            NBody = serialize_body(ContentType, Body),
-            {NPathQuery, Headers, NBody}
-    end.
-
-append_query(Path, []) ->
-    Path;
-append_query(Path, Query) ->
-    Path ++ "?" ++ binary_to_list(qs(Query)).
-
-qs(KVs) ->
-    qs(KVs, []).
-
-qs([], Acc) ->
-    <<$&, Qs/binary>> = iolist_to_binary(lists:reverse(Acc)),
-    Qs;
-qs([{K, V} | More], Acc) ->
-    qs(More, [["&", uri_encode(K), "=", uri_encode(V)] | Acc]).
-
-serialize_body(<<"application/json">>, Body) ->
-    emqx_utils_json:encode(Body);
-serialize_body(<<"application/x-www-form-urlencoded">>, Body) ->
-    qs(maps:to_list(Body)).
 
 handle_response(Headers, Body) ->
     ContentType = proplists:get_value(<<"content-type">>, Headers),
@@ -239,26 +226,31 @@ parse_body(<<"application/x-www-form-urlencoded", _/binary>>, Body) ->
 parse_body(ContentType, _) ->
     {error, {unsupported_content_type, ContentType}}.
 
-uri_encode(T) ->
-    emqx_http_lib:uri_encode(to_list(T)).
-
 request_for_log(Credential, #{url := Url, method := Method} = State) ->
     SafeCredential = emqx_authn_utils:without_password(Credential),
     case generate_request(SafeCredential, State) of
-        {PathQuery, Headers} ->
+        {ok, {PathQuery, Headers}} ->
             #{
                 method => Method,
                 url => Url,
                 path_query => PathQuery,
                 headers => Headers
             };
-        {PathQuery, Headers, Body} ->
+        {ok, {PathQuery, Headers, Body}} ->
             #{
                 method => Method,
                 url => Url,
                 path_query => PathQuery,
                 headers => Headers,
                 body => Body
+            };
+        %% we can't get here actually because the real request was already generated
+        %% successfully, so generating it with hidden password won't fail either.
+        {error, Reason} ->
+            #{
+                method => Method,
+                url => Url,
+                error => Reason
             }
     end.
 
@@ -269,20 +261,5 @@ response_for_log({ok, StatusCode, Headers, Body}) ->
 response_for_log({error, Error}) ->
     #{error => Error}.
 
-to_list(A) when is_atom(A) ->
+ensure_binary_names(Headers) ->
-    atom_to_list(A);
+    emqx_utils_maps:binary_key_map(Headers).
-to_list(B) when is_binary(B) ->
-    binary_to_list(B);
-to_list(L) when is_list(L) ->
-    L.
-
-ensure_header_name_type(Headers) ->
-    Fun = fun
-        (Key, _Val, Acc) when is_binary(Key) ->
-            Acc;
-        (Key, Val, Acc) when is_atom(Key) ->
-            Acc2 = maps:remove(Key, Acc),
-            BinKey = erlang:atom_to_binary(Key),
-            Acc2#{BinKey => Val}
-    end,
-    maps:fold(Fun, Headers, Headers).

apps/emqx_auth_http/src/emqx_authz_http.erl

@@ -85,34 +85,42 @@ authorize(
         request_timeout := RequestTimeout
     } = Config
 ) ->
-    Request = generate_request(Action, Topic, Client, Config),
+    case generate_request(Action, Topic, Client, Config) of
-    case emqx_resource:simple_sync_query(ResourceID, {Method, Request, RequestTimeout}) of
+        {ok, Request} ->
-        {ok, 204, _Headers} ->
+            case emqx_resource:simple_sync_query(ResourceID, {Method, Request, RequestTimeout}) of
-            {matched, allow};
+                {ok, 204, _Headers} ->
-        {ok, 200, Headers, Body} ->
+                    {matched, allow};
-            ContentType = emqx_authz_utils:content_type(Headers),
+                {ok, 200, Headers, Body} ->
-            case emqx_authz_utils:parse_http_resp_body(ContentType, Body) of
+                    ContentType = emqx_authz_utils:content_type(Headers),
-                error ->
+                    case emqx_authz_utils:parse_http_resp_body(ContentType, Body) of
-                    ?SLOG(error, #{
+                        error ->
-                        msg => authz_http_response_incorrect,
+                            ?SLOG(error, #{
-                        content_type => ContentType,
+                                msg => authz_http_response_incorrect,
-                        body => Body
+                                content_type => ContentType,
-                    }),
+                                body => Body
+                            }),
+                            nomatch;
+                        Result ->
+                            {matched, Result}
+                    end;
+                {ok, Status, Headers} ->
+                    log_nomtach_msg(Status, Headers, undefined),
                     nomatch;
-                Result ->
+                {ok, Status, Headers, Body} ->
-                    {matched, Result}
+                    log_nomtach_msg(Status, Headers, Body),
+                    nomatch;
+                {error, Reason} ->
+                    ?tp(authz_http_request_failure, #{error => Reason}),
+                    ?SLOG(error, #{
+                        msg => "http_server_query_failed",
+                        resource => ResourceID,
+                        reason => Reason
+                    }),
+                    ignore
             end;
-        {ok, Status, Headers} ->
-            log_nomtach_msg(Status, Headers, undefined),
-            nomatch;
-        {ok, Status, Headers, Body} ->
-            log_nomtach_msg(Status, Headers, Body),
-            nomatch;
         {error, Reason} ->
-            ?tp(authz_http_request_failure, #{error => Reason}),
             ?SLOG(error, #{
-                msg => "http_server_query_failed",
+                msg => "http_request_generation_failed",
-                resource => ResourceID,
                 reason => Reason
             }),
             ignore
@@ -156,86 +164,29 @@ parse_config(
         method => Method,
         request_base => RequestBase,
         headers => Headers,
-        base_path_template => emqx_authz_utils:parse_str(Path, allowed_vars()),
+        base_path_template => emqx_auth_utils:parse_str(Path, allowed_vars()),
-        base_query_template => emqx_authz_utils:parse_deep(
+        base_query_template => emqx_auth_utils:parse_deep(
             cow_qs:parse_qs(Query),
             allowed_vars()
         ),
-        body_template => emqx_authz_utils:parse_deep(
+        body_template =>
-            maps:to_list(maps:get(body, Conf, #{})),
+            emqx_auth_utils:parse_deep(
-            allowed_vars()
+                emqx_utils_maps:binary_key_map(maps:get(body, Conf, #{})),
-        ),
+                allowed_vars()
+            ),
         request_timeout => ReqTimeout,
         %% pool_type default value `random`
         pool_type => random
     }.
 
-generate_request(
+generate_request(Action, Topic, Client, Config) ->
-    Action,
-    Topic,
-    Client,
-    #{
-        method := Method,
-        headers := Headers,
-        base_path_template := BasePathTemplate,
-        base_query_template := BaseQueryTemplate,
-        body_template := BodyTemplate
-    }
-) ->
     Values = client_vars(Client, Action, Topic),
-    Path = emqx_authz_utils:render_urlencoded_str(BasePathTemplate, Values),
+    emqx_auth_utils:generate_request(Config, Values).
-    Query = emqx_authz_utils:render_deep(BaseQueryTemplate, Values),
-    Body = emqx_authz_utils:render_deep(BodyTemplate, Values),
-    case Method of
-        get ->
-            NPath = append_query(Path, Query ++ Body),
-            {NPath, Headers};
-        _ ->
-            NPath = append_query(Path, Query),
-            NBody = serialize_body(
-                proplists:get_value(<<"accept">>, Headers, <<"application/json">>),
-                Body
-            ),
-            {NPath, Headers, NBody}
-    end.
-
-append_query(Path, []) ->
-    to_list(Path);
-append_query(Path, Query) ->
-    to_list(Path) ++ "?" ++ to_list(query_string(Query)).
-
-query_string(Body) ->
-    query_string(Body, []).
-
-query_string([], Acc) ->
-    case iolist_to_binary(lists:reverse(Acc)) of
-        <<$&, Str/binary>> ->
-            Str;
-        <<>> ->
-            <<>>
-    end;
-query_string([{K, V} | More], Acc) ->
-    query_string(More, [["&", uri_encode(K), "=", uri_encode(V)] | Acc]).
-
-uri_encode(T) ->
-    emqx_http_lib:uri_encode(to_list(T)).
-
-serialize_body(<<"application/json">>, Body) ->
-    emqx_utils_json:encode(Body);
-serialize_body(<<"application/x-www-form-urlencoded">>, Body) ->
-    query_string(Body).
 
 client_vars(Client, Action, Topic) ->
     Vars = emqx_authz_utils:vars_for_rule_query(Client, Action),
     Vars#{topic => Topic}.
 
-to_list(A) when is_atom(A) ->
-    atom_to_list(A);
-to_list(B) when is_binary(B) ->
-    binary_to_list(B);
-to_list(L) when is_list(L) ->
-    L.
-
 allowed_vars() ->
     allowed_vars(emqx_authz:feature_available(rich_actions)).
 

apps/emqx_auth_http/test/emqx_authn_http_SUITE.erl

@@ -140,6 +140,7 @@ t_create_invalid(_Config) ->
     ).
 
 t_authenticate(_Config) ->
+    ok = emqx_logger:set_primary_log_level(debug),
     ok = lists:foreach(
         fun(Sample) ->
             ct:pal("test_user_auth sample: ~p", [Sample]),
@@ -148,12 +149,15 @@ t_authenticate(_Config) ->
         samples()
     ).
 
-test_user_auth(#{
+test_user_auth(
-    handler := Handler,
+    #{
-    config_params := SpecificConfgParams,
+        handler := Handler,
-    result := Expect
+        config_params := SpecificConfgParams,
-}) ->
+        result := Expect
-    Result = perform_user_auth(SpecificConfgParams, Handler, ?CREDENTIALS),
+    } = Sample
+) ->
+    Credentials = maps:merge(?CREDENTIALS, maps:get(credentials, Sample, #{})),
+    Result = perform_user_auth(SpecificConfgParams, Handler, Credentials),
     ?assertEqual(Expect, Result).
 
 perform_user_auth(SpecificConfgParams, Handler, Credentials) ->
@@ -180,7 +184,7 @@ t_authenticate_path_placeholders(_Config) ->
         fun(Req0, State) ->
             Req =
                 case cowboy_req:path(Req0) of
-                    <<"/auth/p%20ath//us%20er/auth//">> ->
+                    <<"/auth/p%20ath//us+er/auth//">> ->
                         cowboy_req:reply(
                             200,
                             #{<<"content-type">> => <<"application/json">>},
@@ -563,6 +567,31 @@ samples() ->
             result => {ok, #{is_superuser => true, client_attrs => #{<<"fid">> => <<"n11">>}}}
         },
 
+        %% get request with non-utf8 password
+        #{
+            handler => fun(Req0, State) ->
+                #{
+                    password := <<255, 255, 255>>
+                } = cowboy_req:match_qs([password], Req0),
+                Req = cowboy_req:reply(
+                    200,
+                    #{<<"content-type">> => <<"application/json">>},
+                    emqx_utils_json:encode(#{
+                        result => allow,
+                        is_superuser => true,
+                        client_attrs => #{}
+                    }),
+                    Req0
+                ),
+                {ok, Req, State}
+            end,
+            config_params => #{},
+            credentials => #{
+                password => <<255, 255, 255>>
+            },
+            result => {ok, #{is_superuser => true, client_attrs => #{}}}
+        },
+
         %% get request with url-form-encoded body response
         #{
             handler => fun(Req0, State) ->
@@ -623,6 +652,30 @@ samples() ->
             result => {ok, #{is_superuser => false, client_attrs => #{}}}
         },
 
+        %% post request, no content-type header
+        #{
+            handler => fun(Req0, State) ->
+                {ok, RawBody, Req1} = cowboy_req:read_body(Req0),
+                #{
+                    <<"username">> := <<"plain">>,
+                    <<"password">> := <<"plain">>
+                } = emqx_utils_json:decode(RawBody, [return_maps]),
+                <<"application/json">> = cowboy_req:header(<<"content-type">>, Req0),
+                Req = cowboy_req:reply(
+                    200,
+                    #{<<"content-type">> => <<"application/json">>},
+                    emqx_utils_json:encode(#{result => allow, is_superuser => false}),
+                    Req1
+                ),
+                {ok, Req, State}
+            end,
+            config_params => #{
+                <<"method">> => <<"post">>,
+                <<"headers">> => #{}
+            },
+            result => {ok, #{is_superuser => false, client_attrs => #{}}}
+        },
+
         %% simple post request, application/x-www-form-urlencoded
         #{
             handler => fun(Req0, State) ->
@@ -686,6 +739,62 @@ samples() ->
             result => {ok, #{is_superuser => false, client_attrs => #{}}}
         },
 
+        %% post request with non-utf8 password, application/json
+        #{
+            handler => fun(Req0, State) ->
+                Req = cowboy_req:reply(
+                    200,
+                    #{<<"content-type">> => <<"application/json">>},
+                    emqx_utils_json:encode(#{result => allow, is_superuser => false}),
+                    Req0
+                ),
+                {ok, Req, State}
+            end,
+            config_params => #{
+                <<"method">> => <<"post">>,
+                <<"headers">> => #{<<"content-type">> => <<"application/json">>},
+                <<"body">> => #{
+                    <<"password">> => ?PH_PASSWORD
+                }
+            },
+            credentials => #{
+                password => <<255, 255, 255>>
+            },
+            %% non-utf8 password cannot be encoded in json
+            result => {error, not_authorized}
+        },
+
+        %% post request with non-utf8 password, form urlencoded
+        #{
+            handler => fun(Req0, State) ->
+                {ok, PostVars, Req1} = cowboy_req:read_urlencoded_body(Req0),
+                #{
+                    <<"password">> := <<255, 255, 255>>
+                } = maps:from_list(PostVars),
+                Req = cowboy_req:reply(
+                    200,
+                    #{<<"content-type">> => <<"application/json">>},
+                    emqx_utils_json:encode(#{result => allow, is_superuser => false}),
+                    Req1
+                ),
+                {ok, Req, State}
+            end,
+            config_params => #{
+                <<"method">> => <<"post">>,
+                <<"headers">> => #{
+                    <<"content-type">> =>
+                        <<"application/x-www-form-urlencoded">>
+                },
+                <<"body">> => #{
+                    <<"password">> => ?PH_PASSWORD
+                }
+            },
+            credentials => #{
+                password => <<255, 255, 255>>
+            },
+            result => {ok, #{is_superuser => false, client_attrs => #{}}}
+        },
+
         %% custom headers
         #{
             handler => fun(Req0, State) ->

apps/emqx_auth_http/test/emqx_authz_http_SUITE.erl

@@ -253,9 +253,9 @@ t_path(_Config) ->
         fun(Req0, State) ->
             ?assertEqual(
                 <<
-                    "/authz/use%20rs/"
+                    "/authz/use+rs/"
-                    "user%20name/"
+                    "user+name/"
-                    "client%20id/"
+                    "client+id/"
                     "127.0.0.1/"
                     "MQTT/"
                     "MOUNTPOINT/"
@@ -270,7 +270,7 @@ t_path(_Config) ->
         end,
         #{
             <<"url">> => <<
-                "http://127.0.0.1:33333/authz/use%20rs/"
+                "http://127.0.0.1:33333/authz/use+rs/"
                 "${username}/"
                 "${clientid}/"
                 "${peerhost}/"
@@ -402,7 +402,7 @@ t_placeholder_and_body(_Config) ->
                 cowboy_req:path(Req0)
             ),
 
-            {ok, [{PostVars, true}], Req1} = cowboy_req:read_urlencoded_body(Req0),
+            {ok, PostVars, Req1} = cowboy_req:read_urlencoded_body(Req0),
 
             ?assertMatch(
                 #{
@@ -416,7 +416,7 @@ t_placeholder_and_body(_Config) ->
                     <<"CN">> := ?PH_CERT_CN_NAME,
                     <<"CS">> := ?PH_CERT_SUBJECT
                 },
-                emqx_utils_json:decode(PostVars, [return_maps])
+                maps:from_list(PostVars)
             ),
             {ok, ?AUTHZ_HTTP_RESP(allow, Req1), State}
         end,
@@ -536,7 +536,7 @@ t_disallowed_placeholders_path(_Config) ->
             {ok, ?AUTHZ_HTTP_RESP(allow, Req), State}
         end,
         #{
-            <<"url">> => <<"http://127.0.0.1:33333/authz/use%20rs/${typo}">>
+            <<"url">> => <<"http://127.0.0.1:33333/authz/use+rs/${typo}">>
         }
     ),
 

apps/emqx_auth_jwt/src/emqx_authn_jwt.erl

@@ -216,7 +216,7 @@ may_decode_secret(true, Secret) ->
 render_expected([], _Variables) ->
     [];
 render_expected([{Name, ExpectedTemplate} | More], Variables) ->
-    Expected = emqx_authn_utils:render_str(ExpectedTemplate, Variables),
+    Expected = emqx_auth_utils:render_str(ExpectedTemplate, Variables),
     [{Name, Expected} | render_expected(More, Variables)].
 
 verify(undefined, _, _, _, _) ->
@@ -364,7 +364,7 @@ handle_verify_claims(VerifyClaims) ->
 handle_verify_claims([], Acc) ->
     Acc;
 handle_verify_claims([{Name, Expected0} | More], Acc) ->
-    Expected1 = emqx_authn_utils:parse_str(Expected0, ?ALLOWED_VARS),
+    Expected1 = emqx_auth_utils:parse_str(Expected0, ?ALLOWED_VARS),
     handle_verify_claims(More, [{Name, Expected1} | Acc]).
 
 binary_to_number(Bin) ->

apps/emqx_auth_mongodb/src/emqx_authn_mongodb.erl

@@ -61,14 +61,27 @@ authenticate(#{auth_method := _}, _) ->
 authenticate(#{password := undefined}, _) ->
     {error, bad_username_or_password};
 authenticate(
-    #{password := Password} = Credential,
+    Credential, #{filter_template := FilterTemplate} = State
+) ->
+    try emqx_auth_utils:render_deep_for_json(FilterTemplate, Credential) of
+        Filter ->
+            authenticate_with_filter(Filter, Credential, State)
+    catch
+        error:{encode_error, _} = EncodeError ->
+            ?TRACE_AUTHN_PROVIDER(error, "mongodb_render_filter_failed", #{
+                reason => EncodeError
+            }),
+            ignore
+    end.
+
+authenticate_with_filter(
+    Filter,
+    #{password := Password},
     #{
         collection := Collection,
-        filter_template := FilterTemplate,
         resource_id := ResourceId
     } = State
 ) ->
-    Filter = emqx_authn_utils:render_deep(FilterTemplate, Credential),
     case emqx_resource:simple_sync_query(ResourceId, {find_one, Collection, Filter, #{}}) of
         {ok, undefined} ->
             ignore;

apps/emqx_auth_mongodb/src/emqx_authz_mongodb.erl

@@ -50,11 +50,11 @@ description() ->
 create(#{filter := Filter} = Source) ->
     ResourceId = emqx_authz_utils:make_resource_id(?MODULE),
     {ok, _Data} = emqx_authz_utils:create_resource(ResourceId, emqx_mongodb, Source),
-    FilterTemp = emqx_authz_utils:parse_deep(Filter, ?ALLOWED_VARS),
+    FilterTemp = emqx_auth_utils:parse_deep(Filter, ?ALLOWED_VARS),
     Source#{annotations => #{id => ResourceId}, filter_template => FilterTemp}.
 
 update(#{filter := Filter} = Source) ->
-    FilterTemp = emqx_authz_utils:parse_deep(Filter, ?ALLOWED_VARS),
+    FilterTemp = emqx_auth_utils:parse_deep(Filter, ?ALLOWED_VARS),
     case emqx_authz_utils:update_resource(emqx_mongodb, Source) of
         {error, Reason} ->
             error({load_config_error, Reason});
@@ -69,13 +69,23 @@ authorize(
     Client,
     Action,
     Topic,
-    #{
+    #{filter_template := FilterTemplate} = Config
-        collection := Collection,
-        filter_template := FilterTemplate,
-        annotations := #{id := ResourceID}
-    }
 ) ->
-    RenderedFilter = emqx_authz_utils:render_deep(FilterTemplate, Client),
+    try emqx_auth_utils:render_deep_for_json(FilterTemplate, Client) of
+        RenderedFilter -> authorize_with_filter(RenderedFilter, Client, Action, Topic, Config)
+    catch
+        error:{encode_error, _} = EncodeError ->
+            ?SLOG(error, #{
+                msg => "mongo_authorize_error",
+                reason => EncodeError
+            }),
+            nomatch
+    end.
+
+authorize_with_filter(RenderedFilter, Client, Action, Topic, #{
+    collection := Collection,
+    annotations := #{id := ResourceID}
+}) ->
     case emqx_resource:simple_sync_query(ResourceID, {find, Collection, RenderedFilter, #{}}) of
         {error, Reason} ->
             ?SLOG(error, #{

apps/emqx_auth_mysql/src/emqx_authn_mysql.erl

@@ -68,7 +68,7 @@ authenticate(
         password_hash_algorithm := Algorithm
     }
 ) ->
-    Params = emqx_authn_utils:render_sql_params(TmplToken, Credential),
+    Params = emqx_auth_utils:render_sql_params(TmplToken, Credential),
     case
         emqx_resource:simple_sync_query(ResourceId, {prepared_query, ?PREPARE_KEY, Params, Timeout})
     of

apps/emqx_auth_mysql/src/emqx_authz_mysql.erl

@@ -50,14 +50,14 @@ description() ->
     "AuthZ with Mysql".
 
 create(#{query := SQL} = Source0) ->
-    {PrepareSQL, TmplToken} = emqx_authz_utils:parse_sql(SQL, '?', ?ALLOWED_VARS),
+    {PrepareSQL, TmplToken} = emqx_auth_utils:parse_sql(SQL, '?', ?ALLOWED_VARS),
     ResourceId = emqx_authz_utils:make_resource_id(?MODULE),
     Source = Source0#{prepare_statement => #{?PREPARE_KEY => PrepareSQL}},
     {ok, _Data} = emqx_authz_utils:create_resource(ResourceId, emqx_mysql, Source),
     Source#{annotations => #{id => ResourceId, tmpl_token => TmplToken}}.
 
 update(#{query := SQL} = Source0) ->
-    {PrepareSQL, TmplToken} = emqx_authz_utils:parse_sql(SQL, '?', ?ALLOWED_VARS),
+    {PrepareSQL, TmplToken} = emqx_auth_utils:parse_sql(SQL, '?', ?ALLOWED_VARS),
     Source = Source0#{prepare_statement => #{?PREPARE_KEY => PrepareSQL}},
     case emqx_authz_utils:update_resource(emqx_mysql, Source) of
         {error, Reason} ->
@@ -81,7 +81,7 @@ authorize(
     }
 ) ->
     Vars = emqx_authz_utils:vars_for_rule_query(Client, Action),
-    RenderParams = emqx_authz_utils:render_sql_params(TmplToken, Vars),
+    RenderParams = emqx_auth_utils:render_sql_params(TmplToken, Vars),
     case
         emqx_resource:simple_sync_query(ResourceID, {prepared_query, ?PREPARE_KEY, RenderParams})
     of

apps/emqx_auth_postgresql/src/emqx_authn_postgresql.erl

@@ -76,7 +76,7 @@ authenticate(
         password_hash_algorithm := Algorithm
     }
 ) ->
-    Params = emqx_authn_utils:render_sql_params(PlaceHolders, Credential),
+    Params = emqx_auth_utils:render_sql_params(PlaceHolders, Credential),
     case emqx_resource:simple_sync_query(ResourceId, {prepared_query, ResourceId, Params}) of
         {ok, _Columns, []} ->
             ignore;

apps/emqx_auth_postgresql/src/emqx_authz_postgresql.erl

@@ -50,7 +50,7 @@ description() ->
     "AuthZ with PostgreSQL".
 
 create(#{query := SQL0} = Source) ->
-    {SQL, PlaceHolders} = emqx_authz_utils:parse_sql(SQL0, '$n', ?ALLOWED_VARS),
+    {SQL, PlaceHolders} = emqx_auth_utils:parse_sql(SQL0, '$n', ?ALLOWED_VARS),
     ResourceID = emqx_authz_utils:make_resource_id(emqx_postgresql),
     {ok, _Data} = emqx_authz_utils:create_resource(
         ResourceID,
@@ -60,7 +60,7 @@ create(#{query := SQL0} = Source) ->
     Source#{annotations => #{id => ResourceID, placeholders => PlaceHolders}}.
 
 update(#{query := SQL0, annotations := #{id := ResourceID}} = Source) ->
-    {SQL, PlaceHolders} = emqx_authz_utils:parse_sql(SQL0, '$n', ?ALLOWED_VARS),
+    {SQL, PlaceHolders} = emqx_auth_utils:parse_sql(SQL0, '$n', ?ALLOWED_VARS),
     case
         emqx_authz_utils:update_resource(
             emqx_postgresql,
@@ -88,7 +88,7 @@ authorize(
     }
 ) ->
     Vars = emqx_authz_utils:vars_for_rule_query(Client, Action),
-    RenderedParams = emqx_authz_utils:render_sql_params(Placeholders, Vars),
+    RenderedParams = emqx_auth_utils:render_sql_params(Placeholders, Vars),
     case
         emqx_resource:simple_sync_query(ResourceID, {prepared_query, ResourceID, RenderedParams})
     of

apps/emqx_auth_redis/src/emqx_authn_redis.erl

@@ -74,7 +74,7 @@ authenticate(
         password_hash_algorithm := Algorithm
     }
 ) ->
-    NKey = emqx_authn_utils:render_str(KeyTemplate, Credential),
+    NKey = emqx_auth_utils:render_str(KeyTemplate, Credential),
     Command = [CommandName, NKey | Fields],
     case emqx_resource:simple_sync_query(ResourceId, {cmd, Command}) of
         {ok, []} ->

apps/emqx_auth_redis/src/emqx_authz_redis.erl

@@ -75,7 +75,7 @@ authorize(
     }
 ) ->
     Vars = emqx_authz_utils:vars_for_rule_query(Client, Action),
-    Cmd = emqx_authz_utils:render_deep(CmdTemplate, Vars),
+    Cmd = emqx_auth_utils:render_deep_for_raw(CmdTemplate, Vars),
     case emqx_resource:simple_sync_query(ResourceID, {cmd, Cmd}) of
         {ok, Rows} ->
             do_authorize(Client, Action, Topic, Rows);
@@ -134,7 +134,7 @@ parse_cmd(Query) ->
     case emqx_redis_command:split(Query) of
         {ok, Cmd} ->
             ok = validate_cmd(Cmd),
-            emqx_authz_utils:parse_deep(Cmd, ?ALLOWED_VARS);
+            emqx_auth_utils:parse_deep(Cmd, ?ALLOWED_VARS);
         {error, Reason} ->
             error({invalid_redis_cmd, Reason, Query})
     end.

apps/emqx_bridge_http/src/emqx_bridge_http_connector.erl

@@ -843,23 +843,43 @@ formalize_request(_Method, BasePath, {Path, Headers}) ->
 %%
 %% See also: `join_paths_test_/0`
 join_paths(Path1, Path2) ->
-    do_join_paths(lists:reverse(to_list(Path1)), to_list(Path2)).
+    [without_trailing_slash(Path1), $/, without_starting_slash(Path2)].
 
-%% "abc/" + "/cde"
+without_starting_slash(Path) ->
-do_join_paths([$/ | Path1], [$/ | Path2]) ->
+    case do_without_starting_slash(Path) of
-    lists:reverse(Path1) ++ [$/ | Path2];
+        empty -> <<>>;
-%% "abc/" + "cde"
+        Other -> Other
-do_join_paths([$/ | Path1], Path2) ->
+    end.
-    lists:reverse(Path1) ++ [$/ | Path2];
-%% "abc" + "/cde"
-do_join_paths(Path1, [$/ | Path2]) ->
-    lists:reverse(Path1) ++ [$/ | Path2];
-%% "abc" + "cde"
-do_join_paths(Path1, Path2) ->
-    lists:reverse(Path1) ++ [$/ | Path2].
 
-to_list(List) when is_list(List) -> List;
+do_without_starting_slash([]) ->
-to_list(Bin) when is_binary(Bin) -> binary_to_list(Bin).
+    empty;
+do_without_starting_slash(<<>>) ->
+    empty;
+do_without_starting_slash([$/ | Rest]) ->
+    Rest;
+do_without_starting_slash([C | _Rest] = Path) when is_integer(C) andalso C =/= $/ ->
+    Path;
+do_without_starting_slash(<<$/, Rest/binary>>) ->
+    Rest;
+do_without_starting_slash(<<C, _Rest/binary>> = Path) when is_integer(C) andalso C =/= $/ ->
+    Path;
+%% On actual lists the recursion should very quickly exhaust
+do_without_starting_slash([El | Rest]) ->
+    case do_without_starting_slash(El) of
+        empty -> do_without_starting_slash(Rest);
+        ElRest -> [ElRest | Rest]
+    end.
+
+without_trailing_slash(Path) ->
+    case iolist_to_binary(Path) of
+        <<>> ->
+            <<>>;
+        B ->
+            case binary:last(B) of
+                $/ -> binary_part(B, 0, byte_size(B) - 1);
+                _ -> B
+            end
+    end.
 
 to_bin(Bin) when is_binary(Bin) ->
     Bin;
@@ -986,6 +1006,9 @@ clientid(Msg) -> maps:get(clientid, Msg, undefined).
 -ifdef(TEST).
 -include_lib("eunit/include/eunit.hrl").
 
+iolists_equal(L1, L2) ->
+    iolist_to_binary(L1) =:= iolist_to_binary(L2).
+
 redact_test_() ->
     TestData = #{
         headers => [
@@ -999,19 +1022,57 @@ redact_test_() ->
 
 join_paths_test_() ->
     [
-        ?_assertEqual("abc/cde", join_paths("abc", "cde")),
+        ?_assert(iolists_equal("abc/cde", join_paths("abc", "cde"))),
-        ?_assertEqual("abc/cde", join_paths("abc", "/cde")),
+        ?_assert(iolists_equal("abc/cde", join_paths(<<"abc">>, <<"cde">>))),
-        ?_assertEqual("abc/cde", join_paths("abc/", "cde")),
+        ?_assert(
-        ?_assertEqual("abc/cde", join_paths("abc/", "/cde")),
+            iolists_equal(
+                "abc/cde",
+                join_paths([["a"], <<"b">>, <<"c">>], [
+                    [[[], <<>>], <<>>, <<"c">>], <<"d">>, <<"e">>
+                ])
+            )
+        ),
 
-        ?_assertEqual("/", join_paths("", "")),
+        ?_assert(iolists_equal("abc/cde", join_paths("abc", "/cde"))),
-        ?_assertEqual("/cde", join_paths("", "cde")),
+        ?_assert(iolists_equal("abc/cde", join_paths(<<"abc">>, <<"/cde">>))),
-        ?_assertEqual("/cde", join_paths("", "/cde")),
+        ?_assert(
-        ?_assertEqual("/cde", join_paths("/", "cde")),
+            iolists_equal(
-        ?_assertEqual("/cde", join_paths("/", "/cde")),
+                "abc/cde",
+                join_paths([["a"], <<"b">>, <<"c">>], [
+                    [<<>>, [[], <<>>], <<"/c">>], <<"d">>, <<"e">>
+                ])
+            )
+        ),
 
-        ?_assertEqual("//cde/", join_paths("/", "//cde/")),
+        ?_assert(iolists_equal("abc/cde", join_paths("abc/", "cde"))),
-        ?_assertEqual("abc///cde/", join_paths("abc//", "//cde/"))
+        ?_assert(iolists_equal("abc/cde", join_paths(<<"abc/">>, <<"cde">>))),
+        ?_assert(
+            iolists_equal(
+                "abc/cde",
+                join_paths([["a"], <<"b">>, <<"c">>, [<<"/">>]], [
+                    [[[], [], <<>>], <<>>, [], <<"c">>], <<"d">>, <<"e">>
+                ])
+            )
+        ),
+
+        ?_assert(iolists_equal("abc/cde", join_paths("abc/", "/cde"))),
+        ?_assert(iolists_equal("abc/cde", join_paths(<<"abc/">>, <<"/cde">>))),
+        ?_assert(
+            iolists_equal(
+                "abc/cde",
+                join_paths([["a"], <<"b">>, <<"c">>, [<<"/">>]], [
+                    [[[], <<>>], <<>>, [[$/]], <<"c">>], <<"d">>, <<"e">>
+                ])
+            )
+        ),
+
+        ?_assert(iolists_equal("/", join_paths("", ""))),
+        ?_assert(iolists_equal("/cde", join_paths("", "cde"))),
+        ?_assert(iolists_equal("/cde", join_paths("", "/cde"))),
+        ?_assert(iolists_equal("/cde", join_paths("/", "cde"))),
+        ?_assert(iolists_equal("/cde", join_paths("/", "/cde"))),
+        ?_assert(iolists_equal("//cde/", join_paths("/", "//cde/"))),
+        ?_assert(iolists_equal("abc///cde/", join_paths("abc//", "//cde/")))
     ].
 
 -endif.

apps/emqx_utils/src/emqx_template.erl

@@ -65,7 +65,7 @@
 -type accessor() :: [binary()].
 -type varname() :: string().
 
--type scalar() :: atom() | unicode:chardata() | number().
+-type scalar() :: atom() | unicode:chardata() | binary() | number().
 -type binding() :: scalar() | list(scalar()) | bindings().
 -type bindings() :: #{atom() | binary() => binding()}.
 
@@ -346,7 +346,7 @@ render_deep({tuple, Template}, Context, Opts) when is_list(Template) ->
     {list_to_tuple(Term), Errors};
 render_deep(Template, Context, Opts) when is_list(Template) ->
     {String, Errors} = render(Template, Context, Opts),
-    {unicode:characters_to_binary(String), Errors};
+    {character_segments_to_binary(String), Errors};
 render_deep(Term, _Bindings, _Opts) ->
     {Term, []}.
 
@@ -424,3 +424,20 @@ to_string(List) when is_list(List) ->
         true -> List;
         false -> emqx_utils_json:encode(List)
     end.
+
+character_segments_to_binary(StringSegments) ->
+    iolist_to_binary(
+        lists:map(
+            fun
+                ($$) ->
+                    $$;
+                (Bin) when is_binary(Bin) -> Bin;
+                (Chars) when is_list(Chars) ->
+                    case unicode:characters_to_binary(Chars) of
+                        Bin when is_binary(Bin) -> Bin;
+                        _ -> emqx_utils_json:encode(Chars)
+                    end
+            end,
+            StringSegments
+        )
+    ).

changes/ce/fix-13040.en.md

@@ -0,0 +1,7 @@
+Improve HTTP authentication
+
+* Emit more meaningful error log message for unknown/missing HTTP content-type header.
+* Fix double encoding of query params in authentication HTTP requests.
+* Emit meaningful error message if POST method and JSON content type are configured for
+authentication HTTP request but JSON template cannot be rendered into a valid JSON. E.g.
+when a template contains `${password}` placeholder, but a client passed non-utf8 password.