Skip to content

fix(iframe-app): prevent client-side open redirect in EasyAuth login popup#9373

Open
rllyy97 wants to merge 2 commits into
mainfrom
rllyy97-fix-client-side-open-redirect
Open

fix(iframe-app): prevent client-side open redirect in EasyAuth login popup#9373
rllyy97 wants to merge 2 commits into
mainfrom
rllyy97-fix-client-side-open-redirect

Conversation

@rllyy97

@rllyy97 rllyy97 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

Fixes CodeQL code-scanning alert #100Client-side Open URL redirect (SM05376, CWE-601 / CWE-79).

The iframe-app EasyAuth login popup (openLoginPopup in apps/iframe-app/src/lib/authHandler.ts) built its target URL by concatenating the baseUrl and signInEndpoint options and passed the result straight to window.open. Those values ultimately derive from window.location (via the agentCard URL parameter), so CodeQL flagged the window.open call as an open-redirect sink — a crafted value could navigate the login popup to a malicious origin (e.g. https://trusted@evil.example.net/...).

The fix adds getValidatedLoginUrl(), which normalizes the URL with the URL constructor and strictly validates it before opening — the exact pattern recommended in the alert guidance (normalize as a URL object, then strict origin/host match):

  • Enforces https: (allows http: only for localhost / 127.0.0.1 during local development).
  • Allows the hosting page's own origin (window.location.origin) — EasyAuth lives on the app's own origin.
  • Otherwise requires a trusted Microsoft Logic Apps host (*.logic.azure.com / *.logic-apps.azure.com), kept in sync with the existing agentCard allowlist in config-parser.ts.
  • Untrusted or malformed URLs are rejected via the existing onFailed callback and never reach window.open.

Impact of Change

  • Users: No change to legitimate login flows (same-origin and trusted Logic Apps domains behave exactly as before). Malicious/untrusted redirect targets are now blocked instead of opened.
  • Developers: No API changes. openLoginPopup gains an internal validation step; untrusted inputs now surface through the existing onFailed callback.
  • System: Closes a client-side open-redirect vector (CWE-601) in the A2A chat iframe app. No new dependencies.

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in: local vitest run (apps/iframe-app)

Added unit tests in authHandler.test.ts covering: a trusted cross-origin Logic Apps domain (allowed), an untrusted origin (blocked), a sign-in endpoint escaping the base origin via userinfo @ (blocked), a non-HTTPS non-localhost URL (blocked), and a malformed URL (blocked). All existing openLoginPopup tests continue to pass.

  • authHandler.test.ts — 41 tests pass
  • ✅ Full iframe-app suite — 280 passed / 1 skipped
  • biome check clean on changed files

Contributors

Screenshots/Videos

N/A — no visual changes.

…popup

The EasyAuth login popup built its target URL by concatenating the (user-influenceable) baseUrl and signInEndpoint and passed it straight to window.open, which CodeQL flagged as a client-side open-redirect sink (SM05376 / CWE-601, alert #100).

Normalize the URL with the URL constructor and strictly validate it before opening: enforce https (http allowed only for localhost during dev), allow the hosting page's own origin, and otherwise require a trusted *.logic.azure.com / *.logic-apps.azure.com host. Untrusted or malformed URLs are rejected via onFailed and never reach window.open.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 7, 2026 17:16
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: fix(iframe-app): prevent client-side open redirect in EasyAuth login popup
  • Issue: None. The title is specific, descriptive, and matches the change.
  • Recommendation: No change needed.

Commit Type

  • Properly selected (fix).
  • Only one commit type is selected, which is correct.

⚠️ Risk Level

  • The selected label/body risk level is Low, and the PR label also matches (risk:low).
  • Based on the diff, low is reasonable, though this is a security-related change; if you want to be extra conservative, you could consider whether your team prefers medium for auth/redirect handling changes. As submitted, it is consistent and acceptable.

What & Why

  • Current: Clear and detailed explanation of the CodeQL alert, the vulnerability, and the validation approach.
  • Issue: None. This is well written and sufficiently explains the motivation and fix.
  • Recommendation: No change needed.

Impact of Change

  • The impact section is filled out clearly and accurately for users, developers, and system impact.
  • Recommendation:
    • Users: Good as written.
    • Developers: Good as written.
    • System: Good as written.

Test Plan

  • Unit tests were added/updated in the diff, which satisfies the test-plan requirement.
  • The body clearly references the updated authHandler.test.ts coverage and local vitest results.

⚠️ Contributors

  • Blank, which is allowed.
  • Recommendation: Optional, but if anyone helped with the security review or implementation, consider adding them for credit.

Screenshots/Videos

  • Marked N/A, which is appropriate because there are no visual changes.

Summary Table

Section Status Recommendation
Title No changes needed
Commit Type No changes needed
Risk Level Matches label/body; optional reconsideration only if your team treats security fixes as medium
What & Why No changes needed
Impact of Change No changes needed
Test Plan Unit tests in diff satisfy the requirement
Contributors ⚠️ Optional: add contributors if applicable
Screenshots/Videos N/A is appropriate

Overall: this PR passes. The advised risk level remains low, matching the submitter’s assessment.


Last updated: Tue, 07 Jul 2026 17:43:01 GMT

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 addresses a CodeQL “Client-side Open URL redirect” finding in the iframe-app EasyAuth login flow by validating the computed login URL before passing it to window.open, preventing attacker-influenced navigation to untrusted origins.

Changes:

  • Added getValidatedLoginUrl() plus a Logic Apps domain allowlist to normalize/validate login popup URLs.
  • Updated openLoginPopup to block untrusted/invalid URLs and invoke onFailed instead of opening the popup.
  • Added unit tests covering trusted cross-origin domains and several blocked URL cases (untrusted origin, userinfo @ escape attempt, non-HTTPS, malformed URL).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
apps/iframe-app/src/lib/authHandler.ts Adds URL normalization/allowlist validation and blocks unsafe window.open navigation.
apps/iframe-app/src/lib/tests/authHandler.test.ts Adds test coverage for allowed/blocked login popup URLs.

Comment thread apps/iframe-app/src/lib/authHandler.ts
Comment thread apps/iframe-app/src/lib/authHandler.ts Outdated
Comment thread apps/iframe-app/src/lib/authHandler.ts
- Reject login popup URLs that embed userinfo (user:pass@host), a URL
  spoofing vector, even when the host is otherwise trusted
- Extract the trusted Logic Apps domain allowlist into a shared
  trusted-domains module so authHandler and config-parser can't drift
- Make LoginPopupOptions.signInEndpoint required to match IdentityProvider
- Add a test covering userinfo rejection on a trusted host

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rllyy97 rllyy97 added the risk:low Low risk change with minimal impact label Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

📊 Coverage Check

🎉 All changed files have adequate test coverage!

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

Labels

pr-validated risk:low Low risk change with minimal impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants