Skip to content

Refactor password change#3989

Merged
strehle merged 10 commits into
developfrom
fix-passwd-change
Jul 24, 2026
Merged

Refactor password change#3989
strehle merged 10 commits into
developfrom
fix-passwd-change

Conversation

@strehle

@strehle strehle commented Jul 21, 2026

Copy link
Copy Markdown
Member

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.

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.

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 1 comment.

Comments suppressed due to low confidence (2)

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

  • Avoid hard-coded HTTP status 422 here; use UNPROCESSABLE_ENTITY.value() for clarity and consistency with the rest of the class.
            change = JsonUtils.readValue(expiringCode.getData(), PasswordChange.class);
        } catch (JsonUtils.JsonUtilException _) {
            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/UaaResetPasswordService.java:109

  • Avoid hard-coded HTTP status 422 here; use UNPROCESSABLE_ENTITY.value() for readability and to prevent status-code drift if this ever changes.
        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);
        }

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 2 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 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 1 comment.

Comment on lines +241 to +245
@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\"}", ExpiringCodeType.INVITATION.name());
@strehle
strehle merged commit 6c0cca0 into develop Jul 24, 2026
28 checks passed
@strehle
strehle deleted the fix-passwd-change branch July 24, 2026 14:45
@github-project-automation github-project-automation Bot moved this from Pending Merge | Prioritized to Done in Foundational Infrastructure Working Group Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants