Skip to content

Refactor password change#3989

Open
strehle wants to merge 3 commits into
developfrom
fix-passwd-change
Open

Refactor password change#3989
strehle wants to merge 3 commits into
developfrom
fix-passwd-change

Conversation

@strehle

@strehle strehle commented Jul 21, 2026

Copy link
Copy Markdown
Member

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens the password reset / password change flows by ensuring only “forgot password” expiring codes are accepted (and that invitation codes are rejected), and updates/extends test coverage accordingly.

Changes:

  • Enforce a forgot-password intent prefix check when rendering /reset_password and when handling /password_change.
  • Update existing tests to generate/reset codes with a forgot-password intent.
  • Add negative tests to ensure invitation codes (and null-intent codes) are rejected with 422.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
server/src/main/java/org/cloudfoundry/identity/uaa/account/UaaResetPasswordService.java Adds intent validation for password-change via reset code.
server/src/main/java/org/cloudfoundry/identity/uaa/account/ResetPasswordController.java Adds intent validation when resolving reset-password page codes.
server/src/test/java/org/cloudfoundry/identity/uaa/login/ResetPasswordControllerTest.java Updates reset-password page tests and adds invitation-code rejection coverage.
server/src/test/java/org/cloudfoundry/identity/uaa/account/PasswordResetEndpointTest.java Updates password-change tests for intent; adds invitation/null-intent rejection tests.

Comment on lines +91 to +94
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 on lines +201 to +205
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.", code.getCode());
return null;
}
}

@Test
void resetPasswordPage_withInvitationCode_returns422() throws Exception {
}

@Test
void changePassword_withInvitationCode_returns422() throws Exception {
}

@Test
void changePassword_withNullIntentCode_returns422() throws Exception {
Comment on lines +456 to +457
mockMvc.perform(post)
.andExpect(status().isUnprocessableEntity());

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

server/src/main/java/org/cloudfoundry/identity/uaa/account/UaaResetPasswordService.java:94

  • The intent validation only checks for the forgot-password prefix, but the code generator sets intent to the full value FORGOT_PASSWORD_INTENT_PREFIX + userId and expires by that exact intent. With the current startsWith check, a code whose intent doesn’t match the user_id in the payload would still be accepted, undermining the purpose of intent-scoping and making it easier for mismatched/forged codes to be replayed across users if they ever exist in the store.
    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);
        }

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

  • checkIfUserExists logs the raw expiring code when the intent isn’t a forgot-password intent. The code value is a secret token and shouldn’t be written to logs. Also, the intent check only validates the prefix; since forgot-password codes are generated with intent FORGOT_PASSWORD_INTENT_PREFIX + user_id, it’s safer to require the full intent to match the user_id in the code payload.
        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.", code.getCode());
            return null;
        }

@github-project-automation github-project-automation Bot moved this from Inbox to Pending Merge | Prioritized in Foundational Infrastructure Working Group Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Pending Merge | Prioritized

Development

Successfully merging this pull request may close these issues.

3 participants