v4: Convert %(client: ) to new xlat api (#4038)

* Convert %(client: ) to new xlat api

* Update documentation for %(client: ) xlat

* Update existing uses of %(client: ) xlat in raddb

* Update existing reference to %(client: ) in tests

* Add specific test for %(client: ) xlat

* Tidy client map test

* Fix docs

They were very wrong...

* Fix examples in config file too

Co-authored-by: Arran Cudbard-Bell <a.cudbardb@freeradius.org>

diff --git a/doc/antora/modules/raddb/pages/mods-available/client.adoc b/doc/antora/modules/raddb/pages/mods-available/client.adoc
index 6d4d675d2942719180466a485c2e2be7ab1fbf18..2e78555f4b17f2502423cc1c73a6fa2f243c7c8d 100644 (file)
--- a/doc/antora/modules/raddb/pages/mods-available/client.adoc
+++ b/doc/antora/modules/raddb/pages/mods-available/client.adoc
@@ -73,9 +73,9 @@ map client [<ipaddr>] {            // <1>
 [options="header,autowidth"]
 |===
 | XLAT                          | Description
-| `%{client:nas_type}`          | Expands to client's `nas_type` (or "" if nas_type not set).
-| `%{client:<ipaddr>.nas_type}` | Expands to the `nas_type` of the client specified
-                                  by `<ipaddr>` (or "" if `nas_type` not set).
+| `%(client:<field>)`           | Expands to a field in the client definition.
+| `%(client:<field> <ipaddr>)`  | Expands to a field in the client definition specified
+                                  by `<ipaddr>`.
 |===
 
 

diff --git a/doc/antora/modules/reference/pages/xlat/builtin.adoc b/doc/antora/modules/reference/pages/xlat/builtin.adoc
index f4912202b96b2e2418020ef4778452077cf61a6f..1205e52a6c381408beb32388a138ac3a08111f1f 100644 (file)
--- a/doc/antora/modules/reference/pages/xlat/builtin.adoc
+++ b/doc/antora/modules/reference/pages/xlat/builtin.adoc
@@ -188,7 +188,7 @@ Server installed in /opt/freeradius
 Module rlm_exec.shell_escape = yes
 ```
 
-=== %{client:<key>}
+=== %(client:<key>)
 
 Refers to a variable that was defined in the client section for the
 current client. See the sections `client { ... }` in `clients.conf`.
@@ -199,7 +199,7 @@ current client. See the sections `client { ... }` in `clients.conf`.
 
 [source,unlang]
 ----
-"The client ipaddr is %{client:ipaddr}"
+"The client ipaddr is %(client:ipaddr)"
 ----
 
 .Output
@@ -922,8 +922,8 @@ The packet type (`Access-Request`, etc.)
 === +%{Packet-SRC-IP-Address} and %{Packet-SRC-IPv6-Address}+
 
 The source IPv4 or IPv6 address of the packet. See also the expansions
-`%{client:ipaddr}` and `%{client:ipv6addr}`. The two expansions
-should be identical, unless `%{client:ipaddr}` contains a DNS hostname.
+`%(client:ipaddr)` and `%(client:ipv6addr)`. The two expansions
+should be identical, unless `%(client:ipaddr)` contains a DNS hostname.
 
 === +%{Packet-DST-IP-Address} and %{Packet-DST-IPv6-Address}+
 

diff --git a/raddb/mods-available/client b/raddb/mods-available/client
index 5cf8b2a9b00301c805dc80137e613faa12e30638..efff6539c2cb9a9d963b808fa0776124fbd0babd 100644 (file)
--- a/raddb/mods-available/client
+++ b/raddb/mods-available/client
@@ -76,9 +76,9 @@
 #  [options="header,autowidth"]
 #  |===
 #  | XLAT                          | Description
-#  | `%{client:nas_type}`          | Expands to client's `nas_type` (or "" if nas_type not set).
-#  | `%{client:<ipaddr>.nas_type}` | Expands to the `nas_type` of the client specified
-#                                    by `<ipaddr>` (or "" if `nas_type` not set).
+#  | `%(client:<field>)`           | Expands to a field in the client definition.
+#  | `%(client:<field> <ipaddr>)`  | Expands to a field in the client definition specified
+                                     by `<ipaddr>`.
 #  |===
 #
 

diff --git a/raddb/mods-config/perl/example.pl b/raddb/mods-config/perl/example.pl
index 0dc1693dfc1f5e9221abb2e7ea57ea956c1887cf..d712e5e1c51d3a8caa23d22b0ed04f0606d02268 100644 (file)
--- a/raddb/mods-config/perl/example.pl
+++ b/raddb/mods-config/perl/example.pl
@@ -126,7 +126,7 @@ sub authenticate {
                return RLM_MODULE_REJECT;
        } else {
                # Accept user and set some attribute
-               if (&radiusd::xlat("%{client:group}") eq 'UltraAllInclusive') {
+               if (&radiusd::xlat("%(client:group)") eq 'UltraAllInclusive') {
                        # User called from NAS with unlim plan set, set higher limits
                        $RAD_REPLY{'h323-credit-amount'} = "1000000";
                } else {

diff --git a/raddb/policy.d/abfab-tr b/raddb/policy.d/abfab-tr
index e84867e274a9d2b0b5dcda36c36c43ff8058a2c6..37c001340fa0e566a679148a8b4748cb1fbc2b40 100644 (file)
--- a/raddb/policy.d/abfab-tr
+++ b/raddb/policy.d/abfab-tr
@@ -25,8 +25,8 @@ abfab_psk_authorize {
 
 abfab_client_check {
        # check that the acceptor host name is correct
-       if ("%{client:gss_acceptor_host_name}" && &GSS-acceptor-host-name) {
-               if ("%{client:gss_acceptor_host_name}" != "%{gss-acceptor-host-name}") {
+       if ("%(client:gss_acceptor_host_name)" && &GSS-acceptor-host-name) {
+               if ("%(client:gss_acceptor_host_name)" != "%{gss-acceptor-host-name}") {
                        update reply {
                                &Reply-Message = "GSS-Acceptor-Host-Name incorrect"
                        }
@@ -35,16 +35,16 @@ abfab_client_check {
        }
 
        # set trust-router-coi attribute from the client configuration
-       if ("%{client:trust_router_coi}") {
+       if ("%(client:trust_router_coi)") {
                update request {
-                       &Trust-Router-COI := "%{client:trust_router_coi}"
+                       &Trust-Router-COI := "%(client:trust_router_coi)"
                }
        }
 
        # set gss-acceptor-realm-name attribute from the client configuration
-       if ("%{client:gss_acceptor_realm_name}") {
+       if ("%(client:gss_acceptor_realm_name)") {
                update request {
-                       &GSS-Acceptor-Realm-Name := "%{client:gss_acceptor_realm_name}"
+                       &GSS-Acceptor-Realm-Name := "%(client:gss_acceptor_realm_name)"
                }
        }
 }

diff --git a/raddb/policy.d/cui b/raddb/policy.d/cui
index ea834f403e9474e0189b28fdb21fb0a187de6d28..b26821e9eb36dcccbf3eb759148f1776f0980ef6 100644 (file)
--- a/raddb/policy.d/cui
+++ b/raddb/policy.d/cui
@@ -39,7 +39,7 @@ cui_require_operator_name = "no"
 #  }
 #
 cui.authorize {
-       if ("%{client:add_cui}" == 'yes') {
+       if ("%(client:add_cui)" == 'yes') {
                update request {
                        &Chargeable-User-Identity := 0x00
                }

diff --git a/raddb/policy.d/operator-name b/raddb/policy.d/operator-name
index 47f519aecb41af57e865c33e763d124abed5f2be..279e2932dc2c1f6097d058a54fb3d9c2a6a15813 100644 (file)
--- a/raddb/policy.d/operator-name
+++ b/raddb/policy.d/operator-name
@@ -26,9 +26,9 @@
 #  an Operator-Name attribute
 #
 operator-name.authorize {
-       if ("%{client:Operator-Name}") {
+       if ("%(client:Operator-Name)") {
                update request {
-                       &Operator-Name = "%{client:Operator-Name}"
+                       &Operator-Name = "%(client:Operator-Name)"
                }
        }
 }

diff --git a/src/modules/rlm_client/rlm_client.c b/src/modules/rlm_client/rlm_client.c
index 07c39d10904d37a57ecdb845257efe3631448327..dd040b1c43538b32e162513a815bab562c84afa6 100644 (file)
--- a/src/modules/rlm_client/rlm_client.c
+++ b/src/modules/rlm_client/rlm_client.c
@@ -207,63 +207,72 @@ finish:
        return rcode;
 }
 
+static xlat_arg_parser_t const xlat_client_args[] = {
+       { .required = true, .single = true, .type = FR_TYPE_STRING },
+       { .single = true, .type = FR_TYPE_STRING },
+       XLAT_ARG_PARSER_TERMINATOR
+};
+
 /** xlat to get client config data
  *
  * Example:
 @verbatim
-%{client:[<ipaddr>.]foo}
+%(client:foo [<ipaddr>])
 @endverbatim
  *
  * @ingroup xlat_functions
  */
-static ssize_t xlat_client(TALLOC_CTX *ctx, char **out, UNUSED size_t outlen,
-                          UNUSED void const *mod_inst, UNUSED void const *xlat_inst,
-                          request_t *request, char const *fmt)
+static xlat_action_t xlat_client(TALLOC_CTX *ctx, fr_dcursor_t *out, request_t *request,
+                                UNUSED void const *xlat_inst, UNUSED void *xlat_thread_inst,
+                                fr_value_box_list_t *in)
 {
        char const      *value = NULL;
-       char            buffer[INET6_ADDRSTRLEN], *q;
-       char const      *p = fmt;
        fr_ipaddr_t     ip;
        CONF_PAIR       *cp;
        RADCLIENT       *client = NULL;
-
-       *out = NULL;
-
-       q = strrchr(p, '.');
-       if (q) {
-               strlcpy(buffer, p, (q + 1) - p);
-               if (fr_inet_pton(&ip, buffer, -1, AF_UNSPEC, false, true) < 0) goto request_client;
-
-               p = q + 1;
+       fr_value_box_t  *field = fr_dlist_head(in);
+       fr_value_box_t  *client_ip = fr_dlist_next(in, field);
+       fr_value_box_t  *vb;
+
+       if (client_ip) {
+               if (fr_inet_pton(&ip, client_ip->vb_strvalue, -1, AF_UNSPEC, false, true) < 0) {
+                       RDEBUG("Invalid client IP address \"%s\"", client_ip->vb_strvalue);
+                       return XLAT_ACTION_FAIL;
+               }
 
                client = client_find(NULL, &ip, IPPROTO_IP);
                if (!client) {
-                       RDEBUG("No client found with IP \"%s\"", buffer);
-                       return 0;
+                       RDEBUG("No client found with IP \"%s\"", client_ip->vb_strvalue);
+                       return XLAT_ACTION_FAIL;
                }
        } else {
-       request_client:
                client = request->client;
                if (!client) {
                        RERROR("No client associated with this request");
-
-                       return -1;
+                       return XLAT_ACTION_FAIL;
                }
        }
 
-       cp = cf_pair_find(client->cs, p);
+       cp = cf_pair_find(client->cs, field->vb_strvalue);
        if (!cp || !(value = cf_pair_value(cp))) {
-               if (strcmp(fmt, "shortname") == 0 && request->client->shortname) {
+               if (strcmp(field->vb_strvalue, "shortname") == 0 && request->client->shortname) {
                        value = request->client->shortname;
                }
-               else if (strcmp(fmt, "nas_type") == 0 && request->client->nas_type) {
+               else if (strcmp(field->vb_strvalue, "nas_type") == 0 && request->client->nas_type) {
                        value = request->client->nas_type;
                }
-               if (!value) return 0;
+               if (!value) return XLAT_ACTION_DONE;
+       }
+
+       MEM(vb = fr_value_box_alloc_null(ctx));
+
+       if (fr_value_box_strdup(ctx, vb, NULL, value, false) < 0) {
+               talloc_free(vb);
+               return XLAT_ACTION_FAIL;
        }
 
-       *out = talloc_typed_strdup(ctx, value);
-       return talloc_array_length(*out) - 1;
+       fr_dcursor_append(out, vb);
+       return XLAT_ACTION_DONE;
 }
 
 
@@ -343,7 +352,9 @@ static unlang_action_t CC_HINT(nonnull) mod_authorize(rlm_rcode_t *p_result, UNU
  */
 static int mod_bootstrap(void *instance, UNUSED CONF_SECTION *conf)
 {
-       xlat_register_legacy(instance, "client", xlat_client, NULL, NULL, 0, 0);
+       xlat_t  *xlat;
+       xlat = xlat_register(instance, "client", xlat_client, false);
+       xlat_func_args(xlat, xlat_client_args);
        map_proc_register(instance, "client", map_proc_client, NULL, 0);
 
        return 0;

diff --git a/src/modules/rlm_perl/rlm_perl.c b/src/modules/rlm_perl/rlm_perl.c
index 10783e4b19bde5ce9e0f5ddef45c92ac418b786d..2d7363a44921021b15922cdae04ec7f3d4a3bead 100644 (file)
--- a/src/modules/rlm_perl/rlm_perl.c
+++ b/src/modules/rlm_perl/rlm_perl.c
@@ -311,7 +311,7 @@ static XS(XS_radiusd_log)
 /*
  *     This is a wraper for xlat_aeval
  *     Now users are able to get data that is accessible only via xlat
- *     e.g. %{client:...}
+ *     e.g. %(client:...)
  *     Call syntax is radiusd::xlat(string), string will be handled the
  *     same way it is described in EXPANSIONS section of man unlang
  */

diff --git a/src/tests/modules/client/map.unlang b/src/tests/modules/client/map.unlang
index 0990589be3f1d59516cb31d96df31ad2bba9ab3a..fc9c52344c7d793304dc3101f2c702326c0ed125 100644 (file)
--- a/src/tests/modules/client/map.unlang
+++ b/src/tests/modules/client/map.unlang
@@ -2,10 +2,7 @@ map client {
        &Tmp-String-0 := 'nas_type'
 }
 
-if (&Tmp-String-0 == 'a_type') {
-       test_pass
-}
-else {
+if (&Tmp-String-0 != 'a_type') {
        test_fail
 }
 
@@ -13,10 +10,7 @@ map client 127.0.0.1 {
        &Tmp-String-0 := 'nas_type'
 }
 
-if (&Tmp-String-0 == 'a_type') {
-       test_pass
-}
-else {
+if (&Tmp-String-0 != 'a_type') {
        test_fail
 }
 
@@ -24,10 +18,7 @@ map client 127.0.0.2 {
        &Tmp-String-0 := 'nas_type'
 }
 
-if (&Tmp-String-0 == 'b_type') {
-       test_pass
-}
-else {
+if (&Tmp-String-0 != 'b_type') {
        test_fail
 }
 
@@ -35,10 +26,7 @@ map client 127.0.0.5 {
        &Tmp-String-0 := 'nas_type'
 }
 
-if (&Tmp-String-0 == 'b_type') {
-       test_pass
-}
-else {
+if (&Tmp-String-0 != 'b_type') {
        test_fail
 }
 
@@ -49,24 +37,15 @@ map client {
        &Tmp-String-1 += 'group'
 }
 
-if (&Tmp-String-1[0] == 'a') {
-       test_pass
-}
-else {
+if (&Tmp-String-1[0] != 'a') {
        test_fail
 }
 
-if (&Tmp-String-1[1] == 'b') {
-       test_pass
-}
-else {
+if (&Tmp-String-1[1] != 'b') {
        test_fail
 }
 
-if (&Tmp-String-1[2] == 'c') {
-       test_pass
-}
-else {
+if (&Tmp-String-1[2] != 'c') {
        test_fail
 }
 
@@ -74,24 +53,15 @@ map client 127.0.0.2 {
        &Tmp-String-2 += 'group'
 }
 
-if (&Tmp-String-2[0] == 'd') {
-       test_pass
-}
-else {
+if (&Tmp-String-2[0] != 'd') {
        test_fail
 }
 
-if (&Tmp-String-2[1] == 'e') {
-       test_pass
-}
-else {
+if (&Tmp-String-2[1] != 'e') {
        test_fail
 }
 
-if (&Tmp-String-2[2] == 'f') {
-       test_pass
-}
-else {
+if (&Tmp-String-2[2] != 'f') {
        test_fail
 }
 
@@ -106,14 +76,9 @@ map client {
 if (&Tmp-String-3) {
        test_fail
 }
-else {
-       test_pass
-}
 
 if (&Tmp-String-4) {
        test_fail
 }
-else {
-       test_pass
-}
 
+test_pass
\ No newline at end of file

diff --git a/src/tests/modules/client/xlat.unlang b/src/tests/modules/client/xlat.unlang
new file mode 100644 (file)
index 0000000..d705c4a
--- /dev/null
+++ b/src/tests/modules/client/xlat.unlang
@@ -0,0 +1,53 @@
+update request {
+        &Tmp-String-0 := "%(client:nas_type)"
+}
+
+if (&Tmp-String-0 != 'a_type') {
+        test_fail
+}
+
+update request {
+        &Tmp-String-0 := "%(client:nas_type 127.0.0.1)"
+}
+
+if (&Tmp-String-0 != 'a_type') {
+        test_fail
+}
+
+update request {
+        &Tmp-String-0 := "%(client:nas_type 127.0.0.2)"
+}
+
+if (&Tmp-String-0 != 'b_type') {
+        test_fail
+}
+
+update request {
+       &Tmp-String-0 := "%(client:nas_type 127.0.0.5)"
+}
+
+if (&Tmp-String-0 != 'b_type') {
+       test_fail
+}
+
+#
+#  Test non-existent client properties
+#
+update request {
+       &Tmp-String-3 := "%(client:non-existent-attr)"
+       &Tmp-String-4 += "%(client:non-existing-attr2)"
+}
+
+if (&Tmp-String-3 != "") {
+       test_fail
+}
+
+if (&Tmp-String-4 != "") {
+       test_fail
+}
+
+#
+#  Tests for multi-valued keys to be added when update takes multiple values
+#
+
+test_pass
\ No newline at end of file

diff --git a/src/tests/modules/perl/test.pl b/src/tests/modules/perl/test.pl
index 40274fdb69b15afb7d8557cfcc1c7faa272cd180..9eecf91dcb44b11890be85d4ee196f8be83de72c 100644 (file)
--- a/src/tests/modules/perl/test.pl
+++ b/src/tests/modules/perl/test.pl
@@ -70,7 +70,7 @@ sub authenticate {
                return RLM_MODULE_REJECT;
        } else {
                # Accept user and set some attribute
-               if (&radiusd::xlat("%{client:group}") eq 'UltraAllInclusive') {
+               if (&radiusd::xlat("%(client:group)") eq 'UltraAllInclusive') {
                        # User called from NAS with unlim plan set, set higher limits
                        $RAD_REPLY{'h323-credit-amount'} = "1000000";
                } else {