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="••••••"/>
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="••••••"/>
<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="••••••"/>
<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