You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Adam Heath <do...@brainfood.com> on 2009/09/15 22:22:46 UTC
Re: svn commit: r802563 - in /ofbiz/trunk/framework: security/src/org/ofbiz/security/OFBizSecurity.java
security/src/org/ofbiz/security/Security.java security/src/org/ofbiz/security/SecurityFactory.java
webapp/src/org/ofbiz/webapp/control/LoginWorker.java
adrianc@apache.org wrote:
> Author: adrianc
> Date: Sun Aug 9 17:20:06 2009
> New Revision: 802563
>
> URL: http://svn.apache.org/viewvc?rev=802563&view=rev
> Log:
> Converted Security.java to an interface. No functional change.
>
> Modified:
> ofbiz/trunk/framework/security/src/org/ofbiz/security/OFBizSecurity.java
> ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java
> ofbiz/trunk/framework/security/src/org/ofbiz/security/SecurityFactory.java
> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/LoginWorker.java
>
> Modified: ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java?rev=802563&r1=802562&r2=802563&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java (original)
> +++ ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java Sun Aug 9 17:20:06 2009
> @@ -23,36 +23,17 @@
>
> import javax.servlet.http.HttpSession;
>
> -import org.ofbiz.base.util.cache.UtilCache;
> import org.ofbiz.entity.GenericDelegator;
> import org.ofbiz.entity.GenericValue;
>
> /**
> - * Security handler: This class is an abstract implementation for all commononly used security aspects.
> + * Security interface. This interface defines security-related methods.
> */
> -public abstract class Security {
> +public interface Security {
>
> - /**
> - * UtilCache to cache a Collection of UserLoginSecurityGroup entities for each UserLogin, by userLoginId.
> - */
> - public static UtilCache<String, List<GenericValue>> userLoginSecurityGroupByUserLoginId = new UtilCache<String, List<GenericValue>>("security.UserLoginSecurityGroupByUserLoginId");
This change should not be done in trunk, if it is part of the
ExecutionContext work. This change broke addUserLoginToSecurityGroup
and related services, as the simple methods called by the service
contain embedded bsh, that accesses fields directly in the Security class.
I have a fix for this, but really, this kind of dramatic change should
be tested for an extended period on a branch.
==
Sourced file: <Inline eval of:
org.ofbiz.security.Security.userLoginSecurityGroupByUserLoginId.remove(parameters.get("userLoginId"));
; > : No static field
or inner class: userLoginSecurityGroupByUserLoginId of interface
org.ofbiz.security.Security : at Line: 1 : in file: <Inline eval of:
org.ofbiz.security.S
ecurity.userLoginSecurityGroupByUserLoginId.remove(parameters.get("userLoginId"));
; > : org .ofbiz .security .Security .userLoginSecurityGroupByUserLoginId
.remove ( parameters .get ( "userLoginId" ) )
==
Re: svn commit: r802563 - in /ofbiz/trunk/framework: security/src/org/ofbiz/security/OFBizSecurity.java
security/src/org/ofbiz/security/Security.java security/src/org/ofbiz/security/SecurityFactory.java
webapp/src/org/ofbiz/webapp/control/LoginWorker.java
Posted by Adrian Crum <ad...@hlmksw.com>.
The change is part of the security redesign. The plan is to extract the
interface, then have swappable implementations.
I try to test things thoroughly, but there is always a chance I'll miss
something. Thank you for the info!
-Adrian
Adam Heath wrote:
> Adam Heath wrote:
>> adrianc@apache.org wrote:
>>> Author: adrianc
>>> Date: Sun Aug 9 17:20:06 2009
>>> New Revision: 802563
>>>
>>> URL: http://svn.apache.org/viewvc?rev=802563&view=rev
>>> Log:
>>> Converted Security.java to an interface. No functional change.
>>>
>>> Modified:
>>> ofbiz/trunk/framework/security/src/org/ofbiz/security/OFBizSecurity.java
>>> ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java
>>> ofbiz/trunk/framework/security/src/org/ofbiz/security/SecurityFactory.java
>>> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/LoginWorker.java
>>>
>>> Modified: ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java?rev=802563&r1=802562&r2=802563&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java (original)
>>> +++ ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java Sun Aug 9 17:20:06 2009
>>> @@ -23,36 +23,17 @@
>>>
>>> import javax.servlet.http.HttpSession;
>>>
>>> -import org.ofbiz.base.util.cache.UtilCache;
>>> import org.ofbiz.entity.GenericDelegator;
>>> import org.ofbiz.entity.GenericValue;
>>>
>>> /**
>>> - * Security handler: This class is an abstract implementation for all commononly used security aspects.
>>> + * Security interface. This interface defines security-related methods.
>>> */
>>> -public abstract class Security {
>>> +public interface Security {
>>>
>>> - /**
>>> - * UtilCache to cache a Collection of UserLoginSecurityGroup entities for each UserLogin, by userLoginId.
>>> - */
>>> - public static UtilCache<String, List<GenericValue>> userLoginSecurityGroupByUserLoginId = new UtilCache<String, List<GenericValue>>("security.UserLoginSecurityGroupByUserLoginId");
>>
>> This change should not be done in trunk, if it is part of the
>> ExecutionContext work. This change broke addUserLoginToSecurityGroup
>> and related services, as the simple methods called by the service
>> contain embedded bsh, that accesses fields directly in the Security class.
>
> Er, sorry, maybe a little harsh. But I get the feeling that this
> change wasn't tested as well as it should have been. If something is
> as low level as this, we need to be *very* careful about changing things.
>
>> I have a fix for this, but really, this kind of dramatic change should
>> be tested for an extended period on a branch.
>>
>> ==
>> Sourced file: <Inline eval of:
>> org.ofbiz.security.Security.userLoginSecurityGroupByUserLoginId.remove(parameters.get("userLoginId"));
>> ; > : No static field
>> or inner class: userLoginSecurityGroupByUserLoginId of interface
>> org.ofbiz.security.Security : at Line: 1 : in file: <Inline eval of:
>> org.ofbiz.security.S
>> ecurity.userLoginSecurityGroupByUserLoginId.remove(parameters.get("userLoginId"));
>> ; > : org .ofbiz .security .Security .userLoginSecurityGroupByUserLoginId
>> .remove ( parameters .get ( "userLoginId" ) )
>> ==
>
>
Re: svn commit: r802563 - in /ofbiz/trunk/framework: security/src/org/ofbiz/security/OFBizSecurity.java
security/src/org/ofbiz/security/Security.java security/src/org/ofbiz/security/SecurityFactory.java
webapp/src/org/ofbiz/webapp/control/LoginWorker.java
Posted by Adam Heath <do...@brainfood.com>.
Adam Heath wrote:
> adrianc@apache.org wrote:
>> Author: adrianc
>> Date: Sun Aug 9 17:20:06 2009
>> New Revision: 802563
>>
>> URL: http://svn.apache.org/viewvc?rev=802563&view=rev
>> Log:
>> Converted Security.java to an interface. No functional change.
>>
>> Modified:
>> ofbiz/trunk/framework/security/src/org/ofbiz/security/OFBizSecurity.java
>> ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java
>> ofbiz/trunk/framework/security/src/org/ofbiz/security/SecurityFactory.java
>> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/LoginWorker.java
>>
>> Modified: ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java?rev=802563&r1=802562&r2=802563&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java (original)
>> +++ ofbiz/trunk/framework/security/src/org/ofbiz/security/Security.java Sun Aug 9 17:20:06 2009
>> @@ -23,36 +23,17 @@
>>
>> import javax.servlet.http.HttpSession;
>>
>> -import org.ofbiz.base.util.cache.UtilCache;
>> import org.ofbiz.entity.GenericDelegator;
>> import org.ofbiz.entity.GenericValue;
>>
>> /**
>> - * Security handler: This class is an abstract implementation for all commononly used security aspects.
>> + * Security interface. This interface defines security-related methods.
>> */
>> -public abstract class Security {
>> +public interface Security {
>>
>> - /**
>> - * UtilCache to cache a Collection of UserLoginSecurityGroup entities for each UserLogin, by userLoginId.
>> - */
>> - public static UtilCache<String, List<GenericValue>> userLoginSecurityGroupByUserLoginId = new UtilCache<String, List<GenericValue>>("security.UserLoginSecurityGroupByUserLoginId");
>
>
> This change should not be done in trunk, if it is part of the
> ExecutionContext work. This change broke addUserLoginToSecurityGroup
> and related services, as the simple methods called by the service
> contain embedded bsh, that accesses fields directly in the Security class.
Er, sorry, maybe a little harsh. But I get the feeling that this
change wasn't tested as well as it should have been. If something is
as low level as this, we need to be *very* careful about changing things.
>
> I have a fix for this, but really, this kind of dramatic change should
> be tested for an extended period on a branch.
>
> ==
> Sourced file: <Inline eval of:
> org.ofbiz.security.Security.userLoginSecurityGroupByUserLoginId.remove(parameters.get("userLoginId"));
> ; > : No static field
> or inner class: userLoginSecurityGroupByUserLoginId of interface
> org.ofbiz.security.Security : at Line: 1 : in file: <Inline eval of:
> org.ofbiz.security.S
> ecurity.userLoginSecurityGroupByUserLoginId.remove(parameters.get("userLoginId"));
> ; > : org .ofbiz .security .Security .userLoginSecurityGroupByUserLoginId
> .remove ( parameters .get ( "userLoginId" ) )
> ==