XWIKI-18787: Authentication API does not return proper results (#1651)

* Refactor a bit the Authentication API to be more sound * Fix related tests * Fix missing escape
xwiki · Jun 29, 2021 · d8a3cce · d8a3cce
1 parent 607934c
commit d8a3cce
Show file tree

Hide file tree

Showing 11 changed files with 222 additions and 213 deletions.
diff --git a/xwiki-platform-core/pom.xml b/xwiki-platform-core/pom.xml
@@ -156,6 +156,24 @@
                    </differences>
                  </revapi.differences>
             -->
+            <revapi.differences>
+              <criticality>highlight</criticality>
+              <differences>
+                <item>
+                  <ignore>true</ignore>
+                  <code>java.method.returnTypeChanged</code>
+                  <old>method javax.mail.internet.InternetAddress org.xwiki.security.authentication.script.AuthenticationScriptService::requestResetPassword(org.xwiki.user.UserReference) throws org.xwiki.security.authentication.ResetPasswordException</old>
+                  <new>method void org.xwiki.security.authentication.script.AuthenticationScriptService::requestResetPassword(org.xwiki.user.UserReference) throws org.xwiki.security.authentication.ResetPasswordException</new>
+                  <justification>Unstable API: this API was not properly designed and shouldn't have returned that in first place.</justification>
+                </item>
+                <item>
+                  <ignore>true</ignore>
+                  <code>java.method.removed</code>
+                  <old>method javax.mail.internet.InternetAddress org.xwiki.security.authentication.ResetPasswordRequestResponse::getUserEmail()</old>
+                  <justification>Unstable API: this API was not properly designed and shouldn't have returned that in first place.</justification>
+                </item>
+              </differences>
+            </revapi.differences>
           </analysisConfiguration>
         </configuration>
       </plugin>

diff --git a/...inistration-test-docker/src/test/it/org/xwiki/administration/test/ui/ResetPasswordIT.java b/...inistration-test-docker/src/test/it/org/xwiki/administration/test/ui/ResetPasswordIT.java
@@ -117,17 +117,19 @@ public void resetForgottenPassword(TestUtils setup) throws Exception
         // Try to reset the password of a non existent user
         resetPasswordPage.setUserName("SomeUserThatDoesNotExist");
         resetPasswordPage = resetPasswordPage.clickResetPassword();
-        assertFalse(resetPasswordPage.isResetPasswordSent());
-        assertTrue(resetPasswordPage.getMessage().contains("user does not exist"));
+
+        // there should not have any indication if the user exists or not.
+        assertTrue(resetPasswordPage.isFormSubmitted());
 
         // Try again
-        resetPasswordPage = resetPasswordPage.clickRetry();
+        resetPasswordPage = ResetPasswordPage.gotoPage();
 
         // Try to reset the password of our user, when he has no email set
         resetPasswordPage.setUserName(userName);
         resetPasswordPage.clickResetPassword();
-        assertFalse(resetPasswordPage.isResetPasswordSent());
-        assertTrue(resetPasswordPage.getMessage().contains("email address not provided"));
+
+        // there should not have any indication if an email address is provided or not.
+        assertTrue(resetPasswordPage.isFormSubmitted());
 
         // Try again. This time, set the user's email address in the profile
         setup.loginAsSuperAdmin();
@@ -145,7 +147,7 @@ public void resetForgottenPassword(TestUtils setup) throws Exception
             "Actual message: " + newResetPasswordPage.getMessage());
 
         // Check the result
-        assertTrue(resetPasswordPage.isResetPasswordSent());
+        assertTrue(resetPasswordPage.isFormSubmitted());
         // Check the emails received by the user
         assertTrue(this.mail.waitForIncomingEmail(1));
         MimeMessage[] receivedEmails = this.mail.getReceivedMessages();

diff --git a/...on-test-pageobjects/src/main/java/org/xwiki/administration/test/po/ResetPasswordPage.java b/...on-test-pageobjects/src/main/java/org/xwiki/administration/test/po/ResetPasswordPage.java
@@ -67,7 +67,13 @@ public ResetPasswordPage clickResetPassword()
         return new ResetPasswordPage();
     }
 
-    public boolean isResetPasswordSent()
+    /**
+     * This method only checks if the form was properly submitted and didn't return an error.
+     * It does not mean that an email was necessarily sent.
+     *
+     * @return {@code true} if the form is properly submitted.
+     */
+    public boolean isFormSubmitted()
     {
         // If there is no form and we see an info box, then the request was sent.
         return !getDriver().hasElementWithoutWaiting(By.cssSelector("#resetPasswordForm"))

diff --git a/...i-platform-core/xwiki-platform-oldcore/src/main/resources/ApplicationResources.properties b/...i-platform-core/xwiki-platform-oldcore/src/main/resources/ApplicationResources.properties
@@ -2449,7 +2449,7 @@ xe.admin.passwordReset.title=Forgot your password?
 xe.admin.passwordReset.instructions=Please enter your username to start the password reset process.
 xe.admin.passwordReset.username.label=Username
 xe.admin.passwordReset.submit=Reset password
-xe.admin.passwordReset.emailSent=An e-mail was sent to {0}. Please follow the instructions in that e-mail to complete the password reset procedure.
+xe.admin.passwordReset.emailSentToUsername=An e-mail was sent to the address configured for user "{0}". Please follow the instructions in that e-mail to complete the password reset procedure.
 xe.admin.passwordReset.login=Login \u00BB
 xe.admin.passwordReset.error.noUser=The {0} user does not exist.
 xe.admin.passwordReset.error.ldapUser=The {0} user is an LDAP user. In that case the password has to be changed on the LDAP server.
@@ -5501,6 +5501,11 @@ xe.admin.forgotUsername.result=Your username is: {0}
 xe.admin.forgotUsername.multipleResults=The following usernames are registered with this email address:
 xe.admin.forgotUsername.error.noAccount=No account is registered using this email address.
 
+#######################################
+## until 12.10.9, 13.6RC1, 13.4.1
+#######################################
+xe.admin.passwordReset.emailSent=An e-mail was sent to {0}. Please follow the instructions in that e-mail to complete the password reset procedure.
+
 ## Used to indicate where deprecated keys end
 #@deprecatedend
 

diff --git a/...ion-api/src/main/java/org/xwiki/security/authentication/ResetPasswordRequestResponse.java b/...ion-api/src/main/java/org/xwiki/security/authentication/ResetPasswordRequestResponse.java
@@ -19,8 +19,6 @@
  */
 package org.xwiki.security.authentication;
 
-import javax.mail.internet.InternetAddress;
-
 import org.xwiki.stability.Unstable;
 import org.xwiki.user.UserReference;
 
@@ -38,11 +36,6 @@ public interface ResetPasswordRequestResponse
      */
     UserReference getUserReference();
 
-    /**
-     * @return the email address of the user for whom the reset password request have been performed.
-     */
-    InternetAddress getUserEmail();
-
     /**
      * @return the verification code to be send to the user.
      */