Skip to content

Refactor password change#3989

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

Refactor password change#3989
strehle wants to merge 6 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.

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;
        }

duanemay
duanemay previously approved these changes Jul 23, 2026
@github-project-automation github-project-automation Bot moved this from Inbox to Pending Merge | Prioritized in Foundational Infrastructure Working Group Jul 23, 2026

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.

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 3 comments.

Thread.sleep(1000 - (System.currentTimeMillis() % 1000) + 10); //because password last modified is second only
PasswordChange change = new PasswordChange(user.getId(), user.getUserName(), user.getPasswordLastModified(), "", "");
ExpiringCode code = codeStore.generateCode(JsonUtils.writeValueAsString(change), new Timestamp(System.currentTimeMillis() + 50000), null, IdentityZoneHolder.get().getId());
ExpiringCode code = codeStore.generateCode(JsonUtils.writeValueAsString(change), new Timestamp(System.currentTimeMillis() + 50000), FORGOT_PASSWORD_INTENT_PREFIX + user.getId(), IdentityZoneHolder.get().getId());
Thread.sleep(1000 - (System.currentTimeMillis() % 1000) + 10); //because password last modified is second only
PasswordChange change = new PasswordChange(user.getId(), user.getUserName(), user.getPasswordLastModified(), "", "");
ExpiringCode code = codeStore.generateCode(JsonUtils.writeValueAsString(change), new Timestamp(System.currentTimeMillis() + 50000), null, IdentityZoneHolder.get().getId());
ExpiringCode code = codeStore.generateCode(JsonUtils.writeValueAsString(change), new Timestamp(System.currentTimeMillis() + 50000), FORGOT_PASSWORD_INTENT_PREFIX + user.getId(), IdentityZoneHolder.get().getId());
Comment on lines +365 to +368
ExpiringCode code = codeStore.generateCode(JsonUtils.writeValueAsString(passwordChange), new Timestamp(System.currentTimeMillis() + UaaResetPasswordService.PASSWORD_RESET_LIFETIME), FORGOT_PASSWORD_INTENT_PREFIX + user.getId(), IdentityZoneHolder.get().getId());
mockMvc.perform(createChangePasswordRequest(user, code, true, "d3faultPasswd", "d3faultPasswd"));

code = codeStore.generateCode(JsonUtils.writeValueAsString(passwordChange), new Timestamp(System.currentTimeMillis() + UaaResetPasswordService.PASSWORD_RESET_LIFETIME), null, IdentityZoneHolder.get().getId());
code = codeStore.generateCode(JsonUtils.writeValueAsString(passwordChange), new Timestamp(System.currentTimeMillis() + UaaResetPasswordService.PASSWORD_RESET_LIFETIME), FORGOT_PASSWORD_INTENT_PREFIX + user.getId(), IdentityZoneHolder.get().getId());
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