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

[Jan Schneider]

Zitat von Michael J Rubinsky <mrubinsk at horde.org>:

> 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?

Wouldn't it be sufficient to make sure the user name is a string? In  
that case, it's not FALSE for guest users which casts to "f" in  
Postgres.

-- 
Jan Schneider
The Horde Project
http://www.horde.org/
https://www.facebook.com/hordeproject