From da5b01aa46b555de6e823525938fa1c1bceccbc9 Mon Sep 17 00:00:00 2001 From: zmstone Date: Wed, 10 Apr 2024 11:24:17 +0200 Subject: [PATCH] refactor(client_attr): allow more than one initial extraction --- apps/emqx/src/emqx_channel.erl | 145 +++++++++++------- apps/emqx/src/emqx_schema.erl | 4 +- apps/emqx/test/emqx_config_SUITE.erl | 2 +- apps/emqx/test/emqx_listeners_SUITE.erl | 2 +- .../test/emqx_authz/emqx_authz_SUITE.erl | 2 +- changes/ce/feat-12750.en.md | 2 +- rel/i18n/emqx_schema.hocon | 3 +- 7 files changed, 100 insertions(+), 60 deletions(-) diff --git a/apps/emqx/src/emqx_channel.erl b/apps/emqx/src/emqx_channel.erl index 27babfcc9..a0fbae441 100644 --- a/apps/emqx/src/emqx_channel.erl +++ b/apps/emqx/src/emqx_channel.erl @@ -269,8 +269,7 @@ init( }, Zone ), - AttrExtractionConfig = get_mqtt_conf(Zone, client_attrs_init), - ClientInfo = initialize_client_attrs_from_cert(AttrExtractionConfig, ClientInfo0, Peercert), + ClientInfo = initialize_client_attrs_from_cert(ClientInfo0, Peercert), {NClientInfo, NConnInfo} = take_ws_cookie(ClientInfo, ConnInfo), #channel{ conninfo = NConnInfo, @@ -1575,7 +1574,7 @@ enrich_client(ConnPkt, Channel = #channel{clientinfo = ClientInfo}) -> fun maybe_username_as_clientid/2, fun maybe_assign_clientid/2, %% attr init should happen after clientid and username assign - fun maybe_set_client_initial_attr/2 + fun maybe_set_client_initial_attrs/2 ], ConnPkt, ClientInfo @@ -1587,7 +1586,17 @@ enrich_client(ConnPkt, Channel = #channel{clientinfo = ClientInfo}) -> {error, ReasonCode, Channel#channel{clientinfo = NClientInfo}} end. -initialize_client_attrs_from_cert( +initialize_client_attrs_from_cert(#{zone := Zone} = ClientInfo, Peercert) -> + Inits = get_client_attrs_init_config(Zone), + lists:foldl( + fun(Init, Acc) -> + do_initialize_client_attrs_from_cert(Init, Acc, Peercert) + end, + ClientInfo, + Inits + ). + +do_initialize_client_attrs_from_cert( #{ extract_from := From, extract_regexp := Regexp, @@ -1596,21 +1605,24 @@ initialize_client_attrs_from_cert( ClientInfo, Peercert ) when From =:= cn orelse From =:= dn -> - case extract_client_attr_from_cert(From, Regexp, Peercert) of - {ok, Value} -> - ?SLOG( - debug, - #{ - msg => "client_attr_init_from_cert", - extracted_as => AttrName, - extracted_value => Value - } - ), - ClientInfo#{client_attrs => #{AttrName => Value}}; - _ -> - ClientInfo#{client_attrs => #{}} - end; -initialize_client_attrs_from_cert(_, ClientInfo, _Peercert) -> + Attrs0 = maps:get(client_attrs, ClientInfo, #{}), + Attrs = + case extract_client_attr_from_cert(From, Regexp, Peercert) of + {ok, Value} -> + ?SLOG( + debug, + #{ + msg => "client_attr_init_from_cert", + extracted_as => AttrName, + extracted_value => Value + } + ), + Attrs0#{AttrName => Value}; + _ -> + Attrs0 + end, + ClientInfo#{client_attrs => Attrs}; +do_initialize_client_attrs_from_cert(_, ClientInfo, _Peercert) -> ClientInfo. extract_client_attr_from_cert(cn, Regexp, Peercert) -> @@ -1668,27 +1680,51 @@ maybe_assign_clientid(#mqtt_packet_connect{clientid = <<>>}, ClientInfo) -> maybe_assign_clientid(#mqtt_packet_connect{clientid = ClientId}, ClientInfo) -> {ok, ClientInfo#{clientid => ClientId}}. -maybe_set_client_initial_attr(ConnPkt, #{zone := Zone} = ClientInfo0) -> - Config = get_mqtt_conf(Zone, client_attrs_init), - ClientInfo = initialize_client_attrs_from_user_property(Config, ConnPkt, ClientInfo0), - Attrs = maps:get(client_attrs, ClientInfo, #{}), - case extract_attr_from_clientinfo(Config, ClientInfo) of - {ok, Value} -> - #{extract_as := Name} = Config, - ?SLOG( - debug, - #{ - msg => "client_attr_init_from_clientinfo", - extracted_as => Name, - extracted_value => Value - } - ), - {ok, ClientInfo#{client_attrs => Attrs#{Name => Value}}}; - _ -> - {ok, ClientInfo} +get_client_attrs_init_config(Zone) -> + case get_mqtt_conf(Zone, client_attrs_init, []) of + L when is_list(L) -> L; + M when is_map(M) -> [M] end. -initialize_client_attrs_from_user_property( +maybe_set_client_initial_attrs(ConnPkt, #{zone := Zone} = ClientInfo0) -> + Inits = get_client_attrs_init_config(Zone), + ClientInfo = initialize_client_attrs_from_user_property(Inits, ConnPkt, ClientInfo0), + {ok, initialize_client_attrs_from_clientinfo(Inits, ClientInfo)}. + +initialize_client_attrs_from_clientinfo(Inits, ClientInfo) -> + lists:foldl( + fun(Init, Acc) -> + Attrs = maps:get(client_attrs, ClientInfo, #{}), + case extract_attr_from_clientinfo(Init, ClientInfo) of + {ok, Value} -> + #{extract_as := Name} = Init, + ?SLOG( + debug, + #{ + msg => "client_attr_init_from_clientinfo", + extracted_as => Name, + extracted_value => Value + } + ), + Acc#{client_attrs => Attrs#{Name => Value}}; + _ -> + Acc + end + end, + ClientInfo, + Inits + ). + +initialize_client_attrs_from_user_property(Inits, ConnPkt, ClientInfo) -> + lists:foldl( + fun(Init, Acc) -> + do_initialize_client_attrs_from_user_property(Init, ConnPkt, Acc) + end, + ClientInfo, + Inits + ). + +do_initialize_client_attrs_from_user_property( #{ extract_from := user_property, extract_as := PropertyKey @@ -1696,21 +1732,24 @@ initialize_client_attrs_from_user_property( ConnPkt, ClientInfo ) -> - case extract_client_attr_from_user_property(ConnPkt, PropertyKey) of - {ok, Value} -> - ?SLOG( - debug, - #{ - msg => "client_attr_init_from_user_property", - extracted_as => PropertyKey, - extracted_value => Value - } - ), - ClientInfo#{client_attrs => #{PropertyKey => Value}}; - _ -> - ClientInfo - end; -initialize_client_attrs_from_user_property(_, _ConnInfo, ClientInfo) -> + Attrs0 = maps:get(client_attrs, ClientInfo, #{}), + Attrs = + case extract_client_attr_from_user_property(ConnPkt, PropertyKey) of + {ok, Value} -> + ?SLOG( + debug, + #{ + msg => "client_attr_init_from_user_property", + extracted_as => PropertyKey, + extracted_value => Value + } + ), + Attrs0#{PropertyKey => Value}; + _ -> + Attrs0 + end, + ClientInfo#{client_attrs => Attrs}; +do_initialize_client_attrs_from_user_property(_, _ConnPkt, ClientInfo) -> ClientInfo. extract_client_attr_from_user_property( diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index 427df5db0..ef1ba9999 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -3552,9 +3552,9 @@ mqtt_general() -> )}, {"client_attrs_init", sc( - hoconsc:union([disabled, ref("client_attrs_init")]), + hoconsc:union([hoconsc:array(ref("client_attrs_init")), ref("client_attrs_init")]), #{ - default => disabled, + default => [], desc => ?DESC("client_attrs_init") } )} diff --git a/apps/emqx/test/emqx_config_SUITE.erl b/apps/emqx/test/emqx_config_SUITE.erl index b3e60f793..a9b4a8328 100644 --- a/apps/emqx/test/emqx_config_SUITE.erl +++ b/apps/emqx/test/emqx_config_SUITE.erl @@ -454,7 +454,7 @@ zone_global_defaults() -> upgrade_qos => false, use_username_as_clientid => false, wildcard_subscription => true, - client_attrs_init => disabled + client_attrs_init => [] }, overload_protection => #{ diff --git a/apps/emqx/test/emqx_listeners_SUITE.erl b/apps/emqx/test/emqx_listeners_SUITE.erl index acd7656d7..d49b5f893 100644 --- a/apps/emqx/test/emqx_listeners_SUITE.erl +++ b/apps/emqx/test/emqx_listeners_SUITE.erl @@ -170,7 +170,7 @@ t_client_attr_as_mountpoint(_Config) -> ?assertMatch([_], emqx_router:match_routes(MatchTopic)), emqtt:stop(Client) end), - emqx_config:put_zone_conf(default, [mqtt, client_attrs_init], disabled), + emqx_config:put_zone_conf(default, [mqtt, client_attrs_init], []), ok. t_current_conns_tcp(_Config) -> diff --git a/apps/emqx_auth/test/emqx_authz/emqx_authz_SUITE.erl b/apps/emqx_auth/test/emqx_authz/emqx_authz_SUITE.erl index c88dcc244..37ac27a9b 100644 --- a/apps/emqx_auth/test/emqx_authz/emqx_authz_SUITE.erl +++ b/apps/emqx_auth/test/emqx_authz/emqx_authz_SUITE.erl @@ -572,7 +572,7 @@ t_alias_prefix(_Config) -> ?assertMatch({ok, _, [?RC_NOT_AUTHORIZED]}, emqtt:subscribe(C, SubTopicNotAllowed)), unlink(C), emqtt:stop(C), - emqx_config:put_zone_conf(default, [mqtt, client_attrs_init], disalbed), + emqx_config:put_zone_conf(default, [mqtt, client_attrs_init], []), ok. %% client is allowed by ACL to publish to its LWT topic, is connected, diff --git a/changes/ce/feat-12750.en.md b/changes/ce/feat-12750.en.md index bd7375168..d0a70e6fc 100644 --- a/changes/ce/feat-12750.en.md +++ b/changes/ce/feat-12750.en.md @@ -7,7 +7,7 @@ an MQTT connection. ### Initialization of `client_attrs` -- The `client_attrs` field can be initially populated based on the configuration from one of the +- The `client_attrs` fields can be initially populated based on the configuration from one of the following sources: - `cn`: The common name from the TLS client's certificate. - `dn`: The distinguished name from the TLS client's certificate, that is, the certificate "Subject". diff --git a/rel/i18n/emqx_schema.hocon b/rel/i18n/emqx_schema.hocon index 0bd8c74d5..90fbfeefc 100644 --- a/rel/i18n/emqx_schema.hocon +++ b/rel/i18n/emqx_schema.hocon @@ -1575,7 +1575,8 @@ client_attrs_init { label: "Client Attributes Initialization" desc: """~ Specify how to initialize client attributes. - One initial client attribute can be initialized as `client_attrs.NAME`, + This config accepts one initialization rule, or a list of rules. + Client attributes can be initialized as `client_attrs.NAME`, where `NAME` is the name of the attribute specified in the config `extract_as`. The initialized client attribute will be stored in the `client_attrs` property with the specified name, and can be used as a placeholder in a template for authentication and authorization.