Merge pull request #8341 from emqx/copy-of_main-v4.3

merge main-v4.3 into main-v4.4
2022-06-29 15:40:25 +08:00 · 2022-06-29 15:40:25 +08:00 · 56775beede
parent a365c40765 c6573c708f
commit 56775beede
4 changed files with 73 additions and 8 deletions
--- a/CHANGES-4.3.md
+++ b/CHANGES-4.3.md
@ -32,6 +32,8 @@ File format:
  subscribed to. [#8288]
 - Ensuring that exhook dispatches the client events are sequential. [#8311]
 - Ensure start dashboard ok event if default_username is missing.
+- Fix key update from JWKS server by JWT auth. [#8337]
+- Better errors for JWT claim validations. [#8337]

 ## v4.3.15

--- a/apps/emqx_auth_jwt/src/emqx_auth_jwt.appup.src
+++ b/apps/emqx_auth_jwt/src/emqx_auth_jwt.appup.src
@ -1,9 +1,11 @@
 %% -*- mode: erlang -*-
 %% Unless you know what you are doing, DO NOT edit manually!!
 {VSN,
-  [{"4.4.2",[{load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}]},
+  [{"4.4.2",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]},
+             {load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}]},
   {<<"4\\.4\\.[0-1]">>,[{restart_application,emqx_auth_jwt}]},
   {<<".*">>,[]}],
-  [{"4.4.2",[{load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}]},
+  [{"4.4.2",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]},
+             {load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}]},
   {<<"4\\.4\\.[0-1]">>,[{restart_application,emqx_auth_jwt}]},
   {<<".*">>,[]}]}.
--- a/apps/emqx_auth_jwt/src/emqx_auth_jwt_svr.erl
+++ b/apps/emqx_auth_jwt/src/emqx_auth_jwt_svr.erl
@ -20,6 +20,7 @@

 -include_lib("emqx/include/logger.hrl").
 -include_lib("jose/include/jose_jwk.hrl").
+-include_lib("snabbkaffe/include/snabbkaffe.hrl").

 -logger_header("[JWT-SVR]").

@ -114,7 +115,8 @@ handle_cast(_Msg, State) ->

 handle_info({timeout, _TRef, refresh}, State = #state{addr = Addr}) ->
    NState = try
-                 true = ets:insert(?TAB, {remote, request_jwks(Addr)})
+                 true = ets:insert(?TAB, {remote, request_jwks(Addr)}),
+                 State
             catch _:_ ->
                 State
             end,
@ -147,7 +149,9 @@ request_jwks(Addr) ->
        {ok, {_Code, _Headers, Body}} ->
            try
                JwkSet = jose_jwk:from(emqx_json:decode(Body, [return_maps])),
-                {_, Jwks} = JwkSet#jose_jwk.keys, Jwks
+                {_, Jwks} = JwkSet#jose_jwk.keys,
+                ?tp(debug, emqx_auth_jwt_svr_jwks_updated, #{jwks => Jwks, pid => self()}),
+                Jwks
            catch _:_ ->
                ?LOG(error, "Invalid jwks server response: ~p~n", [Body]),
                error(badarg)
@ -194,7 +198,11 @@ do_verify(JwsCompacted, [Jwk|More]) ->
            case check_claims(Claims) of
                {false, <<"exp">>} ->
                    {error, {invalid_signature, expired}};
-                NClaims ->
+                {false, <<"iat">>} ->
+                    {error, {invalid_signature, issued_in_future}};
+                {false, <<"nbf">>} ->
+                    {error, {invalid_signature, not_valid_yet}};
+                {true, NClaims} ->
                    {ok, NClaims}
            end;
        {false, _, _} ->
@ -230,7 +238,7 @@ with_int_value(Fun) ->
    end.

 do_check_claim([], Claims) ->
-    Claims;
+    {true, Claims};
 do_check_claim([{K, F}|More], Claims) ->
    case Claims of
        #{K := V} ->
--- a/apps/emqx_auth_jwt/test/emqx_auth_jwt_SUITE.erl
+++ b/apps/emqx_auth_jwt/test/emqx_auth_jwt_SUITE.erl
@ -22,6 +22,7 @@
 -include_lib("emqx/include/emqx_mqtt.hrl").
 -include_lib("eunit/include/eunit.hrl").
 -include_lib("common_test/include/ct.hrl").
+-include_lib("snabbkaffe/include/snabbkaffe.hrl").

 all() -> emqx_ct:all(?MODULE).

@ -73,7 +74,7 @@ t_check_auth(_Config) ->
    Plain = #{clientid => <<"client1">>, username => <<"plain">>, zone => external},
    Jwt = sign([{clientid, <<"client1">>},
                {username, <<"plain">>},
-                {exp, os:system_time(seconds) + 3}], <<"HS256">>, <<"emqxsecret">>),
+                {exp, os:system_time(seconds) + 2}], <<"HS256">>, <<"emqxsecret">>),
    ct:pal("Jwt: ~p~n", [Jwt]),

    Result0 = emqx_access_control:authenticate(Plain#{password => Jwt}),
@ -82,7 +83,7 @@ t_check_auth(_Config) ->

    ct:sleep(3100),
    Result1 = emqx_access_control:authenticate(Plain#{password => Jwt}),
-    ct:pal("Auth result after 1000ms: ~p~n", [Result1]),
+    ct:pal("Auth result after 3100ms: ~p~n", [Result1]),
    ?assertMatch({error, _}, Result1),

    Jwt_Error = sign([{client_id, <<"client1">>},
@ -93,6 +94,32 @@ t_check_auth(_Config) ->
    ?assertEqual({error, invalid_signature}, Result2),
    ?assertMatch({error, _}, emqx_access_control:authenticate(Plain#{password => <<"asd">>})).

+t_check_nbf(init, _Config) ->
+    application:unset_env(emqx_auth_jwt, verify_claims).
+t_check_nbf(_Config) ->
+    Plain = #{clientid => <<"client1">>, username => <<"plain">>, zone => external},
+    Jwt = sign([{clientid, <<"client1">>},
+                {username, <<"plain">>},
+                {nbf, os:system_time(seconds) + 3}], <<"HS256">>, <<"emqxsecret">>),
+    ct:pal("Jwt: ~p~n", [Jwt]),
+
+    Result0 = emqx_access_control:authenticate(Plain#{password => Jwt}),
+    ct:pal("Auth result: ~p~n", [Result0]),
+    ?assertEqual({error, {invalid_signature, not_valid_yet}}, Result0).
+
+t_check_iat(init, _Config) ->
+    application:unset_env(emqx_auth_jwt, verify_claims).
+t_check_iat(_Config) ->
+    Plain = #{clientid => <<"client1">>, username => <<"plain">>, zone => external},
+    Jwt = sign([{clientid, <<"client1">>},
+                {username, <<"plain">>},
+                {iat, os:system_time(seconds) + 3}], <<"HS256">>, <<"emqxsecret">>),
+    ct:pal("Jwt: ~p~n", [Jwt]),
+
+    Result0 = emqx_access_control:authenticate(Plain#{password => Jwt}),
+    ct:pal("Auth result: ~p~n", [Result0]),
+    ?assertEqual({error, {invalid_signature, issued_in_future}}, Result0).
+
 t_check_auth_invalid_exp(init, _Config) ->
    application:unset_env(emqx_auth_jwt, verify_claims).
 t_check_auth_invalid_exp(_Config) ->
@ -201,6 +228,32 @@ t_check_claims_kid_in_header(_Config) ->
    ct:pal("Auth result: ~p~n", [Result0]),
    ?assertMatch({ok, #{auth_result := success, jwt_claims := _}}, Result0).

+t_keys_update(init, _Config) ->
+    ok = meck:new(httpc, [passthrough, no_history]),
+    ok = meck:expect(
+           httpc,
+           request,
+           fun(get, _, _, _) ->
+                   {ok,
+                    {200,
+                     [],
+                     jiffy:encode(#{<<"keys">> => []})}}
+           end),
+
+    application:set_env(emqx_auth_jwt, verify_claims, []),
+    application:set_env(emqx_auth_jwt, refresh_interval, 100),
+    application:set_env(emqx_auth_jwt, jwks, "http://localhost:4001/keys.json").
+t_keys_update(_Config) ->
+    ?check_trace(
+       snabbkaffe:block_until(
+         ?match_n_events(2, #{?snk_kind := emqx_auth_jwt_svr_jwks_updated}),
+         _Timeout    = infinity,
+         _BackInTIme = 0),
+       fun(_, Trace) ->
+               ?assertMatch([#{pid := Pid}, #{pid := Pid} |  _],
+                            ?of_kind(emqx_auth_jwt_svr_jwks_updated, Trace))
+       end).
+
 t_check_jwt_acl(init, _Config) ->
    application:set_env(emqx_auth_jwt, verify_claims, [{sub, <<"value">>}]).
 t_check_jwt_acl(_Config) ->