[common] Expose a telemetry-only Auth UX JS bridge to brokerless (OneAuth) hosts, AB#3688630, Fixes AB#3688630 - #3209
Draft
wzhipan wants to merge 7 commits into
Conversation
|
✅ Work item link check complete. Description contains link AB#3688630 to an Azure Boards work item. |
|
❌ Invalid work item number: AB#3688630
Click here to learn more. |
…s, AB#3688630 Non-brokered (OneAuth) flows never reach the Auth UX JavaScript bridge: it is gated on ProcessUtil.isRunningOnAuthService(), which is permanently false outside the broker's isolated :auth process. Those flows therefore cannot report the onboarding error codes the log_telemetry action exists to carry, even though the OneAuth core already builds the onboarding seed for them. Rather than widening the existing gate -- which would hand every MSAL client the full bridge, including the number-match device store -- this adds a second, narrower surface: - AuthUxJavaScriptInterface takes a telemetryOnly capability. When set it serves log_telemetry and refuses every other action, so the only effect a page can have is appending to the onboarding telemetry blob. Enforced at dispatch rather than only at the registration gate, so a future caller that constructs the bridge for a brokerless host cannot re-expose the mutating path by forgetting a check at its own call site. - shouldExposeJavaScriptInterface now selects the flight by host: the broker's :auth process keeps ENABLE_JS_API_FOR_AUTHUX, and any other host is gated by the new ENABLE_BROKERLESS_TELEMETRY_JS_API_FOR_AUTHUX (default off). Keeping the flights independent means the brokerless surface can be turned off without also disabling number-matching for the broker. The host allow-list (H1) still applies to both. - The capability is resolved in createAuthUxJavaScriptInterface(), because onPageStarted rebuilds the bridge on every navigation; a capability passed at a single call site would be silently dropped on the next page load, which is exactly how the telemetry sink was lost before #3201. Behaviour inside the broker is unchanged: the constructor default is false and the :auth branch of the gate is the same predicate it was. Tests (+8): number_matching inert in telemetry-only mode and its :auth-process control (so "the store stayed empty" cannot pass for the wrong reason); log_telemetry still served; a refused message does not abort the page; the constructor default; and the four gate combinations. Revert-tested -- dropping the dispatch guard fails "a telemetry-only bridge must never write to the number-match store", and collapsing the two flights fails "the broker's flight must not expose a bridge outside the :auth process". Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
wzhipan
force-pushed
the
copilot/pbi-3688630-telemetry-only-bridge
branch
from
August 8, 2026 04:07
a1b3abd to
433445e
Compare
github-actions
Bot
changed the title
…#3197 squash-merge) into the telemetry-only bridge branch
# Conflicts: # common/src/main/java/com/microsoft/identity/common/internal/ui/webview/AzureActiveDirectoryWebViewClient.java
…ht, AB#3688630
The ENABLE_BROKERLESS_TELEMETRY_JS_API_FOR_AUTHUX flight added earlier in this PR
could never have been turned on in production, so the feature would have shipped
as dead code.
CommonFlightsManager only returns real ECS values in a process that called
initializeCommonFlightsManager, and the broker is its ONLY caller -- neither
oneauth nor msal ever does. The brokered flow works because its WebView runs
inside the broker's :auth process; the non-brokered flow runs in the host
application's own process, where the manager falls back to
DefaultValueFlightsProvider and every flight resolves to its compiled-in default.
The brokerless flight was default-off, so it would have read false forever, in
every host, regardless of how it was ramped.
Exposure now follows the onboarding seed instead: the telemetry-only bridge is
registered only when the request carries an onboarding telemetry recorder. This
is a better gate than the flight was even setting the ECS problem aside --
- it is self-limiting: the bridge exists only to append to the onboarding blob,
so without a recorder it could have no effect anyway;
- plain MSAL clients never seed onboarding telemetry and are therefore never
exposed, which was the widening the two-flight split existed to prevent;
- the off switch becomes "stop seeding", a decision the caller already owns per
request, in a process where it actually takes effect.
The brokered path is untouched: inside :auth the full bridge (which includes the
number-match device store) stays behind ENABLE_JS_API_FOR_AUTHUX, which is real
there. The host allow-list still gates both.
The recorder is available in time: WebViewAuthorizationFragment attaches it
(L393) before both initializeAuthUxJavaScriptApi (L399) and launchWebView (L400),
and hasOnboardingTelemetryRecorder() is read per navigation rather than captured,
so a recorder attached or cleared later is honoured.
Tests: the four flight-based gate tests become five recorder-based ones, adding
one for the gate closing again when the recorder is cleared. Revert-tested --
forcing the gate open fails "the broker's flight must not expose a bridge outside
the :auth process" and "the gate must be re-evaluated, not captured at first
navigation". 212 green.
Note this is exactly what the local E2E run could not catch: it flipped the
flight default to true in the build under test, which masked the deployment
problem while validating the code path.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
✅ Work item link check complete. Description contains link AB#3688630 to an Azure Boards work item. |
One entry per PR across the stack, matching the repo convention (89 of 90 released entries are single-entry; median ~101 chars). The #3209 line drops from 562 to 224 characters and now leads with what a reader needs -- non-brokered flows get a telemetry-only bridge, gated on the onboarding seed -- leaving the rationale to the PR description. Also de-duplicated #3201 and #3204: merging each consolidation up the stack APPENDED the new line rather than replacing the old ones, so the block had to be rebuilt canonically. Worth remembering -- a changelog edit merged upward is not idempotent, and the count has to be re-censused after every merge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…688630-telemetry-only-bridge
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
1 participant
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.
Related work item: AB#3688630
Why
Non-brokered (OneAuth) flows never reach the Auth UX JavaScript bridge. Exposure is gated on
ProcessUtil.isRunningOnAuthService(), which is permanently false outside the broker's isolated:authprocess — no OneAuth manifest declares that process. So a brokerless flow cannot report theonboarding error codes
log_telemetryexists to carry, even though the OneAuth core already buildsthe onboarding seed for it (
InteractiveRequest.cppcreates the seed forMODE_NON_BROKEREDtoo).The obvious fix — relax
isRunningOnAuthService— is the wrong one. That predicate is the only thingkeeping the full bridge, including the number-match device store, inside the broker's isolated
process. Removing it would hand that surface to every MSAL client that renders an Auth UX page.
What
A second, narrower surface instead of a wider single one.
1. The bridge takes a
telemetryOnlycapability. When set it serveslog_telemetryand refusesevery other action, so the only effect a page can have is appending to the onboarding telemetry blob.
Enforced at dispatch, not only at the registration gate. A restriction that lives only in the
gate is one forgotten check away from being void; enforcing it inside the bridge means a future
caller that constructs it for a brokerless host cannot re-expose the mutating path from its own call
site. The
whenblock already matchedaction_name == log_telemetryfirst (deliberately, so aparams.operationsmuggled into a telemetry message can never reach the number-match store), so theguard sits on the one remaining mutating branch.
2. Exposure is gated by host — and outside
:auth, deliberately not by a flight.:authprocessENABLE_JS_API_FOR_AUTHUX(unchanged)An earlier revision of this PR used a second CommonFlight here. That flight could never have been
turned on, so the feature would have shipped as dead code:
CommonFlightsManageronly returns realECS values in a process that called
initializeCommonFlightsManager, and the broker is its onlycaller — neither
oneauthnormsalever does. The brokered flow works because its WebView runsinside
:auth; a brokerless host falls back toDefaultValueFlightsProviderand reads thecompiled-in default, forever.
Gating on the seed is better than the flight was even setting that aside:
can have no effect at all;
widening a second flight was meant to prevent;
where it actually takes effect.
3. The capability is resolved in
createAuthUxJavaScriptInterface().onPageStartedrebuildsthe bridge on every navigation. A capability passed at a single call site would be silently dropped
on the next page load — which is exactly how the telemetry sink was lost before #3201 fixed it.
hasOnboardingTelemetryRecorder()is likewise read per navigation rather than captured, so arecorder attached or cleared later is honoured. The recorder is available in time:
WebViewAuthorizationFragmentattaches it (L393) before bothinitializeAuthUxJavaScriptApi(L399)and
launchWebView(L400).Behaviour inside the broker is unchanged: the constructor default is
false, and the:authbranchof the gate is the same predicate and the same flight it was.
Tests
+11, all revert-tested.
telemetry-only mode makes number_matching inertAuthService_KeepsTheFullBridge:authtelemetry-only mode still forwards log_telemetrytelemetry-only mode does not suppress the whole messagedefault mode is not telemetry-onlyBrokerlessHost_ClosedWithoutARecorderBrokerlessHost_OpenedByTheRecorderBrokerlessHost_ClosesWhenRecorderClearedBrokerlessHost_StillHonoursUriAllowListAuthService_UsesTheBrokerFlight:auththe flight decides; a recorder is neither required nor sufficientThe control test matters more than it looks:
NumberMatchHelper.storeNumberMatchsilently drops asession id that is not a GUID or 8-char alphanumeric, so a malformed fixture would have made "the
store stayed empty" true for the wrong reason and the security assertion vacuous.
212 tests green in area.
Verified on a device
Non-brokered E2E on a real device (OneAuthTestApp, broker deliberately uninstalled): the recorder is
registered, found by correlation id, the bridge binds with
telemetryOnly=true, alog_telemetryposted over real JavaScript reaches the blob as
"onboarding_mode":"non-brokered", and anumber_matchingmessage is refused with the store left empty.true, which is preciselywhat masked the flight problem described above — worth knowing when weighing the E2E evidence.
What this does not do
No host turns this on yet. The OneAuth-side work — registering its already-built
OnboardingTelemetryRecorderinOnboardingRecorderRegistryso the WebView fragment can find it —lives in the OneAuth repo and is tracked separately. It is ~6 lines, verified on device.