Merge pull request #6587 from savonarola/improve-authz-coverage-more

chore(authz): increase coverage
This commit is contained in:
Ilya Averyanov 2021-12-31 18:58:21 +03:00 committed by GitHub
commit 384493369b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 118 additions and 84 deletions

View File

@ -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) ->

View File

@ -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);

View File

@ -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) ->

View File

@ -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,
@ -65,9 +72,9 @@ authorize(Client, PubSub, Topic,
[] -> nomatch;
Rows ->
Rules = [ emqx_authz_rule:compile({Permission, all, Action, Topics})
|| #{<<"topics">> := Topics,
<<"permission">> := Permission,
<<"action">> := Action} <- Rows],
|| #{<<"topics">> := Topics,
<<"permission">> := Permission,
<<"action">> := Action} <- Rows],
do_authorize(Client, PubSub, Topic, Rules)
end.

View File

@ -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).

View File

@ -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,

View File

@ -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">>,
<<"192.168.0.1:27017">>
],
<<"servers">> => <<"127.0.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">> := #{
<<"enable">> => true,
<<"cacertfile">> => <<"fake cacert file">>,
<<"certfile">> => <<"fake cert file">>,
<<"keyfile">> => <<"fake key file">>,
<<"verify">> => false
?SOURCE2#{<<"ssl">> => #{
<<"enable">> => <<"true">>,
<<"cacertfile">> => Cacertfile,
<<"certfile">> => Certfile,
<<"keyfile">> => Keyfile,
<<"verify">> => <<"verify_none">>
}}),
{ok, 200, Result4} = request(get, uri(["authorization", "sources", "mongodb"]), []),
?assertMatch(#{<<"type">> := <<"mongodb">>,
<<"ssl">> := #{<<"enable">> := true,
<<"cacertfile">> := <<"fake cacert file">>,
<<"certfile">> := <<"fake cert file">>,
<<"keyfile">> := <<"fake key file">>,
<<"verify">> := false
<<"ssl">> := #{<<"enable">> := <<"true">>,
<<"cacertfile">> := ?MATCH_CERT,
<<"certfile">> := ?MATCH_CERT,
<<"keyfile">> := ?MATCH_RSA_KEY,
<<"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"]))),
?assert(filelib:is_file(filename:join([data_dir(), "certs", "key-fake.pem"]))),
{ok, Cacert} = file:read_file(Cacertfile),
{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"]), []),
?assertEqual([], get_sources(Result5)),
{ok, 200, Result6} = request(get, uri(["authorization", "sources"]), []),
?assertEqual([], get_sources(Result6)),
?assertEqual([], emqx:get_config([authorization, sources])),
ok.

View File

@ -233,8 +233,8 @@ t_json_body(_Config) ->
?assertMatch(
#{<<"username">> := <<"user name">>,
<<"CLIENT_client id">> := <<"client id">>,
<<"peerhost">> := <<"127.0.0.1">>,
<<"proto_name">> := <<"MQTT">>,
<<"peerhost">> := [<<"127.0.0.1">>, 1],
<<"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}">>,
<<"proto_name">> => <<"${proto_name}">>,
<<"peerhost">> => [<<"${peerhost}">>, 1],
<<"proto_name">> => #{<<"proto">> => <<"${proto_name}">>},
<<"mountpoint">> => <<"${mountpoint}">>,
<<"topic">> => <<"${topic}">>,
<<"action">> => <<"${action}">>}

View File

@ -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
%%------------------------------------------------------------------------------