fix(auth_http): fix query encoding

* ignore authenticator if JSON format is set up for requests, but non-utf8 data is going to be sent
* use application/json format by default
* fix encoding of query part of the requests

apps/emqx_auth/src/emqx_authn/emqx_authn_utils.erl

@@ -28,7 +28,8 @@
     parse_str/1,
     parse_str/2,
     parse_sql/2,
-    render_deep/2,
+    render_deep_for_json/2,
+    render_deep_for_url/2,
     render_str/2,
     render_urlencoded_str/2,
     render_sql_params/2,
@@ -166,13 +167,23 @@ prerender_disallowed_placeholders(Template) ->
     }),
     Result.
 
-render_deep(Template, Credential) ->
+render_deep_for_json(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}
+        #{var_trans => fun to_string_for_json/2}
+    ),
+    Term.
+
+render_deep_for_url(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_for_urlencode/2}
     ),
     Term.
 
@@ -202,7 +213,7 @@ render_sql_params(ParamList, Credential) ->
     {Row, _Errors} = emqx_template:render(
         ParamList,
         mapping_credential(Credential),
-        #{var_trans => fun to_sql_valaue/2}
+        #{var_trans => fun to_sql_value/2}
     ),
     Row.
 
@@ -328,12 +339,43 @@ without_password(Credential, [Name | Rest]) ->
     end.
 
 to_urlencoded_string(Name, Value) ->
-    emqx_http_lib:uri_encode(to_string(Name, Value)).
+    <<"q=", EncodedValue/binary>> = uri_string:compose_query([{<<"q">>, to_string(Name, Value)}]),
+    EncodedValue.
 
 to_string(Name, Value) ->
     emqx_template:to_string(render_var(Name, Value)).
 
-to_sql_valaue(Name, Value) ->
+%% Any data may be urlencoded, so we allow non-unicode binaries here.
+
+to_string_for_urlencode(Name, Value) ->
+    to_string_for_urlencode(render_var(Name, Value)).
+
+to_string_for_urlencode(Value) when is_binary(Value) ->
+    Value;
+to_string_for_urlencode(Value) when is_list(Value) ->
+    unicode:characters_to_binary(Value);
+to_string_for_urlencode(Value) ->
+    emqx_template:to_string(Value).
+
+%% JSON strings are sequences of unicode characters, not bytes.
+%% So we force all rendered data to be unicode.
+
+to_string_for_json(Name, Value) ->
+    to_unicode_string(Name, render_var(Name, Value)).
+
+to_unicode_string(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_unicode_string(_Name, Value) ->
+    emqx_template:to_string(Value).
+
+to_sql_value(Name, Value) ->
     emqx_utils_sql:to_sql_value(render_var(Name, Value)).
 
 render_var(_, undefined) ->
@@ -343,6 +385,8 @@ render_var(_, undefined) ->
     <<>>;
 render_var(?VAR_PEERHOST, Value) ->
     inet:ntoa(Value);
+render_var(?VAR_PASSWORD, Value) ->
+    iolist_to_binary(Value);
 render_var(_Name, Value) ->
     Value.
 

apps/emqx_auth_http/src/emqx_authn_http.erl

@@ -28,6 +28,8 @@
     destroy/1
 ]).
 
+-define(DEFAULT_CONTENT_TYPE, <<"application/json">>).
+
 %%------------------------------------------------------------------------------
 %% APIs
 %%------------------------------------------------------------------------------
@@ -68,8 +70,11 @@ 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} ->
+            Response = emqx_resource:simple_sync_query(
+                ResourceId, {Method, Request, RequestTimeout}
+            ),
             ?TRACE_AUTHN_PROVIDER("http_response", #{
                 request => request_for_log(Credential, State),
                 response => response_for_log(Response),
@@ -86,6 +91,14 @@ authenticate(
                     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.
 
 destroy(#{resource_id := ResourceId}) ->
@@ -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(
     #{
@@ -134,16 +156,20 @@ parse_config(
         request_timeout := RequestTimeout
     } = Config
 ) ->
+    ct:print("parse_config: ~p~n", [Config]),
     {RequestBase, Path, Query} = emqx_auth_utils:parse_url(RawUrl),
     State = #{
         method => Method,
         path => Path,
-        headers => ensure_header_name_type(Headers),
+        headers => 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
     },
@@ -163,36 +189,38 @@ generate_request(Credential, #{
 }) ->
     Headers = maps:to_list(Headers0),
     Path = emqx_authn_utils:render_urlencoded_str(BasePathTemplate, Credential),
-    Query = emqx_authn_utils:render_deep(BaseQueryTemplate, Credential),
+    Query = emqx_authn_utils:render_deep_for_url(BaseQueryTemplate, Credential),
-    Body = emqx_authn_utils:render_deep(BodyTemplate, Credential),
     case Method of
         get ->
+            Body = emqx_authn_utils:render_deep_for_url(BodyTemplate, Credential),
             NPathQuery = append_query(to_list(Path), to_list(Query) ++ maps:to_list(Body)),
-            {NPathQuery, Headers};
+            {ok, {NPathQuery, Headers}};
         post ->
+            ContentType = post_request_content_type(Headers),
+            try
+                Body = serialize_body(ContentType, BodyTemplate, Credential),
                 NPathQuery = append_query(to_list(Path), to_list(Query)),
-            ContentType = proplists:get_value(<<"content-type">>, Headers),
+                {ok, {NPathQuery, Headers, Body}}
-            NBody = serialize_body(ContentType, Body),
+            catch
-            {NPathQuery, Headers, NBody}
+                error:{encode_error, _} = Reason ->
+                    {error, Reason}
+            end
     end.
 
 append_query(Path, []) ->
     Path;
 append_query(Path, Query) ->
-    Path ++ "?" ++ binary_to_list(qs(Query)).
+    ct:print("append_query: ~p~n", [Query]),
+    Path ++ "?" ++ qs(Query).
 
 qs(KVs) ->
-    qs(KVs, []).
+    uri_string:compose_query(KVs).
 
-qs([], Acc) ->
+serialize_body(<<"application/json">>, BodyTemplate, Credential) ->
-    <<$&, Qs/binary>> = iolist_to_binary(lists:reverse(Acc)),
+    Body = emqx_authn_utils:render_deep_for_json(BodyTemplate, Credential),
-    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) ->
+serialize_body(<<"application/x-www-form-urlencoded">>, BodyTemplate, Credential) ->
+    Body = emqx_authn_utils:render_deep_for_url(BodyTemplate, Credential),
     qs(maps:to_list(Body)).
 
 handle_response(Headers, Body) ->
@@ -239,8 +267,8 @@ parse_body(<<"application/x-www-form-urlencoded", _/binary>>, Body) ->
 parse_body(ContentType, _) ->
     {error, {unsupported_content_type, ContentType}}.
 
-uri_encode(T) ->
+post_request_content_type(Headers) ->
-    emqx_http_lib:uri_encode(to_list(T)).
+    proplists:get_value(<<"content-type">>, Headers, ?DEFAULT_CONTENT_TYPE).
 
 request_for_log(Credential, #{url := Url, method := Method} = State) ->
     SafeCredential = emqx_authn_utils:without_password(Credential),
@@ -276,7 +304,7 @@ to_list(B) when is_binary(B) ->
 to_list(L) when is_list(L) ->
     L.
 
-ensure_header_name_type(Headers) ->
+ensure_binary_names(Headers) ->
     Fun = fun
         (Key, _Val, Acc) when is_binary(Key) ->
             Acc;

apps/emqx_auth_http/test/emqx_authn_http_SUITE.erl

@@ -152,8 +152,9 @@ test_user_auth(#{
     handler := Handler,
     config_params := SpecificConfgParams,
     result := Expect
-}) ->
+} = Sample) ->
-    Result = perform_user_auth(SpecificConfgParams, Handler, ?CREDENTIALS),
+    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 +181,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 +564,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 +649,31 @@ 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]),
+                ct:print("headers: ~p", [cowboy_req:headers(Req0)]),
+                <<"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 +737,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_mongodb/src/emqx_authn_mongodb.erl

@@ -68,7 +68,7 @@ authenticate(
         resource_id := ResourceId
     } = State
 ) ->
-    Filter = emqx_authn_utils:render_deep(FilterTemplate, Credential),
+    Filter = emqx_authn_utils:render_deep_for_json(FilterTemplate, Credential),
     case emqx_resource:simple_sync_query(ResourceId, {find_one, Collection, Filter, #{}}) of
         {ok, undefined} ->
             ignore;

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,21 @@ to_string(List) when is_list(List) ->
         true -> List;
         false -> emqx_utils_json:encode(List)
     end.
+
+character_segments_to_binary(StringSegments) ->
+    ct:print("characters_to_binary: ~p~n", [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
+        )
+    ).