Skip to content

Return safe plain-text OAuth callback errors - #1905

Open
Paul Lizer (paullizer) wants to merge 2 commits into
Developmentfrom
paullizer-oauth-error-responses
Open

Paul Lizer (paullizer) wants to merge 2 commits into
Developmentfrom
paullizer-oauth-error-responses

Conversation

@paullizer

@paullizer Paul Lizer (paullizer) commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Both ordinary provider-error branches in /getAToken and /getATokenApi now return an explicit text/plain; charset=utf-8 HTTP 400 with the stable message Login failed. Please try signing in again. Provider text is not reflected; [AUTH_CALLBACK] diagnostics contain only a recognized error code, the callback path, and description-presence metadata.
  • Preserve missing-code behavior, login/API and Teams contracts, and Microsoft 365 prefix dispatch, state/session/account/nonce guards, principal identity, and separate credentials. Add 33 real Flask runtime regressions; repair only the approved stale group-helper imports in the existing personal-notification test fixture. Production scope remains one route module plus the 0.261.423 version bump; the follow-up contains only an equivalent test-fixture simplification and assigned release/reference documentation.

Linked issue

Refs #1903 — historical, non-closing reference to the Development integration baseline. No new issue was requested or created.

Release Notes & Latest Features

  • New Feature
  • Bug Fix
  • UI Enhancement
  • Breaking Change
  • Internal only

Is this visible to end users?

  • Yes
  • No

Is this admin-facing (Admin Settings, governance, deployment, config)?

  • Yes
  • No

Should this become a Latest Feature card?

  • Yes
  • No
  • Already added

Screenshot needed for the card?

  • Yes
  • No
  • Attached

Version bump

  • application/single_app/config.py VERSION third segment bumped, or not needed because this is docs-only
  • deployers/version.txt bumped, or not needed because deployers/ was not changed

0.261.422 → 0.261.423, using the assigned first version slot. Development was refreshed immediately before finalization and again before the original commit; it remained 4dd6d80230fe60e3cbfe8544d1d52308a14a7387. The documentation/test-only follow-up does not change production code or bump the version again. Deployer files were not changed.

Testing / validation

  • Windows test environment: $env:PYTHONPATH = "$PWD\application\single_app;$PWD\functional_tests". The Cosmos network-guard plugin blocks external socket/DNS access before collection. No dependency installation or shared-host package changes were needed.
  • Final head: 74b2a23f382534c5a48969d22924758be5f3b284. Requested runtime selections were rerun after the equivalent test-only correction; route-policy, exact committed-diff guardrails, syntax, whitespace, and generator checks were reconfirmed on the final head. These are offline runtime fixtures, not live Azure/Graph, full-app bootstrap, or Azure Playwright browser validation. The fixed body and actual declared media type are verified at the Flask HTTP boundary; production security-header and bootstrap wiring are unchanged.
Command Result
python -m pytest -q -p test_support.cosmos_network_guard functional_tests\test_getatoken_missing_code_redirect.py functional_tests\test_m365_routes.py functional_tests\test_m365_connections.py functional_tests\test_m365_chat_connection_persistence.py functional_tests\test_oauth_error_responses.py 150 passed; 119 subtests passed, including all 33 new runtime regressions; rerun after the test-only correction.
python .\scripts\check_swagger_routes.py .\application\single_app\route_frontend_authentication.py .\application\single_app\config.py Passed; 1 changed route file validated.
python .\functional_tests\route_tests\test_route_blueprint_policy_inventory.py 15/15 passed on the final head.
python .\functional_tests\route_tests\test_route_unauthenticated_policy_contract.py 10/10 passed on the final head.
python .\functional_tests\route_tests\test_route_policy_test_coverage.py 3/3 passed on the final head.
python .\scripts\check_broken_access_control.py --base-sha origin/Development --head-sha HEAD .\application\single_app\route_frontend_authentication.py .\application\single_app\config.py Passed; exact final committed diff, 2 application files.
python .\scripts\check_xss_sinks.py --base-sha origin/Development --head-sha HEAD .\application\single_app\route_frontend_authentication.py .\application\single_app\config.py Passed; exact final committed diff, 2 application files.
python -m pytest -q -p test_support.cosmos_network_guard functional_tests\test_docs_release_notes_integrity.py functional_tests\test_docs_app_surface_coverage.py functional_tests\test_docs_site_quality.py functional_tests\test_docs_latest_release_catalog_parity.py functional_tests\test_docs_published_references.py::test_release_generation_rejects_source_relative_reader_links 21 passed after the assigned release/reference update; source-site checks only, no built-site validation.
python .\scripts\build_release_notes_pages.py Regenerated 15 pages while preserving all 317 source release sections.
python .\scripts\build_release_notes_pages.py --check Passed; split release pages are current.
python .\scripts\build_latest_release_docs.py --check Passed; the independent app-driven Latest Features catalog remains unchanged.
python -m py_compile .\application\single_app\route_frontend_authentication.py .\application\single_app\config.py .\functional_tests\test_oauth_error_responses.py .\functional_tests\test_m365_routes.py Passed.
git -c core.whitespace=blank-at-eol,blank-at-eof,space-before-tab,cr-at-eol diff --cached --check Passed before each commit.
git -c core.whitespace=blank-at-eol,blank-at-eof,space-before-tab,cr-at-eol diff --check origin/Development...HEAD Passed; all 14 committed files.
gh run watch 42388538946 --repo microsoft/simplechat --interval 30 --exit-status Success on final head; Python, JavaScript/TypeScript, and Actions CodeQL jobs all passed.
gh run watch 42388538920 --repo microsoft/simplechat --interval 30 --exit-status Success; fresh Cosmos Response Contracts passed on final head.

Before-change evidence:

Command Baseline result
python -m pytest -q -p test_support.cosmos_network_guard functional_tests\test_getatoken_missing_code_redirect.py functional_tests\test_m365_routes.py functional_tests\test_m365_connections.py functional_tests\test_m365_chat_connection_persistence.py At unchanged Development: 116 passed, 1 failed. The existing notification fixture lacked the assert_group_role/get_user_groups imports now required by functions_notifications.py. The approved test-only parity repair rejects group access, represents no memberships, asserts neither helper is called, and retains all subject/deduplication/fault assertions.
python -m pytest -q -p test_support.cosmos_network_guard functional_tests\test_oauth_error_responses.py --tb=short Before callback edits: 10 failed, 23 passed. Both real registered callbacks returned text/html for the five provider-error cases; the compatibility cases already passed.

Fresh exact-head CodeQL evidence:

CodeQL Advanced run 42388538946 succeeded for head 74b2a23f382534c5a48969d22924758be5f3b284. Python job 128451004105 produced analysis 2118612303 on refs/pull/1905/merge, merge SHA e6aca9976db7f7b8e741a5312466adf396209b1d, with analysis key .github/workflows/codeql.yml:analyze and category /language:python. It reports 0 results across 172 rules, with no processing errors or warnings. Downloaded SARIF confirms both py/reflective-xss and py/unnecessary-lambda remain enabled and reports no results; the two selected callback findings and selected alert instances are absent on this PR ref.

The first PR scan removed the two selected callback findings but identified one redundant lambda in the new test-only settings seam. The final commit replaces it with the identical dict callable; the final scan above confirms the no-new-findings criterion. Final JavaScript/TypeScript analysis 2118596556 and Actions analysis 2118589418 also report 0 results on the same merge SHA. Baseline Python analysis 2117078539 retains the exact two original callback sinks for before/after comparison.

Fresh Cosmos contracts run 42388538920 also succeeded for final head; job 128451003970 passed with external network blocked before collection. Scanning configuration, rules, severity, exclusions, and dismissals are unchanged. This is PR-ref evidence, not a claim that existing Development/main findings are globally closed. Final Development verification is parent-owned after a user-approved merge; this PR must not be merged automatically.

Documentation

  • Release notes updated, or not needed
  • Feature documentation updated, or not needed
  • Fix documentation updated, or not needed

Fix documentation: docs/explanation/fixes/OAUTH_ERROR_RESPONSE_XSS_FIX.md, version 0.261.423. The coordinator-assigned concise Bug Fix entry is under v0.261.423 in docs/explanation/release_notes.md, and the split release pages were regenerated. The required generator's version-index/archive-boundary reflow accounts for the extra documentation diff; all 317 source release sections are preserved. Historical #1903 references are non-closing. No application surface or Latest Features card was added or renamed; the application inventory and app-driven catalog are unchanged.

Security checklist

  • New Flask routes include @swagger_route(security=get_auth_security())
  • Settings sent to non-admin frontends use sanitize_settings_for_user()
  • Browser JavaScript is served from local SimpleChat static assets only; no CDN-hosted JS
  • No secrets, keys, connection strings, or local-only artifacts are included

No new routes, settings payloads, browser assets, shared Graph/M365 helper edits, deployment changes, or deferred findings are included. Both non-amended commits include the required co-author trailer. Every merge still requires the user's approval; auto-merge remains disabled.

Return a stable plain-text HTTP 400 for provider errors in both registered callbacks, with safe server-side diagnostics. Preserve login, Teams, and independent Microsoft 365 flows with offline runtime regressions; repair the approved stale M365 test stub and finalize app version 0.261.423.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add the assigned v0.261.423 Bug Fix release note and historical non-closing Refs #1903 reference, regenerate split release pages without dropping source history, and remove the redundant test settings wrapper reported by CodeQL. Keep production code and the application version unchanged from the original fix.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants