[dev] [commits] Horde branch master updated. 5df37f9934afeee9f4741d41f92c06cfc4b39ca9

[Jan Schneider]

Zitat von Michael M Slusarz <slusarz at horde.org>:

> Quoting Jan Schneider <jan at horde.org>:
>
>> commit 5df37f9934afeee9f4741d41f92c06cfc4b39ca9
>> Author: Jan Schneider <jan at horde.org>
>> Date:   Sun Aug 23 23:11:47 2009 +0200
>>
>>    This check doesn't make any sense to me, and it break guest  
>> application access.
>>
>> framework/Core/lib/Horde/Registry.php |   10 ++--------
>> 1 files changed, 2 insertions(+), 8 deletions(-)
>
> This makes things worse.  Imagine these scenarios:
>
> 1.) Admin user, non-imap auth, not authenticated to imp, calling  
> hasPermission('imp', PERMS_EDIT | PERMS_DELETE)
>
> hasPermission() *must* return false here.  You can't do anything in  
> imp unless you are authenticated - imp won't work at all until the  
> user (whether an admin or not) is authenticated.  Being a horde  
> admin has absolutely nothing to do with imp authentication - they  
> are entirely independent.
>
> 2.) Non-admin user, non-imap auth, not authenticated to imp, no  
> default imp permissions, calling hasPermission('imp', EDIT_PERMS)
>
> This change now makes hasPermission() always return true for any  
> permission level for any user (!$GLOBALS['perms']->exists('imp')  
> returns true).  That is obviously not correct.
>
> I'll agree that guest access is broken with the old code.  But this  
> change makes things worse (especially #2).  The proper fix probably  
> lies in Horde_Auth_Application - the default transparent()  
> authentication method, for apps that don't require any additional  
> authentication, should do the proper guest permission checking there.

That sounds awkward, permission checking is authorization, so it  
shouldn't be done in the authentication code. How about having the  
application report back to Horde_Auth_Application whether they require  
authentication, and use that information? Or can we maybe even already  
assume that having an 'authenticate' API method make this a requirement?
That solution would involve a new Horde_Auth::requireAuthentication() method.

Jan.

-- 
Do you need professional PHP or Horde consulting?
http://horde.org/consulting/