Merge pull request #12418 from savonarola/0129-fix-auth-claim-schema

feat(jwt_auth): improve verify_claims handling and docs

.github/workflows/run_conf_tests.yaml

@@ -39,10 +39,10 @@ jobs:
       - name: print erlang log
         if: failure()
         run: |
-          cat _build/${{ matrix.profile }}/rel/emqx/logs/erlang.log.*
+          cat _build/${{ matrix.profile }}/rel/emqx/log/erlang.log.*
       - uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 # v4.3.1
         if: failure()
         with:
           name: conftest-logs-${{ matrix.profile }}
-          path: _build/${{ matrix.profile }}/rel/emqx/logs
+          path: _build/${{ matrix.profile }}/rel/emqx/log
           retention-days: 7

apps/emqx/test/emqx_cth_peer.erl

@@ -62,7 +62,7 @@ stop(Node) when is_atom(Node) ->
             unlink(Pid),
             ok = peer:stop(Pid);
         false ->
-            ct:pal("The control process for node ~p is unexpetedly down", [Node]),
+            ct:pal("The control process for node ~p is unexpectedly down", [Node]),
             ok
     end.
 

apps/emqx_auth/src/emqx_authn/emqx_authn_utils.erl

@@ -26,6 +26,7 @@
     check_password_from_selected_map/3,
     parse_deep/1,
     parse_str/1,
+    parse_str/2,
     parse_sql/2,
     render_deep/2,
     render_str/2,
@@ -113,28 +114,31 @@ check_password_from_selected_map(Algorithm, Selected, Password) ->
 
 parse_deep(Template) ->
     Result = emqx_template:parse_deep(Template),
-    handle_disallowed_placeholders(Result, {deep, Template}).
+    handle_disallowed_placeholders(Result, ?ALLOWED_VARS, {deep, Template}).
+
+parse_str(Template, AllowedVars) ->
+    Result = emqx_template:parse(Template),
+    handle_disallowed_placeholders(Result, AllowedVars, {string, Template}).
 
 parse_str(Template) ->
-    Result = emqx_template:parse(Template),
-    handle_disallowed_placeholders(Result, {string, Template}).
+    parse_str(Template, ?ALLOWED_VARS).
 
 parse_sql(Template, ReplaceWith) ->
     {Statement, Result} = emqx_template_sql:parse_prepstmt(
         Template,
         #{parameters => ReplaceWith, strip_double_quote => true}
     ),
-    {Statement, handle_disallowed_placeholders(Result, {string, Template})}.
+    {Statement, handle_disallowed_placeholders(Result, ?ALLOWED_VARS, {string, Template})}.
 
-handle_disallowed_placeholders(Template, Source) ->
-    case emqx_template:validate(?ALLOWED_VARS, Template) of
+handle_disallowed_placeholders(Template, AllowedVars, Source) ->
+    case emqx_template:validate(AllowedVars, Template) of
         ok ->
             Template;
         {error, Disallowed} ->
             ?tp(warning, "authn_template_invalid", #{
                 template => Source,
                 reason => Disallowed,
-                allowed => #{placeholders => ?ALLOWED_VARS},
+                allowed => #{placeholders => AllowedVars},
                 notice =>
                     "Disallowed placeholders will be rendered as is."
                     " However, consider using `${$}` escaping for literal `$` where"

apps/emqx_auth_jwt/src/emqx_authn_jwt.erl

@@ -18,6 +18,7 @@
 
 -include_lib("emqx_auth/include/emqx_authn.hrl").
 -include_lib("emqx/include/logger.hrl").
+-include_lib("emqx/include/emqx_placeholder.hrl").
 
 -export([
     create/2,
@@ -26,8 +27,9 @@
     destroy/1
 ]).
 
--export([
-    handle_placeholder/1
+-define(ALLOWED_VARS, [
+    ?VAR_CLIENTID,
+    ?VAR_USERNAME
 ]).
 
 %%------------------------------------------------------------------------------
@@ -83,7 +85,7 @@ authenticate(
 ) ->
     JWT = maps:get(From, Credential),
     JWKs = [JWK],
-    VerifyClaims = replace_placeholder(VerifyClaims0, Credential),
+    VerifyClaims = render_expected(VerifyClaims0, Credential),
     verify(JWT, JWKs, VerifyClaims, AclClaimName);
 authenticate(
     Credential,
@@ -103,7 +105,7 @@ authenticate(
             ignore;
         {ok, JWKs} ->
             JWT = maps:get(From, Credential),
-            VerifyClaims = replace_placeholder(VerifyClaims0, Credential),
+            VerifyClaims = render_expected(VerifyClaims0, Credential),
             verify(JWT, JWKs, VerifyClaims, AclClaimName)
     end.
 
@@ -203,16 +205,11 @@ may_decode_secret(true, Secret) ->
             {error, {invalid_parameter, secret}}
     end.
 
-replace_placeholder(L, Variables) ->
-    replace_placeholder(L, Variables, []).
-
-replace_placeholder([], _Variables, Acc) ->
-    Acc;
-replace_placeholder([{Name, {placeholder, PL}} | More], Variables, Acc) ->
-    Value = maps:get(PL, Variables),
-    replace_placeholder(More, Variables, [{Name, Value} | Acc]);
-replace_placeholder([{Name, Value} | More], Variables, Acc) ->
-    replace_placeholder(More, Variables, [{Name, Value} | Acc]).
+render_expected([], _Variables) ->
+    [];
+render_expected([{Name, ExpectedTemplate} | More], Variables) ->
+    Expected = emqx_authn_utils:render_str(ExpectedTemplate, Variables),
+    [{Name, Expected} | render_expected(More, Variables)].
 
 verify(undefined, _, _, _) ->
     ignore;
@@ -348,23 +345,8 @@ handle_verify_claims(VerifyClaims) ->
 handle_verify_claims([], Acc) ->
     Acc;
 handle_verify_claims([{Name, Expected0} | More], Acc) ->
-    Expected = handle_placeholder(Expected0),
-    handle_verify_claims(More, [{Name, Expected} | Acc]).
-
-handle_placeholder(Placeholder0) ->
-    case re:run(Placeholder0, "^\\$\\{[a-z0-9\\-]+\\}$", [{capture, all}]) of
-        {match, [{Offset, Length}]} ->
-            Placeholder1 = binary:part(Placeholder0, Offset + 2, Length - 3),
-            Placeholder2 = validate_placeholder(Placeholder1),
-            {placeholder, Placeholder2};
-        nomatch ->
-            Placeholder0
-    end.
-
-validate_placeholder(<<"clientid">>) ->
-    clientid;
-validate_placeholder(<<"username">>) ->
-    username.
+    Expected1 = emqx_authn_utils:parse_str(Expected0, ?ALLOWED_VARS),
+    handle_verify_claims(More, [{Name, Expected1} | Acc]).
 
 binary_to_number(Bin) ->
     case string:to_integer(Bin) of
@@ -377,7 +359,7 @@ binary_to_number(Bin) ->
             end
     end.
 
-%% Pars rules which can be in two different formats:
+%% Parse rules which can be in two different formats:
 %% 1. #{<<"pub">> => [<<"a/b">>, <<"c/d">>], <<"sub">> => [...], <<"all">> => [...]}
 %% 2. [#{<<"permission">> => <<"allow">>, <<"action">> => <<"publish">>, <<"topic">> => <<"a/b">>}, ...]
 parse_rules(Rules) when is_map(Rules) ->

apps/emqx_auth_jwt/src/emqx_authn_jwt_schema.erl

@@ -28,6 +28,11 @@
 
 -include("emqx_auth_jwt.hrl").
 -include_lib("hocon/include/hoconsc.hrl").
+-include_lib("typerefl/include/types.hrl").
+
+-type validated_value_type() :: clientid | username | binary().
+
+-reflect_type([validated_value_type/0]).
 
 namespace() -> "authn".
 
@@ -152,18 +157,29 @@ refresh_interval(validator) -> [fun(I) -> I > 0 end];
 refresh_interval(_) -> undefined.
 
 verify_claims(type) ->
-    %% user input is a map, converted to a list of {binary(), binary()}
-    typerefl:alias("map", list());
+    %% user input is a map, converted to a list of {binary(), validated_value_type()}
+    typerefl:alias("map", list({binary(), validated_value_type()}));
 verify_claims(desc) ->
     ?DESC(?FUNCTION_NAME);
 verify_claims(default) ->
-    [];
+    #{};
 verify_claims(validator) ->
     [fun do_check_verify_claims/1];
 verify_claims(converter) ->
     fun
         (VerifyClaims) when is_map(VerifyClaims) ->
             [{to_binary(K), V} || {K, V} <- maps:to_list(VerifyClaims)];
+        (VerifyClaims) when is_list(VerifyClaims) ->
+            lists:map(
+                fun
+                    (#{<<"name">> := Key, <<"value">> := Value}) ->
+                        {Key, Value};
+                    (Other) ->
+                        Other
+                end,
+                VerifyClaims
+            );
+        %% this will make validation fail, because it is not a list
         (VerifyClaims) ->
             VerifyClaims
     end;
@@ -174,10 +190,12 @@ verify_claims(_) ->
 
 do_check_verify_claims([]) ->
     true;
-do_check_verify_claims([{Name, Expected} | More]) ->
+%% _Expected can't be invalid since tuples may come only from converter
+do_check_verify_claims([{Name, _Expected} | More]) ->
     check_claim_name(Name) andalso
-        check_claim_expected(Expected) andalso
-        do_check_verify_claims(More).
+        do_check_verify_claims(More);
+do_check_verify_claims(_) ->
+    false.
 
 check_claim_name(exp) ->
     false;
@@ -193,14 +211,6 @@ check_claim_name(Name) when
 check_claim_name(_) ->
     true.
 
-check_claim_expected(Expected) ->
-    try emqx_authn_jwt:handle_placeholder(Expected) of
-        _ -> true
-    catch
-        _:_ ->
-            false
-    end.
-
 from(type) -> hoconsc:enum([username, password]);
 from(desc) -> ?DESC(?FUNCTION_NAME);
 from(default) -> password;

apps/emqx_auth_jwt/test/emqx_authn_jwt_SUITE.erl

@@ -219,6 +219,37 @@ t_jwt_in_username(_) ->
     },
     ?assertMatch({ok, #{is_superuser := false}}, emqx_authn_jwt:authenticate(Credential, State)).
 
+t_complex_template(_) ->
+    Secret = <<"abcdef">>,
+    Config = #{
+        mechanism => jwt,
+        from => password,
+        acl_claim_name => <<"acl">>,
+        use_jwks => false,
+        algorithm => 'hmac-based',
+        secret => Secret,
+        secret_base64_encoded => false,
+        verify_claims => [{<<"id">>, <<"${username}-${clientid}">>}]
+    },
+    {ok, State} = emqx_authn_jwt:create(?AUTHN_ID, Config),
+
+    Payload0 = #{<<"id">> => <<"myuser-myclient">>, <<"exp">> => erlang:system_time(second) + 60},
+    JWS0 = generate_jws('hmac-based', Payload0, Secret),
+    Credential0 = #{
+        clientid => <<"myclient">>,
+        username => <<"myuser">>,
+        password => JWS0
+    },
+    ?assertMatch({ok, #{is_superuser := false}}, emqx_authn_jwt:authenticate(Credential0, State)),
+
+    Payload1 = #{<<"id">> => <<"-myclient">>, <<"exp">> => erlang:system_time(second) + 60},
+    JWS1 = generate_jws('hmac-based', Payload1, Secret),
+    Credential1 = #{
+        clientid => <<"myclient">>,
+        password => JWS1
+    },
+    ?assertMatch({ok, #{is_superuser := false}}, emqx_authn_jwt:authenticate(Credential1, State)).
+
 t_jwks_renewal(_Config) ->
     {ok, _} = emqx_authn_http_test_server:start_link(?JWKS_PORT, ?JWKS_PATH, server_ssl_opts()),
     ok = emqx_authn_http_test_server:set_handler(fun jwks_handler/2),
@@ -415,6 +446,38 @@ t_verify_claims(_) ->
     },
     ?assertMatch({ok, #{is_superuser := false}}, emqx_authn_jwt:authenticate(Credential5, State1)).
 
+t_verify_claim_clientid(_) ->
+    Secret = <<"abcdef">>,
+    Config = #{
+        mechanism => jwt,
+        from => password,
+        acl_claim_name => <<"acl">>,
+        use_jwks => false,
+        algorithm => 'hmac-based',
+        secret => Secret,
+        secret_base64_encoded => false,
+        verify_claims => [{<<"cl">>, <<"${clientid}">>}]
+    },
+    {ok, State} = emqx_authn_jwt:create(?AUTHN_ID, Config),
+
+    Payload0 = #{<<"cl">> => <<"mycl">>, <<"exp">> => erlang:system_time(second) + 60},
+    JWS0 = generate_jws('hmac-based', Payload0, Secret),
+    Credential0 = #{
+        username => <<"myuser">>,
+        clientid => <<"mycl">>,
+        password => JWS0
+    },
+    ?assertMatch({ok, #{is_superuser := false}}, emqx_authn_jwt:authenticate(Credential0, State)),
+
+    Credential1 = #{
+        username => <<"myuser">>,
+        clientid => <<"mycl-invalid">>,
+        password => JWS0
+    },
+    ?assertMatch(
+        {error, bad_username_or_password}, emqx_authn_jwt:authenticate(Credential1, State)
+    ).
+
 t_jwt_not_allow_empty_claim_name(_) ->
     Request = #{
         <<"use_jwks">> => false,
@@ -450,10 +513,89 @@ t_jwt_not_allow_empty_claim_name(_) ->
         )
     ).
 
+t_schema(_Config) ->
+    RawClaims0 = [
+        #{<<"name">> => <<"a">>, <<"value">> => <<"v">>},
+        #{<<"name">> => <<"b">>, <<"value">> => <<"${username}">>},
+        #{<<"name">> => <<"c">>, <<"value">> => <<"${clientid}">>}
+    ],
+    ?assertMatch(
+        {ok, [
+            {<<"a">>, <<"v">>},
+            {<<"b">>, <<"${username}">>},
+            {<<"c">>, <<"${clientid}">>}
+        ]},
+        check_schema(RawClaims0)
+    ),
+
+    RawClaims1 = [#{<<"key">> => <<"a">>, <<"value">> => <<"v">>}],
+    ?assertMatch(
+        {error, _},
+        check_schema(RawClaims1)
+    ),
+    RawClaims2 = #{
+        <<"a">> => <<"v">>,
+        <<"b">> => <<"${username}">>,
+        <<"c">> => <<"${clientid}">>
+    },
+    ?assertMatch(
+        {ok, [
+            {<<"a">>, <<"v">>},
+            {<<"b">>, <<"${username}">>},
+            {<<"c">>, <<"${clientid}">>}
+        ]},
+        check_schema(RawClaims2)
+    ),
+    ?assertMatch(
+        {ok, [{<<"x">>, <<"${foo}">>}]},
+        check_schema(#{<<"x">> => <<"${foo}">>})
+    ),
+    ?assertMatch(
+        {error, _},
+        check_schema([<<"foo">>])
+    ),
+    ?assertMatch(
+        {error, _},
+        check_schema([#{}])
+    ),
+    ?assertMatch(
+        {error, _},
+        check_schema([[]])
+    ),
+    ?assertMatch(
+        {error, _},
+        check_schema(<<"val">>)
+    ).
+
 %%------------------------------------------------------------------------------
 %% Helpers
 %%------------------------------------------------------------------------------
 
+check_schema(RawClaims) ->
+    Config = #{
+        <<"conf">> =>
+            #{
+                <<"use_jwks">> => false,
+                <<"algorithm">> => <<"hmac-based">>,
+                <<"acl_claim_name">> => <<"acl">>,
+                <<"secret">> => <<"secret">>,
+                <<"mechanism">> => <<"jwt">>,
+                <<"verify_claims">> => RawClaims
+            }
+    },
+    UnionMemberSelector =
+        fun
+            (all_union_members) -> emqx_authn_jwt_schema:refs();
+            ({value, Value}) -> emqx_authn_jwt_schema:select_union_member(Value)
+        end,
+    Schema = #{roots => [{conf, hoconsc:union(UnionMemberSelector)}]},
+    case emqx_hocon:check(Schema, Config) of
+        {ok, #{conf := #{verify_claims := VerifyClaims}}} ->
+            {ok, VerifyClaims};
+        Error ->
+            Error
+    end.
+
 jwks_handler(Req0, State) ->
     JWK = jose_jwk:from_pem_file(test_rsa_key(public)),
     JWKS = jose_jwk_set:to_map([JWK], #{}),

changes/ce/feat-12418.en.md

@@ -0,0 +1,17 @@
+For JWT authentication, the claims to verify now may be provided as a list of maps:
+```
+[
+    {
+        name = "claim_name"
+        value = "${username}"
+    },
+    ...
+]
+```
+
+Expected values now treated as templates, uniformly with the oither authenticators.
+They now allow arbitrary expressions including `${username}` and `${clientid}` variables.
+Previousy, only fixed `"${username}"` `"${clientid}"` values were supported for interpolation.
+
+Improved the documentation for the `verify_claims` parameter.
+

rel/i18n/emqx_authn_jwt_schema.hocon

@@ -130,10 +130,17 @@ verify.label:
 """Verify"""
 
 verify_claims.desc:
-"""A list of custom claims to validate, which is a list of name/value pairs.
+"""A list of custom claims to validate. The allowed formats are the following:
+A map where claim names are map keys and expected values are map values:
+ <code>{ claim_name = "${username}", ...}</code>.
+
+A list of maps with <code>name</code> (claim name) and <code>value</code> (expected claim value) keys:
+ <code>[{name = "claim_name", value = "${username}"}, ...]</code>.
+
 Values can use the following placeholders:
 - <code>${username}</code>: Will be replaced at runtime with <code>Username</code> used by the client when connecting
 - <code>${clientid}</code>: Will be replaced at runtime with <code>Client ID</code> used by the client when connecting
+
 Authentication will verify that the value of claims in the JWT (taken from the Password field) matches what is required in <code>verify_claims</code>."""
 
 verify_claims.label: