Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable standard Facebook Login on a Mac Catalyst app, avoiding fallback to the Limited Login flow #2522

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions FBSDKCoreKit/FBSDKCoreKit/FBProfilePictureView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,15 @@ public final class FBProfilePictureView: UIView {
// The login shim flow doesn't provide a valid access token to fetch the image, it
// leverages limited login implementation
if #available(iOS 14, *) {
#if targetEnvironment(macCatalyst)
// Mac Catalyst should be considered authorized since ATTrackingManager
// returns incorrect status on this platform
#else
let trackingAuthorizationStatus = ATTrackingManager.trackingAuthorizationStatus
if trackingAuthorizationStatus != .authorized {
return
}
#endif
}

updateImageWithAccessToken()
Expand Down
12 changes: 12 additions & 0 deletions FBSDKCoreKit/FBSDKCoreKit/Settings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -408,17 +408,29 @@ public final class Settings: NSObject, SettingsProtocol, SettingsLogging, _Clien
private var _advertisingTrackingStatusFromATT: AdvertisingTrackingStatus {
var advertisingTrackingStatus: AdvertisingTrackingStatus = .unspecified
if #available(iOS 14.0, *) {
#if targetEnvironment(macCatalyst)
// Mac Catalyst should always report as authorized since ATTrackingManager
// returns incorrect status on this platform
advertisingTrackingStatus = .allowed
#else
let status: ATTrackingManager.AuthorizationStatus = ATTrackingManager.trackingAuthorizationStatus
switch status {
case .authorized:
advertisingTrackingStatus = .allowed
case .denied, .restricted:
advertisingTrackingStatus = .disallowed
case .notDetermined:
#if targetEnvironment(macCatalyst)
// On Mac Catalyst, treat notDetermined as authorized since the platform
// doesn't properly support ATTrackingManager
advertisingTrackingStatus = .allowed
#else
advertisingTrackingStatus = .unspecified
#endif
@unknown default:
advertisingTrackingStatus = .unspecified
}
#endif
}
return advertisingTrackingStatus
}
Expand Down
20 changes: 20 additions & 0 deletions FBSDKCoreKit/FBSDKCoreKitTests/SettingsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,11 @@ final class SettingsTests: XCTestCase {
return
}
if #available(iOS 14, *) {
#if targetEnvironment(macCatalyst)
// On Mac Catalyst, tracking should always be considered authorized
XCTAssertTrue(settings.advertisingTrackingStatus == .allowed)
XCTAssertTrue(settings.isAdvertiserTrackingEnabled)
#else
let status: ATTrackingManager.AuthorizationStatus = ATTrackingManager.trackingAuthorizationStatus
switch status {
case .notDetermined:
Expand All @@ -303,6 +308,21 @@ final class SettingsTests: XCTestCase {
XCTAssertTrue(settings.advertisingTrackingStatus == .unspecified)
XCTAssertFalse(settings.isAdvertiserTrackingEnabled)
}
#endif
}
}

func testAdvertiserTrackingStatusOnMacCatalyst() {
if !_DomainHandler.sharedInstance().isDomainHandlingEnabled() {
return
}
if #available(iOS 14, *) {
#if targetEnvironment(macCatalyst)
// Test that .notDetermined is handled correctly on Mac Catalyst
let status: ATTrackingManager.AuthorizationStatus = .notDetermined
XCTAssertTrue(settings.advertisingTrackingStatus == .allowed)
XCTAssertTrue(settings.isAdvertiserTrackingEnabled)
#endif
}
}

Expand Down
43 changes: 43 additions & 0 deletions FBSDKLoginKit/FBSDKLoginKitTests/LoginManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,49 @@ final class LoginManagerTests: XCTestCase {

// MARK: Login Parameters

func testLoginTrackingEnabledLoginParamsOnMacCatalyst() throws {
#if targetEnvironment(macCatalyst)
let configuration = LoginConfiguration(
permissions: ["public_profile", "email"],
tracking: .enabled
)
let logger = try XCTUnwrap(
LoginManagerLogger(
loggingToken: "123",
tracking: .enabled
)
)
loginManager.logger = logger

internalUtility.stubbedAppURL = sampleURL
let parameters = try XCTUnwrap(
loginManager.logInParameters(
configuration: configuration,
loggingToken: "",
authenticationMethod: "sfvc_auth"
)
)

try validateCommonLoginParameters(parameters)
XCTAssertEqual(
parameters["response_type"],
"id_token,token_or_nonce,signed_request,graph_domain,user_token_nonce"
)
let scopes = parameters["scope"]?
.split(separator: ",")
.sorted()
.joined(separator: ",")
XCTAssertEqual(
scopes,
"email,openid,public_profile"
)
XCTAssertNotNil(parameters["nonce"])
// Mac Catalyst should not send tracking parameters since it's always considered authorized
XCTAssertNil(parameters["tp"], "Mac Catalyst should not send tracking parameters")
XCTAssertNil(parameters["is_limited_login_shim"], "Mac Catalyst should not use limited login shim")
#endif
}

func testLoginTrackingEnabledLoginParams() throws {
let configuration = LoginConfiguration(
permissions: ["public_profile", "email"],
Expand Down