Skip to content

Commit

Permalink
Enable sensitive parameter filtering
Browse files Browse the repository at this point in the history
Summary:
Some advertisers pass "sensitive parameters" as a part of their events. We want to filter out these sensitive parameters on the client-side to minimize risk.

This diff enables and applies the ```SensitiveParamsManager``` in the app events flow.

Reviewed By: Nathaaaalie

Differential Revision: D54397251

fbshipit-source-id: 0557ab76815c08600dfbf69ab8cf870428496bf8
  • Loading branch information
ryantobinmeta authored and facebook-github-bot committed Mar 22, 2024
1 parent f0f0b7a commit 3b1e98e
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 8 deletions.
12 changes: 12 additions & 0 deletions FBSDKCoreKit/FBSDKCoreKit/AppEvents/FBSDKAppEvents.m
Expand Up @@ -984,6 +984,11 @@ - (void)fetchServerConfiguration:(FBSDKCodeBlock)callback
[self.redactedEventsManager enable];
}
}];
[self.featureChecker checkFeature:FBSDKFeatureFilterSensitiveParams completionBlock:^(BOOL enabled) {
if (enabled) {
[self.sensitiveParamsManager enable];
}
}];
if (@available(iOS 14.0, *)) {
__weak FBSDKAppEvents *weakSelf = self;
[self.featureChecker checkFeature:FBSDKFeatureATELogging completionBlock:^(BOOL enabled) {
Expand Down Expand Up @@ -1157,6 +1162,13 @@ - (void) logEvent:(FBSDKAppEventName)eventName
} @catch(NSException *exception) {}
}

BOOL isProtectedModeApplied = (self.protectedModeManager && [FBSDKProtectedModeManager isProtectedModeAppliedWithParameters:parameters]);
if (!isProtectedModeApplied && self.sensitiveParamsManager) {
@try {
parameters = [self.sensitiveParamsManager processParameters:parameters eventName:eventName];
} @catch(NSException *exception) {}
}

NSMutableDictionary<FBSDKAppEventParameterName, id> *eventDictionary = [NSMutableDictionary dictionaryWithDictionary:parameters ?: @{}];
[FBSDKTypeUtility dictionary:eventDictionary setObject:eventName forKey:FBSDKAppEventParameterNameEventName];
if (!eventDictionary[FBSDKAppEventParameterNameLogTime]) {
Expand Down
Expand Up @@ -9,8 +9,9 @@
import Foundation

@objc(FBSDKProtectedModeManager)
final class ProtectedModeManager: NSObject, _AppEventsParameterProcessing {
public final class ProtectedModeManager: NSObject, _AppEventsParameterProcessing {
private var isEnabled = false
private static let pmKey = AppEvents.ParameterName(rawValue: "pm")
private let standardParametersDefault: Set<String> = [
"_currency",
"_valueToSum",
Expand Down Expand Up @@ -159,7 +160,7 @@ final class ProtectedModeManager: NSObject, _AppEventsParameterProcessing {
serverConfigurationProvider: _ServerConfigurationManager.shared
)

func enable() {
public func enable() {
guard let dependencies = try? getDependencies() else {
return
}
Expand All @@ -178,7 +179,7 @@ final class ProtectedModeManager: NSObject, _AppEventsParameterProcessing {
isEnabled = true
}

@objc func processParameters(
@objc public func processParameters(
_ parameters: [AppEvents.ParameterName: Any]?,
eventName: AppEvents.Name?
) -> [AppEvents.ParameterName: Any]? {
Expand All @@ -195,10 +196,17 @@ final class ProtectedModeManager: NSObject, _AppEventsParameterProcessing {
params.removeValue(forKey: appEventsParameterName)
}
}
let pmKey = AppEvents.ParameterName(rawValue: "pm")
params[pmKey] = true
params[ProtectedModeManager.pmKey] = true
return params
}

@objc public static func isProtectedModeApplied(parameters: [AppEvents.ParameterName: Any]?) -> Bool {
guard let parameters else {
return false
}
return parameters.keys.contains(ProtectedModeManager.pmKey) &&
parameters[ProtectedModeManager.pmKey] as? Bool == true
}
}

extension ProtectedModeManager: DependentAsObject {
Expand Down
Expand Up @@ -910,6 +910,27 @@ final class AppEventsTests: XCTestCase {
)
}

func testLogEventProcessParametersWithSensitiveParamsManager() {
let parameters: [AppEvents.ParameterName: String] = [.init("key"): "value"]
appEvents.logEvent(
eventName,
valueToSum: NSNumber(value: purchaseAmount),
parameters: parameters,
isImplicitlyLogged: false,
accessToken: nil
)
XCTAssertEqual(
sensitiveParamsManager.capturedEventName,
eventName,
"AppEvents instance should submit the event name to the SensitiveParamsManager."
)
XCTAssertEqual(
sensitiveParamsManager.capturedParameters as? [AppEvents.ParameterName: String],
parameters,
"AppEvents instance should submit the parameters to the SensitiveParamsManager."
)
}

// MARK: - Test for log push notification

func testLogPushNotificationOpen() throws {
Expand Down Expand Up @@ -1513,7 +1534,7 @@ final class AppEventsTests: XCTestCase {

XCTAssertTrue(
redactedEventsManager.enabledWasCalled,
"Should enable blocklist events when the feature is enabled and the server configuration allows it"
"Should enable redacted events when the feature is enabled and the server configuration allows it"
)
}

Expand All @@ -1523,7 +1544,31 @@ final class AppEventsTests: XCTestCase {
serverConfigurationProvider.capturedCompletionBlock?(nil, nil)
XCTAssertTrue(
featureManager.capturedFeaturesContains(.filterRedactedEvents),
"Fetching a configuration should check if the BlocklistEvents feature is enabled"
"Fetching a configuration should check if the RedactedEvents feature is enabled"
)
}

func testEnablingSensitiveParamsFiltering() {
appEvents.fetchServerConfiguration(nil)
appEventsConfigurationProvider.firstCapturedBlock?()
let configuration = TestServerConfiguration(appID: name)

serverConfigurationProvider.capturedCompletionBlock?(configuration, nil)
featureManager.completeCheck(forFeature: .filterSensitiveParams, with: true)

XCTAssertTrue(
sensitiveParamsManager.enabledWasCalled,
"Should enable sensitive parameter filtering when the feature is enabled and the server configuration allows it"
)
}

func testFetchingConfigurationIncludingSensitiveParamsFiltering() {
appEvents.fetchServerConfiguration(nil)
appEventsConfigurationProvider.firstCapturedBlock?()
serverConfigurationProvider.capturedCompletionBlock?(nil, nil)
XCTAssertTrue(
featureManager.capturedFeaturesContains(.filterSensitiveParams),
"Fetching a configuration should check if the SensitiveParams feature is enabled"
)
}

Expand Down
Expand Up @@ -10,7 +10,7 @@

final class ProtectedModeManagerTests: XCTestCase {
let protectedModeManager = ProtectedModeManager()
let sampleParameters: [AppEvents.ParameterName: Any] = [
var sampleParameters: [AppEvents.ParameterName: Any] = [
AppEvents.ParameterName.pushCampaign: "fb_mobile_catalog_update",
AppEvents.ParameterName.productTitle: "Coffee 4",
AppEvents.ParameterName.logTime: 1686615025,
Expand All @@ -33,4 +33,15 @@ final class ProtectedModeManagerTests: XCTestCase {
"It should do the parameter filtering when enbaled"
)
}

func testIsProtectedModeApplied1() {
let isProtectedModeApplied = ProtectedModeManager.isProtectedModeApplied(parameters: sampleParameters)
XCTAssertFalse(isProtectedModeApplied)
}

func testIsProtectedModeApplied2() {
sampleParameters[AppEvents.ParameterName(rawValue: "pm")] = true
let isProtectedModeApplied = ProtectedModeManager.isProtectedModeApplied(parameters: sampleParameters)
XCTAssertTrue(isProtectedModeApplied)
}
}
Expand Up @@ -189,6 +189,10 @@ final class CoreKitConfiguratorTests: XCTestCase {
AppEvents.shared.internalUtility,
"AppEvents should not have an internal utility by default"
)
XCTAssertNil(
AppEvents.shared.sensitiveParamsManager,
"AppEvents should not have a sensitiveParamsManager by default"
)

configurator.performConfiguration()

Expand Down Expand Up @@ -268,6 +272,10 @@ final class CoreKitConfiguratorTests: XCTestCase {
AppEvents.shared.internalUtility === components.internalUtility,
"AppEvents should be configured with the internal utility"
)
XCTAssertTrue(
AppEvents.shared.sensitiveParamsManager === components.sensitiveParamsManager,
"AppEvents should be configured with sensitiveParamsManager"
)
}

func testConfiguringNonTVAppEvents() {
Expand Down
Expand Up @@ -12,6 +12,8 @@ final class TestSensitiveParamsManager: _AppEventsParameterProcessing {

var enabledWasCalled = false
var processParametersWasCalled = false
var capturedParameters: [AppEvents.ParameterName: Any]?
var capturedEventName: AppEvents.Name?

func enable() {
enabledWasCalled = true
Expand All @@ -22,6 +24,8 @@ final class TestSensitiveParamsManager: _AppEventsParameterProcessing {
eventName: AppEvents.Name?
) -> [AppEvents.ParameterName: Any]? {
processParametersWasCalled = true
capturedParameters = parameters
capturedEventName = eventName
return parameters
}
}

0 comments on commit 3b1e98e

Please sign in to comment.