Skip to content

self service improvement - #4100

Draft
fhanik wants to merge 31 commits into
developfrom
pr/self-service-improvement
Draft

self service improvement#4100
fhanik wants to merge 31 commits into
developfrom
pr/self-service-improvement

Conversation

@fhanik

@fhanik fhanik commented Oct 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Self-service account confirmation now requires the owner to set their password
through the activation link. Previously the password typed on the sign-up form was
activated as soon as the account was confirmed — but confirming the link only proves
ownership of the email address, not that the person confirming chose that password.

Change

On confirmation, completeActivation now:

  • verifies the account,
  • replaces the stored password with a random value so no previously submitted
    password can be used to sign in,
  • issues an ownership-bound, single-use reset code.

/verify_user then sends the owner to the reset-password page (showing "A password
change is required due to changes in the system.") to set their own password via the
existing code-authenticated flow.

Behavior changes

  • Signup requires setting the password via the confirmation link; the sign-up-form
    password is never used to sign in.
  • Landing after confirmation changes from the login page to the password-set page,
    then login once the password is set.
  • An explicit client redirect_uri is still honored; the signup_redirect_url
    fallback currently lands on home (see open item).

Tests

  • Added tests covering the confirm → set-password → sign-in flow.
  • Updated existing self-service unit/mock and Selenium flows. Unit suites green
    (server, uaa); integrationTest to run in CI.

Open item

  • Restore the signup_redirect_url fallback redirect and its test assertions.

@github-actions github-actions Bot added the size/m label Oct 7, 2026
@duanemay
duanemay requested a balanced review from Copilot October 7, 2026 20:27
Copilot AI reviewed Oct 7, 2026
View reviewed changes

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.

🟡 Changes recommended

Account verification has a credential race, and the configured signup redirect fallback is lost.

4 open findings
What changed in this PR

Updates self-service signup so activation verifies email ownership, invalidates the registration password, and redirects users through password setup.

Changes:

  • Generates a reset code during account activation.
  • Adds forced-password messaging and updates browser flows.
  • Expands unit, MockMvc, and integration coverage.
File Description
server/​.../​AccountCreationService.java Adds reset code to activation responses.
server/​.../​AccountsController.java Redirects activation to password setup.
server/​.../​EmailAccountCreationService.java Invalidates signup passwords and creates reset codes.
server/​.../​ResetPasswordController.java Adds forced-change page messaging.
server/​.../​AccountsControllerTest.java Updates activation redirect tests.
server/​.../​EmailAccountCreationServiceTests.java Updates service construction and reset mocks.
uaa/​.../​AccountsControllerMockMvcTests.java Covers the new default-zone flow.
uaa/​.../​AccountsControllerMockMvcZonePathTests.java Covers zone-path activation flows.
uaa/​.../​CreateAccountIT.java Updates end-to-end signup scenarios.
uaa/​.../​IntegrationTestUtils.java Completes password setup in Selenium fixtures.
uaa/​src/​main/​resources/​messages.properties Adds forced-change text.
docs/​UAA-Configuration-Reference.md Documents the changed signup behavior.

🧠 Review effort: Balanced

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Comment thread server/src/main/java/org/cloudfoundry/identity/uaa/account/EmailAccountCreationService.java
Comment thread server/src/main/java/org/cloudfoundry/identity/uaa/account/EmailAccountCreationService.java Outdated
Comment thread uaa/src/test/java/org/cloudfoundry/identity/uaa/login/AccountsControllerMockMvcTests.java Outdated
Comment thread ...rc/test/java/org/cloudfoundry/identity/uaa/login/AccountsControllerMockMvcZonePathTests.java Outdated
@fhanik
fhanik force-pushed the pr/self-service-improvement branch from 838c001 to 6b46280 Compare October 7, 2026 21:00
@fhanik
fhanik marked this pull request as ready for review October 7, 2026 21:29
@strehle
strehle requested a balanced review from Copilot October 8, 2026 14:09
@fhanik
fhanik marked this pull request as draft October 8, 2026 14:11
Copilot AI reviewed Oct 8, 2026
View reviewed changes

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.

🟡 Changes recommended

Reset-code generation can fail after verification, leaving an activated account with an unknown password and no retryable activation link.

1 open finding
4 resolved since last review

🧠 Review effort: Balanced

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Comment thread server/src/main/java/org/cloudfoundry/identity/uaa/account/EmailAccountCreationService.java
@fhanik
fhanik force-pushed the pr/self-service-improvement branch from 6b46280 to db06278 Compare October 22, 2026 13:33
fhanik and others added 6 commits October 22, 2026 08:53
@fhanik
test: confirmation must not authorize a pre-registered password
a7f9c8c 
Self-service account confirmation (/verify_user) currently marks the
account verified while leaving the password that was submitted on the
registration form in place. Clicking the confirmation link only proves
ownership of the email address; it does not prove that the clicker chose
the stored password. As a result the password supplied by whoever
registered the address becomes a usable credential once the owner
confirms the account.

Add two red MockMvc tests on the self-service flow:

- confirmingAccountDoesNotAuthorizeThePreRegisteredPassword: a single
  registration followed by confirmation must not let the registration
  password authenticate.
- confirmingAccountDoesNotAuthorizePasswordFromEarlierRegistration: when
  the address is registered twice before confirmation, the earlier
  password must not authenticate after the owner confirms via the most
  recent link.

Both tests fail today, demonstrating the gap. The following commit
closes it.
@fhanik
fix: establish self-service password at email confirmation
d85201c 
Self-service registration stored the password from the sign-up form and
activated it the moment the account was confirmed. Because confirming the
activation link only proves ownership of the email address — not that the
person confirming chose the stored password — a password set before the
address was verified could end up authorizing access to the confirmed
account.

Establish the password at confirmation instead:

- EmailAccountCreationService.completeActivation now verifies the account,
  discards the stored password by replacing it with an unguessable random
  value, flags the account as requiring a password change, and issues an
  ownership-bound, single-use reset code for the user.
- AccountsController redirects /verify_user to the reset-password page
  (with force_change=true) so the confirming owner sets their own password
  through the same code-authenticated flow used for password recovery; no
  previous password is required.
- ResetPasswordController surfaces a "password change required due to
  changes in the system" notice on that page (new reset_password.force_change
  message).

A setting-independent consequence: the password typed on the sign-up form is
never usable to authenticate; only the owner who confirms via the emailed
link can set the account's password.

Existing self-service tests are updated to complete the new
confirm-then-set-password flow, and the configuration reference documents the
behavior. The security tests added in the previous commit now pass.
@fhanik
fix: preserve the signup redirect across account confirmation
da35893 
The confirmation flow now carries the resolved signup redirect (including
the client's signup_redirect_url fallback) into the reset code, so the owner
is returned to it after setting their password. Previously only an explicit,
registered redirect_uri survived; the signup_redirect_url fallback was lost
and the owner landed on "home".

completeActivation now passes the resolved redirect location (from
getRedirect, which honours signup_redirect_url) to forgotPassword instead of
the raw redirect_uri. The reset flow re-matches it against the client's
registered redirect URIs before applying it.

Restores the redirect assertions in the self-service tests for both the
explicit and fallback client-redirect cases.

test: cover confirmation password/redirect behavior end to end

Add coverage for the self-service confirmation changes:

- EmailAccountCreationServiceTests: assert completeActivation discards the
  registration-form password (replacing it with a different value), marks the
  account as requiring a password change, and issues an ownership-bound reset
  code.
- CreateAccountIT: end-to-end test confirming that the registration-form
  password cannot sign in after confirmation, and that only the password the
  owner sets through the link works.

Also completes the configuration-reference note to describe that a configured
client/signup redirect is applied after the owner sets their password.
@fhanik
fix: verify the account only after discarding the registration password
4c28ef2 
completeActivation verified the account before replacing the registration
password and setting the change-required flag. Because these are separate
provisioning updates, a concurrent login in the gap could authenticate with
the registration password against the already-verified account.

Replace the password and set the flag while the account is still unverified,
then verify it last (version -1, matching the password-reset flow), so the
account is never simultaneously verified and holding the registration
password. Add an ordering assertion to the service test to lock this in.

Addresses PR review feedback.
@fhanik @claude
test: email change verification keeps redirect handling within the re…
d7e3cd8 
…gistered redirect rules

Two behaviors, both covered at service, controller and full-flow level:

- When the client has no change-email redirect URL, completeVerification
  returns the requested URL unchanged even if it matches none of the client's
  registered redirect URIs. The tests expect no redirect in that case, for a
  client without registered URIs, for a requested URL that matches none, and for
  no requested URL, plus a guard that a matching requested URL is still returned.
- For a user who is not signed in, the verification redirect to the login page
  appends the redirect URL to the query string as is, so a URL containing '&' or
  '#' is split into extra parameters or a fragment. The tests expect the URL to
  arrive as the single decoded value of form_redirect_uri. The existing
  controller test that pinned the raw string now checks the decoded value, so it
  holds for either encoding.

Full-flow MockMvc tests run in the default zone and, per zone resolution mode,
in a created zone. These tests fail until the production changes follow.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
fix: email change verification keeps redirect handling within the reg…
472c9ce 
…istered redirect rules

- completeVerification returns no redirect when the client has no change-email
  redirect URL and the requested URL matches none of the client's registered
  redirect URIs, so the controller falls back to the profile or login page.
- The login redirect for a user who is not signed in carries the redirect URL as
  the single encoded value of form_redirect_uri, as the LDAP invitation path
  already does.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added size/l and removed size/m labels Oct 22, 2026
@fhanik
fhanik force-pushed the pr/self-service-improvement branch from 020b13e to 472c9ce Compare October 22, 2026 16:08
fhanik and others added 6 commits October 22, 2026 09:38
@fhanik @claude
test: invited user must supply an email when accepting through an ext…
95be05d 
…ernal login, unless a fresh invitee

When a user accepts an invitation through an external identity provider, the
email in the response is compared with the invited email.

- SAML: a response without an email is accepted for any invited user. Add tests
  that expect the login to be rejected, with a message for each reason, when the
  invited user is already verified and when the invited user's username differs
  from the invited email, and that the stored user is left unchanged. Add a guard
  that a fresh invitee (unverified, username equal to the email) can still accept
  without an email. The two rejection tests fail until the production change
  follows.
- OIDC: a token without an email claim gets an email generated from the username
  claim, which does not match the invited email. Add a test that pins this
  rejection and that no invitation event is published. It passes today.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
fix: invited user must supply an email when accepting through SAML un…
a80180c 
…less a fresh invitee

When the SAML response carries no email, the invited user's own email was filled
in for every invited user. Do that only for a fresh invitee (unverified, username
equal to the email). An invited user who is already verified, or whose username
differs from the invited email, is rejected with a message naming the reason, and
the stored user is left unchanged.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
test: password change page guard treats a trailing slash like the pla…
29a96cd 
…in path

The controller serves /force_password_change and /force_password_change/, but the
filter that guards the page only recognises the plain path.

- A signed-in user with no password change pending who requests the page with a
  trailing slash is not redirected away, and a submit to that path replaces the
  password. Add filter tests for GET and POST, and MockMvc tests (default zone and
  per zone resolution mode) that submit to each path and check the old password
  still signs in. The plain path passes today; the trailing-slash path fails.
- A user whose password change is pending and who requests the trailing-slash
  path is redirected to the plain path instead of being let through. Add filter
  tests that expect the request to pass through.

These tests fail until the production change follows.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
fix: password change page guard treats a trailing slash like the plai…
2f6ecc3 
…n path

The filter that guards the password change page now recognises both paths the
controller serves, /force_password_change and /force_password_change/. A signed-in
user with no password change pending is redirected away from either, and a user
whose change is pending is let through on either.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
test: password change submit is refused when no change is pending
d74b739 
The password change page is guarded by a filter, but the controller itself accepts
a submit whether or not a change is pending for the session. Add a test that
expects a submit with no pending change to be redirected to the home page without
changing the password, for both paths the controller serves. The existing submit
tests now mark the change as pending in the session, as a real flow does.

This test fails until the production change follows.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
fix: password change submit is refused when no change is pending
908adcf 
The controller now checks that a password change is pending for the session
before it accepts a submit, and otherwise redirects to the home page without
changing the password. The filter in front of the page already does the same;
this keeps the controller correct on its own.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added size/xl and removed size/l labels Oct 22, 2026
@github-actions

github-actions Bot commented Oct 22, 2026

Copy link
Copy Markdown

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

fhanik and others added 19 commits October 22, 2026 11:11
@fhanik @claude
test: invitation acceptance keeps the redirect URL as one login redir…
5fff537 
…ect parameter

After an invitation is accepted, the redirect URL stored with the invitation is
appended to the login redirect as is. A URL with a query string or a fragment, which
a client with a wildcard redirect pattern allows, is split into extra parameters or a
fragment instead of arriving as the single value of form_redirect_uri.

Add controller tests (default and per zone request path mode) and an end-to-end test
with a client that has a wildcard redirect pattern. They expect the URL to arrive as
the single decoded value of form_redirect_uri. The two existing end-to-end checks that
pinned the raw string now check the decoded value, so they hold for either encoding.

These tests fail until the production change follows.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
fix: invitation acceptance carries the redirect URL as one login redi…
d884d07 
…rect parameter

The redirect URL stored with the invitation is now URL-encoded when it is appended to
the login redirect, as the LDAP invitation path already does, so a URL with a query
string or a fragment arrives as the single value of form_redirect_uri.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
test: password reset redirect keeps the carried URL as one login redi…
14c6147 
…rect parameter

After a password is set through the reset flow, which the self-service signup
confirmation also uses, the redirect URL carried with the reset code is appended to
the login redirect as is. A URL with a query string or a fragment is split into extra
parameters or a fragment instead of arriving as the single value of form_redirect_uri.

Add a filter test, signup-flow tests (default zone and the zone path variant) and a
browser based test with a client that has a wildcard redirect pattern. They expect the
URL to arrive as the single decoded value of form_redirect_uri. The existing checks that
pinned the raw string, including the existing browser based one, now check the decoded
value, so they hold for either encoding.

These tests fail until the production change follows.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
fix: password reset redirect carries the URL as one login redirect pa…
ad50ef9 
…rameter

The redirect URL carried with the reset code is now URL-encoded when it is appended to
the login redirect, as the LDAP invitation path already does, so a URL with a query
string or a fragment arrives as the single value of form_redirect_uri. This covers the
redirect after a self-service signup is confirmed and the password is set.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
test: invitation code re-issued during LDAP acceptance keeps its intent
7a8a5b0 
When a user accepts an invitation by signing in with LDAP credentials, the invitation
code is re-issued, and the re-issued code is handed to the acceptance step. It is
created without an intent, although the code it replaces has one, and unlike the other
places in this controller that re-issue the code.

Add tests, for the default request path and per zone request path mode, that expect the
code re-issued on a successful acceptance, and both codes re-issued when the
authenticated email does not match the invited email, to carry the intent of the code
they replace. The existing tests, which use a code without an intent, are unchanged.

These tests fail until the production change follows.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
fix: invitation code re-issued during LDAP acceptance keeps its intent
5dc63b9 
When a user accepts an invitation by signing in with LDAP credentials, the invitation
code is re-issued without an intent, so the code handed to the acceptance step no longer
says what it was issued for. Re-issue it with the intent of the code it replaces, in the
successful acceptance and in the retry after an email mismatch, as the other places in
this controller already do.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
test: code creation requires an intent
6ce209b 
POST /Codes now has tests that expect an intent to be given, with a clear
400 message when it is missing. Any given intent is stored as supplied.
Existing POST /Codes tests now send an intent.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
fix: code creation requires an intent
f43ef14 
POST /Codes returns a 400 with a clear message when the intent is missing
or blank, and stores the given intent. Codes already stored without an
intent stay redeemable; redeeming one is logged at debug level.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
docs: document POST /Codes and its required intent
4835e4f 
Adds the endpoint to UAA-APIs.rst and the API documentation, with a REST
Docs test that generates the snippets.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
test: passcode login requires the intent the passcode endpoint stores
7b4ccf3 
A code whose data parses as passcode information is only accepted when its
intent is PASSCODE followed by the user id.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
fix: passcode login requires the intent the passcode endpoint stores
13f9e14 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
test: reset password code lifetime is not extended by opening the lin…
2f1c8f7 
…k or a rejected password

Opening the reset link twice keeps showing the form, and neither opening
the link nor a rejected password gives the code a later expiry.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
fix: reset password code lifetime is not extended by opening the link…
30f0efe 
… or a rejected password

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
test: inviting a verified user is reported as a failed invite
2295fd8 
A verified user gets no invitation code and no link; users who are not
verified yet are invited as before.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
fix: inviting a verified user is reported as a failed invite
422409a 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
test: signup and email change answer the same for an existing account…
c99432d 
…, and the confirmation page shows the address

An address that already has a verified account gets the same redirect as a
new address and the owner is emailed instead; the signup confirmation page
shows the address the email was sent to.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
fix: signup and email change answer the same for an existing account,…
bd8a6ba 
… and the confirmation page shows the address

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
refactor: remove the duplicate-email handling in ChangeEmailControlle…
d9d533e 
…r that can no longer be reached

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik @claude
docs: describe how signup and email change treat an address that alre…
35558ee 
…ady has an account

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@fhanik
fhanik force-pushed the pr/self-service-improvement branch from 8bb5db4 to 35558ee Compare October 23, 2026 21:49

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

size/xl

Projects

Foundational Infrastructure Working Group
Status: Inbox

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@fhanik