fix(jwt auth): improve JWT handling

apps/emqx_authn/src/simple_authn/emqx_authn_jwt.erl

@@ -384,25 +384,47 @@ verify_claims(Claims, VerifyClaims0) ->
     Now = os:system_time(seconds),
     VerifyClaims =
         [
-            {<<"exp">>, fun(ExpireTime) ->
-                is_integer(ExpireTime) andalso Now < ExpireTime
-            end},
-            {<<"iat">>, fun(IssueAt) ->
-                is_integer(IssueAt) andalso IssueAt =< Now
-            end},
-            {<<"nbf">>, fun(NotBefore) ->
-                is_integer(NotBefore) andalso NotBefore =< Now
-            end}
+            {<<"exp">>, required,
+                with_int_value(fun(ExpireTime) ->
+                    Now < ExpireTime
+                end)},
+            {<<"iat">>, optional,
+                with_int_value(fun(IssueAt) ->
+                    IssueAt =< Now
+                end)},
+            {<<"nbf">>, optional,
+                with_int_value(fun(NotBefore) ->
+                    NotBefore =< Now
+                end)}
         ] ++ VerifyClaims0,
     do_verify_claims(Claims, VerifyClaims).
 
+with_int_value(Fun) ->
+    fun(Value) ->
+        case Value of
+            Int when is_integer(Int) -> Fun(Int);
+            Bin when is_binary(Bin) ->
+                case string:to_integer(Bin) of
+                    {Int, <<>>} -> Fun(Int);
+                    _ -> false
+                end;
+            Str when is_list(Str) ->
+                case string:to_integer(Str) of
+                    {Int, ""} -> Fun(Int);
+                    _ -> false
+                end
+        end
+    end.
+
 do_verify_claims(_Claims, []) ->
     ok;
-do_verify_claims(Claims, [{Name, Fun} | More]) when is_function(Fun) ->
-    case maps:take(Name, Claims) of
-        error ->
+do_verify_claims(Claims, [{Name, Required, Fun} | More]) when is_function(Fun) ->
+    case {Required, maps:take(Name, Claims)} of
+        {optional, error} ->
             do_verify_claims(Claims, More);
-        {Value, NClaims} ->
+        {required, error} ->
+            {error, {missing_claim, Name}};
+        {_, {Value, NClaims}} ->
             case Fun(Value) of
                 true ->
                     do_verify_claims(NClaims, More);

apps/emqx_authn/test/emqx_authn_jwt_SUITE.erl

@@ -64,7 +64,7 @@ t_jwt_authenticator_hmac_based(_) ->
     },
     {ok, State} = emqx_authn_jwt:create(?AUTHN_ID, Config),
 
-    Payload = #{<<"username">> => <<"myuser">>},
+    Payload = #{<<"username">> => <<"myuser">>, <<"exp">> => erlang:system_time(second) + 60},
     JWS = generate_jws('hmac-based', Payload, Secret),
     Credential = #{
         username => <<"myuser">>,
@@ -72,7 +72,11 @@ t_jwt_authenticator_hmac_based(_) ->
     },
     ?assertMatch({ok, #{is_superuser := false}}, emqx_authn_jwt:authenticate(Credential, State)),
 
-    Payload1 = #{<<"username">> => <<"myuser">>, <<"is_superuser">> => true},
+    Payload1 = #{
+        <<"username">> => <<"myuser">>,
+        <<"is_superuser">> => true,
+        <<"exp">> => erlang:system_time(second) + 60
+    },
     JWS1 = generate_jws('hmac-based', Payload1, Secret),
     Credential1 = #{
         username => <<"myuser">>,
@@ -129,7 +133,8 @@ t_jwt_authenticator_hmac_based(_) ->
     %% Issued At
     Payload5 = #{
         <<"username">> => <<"myuser">>,
-        <<"iat">> => erlang:system_time(second) - 60
+        <<"iat">> => erlang:system_time(second) - 60,
+        <<"exp">> => erlang:system_time(second) + 60
     },
     JWS5 = generate_jws('hmac-based', Payload5, Secret),
     Credential5 = Credential#{password => JWS5},
@@ -148,7 +153,8 @@ t_jwt_authenticator_hmac_based(_) ->
     %% Not Before
     Payload7 = #{
         <<"username">> => <<"myuser">>,
-        <<"nbf">> => erlang:system_time(second) - 60
+        <<"nbf">> => erlang:system_time(second) - 60,
+        <<"exp">> => erlang:system_time(second) + 60
     },
     JWS7 = generate_jws('hmac-based', Payload7, Secret),
     Credential7 = Credential6#{password => JWS7},
@@ -156,7 +162,8 @@ t_jwt_authenticator_hmac_based(_) ->
 
     Payload8 = #{
         <<"username">> => <<"myuser">>,
-        <<"nbf">> => erlang:system_time(second) + 60
+        <<"nbf">> => erlang:system_time(second) + 60,
+        <<"exp">> => erlang:system_time(second) + 60
     },
     JWS8 = generate_jws('hmac-based', Payload8, Secret),
     Credential8 = Credential#{password => JWS8},
@@ -179,7 +186,7 @@ t_jwt_authenticator_public_key(_) ->
     },
     {ok, State} = emqx_authn_jwt:create(?AUTHN_ID, Config),
 
-    Payload = #{<<"username">> => <<"myuser">>},
+    Payload = #{<<"username">> => <<"myuser">>, <<"exp">> => erlang:system_time(second) + 60},
     JWS = generate_jws('public-key', Payload, PrivateKey),
     Credential = #{
         username => <<"myuser">>,
@@ -261,8 +268,16 @@ t_jwks_renewal(_Config) ->
         verify_claims => [{<<"foo">>, <<"${username}">>}]
     },
 
-    Payload1 = #{<<"username">> => <<"myuser">>, <<"foo">> => <<"myuser">>},
-    Payload2 = #{<<"username">> => <<"myuser">>, <<"foo">> => <<"notmyuser">>},
+    Payload1 = #{
+        <<"username">> => <<"myuser">>,
+        <<"foo">> => <<"myuser">>,
+        <<"exp">> => erlang:system_time(second) + 10
+    },
+    Payload2 = #{
+        <<"username">> => <<"myuser">>,
+        <<"foo">> => <<"notmyuser">>,
+        <<"exp">> => erlang:system_time(second) + 10
+    },
     JWS1 = generate_jws('public-key', Payload1, PrivateKey),
     JWS2 = generate_jws('public-key', Payload2, PrivateKey),
     Credential1 = #{
@@ -301,7 +316,11 @@ t_jwt_authenticator_verify_claims(_) ->
     },
     {ok, State0} = emqx_authn_jwt:create(?AUTHN_ID, Config0),
 
-    Payload0 = #{<<"username">> => <<"myuser">>, <<"foo">> => <<"bar">>},
+    Payload0 = #{
+        <<"username">> => <<"myuser">>,
+        <<"foo">> => <<"bar">>,
+        <<"exp">> => erlang:system_time(second) + 10
+    },
     JWS0 = generate_jws('hmac-based', Payload0, Secret),
     Credential0 = #{
         username => <<"myuser">>,
@@ -314,7 +333,7 @@ t_jwt_authenticator_verify_claims(_) ->
     },
     {ok, State1} = emqx_authn_jwt:update(Config1, State0),
 
-    Payload1 = #{<<"username">> => <<"myuser">>},
+    Payload1 = #{<<"username">> => <<"myuser">>, <<"exp">> => erlang:system_time(second) + 10},
     JWS1 = generate_jws('hmac-based', Payload1, Secret),
     Credential1 = #{
         username => <<"myuser">>,
@@ -324,7 +343,11 @@ t_jwt_authenticator_verify_claims(_) ->
         {error, bad_username_or_password}, emqx_authn_jwt:authenticate(Credential1, State1)
     ),
 
-    Payload2 = #{<<"username">> => <<"myuser">>, <<"foo">> => <<"notmyuser">>},
+    Payload2 = #{
+        <<"username">> => <<"myuser">>,
+        <<"foo">> => <<"notmyuser">>,
+        <<"exp">> => erlang:system_time(second) + 10
+    },
     JWS2 = generate_jws('hmac-based', Payload2, Secret),
     Credential2 = #{
         username => <<"myuser">>,
@@ -334,13 +357,28 @@ t_jwt_authenticator_verify_claims(_) ->
         {error, bad_username_or_password}, emqx_authn_jwt:authenticate(Credential2, State1)
     ),
 
-    Payload3 = #{<<"username">> => <<"myuser">>, <<"foo">> => <<"myuser">>},
+    Payload3 = #{
+        <<"username">> => <<"myuser">>,
+        <<"foo">> => <<"myuser">>,
+        <<"exp">> => erlang:system_time(second) + 10
+    },
     JWS3 = generate_jws('hmac-based', Payload3, Secret),
     Credential3 = #{
         username => <<"myuser">>,
         password => JWS3
     },
-    ?assertMatch({ok, #{is_superuser := false}}, emqx_authn_jwt:authenticate(Credential3, State1)).
+    ?assertMatch({ok, #{is_superuser := false}}, emqx_authn_jwt:authenticate(Credential3, State1)),
+
+    %% No exp
+    Payload4 = #{<<"username">> => <<"myuser">>, <<"foo">> => <<"myuser">>},
+    JWS4 = generate_jws('hmac-based', Payload4, Secret),
+    Credential4 = #{
+        username => <<"myuser">>,
+        password => JWS4
+    },
+    ?assertEqual(
+        {error, bad_username_or_password}, emqx_authn_jwt:authenticate(Credential4, State1)
+    ).
 
 t_jwt_not_allow_empty_claim_name(_) ->
     Request = #{

apps/emqx_authz/src/emqx_authz_jwt.erl

@@ -69,20 +69,22 @@ verify(JWT) ->
     Now = erlang:system_time(second),
     VerifyClaims =
         [
-            {<<"exp">>, fun(ExpireTime) ->
-                is_integer(ExpireTime) andalso Now < ExpireTime
-            end},
-            {<<"iat">>, fun(IssueAt) ->
-                is_integer(IssueAt) andalso IssueAt =< Now
-            end},
-            {<<"nbf">>, fun(NotBefore) ->
-                is_integer(NotBefore) andalso NotBefore =< Now
-            end}
+            {<<"exp">>, required,
+                with_int_value(fun(ExpireTime) ->
+                    Now < ExpireTime
+                end)},
+            {<<"iat">>, optional,
+                with_int_value(fun(IssueAt) ->
+                    IssueAt =< Now
+                end)},
+            {<<"nbf">>, optional,
+                with_int_value(fun(NotBefore) ->
+                    NotBefore =< Now
+                end)}
         ],
     IsValid = lists:all(
-        fun({ClaimName, Validator}) ->
-            (not maps:is_key(ClaimName, JWT)) orelse
-                Validator(maps:get(ClaimName, JWT))
+        fun({ClaimName, Required, Validator}) ->
+            verify_claim(ClaimName, Required, JWT, Validator)
         end,
         VerifyClaims
     ),
@@ -91,6 +93,31 @@ verify(JWT) ->
         false -> error
     end.
 
+with_int_value(Fun) ->
+    fun(Value) ->
+        case Value of
+            Int when is_integer(Int) -> Fun(Int);
+            Bin when is_binary(Bin) ->
+                case string:to_integer(Bin) of
+                    {Int, <<>>} -> Fun(Int);
+                    _ -> false
+                end;
+            Str when is_list(Str) ->
+                case string:to_integer(Str) of
+                    {Int, ""} -> Fun(Int);
+                    _ -> false
+                end
+        end
+    end.
+
+verify_claim(ClaimName, Required, JWT, Validator) ->
+    case JWT of
+        #{ClaimName := Value} ->
+            Validator(Value);
+        #{} ->
+            Required =:= optional
+    end.
+
 do_authorize(Client, PubSub, Topic, AclRules) ->
     do_authorize(Client, PubSub, Topic, AclRules, ?JWT_RULE_NAMES).
 

apps/emqx_authz/test/emqx_authz_jwt_SUITE.erl

@@ -241,6 +241,33 @@ t_check_no_acl_claim(_Config) ->
 
     ok = emqtt:disconnect(C).
 
+t_check_str_exp(_Config) ->
+    Payload = #{
+        <<"username">> => <<"username">>,
+        <<"exp">> => integer_to_binary(erlang:system_time(second) + 10),
+        <<"acl">> => #{<<"sub">> => [<<"a/b">>]}
+    },
+
+    JWT = generate_jws(Payload),
+
+    {ok, C} = emqtt:start_link(
+        [
+            {clean_start, true},
+            {proto_ver, v5},
+            {clientid, <<"clientid">>},
+            {username, <<"username">>},
+            {password, JWT}
+        ]
+    ),
+    {ok, _} = emqtt:connect(C),
+
+    ?assertMatch(
+        {ok, #{}, [0]},
+        emqtt:subscribe(C, <<"a/b">>, 0)
+    ),
+
+    ok = emqtt:disconnect(C).
+
 t_check_expire(_Config) ->
     Payload = #{
         <<"username">> => <<"username">>,