Merge pull request #8337 from savonarola/jwt-fix-timer-4.3

Fix JWT handling

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
 

apps/emqx_auth_jwt/src/emqx_auth_jwt.app.src

@@ -1,6 +1,6 @@
 {application, emqx_auth_jwt,
  [{description, "EMQ X Authentication with JWT"},
-  {vsn, "4.3.3"}, % strict semver, bump manually!
+  {vsn, "4.3.4"}, % strict semver, bump manually!
   {modules, []},
   {registered, [emqx_auth_jwt_sup]},
   {applications, [kernel,stdlib,jose]},

apps/emqx_auth_jwt/src/emqx_auth_jwt.appup.src

@@ -1,9 +1,9 @@
 %% -*- mode: erlang -*-
 %% Unless you know what you are doing, DO NOT edit manually!!
 {VSN,
-  [{<<"4\\.3\\.[0-2]">>,
+  [{"4.3.3",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]},
-    [{restart_application,emqx_auth_jwt}]},
+   {<<"4\\.3\\.[0-2]">>,[{restart_application,emqx_auth_jwt}]},
    {<<".*">>,[]}],
-  [{<<"4\\.3\\.[0-2]">>,
+  [{"4.3.3",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]},
-    [{restart_application,emqx_auth_jwt}]},
+   {<<"4\\.3\\.[0-2]">>,[{restart_application,emqx_auth_jwt}]},
    {<<".*">>,[]}]}.

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} ->

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