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

[Michael M Slusarz]

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

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

>>> Regarding transparent authentication, I don't understand why the  
>>> auth hooks should be ignored, or is this just a description of the  
>>> current state? We should make both hooks work here too, I don't  
>>> see a reason why they would make less sense for transparent auth.
>>
>> Well, for transparent authentication, it is possible that there  
>> won't be a username and/or credentials.  So that might be a bit  
>> confusing passing to a hook.  I guess as long as that is  
>> documented, this should be fine.
>
> Yes. Especially since with transparent authentication it might make  
> sense to change or set the credentials in a hook.

This has been added.  Regular auth vs. transparent auth is, for lack  
of a better term, transparent to the hook.  It receives the same  
parameters in either case.

michael

-- 
___________________________________
Michael Slusarz [slusarz at horde.org]