fix(auth): use empty strings for absent placeholder values

.gitignore

@@ -68,4 +68,3 @@ apps/emqx/test/emqx_static_checks_data/master.bpapi
 # rendered configurations
 *.conf.rendered
 lux_logs/
-.ci/docker-compose-file/redis/*.log

CHANGES-5.0.md

@@ -8,6 +8,7 @@
 
 * Check ACLs for last will testament topic before publishing the message. [#8930](https://github.com/emqx/emqx/pull/8930)
 * Fix GET /listeners API crash When some nodes still in initial configuration. [#9002](https://github.com/emqx/emqx/pull/9002)
+* Fix empty variable interpolation in authentication and authorization. Placeholders for undefined variables are rendered now as empty strings and do not cause errors anymore. [#8963](https://github.com/emqx/emqx/pull/8963)
 
 # 5.0.8
 

apps/emqx_authn/include/emqx_authn.hrl

@@ -38,8 +38,4 @@
 
 -define(RESOURCE_GROUP, <<"emqx_authn">>).
 
--define(WITH_SUCCESSFUL_RENDER(Code),
-    emqx_authn_utils:with_successful_render(?MODULE, fun() -> Code end)
-).
-
 -endif.

apps/emqx_authn/src/emqx_authn_utils.erl

@@ -34,8 +34,7 @@
     ensure_apps_started/1,
     cleanup_resources/0,
     make_resource_id/1,
-    without_password/1,
+    without_password/1
-    with_successful_render/2
 ]).
 
 -define(AUTHN_PLACEHOLDERS, [
@@ -137,18 +136,6 @@ render_sql_params(ParamList, Credential) ->
         #{return => rawlist, var_trans => fun handle_sql_var/2}
     ).
 
-with_successful_render(Provider, Fun) when is_function(Fun, 0) ->
-    try
-        Fun()
-    catch
-        error:{cannot_get_variable, Name} ->
-            ?TRACE_AUTHN(error, "placeholder_interpolation_failed", #{
-                provider => Provider,
-                placeholder => Name
-            }),
-            ignore
-    end.
-
 %% true
 is_superuser(#{<<"is_superuser">> := <<"true">>}) ->
     #{is_superuser => true};
@@ -230,15 +217,15 @@ without_password(Credential, [Name | Rest]) ->
             without_password(Credential, Rest)
     end.
 
-handle_var({var, Name}, undefined) ->
+handle_var({var, _Name}, undefined) ->
-    error({cannot_get_variable, Name});
+    <<>>;
 handle_var({var, <<"peerhost">>}, PeerHost) ->
     emqx_placeholder:bin(inet:ntoa(PeerHost));
 handle_var(_, Value) ->
     emqx_placeholder:bin(Value).
 
-handle_sql_var({var, Name}, undefined) ->
+handle_sql_var({var, _Name}, undefined) ->
-    error({cannot_get_variable, Name});
+    <<>>;
 handle_sql_var({var, <<"peerhost">>}, PeerHost) ->
     emqx_placeholder:bin(inet:ntoa(PeerHost));
 handle_sql_var(_, Value) ->

apps/emqx_authn/src/simple_authn/emqx_authn_http.erl

@@ -187,29 +187,25 @@ authenticate(
         request_timeout := RequestTimeout
     } = State
 ) ->
-    ?WITH_SUCCESSFUL_RENDER(
+    Request = generate_request(Credential, State),
-        begin
+    Response = emqx_resource:query(ResourceId, {Method, Request, RequestTimeout}),
-            Request = generate_request(Credential, State),
+    ?TRACE_AUTHN_PROVIDER("http_response", #{
-            Response = emqx_resource:query(ResourceId, {Method, Request, RequestTimeout}),
+        request => request_for_log(Credential, State),
-            ?TRACE_AUTHN_PROVIDER("http_response", #{
+        response => response_for_log(Response),
-                request => request_for_log(Credential, State),
+        resource => ResourceId
-                response => response_for_log(Response),
+    }),
-                resource => ResourceId
+    case Response of
-            }),
+        {ok, 204, _Headers} ->
-            case Response of
+            {ok, #{is_superuser => false}};
-                {ok, 204, _Headers} ->
+        {ok, 200, Headers, Body} ->
-                    {ok, #{is_superuser => false}};
+            handle_response(Headers, Body);
-                {ok, 200, Headers, Body} ->
+        {ok, _StatusCode, _Headers} = Response ->
-                    handle_response(Headers, Body);
+            ignore;
-                {ok, _StatusCode, _Headers} = Response ->
+        {ok, _StatusCode, _Headers, _Body} = Response ->
-                    ignore;
+            ignore;
-                {ok, _StatusCode, _Headers, _Body} = Response ->
+        {error, _Reason} ->
-                    ignore;
+            ignore
-                {error, _Reason} ->
+    end.
-                    ignore
-            end
-        end
-    ).
 
 destroy(#{resource_id := ResourceId}) ->
     _ = emqx_resource:remove_local(ResourceId),

apps/emqx_authn/src/simple_authn/emqx_authn_mongodb.erl

@@ -162,39 +162,35 @@ authenticate(
         resource_id := ResourceId
     } = State
 ) ->
-    ?WITH_SUCCESSFUL_RENDER(
+    Filter = emqx_authn_utils:render_deep(FilterTemplate, Credential),
-        begin
+    case emqx_resource:query(ResourceId, {find_one, Collection, Filter, #{}}) of
-            Filter = emqx_authn_utils:render_deep(FilterTemplate, Credential),
+        undefined ->
-            case emqx_resource:query(ResourceId, {find_one, Collection, Filter, #{}}) of
+            ignore;
-                undefined ->
+        {error, Reason} ->
-                    ignore;
+            ?TRACE_AUTHN_PROVIDER(error, "mongodb_query_failed", #{
-                {error, Reason} ->
+                resource => ResourceId,
-                    ?TRACE_AUTHN_PROVIDER(error, "mongodb_query_failed", #{
+                collection => Collection,
+                filter => Filter,
+                reason => Reason
+            }),
+            ignore;
+        Doc ->
+            case check_password(Password, Doc, State) of
+                ok ->
+                    {ok, is_superuser(Doc, State)};
+                {error, {cannot_find_password_hash_field, PasswordHashField}} ->
+                    ?TRACE_AUTHN_PROVIDER(error, "cannot_find_password_hash_field", #{
                         resource => ResourceId,
                         collection => Collection,
                         filter => Filter,
-                        reason => Reason
+                        document => Doc,
+                        password_hash_field => PasswordHashField
                     }),
                     ignore;
-                Doc ->
+                {error, Reason} ->
-                    case check_password(Password, Doc, State) of
+                    {error, Reason}
-                        ok ->
-                            {ok, is_superuser(Doc, State)};
-                        {error, {cannot_find_password_hash_field, PasswordHashField}} ->
-                            ?TRACE_AUTHN_PROVIDER(error, "cannot_find_password_hash_field", #{
-                                resource => ResourceId,
-                                collection => Collection,
-                                filter => Filter,
-                                document => Doc,
-                                password_hash_field => PasswordHashField
-                            }),
-                            ignore;
-                        {error, Reason} ->
-                            {error, Reason}
-                    end
             end
-        end
+    end.
-    ).
 
 %%------------------------------------------------------------------------------
 %% Internal functions

apps/emqx_authn/src/simple_authn/emqx_authn_mysql.erl

@@ -113,36 +113,32 @@ authenticate(
         password_hash_algorithm := Algorithm
     }
 ) ->
-    ?WITH_SUCCESSFUL_RENDER(
+    Params = emqx_authn_utils:render_sql_params(TmplToken, Credential),
-        begin
+    case emqx_resource:query(ResourceId, {prepared_query, ?PREPARE_KEY, Params, Timeout}) of
-            Params = emqx_authn_utils:render_sql_params(TmplToken, Credential),
+        {ok, _Columns, []} ->
-            case emqx_resource:query(ResourceId, {prepared_query, ?PREPARE_KEY, Params, Timeout}) of
+            ignore;
-                {ok, _Columns, []} ->
+        {ok, Columns, [Row | _]} ->
-                    ignore;
+            Selected = maps:from_list(lists:zip(Columns, Row)),
-                {ok, Columns, [Row | _]} ->
+            case
-                    Selected = maps:from_list(lists:zip(Columns, Row)),
+                emqx_authn_utils:check_password_from_selected_map(
-                    case
+                    Algorithm, Selected, Password
-                        emqx_authn_utils:check_password_from_selected_map(
+                )
-                            Algorithm, Selected, Password
+            of
-                        )
+                ok ->
-                    of
+                    {ok, emqx_authn_utils:is_superuser(Selected)};
-                        ok ->
-                            {ok, emqx_authn_utils:is_superuser(Selected)};
-                        {error, Reason} ->
-                            {error, Reason}
-                    end;
                 {error, Reason} ->
-                    ?TRACE_AUTHN_PROVIDER(error, "mysql_query_failed", #{
+                    {error, Reason}
-                        resource => ResourceId,
+            end;
-                        tmpl_token => TmplToken,
+        {error, Reason} ->
-                        params => Params,
+            ?TRACE_AUTHN_PROVIDER(error, "mysql_query_failed", #{
-                        timeout => Timeout,
+                resource => ResourceId,
-                        reason => Reason
+                tmpl_token => TmplToken,
-                    }),
+                params => Params,
-                    ignore
+                timeout => Timeout,
-            end
+                reason => Reason
-        end
+            }),
-    ).
+            ignore
+    end.
 
 parse_config(
     #{

apps/emqx_authn/src/simple_authn/emqx_authn_pgsql.erl

@@ -115,35 +115,31 @@ authenticate(
         password_hash_algorithm := Algorithm
     }
 ) ->
-    ?WITH_SUCCESSFUL_RENDER(
+    Params = emqx_authn_utils:render_sql_params(PlaceHolders, Credential),
-        begin
+    case emqx_resource:query(ResourceId, {prepared_query, ResourceId, Params}) of
-            Params = emqx_authn_utils:render_sql_params(PlaceHolders, Credential),
+        {ok, _Columns, []} ->
-            case emqx_resource:query(ResourceId, {prepared_query, ResourceId, Params}) of
+            ignore;
-                {ok, _Columns, []} ->
+        {ok, Columns, [Row | _]} ->
-                    ignore;
+            NColumns = [Name || #column{name = Name} <- Columns],
-                {ok, Columns, [Row | _]} ->
+            Selected = maps:from_list(lists:zip(NColumns, erlang:tuple_to_list(Row))),
-                    NColumns = [Name || #column{name = Name} <- Columns],
+            case
-                    Selected = maps:from_list(lists:zip(NColumns, erlang:tuple_to_list(Row))),
+                emqx_authn_utils:check_password_from_selected_map(
-                    case
+                    Algorithm, Selected, Password
-                        emqx_authn_utils:check_password_from_selected_map(
+                )
-                            Algorithm, Selected, Password
+            of
-                        )
+                ok ->
-                    of
+                    {ok, emqx_authn_utils:is_superuser(Selected)};
-                        ok ->
-                            {ok, emqx_authn_utils:is_superuser(Selected)};
-                        {error, Reason} ->
-                            {error, Reason}
-                    end;
                 {error, Reason} ->
-                    ?TRACE_AUTHN_PROVIDER(error, "postgresql_query_failed", #{
+                    {error, Reason}
-                        resource => ResourceId,
+            end;
-                        params => Params,
+        {error, Reason} ->
-                        reason => Reason
+            ?TRACE_AUTHN_PROVIDER(error, "postgresql_query_failed", #{
-                    }),
+                resource => ResourceId,
-                    ignore
+                params => Params,
-            end
+                reason => Reason
-        end
+            }),
-    ).
+            ignore
+    end.
 
 parse_config(
     #{

apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl

@@ -133,37 +133,33 @@ authenticate(
         password_hash_algorithm := Algorithm
     }
 ) ->
-    ?WITH_SUCCESSFUL_RENDER(
+    NKey = emqx_authn_utils:render_str(KeyTemplate, Credential),
-        begin
+    Command = [CommandName, NKey | Fields],
-            NKey = emqx_authn_utils:render_str(KeyTemplate, Credential),
+    case emqx_resource:query(ResourceId, {cmd, Command}) of
-            Command = [CommandName, NKey | Fields],
+        {ok, []} ->
-            case emqx_resource:query(ResourceId, {cmd, Command}) of
+            ignore;
-                {ok, []} ->
+        {ok, Values} ->
-                    ignore;
+            Selected = merge(Fields, Values),
-                {ok, Values} ->
+            case
-                    Selected = merge(Fields, Values),
+                emqx_authn_utils:check_password_from_selected_map(
-                    case
+                    Algorithm, Selected, Password
-                        emqx_authn_utils:check_password_from_selected_map(
+                )
-                            Algorithm, Selected, Password
+            of
-                        )
+                ok ->
-                    of
+                    {ok, emqx_authn_utils:is_superuser(Selected)};
-                        ok ->
+                {error, _Reason} ->
-                            {ok, emqx_authn_utils:is_superuser(Selected)};
-                        {error, _Reason} ->
-                            ignore
-                    end;
-                {error, Reason} ->
-                    ?TRACE_AUTHN_PROVIDER(error, "redis_query_failed", #{
-                        resource => ResourceId,
-                        cmd => Command,
-                        keys => NKey,
-                        fields => Fields,
-                        reason => Reason
-                    }),
                     ignore
-            end
+            end;
-        end
+        {error, Reason} ->
-    ).
+            ?TRACE_AUTHN_PROVIDER(error, "redis_query_failed", #{
+                resource => ResourceId,
+                cmd => Command,
+                keys => NKey,
+                fields => Fields,
+                reason => Reason
+            }),
+            ignore
+    end.
 
 %%------------------------------------------------------------------------------
 %% Internal functions

apps/emqx_authn/test/emqx_authn_http_SUITE.erl

@@ -166,6 +166,49 @@ test_user_auth(#{
         ?GLOBAL
     ).
 
+t_no_value_for_placeholder(_Config) ->
+    Handler = fun(Req0, State) ->
+        {ok, RawBody, Req1} = cowboy_req:read_body(Req0),
+        #{
+            <<"cert_subject">> := <<"">>,
+            <<"cert_common_name">> := <<"">>
+        } = jiffy:decode(RawBody, [return_maps]),
+        Req = cowboy_req:reply(
+            200,
+            #{<<"content-type">> => <<"application/json">>},
+            jiffy:encode(#{result => allow, is_superuser => false}),
+            Req1
+        ),
+        {ok, Req, State}
+    end,
+
+    SpecificConfgParams = #{
+        <<"method">> => <<"post">>,
+        <<"headers">> => #{<<"content-type">> => <<"application/json">>},
+        <<"body">> => #{
+            <<"cert_subject">> => ?PH_CERT_SUBJECT,
+            <<"cert_common_name">> => ?PH_CERT_CN_NAME
+        }
+    },
+
+    AuthConfig = maps:merge(raw_http_auth_config(), SpecificConfgParams),
+
+    {ok, _} = emqx:update_config(
+        ?PATH,
+        {create_authenticator, ?GLOBAL, AuthConfig}
+    ),
+
+    ok = emqx_authn_http_test_server:set_handler(Handler),
+
+    Credentials = maps:without([cert_subject, cert_common_name], ?CREDENTIALS),
+
+    ?assertMatch({ok, _}, emqx_access_control:authenticate(Credentials)),
+
+    emqx_authn_test_lib:delete_authenticators(
+        [authentication],
+        ?GLOBAL
+    ).
+
 t_destroy(_Config) ->
     AuthConfig = raw_http_auth_config(),
 
@@ -247,27 +290,6 @@ t_update(_Config) ->
         emqx_access_control:authenticate(?CREDENTIALS)
     ).
 
-t_interpolation_error(_Config) ->
-    {ok, _} = emqx:update_config(
-        ?PATH,
-        {create_authenticator, ?GLOBAL, raw_http_auth_config()}
-    ),
-
-    Headers = #{<<"content-type">> => <<"application/json">>},
-    Response = ?SERVER_RESPONSE_JSON(allow),
-
-    ok = emqx_authn_http_test_server:set_handler(
-        fun(Req0, State) ->
-            Req = cowboy_req:reply(200, Headers, Response, Req0),
-            {ok, Req, State}
-        end
-    ),
-
-    ?assertMatch(
-        ?EXCEPTION_DENY,
-        emqx_access_control:authenticate(maps:without([username], ?CREDENTIALS))
-    ).
-
 t_is_superuser(_Config) ->
     Config = raw_http_auth_config(),
     {ok, _} = emqx:update_config(
@@ -431,26 +453,6 @@ samples() ->
             result => {ok, #{is_superuser => false, user_property => #{}}}
         },
 
-        %% simple get request, no username
-        #{
-            handler => fun(Req0, State) ->
-                #{
-                    username := <<"plain">>,
-                    password := <<"plain">>
-                } = cowboy_req:match_qs([username, password], Req0),
-
-                Req = cowboy_req:reply(
-                    200,
-                    #{<<"content-type">> => <<"application/json">>},
-                    jiffy:encode(#{result => allow, is_superuser => false}),
-                    Req0
-                ),
-                {ok, Req, State}
-            end,
-            config_params => #{},
-            result => {ok, #{is_superuser => false, user_property => #{}}}
-        },
-
         %% get request with json body response
         #{
             handler => fun(Req0, State) ->

apps/emqx_authn/test/emqx_authn_mongo_SUITE.erl

@@ -288,20 +288,6 @@ raw_mongo_auth_config() ->
 
 user_seeds() ->
     [
-        #{
-            data => #{
-                username => <<"plain">>,
-                password_hash => <<"plainsalt">>,
-                salt => <<"salt">>,
-                is_superuser => <<"1">>
-            },
-            credentials => #{
-                password => <<"plain">>
-            },
-            config_params => #{},
-            result => {error, not_authorized}
-        },
-
         #{
             data => #{
                 username => <<"plain">>,

apps/emqx_authn/test/emqx_authn_mysql_SUITE.erl

@@ -258,20 +258,6 @@ raw_mysql_auth_config() ->
 
 user_seeds() ->
     [
-        #{
-            data => #{
-                username => "plain",
-                password_hash => "plainsalt",
-                salt => "salt",
-                is_superuser_str => "1"
-            },
-            credentials => #{
-                password => <<"plain">>
-            },
-            config_params => #{},
-            result => {error, not_authorized}
-        },
-
         #{
             data => #{
                 username => "plain",

apps/emqx_authn/test/emqx_authn_pgsql_SUITE.erl

@@ -320,20 +320,6 @@ raw_pgsql_auth_config() ->
 
 user_seeds() ->
     [
-        #{
-            data => #{
-                username => "plain",
-                password_hash => "plainsalt",
-                salt => "salt",
-                is_superuser_str => "1"
-            },
-            credentials => #{
-                password => <<"plain">>
-            },
-            config_params => #{},
-            result => {error, not_authorized}
-        },
-
         #{
             data => #{
                 username => "plain",

apps/emqx_authn/test/emqx_authn_redis_SUITE.erl

@@ -292,20 +292,6 @@ raw_redis_auth_config() ->
 
 user_seeds() ->
     [
-        #{
-            data => #{
-                password_hash => <<"plainsalt">>,
-                salt => <<"salt">>,
-                is_superuser => <<"1">>
-            },
-            credentials => #{
-                password => <<"plain">>
-            },
-            key => <<"mqtt_user:plain">>,
-            config_params => #{},
-            result => {error, not_authorized}
-        },
-
         #{
             data => #{
                 password_hash => <<"plainsalt">>,

apps/emqx_authz/src/emqx_authz.erl

@@ -391,14 +391,6 @@ do_authorize(
         Matched ->
             {Matched, Type}
     catch
-        error:{cannot_get_variable, Name} ->
-            emqx_metrics_worker:inc(authz_metrics, Type, nomatch),
-            ?SLOG(warning, #{
-                msg => "placeholder_interpolation_failed",
-                placeholder => Name,
-                authorize_type => Type
-            }),
-            do_authorize(Client, PubSub, Topic, Tail);
         Class:Reason:Stacktrace ->
             emqx_metrics_worker:inc(authz_metrics, Type, nomatch),
             ?SLOG(warning, #{

apps/emqx_authz/src/emqx_authz_utils.erl

@@ -181,15 +181,15 @@ convert_client_var({dn, DN}) -> {cert_subject, DN};
 convert_client_var({protocol, Proto}) -> {proto_name, Proto};
 convert_client_var(Other) -> Other.
 
-handle_var({var, Name}, undefined) ->
+handle_var({var, _Name}, undefined) ->
-    error({cannot_get_variable, Name});
+    <<>>;
 handle_var({var, <<"peerhost">>}, IpAddr) ->
     inet_parse:ntoa(IpAddr);
 handle_var(_Name, Value) ->
     emqx_placeholder:bin(Value).
 
-handle_sql_var({var, Name}, undefined) ->
+handle_sql_var({var, _Name}, undefined) ->
-    error({cannot_get_variable, Name});
+    <<>>;
 handle_sql_var({var, <<"peerhost">>}, IpAddr) ->
     inet_parse:ntoa(IpAddr);
 handle_sql_var(_Name, Value) ->

apps/emqx_authz/test/emqx_authz_http_SUITE.erl

@@ -364,6 +364,46 @@ t_placeholder_and_body(_Config) ->
         emqx_access_control:authorize(ClientInfo, publish, <<"t">>)
     ).
 
+t_no_value_for_placeholder(_Config) ->
+    ok = setup_handler_and_config(
+        fun(Req0, State) ->
+            ?assertEqual(
+                <<"/authz/users/">>,
+                cowboy_req:path(Req0)
+            ),
+
+            {ok, RawBody, Req1} = cowboy_req:read_body(Req0),
+
+            ?assertMatch(
+                #{
+                    <<"mountpoint">> := <<"[]">>
+                },
+                jiffy:decode(RawBody, [return_maps])
+            ),
+            {ok, ?AUTHZ_HTTP_RESP(allow, Req1), State}
+        end,
+        #{
+            <<"method">> => <<"post">>,
+            <<"body">> => #{
+                <<"mountpoint">> => <<"[${mountpoint}]">>
+            }
+        }
+    ),
+
+    ClientInfo = #{
+        clientid => <<"client id">>,
+        username => <<"user name">>,
+        peerhost => {127, 0, 0, 1},
+        protocol => <<"MQTT">>,
+        zone => default,
+        listener => {tcp, default}
+    },
+
+    ?assertEqual(
+        allow,
+        emqx_access_control:authorize(ClientInfo, publish, <<"t">>)
+    ).
+
 t_create_replace(_Config) ->
     ClientInfo = #{
         clientid => <<"clientid">>,