From d7c21020f6b8f46d01d633e661d191fcb4ec3cf3 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Thu, 30 Jun 2022 11:26:56 +0800 Subject: [PATCH 1/5] feat(authn-redis): needs to compatible with 4.x auth data --- apps/emqx_authn/src/emqx_authn_utils.erl | 19 ++++++++++------ .../test/emqx_authn_redis_SUITE.erl | 22 +++++++++++++++++++ 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/apps/emqx_authn/src/emqx_authn_utils.erl b/apps/emqx_authn/src/emqx_authn_utils.erl index 1bd65c7c8..70d6a0b5f 100644 --- a/apps/emqx_authn/src/emqx_authn_utils.erl +++ b/apps/emqx_authn/src/emqx_authn_utils.erl @@ -72,13 +72,18 @@ start_resource_if_enabled(Result, _ResourceId, _Config) -> check_password_from_selected_map(_Algorithm, _Selected, undefined) -> {error, bad_username_or_password}; -check_password_from_selected_map( - Algorithm, #{<<"password_hash">> := Hash} = Selected, Password -) -> - Salt = maps:get(<<"salt">>, Selected, <<>>), - case emqx_authn_password_hashing:check_password(Algorithm, Salt, Hash, Password) of - true -> ok; - false -> {error, bad_username_or_password} +check_password_from_selected_map(Algorithm, Selected, Password) -> + Hash = maps:get(<<"password_hash">>, Selected, + maps:get(<<"password">>>, Selected, undefined)), + case Hash of + undefined -> {error, bad_username_or_password}; + _ -> + Salt = maps:get(<<"salt">>, Selected, <<>>), + case emqx_authn_password_hashing:check_password( + Algorithm, Salt, Hash, Password) of + true -> ok; + false -> {error, bad_username_or_password} + end end. parse_deep(Template) -> diff --git a/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl b/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl index 77e35bf3b..3423879f6 100644 --- a/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl @@ -453,6 +453,28 @@ user_seeds() -> <<"password_hash_algorithm">> => #{<<"name">> => <<"bcrypt">>} }, result => {error, bad_username_or_password} + }, + + #{ + data => #{ + password => + <<"a3c7f6b085c3e5897ffb9b86f18a9d905063f8550a74444b5892e193c1b50428">>, + is_superuser => <<"1">> + }, + credentials => #{ + clientid => <<"sha256_no_salt">>, + password => <<"sha256_no_salt">> + }, + key => <<"mqtt_user:sha256_no_salt">>, + config_params => #{ + %% Needs to be compatible with emqx 4.x auth data + <<"cmd">> => <<"HMGET mqtt_user:${clientid} password is_superuser">>, + <<"password_hash_algorithm">> => #{ + <<"name">> => <<"sha256">>, + <<"salt_position">> => <<"disable">> + } + }, + result => {ok, #{is_superuser => true}} } ]. From 8f696b6f8cd968b585c02229a492293ae0e0abc9 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Thu, 30 Jun 2022 12:50:56 +0800 Subject: [PATCH 2/5] fix(authn): correct peerhost placeholder rendering --- apps/emqx_authn/src/emqx_authn_utils.erl | 21 ++++++++++---- .../emqx_authn/test/emqx_authn_http_SUITE.erl | 28 +++++++++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/apps/emqx_authn/src/emqx_authn_utils.erl b/apps/emqx_authn/src/emqx_authn_utils.erl index 70d6a0b5f..a41a27d2a 100644 --- a/apps/emqx_authn/src/emqx_authn_utils.erl +++ b/apps/emqx_authn/src/emqx_authn_utils.erl @@ -73,14 +73,21 @@ start_resource_if_enabled(Result, _ResourceId, _Config) -> check_password_from_selected_map(_Algorithm, _Selected, undefined) -> {error, bad_username_or_password}; check_password_from_selected_map(Algorithm, Selected, Password) -> - Hash = maps:get(<<"password_hash">>, Selected, - maps:get(<<"password">>>, Selected, undefined)), + Hash = maps:get( + <<"password_hash">>, + Selected, + maps:get(<<"password">>, Selected, undefined) + ), case Hash of - undefined -> {error, bad_username_or_password}; + undefined -> + {error, bad_username_or_password}; _ -> Salt = maps:get(<<"salt">>, Selected, <<>>), - case emqx_authn_password_hashing:check_password( - Algorithm, Salt, Hash, Password) of + case + emqx_authn_password_hashing:check_password( + Algorithm, Salt, Hash, Password + ) + of true -> ok; false -> {error, bad_username_or_password} end @@ -165,10 +172,14 @@ make_resource_id(Name) -> 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) -> error({cannot_get_variable, Name}); +handle_sql_var({var, <<"peerhost">>}, PeerHost) -> + emqx_placeholder:bin(inet:ntoa(PeerHost)); handle_sql_var(_, Value) -> emqx_placeholder:sql_data(Value). diff --git a/apps/emqx_authn/test/emqx_authn_http_SUITE.erl b/apps/emqx_authn/test/emqx_authn_http_SUITE.erl index fc79970af..4d17c7191 100644 --- a/apps/emqx_authn/test/emqx_authn_http_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_http_SUITE.erl @@ -29,8 +29,10 @@ -define(HTTP_PORT, 32333). -define(HTTP_PATH, "/auth"). -define(CREDENTIALS, #{ + clientid => <<"clienta">>, username => <<"plain">>, password => <<"plain">>, + peerhost => {127, 0, 0, 1}, listener => 'tcp:default', protocol => mqtt }). @@ -390,6 +392,32 @@ samples() -> result => {ok, #{is_superuser => false}} }, + %% simple post request for placeholders, application/json + #{ + handler => fun(Req0, State) -> + {ok, RawBody, Req1} = cowboy_req:read_body(Req0), + #{ + <<"username">> := <<"plain">>, + <<"password">> := <<"plain">>, + <<"clientid">> := <<"clienta">>, + <<"peerhost">> := <<"127.0.0.1">> + } = jiffy:decode(RawBody, [return_maps]), + Req = cowboy_req:reply(200, Req1), + {ok, Req, State} + end, + config_params => #{ + <<"method">> => <<"post">>, + <<"headers">> => #{<<"content-type">> => <<"application/json">>}, + <<"body">> => #{ + <<"clientid">> => ?PH_CLIENTID, + <<"username">> => ?PH_USERNAME, + <<"password">> => ?PH_PASSWORD, + <<"peerhost">> => ?PH_PEERHOST + } + }, + result => {ok, #{is_superuser => false}} + }, + %% custom headers #{ handler => fun(Req0, State) -> From 7de23f5863367a3599d08ed218e6bfc4e8db5dda Mon Sep 17 00:00:00 2001 From: JianBo He Date: Thu, 30 Jun 2022 13:26:58 +0800 Subject: [PATCH 3/5] chore(authn-redis): checking password field --- apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl b/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl index 6c0839a1b..d0958546d 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl +++ b/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl @@ -210,8 +210,8 @@ parse_cmd(Cmd) -> end. check_fields(Fields) -> - HasPassHash = lists:member("password_hash", Fields), - KnownFields = ["password_hash", "salt", "is_superuser"], + HasPassHash = lists:member("password_hash", Fields) orelse lists:member("password", Fields), + KnownFields = ["password_hash", "password", "salt", "is_superuser"], UnknownFields = [F || F <- Fields, not lists:member(F, KnownFields)], case {HasPassHash, UnknownFields} of From 9acfd0ff9ea726ecf2cd6a6c16fb71c8fe669810 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Thu, 30 Jun 2022 15:23:09 +0800 Subject: [PATCH 4/5] chore: update emqx_authn.appup.src --- apps/emqx_authn/src/emqx_authn.appup.src | 10 ++++-- apps/emqx_authn/src/emqx_authn_utils.erl | 2 +- .../src/simple_authn/emqx_authn_redis.erl | 31 ++++++------------- 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/apps/emqx_authn/src/emqx_authn.appup.src b/apps/emqx_authn/src/emqx_authn.appup.src index eccbcd60b..f165b2f77 100644 --- a/apps/emqx_authn/src/emqx_authn.appup.src +++ b/apps/emqx_authn/src/emqx_authn.appup.src @@ -1,5 +1,11 @@ %% -*- mode: erlang -*- %% Unless you know what you are doing, DO NOT edit manually!! {VSN, - [{"0.1.0",[{load_module,emqx_authn_utils,brutal_purge,soft_purge,[]}]}], - [{"0.1.0",[{load_module,emqx_authn_utils,brutal_purge,soft_purge,[]}]}]}. + [{"0.1.0",[ + {load_module,emqx_authn_utils,brutal_purge,soft_purge,[]}, + {load_module,emqx_authn_redis,brutal_purge,soft_purge,[]}] + }], + [{"0.1.0",[ + {load_module,emqx_authn_utils,brutal_purge,soft_purge,[]}, + {load_module,emqx_authn_redis,brutal_purge,soft_purge,[]}] + }]}. diff --git a/apps/emqx_authn/src/emqx_authn_utils.erl b/apps/emqx_authn/src/emqx_authn_utils.erl index a41a27d2a..6e8976cee 100644 --- a/apps/emqx_authn/src/emqx_authn_utils.erl +++ b/apps/emqx_authn/src/emqx_authn_utils.erl @@ -80,7 +80,7 @@ check_password_from_selected_map(Algorithm, Selected, Password) -> ), case Hash of undefined -> - {error, bad_username_or_password}; + {error, not_authorized}; _ -> Salt = maps:get(<<"salt">>, Selected, <<>>), case diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl b/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl index d0958546d..d2003f26a 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl +++ b/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl @@ -138,27 +138,16 @@ authenticate( {ok, []} -> ignore; {ok, Values} -> - case merge(Fields, Values) of - #{<<"password_hash">> := _} = Selected -> - case - emqx_authn_utils:check_password_from_selected_map( - Algorithm, Selected, Password - ) - of - ok -> - {ok, emqx_authn_utils:is_superuser(Selected)}; - {error, Reason} -> - {error, Reason} - end; - _ -> - ?SLOG(error, #{ - msg => "cannot_find_password_hash_field", - cmd => Command, - keys => NKey, - fields => Fields, - resource => ResourceId - }), - ignore + Selected = merge(Fields, Values), + case + emqx_authn_utils:check_password_from_selected_map( + Algorithm, Selected, Password + ) + of + ok -> + {ok, emqx_authn_utils:is_superuser(Selected)}; + {error, Reason} -> + {error, Reason} end; {error, Reason} -> ?SLOG(error, #{ From 3696c3d33355ee5b7f0276577ea1b1c31615c1af Mon Sep 17 00:00:00 2001 From: JianBo He Date: Fri, 1 Jul 2022 09:51:37 +0800 Subject: [PATCH 5/5] chore: print coverall fails log --- .github/workflows/run_test_cases.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/run_test_cases.yaml b/.github/workflows/run_test_cases.yaml index d38acaf31..8a87bc05f 100644 --- a/.github/workflows/run_test_cases.yaml +++ b/.github/workflows/run_test_cases.yaml @@ -204,6 +204,9 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: make coveralls + - name: get coveralls logs + if: failure() + run: cat rebar3.crashdump # do this in a separate job upload_coverdata: