CVE-2015-3189

@@ -1,5 +1,5 @@
 /*******************************************************************************
- *     Cloud Foundry 
+ *     Cloud Foundry
  *     Copyright (c) [2009-2014] Pivotal Software, Inc. All Rights Reserved.
  *
  *     This product is licensed to you under the Apache License, Version 2.0 (the "License").
@@ -15,6 +15,7 @@
 import java.sql.Timestamp;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Date;
 import java.util.Map;
 
 import static org.hamcrest.Matchers.containsString;
@@ -30,6 +31,7 @@
 import org.cloudfoundry.identity.uaa.codestore.ExpiringCodeStore;
 import org.cloudfoundry.identity.uaa.error.UaaException;
 import org.cloudfoundry.identity.uaa.login.test.ThymeleafConfig;
+import org.cloudfoundry.identity.uaa.scim.ScimMeta;
 import org.cloudfoundry.identity.uaa.scim.ScimUser;
 import org.cloudfoundry.identity.uaa.scim.ScimUserProvisioning;
 import org.cloudfoundry.identity.uaa.scim.endpoints.PasswordResetEndpoints;
@@ -193,6 +195,7 @@ public void testForgotPasswordWhenTheCodeIsDenied() throws Exception {
     @Test
     public void testResetPassword() throws Exception {
         ScimUser user = new ScimUser("usermans-id","userman","firstName","lastName");
+        user.setMeta(new ScimMeta(new Date(System.currentTimeMillis()-(1000*60*60*24)), new Date(System.currentTimeMillis()-(1000*60*60*24)), 0));
         user.setPrimaryEmail("user@example.com");
         when(scimUserProvisioning.retrieve(eq("usermans-id"))).thenReturn(user);
         when(codeStore.retrieveCode(eq("secret_code"))).thenReturn(new ExpiringCode("code", new Timestamp(System.currentTimeMillis()), "usermans-id"));

@@ -1,5 +1,5 @@
 /*******************************************************************************
- *     Cloud Foundry 
+ *     Cloud Foundry
  *     Copyright (c) [2009-2014] Pivotal Software, Inc. All Rights Reserved.
  *
  *     This product is licensed to you under the Apache License, Version 2.0 (the "License").
@@ -22,6 +22,8 @@
 import org.cloudfoundry.identity.uaa.scim.ScimUserProvisioning;
 import org.cloudfoundry.identity.uaa.scim.exception.ScimResourceNotFoundException;
 import org.cloudfoundry.identity.uaa.user.UaaUser;
+import org.cloudfoundry.identity.uaa.util.JsonUtils;
+import org.cloudfoundry.identity.uaa.util.JsonUtils.JsonUtilException;
 import org.codehaus.jackson.annotate.JsonProperty;
 import org.codehaus.jackson.map.ObjectMapper;
 import org.springframework.context.ApplicationEvent;
@@ -49,6 +51,7 @@
 import static org.springframework.http.HttpStatus.NOT_FOUND;
 import static org.springframework.http.HttpStatus.OK;
 import static org.springframework.http.HttpStatus.UNAUTHORIZED;
+import static org.springframework.http.HttpStatus.UNPROCESSABLE_ENTITY;
 
 @Controller
 public class PasswordResetEndpoints implements ApplicationEventPublisherAware {
@@ -85,7 +88,8 @@ public ResponseEntity<Map<String,String>> resetPassword(@RequestBody String emai
             }
         }
         ScimUser scimUser = results.get(0);
-        String code = expiringCodeStore.generateCode(scimUser.getId(), new Timestamp(System.currentTimeMillis() + PASSWORD_RESET_LIFETIME)).getCode();
+        PasswordChange change = new PasswordChange(scimUser.getId(), scimUser.getUserName());
+        String code = expiringCodeStore.generateCode(JsonUtils.writeValueAsString(change), new Timestamp(System.currentTimeMillis() + PASSWORD_RESET_LIFETIME)).getCode();
         publish(new ResetPasswordRequestEvent(email, code, SecurityContextHolder.getContext().getAuthentication()));
         response.put("code", code);
         response.put("user_id", scimUser.getId());
@@ -139,20 +143,31 @@ private ResponseEntity<Map<String,String>> changePasswordUsernamePasswordAuthent
         }
     }
 
-    private ResponseEntity<Map<String,String>> changePasswordCodeAuthenticated(PasswordChange passwordChange) {
+    protected ResponseEntity<Map<String,String>> changePasswordCodeAuthenticated(PasswordChange passwordChange) {
         ExpiringCode expiringCode = expiringCodeStore.retrieveCode(passwordChange.getCode());
         if (expiringCode == null) {
             return new ResponseEntity<>(BAD_REQUEST);
         }
-        String userId = expiringCode.getData();
+        String userId;
+        String userName = null;
+        try {
+            PasswordChange change = JsonUtils.readValue(expiringCode.getData(), PasswordChange.class);
+            userId = change.getUserId();
+            userName = change.getUsername();
+        } catch (JsonUtilException x) {
+            userId = expiringCode.getData();
+        }
         ScimUser user = scimUserProvisioning.retrieve(userId);
+        Map<String,String> userInfo = new HashMap<>();
         try {
+            if (isUserModified(user, expiringCode.getExpiresAt(), userName)) {
+                return new ResponseEntity<>(BAD_REQUEST);
+            }
             if (!user.isVerified()) {
                 scimUserProvisioning.verifyUser(userId, -1);
             }
             scimUserProvisioning.changePassword(userId, null, passwordChange.getNewPassword());
             publish(new PasswordChangeEvent("Password changed", getUaaUser(user), SecurityContextHolder.getContext().getAuthentication()));
-            Map<String,String> userInfo = new HashMap<>();
             userInfo.put("user_id", user.getId());
             userInfo.put("username", user.getUserName());
             userInfo.put("email", user.getPrimaryEmail());
@@ -169,7 +184,19 @@ private ResponseEntity<Map<String,String>> changePasswordCodeAuthenticated(Passw
         }
     }
 
-    private UaaUser getUaaUser(ScimUser scimUser) {
+    protected boolean isUserModified(ScimUser user, Timestamp expiresAt, String userName) {
+        if (userName!=null) {
+            return ! userName.equals(user.getUserName());
+        }
+        //left over from when all we stored in the code was the user ID
+        //here we will check the timestamp
+        //TODO - REMOVE THIS IN FUTURE RELEASE, ALL LINKS HAVE BEEN EXPIRED (except test created ones)
+        long codeCreated = expiresAt.getTime() - PASSWORD_RESET_LIFETIME;
+        long userModified = user.getMeta().getLastModified().getTime();
+        return (userModified > codeCreated);
+    }
+
+    protected UaaUser getUaaUser(ScimUser scimUser) {
         Date today = new Date();
         return new UaaUser(scimUser.getId(), scimUser.getUserName(), "N/A", scimUser.getPrimaryEmail(), null,
             scimUser.getGivenName(),
@@ -178,6 +205,16 @@ private UaaUser getUaaUser(ScimUser scimUser) {
     }
 
     public static class PasswordChange {
+        public PasswordChange() {}
+
+        public PasswordChange(String userId, String username) {
+            this.userId = userId;
+            this.username = username;
+        }
+
+        @JsonProperty("user_id")
+        private String userId;
+
         @JsonProperty("username")
         private String username;
 
@@ -221,6 +258,14 @@ public String getNewPassword() {
         public void setNewPassword(String newPassword) {
             this.newPassword = newPassword;
         }
+
+        public String getUserId() {
+            return userId;
+        }
+
+        public void setUserId(String userId) {
+            this.userId = userId;
+        }
     }
 
     protected void publish(ApplicationEvent event) {

@@ -16,9 +16,12 @@
 import org.cloudfoundry.identity.uaa.authentication.Origin;
 import org.cloudfoundry.identity.uaa.codestore.ExpiringCode;
 import org.cloudfoundry.identity.uaa.codestore.ExpiringCodeStore;
+import org.cloudfoundry.identity.uaa.scim.ScimMeta;
 import org.cloudfoundry.identity.uaa.scim.ScimUser;
 import org.cloudfoundry.identity.uaa.scim.ScimUserProvisioning;
+import org.cloudfoundry.identity.uaa.scim.endpoints.PasswordResetEndpoints.PasswordChange;
 import org.cloudfoundry.identity.uaa.test.MockAuthentication;
+import org.cloudfoundry.identity.uaa.util.JsonUtils;
 import org.codehaus.jackson.map.ObjectMapper;
 import org.junit.Before;
 import org.junit.Test;
@@ -30,7 +33,9 @@
 
 import java.sql.Timestamp;
 import java.util.Arrays;
+import java.util.Date;
 
+import static org.cloudfoundry.identity.uaa.scim.endpoints.PasswordResetEndpoints.PASSWORD_RESET_LIFETIME;
 import static org.hamcrest.Matchers.containsString;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
@@ -54,12 +59,22 @@ public void setUp() throws Exception {
         mockMvc = MockMvcBuilders.standaloneSetup(controller).build();
 
         Mockito.when(expiringCodeStore.generateCode(eq("id001"), any(Timestamp.class)))
-                .thenReturn(new ExpiringCode("secret_code", new Timestamp(System.currentTimeMillis() + 1000), "id001"));
+                .thenReturn(new ExpiringCode("secret_code", new Timestamp(System.currentTimeMillis() + PASSWORD_RESET_LIFETIME), "id001"));
+
+        PasswordChange change = new PasswordChange("id001", "user@example.com");
+        Mockito.when(expiringCodeStore.generateCode(eq(JsonUtils.writeValueAsString(change)), any(Timestamp.class)))
+            .thenReturn(new ExpiringCode("secret_code", new Timestamp(System.currentTimeMillis() + PASSWORD_RESET_LIFETIME), "id001"));
+
+        change = new PasswordChange("id001", "user\"'@example.com");
+        Mockito.when(expiringCodeStore.generateCode(eq(JsonUtils.writeValueAsString(change)), any(Timestamp.class)))
+            .thenReturn(new ExpiringCode("secret_code", new Timestamp(System.currentTimeMillis() + PASSWORD_RESET_LIFETIME), "id001"));
+
     }
 
     @Test
     public void testCreatingAPasswordResetWhenTheUsernameExists() throws Exception {
         ScimUser user = new ScimUser("id001", "user@example.com", null, null);
+        user.setMeta(new ScimMeta(new Date(System.currentTimeMillis()-(1000*60*60*24)), new Date(System.currentTimeMillis()-(1000*60*60*24)), 0));
         user.addEmail("user@example.com");
         Mockito.when(scimUserProvisioning.query("userName eq \"user@example.com\" and origin eq \"" + Origin.UAA + "\""))
                 .thenReturn(Arrays.asList(user));
@@ -95,6 +110,7 @@ public void testCreatingAPasswordResetWhenTheUserHasNonUaaOrigin() throws Except
             .thenReturn(Arrays.<ScimUser>asList());
 
         ScimUser user = new ScimUser("id001", "user@example.com", null, null);
+        user.setMeta(new ScimMeta(new Date(System.currentTimeMillis()-(1000*60*60*24)), new Date(System.currentTimeMillis()-(1000*60*60*24)), 0));
         user.addEmail("user@example.com");
         user.setOrigin(Origin.LDAP);
         Mockito.when(scimUserProvisioning.query("userName eq \"user@example.com\""))
@@ -113,6 +129,7 @@ public void testCreatingAPasswordResetWhenTheUserHasNonUaaOrigin() throws Except
     @Test
     public void testCreatingAPasswordResetWithAUsernameContainingSpecialCharacters() throws Exception {
         ScimUser user = new ScimUser("id001", "user\"'@example.com", null, null);
+        user.setMeta(new ScimMeta(new Date(System.currentTimeMillis()-(1000*60*60*24)), new Date(System.currentTimeMillis()-(1000*60*60*24)), 0));
         user.addEmail("user\"'@example.com");
         Mockito.when(scimUserProvisioning.query("userName eq \"user\\\"'@example.com\" and origin eq \"" + Origin.UAA + "\""))
             .thenReturn(Arrays.asList(user));
@@ -146,9 +163,10 @@ public void testCreatingAPasswordResetWithAUsernameContainingSpecialCharacters()
     @Test
     public void testChangingAPasswordWithAValidCode() throws Exception {
         Mockito.when(expiringCodeStore.retrieveCode("secret_code"))
-                .thenReturn(new ExpiringCode("secret_code", new Timestamp(System.currentTimeMillis()), "eyedee"));
+                .thenReturn(new ExpiringCode("secret_code", new Timestamp(System.currentTimeMillis()+ PASSWORD_RESET_LIFETIME), "eyedee"));
 
         ScimUser scimUser = new ScimUser("eyedee", "user@example.com", "User", "Man");
+        scimUser.setMeta(new ScimMeta(new Date(System.currentTimeMillis()-(1000*60*60*24)), new Date(System.currentTimeMillis()-(1000*60*60*24)), 0));
         scimUser.addEmail("user@example.com");
         Mockito.when(scimUserProvisioning.retrieve("eyedee")).thenReturn(scimUser);
 
@@ -170,9 +188,10 @@ public void testChangingAPasswordWithAValidCode() throws Exception {
     @Test
     public void testChangingAPasswordForUnverifiedUser() throws Exception {
         Mockito.when(expiringCodeStore.retrieveCode("secret_code"))
-                .thenReturn(new ExpiringCode("secret_code", new Timestamp(System.currentTimeMillis()), "eyedee"));
+                .thenReturn(new ExpiringCode("secret_code", new Timestamp(System.currentTimeMillis()+ PASSWORD_RESET_LIFETIME), "eyedee"));
 
         ScimUser scimUser = new ScimUser("eyedee", "user@example.com", "User", "Man");
+        scimUser.setMeta(new ScimMeta(new Date(System.currentTimeMillis()-(1000*60*60*24)), new Date(System.currentTimeMillis()-(1000*60*60*24)), 0));
         scimUser.addEmail("user@example.com");
         scimUser.setVerified(false);
         Mockito.when(scimUserProvisioning.retrieve("eyedee")).thenReturn(scimUser);
@@ -196,6 +215,7 @@ public void testChangingAPasswordForUnverifiedUser() throws Exception {
     @Test
     public void testChangingAPasswordWithAUsernameAndPassword() throws Exception {
         ScimUser user = new ScimUser("id001", "user@example.com", null, null);
+        user.setMeta(new ScimMeta(new Date(System.currentTimeMillis()-(1000*60*60*24)), new Date(System.currentTimeMillis()-(1000*60*60*24)), 0));
         user.addEmail("user@example.com");
         Mockito.when(scimUserProvisioning.query("userName eq \"user@example.com\""))
                 .thenReturn(Arrays.asList(user));

@@ -20,7 +20,9 @@
 import org.cloudfoundry.identity.uaa.codestore.ExpiringCodeStore;
 import org.cloudfoundry.identity.uaa.scim.ScimUser;
 import org.cloudfoundry.identity.uaa.scim.ScimUserProvisioning;
+import org.cloudfoundry.identity.uaa.scim.endpoints.PasswordResetEndpoints;
 import org.cloudfoundry.identity.uaa.test.YamlServletProfileInitializerContextInitializer;
+import org.cloudfoundry.identity.uaa.util.JsonUtils;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -35,8 +37,10 @@
 import org.springframework.web.context.support.XmlWebApplicationContext;
 
 import java.sql.Timestamp;
+import java.util.Arrays;
 import java.util.List;
 
+import static org.cloudfoundry.identity.uaa.scim.endpoints.PasswordResetEndpoints.PASSWORD_RESET_LIFETIME;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.junit.Assert.assertEquals;
@@ -74,12 +78,18 @@ public static void cleanUpAfterPasswordReset() throws Exception {
         webApplicationContext.destroy();
     }
 
+
     @Test
-    public void testResettingAPassword() throws Exception {
+    public void testResettingAPasswordUsingUsernameToEnsureNoModification() throws Exception {
+
         List<ScimUser> users = webApplicationContext.getBean(ScimUserProvisioning.class).query("username eq \"marissa\"");
         assertNotNull(users);
         assertEquals(1, users.size());
-        ExpiringCode code = codeStore.generateCode(users.get(0).getId(), new Timestamp(System.currentTimeMillis()+50000));
+        PasswordResetEndpoints.PasswordChange change = new PasswordResetEndpoints.PasswordChange();
+        change.setUserId(users.get(0).getId());
+        change.setUsername(users.get(0).getUserName());
+
+        ExpiringCode code = codeStore.generateCode(JsonUtils.writeValueAsString(change), new Timestamp(System.currentTimeMillis()+ PASSWORD_RESET_LIFETIME));
 
         MockHttpServletRequestBuilder post = post("/reset_password.do")
             .param("code", code.getCode())
@@ -101,4 +111,93 @@ public void testResettingAPassword() throws Exception {
         assertThat(principal.getEmail(), equalTo(users.get(0).getPrimaryEmail()));
         assertThat(principal.getOrigin(), equalTo(Origin.UAA));
     }
+
+    @Test
+    public void testResettingAPasswordFailsWhenUsernameChanged() throws Exception {
+
+        ScimUserProvisioning userProvisioning = webApplicationContext.getBean(ScimUserProvisioning.class);
+        List<ScimUser> users = userProvisioning.query("username eq \"marissa\"");
+        assertNotNull(users);
+        assertEquals(1, users.size());
+        ScimUser user = users.get(0);
+        PasswordResetEndpoints.PasswordChange change = new PasswordResetEndpoints.PasswordChange();
+        change.setUserId(user.getId());
+        change.setUsername(user.getUserName());
+
+        ExpiringCode code = codeStore.generateCode(JsonUtils.writeValueAsString(change), new Timestamp(System.currentTimeMillis()+50000));
+
+        String formerUsername = user.getUserName();
+        user.setUserName("newusername");
+        user = userProvisioning.update(user.getId(), user);
+        try {
+            MockHttpServletRequestBuilder post = post("/reset_password.do")
+                .param("code", code.getCode())
+                .param("email", user.getPrimaryEmail())
+                .param("password", "newpassword")
+                .param("password_confirmation", "newpassword");
+
+            mockMvc.perform(post)
+                .andExpect(status().isUnprocessableEntity());
+        } finally {
+            user.setUserName(formerUsername);
+            userProvisioning.update(user.getId(), user);
+        }
+    }
+
+    @Test
+    public void testResettingAPasswordUsingTimestampForUserModification() throws Exception {
+        List<ScimUser> users = webApplicationContext.getBean(ScimUserProvisioning.class).query("username eq \"marissa\"");
+        assertNotNull(users);
+        assertEquals(1, users.size());
+        ExpiringCode code = codeStore.generateCode(users.get(0).getId(), new Timestamp(System.currentTimeMillis()+ PASSWORD_RESET_LIFETIME));
+
+        MockHttpServletRequestBuilder post = post("/reset_password.do")
+            .param("code", code.getCode())
+            .param("email", users.get(0).getPrimaryEmail())
+            .param("password", "newpassword")
+            .param("password_confirmation", "newpassword");
+
+        MvcResult mvcResult = mockMvc.perform(post)
+            .andExpect(status().isFound())
+            .andExpect(redirectedUrl("home"))
+            .andReturn();
+
+        SecurityContext securityContext = (SecurityContext) mvcResult.getRequest().getSession().getAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY);
+        Authentication authentication = securityContext.getAuthentication();
+        assertThat(authentication.getPrincipal(), instanceOf(UaaPrincipal.class));
+        UaaPrincipal principal = (UaaPrincipal) authentication.getPrincipal();
+        assertThat(principal.getId(), equalTo(users.get(0).getId()));
+        assertThat(principal.getName(), equalTo(users.get(0).getUserName()));
+        assertThat(principal.getEmail(), equalTo(users.get(0).getPrimaryEmail()));
+        assertThat(principal.getOrigin(), equalTo(Origin.UAA));
+    }
+
+    @Test
+    public void testResettingAPasswordUsingTimestampUserModified() throws Exception {
+        ScimUserProvisioning userProvisioning = webApplicationContext.getBean(ScimUserProvisioning.class);
+        List<ScimUser> users = userProvisioning.query("username eq \"marissa\"");
+        assertNotNull(users);
+        assertEquals(1, users.size());
+        ScimUser user = users.get(0);
+        ExpiringCode code = codeStore.generateCode(user.getId(), new Timestamp(System.currentTimeMillis() + PASSWORD_RESET_LIFETIME));
+
+        MockHttpServletRequestBuilder post = post("/reset_password.do")
+            .param("code", code.getCode())
+            .param("email", user.getPrimaryEmail())
+            .param("password", "newpassword")
+            .param("password_confirmation", "newpassword");
+
+        if (Arrays.asList(webApplicationContext.getEnvironment().getActiveProfiles()).contains("mysql")) {
+            Thread.sleep(1050);
+        } else {
+            Thread.sleep(50);
+        }
+
+        userProvisioning.update(user.getId(), user);
+
+        mockMvc.perform(post)
+            .andExpect(status().isUnprocessableEntity());
+
+
+    }
 }