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

[Michael J Rubinsky]

Quoting Michael M Slusarz <slusarz at horde.org>:

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

 From the admittedly brief look I took at it, there was at least one  
spot that was using it for the maximum login attempts functionality,  
though I have no idea how/if that works properly since (as we see) the  
auth username isn't available at that time anyway. I don't have the  
code in front of me, but I remember another spot was during the  
registry initialization, though I'm not sure of the rationale for  
creating it there.

We could decorate the history object with a core wrapper object that  
can initialize the history object only when the auth is available.
-- 
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: 5849 bytes
Desc: S/MIME Signature
URL: <http://lists.horde.org/archives/horde/attachments/20140326/f77c3d72/attachment-0001.bin>