Address comments

AzureAD · Sep 5, 2023 · 68a5180 · 68a5180
1 parent d7364ba
commit 68a5180
Show file tree

Hide file tree

Showing 9 changed files with 31 additions and 22 deletions.
diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractClientApplicationBase.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractClientApplicationBase.java
@@ -333,7 +333,7 @@ public abstract static class Builder<T extends Builder<T>> {
         private String azureRegion;
         private Integer connectTimeoutForDefaultHttpClient;
         private Integer readTimeoutForDefaultHttpClient;
-        protected boolean instanceDiscovery = true;
+        protected boolean isInstanceDiscoveryEnabled = true;
 
         /**
          * Constructor to create instance of Builder of client application
@@ -677,7 +677,7 @@ public T azureRegion(String val) {
         yet still want MSAL to accept any authority that you will provide,
         you can use a ``False`` to unconditionally disable Instance Discovery. */
         public T instanceDiscovery(boolean val) {
-            instanceDiscovery = val;
+            isInstanceDiscoveryEnabled = val;
             return self();
         }
 
@@ -709,7 +709,7 @@ public T instanceDiscovery(boolean val) {
         clientCapabilities = builder.clientCapabilities;
         autoDetectRegion = builder.autoDetectRegion;
         azureRegion = builder.azureRegion;
-        instanceDiscovery = builder.instanceDiscovery;
+        instanceDiscovery = builder.isInstanceDiscoveryEnabled;
 
         if (aadAadInstanceDiscoveryResponse != null) {
             AadInstanceDiscoveryProvider.cacheInstanceDiscoveryMetadata(

diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractManagedIdentitySource.java
@@ -16,6 +16,7 @@
 import java.io.IOException;
 import java.net.HttpURLConnection;
 import java.net.MalformedURLException;
+import java.net.SocketException;
 import java.net.URISyntaxException;
 
 //base class for all sources that support managed identity
@@ -57,6 +58,12 @@ public ManagedIdentityResponse getManagedIdentityResponse(
             response = HttpHelper.executeHttpRequest(httpRequest, managedIdentityRequest.requestContext(), serviceBundle);
         } catch (URISyntaxException e) {
             throw new RuntimeException(e);
+        } catch (MsalClientException e) {
+            if (e.getCause() instanceof SocketException) {
+                throw new MsalManagedIdentityException(MsalError.MANAGED_IDENTITY_UNREACHABLE_NETWORK, e.getMessage(), managedIdentitySourceType);
+            }
+
+            throw e;
         }
 
         return handleResponse(parameters, response);
@@ -103,6 +110,7 @@ protected ManagedIdentityResponse getSuccessfulResponse(IHttpResponse response)
                 || managedIdentityResponse.getAccessToken().isEmpty() || managedIdentityResponse.getExpiresOn() == null
                 || managedIdentityResponse.getExpiresOn().isEmpty()) {
             LOG.error("[Managed Identity] Response is either null or insufficient for authentication.");
+            throw new MsalManagedIdentityException(MsalError.MANAGED_IDENTITY_REQUEST_FAILED, MsalErrorMessage.MANAGED_IDENTITY_UNEXPECTED_RESPONSE, managedIdentitySourceType);
         }
 
         return managedIdentityResponse;

diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByManagedIdentitySupplier.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AcquireTokenByManagedIdentitySupplier.java
@@ -9,7 +9,7 @@
 import java.util.HashSet;
 import java.util.Set;
 
-public class AcquireTokenByManagedIdentitySupplier extends AuthenticationResultSupplier {
+class AcquireTokenByManagedIdentitySupplier extends AuthenticationResultSupplier {
 
     private static final Logger LOG = LoggerFactory.getLogger(AcquireTokenByManagedIdentitySupplier.class);
 
@@ -62,8 +62,13 @@ AuthenticationResult execute() throws Exception {
 
                 return supplier.execute();
             } catch (MsalClientException ex) {
-                LOG.debug(String.format("Cache lookup failed: %s", ex.getMessage()));
-                return fetchNewAccessTokenAndSaveToCache(tokenRequestExecutor, clientApplication.authenticationAuthority.host);
+                if (ex.errorCode().equals(AuthenticationErrorCode.CACHE_MISS)) {
+                    LOG.debug(String.format("Cache lookup failed: %s", ex.getMessage()));
+                    return fetchNewAccessTokenAndSaveToCache(tokenRequestExecutor, clientApplication.authenticationAuthority.host);
+                } else {
+                    LOG.error("Error occurred while cache lookup. " + ex.getMessage());
+                    throw ex;
+                }
             }
         }
 

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
@@ -80,10 +80,6 @@ private HttpResponse executeHttpPost(HttpRequest httpRequest) throws Exception {
     private HttpURLConnection openConnection(final URL finalURL)
             throws IOException {
         URLConnection connection;
-        HttpURLConnection httpConnection = null;
-        HttpsURLConnection httpsConnection = null;
-
-        Boolean isHttp = false;
 
         if (proxy != null) {
             connection = finalURL.openConnection(proxy);
@@ -95,17 +91,16 @@ private HttpURLConnection openConnection(final URL finalURL)
         connection.setReadTimeout(readTimeout);
 
         if (connection instanceof HttpURLConnection) {
-            httpConnection = (HttpURLConnection) connection;
-            isHttp = true;
+            return (HttpURLConnection) connection;
         } else {
-            httpsConnection = (HttpsURLConnection) connection;
-        }
+            HttpsURLConnection httpsConnection = (HttpsURLConnection) connection;
 
-        if (!isHttp && sslSocketFactory != null) {
-            httpsConnection.setSSLSocketFactory(sslSocketFactory);
-        }
+            if (sslSocketFactory != null) {
+                httpsConnection.setSSLSocketFactory(sslSocketFactory);
+            }
 
-        return isHttp? httpConnection : httpsConnection;
+            return httpsConnection;
+        }
     }
 
     private void configureAdditionalHeaders(final HttpURLConnection conn, final HttpRequest httpRequest) {

diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/EnvironmentVariables.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/EnvironmentVariables.java
@@ -3,7 +3,7 @@
 
 package com.microsoft.aad.msal4j;
 
-public class EnvironmentVariables implements IEnvironmentVariables {
+class EnvironmentVariables implements IEnvironmentVariables {
 
     @Override
     public String getEnvironmentVariable(String envVariable) {

diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/HttpHelper.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/HttpHelper.java
@@ -103,7 +103,6 @@ private static IHttpResponse executeHttpRequestWithRetries(HttpRequest httpReque
             Thread.sleep(RETRY_DELAY_MS);
         }
 
-        log.info(httpResponse.body());
         return httpResponse;
     }
 

diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityApplication.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityApplication.java
@@ -69,7 +69,7 @@ private Builder(ManagedIdentityId managedIdentityId) {
                     "system_assigned_managed_identity" : managedIdentityId.getUserAssignedId());
 
             this.managedIdentityId = managedIdentityId;
-            this.instanceDiscovery = false;
+            this.isInstanceDiscoveryEnabled = false;
         }
 
         public Builder resource(String resource) {

diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/MsalError.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/MsalError.java
@@ -11,4 +11,6 @@ class MsalError {
     public static final String MANAGED_IDENTITY_REQUEST_FAILED = "managed_identity_request_failed";
 
     public static final String SCOPES_REQUIRED = "scopes_required_client_credentials";
+
+    public static final String MANAGED_IDENTITY_UNREACHABLE_NETWORK = "managed_identity_unreachable_network";
 }
diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/MsalManagedIdentityException.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/MsalManagedIdentityException.java
@@ -8,7 +8,7 @@
 
 public class MsalManagedIdentityException extends MsalServiceException{
 
-    ManagedIdentitySourceType managedIdentitySourceType;
+    public ManagedIdentitySourceType managedIdentitySourceType;
 
     public MsalManagedIdentityException(String errorCode, String errorMessage, ManagedIdentitySourceType sourceType ){
         super(errorMessage, errorCode);