Re: [Freeipa-devel] PATCH: Improve on #2795 patches

[Simo Sorce]

On Wed, 2016-07-20 at 15:17 +0200, David Kupka wrote:
> On 20/07/16 12:11, Simo Sorce wrote:
> > Attached patch introduces a helper function and avoids the questionable
> > replace+delete operations where possible (still employed in the
> > entry_to_mods function).
> > Compiles and I am about to test it, but I'd like feedback on it if
> > anyone wants to take a look.
> >
> > Simo.
> >
> >
> >
> 
> Looks and works good, ACK.
> 

Pushed to master: ab4fcb0fe25e313c93caae3b90f68b4010a9f2eb

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] PATCH: Improve on #2795 patches

[David Kupka]

On 20/07/16 12:11, Simo Sorce wrote:

Attached patch introduces a helper function and avoids the questionable
replace+delete operations where possible (still employed in the
entry_to_mods function).
Compiles and I am about to test it, but I'd like feedback on it if
anyone wants to take a look.

Simo.





Looks and works good, ACK.

--
David Kupka

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] PATCH: Improve on #2795 patches

[Simo Sorce]

Attached patch introduces a helper function and avoids the questionable
replace+delete operations where possible (still employed in the
entry_to_mods function).
Compiles and I am about to test it, but I'd like feedback on it if
anyone wants to take a look.

Simo.From fec7ed2d2d7d8352d1a6a9cf5607476c9fd5d65f Mon Sep 17 00:00:00 2001
From: Simo Sorce 
Date: Tue, 19 Jul 2016 07:43:50 -0400
Subject: [PATCH] Simplify date manipulation in pwd plugin

Use a helper function to perform operations on dates in LDAP attributes.

Related to #2795

Signed-off-by: Simo Sorce 
---
 daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c  | 66 +--
 daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h  |  2 +
 daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c | 31 ---
 3 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
index 0bb50fc319e2b2520d36534d369ad42f95c80c8e..cab7b7c7bf0816de736cceaa9a8067920b770a2e 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/common.c
@@ -702,6 +702,33 @@ next:
 return kvno;
 }
 
+int ipapwd_setdate(Slapi_Entry *source, Slapi_Mods *smods, const char *attr,
+   time_t date, bool remove)
+{
+char timestr[GENERALIZED_TIME_LENGTH+1];
+struct tm utctime;
+Slapi_Attr *t;
+bool exists;
+
+exists = (slapi_entry_attr_find(source, attr, ) == 0);
+
+if (remove) {
+if (exists) {
+ slapi_mods_add_mod_values(smods, LDAP_MOD_DELETE, attr, NULL);
+}
+return LDAP_SUCCESS;
+}
+
+if (!gmtime_r(, )) {
+LOG_FATAL("failed to convert %s date\n", attr);
+return LDAP_OPERATIONS_ERROR;
+}
+strftime(timestr, GENERALIZED_TIME_LENGTH + 1, "%Y%m%d%H%M%SZ", );
+slapi_mods_add_string(smods, exists ?  LDAP_MOD_REPLACE : LDAP_MOD_ADD,
+  attr, timestr);
+return LDAP_SUCCESS;
+}
+
 /* Modify the Password attributes of the entry */
 int ipapwd_SetPassword(struct ipapwd_krbcfg *krbcfg,
struct ipapwd_data *data, int is_krb)
@@ -711,8 +738,6 @@ int ipapwd_SetPassword(struct ipapwd_krbcfg *krbcfg,
 Slapi_Value **svals = NULL;
 Slapi_Value **ntvals = NULL;
 Slapi_Value **pwvals = NULL;
-struct tm utctime;
-char timestr[GENERALIZED_TIME_LENGTH+1];
 char *nt = NULL;
 int is_smb = 0;
 int is_ipant = 0;
@@ -764,34 +789,19 @@ int ipapwd_SetPassword(struct ipapwd_krbcfg *krbcfg,
 		 * keytab so don't set it on hosts.
 		 */
 if (!is_host) {
-	/* change Last Password Change field with the current date */
-			if (!gmtime_r(&(data->timeNow), )) {
-LOG_FATAL("failed to retrieve current date (buggy gmtime_r ?)\n");
-ret = LDAP_OPERATIONS_ERROR;
-goto free_and_return;
-			}
-			strftime(timestr, GENERALIZED_TIME_LENGTH + 1,
- "%Y%m%d%H%M%SZ", );
-			slapi_mods_add_string(smods, LDAP_MOD_REPLACE,
-  "krbLastPwdChange", timestr);
+	/* change Last Password Change field with the current date */
+ret = ipapwd_setdate(data->target, smods, "krbLastPwdChange",
+ data->timeNow, false);
+if (ret != LDAP_SUCCESS)
+goto free_and_return;
 
-			/* set Password Expiration date */
-			if (!gmtime_r(&(data->expireTime), )) {
-LOG_FATAL("failed to convert expiration date\n");
-ret = LDAP_OPERATIONS_ERROR;
-goto free_and_return;
-			}
-			strftime(timestr, GENERALIZED_TIME_LENGTH + 1,
- "%Y%m%d%H%M%SZ", );
-			slapi_mods_add_string(smods, LDAP_MOD_REPLACE,
-  "krbPasswordExpiration", timestr);
-			if (data->expireTime == 0) {
-			slapi_mods_add_string(smods, LDAP_MOD_DELETE,
-			  "krbPasswordExpiration", timestr);
-			}
-
-		}
+	/* set Password Expiration date */
+ret = ipapwd_setdate(data->target, smods, "krbPasswordExpiration",
+ data->expireTime, (data->expireTime == 0));
+if (ret != LDAP_SUCCESS)
+goto free_and_return;
 	}
+}
 
 if (nt && is_smb) {
 slapi_mods_add_string(smods, LDAP_MOD_REPLACE,
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h
index 83c0222635ece033a37b3540201ae674b5191257..e96aa44d2fb19251c43d8a981dea5f8441007c6a 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h
@@ -119,6 +119,8 @@ int ipapwd_gen_checks(Slapi_PBlock *pb, char **errMesg,
 int ipapwd_CheckPolicy(struct ipapwd_data *data);
 int ipapwd_getEntry(const char *dn, Slapi_Entry **e2, char **attrlist);
 int ipapwd_get_cur_kvno(Slapi_Entry *target);
+int ipapwd_setdate(Slapi_Entry *source, Slapi_Mods *smods, const char *attr,