Skip to content

Refactor password change #3989

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
strehle wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Refactor password change #3989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d6501ff
Refactor password change
strehle Jul 21, 2026
d6a0213
test fix
strehle Jul 21, 2026
da19d3b
test fix
strehle Jul 21, 2026
2642cb3
review
strehle Jul 23, 2026
7a39d4b
Merge remote-tracking branch 'origin/develop' into fix-passwd-change
strehle Jul 23, 2026
2ccad8d
review
strehle Jul 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions server/src/main/java/org/cloudfoundry/identity/uaa/account/ResetPasswordController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.sql.Timestamp;
import java.util.Map;

import static org.cloudfoundry.identity.uaa.account.UaaResetPasswordService.FORGOT_PASSWORD_INTENT_PREFIX;
import static org.springframework.util.StringUtils.hasText;

@Controller
Expand Down Expand Up @@ -197,6 +198,11 @@ private ExpiringCode checkIfUserExists(ExpiringCode code) {
logger.debug("reset_password ExpiringCode object is null. Aborting.");
return null;
}
String intent = code.getIntent();
if (intent == null || !intent.startsWith(FORGOT_PASSWORD_INTENT_PREFIX)) {
logger.debug("reset_password ExpiringCode intent is not a forgot-password intent. Aborting.");
return null;
}
Comment thread
strehle marked this conversation as resolved.
if (!hasText(code.getData())) {
logger.debug("reset_password ExpiringCode[{}] data string is null or empty. Aborting.", code.getCode());
return null;
Expand All @@ -208,6 +214,10 @@ private ExpiringCode checkIfUserExists(ExpiringCode code) {
return null;
}
String userId = data.get("user_id");
if (!intent.equals(FORGOT_PASSWORD_INTENT_PREFIX + userId)) {
logger.debug("reset_password ExpiringCode intent does not match user_id. Aborting.");
return null;
}
try {
userDatabase.retrieveUserById(userId);
} catch (UsernameNotFoundException _) {
Expand Down
7 changes: 7 additions & 0 deletions server/src/main/java/org/cloudfoundry/identity/uaa/account/UaaResetPasswordService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ public void resetUserPassword(String userId, String password) {
}

private ResetPasswordResponse changePasswordCodeAuthenticated(ExpiringCode expiringCode, String newPassword) {
String intent = expiringCode.getIntent();
if (intent == null || !intent.startsWith(FORGOT_PASSWORD_INTENT_PREFIX)) {
throw new InvalidCodeException("invalid_code", "Sorry, your reset password link is no longer valid. Please request a new one", 422);
}
Comment thread
strehle marked this conversation as resolved.
String userId;
String userName;
Date passwordLastModified;
Expand All @@ -100,6 +104,9 @@ private ResetPasswordResponse changePasswordCodeAuthenticated(ExpiringCode expir
throw new InvalidCodeException("invalid_code", "Sorry, your reset password link is no longer valid. Please request a new one", 422);
}
userId = change.getUserId();
if (!intent.equals(FORGOT_PASSWORD_INTENT_PREFIX + userId)) {
throw new InvalidCodeException("invalid_code", "Sorry, your reset password link is no longer valid. Please request a new one", 422);
}
userName = change.getUsername();
passwordLastModified = change.getPasswordModifiedTime();
clientId = change.getClientId();
Expand Down
57 changes: 53 additions & 4 deletions server/src/test/java/org/cloudfoundry/identity/uaa/account/PasswordResetEndpointTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
import java.util.Date;

import static org.assertj.core.api.Assertions.assertThat;
import static org.cloudfoundry.identity.uaa.account.UaaResetPasswordService.FORGOT_PASSWORD_INTENT_PREFIX;
import static org.cloudfoundry.identity.uaa.codestore.ExpiringCodeType.AUTOLOGIN;
import static org.cloudfoundry.identity.uaa.codestore.ExpiringCodeType.INVITATION;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
Expand Down Expand Up @@ -275,7 +277,8 @@ void creatingAPasswordResetWithAUsernameContainingSpecialCharacters() throws Exc
@ValueSource(strings = {"/password_change", "/password_change/"})
void changingAPasswordWithAValidCode(String url) throws Exception {
ExpiringCode code = new ExpiringCode("secret_code", new Timestamp(System.currentTimeMillis() + UaaResetPasswordService.PASSWORD_RESET_LIFETIME),
"{\"user_id\":\"eyedee\",\"username\":\"user@example.com\",\"passwordModifiedTime\":null,\"client_id\":\"\",\"redirect_uri\":\"\"}", null);
"{\"user_id\":\"eyedee\",\"username\":\"user@example.com\",\"passwordModifiedTime\":null,\"client_id\":\"\",\"redirect_uri\":\"\"}",
FORGOT_PASSWORD_INTENT_PREFIX + "eyedee");
when(mockExpiringCodeStore.retrieveCode("secret_code", currentZoneId)).thenReturn(code);

ScimUser scimUser = new ScimUser("eyedee", "user@example.com", "User", "Man");
Expand Down Expand Up @@ -323,7 +326,8 @@ void changingPasswordWithInvalidCode() throws Exception {
@Test
void changingAPasswordForUnverifiedUser() throws Exception {
ExpiringCode code = new ExpiringCode("secret_code", new Timestamp(System.currentTimeMillis() + UaaResetPasswordService.PASSWORD_RESET_LIFETIME),
"{\"user_id\":\"eyedee\",\"username\":\"user@example.com\",\"passwordModifiedTime\":null,\"client_id\":\"\",\"redirect_uri\":\"\"}", null);
"{\"user_id\":\"eyedee\",\"username\":\"user@example.com\",\"passwordModifiedTime\":null,\"client_id\":\"\",\"redirect_uri\":\"\"}",
FORGOT_PASSWORD_INTENT_PREFIX + "eyedee");
when(mockExpiringCodeStore.retrieveCode("secret_code", currentZoneId)).thenReturn(code);

ScimUser scimUser = new ScimUser("eyedee", "user@example.com", "User", "Man");
Expand Down Expand Up @@ -369,7 +373,7 @@ void passwordsMustSatisfyPolicy() throws Exception {
when(mockExpiringCodeStore.retrieveCode("emailed_code", currentZoneId))
.thenReturn(new ExpiringCode("emailed_code", new Timestamp(System.currentTimeMillis() + UaaResetPasswordService.PASSWORD_RESET_LIFETIME),
"{\"user_id\":\"eyedee\",\"username\":\"user@example.com\",\"passwordModifiedTime\":null,\"client_id\":\"\",\"redirect_uri\":\"\"}",
null));
FORGOT_PASSWORD_INTENT_PREFIX + "eyedee"));

MockHttpServletRequestBuilder post = post("/password_change")
.contentType(APPLICATION_JSON)
Expand All @@ -391,7 +395,7 @@ void changePassword_Returns422UnprocessableEntity_NewPasswordSameAsOld() throws
when(mockExpiringCodeStore.retrieveCode("emailed_code", currentZoneId))
.thenReturn(new ExpiringCode("emailed_code", new Timestamp(System.currentTimeMillis() + UaaResetPasswordService.PASSWORD_RESET_LIFETIME),
"{\"user_id\":\"eyedee\",\"username\":\"user@example.com\",\"passwordModifiedTime\":null,\"client_id\":\"\",\"redirect_uri\":\"\"}",
null));
FORGOT_PASSWORD_INTENT_PREFIX + "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));
Expand All @@ -414,4 +418,49 @@ void changePassword_Returns422UnprocessableEntity_NewPasswordSameAsOld() throws
assertThat(JsonUtils.readTree(result.getResponse().getContentAsString()))
.isEqualTo(JsonUtils.readTree(new JSONObject().put("error_description", "Your new password cannot be the same as the old password.").put("message", "Your new password cannot be the same as the old password.").put("error", "invalid_password").toString()));
}

@Test
void changePassword_withInvitationCode_returns422() throws Exception {
Comment thread
strehle marked this conversation as resolved.
String inviteData = "{\"user_id\":\"eyedee\",\"client_id\":\"invite-client\",\"created_new_user\":\"false\"}";
ExpiringCode inviteCode = new ExpiringCode("invite_code", new Timestamp(System.currentTimeMillis() + UaaResetPasswordService.PASSWORD_RESET_LIFETIME),
inviteData, INVITATION.name());
when(mockExpiringCodeStore.retrieveCode("invite_code", currentZoneId)).thenReturn(inviteCode);

MockHttpServletRequestBuilder post = post("/password_change")
.contentType(APPLICATION_JSON)
.content("{\"code\":\"invite_code\",\"new_password\":\"new_secret\"}")
.accept(APPLICATION_JSON);

MvcResult result = mockMvc.perform(post)
.andExpect(status().isUnprocessableEntity())
.andReturn();
assertThat(JsonUtils.readTree(result.getResponse().getContentAsString()))
.isEqualTo(JsonUtils.readTree(new JSONObject()
.put("error_description", "Sorry, your reset password link is no longer valid. Please request a new one")
.put("message", "Sorry, your reset password link is no longer valid. Please request a new one")
.put("error", "invalid_code")
.toString()));
}

@Test
void changePassword_withNullIntentCode_returns422() throws Exception {
Comment thread
strehle marked this conversation as resolved.
ExpiringCode nullIntentCode = new ExpiringCode("null_intent_code", new Timestamp(System.currentTimeMillis() + UaaResetPasswordService.PASSWORD_RESET_LIFETIME),
"{\"user_id\":\"eyedee\",\"username\":\"user@example.com\",\"passwordModifiedTime\":null,\"client_id\":\"\",\"redirect_uri\":\"\"}", null);
when(mockExpiringCodeStore.retrieveCode("null_intent_code", currentZoneId)).thenReturn(nullIntentCode);

MockHttpServletRequestBuilder post = post("/password_change")
.contentType(APPLICATION_JSON)
.content("{\"code\":\"null_intent_code\",\"new_password\":\"new_secret\"}")
.accept(APPLICATION_JSON);

MvcResult result = mockMvc.perform(post)
.andExpect(status().isUnprocessableEntity())
.andReturn();
assertThat(JsonUtils.readTree(result.getResponse().getContentAsString()))
.isEqualTo(JsonUtils.readTree(new JSONObject()
.put("error_description", "Sorry, your reset password link is no longer valid. Please request a new one")
.put("message", "Sorry, your reset password link is no longer valid. Please request a new one")
.put("error", "invalid_code")
.toString()));
}
}
22 changes: 19 additions & 3 deletions server/src/test/java/org/cloudfoundry/identity/uaa/login/ResetPasswordControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@
import java.sql.Timestamp;

import static org.assertj.core.api.Assertions.assertThat;
import static org.cloudfoundry.identity.uaa.account.UaaResetPasswordService.FORGOT_PASSWORD_INTENT_PREFIX;
import static org.cloudfoundry.identity.uaa.codestore.ExpiringCodeType.INVITATION;
import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.contains;
import static org.mockito.Mockito.eq;
Expand Down Expand Up @@ -290,7 +292,7 @@ void instructions() throws Exception {

@Test
void resetPasswordPage() throws Exception {
ExpiringCode code = codeStore.generateCode("{\"user_id\" : \"some-user-id\"}", new Timestamp(System.currentTimeMillis() + 1000000), null, IdentityZoneHolder.get().getId());
ExpiringCode code = codeStore.generateCode("{\"user_id\" : \"some-user-id\"}", new Timestamp(System.currentTimeMillis() + 1000000), FORGOT_PASSWORD_INTENT_PREFIX + "some-user-id", IdentityZoneHolder.get().getId());
MvcResult result = mockMvc.perform(get("/reset_password").param("email", "user@example.com").param("code", code.getCode()))
.andExpect(status().isOk())
.andDo(print())
Expand All @@ -305,7 +307,7 @@ void resetPasswordPage() throws Exception {

@Test
void resetPasswordPageWithPriorHeadRequest() throws Exception {
ExpiringCode code = codeStore.generateCode("{\"user_id\" : \"some-user-id\"}", new Timestamp(System.currentTimeMillis() + 1000000), null, IdentityZoneHolder.get().getId());
ExpiringCode code = codeStore.generateCode("{\"user_id\" : \"some-user-id\"}", new Timestamp(System.currentTimeMillis() + 1000000), FORGOT_PASSWORD_INTENT_PREFIX + "some-user-id", IdentityZoneHolder.get().getId());
mockMvc.perform(head("/reset_password").param("email", "user@example.com").param("code", code.getCode()))
.andExpect(status().isOk());
MvcResult result = mockMvc.perform(get("/reset_password").param("email", "user@example.com").param("code", code.getCode()))
Expand All @@ -322,7 +324,7 @@ void resetPasswordPageWithPriorHeadRequest() throws Exception {

@Test
void resetPasswordPageDuplicate() throws Exception {
ExpiringCode code = codeStore.generateCode("{\"user_id\" : \"some-user-id\"}", new Timestamp(System.currentTimeMillis() + 1000000), null, IdentityZoneHolder.get().getId());
ExpiringCode code = codeStore.generateCode("{\"user_id\" : \"some-user-id\"}", new Timestamp(System.currentTimeMillis() + 1000000), FORGOT_PASSWORD_INTENT_PREFIX + "some-user-id", IdentityZoneHolder.get().getId());
mockMvc.perform(get("/reset_password").param("email", "user@example.com").param("code", code.getCode()))
.andExpect(status().isOk())
.andExpect(view().name("reset_password"));
Expand All @@ -339,6 +341,20 @@ void resetPasswordPageWhenExpiringCodeNull() throws Exception {
.andExpect(model().attribute("message_code", "bad_code"));
}

@Test
void resetPasswordPageWithInvitationCodeReturns422() throws Exception {
ExpiringCode inviteCode = codeStore.generateCode(
"{\"user_id\":\"some-user-id\",\"client_id\":\"invite-client\",\"created_new_user\":\"false\"}",
new Timestamp(System.currentTimeMillis() + 1000000),
INVITATION.name(),
IdentityZoneHolder.get().getId());

mockMvc.perform(get("/reset_password").param("code", inviteCode.getCode()))
.andExpect(status().isUnprocessableEntity())
.andExpect(view().name("forgot_password"))
.andExpect(model().attribute("message_code", "bad_code"));
}

@EnableWebMvc
@Import(ThymeleafConfig.class)
static class ContextConfiguration implements WebMvcConfigurer {
Expand Down
18 changes: 16 additions & 2 deletions server/src/test/java/org/cloudfoundry/identity/uaa/login/UaaResetPasswordServiceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,8 @@ void resetPassword_InvalidPasswordException_NewPasswordSameAsOld() {
user.setMeta(new ScimMeta(new Date(), new Date(), 0));
user.setPrimaryEmail("foo@example.com");
ExpiringCode expiringCode = new ExpiringCode("good_code",
new Timestamp(System.currentTimeMillis() + UaaResetPasswordService.PASSWORD_RESET_LIFETIME), "{\"user_id\":\"user-id\",\"username\":\"username\",\"passwordModifiedTime\":null,\"client_id\":\"\",\"redirect_uri\":\"\"}", null);
new Timestamp(System.currentTimeMillis() + UaaResetPasswordService.PASSWORD_RESET_LIFETIME), "{\"user_id\":\"user-id\",\"username\":\"username\",\"passwordModifiedTime\":null,\"client_id\":\"\",\"redirect_uri\":\"\"}",
UaaResetPasswordService.FORGOT_PASSWORD_INTENT_PREFIX + "user-id");
when(codeStore.retrieveCode("good_code", currentZoneId)).thenReturn(expiringCode);
when(scimUserProvisioning.retrieve("user-id", currentZoneId)).thenReturn(user);
when(scimUserProvisioning.checkPasswordMatches("user-id", "Passwo3dAsOld", currentZoneId))
Expand Down Expand Up @@ -236,6 +237,18 @@ void resetPassword_InvalidCodeData() {
}
}

@Test
void resetPassword_rejectsNonForgotPasswordIntent() {
ExpiringCode inviteCode = new ExpiringCode("good_code",
new Timestamp(System.currentTimeMillis() + UaaResetPasswordService.PASSWORD_RESET_LIFETIME),
"{\"user_id\":\"user-id\",\"client_id\":\"invite-client\",\"created_new_user\":\"false\"}", "INVITATION");

assertThatThrownBy(() -> uaaResetPasswordService.resetPassword(inviteCode, "new_secret"))
.isInstanceOf(InvalidCodeException.class)
.hasMessage("Sorry, your reset password link is no longer valid. Please request a new one");
verify(scimUserProvisioning, times(0)).changePassword(anyString(), any(), anyString(), anyString());
}

@Test
void resetPassword_WithInvalidClientId() {
ExpiringCode code = setupResetPassword("invalid_client", "redirect.example.com");
Expand Down Expand Up @@ -320,7 +333,8 @@ private ExpiringCode setupResetPassword(String clientId, String redirectUri) {
String zoneId = currentZoneId;
when(scimUserProvisioning.retrieve(eq("usermans-id"), eq(zoneId))).thenReturn(user);
ExpiringCode code = new ExpiringCode("code", new Timestamp(System.currentTimeMillis()),
"{\"user_id\":\"usermans-id\",\"username\":\"userman\",\"passwordModifiedTime\":null,\"client_id\":\"" + clientId + "\",\"redirect_uri\":\"" + redirectUri + "\"}", null);
"{\"user_id\":\"usermans-id\",\"username\":\"userman\",\"passwordModifiedTime\":null,\"client_id\":\"" + clientId + "\",\"redirect_uri\":\"" + redirectUri + "\"}",
UaaResetPasswordService.FORGOT_PASSWORD_INTENT_PREFIX + "usermans-id");
when(codeStore.retrieveCode(eq("secret_code"), anyString())).thenReturn(code);
SecurityContext securityContext = mock(SecurityContext.class);
when(securityContext.getAuthentication()).thenReturn(new MockAuthentication());
Expand Down
Loading
Loading