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(
-                form.get(UserActionHelper.DESIRED_PASS))) {
+        //Make sure password is not empty
+        if (!pw.isEmpty()) {
+            // Validate the password
+            if (pw.length() < UserDefaults.get().getMinPasswordLength()) {
+                errors.add(ActionMessages.GLOBAL_MESSAGE,
+                        new ActionMessage("error.minpassword",
+                                UserDefaults.get().getMinPasswordLength()));
+            }
+            if (Pattern.compile("[\\t\\n]").matcher(pw).find()) {
+                errors.add(ActionMessages.GLOBAL_MESSAGE,
+                        new ActionMessage("error.invalidpasswordcharacters"));
+            }
+            if (pw.length() > UserDefaults.get().getMaxPasswordLength()) {
+                errors.add(ActionMessages.GLOBAL_MESSAGE,
+                        new ActionMessage("error.maxpassword",
+                                targetUser.getPassword()));
+            }
+
+            //Set the password only if there are no errors at all
             if (errors.isEmpty()) {
-                //Set the password only if there are no errors at all
                 targetUser.setPassword(pw);
             }
         }
diff --git a/java/code/src/com/redhat/rhn/frontend/action/user/validation/userDetailsForm.xsd b/java/code/src/com/redhat/rhn/frontend/action/user/validation/userDetailsForm.xsd
index eb56b63..c82e50a 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/user/validation/userDetailsForm.xsd
+++ b/java/code/src/com/redhat/rhn/frontend/action/user/validation/userDetailsForm.xsd
@@ -2,18 +2,6 @@
 
 <schema targetNamespace="http://rhn.redhat.com";
 	xmlns="http://www.w3.org/1999/XMLSchema"; xmlns:rhn="http://rhn.redhat.com";>
-	<attribute name="desiredpassword">
-        <simpleType baseType="string">
-            <minLength value="5" />
-            <maxLength value="64" />
-        </simpleType>
-    </attribute>
-    <attribute name="desiredpasswordConfirm">
-        <simpleType baseType="string">
-            <minLength value="5" />
-            <maxLength value="64" />
-        </simpleType>
-    </attribute>
     <attribute name="firstNames">
 		<simpleType baseType="string">
 			<minLength value="1" />
-- 
1.8.4.5

>From 24b0cf83cef5ba3ec799447d0d1fa27441362c56 Mon Sep 17 00:00:00 2001
From: Maximilian Meister <mmeis...@suse.de>
Date: Tue, 4 Feb 2014 10:41:45 +0100
Subject: [PATCH 3/4] swapping struts tag with basic input html tag to allow
 the use of the placeholder attribute

---
 .../WEB-INF/pages/common/fragments/user/edit_user_table_rows.jspf     | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/java/code/webapp/WEB-INF/pages/common/fragments/user/edit_user_table_rows.jspf b/java/code/webapp/WEB-INF/pages/common/fragments/user/edit_user_table_rows.jspf
index e00b70b..c8ce65e 100644
--- a/java/code/webapp/WEB-INF/pages/common/fragments/user/edit_user_table_rows.jspf
+++ b/java/code/webapp/WEB-INF/pages/common/fragments/user/edit_user_table_rows.jspf
@@ -48,7 +48,7 @@
     <label class="col-lg-3 control-label"><bean:message key="password.displayname"/></label>
     <div class="col-lg-6">
         <div id="desiredpassword-input-group" class="input-group">
-            <html:password property="desiredpassword" styleClass="form-control" maxlength="${passwordLength}"/>
+            <input type="password" name="desiredpassword" class="form-control" size="15" maxlength="${passwordLength}" placeholder="&bull;&bull;&bull;&bull;&bull;&bull;"/>
             <span class="input-group-addon">
                 <i class="fa fa-check-circle text-success fa-1-5x" id="desiredtick"></i>
             </span>
@@ -60,7 +60,7 @@
     <label class="col-lg-3 control-label"><bean:message key="confirmpass.displayname"/></label>
     <div class="col-lg-6">
         <div class="input-group">
-            <html:password property="desiredpasswordConfirm" styleClass="form-control" onkeyup="updateTickIcon()" maxlength="${passwordLength}" styleId="confirmpass"/>
+            <input id="confirmpass" type="password" name="desiredpasswordConfirm" class="form-control" size="15" maxlength="${passwordLength}" onkeyup="updateTickIcon()" placeholder="&bull;&bull;&bull;&bull;&bull;&bull;"/>
             <span class="input-group-addon">
                 <i class="fa fa-check-circle text-success fa-1-5x" id="confirmtick"></i>
             </span>
-- 
1.8.4.5

>From d6b7844e26408a3e0bedcb218b5c1cc9e9ccb496 Mon Sep 17 00:00:00 2001
From: Maximilian Meister <mmeis...@suse.de>
Date: Tue, 4 Feb 2014 10:42:50 +0100
Subject: [PATCH 4/4] changes in the logic to update the tick icon.
 differentiating between edit user form and create user form.

---
 .../javascript/spacewalk-pwstrength-handler.js     | 35 +++++++++++++++-------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/web/html/javascript/spacewalk-pwstrength-handler.js b/web/html/javascript/spacewalk-pwstrength-handler.js
index 220e107..1b42f24 100644
--- a/web/html/javascript/spacewalk-pwstrength-handler.js
+++ b/web/html/javascript/spacewalk-pwstrength-handler.js
@@ -8,7 +8,6 @@ function setupPasswordStrengthMeter() {
         onKeyUp: function (evt) {
             $('input[name="desiredpassword"]').popover('show');
             //when there are no errors the popover disappears
-            var desiredpassval = $.trim($('input[name="desiredpassword"]').val());
             if ($('ul.error-list').is(':empty')) {
                 $('input[name="desiredpassword"]').popover('destroy');
             }
@@ -57,7 +56,9 @@ function setupPasswordStrengthMeter() {
 // check if confirm password input field matches with password input field
 // swap icons in the input-group-addon
 function updateTickIcon() {
-    var desiredpassval = $.trim($('input[name="desiredpassword"]').val());
+    var desiredpassVal = $.trim($('input[name="desiredpassword"]').val());
+    var desiredpassConfirmVal = $.trim($('#confirmpass').val());
+    var placeholderAttr = $('input[name="desiredpassword"]').attr('placeholder');
     function success(element) {
         element.removeClass("fa-times-circle text-danger");
         element.addClass("fa-check-circle text-success");
@@ -66,17 +67,31 @@ function updateTickIcon() {
         element.removeClass("fa-check-circle text-success");
         element.addClass("fa-times-circle text-danger");
     }
-    if (desiredpassval.length < 5 && $("#desiredtick").hasClass("text-success")) {
-        danger($("#desiredtick"));
-    }
-    else if (desiredpassval.length >= 5 && $("#desiredtick").hasClass("text-danger")) {
+
+    // on the edit user page
+    if ((typeof placeholderAttr !== 'undefined' && placeholderAttr !== false)) {
+        // icons are green
         success($("#desiredtick"));
-    }
-    if ($("#confirmpass").val() == desiredpassval && desiredpassval.length >= 5) {
         success($("#confirmtick"));
+        if (desiredpassVal.length > 0 && desiredpassVal.length < 5) {
+            danger($("#desiredtick"));
+            danger($("#confirmtick")); 
+        }
+        else if (desiredpassVal != desiredpassConfirmVal) {
+            danger($("#confirmtick"));
+        }
     }
-    else if ($("#confirmtick").hasClass("text-success")) {
-        danger($("#confirmtick"));
+    // on create user pages
+    else {
+        // icons are red
+        danger($("#desiredtick"));
+        danger($("#confirmtick")); 
+        if (desiredpassVal.length >= 5) {
+            success($("#desiredtick"));
+        }
+        if (desiredpassVal == desiredpassConfirmVal && desiredpassVal.length >= 5) {
+            success($("#confirmtick"));
+        }
     }
 }
 
-- 
1.8.4.5
_______________________________________________
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Reply via email to