Native auth safe logging test (#2092)

Yuki-YuXin · web-flow · commit c207fccf5281 · 2024-05-14T10:11:41.000+01:00
### Comments and Changes Summary:

- Setup external logger and remove it for every test under `@Before `
and `@After`.
- Use Regex rules to filter the log message under `verify().log()`.
There are two pools for the regex rules: `sensitivePIIMessages` - items
should not exist in the log regardless of allowPII value;
`permittedPIIMessages `- items should not exist in the log when
`allowPII=false` but can exist when `allowPII=true`
- Use `@ParameterizedRobolectricTestRunner.Parameters` in order to test
both `allow=false &amp; allowPII=true` for
`RunWith(ParameterizedRobolectricTestRunner::class)
NativeAuthPublicClientApplicationKotlinTest(private val allowPII:
Boolean)`
- `verifyLogDoesNotContain` checks `message ` and 'containsPII'.
`verifyLogDoesNotContain ` checks `message`.
diff --git a/msal/src/main/java/com/microsoft/identity/client/Logger.java b/msal/src/main/java/com/microsoft/identity/client/Logger.java
@@ -175,6 +175,11 @@ public void log(String tag, com.microsoft.identity.common.internal.logging.Logge
         mExternalLogger = externalLogger;
     }
 
+    public synchronized void removeExternalLogger() {
+        mExternalLogger = null;
+    }
+
+
     /**
      * Enable/Disable the Android logcat logging. By default, the sdk enables it.
      *
diff --git a/msal/src/test/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplicationJavaTest.java b/msal/src/test/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplicationJavaTest.java
@@ -27,6 +27,7 @@
 import android.app.Activity;
 import android.content.Context;
 
+import com.microsoft.identity.client.ILoggerCallback;
 import com.microsoft.identity.client.Logger;
 import com.microsoft.identity.client.PublicClientApplication;
 import com.microsoft.identity.client.e2e.shadows.ShadowAndroidSdkStorageEncryptionManager;
@@ -74,6 +75,7 @@
 import com.microsoft.identity.common.java.util.ResultFuture;
 import com.microsoft.identity.internal.testutils.TestUtils;
 import com.microsoft.identity.nativeauth.statemachine.states.SignUpPasswordRequiredState;
+import com.microsoft.identity.nativeauth.utils.LoggerCheckHelper;
 
 import org.junit.After;
 import org.junit.AfterClass;
@@ -82,13 +84,17 @@
 import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mock;
 import org.mockito.Mockito;
-import org.robolectric.RobolectricTestRunner;
+import org.mockito.MockitoAnnotations;
+import org.robolectric.ParameterizedRobolectricTestRunner;
 import org.robolectric.annotation.Config;
 import org.robolectric.annotation.LooperMode;
 
 import java.io.File;
 import java.lang.reflect.InvocationTargetException;
+import java.util.Arrays;
+import java.util.Collection;
 import java.util.UUID;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
@@ -106,7 +112,7 @@
 import static org.mockito.Mockito.spy;
 import static org.robolectric.annotation.LooperMode.Mode.LEGACY;
 
-@RunWith(RobolectricTestRunner.class)
+@RunWith(ParameterizedRobolectricTestRunner.class)
 @LooperMode(LEGACY)
 @Config(shadows = {ShadowAndroidSdkStorageEncryptionManager.class})
 public class NativeAuthPublicClientApplicationJavaTest extends PublicClientApplicationAbstractTest {
@@ -115,11 +121,20 @@ public class NativeAuthPublicClientApplicationJavaTest extends PublicClientAppli
     private IPlatformComponents components;
     private Activity activity;
     private INativeAuthPublicClientApplication application;
+    private LoggerCheckHelper loggerCheckHelper;
     private final String username = "user@email.com";
     private final String invalidUsername = "invalidUsername";
     private final char[] password = "verySafePassword".toCharArray();
     private final String code = "1234";
     private final String emptyString = "";
+    private final boolean allowPII;
+
+    public NativeAuthPublicClientApplicationJavaTest(boolean allowPII) {
+        this.allowPII = allowPII;
+    }
+
+    @Mock
+    private ILoggerCallback externalLogger;
 
 
     @Override
@@ -137,21 +152,26 @@ public static void tearDownClass() {
         setUseMockApiForNativeAuth(false);
     }
 
+    @ParameterizedRobolectricTestRunner.Parameters
+    public static Collection<Boolean> data() {
+        return Arrays.asList(true, false);
+    }
+
     @Before
     public void setup() {
+        MockitoAnnotations.initMocks(this);
         context = ApplicationProvider.getApplicationContext();
         components = AndroidPlatformComponentsFactory.createFromContext(context);
         activity = Mockito.mock(Activity.class);
+        loggerCheckHelper = new LoggerCheckHelper(externalLogger, allowPII);
         Mockito.when(activity.getApplicationContext()).thenReturn(context);
         setupPCA();
-        Logger.getInstance().setEnableLogcatLog(true);
-        Logger.getInstance().setEnablePII(true);
-        Logger.getInstance().setLogLevel(Logger.LogLevel.VERBOSE);
         CommandDispatcherHelper.clear();
     }
 
     @After
     public void cleanup() {
+        loggerCheckHelper.checkSafeLogging();
         AcquireTokenTestHelper.setAccount(null);
         // remove everything from cache after test ends
         TestUtils.clearCache(SHARED_PREFERENCES_NAME);
diff --git a/msal/src/test/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplicationKotlinTest.kt b/msal/src/test/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplicationKotlinTest.kt
@@ -25,15 +25,29 @@ package com.microsoft.identity.nativeauth
 import android.app.Activity
 import android.content.Context
 import androidx.test.core.app.ApplicationProvider
+import com.microsoft.identity.client.ILoggerCallback
+import com.microsoft.identity.client.Logger
 import com.microsoft.identity.client.PublicClientApplication
 import com.microsoft.identity.client.e2e.shadows.ShadowAndroidSdkStorageEncryptionManager
 import com.microsoft.identity.client.e2e.tests.PublicClientApplicationAbstractTest
 import com.microsoft.identity.client.e2e.utils.AcquireTokenTestHelper
 import com.microsoft.identity.client.exception.MsalClientException
 import com.microsoft.identity.client.exception.MsalException
+import com.microsoft.identity.common.components.AndroidPlatformComponentsFactory
+import com.microsoft.identity.common.internal.controllers.CommandDispatcherHelper
+import com.microsoft.identity.common.java.exception.BaseException
+import com.microsoft.identity.common.java.interfaces.IPlatformComponents
+import com.microsoft.identity.common.java.nativeauth.BuildValues
+import com.microsoft.identity.common.java.util.ResultFuture
+import com.microsoft.identity.common.nativeauth.MockApiEndpoint
+import com.microsoft.identity.common.nativeauth.MockApiResponseType
+import com.microsoft.identity.common.nativeauth.MockApiUtils.Companion.configureMockApi
+import com.microsoft.identity.internal.testutils.TestUtils
+import com.microsoft.identity.nativeauth.statemachine.errors.ErrorTypes
 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
@@ -48,20 +62,9 @@ import com.microsoft.identity.nativeauth.statemachine.results.SignInResult
 import com.microsoft.identity.nativeauth.statemachine.results.SignOutResult
 import com.microsoft.identity.nativeauth.statemachine.results.SignUpResendCodeResult
 import com.microsoft.identity.nativeauth.statemachine.results.SignUpResult
-import com.microsoft.identity.common.components.AndroidPlatformComponentsFactory
-import com.microsoft.identity.common.internal.controllers.CommandDispatcherHelper
-import com.microsoft.identity.common.nativeauth.MockApiEndpoint
-import com.microsoft.identity.common.nativeauth.MockApiResponseType
-import com.microsoft.identity.common.nativeauth.MockApiUtils.Companion.configureMockApi
-import com.microsoft.identity.common.java.exception.BaseException
-import com.microsoft.identity.common.java.interfaces.IPlatformComponents
-import com.microsoft.identity.common.java.nativeauth.BuildValues
-import com.microsoft.identity.common.java.util.ResultFuture
-import com.microsoft.identity.internal.testutils.TestUtils
-import com.microsoft.identity.nativeauth.statemachine.errors.ErrorTypes
 import com.microsoft.identity.nativeauth.statemachine.states.SignInContinuationState
+import com.microsoft.identity.nativeauth.utils.LoggerCheckHelper
 import com.microsoft.identity.nativeauth.utils.mockCorrelationId
-import com.microsoft.identity.nativeauth.statemachine.errors.SignInContinuationError
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.test.runTest
@@ -76,35 +79,55 @@ import org.junit.BeforeClass
 import org.junit.Ignore
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.ArgumentMatcher
+import org.mockito.ArgumentMatchers.anyBoolean
+import org.mockito.Mock
 import org.mockito.Mockito
+import org.mockito.MockitoAnnotations
+import org.mockito.kotlin.any
+import org.mockito.kotlin.argThat
+import org.mockito.kotlin.eq
 import org.mockito.kotlin.mock
+import org.mockito.kotlin.never
 import org.mockito.kotlin.spy
+import org.mockito.kotlin.verify
 import org.mockito.kotlin.whenever
-import org.robolectric.RobolectricTestRunner
+import org.robolectric.ParameterizedRobolectricTestRunner
 import org.robolectric.annotation.Config
 import java.io.File
 import java.util.UUID
 import java.util.concurrent.ExecutionException
 import java.util.concurrent.TimeUnit
 import java.util.concurrent.TimeoutException
 
+
 @ExperimentalCoroutinesApi
-@RunWith(RobolectricTestRunner::class)
+@RunWith(ParameterizedRobolectricTestRunner::class)
 @Config(shadows = [ShadowAndroidSdkStorageEncryptionManager::class])
-class NativeAuthPublicClientApplicationKotlinTest : PublicClientApplicationAbstractTest() {
+class NativeAuthPublicClientApplicationKotlinTest(private val allowPII: Boolean) : PublicClientApplicationAbstractTest() {
     private lateinit var context: Context
     private lateinit var components: IPlatformComponents
     private lateinit var activity: Activity
     private lateinit var application: INativeAuthPublicClientApplication
+    private lateinit var loggerCheckHelper: LoggerCheckHelper
     private val username = "user@email.com"
     private val invalidUsername = "invalidUsername"
     private val password = "verySafePassword".toCharArray()
     private val code = "1234"
     private val emptyString = ""
 
+    @Mock
+    private lateinit var externalLogger: ILoggerCallback
+
     override fun getConfigFilePath() = "src/test/res/raw/native_auth_native_only_test_config.json"
 
     companion object {
+        @JvmStatic
+        @ParameterizedRobolectricTestRunner.Parameters
+        fun data(): Collection<Boolean> {
+            return listOf(true, false)
+        }
+
         @BeforeClass
         @JvmStatic
         fun setupClass() {
@@ -120,16 +143,19 @@ class NativeAuthPublicClientApplicationKotlinTest : PublicClientApplicationAbstr
 
     @Before
     override fun setup() {
+        MockitoAnnotations.initMocks(this)
         context = ApplicationProvider.getApplicationContext()
         components = AndroidPlatformComponentsFactory.createFromContext(context)
         activity = Mockito.mock(Activity::class.java)
+        loggerCheckHelper = LoggerCheckHelper(externalLogger, allowPII)
         whenever(activity.applicationContext).thenReturn(context)
         setupPCA()
         CommandDispatcherHelper.clear()
     }
 
     @After
     fun cleanup() {
+        loggerCheckHelper.checkSafeLogging()
         AcquireTokenTestHelper.setAccount(null)
         // remove everything from cache after test ends
         TestUtils.clearCache(SHARED_PREFERENCES_NAME)
@@ -424,6 +450,7 @@ class NativeAuthPublicClientApplicationKotlinTest : PublicClientApplicationAbstr
         )
         val result = continuationTokenState.signIn(scopes = null)
         assertTrue(result is SignInContinuationError)
+
     }
 
     /**
diff --git a/msal/src/test/java/com/microsoft/identity/nativeauth/utils/LoggerCheckHelper.kt b/msal/src/test/java/com/microsoft/identity/nativeauth/utils/LoggerCheckHelper.kt
@@ -0,0 +1,89 @@
+package com.microsoft.identity.nativeauth.utils
+
+import com.microsoft.identity.client.ILoggerCallback
+import com.microsoft.identity.client.Logger
+import org.mockito.ArgumentMatcher
+import org.mockito.ArgumentMatchers
+import org.mockito.kotlin.any
+import org.mockito.kotlin.argThat
+import org.mockito.kotlin.eq
+import org.mockito.kotlin.never
+import org.mockito.kotlin.verify
+
+class LoggerCheckHelper(private val externalLogger: ILoggerCallback, private val allowPII: Boolean) {
+
+    private val sensitivePIIMessages = listOf(
+        """(?<![\[\(])["]password["][:=]?(?![\]\)\}])""",  // '"password":' '"password"=' exclude 'password' '"challengeType":["password"]' '"challenge_type":"password"}'
+        """(?<![\s\?\(])(code)[:=]""",  // 'code:' 'code=' exclude 'codeLength' 'error?code'
+        """(?<![\(])continuationToken[:=]""",
+        """(?<![\(])attributes[:=]""",
+        """(?i)\b(accessToken|access_token)[:=]""", // access_token, accessToken
+        """(?i)\b(refreshToken|refresh_token)[:=]""",
+        """(?i)\b(idToken|id_token)[:=]""",
+        """(?i)\b(continuation_token)[:=]""",
+        """^[A-Za-z0-9_-]+\\.[A-Za-z0-9_-]+\\.[A-Za-z0-9_-]+$""" // JWT token
+    )
+    private val permittedPIIMessages = listOf(
+        """(?<![\(])username[:=]""",
+        """(?i)\b(challengeTargetLabel|challenge_target_label)[:=]"""
+    )
+
+    init {
+        setupLogger()
+    }
+
+    private fun setupLogger() {
+        Logger.getInstance().setLogLevel(Logger.LogLevel.INFO)
+        Logger.getInstance().setEnablePII(allowPII)
+        Logger.getInstance().setExternalLogger(externalLogger)
+    }
+
+    private fun clearLogger() {
+        Logger.getInstance().removeExternalLogger()
+    }
+
+    fun checkSafeLogging() {
+        var allowList = listOf<String>()
+        val disableList: List<String>
+
+        if (allowPII) {
+            allowList = permittedPIIMessages
+            disableList = sensitivePIIMessages
+        } else {
+            disableList = sensitivePIIMessages + permittedPIIMessages
+        }
+
+        allowList.forEach { regex ->
+            verifyLogCouldContain(regex)
+        }
+        disableList.forEach { regex ->
+            verifyLogDoesNotContain(regex)
+        }
+
+        clearLogger()
+    }
+
+    private fun verifyLogDoesNotContain(regex: String) {
+        verify(externalLogger, never()).log(
+            any(),
+            any(),
+            argThat(RegexMatcher(regex)),
+            ArgumentMatchers.anyBoolean()
+        )
+    }
+
+    private fun verifyLogCouldContain(regex: String) {
+        verify(externalLogger, never()).log(
+            any(),
+            any(),
+            argThat(RegexMatcher(regex)),  // allowList items are logged but the containsPII should be true.
+            eq(false)
+        )
+    }
+
+    class RegexMatcher(private val regex: String) : ArgumentMatcher<String> {
+        override fun matches(argument: String?): Boolean {
+            return regex.toRegex().containsMatchIn(argument ?: "")
+        }
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -175,6 +175,11 @@ public void log(String tag, com.microsoft.identity.common.internal.logging.Logge`
`175`	`175`	`mExternalLogger = externalLogger;`
`176`	`176`	`}`
`177`	`177`
	`178`	`+ public synchronized void removeExternalLogger() {`
	`179`	`+ mExternalLogger = null;`
	`180`	`+ }`
	`181`	`+`
	`182`	`+`
`178`	`183`	`/**`
`179`	`184`	`* Enable/Disable the Android logcat logging. By default, the sdk enables it.`
`180`	`185`	`*`