URL: https://github.com/SSSD/sssd/pull/650 Author: jhrozek Title: #650: Implement a hybrid mode of generating private groups Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/650/head:pr650 git checkout pr650
From ae473f3879b39829c0f925deb29bec6facf93767 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Tue, 21 Aug 2018 16:00:34 +0200 Subject: [PATCH 1/7] UTIL: Add a is_domain_mpg shorthand Instead of looking into the domain structure directly, add a sss_domain_is_mpg() function. This will make sense when we add a third state instead of the boolean that will also be mpg-like. Related: https://pagure.io/SSSD/sssd/issue/3822 --- src/db/sysdb_ops.c | 12 ++++++------ src/db/sysdb_search.c | 6 +++--- src/providers/ad/ad_id.c | 2 +- src/providers/ad/ad_subdomains.c | 2 +- src/providers/ipa/ipa_s2n_exop.c | 4 ++-- src/providers/ipa/ipa_subdomains_id.c | 2 +- src/providers/ldap/ldap_id.c | 2 +- src/providers/ldap/sdap_async_users.c | 6 +++--- src/responder/nss/nss_protocol_sid.c | 4 +++- src/util/domain_info_utils.c | 5 +++++ src/util/util.h | 2 ++ 11 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index 03bc25fae0..59fb227a41 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -504,7 +504,7 @@ static int sysdb_search_by_name(TALLOC_CTX *mem_ctx, break; case SYSDB_GROUP: def_attrs[1] = SYSDB_GIDNUM; - if (domain->mpg && + if (sss_domain_is_mpg(domain) && (!local_provider_is_built() || strcasecmp(domain->provider, "local") != 0)) { /* When searching a group by name in a MPG domain, we also @@ -1984,7 +1984,7 @@ int sysdb_add_user(struct sss_domain_info *domain, int ret; bool posix; - if (domain->mpg) { + if (sss_domain_is_mpg(domain)) { if (gid != 0) { DEBUG(SSSDBG_FATAL_FAILURE, "Cannot add user with arbitrary GID in MPG domain!\n"); @@ -2021,7 +2021,7 @@ int sysdb_add_user(struct sss_domain_info *domain, return ret; } - if (domain->mpg) { + if (sss_domain_is_mpg(domain)) { /* In MPG domains you can't have groups with the same name or GID * as users, search if a group with the same name exists. * Don't worry about users, if we try to add a user with the same @@ -2106,7 +2106,7 @@ int sysdb_add_user(struct sss_domain_info *domain, ret = sysdb_attrs_add_uint32(id_attrs, SYSDB_UIDNUM, id); if (ret) goto done; - if (domain->mpg) { + if (sss_domain_is_mpg(domain)) { ret = sysdb_attrs_add_uint32(id_attrs, SYSDB_GIDNUM, id); if (ret) goto done; } @@ -2240,7 +2240,7 @@ int sysdb_add_group(struct sss_domain_info *domain, return ret; } - if (domain->mpg) { + if (sss_domain_is_mpg(domain)) { /* In MPG domains you can't have groups with the same name as users, * search if a group with the same name exists. * Don't worry about users, if we try to add a user with the same @@ -2857,7 +2857,7 @@ static errno_t sysdb_store_user_attrs(struct sss_domain_info *domain, if (ret) return ret; } - if (uid && !gid && domain->mpg) { + if (uid && !gid && sss_domain_is_mpg(domain)) { ret = sysdb_attrs_add_uint32(attrs, SYSDB_GIDNUM, uid); if (ret) return ret; } diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c index 43341d4462..f0918bf9a3 100644 --- a/src/db/sysdb_search.c +++ b/src/db/sysdb_search.c @@ -909,7 +909,7 @@ int sysdb_getgrnam(TALLOC_CTX *mem_ctx, goto done; } - if (domain->mpg) { + if (sss_domain_is_mpg(domain)) { /* In case the domain supports magic private groups we *must* * check whether the searched name is the very same as the * originalADname attribute. @@ -1108,7 +1108,7 @@ int sysdb_getgrgid_attrs(TALLOC_CTX *mem_ctx, } } - if (domain->mpg) { + if (sss_domain_is_mpg(domain)) { /* In case the domain supports magic private groups we *must* * check whether the searched gid is the very same as the * originalADgidNumber attribute. @@ -1216,7 +1216,7 @@ int sysdb_enumgrent_filter(TALLOC_CTX *mem_ctx, return ENOMEM; } - if (domain->mpg) { + if (sss_domain_is_mpg(domain)) { base_filter = SYSDB_GRENT_MPG_FILTER; base_dn = sysdb_domain_dn(tmp_ctx, domain); } else { diff --git a/src/providers/ad/ad_id.c b/src/providers/ad/ad_id.c index 1da48433ee..c3bda16626 100644 --- a/src/providers/ad/ad_id.c +++ b/src/providers/ad/ad_id.c @@ -1100,7 +1100,7 @@ ad_get_account_domain_send(TALLOC_CTX *mem_ctx, state->attrs[0] = "objectclass"; state->attrs[1] = NULL; - if (params->be_ctx->domain->mpg == true + if (sss_domain_is_mpg(params->be_ctx->domain) == true || state->entry_type == BE_REQ_USER_AND_GROUP) { state->twopass = true; if (state->entry_type == BE_REQ_USER_AND_GROUP) { diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c index 549c2c1f76..cde8d8db1d 100644 --- a/src/providers/ad/ad_subdomains.c +++ b/src/providers/ad/ad_subdomains.c @@ -506,7 +506,7 @@ ad_subdom_store(struct sdap_idmap_ctx *idmap_ctx, * inherit the MPG setting from the parent domain so that the * auto_private_groups options works for trusted domains as well */ - mpg = domain->mpg; + mpg = sss_domain_is_mpg(domain); } ret = sysdb_subdomain_store(domain->sysdb, name, realm, flat, sid_str, diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c index f1626ede1a..e91f134cd2 100644 --- a/src/providers/ipa/ipa_s2n_exop.c +++ b/src/providers/ipa/ipa_s2n_exop.c @@ -2352,7 +2352,7 @@ static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom, } gid = 0; - if (dom->mpg == false) { + if (sss_domain_is_mpg(dom) == false) { gid = attrs->a.user.pw_gid; } else { /* The extdom plugin always returns the objects with the @@ -2423,7 +2423,7 @@ static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom, missing[0] == NULL ? NULL : discard_const(missing), dom->user_timeout, now); - if (ret == EEXIST && dom->mpg == true) { + if (ret == EEXIST && sss_domain_is_mpg(dom) == true) { /* This handles the case where getgrgid() was called for * this user, so a group was created in the cache */ diff --git a/src/providers/ipa/ipa_subdomains_id.c b/src/providers/ipa/ipa_subdomains_id.c index 5b975a5c3c..a1273cf9a1 100644 --- a/src/providers/ipa/ipa_subdomains_id.c +++ b/src/providers/ipa/ipa_subdomains_id.c @@ -1023,7 +1023,7 @@ apply_subdomain_homedir(TALLOC_CTX *mem_ctx, struct sss_domain_info *dom, for (c = 0; c < msg_el->num_values; c++) { if (strncmp(SYSDB_USER_CLASS, (const char *)msg_el->values[c].data, msg_el->values[c].length) == 0 - || (dom->mpg + || (sss_domain_is_mpg(dom) && strncmp(SYSDB_GROUP_CLASS, (const char *)msg_el->values[c].data, msg_el->values[c].length) == 0)) { diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c index 009732b0c2..f24cd27640 100644 --- a/src/providers/ldap/ldap_id.c +++ b/src/providers/ldap/ldap_id.c @@ -954,7 +954,7 @@ static void groups_get_done(struct tevent_req *subreq) } if (ret == ENOENT - && state->domain->mpg == true + && sss_domain_is_mpg(state->domain) == true && !state->conn->no_mpg_user_fallback) { /* The requested filter did not find a group. Before giving up, we must * also check if the GID can be resolved through a primary group of a diff --git a/src/providers/ldap/sdap_async_users.c b/src/providers/ldap/sdap_async_users.c index 5ffba77707..92eeda1d36 100644 --- a/src/providers/ldap/sdap_async_users.c +++ b/src/providers/ldap/sdap_async_users.c @@ -389,7 +389,7 @@ int sdap_save_user(TALLOC_CTX *memctx, goto done; } - if (IS_SUBDOMAIN(dom) || dom->mpg == true) { + if (IS_SUBDOMAIN(dom) || sss_domain_is_mpg(dom) == true) { /* For subdomain users, only create the private group as * the subdomain is an MPG domain. * But we have to save the GID of the original primary group @@ -411,7 +411,7 @@ int sdap_save_user(TALLOC_CTX *memctx, */ ret = sysdb_attrs_add_uint32(attrs, SYSDB_GIDNUM, gid); if (ret != EOK) goto done; - } else if (dom->mpg) { + } else if (sss_domain_is_mpg(dom)) { /* Likewise, if a domain is set to contain 'magic private groups', do * not process the real GID, but save it in the cache as originalGID * (if available) @@ -470,7 +470,7 @@ int sdap_save_user(TALLOC_CTX *memctx, /* check that the gid is valid for this domain */ if (is_posix == true && IS_SUBDOMAIN(dom) == false - && dom->mpg == false + && sss_domain_is_mpg(dom) == false && OUT_OF_ID_RANGE(gid, dom->id_min, dom->id_max)) { DEBUG(SSSDBG_CRIT_FAILURE, "User [%s] filtered out! (primary gid out of range)\n", diff --git a/src/responder/nss/nss_protocol_sid.c b/src/responder/nss/nss_protocol_sid.c index 3f60967d75..96cb8617bc 100644 --- a/src/responder/nss/nss_protocol_sid.c +++ b/src/responder/nss/nss_protocol_sid.c @@ -75,7 +75,9 @@ nss_get_id_type(struct nss_cmd_ctx *cmd_ctx, return EOK; } - ret = find_sss_id_type(result->msgs[0], result->domain->mpg, _type); + ret = find_sss_id_type(result->msgs[0], + sss_domain_is_mpg(result->domain), + _type); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Unable to find ID type [%d]: %s\n", ret, sss_strerror(ret)); diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c index ffb8cdf102..40a38ee8d7 100644 --- a/src/util/domain_info_utils.c +++ b/src/util/domain_info_utils.c @@ -935,3 +935,8 @@ bool is_files_provider(struct sss_domain_info *domain) domain->provider != NULL && strcasecmp(domain->provider, "files") == 0; } + +bool sss_domain_is_mpg(struct sss_domain_info *domain) +{ + return domain->mpg; +} diff --git a/src/util/util.h b/src/util/util.h index 90f9d3652d..63d13babfa 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -573,6 +573,8 @@ void sss_domain_info_set_output_fqnames(struct sss_domain_info *domain, bool sss_domain_info_get_output_fqnames(struct sss_domain_info *domain); +bool sss_domain_is_mpg(struct sss_domain_info *domain); + #define IS_SUBDOMAIN(dom) ((dom)->parent != NULL) #define DOM_HAS_VIEWS(dom) ((dom)->has_views) From ae57091193c772d573c80ebdd38ca9be2d92329e Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Sun, 2 Sep 2018 22:37:42 +0200 Subject: [PATCH 2/7] UTIL: Convert bool mpg to an enum mpg_mode Instead of bool mpg inside struct sss_domain_info, let's introduce enum mpg_mode that currently maps pretty much 1:1 to the boolean. In future patches, a third value will be added. Also adds a getter for the mpg_mode value because we want to discourage getting or setting the value directly. Instead, the sss_domain_info structure should be opaque in the future. Related: https://pagure.io/SSSD/sssd/issue/3822 --- src/confdb/confdb.c | 14 ++++++-- src/confdb/confdb.h | 7 +++- src/db/sysdb.h | 3 +- src/db/sysdb_private.h | 2 +- src/db/sysdb_subdomains.c | 35 +++++++++++-------- src/providers/ad/ad_subdomains.c | 14 +++++--- src/providers/ipa/ipa_subdomains.c | 17 +++++++-- src/tests/cmocka/test_fqnames.c | 3 +- src/tests/cmocka/test_ipa_subdomains_server.c | 4 +-- src/tests/cmocka/test_ldap_id_cleanup.c | 2 +- src/tests/cmocka/test_nss_srv.c | 2 +- src/tests/cmocka/test_responder_cache_req.c | 4 +-- src/tests/cmocka/test_sysdb_subdomains.c | 28 +++++++-------- src/tests/cmocka/test_sysdb_views.c | 6 ++-- src/tests/sysdb-tests.c | 14 ++++---- src/util/domain_info_utils.c | 7 +++- src/util/util.h | 2 ++ 17 files changed, 105 insertions(+), 59 deletions(-) diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c index 07a1b168b2..14b48b1762 100644 --- a/src/confdb/confdb.c +++ b/src/confdb/confdb.c @@ -875,6 +875,7 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, char *default_domain; bool fqnames_default = false; int memcache_timeout; + bool is_mpg; tmp_ctx = talloc_new(mem_ctx); if (!tmp_ctx) return ENOMEM; @@ -939,7 +940,7 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, goto done; } - ret = get_entry_as_bool(res->msgs[0], &domain->mpg, + ret = get_entry_as_bool(res->msgs[0], &is_mpg, CONFDB_DOMAIN_AUTO_UPG, 0); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, @@ -954,7 +955,16 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, ret = EINVAL; goto done; } + } + if (is_mpg) { + domain->mpg_mode = MPG_ENABLED; + } else { + domain->mpg_mode = MPG_DISABLED; + } + + if (local_provider_is_built() + && strcasecmp(domain->provider, "local") == 0) { /* If this is the local provider, we need to ensure that * no other provider was specified for other types, since * the local provider cannot load them. @@ -990,7 +1000,7 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, } /* The LOCAL provider use always Magic Private Groups */ - domain->mpg = true; + domain->mpg_mode = MPG_ENABLED; } domain->timeout = ldb_msg_find_attr_as_int(res->msgs[0], diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index 741d4bc47d..152cc1ffbd 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -310,6 +310,11 @@ enum sss_domain_type { DOM_TYPE_APPLICATION, }; +enum sss_domain_mpg_mode { + MPG_DISABLED, + MPG_ENABLED, +}; + /** * Data structure storing all of the basic features * of a domain. @@ -324,7 +329,7 @@ struct sss_domain_info { bool enumerate; char **sd_enumerate; bool fqnames; - bool mpg; + enum sss_domain_mpg_mode mpg_mode; bool ignore_group_members; uint32_t id_min; uint32_t id_max; diff --git a/src/db/sysdb.h b/src/db/sysdb.h index f6e31605ee..89b0d95715 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -523,7 +523,8 @@ sysdb_set_site(struct sss_domain_info *dom, errno_t sysdb_subdomain_store(struct sysdb_ctx *sysdb, const char *name, const char *realm, const char *flat_name, const char *domain_id, - bool mpg, bool enumerate, const char *forest, + enum sss_domain_mpg_mode mpg_mode, + bool enumerate, const char *forest, uint32_t trust_direction, struct ldb_message_element *upn_suffixes); diff --git a/src/db/sysdb_private.h b/src/db/sysdb_private.h index d1f2bd40a4..c297715cdb 100644 --- a/src/db/sysdb_private.h +++ b/src/db/sysdb_private.h @@ -198,7 +198,7 @@ struct sss_domain_info *new_subdomain(TALLOC_CTX *mem_ctx, const char *realm, const char *flat_name, const char *id, - bool mpg, + enum sss_domain_mpg_mode mpg_mode, bool enumerate, const char *forest, const char **upn_suffixes, diff --git a/src/db/sysdb_subdomains.c b/src/db/sysdb_subdomains.c index bdd5245f20..dced9a95ec 100644 --- a/src/db/sysdb_subdomains.c +++ b/src/db/sysdb_subdomains.c @@ -34,7 +34,7 @@ struct sss_domain_info *new_subdomain(TALLOC_CTX *mem_ctx, const char *realm, const char *flat_name, const char *id, - bool mpg, + enum sss_domain_mpg_mode mpg_mode, bool enumerate, const char *forest, const char **upn_suffixes, @@ -126,7 +126,7 @@ struct sss_domain_info *new_subdomain(TALLOC_CTX *mem_ctx, dom->enumerate = enumerate; dom->fqnames = true; - dom->mpg = mpg; + dom->mpg_mode = mpg_mode; dom->state = DOM_ACTIVE; /* use fully qualified names as output in order to avoid causing @@ -320,7 +320,8 @@ errno_t sysdb_update_subdomains(struct sss_domain_info *domain, const char *flat; const char *id; const char *forest; - bool mpg; + bool is_mpg; + enum sss_domain_mpg_mode mpg_mode; bool enumerate; uint32_t trust_direction; struct ldb_message_element *tmp_el; @@ -376,8 +377,13 @@ errno_t sysdb_update_subdomains(struct sss_domain_info *domain, id = ldb_msg_find_attr_as_string(res->msgs[i], SYSDB_SUBDOMAIN_ID, NULL); - mpg = ldb_msg_find_attr_as_bool(res->msgs[i], - SYSDB_SUBDOMAIN_MPG, false); + is_mpg = ldb_msg_find_attr_as_bool(res->msgs[i], + SYSDB_SUBDOMAIN_MPG, false); + if (is_mpg) { + mpg_mode = MPG_ENABLED; + } else { + mpg_mode = MPG_DISABLED; + } enumerate = ldb_msg_find_attr_as_bool(res->msgs[i], SYSDB_SUBDOMAIN_ENUM, false); @@ -440,12 +446,12 @@ errno_t sysdb_update_subdomains(struct sss_domain_info *domain, } } - if (dom->mpg != mpg) { + if (dom->mpg_mode != mpg_mode) { DEBUG(SSSDBG_TRACE_INTERNAL, "MPG state change from [%s] to [%s]!\n", - dom->mpg ? "true" : "false", - mpg ? "true" : "false"); - dom->mpg = mpg; + dom->mpg_mode == MPG_ENABLED ? "true" : "false", + mpg_mode == MPG_ENABLED ? "true" : "false"); + dom->mpg_mode = mpg_mode; } if (dom->enumerate != enumerate) { @@ -515,7 +521,7 @@ errno_t sysdb_update_subdomains(struct sss_domain_info *domain, /* If not found in loop it is a new subdomain */ if (dom == NULL) { dom = new_subdomain(domain, domain, name, realm, - flat, id, mpg, enumerate, forest, + flat, id, mpg_mode, enumerate, forest, upn_suffixes, trust_direction, confdb); if (dom == NULL) { ret = ENOMEM; @@ -894,7 +900,8 @@ errno_t sysdb_master_domain_add_info(struct sss_domain_info *domain, errno_t sysdb_subdomain_store(struct sysdb_ctx *sysdb, const char *name, const char *realm, const char *flat_name, const char *domain_id, - bool mpg, bool enumerate, const char *forest, + enum sss_domain_mpg_mode mpg_mode, + bool enumerate, const char *forest, uint32_t trust_direction, struct ldb_message_element *upn_suffixes) { @@ -985,8 +992,8 @@ errno_t sysdb_subdomain_store(struct sysdb_ctx *sysdb, } tmp_bool = ldb_msg_find_attr_as_bool(res->msgs[0], SYSDB_SUBDOMAIN_MPG, - !mpg); - if (tmp_bool != mpg) { + !(mpg_mode == MPG_ENABLED)); + if (tmp_bool != (mpg_mode == MPG_ENABLED)) { mpg_flags = LDB_FLAG_MOD_REPLACE; } tmp_bool = ldb_msg_find_attr_as_bool(res->msgs[0], SYSDB_SUBDOMAIN_ENUM, @@ -1099,7 +1106,7 @@ errno_t sysdb_subdomain_store(struct sysdb_ctx *sysdb, } ret = ldb_msg_add_string(msg, SYSDB_SUBDOMAIN_MPG, - mpg ? "TRUE" : "FALSE"); + (mpg_mode == MPG_ENABLED) ? "TRUE" : "FALSE"); if (ret != LDB_SUCCESS) { ret = sysdb_error_to_errno(ret); goto done; diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c index cde8d8db1d..5b046773cf 100644 --- a/src/providers/ad/ad_subdomains.c +++ b/src/providers/ad/ad_subdomains.c @@ -451,7 +451,8 @@ ad_subdom_store(struct sdap_idmap_ctx *idmap_ctx, struct ldb_message_element *el; char *sid_str = NULL; uint32_t trust_type; - bool mpg; + bool use_id_mapping; + enum sss_domain_mpg_mode mpg_mode; tmp_ctx = talloc_new(NULL); if (tmp_ctx == NULL) { @@ -500,17 +501,20 @@ ad_subdom_store(struct sdap_idmap_ctx *idmap_ctx, goto done; } - mpg = sdap_idmap_domain_has_algorithmic_mapping(idmap_ctx, name, sid_str); - if (mpg == false) { + use_id_mapping = sdap_idmap_domain_has_algorithmic_mapping(idmap_ctx, + name, sid_str); + if (use_id_mapping == true) { + mpg_mode = MPG_ENABLED; + } else { /* Domains that use the POSIX attributes set by the admin must * inherit the MPG setting from the parent domain so that the * auto_private_groups options works for trusted domains as well */ - mpg = sss_domain_is_mpg(domain); + mpg_mode = get_domain_mpg_mode(domain); } ret = sysdb_subdomain_store(domain->sysdb, name, realm, flat, sid_str, - mpg, enumerate, domain->forest, 0, NULL); + mpg_mode, enumerate, domain->forest, 0, NULL); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "sysdb_subdomain_store failed.\n"); goto done; diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c index 1b443559ea..d86ca4cc56 100644 --- a/src/providers/ipa/ipa_subdomains.c +++ b/src/providers/ipa/ipa_subdomains.c @@ -571,7 +571,8 @@ static errno_t ipa_subdom_store(struct sss_domain_info *parent, const char *id; char *forest = NULL; int ret; - bool mpg; + bool use_id_mapping; + enum sss_domain_mpg_mode mpg_mode; bool enumerate; uint32_t direction; struct ldb_message_element *alternative_domain_suffixes = NULL; @@ -611,7 +612,17 @@ static errno_t ipa_subdom_store(struct sss_domain_info *parent, goto done; } - mpg = sdap_idmap_domain_has_algorithmic_mapping(sdap_idmap_ctx, name, id); + use_id_mapping = sdap_idmap_domain_has_algorithmic_mapping(sdap_idmap_ctx, + name, id); + if (use_id_mapping == true) { + mpg_mode = MPG_ENABLED; + } else { + /* Domains that use the POSIX attributes set by the admin must + * inherit the MPG setting from the parent domain so that the + * auto_private_groups options works for trusted domains as well + */ + mpg_mode = get_domain_mpg_mode(parent); + } ret = ipa_subdom_get_forest(tmp_ctx, sysdb_ctx_get_ldb(parent->sysdb), attrs, &forest); @@ -639,7 +650,7 @@ static errno_t ipa_subdom_store(struct sss_domain_info *parent, } ret = sysdb_subdomain_store(parent->sysdb, name, realm, flat, - id, mpg, enumerate, forest, + id, mpg_mode, enumerate, forest, direction, alternative_domain_suffixes); if (ret) { DEBUG(SSSDBG_OP_FAILURE, "sysdb_subdomain_store failed.\n"); diff --git a/src/tests/cmocka/test_fqnames.c b/src/tests/cmocka/test_fqnames.c index b6f58a7712..b374be2108 100644 --- a/src/tests/cmocka/test_fqnames.c +++ b/src/tests/cmocka/test_fqnames.c @@ -309,7 +309,8 @@ static int parse_name_test_setup(void **state) * discovered */ test_ctx->subdom = new_subdomain(dom, dom, SUBDOMNAME, NULL, SUBFLATNAME, - NULL, false, false, NULL, NULL, 0, NULL); + NULL, MPG_DISABLED, false, + NULL, NULL, 0, NULL); assert_non_null(test_ctx->subdom); check_leaks_push(test_ctx); diff --git a/src/tests/cmocka/test_ipa_subdomains_server.c b/src/tests/cmocka/test_ipa_subdomains_server.c index 11cec6721e..e034df15bf 100644 --- a/src/tests/cmocka/test_ipa_subdomains_server.c +++ b/src/tests/cmocka/test_ipa_subdomains_server.c @@ -253,14 +253,14 @@ static void add_test_subdomains(struct trust_test_ctx *test_ctx, ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, SUBDOM_NAME, SUBDOM_REALM, NULL, SUBDOM_SID, - true, false, SUBDOM_REALM, + MPG_ENABLED, false, SUBDOM_REALM, direction, NULL); assert_int_equal(ret, EOK); ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, CHILD_NAME, CHILD_REALM, CHILD_FLAT, CHILD_SID, - true, false, SUBDOM_REALM, + MPG_ENABLED, false, SUBDOM_REALM, direction, NULL); assert_int_equal(ret, EOK); diff --git a/src/tests/cmocka/test_ldap_id_cleanup.c b/src/tests/cmocka/test_ldap_id_cleanup.c index 177022e201..d8956f2c80 100644 --- a/src/tests/cmocka/test_ldap_id_cleanup.c +++ b/src/tests/cmocka/test_ldap_id_cleanup.c @@ -121,7 +121,7 @@ static int test_sysdb_setup(void **state) ret = setup_sysdb_tests(&test_ctx); assert_int_equal(ret, EOK); - test_ctx->domain->mpg = false; + test_ctx->domain->mpg_mode = MPG_DISABLED; /* set options */ test_ctx->opts = talloc_zero(test_ctx, struct sdap_options); diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c index 1f7de7bd90..0ae1775719 100644 --- a/src/tests/cmocka/test_nss_srv.c +++ b/src/tests/cmocka/test_nss_srv.c @@ -3480,7 +3480,7 @@ static int nss_subdom_test_setup_common(void **state, bool nonfqnames) ret = sysdb_subdomain_store(nss_test_ctx->tctx->sysdb, testdom[0], testdom[1], testdom[2], testdom[3], - false, false, NULL, 0, NULL); + MPG_DISABLED, false, NULL, 0, NULL); assert_int_equal(ret, EOK); ret = sysdb_update_subdomains(nss_test_ctx->tctx->dom, diff --git a/src/tests/cmocka/test_responder_cache_req.c b/src/tests/cmocka/test_responder_cache_req.c index f564c6582b..b4cf7f9fe8 100644 --- a/src/tests/cmocka/test_responder_cache_req.c +++ b/src/tests/cmocka/test_responder_cache_req.c @@ -686,13 +686,13 @@ int test_subdomain_setup(void **state) test_ctx->subdomain = new_subdomain(test_ctx, test_ctx->tctx->dom, testdom[0], testdom[1], testdom[2], testdom[3], - false, false, NULL, NULL, 0, + MPG_DISABLED, false, NULL, NULL, 0, test_ctx->tctx->confdb); assert_non_null(test_ctx->subdomain); ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, testdom[0], testdom[1], testdom[2], testdom[3], - false, false, NULL, 0, NULL); + MPG_DISABLED, false, NULL, 0, NULL); assert_int_equal(ret, EOK); ret = sysdb_update_subdomains(test_ctx->tctx->dom, diff --git a/src/tests/cmocka/test_sysdb_subdomains.c b/src/tests/cmocka/test_sysdb_subdomains.c index ead76bfff2..d2116dc237 100644 --- a/src/tests/cmocka/test_sysdb_subdomains.c +++ b/src/tests/cmocka/test_sysdb_subdomains.c @@ -100,7 +100,7 @@ static void test_sysdb_subdomain_create(void **state) ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, dom1[0], dom1[1], dom1[2], dom1[3], - false, false, NULL, 0, NULL); + MPG_DISABLED, false, NULL, 0, NULL); assert_int_equal(ret, EOK); ret = sysdb_update_subdomains(test_ctx->tctx->dom, test_ctx->tctx->confdb); @@ -112,7 +112,7 @@ static void test_sysdb_subdomain_create(void **state) ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, dom2[0], dom2[1], dom2[2], dom2[3], - false, false, NULL, 1, NULL); + MPG_DISABLED, false, NULL, 1, NULL); assert_int_equal(ret, EOK); ret = sysdb_update_subdomains(test_ctx->tctx->dom, test_ctx->tctx->confdb); @@ -125,12 +125,12 @@ static void test_sysdb_subdomain_create(void **state) /* Reverse the trust directions */ ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, dom1[0], dom1[1], dom1[2], dom1[3], - false, false, NULL, 1, NULL); + MPG_DISABLED, false, NULL, 1, NULL); assert_int_equal(ret, EOK); ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, dom2[0], dom2[1], dom2[2], dom2[3], - false, false, NULL, 0, NULL); + MPG_DISABLED, false, NULL, 0, NULL); assert_int_equal(ret, EOK); ret = sysdb_update_subdomains(test_ctx->tctx->dom, test_ctx->tctx->confdb); @@ -212,26 +212,26 @@ static void test_sysdb_link_forest_root_ipa(void **state) ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, dom1[0], dom1[1], dom1[2], dom1[3], - false, false, dom1[4], 0, NULL); + MPG_DISABLED, false, dom1[4], 0, NULL); assert_int_equal(ret, EOK); ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, child_dom1[0], child_dom1[1], child_dom1[2], child_dom1[3], - false, false, child_dom1[4], + MPG_DISABLED, false, child_dom1[4], 0, NULL); assert_int_equal(ret, EOK); ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, dom2[0], dom2[1], dom2[2], dom2[3], - false, false, dom2[4], + MPG_DISABLED, false, dom2[4], 0, NULL); assert_int_equal(ret, EOK); ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, child_dom2[0], child_dom2[1], child_dom2[2], child_dom2[3], - false, false, child_dom2[4], + MPG_DISABLED, false, child_dom2[4], 0, NULL); assert_int_equal(ret, EOK); @@ -304,14 +304,14 @@ static void test_sysdb_link_forest_root_ad(void **state) ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, child_dom[0], child_dom[1], child_dom[2], child_dom[3], - false, false, child_dom[4], + MPG_DISABLED, false, child_dom[4], 0, NULL); assert_int_equal(ret, EOK); ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, sub_dom[0], sub_dom[1], sub_dom[2], sub_dom[3], - false, false, sub_dom[4], + MPG_DISABLED, false, sub_dom[4], 0, NULL); assert_int_equal(ret, EOK); @@ -381,14 +381,14 @@ static void test_sysdb_link_forest_member_ad(void **state) ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, sub_dom[0], sub_dom[1], sub_dom[2], sub_dom[3], - false, false, sub_dom[4], + MPG_DISABLED, false, sub_dom[4], 0, NULL); assert_int_equal(ret, EOK); ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, forest_root[0], forest_root[1], forest_root[2], forest_root[3], - false, false, forest_root[4], + MPG_DISABLED, false, forest_root[4], 0, NULL); assert_int_equal(ret, EOK); @@ -466,7 +466,7 @@ static void test_sysdb_link_ad_multidom(void **state) ret = sysdb_subdomain_store(main_dom1->sysdb, child_dom[0], child_dom[1], child_dom[2], child_dom[3], - false, false, child_dom[4], + MPG_DISABLED, false, child_dom[4], 0, NULL); assert_int_equal(ret, EOK); @@ -487,7 +487,7 @@ static void test_sysdb_link_ad_multidom(void **state) ret = sysdb_subdomain_store(main_dom2->sysdb, dom2_forest_root[0], dom2_forest_root[1], dom2_forest_root[2], dom2_forest_root[3], - false, false, dom2_forest_root[4], 0, NULL); + MPG_DISABLED, false, dom2_forest_root[4], 0, NULL); assert_int_equal(ret, EOK); ret = sysdb_master_domain_update(main_dom2); diff --git a/src/tests/cmocka/test_sysdb_views.c b/src/tests/cmocka/test_sysdb_views.c index 8ef69b2712..72daa44502 100644 --- a/src/tests/cmocka/test_sysdb_views.c +++ b/src/tests/cmocka/test_sysdb_views.c @@ -158,7 +158,7 @@ static void test_sysdb_store_override(void **state) struct sysdb_test_ctx *test_ctx = talloc_get_type_abort(*state, struct sysdb_test_ctx); - test_ctx->domain->mpg = false; + test_ctx->domain->mpg_mode = MPG_DISABLED; name = sss_create_internal_fqname(test_ctx, TEST_USER_NAME, test_ctx->domain->name); assert_non_null(name); @@ -388,7 +388,7 @@ void test_sysdb_delete_view_tree(void **state) struct sysdb_test_ctx *test_ctx = talloc_get_type_abort(*state, struct sysdb_test_ctx); - test_ctx->domain->mpg = false; + test_ctx->domain->mpg_mode = MPG_DISABLED; ret = sysdb_update_view_name(test_ctx->domain->sysdb, TEST_VIEW_NAME); assert_int_equal(ret, EOK); @@ -455,7 +455,7 @@ void test_sysdb_invalidate_overrides(void **state) struct sysdb_test_ctx *test_ctx = talloc_get_type_abort(*state, struct sysdb_test_ctx); - test_ctx->domain->mpg = false; + test_ctx->domain->mpg_mode = MPG_DISABLED; name = sss_create_internal_fqname(test_ctx, TEST_USER_NAME, test_ctx->domain->name); assert_non_null(name); diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c index d3117cd9df..6ce787a441 100644 --- a/src/tests/sysdb-tests.c +++ b/src/tests/sysdb-tests.c @@ -1095,7 +1095,7 @@ START_TEST(test_user_group_by_name) * ldap provider differently with auto_private_groups. */ test_ctx->domain->provider = discard_const_p(char, "ldap"); - test_ctx->domain->mpg = true; + test_ctx->domain->mpg_mode = MPG_ENABLED; data = test_data_new_user(test_ctx, _i); fail_if(data == NULL); @@ -1337,7 +1337,7 @@ START_TEST (test_sysdb_enumgrent) return; } - test_ctx->domain->mpg = true; + test_ctx->domain->mpg_mode = MPG_ENABLED; ret = sysdb_enumgrent(test_ctx, test_ctx->domain, @@ -1541,7 +1541,7 @@ START_TEST (test_sysdb_get_user_attr_subdomain) /* Create subdomain */ subdomain = new_subdomain(test_ctx, test_ctx->domain, "test.sub", "TEST.SUB", "test", "S-3", - false, false, NULL, NULL, 0, NULL); + MPG_DISABLED, false, NULL, NULL, 0, NULL); fail_if(subdomain == NULL, "Failed to create new subdomain."); ret = sss_names_init_from_args(test_ctx, @@ -5946,7 +5946,7 @@ START_TEST(test_sysdb_search_object_by_id) fail_unless(ret == EOK, "sysdb_add_group failed with [%d][%s].", ret, strerror(ret)); - test_ctx->domain->mpg = false; + test_ctx->domain->mpg_mode = MPG_DISABLED; ret = sysdb_add_user(test_ctx->domain, "user1", 4001, id, "User 1", "/home/user1", "/bin/bash", NULL, NULL, 0, 0); @@ -6271,7 +6271,7 @@ START_TEST(test_sysdb_subdomain_store_user) subdomain = new_subdomain(test_ctx, test_ctx->domain, testdom[0], testdom[1], testdom[2], testdom[3], - false, false, NULL, NULL, 0, NULL); + MPG_DISABLED, false, NULL, NULL, 0, NULL); fail_unless(subdomain != NULL, "Failed to create new subdomain."); ret = sysdb_subdomain_store(test_ctx->sysdb, testdom[0], testdom[1], testdom[2], testdom[3], @@ -6350,7 +6350,7 @@ START_TEST(test_sysdb_subdomain_user_ops) subdomain = new_subdomain(test_ctx, test_ctx->domain, testdom[0], testdom[1], testdom[2], testdom[3], - false, false, NULL, NULL, 0, NULL); + MPG_DISABLED, false, NULL, NULL, 0, NULL); fail_unless(subdomain != NULL, "Failed to create new subdomain."); ret = sysdb_subdomain_store(test_ctx->sysdb, testdom[0], testdom[1], testdom[2], testdom[3], @@ -6423,7 +6423,7 @@ START_TEST(test_sysdb_subdomain_group_ops) subdomain = new_subdomain(test_ctx, test_ctx->domain, testdom[0], testdom[1], testdom[2], testdom[3], - false, false, NULL, NULL, 0, NULL); + MPG_DISABLED, false, NULL, NULL, 0, NULL); fail_unless(subdomain != NULL, "Failed to create new subdomain."); ret = sysdb_subdomain_store(test_ctx->sysdb, testdom[0], testdom[1], testdom[2], testdom[3], diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c index 40a38ee8d7..b42536aa8d 100644 --- a/src/util/domain_info_utils.c +++ b/src/util/domain_info_utils.c @@ -938,5 +938,10 @@ bool is_files_provider(struct sss_domain_info *domain) bool sss_domain_is_mpg(struct sss_domain_info *domain) { - return domain->mpg; + return domain->mpg_mode == MPG_ENABLED; +} + +enum sss_domain_mpg_mode get_domain_mpg_mode(struct sss_domain_info *domain) +{ + return domain->mpg_mode; } diff --git a/src/util/util.h b/src/util/util.h index 63d13babfa..da49c898af 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -575,6 +575,8 @@ bool sss_domain_info_get_output_fqnames(struct sss_domain_info *domain); bool sss_domain_is_mpg(struct sss_domain_info *domain); +enum sss_domain_mpg_mode get_domain_mpg_mode(struct sss_domain_info *domain); + #define IS_SUBDOMAIN(dom) ((dom)->parent != NULL) #define DOM_HAS_VIEWS(dom) ((dom)->has_views) From b07c28e45d427fe286fc235d1ed4bbc963cc49d3 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Sun, 2 Sep 2018 17:37:43 +0200 Subject: [PATCH 3/7] CONFDB: Read auto_private_groups as string, not bool In preparation to adding the third value of auto_private_groups, this patch reads the confdb value as string and checks for the option values on its own. Related: https://pagure.io/SSSD/sssd/issue/3822 --- src/confdb/confdb.c | 19 +++++----- src/db/sysdb_subdomains.c | 47 +++++++++++++++++++----- src/tests/cmocka/test_sysdb_subdomains.c | 19 ++++++++++ src/util/domain_info_utils.c | 12 ++++++ src/util/util.h | 1 + 5 files changed, 79 insertions(+), 19 deletions(-) diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c index 14b48b1762..6b84804a5e 100644 --- a/src/confdb/confdb.c +++ b/src/confdb/confdb.c @@ -875,7 +875,6 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, char *default_domain; bool fqnames_default = false; int memcache_timeout; - bool is_mpg; tmp_ctx = talloc_new(mem_ctx); if (!tmp_ctx) return ENOMEM; @@ -940,12 +939,9 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, goto done; } - ret = get_entry_as_bool(res->msgs[0], &is_mpg, - CONFDB_DOMAIN_AUTO_UPG, 0); - if (ret != EOK) { - DEBUG(SSSDBG_FATAL_FAILURE, - "Invalid value for %s\n", CONFDB_DOMAIN_AUTO_UPG); - goto done; + tmp = ldb_msg_find_attr_as_string(res->msgs[0], CONFDB_DOMAIN_AUTO_UPG, NULL); + if (tmp == NULL || *tmp == '\0') { + tmp = "false"; } if (strcasecmp(domain->provider, "local") == 0) { @@ -957,10 +953,15 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, } } - if (is_mpg) { + if (strcasecmp(tmp, "FALSE") == 0) { + domain->mpg_mode = MPG_DISABLED; + } else if (strcasecmp(tmp, "TRUE") == 0) { domain->mpg_mode = MPG_ENABLED; } else { - domain->mpg_mode = MPG_DISABLED; + DEBUG(SSSDBG_FATAL_FAILURE, + "Invalid value for %s\n", CONFDB_DOMAIN_AUTO_UPG); + ret = EINVAL; + goto done; } if (local_provider_is_built() diff --git a/src/db/sysdb_subdomains.c b/src/db/sysdb_subdomains.c index dced9a95ec..6083e08d8a 100644 --- a/src/db/sysdb_subdomains.c +++ b/src/db/sysdb_subdomains.c @@ -320,7 +320,7 @@ errno_t sysdb_update_subdomains(struct sss_domain_info *domain, const char *flat; const char *id; const char *forest; - bool is_mpg; + const char *str_mpg_mode; enum sss_domain_mpg_mode mpg_mode; bool enumerate; uint32_t trust_direction; @@ -377,11 +377,20 @@ errno_t sysdb_update_subdomains(struct sss_domain_info *domain, id = ldb_msg_find_attr_as_string(res->msgs[i], SYSDB_SUBDOMAIN_ID, NULL); - is_mpg = ldb_msg_find_attr_as_bool(res->msgs[i], - SYSDB_SUBDOMAIN_MPG, false); - if (is_mpg) { + str_mpg_mode = ldb_msg_find_attr_as_string(res->msgs[i], + SYSDB_SUBDOMAIN_MPG, NULL); + if (str_mpg_mode == NULL || *str_mpg_mode == '\0') { + str_mpg_mode = "false"; + } + + if (strcasecmp(str_mpg_mode, "FALSE") == 0) { + mpg_mode = MPG_DISABLED; + } else if (strcasecmp(str_mpg_mode, "TRUE") == 0) { mpg_mode = MPG_ENABLED; } else { + DEBUG(SSSDBG_MINOR_FAILURE, + "Invalid value for %s\n, assuming disabled", + SYSDB_SUBDOMAIN_MPG); mpg_mode = MPG_DISABLED; } @@ -991,11 +1000,23 @@ errno_t sysdb_subdomain_store(struct sysdb_ctx *sysdb, } } - tmp_bool = ldb_msg_find_attr_as_bool(res->msgs[0], SYSDB_SUBDOMAIN_MPG, - !(mpg_mode == MPG_ENABLED)); - if (tmp_bool != (mpg_mode == MPG_ENABLED)) { - mpg_flags = LDB_FLAG_MOD_REPLACE; + tmp_str = ldb_msg_find_attr_as_string(res->msgs[0], + SYSDB_SUBDOMAIN_MPG, + "false"); + /* If mpg_mode changed we need to replace the old value in sysdb */ + switch (mpg_mode) { + case MPG_ENABLED: + if (strcasecmp(tmp_str, "true") != 0) { + mpg_flags = LDB_FLAG_MOD_REPLACE; + } + break; + case MPG_DISABLED: + if (strcasecmp(tmp_str, "false") != 0) { + mpg_flags = LDB_FLAG_MOD_REPLACE; + } + break; } + tmp_bool = ldb_msg_find_attr_as_bool(res->msgs[0], SYSDB_SUBDOMAIN_ENUM, !enumerate); if (tmp_bool != enumerate) { @@ -1105,8 +1126,14 @@ errno_t sysdb_subdomain_store(struct sysdb_ctx *sysdb, goto done; } - ret = ldb_msg_add_string(msg, SYSDB_SUBDOMAIN_MPG, - (mpg_mode == MPG_ENABLED) ? "TRUE" : "FALSE"); + tmp_str = str_domain_mpg_mode(mpg_mode); + if (tmp_str == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "Couldn't convert mpg_mode to string\n"); + ret = EINVAL; + goto done; + } + + ret = ldb_msg_add_string(msg, SYSDB_SUBDOMAIN_MPG, tmp_str); if (ret != LDB_SUCCESS) { ret = sysdb_error_to_errno(ret); goto done; diff --git a/src/tests/cmocka/test_sysdb_subdomains.c b/src/tests/cmocka/test_sysdb_subdomains.c index d2116dc237..f919091983 100644 --- a/src/tests/cmocka/test_sysdb_subdomains.c +++ b/src/tests/cmocka/test_sysdb_subdomains.c @@ -109,6 +109,7 @@ static void test_sysdb_subdomain_create(void **state) assert_non_null(test_ctx->tctx->dom->subdomains); assert_string_equal(test_ctx->tctx->dom->subdomains->name, dom1[0]); assert_int_equal(test_ctx->tctx->dom->subdomains->trust_direction, 0); + assert_true(test_ctx->tctx->dom->subdomains->mpg_mode == MPG_DISABLED); ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, dom2[0], dom2[1], dom2[2], dom2[3], @@ -121,6 +122,7 @@ static void test_sysdb_subdomain_create(void **state) assert_non_null(test_ctx->tctx->dom->subdomains->next); assert_string_equal(test_ctx->tctx->dom->subdomains->next->name, dom2[0]); assert_int_equal(test_ctx->tctx->dom->subdomains->next->trust_direction, 1); + assert_true(test_ctx->tctx->dom->subdomains->next->mpg_mode == MPG_DISABLED); /* Reverse the trust directions */ ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, @@ -153,6 +155,23 @@ static void test_sysdb_subdomain_create(void **state) assert_int_equal( sss_domain_get_state(test_ctx->tctx->dom->subdomains->next), DOM_DISABLED); + + /* Test that changing the MPG status works */ + ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, + dom1[0], dom1[1], dom1[2], dom1[3], + MPG_ENABLED, false, NULL, 1, NULL); + assert_int_equal(ret, EOK); + + ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, + dom2[0], dom2[1], dom2[2], dom2[3], + MPG_ENABLED, false, NULL, 0, NULL); + assert_int_equal(ret, EOK); + + ret = sysdb_update_subdomains(test_ctx->tctx->dom, test_ctx->tctx->confdb); + assert_int_equal(ret, EOK); + + assert_true(test_ctx->tctx->dom->subdomains->mpg_mode == MPG_ENABLED); + assert_true(test_ctx->tctx->dom->subdomains->next->mpg_mode == MPG_ENABLED); } static void test_sysdb_master_domain_ops(void **state) diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c index b42536aa8d..ec0e2cd450 100644 --- a/src/util/domain_info_utils.c +++ b/src/util/domain_info_utils.c @@ -945,3 +945,15 @@ enum sss_domain_mpg_mode get_domain_mpg_mode(struct sss_domain_info *domain) { return domain->mpg_mode; } + +const char *str_domain_mpg_mode(enum sss_domain_mpg_mode mpg_mode) +{ + switch (mpg_mode) { + case MPG_ENABLED: + return "true"; + case MPG_DISABLED: + return "false"; + } + + return NULL; +} diff --git a/src/util/util.h b/src/util/util.h index da49c898af..0a5630cb40 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -576,6 +576,7 @@ bool sss_domain_info_get_output_fqnames(struct sss_domain_info *domain); bool sss_domain_is_mpg(struct sss_domain_info *domain); enum sss_domain_mpg_mode get_domain_mpg_mode(struct sss_domain_info *domain); +const char *str_domain_mpg_mode(enum sss_domain_mpg_mode mpg_mode); #define IS_SUBDOMAIN(dom) ((dom)->parent != NULL) From 58c648d7fe6ad7753076167a196cc892e262074f Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Sat, 9 Mar 2019 19:48:36 +0100 Subject: [PATCH 4/7] CONFDB/SYSDB: Add the hybrid MPG mode Permits a new option value 'hybrid' for the auto_private_groups option. The option was even previously marked as a string option in both the configAPI and the man pages, so we don't have to change the type now. If the hybrid mode is selected and the user's original GID number is available, then during initgroups and getpwnam, it is used as their primary GID instead of the MPG group. The original group is also not added as a secondary group during initgroups in this case. Related: https://pagure.io/SSSD/sssd/issue/3822 --- src/confdb/confdb.c | 11 +----- src/confdb/confdb.h | 1 + src/db/sysdb_subdomains.c | 17 ++++------ src/man/sssd.conf.5.xml | 43 +++++++++++++++++++++--- src/tests/cmocka/test_sysdb_subdomains.c | 16 +++++++++ src/util/domain_info_utils.c | 18 ++++++++++ src/util/util.h | 1 + 7 files changed, 82 insertions(+), 25 deletions(-) diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c index 6b84804a5e..a2c5ca3e21 100644 --- a/src/confdb/confdb.c +++ b/src/confdb/confdb.c @@ -953,16 +953,7 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb, } } - if (strcasecmp(tmp, "FALSE") == 0) { - domain->mpg_mode = MPG_DISABLED; - } else if (strcasecmp(tmp, "TRUE") == 0) { - domain->mpg_mode = MPG_ENABLED; - } else { - DEBUG(SSSDBG_FATAL_FAILURE, - "Invalid value for %s\n", CONFDB_DOMAIN_AUTO_UPG); - ret = EINVAL; - goto done; - } + domain->mpg_mode = str_to_domain_mpg_mode(tmp); if (local_provider_is_built() && strcasecmp(domain->provider, "local") == 0) { diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index 152cc1ffbd..338656b094 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -313,6 +313,7 @@ enum sss_domain_type { enum sss_domain_mpg_mode { MPG_DISABLED, MPG_ENABLED, + MPG_HYBRID, }; /** diff --git a/src/db/sysdb_subdomains.c b/src/db/sysdb_subdomains.c index 6083e08d8a..e380e6c8bc 100644 --- a/src/db/sysdb_subdomains.c +++ b/src/db/sysdb_subdomains.c @@ -382,17 +382,7 @@ errno_t sysdb_update_subdomains(struct sss_domain_info *domain, if (str_mpg_mode == NULL || *str_mpg_mode == '\0') { str_mpg_mode = "false"; } - - if (strcasecmp(str_mpg_mode, "FALSE") == 0) { - mpg_mode = MPG_DISABLED; - } else if (strcasecmp(str_mpg_mode, "TRUE") == 0) { - mpg_mode = MPG_ENABLED; - } else { - DEBUG(SSSDBG_MINOR_FAILURE, - "Invalid value for %s\n, assuming disabled", - SYSDB_SUBDOMAIN_MPG); - mpg_mode = MPG_DISABLED; - } + mpg_mode = str_to_domain_mpg_mode(str_mpg_mode); enumerate = ldb_msg_find_attr_as_bool(res->msgs[i], SYSDB_SUBDOMAIN_ENUM, false); @@ -1015,6 +1005,11 @@ errno_t sysdb_subdomain_store(struct sysdb_ctx *sysdb, mpg_flags = LDB_FLAG_MOD_REPLACE; } break; + case MPG_HYBRID: + if (strcasecmp(tmp_str, "hybrid") != 0) { + mpg_flags = LDB_FLAG_MOD_REPLACE; + } + break; } tmp_bool = ldb_msg_find_attr_as_bool(res->msgs[0], SYSDB_SUBDOMAIN_ENUM, diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml index bea25c6228..6e0b3dc6ce 100644 --- a/src/man/sssd.conf.5.xml +++ b/src/man/sssd.conf.5.xml @@ -3066,10 +3066,45 @@ subdomain_inherit = ldap_purge_cache_timeout <term>auto_private_groups (string)</term> <listitem> <para> - If this option is enabled, SSSD will automatically - create user private groups based on user's - UID number. The GID number is ignored in this case. - </para> + This option takes any of three available values: + <variablelist> + <varlistentry> + <term>true</term> + <listitem> + <para> + Create user's private group unconditionally from user's UID number. + The GID number is ignored in this case. + </para> + </listitem> + </varlistentry> + <varlistentry> + <term>false</term> + <listitem> + <para> + Always use the user's primary GID number. The GID number must refer + to a group object in the LDAP database. + </para> + </listitem> + </varlistentry> + <varlistentry> + <term>hybrid</term> + <listitem> + <para> + If the primary GID number in the user object is different from the UID + number value, its value must correspond to a group object. However, if + the GID number is the same as the UID number value and the corresponding + group object does not exist, a user private group is automatically + generated. + </para> + <para> + This feature is useful for environments that wish to stop maintaining + a separate group objects for the user private groups, but also wish + to retain the existing user private groups. + </para> + </listitem> + </varlistentry> + </variablelist> + </para> <para> For POSIX subdomains, setting the option in the main domain is inherited in the subdomain. diff --git a/src/tests/cmocka/test_sysdb_subdomains.c b/src/tests/cmocka/test_sysdb_subdomains.c index f919091983..7830425af4 100644 --- a/src/tests/cmocka/test_sysdb_subdomains.c +++ b/src/tests/cmocka/test_sysdb_subdomains.c @@ -172,6 +172,22 @@ static void test_sysdb_subdomain_create(void **state) assert_true(test_ctx->tctx->dom->subdomains->mpg_mode == MPG_ENABLED); assert_true(test_ctx->tctx->dom->subdomains->next->mpg_mode == MPG_ENABLED); + + ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, + dom1[0], dom1[1], dom1[2], dom1[3], + MPG_HYBRID, false, NULL, 1, NULL); + assert_int_equal(ret, EOK); + + ret = sysdb_subdomain_store(test_ctx->tctx->sysdb, + dom2[0], dom2[1], dom2[2], dom2[3], + MPG_HYBRID, false, NULL, 0, NULL); + assert_int_equal(ret, EOK); + + ret = sysdb_update_subdomains(test_ctx->tctx->dom, test_ctx->tctx->confdb); + assert_int_equal(ret, EOK); + + assert_true(test_ctx->tctx->dom->subdomains->mpg_mode == MPG_HYBRID); + assert_true(test_ctx->tctx->dom->subdomains->next->mpg_mode == MPG_HYBRID); } static void test_sysdb_master_domain_ops(void **state) diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c index ec0e2cd450..9d65eebf67 100644 --- a/src/util/domain_info_utils.c +++ b/src/util/domain_info_utils.c @@ -953,7 +953,25 @@ const char *str_domain_mpg_mode(enum sss_domain_mpg_mode mpg_mode) return "true"; case MPG_DISABLED: return "false"; + case MPG_HYBRID: + return "hybrid"; } return NULL; } + +enum sss_domain_mpg_mode str_to_domain_mpg_mode(const char *str_mpg_mode) +{ + if (strcasecmp(str_mpg_mode, "FALSE") == 0) { + return MPG_DISABLED; + } else if (strcasecmp(str_mpg_mode, "TRUE") == 0) { + return MPG_ENABLED; + } else if (strcasecmp(str_mpg_mode, "HYBRID") == 0) { + return MPG_HYBRID; + } + + DEBUG(SSSDBG_MINOR_FAILURE, + "Invalid value for %s\n, assuming disabled", + SYSDB_SUBDOMAIN_MPG); + return MPG_DISABLED; +} diff --git a/src/util/util.h b/src/util/util.h index 0a5630cb40..162c2d483e 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -577,6 +577,7 @@ bool sss_domain_is_mpg(struct sss_domain_info *domain); enum sss_domain_mpg_mode get_domain_mpg_mode(struct sss_domain_info *domain); const char *str_domain_mpg_mode(enum sss_domain_mpg_mode mpg_mode); +enum sss_domain_mpg_mode str_to_domain_mpg_mode(const char *str_mpg_mode); #define IS_SUBDOMAIN(dom) ((dom)->parent != NULL) From ff5ecfee5543bf3f4e3e31b44aee4ae1451f214e Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Sat, 9 Mar 2019 19:50:00 +0100 Subject: [PATCH 5/7] CACHE_REQ: Add cache_req_data_get_type() Adds a utility function which returns the lookup type stored in struct cache_req_data. This will be used later to switch between different lookups as appropriate. Related: https://pagure.io/SSSD/sssd/issue/3822 --- src/responder/common/cache_req/cache_req.h | 5 +++++ src/responder/common/cache_req/cache_req_data.c | 11 +++++++++++ 2 files changed, 16 insertions(+) diff --git a/src/responder/common/cache_req/cache_req.h b/src/responder/common/cache_req/cache_req.h index 2c88853887..84dd22c252 100644 --- a/src/responder/common/cache_req/cache_req.h +++ b/src/responder/common/cache_req/cache_req.h @@ -130,6 +130,11 @@ cache_req_data_set_bypass_cache(struct cache_req_data *data, void cache_req_data_set_bypass_dp(struct cache_req_data *data, bool bypass_dp); + + +enum cache_req_type +cache_req_data_get_type(struct cache_req_data *data); + /* Output data. */ struct cache_req_result { diff --git a/src/responder/common/cache_req/cache_req_data.c b/src/responder/common/cache_req/cache_req_data.c index e7a4fcce00..be8b9d8425 100644 --- a/src/responder/common/cache_req/cache_req_data.c +++ b/src/responder/common/cache_req/cache_req_data.c @@ -378,3 +378,14 @@ cache_req_data_set_bypass_dp(struct cache_req_data *data, data->bypass_dp = bypass_dp; } + +enum cache_req_type +cache_req_data_get_type(struct cache_req_data *data) +{ + if (data == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "cache_req_data should never be NULL\n"); + return CACHE_REQ_SENTINEL; + } + + return data->type; +} From ea3a8fe9c5006f6651ee211009798878483e6792 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Sat, 9 Mar 2019 19:50:23 +0100 Subject: [PATCH 6/7] NSS: Add the hybrid-MPG mode Implements the functionality of the hybrid private group mapping. Uncharacteristically, all the functionality is implemented in the responder only. This is because this hybrid mode must not shadow real groups with autogenerated ones, not even if the real group comes from another domain. Therefore, the user or group resolution must really call the full cache_req requests. Related: https://pagure.io/SSSD/sssd/issue/3822 --- src/responder/nss/nss_get_object.c | 250 ++++++++++++++++++++++++++++- 1 file changed, 248 insertions(+), 2 deletions(-) diff --git a/src/responder/nss/nss_get_object.c b/src/responder/nss/nss_get_object.c index 15faced006..2ef34c564c 100644 --- a/src/responder/nss/nss_get_object.c +++ b/src/responder/nss/nss_get_object.c @@ -148,9 +148,103 @@ memcache_delete_entry(struct nss_ctx *nss_ctx, return EOK; } +static struct cache_req_data * +hybrid_domain_retry_data(TALLOC_CTX *mem_ctx, + struct cache_req_data *orig, + const char *input_name, + uint32_t input_id) +{ + enum cache_req_type cr_type = cache_req_data_get_type(orig); + struct cache_req_data *hybrid_data = NULL; + + if (cr_type == CACHE_REQ_GROUP_BY_ID) { + DEBUG(SSSDBG_TRACE_FUNC, + "Retrying group-by-ID lookup in user space\n"); + hybrid_data = cache_req_data_id(mem_ctx, + CACHE_REQ_USER_BY_ID, + input_id); + } else if (cr_type == CACHE_REQ_GROUP_BY_NAME) { + DEBUG(SSSDBG_TRACE_FUNC, + "Retrying group-by-name lookup in user space\n"); + hybrid_data = cache_req_data_name(mem_ctx, + CACHE_REQ_USER_BY_NAME, + input_name); + } + + return hybrid_data; +} + +static struct cache_req_data * +hybrid_domain_verify_gid_data(TALLOC_CTX *mem_ctx, + struct cache_req_result *user_group) +{ + gid_t gid; + + /* read the GID of this 'group' and use it to construct + * a cache_req_data struct + */ + gid = sss_view_ldb_msg_find_attr_as_uint64(user_group->domain, + user_group->msgs[0], + SYSDB_GIDNUM, + 0); + if (gid == 0) { + DEBUG(SSSDBG_CRIT_FAILURE, "A user with no GID?\n"); + return NULL; + } + + return cache_req_data_id(mem_ctx, + CACHE_REQ_GROUP_BY_ID, + gid); +} + +static int +hybrid_domain_user_to_group(struct cache_req_result *result) +{ + errno_t ret; + uid_t uid; + gid_t gid; + + /* There must be exactly one entry.. */ + if (result == NULL || result->count != 1) { + DEBUG(SSSDBG_CRIT_FAILURE, + "No result or wrong number of entries, expected 1 entry\n"); + return ENOENT; + } + + /* ...which has uidNumber equal to gidNumber */ + uid = sss_view_ldb_msg_find_attr_as_uint64(result->domain, + result->msgs[0], + SYSDB_UIDNUM, + 0); + + gid = sss_view_ldb_msg_find_attr_as_uint64(result->domain, + result->msgs[0], + SYSDB_GIDNUM, + 0); + + if (uid == 0 || uid != gid) { + DEBUG(SSSDBG_TRACE_INTERNAL, "UID and GID differ\n"); + return ENOENT; + } + + /* OK, we have a user with uid == gid; let's pretend this is a group */ + ret = ldb_msg_add_string(result->msgs[0], + SYSDB_OBJECTCATEGORY, + SYSDB_GROUP_CLASS); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, "Cannot add group class\n"); + return ret; + } + + return EOK; +} + struct nss_get_object_state { struct nss_ctx *nss_ctx; struct resp_ctx *rctx; + struct tevent_context *ev; + struct cli_ctx *cli_ctx; + struct cache_req_data *data; /* We delete object from memory cache if it is not found */ enum sss_mc_type memcache; @@ -161,6 +255,11 @@ struct nss_get_object_state { }; static void nss_get_object_done(struct tevent_req *subreq); +static errno_t nss_get_hybrid_object_step(struct tevent_req *req); +static void nss_get_hybrid_object_done(struct tevent_req *subreq); +static void nss_get_hybrid_gid_verify_done(struct tevent_req *subreq); +static void nss_get_object_finish_req(struct tevent_req *req, + errno_t ret); /* Cache request data memory context is stolen to internal state. */ struct tevent_req * @@ -182,8 +281,9 @@ nss_get_object_send(TALLOC_CTX *mem_ctx, DEBUG(SSSDBG_CRIT_FAILURE, "Unable to create tevent request!\n"); return NULL; } - - talloc_steal(state, data); + state->ev = ev; + state->cli_ctx = cli_ctx; + state->data = talloc_steal(state, data); state->rctx = cli_ctx->rctx; state->nss_ctx = talloc_get_type(cli_ctx->rctx->pvt_ctx, struct nss_ctx); @@ -225,6 +325,7 @@ static void nss_get_object_done(struct tevent_req *subreq) struct nss_get_object_state *state; struct tevent_req *req; errno_t ret; + errno_t retry_ret; req = tevent_req_callback_data(subreq, struct tevent_req); state = tevent_req_data(req, struct nss_get_object_state); @@ -232,6 +333,29 @@ static void nss_get_object_done(struct tevent_req *subreq) ret = cache_req_single_domain_recv(state, subreq, &state->result); talloc_zfree(subreq); + if (ret == ENOENT + && state->nss_ctx->rctx->domains->mpg_mode == MPG_HYBRID) { + retry_ret = nss_get_hybrid_object_step(req); + if (retry_ret == EAGAIN) { + /* Retrying hybrid search */ + return; + } + /* Otherwise return the value of ret as returned from + * cache_req_single_domain_recv + */ + } + + nss_get_object_finish_req(req, ret); + return; +} + +static void nss_get_object_finish_req(struct tevent_req *req, + errno_t ret) +{ + struct nss_get_object_state *state; + + state = tevent_req_data(req, struct nss_get_object_state); + switch (ret) { case EOK: if (state->memcache != SSS_MC_NONE) { @@ -259,7 +383,129 @@ static void nss_get_object_done(struct tevent_req *subreq) tevent_req_error(req, ret); break; } +} + +static errno_t nss_get_hybrid_object_step(struct tevent_req *req) +{ + struct tevent_req *subreq; + struct nss_get_object_state *state; + + state = tevent_req_data(req, struct nss_get_object_state); + + state->data = hybrid_domain_retry_data(state, + state->data, + state->input_name, + state->input_id); + if (state->data == NULL) { + DEBUG(SSSDBG_TRACE_FUNC, "This request cannot be retried\n"); + return EOK; + } + subreq = cache_req_send(req, + state->ev, + state->cli_ctx->rctx, + state->cli_ctx->rctx->ncache, + state->nss_ctx->cache_refresh_percent, + CACHE_REQ_POSIX_DOM, + NULL, + state->data); + if (subreq == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "Unable to send cache request!\n"); + return ENOMEM; + } + tevent_req_set_callback(subreq, nss_get_hybrid_object_done, req); + + return EAGAIN; +} + +static void nss_get_hybrid_object_done(struct tevent_req *subreq) +{ + struct nss_get_object_state *state; + struct tevent_req *req; + errno_t ret; + + req = tevent_req_callback_data(subreq, struct tevent_req); + state = tevent_req_data(req, struct nss_get_object_state); + + ret = cache_req_single_domain_recv(state, subreq, &state->result); + talloc_zfree(subreq); + if (ret != EOK) { + goto done; + } + + DEBUG(SSSDBG_TRACE_INTERNAL, "Converting user object to a group\n"); + ret = hybrid_domain_user_to_group(state->result); + if (ret != EOK) { + goto done; + } + + /* If the "group" was requested by name, we also must verify that + * no other group with this ID exists in any domain, otherwise + * we would have returned a private group that should be shadowed, + * this record would have been inserted into the memcache and then + * even getgrgid() would return this unexpected group + */ + if (cache_req_data_get_type(state->data) == CACHE_REQ_USER_BY_NAME) { + DEBUG(SSSDBG_TRACE_FUNC, "Will verify if MPG group is shadowed\n"); + talloc_zfree(state->data); + state->data = hybrid_domain_verify_gid_data(state, state->result); + if (state->data == NULL) { + nss_get_object_finish_req(req, EINVAL); + return; + } + + subreq = cache_req_send(req, + state->ev, + state->cli_ctx->rctx, + state->cli_ctx->rctx->ncache, + state->nss_ctx->cache_refresh_percent, + CACHE_REQ_POSIX_DOM, + NULL, + state->data); + if (subreq == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "Unable to send cache request!\n"); + tevent_req_error(req, ENOENT); + return; + } + tevent_req_set_callback(subreq, nss_get_hybrid_gid_verify_done, req); + return; + } + +done: + nss_get_object_finish_req(req, ret); + return; +} + +static void nss_get_hybrid_gid_verify_done(struct tevent_req *subreq) +{ + struct nss_get_object_state *state; + struct cache_req_result *real_gr_result; + struct tevent_req *req; + errno_t ret; + + req = tevent_req_callback_data(subreq, struct tevent_req); + state = tevent_req_data(req, struct nss_get_object_state); + + ret = cache_req_single_domain_recv(state, subreq, &real_gr_result); + talloc_zfree(subreq); + if (ret == ENOENT) { + /* There is no real group with the same GID as the autogenerated + * one we were checking, so let's return the autogenerated one + */ + ret = EOK; + goto done; + } else if (ret == EOK) { + /* The autogenerated group is shadowed by a real one. Don't return + * anything. + */ + DEBUG(SSSDBG_TRACE_FUNC, + "A real entry would be shadowed by MPG entry\n"); + ret = ENOENT; + goto done; + } + +done: + nss_get_object_finish_req(req, ret); return; } From 3173da0ac339c6d42317d7cfe4a0338a59bbb0bd Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Sat, 9 Mar 2019 19:47:58 +0100 Subject: [PATCH 7/7] TESTS: Add integration tests for auto_private_groups=hybrid Related: https://pagure.io/SSSD/sssd/issue/3822 --- src/tests/intg/test_ldap.py | 264 ++++++++++++++++++++++++++++++++++++ 1 file changed, 264 insertions(+) diff --git a/src/tests/intg/test_ldap.py b/src/tests/intg/test_ldap.py index 43edc6d893..63f6ea4eda 100644 --- a/src/tests/intg/test_ldap.py +++ b/src/tests/intg/test_ldap.py @@ -1451,6 +1451,270 @@ def test_ldap_auto_private_groups_direct_no_gid(ldap_conn, mpg_setup_no_gid): ) +@pytest.fixture +def mpg_setup_hybrid(request, ldap_conn): + """ + This setup creates two users - one with a GID that corresponds to + a group and another with GID that does not. + """ + ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) + + ent_list.add_user("user_with_group", 1001, 2001) + ent_list.add_group_bis("user_with_group_pvt", 2001) + ent_list.add_group_bis("with_group_group1", 10010, ["user_with_group"]) + ent_list.add_group_bis("with_group_group2", 10011, ["user_with_group"]) + + ent_list.add_user("user_with_no_group", 1002, 1002) + # Note - there is no group for the GID 1002 + ent_list.add_group_bis("no_group_group1", 10020, ["user_with_no_group"]) + ent_list.add_group_bis("no_group_group2", 10021, ["user_with_no_group"]) + + # This user has their gid different from the UID, but there is + # no group with gid 2003 + ent_list.add_user("user_with_unresolvable_gid", 1003, 2003) + ent_list.add_group_bis("unresolvable_group1", + 10030, + ["user_with_unresolvable_gid"]) + ent_list.add_group_bis("unresolvable_group2", + 10031, + ["user_with_unresolvable_gid"]) + + # This user's autogenerated private group should be shadowed + # by the real one + ent_list.add_user("user_with_real_group", 1004, 1004) + ent_list.add_user("user_in_pvt_group", 1005, 1005) + ent_list.add_group_bis("user_with_real_group_pvt", + 1004, + ['user_in_pvt_group']) + ent_list.add_group_bis("with_real_group_group1", + 10040, + ["user_with_real_group"]) + ent_list.add_group_bis("with_real_group_group2", + 10041, + ["user_with_real_group"]) + + # Test shadowing again, but this time with the same name + ent_list.add_user("u_g_same_name", 1006, 1006) + ent_list.add_group_bis("u_g_same_name", + 1006, + ['user_in_pvt_group']) + ent_list.add_group_bis("u_g_same_g1", + 10060, + ["u_g_same_name"]) + ent_list.add_group_bis("u_g_same_g2", + 10061, + ["u_g_same_name"]) + + create_ldap_entries(ldap_conn, ent_list) + create_ldap_cleanup(request, ldap_conn, None) + + conf = \ + format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS) + \ + unindent(""" + [domain/LDAP] + auto_private_groups = hybrid + """).format(**locals()) + create_conf_fixture(request, conf) + create_sssd_fixture(request) + return None + + +def test_ldap_auto_private_groups_hybrid_direct(ldap_conn, mpg_setup_hybrid): + """ + Integration test for auto_private_groups=hybrid. This test checks the + resolution of the users and their groups. + + See also ticket https://pagure.io/SSSD/sssd/issue/1872 + """ + # Make sure the user's GID is taken from their gidNumber, if available + ent.assert_passwd_by_name("user_with_group", + dict(name="user_with_group", uid=1001, gid=2001)) + + # The user's secondary groups list must be correct as well and include + # the primary gid, too + user_with_group_ids = [2001, 10010, 10011] + (res, errno, gids) = sssd_id.call_sssd_initgroups("user_with_group", 2001) + assert res == sssd_id.NssReturnCode.SUCCESS + + assert sorted(gids) == sorted(user_with_group_ids), \ + "result: %s\n expected %s" % ( + ", ".join(["%s" % s for s in sorted(gids)]), + ", ".join(["%s" % s for s in sorted(user_with_group_ids)]) + ) + + # On the other hand, if the gidNumber is the same as UID, SSSD should + # just autogenerate the private group on its own + ent.assert_passwd_by_name("user_with_no_group", + dict(name="user_with_no_group", + uid=1002, gid=1002)) + + # The user's secondary groups list must be correct as well. Since there was + # no original GID, it is not added to the list + user_without_group_ids = [1002, 10020, 10021] + (res, errno, gids) = sssd_id.call_sssd_initgroups("user_with_no_group", + 1002) + assert res == sssd_id.NssReturnCode.SUCCESS + + assert sorted(gids) == sorted(user_without_group_ids), \ + "result: %s\n expected %s" % ( + ", ".join(["%s" % s for s in sorted(gids)]), + ", ".join(["%s" % s for s in sorted(user_without_group_ids)]) + ) + + ent.assert_passwd_by_name("user_with_unresolvable_gid", + dict(name="user_with_unresolvable_gid", + uid=1003, gid=2003)) + unresolvable_group_ids = [2003, 10030, 10031] + (res, errno, gids) = sssd_id.call_sssd_initgroups( + "user_with_unresolvable_gid", + 2003) + assert res == sssd_id.NssReturnCode.SUCCESS + + assert sorted(gids) == sorted(unresolvable_group_ids), \ + "result: %s\n expected %s" % ( + ", ".join(["%s" % s for s in sorted(gids)]), + ", ".join(["%s" % s for s in sorted(unresolvable_group_ids)]) + ) + + ent.assert_passwd_by_name("user_with_real_group", + dict(name="user_with_real_group", + uid=1004, gid=1004)) + with_real_group_ids = [1004, 10040, 10041] + (res, errno, gids) = sssd_id.call_sssd_initgroups( + "user_with_real_group", + 1004) + assert res == sssd_id.NssReturnCode.SUCCESS + + assert sorted(gids) == sorted(with_real_group_ids), \ + "result: %s\n expected %s" % ( + ", ".join(["%s" % s for s in sorted(gids)]), + ", ".join(["%s" % s for s in sorted(with_real_group_ids)]) + ) + + +def test_ldap_auto_private_groups_hybrid_priv_group_byname(ldap_conn, + mpg_setup_hybrid): + """ + Integration test for auto_private_groups=hybrid. This test checks the + resolution of user private groups by name. + + See also ticket https://pagure.io/SSSD/sssd/issue/1872 + """ + # gidNumber is resolvable by name.. + ent.assert_group_by_name("user_with_group_pvt", + dict(gid=2001, + mem=ent.contains_only())) + + # ..but since this user /has/ a gidNumber set, their autogenerated group + # should not be resolvable + with pytest.raises(KeyError): + grp.getgrnam("user_with_group") + + # Finally, the autogenerated group for the user with + # uidNumber==gidNumber must be resolvable + ent.assert_group_by_name("user_with_no_group", + dict(gid=1002, + mem=ent.contains_only())) + + # A gid that is different from an UID must not resolve to a private + # group even if the private group does not exist + with pytest.raises(KeyError): + grp.getgrnam("user_with_unresolvable_gid") + + # If there is a user with the same UID and GID but there is a real + # group corresponding to the primary GID, the real group should take + # precedence and the automatic group should not be resolvable + ent.assert_group_by_name("user_with_real_group_pvt", + dict(gid=1004, + mem=ent.contains_only('user_in_pvt_group',))) + + # getgrnam should not return + with pytest.raises(KeyError): + grp.getgrnam("user_with_real_group") + + +def test_ldap_auto_private_groups_hybrid_priv_group_byid(ldap_conn, + mpg_setup_hybrid): + """ + Integration test for auto_private_groups=hybrid. This test checks the + resolution of user private groups by name. + + See also ticket https://pagure.io/SSSD/sssd/issue/1872 + """ + # Make sure the private group of user who has this group set in their + # gidNumber is resolvable by ID + ent.assert_group_by_gid(2001, + dict(name="user_with_group_pvt", + mem=ent.contains_only())) + + # ..but since this user /has/ a gidNumber set different from the uidNumber, + # their autogenerated group + # should not be resolvable + with pytest.raises(KeyError): + grp.getgrgid(1001) + + # Finally, the autogenerated group for the user with + # uidNumber==gidNumber must be resolvable + ent.assert_group_by_gid(1002, + dict(name="user_with_no_group", + mem=ent.contains_only())) + + # A gid that is different from an UID must not resolve to a private + # group even if the private group does not exist + with pytest.raises(KeyError): + grp.getgrgid(2003) + + # Conversely, a GID that corresponds to a group must not resolve to + # the autogenerated group (IOW, the autogenerated group should not + # shadow the real one + ent.assert_group_by_gid(1004, + dict(name="user_with_real_group_pvt", + mem=ent.contains_only('user_in_pvt_group'))) + + +def test_ldap_auto_private_groups_hybrid_name_gid_identical(ldap_conn, + mpg_setup_hybrid): + """ + See also ticket https://pagure.io/SSSD/sssd/issue/1872 + """ + ent.assert_passwd_by_name("u_g_same_name", + dict(name="u_g_same_name", + uid=1006, gid=1006)) + user_without_group_ids = [1006, 10060, 10061] + (res, errno, gids) = sssd_id.call_sssd_initgroups("u_g_same_name", + 1006) + assert res == sssd_id.NssReturnCode.SUCCESS + + assert sorted(gids) == sorted(user_without_group_ids), \ + "result: %s\n expected %s" % ( + ", ".join(["%s" % s for s in sorted(gids)]), + ", ".join(["%s" % s for s in sorted(user_without_group_ids)]) + ) + ent.assert_group_by_gid(1006, + dict(name="u_g_same_name", + mem=ent.contains_only('user_in_pvt_group'))) + + +def test_ldap_auto_private_groups_hybrid_initgr(ldap_conn, mpg_setup_hybrid): + """ + See also ticket https://pagure.io/SSSD/sssd/issue/1872 + """ + user_without_group_ids = [1004, 10040, 10041] + (res, errno, gids) = sssd_id.call_sssd_initgroups("user_with_real_group", + 1004) + assert res == sssd_id.NssReturnCode.SUCCESS + + assert sorted(gids) == sorted(user_without_group_ids), \ + "result: %s\n expected %s" % ( + ", ".join(["%s" % s for s in sorted(gids)]), + ", ".join(["%s" % s for s in sorted(user_without_group_ids)]) + ) + + ent.assert_group_by_gid(1004, + dict(name="user_with_real_group_pvt", + mem=ent.contains_only('user_in_pvt_group'))) + + def rename_setup_no_cleanup(request, ldap_conn, cleanup_ent=None): ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) ent_list.add_user("user1", 1001, 2001)
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
