From 204747cd41ca2d4b819e7502f889f2835b3dd0f5 Mon Sep 17 00:00:00 2001 From: Sowmya Malayanur <69237821+somalaya@users.noreply.github.com> Date: Tue, 26 Nov 2024 20:51:18 -0800 Subject: [PATCH 1/7] Merge Somalaya/release/18.2.2 to dev (#2546) Co-authored-by: iamgusain <75644120+iamgusain@users.noreply.github.com> Co-authored-by: Danilo Raspa <105228698+nilo-ms@users.noreply.github.com> --- changelog.txt | 5 +++++ common4j/versioning/version.properties | 2 +- versioning/version.properties | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/changelog.txt b/changelog.txt index 65abd07a7b..119a66da9f 100644 --- a/changelog.txt +++ b/changelog.txt @@ -9,6 +9,11 @@ Version 18.2.2 (common4j 15.2.2) - [PATCH] Debug errors in Keystore layer (#2544) +Version 18.2.1 +---------- +(common4j 15.2.1) +- [PATCH] Translate MFA token error to UIRequiredException instead of ServiceException (#2538) + Version 18.2.0 ---------- (common4j 15.2.0) diff --git a/common4j/versioning/version.properties b/common4j/versioning/version.properties index 6c84e479d6..2e09209ce8 100644 --- a/common4j/versioning/version.properties +++ b/common4j/versioning/version.properties @@ -1,4 +1,4 @@ #Wed May 12 20:08:39 UTC 2021 -versionName=15.2.0 +versionName=15.2.2 versionCode=1 latestPatchVersion=227 diff --git a/versioning/version.properties b/versioning/version.properties index 9e4f9c010e..88ae25cca5 100644 --- a/versioning/version.properties +++ b/versioning/version.properties @@ -1,4 +1,4 @@ #Tue Apr 06 22:55:08 UTC 2021 -versionName=18.2.0 +versionName=18.2.2 versionCode=1 latestPatchVersion=234 From 3d7fe9e660aed4cb8cafdb5f3041ed0618bf95d8 Mon Sep 17 00:00:00 2001 From: Dome Pongmongkol <19558668+rpdome@users.noreply.github.com> Date: Thu, 5 Dec 2024 09:43:52 -0800 Subject: [PATCH 2/7] 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) --- .../LabApiAuthenticationClient.java | 4 +- changelog.txt | 1 + .../common/internal/broker/BrokerResult.java | 3 +- .../migration/AdalMigrationAdapter.java | 3 +- .../result/AdalBrokerResultAdapter.java | 2 +- .../result/MsalBrokerResultAdapter.java | 12 +-- .../AzureActiveDirectoryAuthorityTests.java | 4 +- .../common/java/authorities/Authority.java | 16 +--- .../AzureActiveDirectoryAuthority.java | 3 +- .../SilentTokenCommandParameters.java | 12 +-- .../java/controllers/ExceptionAdapter.java | 6 +- .../common/java/exception/BaseException.java | 17 +++- .../java/exception/ClientException.java | 3 +- .../common/java/exception/ConnectionError.kt | 56 +++++++++++++ .../java/exception/IErrorInformation.java | 7 ++ .../java/exception/ServiceException.java | 18 ----- .../java/exception/TerminalException.java | 6 ++ ...reTokenNoFixedScopesCommandParameters.java | 15 +--- .../common/java/net/AbstractHttpClient.java | 27 ++++--- .../identity/common/java/net/HttpClient.java | 45 +++++------ .../common/java/net/IRetryPolicy.java | 7 +- .../common/java/net/NoRetryPolicy.java | 4 +- .../java/net/StatusCodeAndExceptionRetry.java | 7 +- .../java/net/UrlConnectionHttpClient.java | 78 ++++++++++--------- .../AzureActiveDirectory.java | 23 ++++-- .../java/providers/oauth2/OAuth2Strategy.java | 2 +- .../OpenIdProviderConfigurationClient.java | 4 +- .../identity/common/java/util/UrlUtil.java | 19 +++++ .../net/HttpUrlConnectionFactoryTest.java | 2 + .../java/net/UrlConnectionHttpClientTest.java | 56 ++++++------- .../identity/http/HttpRequestExaminer.java | 3 +- .../identity/http/HttpRequestInterceptor.java | 5 +- .../identity/http/HttpRequestRewriter.java | 3 +- .../identity/http/HttpResponseRewriter.java | 3 +- .../identity/http/InterceptedHttpClient.java | 3 +- .../identity/http/MockHttpClient.java | 7 +- .../identity/shadow/ShadowHttpClient.java | 19 ++--- 37 files changed, 294 insertions(+), 211 deletions(-) create mode 100644 common4j/src/main/com/microsoft/identity/common/java/exception/ConnectionError.kt diff --git a/LabApiUtilities/src/main/com/microsoft/identity/labapi/utilities/authentication/LabApiAuthenticationClient.java b/LabApiUtilities/src/main/com/microsoft/identity/labapi/utilities/authentication/LabApiAuthenticationClient.java index 9b63297a90..9b76a87782 100644 --- a/LabApiUtilities/src/main/com/microsoft/identity/labapi/utilities/authentication/LabApiAuthenticationClient.java +++ b/LabApiUtilities/src/main/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 index 119a66da9f..b87a0e2696 100644 --- 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 index d253431787..9280a137d1 100644 --- 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/common/src/main/java/com/microsoft/identity/common/internal/migration/AdalMigrationAdapter.java b/common/src/main/java/com/microsoft/identity/common/internal/migration/AdalMigrationAdapter.java index 66507c75fb..fe08235409 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/migration/AdalMigrationAdapter.java +++ b/common/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/common/src/main/java/com/microsoft/identity/common/internal/result/AdalBrokerResultAdapter.java b/common/src/main/java/com/microsoft/identity/common/internal/result/AdalBrokerResultAdapter.java index 4f70a20e8f..1fee8a5353 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/result/AdalBrokerResultAdapter.java +++ b/common/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/common/src/main/java/com/microsoft/identity/common/internal/result/MsalBrokerResultAdapter.java b/common/src/main/java/com/microsoft/identity/common/internal/result/MsalBrokerResultAdapter.java index 9ae40e4ff3..cc9048aee1 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/result/MsalBrokerResultAdapter.java +++ b/common/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/common/src/test/java/com/microsoft/identity/common/internal/authorities/AzureActiveDirectoryAuthorityTests.java b/common/src/test/java/com/microsoft/identity/common/internal/authorities/AzureActiveDirectoryAuthorityTests.java index 2c0ce4a3eb..d2a036df62 100644 --- a/common/src/test/java/com/microsoft/identity/common/internal/authorities/AzureActiveDirectoryAuthorityTests.java +++ b/common/src/test/java/com/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 index 1c4c79149c..ae39929290 100644 --- 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/common4j/src/main/com/microsoft/identity/common/java/authorities/AzureActiveDirectoryAuthority.java b/common4j/src/main/com/microsoft/identity/common/java/authorities/AzureActiveDirectoryAuthority.java index aa73f9c297..4d3d9abf8a 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/authorities/AzureActiveDirectoryAuthority.java +++ b/common4j/src/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/common4j/src/main/com/microsoft/identity/common/java/commands/parameters/SilentTokenCommandParameters.java b/common4j/src/main/com/microsoft/identity/common/java/commands/parameters/SilentTokenCommandParameters.java index e4fa0770bc..8c46f33fc0 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/commands/parameters/SilentTokenCommandParameters.java +++ b/common4j/src/main/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 index 7a8be253a2..fa5e2b4755 100644 --- 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 index ec97a39d37..7eaefcc580 100644 --- 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 index 3039db0e58..746b737fb6 100644 --- 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 new file mode 100644 index 0000000000..31a9ad12ff --- /dev/null +++ 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 index e64c55b635..038f75ea38 100644 --- 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(); } diff --git a/common4j/src/main/com/microsoft/identity/common/java/exception/ServiceException.java b/common4j/src/main/com/microsoft/identity/common/java/exception/ServiceException.java index 186e29a55e..bea4a3ab5f 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/exception/ServiceException.java +++ b/common4j/src/main/com/microsoft/identity/common/java/exception/ServiceException.java @@ -30,8 +30,6 @@ import java.util.HashMap; import java.util.List; -import edu.umd.cs.findbugs.annotations.Nullable; - public class ServiceException extends BaseException { // This is needed for backward compatibility with older versions of MSAL (pre 3.0.0) @@ -98,8 +96,6 @@ public class ServiceException extends BaseException { */ public static final String MsaGrantedRefreshTokenNotSupportedOnAadTenantErrorCode = "1000030"; - private String mOauthSubErrorCode; - private int mHttpStatusCode; private HashMap mHttpResponseBody = null; @@ -119,13 +115,6 @@ public int getHttpStatusCode() { return mHttpStatusCode; } - /** - * @return The OAuth sub error code for the exception, could be null. - */ - public String getOAuthSubErrorCode() { - return mOauthSubErrorCode; - } - /** * Gets the response body that may be returned by the service. * @@ -144,13 +133,6 @@ public HashMap> getHttpResponseHeaders() { return mHttpResponseHeaders; } - /** - * @param subErrorCode - The sub error code for the exception. - */ - public void setOauthSubErrorCode(@Nullable final String subErrorCode) { - mOauthSubErrorCode = subErrorCode; - } - /** * Set response headers of the error received from the Service. * diff --git a/common4j/src/main/com/microsoft/identity/common/java/exception/TerminalException.java b/common4j/src/main/com/microsoft/identity/common/java/exception/TerminalException.java index ac478eeb66..f04a91997c 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/exception/TerminalException.java +++ b/common4j/src/main/com/microsoft/identity/common/java/exception/TerminalException.java @@ -87,4 +87,10 @@ public TerminalException(final @Nullable String message, super(message); this.mErrorCode = errorCode; } + + @Nullable + @Override + public String getSubErrorCode() { + return null; + } } diff --git a/common4j/src/main/com/microsoft/identity/common/java/nativeauth/commands/parameters/AcquireTokenNoFixedScopesCommandParameters.java b/common4j/src/main/com/microsoft/identity/common/java/nativeauth/commands/parameters/AcquireTokenNoFixedScopesCommandParameters.java index a16a92ad13..e3d807d124 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/nativeauth/commands/parameters/AcquireTokenNoFixedScopesCommandParameters.java +++ b/common4j/src/main/com/microsoft/identity/common/java/nativeauth/commands/parameters/AcquireTokenNoFixedScopesCommandParameters.java @@ -22,7 +22,6 @@ // THE SOFTWARE. package com.microsoft.identity.common.java.nativeauth.commands.parameters; -import com.google.gson.annotations.Expose; import com.microsoft.identity.common.java.authscheme.AbstractAuthenticationScheme; import com.microsoft.identity.common.java.dto.IAccountRecord; import com.microsoft.identity.common.java.exception.ArgumentException; @@ -32,11 +31,6 @@ import com.microsoft.identity.common.java.providers.microsoft.azureactivedirectory.AzureActiveDirectory; import com.microsoft.identity.common.java.providers.microsoft.azureactivedirectory.AzureActiveDirectoryCloud; -import java.io.IOException; -import java.net.URISyntaxException; -import java.util.List; -import java.util.Map; - import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.NonNull; @@ -126,12 +120,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( @@ -145,7 +136,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/net/AbstractHttpClient.java b/common4j/src/main/com/microsoft/identity/common/java/net/AbstractHttpClient.java index ca8dd22a5b..50441f82cd 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/net/AbstractHttpClient.java +++ b/common4j/src/main/com/microsoft/identity/common/java/net/AbstractHttpClient.java @@ -22,12 +22,11 @@ // THE SOFTWARE. package com.microsoft.identity.common.java.net; -import java.io.IOException; +import com.microsoft.identity.common.java.exception.ClientException; + import java.net.URL; import java.util.Map; -import javax.net.ssl.SSLContext; - import edu.umd.cs.findbugs.annotations.Nullable; import lombok.NonNull; @@ -41,65 +40,65 @@ public abstract class AbstractHttpClient implements HttpClient { public HttpResponse method(@NonNull final String httpMethod, @NonNull final URL requestUrl, @NonNull final Map requestHeaders, - @Nullable final byte[] requestContent) throws IOException { - return method(HttpClient.HttpMethod.validateAndNormalizeMethod(httpMethod), requestUrl, requestHeaders, requestContent); + @Nullable final byte[] requestContent) throws ClientException { + return method(HttpMethod.validateAndNormalizeMethod(httpMethod), requestUrl, requestHeaders, requestContent); } @Override public abstract HttpResponse method(@NonNull final HttpMethod httpMethod, @NonNull final URL requestUrl, @NonNull final Map requestHeaders, - @Nullable final byte[] requestContent) throws IOException; + @Nullable final byte[] requestContent) throws ClientException; @Override public HttpResponse put(@NonNull final URL requestUrl, @NonNull final Map requestHeaders, - @Nullable final byte[] requestContent) throws IOException { + @Nullable final byte[] requestContent) throws ClientException { return method(HttpMethod.PUT, requestUrl, requestHeaders, requestContent); } @Override public HttpResponse patch(@NonNull final URL requestUrl, @NonNull final Map requestHeaders, - @Nullable final byte[] requestContent) throws IOException { + @Nullable final byte[] requestContent) throws ClientException { return method(HttpMethod.PATCH, requestUrl, requestHeaders, requestContent); } @Override public HttpResponse options(@NonNull final URL requestUrl, - @NonNull final Map requestHeaders) throws IOException { + @NonNull final Map requestHeaders) throws ClientException { return method(HttpMethod.OPTIONS, requestUrl, requestHeaders, null); } @Override public HttpResponse post(@NonNull final URL requestUrl, @NonNull final Map requestHeaders, - @Nullable final byte[] requestContent) throws IOException { + @Nullable final byte[] requestContent) throws ClientException { return method(HttpMethod.POST, requestUrl, requestHeaders, requestContent); } @Override public HttpResponse delete(@NonNull final URL requestUrl, @NonNull final Map requestHeaders, - @Nullable final byte[] requestContent) throws IOException { + @Nullable final byte[] requestContent) throws ClientException { return method(HttpMethod.DELETE, requestUrl, requestHeaders, requestContent); } @Override public HttpResponse get(@NonNull final URL requestUrl, - @NonNull final Map requestHeaders) throws IOException { + @NonNull final Map requestHeaders) throws ClientException { return method(HttpMethod.GET, requestUrl, requestHeaders, null); } @Override public HttpResponse head(@NonNull final URL requestUrl, - @NonNull final Map requestHeaders) throws IOException { + @NonNull final Map requestHeaders) throws ClientException { return method(HttpMethod.HEAD, requestUrl, requestHeaders, null); } @Override public HttpResponse trace(@NonNull final URL requestUrl, - @NonNull final Map requestHeaders) throws IOException { + @NonNull final Map requestHeaders) throws ClientException { return method(HttpMethod.TRACE, requestUrl, requestHeaders, null); } } diff --git a/common4j/src/main/com/microsoft/identity/common/java/net/HttpClient.java b/common4j/src/main/com/microsoft/identity/common/java/net/HttpClient.java index 75b3ba909c..9eba5b4b64 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/net/HttpClient.java +++ b/common4j/src/main/com/microsoft/identity/common/java/net/HttpClient.java @@ -22,17 +22,14 @@ // THE SOFTWARE. package com.microsoft.identity.common.java.net; +import com.microsoft.identity.common.java.exception.ClientException; import com.microsoft.identity.common.java.util.StringUtil; import lombok.NonNull; -import java.io.IOException; import java.net.URL; import java.util.LinkedHashMap; import java.util.Map; -import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLSocket; - /** * An interface providing access to resources backed by web requests. This provides access to only * the verbs enumerated by HttpMethod. @@ -47,12 +44,12 @@ public interface HttpClient { * @param requestHeaders the headers for the request. * @param requestContent the body content of the request, if applicable. May be null. * @return an HttpResponse with the result of the call. - * @throws IOException if there was a communication problem. + * @throws ClientException if there was a communication problem. */ HttpResponse method(@NonNull String httpMethod, @NonNull URL requestUrl, @NonNull Map requestHeaders, - byte[] requestContent) throws IOException; + byte[] requestContent) throws ClientException; /** * Execute an arbitrary method. @@ -61,12 +58,12 @@ HttpResponse method(@NonNull String httpMethod, * @param requestHeaders the headers for the request. * @param requestContent the body content of the request, if applicable. May be null. * @return an HttpResponse with the result of the call. - * @throws IOException if there was a communication problem. + * @throws ClientException if there was a communication problem. */ HttpResponse method(@NonNull HttpMethod httpMethod, @NonNull URL requestUrl, @NonNull Map requestHeaders, - byte[] requestContent) throws IOException; + byte[] requestContent) throws ClientException; /** * Execute an HTTP PUT request. @@ -74,11 +71,11 @@ HttpResponse method(@NonNull HttpMethod httpMethod, * @param requestHeaders the headers for the request. * @param requestContent the body content of the request, if applicable. May be null. * @return an HttpResponse with the result of the call. - * @throws IOException if there was a communication problem. + * @throws ClientException if there was a communication problem. */ HttpResponse put(@NonNull URL requestUrl, @NonNull Map requestHeaders, - byte[] requestContent) throws IOException; + byte[] requestContent) throws ClientException; /** * Execute an HTTP PATCH request. @@ -86,21 +83,21 @@ HttpResponse put(@NonNull URL requestUrl, * @param requestHeaders the headers for the request. * @param requestContent the body content of the request, if applicable. May be null. * @return an HttpResponse with the result of the call. - * @throws IOException if there was a communication problem. + * @throws ClientException if there was a communication problem. */ HttpResponse patch(@NonNull URL requestUrl, @NonNull Map requestHeaders, - byte[] requestContent) throws IOException; + byte[] requestContent) throws ClientException; /** * Execute an HTTP OPTIONS request. * @param requestUrl the URL of the resource to operate on. * @param requestHeaders the headers for the request. * @return an HttpResponse with the result of the call. - * @throws IOException if there was a communication problem. + * @throws ClientException if there was a communication problem. */ HttpResponse options(@NonNull final URL requestUrl, - @NonNull final Map requestHeaders) throws IOException; + @NonNull final Map requestHeaders) throws ClientException; /** * Execute an HTTP POST request. @@ -108,11 +105,11 @@ HttpResponse options(@NonNull final URL requestUrl, * @param requestHeaders the headers for the request. * @param requestContent the body content of the request, if applicable. May be null. * @return an HttpResponse with the result of the call. - * @throws IOException if there was a communication problem. + * @throws ClientException if there was a communication problem. */ HttpResponse post(@NonNull URL requestUrl, @NonNull Map requestHeaders, - byte[] requestContent) throws IOException; + byte[] requestContent) throws ClientException; /** * Execute an HTTP PATCH request. @@ -120,41 +117,41 @@ HttpResponse post(@NonNull URL requestUrl, * @param requestHeaders the headers for the request. * @param requestContent the body content of the request, if applicable. May be null. * @return an HttpResponse with the result of the call. - * @throws IOException if there was a communication problem. + * @throws ClientException if there was a communication problem. */ HttpResponse delete(@NonNull final URL requestUrl, @NonNull final Map requestHeaders, - byte[] requestContent) throws IOException; + byte[] requestContent) throws ClientException; /** * Execute an HTTP GET request. * @param requestUrl the URL of the resource to operate on. * @param requestHeaders the headers for the request. * @return an HttpResponse with the result of the call. - * @throws IOException if there was a communication problem. + * @throws ClientException if there was a communication problem. */ HttpResponse get(@NonNull final URL requestUrl, - @NonNull final Map requestHeaders) throws IOException; + @NonNull final Map requestHeaders) throws ClientException; /** * Execute an HTTP HEAD request. * @param requestUrl the URL of the resource to operate on. * @param requestHeaders the headers for the request. * @return an HttpResponse with the result of the call. - * @throws IOException if there was a communication problem. + * @throws ClientException if there was a communication problem. */ HttpResponse head(@NonNull final URL requestUrl, - @NonNull final Map requestHeaders) throws IOException; + @NonNull final Map requestHeaders) throws ClientException; /** * Execute an HTTP TRACE request. * @param requestUrl the URL of the resource to operate on. * @param requestHeaders the headers for the request. * @return an HttpResponse with the result of the call. - * @throws IOException if there was a communication problem. + * @throws ClientException if there was a communication problem. */ HttpResponse trace(@NonNull final URL requestUrl, - @NonNull final Map requestHeaders) throws IOException; + @NonNull final Map requestHeaders) throws ClientException; /** * An enumeration of the HTTP verbs supported by this client interface. diff --git a/common4j/src/main/com/microsoft/identity/common/java/net/IRetryPolicy.java b/common4j/src/main/com/microsoft/identity/common/java/net/IRetryPolicy.java index f4985fff54..f179499170 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/net/IRetryPolicy.java +++ b/common4j/src/main/com/microsoft/identity/common/java/net/IRetryPolicy.java @@ -22,7 +22,8 @@ // THE SOFTWARE. package com.microsoft.identity.common.java.net; -import java.io.IOException; +import com.microsoft.identity.common.java.exception.ClientException; + import java.util.concurrent.Callable; /** @@ -37,7 +38,7 @@ public interface IRetryPolicy { * Evaluate the object returned from a callable and return the result. * @param supplier an object to call for a result. * @return the result of calling the supplier. - * @throws IOException if an IO error occurs. + * @throws ClientException if a Network exception error occurs. */ - T attempt (Callable supplier) throws IOException; + T attempt (Callable supplier) throws ClientException; } diff --git a/common4j/src/main/com/microsoft/identity/common/java/net/NoRetryPolicy.java b/common4j/src/main/com/microsoft/identity/common/java/net/NoRetryPolicy.java index 62a3839c74..b19516d476 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/net/NoRetryPolicy.java +++ b/common4j/src/main/com/microsoft/identity/common/java/net/NoRetryPolicy.java @@ -22,6 +22,8 @@ // THE SOFTWARE. package com.microsoft.identity.common.java.net; +import com.microsoft.identity.common.java.exception.ClientException; + import net.jcip.annotations.Immutable; import net.jcip.annotations.ThreadSafe; @@ -38,7 +40,7 @@ public class NoRetryPolicy implements IRetryPolicy { @Override @SneakyThrows - public HttpResponse attempt(Callable supplier) throws IOException { + public HttpResponse attempt(Callable supplier) throws ClientException { return supplier.call(); } } diff --git a/common4j/src/main/com/microsoft/identity/common/java/net/StatusCodeAndExceptionRetry.java b/common4j/src/main/com/microsoft/identity/common/java/net/StatusCodeAndExceptionRetry.java index 396008b8d3..b608d318f1 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/net/StatusCodeAndExceptionRetry.java +++ b/common4j/src/main/com/microsoft/identity/common/java/net/StatusCodeAndExceptionRetry.java @@ -22,6 +22,7 @@ // THE SOFTWARE. package com.microsoft.identity.common.java.net; +import com.microsoft.identity.common.java.exception.ClientException; import com.microsoft.identity.common.java.util.ported.Function; import net.jcip.annotations.Immutable; import net.jcip.annotations.ThreadSafe; @@ -72,7 +73,7 @@ public Boolean apply(HttpResponse input) { private final int extensionFactor = 2; @Override - public HttpResponse attempt(Callable supplier) throws IOException { + public HttpResponse attempt(Callable supplier) throws ClientException { int attemptNumber = number; int cumulativeDelay = initialDelay; do { @@ -84,8 +85,8 @@ public HttpResponse attempt(Callable supplier) throws IOException } } catch (final Exception e) { if (attemptNumber <= 0 || !isRetryableException.apply(e)) { - if (e instanceof IOException) { - throw (IOException) e; + if (e instanceof ClientException) { + throw (ClientException) e; } else { throw new RetryFailedException(e); diff --git a/common4j/src/main/com/microsoft/identity/common/java/net/UrlConnectionHttpClient.java b/common4j/src/main/com/microsoft/identity/common/java/net/UrlConnectionHttpClient.java index 9a79e7085f..25ab5b9f88 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/net/UrlConnectionHttpClient.java +++ b/common4j/src/main/com/microsoft/identity/common/java/net/UrlConnectionHttpClient.java @@ -23,11 +23,14 @@ package com.microsoft.identity.common.java.net; import static com.microsoft.identity.common.java.AuthenticationConstants.AAD.CLIENT_REQUEST_ID; +import static com.microsoft.identity.common.java.exception.ClientException.IO_ERROR; import static com.microsoft.identity.common.java.net.HttpConstants.HeaderField.CONTENT_TYPE; import static com.microsoft.identity.common.java.net.HttpConstants.HeaderField.XMS_CCS_REQUEST_ID; import static com.microsoft.identity.common.java.net.HttpConstants.HeaderField.XMS_CCS_REQUEST_SEQUENCE; import com.microsoft.identity.common.java.AuthenticationConstants; +import com.microsoft.identity.common.java.exception.ClientException; +import com.microsoft.identity.common.java.exception.ConnectionError; import com.microsoft.identity.common.java.flighting.CommonFlight; import com.microsoft.identity.common.java.flighting.CommonFlightsManager; import com.microsoft.identity.common.java.logging.Logger; @@ -50,6 +53,7 @@ import java.io.InputStreamReader; import java.io.OutputStream; import java.net.HttpURLConnection; +import java.net.ProtocolException; import java.net.SocketTimeoutException; import java.net.URL; import java.util.Date; @@ -57,7 +61,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.Callable; import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nullable; @@ -92,7 +95,7 @@ public class UrlConnectionHttpClient extends AbstractHttpClient { public static final int DEFAULT_READ_TIME_OUT_MS = 30000; protected static final int DEFAULT_STREAM_BUFFER_SIZE_BYTE = 1024; - private static final transient AtomicReference defaultReference = new AtomicReference<>(null); + private static final AtomicReference defaultReference = new AtomicReference<>(null); /** * Retry policy of this HttpClient. Default is {@link NoRetryPolicy}. @@ -195,7 +198,7 @@ public Boolean apply(HttpResponse response) { }) .isRetryableException(new Function() { public Boolean apply(Exception e) { - return e instanceof SocketTimeoutException; + return ConnectionError.CONNECTION_TIMEOUT.compare(e); } }) .build()) @@ -243,25 +246,16 @@ private static void recordHttpTelemetryEventEnd(final HttpResponse response) { * @param requestHeaders Headers used to send the http request. * @param requestContent Optional request body, if applicable. * @return HttpResponse The response for this request. - * @throws IOException If an error is encountered while servicing this request. + * @throws ClientException If an error is encountered while servicing this request. */ @Override public HttpResponse method(@NonNull final HttpClient.HttpMethod httpMethod, @NonNull final URL requestUrl, @NonNull final Map requestHeaders, - final byte[] requestContent) throws IOException { + final byte[] requestContent) throws ClientException { recordHttpTelemetryEventStart(httpMethod.name(), requestUrl, requestHeaders.get(CLIENT_REQUEST_ID)); final HttpRequest request = constructHttpRequest(httpMethod, requestUrl, requestHeaders, requestContent); - return retryPolicy.attempt(new Callable() { - public HttpResponse call() throws IOException { - return executeHttpSend(request, new Consumer() { - @Override - public void accept(HttpResponse httpResponse) { - recordHttpTelemetryEventEnd(httpResponse); - } - }); - } - }); + return retryPolicy.attempt(() -> executeHttpSend(request, UrlConnectionHttpClient::recordHttpTelemetryEventEnd)); } /** @@ -270,12 +264,12 @@ public void accept(HttpResponse httpResponse) { * @param requestHeaders the headers for the request. * @param requestContent the body content of the request, if applicable. May be null. * @return an HttpResponse with the result of the call. - * @throws IOException if there was a communication problem. + * @throws ClientException if there was a communication problem. */ @Override public HttpResponse patch(@NonNull final URL requestUrl, @NonNull final Map requestHeaders, - @edu.umd.cs.findbugs.annotations.Nullable final byte[] requestContent) throws IOException { + @Nullable final byte[] requestContent) throws ClientException { recordHttpTelemetryEventStart(HttpMethod.PATCH.name(), requestUrl, requestHeaders.get(CLIENT_REQUEST_ID)); final HttpRequest request = new HttpRequest( requestUrl, @@ -284,16 +278,7 @@ public HttpResponse patch(@NonNull final URL requestUrl, requestContent, null ); - return retryPolicy.attempt(new Callable() { - public HttpResponse call() throws IOException { - return executeHttpSend(request, new Consumer() { - @Override - public void accept(HttpResponse httpResponse) { - recordHttpTelemetryEventEnd(httpResponse); - } - }); - } - }); + return retryPolicy.attempt(() -> executeHttpSend(request, UrlConnectionHttpClient::recordHttpTelemetryEventEnd)); } private static HttpRequest constructHttpRequest(@NonNull HttpClient.HttpMethod httpMethod, @@ -328,7 +313,7 @@ private static HttpRequest constructHttpRequest(@NonNull HttpClient.HttpMethod h * @return The converted string * @throws IOException Thrown when failing to access inputStream stream. */ - private String convertStreamToString(final InputStream inputStream) throws IOException { + private String convertStreamToString(final InputStream inputStream) throws ClientException { try { final BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream, AuthenticationConstants.CHARSET_UTF8)); @@ -341,6 +326,8 @@ private String convertStreamToString(final InputStream inputStream) throws IOExc } return stringBuilder.toString(); + } catch (IOException e) { + throw ConnectionError.FAILED_TO_READ_FROM_INPUT_STREAM.getClientException(e); } finally { safeCloseStream(inputStream); } @@ -365,7 +352,7 @@ private static void safeCloseStream(final Closeable stream) { } } - private HttpResponse executeHttpSend(HttpRequest request, Consumer completionCallback) throws IOException { + private HttpResponse executeHttpSend(HttpRequest request, Consumer completionCallback) throws ClientException { final HttpURLConnection urlConnection = setupConnection(request); sendRequest(urlConnection, request.getRequestContent(), request.getRequestHeaders().get(HttpConstants.HeaderField.CONTENT_TYPE)); @@ -375,17 +362,23 @@ private HttpResponse executeHttpSend(HttpRequest request, Consumer try { try { responseStream = urlConnection.getInputStream(); - } catch (final SocketTimeoutException socketTimeoutException) { + } catch (final SocketTimeoutException e) { // SocketTimeoutExcetion is thrown when connection timeout happens. For connection // timeout, we want to retry once. Throw the exception to the upper layer, and the // upper layer will handle the retry. - throw socketTimeoutException; + throw ConnectionError.CONNECTION_TIMEOUT.getClientException(e); } catch (final IOException ioException) { // 404, for example, will generate an exception. We should catch it. responseStream = urlConnection.getErrorStream(); } - final int statusCode = urlConnection.getResponseCode(); + final int statusCode; + try { + statusCode = urlConnection.getResponseCode(); + } catch (IOException e) { + throw ConnectionError.FAILED_TO_GET_RESPONSE_CODE.getClientException(e); + } + final Date date = new Date(urlConnection.getDate()); final String responseBody = responseStream == null @@ -435,9 +428,14 @@ private HttpResponse executeHttpSend(HttpRequest request, Consumer return response; } - private HttpURLConnection setupConnection(HttpRequest request) throws IOException { + private HttpURLConnection setupConnection(HttpRequest request) throws ClientException { final String methodName = ":setupConnection"; - final HttpURLConnection urlConnection = HttpUrlConnectionFactory.createHttpURLConnection(request.getRequestUrl()); + final HttpURLConnection urlConnection; + try { + urlConnection = HttpUrlConnectionFactory.createHttpURLConnection(request.getRequestUrl()); + } catch (IOException e) { + throw ConnectionError.FAILED_TO_OPEN_CONNECTION.getClientException(e); + } // Apply request headers and update the headers with default attributes first final Set> headerEntries = request.getRequestHeaders().entrySet(); @@ -456,7 +454,12 @@ private HttpURLConnection setupConnection(HttpRequest request) throws IOExceptio Logger.warn(TAG + methodName, "gets a request from an unexpected protocol: " + request.getRequestUrl().getProtocol()); } - urlConnection.setRequestMethod(request.getRequestMethod()); + try { + urlConnection.setRequestMethod(request.getRequestMethod()); + } catch (ProtocolException e) { + throw ConnectionError.FAILED_TO_SET_REQUEST_METHOD.getClientException(e); + } + urlConnection.setConnectTimeout(getConnectTimeoutMs()); urlConnection.setReadTimeout(getReadTimeoutMs()); urlConnection.setInstanceFollowRedirects(true); @@ -476,7 +479,7 @@ private int getConnectTimeoutMs() { private static void sendRequest(@NonNull final HttpURLConnection connection, final byte[] contentRequest, - final String requestContentType) throws IOException { + final String requestContentType) throws ClientException { if (contentRequest == null) { return; } @@ -494,6 +497,8 @@ private static void sendRequest(@NonNull final HttpURLConnection connection, try { out = connection.getOutputStream(); out.write(contentRequest); + } catch (IOException e) { + throw ConnectionError.FAILED_TO_WRITE_TO_OUTPUT_STREAM.getClientException(e); } finally { safeCloseStream(out); } @@ -510,5 +515,4 @@ public static boolean isRetryableError(final int statusCode) { || statusCode == HttpURLConnection.HTTP_GATEWAY_TIMEOUT || statusCode == HttpURLConnection.HTTP_UNAVAILABLE; } - } diff --git a/common4j/src/main/com/microsoft/identity/common/java/providers/microsoft/azureactivedirectory/AzureActiveDirectory.java b/common4j/src/main/com/microsoft/identity/common/java/providers/microsoft/azureactivedirectory/AzureActiveDirectory.java index 0d5617d010..77c5a8d5b7 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/providers/microsoft/azureactivedirectory/AzureActiveDirectory.java +++ b/common4j/src/main/com/microsoft/identity/common/java/providers/microsoft/azureactivedirectory/AzureActiveDirectory.java @@ -39,12 +39,14 @@ import com.microsoft.identity.common.java.util.ObjectMapper; import com.microsoft.identity.common.java.util.StringUtil; import com.microsoft.identity.common.java.util.CommonURIBuilder; +import com.microsoft.identity.common.java.util.UrlUtil; import org.json.JSONException; import java.io.IOException; import java.lang.reflect.Type; import java.net.HttpURLConnection; +import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; import java.net.URL; @@ -182,16 +184,21 @@ public static synchronized String getDefaultCloudUrl() { } public static synchronized void performCloudDiscovery() - throws IOException, URISyntaxException { + throws ClientException { final String methodName = ":performCloudDiscovery"; - final URI instanceDiscoveryRequestUri = new CommonURIBuilder(getDefaultCloudUrl() + AAD_INSTANCE_DISCOVERY_ENDPOINT) - .setParameter(API_VERSION, API_VERSION_VALUE) - .setParameter(AUTHORIZATION_ENDPOINT, AUTHORIZATION_ENDPOINT_VALUE) - .build(); + final URI instanceDiscoveryRequestUri; + try { + instanceDiscoveryRequestUri = new CommonURIBuilder(getDefaultCloudUrl() + AAD_INSTANCE_DISCOVERY_ENDPOINT) + .setParameter(API_VERSION, API_VERSION_VALUE) + .setParameter(AUTHORIZATION_ENDPOINT, AUTHORIZATION_ENDPOINT_VALUE) + .build(); + } catch (URISyntaxException e) { + throw new ClientException(ClientException.MALFORMED_URL, e.getMessage(), e); + } - final HttpResponse response = - httpClient.get(new URL(instanceDiscoveryRequestUri.toString()), - new HashMap()); + final HttpResponse response = httpClient.get( + UrlUtil.makeUrl(instanceDiscoveryRequestUri.toString()), + new HashMap<>()); if (response.getStatusCode() >= HttpURLConnection.HTTP_BAD_REQUEST) { Logger.warn(TAG + methodName, "Error getting cloud information"); diff --git a/common4j/src/main/com/microsoft/identity/common/java/providers/oauth2/OAuth2Strategy.java b/common4j/src/main/com/microsoft/identity/common/java/providers/oauth2/OAuth2Strategy.java index ef4707a961..0d82381873 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/providers/oauth2/OAuth2Strategy.java +++ b/common4j/src/main/com/microsoft/identity/common/java/providers/oauth2/OAuth2Strategy.java @@ -325,7 +325,7 @@ protected final void setAuthorizationEndpoint(final String authorizationEndpoint mAuthorizationEndpoint = authorizationEndpoint; } - public AuthorizationResult getDeviceCode(@NonNull final MicrosoftStsAuthorizationRequest authorizationRequest) throws IOException { + public AuthorizationResult getDeviceCode(@NonNull final MicrosoftStsAuthorizationRequest authorizationRequest) throws IOException, ClientException { final String methodName = ":getDeviceCode"; // Set up headers and request body diff --git a/common4j/src/main/com/microsoft/identity/common/java/providers/oauth2/OpenIdProviderConfigurationClient.java b/common4j/src/main/com/microsoft/identity/common/java/providers/oauth2/OpenIdProviderConfigurationClient.java index 80ce139b21..3da03afa4e 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/providers/oauth2/OpenIdProviderConfigurationClient.java +++ b/common4j/src/main/com/microsoft/identity/common/java/providers/oauth2/OpenIdProviderConfigurationClient.java @@ -25,6 +25,7 @@ import static com.microsoft.identity.common.java.exception.ServiceException.OPENID_PROVIDER_CONFIGURATION_FAILED_TO_LOAD; import com.google.gson.Gson; +import com.microsoft.identity.common.java.exception.ClientException; import com.microsoft.identity.common.java.exception.ServiceException; import com.microsoft.identity.common.java.logging.Logger; import com.microsoft.identity.common.java.net.HttpClient; @@ -36,6 +37,7 @@ import java.io.IOException; import java.net.HttpURLConnection; +import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; import java.util.HashMap; @@ -194,7 +196,7 @@ private synchronized OpenIdProviderConfiguration loadOpenIdProviderConfiguration cacheConfiguration(configUrl, parsedConfig); return parsedConfig; - } catch (final IOException | URISyntaxException e) { + } catch (final ClientException | MalformedURLException | URISyntaxException e) { throw new ServiceException( OPENID_PROVIDER_CONFIGURATION_FAILED_TO_LOAD, "IOException while requesting metadata", diff --git a/common4j/src/main/com/microsoft/identity/common/java/util/UrlUtil.java b/common4j/src/main/com/microsoft/identity/common/java/util/UrlUtil.java index f8d8cabd2a..a1e077a63f 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/util/UrlUtil.java +++ b/common4j/src/main/com/microsoft/identity/common/java/util/UrlUtil.java @@ -22,7 +22,10 @@ // THE SOFTWARE. package com.microsoft.identity.common.java.util; +import static com.microsoft.identity.common.java.exception.ClientException.IO_ERROR; + import com.microsoft.identity.common.java.exception.ClientException; +import com.microsoft.identity.common.java.exception.ConnectionError; import com.microsoft.identity.common.java.logging.Logger; import java.io.UnsupportedEncodingException; @@ -246,6 +249,8 @@ static Map urlFormDecodeData(@NonNull final String urlParameter, /** * This creates a url from a String, rewriting any malformedUrlExceptions to runtime. + * Should be use for tests only. + * * @param urlString the string to convert. * @return the corresponding {@link URL}. */ @@ -257,6 +262,20 @@ public static URL makeUrlSilent(String urlString) { } } + /** + * This creates a url from a String, rewriting any malformedUrlExceptions to ClientException. + * + * @param urlString the string to convert. + * @return the corresponding {@link URL}. + */ + public static URL makeUrl(@NonNull final String urlString) throws ClientException { + try { + return new URL(urlString); + } catch (MalformedURLException e) { + throw new ClientException(ClientException.MALFORMED_URL, e.getMessage(), e); + } + } + public static String removeTrailingSlash(@NonNull final String urlString) { return urlString.replaceFirst("/*$", ""); } diff --git a/common4j/src/test/com/microsoft/identity/common/java/net/HttpUrlConnectionFactoryTest.java b/common4j/src/test/com/microsoft/identity/common/java/net/HttpUrlConnectionFactoryTest.java index d2b870544e..b9b58509a5 100644 --- a/common4j/src/test/com/microsoft/identity/common/java/net/HttpUrlConnectionFactoryTest.java +++ b/common4j/src/test/com/microsoft/identity/common/java/net/HttpUrlConnectionFactoryTest.java @@ -23,6 +23,8 @@ package com.microsoft.identity.common.java.net; +import com.microsoft.identity.common.java.exception.ClientException; + import org.junit.After; import org.junit.Assert; import org.junit.Ignore; diff --git a/common4j/src/test/com/microsoft/identity/common/java/net/UrlConnectionHttpClientTest.java b/common4j/src/test/com/microsoft/identity/common/java/net/UrlConnectionHttpClientTest.java index e11ce1f672..39e2c2ba40 100644 --- a/common4j/src/test/com/microsoft/identity/common/java/net/UrlConnectionHttpClientTest.java +++ b/common4j/src/test/com/microsoft/identity/common/java/net/UrlConnectionHttpClientTest.java @@ -28,6 +28,8 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import com.microsoft.identity.common.java.exception.ClientException; +import com.microsoft.identity.common.java.exception.ConnectionError; import com.microsoft.identity.http.MockConnection; import com.microsoft.identity.http.ResponseBody; @@ -56,7 +58,6 @@ import java.util.UUID; import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLHandshakeException; /** * Tests for {@link UrlConnectionHttpClient}. @@ -86,7 +87,7 @@ public void tearDown() { * Verify the expected exception is thrown when sending get request with null url. */ @Test(expected = NullPointerException.class) - public void testNullRequestUrl() throws IOException { + public void testNullRequestUrl() throws ClientException, IOException { sNoRetryClient.get(null, Collections.emptyMap()); } @@ -920,7 +921,7 @@ public void testHttpPostFailedWithSocketTimeoutRetryFailedWithNonRetryableCode() * Verify that initial http post fails with {@link SocketTimeoutException}, when we're not * running in legacy retry mode the socket timeout exception propagates to the caller. */ - @Test(expected = SocketTimeoutException.class) + @Test public void testHttpPostFailedWithSocketTimeoutNoRetriesDoesNotRetry() throws Exception { // The first connection fails with retryable SocketTimeout, the retry connection fails with 400. final HttpURLConnection firstConnection = MockConnection.getMockedConnectionWithSocketTimeout(); @@ -935,11 +936,9 @@ public void testHttpPostFailedWithSocketTimeoutNoRetriesDoesNotRetry() throws Ex try { assertEquals(2, HttpUrlConnectionFactory.getMockedConnectionCountInQueue()); sendWithMethodWithoutRetry(HttpTestMethod.POST); - } catch (final IOException e) { - if (!(e instanceof SocketTimeoutException)) { - fail(); - } - throw e; + fail(); + } catch (final ClientException e) { + Assert.assertTrue(ConnectionError.CONNECTION_TIMEOUT.compare(e)); } finally { assertEquals(1, HttpUrlConnectionFactory.getMockedConnectionCountInQueue()); final InOrder inOrder = Mockito.inOrder(firstConnection, secondConnection); @@ -1113,7 +1112,7 @@ private URL getRequestUrl() throws MalformedURLException { */ @Test - public void testNoSSL() throws IOException { + public void testNoSSL() throws ClientException, IOException { final HttpResponse response = sNoRetryClient.method( HttpClient.HttpMethod.GET, new URL("http://http.badssl.com/"), @@ -1127,7 +1126,7 @@ public void testNoSSL() throws IOException { @Test @Ignore("Ignored because our pipeline doesn't support TLS1.0 and TLS1.1. This still can be run locally.") - public void testTLS1() throws IOException { + public void testTLS1() throws ClientException, IOException { final HttpResponse response = sNoRetryClient.method( HttpClient.HttpMethod.GET, new URL("https://tls-v1-0.badssl.com:1010/"), @@ -1141,7 +1140,7 @@ public void testTLS1() throws IOException { @Test @Ignore("Ignored because our pipeline doesn't support TLS1.0 and TLS1.1. This still can be run locally.") - public void testTLS11() throws IOException, NoSuchAlgorithmException, KeyManagementException { + public void testTLS11() throws ClientException, IOException { final HttpResponse response = sNoRetryClient.method( HttpClient.HttpMethod.GET, new URL("https://tls-v1-1.badssl.com:1011/"), @@ -1154,7 +1153,7 @@ public void testTLS11() throws IOException, NoSuchAlgorithmException, KeyManagem } @Test - public void testTLS12() throws IOException { + public void testTLS12() throws ClientException, IOException { final HttpResponse response = sNoRetryClient.method( HttpClient.HttpMethod.GET, new URL("https://tls-v1-2.badssl.com:1012/"), @@ -1167,7 +1166,7 @@ public void testTLS12() throws IOException { } @Test - public void testTLSWithTLS11Context() throws IOException, NoSuchAlgorithmException, KeyManagementException { + public void testTLSWithTLS11Context() throws ClientException, IOException, NoSuchAlgorithmException, KeyManagementException { final SSLContext context = SSLContext.getInstance("TLSv1.1"); context.init(null, null, new SecureRandom()); @@ -1192,7 +1191,7 @@ public void testTLSWithTLS11Context() throws IOException, NoSuchAlgorithmExcepti } @Test - public void testPickHighestTLS() throws IOException { + public void testPickHighestTLS() throws ClientException, IOException { final HttpResponse response = sNoRetryClient.method( HttpClient.HttpMethod.GET, new URL("https://tls13.akamai.io/"), @@ -1204,24 +1203,27 @@ public void testPickHighestTLS() throws IOException { Assert.assertEquals("TLSv1.3", SSLSocketFactoryWrapper.getLastHandshakeTLSversion()); } - @Test(expected = SSLHandshakeException.class) - public void testConnectingToTLS13ServerWhileEnforcing12OnClientSide() throws IOException { + @Test + public void testConnectingToTLS13ServerWhileEnforcing12OnClientSide() throws ClientException, IOException { final UrlConnectionHttpClient client = UrlConnectionHttpClient.builder() .supportedSslProtocols(Arrays.asList("TLSv1.3")) .build(); - final HttpResponse response = client.method( - HttpClient.HttpMethod.GET, - new URL("https://tls-v1-2.badssl.com:1012/"), - new LinkedHashMap(), - null - ); - - Assert.fail(); + try { + final HttpResponse response = client.method( + HttpClient.HttpMethod.GET, + new URL("https://tls-v1-2.badssl.com:1012/"), + new LinkedHashMap(), + null + ); + Assert.fail(); + } catch (ClientException e){ + Assert.assertTrue(ConnectionError.FAILED_TO_GET_RESPONSE_CODE.compare(e)); + } } @Test - public void testSpecifyingSupportedSSLVersion() throws IOException { + public void testSpecifyingSupportedSSLVersion() throws ClientException, IOException { final UrlConnectionHttpClient client = UrlConnectionHttpClient.builder() .supportedSslProtocols(Arrays.asList("TLSv1.2")) .build(); @@ -1238,7 +1240,7 @@ public void testSpecifyingSupportedSSLVersion() throws IOException { } @Test(expected = IllegalStateException.class) - public void testConnectingToHttpsButGetHttpUrlConnection() throws IOException { + public void testConnectingToHttpsButGetHttpUrlConnection() throws ClientException, IOException { HttpUrlConnectionFactory.addMockedConnection(MockConnection.getMockedHttpConnection()); final HttpResponse response = sNoRetryClient.method( HttpClient.HttpMethod.GET, @@ -1251,7 +1253,7 @@ public void testConnectingToHttpsButGetHttpUrlConnection() throws IOException { } @Test - public void testConnectingToHttp() throws IOException { + public void testConnectingToHttp() throws ClientException, IOException { HttpUrlConnectionFactory.addMockedConnection(MockConnection.getMockedHttpConnection()); final HttpResponse response = sNoRetryClient.method( HttpClient.HttpMethod.GET, diff --git a/common4j/src/testFixtures/java/com/microsoft/identity/http/HttpRequestExaminer.java b/common4j/src/testFixtures/java/com/microsoft/identity/http/HttpRequestExaminer.java index 09ead0e362..9a8948b9cc 100644 --- a/common4j/src/testFixtures/java/com/microsoft/identity/http/HttpRequestExaminer.java +++ b/common4j/src/testFixtures/java/com/microsoft/identity/http/HttpRequestExaminer.java @@ -22,6 +22,7 @@ // THE SOFTWARE. package com.microsoft.identity.http; +import com.microsoft.identity.common.java.exception.ClientException; import com.microsoft.identity.common.java.net.HttpClient; import com.microsoft.identity.common.java.net.HttpResponse; @@ -45,7 +46,7 @@ public abstract class HttpRequestExaminer implements HttpRequestInterceptor { public HttpResponse performIntercept(@NonNull final HttpClient.HttpMethod httpMethod, @NonNull final URL requestUrl, @NonNull final Map requestHeaders, - @Nullable final byte[] requestContent) throws IOException { + @Nullable final byte[] requestContent) throws ClientException { examineRequestHeaders(requestHeaders); examineRequestUrl(requestUrl); return mOriginalClient.method( diff --git a/common4j/src/testFixtures/java/com/microsoft/identity/http/HttpRequestInterceptor.java b/common4j/src/testFixtures/java/com/microsoft/identity/http/HttpRequestInterceptor.java index a16c41bf8c..5c3d705c61 100644 --- a/common4j/src/testFixtures/java/com/microsoft/identity/http/HttpRequestInterceptor.java +++ b/common4j/src/testFixtures/java/com/microsoft/identity/http/HttpRequestInterceptor.java @@ -22,6 +22,7 @@ // THE SOFTWARE. package com.microsoft.identity.http; +import com.microsoft.identity.common.java.exception.ClientException; import com.microsoft.identity.common.java.net.HttpClient; import com.microsoft.identity.common.java.net.HttpResponse; @@ -43,11 +44,11 @@ public interface HttpRequestInterceptor { * @param requestHeaders the request headers * @param requestContent the request content * @return the http response object - * @throws IOException throws an exception when something went wrong during the http request + * @throws ClientException throws an exception when something went wrong during the http request */ HttpResponse performIntercept(@NonNull HttpClient.HttpMethod httpMethod, @NonNull URL requestUrl, @NonNull Map requestHeaders, - @Nullable byte[] requestContent) throws IOException; + @Nullable byte[] requestContent) throws ClientException; } diff --git a/common4j/src/testFixtures/java/com/microsoft/identity/http/HttpRequestRewriter.java b/common4j/src/testFixtures/java/com/microsoft/identity/http/HttpRequestRewriter.java index 6ae084fb84..45e2f3cfee 100644 --- a/common4j/src/testFixtures/java/com/microsoft/identity/http/HttpRequestRewriter.java +++ b/common4j/src/testFixtures/java/com/microsoft/identity/http/HttpRequestRewriter.java @@ -22,6 +22,7 @@ // THE SOFTWARE. package com.microsoft.identity.http; +import com.microsoft.identity.common.java.exception.ClientException; import com.microsoft.identity.common.java.net.HttpClient; import com.microsoft.identity.common.java.net.HttpResponse; @@ -46,7 +47,7 @@ public abstract class HttpRequestRewriter implements HttpRequestInterceptor { public HttpResponse performIntercept(@NonNull final HttpClient.HttpMethod httpMethod, @NonNull final URL requestUrl, @NonNull final Map requestHeaders, - @Nullable final byte[] requestContent) throws IOException { + @Nullable final byte[] requestContent) throws ClientException { return mOriginalClient.method( rewriteHttpMethod(httpMethod), rewriteRequestUrl(requestUrl), diff --git a/common4j/src/testFixtures/java/com/microsoft/identity/http/HttpResponseRewriter.java b/common4j/src/testFixtures/java/com/microsoft/identity/http/HttpResponseRewriter.java index 807179f0e7..fe1c849158 100644 --- a/common4j/src/testFixtures/java/com/microsoft/identity/http/HttpResponseRewriter.java +++ b/common4j/src/testFixtures/java/com/microsoft/identity/http/HttpResponseRewriter.java @@ -22,6 +22,7 @@ // THE SOFTWARE. package com.microsoft.identity.http; +import com.microsoft.identity.common.java.exception.ClientException; import com.microsoft.identity.common.java.net.HttpClient; import com.microsoft.identity.common.java.net.HttpResponse; @@ -46,7 +47,7 @@ public abstract class HttpResponseRewriter implements HttpRequestInterceptor { public HttpResponse performIntercept(@NonNull final HttpClient.HttpMethod httpMethod, @NonNull final URL requestUrl, @NonNull final Map requestHeaders, - @Nullable final byte[] requestContent) throws IOException { + @Nullable final byte[] requestContent) throws ClientException { final HttpResponse originalResponse = mOriginalClient.method( httpMethod, requestUrl, diff --git a/common4j/src/testFixtures/java/com/microsoft/identity/http/InterceptedHttpClient.java b/common4j/src/testFixtures/java/com/microsoft/identity/http/InterceptedHttpClient.java index 491f556447..95051a565c 100644 --- a/common4j/src/testFixtures/java/com/microsoft/identity/http/InterceptedHttpClient.java +++ b/common4j/src/testFixtures/java/com/microsoft/identity/http/InterceptedHttpClient.java @@ -22,6 +22,7 @@ // THE SOFTWARE. package com.microsoft.identity.http; +import com.microsoft.identity.common.java.exception.ClientException; import com.microsoft.identity.common.java.net.AbstractHttpClient; import com.microsoft.identity.common.java.net.HttpClient; import com.microsoft.identity.common.java.net.HttpResponse; @@ -47,7 +48,7 @@ public InterceptedHttpClient(@NonNull final HttpClient httpClient) { public HttpResponse method(@NonNull final HttpMethod httpMethod, @NonNull final URL requestUrl, @NonNull final Map requestHeaders, - @Nullable final byte[] requestContent) throws IOException { + @Nullable final byte[] requestContent) throws ClientException { final HttpRequestInterceptor interceptor = MockHttpClient.getInterceptor(httpMethod, requestUrl, requestHeaders, requestContent); if (interceptor == null) { return mClient.method(httpMethod, requestUrl, requestHeaders, requestContent); diff --git a/common4j/src/testFixtures/java/com/microsoft/identity/http/MockHttpClient.java b/common4j/src/testFixtures/java/com/microsoft/identity/http/MockHttpClient.java index e023caad47..d1079b7bce 100644 --- a/common4j/src/testFixtures/java/com/microsoft/identity/http/MockHttpClient.java +++ b/common4j/src/testFixtures/java/com/microsoft/identity/http/MockHttpClient.java @@ -22,6 +22,7 @@ // THE SOFTWARE. package com.microsoft.identity.http; +import com.microsoft.identity.common.java.exception.ClientException; import com.microsoft.identity.common.java.net.HttpClient; import com.microsoft.identity.common.java.net.HttpClient.HttpMethod; import com.microsoft.identity.common.java.net.HttpRequest; @@ -124,7 +125,7 @@ public static HttpRequestInterceptor getInterceptor( final HttpRequestInterceptor httpRequestInterceptor = interceptors.get(matcher); return new HttpRequestInterceptor() { @Override - public HttpResponse performIntercept(@NonNull HttpClient.HttpMethod httpMethod, @NonNull URL requestUrl, @NonNull Map requestHeaders, @Nullable byte[] requestContent) throws IOException { + public HttpResponse performIntercept(@NonNull HttpClient.HttpMethod httpMethod, @NonNull URL requestUrl, @NonNull Map requestHeaders, @Nullable byte[] requestContent) throws ClientException { if (sSaveRequests.get()) { sInterceptedRequests.add(new HttpRequest(url, requestHeaders, method.name(), requestContent, null)); } @@ -150,7 +151,7 @@ public HttpResponse performIntercept( @NonNull HttpMethod httpMethod, @NonNull URL requestUrl, @NonNull Map requestHeaders, - @Nullable byte[] requestContent) throws IOException { + @Nullable byte[] requestContent) throws ClientException { return httpResponse; } }); @@ -252,7 +253,7 @@ public HttpResponse performIntercept( @NonNull HttpMethod httpMethod, @NonNull URL requestUrl, @NonNull Map requestHeaders, - @Nullable byte[] requestContent) throws IOException { + @Nullable byte[] requestContent) throws ClientException { return httpResponse; } } diff --git a/common4j/src/testFixtures/java/com/microsoft/identity/shadow/ShadowHttpClient.java b/common4j/src/testFixtures/java/com/microsoft/identity/shadow/ShadowHttpClient.java index 4abab7e7d5..25a50f1659 100644 --- a/common4j/src/testFixtures/java/com/microsoft/identity/shadow/ShadowHttpClient.java +++ b/common4j/src/testFixtures/java/com/microsoft/identity/shadow/ShadowHttpClient.java @@ -22,6 +22,7 @@ // THE SOFTWARE. package com.microsoft.identity.shadow; +import com.microsoft.identity.common.java.exception.ClientException; import com.microsoft.identity.common.java.net.AbstractHttpClient; import com.microsoft.identity.common.java.net.HttpClient; import com.microsoft.identity.common.java.net.HttpResponse; @@ -57,7 +58,7 @@ public class ShadowHttpClient { public HttpResponse method(@NonNull HttpClient.HttpMethod httpMethod, @NonNull URL requestUrl, @NonNull Map requestHeaders, - @Nullable byte[] requestContent) throws IOException { + @Nullable byte[] requestContent) throws ClientException { final HttpRequestInterceptor interceptor = MockHttpClient.getInterceptor(httpMethod, requestUrl, requestHeaders, requestContent); if (interceptor == null) { return UrlConnectionHttpClient.getDefaultInstance().method(httpMethod, requestUrl, requestHeaders, requestContent); @@ -69,52 +70,52 @@ public HttpResponse method(@NonNull HttpClient.HttpMethod httpMethod, @Implementation public HttpResponse put(@NonNull URL requestUrl, @NonNull Map requestHeaders, - @Nullable byte[] requestContent) throws IOException { + @Nullable byte[] requestContent) throws ClientException { return method(HttpClient.HttpMethod.PUT, requestUrl, requestHeaders, requestContent); } @Implementation public HttpResponse patch(@NonNull URL requestUrl, @NonNull Map requestHeaders, - @Nullable byte[] requestContent) throws IOException { + @Nullable byte[] requestContent) throws ClientException { return method(HttpClient.HttpMethod.PATCH, requestUrl, requestHeaders, requestContent); } @Implementation public HttpResponse options(@NonNull URL requestUrl, - @NonNull Map requestHeaders) throws IOException { + @NonNull Map requestHeaders) throws ClientException { return method(HttpClient.HttpMethod.OPTIONS, requestUrl, requestHeaders, null); } @Implementation protected HttpResponse post(@NonNull URL requestUrl, @NonNull Map requestHeaders, - @Nullable byte[] requestContent) throws IOException { + @Nullable byte[] requestContent) throws ClientException { return method(HttpClient.HttpMethod.POST, requestUrl, requestHeaders, requestContent); } @Implementation public HttpResponse delete(@NonNull URL requestUrl, @NonNull Map requestHeaders, - @Nullable byte[] requestContent) throws IOException { + @Nullable byte[] requestContent) throws ClientException { return method(HttpClient.HttpMethod.DELETE, requestUrl, requestHeaders, requestContent); } @Implementation public HttpResponse get(@NonNull URL requestUrl, - @NonNull Map requestHeaders) throws IOException { + @NonNull Map requestHeaders) throws ClientException { return method(HttpClient.HttpMethod.GET, requestUrl, requestHeaders, null); } @Implementation public HttpResponse head(@NonNull URL requestUrl, - @NonNull Map requestHeaders) throws IOException { + @NonNull Map requestHeaders) throws ClientException { return method(HttpClient.HttpMethod.HEAD, requestUrl, requestHeaders, null); } @Implementation public HttpResponse trace(@NonNull URL requestUrl, - @NonNull Map requestHeaders) throws IOException { + @NonNull Map requestHeaders) throws ClientException { return method(HttpClient.HttpMethod.TRACE, requestUrl, requestHeaders, null); } } From 1bb242a87b0ed23bcfb19f012fcf15d4bed7850a Mon Sep 17 00:00:00 2001 From: Brian Melton-Grace Date: Thu, 5 Dec 2024 10:07:26 -0800 Subject: [PATCH 3/7] Correct misspelling of Lombok (#2526) See diff for details - minor spelling correction. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 99a63f95d7..756e1a7176 100644 --- a/README.md +++ b/README.md @@ -21,4 +21,4 @@ For more information see the [Code of Conduct FAQ](https://opensource.microsoft. contact [opencode@microsoft.com](mailto:opencode@microsoft.com) with any additional questions or comments. ### Android Studio Build Requirement -Please note that this project uses [Lombok](https://projectlombok.org/) internally and while using Android Studio you will need to install [Lobmok Plugin](https://plugins.jetbrains.com/plugin/6317-lombok) to get the project to build successfully within Android Studio. +Please note that this project uses [Lombok](https://projectlombok.org/) internally and while using Android Studio you will need to install [Lombok Plugin](https://plugins.jetbrains.com/plugin/6317-lombok) to get the project to build successfully within Android Studio. From ebdac47732c2715d75a9c6a70036716a85110d57 Mon Sep 17 00:00:00 2001 From: Dome Pongmongkol <19558668+rpdome@users.noreply.github.com> Date: Wed, 11 Dec 2024 12:09:28 -0800 Subject: [PATCH 4/7] Correlated suberrors with how OneAuth categorizes them. (#2555) https://office.visualstudio.com/OneAuth/_git/OneAuth?path=%2Fmsal%2Fsource%2Fandroid%2Fmsal%2Fapp%2Fsrc%2Fmain%2Fjava%2Fcom%2Fmicrosoft%2Fidentity%2Finternal%2Fhttp%2FHttpClient.java&_a=contents&version=GBmaster so that us and oneauth has the same understanding of what happens. There are 2 minor differences. 1. We have a separate timeout error (for our retry logic) 2. we have a separate "unknown exception" suberror, so that we can properly identify and assign any errors that falls into this bracket in the future. --- changelog.txt | 2 +- .../common/java/exception/ConnectionError.kt | 70 ++++++++++++++----- .../java/net/UrlConnectionHttpClient.java | 57 +++++++-------- .../java/net/UrlConnectionHttpClientTest.java | 2 +- 4 files changed, 79 insertions(+), 52 deletions(-) diff --git a/changelog.txt b/changelog.txt index b87a0e2696..ea73f5dad9 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,6 +1,6 @@ vNext ---------- -- [MINOR] Add suberror for network errors (#2537) +- [MAJOR] 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/common4j/src/main/com/microsoft/identity/common/java/exception/ConnectionError.kt b/common4j/src/main/com/microsoft/identity/common/java/exception/ConnectionError.kt index 31a9ad12ff..858797c192 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/exception/ConnectionError.kt +++ b/common4j/src/main/com/microsoft/identity/common/java/exception/ConnectionError.kt @@ -22,27 +22,19 @@ // THE SOFTWARE. package com.microsoft.identity.common.java.exception +import java.io.EOFException +import java.net.ConnectException +import java.net.SocketException +import java.net.SocketTimeoutException +import java.net.UnknownHostException +import javax.net.ssl.SSLException + 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"), + NO_NETWORK("ce_no_network"), + NETWORK_TEMPORARILY_UNAVAILABLE("ce_network_temporarily_unavailable"), + UNEXPECTED_EXCEPTION("ce_unexpected_exception"), 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. **/ @@ -53,4 +45,46 @@ enum class ConnectionError(val value: String) { return this.value == throwable.subErrorCode } + + companion object { + /** + * Converts this [ConnectionError] into a [ClientException] + **/ + @JvmStatic + fun getClientException(cause: Throwable): ClientException { + val e = ClientException( + ClientException.IO_ERROR, + "An IO error occurred in the network layer: " + cause.message, + cause + ) + + e.subErrorCode = getConnectionError(cause).value + return e + } + + /** + * Converts a [Throwable] into a suberrorCode + **/ + private fun getConnectionError(cause: Throwable): ConnectionError { + if (cause is SocketTimeoutException) { + // Slow or unreliable network + return CONNECTION_TIMEOUT + } + + if (cause is EOFException // Unexpected disconnect + || cause is SSLException // SSL Handshake failed + || cause is ConnectException // Remote socket connection failed + ) { + return NETWORK_TEMPORARILY_UNAVAILABLE + } + + if (cause is UnknownHostException // Unable to query DNS for hostname + || cause is SocketException // No conn to remote socket, or Airplane mode, or no internet permission will hit this + ) { + return NO_NETWORK + } + + return UNEXPECTED_EXCEPTION + } + } } diff --git a/common4j/src/main/com/microsoft/identity/common/java/net/UrlConnectionHttpClient.java b/common4j/src/main/com/microsoft/identity/common/java/net/UrlConnectionHttpClient.java index 25ab5b9f88..85281883b6 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/net/UrlConnectionHttpClient.java +++ b/common4j/src/main/com/microsoft/identity/common/java/net/UrlConnectionHttpClient.java @@ -48,14 +48,19 @@ import java.io.BufferedReader; import java.io.Closeable; +import java.io.EOFException; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStream; +import java.net.ConnectException; import java.net.HttpURLConnection; +import java.net.NoRouteToHostException; import java.net.ProtocolException; +import java.net.SocketException; import java.net.SocketTimeoutException; import java.net.URL; +import java.net.UnknownHostException; import java.util.Date; import java.util.HashMap; import java.util.List; @@ -66,6 +71,7 @@ import javax.annotation.Nullable; import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLException; import javax.net.ssl.SSLSocketFactory; import io.opentelemetry.api.trace.Span; @@ -313,7 +319,7 @@ private static HttpRequest constructHttpRequest(@NonNull HttpClient.HttpMethod h * @return The converted string * @throws IOException Thrown when failing to access inputStream stream. */ - private String convertStreamToString(final InputStream inputStream) throws ClientException { + private String convertStreamToString(final InputStream inputStream) throws IOException { try { final BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream, AuthenticationConstants.CHARSET_UTF8)); @@ -326,8 +332,6 @@ private String convertStreamToString(final InputStream inputStream) throws Clien } return stringBuilder.toString(); - } catch (IOException e) { - throw ConnectionError.FAILED_TO_READ_FROM_INPUT_STREAM.getClientException(e); } finally { safeCloseStream(inputStream); } @@ -353,10 +357,17 @@ private static void safeCloseStream(final Closeable stream) { } private HttpResponse executeHttpSend(HttpRequest request, Consumer completionCallback) throws ClientException { - final HttpURLConnection urlConnection = setupConnection(request); - - sendRequest(urlConnection, request.getRequestContent(), request.getRequestHeaders().get(HttpConstants.HeaderField.CONTENT_TYPE)); + try { + final HttpURLConnection urlConnection = setupConnection(request); + sendRequest(urlConnection, request.getRequestContent(), request.getRequestHeaders().get(HttpConstants.HeaderField.CONTENT_TYPE)); + return getHttpResponse(completionCallback, urlConnection); + } catch (final IOException e) { + throw ConnectionError.getClientException(e); + } + } + private HttpResponse getHttpResponse(Consumer completionCallback, + HttpURLConnection urlConnection) throws IOException { InputStream responseStream = null; HttpResponse response = null; try { @@ -366,18 +377,13 @@ private HttpResponse executeHttpSend(HttpRequest request, Consumer // SocketTimeoutExcetion is thrown when connection timeout happens. For connection // timeout, we want to retry once. Throw the exception to the upper layer, and the // upper layer will handle the retry. - throw ConnectionError.CONNECTION_TIMEOUT.getClientException(e); + throw e; } catch (final IOException ioException) { // 404, for example, will generate an exception. We should catch it. responseStream = urlConnection.getErrorStream(); } - final int statusCode; - try { - statusCode = urlConnection.getResponseCode(); - } catch (IOException e) { - throw ConnectionError.FAILED_TO_GET_RESPONSE_CODE.getClientException(e); - } + final int statusCode = urlConnection.getResponseCode(); final Date date = new Date(urlConnection.getDate()); @@ -401,12 +407,12 @@ private HttpResponse executeHttpSend(HttpRequest request, Consumer ); span.setAttribute( - com.microsoft.identity.common.java.opentelemetry.AttributeName.ccs_request_id.name(), + AttributeName.ccs_request_id.name(), response.getHeaderValue(XMS_CCS_REQUEST_ID, 0) ); span.setAttribute( - com.microsoft.identity.common.java.opentelemetry.AttributeName.ccs_request_sequence.name(), + AttributeName.ccs_request_sequence.name(), response.getHeaderValue(XMS_CCS_REQUEST_SEQUENCE, 0) ); } @@ -419,7 +425,6 @@ private HttpResponse executeHttpSend(HttpRequest request, Consumer AttributeName.http_status_code.name(), response.getStatusCode() ); - } finally { completionCallback.accept(response); safeCloseStream(responseStream); @@ -428,14 +433,9 @@ private HttpResponse executeHttpSend(HttpRequest request, Consumer return response; } - private HttpURLConnection setupConnection(HttpRequest request) throws ClientException { + private HttpURLConnection setupConnection(HttpRequest request) throws IOException { final String methodName = ":setupConnection"; - final HttpURLConnection urlConnection; - try { - urlConnection = HttpUrlConnectionFactory.createHttpURLConnection(request.getRequestUrl()); - } catch (IOException e) { - throw ConnectionError.FAILED_TO_OPEN_CONNECTION.getClientException(e); - } + final HttpURLConnection urlConnection = HttpUrlConnectionFactory.createHttpURLConnection(request.getRequestUrl()); // Apply request headers and update the headers with default attributes first final Set> headerEntries = request.getRequestHeaders().entrySet(); @@ -454,12 +454,7 @@ private HttpURLConnection setupConnection(HttpRequest request) throws ClientExce Logger.warn(TAG + methodName, "gets a request from an unexpected protocol: " + request.getRequestUrl().getProtocol()); } - try { - urlConnection.setRequestMethod(request.getRequestMethod()); - } catch (ProtocolException e) { - throw ConnectionError.FAILED_TO_SET_REQUEST_METHOD.getClientException(e); - } - + urlConnection.setRequestMethod(request.getRequestMethod()); urlConnection.setConnectTimeout(getConnectTimeoutMs()); urlConnection.setReadTimeout(getReadTimeoutMs()); urlConnection.setInstanceFollowRedirects(true); @@ -479,7 +474,7 @@ private int getConnectTimeoutMs() { private static void sendRequest(@NonNull final HttpURLConnection connection, final byte[] contentRequest, - final String requestContentType) throws ClientException { + final String requestContentType) throws IOException { if (contentRequest == null) { return; } @@ -497,8 +492,6 @@ private static void sendRequest(@NonNull final HttpURLConnection connection, try { out = connection.getOutputStream(); out.write(contentRequest); - } catch (IOException e) { - throw ConnectionError.FAILED_TO_WRITE_TO_OUTPUT_STREAM.getClientException(e); } finally { safeCloseStream(out); } diff --git a/common4j/src/test/com/microsoft/identity/common/java/net/UrlConnectionHttpClientTest.java b/common4j/src/test/com/microsoft/identity/common/java/net/UrlConnectionHttpClientTest.java index 39e2c2ba40..4609f9c010 100644 --- a/common4j/src/test/com/microsoft/identity/common/java/net/UrlConnectionHttpClientTest.java +++ b/common4j/src/test/com/microsoft/identity/common/java/net/UrlConnectionHttpClientTest.java @@ -1218,7 +1218,7 @@ public void testConnectingToTLS13ServerWhileEnforcing12OnClientSide() throws Cli ); Assert.fail(); } catch (ClientException e){ - Assert.assertTrue(ConnectionError.FAILED_TO_GET_RESPONSE_CODE.compare(e)); + Assert.assertTrue(ConnectionError.NETWORK_TEMPORARILY_UNAVAILABLE.compare(e)); } } From 03889f08f82291d494d53ab832cfd527cc273a65 Mon Sep 17 00:00:00 2001 From: pedro romero vargas <76129899+p3dr0rv@users.noreply.github.com> Date: Thu, 12 Dec 2024 18:10:16 -0800 Subject: [PATCH 5/7] Add switch_browser toMicrosoftStsAuthorizationRequest, Fixes AB#3079799 (#2550) [AB#3079799](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3079799) MicrosoftStsAuthorizationRequest is the object used to represent the request parameters on an authorization request, here we add the new property to represent the parameter switch_browser that will be sent to STS when DUNA is enabled. https://github.com/AzureAD/ad-accounts-for-android/pull/3012 --------- Co-authored-by: Shahzaib <37125644+shahzaibj@users.noreply.github.com> --- changelog.txt | 1 + .../MicrosoftStsAuthorizationRequest.java | 15 +++++++++++++++ .../MicrosoftStsAuthorizationRequestTests.java | 11 +++++++++++ 3 files changed, 27 insertions(+) diff --git a/changelog.txt b/changelog.txt index ea73f5dad9..2b72b69221 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,5 +1,6 @@ vNext ---------- +- [MINOR] Add switch_browser toMicrosoftStsAuthorizationRequest (#2550) - [MAJOR] 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) diff --git a/common4j/src/main/com/microsoft/identity/common/java/providers/microsoft/microsoftsts/MicrosoftStsAuthorizationRequest.java b/common4j/src/main/com/microsoft/identity/common/java/providers/microsoft/microsoftsts/MicrosoftStsAuthorizationRequest.java index 293df8c85d..eb623a63b4 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/providers/microsoft/microsoftsts/MicrosoftStsAuthorizationRequest.java +++ b/common4j/src/main/com/microsoft/identity/common/java/providers/microsoft/microsoftsts/MicrosoftStsAuthorizationRequest.java @@ -125,6 +125,13 @@ public class MicrosoftStsAuthorizationRequest extends MicrosoftAuthorizationRequ // TODO private transient InstanceDiscoveryMetadata mInstanceDiscoveryMetadata; // TODO private boolean mIsExtendedLifetimeEnabled = false; + // The value 1 indicates that client supports DUNA flow. + @Getter + @Accessors(prefix = "m") + @SerializedName("switch_browser") + private final String mSwitchBrowser; + + public static final class Prompt { /** * AcquireToken will send prompt=select_account to the authorize endpoint. Shows a list of users from which can be @@ -170,6 +177,7 @@ protected MicrosoftStsAuthorizationRequest(final Builder builder) { mApplicationIdentifier = builder.mApplicationIdentifier; mMamEnrollmentIdentifier = builder.mMamEnrollmentIdentifier; mOpenIdProviderConfiguration = builder.mOpenIdProviderConfiguration; + mSwitchBrowser = builder.mSwitchBrowser; } public static class Builder extends MicrosoftAuthorizationRequest.Builder { @@ -186,6 +194,8 @@ public static class Builder extends MicrosoftAuthorizationRequest.Builder mFlightParameters = new HashMap<>(); private OpenIdProviderConfiguration mOpenIdProviderConfiguration; + private String mSwitchBrowser; + public MicrosoftStsAuthorizationRequest.Builder setUid(String uid) { mUid = uid; return self(); @@ -241,6 +251,11 @@ public MicrosoftStsAuthorizationRequest.Builder setOpenIdProviderConfiguration(O return self(); } + public MicrosoftStsAuthorizationRequest.Builder setSwitchBrowser(final String value) { + mSwitchBrowser = value; + return self(); + } + @Override public MicrosoftStsAuthorizationRequest.Builder self() { return this; diff --git a/common4j/src/test/com/microsoft/identity/common/java/providers/microsoft/microsoftsts/MicrosoftStsAuthorizationRequestTests.java b/common4j/src/test/com/microsoft/identity/common/java/providers/microsoft/microsoftsts/MicrosoftStsAuthorizationRequestTests.java index 701a4f1d69..affdc4a404 100644 --- a/common4j/src/test/com/microsoft/identity/common/java/providers/microsoft/microsoftsts/MicrosoftStsAuthorizationRequestTests.java +++ b/common4j/src/test/com/microsoft/identity/common/java/providers/microsoft/microsoftsts/MicrosoftStsAuthorizationRequestTests.java @@ -296,4 +296,15 @@ public void testSetFlightParameters() throws MalformedURLException, ClientExcept assertTrue("Flight Param 1", actualCodeRequestUrl.contains(MOCK_FLIGHT_QUERY_1 + "=" + MOCK_FLIGHT_VALUE_1)); assertTrue("Flight Param 2", actualCodeRequestUrl.contains(MOCK_FLIGHT_QUERY_2 + "=" + MOCK_FLIGHT_VALUE_2)); } + + @Test + public void testRequestWithSwitchBrowser() throws ClientException, MalformedURLException { + final MicrosoftStsAuthorizationRequest request = new MicrosoftStsAuthorizationRequest.Builder() + .setAuthority(getValidRequestUrl()) + .setSwitchBrowser("1") + .build(); + + final String actualCodeRequestUrl = request.getAuthorizationRequestAsHttpRequest().toString(); + assertTrue(actualCodeRequestUrl.contains("switch_browser=1")); + } } From e77bcfcb764a07aae07ff0c30d06662e5305f1b8 Mon Sep 17 00:00:00 2001 From: Dome Pongmongkol <19558668+rpdome@users.noreply.github.com> Date: Fri, 13 Dec 2024 14:24:10 -0800 Subject: [PATCH 6/7] Remove unnecessary spans, Fixes AB#3074869 (#2556) [AB#3074869](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3074869) --- .../identity/common/java/opentelemetry/SpanName.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/common4j/src/main/com/microsoft/identity/common/java/opentelemetry/SpanName.java b/common4j/src/main/com/microsoft/identity/common/java/opentelemetry/SpanName.java index dc734882be..67e306b79f 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/opentelemetry/SpanName.java +++ b/common4j/src/main/com/microsoft/identity/common/java/opentelemetry/SpanName.java @@ -26,34 +26,25 @@ public enum SpanName { AcquirePrtUsingBrt, AcquireTokenInteractive, AcquireTokenSilent, - CryptoFactoryEvent, SetScopeForDMAgentForFoci, GetAccounts, RemoveAccount, WorkplaceJoin, ATIInteractively, ATISilently, - DoDiscovery, WorkplaceLeave, DeviceState, CertBasedAuth, - UploadBrokerLogs, - InitializePowerLift, MSAL_PerformIpcStrategy, DeviceRegistrationApi, WorkplaceJoinApi, AcquireTokenDcf, AcquireTokenDcfAuthRequest, AcquireTokenDcfFetchToken, - AccountStorageWithBackup, EncryptionManager, Passthrough, BrokerOperationRequestDispatcher, - BrokerDiscoveryManagerGetActiveBroker, BrokerDiscoveryManagerPerformDiscoveryProcess, - BrokerDiscoveryMetadataAggregator, - BrokerSelectionProtocolManager, - BrokerDiscoveryV1ProtocolBroadcastResult, Fido, BrokerAccountServiceRemoveAccounts, AcquirePrtUsingTransferToken, From 2541bf63e5680c8af1b00da05993d11bcc3e4893 Mon Sep 17 00:00:00 2001 From: Sowmya Malayanur <69237821+somalaya@users.noreply.github.com> Date: Wed, 18 Dec 2024 11:17:17 -0800 Subject: [PATCH 7/7] Replace Deprecated Keystore API for Android 28+, Fixes AB#3110184 (#2558) Issue : https://portal.microsofticm.com/imp/v5/incidents/details/540463066/summary Keystore operation "unwrap" is failing on Pixel 5 Android 14 devices. It is still not clear why the operation would fail specifically on Pixel 5 devices but it could be some bug on google side which is fixed for all other devices through a patch. Updates for Pixel 5 are stopped hence it may not have the google fix. However, the getSpecForKeyStoreKey method used while wrapping the key was using a deprecated API (KeyPairGeneratorSpec). It was deprecated in Android 23. Updating it to latest one has somehow resolved the issue for 1 customer (Since there are no updates from other customers, we assumed that it is the fix). Exception : [YPC] 2024-11-14 17:34:29.29 [25795][917] ERROR [AndroidKeyStoreUtil:unwrap] [2024-11-14 12:04:29 - thread_id: 911, correlation_id: UNSET - Android 34] invalid_key java.security.InvalidKeyException: Keystore operation failed at android.security.keystore2.KeyStoreCryptoOperationUtils.getInvalidKeyException(KeyStoreCryptoOperationUtils.java:128) at android.security.keystore2.KeyStoreCryptoOperationUtils.getExceptionForCipherInit(KeyStoreCryptoOperationUtils.java:152) at android.security.keystore2.AndroidKeyStoreCipherSpiBase.ensureKeystoreOperationInitialized(AndroidKeyStoreCipherSpiBase.java:354) Fix : Removed the deprecated API KeyPairGeneratorSpec and using the new one KeyGenParameterSpec which lets us set the purpose as PURPOSE_WRAP_KEY Testing : Ran the pipeline to confirm if the instrumented and UI tests are running as expected https://identitydivision.visualstudio.com/Engineering/_build/results?buildId=1401690&view=ms.vss-test-web.build-test-results-tab&runId=4352544&resultId=100000&paneView=debug and https://identitydivision.visualstudio.com/Engineering/_build/results?buildId=1401664&view=logs&s=60296c01-192d-58d3-82b8-da4d468e44bd Fixes [AB#3110184](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3110184) --- changelog.txt | 1 + .../crypto/AndroidWrappedKeyLoader.java | 35 ++++++++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/changelog.txt b/changelog.txt index 2b72b69221..c90191cd72 100644 --- a/changelog.txt +++ b/changelog.txt @@ -5,6 +5,7 @@ vNext - [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) +- [MINOR] Replace Deprecated Keystore API for Android 28+ (#2558) Version 18.2.2 ---------- diff --git a/common/src/main/java/com/microsoft/identity/common/crypto/AndroidWrappedKeyLoader.java b/common/src/main/java/com/microsoft/identity/common/crypto/AndroidWrappedKeyLoader.java index b7609218a0..019d108b97 100644 --- a/common/src/main/java/com/microsoft/identity/common/crypto/AndroidWrappedKeyLoader.java +++ b/common/src/main/java/com/microsoft/identity/common/crypto/AndroidWrappedKeyLoader.java @@ -26,6 +26,8 @@ import android.content.Context; import android.os.Build; import android.security.KeyPairGeneratorSpec; +import android.security.keystore.KeyGenParameterSpec; +import android.security.keystore.KeyProperties; import androidx.annotation.RequiresApi; @@ -44,7 +46,9 @@ import java.security.KeyStore; import java.security.spec.AlgorithmParameterSpec; import java.util.Calendar; +import java.util.Date; import java.util.Locale; +import java.util.concurrent.TimeUnit; import javax.crypto.SecretKey; import javax.security.auth.x500.X500Principal; @@ -269,12 +273,13 @@ public void deleteSecretKeyFromStorage() throws ClientException { /** * Generate a self-signed cert and derive an AlgorithmParameterSpec from that. * This is for the key to be generated in {@link KeyStore} via {@link KeyPairGenerator} + * Note : This is now only for API level < 28 * * @param context an Android {@link Context} object. * @return a {@link AlgorithmParameterSpec} for the keystore key (that we'll use to wrap the secret key). */ @RequiresApi(api = Build.VERSION_CODES.JELLY_BEAN_MR2) - private static AlgorithmParameterSpec getSpecForKeyStoreKey(@NonNull final Context context, + private static AlgorithmParameterSpec getLegacySpecForKeyStoreKey(@NonNull final Context context, @NonNull final String alias) { // Generate a self-signed cert. final String certInfo = String.format(Locale.ROOT, "CN=%s, OU=%s", @@ -295,6 +300,34 @@ private static AlgorithmParameterSpec getSpecForKeyStoreKey(@NonNull final Conte .build(); } + /** + * Generate a self-signed cert and derive an AlgorithmParameterSpec from that. + * This is for the key to be generated in {@link KeyStore} via {@link KeyPairGenerator} + * + * @param context an Android {@link Context} object. + * @return a {@link AlgorithmParameterSpec} for the keystore key (that we'll use to wrap the secret key). + */ + private static AlgorithmParameterSpec getSpecForKeyStoreKey(@NonNull final Context context, @NonNull final String alias) { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.P) { + return getLegacySpecForKeyStoreKey(context, alias); + } else { + final String certInfo = String.format(Locale.ROOT, "CN=%s, OU=%s", + alias, + context.getPackageName()); + final int certValidYears = 100; + int purposes = KeyProperties.PURPOSE_WRAP_KEY | KeyProperties.PURPOSE_ENCRYPT | KeyProperties.PURPOSE_DECRYPT; + return new KeyGenParameterSpec.Builder(alias, purposes) + .setCertificateSubject(new X500Principal(certInfo)) + .setCertificateSerialNumber(BigInteger.ONE) + .setCertificateNotBefore(new Date()) + .setCertificateNotAfter(new Date(System.currentTimeMillis() + TimeUnit.DAYS.toMillis(365 * certValidYears))) + .setKeySize(2048) + .setDigests(KeyProperties.DIGEST_SHA256, KeyProperties.DIGEST_SHA512) + .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_RSA_PKCS1) + .build(); + } + } + /** * Get the file that stores the wrapped key. */