[dev] [cvs] commit: horde login.php horde/templates/index frames_index.inc horde/templates/login header.inc login.inc mobile.inc horde/services changepassword.php facebook.php logintasks.php twitter.php twitterapi.php horde/services/portal edit.php ...

[Jan Schneider]

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

> Quoting Jan Schneider <jan at horde.org>:
>
>> Zitat von Michael M Slusarz <slusarz at horde.org>:
>>
>>> Quoting Jan Schneider <jan at horde.org>:
>>>
>>>>> The auth code in an app's base.php file looks like this:
>>>>>
>>>>> $registry = Horde_Registry::singleton();
>>>>> try {
>>>>> $registry->pushApp($appname, $check_auth_and_perms?);
>>>>> } catch (Horde_Exception $e) {
>>>>> Horde_Auth::authenticationFailureRedirect($app, $e);
>>>>> }
>>>>
>>>> Shouldn't this rather be a Horde_Auth_Exception? This seems like  
>>>> a perfect example why we should have special Exceptions in *any*  
>>>> library that throws them.
>>>
>>> Are we going to allow functions to throw multiple types of  
>>> exceptions?  That might be a bit confusing.  It seems to me that  
>>> it is the job of the calling function to either catch these  
>>> Exceptions and rethrow them, or else assume they are going to fall  
>>> all the way and be recorded as fatal.
>>
>> Yes, true, it should be re-thrown. It just shouldn't be a generic  
>> exception. What I'm actually trying to say is, that we should  
>> redirect to login page there, no matter which exception we see. We  
>> should only do this if authentication is really required.
>
> pushApp() will only throw an Exception if authentication *is* really  
> required.  If it isn't, Horde_Auth_Application will automatically  
> authenticate to that app via the transparent() function (any  
> application that doesn't contain either an authAuthenticate or  
> authTransparent API call is considered to need no authentication  
> outside of base Horde authentication).

This is far too much "knowledge" that the consumer of pushApp() should  
have. And assuming that the only exception that pushApp() might throw  
happens if the user is not authenticated, is too short sighted IMO. We  
need a specific exception for that case, i.e.  
Horde_Exception_NotAuthenticated, so that with any other exception the  
global exception handler is called, and not the user silently being  
redirected to the login page.

>>>>> * Hooks probably don't work yet.  We may need to rethink hooks a  
>>>>> bit.  First, there should not be need for app-specific  
>>>>> pre/post-auth hooks.  This will all be handled by a single hook  
>>>>> in Horde ($app will be one of the parameters passed to the  
>>>>> hook).  Unfortunately, pre-auth hooks don't make any sense for  
>>>>> transparent auth or for apps that don't need authentication  
>>>>> since any value returned from the pre-auth hook is totally  
>>>>> ignored.  I recommend the following refactoring of these hooks:
>>>>>
>>>>> + pre-auth hook: called only for horde-auth/apps that need  
>>>>> authentication. This hook is solely for the purpose of altering  
>>>>> auth credentials.
>>>>> + post-auth hook: called for all apps after user has been  
>>>>> authenticated to the module. App-specific setup can be handled  
>>>>> in here. Return value of false indicates application is not  
>>>>> available.
>>>>
>>>> Does that mean that post-auth is called for every module?
>>>
>>> Yes.  This removes the need for per-app hooks to do work after first login.
>>
>> Sorry, I still don't get this. Is this hook called for every  
>> installed application, once the user logged in?
>
> It is called for every application the first time it is accessed in  
> the session.  If you have enabled something like chora, and it is  
> never accessed, the postauth hook is never called.

That's pretty useful, but should be a different hook than postauth. It  
might confuse others too. I expect postauth to do the same like the  
old postauth hook, i.e. being called directly after the (Horde)  
authentication, as a last resort to deny authentication.
A hook that being called on first access to an application should be  
clearly named like that, e.g. "firstaccess" or "application_init" or  
even separate ones ("initialize") for each application in each app's  
hook.php.

Jan.

-- 
Do you need professional PHP or Horde consulting?
http://horde.org/consulting/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digitale PGP-Unterschrift
URL: <http://lists.horde.org/archives/dev/attachments/20090723/e0a28e20/attachment.bin>