chore: support strip double quote in authn/authz

more compatibility for https://github.com/emqx/emqx/pull/8827
2022-09-14 12:05:15 +08:00 · 2022-09-14 12:05:15 +08:00 · a461375b30
parent 53bc6f47e7
commit a461375b30
8 changed files with 152 additions and 25 deletions
--- a/apps/emqx_authn/src/emqx_authn_utils.erl
+++ b/apps/emqx_authn/src/emqx_authn_utils.erl
@ -112,7 +112,8 @@ parse_sql(Template, ReplaceWith) ->
        Template,
        #{
            replace_with => ReplaceWith,
-            placeholders => ?AUTHN_PLACEHOLDERS
+            placeholders => ?AUTHN_PLACEHOLDERS,
+            strip_double_quote => true
        }
    ).

--- a/apps/emqx_authn/test/emqx_authn_mysql_SUITE.erl
+++ b/apps/emqx_authn/test/emqx_authn_mysql_SUITE.erl
@ -332,6 +332,32 @@ user_seeds() ->
            result => {ok, #{is_superuser => true}}
        },

+        %% strip double quote support
+        #{
+            data => #{
+                username => "sha256",
+                password_hash => "ac63a624e7074776d677dd61a003b8c803eb11db004d0ec6ae032a5d7c9c5caf",
+                salt => "salt",
+                is_superuser_int => 1
+            },
+            credentials => #{
+                username => <<"sha256">>,
+                password => <<"sha256">>
+            },
+            config_params => #{
+                <<"query">> =>
+                    <<
+                        "SELECT password_hash, salt, is_superuser_int as is_superuser\n"
+                        "                            FROM users where username = \"${username}\" LIMIT 1"
+                    >>,
+                <<"password_hash_algorithm">> => #{
+                    <<"name">> => <<"sha256">>,
+                    <<"salt_position">> => <<"prefix">>
+                }
+            },
+            result => {ok, #{is_superuser => true}}
+        },
+
        #{
            data => #{
                username => "sha256",
--- a/apps/emqx_authn/test/emqx_authn_pgsql_SUITE.erl
+++ b/apps/emqx_authn/test/emqx_authn_pgsql_SUITE.erl
@ -394,6 +394,32 @@ user_seeds() ->
            result => {ok, #{is_superuser => true}}
        },

+        %% strip double quote support
+        #{
+            data => #{
+                username => "sha256",
+                password_hash => "ac63a624e7074776d677dd61a003b8c803eb11db004d0ec6ae032a5d7c9c5caf",
+                salt => "salt",
+                is_superuser_int => 1
+            },
+            credentials => #{
+                username => <<"sha256">>,
+                password => <<"sha256">>
+            },
+            config_params => #{
+                <<"query">> =>
+                    <<
+                        "SELECT password_hash, salt, is_superuser_int as is_superuser\n"
+                        "                            FROM users where username = \"${username}\" LIMIT 1"
+                    >>,
+                <<"password_hash_algorithm">> => #{
+                    <<"name">> => <<"sha256">>,
+                    <<"salt_position">> => <<"prefix">>
+                }
+            },
+            result => {ok, #{is_superuser => true}}
+        },
+
        #{
            data => #{
                username => "sha256",
--- a/apps/emqx_authz/src/emqx_authz_utils.erl
+++ b/apps/emqx_authz/src/emqx_authz_utils.erl
@ -110,7 +110,8 @@ parse_sql(Template, ReplaceWith, PlaceHolders) ->
        Template,
        #{
            replace_with => ReplaceWith,
-            placeholders => PlaceHolders
+            placeholders => PlaceHolders,
+            strip_double_quote => true
        }
    ).

--- a/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl
+++ b/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl
@ -202,6 +202,34 @@ t_lookups(_Config) ->
        }
    ),

+    ok = emqx_authz_test_lib:test_samples(
+        ClientInfo,
+        [
+            {allow, subscribe, <<"a">>},
+            {deny, subscribe, <<"b">>}
+        ]
+    ),
+
+    %% strip double quote support
+
+    ok = init_table(),
+    ok = q(
+        <<
+            "INSERT INTO acl(clientid, topic, permission, action)"
+            "VALUES(?, ?, ?, ?)"
+        >>,
+        [<<"clientid">>, <<"a">>, <<"allow">>, <<"subscribe">>]
+    ),
+
+    ok = setup_config(
+        #{
+            <<"query">> => <<
+                "SELECT permission, action, topic "
+                "FROM acl WHERE clientid = \"${clientid}\""
+            >>
+        }
+    ),
+
    ok = emqx_authz_test_lib:test_samples(
        ClientInfo,
        [
--- a/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl
+++ b/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl
@ -202,6 +202,34 @@ t_lookups(_Config) ->
        }
    ),

+    ok = emqx_authz_test_lib:test_samples(
+        ClientInfo,
+        [
+            {allow, subscribe, <<"a">>},
+            {deny, subscribe, <<"b">>}
+        ]
+    ),
+
+    %% strip double quote support
+
+    ok = init_table(),
+    ok = insert(
+        <<
+            "INSERT INTO acl(clientid, topic, permission, action)"
+            "VALUES($1, $2, $3, $4)"
+        >>,
+        [<<"clientid">>, <<"a">>, <<"allow">>, <<"subscribe">>]
+    ),
+
+    ok = setup_config(
+        #{
+            <<"query">> => <<
+                "SELECT permission, action, topic "
+                "FROM acl WHERE clientid = \"${clientid}\""
+            >>
+        }
+    ),
+
    ok = emqx_authz_test_lib:test_samples(
        ClientInfo,
        [
--- a/apps/emqx_plugin_libs/src/emqx_placeholder.erl
+++ b/apps/emqx_plugin_libs/src/emqx_placeholder.erl
@ -39,7 +39,10 @@
    sql_data/1
 ]).

-define(EX_PLACE_HOLDER, "(\\$\\{[a-zA-Z0-9\\._]+\\}|\"\\$\\{[a-zA-Z0-9\\._]+\\}\")").
+-define(EX_PLACE_HOLDER, "(\\$\\{[a-zA-Z0-9\\._]+\\})").
+
+-define(EX_PLACE_HOLDER_DOUBLE_QUOTE, "(\\$\\{[a-zA-Z0-9\\._]+\\}|\"\\$\\{[a-zA-Z0-9\\._]+\\}\")").
+
 %% Space and CRLF
 -define(EX_WITHE_CHARS, "\\s").

@ -57,7 +60,8 @@

 -type preproc_sql_opts() :: #{
    placeholders => list(binary()),
-    replace_with => '?' | '$n'
+    replace_with => '?' | '$n',
+    strip_double_quote => boolean()
 }.

 -type preproc_deep_opts() :: #{
@ -89,7 +93,7 @@ preproc_tmpl(Str) ->
 preproc_tmpl(Str, Opts) ->
    RE = preproc_var_re(Opts),
    Tokens = re:split(Str, RE, [{return, binary}, group, trim]),
-    do_preproc_tmpl(Tokens, []).
+    do_preproc_tmpl(Opts, Tokens, []).

 -spec proc_tmpl(tmpl_token(), map()) -> binary().
 proc_tmpl(Tokens, Data) ->
@ -140,10 +144,11 @@ preproc_sql(Sql, ReplaceWith) when is_atom(ReplaceWith) ->
    preproc_sql(Sql, #{replace_with => ReplaceWith});
 preproc_sql(Sql, Opts) ->
    RE = preproc_var_re(Opts),
+    Strip = maps:get(strip_double_quote, Opts, false),
    ReplaceWith = maps:get(replace_with, Opts, '?'),
    case re:run(Sql, RE, [{capture, all_but_first, binary}, global]) of
        {match, PlaceHolders} ->
-            PhKs = [parse_nested(unwrap(Phld)) || [Phld | _] <- PlaceHolders],
+            PhKs = [parse_nested(unwrap(Phld, Strip)) || [Phld | _] <- PlaceHolders],
            {replace_with(Sql, RE, ReplaceWith), [{var, Phld} || Phld <- PhKs]};
        nomatch ->
            {Sql, []}
@ -234,29 +239,36 @@ get_phld_var(Fun, Data) when is_function(Fun) ->
 get_phld_var(Phld, Data) ->
    emqx_rule_maps:nested_get(Phld, Data).

-preproc_var_re(#{placeholders := PHs}) ->
+preproc_var_re(#{placeholders := PHs, strip_double_quote := true}) ->
    Res = [ph_to_re(PH) || PH <- PHs],
    QuoteRes = ["\"" ++ Re ++ "\"" || Re <- Res],
    "(" ++ string:join(Res ++ QuoteRes, "|") ++ ")";
+preproc_var_re(#{placeholders := PHs}) ->
+    "(" ++ string:join([ph_to_re(PH) || PH <- PHs], "|") ++ ")";
+preproc_var_re(#{strip_double_quote := true}) ->
+    ?EX_PLACE_HOLDER_DOUBLE_QUOTE;
 preproc_var_re(#{}) ->
    ?EX_PLACE_HOLDER.

 ph_to_re(VarPH) ->
    re:replace(VarPH, "[\\$\\{\\}]", "\\\\&", [global, {return, list}]).

-do_preproc_tmpl([], Acc) ->
+do_preproc_tmpl(_Opts, [], Acc) ->
    lists:reverse(Acc);
-do_preproc_tmpl([[Str, Phld] | Tokens], Acc) ->
+do_preproc_tmpl(Opts, [[Str, Phld] | Tokens], Acc) ->
+    Strip = maps:get(strip_double_quote, Opts, false),
    do_preproc_tmpl(
+        Opts,
        Tokens,
        put_head(
            var,
-            parse_nested(unwrap(Phld)),
+            parse_nested(unwrap(Phld, Strip)),
            put_head(str, Str, Acc)
        )
    );
-do_preproc_tmpl([[Str] | Tokens], Acc) ->
+do_preproc_tmpl(Opts, [[Str] | Tokens], Acc) ->
    do_preproc_tmpl(
+        Opts,
        Tokens,
        put_head(str, Str, Acc)
    ).
@ -293,10 +305,10 @@ parse_nested(Attr) ->
        Nested -> {path, [{key, P} || P <- Nested]}
    end.

-unwrap(<<"${", Val/binary>>) ->
-    binary:part(Val, {0, byte_size(Val) - 1});
-unwrap(<<"\"${", Val/binary>>) ->
-    binary:part(Val, {0, byte_size(Val) - 2}).
+unwrap(<<"\"${", Val/binary>>, _StripDoubleQuote = true) ->
+    binary:part(Val, {0, byte_size(Val) - 2});
+unwrap(<<"${", Val/binary>>, _StripDoubleQuote) ->
+    binary:part(Val, {0, byte_size(Val) - 1}).

 quote_sql(Str) ->
    quote(Str, <<"\\\\'">>).
--- a/apps/emqx_plugin_libs/test/emqx_placeholder_SUITE.erl
+++ b/apps/emqx_plugin_libs/test/emqx_placeholder_SUITE.erl
@ -150,20 +150,25 @@ t_preproc_sql6(_) ->
        emqx_placeholder:proc_sql(ParamsTokens, Selected)
    ).

-t_preproc_sql7(_) ->
+t_preproc_sql_strip_double_quote(_) ->
    Selected = #{a => <<"a">>, b => <<"b">>},
+    Opts = #{replace_with => '$n', placeholders => [<<"${a}">>]},
+
+    %% no strip_double_quote option: "${key}" -> "value"
    {PrepareStatement, ParamsTokens} = emqx_placeholder:preproc_sql(
        <<"a:\"${a}\",b:\"${b}\"">>,
-        #{
-            replace_with => '$n',
-            placeholders => [<<"${a}">>]
-        }
+        Opts
    ),
-    ?assertEqual(<<"a:$1,b:\"${b}\"">>, PrepareStatement),
-    ?assertEqual(
-        [<<"a">>],
-        emqx_placeholder:proc_sql(ParamsTokens, Selected)
-    ).
+    ?assertEqual(<<"a:\"$1\",b:\"${b}\"">>, PrepareStatement),
+    ?assertEqual([<<"a">>], emqx_placeholder:proc_sql(ParamsTokens, Selected)),
+
+    %% strip_double_quote = true:  "${key}" -> value
+    {PrepareStatement1, ParamsTokens1} = emqx_placeholder:preproc_sql(
+        <<"a:\"${a}\",b:\"${b}\"">>,
+        Opts#{strip_double_quote => true}
+    ),
+    ?assertEqual(<<"a:$1,b:\"${b}\"">>, PrepareStatement1),
+    ?assertEqual([<<"a">>], emqx_placeholder:proc_sql(ParamsTokens1, Selected)).

 t_preproc_tmpl_deep(_) ->
    Selected = #{a => <<"1">>, b => 1, c => 1.0, d => #{d1 => <<"hi">>}},