Hi Ben, I have made some changes to the attached classes...
AbstractProcessingFilter ----------------------------- I have added some properties to the class. Each of the properties represents a URL that the user is redirected to in the event that the AuthenticationManager implementation throws a specific subclass of AuthenticationException. authenticationServiceFailureUrl - AuthenticationServiceException authenticationCredentialCheckFailureUrl - BadCredentialsException authenticationDisabledFailureUrl - DisabledException authenticationLockedFailureUrl - LockedException authenticationProxyUntrustedFailureUrl - ProxyUntrustedException authenticationUsernameNotFoundFailureUrl - UsernameNotFoundException The catch block in the doFilter method catches 'AuthenticationException' and instanceof checks are performed along with a check to dermine if the corresponding property is null. If the exception type is found and the corresponding URL is not null, the user is redirected to that URL. If none of the checks match then the current 'authenticationFailureUrl' is used. I didnt use multiple catch blocks to do this switching because the instanceof enabled me to reuse the existing code in the existing catch block. AuthenicationException ------------------------------- I have added an 'Authentication' attribute to the exception. This attribute is not mandatory and not set in the constructor. It is set at a later time. It could be placed in the constructor but this would involve changing all the subclasses and all the places the subclasses are instansiated. ProviderManager ------------------------------- I have wrapped the calls to provider.authenticate in a try block that catchs AuthenticationException. This is so I can intercept the exception to populate the authentication object into the Exception, it is then rethrown. Also the final ProviderNotFoundException is created first so that the authentication may be set before the instance is thrown. An implication of this approch is that custom implementations of AuthenticationManager will need to do their own work to populate the exception with the authentication object. I did not see this as a major problem as there seems to be little reason to create a custom AuthenticationManager rather than a AuthenticationProvider. An alternative might be to make AuthenticationManager an abstract class and use the template pattern to move the authentication population up into AuthenticationManager but this change was too invasive for my taste. I have performed some tests of these changes by integrating them into my project and doing some functional tests. Everything seems to work ok for me. Perhaps you would like to integrate these changes into the next version of acegi. If not, perhaps somebody on the list has similar requirements to me and would like to include these changes in their project. In either case, please accept these changes as is and distributed under the apache license as the rest of the acegi code. Regards Wesley Hall > -----Original Message----- > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED] Behalf Of > Wesley Hall > Sent: 21 July 2004 20:26 > To: [EMAIL PROTECTED] > Subject: RE: [Acegisecurity-developer] Suggestions for changes to > AbstractProcessingFilter > > > Ben, > > Thanks for taking the time to respond. (Comments below) > > > -----Original Message----- > > From: [EMAIL PROTECTED] > > [mailto:[EMAIL PROTECTED] Behalf Of > > Ben Alex > > Sent: 20 July 2004 04:02 > > To: [EMAIL PROTECTED] > > Subject: Re: [Acegisecurity-developer] Suggestions for changes to > > AbstractProcessingFilter > > > > > > Hi Wesley > > > > Thanks for the suggestions. Comments below. > > > > Wesley Hall wrote: > > > > >Hello all, > > > > > >I have a couple of suggestions for changes to > > AbstractProcessingFilter. I am > > >not sure on process for submitting patches but I am happy to make these > > >changes myself if somebody would care to provide this information. > > > > > >My first suggestion is to provide alternate failure URLs for the > > different > > >failure reasons. These URLs shouldnt need to be madatory as the > > system could > > >default to the mandatory failure URL. > > > > > >I have looked at the code for this class and it seems that the system > > >catches an AuthenticatationException and if this is caught > redirects the > > >user to the specified failure URL. If this catch block was > > extended to catch > > >the relevant AuthenticationException subtypes the > functionality could be > > >easily extended to redirect to different URLs on different events if > > >required by the developer. If there is no URL configured for the > > particular > > >exception type then the system should default to redirecting to > > the existing > > >failure URL. > > > > > > > > Most Acegi Security implementations use SecurityEnforcementFilter. It is > > that class which distinguishes between authorization vs authentication > > failures in lower-down classes, providing a 403 in the event of an > > AccessDeniedException and a redirect to the AuthenticationEntryPoint in > > the event of an AuthenticationException. The > > AuthenticationProcessingFilter logic is only executed when an actual > > authentication attempt is made (eg a BASIC authentication header is > > detected, a form-based login is submitted etc) so it is unlikely it > > would ever see say an AccessDeniedException. So if you'd like > > alternative handling of AccessDeniedExceptions, it might be more helpful > > to put it within the SecurityEnforcementFilter. > > The functionality I am suggesting relates purely to when an authentication > attempt is made. I have written an AuthenticationProvider > implementation as > my requirements go slightly beyond the features provided within the > DaoProvider. My implementation of the authenticate method throws different > AuthenticationException subtypes (currently only subtypes > provided by acegi > such as DisabledException). It would be useful for me to redirect the user > to different URLs depending on which exception type was thrown by the > provider. > > My suggestion to provide this feature without breaking existing > functionality is to add some extra properties to AbstractProcessingFilter > (which currently holds the generic 'authenticationFailureUrl') to include > optional URLs to redirect the user in the event of a DisabledException or > LockedException (for instance). Extend the catch block (that currently > catches the AuthenticationException superclass) to catch the main > subclasses > and in each catch block check if the URL property for that type of failure > is null. If it is null, continue with existing behaviour and > forward to the > mandatory authenticationFailureUrl, but if it is not null, forward to this > specific URL instead. The definition for the bean in the spring > configuration file becomes... > > <bean id="authenticationProcessingFilter" > class="net.sf.acegisecurity.ui.webapp.AuthenticationProcessingFilter"> > <property name="authenticationManager"><ref > bean="authenticationManager"/></property> > <property > name="authenticationFailureUrl"><value>/logon.jsp?error=1</value>< > /property> > <property > name="accountDisabledUrl"><value>/logon.jsp?error=2</value></property> > <property > name="accountLockedUrl"><value>/logon.jsp?error=3</value></property> > <property > name="defaultTargetUrl"><value>/secure/index.html</value></property> > <property > name="filterProcessesUrl"><value>/j_acegi_security_check</value></ > property> > </bean> > > ...for example. Here the system provides the error page with the parameter > error=2 for DisabledException and error=3 for LockedException and defaults > to error=1 for all other exceptions. As the new properties are > optional, and > the system defaults to the existing mandatory url, backward > compatibility is > maintained. > > I may have some time this evening to make these changes, I will need them > for my project anyway, I will of course provide them back to you > and you can > decide whether there is merit in including them in a future > version. As I am > not sure of the preferred format to supply patches I will just email the > altered files to this mailing list. Ideally I would like to take > a branch of > the current head to make these changes on, but I assume sourceforge doesnt > have seperate permissions for branch creation and committing to > the head and > I understand that you may not want to give commit access to anyone that > comes along and asks for it. > > > >The second suggestion is that, upon authentication failure, the > > system could > > >place the authentication object (that failed) into the session. If the > > >failure pages are dynamic then the failure pages could perform some > > >application specific logic to display even more information to > > the user. For > > >example... "Authentication has failed. Your account was disabled by > > >'joe_superuser' at 19/07/04 at 14:22". > > > > > > > > > > > A challenge with this would be to obtain a "failed" Authentication > > object. Subclasses of AuthenticationProcessingFilter implement a "public > > abstract Authentication attemptAuthentication(HttpServletRequest)" > > method which is responsible for returning the successful Authentication > > or throwing a relevant exception. Of course there are ways to do it > > (mandate the attempted Authentication inside the returned exception, > > have subclasses place the attempted Authentication in a well-known > > location etc) but it would require subclass-level code changes. Your > > more informative error message requirement can presently be accommodated > > via the use of more detailed exception messages. The exception causing a > > failed authentication is put into the HttpSession against the > > ACEGI_SECURITY_LAST_EXCEPTION_KEY attribute. So if you wanted the sort > > of details provided in your example, you could modify your > > AuthenticationManager (or more likely, a custom > > DaoAuthenticationProvider subclass) to provide this extra level of > > information. > > A thought... why not add an attribute of type 'Authentication' to the > AuthenticationException class. This would allow the system to provide the > "failed" Authentication with the AuthenticationException. This > seems like a > fairly natural design move as the first thought after catching an > AuthenicationException might be "so which 'Authentication' failed?". > > Again, happy to make this change. > > Regards > > Wesley Hall > > > > > ------------------------------------------------------- > This SF.Net email is sponsored by BEA Weblogic Workshop > FREE Java Enterprise J2EE developer tools! > Get your free copy of BEA WebLogic Workshop 8.1 today. > http://ads.osdn.com/?ad_id=4721&alloc_id=10040&op=click > _______________________________________________ > Acegisecurity-developer mailing list > [EMAIL PROTECTED] > https://lists.sourceforge.net/lists/listinfo/acegisecurity-developer
AuthenticationException.java
Description: Binary data
AbstractProcessingFilter.java
Description: Binary data
ProviderManager.java
Description: Binary data