From 9e4f318faba0aaeb759372df9eea4a0815cfd63a Mon Sep 17 00:00:00 2001 From: Zhipan Wang Date: Fri, 7 Aug 2026 00:55:06 -0700 Subject: [PATCH 1/3] [common] Wire onboarding recorder into the :auth WebView host (brokered), AB#3708195 The broker builds an onboarding telemetry recorder per request from the seed, but the WebView that renders the interactive auth / remediation pages lives in a separate AuthorizationActivity created by the OS from an Intent. A live recorder cannot ride an Intent, so the WebView-side onboarding hooks were inert for brokered flows: steps observed in the WebView (MDM enrollment, Company Portal launch, broker install), the last loaded domain, and the Auth UX log_telemetry error code never reached the blob the broker finalizes and returns. Both components run in the broker :auth process, so this adds an in-process handoff keyed by the request correlationId. OnboardingRecorderRegistry (new) - Rejects DiagnosticContext.UNSET_CORRELATION_ID as a key. Every thread whose request context was never set carries that sentinel, and the authorization Intent extra is populated by reading the request-context map directly rather than through getThreadCorrelationId(), so the raw sentinel can reach the registry. It is shared by definition: accepting it would let two unrelated requests resolve to the same recorder and merge one flow's blocking errors into the other's uploaded blob. Rejecting it makes the feature inert (and logs a warning) instead of silently mis-attributing telemetry. - Bounded, access-ordered map (cap 16). Entries must be unregistered on terminal outcome, and because this is process-static state in a process that lives as long as the device is up, a missed unregister would otherwise leak a recorder permanently - the recorder object and its collected steps / blocking errors, not an Activity, since OnboardingTelemetryRecorder holds only the application context. On overflow it evicts the least-recently-used entry and logs a warning, bounding the leak while surfacing the underlying bug. AuthorizationFragment - Captures the correlation id in extractState and round-trips it through onSaveInstanceState. The read was already a base-class responsibility, so the save belongs next to it: putting it in one subclass left the other two (BrowserAuthorizationFragment, CurrentTaskBrowserAuthorizationFragment) still blanking their diagnostic context after activity recreation, and invited the pair to drift apart again. All three subclasses already call super, so the parent implementation fixes all of them with no duplication. - This also repairs a pre-existing bug on that path: extractState previously fed setDiagnosticContextForNewThread(null) after a recreation, so every subsequent log line for the request lost its correlation id. WebViewAuthorizationFragment - Resolves the recorder in onCreateView from the inherited mCorrelationId and attaches it to the WebView client before initializeAuthUxJavaScriptApi(...). No-op when the request seeded no recorder, which is the MSAL-client path. DiagnosticContext - UNSET_CORRELATION_ID is now public, and the internal duplicate string literal in getThreadCorrelationId() references it. Callers that use the correlation id as a KEY rather than for logging have to reject the sentinel explicitly, and a copied "UNSET" literal in the registry would silently stop matching if this ever changed - reintroducing cross-request contamination with no signal. Logger - Notes that its own UNSET literal is a display placeholder, deliberately independent of the DiagnosticContext sentinel: it also stands in for a missing thread id and is never used as a key, so it must not be collapsed into the new constant. Note this PR alone changes no behaviour for the recorder handoff: register/unregister have no caller in this repo (both live in the paired broker PR). Merge order: #3197 -> #3201 -> #3204 -> broker PR. Tests - OnboardingRecorderRegistryTest (15): keying, the UNSET sentinel (including the concrete two-request cross-wiring it prevents), removal / lifecycle, absent-key behaviour, null and empty inputs, idempotent unregister, and the leak bound. - AuthorizationFragmentCorrelationIdTest (6, new): the fragment half of the handoff - extractState captures the id, the captured id resolves the registered recorder, it survives save/restore, and the no-recorder / missing-id / sentinel paths resolve to null rather than throwing. Without these, a regression in the correlation-id plumbing would leave every hook inert while the registry tests still passed. Revert-tested: removing the sentinel guard fails with "the sentinel must never become a key expected:<0> but was:<1>"; disabling the cap fails with "expected:<16> but was:<100>"; switching eviction to insertion order evicts a live in-use recorder; dropping the onSaveInstanceState round-trip fails with "the correlation id must round-trip through the saved bundle expected:<...> but was:". Verified end-to-end on device: built brokerHost against this change and drove a brokered MAM/Conditional-Access flow; the finalized blob carried the WebView- observed steps, last_loaded_domain, and blocking_errors ["530003"] from the Auth UX bridge. 246 tests green across the onboarding / Auth UX / authorization-fragment suites. --- changelog.txt | 3 + .../oauth2/AuthorizationFragment.java | 28 +- .../oauth2/WebViewAuthorizationFragment.java | 22 ++ .../telemetry/OnboardingRecorderRegistry.kt | 198 ++++++++++++++ ...uthorizationFragmentCorrelationIdTest.java | 176 ++++++++++++ .../OnboardingRecorderRegistryTest.kt | 251 ++++++++++++++++++ .../java/logging/DiagnosticContext.java | 16 +- .../identity/common/java/logging/Logger.java | 8 + 8 files changed, 698 insertions(+), 4 deletions(-) create mode 100644 common/src/main/java/com/microsoft/identity/common/internal/telemetry/OnboardingRecorderRegistry.kt create mode 100644 common/src/test/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationFragmentCorrelationIdTest.java create mode 100644 common/src/test/java/com/microsoft/identity/common/internal/telemetry/OnboardingRecorderRegistryTest.kt diff --git a/changelog.txt b/changelog.txt index 1b53c4aee9..148654d9dd 100644 --- a/changelog.txt +++ b/changelog.txt @@ -9,6 +9,9 @@ vNext - [MINOR] Record the Auth UX log_telemetry server error code in the onboarding telemetry blob's blocking-errors list, excluding non-onboarding AADSTS codes (50058/50097/50126) for iOS parity and de-duplicating repeats at the WebView client (one per authorization request), leaving the recorder's list append-only and chronological for other callers (#3201) - [PATCH] Restrict page-supplied Auth UX error codes to numeric server codes and cap the number recorded per authorization request, so a page cannot post one of the broker's own symbolic blocking-error constants or grow the onboarding blob without bound (#3201) - [PATCH] Fix Auth UX JavaScript bridge availability when the initial authorization URL is not allow-listed, and stop injecting the postMessageToBroker shim after the interface has been removed - also affects the number-matching path (#3201) +- [MINOR] Wire the broker's onboarding telemetry recorder into the :auth WebView host via a bounded, correlation-id-keyed registry, activating the previously inert WebView onboarding hooks for brokered flows; the registry rejects the DiagnosticContext unset-correlation-id sentinel so two requests without a request context can never share a recorder (#3204) +- [MINOR] Expose DiagnosticContext.UNSET_CORRELATION_ID so callers that use the correlation id as a key, rather than only logging it, can reject the shared unset sentinel explicitly (#3204) +- [PATCH] Round-trip the authorization request's correlation id through AuthorizationFragment's saved instance state, so a recreated fragment restores its diagnostic context instead of blanking it (affects all three authorization fragments; previously every log line after a recreation lost its correlation id) (#3204) - [PATCH] Stop a best-effort session-correlation persistence failure (e.g. SharedPreferences unavailable on credential-encrypted storage before first unlock) from propagating out of OnboardingTelemetryRecorder.addBlockingError after the error was already appended, which also affects the broker's blocking-error call sites (#3201) Version 24.5.0 diff --git a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationFragment.java b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationFragment.java index 5ee8466932..fbe75a36d5 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationFragment.java +++ b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationFragment.java @@ -76,6 +76,17 @@ public abstract class AuthorizationFragment extends Fragment { */ protected boolean mAuthResultSent = false; + /** + * Correlation id of this authorization request, read from the state bundle in + * {@link #extractState(Bundle)}. + * + *

Lives here rather than in a subclass because the read is a base-class responsibility: this + * class restores the diagnostic context from it, and {@link #onSaveInstanceState(Bundle)} below + * round-trips it so a recreated fragment restores that context instead of blanking it. Keeping + * the save next to the read means a new subclass cannot forget it. + */ + protected String mCorrelationId; + /** * Listens to an operation cancellation event. */ @@ -165,7 +176,22 @@ user commit() rather than commitNow() that the fragment manager that we were rem * @param state a bundle containing data provided when the activity was created */ void extractState(@NonNull final Bundle state) { - setDiagnosticContextForNewThread(state.getString(DiagnosticContext.CORRELATION_ID)); + mCorrelationId = state.getString(DiagnosticContext.CORRELATION_ID); + setDiagnosticContextForNewThread(mCorrelationId); + } + + /** + * {@inheritDoc} + * + *

Round-trips the correlation id, which {@link #extractState(Bundle)} reads back on the + * recreation path. Without this the key is absent from the saved bundle and a recreated fragment + * blanks its diagnostic context, so every subsequent log line for the request loses its join + * key — and, for the WebView fragment, the onboarding recorder can no longer be resolved. + */ + @Override + public void onSaveInstanceState(@NonNull final Bundle outState) { + super.onSaveInstanceState(outState); + outState.putString(DiagnosticContext.CORRELATION_ID, mCorrelationId); } /** diff --git a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java index 7cd29111ad..c5f9c44e27 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java +++ b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java @@ -75,6 +75,9 @@ import com.microsoft.identity.common.internal.ui.webview.WebViewUtil; import com.microsoft.identity.common.internal.ui.webview.switchbrowser.SwitchBrowserStatusCallback; import com.microsoft.identity.common.internal.ui.webview.switchbrowser.SwitchBrowserProtocolCoordinator; +import com.microsoft.identity.common.internal.telemetry.OnboardingRecorderRegistry; +import com.microsoft.identity.common.internal.telemetry.OnboardingTelemetryRecorder; +import com.microsoft.identity.common.java.logging.DiagnosticContext; import com.microsoft.identity.common.java.WarningType; import com.microsoft.identity.common.java.constants.FidoConstants; import com.microsoft.identity.common.java.exception.ClientException; @@ -374,6 +377,25 @@ public Map getUrlStatusMap() { } ); setUpWebView(view, mAADWebViewClient); + + // Onboarding telemetry (brokered): if AccountChooser seeded a recorder for this request, + // hand it to the WebView client so WebView-observed onboarding steps (MDM enrollment, + // Company Portal launch, broker install) and the Auth UX log_telemetry error code are + // recorded into the same blob the broker finalizes and returns. Keyed by correlationId via + // OnboardingRecorderRegistry (owner + WebView both run in the broker :auth process). No-op + // when the request seeded no recorder, or when the correlation id is unusable as a key. + // AB#3708195. + final OnboardingTelemetryRecorder onboardingRecorder = + OnboardingRecorderRegistry.get(mCorrelationId); + if (onboardingRecorder != null) { + Logger.info(methodTag, mCorrelationId, + "Onboarding telemetry: attaching recorder to WebView client"); + mAADWebViewClient.setOnboardingTelemetryRecorder(onboardingRecorder); + } else { + Logger.verbose(methodTag, mCorrelationId, + "Onboarding telemetry: no recorder registered for this request"); + } + mAADWebViewClient.initializeAuthUxJavaScriptApi(mWebView, mAuthorizationRequestUrl); launchWebView(mAuthorizationRequestUrl, mRequestHeaders); return view; diff --git a/common/src/main/java/com/microsoft/identity/common/internal/telemetry/OnboardingRecorderRegistry.kt b/common/src/main/java/com/microsoft/identity/common/internal/telemetry/OnboardingRecorderRegistry.kt new file mode 100644 index 0000000000..6f66b88470 --- /dev/null +++ b/common/src/main/java/com/microsoft/identity/common/internal/telemetry/OnboardingRecorderRegistry.kt @@ -0,0 +1,198 @@ +// Copyright (c) Microsoft Corporation. +// All rights reserved. +// +// This code is licensed under the MIT License. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files(the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions : +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +package com.microsoft.identity.common.internal.telemetry + +import androidx.annotation.VisibleForTesting +import com.microsoft.identity.common.java.logging.DiagnosticContext +import com.microsoft.identity.common.logging.Logger + +/** + * In-process handoff for the onboarding telemetry recorder between the component that OWNS it and + * the interactive WebView host that CONSUMES it. + * + * On the broker side the recorder is built per-request from the onboarding seed by + * `AccountChooser` (main broker logic), while the WebView that renders the interactive auth / + * remediation pages lives in a separate `AuthorizationActivity` created by the OS from an Intent. + * A live recorder instance cannot ride an Intent, but both components run in the broker `:auth` + * process, so a process-static registry keyed by the request **correlationId** bridges them: + * + * - `AccountChooser` [register]s its recorder when it seeds one. + * - [com.microsoft.identity.common.internal.providers.oauth2.WebViewAuthorizationFragment] looks it + * up by correlationId and hands it to + * [com.microsoft.identity.common.internal.ui.webview.AzureActiveDirectoryWebViewClient.setOnboardingTelemetryRecorder] + * so WebView-observed onboarding steps (MDM enrollment, Company Portal launch, broker install) + * and the Auth UX `log_telemetry` error code are recorded into the same blob that is finalized + * and returned in the broker result. + * + * Entries MUST be [unregister]ed on terminal outcome / teardown to avoid leaking recorders across + * requests. Stores the concrete [OnboardingTelemetryRecorder] because the WebView client's + * telemetry hooks (e.g. `setLastLoadedDomain`) use methods that are not on the common4j interface. + * + * **The correlation id must be a real request id.** [DiagnosticContext.UNSET_CORRELATION_ID] is the + * value every thread carries before its request context is set, and the authorization Intent extra + * is populated by reading the request context map directly rather than through + * `getThreadCorrelationId()` — so the raw sentinel can reach this class. It is shared by definition, + * so accepting it as a key would let two unrelated requests resolve to the same recorder and merge + * one flow's blocking errors into the other's uploaded blob. All three accessors reject it: the + * feature goes inert (and [register] warns) instead of silently mis-attributing telemetry, which for + * a component whose entire purpose is correct attribution is the only acceptable failure. + * + * Because this is process-static state in the long-lived broker `:auth` process, a missed + * [unregister] would leak a recorder permanently — the recorder object and its collected steps / + * blocking errors, not an Activity, since [OnboardingTelemetryRecorder] deliberately holds only the + * application context. The map is therefore capped at [MAX_ENTRIES]: once full, registering evicts + * the least-recently-used entry and logs a warning. That converts an unbounded leak into a bounded + * one and makes the underlying bug visible, rather than hiding it. The cap is far above real + * concurrency (the broker drives one interactive request at a time), so eviction should not happen + * at all; if the warning appears, it is the signal that a terminal path is failing to unregister. + * + * Eviction is least-recently-*used* rather than oldest-registered. In today's flow that is not + * load-bearing — the WebView host calls [get] exactly once per request and holds the reference + * thereafter, so evicting a live entry would not disturb the in-flight request anyway — but it is + * free, and it is the safer default if a future caller ever re-resolves mid-request. + */ +object OnboardingRecorderRegistry { + + private val TAG = OnboardingRecorderRegistry::class.java.simpleName + + /** + * Upper bound on concurrently-registered recorders. Generous relative to real concurrency; it + * exists to bound a leak from a missed [unregister], not to constrain legitimate use. + */ + private const val MAX_ENTRIES = 16 + + // Access-ordered LinkedHashMap so eviction drops the least-recently-touched entry rather than + // simply the oldest-registered one. Guarded by its own monitor: LinkedHashMap is not + // thread-safe, and get() mutates access order, so reads need the lock too. + private val recorders = object : LinkedHashMap( + /* initialCapacity = */ MAX_ENTRIES, + /* loadFactor = */ 0.75f, + /* accessOrder = */ true + ) { + override fun removeEldestEntry( + eldest: MutableMap.MutableEntry? + ): Boolean { + if (size <= MAX_ENTRIES) { + return false + } + // Recorded rather than logged here: this runs inside put(), which runs under the + // registry lock, and every other log on this class is emitted outside it. + evictedKey = eldest?.key + return true + } + } + + // Written under the registry lock by removeEldestEntry, read and cleared by register() + // immediately after the lock is released, so the warning is logged off-lock. + private var evictedKey: String? = null + + /** + * Returns [correlationId] when it can safely key an entry — present, and not + * [DiagnosticContext.UNSET_CORRELATION_ID] — or null when it cannot. + * + * The sentinel is the value carried by any thread whose request context was never set, so it is + * shared rather than unique. Keying on it would let unrelated requests resolve to the same + * recorder — see this class's KDoc. + * + * Returns the key rather than a Boolean so callers get a non-null String to index the map with. + */ + private fun usableKeyOrNull(correlationId: String?): String? = + if (!correlationId.isNullOrEmpty() && + correlationId != DiagnosticContext.UNSET_CORRELATION_ID + ) { + correlationId + } else { + null + } + + /** + * Register [recorder] for [correlationId]. No-op when the recorder is null or the correlation id + * is not usable as a key (null, empty, or the unset sentinel), which is logged because a seeded + * recorder that cannot be handed off is a real defect. + */ + @JvmStatic + fun register(correlationId: String?, recorder: OnboardingTelemetryRecorder?) { + if (recorder == null) { + return + } + val key = usableKeyOrNull(correlationId) + if (key == null) { + Logger.warn( + TAG, correlationId, + "Not registering the onboarding recorder: the correlation id is missing or is the " + + "unset sentinel, so it cannot identify this request. Onboarding telemetry will " + + "be inert for it." + ) + return + } + val evicted = synchronized(recorders) { + recorders[key] = recorder + evictedKey.also { evictedKey = null } + } + Logger.info(TAG, key, "Registered onboarding recorder") + if (evicted != null) { + Logger.warn( + TAG, evicted, + "Onboarding recorder registry was full ($MAX_ENTRIES); evicted the " + + "least-recently-used entry. A terminal path is likely failing to unregister." + ) + } + } + + /** + * Return the recorder registered for [correlationId], or null when none is registered (e.g. the + * request carried no onboarding seed) or the correlation id is not usable as a key. + * + * Deliberately silent: the no-seed path calls this on every interactive request. + */ + @JvmStatic + fun get(correlationId: String?): OnboardingTelemetryRecorder? { + val key = usableKeyOrNull(correlationId) ?: return null + return synchronized(recorders) { recorders[key] } + } + + /** + * Remove the recorder registered for [correlationId]. Safe to call when none is registered or + * the correlation id is not usable as a key. + */ + @JvmStatic + fun unregister(correlationId: String?) { + val key = usableKeyOrNull(correlationId) ?: return + val removed = synchronized(recorders) { recorders.remove(key) } + if (removed != null) { + Logger.info(TAG, key, "Unregistered onboarding recorder") + } + } + + /** Number of currently-registered recorders. Test-only. */ + @JvmStatic + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + fun size(): Int = synchronized(recorders) { recorders.size } + + /** Drop all entries so one test cannot observe another's registrations. Test-only. */ + @JvmStatic + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + fun clearForTest() { + synchronized(recorders) { recorders.clear() } + } +} diff --git a/common/src/test/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationFragmentCorrelationIdTest.java b/common/src/test/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationFragmentCorrelationIdTest.java new file mode 100644 index 0000000000..03f0cb9b9e --- /dev/null +++ b/common/src/test/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationFragmentCorrelationIdTest.java @@ -0,0 +1,176 @@ +// Copyright (c) Microsoft Corporation. +// All rights reserved. +// +// This code is licensed under the MIT License. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files(the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions : +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +package com.microsoft.identity.common.internal.providers.oauth2; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; + +import android.content.Context; +import android.os.Bundle; + +import androidx.test.core.app.ApplicationProvider; + +import com.microsoft.identity.common.internal.telemetry.OnboardingRecorderRegistry; +import com.microsoft.identity.common.internal.telemetry.OnboardingTelemetryRecorder; +import com.microsoft.identity.common.java.logging.DiagnosticContext; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; + +/** + * AB#3708195: covers the correlation-id half of the onboarding recorder handoff — the wiring + * between {@link AuthorizationFragment#extractState(Bundle)}, the recorder registry, and the state + * bundle. + * + *

The registry's own suite proves storage in isolation; these tests prove the fragment side + * actually reaches it, including across activity recreation. Without them a regression in the + * correlation-id plumbing would leave every WebView onboarding hook inert while the registry tests + * still passed — the exact "green tests, dead feature" state this PR exists to fix. + * + *

Follows the {@code AuthorizationFragmentUrlTrackingTest} pattern: a minimal concrete subclass + * exercising the base-class behaviour directly, rather than driving a full Fragment lifecycle. + */ +@RunWith(RobolectricTestRunner.class) +public class AuthorizationFragmentCorrelationIdTest { + + private static final String CORRELATION_ID = "e4f1a0c2-0000-4a1b-9f3e-000000000001"; + + /** Minimal concrete subclass; the behaviour under test all lives in the base class. */ + private static class TestAuthorizationFragment extends AuthorizationFragment { + String correlationId() { + return mCorrelationId; + } + } + + private TestAuthorizationFragment mFragment; + + @Before + public void setUp() { + mFragment = new TestAuthorizationFragment(); + OnboardingRecorderRegistry.clearForTest(); + } + + @After + public void tearDown() { + OnboardingRecorderRegistry.clearForTest(); + } + + @Test + public void testExtractState_CapturesCorrelationIdFromBundle() { + final Bundle state = new Bundle(); + state.putString(DiagnosticContext.CORRELATION_ID, CORRELATION_ID); + + mFragment.extractState(state); + + assertEquals(CORRELATION_ID, mFragment.correlationId()); + } + + @Test + public void testExtractedCorrelationId_ResolvesTheRegisteredRecorder() { + // The whole point of capturing the id: it has to be the key the owner registered under. + final OnboardingTelemetryRecorder recorder = newRecorder(); + OnboardingRecorderRegistry.register(CORRELATION_ID, recorder); + + final Bundle state = new Bundle(); + state.putString(DiagnosticContext.CORRELATION_ID, CORRELATION_ID); + mFragment.extractState(state); + + assertSame(recorder, OnboardingRecorderRegistry.get(mFragment.correlationId())); + } + + @Test + public void testCorrelationId_SurvivesSaveAndRestore() { + // The regression this guards: onSaveInstanceState originally did not round-trip + // CORRELATION_ID, so a recreated fragment (a config change AuthorizationActivity does not + // declare, e.g. uiMode) read null back, blanked its diagnostic context, and could no longer + // resolve its recorder. The Intent survives recreation but this fragment reads the bundle. + final OnboardingTelemetryRecorder recorder = newRecorder(); + OnboardingRecorderRegistry.register(CORRELATION_ID, recorder); + + final Bundle initialState = new Bundle(); + initialState.putString(DiagnosticContext.CORRELATION_ID, CORRELATION_ID); + mFragment.extractState(initialState); + + // Activity recreation: the framework hands back only what onSaveInstanceState wrote. + final Bundle savedState = new Bundle(); + mFragment.onSaveInstanceState(savedState); + + final TestAuthorizationFragment recreated = new TestAuthorizationFragment(); + recreated.extractState(savedState); + + assertEquals("the correlation id must round-trip through the saved bundle", + CORRELATION_ID, recreated.correlationId()); + assertNotNull("the recorder must still be resolvable after recreation", + OnboardingRecorderRegistry.get(recreated.correlationId())); + assertSame(recorder, OnboardingRecorderRegistry.get(recreated.correlationId())); + } + + @Test + public void testNoRecorderRegistered_ResolvesToNullNotThrow() { + // The MSAL-client path: no onboarding seed, so nothing is ever registered. The host resolves + // unconditionally and must simply stay inert. + final Bundle state = new Bundle(); + state.putString(DiagnosticContext.CORRELATION_ID, CORRELATION_ID); + mFragment.extractState(state); + + assertNull(OnboardingRecorderRegistry.get(mFragment.correlationId())); + } + + @Test + public void testMissingCorrelationId_ResolvesToNullNotThrow() { + // A bundle without the key must not blow up the fragment or the registry lookup. + mFragment.extractState(new Bundle()); + + assertNull(mFragment.correlationId()); + assertNull(OnboardingRecorderRegistry.get(mFragment.correlationId())); + } + + @Test + public void testUnsetSentinelCorrelationId_DoesNotResolveAnotherRequestsRecorder() { + // A request whose thread never had a request context carries the shared UNSET sentinel into + // the Intent extra and therefore into this bundle. It must not act as a key. + OnboardingRecorderRegistry.register(CORRELATION_ID, newRecorder()); + + final Bundle state = new Bundle(); + state.putString(DiagnosticContext.CORRELATION_ID, DiagnosticContext.UNSET_CORRELATION_ID); + mFragment.extractState(state); + + assertNull("the sentinel must never resolve a recorder", + OnboardingRecorderRegistry.get(mFragment.correlationId())); + } + + private OnboardingTelemetryRecorder newRecorder() { + return new OnboardingTelemetryRecorder( + "{\"schema_version\":\"1.0.0\"," + + "\"session_correlation_id\":\"test-uuid-123\"," + + "\"onboarding_mode\":\"brokered\"}", + "test-client-id", + "scope1", + ApplicationProvider.getApplicationContext()); + } +} diff --git a/common/src/test/java/com/microsoft/identity/common/internal/telemetry/OnboardingRecorderRegistryTest.kt b/common/src/test/java/com/microsoft/identity/common/internal/telemetry/OnboardingRecorderRegistryTest.kt new file mode 100644 index 0000000000..d68f8c3d2d --- /dev/null +++ b/common/src/test/java/com/microsoft/identity/common/internal/telemetry/OnboardingRecorderRegistryTest.kt @@ -0,0 +1,251 @@ +// Copyright (c) Microsoft Corporation. +// All rights reserved. +// +// This code is licensed under the MIT License. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files(the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions : +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +package com.microsoft.identity.common.internal.telemetry + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import com.microsoft.identity.common.java.logging.DiagnosticContext +import org.junit.After +import org.junit.Assert +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * AB#3708195: contract tests for the process-static recorder handoff between the broker's + * `AccountChooser` (owner) and `WebViewAuthorizationFragment` (consumer). + * + * This is a singleton holding recorders for the life of the broker `:auth` process, so the cases + * that matter are correlation-id keying (one request must never see another's recorder — including + * via the shared `UNSET` sentinel), removal (a missed unregister leaks the recorder and its + * collected steps / blocking errors until eviction), and absent-key behaviour (the no-seed path must + * get null rather than throw). + */ +@RunWith(RobolectricTestRunner::class) +class OnboardingRecorderRegistryTest { + + @Before + fun setup() = OnboardingRecorderRegistry.clearForTest() + + @After + fun tearDown() = OnboardingRecorderRegistry.clearForTest() + + // --- Keying --- + + @Test + fun testRegisterThenGet_ReturnsSameInstance() { + val recorder = newRecorder() + + OnboardingRecorderRegistry.register("correlation-a", recorder) + + Assert.assertSame(recorder, OnboardingRecorderRegistry.get("correlation-a")) + } + + @Test + fun testGet_WithDifferentCorrelationId_DoesNotCrossWire() { + // The failure this guards is two concurrent requests sharing a recorder, which would merge + // one flow's blocking errors and steps into the other's uploaded blob. + val recorderA = newRecorder() + val recorderB = newRecorder() + OnboardingRecorderRegistry.register("correlation-a", recorderA) + OnboardingRecorderRegistry.register("correlation-b", recorderB) + + Assert.assertSame(recorderA, OnboardingRecorderRegistry.get("correlation-a")) + Assert.assertSame(recorderB, OnboardingRecorderRegistry.get("correlation-b")) + Assert.assertNotSame( + OnboardingRecorderRegistry.get("correlation-a"), + OnboardingRecorderRegistry.get("correlation-b") + ) + } + + @Test + fun testRegister_SameCorrelationIdTwice_LastWins() { + val first = newRecorder() + val second = newRecorder() + + OnboardingRecorderRegistry.register("correlation-a", first) + OnboardingRecorderRegistry.register("correlation-a", second) + + Assert.assertSame(second, OnboardingRecorderRegistry.get("correlation-a")) + Assert.assertEquals("re-registering must replace, not accumulate", 1, OnboardingRecorderRegistry.size()) + } + + // --- Absent keys / null-and-empty inputs --- + + @Test + fun testGet_UnknownCorrelationId_ReturnsNullNotThrow() { + // The MSAL-client path: no onboarding seed, so nothing is ever registered. The WebView host + // calls get() unconditionally and must simply stay inert. + Assert.assertNull(OnboardingRecorderRegistry.get("never-registered")) + } + + @Test + fun testGet_NullOrEmptyCorrelationId_ReturnsNull() { + OnboardingRecorderRegistry.register("correlation-a", newRecorder()) + + Assert.assertNull(OnboardingRecorderRegistry.get(null)) + Assert.assertNull(OnboardingRecorderRegistry.get("")) + } + + @Test + fun testRegister_NullOrEmptyCorrelationId_IsNoOp() { + OnboardingRecorderRegistry.register(null, newRecorder()) + OnboardingRecorderRegistry.register("", newRecorder()) + + Assert.assertEquals(0, OnboardingRecorderRegistry.size()) + } + + @Test + fun testRegister_NullRecorder_IsNoOp() { + OnboardingRecorderRegistry.register("correlation-a", null) + + Assert.assertEquals(0, OnboardingRecorderRegistry.size()) + Assert.assertNull(OnboardingRecorderRegistry.get("correlation-a")) + } + + // --- Removal / lifecycle --- + + @Test + fun testUnregister_RemovesOnlyTheTargetedEntry() { + val recorderB = newRecorder() + OnboardingRecorderRegistry.register("correlation-a", newRecorder()) + OnboardingRecorderRegistry.register("correlation-b", recorderB) + + OnboardingRecorderRegistry.unregister("correlation-a") + + Assert.assertNull("the released recorder must not be retrievable", OnboardingRecorderRegistry.get("correlation-a")) + Assert.assertSame("an unrelated request must be untouched", recorderB, OnboardingRecorderRegistry.get("correlation-b")) + Assert.assertEquals(1, OnboardingRecorderRegistry.size()) + } + + @Test + fun testUnregister_UnknownOrNullCorrelationId_IsSafeNoOp() { + OnboardingRecorderRegistry.register("correlation-a", newRecorder()) + + OnboardingRecorderRegistry.unregister("never-registered") + OnboardingRecorderRegistry.unregister(null) + OnboardingRecorderRegistry.unregister("") + + Assert.assertEquals(1, OnboardingRecorderRegistry.size()) + } + + @Test + fun testUnregister_Twice_IsIdempotent() { + // The broker unregisters in a finally on the finalize path; a retried or doubled teardown + // must not throw. + OnboardingRecorderRegistry.register("correlation-a", newRecorder()) + + OnboardingRecorderRegistry.unregister("correlation-a") + OnboardingRecorderRegistry.unregister("correlation-a") + + Assert.assertEquals(0, OnboardingRecorderRegistry.size()) + } + + // --- Leak bound --- + + @Test + fun testRegistry_IsBoundedWhenCallersFailToUnregister() { + // This registry is process-static in the long-lived broker :auth process, so a terminal path + // that forgets to unregister would leak a recorder (and its Context) permanently. Simulate + // that: register far past the cap and never release. + repeat(100) { i -> OnboardingRecorderRegistry.register("correlation-$i", newRecorder()) } + + Assert.assertEquals( + "a missed unregister must be a bounded leak, not an unbounded one", + 16, OnboardingRecorderRegistry.size() + ) + Assert.assertNull("the oldest entries must have been evicted", OnboardingRecorderRegistry.get("correlation-0")) + Assert.assertNotNull("the newest entry must survive", OnboardingRecorderRegistry.get("correlation-99")) + } + + @Test + fun testEviction_KeepsTheEntryStillBeingUsed() { + // Eviction is least-recently-USED, not least-recently-registered. Today's host resolves its + // recorder once and holds the reference, so this is not load-bearing for the current flow — + // it is the safer default if a future caller ever re-resolves mid-request. + val liveRecorder = newRecorder() + OnboardingRecorderRegistry.register("live-request", liveRecorder) + + repeat(20) { i -> + OnboardingRecorderRegistry.register("leaked-$i", newRecorder()) + // Simulates a caller that re-resolves rather than caching the reference. + Assert.assertSame(liveRecorder, OnboardingRecorderRegistry.get("live-request")) + } + + Assert.assertSame( + "the entry in active use must survive eviction pressure", + liveRecorder, OnboardingRecorderRegistry.get("live-request") + ) + } + + // --- Correlation-id sentinel --- + + @Test + fun testRegister_UnsetSentinelCorrelationId_IsRejected() { + // DiagnosticContext seeds every thread's request context with UNSET, and the authorization + // Intent extra is populated from that map directly rather than through + // getThreadCorrelationId(), so the raw sentinel can reach this registry. It is shared by + // definition: accepting it would let two unrelated requests resolve to the same recorder and + // merge one flow's blocking errors into the other's uploaded blob. + OnboardingRecorderRegistry.register(DiagnosticContext.UNSET_CORRELATION_ID, newRecorder()) + + Assert.assertEquals("the sentinel must never become a key", 0, OnboardingRecorderRegistry.size()) + Assert.assertNull(OnboardingRecorderRegistry.get(DiagnosticContext.UNSET_CORRELATION_ID)) + } + + @Test + fun testUnsetSentinel_CannotCrossWireTwoRequests() { + // The concrete failure the guard prevents: two requests whose threads never had a request + // context both fall back to the sentinel. Without the guard the second registration silently + // displaces the first and BOTH WebView hosts resolve to request two's recorder. + OnboardingRecorderRegistry.register(DiagnosticContext.UNSET_CORRELATION_ID, newRecorder()) + OnboardingRecorderRegistry.register(DiagnosticContext.UNSET_CORRELATION_ID, newRecorder()) + + Assert.assertEquals(0, OnboardingRecorderRegistry.size()) + Assert.assertNull( + "inert is the correct failure; sharing a recorder would mis-attribute telemetry", + OnboardingRecorderRegistry.get(DiagnosticContext.UNSET_CORRELATION_ID) + ) + } + + @Test + fun testUnregister_UnsetSentinel_DoesNotDisturbRealEntries() { + val real = newRecorder() + OnboardingRecorderRegistry.register("correlation-a", real) + + OnboardingRecorderRegistry.unregister(DiagnosticContext.UNSET_CORRELATION_ID) + + Assert.assertSame(real, OnboardingRecorderRegistry.get("correlation-a")) + Assert.assertEquals(1, OnboardingRecorderRegistry.size()) + } + + private fun newRecorder(): OnboardingTelemetryRecorder = OnboardingTelemetryRecorder( + "{\"schema_version\":\"1.0.0\"," + + "\"session_correlation_id\":\"test-uuid-123\"," + + "\"onboarding_mode\":\"brokered\"}", + "test-client-id", + "scope1", + ApplicationProvider.getApplicationContext() + ) +} diff --git a/common4j/src/main/com/microsoft/identity/common/java/logging/DiagnosticContext.java b/common4j/src/main/com/microsoft/identity/common/java/logging/DiagnosticContext.java index dc5bea13d9..baf5a8439e 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/logging/DiagnosticContext.java +++ b/common4j/src/main/com/microsoft/identity/common/java/logging/DiagnosticContext.java @@ -31,7 +31,17 @@ public enum DiagnosticContext { public static final String CORRELATION_ID = "correlation_id"; public static final String THREAD_ID = "thread_id"; - private static final String UNSET = "UNSET"; + + /** + * Value {@link #CORRELATION_ID} carries on a thread whose request context was never set. + * + * Public because callers that use the correlation id as a key — rather than + * just logging it — must reject this value explicitly: every unset thread shares it, so it is + * not an identifier. {@link #getThreadCorrelationId()} substitutes a random UUID, but code that + * reads {@link #getRequestContext()} directly (e.g. when populating an Intent extra) sees the + * raw sentinel. + */ + public static final String UNSET_CORRELATION_ID = "UNSET"; // This is thread-safe. @SuppressFBWarnings("SE_BAD_FIELD_STORE") @@ -41,7 +51,7 @@ public enum DiagnosticContext { protected RequestContext initialValue() { final RequestContext defaultRequestContext = new RequestContext(); defaultRequestContext.put(THREAD_ID, String.valueOf(Thread.currentThread().getId())); - defaultRequestContext.put(CORRELATION_ID, UNSET); + defaultRequestContext.put(CORRELATION_ID, UNSET_CORRELATION_ID); return defaultRequestContext; } }; @@ -78,7 +88,7 @@ public IRequestContext getRequestContext() { public String getThreadCorrelationId() { IRequestContext context = getRequestContext(); String correlationId = context.get(DiagnosticContext.CORRELATION_ID); - if (correlationId == null || correlationId.equals("UNSET")) { + if (correlationId == null || correlationId.equals(UNSET_CORRELATION_ID)) { correlationId = UUID.randomUUID().toString(); } return correlationId; diff --git a/common4j/src/main/com/microsoft/identity/common/java/logging/Logger.java b/common4j/src/main/com/microsoft/identity/common/java/logging/Logger.java index 0dc3862b0f..298f46d4a1 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/logging/Logger.java +++ b/common4j/src/main/com/microsoft/identity/common/java/logging/Logger.java @@ -48,6 +48,14 @@ public class Logger { private static final ExecutorService sLogExecutor = Executors.newSingleThreadExecutor(); private static final String DATE_FORMAT = "yyyy-MM-dd HH:mm:ss"; + /** + * Display placeholder for a value missing from the request context when formatting a log line. + * + *

Deliberately independent of {@link DiagnosticContext#UNSET_CORRELATION_ID}, despite the + * identical text: this one also stands in for a missing thread id, and it is never used as a + * key or compared against. Callers that use the correlation id as a key must reject the + * {@code DiagnosticContext} sentinel instead — see its javadoc. + */ private static final String UNSET = "UNSET"; // Turn on the VERBOSE level logging by default. From 4768caa67caf480a10f240c792401f3c90be3112 Mon Sep 17 00:00:00 2001 From: Zhipan Wang Date: Mon, 10 Aug 2026 18:47:55 -0700 Subject: [PATCH 2/3] Do not persist a null correlation id in the saved bundle, AB#3708195 Review nit from Prvnkmr337: extractState can leave mCorrelationId null (an MSAL client with no diagnostic context), and onSaveInstanceState then stored that null under CORRELATION_ID, which setDiagnosticContextForNewThread later put() into a RequestContext with no null guard. Verified his analysis rather than assuming: RequestContext extends HashMap, so put(key, null) is legal, and the only reader -- DiagnosticContext .getThreadCorrelationId() -- guards null and substitutes a random UUID. So this is safe today and matches pre-existing behaviour. Taken anyway as defence-in-depth, because that safety is load-bearing on the map type: a future swap to ConcurrentHashMap/Hashtable would make put(key, null) throw. Guarding the write is the cheaper end -- an absent key is already handled identically to "never saved", so nothing downstream has to tolerate a null at all. Test: testNullCorrelationId_IsNotWrittenToTheSavedBundle asserts the key is absent and that the recreation path still reads back null without throwing. Revert-tested: removing the guard fails with "a null correlation id must not be written under the key". No changelog entry -- the existing #3204 round-trip entry already covers this method's behaviour. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../oauth2/AuthorizationFragment.java | 10 +++++++- ...uthorizationFragmentCorrelationIdTest.java | 24 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationFragment.java b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationFragment.java index fbe75a36d5..7f32a46c98 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationFragment.java +++ b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationFragment.java @@ -187,11 +187,19 @@ void extractState(@NonNull final Bundle state) { * recreation path. Without this the key is absent from the saved bundle and a recreated fragment * blanks its diagnostic context, so every subsequent log line for the request loses its join * key — and, for the WebView fragment, the onboarding recorder can no longer be resolved. + * + *

A null id is not written at all rather than stored as a null value. Storing it is harmless + * today — {@code RequestContext extends HashMap}, so the downstream {@code put} accepts null, + * and the only reader substitutes a random UUID — but that safety is load-bearing on the map + * type. Skipping the write keeps the absent case indistinguishable from "never saved", which is + * already handled, instead of relying on a null surviving every layer below. */ @Override public void onSaveInstanceState(@NonNull final Bundle outState) { super.onSaveInstanceState(outState); - outState.putString(DiagnosticContext.CORRELATION_ID, mCorrelationId); + if (mCorrelationId != null) { + outState.putString(DiagnosticContext.CORRELATION_ID, mCorrelationId); + } } /** diff --git a/common/src/test/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationFragmentCorrelationIdTest.java b/common/src/test/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationFragmentCorrelationIdTest.java index 03f0cb9b9e..4c13705e68 100644 --- a/common/src/test/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationFragmentCorrelationIdTest.java +++ b/common/src/test/java/com/microsoft/identity/common/internal/providers/oauth2/AuthorizationFragmentCorrelationIdTest.java @@ -23,6 +23,7 @@ package com.microsoft.identity.common.internal.providers.oauth2; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; @@ -130,6 +131,29 @@ public void testCorrelationId_SurvivesSaveAndRestore() { assertSame(recorder, OnboardingRecorderRegistry.get(recreated.correlationId())); } + @Test + public void testNullCorrelationId_IsNotWrittenToTheSavedBundle() { + // A fragment that never saw a correlation id (an MSAL client with no diagnostic context) + // must not persist a null under the key. Storing one is harmless today only because + // RequestContext extends HashMap and the sole reader substitutes a UUID for null — safety + // that would evaporate if the map type ever became a ConcurrentHashMap/Hashtable, where + // put(key, null) throws. Leaving the key absent keeps this identical to the already-handled + // "never saved" case rather than depending on null surviving every layer below. + final TestAuthorizationFragment fragment = new TestAuthorizationFragment(); + fragment.extractState(new Bundle()); // no CORRELATION_ID present -> mCorrelationId is null + + final Bundle savedState = new Bundle(); + fragment.onSaveInstanceState(savedState); + + assertFalse("a null correlation id must not be written under the key", + savedState.containsKey(DiagnosticContext.CORRELATION_ID)); + + // And the recreation path still behaves: absent key reads back as null, no throw. + final TestAuthorizationFragment recreated = new TestAuthorizationFragment(); + recreated.extractState(savedState); + assertNull(recreated.correlationId()); + } + @Test public void testNoRecorderRegistered_ResolvesToNullNotThrow() { // The MSAL-client path: no onboarding seed, so nothing is ever registered. The host resolves From d2b0301e29f51a9cb0f7b7234d703f2c07209b13 Mon Sep 17 00:00:00 2001 From: Zhipan Wang Date: Tue, 11 Aug 2026 10:50:04 -0700 Subject: [PATCH 3/3] Consolidate the changelog to one concise entry, AB#3708195 Three #3204 entries -> one, matching the repo's one-entry-per-PR convention (89 of 90 released entries are single-entry; median ~101 chars, these were 358 / 189 / 318). Kept the correlation-id round-trip fix in the line because it affects all three authorization fragments and every log line after a recreation, so it is worth finding from the changelog. The UNSET_CORRELATION_ID exposure exists to support this feature and is covered by the PR. Also de-duplicated the #3201 block: merging the consolidation up appended the new line instead of replacing the old three, which is the changelog merge hazard this stack has hit before. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- changelog.txt | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/changelog.txt b/changelog.txt index 5943774f3e..36c5ff52b1 100644 --- a/changelog.txt +++ b/changelog.txt @@ -6,13 +6,8 @@ vNext - [PATCH] Security: validate the target of WebView broker-install intent-scheme redirects against an allow-list (Google Play Store only), clearing any explicit component/selector and stripping URI-permission grant flags so resolution is driven solely by the validated package; gated by ENABLE_BROKER_INSTALL_INTENT_VALIDATION CommonFlight (default off) and recording the launched / blocked outcome via the is_broker_install_intent_blocked span attribute (#3170) - [PATCH] Rename WebAppError 'error' field to 'code' in WebApps API error response (#3192) - [MINOR] Add a non-mutating log_telemetry action to the Auth UX JavaScript bridge that forwards a validated, page-reported server error code to an onboarding telemetry sink seam, kept strictly separate from the number-matching device-store path (#3197) -- [MINOR] Record the Auth UX log_telemetry server error code in the onboarding telemetry blob's blocking-errors list. Page-supplied codes are restricted to numeric server codes so a page cannot post one of the broker's own symbolic blocking-error constants; non-onboarding AADSTS codes (50058/50097/50126) and eSTS's "0" no-error sentinel are excluded for iOS parity; repeats are de-duplicated at the WebView client and the number recorded is capped, both per authorization request. The recorder's list stays append-only and chronological for its other callers (#3201) -- [PATCH] Fix Auth UX JavaScript bridge availability when the initial authorization URL is not allow-listed, and stop injecting the postMessageToBroker shim after the interface has been removed - also affects the number-matching path (#3201) -- [MINOR] Wire the broker's onboarding telemetry recorder into the :auth WebView host via a bounded, correlation-id-keyed registry, activating the previously inert WebView onboarding hooks for brokered flows; the registry rejects the DiagnosticContext unset-correlation-id sentinel so two requests without a request context can never share a recorder (#3204) -- [MINOR] Expose DiagnosticContext.UNSET_CORRELATION_ID so callers that use the correlation id as a key, rather than only logging it, can reject the shared unset sentinel explicitly (#3204) -- [PATCH] Round-trip the authorization request's correlation id through AuthorizationFragment's saved instance state, so a recreated fragment restores its diagnostic context instead of blanking it (affects all three authorization fragments; previously every log line after a recreation lost its correlation id) (#3204) -- [PATCH] Stop a best-effort session-correlation persistence failure (e.g. SharedPreferences unavailable on credential-encrypted storage before first unlock) from propagating out of OnboardingTelemetryRecorder.addBlockingError after the error was already appended, which also affects the broker's blocking-error call sites (#3201) - [MINOR] Record the Auth UX log_telemetry server error code in the onboarding telemetry blob's blocking-errors list, and fix Auth UX bridge availability when the initial authorization URL is not allow-listed (#3201) +- [MINOR] Wire the broker's onboarding telemetry recorder into the :auth WebView host, and fix AuthorizationFragment losing the request's correlation id across recreation (affects all three authorization fragments) (#3204) Version 24.5.0 ----------