chore(authz): increase coverage

apps/emqx/src/emqx_tls_lib.erl

@@ -34,6 +34,9 @@
 
 -include("logger.hrl").
 
+-define(IS_TRUE(Val), ((Val =:= true) or (Val =:= <<"true">>))).
+-define(IS_FALSE(Val), ((Val =:= false) or (Val =:= <<"false">>))).
+
 -define(SSL_FILE_OPT_NAMES, [<<"keyfile">>, <<"certfile">>, <<"cacertfile">>]).
 
 %% non-empty string
@@ -235,7 +238,8 @@ ensure_ssl_files(Dir, Opts) ->
     ensure_ssl_files(Dir, Opts, _DryRun = false).
 
 ensure_ssl_files(_Dir, undefined, _DryRun) -> {ok, undefined};
-ensure_ssl_files(_Dir, #{<<"enable">> := false} = Opts, _DryRun) -> {ok, Opts};
+ensure_ssl_files(_Dir, #{<<"enable">> := False} = Opts, _DryRun) when ?IS_FALSE(False) ->
+    {ok, Opts};
 ensure_ssl_files(Dir, Opts, DryRun) ->
     ensure_ssl_files(Dir, Opts, ?SSL_FILE_OPT_NAMES, DryRun).
 
@@ -352,9 +356,9 @@ is_valid_pem_file(Path) ->
 
 %% @doc This is to return SSL file content in management APIs.
 file_content_as_options(undefined) -> undefined;
-file_content_as_options(#{<<"enable">> := false} = SSL) ->
+file_content_as_options(#{<<"enable">> := False} = SSL) when ?IS_FALSE(False) ->
     {ok, maps:without(?SSL_FILE_OPT_NAMES, SSL)};
-file_content_as_options(#{<<"enable">> := true} = SSL) ->
+file_content_as_options(#{<<"enable">> := True} = SSL) when ?IS_TRUE(True) ->
     file_content_as_options(?SSL_FILE_OPT_NAMES, SSL).
 
 file_content_as_options([], SSL) ->

apps/emqx_authz/src/emqx_authz_api_sources.erl

@@ -445,7 +445,8 @@ read_certs(#{<<"ssl">> := SSL} = Source) ->
     end;
 read_certs(Source) -> Source.
 
-maybe_write_certs(#{<<"ssl">> := #{<<"enable">> := true} = SSL} = Source) ->
+maybe_write_certs(#{<<"ssl">> := #{<<"enable">> := True} = SSL} = Source)
+  when (True =:= true) or (True =:= <<"true">>) ->
     Type = maps:get(<<"type">>, Source),
     {ok, Return} = emqx_tls_lib:ensure_ssl_files(filename:join(["authz", Type]), SSL),
     maps:put(<<"ssl">>, Return, Source);

apps/emqx_authz/src/emqx_authz_http.erl

@@ -29,7 +29,6 @@
         , destroy/1
         , dry_run/1
         , authorize/4
-        , parse_url/1
         ]).
 
 -ifdef(TEST).
@@ -113,18 +112,6 @@ authorize(Client, PubSub, Topic,
             ignore
     end.
 
-parse_url(URL)
-  when URL =:= undefined ->
-    #{};
-parse_url(URL) ->
-    {ok, URIMap} = emqx_http_lib:uri_parse(URL),
-    case maps:get(query, URIMap, undefined) of
-        undefined ->
-            URIMap#{query => ""};
-        _ ->
-            URIMap
-    end.
-
 query_string(Body) ->
     query_string(maps:to_list(Body), []).
 
@@ -147,13 +134,13 @@ replvar_deep(Map, PubSub, Topic, Vars, VarEncode) when is_map(Map) ->
       lists:map(
         fun({Key, Value}) ->
                 {replvar(Key, PubSub, Topic, Vars, VarEncode),
-                 replvar(Value, PubSub, Topic, Vars, VarEncode)}
+                 replvar_deep(Value, PubSub, Topic, Vars, VarEncode)}
         end,
         maps:to_list(Map)));
 replvar_deep(List, PubSub, Topic, Vars, VarEncode) when is_list(List) ->
     lists:map(
       fun(Value) ->
-              replvar(Value, PubSub, Topic, Vars, VarEncode)
+              replvar_deep(Value, PubSub, Topic, Vars, VarEncode)
       end,
       List);
 replvar_deep(Number, _PubSub, _Topic, _Vars, _VarEncode) when is_number(Number) ->

apps/emqx_authz/src/emqx_authz_mongodb.erl

@@ -56,7 +56,14 @@ authorize(Client, PubSub, Topic,
               selector := Selector,
               annotations := #{id := ResourceID}
              }) ->
-    case emqx_resource:query(ResourceID, {find, Collection, replvar(Selector, Client), #{}}) of
+    RenderedSelector = replvar(Selector, Client),
+    Result = try
+                 emqx_resource:query(ResourceID, {find, Collection, RenderedSelector, #{}})
+             catch
+                 error:Error -> {error, Error}
+             end,
+
+    case Result of
         {error, Reason} ->
             ?SLOG(error, #{msg => "query_mongo_error",
                            reason => Reason,

apps/emqx_authz/src/emqx_authz_schema.erl

@@ -28,7 +28,6 @@
 -export([ namespace/0
         , roots/0
         , fields/1
-        , validations/0
         ]).
 
 -import(emqx_schema, [mk_duration/2]).
@@ -153,11 +152,6 @@ mongo_common_fields() ->
                  default => true}}
     ].
 
-validations() ->
-    [ {check_ssl_opts, fun check_ssl_opts/1}
-    , {check_headers, fun check_headers/1}
-    ].
-
 headers(type) -> map();
 headers(converter) ->
     fun(Headers) ->
@@ -196,28 +190,6 @@ transform_header_name(Headers) ->
                       maps:put(K, V, Acc)
               end, #{}, Headers).
 
-check_ssl_opts(Conf)
-  when Conf =:= #{} ->
-    true;
-check_ssl_opts(Conf) ->
-    case emqx_authz_http:parse_url(hocon_schema:get_value("config.base_url", Conf)) of
-        #{scheme := https} ->
-            case hocon_schema:get_value("config.ssl.enable", Conf) of
-                true -> ok;
-                false -> false
-            end;
-        #{scheme := http} ->
-            ok
-    end.
-
-check_headers(Conf)
-    when Conf =:= #{} ->
-    true;
-check_headers(Conf) ->
-    Method = to_bin(hocon_schema:get_value("config.method", Conf)),
-    Headers = hocon_schema:get_value("config.headers", Conf),
-    Method =:= <<"post">> orelse (not maps:is_key(<<"content-type">>, Headers)).
-
 union_array(Item) when is_list(Item) ->
     hoconsc:array(hoconsc:union(Item)).
 
@@ -253,9 +225,3 @@ to_list(A) when is_atom(A) ->
 to_list(B) when is_binary(B) ->
     binary_to_list(B).
 
-to_bin(A) when is_atom(A) ->
-    atom_to_binary(A);
-to_bin(B) when is_binary(B) ->
-    B;
-to_bin(L) when is_list(L) ->
-    list_to_binary(L).

apps/emqx_authz/test/emqx_authz_SUITE.erl

@@ -164,6 +164,16 @@ t_update_source(_) ->
 
     {ok, _} = emqx_authz:update(?CMD_REPLACE, []).
 
+t_delete_source(_) ->
+    {ok, _} = emqx_authz:update(?CMD_REPLACE, [?SOURCE1]),
+
+    ?assertMatch([ #{type := http,  enable := true}
+                 ], emqx_conf:get([authorization, sources], [])),
+
+    {ok, _} = emqx_authz:update({?CMD_DELETE, http},  #{}),
+
+    ?assertMatch([], emqx_conf:get([authorization, sources], [])).
+
 t_move_source(_) ->
     {ok, _} = emqx_authz:update(?CMD_REPLACE,
                                 [?SOURCE1, ?SOURCE2, ?SOURCE3,

apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl

@@ -29,7 +29,9 @@
 
 -define(SOURCE1, #{<<"type">> => <<"http">>,
                    <<"enable">> => true,
-                   <<"url">> => <<"https://fake.com:443/">>,
+                   <<"base_url">> => <<"https://fake.com:443/">>,
+                   <<"path">> => <<"foo">>,
+                   <<"query">> => <<"a=b">>,
                    <<"headers">> => #{},
                    <<"method">> => <<"get">>,
                    <<"request_timeout">> => <<"5s">>
@@ -37,9 +39,7 @@
 -define(SOURCE2, #{<<"type">> => <<"mongodb">>,
                    <<"enable">> => true,
                    <<"mongo_type">> => <<"sharded">>,
-                   <<"servers">> => [<<"127.0.0.1:27017">>,
+                   <<"servers">> => <<"127.0.0.1:27017,192.168.0.1:27017">>,
-                                     <<"192.168.0.1:27017">>
-                                    ],
                    <<"pool_size">> => 1,
                    <<"database">> => <<"mqtt">>,
                    <<"ssl">> => #{<<"enable">> => false},
@@ -87,9 +87,11 @@
   "\n{allow,{ipaddr,\"127.0.0.1\"},all,[\"$SYS/#\",\"#\"]}.">>
                   }).
 
+-define(MATCH_RSA_KEY, <<"-----BEGIN RSA PRIVATE KEY", _/binary>>).
+-define(MATCH_CERT, <<"-----BEGIN CERTIFICATE", _/binary>>).
+
 all() ->
-    []. %% Todo: Waiting for @terry-xiaoyu to fix the config_not_found error
+    emqx_common_test_helpers:all(?MODULE).
-    % emqx_common_test_helpers:all(?MODULE).
 
 groups() ->
     [].
@@ -98,7 +100,7 @@ init_per_suite(Config) ->
     meck:new(emqx_resource, [non_strict, passthrough, no_history, no_link]),
     meck:expect(emqx_resource, create, fun(_, _, _) -> {ok, meck_data} end),
     meck:expect(emqx_resource, create_dry_run,
-                fun(emqx_connector_mysql, _) -> {ok, meck_data};
+                fun(emqx_connector_mysql, _) -> ok;
                    (T, C) -> meck:passthrough([T, C])
                 end),
     meck:expect(emqx_resource, update, fun(_, _, _, _) -> {ok, meck_data} end),
@@ -186,26 +188,65 @@ t_api(_) ->
     {ok, 200, Result3} = request(get, uri(["authorization", "sources", "http"]), []),
     ?assertMatch(#{<<"type">> := <<"http">>, <<"enable">> := false}, jsx:decode(Result3)),
 
+    Keyfile = emqx_common_test_helpers:app_path(
+                emqx,
+                filename:join(["etc", "certs", "key.pem"])),
+    Certfile = emqx_common_test_helpers:app_path(
+                 emqx,
+                 filename:join(["etc", "certs", "cert.pem"])),
+    Cacertfile = emqx_common_test_helpers:app_path(
+                   emqx,
+                   filename:join(["etc", "certs", "cacert.pem"])),
+
     {ok, 204, _} = request(put, uri(["authorization", "sources", "mongodb"]),
-                           ?SOURCE2#{<<"ssl">> := #{
+                           ?SOURCE2#{<<"ssl">> => #{
-                                         <<"enable">> => true,
+                                         <<"enable">> => <<"true">>,
-                                         <<"cacertfile">> => <<"fake cacert file">>,
+                                         <<"cacertfile">> => Cacertfile,
-                                         <<"certfile">> => <<"fake cert file">>,
+                                         <<"certfile">> => Certfile,
-                                         <<"keyfile">> => <<"fake key file">>,
+                                         <<"keyfile">> => Keyfile,
-                                         <<"verify">> => false
+                                         <<"verify">> => <<"verify_none">>
                                         }}),
     {ok, 200, Result4} = request(get, uri(["authorization", "sources", "mongodb"]), []),
     ?assertMatch(#{<<"type">> := <<"mongodb">>,
-                   <<"ssl">> := #{<<"enable">> := true,
+                   <<"ssl">> := #{<<"enable">> := <<"true">>,
-                                  <<"cacertfile">> := <<"fake cacert file">>,
+                                  <<"cacertfile">> := ?MATCH_CERT,
-                                  <<"certfile">> := <<"fake cert file">>,
+                                  <<"certfile">> := ?MATCH_CERT,
-                                  <<"keyfile">> := <<"fake key file">>,
+                                  <<"keyfile">> := ?MATCH_RSA_KEY,
-                                  <<"verify">> := false
+                                  <<"verify">> := <<"verify_none">>
                                  }
                   }, jsx:decode(Result4)),
-    ?assert(filelib:is_file(filename:join([data_dir(), "certs", "cacert-fake.pem"]))),
+
-    ?assert(filelib:is_file(filename:join([data_dir(), "certs", "cert-fake.pem"]))),
+    {ok, Cacert} = file:read_file(Cacertfile),
-    ?assert(filelib:is_file(filename:join([data_dir(), "certs", "key-fake.pem"]))),
+    {ok, Cert} = file:read_file(Certfile),
+    {ok, Key} = file:read_file(Keyfile),
+
+    {ok, 204, _} = request(put, uri(["authorization", "sources", "mongodb"]),
+                           ?SOURCE2#{<<"ssl">> => #{
+                                         <<"enable">> => <<"true">>,
+                                         <<"cacertfile">> => Cacert,
+                                         <<"certfile">> => Cert,
+                                         <<"keyfile">> => Key,
+                                         <<"verify">> => <<"verify_none">>
+                                        }}),
+    {ok, 200, Result5} = request(get, uri(["authorization", "sources", "mongodb"]), []),
+    ?assertMatch(#{<<"type">> := <<"mongodb">>,
+                   <<"ssl">> := #{<<"enable">> := <<"true">>,
+                                  <<"cacertfile">> := ?MATCH_CERT,
+                                  <<"certfile">> := ?MATCH_CERT,
+                                  <<"keyfile">> := ?MATCH_RSA_KEY,
+                                  <<"verify">> := <<"verify_none">>
+                                 }
+                  }, jsx:decode(Result5)),
+
+
+    #{ssl := #{cacertfile := SavedCacertfile,
+               certfile := SavedCertfile,
+               keyfile := SavedKeyfile
+              }} = emqx_authz:lookup(mongodb),
+
+    ?assert(filelib:is_file(SavedCacertfile)),
+    ?assert(filelib:is_file(SavedCertfile)),
+    ?assert(filelib:is_file(SavedKeyfile)),
 
     {ok, 204, _} = request(
                      put,
@@ -229,8 +270,8 @@ t_api(_) ->
                          uri(["authorization", "sources", binary_to_list(Type)]),
                          [])
       end, Sources),
-    {ok, 200, Result5} = request(get, uri(["authorization", "sources"]), []),
+    {ok, 200, Result6} = request(get, uri(["authorization", "sources"]), []),
-    ?assertEqual([], get_sources(Result5)),
+    ?assertEqual([], get_sources(Result6)),
     ?assertEqual([], emqx:get_config([authorization, sources])),
     ok.
 

apps/emqx_authz/test/emqx_authz_http_SUITE.erl

@@ -233,8 +233,8 @@ t_json_body(_Config) ->
                    ?assertMatch(
                       #{<<"username">> := <<"user name">>,
                         <<"CLIENT_client id">> := <<"client id">>,
-                        <<"peerhost">> := <<"127.0.0.1">>,
+                        <<"peerhost">> := [<<"127.0.0.1">>, 1],
-                        <<"proto_name">> := <<"MQTT">>,
+                        <<"proto_name">> := #{<<"proto">> := <<"MQTT">>},
                         <<"mountpoint">> := <<"MOUNTPOINT">>,
                         <<"topic">> := <<"t">>,
                         <<"action">> := <<"publish">>},
@@ -253,8 +253,8 @@ t_json_body(_Config) ->
                              "action/${action}">>,
              <<"body">> => #{<<"username">> => <<"${username}">>,
                              <<"CLIENT_${clientid}">> => <<"${clientid}">>,
-                             <<"peerhost">> => <<"${peerhost}">>,
+                             <<"peerhost">> => [<<"${peerhost}">>, 1],
-                             <<"proto_name">> => <<"${proto_name}">>,
+                             <<"proto_name">> => #{<<"proto">> => <<"${proto_name}">>},
                              <<"mountpoint">> => <<"${mountpoint}">>,
                              <<"topic">> => <<"${topic}">>,
                              <<"action">> => <<"${action}">>}

apps/emqx_authz/test/emqx_authz_mongodb_SUITE.erl

@@ -167,6 +167,24 @@ t_lookups(_Config) ->
            [{allow, subscribe, <<"a">>},
             {deny, subscribe, <<"b">>}]).
 
+t_bad_selector(_Config) ->
+    ClientInfo = #{clientid => <<"clientid">>,
+                   cn => <<"cn">>,
+                   dn => <<"dn">>,
+                   username => <<"username">>,
+                   peerhost => {127,0,0,1},
+                   zone => default,
+                   listener => {tcp, default}
+                  },
+
+    ok = setup_config(
+           #{<<"selector">> => #{<<"$in">> => #{<<"a">> => 1}}}),
+
+    ok = emqx_authz_test_lib:test_samples(
+           ClientInfo,
+           [{deny, subscribe, <<"a">>},
+            {deny, subscribe, <<"b">>}]).
+
 %%------------------------------------------------------------------------------
 %% Helpers
 %%------------------------------------------------------------------------------