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

Fri May 16 13:31:47 UTC 2014

Quoting Jan Schneider <jan at horde.org>:

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

For true guest access, yes. The actual issue though is that since the  
injector acts as a singleton, if the history object is initialized  
before authentication (like happens when count bad logins is enabled),  
it will NEVER receive the authenticated user name. Since 99.99% of the  
Horde_History::log() calls are passed without a 'who' parameter, this  
means the username will never be committed to history.

-- 
mike
The Horde Project
http://www.horde.org
https://www.facebook.com/hordeproject
https://www.twitter.com/hordeproject
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5869 bytes
Desc: S/MIME Signature
URL: <http://lists.horde.org/archives/horde/attachments/20140516/576dd718/attachment.bin>