From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Fri, 23 Feb 2024 22:36:12 +0000 (-0700)
Subject: Skip LDAP group processing on suspension
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4dc80918e0c6e15ef5bc5c0ea56da9c013577ee3;p=thirdparty%2Ffreeradius-server.git

Skip LDAP group processing on suspension
---

diff --git a/raddb/mods-available/ldap b/raddb/mods-available/ldap
index 76ffbe57603..b68260426d8 100644
--- a/raddb/mods-available/ldap
+++ b/raddb/mods-available/ldap
@@ -476,6 +476,14 @@ ldap {
 		#  `(<inst>-Group` or `LDAP-Group` if using the default instance).
 		#
 		group_attribute = "${..:instance}-Group"
+
+		#
+		#  skip_on_suspend::
+		#
+		#  Don't process user groups if the user has been suspended.
+		#  If set to 'no', groups will still be processed.
+		#
+		skip_on_suspend = 'yes'
 	}
 
 	#
diff --git a/src/modules/rlm_ldap/groups.c b/src/modules/rlm_ldap/groups.c
index 2f342e1a86b..e7dbcb9dc36 100644
--- a/src/modules/rlm_ldap/groups.c
+++ b/src/modules/rlm_ldap/groups.c
@@ -29,7 +29,6 @@ RCSID("$Id$")
 USES_APPLE_DEPRECATED_API
 
 #include <freeradius-devel/util/debug.h>
-#include <ctype.h>
 
 #define LOG_PREFIX "rlm_ldap groups"
 
@@ -117,7 +116,7 @@ static unlang_action_t ldap_group_name2dn_start(rlm_rcode_t *p_result, UNUSED in
 	char				buffer[LDAP_MAX_GROUP_NAME_LEN + 1];
 	char				*filter;
 
-	if (!inst->groupobj_name_attr) {
+	if (!inst->group.obj_name_attr) {
 		REDEBUG("Told to convert group names to DNs but missing 'group.name_attribute' directive");
 		RETURN_MODULE_INVALID;
 	}
@@ -133,21 +132,21 @@ static unlang_action_t ldap_group_name2dn_start(rlm_rcode_t *p_result, UNUSED in
 	 *	for the entire group list at once.
 	 */
 	filter = talloc_typed_asprintf(group_ctx, "%s%s%s",
-				 inst->groupobj_filter ? "(&" : "",
-				 inst->groupobj_filter ? inst->groupobj_filter : "",
+				 inst->group.obj_filter ? "(&" : "",
+				 inst->group.obj_filter ? inst->group.obj_filter : "",
 				 group_ctx->group_name[0] && group_ctx->group_name[1] ? "(|" : "");
 	while (*name) {
 		fr_ldap_uri_escape_func(request, buffer, sizeof(buffer), *name++, NULL);
-		filter = talloc_asprintf_append_buffer(filter, "(%s=%s)", inst->groupobj_name_attr, buffer);
+		filter = talloc_asprintf_append_buffer(filter, "(%s=%s)", inst->group.obj_name_attr, buffer);
 
 		group_ctx->name_cnt++;
 	}
 	filter = talloc_asprintf_append_buffer(filter, "%s%s",
-					       inst->groupobj_filter ? ")" : "",
+					       inst->group.obj_filter ? ")" : "",
 					       group_ctx->group_name[0] && group_ctx->group_name[1] ? ")" : "");
 
 	return fr_ldap_trunk_search(group_ctx, &group_ctx->query, request, group_ctx->ttrunk,
-				    group_ctx->base_dn->vb_strvalue, inst->groupobj_scope, filter,
+				    group_ctx->base_dn->vb_strvalue, inst->group.obj_scope, filter,
 				    null_attrs, NULL, NULL);
 }
 
@@ -220,7 +219,7 @@ static unlang_action_t ldap_group_name2dn_resume(rlm_rcode_t *p_result, UNUSED i
 		fr_ldap_util_normalise_dn(dn, dn);
 
 		RDEBUG2("Got group DN \"%s\"", dn);
-		MEM(vp = fr_pair_afrom_da(group_ctx->list_ctx, inst->cache_da));
+		MEM(vp = fr_pair_afrom_da(group_ctx->list_ctx, inst->group.cache_da));
 		fr_pair_value_bstrndup(vp, dn, strlen(dn), true);
 		fr_pair_append(&group_ctx->groups, vp);
 		ldap_memfree(dn);
@@ -254,7 +253,7 @@ static unlang_action_t ldap_group_dn2name_start(rlm_rcode_t *p_result, UNUSED in
 	ldap_group_userobj_ctx_t	*group_ctx = talloc_get_type_abort(uctx, ldap_group_userobj_ctx_t);
 	rlm_ldap_t const		*inst = group_ctx->inst;
 
-	if (!inst->groupobj_name_attr) {
+	if (!inst->group.obj_name_attr) {
 		REDEBUG("Told to resolve group DN to name but missing 'group.name_attribute' directive");
 		RETURN_MODULE_INVALID;
 	}
@@ -295,7 +294,7 @@ static unlang_action_t ldap_group_dn2name_resume(rlm_rcode_t *p_result, UNUSED i
 	case LDAP_RESULT_NO_RESULT:
 	case LDAP_RESULT_BAD_DN:
 		REDEBUG("Group DN \"%s\" did not resolve to an object", *group_ctx->dn);
-		rcode = (inst->allow_dangling_group_refs ? RLM_MODULE_NOOP : RLM_MODULE_INVALID);
+		rcode = (inst->group.allow_dangling_refs ? RLM_MODULE_NOOP : RLM_MODULE_INVALID);
 		goto finish;
 
 	default:
@@ -311,14 +310,14 @@ static unlang_action_t ldap_group_dn2name_resume(rlm_rcode_t *p_result, UNUSED i
 		goto finish;
 	}
 
-	values = ldap_get_values_len(query->ldap_conn->handle, entry, inst->groupobj_name_attr);
+	values = ldap_get_values_len(query->ldap_conn->handle, entry, inst->group.obj_name_attr);
 	if (!values) {
-		REDEBUG("No %s attributes found in object", inst->groupobj_name_attr);
+		REDEBUG("No %s attributes found in object", inst->group.obj_name_attr);
 		rcode = RLM_MODULE_INVALID;
 		goto finish;
 	}
 
-	MEM(vp = fr_pair_afrom_da(group_ctx->list_ctx, inst->cache_da));
+	MEM(vp = fr_pair_afrom_da(group_ctx->list_ctx, inst->group.cache_da));
 	fr_pair_value_bstrndup(vp, values[0]->bv_val, values[0]->bv_len, true);
 	fr_pair_append(&group_ctx->groups, vp);
 	RDEBUG2("Group DN \"%s\" resolves to name \"%pV\"", *group_ctx->dn, &vp->data);
@@ -358,7 +357,7 @@ static unlang_action_t ldap_cacheable_userobj_store(rlm_rcode_t *p_result, reque
 		for (vp = fr_pair_list_head(&group_ctx->groups);
 		     vp;
 		     vp = fr_pair_list_next(&group_ctx->groups, vp)) {
-			RDEBUG2("&control.%s += \"%pV\"", group_ctx->inst->cache_da->name, &vp->data);
+			RDEBUG2("&control.%s += \"%pV\"", group_ctx->inst->group.cache_da->name, &vp->data);
 		}
 	}
 
@@ -493,12 +492,12 @@ unlang_action_t rlm_ldap_cacheable_userobj(rlm_rcode_t *p_result, request_t *req
 	for (i = 0; (i < count); i++) {
 		is_dn = fr_ldap_util_is_dn(values[i]->bv_val, values[i]->bv_len);
 
-		if (inst->cacheable_group_dn) {
+		if (inst->group.cacheable_dn) {
 			/*
 			 *	The easy case, we're caching DNs and we got a DN.
 			 */
 			if (is_dn) {
-				MEM(vp = fr_pair_afrom_da(group_ctx->list_ctx, inst->cache_da));
+				MEM(vp = fr_pair_afrom_da(group_ctx->list_ctx, inst->group.cache_da));
 				fr_pair_value_bstrndup(vp, values[i]->bv_val, values[i]->bv_len, true);
 				fr_pair_append(&group_ctx->groups, vp);
 			/*
@@ -517,12 +516,12 @@ unlang_action_t rlm_ldap_cacheable_userobj(rlm_rcode_t *p_result, request_t *req
 			}
 		}
 
-		if (inst->cacheable_group_name) {
+		if (inst->group.cacheable_name) {
 			/*
 			 *	The easy case, we're caching names and we got a name.
 			 */
 			if (!is_dn) {
-				MEM(vp = fr_pair_afrom_da(group_ctx->list_ctx, inst->cache_da));
+				MEM(vp = fr_pair_afrom_da(group_ctx->list_ctx, inst->group.cache_da));
 				fr_pair_value_bstrndup(vp, values[i]->bv_val, values[i]->bv_len, true);
 				fr_pair_append(&group_ctx->groups, vp);
 			/*
@@ -547,7 +546,7 @@ unlang_action_t rlm_ldap_cacheable_userobj(rlm_rcode_t *p_result, request_t *req
 	 *	do the resolution.
 	 */
 	if ((name_p != group_ctx->group_name) || (dn_p != group_ctx->group_dn)) {
-		group_ctx->attrs[0] = inst->groupobj_name_attr;
+		group_ctx->attrs[0] = inst->group.obj_name_attr;
 		if (unlang_function_push(request, ldap_cacheable_userobj_resolve, NULL, ldap_group_userobj_cancel,
 					 ~FR_SIGNAL_CANCEL, UNLANG_SUB_FRAME, group_ctx) < 0) {
 			talloc_free(group_ctx);
@@ -577,9 +576,9 @@ static unlang_action_t ldap_cacheable_groupobj_start(UNUSED rlm_rcode_t *p_resul
 	ldap_group_groupobj_ctx_t	*group_ctx = talloc_get_type_abort(uctx, ldap_group_groupobj_ctx_t);
 	rlm_ldap_t const		*inst = group_ctx->inst;
 
-	group_ctx->attrs[0] = inst->groupobj_name_attr;
+	group_ctx->attrs[0] = inst->group.obj_name_attr;
 	return fr_ldap_trunk_search(group_ctx, &group_ctx->query, request, group_ctx->ttrunk,
-				    group_ctx->base_dn->vb_strvalue, inst->groupobj_scope,
+				    group_ctx->base_dn->vb_strvalue, inst->group.obj_scope,
 				    group_ctx->filter, group_ctx->attrs, NULL, NULL);
 }
 
@@ -643,7 +642,7 @@ static unlang_action_t ldap_cacheable_groupobj_resume(rlm_rcode_t *p_result, UNU
 
 	RDEBUG2("Adding cacheable group object memberships");
 	do {
-		if (inst->cacheable_group_dn) {
+		if (inst->group.cacheable_dn) {
 			dn = ldap_get_dn(query->ldap_conn->handle, entry);
 			if (!dn) {
 				ldap_get_option(query->ldap_conn->handle, LDAP_OPT_RESULT_CODE, &ldap_errno);
@@ -653,7 +652,7 @@ static unlang_action_t ldap_cacheable_groupobj_resume(rlm_rcode_t *p_result, UNU
 			}
 			fr_ldap_util_normalise_dn(dn, dn);
 
-			MEM(pair_append_control(&vp, inst->cache_da) == 0);
+			MEM(pair_append_control(&vp, inst->group.cache_da) == 0);
 			fr_pair_value_strdup(vp, dn, false);
 
 			RINDENT();
@@ -662,13 +661,13 @@ static unlang_action_t ldap_cacheable_groupobj_resume(rlm_rcode_t *p_result, UNU
 			ldap_memfree(dn);
 		}
 
-		if (inst->cacheable_group_name) {
+		if (inst->group.cacheable_name) {
 			struct berval **values;
 
-			values = ldap_get_values_len(query->ldap_conn->handle, entry, inst->groupobj_name_attr);
+			values = ldap_get_values_len(query->ldap_conn->handle, entry, inst->group.obj_name_attr);
 			if (!values) continue;
 
-			MEM(pair_append_control(&vp, inst->cache_da) == 0);
+			MEM(pair_append_control(&vp, inst->group.cache_da) == 0);
 			fr_pair_value_bstrndup(vp, values[0]->bv_val, values[0]->bv_len, true);
 
 			RINDENT();
@@ -696,9 +695,9 @@ unlang_action_t rlm_ldap_cacheable_groupobj(rlm_rcode_t *p_result, request_t *re
 {
 	rlm_ldap_t const		*inst = autz_ctx->inst;
 	ldap_group_groupobj_ctx_t	*group_ctx;
-	char const			*filters[] = { inst->groupobj_filter, inst->groupobj_membership_filter };
+	char const			*filters[] = { inst->group.obj_filter, inst->group.obj_membership_filter };
 
-	if (!inst->groupobj_membership_filter) {
+	if (!inst->group.obj_membership_filter) {
 		RDEBUG2("Skipping caching group objects as directive 'group.membership_filter' is not set");
 		RETURN_MODULE_OK;
 	}
@@ -796,7 +795,7 @@ unlang_action_t rlm_ldap_check_groupobj_dynamic(rlm_rcode_t *p_result, request_t
 	};
 
 	if (fr_ldap_util_is_dn(xlat_ctx->group->vb_strvalue, xlat_ctx->group->vb_length)) {
-		char const *filters[] = { inst->groupobj_filter, inst->groupobj_membership_filter };
+		char const *filters[] = { inst->group.obj_filter, inst->group.obj_membership_filter };
 
 		RINDENT();
 		ret = fr_ldap_xlat_filter(request,
@@ -813,9 +812,9 @@ unlang_action_t rlm_ldap_check_groupobj_dynamic(rlm_rcode_t *p_result, request_t
 		group_ctx->base_dn = xlat_ctx->group;
 	} else {
 		char name_filter[LDAP_MAX_FILTER_STR_LEN];
-		char const *filters[] = { name_filter, inst->groupobj_filter, inst->groupobj_membership_filter };
+		char const *filters[] = { name_filter, inst->group.obj_filter, inst->group.obj_membership_filter };
 
-		if (!inst->groupobj_name_attr) {
+		if (!inst->group.obj_name_attr) {
 			REDEBUG("Told to search for group by name, but missing 'group.name_attribute' "
 				"directive");
 
@@ -823,7 +822,7 @@ unlang_action_t rlm_ldap_check_groupobj_dynamic(rlm_rcode_t *p_result, request_t
 		}
 
 		snprintf(name_filter, sizeof(name_filter), "(%s=%s)",
-			 inst->groupobj_name_attr, xlat_ctx->group->vb_strvalue);
+			 inst->group.obj_name_attr, xlat_ctx->group->vb_strvalue);
 		RINDENT();
 		ret = fr_ldap_xlat_filter(request,
 					  filters, NUM_ELEMENTS(filters),
@@ -851,7 +850,7 @@ static unlang_action_t ldap_dn2name_start (rlm_rcode_t *p_result, UNUSED int *pr
 	ldap_memberof_xlat_ctx_t	*xlat_ctx = group_ctx->xlat_ctx;
 	rlm_ldap_t const		*inst = xlat_ctx->inst;
 
-	if (!inst->groupobj_name_attr) {
+	if (!inst->group.obj_name_attr) {
 		REDEBUG("Told to resolve group DN to name but missing 'group.name_attribute' directive");
 		RETURN_MODULE_INVALID;
 	}
@@ -918,7 +917,7 @@ static unlang_action_t ldap_check_userobj_resume(rlm_rcode_t *p_result, UNUSED i
 			RETURN_MODULE_FAIL;
 		}
 
-		group_ctx->values = ldap_get_values_len(query->ldap_conn->handle, entry, inst->userobj_membership_attr);
+		group_ctx->values = ldap_get_values_len(query->ldap_conn->handle, entry, inst->group.userobj_membership_attr);
 		if (!group_ctx->values) {
 			RDEBUG2("No group membership attribute(s) found in user object");
 			RETURN_MODULE_FAIL;
@@ -960,9 +959,9 @@ static unlang_action_t ldap_check_userobj_resume(rlm_rcode_t *p_result, UNUSED i
 			RETURN_MODULE_INVALID;
 		}
 
-		values = ldap_get_values_len(group_ctx->query->ldap_conn->handle, entry, inst->groupobj_name_attr);
+		values = ldap_get_values_len(group_ctx->query->ldap_conn->handle, entry, inst->group.obj_name_attr);
 		if (!values) {
-			REDEBUG("No %s attributes found in object", inst->groupobj_name_attr);
+			REDEBUG("No %s attributes found in object", inst->group.obj_name_attr);
 			RETURN_MODULE_INVALID;
 		}
 
@@ -1006,7 +1005,7 @@ static unlang_action_t ldap_check_userobj_resume(rlm_rcode_t *p_result, UNUSED i
 
 		value_is_dn = fr_ldap_util_is_dn(value->bv_val, value->bv_len);
 
-		RDEBUG2("Processing %s value \"%pV\" as a %s", inst->userobj_membership_attr,
+		RDEBUG2("Processing %s value \"%pV\" as a %s", inst->group.userobj_membership_attr,
 			fr_box_strvalue_len(value->bv_val, value->bv_len),
 			value_is_dn ? "DN" : "group name");
 
@@ -1115,10 +1114,10 @@ unlang_action_t rlm_ldap_check_userobj_dynamic(rlm_rcode_t *p_result, request_t
 
 	*group_ctx = (ldap_group_userobj_dyn_ctx_t) {
 		.xlat_ctx = xlat_ctx,
-		.attrs = { inst->groupobj_name_attr, NULL }
+		.attrs = { inst->group.obj_name_attr, NULL }
 	};
 
-	RDEBUG2("Checking user object's %s attributes", inst->userobj_membership_attr);
+	RDEBUG2("Checking user object's %s attributes", inst->group.userobj_membership_attr);
 
 	/*
 	 *	If a previous query was required to find the user DN, that will have
@@ -1153,7 +1152,7 @@ unlang_action_t rlm_ldap_check_cached(rlm_rcode_t *p_result,
 	 *	We return RLM_MODULE_INVALID here as an indication
 	 *	the caller should try a dynamic group lookup instead.
 	 */
-	vp =  fr_pair_dcursor_by_da_init(&cursor, &request->control_pairs, inst->cache_da);
+	vp =  fr_pair_dcursor_by_da_init(&cursor, &request->control_pairs, inst->group.cache_da);
 	if (!vp) RETURN_MODULE_INVALID;
 
 	for (vp = fr_dcursor_current(&cursor);
diff --git a/src/modules/rlm_ldap/rlm_ldap.c b/src/modules/rlm_ldap/rlm_ldap.c
index 99e83529e22..5f5b433b92f 100644
--- a/src/modules/rlm_ldap/rlm_ldap.c
+++ b/src/modules/rlm_ldap/rlm_ldap.c
@@ -109,18 +109,19 @@ static conf_parser_t user_config[] = {
  *	Group configuration
  */
 static conf_parser_t group_config[] = {
-	{ FR_CONF_OFFSET("filter", rlm_ldap_t, groupobj_filter) },
-	{ FR_CONF_OFFSET("scope", rlm_ldap_t, groupobj_scope), .dflt = "sub",
+	{ FR_CONF_OFFSET("filter", rlm_ldap_t, group.obj_filter) },
+	{ FR_CONF_OFFSET("scope", rlm_ldap_t, group.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("name_attribute", rlm_ldap_t, groupobj_name_attr), .dflt = "cn" },
-	{ FR_CONF_OFFSET("membership_attribute", rlm_ldap_t, userobj_membership_attr) },
-	{ FR_CONF_OFFSET_FLAGS("membership_filter", CONF_FLAG_XLAT, rlm_ldap_t, groupobj_membership_filter) },
-	{ FR_CONF_OFFSET("cacheable_name", rlm_ldap_t, cacheable_group_name), .dflt = "no" },
-	{ FR_CONF_OFFSET("cacheable_dn", rlm_ldap_t, cacheable_group_dn), .dflt = "no" },
-	{ FR_CONF_OFFSET("cache_attribute", rlm_ldap_t, cache_attribute) },
-	{ FR_CONF_OFFSET("group_attribute", rlm_ldap_t, group_attribute) },
-	{ FR_CONF_OFFSET("allow_dangling_group_ref", rlm_ldap_t, allow_dangling_group_refs), .dflt = "no" },
+	{ FR_CONF_OFFSET("name_attribute", rlm_ldap_t, group.obj_name_attr), .dflt = "cn" },
+	{ FR_CONF_OFFSET("membership_attribute", rlm_ldap_t, group.userobj_membership_attr) },
+	{ FR_CONF_OFFSET_FLAGS("membership_filter", CONF_FLAG_XLAT, rlm_ldap_t, group.obj_membership_filter) },
+	{ FR_CONF_OFFSET("cacheable_name", rlm_ldap_t, group.cacheable_name), .dflt = "no" },
+	{ FR_CONF_OFFSET("cacheable_dn", rlm_ldap_t, group.cacheable_dn), .dflt = "no" },
+	{ FR_CONF_OFFSET("cache_attribute", rlm_ldap_t, group.cache_attribute) },
+	{ FR_CONF_OFFSET("group_attribute", rlm_ldap_t, group.attribute) },
+	{ FR_CONF_OFFSET("allow_dangling_group_ref", rlm_ldap_t, group.allow_dangling_refs), .dflt = "no" },
+	{ FR_CONF_OFFSET("skip_on_suspend", rlm_ldap_t, group.skip_on_suspend), .dflt = "yes"},
 	CONF_PARSER_TERMINATOR
 };
 
@@ -805,7 +806,7 @@ static unlang_action_t ldap_memberof_xlat_results(rlm_rcode_t *p_result, UNUSED
 		if (!xlat_ctx->dn) xlat_ctx->dn = rlm_find_user_dn_cached(request);
 		if (!xlat_ctx->dn) RETURN_MODULE_FAIL;
 
-		if (inst->groupobj_membership_filter) {
+		if (inst->group.obj_membership_filter) {
 			REPEAT_LDAP_MEMBEROF_XLAT_RESULTS;
 			if (rlm_ldap_check_groupobj_dynamic(&rcode, request, xlat_ctx) == UNLANG_ACTION_PUSHED_CHILD) {
 				xlat_ctx->status = GROUP_XLAT_MEMB_FILTER;
@@ -820,7 +821,7 @@ static unlang_action_t ldap_memberof_xlat_results(rlm_rcode_t *p_result, UNUSED
 			goto finish;
 		}
 
-		if (inst->userobj_membership_attr) {
+		if (inst->group.userobj_membership_attr) {
 			REPEAT_LDAP_MEMBEROF_XLAT_RESULTS;
 			if (rlm_ldap_check_userobj_dynamic(&rcode, request, xlat_ctx) == UNLANG_ACTION_PUSHED_CHILD) {
 				xlat_ctx->status = GROUP_XLAT_MEMB_ATTR;
@@ -899,7 +900,7 @@ static xlat_action_t ldap_memberof_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, xlat
 		fr_value_box_bstr_realloc(group_vb, NULL, group_vb, len);
 	}
 
-	if ((group_is_dn && inst->cacheable_group_dn) || (!group_is_dn && inst->cacheable_group_name)) {
+	if ((group_is_dn && inst->group.cacheable_dn) || (!group_is_dn && inst->group.cacheable_name)) {
 		rlm_rcode_t our_rcode;
 
 		rlm_ldap_check_cached(&our_rcode, inst, request, group_vb);
@@ -928,7 +929,7 @@ static xlat_action_t ldap_memberof_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, xlat
 		.inst = inst,
 		.group = group_vb,
 		.dn = rlm_find_user_dn_cached(request),
-		.attrs = { inst->userobj_membership_attr, NULL },
+		.attrs = { inst->group.userobj_membership_attr, NULL },
 		.group_is_dn = group_is_dn,
 		.env_data = env_data
 	};
@@ -1524,7 +1525,10 @@ static unlang_action_t mod_authorize_resume(rlm_rcode_t *p_result, UNUSED int *p
 			autz_ctx->access_state = rlm_ldap_check_access(inst, request, autz_ctx->entry);
 			switch (autz_ctx->access_state) {
 			case LDAP_ACCESS_ALLOWED:
+				break;
+
 			case LDAP_ACCESS_SUSPENDED:
+				if (inst->group.skip_on_suspend) goto post_group;
 				break;
 
 			case LDAP_ACCESS_DISALLOWED:
@@ -1536,10 +1540,10 @@ static unlang_action_t mod_authorize_resume(rlm_rcode_t *p_result, UNUSED int *p
 		/*
 		 *	Check if we need to cache group memberships
 		 */
-		if ((inst->cacheable_group_dn || inst->cacheable_group_name) && (inst->userobj_membership_attr)) {
+		if ((inst->group.cacheable_dn || inst->group.cacheable_name) && (inst->group.userobj_membership_attr)) {
 			REPEAT_MOD_AUTHORIZE_RESUME;
 			if (rlm_ldap_cacheable_userobj(&rcode, request, autz_ctx,
-						       inst->userobj_membership_attr) == UNLANG_ACTION_PUSHED_CHILD) {
+						       inst->group.userobj_membership_attr) == UNLANG_ACTION_PUSHED_CHILD) {
 				autz_ctx->status = LDAP_AUTZ_GROUP;
 				return UNLANG_ACTION_PUSHED_CHILD;
 			}
@@ -1548,7 +1552,7 @@ static unlang_action_t mod_authorize_resume(rlm_rcode_t *p_result, UNUSED int *p
 		FALL_THROUGH;
 
 	case LDAP_AUTZ_GROUP:
-		if (inst->cacheable_group_dn || inst->cacheable_group_name) {
+		if (inst->group.cacheable_dn || inst->group.cacheable_name) {
 			REPEAT_MOD_AUTHORIZE_RESUME;
 			if (rlm_ldap_cacheable_groupobj(&rcode, request, autz_ctx) == UNLANG_ACTION_PUSHED_CHILD) {
 				autz_ctx->status = LDAP_AUTZ_POST_GROUP;
@@ -1559,6 +1563,7 @@ static unlang_action_t mod_authorize_resume(rlm_rcode_t *p_result, UNUSED int *p
 		FALL_THROUGH;
 
 	case LDAP_AUTZ_POST_GROUP:
+	post_group:
 #ifdef WITH_EDIR
 		/*
 		 *	We already have a Password.Cleartext.  Skip edir.
@@ -1792,9 +1797,9 @@ static unlang_action_t CC_HINT(nonnull) mod_authorize(rlm_rcode_t *p_result, mod
 		expanded->attrs[expanded->count++] = inst->userobj_access_attr;
 	}
 
-	if (inst->userobj_membership_attr && (inst->cacheable_group_dn || inst->cacheable_group_name)) {
+	if (inst->group.userobj_membership_attr && (inst->group.cacheable_dn || inst->group.cacheable_name)) {
 		CHECK_EXPANDED_SPACE(expanded);
-		expanded->attrs[expanded->count++] = inst->userobj_membership_attr;
+		expanded->attrs[expanded->count++] = inst->group.userobj_membership_attr;
 	}
 
 	if (inst->profile_attr) {
@@ -2258,8 +2263,8 @@ static int mod_bootstrap(module_inst_ctx_t const *mctx)
 
 	inst->handle_config.name = talloc_typed_asprintf(inst, "rlm_ldap (%s)", mctx->inst->name);
 
-	if (inst->group_attribute) {
-		group_attribute = inst->group_attribute;
+	if (inst->group.attribute) {
+		group_attribute = inst->group.attribute;
 	} else if (cf_section_name2(conf)) {
 		snprintf(buffer, sizeof(buffer), "%s-LDAP-Group", mctx->inst->name);
 		group_attribute = buffer;
@@ -2267,12 +2272,12 @@ static int mod_bootstrap(module_inst_ctx_t const *mctx)
 		group_attribute = "LDAP-Group";
 	}
 
-	inst->group_da = fr_dict_attr_by_name(NULL, fr_dict_root(dict_freeradius), group_attribute);
+	inst->group.da = fr_dict_attr_by_name(NULL, fr_dict_root(dict_freeradius), group_attribute);
 
 	/*
 	 *	If the group attribute was not in the dictionary, create it
 	 */
-	if (!inst->group_da) {
+	if (!inst->group.da) {
 		fr_dict_attr_flags_t	flags;
 
 		memset(&flags, 0, sizeof(flags));
@@ -2282,25 +2287,25 @@ static int mod_bootstrap(module_inst_ctx_t const *mctx)
 			return -1;
 
 		}
-		inst->group_da = fr_dict_attr_by_name(NULL, fr_dict_root(dict_freeradius), group_attribute);
+		inst->group.da = fr_dict_attr_by_name(NULL, fr_dict_root(dict_freeradius), group_attribute);
 	}
 
 	/*
 	 *	Setup the cache attribute
 	 */
-	if (inst->cache_attribute) {
+	if (inst->group.cache_attribute) {
 		fr_dict_attr_flags_t	flags;
 
 		memset(&flags, 0, sizeof(flags));
 		if (fr_dict_attr_add(fr_dict_unconst(dict_freeradius), fr_dict_root(dict_freeradius),
-				     inst->cache_attribute, -1, FR_TYPE_STRING, &flags) < 0) {
+				     inst->group.cache_attribute, -1, FR_TYPE_STRING, &flags) < 0) {
 			PERROR("Error creating cache attribute");
 			return -1;
 
 		}
-		inst->cache_da = fr_dict_attr_by_name(NULL, fr_dict_root(dict_freeradius), inst->cache_attribute);
+		inst->group.cache_da = fr_dict_attr_by_name(NULL, fr_dict_root(dict_freeradius), inst->group.cache_attribute);
 	} else {
-		inst->cache_da = inst->group_da;	/* Default to the group_da */
+		inst->group.cache_da = inst->group.da;	/* Default to the group_da */
 	}
 
 	/*
@@ -2445,8 +2450,8 @@ static int mod_instantiate(module_inst_ctx_t const *mctx)
 	/*
 	 *	Sanity checks for cacheable groups code.
 	 */
-	if (inst->cacheable_group_name && inst->groupobj_membership_filter) {
-		if (!inst->groupobj_name_attr) {
+	if (inst->group.cacheable_name && inst->group.obj_membership_filter) {
+		if (!inst->group.obj_name_attr) {
 			cf_log_err(conf, "Configuration item 'group.name_attribute' must be set if cacheable "
 				      "group names are enabled");
 
diff --git a/src/modules/rlm_ldap/rlm_ldap.h b/src/modules/rlm_ldap/rlm_ldap.h
index da3f285bd40..f47a53e7787 100644
--- a/src/modules/rlm_ldap/rlm_ldap.h
+++ b/src/modules/rlm_ldap/rlm_ldap.h
@@ -41,7 +41,6 @@ typedef struct {
 
 	int		userobj_scope;			//!< Search scope.
 
-	char const	*userobj_membership_attr;	//!< Attribute that describes groups the user is a member of.
 	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.
@@ -59,37 +58,43 @@ typedef struct {
 	/*
 	 *	Group object attributes and filters
 	 */
-	char const	*groupobj_filter;		//!< Filter to retrieve only group objects.
-	int		groupobj_scope;			//!< Search scope.
+	struct {
+		char const	*userobj_membership_attr;	//!< Attribute that describes groups the user is a member of.
 
-	char const	*groupobj_name_attr;		//!< The name of the group.
-	char const	*groupobj_membership_filter;	//!< Filter to only retrieve groups which contain
-							//!< the user as a member.
+		char const	*obj_filter;			//!< Filter to retrieve only group objects.
+		int		obj_scope;			//!< Search scope.
 
-	bool		cacheable_group_name;		//!< If true the server will determine complete set of group
-							//!< memberships for the current user object, and perform any
-							//!< resolution necessary to determine the names of those
-							//!< groups, then right them to the control list (LDAP-Group).
+		char const	*obj_name_attr;			//!< The name of the group.
+		char const	*obj_membership_filter;		//!< Filter to only retrieve groups which contain
+								//!< the user as a member.
 
-	bool		cacheable_group_dn;		//!< If true the server will determine complete set of group
-							//!< memberships for the current user object, and perform any
-							//!< resolution necessary to determine the DNs of those groups,
-							//!< then right them to the control list (LDAP-GroupDN).
+		bool		cacheable_name;			//!< If true the server will determine complete set of group
+								//!< memberships for the current user object, and perform any
+								//!< resolution necessary to determine the names of those
+								//!< groups, then right them to the control list (LDAP-Group).
 
-	char const	*cache_attribute;		//!< Sets the attribute we use when creating and retrieving
-							//!< cached group memberships.
+		bool		cacheable_dn;			//!< If true the server will determine complete set of group
+								//!< memberships for the current user object, and perform any
+								//!< resolution necessary to determine the DNs of those groups,
+								//!< then right them to the control list (LDAP-GroupDN).
 
-	fr_dict_attr_t const	*cache_da;		//!< The DA associated with this specific instance of the
-							//!< rlm_ldap module.
+		char const	*cache_attribute;		//!< Sets the attribute we use when creating and retrieving
+								//!< cached group memberships.
 
-	char const	*group_attribute;		//!< Sets the attribute we use when comparing group
-							//!< group memberships.
+		fr_dict_attr_t const	*cache_da;		//!< The DA associated with this specific instance of the
+								//!< rlm_ldap module.
 
-	fr_dict_attr_t const	*group_da;		//!< The DA associated with this specific instance of the
-							//!< rlm_ldap module.
+		char const	*attribute;			//!< Sets the attribute we use when comparing group
+								//!< group memberships.
 
-	bool		allow_dangling_group_refs;	//!< Don't error if we fail to resolve a group DN referenced
-							///< from a user object.
+		fr_dict_attr_t const	*da;			//!< The DA associated with this specific instance of the
+								//!< rlm_ldap module.
+
+		bool		allow_dangling_refs;		//!< Don't error if we fail to resolve a group DN referenced
+								///< from a user object.
+
+		bool		skip_on_suspend;		//!< Don't process groups if the user is suspended.
+	} group;
 
 	/*
 	 *	Profiles
diff --git a/src/modules/rlm_ldap/user.c b/src/modules/rlm_ldap/user.c
index 1f8cca7ee9c..fc2c8cd851c 100644
--- a/src/modules/rlm_ldap/user.c
+++ b/src/modules/rlm_ldap/user.c
@@ -29,7 +29,6 @@ RCSID("$Id$")
 USES_APPLE_DEPRECATED_API
 
 #include <freeradius-devel/util/debug.h>
-#include <ctype.h>
 
 #define LOG_PREFIX mctx->inst->name