Skip to content

Commit

Permalink
fix psd filtering order bug -Android SDK
Browse files Browse the repository at this point in the history 
Summary: This diff brings PSD filtering before MACA rule matching. Since that step is done in AppEventsLogger, also needed to change the format in which custom params are consumed to the use of Bundle

Reviewed By: KylinChang

Differential Revision: D63471143

fbshipit-source-id: 70f5aa745d21fedce8e51c48e175d3b78727e39c
  • Loading branch information
@adityac-meta @facebook-github-bot
adityac-meta authored and facebook-github-bot committed Sep 26, 2024
1 parent e024423 commit 7ad0224
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 66 deletions.
9 changes: 2 additions & 7 deletions facebook-core/src/main/java/com/facebook/appevents/AppEvent.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,22 @@ import com.facebook.FacebookException
import com.facebook.LoggingBehavior
import com.facebook.appevents.eventdeactivation.EventDeactivationManager.processDeprecatedParameters
import com.facebook.appevents.integrity.IntegrityManager
import com.facebook.appevents.integrity.ProtectedModeManager.protectedModeIsApplied
import com.facebook.appevents.integrity.RedactedEventsManager
import com.facebook.appevents.integrity.SensitiveParamsManager.processFilterSensitiveParams
import com.facebook.appevents.internal.AppEventUtility.bytesToHex
import com.facebook.appevents.internal.Constants
import com.facebook.appevents.restrictivedatafilter.RestrictiveDataManager.processEvent
import com.facebook.appevents.restrictivedatafilter.RestrictiveDataManager.processParameters
import com.facebook.internal.Logger.Companion.log
import com.facebook.internal.Utility.logd
import org.json.JSONException
import org.json.JSONObject
import java.io.ObjectStreamException
import java.io.Serializable
import java.io.UnsupportedEncodingException
import java.security.MessageDigest
import java.security.NoSuchAlgorithmException
import java.util.Locale
import java.util.UUID
import org.json.JSONException
import org.json.JSONObject

@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
class AppEvent : Serializable {
Expand Down Expand Up @@ -141,9 +139,6 @@ class AppEvent : Serializable {
}
paramMap[key] = value.toString()
}
if (!protectedModeIsApplied(parameters)) {
processFilterSensitiveParams(paramMap as MutableMap<String, String?>, name)
}
IntegrityManager.processParameters(paramMap)
processParameters(paramMap as MutableMap<String, String?>, name)
processDeprecatedParameters(paramMap as MutableMap<String, String?>, name)
Expand Down
14 changes: 7 additions & 7 deletions facebook-core/src/main/java/com/facebook/appevents/AppEventsLoggerImpl.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ import com.facebook.appevents.AppEventQueue.getKeySet
import com.facebook.appevents.AppEventQueue.persistToDisk
import com.facebook.appevents.integrity.BlocklistEventsManager.isInBlocklist
import com.facebook.appevents.integrity.MACARuleMatchingManager
import com.facebook.appevents.integrity.ProtectedModeManager
import com.facebook.appevents.integrity.ProtectedModeManager.processParametersForProtectedMode
import com.facebook.appevents.integrity.SensitiveParamsManager.processFilterSensitiveParams
import com.facebook.appevents.internal.ActivityLifecycleTracker.getCurrentSessionGuid
import com.facebook.appevents.internal.ActivityLifecycleTracker.isInBackground
import com.facebook.appevents.internal.ActivityLifecycleTracker.startTracking
Expand All @@ -48,19 +50,14 @@ import com.facebook.internal.Utility.logd
import com.facebook.internal.Utility.stringsEqualOrEmpty
import com.facebook.internal.Validate.sdkInitialized
import com.facebook.internal.instrument.crashshield.AutoHandleExceptions
import org.json.JSONException
import org.json.JSONObject
import java.math.BigDecimal
import java.util.Currency
import java.util.UUID
import java.util.concurrent.Executor
import java.util.concurrent.ScheduledThreadPoolExecutor
import java.util.concurrent.TimeUnit
import kotlin.collections.HashSet
import kotlin.collections.MutableSet
import kotlin.collections.indices
import kotlin.collections.isNotEmpty
import kotlin.collections.toTypedArray
import org.json.JSONException
import org.json.JSONObject

@AutoHandleExceptions
internal class AppEventsLoggerImpl
Expand Down Expand Up @@ -324,6 +321,9 @@ internal constructor(activityName: String, applicationId: String?, accessToken:
}

try {
if (!ProtectedModeManager.protectedModeIsApplied(parameters)) {
processFilterSensitiveParams(parameters, eventName)
}
MACARuleMatchingManager.processParameters(parameters, eventName)
processParametersForProtectedMode(parameters)
val event =
Expand Down
6 changes: 5 additions & 1 deletion facebook-core/src/main/java/com/facebook/appevents/integrity/ProtectedModeManager.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,11 @@ object ProtectedModeManager {
parameters.putString(PROTECTED_MODE_IS_APPLIED_KEY, PROTECTED_MODE_IS_APPLIED_VALUE)
}

fun protectedModeIsApplied(parameters: Bundle): Boolean {
fun protectedModeIsApplied(parameters: Bundle?): Boolean {
if(parameters == null){
// if parameters aren't provided then the flag indicating core setup can't be assumed implicitly
return false
}
return parameters.containsKey(PROTECTED_MODE_IS_APPLIED_KEY) && parameters.get(PROTECTED_MODE_IS_APPLIED_KEY) == PROTECTED_MODE_IS_APPLIED_VALUE
}
}
17 changes: 8 additions & 9 deletions facebook-core/src/main/java/com/facebook/appevents/integrity/SensitiveParamsManager.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

package com.facebook.appevents.integrity

import android.os.Bundle
import com.facebook.FacebookSdk
import com.facebook.internal.FetchedAppSettingsManager
import com.facebook.internal.Utility.convertJSONArrayToHashSet
Expand All @@ -31,7 +32,7 @@ object SensitiveParamsManager {
@JvmStatic
fun enable() {
loadSensitiveParameters()
if (defaultSensitiveParameters.isNullOrEmpty() && sensitiveParameters.isNullOrEmpty()) {
if (defaultSensitiveParameters.isEmpty() && sensitiveParameters.isEmpty()) {
enabled = false
return
}
Expand Down Expand Up @@ -67,7 +68,7 @@ object SensitiveParamsManager {
*/
val sensitiveParamsScope = jsonObject.getString("key")
val sensitiveParams = jsonObject.getJSONArray("value")
sensitiveParamsScope?.let {
sensitiveParamsScope.let {
sensitiveParams?.let {
convertJSONArrayToHashSet(sensitiveParams)?.let {
if (sensitiveParamsScope.equals(DEFAULT_SENSITIVE_PARAMS_KEY)) {
Expand All @@ -87,18 +88,17 @@ object SensitiveParamsManager {
}

@JvmStatic
fun processFilterSensitiveParams(parameters: MutableMap<String, String?>, eventName: String) {
if (!enabled) {
fun processFilterSensitiveParams(parameters: Bundle?, eventName: String){
if (!enabled || parameters == null) {
return
}
if (defaultSensitiveParameters.isNullOrEmpty() && !sensitiveParameters.containsKey(eventName)) {
if (defaultSensitiveParameters.isEmpty() && !sensitiveParameters.containsKey(eventName)) {
return
}

val filteredParamsJSON = JSONArray()
try {
val sensitiveParamsForEvent = sensitiveParameters.get(key = eventName)
val keys: List<String> = ArrayList(parameters.keys)
val keys = ArrayList(parameters.keySet())
for (key in keys) {
if (shouldFilterOut(key, sensitiveParamsForEvent)) {
parameters.remove(key)
Expand All @@ -108,9 +108,8 @@ object SensitiveParamsManager {
} catch (e: Exception) {
/* swallow */
}

if (filteredParamsJSON.length() > 0) {
parameters[SENSITIVE_PARAMS_KEY] = filteredParamsJSON.toString()
parameters.putString(SENSITIVE_PARAMS_KEY, filteredParamsJSON.toString())
}
}

Expand Down
4 changes: 2 additions & 2 deletions facebook-core/src/main/java/com/facebook/internal/FeatureManager.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,11 @@ object FeatureManager {
SuggestedEvents(0x00010401),
IntelligentIntegrity(0x00010402),
ModelRequest(0x00010403),
ProtectedMode(0x00010404), /* filter out the params which are not supported legally */
FilterSensitiveParams(0x00010408), /* filter out the sensitive params */
MACARuleMatching(0x00010405),
ProtectedMode(0x00010404), /* filter out the params which are not supported legally */
BlocklistEvents(0x00010406), /* drop the events in the blocklist */
FilterRedactedEvents(0x00010407), /* replace the event name via the redaction string */
FilterSensitiveParams(0x00010408), /* filter out the sensitive params */
EventDeactivation(0x00010500),
OnDeviceEventProcessing(0x00010600),
OnDevicePostInstallEventProcessing(0x00010601),
Expand Down
116 changes: 76 additions & 40 deletions facebook-core/src/test/kotlin/com/facebook/appevents/integrity/SensitiveParamsManagerTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

package com.facebook.appevents.integrity

import android.os.Bundle
import com.facebook.FacebookPowerMockTestCase
import com.facebook.FacebookSdk
import com.facebook.appevents.integrity.SensitiveParamsManager.disable
Expand All @@ -18,6 +19,7 @@ import org.assertj.core.api.Assertions
import org.json.JSONArray
import org.json.JSONObject
import org.junit.After
import org.junit.Assert
import org.junit.Before
import org.junit.Test
import org.mockito.Mock
Expand Down Expand Up @@ -88,7 +90,7 @@ class SensitiveParamsManagerTest : FacebookPowerMockTestCase() {
processFilterSensitiveParams(mockInputParams, mockEventWithSensitiveParam)

Assertions.assertThat(mockInputParams.containsKey(filteredParamsKey)).isFalse
Assertions.assertThat(mockInputParams).isEqualTo(expectedFinalParamsWithoutChange)
assertEqual(mockInputParams, expectedFinalParamsWithoutChange)
}

@Test
Expand All @@ -99,23 +101,21 @@ class SensitiveParamsManagerTest : FacebookPowerMockTestCase() {
processFilterSensitiveParams(mockInputParams, mockEventWithSensitiveParam)

Assertions.assertThat(mockInputParams.containsKey(filteredParamsKey)).isFalse
Assertions.assertThat(mockInputParams).isEqualTo(expectedFinalParamsWithoutChange)
assertEqual(mockInputParams, expectedFinalParamsWithoutChange)
}

@Test
fun `test fetched sensitive params list is not null from the server and no params need to be filtered`() {
initMockFetchedAppSettings(mockSensitiveParamsFromServer)

var mockInputParamsWithoutSensitiveParams = HashMap<String, String?>()
mockInputParamsWithoutSensitiveParams[mockNonSensitiveParam] = mockNonSensitiveParamValue
val mockInputParamsWithoutSensitiveParams = Bundle()
mockInputParamsWithoutSensitiveParams.putString(mockNonSensitiveParam, mockNonSensitiveParamValue)

enable()
processFilterSensitiveParams(mockInputParamsWithoutSensitiveParams, mockEventNameWithoutSensitiveParams)

var expectedFinalParamsWithoutChange = mockInputParamsWithoutSensitiveParams.toMap()

Assertions.assertThat(mockInputParamsWithoutSensitiveParams.containsKey(filteredParamsKey)).isFalse
Assertions.assertThat(mockInputParamsWithoutSensitiveParams).isEqualTo(expectedFinalParamsWithoutChange)
assertEqual(mockInputParamsWithoutSensitiveParams, mockInputParamsWithoutSensitiveParams)
}

@Test
Expand All @@ -124,16 +124,17 @@ class SensitiveParamsManagerTest : FacebookPowerMockTestCase() {
enable()
processFilterSensitiveParams(mockInputParams, mockEventWithSensitiveParam)

var expectedParams = HashMap<String, String?>()
var filteredParams = JSONArray()
val filteredParams = JSONArray()
filteredParams.put(mockSensitiveParam3)
expectedParams[filteredParamsKey] = filteredParams.toString()
expectedParams[mockNonSensitiveParam] = mockNonSensitiveParamValue
expectedParams[mockSensitiveParam1] = null
expectedParams[mockSensitiveParam2] = null

val expectedParams = Bundle()
expectedParams.putString(filteredParamsKey, filteredParams.toString())
expectedParams.putString(mockNonSensitiveParam, mockNonSensitiveParamValue)
expectedParams.putString(mockSensitiveParam1, null)
expectedParams.putString(mockSensitiveParam2, null)

Assertions.assertThat(mockInputParams.containsKey(filteredParamsKey)).isTrue
Assertions.assertThat(mockInputParams).isEqualTo(expectedParams)
assertEqual(mockInputParams, expectedParams)
}

@Test
Expand All @@ -142,16 +143,17 @@ class SensitiveParamsManagerTest : FacebookPowerMockTestCase() {
enable()
processFilterSensitiveParams(mockInputParams, mockEventWithSensitiveParam)

var expectedParams = HashMap<String, String?>()
var filteredParams = JSONArray()
val filteredParams = JSONArray()
filteredParams.put(mockSensitiveParam3) /* default sensitive param */
filteredParams.put(mockSensitiveParam1) /* specific sensitive params */
filteredParams.put(mockSensitiveParam2) /* specific sensitive params */
expectedParams[filteredParamsKey] = filteredParams.toString()
expectedParams[mockNonSensitiveParam] = mockNonSensitiveParamValue

val expectedParams = Bundle()
expectedParams.putString(filteredParamsKey, filteredParams.toString())
expectedParams.putString(mockNonSensitiveParam, mockNonSensitiveParamValue)

Assertions.assertThat(mockInputParams.containsKey(filteredParamsKey)).isTrue
Assertions.assertThat(mockInputParams).isEqualTo(expectedParams)
assertEqual(mockInputParams, expectedParams)
}

@Test
Expand All @@ -160,16 +162,17 @@ class SensitiveParamsManagerTest : FacebookPowerMockTestCase() {
enable()
processFilterSensitiveParams(mockInputParams, mockEventWithSensitiveParam)

var expectedParams = HashMap<String, String?>()
var filteredParams = JSONArray()
val filteredParams = JSONArray()
filteredParams.put(mockSensitiveParam3) /* default sensitive param */
expectedParams[filteredParamsKey] = filteredParams.toString()
expectedParams[mockNonSensitiveParam] = mockNonSensitiveParamValue
expectedParams[mockSensitiveParam1] = null
expectedParams[mockSensitiveParam2] = null

val expectedParams = Bundle()
expectedParams.putString(filteredParamsKey, filteredParams.toString())
expectedParams.putString(mockNonSensitiveParam, mockNonSensitiveParamValue)
expectedParams.putString(mockSensitiveParam1, null)
expectedParams.putString(mockSensitiveParam2, null)

Assertions.assertThat(mockInputParams.containsKey(filteredParamsKey)).isTrue
Assertions.assertThat(mockInputParams).isEqualTo(expectedParams)
assertEqual(mockInputParams, expectedParams)
}

@Test
Expand All @@ -178,17 +181,48 @@ class SensitiveParamsManagerTest : FacebookPowerMockTestCase() {
enable()
processFilterSensitiveParams(mockInputParams, mockEventWithSensitiveParam)

var expectedParams = HashMap<String, String?>()
var filteredParams = JSONArray()
val filteredParams = JSONArray()
filteredParams.put(mockSensitiveParam1) /* specific sensitive params */
filteredParams.put(mockSensitiveParam2) /* specific sensitive params */

expectedParams[filteredParamsKey] = filteredParams.toString()
expectedParams[mockNonSensitiveParam] = mockNonSensitiveParamValue
expectedParams[mockSensitiveParam3] = null
val expectedParams = Bundle()
expectedParams.putString(filteredParamsKey, filteredParams.toString())
expectedParams.putString(mockNonSensitiveParam, mockNonSensitiveParamValue)
expectedParams.putString(mockSensitiveParam3, null)

Assertions.assertThat(mockInputParams.containsKey(filteredParamsKey)).isTrue
Assertions.assertThat(mockInputParams).isEqualTo(expectedParams)
assertEqual(mockInputParams, expectedParams)
}

private fun isEqual(mockBundle: Bundle?, expectedBundle: Bundle?): Boolean {
if (mockBundle == null && expectedBundle == null) {
return true
}
val s1 = mockBundle?.keySet() ?: return false
val s2 = expectedBundle?.keySet() ?: return false

if (!s1.equals(s2)) {
return false
}

for (s in s1) {
val v1 = mockBundle.get(s)
val v2 = expectedBundle.get(s)

// cant compare serialized lists directly present in _filteredKey
if(s.equals(filteredParamsKey) && v1.toString().length == v2.toString().length) {
continue
}

if (v1 != v2) {
return false
}
}
return true
}

private fun assertEqual(mockBundle: Bundle?, expectedBundle: Bundle?) {
Assert.assertTrue(isEqual(mockBundle, expectedBundle))
}

companion object {
Expand Down Expand Up @@ -219,8 +253,8 @@ class SensitiveParamsManagerTest : FacebookPowerMockTestCase() {
private lateinit var mockSensitiveParamsFromServerWithoutDefault: JSONArray /* specific sensitive params only */
private lateinit var mockSensitiveParamsFromServer: JSONArray /* specific sensitive params and default */

private lateinit var mockInputParams: HashMap<String, String?>
private lateinit var expectedFinalParamsWithoutChange: Map<String, String?>
private lateinit var mockInputParams: Bundle
private lateinit var expectedFinalParamsWithoutChange: Bundle

fun setupTestConfigs() {
emptyJSONArray = JSONArray()
Expand Down Expand Up @@ -248,13 +282,15 @@ class SensitiveParamsManagerTest : FacebookPowerMockTestCase() {
mockSensitiveParamsFromServer.put(mockSpecificSensitiveParams)
mockSensitiveParamsFromServer.put(mockDefaultSensitiveParams)

mockInputParams = HashMap()
mockInputParams[mockSensitiveParam1] = null
mockInputParams[mockSensitiveParam2] = null
mockInputParams[mockSensitiveParam3] = null
mockInputParams[mockNonSensitiveParam] = mockNonSensitiveParamValue
mockInputParams = Bundle()
mockInputParams.putString(mockSensitiveParam1, null)
mockInputParams.putString(mockSensitiveParam2, null)
mockInputParams.putString(mockSensitiveParam3, null)
mockInputParams.putString(mockNonSensitiveParam, mockNonSensitiveParamValue)

expectedFinalParamsWithoutChange = Bundle()
expectedFinalParamsWithoutChange.putAll(mockInputParams)

expectedFinalParamsWithoutChange = mockInputParams.toMap()
}
}
}

0 comments on commit 7ad0224

Please sign in to comment.