Yair Zaslavsky has uploaded a new change for review. Change subject: 5. core: Support custom message to appear in case of expired password ......................................................................
5. core: Support custom message to appear in case of expired password The bug description stays that in case of failed login, the user may get a URL to change the passord, or a custom message. In order to support that, the following changes were introduced: a. manage-domains code -changePasswordUrl was changed to -changePasswordMsg. b. The input is done in interactive mode. c. The read value is encoded (as may contain characters like space, : and ,) d. The manage-domains script was changed accordingly e. The vdc_options relevant entry was changed (so was the ConfigValues relevant enum literal). f. ProsivinalAuthenticationResult reads the entry from the configuration and decodes it. g. A proper VDC BLL message is used depending on whether the content is URL or not (a new VdcBllMessage was introduced). Bug-Url: https://bugzilla.redhat.com/1053601 Change-Id: I9f656f38f7f48e169ed01ad084d7f424e5c749f3 Signed-off-by: Yair Zaslavsky <yzasl...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/authentication/provisional/ProvisionalAuthenticationResult.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties M backend/manager/tools/src/main/java/org/ovirt/engine/core/domains/ConfigurationProvider.java M backend/manager/tools/src/main/java/org/ovirt/engine/core/domains/ManageDomains.java M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java M packaging/bin/engine-manage-domains.sh M packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql 9 files changed, 92 insertions(+), 55 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/95/23895/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/authentication/provisional/ProvisionalAuthenticationResult.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/authentication/provisional/ProvisionalAuthenticationResult.java index de476ce..d663bf6 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/authentication/provisional/ProvisionalAuthenticationResult.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/authentication/provisional/ProvisionalAuthenticationResult.java @@ -1,5 +1,7 @@ package org.ovirt.engine.core.authentication.provisional; +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; @@ -11,27 +13,37 @@ import org.ovirt.engine.core.common.config.Config; import org.ovirt.engine.core.common.config.ConfigValues; import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.utils.log.Log; +import org.ovirt.engine.core.utils.log.LogFactory; public class ProvisionalAuthenticationResult extends AuthenticationResult { - private volatile static Map<String, String> passwordChangeUrlsPerDomain = null; + private volatile static Map<String, String> passwordChangeMsgPerDomain = null; private String domain; private UserAuthenticationResult authResult; + private static Log log = LogFactory.getLog(ProvisionalAuthenticationResult.class); + public ProvisionalAuthenticationResult(String domain, UserAuthenticationResult userAuthResult) { this.authResult = userAuthResult; - if (passwordChangeUrlsPerDomain == null) { + if (passwordChangeMsgPerDomain == null) { synchronized (ProvisionalAuthenticationResult.class) { - if (passwordChangeUrlsPerDomain == null) { - passwordChangeUrlsPerDomain = new HashMap<String, String>(); - String changePasswordUrl = Config.<String> getValue(ConfigValues.ChangePasswordUrl); + if (passwordChangeMsgPerDomain == null) { + passwordChangeMsgPerDomain = new HashMap<String, String>(); + String changePasswordUrl = Config.<String> getValue(ConfigValues.ChangePasswordMsg); String[] pairs = changePasswordUrl.split(","); for (String pair : pairs) { // Split the pair in such a way that if the URL contains :, it will not be split to strings String[] pairParts = pair.split(":", 2); if (pairParts.length >= 2) { - passwordChangeUrlsPerDomain.put(pairParts[0], pairParts[1]); + try { + passwordChangeMsgPerDomain.put(pairParts[0], URLDecoder.decode(pairParts[1], "UTF-8")); + } catch (UnsupportedEncodingException e) { + log.error("Eror in decoding the change password message/url. Message is: " + + e.getMessage()); + log.debug("", e); + } } } } @@ -53,10 +65,15 @@ while (it.hasNext()) { VdcBllMessages current = it.next(); if (current == VdcBllMessages.USER_PASSWORD_EXPIRED) { - String passwordChangeUrl = passwordChangeUrlsPerDomain.get(domain); - if (passwordChangeUrl != null) { - result.add(VdcBllMessages.USER_PASSWORD_EXPIRED_CHANGE_URL_PROVIDED.name()); - result.add(String.format("$URL %1$s", passwordChangeUrl)); + String passwordChangeMsg = passwordChangeMsgPerDomain.get(domain); + if (passwordChangeMsg != null) { + if (passwordChangeMsg.indexOf("http") == 0 || passwordChangeMsg.indexOf("https") == 0) { + result.add(VdcBllMessages.USER_PASSWORD_EXPIRED_CHANGE_URL_PROVIDED.name()); + result.add(String.format("$URL %1$s", passwordChangeMsg)); + } else { + result.add(VdcBllMessages.USER_PASSWORD_EXPIRED_CHANGE_MSG_PROVIDED.name()); + result.add(String.format("$MSG %1$s", passwordChangeMsg)); + } } else { result.add(current.name()); } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java index 5fad38b..6d28ba8 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java @@ -1620,7 +1620,7 @@ @TypeConverterAttribute(String.class) @DefaultValueAttribute("") - ChangePasswordUrl, + ChangePasswordMsg, Invalid; } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java index 5a006f2..8d5cbc4 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java @@ -391,6 +391,7 @@ USER_FAILED_TO_AUTHENTICATE_KERBEROS_ERROR(ErrorType.NO_AUTHENTICATION), USER_PASSWORD_EXPIRED(ErrorType.NO_AUTHENTICATION), USER_PASSWORD_EXPIRED_CHANGE_URL_PROVIDED(ErrorType.NO_AUTHENTICATION), + USER_PASSWORD_EXPIRED_CHANGE_MSG_PROVIDED(ErrorType.NO_AUTHENTICATION), USER_ACCOUNT_DISABLED(ErrorType.NO_AUTHENTICATION), USER_PERMISSION_DENIED(ErrorType.NO_AUTHENTICATION), USER_MUST_EXIST_IN_DB(ErrorType.NO_AUTHENTICATION), diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties index 0f0561e..4d0f17d 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -923,7 +923,8 @@ #Suspected (not in use?) USER_PASSWORD_EXPIRED=Cannot Login. User Password has expired, Please change your password. -USER_PASSWORD_EXPIRED_CHANGE_URL_PROVIDED=Cannot Login. User Password has expired. Use the following URL to change the password: ${URL} +USER_PASSWORD_EXPIRED_CHANGE_URL_PROVIDED=Cannot Login. User Password has expired. Use the following URL to change the password: ${URL} +USER_PASSWORD_EXPIRED_CHANGE_MSG_PROVIDED=Cannot login. User Password has expired. Detailed message: ${MSG} USER_CANNOT_LOGIN_DOMAIN_NOT_SUPPORTED=Cannot Login. The Domain provided is not configured, please contact your system administrator. VM_POOL_CANNOT_DECREASE_VMS_FROM_POOL=Cannot decrease VMs from VM-Pool. diff --git a/backend/manager/tools/src/main/java/org/ovirt/engine/core/domains/ConfigurationProvider.java b/backend/manager/tools/src/main/java/org/ovirt/engine/core/domains/ConfigurationProvider.java index f519f6c..002d6f8 100644 --- a/backend/manager/tools/src/main/java/org/ovirt/engine/core/domains/ConfigurationProvider.java +++ b/backend/manager/tools/src/main/java/org/ovirt/engine/core/domains/ConfigurationProvider.java @@ -3,7 +3,7 @@ import static org.ovirt.engine.core.common.config.ConfigValues.AdUserId; import static org.ovirt.engine.core.common.config.ConfigValues.AdUserName; import static org.ovirt.engine.core.common.config.ConfigValues.AdUserPassword; -import static org.ovirt.engine.core.common.config.ConfigValues.ChangePasswordUrl; +import static org.ovirt.engine.core.common.config.ConfigValues.ChangePasswordMsg; import static org.ovirt.engine.core.common.config.ConfigValues.DomainName; import static org.ovirt.engine.core.common.config.ConfigValues.LDAPProviderTypes; import static org.ovirt.engine.core.common.config.ConfigValues.LDAPSecurityAuthentication; @@ -44,7 +44,7 @@ configVals.put(AdUserId, adUserId); configVals.put(LDAPProviderTypes, ldapProviderTypes); configVals.put(LDAPServerPort, ldapServerPort); - configVals.put(ChangePasswordUrl, passwordChangeUrls); + configVals.put(ChangePasswordMsg, passwordChangeUrls); this.engineConfigExecutable = engineConfigExecutable; this.engineConfigProperties = engineConfigProperties; } diff --git a/backend/manager/tools/src/main/java/org/ovirt/engine/core/domains/ManageDomains.java b/backend/manager/tools/src/main/java/org/ovirt/engine/core/domains/ManageDomains.java index 3af3b9f..e6e9bd7 100644 --- a/backend/manager/tools/src/main/java/org/ovirt/engine/core/domains/ManageDomains.java +++ b/backend/manager/tools/src/main/java/org/ovirt/engine/core/domains/ManageDomains.java @@ -12,8 +12,10 @@ import java.io.FileReader; import java.io.IOException; import java.io.InputStream; +import java.io.UnsupportedEncodingException; import java.net.MalformedURLException; import java.net.URL; +import java.net.URLEncoder; import java.sql.SQLException; import java.text.SimpleDateFormat; import java.util.ArrayList; @@ -79,7 +81,7 @@ private boolean reportAllErrors; private boolean addPermissions; private boolean useDnsLookup; - private boolean changePasswordUrl; + private boolean changePasswordMsg; private final static Logger log = Logger.getLogger(ManageDomains.class); private static final String DEFAULT_LDAP_SERVER_PORT = "389"; @@ -98,7 +100,7 @@ provider, forceDelete, ldapServers, - changePasswordUrl, + changePasswordMsg, } public enum ActionType { @@ -197,8 +199,8 @@ if (parser.hasArg(Arguments.addPermissions.name())) { util.addPermissions = true; } - if (parser.hasArg(Arguments.changePasswordUrl.name())) { - util.changePasswordUrl = true; + if (parser.hasArg(Arguments.changePasswordMsg.name())) { + util.changePasswordMsg = true; } try { @@ -257,7 +259,7 @@ ldapPort = DEFAULT_LDAP_SERVER_PORT; } String changePasswordUrl = - getConfigValue(engineConfigExecutable, engineConfigProperties, ConfigValues.ChangePasswordUrl); + getConfigValue(engineConfigExecutable, engineConfigProperties, ConfigValues.ChangePasswordMsg); if (changePasswordUrl == null) { changePasswordUrl = ""; } @@ -345,24 +347,26 @@ throw new ManageDomainsResult(ManageDomainsResultEnum.INVALID_ARGUMENT_FOR_COMMAND, sb.toString()); } - protected String getChangePasswordUrl(CLIParser parser) throws ManageDomainsResult { - if (!changePasswordUrl) { + protected String getChangePasswordMsg(CLIParser parser) throws ManageDomainsResult, UnsupportedEncodingException { + if (!changePasswordMsg) { return null; } - String changePasswordUrl = parser.getArg(Arguments.changePasswordUrl.name()); - if (StringUtils.isEmpty(changePasswordUrl)) { - throw new ManageDomainsResult(ManageDomainsResultEnum.INVALID_ARGUMENT_FOR_COMMAND, - "Password change URL must not be empty"); + String changePasswordMsgStr = + readInteractively("Please enter message or URL to appear when user tries to login with an expired password: ", + false); + + if (changePasswordMsgStr.indexOf("http") == 0 || changePasswordMsgStr.indexOf("https") == 0) { + try { + URL url = new URL(changePasswordMsgStr); + log.debug("Validated that " + url + " is in correct format"); + } catch (MalformedURLException ex) { + throw new ManageDomainsResult(ManageDomainsResultEnum.INVALID_ARGUMENT_FOR_COMMAND, + "The provided value begins with a URL prefix of either http or https. However this is not a valid URL"); + } } - try { - URL url = new URL(changePasswordUrl); - log.debug("Validated that " + url + " is in correct format"); - } catch (MalformedURLException e) { - throw new ManageDomainsResult(ManageDomainsResultEnum.INVALID_ARGUMENT_FOR_COMMAND, - "The provided string for Password change URL is not a valid URL"); - } - return changePasswordUrl; + // As the message may contain characters like space, "," and ":" - it should be encoded + return URLEncoder.encode(changePasswordMsgStr, "UTF-8"); } private String getPasswordInput(CLIParser parser) throws ManageDomainsResult { @@ -379,7 +383,7 @@ throw new ManageDomainsResult(ManageDomainsResultEnum.EMPTY_PASSWORD_FILE); } } else if (parser.hasArg(Arguments.interactive.name())) { - pass = readPasswordInteractively(); + pass = readInteractively("Enter password:", true); } validatePassword(pass); @@ -393,13 +397,16 @@ } } - private String readPasswordInteractively() { - String password = null; - while (StringUtils.isBlank(password)) { - System.out.print("Enter password:"); - password = new String(System.console().readPassword()); + private String readInteractively(String prompt, boolean isPassword) { + String value = null; + while (StringUtils.isBlank(value)) { + if (isPassword) { + value = new String(System.console().readPassword(prompt)); + } else { + value = System.console().readLine(prompt); + } } - return password; + return value; } private String readPasswordFile(String passwordFile) throws FileNotFoundException, IOException { @@ -528,7 +535,12 @@ List<String> ldapServers = getLdapServers(parser, domainName); validateKdcServers(authMode, domainName); domainNameEntry.setValueForDomain(domainName, null); - String changePasswordUrlStr = getChangePasswordUrl(parser); + String changePasswordUrlStr = null; + try { + changePasswordUrlStr = getChangePasswordMsg(parser); + } catch (UnsupportedEncodingException e) { + log.error("Error in encoding the change password message. ", e); + } String currentAdUserNameEntry = configurationProvider.getConfigValue(ConfigValues.AdUserName); String currentAdUserPasswordEntry = configurationProvider.getConfigValue(ConfigValues.AdUserPassword); @@ -537,7 +549,7 @@ String currentAdUserIdEntry = configurationProvider.getConfigValue(ConfigValues.AdUserId); String currentLDAPProviderTypes = configurationProvider.getConfigValue(ConfigValues.LDAPProviderTypes); String ldapServerPort = configurationProvider.getConfigValue(ConfigValues.LDAPServerPort); - String currentChangePasswordUrl = configurationProvider.getConfigValue(ConfigValues.ChangePasswordUrl); + String currentChangePasswordUrl = configurationProvider.getConfigValue(ConfigValues.ChangePasswordMsg); DomainsConfigurationEntry adUserNameEntry = new DomainsConfigurationEntry(currentAdUserNameEntry, DOMAIN_SEPERATOR, VALUE_SEPERATOR); @@ -561,7 +573,7 @@ authModeEntry.setValueForDomain(domainName, authMode); ldapProviderTypesEntry.setValueForDomain(domainName, ldapProviderType.name()); setLdapServersPerDomain(domainName, ldapServersEntry, StringUtils.join(ldapServers, ",")); - if (changePasswordUrl) { + if (changePasswordMsg) { changePasswordUrlEntry.setValueForDomain(domainName, changePasswordUrlStr); } @@ -644,7 +656,6 @@ } public void editDomain(CLIParser parser) throws ManageDomainsResult { - System.out.println("editting domain"); String authMode; String domainName = parser.getArg(Arguments.domain.toString()).toLowerCase(); authMode = getDomainAuthMode(domainName); @@ -667,7 +678,7 @@ String currentAdUserIdEntry = configurationProvider.getConfigValue(ConfigValues.AdUserId); String currentLdapProviderTypeEntry = configurationProvider.getConfigValue(ConfigValues.LDAPProviderTypes); String ldapServerPort = configurationProvider.getConfigValue(ConfigValues.LDAPServerPort); - String currentChangePasswordUrl = configurationProvider.getConfigValue(ConfigValues.ChangePasswordUrl); + String currentChangePasswordUrl = configurationProvider.getConfigValue(ConfigValues.ChangePasswordMsg); DomainsConfigurationEntry adUserNameEntry = @@ -707,8 +718,12 @@ if (ldapProviderType != null) { ldapProviderTypeEntry.setValueForDomain(domainName, ldapProviderType.name()); } - if (parser.hasArg(Arguments.changePasswordUrl.name())) { - changePaswordUrlEntry.setValueForDomain(domainName, getChangePasswordUrl(parser)); + if (parser.hasArg(Arguments.changePasswordMsg.name())) { + try { + changePaswordUrlEntry.setValueForDomain(domainName, getChangePasswordMsg(parser)); + } catch (UnsupportedEncodingException e) { + log.error("Error in encoding the change password message. ", e); + } } testConfiguration(domainName, @@ -1002,8 +1017,8 @@ configurationProvider.setConfigValue(ConfigValues.LDAPProviderTypes, ldapProviderTypeEntry); - if (changePasswordUrl) { - configurationProvider.setConfigValue(ConfigValues.ChangePasswordUrl, changePasswordUrlEntry); + if (changePasswordMsg) { + configurationProvider.setConfigValue(ConfigValues.ChangePasswordMsg, changePasswordUrlEntry); } } @@ -1036,7 +1051,7 @@ String currentLdapServersEntry = configurationProvider.getConfigValue(ConfigValues.LdapServers); String currentAdUserId = configurationProvider.getConfigValue(ConfigValues.AdUserId); String ldapProviderType = configurationProvider.getConfigValue(ConfigValues.LDAPProviderTypes); - String changePasswordUrl = configurationProvider.getConfigValue(ConfigValues.ChangePasswordUrl); + String changePasswordUrl = configurationProvider.getConfigValue(ConfigValues.ChangePasswordMsg); DomainsConfigurationEntry adUserNameEntry = new DomainsConfigurationEntry(currentAdUserNameEntry, DOMAIN_SEPERATOR, VALUE_SEPERATOR); diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java index b48eb8e..e9e9d61 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java @@ -2506,6 +2506,9 @@ @DefaultStringValue("Cannot Login. User Password has expired. Use the following URL to change the password: ${URL}") String USER_PASSWORD_EXPIRED_CHANGE_URL_PROVIDED(); + @DefaultStringValue("Cannot login. User Password has expired. Detailed message: ${MSG}") + String USER_PASSWORD_EXPIRED_CHANGE_MSG_PROVIDED(); + @DefaultStringValue("Cannot Login. The Domain provided is not configured, please contact your system administrator.") String USER_CANNOT_LOGIN_DOMAIN_NOT_SUPPORTED(); diff --git a/packaging/bin/engine-manage-domains.sh b/packaging/bin/engine-manage-domains.sh index 8519e21..7b2fc18 100755 --- a/packaging/bin/engine-manage-domains.sh +++ b/packaging/bin/engine-manage-domains.sh @@ -11,7 +11,7 @@ cat << __EOF__ engine-manage-domains: add/edit/delete/validate/list domains USAGE: - engine-manage-domains -action=ACTION [-domain=DOMAIN -provider=PROVIDER -user=USER -passwordFile=PASSWORD_FILE -interactive -configFile=PATH -addPermissions -forceDelete -ldapServers=LDAP_SERVERS -changePasswordUrl] -report + engine-manage-domains -action=ACTION [-domain=DOMAIN -provider=PROVIDER -user=USER -passwordFile=PASSWORD_FILE -interactive -configFile=PATH -addPermissions -forceDelete -ldapServers=LDAP_SERVERS -changePasswordMsg] -report Where: ACTION action to perform (add/edit/delete/validate/list). See details below. DOMAIN (mandatory for add, edit and delete) the domain you wish to perform the action on. @@ -21,7 +21,7 @@ interactive alternative for using -passwordFile - read the password interactively. PATH (optional) use the given alternate configuration file. LDAP_SERVERS (optional) a comma delimited list of LDAP servers to be set to the domain. - CHANGE_PASSWORD_URL (optional) a URL to be returned to the user in case the password has expired. + changePasswordMsg (optional) Reads interactively a URL or a message to be returned to the user in case the password has expired. Available actions: add @@ -101,7 +101,7 @@ LdapServers= LDAPProviderTypes= LDAPServerPort= -ChangePasswordUrl= +ChangePasswordMsg= __EOF__ # diff --git a/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql b/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql index 42ae244..543a5de 100644 --- a/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql +++ b/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql @@ -615,8 +615,8 @@ select fn_db_add_config_value('VdsHaReservationIntervalInMinutes','5','general'); select fn_db_add_config_value('DefaultMaximumMigrationDowntime','0','general'); ---Password URL change -select fn_db_add_config_value('ChangePasswordUrl','','general'); +--URL or custom message to be presented upon login when the password of a user has expired. +select fn_db_add_config_value('ChangePasswordMsg','','general'); ------------------------------------------------------------------------------------ -- Update with override section -- To view, visit http://gerrit.ovirt.org/23895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9f656f38f7f48e169ed01ad084d7f424e5c749f3 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.4 Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
