Ticket 47395 47397 v2 correct behaviour of account policy if only stateattr

	is configured or no alternate attr is configured

Bug Description:  The tickets relate to two specific configurations of
	the account policy plugin
	1] if createtimestamp is configured as stateattr it is treated like a
	   normal timstamp attribute and is updated, which should not happen.
	   As a side effect the account is not locked out based on the original
	   createtimestamp
	2] if no altstateattr is configured, always createtimestamp is used, but
	   the intention was to base account inactivation only on lastlogintime

Fix Description:   1] prevent update of createtimestamp, even if used as stateattr
		   2] if no altstateattr is configured still use the default, but
		      accept "1.1" as null value and check only stateattr

https://fedorahosted.org/389/ticket/47395
https://fedorahosted.org/389/ticket/47397

Reviewed by: ?

ldap/servers/plugins/acctpolicy/acct_config.c

@@ -82,12 +82,23 @@ acct_policy_entry2config( Slapi_Entry *e, acctPluginCfg *newcfg ) {
 	newcfg->state_attr_name = get_attr_string_val( e, CFG_LASTLOGIN_STATE_ATTR );
 	if( newcfg->state_attr_name == NULL ) {
 		newcfg->state_attr_name = slapi_ch_strdup( DEFAULT_LASTLOGIN_STATE_ATTR );
+	} else if (!update_is_allowed_attr(newcfg->state_attr_name)) {
+		/* log a warning that this attribute cannot be updated */
+		slapi_log_error( SLAPI_LOG_FATAL, PLUGIN_NAME,
+							 "The configured state attribute [%s] cannot be updated, accounts will always become inactive.\n",
+							 newcfg->state_attr_name );
 	}
 
 	newcfg->alt_state_attr_name = get_attr_string_val( e, CFG_ALT_LASTLOGIN_STATE_ATTR );
+	/* alt_state_attr_name should be optional, but for backward compatibility, 
+	 * if not specified use a default. If the attribute is "1.1", no fallback 
+	 * will be used
+	 */ 
 	if( newcfg->alt_state_attr_name == NULL ) {
 		newcfg->alt_state_attr_name = slapi_ch_strdup( DEFAULT_ALT_LASTLOGIN_STATE_ATTR );
-	}
+	} else if ( !strcmp( newcfg->alt_state_attr_name, "1.1" ) ) {
+                 slapi_ch_free_string( &newcfg->alt_state_attr_name ); /*none - NULL */
+	} /* else use configured value */
 
 	newcfg->spec_attr_name = get_attr_string_val( e, CFG_SPEC_ATTR );
 	if( newcfg->spec_attr_name == NULL ) {

ldap/servers/plugins/acctpolicy/acct_init.c

@@ -132,7 +132,7 @@ acct_policy_start( Slapi_PBlock *pb ) {
 	slapi_log_error( SLAPI_LOG_PLUGIN, PLUGIN_NAME, "acct_policy_start config: "
 		"stateAttrName=%s altStateAttrName=%s specAttrName=%s limitAttrName=%s "
 		"alwaysRecordLogin=%d\n",
-		cfg->state_attr_name, cfg->alt_state_attr_name, cfg->spec_attr_name,
+		cfg->state_attr_name, cfg->alt_state_attr_name?cfg->alt_state_attr_name:"not configured", cfg->spec_attr_name,
 		cfg->limit_attr_name, cfg->always_record_login);
 	return( CALLBACK_OK );
 }

ldap/servers/plugins/acctpolicy/acct_plugin.c

@@ -44,14 +44,16 @@ acct_inact_limit( Slapi_PBlock *pb, const char *dn, Slapi_Entry *target_entry, a
 		cfg->state_attr_name ) ) != NULL ) {
 		slapi_log_error( SLAPI_LOG_PLUGIN, PRE_PLUGIN_NAME,
 			"\"%s\" login timestamp is %s\n", dn, lasttimestr );
-	} else if( ( lasttimestr = get_attr_string_val( target_entry,
-		cfg->alt_state_attr_name ) ) != NULL ) {
+	} else if( cfg->alt_state_attr_name && (( lasttimestr = get_attr_string_val( target_entry,
+		cfg->alt_state_attr_name ) ) != NULL) ) {
 		slapi_log_error( SLAPI_LOG_PLUGIN, PRE_PLUGIN_NAME,
 			"\"%s\" alternate timestamp is %s\n", dn, lasttimestr );
 	} else {
+		/* the primary or alternate attribute might not yet exist eg. 
+		 * if only lastlogintime is specified and it id the first login
+		 */
 		slapi_log_error( SLAPI_LOG_PLUGIN, PRE_PLUGIN_NAME,
-			"\"%s\" has no login or creation timestamp\n", dn );
-		rc = -1;
+			"\"%s\" has no value for stateattr or altstateattr \n", dn );
 		goto done;
 	}
 
@@ -105,6 +107,13 @@ acct_record_login( const char *dn )
 	int skip_mod_attrs = 1; /* value doesn't matter as long as not NULL */
 
 	cfg = get_config();
+
+	/* if we are not allowed to modify the state attr we're done
+         * this could be intentional, so just return
+         */
+	if (! update_is_allowed_attr(cfg->state_attr_name) )
+		return rc;
+ 
 	plugin_id = get_identity();
 
 	timestr = epochtimeToGentime( time( (time_t*)0 ) );
@@ -283,7 +292,6 @@ acct_bind_postop( Slapi_PBlock *pb )
 		} else {
 			if( target_entry && has_attr( target_entry,
 				cfg->spec_attr_name, NULL ) ) {
-				/* This account has a policy specifier */
 				tracklogin = 1;
 			}
 		}

ldap/servers/plugins/acctpolicy/acct_util.c

@@ -255,3 +255,16 @@ epochtimeToGentime( time_t epochtime ) {
 	return( gentimestr );
 }
 
+int update_is_allowed_attr (const char *attr)
+{
+	int i;
+
+        /* check list of attributes that cannot be used for login recording */
+        for (i = 0; protected_attrs_login_recording[i]; i ++) {
+            if (strcasecmp (attr, protected_attrs_login_recording[i]) == 0) {
+                /* this attribute is not allowed */
+                return 0;
+            }
+        }
+	return 1;
+}

ldap/servers/plugins/acctpolicy/acctpolicy.h

@@ -35,6 +35,10 @@ Hewlett-Packard Development Company, L.P.
 #define DEFAULT_INACT_LIMIT_ATTR "accountInactivityLimit"
 #define DEFAULT_RECORD_LOGIN 1
 
+/* attributes that no clients are allowed to add or modify */
+static char *protected_attrs_login_recording [] = { "createTimestamp",
+                                        NULL };
+
 #define PLUGIN_VENDOR "Hewlett-Packard Company"
 #define PLUGIN_VERSION "1.0"
 #define PLUGIN_CONFIG_DN "cn=config,cn=Account Policy Plugin,cn=plugins,cn=config"
@@ -74,6 +78,7 @@ void* get_identity();
 void set_identity(void*);
 time_t gentimeToEpochtime( char *gentimestr );
 char* epochtimeToGentime( time_t epochtime ); 
+int update_is_allowed_attr (const char *attr);
 
 /* acct_config.c */
 int acct_policy_load_config_startup( Slapi_PBlock* pb, void* plugin_id );