Re: svn commit: r1421651 - /ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java

[Adrian Crum-3]

Please don't do this.

There was no demand - Adam and I suggested a better way to do things,
and this is not a better way. There is no need to store the property
values in class fields.

-Adrian

On 12/14/2012 3:27 AM, [hidden email] wrote:

> Author: jleroux
> Date: Fri Dec 14 03:27:40 2012
> New Revision: 1421651
>
> URL: http://svn.apache.org/viewvc?rev=1421651&view=rev
> Log:
> At Adrian and Adam's demand allow to use cache here, use default constructor
> Also refactored things a bit:
> * uses getPropertyAsBoolean
> * set default property values
>
> Modified:
>      ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java
>
> Modified: ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java?rev=1421651&r1=1421650&r2=1421651&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java (original)
> +++ ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java Fri Dec 14 03:27:40 2012
> @@ -64,8 +64,8 @@ public class LoginServices {
>  
>       public static final String module = LoginServices.class.getName();
>       public static final String resource = "SecurityextUiLabels";
> -    public static boolean usePasswordPattern = "true".equals(UtilProperties.getPropertyValue("security.properties", "security.login.password.pattern.enable"));
> -    public static String passwordPattern = UtilProperties.getPropertyValue("security.properties", "security.login.password.pattern");
> +    public boolean usePasswordPattern = UtilProperties.getPropertyAsBoolean("security.properties", "security.login.password.pattern.enable", true);
> +    public String passwordPattern = UtilProperties.getPropertyValue("security.properties", "security.login.password.pattern", "^.*(?=.{5,}).*$");
>  
>       /** Login service to authenticate username and password
>        * @return Map of results including (userLogin) GenericValue object
> @@ -517,7 +517,8 @@ public class LoginServices {
>               }
>           }
>  
> -        checkNewPassword(null, null, currentPassword, currentPasswordVerify, passwordHint, errorMessageList, true, locale);
> +        LoginServices loginServices = new LoginServices();
> +        loginServices.checkNewPassword(null, null, currentPassword, currentPasswordVerify, passwordHint, errorMessageList, true, locale);
>  
>           GenericValue userLoginToCreate = delegator.makeValue("UserLogin", UtilMisc.toMap("userLoginId", userLoginId));
>           userLoginToCreate.set("externalAuthId", externalAuthId);
> @@ -656,7 +657,8 @@ public class LoginServices {
>  
>           List<String> errorMessageList = FastList.newInstance();
>           if (newPassword != null) {
> -            checkNewPassword(userLoginToUpdate, currentPassword, newPassword, newPasswordVerify,
> +            LoginServices loginServices = new LoginServices();
> +            loginServices.checkNewPassword(userLoginToUpdate, currentPassword, newPassword, newPasswordVerify,
>                   passwordHint, errorMessageList, adminUser, locale);
>           }
>  
> @@ -887,7 +889,7 @@ public class LoginServices {
>           return result;
>       }
>  
> -    public static void checkNewPassword(GenericValue userLogin, String currentPassword, String newPassword, String newPasswordVerify, String passwordHint, List<String> errorMessageList, boolean ignoreCurrentPassword, Locale locale) {
> +    public void checkNewPassword(GenericValue userLogin, String currentPassword, String newPassword, String newPasswordVerify, String passwordHint, List<String> errorMessageList, boolean ignoreCurrentPassword, Locale locale) {
>           boolean useEncryption = "true".equals(UtilProperties.getPropertyValue("security.properties", "password.encrypt"));
>  
>           String errMsg = null;
>
>

[Jacques Le Roux]

OK done at revision: 1421738  

Jacques

From: "Adrian Crum" <[hidden email]>

> Please don't do this.
>
> There was no demand - Adam and I suggested a better way to do things,
> and this is not a better way. There is no need to store the property
> values in class fields.
>
> -Adrian
>
> On 12/14/2012 3:27 AM, [hidden email] wrote:
>> Author: jleroux
>> Date: Fri Dec 14 03:27:40 2012
>> New Revision: 1421651
>>
>> URL: http://svn.apache.org/viewvc?rev=1421651&view=rev
>> Log:
>> At Adrian and Adam's demand allow to use cache here, use default constructor
>> Also refactored things a bit:
>> * uses getPropertyAsBoolean
>> * set default property values
>>
>> Modified:
>>      ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java
>>
>> Modified: ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java?rev=1421651&r1=1421650&r2=1421651&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java (original)
>> +++ ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java Fri Dec 14 03:27:40 2012
>> @@ -64,8 +64,8 @@ public class LoginServices {
>>  
>>       public static final String module = LoginServices.class.getName();
>>       public static final String resource = "SecurityextUiLabels";
>> -    public static boolean usePasswordPattern = "true".equals(UtilProperties.getPropertyValue("security.properties", "security.login.password.pattern.enable"));
>> -    public static String passwordPattern = UtilProperties.getPropertyValue("security.properties", "security.login.password.pattern");
>> +    public boolean usePasswordPattern = UtilProperties.getPropertyAsBoolean("security.properties", "security.login.password.pattern.enable", true);
>> +    public String passwordPattern = UtilProperties.getPropertyValue("security.properties", "security.login.password.pattern", "^.*(?=.{5,}).*$");
>>  
>>       /** Login service to authenticate username and password
>>        * @return Map of results including (userLogin) GenericValue object
>> @@ -517,7 +517,8 @@ public class LoginServices {
>>               }
>>           }
>>  
>> -        checkNewPassword(null, null, currentPassword, currentPasswordVerify, passwordHint, errorMessageList, true, locale);
>> +        LoginServices loginServices = new LoginServices();
>> +        loginServices.checkNewPassword(null, null, currentPassword, currentPasswordVerify, passwordHint, errorMessageList, true, locale);
>>  
>>           GenericValue userLoginToCreate = delegator.makeValue("UserLogin", UtilMisc.toMap("userLoginId", userLoginId));
>>           userLoginToCreate.set("externalAuthId", externalAuthId);
>> @@ -656,7 +657,8 @@ public class LoginServices {
>>  
>>           List<String> errorMessageList = FastList.newInstance();
>>           if (newPassword != null) {
>> -            checkNewPassword(userLoginToUpdate, currentPassword, newPassword, newPasswordVerify,
>> +            LoginServices loginServices = new LoginServices();
>> +            loginServices.checkNewPassword(userLoginToUpdate, currentPassword, newPassword, newPasswordVerify,
>>                   passwordHint, errorMessageList, adminUser, locale);
>>           }
>>  
>> @@ -887,7 +889,7 @@ public class LoginServices {
>>           return result;
>>       }
>>  
>> -    public static void checkNewPassword(GenericValue userLogin, String currentPassword, String newPassword, String newPasswordVerify, String passwordHint, List<String> errorMessageList, boolean ignoreCurrentPassword, Locale locale) {
>> +    public void checkNewPassword(GenericValue userLogin, String currentPassword, String newPassword, String newPasswordVerify, String passwordHint, List<String> errorMessageList, boolean ignoreCurrentPassword, Locale locale) {
>>           boolean useEncryption = "true".equals(UtilProperties.getPropertyValue("security.properties", "password.encrypt"));
>>  
>>           String errMsg = null;
>>
>>
>

[Scott Gray-2]

I think Jacques tends to use "demand" where most would use "request", just a language issue.

Regards
Scott

On 14/12/2012, at 9:36 PM, Adrian Crum wrote:

> Please don't do this.
>
> There was no demand - Adam and I suggested a better way to do things, and this is not a better way. There is no need to store the property values in class fields.
>
> -Adrian
>
> On 12/14/2012 3:27 AM, [hidden email] wrote:
>> Author: jleroux
>> Date: Fri Dec 14 03:27:40 2012
>> New Revision: 1421651
>>
>> URL: http://svn.apache.org/viewvc?rev=1421651&view=rev
>> Log:
>> At Adrian and Adam's demand allow to use cache here, use default constructor
>> Also refactored things a bit:
>> * uses getPropertyAsBoolean
>> * set default property values
>>
>> Modified:
>>     ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java
>>
>> Modified: ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java?rev=1421651&r1=1421650&r2=1421651&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java (original)
>> +++ ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java Fri Dec 14 03:27:40 2012
>> @@ -64,8 +64,8 @@ public class LoginServices {
>>        public static final String module = LoginServices.class.getName();
>>      public static final String resource = "SecurityextUiLabels";
>> -    public static boolean usePasswordPattern = "true".equals(UtilProperties.getPropertyValue("security.properties", "security.login.password.pattern.enable"));
>> -    public static String passwordPattern = UtilProperties.getPropertyValue("security.properties", "security.login.password.pattern");
>> +    public boolean usePasswordPattern = UtilProperties.getPropertyAsBoolean("security.properties", "security.login.password.pattern.enable", true);
>> +    public String passwordPattern = UtilProperties.getPropertyValue("security.properties", "security.login.password.pattern", "^.*(?=.{5,}).*$");
>>        /** Login service to authenticate username and password
>>       * @return Map of results including (userLogin) GenericValue object
>> @@ -517,7 +517,8 @@ public class LoginServices {
>>              }
>>          }
>>  -        checkNewPassword(null, null, currentPassword, currentPasswordVerify, passwordHint, errorMessageList, true, locale);
>> +        LoginServices loginServices = new LoginServices();
>> +        loginServices.checkNewPassword(null, null, currentPassword, currentPasswordVerify, passwordHint, errorMessageList, true, locale);
>>            GenericValue userLoginToCreate = delegator.makeValue("UserLogin", UtilMisc.toMap("userLoginId", userLoginId));
>>          userLoginToCreate.set("externalAuthId", externalAuthId);
>> @@ -656,7 +657,8 @@ public class LoginServices {
>>            List<String> errorMessageList = FastList.newInstance();
>>          if (newPassword != null) {
>> -            checkNewPassword(userLoginToUpdate, currentPassword, newPassword, newPasswordVerify,
>> +            LoginServices loginServices = new LoginServices();
>> +            loginServices.checkNewPassword(userLoginToUpdate, currentPassword, newPassword, newPasswordVerify,
>>                  passwordHint, errorMessageList, adminUser, locale);
>>          }
>>  @@ -887,7 +889,7 @@ public class LoginServices {
>>          return result;
>>      }
>>  -    public static void checkNewPassword(GenericValue userLogin, String currentPassword, String newPassword, String newPasswordVerify, String passwordHint, List<String> errorMessageList, boolean ignoreCurrentPassword, Locale locale) {
>> +    public void checkNewPassword(GenericValue userLogin, String currentPassword, String newPassword, String newPasswordVerify, String passwordHint, List<String> errorMessageList, boolean ignoreCurrentPassword, Locale locale) {
>>          boolean useEncryption = "true".equals(UtilProperties.getPropertyValue("security.properties", "password.encrypt"));
>>            String errMsg = null;
>>
>>
>

[Jacopo Cappellato-4]

I am sure Scott is right.
@Jacques: the meaning of demand in English is: "an insistent and peremptory request, made as if by right" and it is different from the French "demande" (I guess) and the Italian "domanda" that simply mean "request". I am pretty sure that Jacques meant to use "demand(e)" as a synonym of "request".

Regards,

Jacopo

On Dec 15, 2012, at 3:18 AM, Scott Gray wrote:

> I think Jacques tends to use "demand" where most would use "request", just a language issue.
>
> Regards
> Scott
>
> On 14/12/2012, at 9:36 PM, Adrian Crum wrote:
>
>> Please don't do this.
>>
>> There was no demand - Adam and I suggested a better way to do things, and this is not a better way. There is no need to store the property values in class fields.
>>
>> -Adrian
>>
>> On 12/14/2012 3:27 AM, [hidden email] wrote:
>>> Author: jleroux
>>> Date: Fri Dec 14 03:27:40 2012
>>> New Revision: 1421651
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1421651&view=rev
>>> Log:
>>> At Adrian and Adam's demand allow to use cache here, use default constructor
>>> Also refactored things a bit:
>>> * uses getPropertyAsBoolean
>>> * set default property values
>>>
>>> Modified:
>>>    ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java
>>>
>>> Modified: ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java?rev=1421651&r1=1421650&r2=1421651&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java (original)
>>> +++ ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java Fri Dec 14 03:27:40 2012
>>> @@ -64,8 +64,8 @@ public class LoginServices {
>>>       public static final String module = LoginServices.class.getName();
>>>     public static final String resource = "SecurityextUiLabels";
>>> -    public static boolean usePasswordPattern = "true".equals(UtilProperties.getPropertyValue("security.properties", "security.login.password.pattern.enable"));
>>> -    public static String passwordPattern = UtilProperties.getPropertyValue("security.properties", "security.login.password.pattern");
>>> +    public boolean usePasswordPattern = UtilProperties.getPropertyAsBoolean("security.properties", "security.login.password.pattern.enable", true);
>>> +    public String passwordPattern = UtilProperties.getPropertyValue("security.properties", "security.login.password.pattern", "^.*(?=.{5,}).*$");
>>>       /** Login service to authenticate username and password
>>>      * @return Map of results including (userLogin) GenericValue object
>>> @@ -517,7 +517,8 @@ public class LoginServices {
>>>             }
>>>         }
>>> -        checkNewPassword(null, null, currentPassword, currentPasswordVerify, passwordHint, errorMessageList, true, locale);
>>> +        LoginServices loginServices = new LoginServices();
>>> +        loginServices.checkNewPassword(null, null, currentPassword, currentPasswordVerify, passwordHint, errorMessageList, true, locale);
>>>           GenericValue userLoginToCreate = delegator.makeValue("UserLogin", UtilMisc.toMap("userLoginId", userLoginId));
>>>         userLoginToCreate.set("externalAuthId", externalAuthId);
>>> @@ -656,7 +657,8 @@ public class LoginServices {
>>>           List<String> errorMessageList = FastList.newInstance();
>>>         if (newPassword != null) {
>>> -            checkNewPassword(userLoginToUpdate, currentPassword, newPassword, newPasswordVerify,
>>> +            LoginServices loginServices = new LoginServices();
>>> +            loginServices.checkNewPassword(userLoginToUpdate, currentPassword, newPassword, newPasswordVerify,
>>>                 passwordHint, errorMessageList, adminUser, locale);
>>>         }
>>> @@ -887,7 +889,7 @@ public class LoginServices {
>>>         return result;
>>>     }
>>> -    public static void checkNewPassword(GenericValue userLogin, String currentPassword, String newPassword, String newPasswordVerify, String passwordHint, List<String> errorMessageList, boolean ignoreCurrentPassword, Locale locale) {
>>> +    public void checkNewPassword(GenericValue userLogin, String currentPassword, String newPassword, String newPasswordVerify, String passwordHint, List<String> errorMessageList, boolean ignoreCurrentPassword, Locale locale) {
>>>         boolean useEncryption = "true".equals(UtilProperties.getPropertyValue("security.properties", "password.encrypt"));
>>>           String errMsg = null;
>>>
>>>
>>
>

[Jacques Le Roux]

Exactly, sorry Adrian :D

Thanks Scott, Jacopo.

Jacques

From: "Jacopo Cappellato" <[hidden email]>

>I am sure Scott is right.
> @Jacques: the meaning of demand in English is: "an insistent and peremptory request, made as if by right" and it is different from the French "demande" (I guess) and the Italian "domanda" that simply mean "request". I am pretty sure that Jacques meant to use "demand(e)" as a synonym of "request".
>
> Regards,
>
> Jacopo
>
> On Dec 15, 2012, at 3:18 AM, Scott Gray wrote:
>
>> I think Jacques tends to use "demand" where most would use "request", just a language issue.
>>
>> Regards
>> Scott
>>
>> On 14/12/2012, at 9:36 PM, Adrian Crum wrote:
>>
>>> Please don't do this.
>>>
>>> There was no demand - Adam and I suggested a better way to do things, and this is not a better way. There is no need to store the property values in class fields.
>>>
>>> -Adrian
>>>
>>> On 12/14/2012 3:27 AM, [hidden email] wrote:
>>>> Author: jleroux
>>>> Date: Fri Dec 14 03:27:40 2012
>>>> New Revision: 1421651
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1421651&view=rev
>>>> Log:
>>>> At Adrian and Adam's demand allow to use cache here, use default constructor
>>>> Also refactored things a bit:
>>>> * uses getPropertyAsBoolean
>>>> * set default property values
>>>>
>>>> Modified:
>>>>    ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java
>>>>
>>>> Modified: ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java?rev=1421651&r1=1421650&r2=1421651&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java (original)
>>>> +++ ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java Fri Dec 14 03:27:40 2012
>>>> @@ -64,8 +64,8 @@ public class LoginServices {
>>>>       public static final String module = LoginServices.class.getName();
>>>>     public static final String resource = "SecurityextUiLabels";
>>>> -    public static boolean usePasswordPattern = "true".equals(UtilProperties.getPropertyValue("security.properties", "security.login.password.pattern.enable"));
>>>> -    public static String passwordPattern = UtilProperties.getPropertyValue("security.properties", "security.login.password.pattern");
>>>> +    public boolean usePasswordPattern = UtilProperties.getPropertyAsBoolean("security.properties", "security.login.password.pattern.enable", true);
>>>> +    public String passwordPattern = UtilProperties.getPropertyValue("security.properties", "security.login.password.pattern", "^.*(?=.{5,}).*$");
>>>>       /** Login service to authenticate username and password
>>>>      * @return Map of results including (userLogin) GenericValue object
>>>> @@ -517,7 +517,8 @@ public class LoginServices {
>>>>             }
>>>>         }
>>>> -        checkNewPassword(null, null, currentPassword, currentPasswordVerify, passwordHint, errorMessageList, true, locale);
>>>> +        LoginServices loginServices = new LoginServices();
>>>> +        loginServices.checkNewPassword(null, null, currentPassword, currentPasswordVerify, passwordHint, errorMessageList, true, locale);
>>>>           GenericValue userLoginToCreate = delegator.makeValue("UserLogin", UtilMisc.toMap("userLoginId", userLoginId));
>>>>         userLoginToCreate.set("externalAuthId", externalAuthId);
>>>> @@ -656,7 +657,8 @@ public class LoginServices {
>>>>           List<String> errorMessageList = FastList.newInstance();
>>>>         if (newPassword != null) {
>>>> -            checkNewPassword(userLoginToUpdate, currentPassword, newPassword, newPasswordVerify,
>>>> +            LoginServices loginServices = new LoginServices();
>>>> +            loginServices.checkNewPassword(userLoginToUpdate, currentPassword, newPassword, newPasswordVerify,
>>>>                 passwordHint, errorMessageList, adminUser, locale);
>>>>         }
>>>> @@ -887,7 +889,7 @@ public class LoginServices {
>>>>         return result;
>>>>     }
>>>> -    public static void checkNewPassword(GenericValue userLogin, String currentPassword, String newPassword, String newPasswordVerify, String passwordHint, List<String> errorMessageList, boolean ignoreCurrentPassword, Locale locale) {
>>>> +    public void checkNewPassword(GenericValue userLogin, String currentPassword, String newPassword, String newPasswordVerify, String passwordHint, List<String> errorMessageList, boolean ignoreCurrentPassword, Locale locale) {
>>>>         boolean useEncryption = "true".equals(UtilProperties.getPropertyValue("security.properties", "password.encrypt"));
>>>>           String errMsg = null;
>>>>
>>>>
>>>
>>
>
>

[Jacques Le Roux]

BTW, I think Henry Yandel is right here https://twitter.com/flamefew/status/277877229445320705
<<*English* isn't a language, it's *two languages* shoved together with lots of stolen glue.>>

He said it ;)

Jacques

Jacques Le Roux wrote:

> Exactly, sorry Adrian :D
>
> Thanks Scott, Jacopo.
>
> Jacques
>
> From: "Jacopo Cappellato" <[hidden email]>
>> I am sure Scott is right.
>> @Jacques: the meaning of demand in English is: "an insistent and peremptory request, made as if by right" and it is different
>> from the French "demande" (I guess) and the Italian "domanda" that simply mean "request". I am pretty sure that Jacques meant to
>> use "demand(e)" as a synonym of "request".  
>>
>> Regards,
>>
>> Jacopo
>>
>> On Dec 15, 2012, at 3:18 AM, Scott Gray wrote:
>>
>>> I think Jacques tends to use "demand" where most would use "request", just a language issue.
>>>
>>> Regards
>>> Scott
>>>
>>> On 14/12/2012, at 9:36 PM, Adrian Crum wrote:
>>>
>>>> Please don't do this.
>>>>
>>>> There was no demand - Adam and I suggested a better way to do things, and this is not a better way. There is no need to store
>>>> the property values in class fields.
>>>>
>>>> -Adrian
>>>>
>>>> On 12/14/2012 3:27 AM, [hidden email] wrote:
>>>>> Author: jleroux
>>>>> Date: Fri Dec 14 03:27:40 2012
>>>>> New Revision: 1421651
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1421651&view=rev
>>>>> Log:
>>>>> At Adrian and Adam's demand allow to use cache here, use default constructor
>>>>> Also refactored things a bit:
>>>>> * uses getPropertyAsBoolean
>>>>> * set default property values