From 2d639694ba4343f8b5e35212cef7f1df89e377d3 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Wed, 27 Mar 2024 14:18:39 +0100 Subject: [PATCH] XWIKI-21571: Change default value of the reset password token lifetime (#3012) Change the mechanism of the reset password token to not reset it at each verification code check, but only when the password is actually reset, and when its lifetime expired. Also provide a mandatory document initializer for the ResetPasswordRequest xclass. Change a bit more the logic: if the token lifetime configuration is set to 0 (which was the default) then we automatically remove the reset password request xobject at first wrong attempt (bad verification code): it will prevent any bruteforce attack. Then if there's a token lifetime configuration set, we don't remove the xobject when a bad attempt is performed: user might have used the wrong mail for example. But we do remove the xobject when it's expired. And if it's expired, or if the code was wrong, in both cases we immediately return an error. Move ResetPasswordIT and ForgotUserNameIT from administration-test-docker to a new module security-authentication-test-docker since it's related to security-authentication module now. --------- Co-authored-by: Manuel Leduc (cherry picked from commit b410dad402669c76b0dcec149b1e8bd334c541c4) --- .../pom.xml | 10 - .../xwiki/administration/test/ui/AllIT.java | 12 - .../XWiki/ResetPasswordRequestClass.xml | 110 --------- .../resources/ApplicationResources.properties | 6 +- .../pom.xml | 8 + .../internal/DefaultResetPasswordManager.java | 208 ++++++++++-------- .../DefaultResetPasswordRequestResponse.java | 5 + ...sswordRequestClassDocumentInitializer.java | 75 +++++++ .../resources/ApplicationResources.properties | 5 +- .../main/resources/META-INF/components.txt | 1 + .../DefaultResetPasswordManagerTest.java | 164 ++++++++++---- .../script/AuthenticationScriptService.java | 5 +- .../pom.xml | 48 ++++ .../pom.xml | 137 ++++++++++++ .../authentication/test/ui/AllIT.java | 46 ++++ .../test/ui/ForgotUsernameIT.java | 3 +- .../test/ui/ResetPasswordIT.java | 19 +- .../src/main/resources/xwiki.properties.vm | 13 +- 18 files changed, 584 insertions(+), 291 deletions(-) delete mode 100644 xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-ui/src/main/resources/XWiki/ResetPasswordRequestClass.xml create mode 100644 xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/java/org/xwiki/security/authentication/internal/ResetPasswordRequestClassDocumentInitializer.java create mode 100644 xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/pom.xml create mode 100644 xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/xwiki-platform-security-authentication-test-docker/pom.xml create mode 100644 xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/xwiki-platform-security-authentication-test-docker/src/test/it/org/xwiki/security/authentication/test/ui/AllIT.java rename xwiki-platform-core/{xwiki-platform-administration/xwiki-platform-administration-test/xwiki-platform-administration-test-docker/src/test/it/org/xwiki/administration => xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/xwiki-platform-security-authentication-test-docker/src/test/it/org/xwiki/security/authentication}/test/ui/ForgotUsernameIT.java (99%) rename xwiki-platform-core/{xwiki-platform-administration/xwiki-platform-administration-test/xwiki-platform-administration-test-docker/src/test/it/org/xwiki/administration => xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/xwiki-platform-security-authentication-test-docker/src/test/it/org/xwiki/security/authentication}/test/ui/ResetPasswordIT.java (94%) diff --git a/xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-test/xwiki-platform-administration-test-docker/pom.xml b/xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-test/xwiki-platform-administration-test-docker/pom.xml index 367ab9a3aa8e..3bf2ce8a5ea3 100644 --- a/xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-test/xwiki-platform-administration-test-docker/pom.xml +++ b/xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-test/xwiki-platform-administration-test-docker/pom.xml @@ -86,16 +86,6 @@ ${project.version} test - - com.icegreen - greenmail-junit5 - test - - - com.sun.mail - jakarta.mail - test - org.xwiki.platform xwiki-platform-flamingo-skin-test-pageobjects diff --git a/xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-test/xwiki-platform-administration-test-docker/src/test/it/org/xwiki/administration/test/ui/AllIT.java b/xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-test/xwiki-platform-administration-test-docker/src/test/it/org/xwiki/administration/test/ui/AllIT.java index 44434ce6aebd..cdaa12e2dce4 100644 --- a/xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-test/xwiki-platform-administration-test-docker/src/test/it/org/xwiki/administration/test/ui/AllIT.java +++ b/xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-test/xwiki-platform-administration-test-docker/src/test/it/org/xwiki/administration/test/ui/AllIT.java @@ -38,12 +38,6 @@ class NestedAdministrationIT extends AdministrationIT { } - @Nested - @DisplayName("Reset Password") - class NestedResetPasswordIT extends ResetPasswordIT - { - } - @Nested @DisplayName("ConfigurableClass") class NestedConfigurableClassIT extends ConfigurableClassIT @@ -56,12 +50,6 @@ class NestedUsersGroupsRightsManagementIT extends UsersGroupsRightsManagementIT { } - @Nested - @DisplayName("Forgot Username") - class NestedForgotUsernameIT extends ForgotUsernameIT - { - } - @Nested @DisplayName("XAR Import") class NestedXARImportIT extends XARImportIT diff --git a/xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-ui/src/main/resources/XWiki/ResetPasswordRequestClass.xml b/xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-ui/src/main/resources/XWiki/ResetPasswordRequestClass.xml deleted file mode 100644 index 4553b999b5d9..000000000000 --- a/xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-ui/src/main/resources/XWiki/ResetPasswordRequestClass.xml +++ /dev/null @@ -1,110 +0,0 @@ - - - - - - XWiki - ResetPasswordRequestClass - - - 0 - xwiki:XWiki.Admin - XWiki.ResetPassword - xwiki:XWiki.Admin - xwiki:XWiki.Admin - 1.1 - Reset Password request class - - false - xwiki/2.0 - true - - - XWiki.ResetPasswordRequestClass - - - - - - - - - - - 0 - verification - 1 - 0 - Request verification string - 30 - Hash - - 0 - - - com.xpn.xwiki.objects.classes.PasswordClass - - - - XWiki.ResetPasswordRequestClass - 0 - XWiki.DocumentSheetBinding - 9d3418bf-70f4-42a0-8e02-ea3e41c45930 - - XWiki.DocumentSheetBinding - - - - - - - - - 0 - - - 0 - input - - - 0 - sheet - 1 - 1 - Sheet - 30 - 0 - - - none - - 0 - - - - com.xpn.xwiki.objects.classes.PageClass - - - - XWiki.ClassSheet - - - diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/resources/ApplicationResources.properties b/xwiki-platform-core/xwiki-platform-oldcore/src/main/resources/ApplicationResources.properties index 6524b747b3aa..a38f8bd5985b 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/resources/ApplicationResources.properties +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/resources/ApplicationResources.properties @@ -2530,7 +2530,6 @@ xe.admin.passwordReset.step2.login=Please login to continue \u00BB xe.admin.passwordReset.step2.backToStep1=Back to the password reset page \u00BB xe.admin.passwordReset.step2.error.emptyPassword=The password cannot be empty. xe.admin.passwordReset.step2.error.verificationMismatch=The two passwords do not match. -xe.admin.passwordReset.step2.error.wrongParameters=Wrong parameters! Another link was already sent or this one was already accessed! xe.admin.passwordReset.step2.error.noProgrammingRights=This page requires programming rights to work, which currently isn't the case. Please notify an administrator of this problem and try again later. xe.admin.passwordReset.step2.versionComment.passwordReset=Password was reset xe.admin.passwordReset.step2.versionComment.changeValidationKey=Refreshed password reset token @@ -5652,6 +5651,11 @@ core.viewers.diff.previousVersion=Previous version core.viewers.diff.nextChange=Next change core.viewers.diff.previousChange=Previous change +####################################### +## until 16.3.0RC1 +####################################### +xe.admin.passwordReset.step2.error.wrongParameters=Wrong parameters! Another link was already sent or this one was already accessed! + ## Used to indicate where deprecated keys end #@deprecatedend diff --git a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/pom.xml b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/pom.xml index 11ba4e307d2b..8203efddab3b 100644 --- a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/pom.xml +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/pom.xml @@ -36,4 +36,12 @@ xwiki-platform-security-authentication-script xwiki-platform-security-authentication-ui + + + integration-tests + + xwiki-platform-security-authentication-test + + + \ No newline at end of file diff --git a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/java/org/xwiki/security/authentication/internal/DefaultResetPasswordManager.java b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/java/org/xwiki/security/authentication/internal/DefaultResetPasswordManager.java index 0327b9de4cdf..eebe948c4e7f 100644 --- a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/java/org/xwiki/security/authentication/internal/DefaultResetPasswordManager.java +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/java/org/xwiki/security/authentication/internal/DefaultResetPasswordManager.java @@ -23,6 +23,7 @@ import java.net.URL; import java.time.Instant; import java.time.temporal.ChronoUnit; +import java.util.Date; import javax.inject.Inject; import javax.inject.Named; @@ -83,13 +84,8 @@ public class DefaultResetPasswordManager implements ResetPasswordManager protected static final LocalDocumentReference LDAP_CLASS_REFERENCE = new LocalDocumentReference(XWIKI_SPACE, "LDAPProfileClass"); - protected static final LocalDocumentReference RESET_PASSWORD_REQUEST_CLASS_REFERENCE = - new LocalDocumentReference(XWIKI_SPACE, "ResetPasswordRequestClass"); - protected static final LocalDocumentReference USER_CLASS_REFERENCE = new LocalDocumentReference(XWIKI_SPACE, "XWikiUsers"); - - protected static final String VERIFICATION_PROPERTY = "verification"; protected static final String TOKEN_LIFETIME = "security.authentication.resetPasswordTokenLifetime"; @Inject @@ -127,16 +123,32 @@ public class DefaultResetPasswordManager implements ResetPasswordManager @Inject private Logger logger; - private boolean checkUserReference(UserReference userReference) throws ResetPasswordException + private static final class UserInformation + { + private boolean userExists; + private InternetAddress userEmail; + private UserProperties userProperties; + + boolean canUserResetPassword() + { + return this.userExists && userEmail != null; + } + } + + private UserInformation getUserInformation(UserReference userReference) throws ResetPasswordException { // FIXME: This check shouldn't be needed if we'd have the proper API to determine which kind of // authentication is used. if (!(userReference instanceof DocumentUserReference)) { throw new ResetPasswordException("Only user having a page on the wiki can reset their password."); } + UserInformation result = new UserInformation(); try { - return this.userManager.exists(userReference); + result.userExists = this.userManager.exists(userReference); + result.userProperties = this.userPropertiesResolver.resolve(userReference); + result.userEmail = result.userProperties.getEmail(); + return result; } catch (UserException e) { throw new ResetPasswordException(String.format("Failed to check if user [%s] exists.", userReference), e); } @@ -145,72 +157,68 @@ private boolean checkUserReference(UserReference userReference) throws ResetPass @Override public ResetPasswordRequestResponse requestResetPassword(UserReference userReference) throws ResetPasswordException { - if (this.checkUserReference(userReference)) { - UserProperties userProperties = this.userPropertiesResolver.resolve(userReference); - InternetAddress email = userProperties.getEmail(); - - if (email != null) { - DocumentUserReference documentUserReference = (DocumentUserReference) userReference; - DocumentReference reference = documentUserReference.getReference(); - XWikiContext context = this.contextProvider.get(); - - try { - XWikiDocument userDocument = context.getWiki().getDocument(reference, context); - - if (userDocument.getXObject(LDAP_CLASS_REFERENCE) != null) { - String exceptionMessage = - this.localizationManager.getTranslationPlain("xe.admin.passwordReset.error.ldapUser", - userReference.toString()); - throw new ResetPasswordException(exceptionMessage); - } - - BaseObject xObject = userDocument.getXObject(RESET_PASSWORD_REQUEST_CLASS_REFERENCE, true, context); - String verificationCode = context.getWiki().generateRandomString(30); - xObject.set(VERIFICATION_PROPERTY, verificationCode, context); + UserInformation userInformation = this.getUserInformation(userReference); + if (userInformation.canUserResetPassword()) { + DocumentUserReference documentUserReference = (DocumentUserReference) userReference; + DocumentReference reference = documentUserReference.getReference(); + XWikiContext context = this.contextProvider.get(); - String saveComment = - this.localizationManager.getTranslationPlain("xe.admin.passwordReset.versionComment"); - context.getWiki().saveDocument(userDocument, saveComment, true, context); + try { + XWikiDocument userDocument = context.getWiki().getDocument(reference, context); - return new DefaultResetPasswordRequestResponse(userReference, verificationCode); - } catch (XWikiException e) { - throw new ResetPasswordException( - "Error when reading user document to perform reset password request.", - e); + if (userDocument.getXObject(LDAP_CLASS_REFERENCE) != null) { + String exceptionMessage = + this.localizationManager.getTranslationPlain("xe.admin.passwordReset.error.ldapUser", + userReference.toString()); + throw new ResetPasswordException(exceptionMessage); } - } else { - // In case the mail is not configured, we log a message to the admin. - this.logger.info("User [{}] asked to reset their password, but did not have any email configured.", - userReference); + + BaseObject xObject = userDocument.getXObject(ResetPasswordRequestClassDocumentInitializer.REFERENCE, + true, context); + String verificationCode = context.getWiki().generateRandomString(30); + xObject.set(ResetPasswordRequestClassDocumentInitializer.VERIFICATION_FIELD, + verificationCode, context); + xObject.setDateValue(ResetPasswordRequestClassDocumentInitializer.REQUEST_DATE_FIELD, new Date()); + String saveComment = + this.localizationManager.getTranslationPlain("xe.admin.passwordReset.versionComment"); + context.getWiki().saveDocument(userDocument, saveComment, true, context); + + return new DefaultResetPasswordRequestResponse(userReference, verificationCode); + } catch (XWikiException e) { + throw new ResetPasswordException( + "Error when reading user document to perform reset password request.", + e); } + } else if (userInformation.userExists && userInformation.userEmail == null) { + // In case the mail is not configured, we log a message to the admin. + this.logger.info("User [{}] asked to reset their password, but did not have any email configured.", + userReference); } - return new DefaultResetPasswordRequestResponse(userReference, null); + return new DefaultResetPasswordRequestResponse(userReference); } @Override public void sendResetPasswordEmailRequest(ResetPasswordRequestResponse requestResponse) throws ResetPasswordException { - if (this.checkUserReference(requestResponse.getUserReference())) { + UserInformation userInformation = this.getUserInformation(requestResponse.getUserReference()); + if (userInformation.canUserResetPassword()) { AuthenticationResourceReference resourceReference = new AuthenticationResourceReference( this.contextProvider.get().getWikiReference(), AuthenticationAction.RESET_PASSWORD); UserReference userReference = requestResponse.getUserReference(); - UserProperties userProperties = this.userPropertiesResolver.resolve(userReference); - InternetAddress email = userProperties.getEmail(); String serializedUserReference = this.referenceSerializer.serialize(userReference); - // FIXME: this should be provided as part of the User API. String formattedName = ""; - if (!StringUtils.isBlank(userProperties.getFirstName())) { - formattedName += userProperties.getFirstName(); + if (!StringUtils.isBlank(userInformation.userProperties.getFirstName())) { + formattedName += userInformation.userProperties.getFirstName(); } - if (!StringUtils.isBlank(userProperties.getLastName())) { + if (!StringUtils.isBlank(userInformation.userProperties.getLastName())) { if (!StringUtils.isBlank(formattedName)) { formattedName += " "; } - formattedName += userProperties.getLastName(); + formattedName += userInformation.userProperties.getLastName(); } if (StringUtils.isBlank(formattedName)) { formattedName = serializedUserReference; @@ -228,30 +236,39 @@ public void sendResetPasswordEmailRequest(ResetPasswordRequestResponse requestRe URL externalVerificationURL = new URL(serverURL, extendedURL.serialize()); this.resetPasswordMailSenderProvider.get() - .sendResetPasswordEmail(formattedName, email, externalVerificationURL); + .sendResetPasswordEmail(formattedName, userInformation.userEmail, externalVerificationURL); } catch (SerializeResourceReferenceException | UnsupportedResourceReferenceException - | MalformedURLException e) { + | MalformedURLException e) { throw new ResetPasswordException("Error when processing information for creating the email.", e); } } } + private int getTokenLifeTime() + { + return this.configurationSource.getProperty(TOKEN_LIFETIME, 60); + } + /** - * Check if the reset password token should be reset. - * To determine if the token should be reset, we check: 1. the token lifetime property, 2. the save date of the - * document. + * Check if the reset password token is expired. * - * @param userDocument - * @return + * @param requestXObject the request xobject to determine if it's expired or not + * @return {@code true} if the token has expired and cannot be used anymore. */ - private boolean shouldTokenBeReset(XWikiDocument userDocument) + private boolean isTokenExpired(BaseObject requestXObject) { - Integer tokenLifeTime = this.configurationSource.getProperty(TOKEN_LIFETIME, 0); - boolean result = true; - if (tokenLifeTime != 0) { - Instant saveInstant = userDocument.getDate().toInstant(); - Instant now = Instant.now(); - return (saveInstant.plus(tokenLifeTime, ChronoUnit.MINUTES).isBefore(now)); + int tokenLifeTime = getTokenLifeTime(); + boolean result = false; + if (tokenLifeTime > 0) { + Date dateValue = + requestXObject.getDateValue(ResetPasswordRequestClassDocumentInitializer.REQUEST_DATE_FIELD); + if (dateValue == null) { + return true; + } else { + Instant saveInstant = dateValue.toInstant(); + Instant now = Instant.now(); + return saveInstant.plus(tokenLifeTime, ChronoUnit.MINUTES).isBefore(now); + } } return result; } @@ -260,25 +277,29 @@ private boolean shouldTokenBeReset(XWikiDocument userDocument) public ResetPasswordRequestResponse checkVerificationCode(UserReference userReference, String verificationCode) throws ResetPasswordException { - if (this.checkUserReference(userReference)) { + ResetPasswordRequestResponse result = new DefaultResetPasswordRequestResponse(userReference); + UserInformation userInformation = this.getUserInformation(userReference); + if (userInformation.canUserResetPassword()) { XWikiContext context = this.contextProvider.get(); DocumentUserReference documentUserReference = (DocumentUserReference) userReference; DocumentReference reference = documentUserReference.getReference(); - String exceptionMessage = - this.localizationManager.getTranslationPlain("xe.admin.passwordReset.step2.error.wrongParameters", - userReference.toString()); + String exceptionMessage = this.localizationManager + .getTranslationPlain("security.authentication.resetPassword.error.badParameters"); try { XWikiDocument userDocument = context.getWiki().getDocument(reference, context); - BaseObject xObject = userDocument.getXObject(RESET_PASSWORD_REQUEST_CLASS_REFERENCE); + BaseObject xObject = userDocument.getXObject(ResetPasswordRequestClassDocumentInitializer.REFERENCE); if (xObject == null) { throw new ResetPasswordException(exceptionMessage); } - String storedVerificationCode = xObject.getStringValue(VERIFICATION_PROPERTY); + String storedVerificationCode = + xObject.getStringValue(ResetPasswordRequestClassDocumentInitializer.VERIFICATION_FIELD); BaseClass xClass = xObject.getXClass(context); - PropertyInterface verification = xClass.get(VERIFICATION_PROPERTY); + PropertyInterface verification = + xClass.get(ResetPasswordRequestClassDocumentInitializer.VERIFICATION_FIELD); + // FIXME: shouldn't we be able to get rid of this check? if (!(verification instanceof PasswordClass)) { throw new ResetPasswordException("Bad definition of ResetPassword XClass."); } @@ -286,35 +307,44 @@ public ResetPasswordRequestResponse checkVerificationCode(UserReference userRefe String equivalentPassword = passwordClass.getEquivalentPassword(storedVerificationCode, verificationCode); - String newVerificationCode = verificationCode; - if (this.shouldTokenBeReset(userDocument)) { - // We ensure to reset the verification code before checking if it's correct to avoid - // any bruteforce attack. - newVerificationCode = context.getWiki().generateRandomString(30); - xObject.set(VERIFICATION_PROPERTY, newVerificationCode, context); - String saveComment = this.localizationManager - .getTranslationPlain("xe.admin.passwordReset.step2.versionComment.changeValidationKey"); - context.getWiki().saveDocument(userDocument, saveComment, true, context); - } - - if (!storedVerificationCode.equals(equivalentPassword)) { + // If the token is expired we remove it right away to avoid any attack. + if (this.isTokenExpired(xObject)) { + this.resetVerificationCode(userDocument, xObject, + "security.authentication.resetPassword.tokenExpired"); + throw new ResetPasswordException(exceptionMessage); + } else if (!storedVerificationCode.equals(equivalentPassword)) { + // If the token is not correct and there's no lifetime duration set, we immediately get rid of it + // so any bruteforce is compromised. + if (getTokenLifeTime() <= 0) { + this.resetVerificationCode(userDocument, xObject, + "security.authentication.resetPassword.badToken"); + } throw new ResetPasswordException(exceptionMessage); + } else { + result = new DefaultResetPasswordRequestResponse(userReference, verificationCode); } - - return new DefaultResetPasswordRequestResponse(userReference, newVerificationCode); } catch (XWikiException e) { throw new ResetPasswordException("Cannot open user document to check verification code.", e); } - } else { - return new DefaultResetPasswordRequestResponse(userReference, null); } + return result; + } + + private void resetVerificationCode(XWikiDocument userDocument, BaseObject xobject, String saveCommentTranslationKey) + throws XWikiException + { + XWikiContext context = this.contextProvider.get(); + userDocument.removeXObject(xobject); + String saveComment = this.localizationManager.getTranslationPlain(saveCommentTranslationKey); + context.getWiki().saveDocument(userDocument, saveComment, true, context); } @Override public void resetPassword(UserReference userReference, String newPassword) throws ResetPasswordException { - if (this.checkUserReference(userReference)) { + UserInformation userInformation = this.getUserInformation(userReference); + if (userInformation.canUserResetPassword()) { if (!this.isPasswordCompliantWithRegistrationRules(newPassword)) { throw new ResetPasswordException("The provided password is not compliant with the password security " + "rules."); @@ -326,7 +356,7 @@ public void resetPassword(UserReference userReference, String newPassword) try { XWikiDocument userDocument = context.getWiki().getDocument(reference, context); - userDocument.removeXObjects(RESET_PASSWORD_REQUEST_CLASS_REFERENCE); + userDocument.removeXObjects(ResetPasswordRequestClassDocumentInitializer.REFERENCE); BaseObject userXObject = userDocument.getXObject(USER_CLASS_REFERENCE); // /!\ We cannot use BaseCollection#setStringValue as it's storing value in plain text. diff --git a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/java/org/xwiki/security/authentication/internal/DefaultResetPasswordRequestResponse.java b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/java/org/xwiki/security/authentication/internal/DefaultResetPasswordRequestResponse.java index 8491ce422efb..2414865c2852 100644 --- a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/java/org/xwiki/security/authentication/internal/DefaultResetPasswordRequestResponse.java +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/java/org/xwiki/security/authentication/internal/DefaultResetPasswordRequestResponse.java @@ -46,6 +46,11 @@ public final class DefaultResetPasswordRequestResponse implements ResetPasswordR this.verificationCode = verificationCode; } + DefaultResetPasswordRequestResponse(UserReference reference) + { + this(reference, null); + } + /** * @return the user for whom a reset password request is performed. */ diff --git a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/java/org/xwiki/security/authentication/internal/ResetPasswordRequestClassDocumentInitializer.java b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/java/org/xwiki/security/authentication/internal/ResetPasswordRequestClassDocumentInitializer.java new file mode 100644 index 000000000000..14670c954ba9 --- /dev/null +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/java/org/xwiki/security/authentication/internal/ResetPasswordRequestClassDocumentInitializer.java @@ -0,0 +1,75 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ + +package org.xwiki.security.authentication.internal; + +import javax.inject.Named; +import javax.inject.Singleton; + +import org.xwiki.component.annotation.Component; +import org.xwiki.model.reference.LocalDocumentReference; + +import com.xpn.xwiki.XWiki; +import com.xpn.xwiki.doc.AbstractMandatoryClassInitializer; +import com.xpn.xwiki.objects.classes.BaseClass; + +/** + * Initializer of the ResetPasswordRequestClass XClass. + * + * @version $Id$ + * @since 16.3.0RC1 + * @since 15.10.9 + */ +@Component +@Singleton +@Named("XWiki.ResetPasswordRequestClass") +public class ResetPasswordRequestClassDocumentInitializer extends AbstractMandatoryClassInitializer +{ + /** + * Reference of the xclass. + */ + public static final LocalDocumentReference REFERENCE = + new LocalDocumentReference(XWiki.SYSTEM_SPACE, "ResetPasswordRequestClass"); + + /** + * Verification field which stores the actual token. + */ + public static final String VERIFICATION_FIELD = "verification"; + + /** + * Date field which stores the date when the request was made. + */ + public static final String REQUEST_DATE_FIELD = "requestDate"; + + /** + * Default constructor. + */ + public ResetPasswordRequestClassDocumentInitializer() + { + super(REFERENCE); + } + + @Override + protected void createClass(BaseClass xclass) + { + xclass.addPasswordField(VERIFICATION_FIELD, "Request verification string", 30); + xclass.addDateField(REQUEST_DATE_FIELD, "Date when the request was performed"); + } +} diff --git a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/resources/ApplicationResources.properties b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/resources/ApplicationResources.properties index 453eb9fe5171..2717b82950ab 100644 --- a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/resources/ApplicationResources.properties +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/resources/ApplicationResources.properties @@ -20,8 +20,9 @@ security.authentication.strategy.captcha.errorMessage=Please fill the captcha form to login. security.authentication.rest.blockedError=Your account may be blocked after too many attempts to login. Please go to the xwiki login page to get more information. security.authentication.strategy.disableAccount.errorMessage=This account has been disabled. Please ask the administrator to enable it back. - security.authentication.security.email.error=Error when sending a security email - +security.authentication.resetPassword.error.badParameters=This link has already been used to reset the password, or the link has expired. Try requesting again to reset your password and use the new link. +security.authentication.resetPassword.tokenExpired=Remove expired token +security.authentication.resetPassword.badToken=Remove token for security reason security.authentication.migration1400600000XWIKI19869.email.subject=Important security issue security.authentication.migration1400600000XWIKI19869.email.content=Dear user, \n\ndue to a bug your password was stored in plain text in our wiki. We cannot exclude that your plain text password was exposed in a data leak. Therefore, you might receive a second email to choose a new password. \nPlease contact the administrator in case of problem or for further questions. diff --git a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/resources/META-INF/components.txt b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/resources/META-INF/components.txt index 8b5e5c126a42..b3f45d366367 100644 --- a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/resources/META-INF/components.txt +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/main/resources/META-INF/components.txt @@ -13,3 +13,4 @@ org.xwiki.security.authentication.internal.DefaultRetrieveUsernameManager org.xwiki.security.authentication.internal.DisableAccountFailureStrategy org.xwiki.security.authentication.internal.R140600000XWIKI19869DataMigrationListener org.xwiki.security.authentication.internal.RegistrationConfigurationSource +org.xwiki.security.authentication.internal.ResetPasswordRequestClassDocumentInitializer diff --git a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/test/java/org/xwiki/security/authentication/internal/DefaultResetPasswordManagerTest.java b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/test/java/org/xwiki/security/authentication/internal/DefaultResetPasswordManagerTest.java index 33b8e5dbd27b..fa4dc16dd76e 100644 --- a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/test/java/org/xwiki/security/authentication/internal/DefaultResetPasswordManagerTest.java +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-default/src/test/java/org/xwiki/security/authentication/internal/DefaultResetPasswordManagerTest.java @@ -75,6 +75,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -88,7 +89,6 @@ @ComponentTest class DefaultResetPasswordManagerTest { - @InjectMockComponents private DefaultResetPasswordManager resetPasswordManager; @@ -154,7 +154,6 @@ void setup() throws Exception when(this.xWiki.getDocument(this.userDocumentReference, this.context)).thenReturn(this.userDocument); this.authenticationMailSender = mock(AuthenticationMailSender.class); when(this.resetPasswordMailSenderProvider.get()).thenReturn(this.authenticationMailSender); - when(this.configurationSource.getProperty(DefaultResetPasswordManager.TOKEN_LIFETIME, 0)).thenReturn(0); } @Test @@ -165,7 +164,7 @@ void requestResetPassword() throws Exception when(this.userProperties.getEmail()).thenReturn(email); BaseObject xObject = mock(BaseObject.class); when(this.userDocument - .getXObject(DefaultResetPasswordManager.RESET_PASSWORD_REQUEST_CLASS_REFERENCE, true, this.context)) + .getXObject(ResetPasswordRequestClassDocumentInitializer.REFERENCE, true, this.context)) .thenReturn(xObject); String verificationCode = "abcde1234"; when(this.xWiki.generateRandomString(30)).thenReturn(verificationCode); @@ -175,7 +174,7 @@ void requestResetPassword() throws Exception ResetPasswordRequestResponse expectedResult = new DefaultResetPasswordRequestResponse(this.userReference, verificationCode); assertEquals(expectedResult, this.resetPasswordManager.requestResetPassword(this.userReference)); - verify(xObject).set(DefaultResetPasswordManager.VERIFICATION_PROPERTY, verificationCode, context); + verify(xObject).set(ResetPasswordRequestClassDocumentInitializer.VERIFICATION_FIELD, verificationCode, context); verify(this.xWiki).saveDocument(this.userDocument, "Save verification code 42", true, this.context); } @@ -184,7 +183,7 @@ void requestResetPasswordUnexistingUser() throws Exception { when(this.userReference.toString()).thenReturn("user:Foobar"); when(this.userManager.exists(this.userReference)).thenReturn(false); - assertEquals(new DefaultResetPasswordRequestResponse(this.userReference, null), + assertEquals(new DefaultResetPasswordRequestResponse(this.userReference), this.resetPasswordManager.requestResetPassword(this.userReference)); } @@ -265,40 +264,35 @@ void sendResetPasswordEmailRequest() throws Exception } @Test - void checkVerificationCode() throws Exception + void checkVerificationCodeGoodCodeNoExpirationDate() throws Exception { + when(this.configurationSource.getProperty(DefaultResetPasswordManager.TOKEN_LIFETIME, 60)).thenReturn(0); when(this.userManager.exists(this.userReference)).thenReturn(true); InternetAddress email = new InternetAddress("foobar@xwiki.org"); when(this.userProperties.getEmail()).thenReturn(email); String verificationCode = "abcd1245"; BaseObject xObject = mock(BaseObject.class); when(this.userDocument - .getXObject(DefaultResetPasswordManager.RESET_PASSWORD_REQUEST_CLASS_REFERENCE)) + .getXObject(ResetPasswordRequestClassDocumentInitializer.REFERENCE)) .thenReturn(xObject); String encodedVerificationCode = "encodedVerificationCode"; - when(xObject.getStringValue(DefaultResetPasswordManager.VERIFICATION_PROPERTY)) + when(xObject.getStringValue(ResetPasswordRequestClassDocumentInitializer.VERIFICATION_FIELD)) .thenReturn(encodedVerificationCode); BaseClass baseClass = mock(BaseClass.class); when(xObject.getXClass(context)).thenReturn(baseClass); PasswordClass passwordClass = mock(PasswordClass.class); - when(baseClass.get(DefaultResetPasswordManager.VERIFICATION_PROPERTY)).thenReturn(passwordClass); + when(baseClass.get(ResetPasswordRequestClassDocumentInitializer.VERIFICATION_FIELD)).thenReturn(passwordClass); when(passwordClass.getEquivalentPassword(encodedVerificationCode, verificationCode)) .thenReturn(encodedVerificationCode); - String newVerificationCode = "foobartest"; - when(xWiki.generateRandomString(30)).thenReturn(newVerificationCode); - String saveComment = "Save new verification code"; - when(this.localizationManager - .getTranslationPlain("xe.admin.passwordReset.step2.versionComment.changeValidationKey")) - .thenReturn(saveComment); DefaultResetPasswordRequestResponse expected = - new DefaultResetPasswordRequestResponse(this.userReference, newVerificationCode); + new DefaultResetPasswordRequestResponse(this.userReference, verificationCode); assertEquals(expected, this.resetPasswordManager.checkVerificationCode(this.userReference, verificationCode)); - verify(this.xWiki).saveDocument(this.userDocument, saveComment, true, context); + verify(this.xWiki, never()).saveDocument(eq(this.userDocument), any(), anyBoolean(), eq(context)); } @Test - void checkVerificationCodeTokenLifetimeNotExpired() throws Exception + void checkGoodVerificationCodeTokenLifetimeNotExpired() throws Exception { when(this.userManager.exists(this.userReference)).thenReturn(true); InternetAddress email = new InternetAddress("foobar@xwiki.org"); @@ -306,19 +300,20 @@ void checkVerificationCodeTokenLifetimeNotExpired() throws Exception String verificationCode = "abcd1245"; BaseObject xObject = mock(BaseObject.class); when(this.userDocument - .getXObject(DefaultResetPasswordManager.RESET_PASSWORD_REQUEST_CLASS_REFERENCE)) + .getXObject(ResetPasswordRequestClassDocumentInitializer.REFERENCE)) .thenReturn(xObject); String encodedVerificationCode = "encodedVerificationCode"; - when(xObject.getStringValue(DefaultResetPasswordManager.VERIFICATION_PROPERTY)) + when(xObject.getStringValue(ResetPasswordRequestClassDocumentInitializer.VERIFICATION_FIELD)) .thenReturn(encodedVerificationCode); BaseClass baseClass = mock(BaseClass.class); when(xObject.getXClass(context)).thenReturn(baseClass); PasswordClass passwordClass = mock(PasswordClass.class); - when(baseClass.get(DefaultResetPasswordManager.VERIFICATION_PROPERTY)).thenReturn(passwordClass); + when(baseClass.get(ResetPasswordRequestClassDocumentInitializer.VERIFICATION_FIELD)).thenReturn(passwordClass); when(passwordClass.getEquivalentPassword(encodedVerificationCode, verificationCode)) .thenReturn(encodedVerificationCode); - when(this.configurationSource.getProperty(DefaultResetPasswordManager.TOKEN_LIFETIME, 0)).thenReturn(15); - when(this.userDocument.getDate()).thenReturn(Date.from(Instant.now().minus(10, ChronoUnit.MINUTES))); + when(this.configurationSource.getProperty(DefaultResetPasswordManager.TOKEN_LIFETIME, 60)).thenReturn(15); + when(xObject.getDateValue(ResetPasswordRequestClassDocumentInitializer.REQUEST_DATE_FIELD)) + .thenReturn(Date.from(Instant.now().minus(14, ChronoUnit.MINUTES))); DefaultResetPasswordRequestResponse expected = new DefaultResetPasswordRequestResponse(this.userReference, verificationCode); @@ -327,6 +322,50 @@ void checkVerificationCodeTokenLifetimeNotExpired() throws Exception verify(this.xWiki, never()).saveDocument(any(), any(), anyBoolean(), any()); } + @Test + void checkGoodVerificationCodeTokenLifetimeExpired() throws Exception + { + when(this.userManager.exists(this.userReference)).thenReturn(true); + InternetAddress email = new InternetAddress("foobar@xwiki.org"); + when(this.userProperties.getEmail()).thenReturn(email); + String verificationCode = "abcd1245"; + BaseObject xObject = mock(BaseObject.class); + when(this.userDocument + .getXObject(ResetPasswordRequestClassDocumentInitializer.REFERENCE)) + .thenReturn(xObject); + String encodedVerificationCode = "encodedVerificationCode"; + when(xObject.getStringValue(ResetPasswordRequestClassDocumentInitializer.VERIFICATION_FIELD)) + .thenReturn(encodedVerificationCode); + BaseClass baseClass = mock(BaseClass.class); + when(xObject.getXClass(context)).thenReturn(baseClass); + PasswordClass passwordClass = mock(PasswordClass.class); + when(baseClass.get(ResetPasswordRequestClassDocumentInitializer.VERIFICATION_FIELD)).thenReturn(passwordClass); + when(passwordClass.getEquivalentPassword(encodedVerificationCode, verificationCode)) + .thenReturn(encodedVerificationCode); + when(this.configurationSource.getProperty(DefaultResetPasswordManager.TOKEN_LIFETIME, 60)).thenReturn(15); + when(xObject.getDateValue(ResetPasswordRequestClassDocumentInitializer.REQUEST_DATE_FIELD)) + .thenReturn(Date.from(Instant.now().minus(17, ChronoUnit.MINUTES))); + + DefaultResetPasswordRequestResponse expected = + new DefaultResetPasswordRequestResponse(this.userReference); + + String saveComment = "Removed expired token"; + when(this.localizationManager + .getTranslationPlain("security.authentication.resetPassword.tokenExpired")) + .thenReturn(saveComment); + + String exceptionMessage = "Wrong verification code"; + when(this.localizationManager + .getTranslationPlain("security.authentication.resetPassword.error.badParameters")) + .thenReturn(exceptionMessage); + + ResetPasswordException resetPasswordException = assertThrows(ResetPasswordException.class, + () -> this.resetPasswordManager.checkVerificationCode(this.userReference, verificationCode)); + assertEquals(exceptionMessage, resetPasswordException.getMessage()); + verify(userDocument).removeXObject(xObject); + verify(this.xWiki).saveDocument(userDocument, saveComment, true, context); + } + @Test void checkVerificationCodeUnexistingUser() throws Exception { @@ -347,36 +386,71 @@ void checkVerificationCodeWrongCode() throws Exception String verificationCode = "abcd1245"; BaseObject xObject = mock(BaseObject.class); when(this.userDocument - .getXObject(DefaultResetPasswordManager.RESET_PASSWORD_REQUEST_CLASS_REFERENCE)) + .getXObject(ResetPasswordRequestClassDocumentInitializer.REFERENCE)) .thenReturn(xObject); String encodedVerificationCode = "encodedVerificationCode"; - when(xObject.getStringValue(DefaultResetPasswordManager.VERIFICATION_PROPERTY)) + when(xObject.getStringValue(ResetPasswordRequestClassDocumentInitializer.VERIFICATION_FIELD)) .thenReturn(encodedVerificationCode); BaseClass baseClass = mock(BaseClass.class); when(xObject.getXClass(context)).thenReturn(baseClass); PasswordClass passwordClass = mock(PasswordClass.class); - when(baseClass.get(DefaultResetPasswordManager.VERIFICATION_PROPERTY)).thenReturn(passwordClass); + when(baseClass.get(ResetPasswordRequestClassDocumentInitializer.VERIFICATION_FIELD)).thenReturn(passwordClass); when(passwordClass.getEquivalentPassword(encodedVerificationCode, verificationCode)) .thenReturn("anotherCode"); - String newVerificationCode = "foobartest"; - when(xWiki.generateRandomString(30)).thenReturn(newVerificationCode); - String saveComment = "Save new verification code"; + when(this.configurationSource.getProperty(DefaultResetPasswordManager.TOKEN_LIFETIME, 60)).thenReturn(15); + when(xObject.getDateValue(ResetPasswordRequestClassDocumentInitializer.REQUEST_DATE_FIELD)) + .thenReturn(Date.from(Instant.now().minus(14, ChronoUnit.MINUTES))); + + String exceptionMessage = "Wrong verification code"; when(this.localizationManager - .getTranslationPlain("xe.admin.passwordReset.step2.versionComment.changeValidationKey")) - .thenReturn(saveComment); + .getTranslationPlain("security.authentication.resetPassword.error.badParameters")) + .thenReturn(exceptionMessage); + + ResetPasswordException resetPasswordException = assertThrows(ResetPasswordException.class, + () -> this.resetPasswordManager.checkVerificationCode(this.userReference, verificationCode)); + assertEquals(exceptionMessage, resetPasswordException.getMessage()); + verify(this.xWiki, never()).saveDocument(eq(this.userDocument), any(), anyBoolean(), eq(context)); + } + + @Test + void checkVerificationCodeWrongCodeNoTokenExpiration() throws Exception + { + when(this.configurationSource.getProperty(DefaultResetPasswordManager.TOKEN_LIFETIME, 60)).thenReturn(0); + when(this.userReference.toString()).thenReturn("user:Foobar"); + when(this.userManager.exists(this.userReference)).thenReturn(true); + InternetAddress email = new InternetAddress("foobar@xwiki.org"); + when(this.userProperties.getEmail()).thenReturn(email); + String verificationCode = "abcd1245"; + BaseObject xObject = mock(BaseObject.class); + when(this.userDocument + .getXObject(ResetPasswordRequestClassDocumentInitializer.REFERENCE)) + .thenReturn(xObject); + String encodedVerificationCode = "encodedVerificationCode"; + when(xObject.getStringValue(ResetPasswordRequestClassDocumentInitializer.VERIFICATION_FIELD)) + .thenReturn(encodedVerificationCode); + BaseClass baseClass = mock(BaseClass.class); + when(xObject.getXClass(context)).thenReturn(baseClass); + PasswordClass passwordClass = mock(PasswordClass.class); + when(baseClass.get(ResetPasswordRequestClassDocumentInitializer.VERIFICATION_FIELD)).thenReturn(passwordClass); + when(passwordClass.getEquivalentPassword(encodedVerificationCode, verificationCode)) + .thenReturn("anotherCode"); String exceptionMessage = "Wrong verification code"; when(this.localizationManager - .getTranslationPlain("xe.admin.passwordReset.step2.error.wrongParameters", "user:Foobar")) + .getTranslationPlain("security.authentication.resetPassword.error.badParameters")) .thenReturn(exceptionMessage); + String saveComment = "Bad token"; + when(this.localizationManager.getTranslationPlain("security.authentication.resetPassword.badToken")) + .thenReturn(saveComment); ResetPasswordException resetPasswordException = assertThrows(ResetPasswordException.class, () -> this.resetPasswordManager.checkVerificationCode(this.userReference, verificationCode)); assertEquals(exceptionMessage, resetPasswordException.getMessage()); - verify(this.xWiki).saveDocument(this.userDocument, saveComment, true, context); + verify(userDocument).removeXObject(xObject); + verify(this.xWiki).saveDocument(userDocument, saveComment, true, context); } @Test - void checkVerificationCodeTokenLifetimeExpired() throws Exception + void checkWrongVerificationCodeTokenLifetimeExpired() throws Exception { when(this.userReference.toString()).thenReturn("user:Foobar"); when(this.userManager.exists(this.userReference)).thenReturn(true); @@ -385,33 +459,33 @@ void checkVerificationCodeTokenLifetimeExpired() throws Exception String verificationCode = "abcd1245"; BaseObject xObject = mock(BaseObject.class); when(this.userDocument - .getXObject(DefaultResetPasswordManager.RESET_PASSWORD_REQUEST_CLASS_REFERENCE)) + .getXObject(ResetPasswordRequestClassDocumentInitializer.REFERENCE)) .thenReturn(xObject); String encodedVerificationCode = "encodedVerificationCode"; - when(xObject.getStringValue(DefaultResetPasswordManager.VERIFICATION_PROPERTY)) + when(xObject.getStringValue(ResetPasswordRequestClassDocumentInitializer.VERIFICATION_FIELD)) .thenReturn(encodedVerificationCode); BaseClass baseClass = mock(BaseClass.class); when(xObject.getXClass(context)).thenReturn(baseClass); PasswordClass passwordClass = mock(PasswordClass.class); - when(baseClass.get(DefaultResetPasswordManager.VERIFICATION_PROPERTY)).thenReturn(passwordClass); + when(baseClass.get(ResetPasswordRequestClassDocumentInitializer.VERIFICATION_FIELD)).thenReturn(passwordClass); when(passwordClass.getEquivalentPassword(encodedVerificationCode, verificationCode)) .thenReturn("anotherCode"); - String newVerificationCode = "foobartest"; - when(xWiki.generateRandomString(30)).thenReturn(newVerificationCode); - String saveComment = "Save new verification code"; + + String saveComment = "Removed expired token"; when(this.localizationManager - .getTranslationPlain("xe.admin.passwordReset.step2.versionComment.changeValidationKey")) + .getTranslationPlain("security.authentication.resetPassword.tokenExpired")) .thenReturn(saveComment); String exceptionMessage = "Wrong verification code"; when(this.localizationManager - .getTranslationPlain("xe.admin.passwordReset.step2.error.wrongParameters", "user:Foobar")) + .getTranslationPlain("security.authentication.resetPassword.error.badParameters")) .thenReturn(exceptionMessage); - when(this.configurationSource.getProperty(DefaultResetPasswordManager.TOKEN_LIFETIME, 0)).thenReturn(15); + when(this.configurationSource.getProperty(DefaultResetPasswordManager.TOKEN_LIFETIME, 60)).thenReturn(15); when(this.userDocument.getDate()).thenReturn(Date.from(Instant.now().minus(16, ChronoUnit.MINUTES))); ResetPasswordException resetPasswordException = assertThrows(ResetPasswordException.class, () -> this.resetPasswordManager.checkVerificationCode(this.userReference, verificationCode)); assertEquals(exceptionMessage, resetPasswordException.getMessage()); + verify(userDocument).removeXObject(xObject); verify(this.xWiki).saveDocument(this.userDocument, saveComment, true, context); } @@ -430,6 +504,8 @@ void checkVerificationCodeNotDocumentReferenceUser() throws Exception void resetPassword() throws Exception { when(this.userManager.exists(this.userReference)).thenReturn(true); + InternetAddress email = new InternetAddress("foobar@xwiki.org"); + when(this.userProperties.getEmail()).thenReturn(email); BaseObject xObject = mock(BaseObject.class); when(this.userDocument .getXObject(DefaultResetPasswordManager.USER_CLASS_REFERENCE)) @@ -440,7 +516,7 @@ void resetPassword() throws Exception .thenReturn(saveComment); String newPassword = "mypassword"; this.resetPasswordManager.resetPassword(this.userReference, newPassword); - verify(this.userDocument).removeXObjects(DefaultResetPasswordManager.RESET_PASSWORD_REQUEST_CLASS_REFERENCE); + verify(this.userDocument).removeXObjects(ResetPasswordRequestClassDocumentInitializer.REFERENCE); verify(xObject).set("password", newPassword, context); } diff --git a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-script/src/main/java/org/xwiki/security/authentication/script/AuthenticationScriptService.java b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-script/src/main/java/org/xwiki/security/authentication/script/AuthenticationScriptService.java index 85ca4824a28c..04b6d6dc70fa 100644 --- a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-script/src/main/java/org/xwiki/security/authentication/script/AuthenticationScriptService.java +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-script/src/main/java/org/xwiki/security/authentication/script/AuthenticationScriptService.java @@ -208,14 +208,13 @@ public void requestResetPassword(UserReference user) throws ResetPasswordExcepti /** * Check that the given verification code is correct. - * Since a new verification code is generated (to avoid reusing a code several times), we also return the new code. * Note that we don't need to protect this API for programming rights: if the verificationCode is not correct a * {@link ResetPasswordException} is thrown and the verificationCode is reset. So a script attacker with wrong - * credentials cannot access the new verification code, or bruteforce it. + * credentials cannot access the verification code, or bruteforce it. * * @param user the user for which to check the verification code. * @param verificationCode the code to check. - * @return a newly generated verification code if it is correct. + * @return the same verification code if it is correct. * @throws ResetPasswordException if the code is not correct or if an error occurs. * @since 13.1RC1 */ diff --git a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/pom.xml b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/pom.xml new file mode 100644 index 000000000000..34c02273b42a --- /dev/null +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/pom.xml @@ -0,0 +1,48 @@ + + + + + + 4.0.0 + + org.xwiki.platform + xwiki-platform-security-authentication + 15.10.9-SNAPSHOT + + xwiki-platform-security-authentication-test + XWiki Platform - Security - Authentication - Tests - Parent POM + pom + Tests Parent Pom for the Security Authentication + + + true + + true + + + + docker + + xwiki-platform-security-authentication-test-docker + + + + diff --git a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/xwiki-platform-security-authentication-test-docker/pom.xml b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/xwiki-platform-security-authentication-test-docker/pom.xml new file mode 100644 index 000000000000..fb0e70d42e5a --- /dev/null +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/xwiki-platform-security-authentication-test-docker/pom.xml @@ -0,0 +1,137 @@ + + + + + + 4.0.0 + + org.xwiki.platform + xwiki-platform-security-authentication-test + 15.10.9-SNAPSHOT + + xwiki-platform-security-authentication-test-docker + XWiki Platform - Security - Authentication - Tests - Functional Docker Tests + + jar + XWiki Platform - Security - Authentication - Tests - Functional Tests in Docker + + + true + + + + + org.xwiki.platform + xwiki-platform-security-authentication-ui + ${project.version} + xar + + + org.xwiki.platform + xwiki-platform-security-authentication-default + ${project.version} + jar + + + + org.xwiki.platform + xwiki-platform-mail-ui + ${project.version} + xar + + + + org.xwiki.platform + xwiki-platform-test-docker + ${project.version} + test + + + org.xwiki.platform + xwiki-platform-administration-test-pageobjects + ${project.version} + test + + + com.icegreen + greenmail-junit5 + test + + + com.sun.mail + jakarta.mail + test + + + org.xwiki.platform + xwiki-platform-flamingo-skin-test-pageobjects + ${project.version} + test + + + + src/test/it + + + + org.apache.maven.plugins + maven-failsafe-plugin + + + + + + clover + + + + org.openclover + clover + + + + + + org.apache.maven.plugins + maven-failsafe-plugin + + + + clover + + + + + + + + diff --git a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/xwiki-platform-security-authentication-test-docker/src/test/it/org/xwiki/security/authentication/test/ui/AllIT.java b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/xwiki-platform-security-authentication-test-docker/src/test/it/org/xwiki/security/authentication/test/ui/AllIT.java new file mode 100644 index 000000000000..9fd1da3b6171 --- /dev/null +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/xwiki-platform-security-authentication-test-docker/src/test/it/org/xwiki/security/authentication/test/ui/AllIT.java @@ -0,0 +1,46 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.xwiki.security.authentication.test.ui; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.xwiki.test.docker.junit5.UITest; + +/** + * All UI tests for the Security Authentication feature. + * + * @version $Id$ + * @since 16.3.0RC1 + */ +@UITest +public class AllIT +{ + @Nested + @DisplayName("Reset Password") + class NestedResetPasswordIT extends ResetPasswordIT + { + } + + @Nested + @DisplayName("Forgot Username") + class NestedForgotUsernameIT extends ForgotUsernameIT + { + } +} diff --git a/xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-test/xwiki-platform-administration-test-docker/src/test/it/org/xwiki/administration/test/ui/ForgotUsernameIT.java b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/xwiki-platform-security-authentication-test-docker/src/test/it/org/xwiki/security/authentication/test/ui/ForgotUsernameIT.java similarity index 99% rename from xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-test/xwiki-platform-administration-test-docker/src/test/it/org/xwiki/administration/test/ui/ForgotUsernameIT.java rename to xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/xwiki-platform-security-authentication-test-docker/src/test/it/org/xwiki/security/authentication/test/ui/ForgotUsernameIT.java index cdf4d2073184..b87a9b993e2d 100644 --- a/xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-test/xwiki-platform-administration-test-docker/src/test/it/org/xwiki/administration/test/ui/ForgotUsernameIT.java +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/xwiki-platform-security-authentication-test-docker/src/test/it/org/xwiki/security/authentication/test/ui/ForgotUsernameIT.java @@ -17,7 +17,7 @@ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA * 02110-1301 USA, or see the FSF site: http://www.fsf.org. */ -package org.xwiki.administration.test.ui; +package org.xwiki.security.authentication.test.ui; import java.util.HashMap; import java.util.Map; @@ -92,6 +92,7 @@ public void stopMail(TestUtils setup, LogCaptureConfiguration logCaptureConfigur private void configureEmail(TestUtils setup, TestConfiguration testConfiguration) { + setup.loginAsSuperAdmin(); setup.updateObject("Mail", "MailConfig", "Mail.SendMailConfigClass", 0, "host", testConfiguration.getServletEngine().getHostIP(), "port", "3025", "sendWaitTime", "0"); } diff --git a/xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-test/xwiki-platform-administration-test-docker/src/test/it/org/xwiki/administration/test/ui/ResetPasswordIT.java b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/xwiki-platform-security-authentication-test-docker/src/test/it/org/xwiki/security/authentication/test/ui/ResetPasswordIT.java similarity index 94% rename from xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-test/xwiki-platform-administration-test-docker/src/test/it/org/xwiki/administration/test/ui/ResetPasswordIT.java rename to xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/xwiki-platform-security-authentication-test-docker/src/test/it/org/xwiki/security/authentication/test/ui/ResetPasswordIT.java index 55d900a92b7f..9623211f0bb5 100644 --- a/xwiki-platform-core/xwiki-platform-administration/xwiki-platform-administration-test/xwiki-platform-administration-test-docker/src/test/it/org/xwiki/administration/test/ui/ResetPasswordIT.java +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-authentication/xwiki-platform-security-authentication-test/xwiki-platform-security-authentication-test-docker/src/test/it/org/xwiki/security/authentication/test/ui/ResetPasswordIT.java @@ -17,7 +17,7 @@ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA * 02110-1301 USA, or see the FSF site: http://www.fsf.org. */ -package org.xwiki.administration.test.ui; +package org.xwiki.security.authentication.test.ui; import java.util.HashMap; import java.util.Map; @@ -60,19 +60,16 @@ // Open the GreenMail port so that the XWiki instance inside a Docker container can use the SMTP server provided // by GreenMail running on the host. 3025 -}, + }, properties = { // The Mail module contributes a Hibernate mapping that needs to be added to hibernate.cfg.xml - "xwikiDbHbmCommonExtraMappings=mailsender.hbm.xml", - // Pages created in the tests need to have PR since we ask for PR to send mails, so we need to exclude them from - // the PR checker. - "xwikiPropertiesAdditionalProperties=test.prchecker.excludePattern=.*:XWiki\\." - + "(ResetPassword|ResetPasswordComplete)" + "xwikiDbHbmCommonExtraMappings=mailsender.hbm.xml,notification-filter-preferences.hbm.xml", }, extraJARs = { // It's currently not possible to install a JAR contributing a Hibernate mapping file as an Extension. Thus // we need to provide the JAR inside WEB-INF/lib. See https://jira.xwiki.org/browse/XWIKI-19932 - "org.xwiki.platform:xwiki-platform-mail-send-storage" + "org.xwiki.platform:xwiki-platform-mail-send-storage", + "org.xwiki.platform:xwiki-platform-notifications-filters-default" } ) public class ResetPasswordIT @@ -102,7 +99,7 @@ public void stopMail(TestUtils setup, LogCaptureConfiguration logCaptureConfigur @Test public void resetForgottenPassword(TestUtils setup) throws Exception { - setup.loginAsSuperAdmin(); + setup.forceGuestUser(); String userName = "testUser" + RandomStringUtils.randomAlphanumeric(6); String password = "password"; @@ -110,9 +107,6 @@ public void resetForgottenPassword(TestUtils setup) throws Exception // Create a user setup.createUser(userName, password, null); - - // Make sure we are not logged in and go to the reset password page - setup.forceGuestUser(); ResetPasswordPage resetPasswordPage = ResetPasswordPage.gotoPage(); // Try to reset the password of a non existent user @@ -242,6 +236,7 @@ private String getResetLink(TestUtils setup, String emailContent, String userNam private void configureEmail(TestUtils setup, TestConfiguration testConfiguration) { + setup.loginAsSuperAdmin(); setup.updateObject("Mail", "MailConfig", "Mail.SendMailConfigClass", 0, "host", testConfiguration.getServletEngine().getHostIP(), "port", "3025", "sendWaitTime", "0"); } diff --git a/xwiki-platform-tools/xwiki-platform-tool-configuration-resources/src/main/resources/xwiki.properties.vm b/xwiki-platform-tools/xwiki-platform-tool-configuration-resources/src/main/resources/xwiki.properties.vm index 9e74537943db..1e7de2ee0e7e 100644 --- a/xwiki-platform-tools/xwiki-platform-tool-configuration-resources/src/main/resources/xwiki.properties.vm +++ b/xwiki-platform-tools/xwiki-platform-tool-configuration-resources/src/main/resources/xwiki.properties.vm @@ -804,14 +804,13 @@ distribution.automaticStartOnWiki=$xwikiPropertiesAutomaticStartOnWiki #-# [Since 13.10.1] #-# [Since 14.0RC1] -#-# Define the lifetime of the token used for resetting passwords in minutes. Note that this value is only used after -#-# first access. -#-# Default value is 0 meaning that the token is immediately revoked when first accessed. -#-# Use a different value if the reset password email link might be accessed several times (e.g. in case of using an -#-# email link verification system): in such case the user will have the defined lifetime to use again the email link. +#-# Define the lifetime of the token used for resetting passwords in minutes. +#-# Default value is 60 meaning that users have 1 hour to access the link sent by email for performing password reset. +#-# Once the password is reset using the link, the token is revoked. +#-# Using 0 here means that the token has no expiration date, however it will be revoked at first wrong access. #-# -#-# The default is: -# security.authentication.resetPasswordTokenLifetime = 0 +#-# The value is in minutes. The default is: +# security.authentication.resetPasswordTokenLifetime = 60 #-# [Since 14.6RC1] #-# [Since 14.4.3]