[horde] Horde_History has boolean auth() -> Re: in horde_histories history_who = 'f'

[Michael J Rubinsky]

Quoting "slusarz at horde.org (Michael M Slusarz)":

> Quoting Steffen <skhorde at smail.inf.fh-bonn-rhein-sieg.de>:
>
>> On Tue, 25 Mar 2014, Michael M Slusarz wrote:
>>
>>> Quoting Michael J Rubinsky <mrubinsk at horde.org>:
>>>
>>>> Quoting Steffen <skhorde at smail.inf.fh-bonn-rhein-sieg.de>:
>>>>
>>>>> On Thu, 20 Mar 2014, Jan Schneider wrote:
>>>>>
>>>>>> Zitat von "Jens-U. Mozdzen" <jmozdzen at nde.ag>:
>>>>>>
>>>>>>> Hi Steffen,
>>>>>>>
>>>>>>> Zitat von Steffen <skhorde at smail.inf.fh-bonn-rhein-sieg.de>:
>>>>>>>> if an user adds/deletes/... events via CalDAV or ActiveSync or
>>>>>>>> Mnemos or Tasks via "Task & notes" for Android,
>>>>>>>> horde_histories gets an entry where history_who is 'f'. I do
>>>>>>>> not have no user 'f' and the users in question do not
>>>>>>>> configured this user 'f', but their own.
>>>>>>>>
>>>>>>>> I've created a backtrace for one issue, when prefs_init()
>>>>>>>> called user data for the non-existing user 'f'. Here
>>>>>>>> _buildNote() called getUserName(), which queried
>>>>>>>> horde_histories, which returned 'f'. The note is located in my
>>>>>>>> notepad and I've setup the app to use my account.
>>>>>>>>
>>>>>>>> If I add a new note with "Task& notes" for Android, I get a
>>>>>>>> new entry in horde_histories with history_who = 'f'. The same
>>>>>>>> applies, if I add a new event with CalDAV clients in KDE and
>>>>>>>> Android.
>>>>>>>>
>>>>>>>> Shouldn't history_who contain the username used to make the
>>>>>>>> changes? Besides the fact, that there might exist the user "f".
>>>>>>>> Or might there a configuration mistake?
>>>>>>
>>>>>> Cannot reproduce, my history table looks fine.
>>>>>>
>>>>>>> I just checked our according table - while most of the entries
>>>>>>> do have the user's email address in the history_who field, some
>>>>>>> do have "0". These entries seem to be spread access all
>>>>>>> applications - object_uid does start with "imp:", "turba:",
>>>>>>> "nag:", "kronolith:" and so on. "history_action" is "add,
>>>>>>> "modify", "delete" - so again spread.
>>>>>>
>>>>>> I see this on exactly 6 records, which I can lightheartedly
>>>>>> justify with some development reminiscence.
>>>>>>
>>>>>>> The first occurrence of such an extraordinary entry was at time
>>>>>>> stamp "1357325373" (Jan 4, 2013), mod_seq 382 - almost one year
>>>>>>> after the first history entry (Feb 2012) on this installation.
>>>>>>> The latest occurrence is from yesterday, so it happens with
>>>>>>> current code, too.
>>>>>
>>>>> In Horde/History.php
>>>>>
>>>>> // here $attributes contains only "action"
>>>>>
>>>>>      if (!isset($attributes['who'])) {
>>>>>          $attributes['who'] = $this->_auth;
>>>>>      }
>>>>>
>>>>> // $this->_auth is "false", I mean:
>>>>> 	if(!is_string($attributes['who'])) {
>>>>>      	my_dump("Log User \$attr=".var_export($attributes, true)."\n");
>>>>> 		// overwrite boolean value
>>>>> 		$attributes['who'] = $GLOBALS['registry']->getAuth();
>>>>> 	}
>>>>>
>>>>> produces:
>>>>> Log User $attr=array (
>>>>> 'action' => 'add',
>>>>> 'who' => false,
>>>>> )
>>>>
>>>> Looks like this may be a chicken-egg problem with getting the
>>>> history driver from the injector. A quick grep shows there are
>>>> places in the code where we grab the history driver through the
>>>> injector before we are authenticated (like in
>>>> Horde_Core_Factory_Auth, for instance). Since the injector acts
>>>> like a singleton when calling ->getInstance(), the history driver
>>>> will never have the current user's username.
>>>
>>> Possible solution: clear the Injector binding map after
>>> authentication? Although there is currently no way of doing this.
>>> Although... I guess we could destroy the global $injector variable
>>> and re-create.  No idea whether this would cause problems elsewhere
>>> though.
>>>
>>> Or else don't log entries without 'who' information.  History
>>> information is probably not useful for non-authenticated users.
>>
>> The information to be logged stems from an _authentificated_ CalDAV
>> or ActiveSync session. So maybe the http authentification is not
>> correctly passed from rpc.php to Horde?
>
> Horde_History is initialized *before* authentication has completed.
> Since we are using a singleton Horde_History instance , it contains
> the authentication information from before authentication and won't
> change.  So it doesn't have anything to do with information not being
> passed ... it's just that the information was not yet available to the
> Horde_History object when first created.
>
> Although a simple solution would be to find out why Horde_History is
> being initialized before authentication is complete.  This could
> possibly be as easy as not initializing that object if getAuth() is
> false.

Revisiting this to try and come up with a solution.

I don't think avoiding initializing History or not logging entries  
that are missing a username is a solution. It's conceivable that some  
resource is made public and is chosen as a synchronization source. If  
we stop tracking history for guest access, this will cause all kinds  
of problems with synchronization in this case. So, even if we no  
longer instantiate the history object before auth takes place, thus  
preventing user 'f' entries from appearing, it will still potentially  
corrupt the history data for the purposes of synchronization.

I think we need to have Horde_Core_Factory_History create a history  
wrapper/decorator object that will internally know if we need to  
recreate the History object when the authentication status changes.

Thoughts?

-- 
mike
The Horde Project
http://www.horde.org
https://www.facebook.com/hordeproject
https://www.twitter.com/hordeproject