bugfix #1019 - wrong throttling on clientid.

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/HttpHelper.java

@@ -125,18 +125,19 @@ IHttpResponse executeHttpRequest(HttpRequest httpRequest) {
 
     private String getRequestThumbprint(RequestContext requestContext) {
         StringBuilder sb = new StringBuilder();
-        sb.append(requestContext.clientId() + POINT_DELIMITER);
-        sb.append(requestContext.authority() + POINT_DELIMITER);
-
-        IAcquireTokenParameters apiParameters = requestContext.apiParameters();
-
-        if (apiParameters instanceof SilentParameters) {
-            IAccount account = ((SilentParameters) apiParameters).account();
-            if (account != null) {
-                sb.append(account.homeAccountId() + POINT_DELIMITER);
+        sb.append(requestContext.clientId()).append(POINT_DELIMITER);
+        sb.append(requestContext.authority()).append(POINT_DELIMITER);
+
+        UserIdentifier userIdentifier = requestContext.userIdentifier();
+        if (userIdentifier != null) {
+            if (!StringHelper.isBlank(userIdentifier.upn())) {
+                sb.append(userIdentifier.upn()).append(POINT_DELIMITER);
+            } else if (!StringHelper.isBlank(userIdentifier.oid())) {
+                sb.append(userIdentifier.oid()).append(POINT_DELIMITER);
             }
         }
 
+        IAcquireTokenParameters apiParameters = requestContext.apiParameters();
         Set<String> sortedScopes = new TreeSet<>(apiParameters.scopes());
         sb.append(String.join(" ", sortedScopes));
 

msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/RequestThrottlingTest.java

@@ -7,6 +7,8 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestInstance;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doReturn;
@@ -27,6 +29,7 @@ class RequestThrottlingTest {
     public final Integer THROTTLE_IN_SEC = 1;
     public TokenEndpointResponseType responseType;
     IHttpClient httpClientMock = mock(IHttpClient.class);
+    private boolean skipInvocationCountCheck = false;
 
 
     @BeforeEach
@@ -36,6 +39,9 @@ void init() {
 
     @AfterEach
     void check() throws Exception {
+        if (skipInvocationCountCheck) {
+            return;
+        }
 
         //throttlingTest() makes three non-throttled calls, so for a test without a retry there should be
         // 3 invocations of httpClientMock.send(), and 6 invocations if the calls are set to retry
@@ -199,4 +205,95 @@ void STSResponseContains_StatusCode500_RetryAfterHeader() throws Exception {
         ThrottlingCache.DEFAULT_THROTTLING_TIME_SEC = 1000;
         throttlingTest(TokenEndpointResponseType.STATUS_CODE_500_RETRY_AFTER_HEADER);
     }
+
+    private UserNamePasswordParameters getUserNamePasswordApiParameters(String username, String scope) {
+        return UserNamePasswordParameters
+                .builder(Collections.singleton(scope), username, "password".toCharArray())
+                .build();
+    }
+
+    // Regression test for issue #1019: a failed request for one user must not throttle a different
+    // user under the same clientId/authority/scope.
+    @Test
+    void STSResponseContains_StatusCode500_DifferentUsersNotThrottledForEachOther() throws Exception {
+        skipInvocationCountCheck = true;
+        ThrottlingCache.clear();
+
+        // user A's request fails with a 500 -> gets cached as a throttled request
+        PublicClientApplication app =
+                getClientApplicationMockedWithOneTokenEndpointResponse(TokenEndpointResponseType.STATUS_CODE_500);
+        try {
+            app.acquireToken(getUserNamePasswordApiParameters("userA@contoso.com", "scope1")).join();
+        } catch (Exception ex) {
+            if (!(ex.getCause() instanceof MsalServiceException)) {
+                fail("Unexpected exception");
+            }
+        }
+
+        // repeating user A's request should be throttled
+        try {
+            app = getPublicClientApp();
+            app.acquireToken(getUserNamePasswordApiParameters("userA@contoso.com", "scope1")).join();
+            fail("Expected MsalThrottlingException");
+        } catch (Exception ex) {
+            if (!(ex.getCause() instanceof MsalThrottlingException)) {
+                fail("Unexpected exception");
+            }
+        }
+
+        // user B, same clientId/authority/scope, must NOT be throttled by user A's failure
+        app = getClientApplicationMockedWithOneTokenEndpointResponse(TokenEndpointResponseType.STATUS_CODE_500);
+        try {
+            app.acquireToken(getUserNamePasswordApiParameters("userB@contoso.com", "scope1")).join();
+        } catch (Exception ex) {
+            if (!(ex.getCause() instanceof MsalServiceException)) {
+                fail("User B should not be throttled by user A's failed request");
+            }
+        }
+    }
+
+    // Confirms that per-account throttling isolation for the silent flow (previously driven by the
+    // `instanceof SilentParameters` special case) is preserved now that the fingerprint is derived
+    // from RequestContext.userIdentifier() instead.
+    @Test
+    void SilentFlow_DifferentAccountsThrottledIndependently() throws Exception {
+        skipInvocationCountCheck = true;
+        ThrottlingCache.clear();
+
+        PublicClientApplication app = getPublicClientApp();
+
+        IHttpClient localHttpClientMock = mock(IHttpClient.class);
+        HttpResponse http500 = new HttpResponse();
+        http500.statusCode(HttpStatus.HTTP_INTERNAL_ERROR);
+        http500.body(TestConfiguration.TOKEN_ENDPOINT_INVALID_GRANT_ERROR_RESPONSE);
+        http500.addHeaders(Collections.singletonMap("Content-Type", Collections.singletonList("application/json")));
+        doReturn(http500).when(localHttpClientMock).send(any());
+
+        HttpHelper httpHelper = new HttpHelper(localHttpClientMock, new DefaultRetryPolicy());
+        ServiceBundle serviceBundle = new ServiceBundle(null, new TelemetryManager(null, false), httpHelper);
+
+        HttpRequest httpRequest = new HttpRequest(HttpMethod.POST,
+                "https://login.microsoftonline.com/common/oauth2/v2.0/token");
+
+        IAccount accountA = mock(IAccount.class);
+        doReturn("oidA.tidA").when(accountA).homeAccountId();
+        SilentParameters paramsA = SilentParameters.builder(Collections.singleton("scope1"), accountA).build();
+        RequestContext contextA = new RequestContext(app, PublicApi.ACQUIRE_TOKEN_SILENTLY, paramsA,
+                UserIdentifier.fromHomeAccountId(accountA.homeAccountId()));
+
+        // account A's request fails with a 500 -> gets cached as a throttled request
+        httpHelper.executeHttpRequest(httpRequest, contextA, serviceBundle);
+        // repeating account A's request should be throttled
+        assertThrows(MsalThrottlingException.class,
+                () -> httpHelper.executeHttpRequest(httpRequest, contextA, serviceBundle));
+
+        // account B, same clientId/authority/scope, must NOT be throttled by account A's failure
+        IAccount accountB = mock(IAccount.class);
+        doReturn("oidB.tidB").when(accountB).homeAccountId();
+        SilentParameters paramsB = SilentParameters.builder(Collections.singleton("scope1"), accountB).build();
+        RequestContext contextB = new RequestContext(app, PublicApi.ACQUIRE_TOKEN_SILENTLY, paramsB,
+                UserIdentifier.fromHomeAccountId(accountB.homeAccountId()));
+
+        assertDoesNotThrow(() -> httpHelper.executeHttpRequest(httpRequest, contextB, serviceBundle));
+    }
 }