[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>:
>
>>> Reading back at your post, I guess I am confused what your concern  
>>> is.  Using Horde::authenticationFailureRefirect(), you will  
>>> *never* be redirected to the login page without there being an  
>>> explanation why authentication was unsuccessful (either auth issue  
>>> or permissions issue).  And in the third error case (application  
>>> not active), this will throw an exception that should result in a  
>>> fatal error.
>>
>> I missed that the exception is being passed to  
>> authenticationFailureRedirect(). But this should still not be  
>> called unconditionally, no matter what exception we see.
>> You say that there are only 3 possible exception cases. But that  
>> could change in any point in the future. And we might not want to  
>> redirect to the login page at this case.
>
> Wouldn't you just add a new exception case to  
> authenticationFailureRedirect()?  That is ten times easier than  
> trying to rewrite authentication error handling in 50 different  
> applications.

That's a good point, I'd have to think more about that.

> If you happen to be in the rare case where you don't want to  
> redirect, an app is free to do whatever it wants before calling  
> authenticationFailureRedirect().  [Probably should change the name  
> of that function to Horde_Auth::failureHandler() or something a bit  
> more concise.]

Renaming the failure handler method would definitely make sense, if it  
doesn't always redirect to the login page.

Looking at the Horde_Auth code right now, I think we still have to  
split out any functionality that's not directly related to Horde_Auth  
functionality, i.e. registry access, browser redirection etc., so that  
the package can be safely used without the framework.

>>> I am against this also.  We have *way* too many hooks as it is.  I  
>>> work on this code everyday, and I don't know what half of the  
>>> hooks do.  In the interest of slimming down configuration, which I  
>>> think we all agree is a big goal of Horde 4, this is one of the  
>>> ways to accomplish that.
>>
>> I disagree. The large number of hooks is, what Horde's real power  
>> of flexibility is. Being able to call hooks at all kind of places  
>> in the code is major selling point, and we should rather extend  
>> this, than slimming it down.
>> There is no disagreement about, that those "grown" hooks need to be  
>> cleaned up.
>
> It's going to be a major PITA to maintain hooks (and the  
> corresponding config options) in every single application.  Although  
> thinking about this a bit more, doing everything in one hook in  
> Horde is also not optimal since we would be in Horde scope (not the  
> app scope) at that point.  Any code written there would need to do  
> manual pushApp()/popApp() calls which is probably asking too much  
> for an admin-level coder.

We actually don't have to maintain any hooks. We just need to provide  
examples. And I agree that from an admin's point of view it makes more  
sense to have a hook per application. Having split out the application  
hooks from Horde to the corresponding application a while ago  
definitely helped understanding and finding hooks.

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/20090724/2e6bcf71/attachment.bin>