diff --git a/msal4j-sdk/pom.xml b/msal4j-sdk/pom.xml index cc2b0aba..9a3c8e3e 100644 --- a/msal4j-sdk/pom.xml +++ b/msal4j-sdk/pom.xml @@ -131,7 +131,7 @@ com.google.guava guava - 32.0.0-jre + 32.1.1-jre test diff --git a/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/HttpClientIT.java b/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/HttpClientIT.java index edc82ef3..66f55986 100644 --- a/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/HttpClientIT.java +++ b/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/HttpClientIT.java @@ -8,10 +8,13 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.BeforeAll; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; import java.util.Collections; +import java.util.concurrent.ExecutionException; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; @TestInstance(TestInstance.Lifecycle.PER_CLASS) class HttpClientIT { @@ -34,6 +37,14 @@ void acquireToken_apacheHttpClient() throws Exception { assertAcquireTokenCommon(user, new ApacheHttpClientAdapter()); } + @Test + void acquireToken_readTimeout() throws Exception { + User user = labUserProvider.getDefaultUser(); + + //Set a 1ms read timeout, which will almost certainly occur before the service can respond + assertAcquireTokenCommon_WithTimeout(user, 1); + } + private void assertAcquireTokenCommon(User user, IHttpClient httpClient) throws Exception { PublicClientApplication pca = PublicClientApplication.builder( @@ -54,4 +65,22 @@ private void assertAcquireTokenCommon(User user, IHttpClient httpClient) assertNotNull(result.idToken()); assertEquals(user.getUpn(), result.account().username()); } + + private void assertAcquireTokenCommon_WithTimeout(User user, int readTimeout) + throws Exception { + PublicClientApplication pca = PublicClientApplication.builder( + user.getAppId()). + authority(TestConstants.ORGANIZATIONS_AUTHORITY). + readTimeoutForDefaultHttpClient(readTimeout). + build(); + + ExecutionException ex = assertThrows(ExecutionException.class, () -> pca.acquireToken(UserNamePasswordParameters. + builder(Collections.singleton(TestConstants.GRAPH_DEFAULT_SCOPE), + user.getUpn(), + user.getPassword().toCharArray()) + .build()) + .get()); + + assertEquals("com.microsoft.aad.msal4j.MsalClientException: java.net.SocketTimeoutException: Read timed out", ex.getMessage()); + } } diff --git a/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/TokenCacheIT.java b/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/TokenCacheIT.java index 9576eaef..d1192f2c 100644 --- a/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/TokenCacheIT.java +++ b/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/TokenCacheIT.java @@ -9,6 +9,7 @@ import org.junit.jupiter.api.TestInstance; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import java.util.Collections; import java.util.HashMap; @@ -170,6 +171,31 @@ void twoAccountsInCache_SameUserDifferentTenants_RemoveAccountTest() throws Exce "/cache_data/remove-account-test-cache.json"); } + @Test + void retrieveAccounts_ADFSOnPrem() throws Exception { + UserQueryParameters query = new UserQueryParameters(); + query.parameters.put(UserQueryParameters.FEDERATION_PROVIDER, FederationProvider.ADFS_2019); + query.parameters.put(UserQueryParameters.USER_TYPE, UserType.ON_PREM); + + User user = labUserProvider.getLabUser(query); + + PublicClientApplication pca = PublicClientApplication.builder( + TestConstants.ADFS_APP_ID). + authority(TestConstants.ADFS_AUTHORITY). + build(); + + pca.acquireToken(UserNamePasswordParameters. + builder(Collections.singleton(TestConstants.ADFS_SCOPE), + user.getUpn(), + user.getPassword().toCharArray()) + .build()) + .get(); + + assertNotNull(pca.getAccounts().join().iterator().next()); + assertEquals(pca.getAccounts().join().size(), 1); + } + + private static class TokenPersistence implements ITokenCacheAccessAspect { String data; diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByInteractiveFlowSupplier.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByInteractiveFlowSupplier.java index 8029c5d8..9e98e6ef 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByInteractiveFlowSupplier.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByInteractiveFlowSupplier.java @@ -209,8 +209,11 @@ private AuthorizationResult getAuthorizationResultFromHttpListener() { expirationTime = TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis()) + 1; } - while (result == null && !interactiveRequest.futureReference().get().isCancelled() && - TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis()) < expirationTime) { + while (result == null && !interactiveRequest.futureReference().get().isCancelled()) { + if (TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis()) > expirationTime) { + LOG.warn(String.format("Listener timed out after %S seconds, no authorization code was returned from the server during that time.", timeFromParameters)); + break; + } result = authorizationResultQueue.poll(100, TimeUnit.MILLISECONDS); } diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthenticationErrorCode.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthenticationErrorCode.java index 78f5260c..22e8299f 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthenticationErrorCode.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthenticationErrorCode.java @@ -115,9 +115,19 @@ public class AuthenticationErrorCode { * A JWT parsing failure, indicating the JWT provided to MSAL is of invalid format. */ public final static String INVALID_JWT = "invalid_jwt"; + /** * Indicates that a Broker implementation is missing from the device, such as when an app developer * does not include one of our broker packages as a dependency in their project, or otherwise cannot - * be accessed by MSAL Java*/ + * be accessed by MSAL Java + */ public final static String MISSING_BROKER = "missing_broker"; + + /** + * Indicates that a timeout occurred during an HTTP call. If this was thrown in relation to a connection timeout error, + * there is likely a network issue preventing the library from reaching a service, such as being blocked by a firewall. + * If this was thrown in relation to a read timeout error, there is likely an issue in the service itself causing a + * slow response, and this may be resolvable by increasing timeouts. For more details, see https://aka.ms/msal4j-http-client + */ + public final static String HTTP_TIMEOUT = "http_timeout"; } diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/DefaultHttpClient.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/DefaultHttpClient.java index a563b369..ee60eccc 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/DefaultHttpClient.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/DefaultHttpClient.java @@ -1,25 +1,30 @@ package com.microsoft.aad.msal4j; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLSocketFactory; import java.io.DataOutputStream; import java.io.IOException; import java.io.InputStream; +import java.net.ConnectException; import java.net.HttpURLConnection; import java.net.Proxy; +import java.net.SocketTimeoutException; import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.Map; class DefaultHttpClient implements IHttpClient { + private final static Logger LOG = LoggerFactory.getLogger(DefaultHttpClient.class); private final Proxy proxy; private final SSLSocketFactory sslSocketFactory; - public int DEFAULT_CONNECT_TIMEOUT = 10000; - public int DEFAULT_READ_TIMEOUT = 15000; - private int connectTimeout = DEFAULT_CONNECT_TIMEOUT; - private int readTimeout = DEFAULT_READ_TIMEOUT; + //By default, rely on the timeout behavior of the services requests are sent to + private int connectTimeout = 0; + private int readTimeout = 0; DefaultHttpClient(Proxy proxy, SSLSocketFactory sslSocketFactory, Integer connectTimeout, Integer readTimeout) { this.proxy = proxy; @@ -117,6 +122,14 @@ private HttpResponse readResponseFromConnection(final HttpsURLConnection conn) t httpResponse.addHeaders(conn.getHeaderFields()); httpResponse.body(inputStreamToString(is)); return httpResponse; + } catch (SocketTimeoutException readException) { + LOG.error("Timeout while waiting for response from service. If custom timeouts were set, increasing them may resolve this issue. See https://aka.ms/msal4j-http-client for more information and solutions."); + + throw readException; + } catch (ConnectException timeoutException) { + LOG.error("Exception while connecting to service, there may be network issues preventing MSAL Java from connecting. See https://aka.ms/msal4j-http-client for more information and solutions."); + + throw timeoutException; } finally { if (is != null) { is.close(); diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenCache.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenCache.java index d12dc186..b5032d7c 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenCache.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenCache.java @@ -346,7 +346,7 @@ Set getAccounts(String clientId) { ((Account) rootAccounts.get(accCached.homeAccountId())).tenantProfiles.put(accCached.realm(), profile); } - if (accCached.homeAccountId().contains(accCached.localAccountId())) { + if (accCached.localAccountId() != null && accCached.homeAccountId().contains(accCached.localAccountId())) { ((Account) rootAccounts.get(accCached.homeAccountId())).username(accCached.username()); } }