self service improvement

docs/UAA-Configuration-Reference.md

@@ -1758,6 +1758,13 @@ List of usernames to delete on bootstrap. Used to remove specific users during s
 
 When `true`, "Create Account" and "Forgot Password" links are displayed on the login page.
 
+> **Note on self-service account confirmation.** Clicking the activation link in the confirmation
+> email verifies ownership of the email address but does **not** activate the password typed on the
+> registration form. The owner is sent to a password-setup page (stating that a password change is
+> required due to changes in the system) and must establish their own password through the link
+> before the account can be used to sign in. This ensures a password chosen before the email address
+> was verified can never be used to authenticate.
+
 [Back to table](#login--branding)
 
 ---

server/src/main/java/org/cloudfoundry/identity/uaa/account/AccountCreationService.java

@@ -19,6 +19,8 @@ class AccountCreationResponse {
         private String email;
         @JsonProperty("redirect_location")
         private String redirectLocation;
+        @JsonProperty("password_reset_code")
+        private String passwordResetCode;
 
         public AccountCreationResponse(String userId, String username, String email, String redirectLocation) {
             this.userId = userId;
@@ -50,5 +52,13 @@ public String getRedirectLocation() {
         public String getEmail() {
             return email;
         }
+
+        public String getPasswordResetCode() {
+            return passwordResetCode;
+        }
+
+        public void setPasswordResetCode(String passwordResetCode) {
+            this.passwordResetCode = passwordResetCode;
+        }
     }
 }

server/src/main/java/org/cloudfoundry/identity/uaa/account/AccountsController.java

@@ -124,12 +124,10 @@ public String verifyUser(Model model,
             return "accounts/link_prompt";
         }
 
-        String redirectLocation = accountCreation.getRedirectLocation();
-        String res = "redirect:/login?success=verify_success";
-        if (!redirectLocation.equals(accountCreationService.getDefaultRedirect())) {
-            res += "&form_redirect_uri=" + redirectLocation;
-        }
-        return res;
+        // Confirmation establishes email ownership but leaves the account without a usable
+        // password. Send the owner to the reset-password page to set their own password; any
+        // client redirect is carried by the reset code and applied once the password is set.
+        return "redirect:/reset_password?code=" + accountCreation.getPasswordResetCode() + "&force_change=true";
     }
 
     private String handleUnprocessableEntity(Model model, HttpServletResponse response, String attributeKey, String attributeValue) {

server/src/main/java/org/cloudfoundry/identity/uaa/account/EmailAccountCreationService.java

@@ -14,6 +14,7 @@
 import org.cloudfoundry.identity.uaa.scim.exception.ScimResourceAlreadyExistsException;
 import org.cloudfoundry.identity.uaa.scim.util.ScimUtils;
 import org.cloudfoundry.identity.uaa.scim.validate.PasswordValidator;
+import org.cloudfoundry.identity.uaa.util.AlphanumericRandomValueStringGenerator;
 import org.cloudfoundry.identity.uaa.util.JsonUtils;
 import org.cloudfoundry.identity.uaa.zone.IdentityZone;
 import org.cloudfoundry.identity.uaa.zone.MergedZoneBrandingInformation;
@@ -52,6 +53,7 @@ public class EmailAccountCreationService implements AccountCreationService {
     private final ScimUserProvisioning scimUserProvisioning;
     private final MultitenantClientServices clientDetailsService;
     private final PasswordValidator passwordValidator;
+    private final ResetPasswordService resetPasswordService;
     private final IdentityZoneManager identityZoneManager;
 
     public EmailAccountCreationService(
@@ -61,6 +63,7 @@ public EmailAccountCreationService(
             ScimUserProvisioning scimUserProvisioning,
             MultitenantClientServices clientDetailsService,
             PasswordValidator passwordValidator,
+            ResetPasswordService resetPasswordService,
             IdentityZoneManager identityZoneManager) {
 
         this.templateEngine = templateEngine;
@@ -69,6 +72,7 @@ public EmailAccountCreationService(
         this.scimUserProvisioning = scimUserProvisioning;
         this.clientDetailsService = clientDetailsService;
         this.passwordValidator = passwordValidator;
+        this.resetPasswordService = resetPasswordService;
         this.identityZoneManager = identityZoneManager;
     }
 
@@ -121,14 +125,28 @@ public AccountCreationResponse completeActivation(String code) {
 
         Map<String, String> data = JsonUtils.readValue(expiringCode.getData(), new TypeReference<Map<String, String>>() {
         });
-        ScimUser user = scimUserProvisioning.retrieve(data.get("user_id"), identityZoneManager.getCurrentIdentityZoneId());
-        user = scimUserProvisioning.verifyUser(user.getId(), user.getVersion(), identityZoneManager.getCurrentIdentityZoneId());
+        String zoneId = identityZoneManager.getCurrentIdentityZoneId();
+        ScimUser user = scimUserProvisioning.retrieve(data.get("user_id"), zoneId);
+        user = scimUserProvisioning.verifyUser(user.getId(), user.getVersion(), zoneId);
+
+        // Confirming the activation link proves ownership of the email address, but it does not
+        // prove that the person confirming is the one who chose the password on the registration
+        // form. Discard the registration password so it can never authenticate, and require the
+        // confirming owner to establish their own password through the ownership-proving link.
+        scimUserProvisioning.changePassword(user.getId(), null, new AlphanumericRandomValueStringGenerator(40).generate(), zoneId);
+        scimUserProvisioning.updatePasswordChangeRequired(user.getId(), true, zoneId);
 
         String clientId = data.get("client_id");
         String redirectUri = data.get("redirect_uri") != null ? data.get("redirect_uri") : "";
         String redirectLocation = getRedirect(clientId, redirectUri);
 
-        return new AccountCreationResponse(user.getId(), user.getUserName(), user.getUserName(), redirectLocation);
+        // Issue an ownership-bound, single-use code so the owner sets their password through the
+        // same reset flow used for password recovery (no previous password is required).
+        ForgotPasswordInfo forgotPasswordInfo = resetPasswordService.forgotPassword(user.getUserName(), clientId, redirectUri);
+
+        AccountCreationResponse response = new AccountCreationResponse(user.getId(), user.getUserName(), user.getUserName(), redirectLocation);
+        response.setPasswordResetCode(forgotPasswordInfo.getResetPasswordCode().getCode());
+        return response;
     }
 
     private String getRedirect(String clientId, String redirectUri) {

server/src/main/java/org/cloudfoundry/identity/uaa/account/ResetPasswordController.java

@@ -176,7 +176,8 @@ public void resetPassword() {
     @GetMapping(value = "/reset_password", params = {"code"})
     public String resetPasswordPage(Model model,
             HttpServletResponse response,
-            @RequestParam("code") String code) {
+            @RequestParam("code") String code,
+            @RequestParam(value = "force_change", required = false) boolean forceChange) {
 
         ExpiringCode expiringCode = checkIfUserExists(codeStore.retrieveCode(code, identityZoneManager.getCurrentIdentityZone().getId()));
         if (expiringCode == null) {
@@ -189,6 +190,9 @@ public String resetPasswordPage(Model model,
             model.addAttribute("code", newCode);
             model.addAttribute("email", uaaUser.getEmail());
             model.addAttribute("username", uaaUser.getUsername());
+            if (forceChange) {
+                model.addAttribute("message_code", "force_change");
+            }
             return "reset_password";
         }
     }

server/src/test/java/org/cloudfoundry/identity/uaa/login/AccountsControllerTest.java

@@ -203,30 +203,32 @@ void passwordMismatch() throws Exception {
 
     @Test
     void verifyUser() throws Exception {
-        when(accountCreationService.completeActivation("the_secret_code"))
-                .thenReturn(new AccountCreationService.AccountCreationResponse("newly-created-user-id", "username", "user@example.com", "//example.com/callback"));
+        AccountCreationService.AccountCreationResponse response = new AccountCreationService.AccountCreationResponse("newly-created-user-id", "username", "user@example.com", "//example.com/callback");
+        response.setPasswordResetCode("the_reset_code");
+        when(accountCreationService.completeActivation("the_secret_code")).thenReturn(response);
 
         MockHttpServletRequestBuilder get = get("/verify_user")
                 .param("code", "the_secret_code");
 
         mockMvc.perform(get)
                 .andExpect(status().isFound())
-                .andExpect(redirectedUrl("/login?success=verify_success&form_redirect_uri=//example.com/callback"));
+                .andExpect(redirectedUrl("/reset_password?code=the_reset_code&force_change=true"));
 
         assertThat(SecurityContextHolder.getContext().getAuthentication()).isNull();
     }
 
     @Test
     void verifyUserWithPriorHeadRequest() throws Exception {
-        when(accountCreationService.completeActivation("the_secret_code"))
-                .thenReturn(new AccountCreationService.AccountCreationResponse("newly-created-user-id", "username", "user@example.com", "//example.com/callback"));
+        AccountCreationService.AccountCreationResponse response = new AccountCreationService.AccountCreationResponse("newly-created-user-id", "username", "user@example.com", "//example.com/callback");
+        response.setPasswordResetCode("the_reset_code");
+        when(accountCreationService.completeActivation("the_secret_code")).thenReturn(response);
 
         mockMvc.perform(head("/verify_user").param("code", "the_secret_code"))
                 .andExpect(status().isFound())
                 .andExpect(redirectedUrl("/login"));
         mockMvc.perform(get("/verify_user").param("code", "the_secret_code"))
                 .andExpect(status().isFound())
-                .andExpect(redirectedUrl("/login?success=verify_success&form_redirect_uri=//example.com/callback"));
+                .andExpect(redirectedUrl("/reset_password?code=the_reset_code&force_change=true"));
 
         assertThat(SecurityContextHolder.getContext().getAuthentication()).isNull();
         Mockito.verify(accountCreationService, times(1)).completeActivation("the_secret_code");

server/src/test/java/org/cloudfoundry/identity/uaa/login/EmailAccountCreationServiceTests.java

@@ -3,6 +3,8 @@
 import org.assertj.core.api.InstanceOfAssertFactories;
 import org.cloudfoundry.identity.uaa.account.AccountCreationService;
 import org.cloudfoundry.identity.uaa.account.EmailAccountCreationService;
+import org.cloudfoundry.identity.uaa.account.ForgotPasswordInfo;
+import org.cloudfoundry.identity.uaa.account.ResetPasswordService;
 import org.cloudfoundry.identity.uaa.codestore.ExpiringCode;
 import org.cloudfoundry.identity.uaa.codestore.ExpiringCodeStore;
 import org.cloudfoundry.identity.uaa.constants.OriginKeys;
@@ -72,6 +74,7 @@ class EmailAccountCreationServiceTests {
     private MultitenantClientServices mockClientDetailsService;
     private ClientDetails mockClientDetails;
     private PasswordValidator mockPasswordValidator;
+    private ResetPasswordService mockResetPasswordService;
     private IdentityZoneManager mockIdentityZoneManager;
     private ScimUser user;
     private ExpiringCode code;
@@ -91,6 +94,7 @@ void setUp() {
         mockClientDetailsService = mock(MultitenantClientServices.class);
         mockClientDetails = mock(ClientDetails.class);
         mockPasswordValidator = mock(PasswordValidator.class);
+        mockResetPasswordService = mock(ResetPasswordService.class);
         mockIdentityZoneManager = mock(IdentityZoneManager.class);
         emailAccountCreationService = initEmailAccountCreationService();
 
@@ -104,6 +108,10 @@ void setUp() {
         currentIdentityZoneId = "zoneId" + randomValueStringGenerator.generate();
 
         when(mockIdentityZoneManager.getCurrentIdentityZoneId()).thenReturn(currentIdentityZoneId);
+
+        ExpiringCode resetCode = new ExpiringCode("reset_password_code", new Timestamp(System.currentTimeMillis()), "{}", "forgot_password_for_id:newly-created-user-id");
+        when(mockResetPasswordService.forgotPassword(anyString(), any(), any()))
+                .thenReturn(new ForgotPasswordInfo("newly-created-user-id", "user@example.com", resetCode));
     }
 
     private EmailAccountCreationService initEmailAccountCreationService() {
@@ -114,6 +122,7 @@ private EmailAccountCreationService initEmailAccountCreationService() {
                 mockScimUserProvisioning,
                 mockClientDetailsService,
                 mockPasswordValidator,
+                mockResetPasswordService,
                 mockIdentityZoneManager
         );
     }

uaa/src/main/resources/messages.properties

@@ -39,6 +39,7 @@ change_password.unauthorized=Please enter a valid current password.
 forgot_password.form_error=Please enter a valid email address.
 forgot_password.bad_code=Sorry, your reset password link is no longer valid. You can request another one below.
 reset_password.form_error=Passwords must match and not be empty.
+reset_password.force_change=A password change is required due to changes in the system. Please set a new password.
 create_account.form_error=Passwords must match and not be empty.
 create_account.other_idp=Account sign-up is not required for this email domain. Please login with the identity provider
 force_password_change.form_error=Passwords must match and not be empty.

uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/CreateAccountIT.java

@@ -95,8 +95,14 @@ void userInitiatedSignup() throws Exception {
                 .doesNotContain("%40");
 
         webDriver.get(link);
-        assertThat(webDriver.findElement(By.cssSelector("h1")).getText()).doesNotContain("Where to?");
+        // Confirming the account lands on the forced password-set page, not a sign-in page.
+        assertThat(webDriver.findElement(By.cssSelector("h1")).getText()).isEqualTo("Reset Password");
 
+        webDriver.findElement(By.name("password")).sendKeys(SECRET);
+        webDriver.findElement(By.name("password_confirmation")).sendKeys(SECRET);
+        webDriver.clickAndWait(By.xpath("//input[@value='Create new password']"));
+
+        // The owner can now sign in with the password they set.
         webDriver.findElement(By.name("username")).sendKeys(userEmail);
         webDriver.findElement(By.name("password")).sendKeys(SECRET);
         webDriver.clickAndWait(By.xpath("//input[@value='Sign in']"));
@@ -130,8 +136,14 @@ void clientInitiatedSignup() throws Exception {
         assertThat(link).isNotEmpty();
 
         webDriver.get(link);
-        assertThat(webDriver.findElement(By.cssSelector("h1")).getText()).doesNotContain("Where to?");
+        // Confirming the account lands on the forced password-set page, not a sign-in page.
+        assertThat(webDriver.findElement(By.cssSelector("h1")).getText()).isEqualTo("Reset Password");
+
+        webDriver.findElement(By.name("password")).sendKeys(SECRET);
+        webDriver.findElement(By.name("password_confirmation")).sendKeys(SECRET);
+        webDriver.clickAndWait(By.xpath("//input[@value='Create new password']"));
 
+        // The owner can now sign in with the password they set.
         webDriver.findElement(By.name("username")).sendKeys(userEmail);
         webDriver.findElement(By.name("password")).sendKeys(SECRET);
         webDriver.clickAndWait(By.xpath("//input[@value='Sign in']"));

uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/util/IntegrationTestUtils.java

@@ -1618,6 +1618,12 @@ public static String createAnotherUser(UaaWebDriver webDriver, String password,
             throw new RuntimeException(e);
         }
 
+        // Confirming the account now lands on a forced password-set page; the owner must establish
+        // their password through the link before the account can be used to log in.
+        webDriver.findElement(By.name("password")).sendKeys(password);
+        webDriver.findElement(By.name("password_confirmation")).sendKeys(password);
+        webDriver.clickAndWait(By.xpath("//input[@value='Create new password']"));
+
         return userEmail;
     }