Hi,

see the attached patches for ticket
https://fedorahosted.org/sssd/ticket/2676

Removing the user during cleanup task completely
removes that user from local database, so that
user is no longer part of any group in sysdb.

Such user may, or may not have been removed from
LDAP so the only way to figure out is check
on the next refresh otherwise we may get
inconsistent results.

The third patch adds integration test for this
Apply and run the intgcheck without the first 2
patches to see the failed test (you will also see that
all the tests after this failed one will end with
errors as well, that is probably due to incomplete
clean up after the ldap tests and I do not intend to
solve this as part of this patchset but will
investigate it later).

Thanks,
Michal
--
Senior Principal Intern
>From 9930382acae9eb708c609355131207a0be42d1e9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Thu, 6 Aug 2015 09:16:03 +0200
Subject: [PATCH 1/3] SYSDB: Add function to expire entry

Ticket:
https://fedorahosted.org/sssd/ticket/2676

Added function to expire entry in sysdb using
its DN.
---
 src/db/sysdb.h          | 10 ++++++-
 src/db/sysdb_ops.c      | 51 ++++++++++++++++++++++++++++++++++++
 src/tests/sysdb-tests.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 129 insertions(+), 1 deletion(-)

diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index 9e28b5c..a48e22d 100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -387,6 +387,11 @@ errno_t sysdb_msg2attrs(TALLOC_CTX *mem_ctx, size_t count,
 /* convert an ldb error into an errno error */
 int sysdb_error_to_errno(int ldberr);
 
+/* Convert ldb value to ldb dn */
+struct ldb_dn *sysdb_ldb_val_to_ldb_dn(TALLOC_CTX *tctx,
+                                       struct sss_domain_info *dom,
+                                       struct ldb_val *ldbval);
+
 /* DNs related helper functions */
 errno_t sysdb_get_rdn(struct sysdb_ctx *sysdb, TALLOC_CTX *mem_ctx,
                       const char *dn, char **_name, char **_val);
@@ -717,11 +722,14 @@ int sysdb_delete_entry(struct sysdb_ctx *sysdb,
                        struct ldb_dn *dn,
                        bool ignore_not_found);
 
-
 int sysdb_delete_recursive(struct sysdb_ctx *sysdb,
                            struct ldb_dn *dn,
                            bool ignore_not_found);
 
+/* Mark entry as expired (permissive) */
+int sysdb_mark_entry_as_expired_ldb_dn(struct sss_domain_info *dom,
+                                       struct ldb_dn *ldbdn);
+
 /* Search Entry */
 int sysdb_search_entry(TALLOC_CTX *mem_ctx,
                        struct sysdb_ctx *sysdb,
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index d1d43eb..c582578 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -3875,3 +3875,54 @@ errno_t sysdb_handle_original_uuid(const char *orig_name,
 
     return EOK;
 }
+
+struct ldb_dn *sysdb_ldb_val_to_ldb_dn(TALLOC_CTX *tctx,
+                                       struct sss_domain_info *dom,
+                                       struct ldb_val *ldbval)
+{
+    return ldb_dn_from_ldb_val(tctx, dom->sysdb->ldb, ldbval);
+}
+
+/* Mark entry as expired if that entry exists in database. */
+int sysdb_mark_entry_as_expired_ldb_dn(struct sss_domain_info *dom,
+                                       struct ldb_dn *ldbdn)
+{
+    struct ldb_message *msg;
+    int ret;
+    TALLOC_CTX *tmp_ctx;
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        return ENOMEM;
+    }
+
+    msg = ldb_msg_new(tmp_ctx);
+    if (msg == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    msg->dn = ldbdn;
+
+    ret = ldb_msg_add_empty(msg, SYSDB_CACHE_EXPIRE,
+                            LDB_FLAG_MOD_REPLACE, NULL);
+    if (ret != LDB_SUCCESS) {
+        goto done;
+    }
+
+    ret = ldb_msg_add_string(msg, SYSDB_CACHE_EXPIRE, "1");
+    if (ret != LDB_SUCCESS) {
+        goto done;
+    }
+
+    ret = sss_ldb_modify_permissive(dom->sysdb->ldb, msg);
+    if (ret != LDB_SUCCESS) {
+        goto done;
+    }
+
+    ret = EOK;
+done:
+    talloc_free(tmp_ctx);
+    return ret;
+}
+
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index 24d1527..3bd03b1 100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -6212,6 +6212,74 @@ START_TEST(test_confdb_list_all_domain_names_multi_dom)
 }
 END_TEST
 
+START_TEST(test_sysdb_mark_entry_as_expired_ldb_dn)
+{
+    errno_t ret;
+    struct sysdb_test_ctx *test_ctx;
+    const char *attrs[] = { SYSDB_CACHE_EXPIRE, NULL };
+    size_t count;
+    struct ldb_message **msgs;
+    uint64_t expire;
+    struct ldb_dn *userdn;
+
+    ret = setup_sysdb_tests(&test_ctx);
+    fail_if(ret != EOK, "Could not setup the test");
+
+    /* Add something to database to test against */
+
+    ret = sysdb_transaction_start(test_ctx->sysdb);
+    ck_assert_int_eq(ret, EOK);
+
+    ret = sysdb_add_user(test_ctx->domain, "testuser",
+                         2000, 0, "Test User", "/home/testuser",
+                         "/bin/bash",
+                         NULL, NULL, 500, 0);
+    ck_assert_int_eq(ret, EOK);
+
+    ret = sysdb_transaction_commit(test_ctx->sysdb);
+    ck_assert_int_eq(ret, EOK);
+
+    ret = sysdb_search_users(test_ctx, test_ctx->domain,
+                             "("SYSDB_UIDNUM"=2000)", attrs, &count, &msgs);
+    ck_assert_int_eq(ret, EOK);
+    ck_assert_int_eq(count, 1);
+
+    expire = ldb_msg_find_attr_as_uint64(msgs[0], SYSDB_CACHE_EXPIRE, 0);
+    ck_assert(expire != 1);
+
+    userdn = sysdb_user_dn(test_ctx, test_ctx->domain, "testuser");
+    ck_assert(userdn != NULL);
+
+    ret = sysdb_transaction_start(test_ctx->sysdb);
+    ck_assert_int_eq(ret, EOK);
+
+    /* Expire entry */
+    ret = sysdb_mark_entry_as_expired_ldb_dn(test_ctx->domain, userdn);
+    ck_assert_int_eq(ret, EOK);
+
+    ret = sysdb_transaction_commit(test_ctx->sysdb);
+    ck_assert_int_eq(ret, EOK);
+
+    ret = sysdb_search_users(test_ctx, test_ctx->domain,
+                             "("SYSDB_UIDNUM"=2000)", attrs, &count, &msgs);
+    ck_assert_int_eq(ret, EOK);
+    ck_assert_int_eq(count, 1);
+
+    expire = ldb_msg_find_attr_as_uint64(msgs[0], SYSDB_CACHE_EXPIRE, 0);
+    ck_assert_int_eq(expire, 1);
+
+    /* Attempt to mark missing entry should be OK */
+    ret = sysdb_transaction_start(test_ctx->sysdb);
+    ck_assert_int_eq(ret, EOK);
+
+    ret = sysdb_mark_entry_as_expired_ldb_dn(test_ctx->domain, userdn);
+    ck_assert_int_eq(ret, EOK);
+
+    ret = sysdb_transaction_commit(test_ctx->sysdb);
+    ck_assert_int_eq(ret, EOK);
+}
+END_TEST
+
 Suite *create_sysdb_suite(void)
 {
     Suite *s = suite_create("sysdb");
@@ -6424,6 +6492,7 @@ Suite *create_sysdb_suite(void)
 
 /* ===== Misc ===== */
     tcase_add_test(tc_sysdb, test_sysdb_set_get_bool);
+    tcase_add_test(tc_sysdb, test_sysdb_mark_entry_as_expired_ldb_dn);
 
 /* Add all test cases to the test suite */
     suite_add_tcase(s, tc_sysdb);
-- 
2.1.0

>From 5d6edc83c09b36f34beb9118d1c528b5229600d8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Wed, 29 Jul 2015 17:18:06 +0200
Subject: [PATCH 2/3] cleanup task: Expire all memberof targets when removing
 user

Ticket:
https://fedorahosted.org/sssd/ticket/2676

When user removed from cache during cleanup task, mark all
his memberof targets as expired to refresh member/ghost
attributes on next request.
---
 src/providers/ldap/ldap_id_cleanup.c | 57 ++++++++++++++++++++++++++++++++++--
 1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/src/providers/ldap/ldap_id_cleanup.c b/src/providers/ldap/ldap_id_cleanup.c
index be9496a..93a9362 100644
--- a/src/providers/ldap/ldap_id_cleanup.c
+++ b/src/providers/ldap/ldap_id_cleanup.c
@@ -169,11 +169,14 @@ done:
 static int cleanup_users_logged_in(hash_table_t *table,
                                    const struct ldb_message *msg);
 
+static int expire_memberof_target_grps(struct sss_domain_info *dom,
+                                       struct ldb_message *user);
+
 static int cleanup_users(struct sdap_options *opts,
                          struct sss_domain_info *dom)
 {
     TALLOC_CTX *tmpctx;
-    const char *attrs[] = { SYSDB_NAME, SYSDB_UIDNUM, NULL };
+    const char *attrs[] = { SYSDB_NAME, SYSDB_UIDNUM, SYSDB_MEMBEROF, NULL };
     time_t now = time(NULL);
     char *subfilter = NULL;
     int account_cache_expiration;
@@ -188,7 +191,7 @@ static int cleanup_users(struct sdap_options *opts,
     if (!tmpctx) {
         return ENOMEM;
     }
-
+    account_cache_expiration = dp_opt_get_int(opts->basic, SDAP_ACCOUNT_CACHE_EXPIRATION);
     account_cache_expiration = dp_opt_get_int(opts->basic, SDAP_ACCOUNT_CACHE_EXPIRATION);
     DEBUG(SSSDBG_TRACE_ALL, "Cache expiration is set to %d days\n",
               account_cache_expiration);
@@ -271,6 +274,17 @@ static int cleanup_users(struct sdap_options *opts,
             DEBUG(SSSDBG_CRIT_FAILURE, "sysdb_delete_user failed: %d\n", ret);
             goto done;
         }
+
+        /* Mark all groups of which user was a member as expired in cache,
+         * so that its ghost/member attributes are refreshed on next
+         * request. */
+        ret = expire_memberof_target_grps(dom, msgs[i]);
+        if (ret && ret != ENOENT) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  "expire_memberof_target_grps failed: [%d]:%s\n",
+                  ret, sss_strerror(ret));
+            goto done;
+        }
     }
 
 done:
@@ -278,6 +292,45 @@ done:
     return ret;
 }
 
+static int expire_memberof_target_grps(struct sss_domain_info *dom,
+                                       struct ldb_message *user)
+{
+    struct ldb_message_element *memberof_el;
+    struct ldb_dn *ldb_dn;
+    int ret;
+    int i;
+    TALLOC_CTX *tmp_ctx;
+
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        return ENOMEM;
+    }
+
+    memberof_el = ldb_msg_find_element(user, SYSDB_MEMBEROF);
+
+    for (i = 0; i < memberof_el->num_values; i++) {
+        ldb_dn = sysdb_ldb_val_to_ldb_dn(tmp_ctx, dom,
+                                         &memberof_el->values[i]);
+        if (ldb_dn == NULL) {
+            ret = ENOMEM;
+            goto done;
+        }
+
+        ret = sysdb_mark_entry_as_expired_ldb_dn(dom, ldb_dn);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  "sysdb_mark_entry_as_expired_ldb_dn failed: [%d]: %s\n",
+                  ret, sss_strerror(ret));
+            goto done;
+        }
+    }
+
+    ret = EOK;
+done:
+    talloc_free(tmp_ctx);
+    return ret;
+}
+
 static int cleanup_users_logged_in(hash_table_t *table,
                                    const struct ldb_message *msg)
 {
-- 
2.1.0

>From cf61a2037fc3cfb6ddee040e6423463a08e74d2c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com>
Date: Thu, 6 Aug 2015 13:16:48 +0200
Subject: [PATCH 3/3] CI: Add regression test for #2676

Ticket:
https://fedorahosted.org/sssd/ticket/2676

Regression test for the above ticket.
---
 src/tests/intg/ldap_test.py | 61 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py
index bfe4e65..19968e3 100644
--- a/src/tests/intg/ldap_test.py
+++ b/src/tests/intg/ldap_test.py
@@ -260,3 +260,64 @@ def test_sanity_rfc2307_bis(ldap_conn, sanity_rfc2307_bis):
         grp.getgrnam("non_existent_group")
     with pytest.raises(KeyError):
         grp.getgrgid(1)
+
+
+@pytest.fixture
+def refresh_after_cleanup_task(request, ldap_conn):
+    ent_list = ldap_ent.List(LDAP_BASE_DN)
+    ent_list.add_user("user1", 1001, 2001)
+
+    ent_list.add_group_bis("group1", 2001, ["user1"])
+    ent_list.add_group_bis("group2", 2002, [], ["group1"])
+
+    create_ldap_fixture(request, ldap_conn, ent_list)
+
+    conf = unindent("""\
+        [sssd]
+        config_file_version     = 2
+        domains                 = LDAP
+        services                = nss
+
+        [nss]
+        memcache_timeout        = 0
+
+        [domain/LDAP]
+        ldap_auth_disable_tls_never_use_in_production = true
+        debug_level             = 0xffff
+        ldap_schema             = rfc2307bis
+        ldap_group_object_class = groupOfNames
+        id_provider             = ldap
+        auth_provider           = ldap
+        sudo_provider           = ldap
+        ldap_uri                = {ldap_conn.ds_inst.ldap_url}
+        ldap_search_base        = {ldap_conn.ds_inst.base_dn}
+
+        entry_cache_timeout = 1
+        entry_cache_group_timeout = 5000
+        ldap_purge_cache_timeout = 3
+
+    """).format(**locals())
+    create_conf_fixture(request, conf)
+    create_sssd_fixture(request)
+    return None
+
+
+def test_refresh_after_cleanup_task(ldap_conn, refresh_after_cleanup_task):
+    """
+    Regression test for ticket:
+    https://fedorahosted.org/sssd/ticket/2676
+    """
+    ent.assert_group_by_name(
+        "group2",
+        dict(mem=ent.contains_only("user1")))
+
+    ent.assert_passwd_by_name(
+        'user1',
+        dict(name='user1', passwd='*', uid=1001, gid=2001,
+             gecos='1001', shell='/bin/bash'))
+
+    time.sleep(10)
+
+    ent.assert_group_by_name(
+        "group2",
+        dict(mem=ent.contains_only("user1")))
-- 
2.1.0

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to