Pr/issue 3701#3951
Open
fhanik wants to merge 6 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses Issue #3701 by changing UAA’s external OAuth callback handling so that a “stale state” (common with multiple concurrent login attempts in different tabs) can be surfaced as a user-friendly /oauth_error page rather than a cryptic “Invalid state parameter” failure.
Changes:
- Detect “concurrent login attempt” on external OAuth callback state mismatches and redirect to
/oauth_errorwith a dedicated session flag. - Render a new friendly concurrent-login message + “Start a new login” link on the external auth error page when that flag is present.
- Add unit, view, and Selenium integration tests covering the concurrent-login scenario and the new error page behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationFilter.java |
Adds concurrent-login detection/redirect behavior when state mismatches. |
server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ConcurrentLoginAttemptException.java |
Introduces a dedicated exception used to signal the concurrent-login scenario. |
server/src/main/java/org/cloudfoundry/identity/uaa/home/HomeController.java |
Consumes oauth_concurrent_login from the session and exposes it to the view. |
server/src/main/resources/templates/web/external_auth_error.html |
Displays a friendly concurrent-login message and restart link when oauth_concurrent_login is set. |
server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationFilterTest.java |
Extends filter unit tests to cover concurrent-login behavior; includes minor test refactors. |
server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthConcurrentLoginScenarioTest.java |
Adds scenario-style tests documenting/validating expected concurrent-login behavior. |
server/src/test/java/org/cloudfoundry/identity/uaa/login/HomeControllerViewTests.java |
Adds view tests asserting the concurrent-login message/link and session attribute consumption. |
uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/pageObjects/OAuthErrorPage.java |
Adds a Selenium page object for /oauth_error assertions and interaction. |
uaa/src/test/java/org/cloudfoundry/identity/uaa/integration/feature/ConcurrentExternalLoginIT.java |
Adds an integration test reproducing the multi-tab stale-state flow and asserting the friendly error page. |
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
strehle
previously approved these changes
Jun 19, 2026
strehle
left a comment
strehle
left a comment
Member
There was a problem hiding this comment.
not sure if you want do more or if this is ready
github-project-automation
Bot
moved this from Inbox
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
…pt detected (#3701) When a user initiates external OAuth login from two browser tabs, UAA stores only the most-recent state in the HTTP session (overwriting the older one). If the older tab's IDP callback arrives, the state check fails with a cryptic error. This detects the specific case (session has a non-null state that differs from the incoming state) and surfaces a human-readable "Another sign-in is already in progress" message with a "Start a new login" link, while keeping the existing CSRF protection intact for all other state-failure cases. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
@strehle ready for review and merge. |
…ogin Use getSession(false) in the callback state check so an expired/missing session is rejected as an invalid login request instead of silently creating a new stateless session, and handle HttpSessionRequiredException in the filter alongside CsrfException. Add a security-monitoring log for rejected callbacks and update the affected unit tests. Co-authored-by: Cursor <cursoragent@cursor.com>
Member
|
I think you see the commit from agent, so you can rebase the commits... |
strehle
previously approved these changes
Jun 19, 2026
strehle
approved these changes
Jun 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR for Issue #3701 - Still needs review before merge