Handle Receiver Callback Exception in CommandDispatcher interactive command

changelog.txt

@@ -1,5 +1,6 @@
 V.Next
 ---------
+- [PATCH] Handle Receiver Callback Exception in CommandDispatcher interactive command (#2305)
 - [MINOR] Add IpcStrategyWithBackup (#2301)
 - [PATCH] Fix MSAL Issue 1864 (#2280)
 - [MINOR] Add AccountManagerBackupIpcStrategyTargetingSpecificBrokerApp (#2294)

common/src/main/java/com/microsoft/identity/common/internal/controllers/LocalMSALController.java

@@ -28,38 +28,33 @@
 import androidx.annotation.WorkerThread;
 
 import com.microsoft.identity.common.internal.broker.BrokerData;
-import com.microsoft.identity.common.java.configuration.LibraryConfiguration;
-import com.microsoft.identity.common.java.controllers.CommandDispatcher;
-import com.microsoft.identity.common.java.eststelemetry.PublicApiId;
-import com.microsoft.identity.common.java.exception.ArgumentException;
-import com.microsoft.identity.common.java.exception.BrokerRequiredException;
-import com.microsoft.identity.common.java.exception.ClientException;
-import com.microsoft.identity.common.java.exception.ErrorStrings;
-import com.microsoft.identity.common.java.exception.ServiceException;
-import com.microsoft.identity.common.java.cache.ICacheRecord;
 import com.microsoft.identity.common.internal.commands.RefreshOnCommand;
-import com.microsoft.identity.common.java.commands.parameters.DeviceCodeFlowCommandParameters;
-import com.microsoft.identity.common.java.commands.parameters.GenerateShrCommandParameters;
-import com.microsoft.identity.common.java.commands.parameters.RemoveAccountCommandParameters;
-import com.microsoft.identity.common.java.dto.AccountRecord;
-import com.microsoft.identity.common.java.exception.UiRequiredException;
-import com.microsoft.identity.common.java.platform.DevicePoPUtils;
-import com.microsoft.identity.common.java.constants.OAuth2ErrorCode;
-import com.microsoft.identity.common.java.controllers.BaseController;
-import com.microsoft.identity.common.java.result.AcquireTokenResult;
-import com.microsoft.identity.common.java.result.GenerateShrResult;
-import com.microsoft.identity.common.java.result.LocalAuthenticationResult;
 import com.microsoft.identity.common.internal.telemetry.Telemetry;
 import com.microsoft.identity.common.internal.telemetry.events.ApiEndEvent;
 import com.microsoft.identity.common.internal.telemetry.events.ApiStartEvent;
-import com.microsoft.identity.common.java.util.ThreadUtils;
 import com.microsoft.identity.common.java.WarningType;
 import com.microsoft.identity.common.java.authorities.Authority;
 import com.microsoft.identity.common.java.authscheme.AbstractAuthenticationScheme;
 import com.microsoft.identity.common.java.authscheme.IPoPAuthenticationSchemeParams;
+import com.microsoft.identity.common.java.cache.ICacheRecord;
 import com.microsoft.identity.common.java.commands.parameters.CommandParameters;
+import com.microsoft.identity.common.java.commands.parameters.DeviceCodeFlowCommandParameters;
+import com.microsoft.identity.common.java.commands.parameters.GenerateShrCommandParameters;
 import com.microsoft.identity.common.java.commands.parameters.InteractiveTokenCommandParameters;
+import com.microsoft.identity.common.java.commands.parameters.RemoveAccountCommandParameters;
 import com.microsoft.identity.common.java.commands.parameters.SilentTokenCommandParameters;
+import com.microsoft.identity.common.java.configuration.LibraryConfiguration;
+import com.microsoft.identity.common.java.controllers.BaseController;
+import com.microsoft.identity.common.java.controllers.CommandDispatcher;
+import com.microsoft.identity.common.java.dto.AccountRecord;
+import com.microsoft.identity.common.java.eststelemetry.PublicApiId;
+import com.microsoft.identity.common.java.exception.ArgumentException;
+import com.microsoft.identity.common.java.exception.BrokerRequiredException;
+import com.microsoft.identity.common.java.exception.ClientException;
+import com.microsoft.identity.common.java.exception.ErrorStrings;
+import com.microsoft.identity.common.java.exception.ServiceException;
+import com.microsoft.identity.common.java.exception.UiRequiredException;
+import com.microsoft.identity.common.java.platform.DevicePoPUtils;
 import com.microsoft.identity.common.java.providers.RawAuthorizationResult;
 import com.microsoft.identity.common.java.providers.microsoft.microsoftsts.MicrosoftStsAuthorizationRequest;
 import com.microsoft.identity.common.java.providers.microsoft.microsoftsts.MicrosoftStsAuthorizationResponse;
@@ -68,22 +63,25 @@
 import com.microsoft.identity.common.java.providers.oauth2.AuthorizationResult;
 import com.microsoft.identity.common.java.providers.oauth2.AuthorizationStatus;
 import com.microsoft.identity.common.java.providers.oauth2.IAuthorizationStrategy;
-import com.microsoft.identity.common.java.providers.oauth2.IResult;
 import com.microsoft.identity.common.java.providers.oauth2.OAuth2Strategy;
 import com.microsoft.identity.common.java.providers.oauth2.OAuth2StrategyParameters;
 import com.microsoft.identity.common.java.providers.oauth2.OAuth2TokenCache;
 import com.microsoft.identity.common.java.providers.oauth2.TokenResult;
 import com.microsoft.identity.common.java.request.SdkType;
+import com.microsoft.identity.common.java.result.AcquireTokenResult;
+import com.microsoft.identity.common.java.result.GenerateShrResult;
+import com.microsoft.identity.common.java.result.LocalAuthenticationResult;
 import com.microsoft.identity.common.java.telemetry.TelemetryEventStrings;
-import com.microsoft.identity.common.java.util.ported.PropertyBag;
+import com.microsoft.identity.common.java.util.ResultFuture;
 import com.microsoft.identity.common.java.util.ResultUtil;
+import com.microsoft.identity.common.java.util.ThreadUtils;
+import com.microsoft.identity.common.java.util.ported.PropertyBag;
 import com.microsoft.identity.common.logging.Logger;
 
 import java.io.IOException;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Future;
 
 import lombok.EqualsAndHashCode;
 
@@ -95,6 +93,9 @@ public class LocalMSALController extends BaseController {
     @SuppressWarnings(WarningType.rawtype_warning)
     private IAuthorizationStrategy mAuthorizationStrategy = null;
 
+    @SuppressWarnings({WarningType.rawtype_warning, WarningType.unchecked_warning})
+    private ResultFuture<AuthorizationResult> mAuthorizationResultFuture = null;
+
     @SuppressWarnings(WarningType.rawtype_warning)
     private AuthorizationRequest mAuthorizationRequest = null;
 
@@ -230,12 +231,14 @@ private AuthorizationResult performAuthorizationRequest(@NonNull final OAuth2Str
         mAuthorizationRequest = getAuthorizationRequest(strategy, parameters);
 
         // Suppressing unchecked warnings due to casting of AuthorizationRequest to GenericAuthorizationRequest and AuthorizationStrategy to GenericAuthorizationStrategy in the arguments of call to requestAuthorization method
-        @SuppressWarnings(WarningType.unchecked_warning) final Future<AuthorizationResult> future = strategy.requestAuthorization(
+        mAuthorizationResultFuture = strategy.requestAuthorization(
                 mAuthorizationRequest,
                 mAuthorizationStrategy
         );
 
-        return future.get();
+        AuthorizationResult result = mAuthorizationResultFuture.get();
+        mAuthorizationResultFuture = null;
+        return result;
     }
 
     @Override
@@ -255,7 +258,17 @@ public void onFinishAuthorizationSession(int requestCode,
                         .put(TelemetryEventStrings.Key.REQUEST_CODE, String.valueOf(requestCode))
         );
 
-        mAuthorizationStrategy.completeAuthorization(requestCode, RawAuthorizationResult.fromPropertyBag(data));
+        try {
+            mAuthorizationStrategy.completeAuthorization(requestCode, RawAuthorizationResult.fromPropertyBag(data));
+        } catch (Exception e){
+            // If the future is somehow initialized and waiting, give the future an exception
+            if (mAuthorizationResultFuture != null && !mAuthorizationResultFuture.isDone()) {
+                mAuthorizationResultFuture.setException(e);
+            } else {
+                // Best Effort
+                throw e;
+            }
+        }
 
         Telemetry.emit(
                 new ApiEndEvent()

common/src/main/java/com/microsoft/identity/common/internal/providers/microsoft/activedirectoryfederationservices/ActiveDirectoryFederationServices2012R2OAuth2Strategy.java

@@ -43,7 +43,7 @@
 import com.microsoft.identity.common.java.providers.oauth2.TokenRequest;
 import com.microsoft.identity.common.java.providers.oauth2.TokenResponse;
 
-import java.util.concurrent.Future;
+import com.microsoft.identity.common.java.util.ResultFuture;
 
 /**
  * Azure Active Directory Federation Services 2012 R2 OAuth Strategy
@@ -70,7 +70,7 @@ public ActiveDirectoryFederationServices2012R2OAuth2Strategy(OAuth2Configuration
     // Suppressing unchecked warnings due to casting of AuthorizationRequest to GenericAuthorizationRequest and AuthorizationStrategy to GenericAuthorizationStrategy in the arguments of call to super class' method requestAuthorization
     @SuppressWarnings(WarningType.unchecked_warning)
     @Override
-    public Future<AuthorizationResult> requestAuthorization(AuthorizationRequest request, IAuthorizationStrategy IAuthorizationStrategy)
+    public ResultFuture<AuthorizationResult> requestAuthorization(AuthorizationRequest request, IAuthorizationStrategy IAuthorizationStrategy)
             throws ClientException {
         return super.requestAuthorization(request, IAuthorizationStrategy);
     }

common/src/main/java/com/microsoft/identity/common/internal/providers/microsoft/activedirectoryfederationservices/ActiveDirectoryFederationServices2016OAuth2Strategy.java

@@ -43,7 +43,7 @@
 import com.microsoft.identity.common.java.providers.oauth2.TokenRequest;
 import com.microsoft.identity.common.java.providers.oauth2.TokenResponse;
 
-import java.util.concurrent.Future;
+import com.microsoft.identity.common.java.util.ResultFuture;
 
 /**
  * Azure Active Directory Federation Services 2016 oAuth2 Strategy.
@@ -68,7 +68,7 @@ public ActiveDirectoryFederationServices2016OAuth2Strategy(OAuth2Configuration c
     // Suppressing unchecked warnings due to casting of AuthorizationRequest to GenericAuthorizationRequest and AuthorizationStrategy to GenericAuthorizationStrategy in the arguments of call to super class' method requestAuthorization
     @SuppressWarnings(WarningType.unchecked_warning)
     @Override
-    public Future<AuthorizationResult> requestAuthorization(AuthorizationRequest request, IAuthorizationStrategy IAuthorizationStrategy) throws ClientException {
+    public ResultFuture<AuthorizationResult> requestAuthorization(AuthorizationRequest request, IAuthorizationStrategy IAuthorizationStrategy) throws ClientException {
         return super.requestAuthorization(request, IAuthorizationStrategy);
     }
 

common/src/main/java/com/microsoft/identity/common/internal/providers/microsoft/azureactivedirectoryb2c/AzureActiveDirectoryB2COAuth2Strategy.java

@@ -43,7 +43,7 @@
 import com.microsoft.identity.common.java.providers.oauth2.TokenResponse;
 import com.microsoft.identity.common.java.providers.oauth2.TokenResult;
 
-import java.util.concurrent.Future;
+import com.microsoft.identity.common.java.util.ResultFuture;
 
 /**
  * Azure Active Directory B2C OAuth Strategy.
@@ -68,7 +68,7 @@ public AzureActiveDirectoryB2COAuth2Strategy(OAuth2Configuration config, OAuth2S
     // Suppressing unchecked warnings due to casting of AuthorizationRequest to GenericAuthorizationRequest and AuthorizationStrategy to GenericAuthorizationStrategy in the arguments of call to super class' method requestAuthorization
     @SuppressWarnings(WarningType.unchecked_warning)
     @Override
-    public Future<AuthorizationResult> requestAuthorization(AuthorizationRequest request, IAuthorizationStrategy authorizationStrategy) throws ClientException {
+    public ResultFuture<AuthorizationResult> requestAuthorization(AuthorizationRequest request, IAuthorizationStrategy authorizationStrategy) throws ClientException {
         return super.requestAuthorization(request, authorizationStrategy);
     }
 

common/src/main/java/com/microsoft/identity/common/internal/ui/browser/BrowserAuthorizationStrategy.java

@@ -33,20 +33,17 @@
 
 import com.microsoft.identity.common.internal.providers.oauth2.AndroidAuthorizationStrategy;
 import com.microsoft.identity.common.internal.providers.oauth2.AuthorizationActivityFactory;
-import com.microsoft.identity.common.java.ui.AuthorizationAgent;
 import com.microsoft.identity.common.java.WarningType;
 import com.microsoft.identity.common.java.exception.ClientException;
 import com.microsoft.identity.common.java.providers.RawAuthorizationResult;
 import com.microsoft.identity.common.java.providers.oauth2.AuthorizationRequest;
 import com.microsoft.identity.common.java.providers.oauth2.AuthorizationResult;
 import com.microsoft.identity.common.java.providers.oauth2.OAuth2Strategy;
-import com.microsoft.identity.common.java.ui.BrowserDescriptor;
+import com.microsoft.identity.common.java.ui.AuthorizationAgent;
 import com.microsoft.identity.common.java.util.ResultFuture;
 import com.microsoft.identity.common.logging.Logger;
 
 import java.net.URI;
-import java.util.List;
-import java.util.concurrent.Future;
 
 import static com.microsoft.identity.common.java.AuthenticationConstants.UIRequest.BROWSER_FLOW;
 
@@ -77,7 +74,7 @@ public void setBrowser(final Browser browser) {
     }
 
     @Override
-    public Future<AuthorizationResult> requestAuthorization(
+    public ResultFuture<AuthorizationResult> requestAuthorization(
             GenericAuthorizationRequest authorizationRequest,
             GenericOAuth2Strategy oAuth2Strategy)
             throws ClientException {

common/src/main/java/com/microsoft/identity/common/internal/ui/webview/EmbeddedWebViewAuthorizationStrategy.java

@@ -22,6 +22,8 @@
 // THE SOFTWARE.
 package com.microsoft.identity.common.internal.ui.webview;
 
+import static com.microsoft.identity.common.java.AuthenticationConstants.UIRequest.BROWSER_FLOW;
+
 import android.app.Activity;
 import android.content.Context;
 import android.content.Intent;
@@ -43,9 +45,6 @@
 import com.microsoft.identity.common.logging.Logger;
 
 import java.net.URI;
-import java.util.concurrent.Future;
-
-import static com.microsoft.identity.common.java.AuthenticationConstants.UIRequest.BROWSER_FLOW;
 
 /**
  * Serve as a class to do the OAuth2 auth code grant flow with Android embedded web view.
@@ -76,8 +75,8 @@ public EmbeddedWebViewAuthorizationStrategy(@NonNull Context applicationContext,
      * The activity result is set in Authorization.setResult() and passed to the onActivityResult() of the calling activity.
      */
     @Override
-    public Future<AuthorizationResult> requestAuthorization(GenericAuthorizationRequest authorizationRequest,
-                                                            GenericOAuth2Strategy oAuth2Strategy) throws ClientException {
+    public ResultFuture<AuthorizationResult> requestAuthorization(GenericAuthorizationRequest authorizationRequest,
+                                                                  GenericOAuth2Strategy oAuth2Strategy) throws ClientException {
         final String methodTag = TAG + ":requestAuthorization";
         mAuthorizationResultFuture = new ResultFuture<>();
         mOAuth2Strategy = oAuth2Strategy;

common4j/src/main/com/microsoft/identity/common/java/controllers/CommandDispatcher.java

@@ -43,9 +43,9 @@
 import com.microsoft.identity.common.java.BuildConfig;
 import com.microsoft.identity.common.java.WarningType;
 import com.microsoft.identity.common.java.commands.BaseCommand;
+import com.microsoft.identity.common.java.commands.DeviceCodeFlowAuthResultCommand;
 import com.microsoft.identity.common.java.commands.DeviceCodeFlowCommand;
 import com.microsoft.identity.common.java.commands.DeviceCodeFlowTokenResultCommand;
-import com.microsoft.identity.common.java.commands.DeviceCodeFlowAuthResultCommand;
 import com.microsoft.identity.common.java.commands.ICommandResult;
 import com.microsoft.identity.common.java.commands.InteractiveTokenCommand;
 import com.microsoft.identity.common.java.commands.SilentTokenCommand;
@@ -89,7 +89,6 @@
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
@@ -708,7 +707,7 @@ private static boolean eligibleToCache ( @NonNull final CommandResult commandRes
             }
         }
 
-        public static void beginInteractive ( final InteractiveTokenCommand command){
+        public static void beginInteractive (final InteractiveTokenCommand command) {
             final String methodName = ":beginInteractive";
             synchronized (sLock) {
 
@@ -752,10 +751,16 @@ public void run() {
 
                             EstsTelemetry.getInstance().emitApiId(command.getPublicApiId());
 
+                            final BaseException[] receiverException = new BaseException[1];
+
                             final LocalBroadcaster.IReceiverCallback resultReceiver = new LocalBroadcaster.IReceiverCallback() {
                                 @Override
                                 public void onReceive(@NonNull PropertyBag dataBag) {
-                                    completeInteractive(dataBag);
+                                    try {
+                                        completeInteractive(dataBag);
+                                    } catch (final Exception e) {
+                                        receiverException[0] = ExceptionAdapter.baseExceptionFromException(e);
+                                    }
                                 }
                             };
 
@@ -772,6 +777,12 @@ public void onReceive(@NonNull PropertyBag dataBag) {
 
                             LocalBroadcaster.INSTANCE.unregisterCallback(RETURN_AUTHORIZATION_REQUEST_RESULT);
 
+                            // If we received an exception during the receiver callback execution,
+                            // we should set that as the command result
+                            if (receiverException[0] != null) {
+                                commandResult = CommandResult.of(CommandResult.ResultStatus.ERROR, receiverException[0], correlationId);
+                            }
+
                             Logger.info(TAG + methodName,
                                     "Completed interactive request for correlation id : **" + correlationId +
                                             statusMsg(commandResult.getStatus().getLogStatus()));

common4j/src/main/com/microsoft/identity/common/java/providers/oauth2/IAuthorizationStrategy.java

@@ -26,7 +26,7 @@
 import com.microsoft.identity.common.java.exception.ClientException;
 import com.microsoft.identity.common.java.providers.RawAuthorizationResult;
 
-import java.util.concurrent.Future;
+import com.microsoft.identity.common.java.util.ResultFuture;
 
 import edu.umd.cs.findbugs.annotations.NonNull;
 
@@ -43,8 +43,8 @@ public interface IAuthorizationStrategy<
     /**
      * Perform the authorization request.
      */
-    Future<AuthorizationResult> requestAuthorization(GenericAuthorizationRequest authorizationRequest,
-                                                     GenericOAuth2Strategy oAuth2Strategy)
+    ResultFuture<AuthorizationResult> requestAuthorization(GenericAuthorizationRequest authorizationRequest,
+                                                           GenericOAuth2Strategy oAuth2Strategy)
             throws ClientException;
 
     /**