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

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

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

>>> * I have simplified the composite driver - its only purpose now is  
>>> to allow separate admin and auth drivers to be combined into a  
>>> single interface.  The wiki page will need to be updated.
>>
>> Was it ever doing anything else? :)
>
> As Matt Selsky stated, it was doing things like mobile detection.   
> But that has been (must be) deprecated.

Understood.

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/20090722/c505ff23/attachment.bin>