Re: [Spacewalk-devel] [PATCH] Adding a password placeholder check when editing a user.
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.
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.
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.
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.
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.
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