SYSDB: Fix setting dataExpireTimestamp if sysdb is supposed to set the current time

sysdb is already able to retrieve the current timestamp if the caller
doesn't specify it. However, for the timestamp cache this came too late
and the timestamp cache used zero as the 'now' time.

Resolves:
https://fedorahosted.org/sssd/ticket/3064

Reviewed-by: Pavel Březina <pbrezina@redhat.com>

2 files changed, 93 insertions, 10 deletions

diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 342e16fb2..67006c155 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -2465,6 +2465,11 @@ int sysdb_store_user(struct sss_domain_info *domain,
     errno_t sret = EOK;
     bool in_transaction = false;
 
+    /* get transaction timestamp */
+    if (now == 0) {
+        now = time(NULL);
+    }
+
     ret = sysdb_check_and_update_ts_usr(domain, name, attrs,
                                         cache_timeout, now);
     if (ret == EOK) {
@@ -2508,11 +2513,6 @@ int sysdb_store_user(struct sss_domain_info *domain,
         DEBUG(SSSDBG_TRACE_LIBS, "User %s does not exist.\n", name);
     }
 
-    /* get transaction timestamp */
-    if (!now) {
-        now = time(NULL);
-    }
-
     if (ret == ENOENT) {
         /* the user doesn't exist, turn into adding a user */
         ret = sysdb_store_new_user(domain, name, uid, gid, gecos, homedir,
@@ -2700,6 +2700,11 @@ int sysdb_store_group(struct sss_domain_info *domain,
     errno_t sret = EOK;
     bool in_transaction = false;
 
+    /* get transaction timestamp */
+    if (!now) {
+        now = time(NULL);
+    }
+
     ret = sysdb_check_and_update_ts_grp(domain, name, attrs,
                                         cache_timeout, now);
     if (ret == EOK) {
@@ -2741,11 +2746,6 @@ int sysdb_store_group(struct sss_domain_info *domain,
         }
     }
 
-    /* get transaction timestamp */
-    if (!now) {
-        now = time(NULL);
-    }
-
     if (new_group) {
         ret = sysdb_store_new_group(domain, name, gid, attrs,
                                     cache_timeout, now);
diff --git a/src/tests/cmocka/test_sysdb_ts_cache.c b/src/tests/cmocka/test_sysdb_ts_cache.c
index d54922996..aa857e7e4 100644
--- a/src/tests/cmocka/test_sysdb_ts_cache.c
+++ b/src/tests/cmocka/test_sysdb_ts_cache.c
@@ -1348,6 +1348,86 @@ static void test_user_byupn(void **state)
     talloc_free(res);
 }
 
+static void test_sysdb_zero_now(void **state)
+{
+    int ret;
+    struct sysdb_ts_test_ctx *test_ctx = talloc_get_type_abort(*state,
+                                                     struct sysdb_ts_test_ctx);
+    struct ldb_result *res = NULL;
+    uint64_t cache_expire_sysdb;
+    uint64_t cache_expire_ts;
+    struct sysdb_attrs *attrs = NULL;
+
+    /* Nothing must be stored in either cache at the beginning of the test */
+    res = sysdb_getpwnam_res(test_ctx, test_ctx->tctx->dom, TEST_USER_NAME);
+    assert_int_equal(res->count, 0);
+    talloc_free(res);
+
+    res = sysdb_getgrnam_res(test_ctx, test_ctx->tctx->dom, TEST_GROUP_NAME);
+    assert_int_equal(res->count, 0);
+    talloc_free(res);
+
+    attrs = create_modstamp_attrs(test_ctx, TEST_MODSTAMP_1);
+    assert_non_null(attrs);
+
+    ret = sysdb_store_user(test_ctx->tctx->dom, TEST_USER_NAME, NULL,
+                           TEST_USER_UID, TEST_USER_GID, TEST_USER_NAME,
+                           "/home/"TEST_USER_NAME, "/bin/bash", NULL,
+                           attrs, NULL, TEST_CACHE_TIMEOUT,
+                           0);
+    talloc_zfree(attrs);
+    assert_int_equal(ret, EOK);
+
+    attrs = create_modstamp_attrs(test_ctx, TEST_MODSTAMP_1);
+    assert_non_null(attrs);
+
+    ret = sysdb_store_group(test_ctx->tctx->dom,
+                            TEST_GROUP_NAME,
+                            TEST_GROUP_GID,
+                            attrs,
+                            TEST_CACHE_TIMEOUT,
+                            0);
+    talloc_zfree(attrs);
+    assert_int_equal(ret, EOK);
+    talloc_zfree(attrs);
+
+    attrs = create_modstamp_attrs(test_ctx, TEST_MODSTAMP_1);
+    assert_non_null(attrs);
+
+    ret = sysdb_store_user(test_ctx->tctx->dom, TEST_USER_NAME, NULL,
+                           TEST_USER_UID, TEST_USER_GID, TEST_USER_NAME,
+                           "/home/"TEST_USER_NAME, "/bin/bash", NULL,
+                           attrs, NULL, TEST_CACHE_TIMEOUT,
+                           0);
+    talloc_zfree(attrs);
+    assert_int_equal(ret, EOK);
+
+    attrs = create_modstamp_attrs(test_ctx, TEST_MODSTAMP_1);
+    assert_non_null(attrs);
+
+    ret = sysdb_store_group(test_ctx->tctx->dom,
+                            TEST_GROUP_NAME,
+                            TEST_GROUP_GID,
+                            attrs,
+                            TEST_CACHE_TIMEOUT,
+                            0);
+    talloc_zfree(attrs);
+    assert_int_equal(ret, EOK);
+
+    /* Even though we passed zero as the timestamp, the timestamp cache should
+     * have used the current time instead
+     */
+    get_pw_timestamp_attrs(test_ctx, TEST_USER_NAME,
+                           &cache_expire_sysdb, &cache_expire_ts);
+    assert_true(cache_expire_sysdb > TEST_CACHE_TIMEOUT);
+    assert_true(cache_expire_ts > TEST_CACHE_TIMEOUT);
+
+    get_gr_timestamp_attrs(test_ctx, TEST_GROUP_NAME,
+                           &cache_expire_sysdb, &cache_expire_ts);
+    assert_true(cache_expire_sysdb > TEST_CACHE_TIMEOUT);
+    assert_true(cache_expire_ts > TEST_CACHE_TIMEOUT);
+}
+
 int main(int argc, const char *argv[])
 {
     int rv;
@@ -1396,6 +1476,9 @@ int main(int argc, const char *argv[])
         cmocka_unit_test_setup_teardown(test_user_byupn,
                                         test_sysdb_ts_setup,
                                         test_sysdb_ts_teardown),
+        cmocka_unit_test_setup_teardown(test_sysdb_zero_now,
+                                        test_sysdb_ts_setup,
+                                        test_sysdb_ts_teardown),
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */