Add suberror for network errors, Fixes AB#3064171 (#2537)

- Make UrlConnectionHttpClient throw a ClientException with IO_ERROR and a specific suberror (Based on where it fails). - Move OAuth2SubError from ServiceException to BaseException (and rename it to a generic "subError") ![image](https://github.com/user-attachments/assets/bd8365e5-87d1-4473-819f-9949f82e61a2) This will allow us to separate network errors from other IO_ERROR [AB#3064171](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3064171)
AzureAD · Dec 5, 2024 · 3d7fe9e · 3d7fe9e
1 parent 204747c
commit 3d7fe9e
Show file tree

Hide file tree

Showing 37 changed files with 294 additions and 211 deletions.
diff --git a/...in/com/microsoft/identity/labapi/utilities/authentication/LabApiAuthenticationClient.java b/...in/com/microsoft/identity/labapi/utilities/authentication/LabApiAuthenticationClient.java
@@ -47,8 +47,8 @@ public class LabApiAuthenticationClient implements IAccessTokenSupplier {
     private final static String AUTHORITY = "https://login.microsoftonline.com/" + TENANT_ID;
     private final static String KEYSTORE_TYPE = "Windows-MY";
     private final static String KEYSTORE_PROVIDER = "SunMSCAPI";
-    private final int DEFAULT_ACCESS_TOKEN_RETRIES = 2;
-    private final int ATTEMPT_RETRY_WAIT = 3;
+    private final static int DEFAULT_ACCESS_TOKEN_RETRIES = 2;
+    private final static int ATTEMPT_RETRY_WAIT = 3;
     private final String mLabCredential;
     private final String mLabCertPassword;
     private final String mScope;

diff --git a/changelog.txt b/changelog.txt
@@ -1,5 +1,6 @@
 vNext
 ----------
+- [MINOR] Add suberror for network errors (#2537)
 - [PATCH] Translate MFA token error to UIRequiredException instead of ServiceException (#2538)
 - [MINOR] Add Child Spans for Interactive Span (#2516)
 - [MINOR] For MSAL CPP flows, match exact claims when deleting AT with intersecting scopes (#2548)

diff --git a/common/src/main/java/com/microsoft/identity/common/internal/broker/BrokerResult.java b/common/src/main/java/com/microsoft/identity/common/internal/broker/BrokerResult.java
@@ -30,7 +30,6 @@
 import com.microsoft.identity.common.java.exception.ErrorStrings;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
@@ -647,7 +646,7 @@ public Builder correlationId(final String correlationId) {
             return this;
         }
 
-        public Builder oauthSubErrorCode(final String subErrorCode) {
+        public Builder subErrorCode(final String subErrorCode) {
             this.mSubErrorCode = subErrorCode;
             return this;
         }

diff --git a/.../src/main/java/com/microsoft/identity/common/internal/migration/AdalMigrationAdapter.java b/.../src/main/java/com/microsoft/identity/common/internal/migration/AdalMigrationAdapter.java
@@ -35,6 +35,7 @@
 import com.google.gson.JsonObject;
 import com.google.gson.JsonSyntaxException;
 import com.microsoft.identity.common.adal.internal.AuthenticationConstants;
+import com.microsoft.identity.common.java.exception.ClientException;
 import com.microsoft.identity.common.java.exception.ServiceException;
 import com.microsoft.identity.common.adal.internal.cache.ADALTokenCacheItem;
 import com.microsoft.identity.common.java.providers.microsoft.MicrosoftAccount;
@@ -239,7 +240,7 @@ public static boolean loadCloudDiscoveryMetadata() {
         if (!AzureActiveDirectory.isInitialized()) {
             try {
                 AzureActiveDirectory.performCloudDiscovery();
-            } catch (final IOException | URISyntaxException e) {
+            } catch (final ClientException e) {
                 Logger.error(
                         methodTag,
                         "Failed to load instance discovery metadata",

diff --git a/.../src/main/java/com/microsoft/identity/common/internal/result/AdalBrokerResultAdapter.java b/.../src/main/java/com/microsoft/identity/common/internal/result/AdalBrokerResultAdapter.java
@@ -295,7 +295,7 @@ private void setServiceExceptionPropertiesToBundle(@NonNull final Bundle resultB
         // so adding values to these constants as well
         resultBundle.putString(AuthenticationConstants.OAuth2.ERROR, serviceException.getErrorCode());
         resultBundle.putString(AuthenticationConstants.OAuth2.ERROR_DESCRIPTION, serviceException.getMessage());
-        resultBundle.putString(AuthenticationConstants.OAuth2.SUBERROR, serviceException.getOAuthSubErrorCode());
+        resultBundle.putString(AuthenticationConstants.OAuth2.SUBERROR, serviceException.getSubErrorCode());
 
         if (null != serviceException.getHttpResponseBody()) {
             resultBundle.putSerializable(

diff --git a/.../src/main/java/com/microsoft/identity/common/internal/result/MsalBrokerResultAdapter.java b/.../src/main/java/com/microsoft/identity/common/internal/result/MsalBrokerResultAdapter.java
@@ -76,7 +76,6 @@
 import com.microsoft.identity.common.java.opentelemetry.OTelUtility;
 import com.microsoft.identity.common.java.opentelemetry.SpanExtension;
 import com.microsoft.identity.common.java.opentelemetry.SpanName;
-import com.microsoft.identity.common.java.providers.microsoft.azureactivedirectory.ClientInfo;
 import com.microsoft.identity.common.java.providers.microsoft.microsoftsts.MicrosoftStsAuthorizationResult;
 import com.microsoft.identity.common.java.providers.oauth2.AuthorizationResult;
 import com.microsoft.identity.common.java.request.SdkType;
@@ -308,6 +307,7 @@ public Bundle bundleFromBaseException(@NonNull final BaseException exception,
         final BrokerResult.Builder builder = new BrokerResult.Builder()
                 .success(false)
                 .errorCode(exception.getErrorCode())
+                .subErrorCode(exception.getSubErrorCode())
                 .errorMessage(exception.getMessage())
                 .exceptionType(exception.getExceptionName())
                 .correlationId(exception.getCorrelationId())
@@ -318,7 +318,7 @@ public Bundle bundleFromBaseException(@NonNull final BaseException exception,
 
         if (exception instanceof ServiceException) {
             final ServiceException serviceException = (ServiceException) exception;
-            builder.oauthSubErrorCode(serviceException.getOAuthSubErrorCode())
+            builder.subErrorCode(serviceException.getSubErrorCode())
                     .httpStatusCode(serviceException.getHttpStatusCode())
                     .httpResponseBody(AuthenticationSchemeTypeAdapter.getGsonInstance().toJson(
                             serviceException.getHttpResponseBody()));
@@ -522,6 +522,7 @@ private BaseException getBaseExceptionFromExceptionType(@NonNull final String ex
             );
         }
 
+        baseException.setSubErrorCode(brokerResult.getSubErrorCode());
         baseException.setCliTelemErrorCode(brokerResult.getCliTelemErrorCode());
         baseException.setCliTelemSubErrorCode(brokerResult.getCliTelemSubErrorCode());
         baseException.setCorrelationId(brokerResult.getCorrelationId());
@@ -595,6 +596,7 @@ private BaseException getBaseExceptionFromErrorCodes(@NonNull final BrokerResult
             );
         }
 
+        baseException.setSubErrorCode(brokerResult.getSubErrorCode());
         baseException.setCliTelemErrorCode(brokerResult.getCliTelemErrorCode());
         baseException.setCliTelemSubErrorCode(brokerResult.getCliTelemSubErrorCode());
         baseException.setCorrelationId(brokerResult.getCorrelationId());
@@ -620,7 +622,7 @@ private IntuneAppProtectionPolicyRequiredException getIntuneProtectionRequiredEx
         exception.setAuthorityUrl(brokerResult.getAuthority());
         exception.setAccountUserId(brokerResult.getLocalAccountId());
         exception.setAccountUpn(brokerResult.getUserName());
-        exception.setOauthSubErrorCode(brokerResult.getSubErrorCode());
+        exception.setSubErrorCode(brokerResult.getSubErrorCode());
         try {
             exception.setHttpResponseBody(HashMapExtensions.jsonStringAsMap(
                     brokerResult.getHttpResponseBody())
@@ -649,8 +651,6 @@ private ServiceException getServiceException(@NonNull final BrokerResult brokerR
                 null
         );
 
-        serviceException.setOauthSubErrorCode(brokerResult.getSubErrorCode());
-
         try {
             serviceException.setHttpResponseBody(
                     brokerResult.getHttpResponseBody() != null ?
@@ -686,7 +686,7 @@ private UiRequiredException getUiRequiredException(@NonNull final BrokerResult b
         );
         if (OAuth2ErrorCode.INTERACTION_REQUIRED.equalsIgnoreCase(errorCode) ||
                 OAuth2ErrorCode.INVALID_GRANT.equalsIgnoreCase(errorCode)) {
-            exception.setOauthSubErrorCode(brokerResult.getSubErrorCode());
+            exception.setSubErrorCode(brokerResult.getSubErrorCode());
         }
         return exception;
     }

diff --git a/...om/microsoft/identity/common/internal/authorities/AzureActiveDirectoryAuthorityTests.java b/...om/microsoft/identity/common/internal/authorities/AzureActiveDirectoryAuthorityTests.java
@@ -44,7 +44,7 @@
 @RunWith(RobolectricTestRunner.class)
 public class AzureActiveDirectoryAuthorityTests {
     @Test
-    public void isSameCloudAsAuthority_Returns_True_For_Authority_With_ValidAliases_For_SameCloud() throws IOException, URISyntaxException {
+    public void isSameCloudAsAuthority_Returns_True_For_Authority_With_ValidAliases_For_SameCloud() throws ClientException {
         final String[] cloudAliasesWW = new String[]{"https://login.microsoftonline.com", "https://login.windows.net", "https://login.microsoft.com", "https://sts.windows.net"};
         final String[] cloudAliasesCN = new String[]{"https://login.chinacloudapi.cn", "https://login.partner.microsoftonline.cn"};
         final String[] cloudAliasesUSGov = new String[]{"https://login.microsoftonline.us", "https://login.usgovcloudapi.net"};
@@ -69,7 +69,7 @@ public void isSameCloudAsAuthority_Returns_True_For_Authority_With_ValidAliases_
     }
 
     @Test
-    public void isSameCloudAsAuthority_Returns_False_For_Authorities_From_Different_Clouds() throws IOException, URISyntaxException {
+    public void isSameCloudAsAuthority_Returns_False_For_Authorities_From_Different_Clouds() throws ClientException {
         final AzureActiveDirectoryAuthority authorityWW = new AzureActiveDirectoryAuthority(new AllAccounts("https://login.microsoftonline.com"));
         final AzureActiveDirectoryAuthority authorityCN = new AzureActiveDirectoryAuthority(new AllAccounts("https://login.partner.microsoftonline.cn"));
         final AzureActiveDirectoryAuthority authorityUSGov = new AzureActiveDirectoryAuthority(new AllAccounts("https://login.microsoftonline.us"));

diff --git a/common4j/src/main/com/microsoft/identity/common/java/authorities/Authority.java b/common4j/src/main/com/microsoft/identity/common/java/authorities/Authority.java
@@ -303,7 +303,7 @@ public int hashCode() {
     private static final Object sLock = new Object();
 
     private static void performCloudDiscovery()
-            throws IOException, URISyntaxException {
+            throws ClientException {
         final String methodName = ":performCloudDiscovery";
         Logger.verbose(
                 TAG + methodName,
@@ -389,18 +389,8 @@ public static KnownAuthorityResult getKnownAuthorityResult(Authority authority)
 
         try {
             performCloudDiscovery();
-        } catch (final IOException ex) {
-            clientException = new ClientException(
-                    ClientException.IO_ERROR,
-                    "Unable to perform cloud discovery",
-                    ex
-            );
-        } catch (final URISyntaxException ex) {
-            clientException = new ClientException(
-                    ClientException.MALFORMED_URL,
-                    "Unable to construct cloud discovery URL",
-                    ex
-            );
+        } catch (final ClientException ex) {
+            clientException = ex;
         }
 
         Logger.info(TAG + methodName, "Cloud discovery complete.");

diff --git a/...rc/main/com/microsoft/identity/common/java/authorities/AzureActiveDirectoryAuthority.java b/...rc/main/com/microsoft/identity/common/java/authorities/AzureActiveDirectoryAuthority.java
@@ -38,7 +38,6 @@
 import com.microsoft.identity.common.java.util.CommonURIBuilder;
 import com.microsoft.identity.common.java.util.StringUtil;
 
-import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URISyntaxException;
@@ -177,7 +176,7 @@ public OAuth2Strategy createOAuth2Strategy(@NonNull final OAuth2StrategyParamete
      */
     //@WorkerThread
     public synchronized boolean isSameCloudAsAuthority(@NonNull final AzureActiveDirectoryAuthority authorityToCheck)
-            throws IOException, URISyntaxException {
+            throws ClientException {
         if (!AzureActiveDirectory.isInitialized()) {
             // Cloud discovery is needed in order to make sure that we have a preferred_network_host_name to cloud aliases mappings
             AzureActiveDirectory.performCloudDiscovery();

diff --git a/.../com/microsoft/identity/common/java/commands/parameters/SilentTokenCommandParameters.java b/.../com/microsoft/identity/common/java/commands/parameters/SilentTokenCommandParameters.java
@@ -31,9 +31,6 @@
 import com.microsoft.identity.common.java.exception.ClientException;
 import com.microsoft.identity.common.java.logging.Logger;
 
-import java.io.IOException;
-import java.net.URISyntaxException;
-
 import lombok.EqualsAndHashCode;
 import lombok.Getter;
 import lombok.experimental.SuperBuilder;
@@ -90,12 +87,9 @@ private boolean authorityMatchesAccountEnvironment() {
             }
             final AzureActiveDirectoryCloud cloud = AzureActiveDirectory.getAzureActiveDirectoryCloudFromHostName(getAccount().getEnvironment());
             return cloud != null && cloud.getPreferredNetworkHostName().equals(getAuthority().getAuthorityURL().getAuthority());
-        } catch (final IOException e) {
-            cause = e;
-            errorCode = ClientException.IO_ERROR;
-        } catch (final URISyntaxException e) {
+        } catch (final ClientException e) {
             cause = e;
-            errorCode = ClientException.MALFORMED_URL;
+            errorCode = e.getErrorCode();
         }
 
         Logger.error(
@@ -109,7 +103,7 @@ private boolean authorityMatchesAccountEnvironment() {
     }
 
     private static void performCloudDiscovery()
-            throws IOException, URISyntaxException {
+            throws ClientException {
         final String methodName = ":performCloudDiscovery";
         Logger.verbose(
                 TAG + methodName,

diff --git a/common4j/src/main/com/microsoft/identity/common/java/controllers/ExceptionAdapter.java b/common4j/src/main/com/microsoft/identity/common/java/controllers/ExceptionAdapter.java
@@ -234,7 +234,7 @@ public static ServiceException getExceptionFromTokenErrorResponse(@NonNull final
                     null);
         }
 
-        outErr.setOauthSubErrorCode(errorResponse.getSubError());
+        outErr.setSubErrorCode(errorResponse.getSubError());
         setHttpResponseUsingTokenErrorResponse(outErr, errorResponse);
         return outErr;
     }
@@ -262,7 +262,7 @@ public static ServiceException convertToNativeAuthException(@NonNull final Servi
                 exception.getHttpStatusCode(),
                 exception
         );
-        outErr.setOauthSubErrorCode(exception.getOAuthSubErrorCode());
+        outErr.setSubErrorCode(exception.getSubErrorCode());
         outErr.setHttpResponseHeaders(exception.getHttpResponseHeaders());
         outErr.setHttpResponseBody(exception.getHttpResponseBody());
         return outErr;
@@ -291,7 +291,7 @@ public static ServiceException getExceptionFromTokenErrorResponse(@Nullable fina
                         (BrokerSilentTokenCommandParameters) commandParameters
                 );
             }
-            policyRequiredException.setOauthSubErrorCode(errorResponse.getSubError());
+            policyRequiredException.setSubErrorCode(errorResponse.getSubError());
             setHttpResponseUsingTokenErrorResponse(policyRequiredException, errorResponse);
 
             return policyRequiredException;

diff --git a/common4j/src/main/com/microsoft/identity/common/java/exception/BaseException.java b/common4j/src/main/com/microsoft/identity/common/java/exception/BaseException.java
@@ -69,6 +69,8 @@ public class BaseException extends Exception implements IErrorInformation, ITele
 
     private String mErrorCode;
 
+    private String mSubErrorCode;
+
     private String mCorrelationId;
 
     // The username of the account that owns the flow.
@@ -133,13 +135,24 @@ public BaseException(final String errorCode, final String errorMessage,
     }
 
     /**
-     * @return The error code for the exception, could be null. {@link BaseException} is the top level base exception, for the
-     * constants value of all the error code.
+     * @return The error code of the exception, may not be null.
      */
     public String getErrorCode() {
         return mErrorCode;
     }
 
+    /**
+     * @return Sub error code of the exception, could be null.
+     */
+    public String getSubErrorCode() { return mSubErrorCode; }
+
+    /**
+     * @param subErrorCode - The sub error code for the exception.
+     */
+    public void setSubErrorCode(@Nullable final String subErrorCode) {
+        mSubErrorCode = subErrorCode;
+    }
+
     /**
      * {@inheritDoc}
      * Return the detailed description explaining why the exception is returned back.

diff --git a/common4j/src/main/com/microsoft/identity/common/java/exception/ClientException.java b/common4j/src/main/com/microsoft/identity/common/java/exception/ClientException.java
@@ -73,7 +73,8 @@ public class ClientException extends BaseException {
     public static final String MULTIPLE_MATCHING_TOKENS_DETECTED = "multiple_matching_tokens_detected";
 
     /**
-     * No active network is available on the device.
+     * Failed to make a network request from the device.
+     * See {@link BaseException#getSubErrorCode()} for more details.
      */
     public static final String DEVICE_NETWORK_NOT_AVAILABLE = "device_network_not_available";
 

diff --git a/common4j/src/main/com/microsoft/identity/common/java/exception/ConnectionError.kt b/common4j/src/main/com/microsoft/identity/common/java/exception/ConnectionError.kt
@@ -0,0 +1,56 @@
+// 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.java.exception
+
+enum class ConnectionError(val value: String) {
+    FAILED_TO_OPEN_CONNECTION("ce_failed_to_open_connection"),
+    FAILED_TO_SET_REQUEST_METHOD("ce_failed_to_set_request_method"),
+    FAILED_TO_WRITE_TO_OUTPUT_STREAM("ce_failed_to_write_to_output_stream"),
+    FAILED_TO_READ_FROM_INPUT_STREAM("ce_failed_to_read_from_input_stream"),
+    FAILED_TO_GET_RESPONSE_CODE("ce_failed_to_get_response_code"),
+    CONNECTION_TIMEOUT("ce_connection_timeout");
+
+    /**
+     * Converts this [ConnectionError] into a [ClientException]
+     **/
+    fun getClientException(cause: Throwable): ClientException {
+        val e = ClientException(
+            ClientException.IO_ERROR,
+            "An IO error occurred in the network layer: " + cause.message,
+            cause
+        )
+        e.subErrorCode = value
+        return e
+    }
+
+    /**
+     * Returns true if the given [Throwable] is a connection error.
+     **/
+    fun compare(throwable: Throwable): Boolean {
+        if (throwable !is ClientException){
+            return false
+        }
+
+        return this.value == throwable.subErrorCode
+    }
+}
diff --git a/common4j/src/main/com/microsoft/identity/common/java/exception/IErrorInformation.java b/common4j/src/main/com/microsoft/identity/common/java/exception/IErrorInformation.java
@@ -39,4 +39,11 @@ public interface IErrorInformation {
      */
     @NonNull
     String getErrorCode();
+
+    /**
+     * Get the sub error code associated with this operation.  May be null.
+     * @return the associated error code.
+     */
+    @Nullable
+    String getSubErrorCode();
 }