Re: [Spacewalk-devel] [PATCH] Adding a password placeholder check when editing a user.

2014-02-04 Thread Maximilian Meister 

On 01/29/2014 02:22 PM, Michael Mraka wrote:


I see :(, I was not aware of it. Anyway I'd still prefer not to use javascript
if not necessary. So in this particular case I'd replace html:password
with direct html input tag:

 input type=password name=desiredpassword class=form-control size=15 maxlength=${passwordLength} 
placeholder=bull;bull;bull;bull;bull;bull;/


Hi Michael,

i changed the patch. So no additional javascript.

* I replaced the struts tag with the standard html input tag to use the 
placeholder attribute
* Some changes to the logic in 
spacewalk-pwstrength-handler.js:updateTickIcon()

* Johannes Renner helped me with the Java code changes.

The question is now in UserEditActionHelper:62 we use more or less the 
same code for validation

as in UpdateUserCommand:132
As this is a small redundancy in code, I wanted to ask if it would make 
sense to put that code into
a public function accessible by both classes, and where this function 
should reside.
Do you think it is worth the extra work, or is the solution in the patch 
acceptable?


Maximilian


--
--
Mit freundlichen Grüßen,
Maximilian Meister
Systems Management Department

SUSE LINUX Products GmbH
Maxfeldstr. 5
D-90409 Nuremberg, Germany

http://www.suse.com

GF: Jeff Hawn, Jennifer Guild, Felix Imendoerffer, HRB 21284 (AG Nuremberg)

From 1a82e2e6dfd888d85451835e14b46f6576a54b6a Mon Sep 17 00:00:00 2001
From: Maximilian Meister mmeis...@suse.de
Date: Tue, 4 Feb 2014 10:40:19 +0100
Subject: [PATCH 1/4] removing obsolete code related to PLACEHOLDER_PASSWORD

---
 .../src/com/redhat/rhn/frontend/action/user/UserActionHelper.java | 3 ---
 .../src/com/redhat/rhn/frontend/action/user/UserEditSetupAction.java  | 4 
 2 files changed, 7 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/user/UserActionHelper.java b/java/code/src/com/redhat/rhn/frontend/action/user/UserActionHelper.java
index 3ff18e6..1510aa1 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/user/UserActionHelper.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/user/UserActionHelper.java
@@ -32,9 +32,6 @@ public class UserActionHelper {
 private UserActionHelper() {
 }
 
-/** placeholder string, package protected; so we don't transmit
- * the actual pw but the form doesn't look empty */
-static final String PLACEHOLDER_PASSWORD = **;
 public static final String DESIRED_PASS = desiredpassword;
 public static final String DESIRED_PASS_CONFIRM = desiredpasswordConfirm;
 
diff --git a/java/code/src/com/redhat/rhn/frontend/action/user/UserEditSetupAction.java b/java/code/src/com/redhat/rhn/frontend/action/user/UserEditSetupAction.java
index f56df31..63f3bf5 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/user/UserEditSetupAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/user/UserEditSetupAction.java
@@ -77,10 +77,6 @@ public class UserEditSetupAction extends RhnAction {
 form.set(lastName, targetUser.getLastName());
 form.set(title, targetUser.getTitle());
 form.set(prefix, targetUser.getPrefix());
-form.set(UserActionHelper.DESIRED_PASS,
-UserActionHelper.PLACEHOLDER_PASSWORD);
-form.set(UserActionHelper.DESIRED_PASS_CONFIRM,
-UserActionHelper.PLACEHOLDER_PASSWORD);
 request.setAttribute(user, targetUser);
 request.setAttribute(mailableAddress, targetUser.getEmail());
 
-- 
1.8.4.5

From 06fffdff67cd791456bc5da1b3946b3bda1457ee Mon Sep 17 00:00:00 2001
From: Maximilian Meister mmeis...@suse.de
Date: Tue, 4 Feb 2014 10:41:13 +0100
Subject: [PATCH 2/4] perform password validation within the java class to
 accept an empty password as no change

---
 .../frontend/action/user/UserEditActionHelper.java | 26 ++
 .../action/user/validation/userDetailsForm.xsd | 12 --
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java b/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
index 3056834..b6660c4 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/user/UserEditActionHelper.java
@@ -14,8 +14,11 @@
  */
 package com.redhat.rhn.frontend.action.user;
 
+import java.util.regex.Pattern;
+
 import com.redhat.rhn.common.conf.Config;
 import com.redhat.rhn.common.conf.ConfigDefaults;
+import com.redhat.rhn.common.conf.UserDefaults;
 import com.redhat.rhn.domain.role.RoleFactory;
 import com.redhat.rhn.domain.user.User;
 import com.redhat.rhn.frontend.struts.RhnAction;
@@ -53,11 +56,26 @@ public abstract class UserEditActionHelper extends RhnAction {
 new ActionMessage(error.password_mismatch));
 }
 
-//Make sure password is not the placeholder
-if (!UserActionHelper.PLACEHOLDER_PASSWORD.equals(
-

Re: [Spacewalk-devel] [PATCH] Adding a password placeholder check when editing a user.

2014-02-04 Thread Michael Mraka 
Maximilian Meister wrote:
% Hi Michael,
% 
% i changed the patch. So no additional javascript.
% 
% * I replaced the struts tag with the standard html input tag to use
% the placeholder attribute
% * Some changes to the logic in
% spacewalk-pwstrength-handler.js:updateTickIcon()
% * Johannes Renner helped me with the Java code changes.

Hi Maximilian,

thanks for the patch update. I've applied to master.
 
% The question is now in UserEditActionHelper:62 we use more or less
% the same code for validation as in UpdateUserCommand:132
% As this is a small redundancy in code, I wanted to ask if it would
% make sense to put that code into
% a public function accessible by both classes, and where this
% function should reside.
% Do you think it is worth the extra work, or is the solution in the
% patch acceptable?

I see. Yes, in a perfect world we should, of course :), somehow reuse the
validation code from UpdateUserCommand in UserEditActionHelper but I'm not
sure whether it's worth the effort.


% Maximilian


Regards,

--
Michael Mráka
Satellite Engineering, Red Hat

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] Adding a password placeholder check when editing a user.

2014-01-29 Thread Michael Mraka 
Maximilian Meister wrote:
% Hi,
% 
% after the password strength meter went through, I have another enhancement
% related to the password field.
% On the edit user page, there are placeholders in the password fields.
% The placeholders are plain *'s, so if I add some characters after
% the placeholder
% like [**newchars] my new password will contain the placeholder
% instead
% of my expectation [oldpassnewchars].
% That could lead to locking out of a user.
% 
% This patch makes sure that you can't lock yourself out accidentally
% like this.

Hello Maximilian,

I think there's an easier way to do it in a plain html without new
javascript file. If you replace 

input type=password name=desiredpassword maxlength=32 value=** 
class=form-control

with

input type=password name=desiredpassword maxlength=32 
placeholder=•• class=form-control

(similar to e.g. search field on the page) there will be greyed out dots
and user have to type whole password and can't submit placeholder string
anymore. ('•' is unicode BULLET char U+2022.) Well, there's one more
step needed - UserEditActionHelper class have to be updated to accept empty
password as no change in password.

What do you think about this?


Regards,

--
Michael Mráka
Satellite Engineering, Red Hat

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] Adding a password placeholder check when editing a user.

2014-01-29 Thread Maximilian Meister 

On 01/29/2014 10:35 AM, Michael Mraka wrote:


Hello Maximilian,

I think there's an easier way to do it in a plain html without new
javascript file. If you replace

 input type=password name=desiredpassword maxlength=32 value=** 
class=form-control

with

 input type=password name=desiredpassword maxlength=32 placeholder=•• 
class=form-control

(similar to e.g. search field on the page) there will be greyed out dots
and user have to type whole password and can't submit placeholder string
anymore. ('•' is unicode BULLET char U+2022.) Well, there's one more
step needed - UserEditActionHelper class have to be updated to accept empty
password as no change in password.

What do you think about this?



Hi Michael,

I have tried this in my first attempt, but the html:password struts tag 
doesn't accept the attribute placeholder=**.


org.apache.jasper.JasperException: 
/WEB-INF/pages/common/fragments/user/edit_user_table_rows.jspf (line: 
51, column: 12) Attribute placeholder invalid for tag password according 
to TLD


What do you suggest?


--
--
Mit freundlichen Grüßen,
Maximilian Meister
Systems Management Department

SUSE LINUX Products GmbH
Maxfeldstr. 5
D-90409 Nuremberg, Germany

http://www.suse.com

GF: Jeff Hawn, Jennifer Guild, Felix Imendoerffer, HRB 21284 (AG Nuremberg)

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] Adding a password placeholder check when editing a user.

2014-01-29 Thread Maximilian Meister 

On 01/29/2014 01:19 PM, Maximilian Meister wrote:


Hi Michael,

I have tried this in my first attempt, but the html:password struts 
tag doesn't accept the attribute placeholder=**.


to reformulate my statement a bit, the html:password struts tag doesn't 
know any placeholder attribute.


See: 
http://struts.apache.org/release/1.3.x/struts-taglib/tlddoc/html/password.html




org.apache.jasper.JasperException: 
/WEB-INF/pages/common/fragments/user/edit_user_table_rows.jspf (line: 
51, column: 12) Attribute placeholder invalid for tag password 
according to TLD


What do you suggest?





--
--
Mit freundlichen Grüßen,
Maximilian Meister
Systems Management Department

SUSE LINUX Products GmbH
Maxfeldstr. 5
D-90409 Nuremberg, Germany

http://www.suse.com

GF: Jeff Hawn, Jennifer Guild, Felix Imendoerffer, HRB 21284 (AG Nuremberg)

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] Adding a password placeholder check when editing a user.

2014-01-29 Thread Michael Mraka 
Maximilian Meister wrote:
% On 01/29/2014 01:19 PM, Maximilian Meister wrote:
% 
% Hi Michael,
% 
% I have tried this in my first attempt, but the html:password
% struts tag doesn't accept the attribute placeholder=**.
% 
% to reformulate my statement a bit, the html:password struts tag
% doesn't know any placeholder attribute.
...

I see :(, I was not aware of it. Anyway I'd still prefer not to use javascript
if not necessary. So in this particular case I'd replace html:password
with direct html input tag:

input type=password name=desiredpassword class=form-control 
size=15 maxlength=${passwordLength} 
placeholder=bull;bull;bull;bull;bull;bull;/


Regards,

--
Michael Mráka
Satellite Engineering, Red Hat

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel