refactor(auth/jwt): support raw rules from jwt acl claim

2023-12-17 14:37:55 +01:00 · 2023-12-17 14:37:55 +01:00 · 2be898ca4d
parent da92e62e8c
commit 2be898ca4d
8 changed files with 272 additions and 29 deletions
--- a/apps/emqx_auth/src/emqx_authz/emqx_authz_rule_raw.erl
+++ b/apps/emqx_auth/src/emqx_authz/emqx_authz_rule_raw.erl
@ -51,11 +51,18 @@ parse_rule(
        Action = validate_rule_action(ActionType, RuleRaw),
        {ok, {Permission, Action, Topics}}
    catch
-        throw:ValidationError ->
-            {error, ValidationError}
+        throw:{Invalid, Which} ->
+            {error, #{
+                reason => Invalid,
+                value => Which
+            }}
    end;
 parse_rule(RuleRaw) ->
-    {error, {invalid_rule, RuleRaw}}.
+    {error, #{
+        reason => invalid_rule,
+        value => RuleRaw,
+        explain => "missing 'permission' or 'action' field"
+    }}.

 -spec format_rule({
    emqx_authz_rule:permission(),
@ -88,7 +95,7 @@ validate_rule_topics(#{<<"topic">> := TopicRaw}) when is_binary(TopicRaw) ->
 validate_rule_topics(#{<<"topics">> := TopicsRaw}) when is_list(TopicsRaw) ->
    lists:map(fun validate_rule_topic/1, TopicsRaw);
 validate_rule_topics(RuleRaw) ->
-    throw({invalid_topics, RuleRaw}).
+    throw({missing_topic_or_topics, RuleRaw}).

 validate_rule_topic(<<"eq ", TopicRaw/binary>>) ->
    {eq, validate_rule_topic(TopicRaw)};
@ -98,8 +105,8 @@ validate_rule_permission(<<"allow">>) -> allow;
 validate_rule_permission(<<"deny">>) -> deny;
 validate_rule_permission(PermissionRaw) -> throw({invalid_permission, PermissionRaw}).

-validate_rule_action_type(<<"publish">>) -> publish;
-validate_rule_action_type(<<"subscribe">>) -> subscribe;
+validate_rule_action_type(P) when P =:= <<"pub">> orelse P =:= <<"publish">> -> publish;
+validate_rule_action_type(S) when S =:= <<"sub">> orelse S =:= <<"subscribe">> -> subscribe;
 validate_rule_action_type(<<"all">>) -> all;
 validate_rule_action_type(ActionRaw) -> throw({invalid_action, ActionRaw}).

@ -152,7 +159,7 @@ validate_rule_qos_atomic(<<"2">>) -> 2;
 validate_rule_qos_atomic(0) -> 0;
 validate_rule_qos_atomic(1) -> 1;
 validate_rule_qos_atomic(2) -> 2;
-validate_rule_qos_atomic(_) -> throw(invalid_qos).
+validate_rule_qos_atomic(QoS) -> throw({invalid_qos, QoS}).

 validate_rule_retain(<<"0">>) -> false;
 validate_rule_retain(<<"1">>) -> true;
--- a/apps/emqx_auth/src/emqx_authz/sources/emqx_authz_client_info.erl
+++ b/apps/emqx_auth/src/emqx_authz/sources/emqx_authz_client_info.erl
@ -34,6 +34,10 @@
    authorize/4
 ]).

+-define(IS_V1(Rules), is_map(Rules)).
+-define(IS_V2(Rules), is_list(Rules)).
+
+%% For v1
 -define(RULE_NAMES, [
    {[pub, <<"pub">>], publish},
    {[sub, <<"sub">>], subscribe},
@ -55,10 +59,46 @@ update(Source) ->

 destroy(_Source) -> ok.

+%% @doc Authorize based on cllientinfo enriched with `acl' data.
+%% e.g. From JWT.
+%%
+%% Supproted rules formats are:
+%%
+%% v1: (always deny when no match)
+%%
+%%    #{
+%%        pub => [TopicFilter],
+%%        sub => [TopicFilter],
+%%        all => [TopicFilter]
+%%    }
+%%
+%% v2: (rules are checked in sequence, passthrough when no match)
+%%
+%%    [{
+%%        Permission :: emqx_authz_rule:permission(),
+%%        Action :: emqx_authz_rule:action_condition(),
+%%        Topics :: emqx_authz_rule:topic_condition()
+%%     }]
+%%
+%%  which is compiled from raw rules like below by emqx_authz_rule_raw
+%%
+%%    [
+%%        #{
+%%            permission := allow | deny
+%%            action := pub | sub | all
+%%            topic => TopicFilter,
+%%            topics => [TopicFilter] %% when 'topic' is not provided
+%%            qos => 0 | 1 | 2 | [0, 1, 2]
+%%            retain => true | false | all %% only for pub action
+%%        }
+%%    ]
+%%
 authorize(#{acl := Acl} = Client, PubSub, Topic, _Source) ->
    case check(Acl) of
-        {ok, Rules} when is_map(Rules) ->
-            do_authorize(Client, PubSub, Topic, Rules);
+        {ok, Rules} when ?IS_V2(Rules) ->
+            authorize_v2(Client, PubSub, Topic, Rules);
+        {ok, Rules} when ?IS_V1(Rules) ->
+            authorize_v1(Client, PubSub, Topic, Rules);
        {error, MatchResult} ->
            MatchResult
    end;
@ -69,7 +109,7 @@ authorize(_Client, _PubSub, _Topic, _Source) ->
 %% Internal functions
 %%--------------------------------------------------------------------

-check(#{expire := Expire, rules := Rules}) when is_map(Rules) ->
+check(#{expire := Expire, rules := Rules}) ->
    Now = erlang:system_time(second),
    case Expire of
        N when is_integer(N) andalso N > Now -> {ok, Rules};
@ -83,13 +123,13 @@ check(#{rules := Rules}) ->
 check(#{}) ->
    {error, nomatch}.

-do_authorize(Client, PubSub, Topic, AclRules) ->
-    do_authorize(Client, PubSub, Topic, AclRules, ?RULE_NAMES).
+authorize_v1(Client, PubSub, Topic, AclRules) ->
+    authorize_v1(Client, PubSub, Topic, AclRules, ?RULE_NAMES).

-do_authorize(_Client, _PubSub, _Topic, _AclRules, []) ->
+authorize_v1(_Client, _PubSub, _Topic, _AclRules, []) ->
    {matched, deny};
-do_authorize(Client, PubSub, Topic, AclRules, [{Keys, Action} | RuleNames]) ->
-    TopicFilters = get_topic_filters(Keys, AclRules, []),
+authorize_v1(Client, PubSub, Topic, AclRules, [{Keys, Action} | RuleNames]) ->
+    TopicFilters = get_topic_filters_v1(Keys, AclRules, []),
    case
        emqx_authz_rule:match(
            Client,
@ -99,13 +139,16 @@ do_authorize(Client, PubSub, Topic, AclRules, [{Keys, Action} | RuleNames]) ->
        )
    of
        {matched, Permission} -> {matched, Permission};
-        nomatch -> do_authorize(Client, PubSub, Topic, AclRules, RuleNames)
+        nomatch -> authorize_v1(Client, PubSub, Topic, AclRules, RuleNames)
    end.

-get_topic_filters([], _Rules, Default) ->
+get_topic_filters_v1([], _Rules, Default) ->
    Default;
-get_topic_filters([Key | Keys], Rules, Default) ->
+get_topic_filters_v1([Key | Keys], Rules, Default) ->
    case Rules of
        #{Key := Value} -> Value;
-        #{} -> get_topic_filters(Keys, Rules, Default)
+        #{} -> get_topic_filters_v1(Keys, Rules, Default)
    end.
+
+authorize_v2(Client, PubSub, Topic, Rules) ->
+    emqx_authz_rule:matches(Client, PubSub, Topic, Rules).
--- a/apps/emqx_auth_jwt/src/emqx_auth_jwt.app.src
+++ b/apps/emqx_auth_jwt/src/emqx_auth_jwt.app.src
@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_auth_jwt, [
    {description, "EMQX JWT Authentication and Authorization"},
-    {vsn, "0.1.1"},
+    {vsn, "0.2.0"},
    {registered, []},
    {mod, {emqx_auth_jwt_app, []}},
    {applications, [
--- a/apps/emqx_auth_jwt/src/emqx_authn_jwt.erl
+++ b/apps/emqx_auth_jwt/src/emqx_authn_jwt.erl
@ -219,14 +219,24 @@ verify(undefined, _, _, _) ->
 verify(JWT, JWKs, VerifyClaims, AclClaimName) ->
    case do_verify(JWT, JWKs, VerifyClaims) of
        {ok, Extra} ->
-            {ok, acl(Extra, AclClaimName)};
+            try
+                {ok, acl(Extra, AclClaimName)}
+            catch
+                throw:{bad_acl_rule, Reason} ->
+                    %% it's a invalid token, so ok to log
+                    ?TRACE_AUTHN_PROVIDER("bad_acl_rule", Reason#{jwt => JWT}),
+                    {error, bad_username_or_password}
+            end;
        {error, {missing_claim, Claim}} ->
+            %% it's a invalid token, so it's ok to log
            ?TRACE_AUTHN_PROVIDER("missing_jwt_claim", #{jwt => JWT, claim => Claim}),
            {error, bad_username_or_password};
        {error, invalid_signature} ->
+            %% it's a invalid token, so it's ok to log
            ?TRACE_AUTHN_PROVIDER("invalid_jwt_signature", #{jwks => JWKs, jwt => JWT}),
            ignore;
        {error, {claims, Claims}} ->
+            %% it's a invalid token, so it's ok to log
            ?TRACE_AUTHN_PROVIDER("invalid_jwt_claims", #{jwt => JWT, claims => Claims}),
            {error, bad_username_or_password}
    end.
@ -237,7 +247,8 @@ acl(Claims, AclClaimName) ->
            #{AclClaimName := Rules} ->
                #{
                    acl => #{
-                        rules => Rules,
+                        rules => parse_rules(Rules),
+                        source_for_logging => jwt,
                        expire => maps:get(<<"exp">>, Claims, undefined)
                    }
                };
@ -363,3 +374,24 @@ binary_to_number(Bin) ->
                _ -> false
            end
    end.
+
+%% Pars 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) ->
+    Rules;
+parse_rules(Rules) when is_list(Rules) ->
+    lists:map(fun parse_rule/1, Rules).
+
+parse_rule(Rule) ->
+    case emqx_authz_rule_raw:parse_rule(Rule) of
+        {ok, {Permission, Action, Topics}} ->
+            try
+                emqx_authz_rule:compile({Permission, all, Action, Topics})
+            catch
+                throw:Reason ->
+                    throw({bad_acl_rule, Reason})
+            end;
+        {error, Reason} ->
+            throw({bad_acl_rule, Reason})
+    end.
--- a/apps/emqx_auth_jwt/test/emqx_authz_jwt_SUITE.erl
+++ b/apps/emqx_auth_jwt/test/emqx_authz_jwt_SUITE.erl
@ -78,7 +78,7 @@ end_per_testcase(_TestCase, _Config) ->
 %%------------------------------------------------------------------------------

 t_topic_rules(_Config) ->
-    Payload = #{
+    JWT = #{
        <<"exp">> => erlang:system_time(second) + 60,
        <<"acl">> => #{
            <<"pub">> => [
@ -99,7 +99,47 @@ t_topic_rules(_Config) ->
        },
        <<"username">> => <<"username">>
    },
-    JWT = generate_jws(Payload),
+    test_topic_rules(JWT).
+
+t_topic_rules_v2(_Config) ->
+    JWT = #{
+        <<"exp">> => erlang:system_time(second) + 60,
+        <<"acl">> => [
+            #{
+                <<"permission">> => <<"allow">>,
+                <<"action">> => <<"pub">>,
+                <<"topics">> => [
+                    <<"eq testpub1/${username}">>,
+                    <<"testpub2/${clientid}">>,
+                    <<"testpub3/#">>
+                ]
+            },
+            #{
+                <<"permission">> => <<"allow">>,
+                <<"action">> => <<"sub">>,
+                <<"topics">> =>
+                    [
+                        <<"eq testsub1/${username}">>,
+                        <<"testsub2/${clientid}">>,
+                        <<"testsub3/#">>
+                    ]
+            },
+            #{
+                <<"permission">> => <<"allow">>,
+                <<"action">> => <<"all">>,
+                <<"topics">> => [
+                    <<"eq testall1/${username}">>,
+                    <<"testall2/${clientid}">>,
+                    <<"testall3/#">>
+                ]
+            }
+        ],
+        <<"username">> => <<"username">>
+    },
+    test_topic_rules(JWT).
+
+test_topic_rules(JWTInput) ->
+    JWT = generate_jws(JWTInput),

    {ok, C} = emqtt:start_link(
        [
@ -350,6 +390,64 @@ t_check_undefined_expire(_Config) ->
        emqx_authz_client_info:authorize(Client, ?AUTHZ_SUBSCRIBE, <<"a/bar">>, undefined)
    ).

+t_invalid_rule(_Config) ->
+    emqx_logger:set_log_level(debug),
+    MakeJWT = fun(Acl) ->
+        generate_jws(#{
+            <<"exp">> => erlang:system_time(second) + 60,
+            <<"username">> => <<"username">>,
+            <<"acl">> => Acl
+        })
+    end,
+    InvalidAclList =
+        [
+            %% missing action
+            [#{<<"permission">> => <<"invalid">>}],
+            %% missing topic or topics
+            [#{<<"permission">> => <<"allow">>, <<"action">> => <<"pub">>}],
+            %% invlaid permission, must be allow | deny
+            [
+                #{
+                    <<"permission">> => <<"invalid">>,
+                    <<"action">> => <<"pub">>,
+                    <<"topic">> => <<"t">>
+                }
+            ],
+            %% invalid action, must be pub | sub | all
+            [
+                #{
+                    <<"permission">> => <<"allow">>,
+                    <<"action">> => <<"invalid">>,
+                    <<"topic">> => <<"t">>
+                }
+            ],
+            %% invalid qos
+            [
+                #{
+                    <<"permission">> => <<"allow">>,
+                    <<"action">> => <<"pub">>,
+                    <<"topics">> => [<<"t">>],
+                    <<"qos">> => 3
+                }
+            ]
+        ],
+    lists:foreach(
+        fun(InvalidAcl) ->
+            {ok, C} = emqtt:start_link(
+                [
+                    {clean_start, true},
+                    {proto_ver, v5},
+                    {clientid, <<"clientid">>},
+                    {username, <<"username">>},
+                    {password, MakeJWT(InvalidAcl)}
+                ]
+            ),
+            unlink(C),
+            ?assertMatch({error, {bad_username_or_password, _}}, emqtt:connect(C))
+        end,
+        InvalidAclList
+    ).
+
 %%------------------------------------------------------------------------------
 %% Helpers
 %%------------------------------------------------------------------------------
--- a/apps/emqx_auth_mnesia/test/emqx_authz_mnesia_SUITE.erl
+++ b/apps/emqx_auth_mnesia/test/emqx_authz_mnesia_SUITE.erl
@ -190,7 +190,7 @@ t_normalize_rules(_Config) ->

    ?assertException(
        error,
-        {invalid_rule, _},
+        #{reason := invalid_rule},
        emqx_authz_mnesia:store_rules(
            {username, <<"username">>},
            [[<<"allow">>, <<"publish">>, <<"t">>]]
@ -199,16 +199,22 @@ t_normalize_rules(_Config) ->

    ?assertException(
        error,
-        {invalid_action, _},
+        #{reason := invalid_action},
        emqx_authz_mnesia:store_rules(
            {username, <<"username">>},
-            [#{<<"permission">> => <<"allow">>, <<"action">> => <<"pub">>, <<"topic">> => <<"t">>}]
+            [
+                #{
+                    <<"permission">> => <<"allow">>,
+                    <<"action">> => <<"badaction">>,
+                    <<"topic">> => <<"t">>
+                }
+            ]
        )
    ),

    ?assertException(
        error,
-        {invalid_permission, _},
+        #{reason := invalid_permission},
        emqx_authz_mnesia:store_rules(
            {username, <<"username">>},
            [
--- a/changes/ce/feat-12189.en.md
+++ b/changes/ce/feat-12189.en.md
@ -0,0 +1,33 @@
+Enhanced JWT ACL Claim Format.
+
+The JWT ACL claim has been upgraded to support a more versatile format.
+It now accepts an array structure, which resembles the file-based ACL rules.
+
+For example:
+
+```json
+[
+  {
+    "permission": "allow",
+    "action": "pub",
+    "topic": "${username}/#",
+    "qos": [0,1],
+    "retain": true
+  },
+  {
+    "permission": "allow",
+    "action": "sub",
+    "topic": "eq ${username}/#",
+    "qos": [0,1]
+  },
+  {
+    "permission": "deny",
+    "action": "all",
+    "topics": ["#"]
+  }
+]
+```
+
+In this new format, when no matching rule is found, the action is not automatically denied.
+This allows the authorization process to proceed to other configured authorization sources.
+If no match is found throughout the chain, the final decision defers to the default permission set in `authorization.no_match`.
--- a/rel/i18n/emqx_authn_jwt_schema.hocon
+++ b/rel/i18n/emqx_authn_jwt_schema.hocon
@ -1,7 +1,31 @@
 emqx_authn_jwt_schema {

 acl_claim_name.desc:
-"""JWT claim name to use for getting ACL rules."""
+"""The JWT claim designated for accessing ACL (Access Control List) rules can be specified,
+such as using the `acl` claim. A typical decoded JWT with this claim might appear as:
+`{"username": "user1", "acl": ...}`.
+
+Supported ACL Rule Formats:
+
+- Object Format:
+  Utilizes action types pub (publish), sub (subscribe), or all (both publish and subscribe).
+  The value is a list of topic filters.
+  Example: `{"pub": ["topic1"], "sub": [], "all": ["${username}/#"]}`.
+  This example signifies that the token owner can publish to topic1 and perform both publish and subscribe
+  actions on topics starting with their username.
+  Note: In this format, if no topic matches, the action is denied, and the authorization process terminates.
+
+- Array Format (resembles File-Based ACL Rules):
+  Example: `[{"permission": "allow", "action": "all", "topic": "${username}/#"}]`.
+  Additionally, the `pub` or `publish` action rules can be extended with `qos` and `retain` field,
+  and `sub` or `subscribe` action rules can be extended with a `qos` field.
+  Note: Here, if no rule matches, the action is not immediately denied.
+  The process continues to other configured authorization sources,
+  and ultimately falls back to the default permission in config `authorization.no_match`.
+
+The ACL claim utilizes MQTT topic wildcard matching rules for publishing or subscribing.
+A special syntax for the 'subscribe' action allows the use of `eq` for an exact match.
+For instance, `eq t/#` permits or denies subscription to `t/#`, but not to `t/1`."""

 acl_claim_name.label:
 """ACL claim name"""