Remove unnecessary isBrowserRequired in errors (#2009)

The existing native auth logic returns BrowserRequired error in more cases, compared to where the API can actually return it. This PR removes the unnecessary isBrowserRequired from errors for certain scenarios (sign in after sign up, resend code) Partner PR in common repo: AzureAD/microsoft-authentication-library-common-for-android#2298
AzureAD · Jan 25, 2024 · 1747b16 · 1747b16
1 parent 3e6354a
commit 1747b16
Show file tree

Hide file tree

Showing 10 changed files with 76 additions and 108 deletions.
diff --git a/common b/common
diff --git a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/errors/Error.kt b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/errors/Error.kt
@@ -24,11 +24,17 @@
 package com.microsoft.identity.nativeauth.statemachine.errors
 
 import com.microsoft.identity.nativeauth.statemachine.results.ResetPasswordResendCodeResult
+import com.microsoft.identity.nativeauth.statemachine.results.ResetPasswordResult
+import com.microsoft.identity.nativeauth.statemachine.results.ResetPasswordStartResult
 import com.microsoft.identity.nativeauth.statemachine.results.ResetPasswordSubmitCodeResult
 import com.microsoft.identity.nativeauth.statemachine.results.SignInResendCodeResult
+import com.microsoft.identity.nativeauth.statemachine.results.SignInResult
 import com.microsoft.identity.nativeauth.statemachine.results.SignInSubmitCodeResult
 import com.microsoft.identity.nativeauth.statemachine.results.SignUpResendCodeResult
+import com.microsoft.identity.nativeauth.statemachine.results.SignUpResult
+import com.microsoft.identity.nativeauth.statemachine.results.SignUpSubmitAttributesResult
 import com.microsoft.identity.nativeauth.statemachine.results.SignUpSubmitCodeResult
+import com.microsoft.identity.nativeauth.statemachine.results.SignUpSubmitPasswordResult
 
 /**
  * ErrorTypes class holds the possible error type values that are shared between the errors
@@ -79,7 +85,14 @@ open class Error(
     open var exception: Exception? = null,
     open val errorCodes: List<Int>? = null
 ) {
-    fun isBrowserRequired(): Boolean = this.errorType == ErrorTypes.BROWSER_REQUIRED
+}
+
+/**
+ * BrowserRequiredError error is an interface for all errors that could require a browser redirection in Native Auth.
+ * All error classes that can potentially return a browser redirection must implement this interface.
+ */
+interface BrowserRequiredError {
+    fun isBrowserRequired(): Boolean = (this as Error).errorType == ErrorTypes.BROWSER_REQUIRED
 }
 
 /**
@@ -105,7 +118,7 @@ class SubmitCodeError(
     override val errorCodes: List<Int>? = null,
     val subError: String? = null,
     override var exception: Exception? = null
-): SignInSubmitCodeResult, SignUpSubmitCodeResult, ResetPasswordSubmitCodeResult, Error(errorType = errorType, error = error, errorMessage= errorMessage, correlationId = correlationId, errorCodes = errorCodes, exception = exception)
+): BrowserRequiredError, SignInSubmitCodeResult, SignUpSubmitCodeResult, ResetPasswordSubmitCodeResult, Error(errorType = errorType, error = error, errorMessage= errorMessage, correlationId = correlationId, errorCodes = errorCodes, exception = exception)
 {
     fun isInvalidCode(): Boolean = this.errorType == ErrorTypes.INVALID_CODE
 }

diff --git a/...rc/main/java/com/microsoft/identity/nativeauth/statemachine/errors/ResetPasswordErrors.kt b/...rc/main/java/com/microsoft/identity/nativeauth/statemachine/errors/ResetPasswordErrors.kt
@@ -58,7 +58,7 @@ class ResetPasswordError(
     override val correlationId: String,
     override val errorCodes: List<Int>? = null,
     override var exception: Exception? = null
-): ResetPasswordResult, ResetPasswordStartResult, Error(errorType = errorType, error = error, errorMessage= errorMessage, correlationId = correlationId, errorCodes = errorCodes, exception = exception) {
+): ResetPasswordResult, ResetPasswordStartResult, BrowserRequiredError, Error(errorType = errorType, error = error, errorMessage= errorMessage, correlationId = correlationId, errorCodes = errorCodes, exception = exception) {
     fun isUserNotFound() : Boolean = this.errorType == ErrorTypes.USER_NOT_FOUND
 }
 

diff --git a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/errors/SignInErrors.kt b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/errors/SignInErrors.kt
@@ -35,7 +35,7 @@ open class SignInError(
     override val correlationId: String,
     override val errorCodes: List<Int>? = null,
     override var exception: Exception? = null
-): SignInResult, Error(errorType = errorType, error = error, errorMessage= errorMessage, correlationId = correlationId, errorCodes = errorCodes, exception = exception) {
+): SignInResult, BrowserRequiredError, Error(errorType = errorType, error = error, errorMessage= errorMessage, correlationId = correlationId, errorCodes = errorCodes, exception = exception) {
     fun isUserNotFound(): Boolean = this.errorType == ErrorTypes.USER_NOT_FOUND
 
     fun isInvalidCredentials(): Boolean = this.errorType == SignInErrorTypes.INVALID_CREDENTIALS
@@ -62,3 +62,23 @@ class SignInSubmitPasswordError(
 ): SignInSubmitPasswordResult, Error(errorType = errorType, error = error, errorMessage= errorMessage, correlationId = correlationId, errorCodes = errorCodes, exception = exception) {
     fun isInvalidCredentials(): Boolean = this.errorType == SignInErrorTypes.INVALID_CREDENTIALS
 }
+
+/**
+ * Sign in continuation error. The error has no utility methods
+ * and must be treated as unexpected. This error is produced by
+ * [com.microsoft.identity.nativeauth.INativeAuthPublicClientApplication.signIn]
+ * @param errorType the error type value of the error that occurred
+ * @param error the error returned by the authentication server.
+ * @param errorMessage the error message returned by the authentication server.
+ * @param correlationId a unique identifier for the request that can help in diagnostics.
+ * @param errorCodes a list of specific error codes returned by the authentication server.
+ * @param exception an internal unexpected exception that happened.
+ */
+open class SignInContinuationError(
+    override val error: String? = null,
+    override val errorMessage: String?,
+    // TODO: The parameter type of correlationId should be changed to String? after PBI https://identitydivision.visualstudio.com/Engineering/_workitems/edit/2774018 is completed
+    override val correlationId: String,
+    override val errorCodes: List<Int>? = null,
+    override var exception: Exception? = null
+): SignInResult, Error(errorType = null, error = error, errorMessage= errorMessage, correlationId = correlationId, errorCodes = errorCodes, exception = exception)
diff --git a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/errors/SignUpErrors.kt b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/errors/SignUpErrors.kt
@@ -76,7 +76,7 @@ open class SignUpError (
     override val correlationId: String,
     override val errorCodes: List<Int>? = null,
     override var exception: Exception? = null
-): SignUpResult, Error(errorType = errorType, error = error, errorMessage= errorMessage, correlationId = correlationId, errorCodes = errorCodes, exception = exception) {
+): SignUpResult, BrowserRequiredError, Error(errorType = errorType, error = error, errorMessage= errorMessage, correlationId = correlationId, errorCodes = errorCodes, exception = exception) {
 
     fun isUserAlreadyExists(): Boolean = this.errorType == SignUpErrorTypes.USER_ALREADY_EXISTS
 
@@ -109,7 +109,7 @@ class SignUpSubmitPasswordError (
     override val errorCodes: List<Int>? = null,
     val subError: String? = null,
     override var exception: Exception? = null
-): SignUpSubmitPasswordResult, Error(errorType = errorType, error = error, errorMessage= errorMessage, correlationId = correlationId, errorCodes = errorCodes, exception = exception) {
+): SignUpSubmitPasswordResult, BrowserRequiredError, Error(errorType = errorType, error = error, errorMessage= errorMessage, correlationId = correlationId, errorCodes = errorCodes, exception = exception) {
 
     fun isInvalidPassword(): Boolean = this.errorType == ErrorTypes.INVALID_PASSWORD
 }
@@ -132,7 +132,7 @@ class SignUpSubmitAttributesError (
     override val correlationId: String,
     override val errorCodes: List<Int>? = null,
     override var exception: Exception? = null
-): SignUpSubmitAttributesResult, Error(errorType = errorType, error = error, errorMessage= errorMessage, correlationId = correlationId, errorCodes = errorCodes, exception = exception) {
+): SignUpSubmitAttributesResult, BrowserRequiredError, Error(errorType = errorType, error = error, errorMessage= errorMessage, correlationId = correlationId, errorCodes = errorCodes, exception = exception) {
 
     fun isInvalidAttributes(): Boolean = this.errorType == SignUpErrorTypes.INVALID_ATTRIBUTES
 }
diff --git a/...rc/main/java/com/microsoft/identity/nativeauth/statemachine/states/ResetPasswordStates.kt b/...rc/main/java/com/microsoft/identity/nativeauth/statemachine/states/ResetPasswordStates.kt
@@ -233,25 +233,17 @@ class ResetPasswordCodeRequiredState internal constructor(
                     )
                 }
 
-                is INativeAuthCommandResult.Redirect -> {
-                    ResendCodeError(
-                        errorType = ErrorTypes.BROWSER_REQUIRED,
-                        error = result.error,
-                        errorMessage = result.errorDescription,
-                        correlationId = result.correlationId
-                    )
-                }
-
+                is INativeAuthCommandResult.Redirect,
                 is INativeAuthCommandResult.UnknownError -> {
                     Logger.warn(
                         TAG,
                         "Resend code received unexpected result: $result"
                     )
                     ResendCodeError(
-                        errorMessage = result.errorDescription,
-                        error = result.error,
-                        correlationId = result.correlationId,
-                        exception = result.exception
+                        errorMessage = (result as INativeAuthCommandResult.Error).errorDescription,
+                        error = (result as INativeAuthCommandResult.Error).error,
+                        correlationId = (result as INativeAuthCommandResult.Error).correlationId,
+                        exception = (result as INativeAuthCommandResult.UnknownError).exception
                     )
                 }
             }

diff --git a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/SignInStates.kt b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/SignInStates.kt
@@ -53,6 +53,7 @@ import com.microsoft.identity.common.java.util.StringUtil
 import com.microsoft.identity.common.java.nativeauth.util.checkAndWrapCommandResultType
 import com.microsoft.identity.nativeauth.statemachine.errors.ErrorTypes
 import com.microsoft.identity.nativeauth.statemachine.errors.ResendCodeError
+import com.microsoft.identity.nativeauth.statemachine.errors.SignInContinuationError
 import com.microsoft.identity.nativeauth.statemachine.errors.SignInError
 import com.microsoft.identity.nativeauth.statemachine.errors.SignInErrorTypes
 import com.microsoft.identity.nativeauth.statemachine.errors.SignInSubmitPasswordError
@@ -244,26 +245,17 @@ class SignInCodeRequiredState internal constructor(
                     )
                 }
 
-                is INativeAuthCommandResult.Redirect -> {
-                    ResendCodeError(
-                        errorType = ErrorTypes.BROWSER_REQUIRED,
-                        error = result.error,
-                        errorMessage = result.errorDescription,
-                        correlationId = result.correlationId
-                    )
-                }
-
-                is INativeAuthCommandResult.UnknownError -> {
+                is INativeAuthCommandResult.Redirect, is INativeAuthCommandResult.UnknownError -> {
                     Logger.warn(
                         TAG,
                         "Resend code received unexpected result: $result"
                     )
                     ResendCodeError(
-                        errorMessage = result.errorDescription,
-                        error = result.error,
-                        correlationId = result.correlationId,
-                        errorCodes = result.errorCodes,
-                        exception = result.exception
+                        errorMessage = (result as INativeAuthCommandResult.Error).errorDescription,
+                        error = (result as INativeAuthCommandResult.Error).error,
+                        correlationId = (result as INativeAuthCommandResult.Error).correlationId,
+                        errorCodes = (result as INativeAuthCommandResult.Error).errorCodes,
+                        exception = (result as INativeAuthCommandResult.UnknownError).exception
                     )
                 }
             }
@@ -379,25 +371,17 @@ class SignInPasswordRequiredState(
                             )
                         )
                     }
-                    is INativeAuthCommandResult.Redirect -> {
-                        SignInSubmitPasswordError(
-                            errorType = ErrorTypes.BROWSER_REQUIRED,
-                            error = result.error,
-                            errorMessage = result.errorDescription,
-                            correlationId = result.correlationId
-                        )
-                    }
-                    is INativeAuthCommandResult.UnknownError -> {
+                    is INativeAuthCommandResult.Redirect, is INativeAuthCommandResult.UnknownError -> {
                         Logger.warn(
                             TAG,
                             "Submit password received unexpected result: $result"
                         )
                         SignInSubmitPasswordError(
-                            errorMessage = result.errorDescription,
-                            error = result.error,
-                            correlationId = result.correlationId,
-                            errorCodes = result.errorCodes,
-                            exception = result.exception
+                            errorMessage = (result as INativeAuthCommandResult.Error).errorDescription,
+                            error = (result as INativeAuthCommandResult.Error).error,
+                            correlationId = (result as INativeAuthCommandResult.Error).correlationId,
+                            errorCodes = (result as INativeAuthCommandResult.Error).errorCodes,
+                            exception = (result as INativeAuthCommandResult.UnknownError).exception
                         )
                     }
                 }
@@ -473,7 +457,7 @@ class SignInContinuationState(
                     TAG,
                     "Sign in after sign up received unexpected result: continuationToken was null"
                 )
-                return@withContext SignInError(
+                return@withContext SignInContinuationError(
                     errorMessage = "Sign In is not available through this state, please use the standalone sign in methods (signInWithCode or signInWithPassword).",
                     error = "invalid_state",
                     correlationId = "UNSET",
@@ -497,27 +481,6 @@ class SignInContinuationState(
             val rawCommandResult = CommandDispatcher.submitSilentReturningFuture(command).get()
 
             return@withContext when (val result = rawCommandResult.checkAndWrapCommandResultType<SignInWithContinuationTokenCommandResult>()) {
-                is SignInCommandResult.CodeRequired -> {
-                    SignInResult.CodeRequired(
-                        nextState = SignInCodeRequiredState(
-                            continuationToken = result.continuationToken,
-                            scopes = scopes,
-                            config = config
-                        ),
-                        codeLength = result.codeLength,
-                        sentTo = result.challengeTargetLabel,
-                        channel = result.challengeChannel
-                    )
-                }
-                is SignInCommandResult.PasswordRequired -> {
-                    SignInResult.PasswordRequired(
-                        nextState = SignInPasswordRequiredState(
-                            continuationToken = result.continuationToken,
-                            scopes = scopes,
-                            config = config
-                        )
-                    )
-                }
                 is SignInCommandResult.Complete -> {
                     val authenticationResult =
                         AuthenticationResultAdapter.adapt(result.authenticationResult)
@@ -528,25 +491,18 @@ class SignInContinuationState(
                         )
                     )
                 }
-                is INativeAuthCommandResult.Redirect -> {
-                    SignInError(
-                        errorType = ErrorTypes.BROWSER_REQUIRED,
-                        error = result.error,
-                        errorMessage = result.errorDescription,
-                        correlationId = result.correlationId
-                    )
-                }
+                is INativeAuthCommandResult.Redirect,
                 is INativeAuthCommandResult.UnknownError -> {
                     Logger.warn(
                         TAG,
                         "Sign in after sign up received unexpected result: $result"
                     )
-                    SignInError(
-                        errorMessage = result.errorDescription,
-                        error = result.error,
-                        correlationId = result.correlationId,
-                        errorCodes = result.errorCodes,
-                        exception = result.exception
+                    SignInContinuationError(
+                        errorMessage = (result as INativeAuthCommandResult.Error).errorDescription,
+                        error = (result as INativeAuthCommandResult.Error).error,
+                        correlationId = (result as INativeAuthCommandResult.Error).correlationId,
+                        errorCodes = (result as INativeAuthCommandResult.Error).errorCodes,
+                        exception = (result as INativeAuthCommandResult.UnknownError).exception
                     )
                 }
             }

diff --git a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/SignUpStates.kt b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/SignUpStates.kt
@@ -273,25 +273,16 @@ class SignUpCodeRequiredState internal constructor(
                     )
                 }
 
-                is INativeAuthCommandResult.Redirect -> {
-                    ResendCodeError(
-                        errorType = ErrorTypes.BROWSER_REQUIRED,
-                        error = result.error,
-                        errorMessage = result.errorDescription,
-                        correlationId = result.correlationId
-                    )
-                }
-
-                is INativeAuthCommandResult.UnknownError -> {
+                is INativeAuthCommandResult.Redirect, is INativeAuthCommandResult.UnknownError -> {
                     Logger.warn(
                         TAG,
                         "Resend code received unexpected result: $result"
                     )
                     ResendCodeError(
-                        errorMessage = result.errorDescription,
-                        error = result.error,
-                        correlationId = result.correlationId,
-                        exception = result.exception
+                        errorMessage = (result as INativeAuthCommandResult.Error).errorDescription,
+                        error = (result as INativeAuthCommandResult.Error).error,
+                        correlationId = (result as INativeAuthCommandResult.Error).correlationId,
+                        exception = (result as INativeAuthCommandResult.UnknownError).exception
                     )
                 }
             }

diff --git a/...est/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplicationJavaTest.java b/...est/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplicationJavaTest.java
@@ -37,6 +37,7 @@
 import com.microsoft.identity.nativeauth.statemachine.errors.GetAccessTokenError;
 import com.microsoft.identity.nativeauth.statemachine.errors.ResetPasswordError;
 import com.microsoft.identity.nativeauth.statemachine.errors.ResetPasswordSubmitPasswordError;
+import com.microsoft.identity.nativeauth.statemachine.errors.SignInContinuationError;
 import com.microsoft.identity.nativeauth.statemachine.errors.SignInError;
 import com.microsoft.identity.nativeauth.statemachine.errors.SignUpError;
 import com.microsoft.identity.nativeauth.statemachine.errors.SignUpSubmitAttributesError;
@@ -1065,12 +1066,7 @@ public void onError(@NonNull BaseException exception) {
         state.signIn(null, callback);
 
         SignInResult result = resultFuture.get(10, TimeUnit.SECONDS);
-        assertTrue(result instanceof SignInError);
-
-        SignInError error = (SignInError)result;
-
-        assertFalse(error.isUserNotFound());
-        assertFalse(error.isBrowserRequired());
+        assertTrue(result instanceof SignInContinuationError);
     }
 
     /**
+1 −0		changelog.txt
+0 −2		common/src/test/java/com/microsoft/identity/common/nativeauth/util/CommandResultUtilTest.kt
+3 −0		common4j/src/main/com/microsoft/identity/common/java/jwt/AbstractJwtRequest.java
+7 −0		common4j/src/main/com/microsoft/identity/common/java/jwt/JwtRequestBody.java
+2 −2		common4j/src/main/com/microsoft/identity/common/java/nativeauth/controllers/results/SignInCommandResult.kt
+11 −10		common4j/src/main/com/microsoft/identity/common/java/telemetry/Telemetry.java