Move user config fields in rlm_ldap to a separate struct

diff --git a/src/modules/rlm_ldap/rlm_ldap.c b/src/modules/rlm_ldap/rlm_ldap.c
index 502ffc5f447784896be6d1932552b7038d08886b..1024604839519454eb0a647d1e960cb622003537 100644 (file)
--- a/src/modules/rlm_ldap/rlm_ldap.c
+++ b/src/modules/rlm_ldap/rlm_ldap.c
@@ -96,14 +96,14 @@ static conf_parser_t profile_config[] = {
  *     User configuration
  */
 static conf_parser_t user_config[] = {
-       { FR_CONF_OFFSET("scope", rlm_ldap_t, userobj_scope), .dflt = "sub",
+       { FR_CONF_OFFSET("scope", rlm_ldap_t, user.obj_scope), .dflt = "sub",
          .func = cf_table_parse_int, .uctx = &(cf_table_parse_ctx_t){ .table = fr_ldap_scope, .len = &fr_ldap_scope_len } },
-       { FR_CONF_OFFSET("sort_by", rlm_ldap_t, userobj_sort_by) },
+       { FR_CONF_OFFSET("sort_by", rlm_ldap_t, user.obj_sort_by) },
 
-       { FR_CONF_OFFSET("access_attribute", rlm_ldap_t, userobj_access_attr) },
-       { FR_CONF_OFFSET("access_positive", rlm_ldap_t, access_positive), .dflt = "yes" },
-       { FR_CONF_OFFSET("access_value_negate", rlm_ldap_t, access_value_negate), .dflt = "false" },
-       { FR_CONF_OFFSET("access_value_suspend", rlm_ldap_t, access_value_suspend), .dflt = "suspended" },
+       { FR_CONF_OFFSET("access_attribute", rlm_ldap_t, user.obj_access_attr) },
+       { FR_CONF_OFFSET("access_positive", rlm_ldap_t, user.access_positive), .dflt = "yes" },
+       { FR_CONF_OFFSET("access_value_negate", rlm_ldap_t, user.access_value_negate), .dflt = "false" },
+       { FR_CONF_OFFSET("access_value_suspend", rlm_ldap_t, user.access_value_suspend), .dflt = "suspended" },
        CONF_PARSER_TERMINATOR
 };
 
@@ -1541,7 +1541,7 @@ static unlang_action_t mod_authorize_resume(rlm_rcode_t *p_result, UNUSED int *p
                /*
                 *      Check for access.
                 */
-               if (inst->userobj_access_attr) {
+               if (inst->user.obj_access_attr) {
                        autz_ctx->access_state = rlm_ldap_check_access(inst, request, autz_ctx->entry);
                        switch (autz_ctx->access_state) {
                        case LDAP_ACCESS_ALLOWED:
@@ -1831,9 +1831,9 @@ static unlang_action_t CC_HINT(nonnull) mod_authorize(rlm_rcode_t *p_result, mod
        /*
         *      Add any additional attributes we need for checking access, memberships, and profiles
         */
-       if (inst->userobj_access_attr) {
+       if (inst->user.obj_access_attr) {
                CHECK_EXPANDED_SPACE(expanded);
-               expanded->attrs[expanded->count++] = inst->userobj_access_attr;
+               expanded->attrs[expanded->count++] = inst->user.obj_access_attr;
        }
 
        if (inst->group.userobj_membership_attr && (inst->group.cacheable_dn || inst->group.cacheable_name)) {
@@ -2178,7 +2178,7 @@ static int mod_detach(module_detach_ctx_t const *mctx)
 {
        rlm_ldap_t *inst = talloc_get_type_abort(mctx->inst->data, rlm_ldap_t);
 
-       if (inst->userobj_sort_ctrl) ldap_control_free(inst->userobj_sort_ctrl);
+       if (inst->user.obj_sort_ctrl) ldap_control_free(inst->user.obj_sort_ctrl);
 
        return 0;
 }
@@ -2611,14 +2611,14 @@ static int mod_instantiate(module_inst_ctx_t const *mctx)
        /*
         *      Build the server side sort control for user objects
         */
-       if (inst->userobj_sort_by) {
+       if (inst->user.obj_sort_by) {
                LDAPSortKey     **keys;
                int             ret;
 
-               ret = ldap_create_sort_keylist(&keys, UNCONST(char *, inst->userobj_sort_by));
+               ret = ldap_create_sort_keylist(&keys, UNCONST(char *, inst->user.obj_sort_by));
                if (ret != LDAP_SUCCESS) {
                        cf_log_err(conf, "Invalid user.sort_by value \"%s\": %s",
-                                     inst->userobj_sort_by, ldap_err2string(ret));
+                                     inst->user.obj_sort_by, ldap_err2string(ret));
                        goto error;
                }
 
@@ -2626,7 +2626,7 @@ static int mod_instantiate(module_inst_ctx_t const *mctx)
                 *      Always set the control as critical, if it's not needed
                 *      the user can comment it out...
                 */
-               ret = ldap_create_sort_control(ldap_global_handle, keys, 1, &inst->userobj_sort_ctrl);
+               ret = ldap_create_sort_control(ldap_global_handle, keys, 1, &inst->user.obj_sort_ctrl);
                ldap_free_sort_keylist(keys);
                if (ret != LDAP_SUCCESS) {
                        ERROR("Failed creating server sort control: %s", ldap_err2string(ret));

diff --git a/src/modules/rlm_ldap/rlm_ldap.h b/src/modules/rlm_ldap/rlm_ldap.h
index 3c0f82e810b0c5e9c2205fe9b7f4f67b5ec0de0e..22cddf3c591a74b939e9963662152fd56f3ebd30 100644 (file)
--- a/src/modules/rlm_ldap/rlm_ldap.h
+++ b/src/modules/rlm_ldap/rlm_ldap.h
@@ -32,28 +32,25 @@ typedef struct {
                                                        //!< identify the autz or acct session the commands were
                                                        //!< issued for.
 #endif
+       struct {
+               /*
+                *      User object attributes and filters
+                */
+               char const      *obj_sort_by;                   //!< List of attributes to sort by.
+               LDAPControl     *obj_sort_ctrl;                 //!< Server side sort control.
 
-       /*
-        *      User object attributes and filters
-        */
-       char const      *userobj_sort_by;               //!< List of attributes to sort by.
-       LDAPControl     *userobj_sort_ctrl;             //!< Server side sort control.
-
-       int             userobj_scope;                  //!< Search scope.
-
-       char const      *userobj_access_attr;           //!< Attribute to check to see if the user should be locked out.
-       bool            access_positive;                //!< If true the presence of the attribute will allow access,
-                                                       //!< else it will deny access.
-
-       char const      *access_value_negate;           //!< If the value of the access_attr matches this, the result
-                                                       ///< will be negated.
-       char const      *access_value_suspend;          //!< Value that indicates suspension.  Is not affected by
-                                                       ///< access_positive and will always allow access, but will apply
-                                                       ///< a different profile.
+               int             obj_scope;                      //!< Search scope.
 
-       char const      *valuepair_attr;                //!< Generic dynamic mapping attribute, contains a RADIUS
-                                                       //!< attribute and value.
+               char const      *obj_access_attr;               //!< Attribute to check to see if the user should be locked out.
+               bool            access_positive;                //!< If true the presence of the attribute will allow access,
+                                                               //!< else it will deny access.
 
+               char const      *access_value_negate;           //!< If the value of the access_attr matches this, the result
+                                                               ///< will be negated.
+               char const      *access_value_suspend;          //!< Value that indicates suspension.  Is not affected by
+                                                               ///< access_positive and will always allow access, but will apply
+                                                               ///< a different profile.
+       } user;
 
        /*
         *      Group object attributes and filters
@@ -96,6 +93,9 @@ typedef struct {
                bool            skip_on_suspend;                //!< Don't process groups if the user is suspended.
        } group;
 
+       char const      *valuepair_attr;                //!< Generic dynamic mapping attribute, contains a RADIUS
+                                                       //!< attribute and value.
+
        /*
         *      Profiles
         */

diff --git a/src/modules/rlm_ldap/user.c b/src/modules/rlm_ldap/user.c
index fc2c8cd851c8efd7985ff3e565118ebc7cb0ef03..9b04d80636f9fce6302b543c789e83e3907186db 100644 (file)
--- a/src/modules/rlm_ldap/user.c
+++ b/src/modules/rlm_ldap/user.c
@@ -72,7 +72,7 @@ static unlang_action_t ldap_find_user_async_result(rlm_rcode_t *p_result, UNUSED
 
        cnt = ldap_count_entries(query->ldap_conn->handle, query->result);
 
-       if ((!user_ctx->inst->userobj_sort_ctrl) && (cnt > 1)) {
+       if ((!user_ctx->inst->user.obj_sort_ctrl) && (cnt > 1)) {
                REDEBUG("Ambiguous search result, returned %i unsorted entries (should return 1 or 0).  "
                        "Enable sorting, or specify a more restrictive base_dn, filter or scope", cnt);
                REDEBUG("The following entries were returned:");
@@ -158,7 +158,7 @@ unlang_action_t rlm_ldap_find_user_async(TALLOC_CTX *ctx, rlm_ldap_t const *inst
 {
        static char const       *tmp_attrs[] = { NULL };
        ldap_user_find_ctx_t    *user_ctx;
-       LDAPControl             *serverctrls[] = { inst->userobj_sort_ctrl, NULL };
+       LDAPControl             *serverctrls[] = { inst->user.obj_sort_ctrl, NULL };
 
        if (!attrs) memset(&attrs, 0, sizeof(tmp_attrs));
 
@@ -179,7 +179,7 @@ unlang_action_t rlm_ldap_find_user_async(TALLOC_CTX *ctx, rlm_ldap_t const *inst
        }
 
        return fr_ldap_trunk_search(user_ctx, &user_ctx->query, request, user_ctx->ttrunk,
-                                   user_ctx->base_dn, user_ctx->inst->userobj_scope, user_ctx->filter,
+                                   user_ctx->base_dn, user_ctx->inst->user.obj_scope, user_ctx->filter,
                                    user_ctx->attrs, serverctrls, NULL);
 }
 
@@ -197,37 +197,37 @@ ldap_access_state_t rlm_ldap_check_access(rlm_ldap_t const *inst, request_t *req
        ldap_access_state_t ret = LDAP_ACCESS_ALLOWED;
        struct berval **values = NULL;
 
-       values = ldap_get_values_len(fr_ldap_handle_thread_local(), entry, inst->userobj_access_attr);
+       values = ldap_get_values_len(fr_ldap_handle_thread_local(), entry, inst->user.obj_access_attr);
        if (values) {
-               size_t negate_value_len = talloc_array_length(inst->access_value_negate) - 1;
-               if (inst->access_positive) {
+               size_t negate_value_len = talloc_array_length(inst->user.access_value_negate) - 1;
+               if (inst->user.access_positive) {
                        if ((values[0]->bv_len >= negate_value_len) &&
-                           (strncasecmp(values[0]->bv_val, inst->access_value_negate, negate_value_len) == 0)) {
+                           (strncasecmp(values[0]->bv_val, inst->user.access_value_negate, negate_value_len) == 0)) {
                                REDEBUG("\"%s\" attribute exists but is set to '%s' - user locked out",
-                                       inst->userobj_access_attr, inst->access_value_negate);
+                                       inst->user.obj_access_attr, inst->user.access_value_negate);
                                ret = LDAP_ACCESS_DISALLOWED;
                                goto done;
                        }
                        /* RLM_MODULE_OK set above... */
                } else if ((values[0]->bv_len < negate_value_len) ||
-                          (strncasecmp(values[0]->bv_val, inst->access_value_negate, negate_value_len) != 0)) {
-                       REDEBUG("\"%s\" attribute exists - user locked out", inst->userobj_access_attr);
+                          (strncasecmp(values[0]->bv_val, inst->user.access_value_negate, negate_value_len) != 0)) {
+                       REDEBUG("\"%s\" attribute exists - user locked out", inst->user.obj_access_attr);
                        ret = LDAP_ACCESS_DISALLOWED;
                        goto done;
                }
                {
-                       size_t suspend_value_len = talloc_array_length(inst->access_value_suspend) - 1;
+                       size_t suspend_value_len = talloc_array_length(inst->user.access_value_suspend) - 1;
                        if ((values[0]->bv_len == suspend_value_len) &&
-                           (strncasecmp(values[0]->bv_val, inst->access_value_suspend, suspend_value_len) == 0)) {
-                               RIDEBUG("\"%s\" attribute exists and indicates suspension", inst->userobj_access_attr);
+                           (strncasecmp(values[0]->bv_val, inst->user.access_value_suspend, suspend_value_len) == 0)) {
+                               RIDEBUG("\"%s\" attribute exists and indicates suspension", inst->user.obj_access_attr);
                                ret = LDAP_ACCESS_SUSPENDED;
                                goto done;
                        }
                }
        done:
                ldap_value_free_len(values);
-       } else if (inst->access_positive) {
-               REDEBUG("No \"%s\" attribute - user locked out", inst->userobj_access_attr);
+       } else if (inst->user.access_positive) {
+               REDEBUG("No \"%s\" attribute - user locked out", inst->user.obj_access_attr);
                ret = LDAP_ACCESS_DISALLOWED;
        }
 
@@ -239,11 +239,12 @@ ldap_access_state_t rlm_ldap_check_access(rlm_ldap_t const *inst, request_t *req
  * Checks to see if after the LDAP to RADIUS mapping has been completed that a reference password.
  *
  * @param[in] request          Current request.
+ * @param[in] inst             Current LDAP instance.
  * @param[in] inst_name                Name of LDAP module instance for debug messages.
  * @param[in] expect_password  Whether we should be expecting a password.
  * @param[in] ttrunk           the connection thread trunk.
  */
-void rlm_ldap_check_reply(request_t *request, char const *inst_name, bool expect_password, fr_ldap_thread_trunk_t const *ttrunk)
+void rlm_ldap_check_reply(request_t *request, rlm_ldap_t const *inst, char const *inst_name, bool expect_password, fr_ldap_thread_trunk_t const *ttrunk)
 {
        fr_pair_t               *parent;
 
@@ -291,31 +292,19 @@ void rlm_ldap_check_reply(request_t *request, char const *inst_name, bool expect
                        break;
 
                default:
-                       if (!ttrunk->config.admin_identity) {
-                               RWDEBUG2("!!! Found map between LDAP attribute and a FreeRADIUS password attribute");
-                               RWDEBUG2("!!! but no password attribute found in search result");
-                               RWDEBUG2("!!! Either:");
-                               RWDEBUG2("!!!  - Ensure the user object contains a password attribute, and that");
-                               RWDEBUG2("!!!    \"%s\" has permission to read that password attribute (recommended)",
-                                        ttrunk->config.admin_identity);
-                               RWDEBUG2("!!!  - Bind as the user by listing %s in the authenticate section, and",
-                                        inst_name);
-                               RWDEBUG2("!!!   setting attribute &control.Auth-Type := '%s' in the authorize section",
-                                        inst_name);
-                               RWDEBUG2("!!!    (pap only)");
-                       } else {
-                               RWDEBUG2("!!! No \"known good\" password added");
-                               RWDEBUG2("!!! but no password attribute found in search result");
-                               RWDEBUG2("!!! Either:");
-                               RWDEBUG2("!!!  - Ensure the user object contains a password attribute, and that");
-                               RWDEBUG2("!!!    'identity' is set to the DN of an account that has permission to read");
-                               RWDEBUG2("!!!    that password attribute");
-                               RWDEBUG2("!!!  - Bind as the user by listing %s in the authenticate section, and",
-                                        inst_name);
-                               RWDEBUG2("!!!   setting attribute &control.Auth-Type := '%s' in the authorize section",
-                                        inst_name);
-                               RWDEBUG2("!!!    (pap only)");
-                       }
+                       RWDEBUG2("!!! Found map between LDAP attribute and a FreeRADIUS password attribute");
+                       RWDEBUG2("!!! but no password attribute found in search result");
+                       RWDEBUG2("!!! Either:");
+                       RWDEBUG2("!!!  - Ensure the user object contains a password attribute, and that");
+                       RWDEBUG2("!!!    %c%s%c has permission to read that password attribute (recommended)",
+                                ttrunk->config.admin_identity ? '"' : '\0',
+                                ttrunk->config.admin_identity ? ttrunk->config.admin_identity : "the bind user",
+                                ttrunk->config.admin_identity ? '"' : '\0');
+                       RWDEBUG2("!!!  - Bind as the user by listing %s in the authenticate section, and",
+                                inst_name);
+                       RWDEBUG2("!!!    setting attribute &control.Auth-Type := '%s' in the authorize section",
+                                inst_name);
+                       RWDEBUG2("!!!    (pap only)");
                        break;
                }
        }