Skip to content

Commit fa699fd

Browse files
Copilotsomalaya
andauthored
fix: move isEstsCloudHost after cross-cloud check; add hasPrtHeaderAttached guard
Agent-Logs-Url: https://github.com/AzureAD/microsoft-authentication-library-common-for-android/sessions/3df0bcf0-ff0c-410c-ad11-23d4c47b5a58 Co-authored-by: somalaya <69237821+somalaya@users.noreply.github.com>
1 parent de89662 commit fa699fd

2 files changed

Lines changed: 37 additions & 4 deletions

File tree

common/src/main/java/com/microsoft/identity/common/internal/ui/webview/AzureActiveDirectoryWebViewClient.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -363,13 +363,14 @@ else if (isRedirectUrl(formattedURL)) {
363363
processSSLProtectionCheck(view, url);
364364
} else if (isHeaderForwardingRequiredUri(url)) {
365365
processHeaderForwardingRequiredUri(view, url);
366-
} else if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_PRT_HEADER_FOR_ESTS_RETURN_REDIRECT)
367-
&& isEstsCloudHost(url)) {
368-
Logger.info(methodTag, "Navigation redirects to eSTS cloud host, re-attaching PRT header.");
369-
processEstsHostRedirect(view, url);
370366
} else if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_ATTACH_PRT_HEADER_WHEN_CROSS_CLOUD) && isCrossCloudRedirect(formattedURL)) {
371367
Logger.info(methodTag,"Navigation contains cross cloud redirect.");
372368
processCrossCloudRedirect(view, url);
369+
} else if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_PRT_HEADER_FOR_ESTS_RETURN_REDIRECT)
370+
&& isEstsCloudHost(url)
371+
&& hasPrtHeaderAttached()) {
372+
Logger.info(methodTag, "Navigation redirects to eSTS cloud host, re-attaching PRT header.");
373+
processEstsHostRedirect(view, url);
373374
} else if (mInWebCpFlow && isWebCpEnrollmentUrl(url)) {
374375
Logger.info(methodTag,"Navigation contains web cp enrollment url.");
375376
processWebCpEnrollmentUrl(view, url);
@@ -1165,6 +1166,15 @@ boolean isEstsCloudHost(@NonNull final String url) {
11651166
}
11661167
}
11671168

1169+
/**
1170+
* Returns true if the initial request headers already contained a PRT credential header,
1171+
* indicating that the original /authorize call was PRT-authenticated.
1172+
*/
1173+
private boolean hasPrtHeaderAttached() {
1174+
return mRequestHeaders != null
1175+
&& mRequestHeaders.containsKey(AuthenticationConstants.Broker.PRT_RESPONSE_HEADER);
1176+
}
1177+
11681178
/**
11691179
* Processes an eSTS host redirect by generating a fresh PRT credential JWT and attaching it.
11701180
*/

common/src/test/java/com/microsoft/identity/common/internal/ui/webview/AzureActiveDirectoryWebViewClientEstsHostRedirectTest.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import androidx.annotation.NonNull;
3737
import androidx.test.core.app.ApplicationProvider;
3838

39+
import com.microsoft.identity.common.adal.internal.AuthenticationConstants;
3940
import com.microsoft.identity.common.internal.mocks.MockCommonFlightsManager;
4041
import com.microsoft.identity.common.internal.ui.webview.challengehandlers.SwitchBrowserRequestHandler;
4142
import com.microsoft.identity.common.java.flighting.CommonFlight;
@@ -99,6 +100,8 @@ public void setPKeyAuthStatus(boolean status) {
99100
false);
100101
final HashMap<String, String> dummyHeaders = new HashMap<>();
101102
dummyHeaders.put("key", "value");
103+
// Include PRT header so tests that verify re-attachment pass the hasPrtHeaderAttached() guard.
104+
dummyHeaders.put(AuthenticationConstants.Broker.PRT_RESPONSE_HEADER, "dummy-prt-value");
102105
mWebViewClient.setRequestHeaders(dummyHeaders);
103106
mWebViewClient.setRequestUrl(TEST_ESTS_URL);
104107
AzureActiveDirectory.ensureCloudDiscovery();
@@ -175,6 +178,26 @@ public void handleUrl_whenFlightEnabled_andNonEstsHost_doesNotHandleAsEstsHostRe
175178
assertFalse(result);
176179
}
177180

181+
@Test
182+
public void handleUrl_whenFlightEnabled_andEstsHost_butNoPrtHeader_doesNotHandleAsEstsHostRedirect() {
183+
final IFlightsProvider mockFlightsProvider = Mockito.mock(IFlightsProvider.class);
184+
when(mockFlightsProvider.isFlightEnabled(CommonFlight.ENABLE_PRT_HEADER_FOR_ESTS_RETURN_REDIRECT))
185+
.thenReturn(true);
186+
final MockCommonFlightsManager mockCommonFlightsManager = new MockCommonFlightsManager();
187+
mockCommonFlightsManager.setMockCommonFlightsProvider(mockFlightsProvider);
188+
CommonFlightsManager.INSTANCE.initializeCommonFlightsManager(mockCommonFlightsManager);
189+
190+
// Remove PRT header from request headers to simulate a flow that was not PRT-authenticated.
191+
final HashMap<String, String> headersWithoutPrt = new HashMap<>();
192+
headersWithoutPrt.put("key", "value");
193+
mWebViewClient.setRequestHeaders(headersWithoutPrt);
194+
195+
final AzureActiveDirectoryWebViewClient spyClient = spy(mWebViewClient);
196+
// Without a PRT header in the initial request, the eSTS host redirect should fall through.
197+
final boolean result = spyClient.shouldOverrideUrlLoading(mMockWebView, TEST_ESTS_URL);
198+
assertFalse(result);
199+
}
200+
178201
// ------- setRequestUrl() / mLoginHint extraction tests -------
179202

180203
@Test

0 commit comments

Comments
 (0)