From 4ee7ffe3bc28d9169c00d2a52d30242ba57134fb Mon Sep 17 00:00:00 2001 From: Manuel Rego Casasnovas Date: Wed, 13 Jul 2011 14:08:08 +0200 Subject: [PATCH] Refactorized PasswordUtil and used JavaScript to default password warnings in "Change Password" page. * Moved JavaScript function for default password warnings to a .js file. * Used this function from PasswordUtil (which is used from UserCRUDController and PasswordController). FEA: ItEr75S07UserSettings --- .../business/users/daos/UserDAO.java | 1 + .../navalplanner/web/users/IUserModel.java | 3 -- .../navalplanner/web/users/PasswordUtil.java | 46 +++++++++++++------ .../web/users/UserCRUDController.java | 24 +--------- .../org/navalplanner/web/users/UserModel.java | 12 ++--- .../web/users/bootstrap/MandatoryUser.java | 3 +- .../users/settings/PasswordController.java | 3 +- .../web/users/settings/PasswordModel.java | 15 +++--- .../main/webapp/js/defaultPasswordWarnings.js | 44 ++++++++++++++++++ .../main/webapp/settings/changePassword.zul | 5 +- .../src/main/webapp/users/users.zul | 32 +------------ 11 files changed, 99 insertions(+), 89 deletions(-) create mode 100644 navalplanner-webapp/src/main/webapp/js/defaultPasswordWarnings.js diff --git a/navalplanner-business/src/main/java/org/navalplanner/business/users/daos/UserDAO.java b/navalplanner-business/src/main/java/org/navalplanner/business/users/daos/UserDAO.java index 50d0296f3..51b12515f 100644 --- a/navalplanner-business/src/main/java/org/navalplanner/business/users/daos/UserDAO.java +++ b/navalplanner-business/src/main/java/org/navalplanner/business/users/daos/UserDAO.java @@ -46,6 +46,7 @@ public class UserDAO extends GenericDAOHibernate implements IUserDAO { @Override + @Transactional(readOnly = true) public User findByLoginName(String loginName) throws InstanceNotFoundException { diff --git a/navalplanner-webapp/src/main/java/org/navalplanner/web/users/IUserModel.java b/navalplanner-webapp/src/main/java/org/navalplanner/web/users/IUserModel.java index 8958dcbe1..94d888e4b 100644 --- a/navalplanner-webapp/src/main/java/org/navalplanner/web/users/IUserModel.java +++ b/navalplanner-webapp/src/main/java/org/navalplanner/web/users/IUserModel.java @@ -28,7 +28,6 @@ import org.navalplanner.business.common.exceptions.ValidationException; import org.navalplanner.business.users.entities.Profile; import org.navalplanner.business.users.entities.User; import org.navalplanner.business.users.entities.UserRole; -import org.navalplanner.web.users.bootstrap.MandatoryUser; /** * Model for UI operations related to {@link User} @@ -124,8 +123,6 @@ public interface IUserModel { String getClearNewPassword(); - boolean hasChangedDefaultPasswordOrDisabled(MandatoryUser admin); - void confirmRemove(User user) throws InstanceNotFoundException; } diff --git a/navalplanner-webapp/src/main/java/org/navalplanner/web/users/PasswordUtil.java b/navalplanner-webapp/src/main/java/org/navalplanner/web/users/PasswordUtil.java index a13adb1aa..d1d6f39b4 100644 --- a/navalplanner-webapp/src/main/java/org/navalplanner/web/users/PasswordUtil.java +++ b/navalplanner-webapp/src/main/java/org/navalplanner/web/users/PasswordUtil.java @@ -22,6 +22,7 @@ package org.navalplanner.web.users; import org.navalplanner.business.common.Registry; import org.navalplanner.business.users.entities.User; import org.navalplanner.web.users.bootstrap.MandatoryUser; +import org.zkoss.zk.ui.util.Clients; /** * A class which is used to encapsulate some common behaviour of passwords. @@ -31,35 +32,35 @@ import org.navalplanner.web.users.bootstrap.MandatoryUser; */ public class PasswordUtil { - private String clearNewPassword; - - public void checkIfChangeDefaultPasswd(User user) { + public static void checkIfChangeDefaultPasswd(User user, + String clearPassword) { if (user.getLoginName().equalsIgnoreCase( MandatoryUser.ADMIN.getLoginName())) { - checkIfChangeDefaultPasswd(MandatoryUser.ADMIN); + checkIfChangeDefaultPasswd(MandatoryUser.ADMIN, clearPassword); return; } if (user.getLoginName().equalsIgnoreCase( MandatoryUser.USER.getLoginName())) { - checkIfChangeDefaultPasswd(MandatoryUser.USER); + checkIfChangeDefaultPasswd(MandatoryUser.USER, clearPassword); return; } if (user.getLoginName().equalsIgnoreCase( MandatoryUser.WSREADER.getLoginName())) { - checkIfChangeDefaultPasswd(MandatoryUser.WSREADER); + checkIfChangeDefaultPasswd(MandatoryUser.WSREADER, clearPassword); return; } if (user.getLoginName().equalsIgnoreCase( MandatoryUser.WSWRITER.getLoginName())) { - checkIfChangeDefaultPasswd(MandatoryUser.WSWRITER); + checkIfChangeDefaultPasswd(MandatoryUser.WSWRITER, clearPassword); return; } } - private void checkIfChangeDefaultPasswd(MandatoryUser user) { + private static void checkIfChangeDefaultPasswd(MandatoryUser user, + String clearPassword) { boolean changedPasswd = true; - if (getClearNewPassword().isEmpty() - || getClearNewPassword().equals(user.getClearPassword())) { + if (clearPassword.isEmpty() + || clearPassword.equals(user.getClearPassword())) { changedPasswd = false; } // save the field changedDefaultAdminPassword in configuration. @@ -67,12 +68,27 @@ public class PasswordUtil { user.getLoginName(), changedPasswd); } - public void setClearNewPassword(String clearNewPassword) { - this.clearNewPassword = clearNewPassword; - } + /** + * It calls a JavaScript method called + * showOrHideDefaultPasswordWarnings defined in + * "/navalplanner-webapp/js/defaultPasswordWarnings.js" to show or hide the + * default password warnings if the user has changed the password or has + * been disabled + */ + public static void showOrHideDefaultPasswordWarnings() { + boolean adminNotDefaultPassword = MandatoryUser.ADMIN + .hasChangedDefaultPasswordOrDisabled(); + boolean userNotDefaultPassword = MandatoryUser.USER + .hasChangedDefaultPasswordOrDisabled(); + boolean wsreaderNotDefaultPassword = MandatoryUser.WSREADER + .hasChangedDefaultPasswordOrDisabled(); + boolean wswriterNotDefaultPassword = MandatoryUser.WSWRITER + .hasChangedDefaultPasswordOrDisabled(); - public String getClearNewPassword() { - return clearNewPassword; + Clients.evalJavaScript("showOrHideDefaultPasswordWarnings(" + + adminNotDefaultPassword + ", " + userNotDefaultPassword + + ", " + wsreaderNotDefaultPassword + ", " + + wswriterNotDefaultPassword + ");"); } } diff --git a/navalplanner-webapp/src/main/java/org/navalplanner/web/users/UserCRUDController.java b/navalplanner-webapp/src/main/java/org/navalplanner/web/users/UserCRUDController.java index 791c2ccf3..d77468e21 100644 --- a/navalplanner-webapp/src/main/java/org/navalplanner/web/users/UserCRUDController.java +++ b/navalplanner-webapp/src/main/java/org/navalplanner/web/users/UserCRUDController.java @@ -40,10 +40,8 @@ import org.navalplanner.web.common.Util; import org.navalplanner.web.common.components.Autocomplete; import org.navalplanner.web.common.entrypoints.EntryPointsHandler; import org.navalplanner.web.common.entrypoints.IURLHandlerRegistry; -import org.navalplanner.web.users.bootstrap.MandatoryUser; import org.zkoss.zk.ui.Component; import org.zkoss.zk.ui.WrongValueException; -import org.zkoss.zk.ui.util.Clients; import org.zkoss.zk.ui.util.GenericForwardComposer; import org.zkoss.zul.Combobox; import org.zkoss.zul.Comboitem; @@ -177,7 +175,7 @@ public class UserCRUDController extends GenericForwardComposer implements userModel.confirmSave(); messagesForUser.showMessage(Level.INFO, _("User saved")); - showOrHideDefaultPasswordWarnings(); + PasswordUtil.showOrHideDefaultPasswordWarnings(); return true; } catch (ValidationException e) { messagesForUser.showInvalidValues(e); @@ -185,26 +183,6 @@ public class UserCRUDController extends GenericForwardComposer implements return false; } - /** - * It calls a JavaScript method to show or hide the default password - * warnings if the user has changed the password or has been disabled - */ - private void showOrHideDefaultPasswordWarnings() { - boolean adminNotDefaultPassword = userModel - .hasChangedDefaultPasswordOrDisabled(MandatoryUser.ADMIN); - boolean userNotDefaultPassword = userModel - .hasChangedDefaultPasswordOrDisabled(MandatoryUser.USER); - boolean wsreaderNotDefaultPassword = userModel - .hasChangedDefaultPasswordOrDisabled(MandatoryUser.WSREADER); - boolean wswriterNotDefaultPassword = userModel - .hasChangedDefaultPasswordOrDisabled(MandatoryUser.WSWRITER); - - Clients.evalJavaScript("showOrHideDefaultPasswordWarnings(" - + adminNotDefaultPassword + ", " + userNotDefaultPassword - + ", " + wsreaderNotDefaultPassword + ", " - + wswriterNotDefaultPassword + ");"); - } - public User getUser() { return userModel.getUser(); } diff --git a/navalplanner-webapp/src/main/java/org/navalplanner/web/users/UserModel.java b/navalplanner-webapp/src/main/java/org/navalplanner/web/users/UserModel.java index 8b6ea8466..78256a819 100644 --- a/navalplanner-webapp/src/main/java/org/navalplanner/web/users/UserModel.java +++ b/navalplanner-webapp/src/main/java/org/navalplanner/web/users/UserModel.java @@ -35,7 +35,6 @@ import org.navalplanner.business.users.entities.Profile; import org.navalplanner.business.users.entities.User; import org.navalplanner.business.users.entities.UserRole; import org.navalplanner.web.common.concurrentdetection.OnConcurrentModification; -import org.navalplanner.web.users.bootstrap.MandatoryUser; import org.navalplanner.web.users.services.IDBPasswordEncoderService; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.config.BeanDefinition; @@ -51,7 +50,7 @@ import org.springframework.transaction.annotation.Transactional; @Service @Scope(BeanDefinition.SCOPE_PROTOTYPE) @OnConcurrentModification(goToPage = "/users/users.zul") -public class UserModel extends PasswordUtil implements IUserModel { +public class UserModel implements IUserModel { @Autowired private IUserDAO userDAO; @@ -99,7 +98,8 @@ public class UserModel extends PasswordUtil implements IUserModel { * changedDefaultAdminPassword. */ if (Configuration.isDefaultPasswordsControl()) { - checkIfChangeDefaultPasswd(user); + PasswordUtil.checkIfChangeDefaultPasswd(user, + getClearNewPassword()); } user.setPassword(dbPasswordEncoderService.encodePassword( @@ -219,12 +219,6 @@ public class UserModel extends PasswordUtil implements IUserModel { return clearNewPassword; } - @Override - @Transactional(readOnly = true) - public boolean hasChangedDefaultPasswordOrDisabled(MandatoryUser user) { - return user.hasChangedDefaultPasswordOrDisabled(); - } - @Override @Transactional public void confirmRemove(User user) diff --git a/navalplanner-webapp/src/main/java/org/navalplanner/web/users/bootstrap/MandatoryUser.java b/navalplanner-webapp/src/main/java/org/navalplanner/web/users/bootstrap/MandatoryUser.java index cc22a2a2d..41c58be23 100644 --- a/navalplanner-webapp/src/main/java/org/navalplanner/web/users/bootstrap/MandatoryUser.java +++ b/navalplanner-webapp/src/main/java/org/navalplanner/web/users/bootstrap/MandatoryUser.java @@ -85,7 +85,8 @@ public enum MandatoryUser { } private static Configuration getConfiguration() { - return Registry.getConfigurationDAO().getConfiguration(); + return Registry.getConfigurationDAO() + .getConfigurationWithReadOnlyTransaction(); } private Set initialRoles; diff --git a/navalplanner-webapp/src/main/java/org/navalplanner/web/users/settings/PasswordController.java b/navalplanner-webapp/src/main/java/org/navalplanner/web/users/settings/PasswordController.java index de9dad0a9..f1a06af21 100644 --- a/navalplanner-webapp/src/main/java/org/navalplanner/web/users/settings/PasswordController.java +++ b/navalplanner-webapp/src/main/java/org/navalplanner/web/users/settings/PasswordController.java @@ -27,6 +27,7 @@ import org.navalplanner.web.common.ConstraintChecker; import org.navalplanner.web.common.IMessagesForUser; import org.navalplanner.web.common.Level; import org.navalplanner.web.common.MessagesForUser; +import org.navalplanner.web.users.PasswordUtil; import org.zkoss.zk.ui.Component; import org.zkoss.zk.ui.WrongValueException; import org.zkoss.zk.ui.util.GenericForwardComposer; @@ -64,7 +65,7 @@ public class PasswordController extends GenericForwardComposer { try { passwordModel.confirmSave(); messages.showMessage(Level.INFO, _("Password saved")); - + PasswordUtil.showOrHideDefaultPasswordWarnings(); } catch (ValidationException e) { messages.showInvalidValues(e); } diff --git a/navalplanner-webapp/src/main/java/org/navalplanner/web/users/settings/PasswordModel.java b/navalplanner-webapp/src/main/java/org/navalplanner/web/users/settings/PasswordModel.java index f156efd39..35c5b478f 100644 --- a/navalplanner-webapp/src/main/java/org/navalplanner/web/users/settings/PasswordModel.java +++ b/navalplanner-webapp/src/main/java/org/navalplanner/web/users/settings/PasswordModel.java @@ -45,7 +45,7 @@ import org.springframework.transaction.annotation.Transactional; @Service @Scope(BeanDefinition.SCOPE_PROTOTYPE) @OnConcurrentModification(goToPage = "/settings/changePassword.zul") -public class PasswordModel extends PasswordUtil implements IPasswordModel{ +public class PasswordModel implements IPasswordModel { @Autowired private IUserDAO userDAO; @@ -55,11 +55,13 @@ public class PasswordModel extends PasswordUtil implements IPasswordModel{ @Autowired private IDBPasswordEncoderService dbPasswordEncoderService; + private String clearPassword; + @Override @Transactional public void confirmSave() throws ValidationException { try { - if (getClearNewPassword() != null) { + if (clearPassword != null) { /* * it ckecks if the user password who have admin role has @@ -67,11 +69,12 @@ public class PasswordModel extends PasswordUtil implements IPasswordModel{ * changedDefaultAdminPassword. */ if (Configuration.isDefaultPasswordsControl()) { - checkIfChangeDefaultPasswd(user); + PasswordUtil + .checkIfChangeDefaultPasswd(user, clearPassword); } user.setPassword(dbPasswordEncoderService.encodePassword( - getClearNewPassword(), user.getLoginName())); + clearPassword, user.getLoginName())); } } catch (IllegalArgumentException e) { } @@ -85,9 +88,9 @@ public class PasswordModel extends PasswordUtil implements IPasswordModel{ // user.getLoginName must exist to do that, and we're // not sure at this point if (password != "") { - setClearNewPassword(password); + clearPassword = password; } else { - setClearNewPassword(null); + clearPassword = null; } } diff --git a/navalplanner-webapp/src/main/webapp/js/defaultPasswordWarnings.js b/navalplanner-webapp/src/main/webapp/js/defaultPasswordWarnings.js new file mode 100644 index 000000000..f35189bb6 --- /dev/null +++ b/navalplanner-webapp/src/main/webapp/js/defaultPasswordWarnings.js @@ -0,0 +1,44 @@ +/* + * This file is part of NavalPlan + * + * Copyright (C) 2011 Igalia, S.L. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +function showOrHideDefaultPasswordWarnings(adminNotDefaultPassword, + userNotDefaultPassword, wsreaderNotDefaultPassword, + wswriterNotDefaultPassword) { + + setDisplayNoneOrInline(document.getElementById("warningDefaultPasswdadmin"), + adminNotDefaultPassword); + + var otherDefaultPassword = adminNotDefaultPassword && + (!userNotDefaultPassword || !wsreaderNotDefaultPassword || !wswriterNotDefaultPassword); + setDisplayNoneOrInline(document.getElementById("warningDefaultPasswdOthers"), + !otherDefaultPassword); + + if (otherDefaultPassword) { + setDisplayNoneOrInline(document.getElementById("warningDefaultPasswduser"), + userNotDefaultPassword); + setDisplayNoneOrInline(document.getElementById("warningDefaultPasswdwsreader"), + wsreaderNotDefaultPassword); + setDisplayNoneOrInline(document.getElementById("warningDefaultPasswdwswriter"), + wswriterNotDefaultPassword); + } +} + +function setDisplayNoneOrInline(component, boolean) { + component.style["display"] = boolean ? "none" : "inline"; +} diff --git a/navalplanner-webapp/src/main/webapp/settings/changePassword.zul b/navalplanner-webapp/src/main/webapp/settings/changePassword.zul index bca9ae5fc..3de4f4f9b 100644 --- a/navalplanner-webapp/src/main/webapp/settings/changePassword.zul +++ b/navalplanner-webapp/src/main/webapp/settings/changePassword.zul @@ -27,7 +27,10 @@ - + + + + diff --git a/navalplanner-webapp/src/main/webapp/users/users.zul b/navalplanner-webapp/src/main/webapp/users/users.zul index 52f8475b2..b3acd3267 100644 --- a/navalplanner-webapp/src/main/webapp/users/users.zul +++ b/navalplanner-webapp/src/main/webapp/users/users.zul @@ -28,37 +28,9 @@ - + - +