From 5f1d0357d044b93fc6444c110d0ed1ed1443db3d Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Tue, 3 Oct 2017 12:34:33 +0200
Subject: [PATCH 1/6] CONFIG: Add a new option auto_private_groups

The auto_private_groups option is used to configure the domain->mpg flag
which was already set automatically for subdomains, but for some time was
not settable by the admin via the configuration file.

The new option name, instead of the old magic_private_groups, was chosen
purely because this name would hopefully be better understood by admins.

The option doesn't do anything yet, it is just added to all the places a
new option should be added to.

Related:
    https://pagure.io/SSSD/sssd/issue/1872
---
 src/confdb/confdb.c                  |  8 ++++++++
 src/confdb/confdb.h                  |  1 +
 src/config/SSSDConfig/__init__.py.in |  1 +
 src/config/SSSDConfigTest.py         |  6 ++++--
 src/config/cfg_rules.ini             |  1 +
 src/config/etc/sssd.api.conf         |  1 +
 src/man/sssd.conf.5.xml              | 13 +++++++++++++
 7 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index fefecc03d..a02822481 100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -936,6 +936,14 @@ static int confdb_get_domain_internal(struct confdb_ctx *cdb,
         goto done;
     }
 
+    ret = get_entry_as_bool(res->msgs[0], &domain->mpg,
+                            CONFDB_DOMAIN_AUTO_UPG, 0);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_FATAL_FAILURE,
+              "Invalid value for %s\n", CONFDB_DOMAIN_AUTO_UPG);
+        goto done;
+    }
+
     if (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
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index bcea99ae4..2539b9069 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -198,6 +198,7 @@
 #define CONFDB_DEFAULT_CACHE_CREDS_MIN_FF_LENGTH 8
 #define CONFDB_DOMAIN_LEGACY_PASS "store_legacy_passwords"
 #define CONFDB_DOMAIN_MPG "magic_private_groups"
+#define CONFDB_DOMAIN_AUTO_UPG "auto_private_groups"
 #define CONFDB_DOMAIN_FQ "use_fully_qualified_names"
 #define CONFDB_DOMAIN_ENTRY_CACHE_TIMEOUT "entry_cache_timeout"
 #define CONFDB_DOMAIN_ACCOUNT_CACHE_EXPIRATION "account_cache_expiration"
diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index 227f76180..7d5cc93dd 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -195,6 +195,7 @@ option_strings = {
     'cached_auth_timeout' : _('How long can cached credentials be used for cached authentication'),
     'full_name_format' : _('Printf-compatible format for displaying fully-qualified names'),
     're_expression' : _('Regex to parse username and domain'),
+    'auto_private_groups' : _('Whether to automatically create private groups for users'),
 
     # [provider/ipa]
     'ipa_domain' : _('IPA domain'),
diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py
index 4a583bdd3..87d1f6e64 100755
--- a/src/config/SSSDConfigTest.py
+++ b/src/config/SSSDConfigTest.py
@@ -624,7 +624,8 @@ def testListOptions(self):
             'subdomain_homedir',
             'full_name_format',
             're_expression',
-            'cached_auth_timeout']
+            'cached_auth_timeout',
+            'auto_private_groups']
 
         self.assertTrue(type(options) == dict,
                         "Options should be a dictionary")
@@ -994,7 +995,8 @@ def testRemoveProvider(self):
             'subdomain_homedir',
             'full_name_format',
             're_expression',
-            'cached_auth_timeout']
+            'cached_auth_timeout',
+            'auto_private_groups']
 
         self.assertTrue(type(options) == dict,
                         "Options should be a dictionary")
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index f3d30b9b3..d0436ddd9 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -382,6 +382,7 @@ option = cached_auth_timeout
 option = wildcard_limit
 option = full_name_format
 option = re_expression
+option = auto_private_groups
 
 #Entry cache timeouts
 option = entry_cache_user_timeout
diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf
index 7f2b8977b..2be2e3e68 100644
--- a/src/config/etc/sssd.api.conf
+++ b/src/config/etc/sssd.api.conf
@@ -185,6 +185,7 @@ subdomain_homedir = str, None, false
 cached_auth_timeout = int, None, false
 full_name_format = str, None, false
 re_expression = str, None, false
+auto_private_groups = str, None, false
 
 #Entry cache timeouts
 entry_cache_user_timeout = int, None, false
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index 11496341d..640aa0647 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -2814,6 +2814,19 @@ subdomain_inherit = ldap_purge_cache_timeout
                         </para>
                     </listitem>
                 </varlistentry>
+                <varlistentry>
+                    <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>
+                        <para>
+                            Default: False
+                        </para>
+                    </listitem>
+                </varlistentry>
             </variablelist>
         </para>
 

From 4814bf5755be519d43588dd92b019aa6ecbf79e9 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Tue, 3 Oct 2017 12:36:02 +0200
Subject: [PATCH 2/6] CONFDB: Remove the obsolete option magic_private_groups

Since this confdb definition was completely unused across the codebase,
this patch just removes the definition.
---
 src/confdb/confdb.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 2539b9069..147194962 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -197,7 +197,6 @@
                                  "cache_credentials_minimal_first_factor_length"
 #define CONFDB_DEFAULT_CACHE_CREDS_MIN_FF_LENGTH 8
 #define CONFDB_DOMAIN_LEGACY_PASS "store_legacy_passwords"
-#define CONFDB_DOMAIN_MPG "magic_private_groups"
 #define CONFDB_DOMAIN_AUTO_UPG "auto_private_groups"
 #define CONFDB_DOMAIN_FQ "use_fully_qualified_names"
 #define CONFDB_DOMAIN_ENTRY_CACHE_TIMEOUT "entry_cache_timeout"

From 7fffbbbb7b8f8e77d003de81a1d49fe4439a47de Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Tue, 3 Oct 2017 12:34:49 +0200
Subject: [PATCH 3/6] SDAP: Allow the mpg flag for the main domain

This commit allows saving the users in the MPG domain in the SDAP
layer.

The commit contains the following changes:
    - abstracts the change where if the primary GID exists in the original
      object, it is saved instead as the SYSDB_PRIMARY_GROUP_GIDNUM attribute,
      which will allow the original primary GID to be exposed as a
      secondary group

    - if the primary GID does not exist, no SYSDB_PRIMARY_GROUP_GIDNUM
      is added. This will allow to handle LDAP objects that only contain
      the UID but no GID. Since this is a new use-case, a test is added
      later

    - a branch that handles the above is added to sdap_save_user() also
      for joined domains that set the MPG flag. Previously, only
      subdomains were handled.

    - to allow passing GID=0 to the sysdb layer, the range check is
      relaxed.

Related:
    https://pagure.io/SSSD/sssd/issue/1872
---
 src/providers/ldap/sdap_async_users.c | 83 +++++++++++++++++++++++++++++++----
 1 file changed, 75 insertions(+), 8 deletions(-)

diff --git a/src/providers/ldap/sdap_async_users.c b/src/providers/ldap/sdap_async_users.c
index 09d096e84..7338b4a15 100644
--- a/src/providers/ldap/sdap_async_users.c
+++ b/src/providers/ldap/sdap_async_users.c
@@ -136,6 +136,38 @@ static errno_t sdap_set_non_posix_flag(struct sysdb_attrs *attrs,
     return EOK;
 }
 
+static int sdap_user_set_mpg(struct sysdb_attrs *user_attrs,
+                             gid_t *_gid)
+{
+    errno_t ret;
+
+    if (_gid == NULL) {
+        return EINVAL;
+    }
+
+    if (*_gid == 0) {
+        /* The original entry had no GID number. This is OK, we just won't add
+         * the SYSDB_PRIMARY_GROUP_GIDNUM attribute
+         */
+        return EOK;
+    }
+
+    ret = sysdb_attrs_add_uint32(user_attrs,
+                                 SYSDB_PRIMARY_GROUP_GIDNUM,
+                                 (uint32_t) *_gid);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_add_uint32 failed.\n");
+        return ret;
+    }
+
+    /* We won't really store gidNumber=0, but the zero value tells
+     * the sysdb layer that no GID is set, which sysdb requires for
+     * MPG-enabled domains
+     */
+    *_gid = 0;
+    return EOK;
+}
+
 /* FIXME: support storing additional attributes */
 int sdap_save_user(TALLOC_CTX *memctx,
                    struct sdap_options *opts,
@@ -357,7 +389,7 @@ int sdap_save_user(TALLOC_CTX *memctx,
             goto done;
         }
 
-        if (IS_SUBDOMAIN(dom)) {
+        if (IS_SUBDOMAIN(dom) || dom->mpg == 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
@@ -365,14 +397,13 @@ int sdap_save_user(TALLOC_CTX *memctx,
              * typically (Unix and AD) the user is not listed in his primary
              * group as a member.
              */
-            ret = sysdb_attrs_add_uint32(user_attrs, SYSDB_PRIMARY_GROUP_GIDNUM,
-                                         (uint32_t) gid);
+            ret = sdap_user_set_mpg(user_attrs, &gid);
             if (ret != EOK) {
-                DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_add_uint32 failed.\n");
+                DEBUG(SSSDBG_OP_FAILURE,
+                      "sdap_user_set_mpg failed [%d]: %s\n", ret,
+                      sss_strerror(ret));
                 goto done;
             }
-
-            gid = 0;
         }
 
         /* Store the GID in the ldap_attrs so it doesn't get
@@ -380,6 +411,41 @@ 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) {
+        /* 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)
+         */
+        ret = sysdb_attrs_get_uint32_t(attrs,
+                                       opts->user_map[SDAP_AT_USER_GID].sys_name,
+                                       &gid);
+        if (ret == ENOENT) {
+            DEBUG(SSSDBG_TRACE_LIBS,
+                  "Missing GID, won't save the %s attribute\n",
+                  SYSDB_PRIMARY_GROUP_GIDNUM);
+
+            /* Store the UID as GID (since we're in a MPG domain so that it doesn't
+             * get treated as a missing attribute and removed
+             */
+            ret = sdap_replace_id(attrs, SYSDB_GIDNUM, uid);
+            if (ret) {
+                DEBUG(SSSDBG_OP_FAILURE, "Cannot set the id-mapped UID\n");
+                goto done;
+            }
+            gid = 0;
+        } else if (ret != EOK) {
+            DEBUG(SSSDBG_MINOR_FAILURE,
+                  "Cannot retrieve GID, won't save the %s attribute\n",
+                  SYSDB_PRIMARY_GROUP_GIDNUM);
+            gid = 0;
+        }
+
+        ret = sdap_user_set_mpg(user_attrs, &gid);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE,
+                  "sdap_user_set_mpg failed [%d]: %s\n", ret, sss_strerror(ret));
+            goto done;
+        }
     } else {
         ret = sysdb_attrs_get_uint32_t(attrs,
                                        opts->user_map[SDAP_AT_USER_GID].sys_name,
@@ -403,8 +469,9 @@ int sdap_save_user(TALLOC_CTX *memctx,
     }
 
     /* check that the gid is valid for this domain */
-    if (is_posix == true && IS_SUBDOMAIN(dom) == false &&
-            OUT_OF_ID_RANGE(gid, dom->id_min, dom->id_max)) {
+    if (is_posix == true && IS_SUBDOMAIN(dom) == false
+            && dom->mpg == 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",
                user_name);

From 9a86eeae219c11126926861c38d6e80efb857702 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Tue, 3 Oct 2017 14:31:18 +0200
Subject: [PATCH 4/6] LDAP: Turn by-GID request into by-UID request for MPG
 domains if needed

If the primary group GID is requested before the user is, we need to
also search the user space to save the user in the back end which then
allows the responder to generate the GID from the user entry.

Related:
    https://pagure.io/SSSD/sssd/issue/1872
---
 src/providers/ldap/ldap_id.c | 150 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 111 insertions(+), 39 deletions(-)

diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index 93204d35e..9f0c762e9 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -694,6 +694,8 @@ struct groups_get_state {
 static int groups_get_retry(struct tevent_req *req);
 static void groups_get_connect_done(struct tevent_req *subreq);
 static void groups_get_posix_check_done(struct tevent_req *subreq);
+static void groups_get_mpg_done(struct tevent_req *subreq);
+static errno_t groups_get_handle_no_group(struct tevent_req *req);
 static void groups_get_search(struct tevent_req *req);
 static void groups_get_done(struct tevent_req *subreq);
 
@@ -1051,8 +1053,6 @@ static void groups_get_done(struct tevent_req *subreq)
                                                       struct tevent_req);
     struct groups_get_state *state = tevent_req_data(req,
                                                      struct groups_get_state);
-    char *endptr;
-    gid_t gid;
     int dp_error = DP_ERR_FATAL;
     int ret;
 
@@ -1078,55 +1078,127 @@ static void groups_get_done(struct tevent_req *subreq)
         return;
     }
 
-    if (ret == ENOENT && state->noexist_delete == true) {
-        switch (state->filter_type) {
-        case BE_FILTER_ENUM:
+    if (ret == ENOENT
+            && state->domain->mpg == true
+            && state->filter_type == BE_FILTER_IDNUM) {
+        /* The requested GID is not a group. Before giving up, we must also check
+         * if the GID can be resolved through a primary group of a user
+         */
+        subreq = users_get_send(state,
+                                state->ev,
+                                state->ctx,
+                                state->sdom,
+                                state->conn,
+                                state->filter_value,
+                                state->filter_type,
+                                NULL,
+                                state->noexist_delete);
+        if (subreq == NULL) {
+            tevent_req_error(req, ENOMEM);
+            return;
+        }
+        tevent_req_set_callback(subreq, groups_get_mpg_done, req);
+        return;
+    } else if (ret == ENOENT && state->noexist_delete == true) {
+        ret = groups_get_handle_no_group(req);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE,
+                  "Could not delete group [%d]: %s\n", ret, sss_strerror(ret));
             tevent_req_error(req, ret);
             return;
-        case BE_FILTER_NAME:
-            ret = sysdb_delete_group(state->domain, state->filter_value, 0);
-            if (ret != EOK && ret != ENOENT) {
-                tevent_req_error(req, ret);
-                return;
-            }
-            break;
-
-        case BE_FILTER_IDNUM:
-            gid = (gid_t) strtouint32(state->filter_value, &endptr, 10);
-            if (errno || *endptr || (state->filter_value == endptr)) {
-                tevent_req_error(req, errno ? errno : EINVAL);
-                return;
-            }
+        }
+    }
 
-            ret = sysdb_delete_group(state->domain, NULL, gid);
-            if (ret != EOK && ret != ENOENT) {
-                tevent_req_error(req, ret);
-                return;
-            }
-            break;
+    state->dp_error = DP_ERR_OK;
+    tevent_req_done(req);
+}
 
-        case BE_FILTER_SECID:
-        case BE_FILTER_UUID:
-            /* Since it is not clear if the SID/UUID belongs to a user or a
-             * group we have nothing to do here. */
-            break;
+static void groups_get_mpg_done(struct tevent_req *subreq)
+{
+    errno_t ret;
+    struct tevent_req *req = tevent_req_callback_data(subreq,
+                                                      struct tevent_req);
+    struct groups_get_state *state = tevent_req_data(req,
+                                                     struct groups_get_state);
 
-        case BE_FILTER_WILDCARD:
-            /* We can't know if all groups are up-to-date, especially in
-             * a large environment. Do not delete any records, let the
-             * responder fetch the entries they are requested in.
-             */
-            break;
+    ret = users_get_recv(subreq, &state->dp_error, &state->sdap_ret);
+    talloc_zfree(subreq);
 
+    if (ret != EOK) {
+        tevent_req_error(req, ret);
+        return;
+    }
 
-        default:
-            tevent_req_error(req, EINVAL);
+    if (state->sdap_ret == ENOENT && state->noexist_delete == true) {
+        ret = groups_get_handle_no_group(req);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE,
+                  "Could not delete group [%d]: %s\n", ret, sss_strerror(ret));
+            tevent_req_error(req, ret);
             return;
         }
     }
 
-    state->dp_error = DP_ERR_OK;
+    /* GID resolved to a user private group, done */
     tevent_req_done(req);
+    return;
+}
+
+static errno_t groups_get_handle_no_group(struct tevent_req *req)
+{
+    struct groups_get_state *state = tevent_req_data(req,
+                                                     struct groups_get_state);
+    errno_t ret;
+    char *endptr;
+    gid_t gid;
+
+    switch (state->filter_type) {
+    case BE_FILTER_ENUM:
+        ret = ENOENT;
+        break;
+    case BE_FILTER_NAME:
+        ret = sysdb_delete_group(state->domain, state->filter_value, 0);
+        if (ret != EOK && ret != ENOENT) {
+            DEBUG(SSSDBG_OP_FAILURE,
+                  "Cannot delete group %s [%d]: %s\n",
+                  state->filter_value, ret, sss_strerror(ret));
+            return ret;
+        }
+        break;
+    case BE_FILTER_IDNUM:
+        gid = (gid_t) strtouint32(state->filter_value, &endptr, 10);
+        if (errno || *endptr || (state->filter_value == endptr)) {
+            ret = errno ? errno : EINVAL;
+            break;
+        }
+
+        ret = sysdb_delete_group(state->domain, NULL, gid);
+        if (ret != EOK && ret != ENOENT) {
+            DEBUG(SSSDBG_OP_FAILURE,
+                  "Cannot delete group %"SPRIgid" [%d]: %s\n",
+                  gid, ret, sss_strerror(ret));
+            return ret;
+        }
+        break;
+    case BE_FILTER_SECID:
+    case BE_FILTER_UUID:
+        /* Since it is not clear if the SID/UUID belongs to a user or a
+         * group we have nothing to do here. */
+        ret = EOK;
+        break;
+    case BE_FILTER_WILDCARD:
+        /* We can't know if all groups are up-to-date, especially in
+         * a large environment. Do not delete any records, let the
+         * responder fetch the entries they are requested in.
+         */
+        ret = EOK;
+        break;
+    default:
+        ret = EINVAL;
+        break;
+    }
+
+    return ret;
 }
 
 int groups_get_recv(struct tevent_req *req, int *dp_error_out, int *sdap_ret)

From 1f25e43824c40781fd49e552d024d14da2b5f32e Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Tue, 10 Oct 2017 11:28:30 +0200
Subject: [PATCH 5/6] SYSDB: Prevent users and groups ID collision in MPG
 domains

This commit makes the check when adding an object in a MPG domain
stricter in the sense that not only same names are allowed in a MPG
domain, but also the same groups are not allowed either.

This commit is a backwards-incompatible change, but one that is needed,
otherwise requesting the duplicate group first and then requesting the
user entry would yield two object when searching by GID.

The patch also adjusts the tests accordingly:
    - test_local_domain.py - makes sure that the group uses a different
      GID than user's UID (previously, both used 10009)
    - sysdb-tests.c
      - adds an explicit test case test_sysdb_mpg_conflict()
      - test_sysdb_search_users_by_orig_dn() and test_upn_dup() now use
        a different ID. The IDs in thoses tests were chosen by random
        and by chance were conflicting with IDs used in a different test
        by another user or a group

Related:
    https://pagure.io/SSSD/sssd/issue/1872
---
 src/db/sysdb_ops.c                  | 37 ++++++++++++++++--
 src/tests/intg/test_local_domain.py | 20 +++++-----
 src/tests/sysdb-tests.c             | 76 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 118 insertions(+), 15 deletions(-)

diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 0e39a629a..9615c9478 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -1960,14 +1960,30 @@ int sysdb_add_user(struct sss_domain_info *domain,
     }
 
     if (domain->mpg) {
-        /* In MPG domains you can't have groups with the same name as users,
-         * search if a group with the same name exists.
+        /* 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
          * name the operation will fail */
 
         ret = sysdb_search_group_by_name(tmp_ctx, domain, name, NULL, &msg);
         if (ret != ENOENT) {
-            if (ret == EOK) ret = EEXIST;
+            if (ret == EOK) {
+                DEBUG(SSSDBG_OP_FAILURE,
+                      "Group named %s already exists in an MPG domain\n",
+                      name);
+                ret = EEXIST;
+            }
+            goto done;
+        }
+
+        ret = sysdb_search_group_by_gid(tmp_ctx, domain, uid, NULL, &msg);
+        if (ret != ENOENT) {
+            if (ret == EOK) {
+                DEBUG(SSSDBG_OP_FAILURE,
+                      "Group with GID [%"SPRIgid"] already exists in an MPG domain\n",
+                      gid);
+                ret = EEXIST;
+            }
             goto done;
         }
     }
@@ -2177,6 +2193,21 @@ int sysdb_add_group(struct sss_domain_info *domain,
             }
             goto done;
         }
+
+        ret = sysdb_search_user_by_uid(tmp_ctx, domain, gid, NULL, &msg);
+        if (ret != ENOENT) {
+            if (ret == EOK) {
+                DEBUG(SSSDBG_TRACE_LIBS,
+                      "User with the same UID exists in MPG domain: [%"SPRIgid"].\n",
+                      gid);
+                ret = EEXIST;
+            } else {
+                DEBUG(SSSDBG_TRACE_LIBS,
+                      "sysdb_search_user_by_uid failed for gid: "
+                      "[%"SPRIgid"].\n", gid);
+            }
+            goto done;
+        }
     }
 
     /* check no other groups with the same gid exist */
diff --git a/src/tests/intg/test_local_domain.py b/src/tests/intg/test_local_domain.py
index 8e1d6fb2b..87f74b08e 100644
--- a/src/tests/intg/test_local_domain.py
+++ b/src/tests/intg/test_local_domain.py
@@ -183,13 +183,13 @@ def test_add_local_user_to_local_group(local_domain_only):
     Regression test for ticket
     https://fedorahosted.org/sssd/ticket/3178
     """
-    subprocess.check_call(["sss_groupadd", "-g", "10009", "group10009"])
+    subprocess.check_call(["sss_groupadd", "-g", "10010", "group10010"])
     subprocess.check_call(["sss_useradd", "-u", "10009", "-M", "user10009"])
-    subprocess.check_call(["sss_usermod", "-a", "group10009", "user10009"])
+    subprocess.check_call(["sss_usermod", "-a", "group10010", "user10009"])
 
     ent.assert_group_by_name(
-        "group10009",
-        dict(name="group10009", passwd="*", gid=10009,
+        "group10010",
+        dict(name="group10010", passwd="*", gid=10010,
              mem=ent.contains_only("user10009")))
 
 
@@ -198,7 +198,7 @@ def test_add_local_group_to_local_group(local_domain_only):
     Regression test for tickets
     https://fedorahosted.org/sssd/ticket/3178
     """
-    subprocess.check_call(["sss_groupadd", "-g", "10009", "group_child"])
+    subprocess.check_call(["sss_groupadd", "-g", "10010", "group_child"])
     subprocess.check_call(["sss_useradd", "-u", "10009", "-M", "user_child"])
     subprocess.check_call(["sss_usermod", "-a", "group_child", "user_child"])
 
@@ -239,15 +239,15 @@ def test_add_local_user_to_local_group_fqdn(local_domain_only_fqdn):
     https://fedorahosted.org/sssd/ticket/3178
     """
     subprocess.check_call(
-        ["sss_groupadd", "-g", "10009", "group10009@LOCAL"])
+        ["sss_groupadd", "-g", "10010", "group10010@LOCAL"])
     subprocess.check_call(
         ["sss_useradd", "-u", "10009", "-M", "user10009@LOCAL"])
     subprocess.check_call(
-        ["sss_usermod", "-a", "group10009@LOCAL", "user10009@LOCAL"])
+        ["sss_usermod", "-a", "group10010@LOCAL", "user10009@LOCAL"])
 
     ent.assert_group_by_name(
-        "group10009@LOCAL",
-        dict(name="group10009@LOCAL", passwd="*", gid=10009,
+        "group10010@LOCAL",
+        dict(name="group10010@LOCAL", passwd="*", gid=10010,
              mem=ent.contains_only("user10009@LOCAL")))
 
 
@@ -257,7 +257,7 @@ def test_add_local_group_to_local_group_fqdn(local_domain_only_fqdn):
     https://fedorahosted.org/sssd/ticket/3178
     """
     subprocess.check_call(
-        ["sss_groupadd", "-g", "10009", "group_child@LOCAL"])
+        ["sss_groupadd", "-g", "10010", "group_child@LOCAL"])
     subprocess.check_call(
         ["sss_useradd", "-u", "10009", "-M", "user_child@LOCAL"])
     subprocess.check_call(
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index 465266108..c66b79431 100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -1482,6 +1482,73 @@ START_TEST (test_sysdb_add_nonposix_user)
 }
 END_TEST
 
+
+START_TEST (test_sysdb_mpg_conflict)
+{
+    int ret;
+    struct sysdb_test_ctx *test_ctx;
+    struct test_data *user_data = NULL;
+    struct test_data *user_fail_data = NULL;
+    struct test_data *group_data = NULL;
+    struct test_data *group_fail_data = NULL;
+
+    /* Setup */
+    ret = setup_sysdb_tests(&test_ctx);
+    fail_if(ret != EOK, "Could not set up the test");
+
+    user_data = test_data_new_user(test_ctx, 975);
+    fail_if(user_data == NULL);
+
+    ret = test_add_user(user_data);
+    fail_if(ret != EOK);
+
+    group_data = test_data_new_group(test_ctx, 864);
+    fail_if(group_data == NULL);
+
+    ret = test_add_group(group_data);
+    fail_if(ret != EOK);
+
+    /* Adding a user with the same UID as existing group's GID must fail */
+    user_fail_data = test_data_new_user(test_ctx, 864);
+    fail_if(user_fail_data == NULL);
+
+    ret = test_add_user(user_fail_data);
+    fail_unless(ret != EOK);
+    talloc_zfree(user_fail_data);
+
+    /* Adding a user with the same name as existing group must fail */
+    user_fail_data = test_data_new_user(test_ctx, 753);
+    fail_if(user_fail_data == NULL);
+    user_fail_data->username = group_data->groupname;
+
+    ret = test_add_user(user_fail_data);
+    fail_unless(ret != EOK);
+    talloc_zfree(user_fail_data);
+
+    /* Adding a group with the same GID as existing user's UID must fail */
+    group_fail_data = test_data_new_group(test_ctx, 975);
+    fail_if(group_fail_data == NULL);
+
+    ret = test_add_group(group_fail_data);
+    fail_unless(ret != EOK);
+    talloc_zfree(group_fail_data);
+
+    /* Adding a group with the same name as existing user's must fail */
+    group_fail_data = test_data_new_group(test_ctx, 642);
+    fail_if(group_fail_data == NULL);
+    group_fail_data->groupname = user_data->username;
+
+    ret = test_add_group(group_fail_data);
+    fail_unless(ret != EOK);
+    talloc_zfree(group_fail_data);
+
+
+    /* Done! */
+    talloc_free(user_data);
+    talloc_free(group_data);
+}
+END_TEST
+
 START_TEST (test_sysdb_add_group_member)
 {
     struct sysdb_test_ctx *test_ctx;
@@ -5453,7 +5520,7 @@ START_TEST(test_sysdb_search_users_by_orig_dn)
     ret = setup_sysdb_tests(&test_ctx);
     fail_if(ret != EOK, "Could not set up the test");
 
-    data = test_data_new_user(test_ctx, 456789);
+    data = test_data_new_user(test_ctx, 3456789);
     fail_if(data == NULL);
 
     data->orig_dn = talloc_asprintf(data, "cn=%s,cn=example,cn=com", data->username);
@@ -6653,7 +6720,7 @@ START_TEST(test_upn_dup)
 
     ret = sysdb_store_user(test_ctx->domain,
                            UPN_USER_NAME"_dup", "x",
-                           23456, 0, "UPN USER DUP", "/home/upn_user_dup",
+                           123456, 0, "UPN USER DUP", "/home/upn_user_dup",
                            "/bin/bash", NULL,
                            attrs, NULL, -1, 0);
     fail_unless(ret == EOK, "Could not store user.");
@@ -7182,6 +7249,11 @@ Suite *create_sysdb_suite(void)
     /* Test adding a non-POSIX user */
     tcase_add_test(tc_sysdb, test_sysdb_add_nonposix_user);
 
+    /* Test adding a user and a group with the same name and ID
+     * in a MPG domain
+     */
+    tcase_add_test(tc_sysdb, test_sysdb_mpg_conflict);
+
 /* ===== NETGROUP TESTS ===== */
 
     /* Create a new netgroup */

From e66765a023f2adeeb475ee8d25fff7680d4126b1 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Tue, 3 Oct 2017 16:55:40 +0200
Subject: [PATCH 6/6] TESTS: Add integration tests for the auto_private_groups
 option

Related:
    https://pagure.io/SSSD/sssd/issue/1872
---
 src/tests/intg/test_enumeration.py |  79 +++++++++++++++-
 src/tests/intg/test_ldap.py        | 187 +++++++++++++++++++++++++++++++++++++
 2 files changed, 263 insertions(+), 3 deletions(-)

diff --git a/src/tests/intg/test_enumeration.py b/src/tests/intg/test_enumeration.py
index fdb8d3768..c7d78155c 100644
--- a/src/tests/intg/test_enumeration.py
+++ b/src/tests/intg/test_enumeration.py
@@ -237,9 +237,7 @@ def sanity_rfc2307(request, ldap_conn):
     create_sssd_fixture(request)
     return None
 
-
-@pytest.fixture
-def sanity_rfc2307_bis(request, ldap_conn):
+def populate_rfc2307bis(request, ldap_conn):
     ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
     ent_list.add_user("user1", 1001, 2001)
     ent_list.add_user("user2", 1002, 2002)
@@ -266,6 +264,11 @@ def sanity_rfc2307_bis(request, ldap_conn):
                            [], ["one_user_group1", "one_user_group2"])
 
     create_ldap_fixture(request, ldap_conn, ent_list)
+
+
+@pytest.fixture
+def sanity_rfc2307_bis(request, ldap_conn):
+    populate_rfc2307bis(request, ldap_conn)
     conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
     create_conf_fixture(request, conf)
     create_sssd_fixture(request)
@@ -695,3 +698,73 @@ def test_vetoed_shells(vetoed_shells):
                  shell="/bin/default")
         )
     )
+
+
+@pytest.fixture
+def sanity_rfc2307_bis_mpg(request, ldap_conn):
+    populate_rfc2307bis(request, ldap_conn)
+
+    ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+    ent_list.add_group_bis("conflict1", 1001)
+    ent_list.add_group_bis("conflict2", 1002)
+    create_ldap_fixture(request, ldap_conn, ent_list)
+
+    conf = \
+        format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS) + \
+        unindent("""
+            [domain/LDAP]
+            auto_private_groups = True
+        """).format(**locals())
+    create_conf_fixture(request, conf)
+    create_sssd_fixture(request)
+    return None
+
+
+def test_ldap_auto_private_groups_enumerate(ldap_conn,
+                                            sanity_rfc2307_bis_mpg):
+    """
+    Test the auto_private_groups together with enumeration
+    """
+    passwd_pattern = ent.contains_only(
+        dict(name='user1', passwd='*', uid=1001, gid=1001, gecos='1001',
+             dir='/home/user1', shell='/bin/bash'),
+        dict(name='user2', passwd='*', uid=1002, gid=1002, gecos='1002',
+             dir='/home/user2', shell='/bin/bash'),
+        dict(name='user3', passwd='*', uid=1003, gid=1003, gecos='1003',
+             dir='/home/user3', shell='/bin/bash')
+    )
+    ent.assert_passwd(passwd_pattern)
+
+    group_pattern = ent.contains_only(
+        dict(name='user1', passwd='*', gid=1001, mem=ent.contains_only()),
+        dict(name='user2', passwd='*', gid=1002, mem=ent.contains_only()),
+        dict(name='user3', passwd='*', gid=1003, mem=ent.contains_only()),
+        dict(name='group1', passwd='*', gid=2001, mem=ent.contains_only()),
+        dict(name='group2', passwd='*', gid=2002, mem=ent.contains_only()),
+        dict(name='group3', passwd='*', gid=2003, mem=ent.contains_only()),
+        dict(name='empty_group1', passwd='*', gid=2010,
+             mem=ent.contains_only()),
+        dict(name='empty_group2', passwd='*', gid=2011,
+             mem=ent.contains_only()),
+        dict(name='two_user_group', passwd='*', gid=2012,
+             mem=ent.contains_only("user1", "user2")),
+        dict(name='group_empty_group', passwd='*', gid=2013,
+             mem=ent.contains_only()),
+        dict(name='group_two_empty_groups', passwd='*', gid=2014,
+             mem=ent.contains_only()),
+        dict(name='one_user_group1', passwd='*', gid=2015,
+             mem=ent.contains_only("user1")),
+        dict(name='one_user_group2', passwd='*', gid=2016,
+             mem=ent.contains_only("user2")),
+        dict(name='group_one_user_group', passwd='*', gid=2017,
+             mem=ent.contains_only("user1")),
+        dict(name='group_two_user_group', passwd='*', gid=2018,
+             mem=ent.contains_only("user1", "user2")),
+        dict(name='group_two_one_user_groups', passwd='*', gid=2019,
+             mem=ent.contains_only("user1", "user2"))
+    )
+    ent.assert_group(group_pattern)
+
+    with pytest.raises(KeyError):
+        grp.getgrnam("conflict1")
+    ent.assert_group_by_gid(1002, dict(name="user2", mem=ent.contains_only()))
diff --git a/src/tests/intg/test_ldap.py b/src/tests/intg/test_ldap.py
index f2467f1ff..163e58efd 100644
--- a/src/tests/intg/test_ldap.py
+++ b/src/tests/intg/test_ldap.py
@@ -34,6 +34,7 @@
 import sssd_id
 import sssd_ldb
 from util import unindent
+from util import run_shell
 from sssd_nss import NssReturnCode
 from sssd_passwd import call_sssd_getpwnam, call_sssd_getpwuid
 from sssd_group import call_sssd_getgrnam, call_sssd_getgrgid
@@ -1169,3 +1170,189 @@ def test_nss_filters_cached(ldap_conn, sanity_nss_filter_cached):
 
     res, _ = call_sssd_getgrgid(0)
     assert res == NssReturnCode.NOTFOUND
+
+
+@pytest.fixture
+def mpg_setup(request, ldap_conn):
+    ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+    ent_list.add_user("user1", 1001, 2001)
+    ent_list.add_user("user2", 1002, 2002)
+
+    ent_list.add_group_bis("group1", 2001)
+    ent_list.add_group_bis("group2", 2002)
+
+    ent_list.add_group_bis("two_user_group", 2012, ["user1", "user2"])
+    ent_list.add_group_bis("one_user_group1", 2015, ["user1"])
+    ent_list.add_group_bis("one_user_group2", 2016, ["user2"])
+
+    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 = True
+        """).format(**locals())
+    create_conf_fixture(request, conf)
+    create_sssd_fixture(request)
+    return None
+
+
+def test_ldap_auto_private_groups_direct(ldap_conn, mpg_setup):
+    """
+    Integration test for auto_private_groups
+
+    See also ticket https://pagure.io/SSSD/sssd/issue/1872
+    """
+    # Make sure the user's GID is taken from their uidNumber
+    ent.assert_passwd_by_name("user1", dict(name="user1", uid=1001, gid=1001))
+    # Make sure the private group is resolvable by name and by GID
+    ent.assert_group_by_name("user1", dict(gid=1001, mem=ent.contains_only()))
+    ent.assert_group_by_gid(1001, dict(name="user1", mem=ent.contains_only()))
+
+    # The group referenced in user's gidNumber attribute should be still
+    # visible, but shouldn't have any relation to the user
+    ent.assert_group_by_name("group1", dict(gid=2001, mem=ent.contains_only()))
+    ent.assert_group_by_gid(2001, dict(name="group1", mem=ent.contains_only()))
+
+    # The user's secondary groups list must be correct as well
+    # Note that the original GID is listed as well -- this is correct and expected
+    # because we save the original GID in the SYSDB_PRIMARY_GROUP_GIDNUM attribute
+    user1_expected_gids = [1001, 2001, 2012, 2015]
+    (res, errno, gids) = sssd_id.call_sssd_initgroups("user1", 1001)
+    assert res == sssd_id.NssReturnCode.SUCCESS
+
+    assert sorted(gids) == sorted(user1_expected_gids), \
+        "result: %s\n expected %s" % (
+            ", ".join(["%s" % s for s in sorted(gids)]),
+            ", ".join(["%s" % s for s in sorted(user1_expected_gids)])
+        )
+
+    # Request user2's private group without resolving the user first. This must
+    # trigger user resolution through by-GID resolution, since the GID
+    # doesn't exist on its own in LDAP
+    ent.assert_group_by_gid(1002, dict(name="user2", mem=ent.contains_only()))
+
+    # Test supplementary groups for user2 as well
+    user1_expected_gids = [1002, 2002, 2012, 2016]
+    (res, errno, gids) = sssd_id.call_sssd_initgroups("user2", 1002)
+    assert res == sssd_id.NssReturnCode.SUCCESS
+
+    assert sorted(gids) == sorted(user1_expected_gids), \
+        "result: %s\n expected %s" % (
+            ", ".join(["%s" % s for s in sorted(gids)]),
+            ", ".join(["%s" % s for s in sorted(user1_expected_gids)])
+        )
+
+
+    # Remove entries and request them again to make sure they are not
+    # resolvable anymore
+    cleanup_ldap_entries(ldap_conn, None)
+
+    if subprocess.call(["sss_cache", "-GU"]) != 0:
+        raise Exception("sssd_cache failed")
+
+    with pytest.raises(KeyError):
+        pwd.getpwnam("user1")
+    with pytest.raises(KeyError):
+        grp.getgrnam("user1")
+    with pytest.raises(KeyError):
+        grp.getgrgid(1002)
+
+
+@pytest.fixture
+def mpg_setup_conflict(request, ldap_conn):
+    ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+    ent_list.add_user("user1", 1001, 2001)
+    ent_list.add_user("user2", 1002, 2002)
+    ent_list.add_group_bis("group1", 1001)
+    ent_list.add_group_bis("group2", 1002)
+    create_ldap_fixture(request, ldap_conn, ent_list)
+
+    conf = \
+        format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS) + \
+        unindent("""
+            [domain/LDAP]
+            auto_private_groups = True
+        """).format(**locals())
+    create_conf_fixture(request, conf)
+    create_sssd_fixture(request)
+    return None
+
+
+def test_ldap_auto_private_groups_conflict(ldap_conn, mpg_setup_conflict):
+    """
+    Make sure that conflicts between groups that are auto-created with the
+    help of the auto_private_groups option and between 'real' LDAP groups
+    are handled in a predictable manner.
+    """
+    # Make sure the user's GID is taken from their uidNumber
+    ent.assert_passwd_by_name("user1", dict(name="user1", uid=1001, gid=1001))
+    # Make sure the private group is resolvable by name and by GID
+    ent.assert_group_by_name("user1", dict(gid=1001, mem=ent.contains_only()))
+    ent.assert_group_by_gid(1001, dict(name="user1", mem=ent.contains_only()))
+
+    # Let's request the group with the same ID as user2's private group
+    # The request should match the 'real' group
+    ent.assert_group_by_gid(1002, dict(name="group2", mem=ent.contains_only()))
+    # But because of the GID conflict, the user cannot be resolved
+    with pytest.raises(KeyError):
+        pwd.getpwnam("user2")
+
+
+@pytest.fixture
+def mpg_setup_no_gid(request, ldap_conn):
+    ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+    ent_list.add_user("user1", 1001, 2001)
+
+    ent_list.add_group_bis("group1", 2001)
+    ent_list.add_group_bis("one_user_group1", 2015, ["user1"])
+
+    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 = True
+            ldap_user_gid_number = no_such_attribute
+        """).format(**locals())
+    create_conf_fixture(request, conf)
+    create_sssd_fixture(request)
+    return None
+
+
+def test_ldap_auto_private_groups_direct_no_gid(ldap_conn, mpg_setup_no_gid):
+    """
+    Integration test for auto_private_groups - test that even a user with
+    no GID assigned at all can be resolved including their autogenerated
+    primary group.
+
+    See also ticket https://pagure.io/SSSD/sssd/issue/1872
+    """
+    # Make sure the user's GID is taken from their uidNumber
+    ent.assert_passwd_by_name("user1", dict(name="user1", uid=1001, gid=1001))
+    # Make sure the private group is resolvable by name and by GID
+    ent.assert_group_by_name("user1", dict(gid=1001, mem=ent.contains_only()))
+    ent.assert_group_by_gid(1001, dict(name="user1", mem=ent.contains_only()))
+
+    # The group referenced in user's gidNumber attribute should be still
+    # visible, but shouldn't have any relation to the user
+    ent.assert_group_by_name("group1", dict(gid=2001, mem=ent.contains_only()))
+    ent.assert_group_by_gid(2001, dict(name="group1", mem=ent.contains_only()))
+
+    # The user's secondary groups list must be correct as well. This time only
+    # the generated group and the explicit secondary group are added, since
+    # there is no original GID
+    user1_expected_gids = [1001, 2015]
+    #run_shell()
+    (res, errno, gids) = sssd_id.call_sssd_initgroups("user1", 1001)
+    assert res == sssd_id.NssReturnCode.SUCCESS
+
+    assert sorted(gids) == sorted(user1_expected_gids), \
+        "result: %s\n expected %s" % (
+            ", ".join(["%s" % s for s in sorted(gids)]),
+            ", ".join(["%s" % s for s in sorted(user1_expected_gids)])
+        )
