Re: [Acegisecurity-developer] Question about AbstractSecurityInterceptor
Venkat Sonnathi wrote: Hi Ben, Please find attached the patch for AbstractSecurityInterceptor.java, Basically, it checks to see if the existing authentication is already autheticated or not and then invoke authenticationManager.authenticate. Hi Venkat I have just committed to CVS various changes to the Authentication.isAuthenticated() handling. Effective herein, AbstractSecurityInterceptor will only call the AuthenticationManager if the Authentication.isAuthenticated() == false. AbstractSecurityInterceptor does not call Authentication.setAuthenticated(true) - instead it leaves this choice to the AuthenticationProvider and/or Authentication concrete implementation to address. Most Authentication implementations now provide a mutable isAuthenticated() property. By mutable, setAuthenticated(false) is guaranteed by the Authentication interface contract to always be allowed. This is used by the RMI class to ensure a remotely presented Authentication is set to untrusted, ensuring the AbstractSecurityInterceptor will trigger authentication. Permitting setAuthenticated(true) (which would therefore bypass further checking at time of security interception) is an implementation choice. The main implementation used by Acegi Security, UsernamePasswordAuthenticationToken, disallows setAuthenticated(true) and instead relies upon the constructor to set the property. This means that AuthenticationProviders should be the only classes that use the UsernamePasswordAuthenticationToken(Object, Object, GrantedAuthority[]) constructor. On the other hand, any class can freely use the UsernamePasswordAuthenticationToken(Object, Object) constructor, as the resulting authentication token will not be trusted (ie isAuthenticated() will always return false). Unit tests pass. Cheers Ben --- SF.Net email is sponsored by: Discover Easy Linux Migration Strategies from IBM. Find simple to follow Roadmaps, straightforward articles, informative Webcasts and more! Get everything you need to get up to speed, fast. http://ads.osdn.com/?ad_id=7477alloc_id=16492op=click ___ Home: http://acegisecurity.sourceforge.net Acegisecurity-developer mailing list Acegisecurity-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/acegisecurity-developer
Re: [Acegisecurity-developer] Question about AbstractSecurityInterceptor
Hi Ben, Thanks for the changes. I looked over the changes. Regards, --Venkat. On 6/22/05, Ben Alex [EMAIL PROTECTED] wrote: Venkat Sonnathi wrote: Hi Ben, Please find attached the patch for AbstractSecurityInterceptor.java, Basically, it checks to see if the existing authentication is already autheticated or not and then invoke authenticationManager.authenticate. Hi Venkat I have just committed to CVS various changes to the Authentication.isAuthenticated() handling. Effective herein, AbstractSecurityInterceptor will only call the AuthenticationManager if the Authentication.isAuthenticated() == false. AbstractSecurityInterceptor does not call Authentication.setAuthenticated(true) - instead it leaves this choice to the AuthenticationProvider and/or Authentication concrete implementation to address. Most Authentication implementations now provide a mutable isAuthenticated() property. By mutable, setAuthenticated(false) is guaranteed by the Authentication interface contract to always be allowed. This is used by the RMI class to ensure a remotely presented Authentication is set to untrusted, ensuring the AbstractSecurityInterceptor will trigger authentication. Permitting setAuthenticated(true) (which would therefore bypass further checking at time of security interception) is an implementation choice. The main implementation used by Acegi Security, UsernamePasswordAuthenticationToken, disallows setAuthenticated(true) and instead relies upon the constructor to set the property. This means that AuthenticationProviders should be the only classes that use the UsernamePasswordAuthenticationToken(Object, Object, GrantedAuthority[]) constructor. On the other hand, any class can freely use the UsernamePasswordAuthenticationToken(Object, Object) constructor, as the resulting authentication token will not be trusted (ie isAuthenticated() will always return false). Unit tests pass. Cheers Ben --- SF.Net email is sponsored by: Discover Easy Linux Migration Strategies from IBM. Find simple to follow Roadmaps, straightforward articles, informative Webcasts and more! Get everything you need to get up to speed, fast. http://ads.osdn.com/?ad_id=7477alloc_id=16492op=click ___ Home: http://acegisecurity.sourceforge.net Acegisecurity-developer mailing list Acegisecurity-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/acegisecurity-developer --- SF.Net email is sponsored by: Discover Easy Linux Migration Strategies from IBM. Find simple to follow Roadmaps, straightforward articles, informative Webcasts and more! Get everything you need to get up to speed, fast. http://ads.osdn.com/?ad_idt77alloc_id492op=click ___ Home: http://acegisecurity.sourceforge.net Acegisecurity-developer mailing list Acegisecurity-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/acegisecurity-developer
Re: [Acegisecurity-developer] Question about AbstractSecurityInterceptor
Hi Ben, Please find attached the patch for AbstractSecurityInterceptor.java, Basically, it checks to see if the existing authentication is already autheticated or not and then invoke authenticationManager.authenticate. I am submitting the patch for first time (cvs diff -Nar HEAD AbstractSecurityInterceptor.java abstract.patch), so please let me know if you want me to re-create it in any other specific way. Thanks, --Venkat. On 5/21/05, Ben Alex [EMAIL PROTECTED] wrote: Venkat Sonnathi wrote: Would this change be in the next release? I would be glad to help if you want. Yes, it will be in 0.9.0. I have added it to my TODO list. You're welcome to email me patches based on current CVS if you would like to. This is was commented by Mansoor. I agree with you - ProviderManager is the not right place for this. Thanks for the clarification - my apologies for the confusion. Best regards Ben --- This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space? Enter now for the Oracle Space Sweepstakes! http://ads.osdn.com/?ad_id=7412alloc_id=16344op=click ___ Home: http://acegisecurity.sourceforge.net Acegisecurity-developer mailing list Acegisecurity-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/acegisecurity-developer abstract.patch Description: Binary data
Re: [Acegisecurity-developer] Question about AbstractSecurityInterceptor
Venkat Sonnathi wrote: Would this change be in the next release? I would be glad to help if you want. Yes, it will be in 0.9.0. I have added it to my TODO list. You're welcome to email me patches based on current CVS if you would like to. This is was commented by Mansoor. I agree with you - ProviderManager is the not right place for this. Thanks for the clarification - my apologies for the confusion. Best regards Ben --- This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space? Enter now for the Oracle Space Sweepstakes! http://ads.osdn.com/?ad_id=7412alloc_id=16344op=click ___ Home: http://acegisecurity.sourceforge.net Acegisecurity-developer mailing list Acegisecurity-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/acegisecurity-developer
Re: [Acegisecurity-developer] Question about AbstractSecurityInterceptor
Hi Ben, On 5/19/05, Ben Alex [EMAIL PROTECTED] wrote: Venkat Sonnathi wrote: I am also a bit puzzled as to why we should reset the flag at the start of each request? In a typical web app, authentication is done once per session. Any pointers to how SecurityContext is propagated for RMI calls? I agree, it shouldn't be required. The net.sf.acegisecurity.context.rmi package propagates a SecurityContext from the client-side to the server-side. The HttpSessionContextIntegrationFilter should not used in such deployments, and therefore HttpSessionContextIntegrationFilter will not need to reset the flag at the start of each request. Would this change be in the next release? I would be glad to help if you want. In relation to your other email, I don't see the value of ProviderManager setting the flag. Doing so would means each AuthenticationProvider implementation cannot make its own decision as to whether or not the Authentication should be treated as valid for the remainder of the request. For consistency with caching, I believe the setting of the flag should occur at the AuthenticationProvider level as it improves the prospects of as yet unknown authentication systems working correctly with Acegi Security. Do you have a specific reason why you'd prefer the ProviderManager set the flag? This is was commented by Mansoor. I agree with you - ProviderManager is the not right place for this. Regards, --Venkat. --- This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space? Enter now for the Oracle Space Sweepstakes! http://ads.osdn.com/?ad_idt12alloc_id344op=click ___ Home: http://acegisecurity.sourceforge.net Acegisecurity-developer mailing list Acegisecurity-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/acegisecurity-developer
Re: [Acegisecurity-developer] Question about AbstractSecurityInterceptor
On 5/18/05, Mansoor, Ghazenfer (EDS) [EMAIL PROTECTED] wrote: -Original Message- Mansoor, Ghazenfer (EDS) wrote: How about adding this check at one central place, AuthenticationManager? I am doing this and I do not see any problem. I set the authenticate to true after successful authentication, and check for isAuthentication() before every call. What sets your Authentication.isAuthenticated() back to false at the start of each request? Why should it be set to false at the start of each request? It should be set to false only at the end of each user session. Logout code will set the context to null (Authentication object prior to .9 version) and user no longer have access to Authentication Object. New session will created a new Authentication Object to start. I am also a bit puzzled as to why we should reset the flag at the start of each request? In a typical web app, authentication is done once per session. Any pointers to how SecurityContext is propagated for RMI calls? Regards, --Venkat. --- This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space? Enter now for the Oracle Space Sweepstakes! http://ads.osdn.com/?ad_idt12alloc_id344op=click ___ Home: http://acegisecurity.sourceforge.net Acegisecurity-developer mailing list Acegisecurity-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/acegisecurity-developer
Re: [Acegisecurity-developer] Question about AbstractSecurityInterceptor
Venkat Sonnathi wrote: I am also a bit puzzled as to why we should reset the flag at the start of each request? In a typical web app, authentication is done once per session. Any pointers to how SecurityContext is propagated for RMI calls? I agree, it shouldn't be required. The net.sf.acegisecurity.context.rmi package propagates a SecurityContext from the client-side to the server-side. The HttpSessionContextIntegrationFilter should not used in such deployments, and therefore HttpSessionContextIntegrationFilter will not need to reset the flag at the start of each request. In relation to your other email, I don't see the value of ProviderManager setting the flag. Doing so would means each AuthenticationProvider implementation cannot make its own decision as to whether or not the Authentication should be treated as valid for the remainder of the request. For consistency with caching, I believe the setting of the flag should occur at the AuthenticationProvider level as it improves the prospects of as yet unknown authentication systems working correctly with Acegi Security. Do you have a specific reason why you'd prefer the ProviderManager set the flag? Best regards Ben --- This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space? Enter now for the Oracle Space Sweepstakes! http://ads.osdn.com/?ad_id=7412alloc_id=16344op=click ___ Home: http://acegisecurity.sourceforge.net Acegisecurity-developer mailing list Acegisecurity-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/acegisecurity-developer
RE: [Acegisecurity-developer] Question about AbstractSecurityInterceptor
-Original Message- Mansoor, Ghazenfer (EDS) wrote: How about adding this check at one central place, AuthenticationManager? I am doing this and I do not see any problem. I set the authenticate to true after successful authentication, and check for isAuthentication() before every call. What sets your Authentication.isAuthenticated() back to false at the start of each request? Why should it be set to false at the start of each request? It should be set to false only at the end of each user session. Logout code will set the context to null (Authentication object prior to .9 version) and user no longer have access to Authentication Object. New session will created a new Authentication Object to start. I would propose the following: - SecurityContext to provide a startRequest() and finishRequest() method that is called by HttpSessionContextIntegrationFilter. - The startRequest() and finishRequest() set Authentication.isAuthenticated() to false. Do it twice in case the Authentication is being presented from a remote system (eg via RMI) which has set the isAuthenticated() to true. - An AuthenticationProvider may, but is not required to, set Authentication.isAuthenticated() to true. If it does set it to true, it means it does not require further callback and the Authentication can safely be used for the remainder of the request. AuthenticationProviders should provide a property setting so this can be switched off (ie they never set the flag to true) as in special situations (like chained AuthenticationProviders or a cache-aware ProviderManager) it might be undesirable. In that case, wouldn't it be better if ProviderManager set this, and ofcourse cache-aware ProviderManager too(if any). - AbstractSecurityInterceptor honours the Authentication.isAuthenticated() property by not calling AuthenticationManager if not required. AbstractSecurityInterceptor never sets Authentication.isAuthenticated() (it does at present) This means there are only two actors changing the Authentication.isAuthenticated() flag: HttpSessionContextIntegrationFilter to clear it, and an AuthenticationProvider to set it. You are only considering DAO authentication provider here, how about the other providers that do not have caching? Most AuthenticationProvider implementations use caching. At least the CAS, X.509 and DAO providers do, and they're the main ones people use. We are using Jaas provider :( Besides, I was supporting that we should address this issue, just noting it isn't a critical performance issue at present. The benefit of these changes is mainly going to be avoiding unnecessary interaction with collections in ProviderManager and the various caches, plus providing a request-specific hook for future expansion. That should improve performance, although today's collection implementations are fairly well-optimised. Best regards Ben --- This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space? Enter now for the Oracle Space Sweepstakes! http://ads.osdn.com/?ad_id=7412alloc_id=16344op=click ___ Home: http://acegisecurity.sourceforge.net Acegisecurity-developer mailing list Acegisecurity-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/acegisecurity-developer --- This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space? Enter now for the Oracle Space Sweepstakes! http://ads.osdn.com/?ad_idt12alloc_id344op=click ___ Home: http://acegisecurity.sourceforge.net Acegisecurity-developer mailing list Acegisecurity-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/acegisecurity-developer
[Acegisecurity-developer] Question about AbstractSecurityInterceptor
Hi, I am exploring AcegiSecurity by following the contacts sample application. I observed that in AbstractSecurityInterceptor.beforeInvocation method authenticationManager.authenticate is being called for every request. Why is it? Can it be optimized to check if authentication is already done and skip this step? For every click the above method is called twice: once as part of FilterSecurityInterceptor and once as part of MethodSecurityInterceptor and if we optimize/check for authentication then the call to authenticationManager.authenticate can be avoided. Thanks, --Venkat. --- This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space? Enter now for the Oracle Space Sweepstakes! http://ads.osdn.com/?ad_idt12alloc_id344op=click ___ Home: http://acegisecurity.sourceforge.net Acegisecurity-developer mailing list Acegisecurity-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/acegisecurity-developer
Re: [Acegisecurity-developer] Question about AbstractSecurityInterceptor
Venkat Sonnathi wrote: Hi, I am exploring AcegiSecurity by following the contacts sample application. I observed that in AbstractSecurityInterceptor.beforeInvocation method authenticationManager.authenticate is being called for every request. Why is it? Can it be optimized to check if authentication is already done and skip this step? For every click the above method is called twice: once as part of FilterSecurityInterceptor and once as part of MethodSecurityInterceptor and if we optimize/check for authentication then the call to authenticationManager.authenticate can be avoided. Thanks, --Venkat. Yes, it could be optimised using a range of strategies such as putting a flag into the ContextHolder (SecurityContextHolder in CVS and from 0.9.0). The trouble is then causing that flag to reset at the start of each request so that only the first AbstractSecurityInterceptor invocation causes delegation to the AuthenticationManager. Indeed if the request was actually authenticated during the request by an authentication mechanism (eg BASIC/Digest/form post) we should accept that AuthenticationManager invocation and not require any AbstractSecurityInterceptor to repeat it. We could refresh the SecurityContextHolder flag this in the HttpSessionContextIntegrationFilter, but then we're reducing its focus on simply storing the SecurityContext between HTTP requests. Better yet, we could have startRequest() and finishRequest() methods in the SecurityContext to encapsulate this sort of logic and have the HttpSessionContextIntegrationFilter call them. Indeed that might prove beneficial for Captcha integration, which also needs to set a once-per-request style flag. What do others think? I have not performed any benchmarks, but given that caching is performed in AuthenticationProvider implementations I would not suspect the double-up of AuthenticationManager invocations would represent an urgent performance constraint at this time. Best regards Ben --- This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space? Enter now for the Oracle Space Sweepstakes! http://ads.osdn.com/?ad_id=7412alloc_id=16344op=click ___ Home: http://acegisecurity.sourceforge.net Acegisecurity-developer mailing list Acegisecurity-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/acegisecurity-developer
RE: [Acegisecurity-developer] Question about AbstractSecurityInterceptor
-Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Ben Alex Sent: Tuesday, May 17, 2005 6:00 PM To: acegisecurity-developer@lists.sourceforge.net Subject: Re: [Acegisecurity-developer] Question about AbstractSecurityInterceptor Venkat Sonnathi wrote: Hi, I am exploring AcegiSecurity by following the contacts sample application. I observed that in AbstractSecurityInterceptor.beforeInvocation method authenticationManager.authenticate is being called for every request. Why is it? Can it be optimized to check if authentication is already done and skip this step? For every click the above method is called twice: once as part of FilterSecurityInterceptor and once as part of MethodSecurityInterceptor and if we optimize/check for authentication then the call to authenticationManager.authenticate can be avoided. Thanks, --Venkat. Yes, it could be optimised using a range of strategies such as putting a flag into the ContextHolder (SecurityContextHolder in CVS and from 0.9.0). The trouble is then causing that flag to reset at the start of each request so that only the first AbstractSecurityInterceptor invocation causes delegation to the AuthenticationManager. Indeed if the request was actually authenticated during the request by an authentication mechanism (eg BASIC/Digest/form post) we should accept that AuthenticationManager invocation and not require any AbstractSecurityInterceptor to repeat it. How about adding this check at one central place, AuthenticationManager? I am doing this and I do not see any problem. I set the authenticate to true after successful authentication, and check for isAuthentication() before every call. We could refresh the SecurityContextHolder flag this in the HttpSessionContextIntegrationFilter, but then we're reducing its focus on simply storing the SecurityContext between HTTP requests. Better yet, we could have startRequest() and finishRequest() methods in the SecurityContext to encapsulate this sort of logic and have the HttpSessionContextIntegrationFilter call them. Indeed that might prove beneficial for Captcha integration, which also needs to set a once-per-request style flag. What do others think? I have not performed any benchmarks, but given that caching is performed in AuthenticationProvider implementations I would not suspect the double-up of AuthenticationManager invocations would represent an urgent performance constraint at this time. You are only considering DAO authentication provider here, how about the other providers that do not have caching? Regards Ghazenfer --- This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space? Enter now for the Oracle Space Sweepstakes! http://ads.osdn.com/?ad_idt12alloc_id344op=click ___ Home: http://acegisecurity.sourceforge.net Acegisecurity-developer mailing list Acegisecurity-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/acegisecurity-developer
Re: [Acegisecurity-developer] Question about AbstractSecurityInterceptor
Mansoor, Ghazenfer (EDS) wrote: How about adding this check at one central place, AuthenticationManager? I am doing this and I do not see any problem. I set the authenticate to true after successful authentication, and check for isAuthentication() before every call. What sets your Authentication.isAuthenticated() back to false at the start of each request? I would propose the following: - SecurityContext to provide a startRequest() and finishRequest() method that is called by HttpSessionContextIntegrationFilter. - The startRequest() and finishRequest() set Authentication.isAuthenticated() to false. Do it twice in case the Authentication is being presented from a remote system (eg via RMI) which has set the isAuthenticated() to true. - An AuthenticationProvider may, but is not required to, set Authentication.isAuthenticated() to true. If it does set it to true, it means it does not require further callback and the Authentication can safely be used for the remainder of the request. AuthenticationProviders should provide a property setting so this can be switched off (ie they never set the flag to true) as in special situations (like chained AuthenticationProviders or a cache-aware ProviderManager) it might be undesirable. - AbstractSecurityInterceptor honours the Authentication.isAuthenticated() property by not calling AuthenticationManager if not required. AbstractSecurityInterceptor never sets Authentication.isAuthenticated() (it does at present) This means there are only two actors changing the Authentication.isAuthenticated() flag: HttpSessionContextIntegrationFilter to clear it, and an AuthenticationProvider to set it. You are only considering DAO authentication provider here, how about the other providers that do not have caching? Most AuthenticationProvider implementations use caching. At least the CAS, X.509 and DAO providers do, and they're the main ones people use. Besides, I was supporting that we should address this issue, just noting it isn't a critical performance issue at present. The benefit of these changes is mainly going to be avoiding unnecessary interaction with collections in ProviderManager and the various caches, plus providing a request-specific hook for future expansion. That should improve performance, although today's collection implementations are fairly well-optimised. Best regards Ben --- This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space? Enter now for the Oracle Space Sweepstakes! http://ads.osdn.com/?ad_id=7412alloc_id=16344op=click ___ Home: http://acegisecurity.sourceforge.net Acegisecurity-developer mailing list Acegisecurity-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/acegisecurity-developer
