diff --git a/.github/workflows/run_conf_tests.yaml b/.github/workflows/run_conf_tests.yaml index 1ba93d40b..40fe8fd75 100644 --- a/.github/workflows/run_conf_tests.yaml +++ b/.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 diff --git a/apps/emqx/test/emqx_cth_peer.erl b/apps/emqx/test/emqx_cth_peer.erl index ca66b42b7..b3849739a 100644 --- a/apps/emqx/test/emqx_cth_peer.erl +++ b/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. diff --git a/apps/emqx_auth/src/emqx_authn/emqx_authn_utils.erl b/apps/emqx_auth/src/emqx_authn/emqx_authn_utils.erl index a2050fbf0..6e49fda24 100644 --- a/apps/emqx_auth/src/emqx_authn/emqx_authn_utils.erl +++ b/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" diff --git a/apps/emqx_auth_jwt/src/emqx_authn_jwt.erl b/apps/emqx_auth_jwt/src/emqx_authn_jwt.erl index 2a12f5acf..d04f8b678 100644 --- a/apps/emqx_auth_jwt/src/emqx_authn_jwt.erl +++ b/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) -> diff --git a/apps/emqx_auth_jwt/src/emqx_authn_jwt_schema.erl b/apps/emqx_auth_jwt/src/emqx_authn_jwt_schema.erl index e7bf0a11a..f52992564 100644 --- a/apps/emqx_auth_jwt/src/emqx_authn_jwt_schema.erl +++ b/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; diff --git a/apps/emqx_auth_jwt/test/emqx_authn_jwt_SUITE.erl b/apps/emqx_auth_jwt/test/emqx_authn_jwt_SUITE.erl index 59abe1bbc..a7eca75aa 100644 --- a/apps/emqx_auth_jwt/test/emqx_authn_jwt_SUITE.erl +++ b/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], #{}), diff --git a/changes/ce/feat-12418.en.md b/changes/ce/feat-12418.en.md new file mode 100644 index 000000000..702508592 --- /dev/null +++ b/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. + diff --git a/rel/i18n/emqx_authn_jwt_schema.hocon b/rel/i18n/emqx_authn_jwt_schema.hocon index 4251358d8..67b301b86 100644 --- a/rel/i18n/emqx_authn_jwt_schema.hocon +++ b/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: + { claim_name = "${username}", ...}. + +A list of maps with name (claim name) and value (expected claim value) keys: + [{name = "claim_name", value = "${username}"}, ...]. + Values can use the following placeholders: - ${username}: Will be replaced at runtime with Username used by the client when connecting - ${clientid}: Will be replaced at runtime with Client ID 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 verify_claims.""" verify_claims.label: