Skip to content

Auth integration with App Multicast Delegate #8422

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.

Sign up for GitHub

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

Closed
maksymmalyhin wants to merge 8 commits into from
Closed

Auth integration with App Multicast Delegate #8422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d65224d
Auth: use MulticastAppDelegate if available
maksymmalyhin Feb 23, 2021
f02c740
Auth test WIP
maksymmalyhin Mar 17, 2021
9e4f97b
Merge remote-tracking branch 'origin/master' into mm/auth-app-delegate
maksymmalyhin Jul 16, 2021
9275ef5
Merge branch 'mm/auth-app-delegate' of github.com:firebase/firebase-i…
maksymmalyhin Jul 16, 2021
fbf3143
fixes
maksymmalyhin Jul 16, 2021
51d4e96
WIP
maksymmalyhin Jul 20, 2021
0e27b25
Merge branch 'master' into mm/auth-app-delegate
maksymmalyhin Jul 20, 2021
218937b
Fix import for SPM and Cocoapods
maksymmalyhin Jul 20, 2021
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
1 change: 1 addition & 0 deletions FirebaseAuth.podspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ supports email and password accounts, as well as several 3rd party authenticatio
s.dependency 'GoogleUtilities/AppDelegateSwizzler', '~> 7.4'
s.dependency 'GoogleUtilities/Environment', '~> 7.4'
s.dependency 'GTMSessionFetcher/Core', '~> 1.5'
s.dependency 'GoogleMulticastAppDelegate', '~> 7.5'

s.test_spec 'unit' do |unit_tests|
unit_tests.scheme = { :code_coverage => true }
Expand Down
24 changes: 21 additions & 3 deletions FirebaseAuth/Sources/Auth/FIRAuth.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,14 @@
#import "FirebaseAuth/Sources/SystemService/FIRAuthAppCredentialManager.h"
#import "FirebaseAuth/Sources/SystemService/FIRAuthNotificationManager.h"
#import "FirebaseAuth/Sources/Utilities/FIRAuthURLPresenter.h"
#endif

#if SWIFT_PACKAGE
Copy link
Member

@paulb777 paulb777 Jul 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Structure public headers as documented in https://github.com/firebase/firebase-ios-sdk/blob/master/HeadersImports.md#header-file-types-and-locations---for-header-file-creators to avoid need for SPM/CocoaPods specific imports.

Copy link
Contributor Author

@maksymmalyhin maksymmalyhin Jul 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, not sure I understand how it's applicable here because there are no headers in the lib currently, but only a single Swift file. But probably it won't be relevant any more after google/GoogleUtilities#50 (comment) since we will have to re-implement the multicast delegate in Objective-C.

Copy link
Member

@paulb777 paulb777 Jul 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right. I made this comment before I saw the implementation was Swift.

Copy link
Contributor Author

@maksymmalyhin maksymmalyhin Aug 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: Validate in Firebase

@import GoogleUtilities_MulticastAppDelegate;
#else // SWIFT_PACKAGE
#import <GoogleMulticastAppDelegate/GoogleMulticastAppDelegate-Swift.h>
#endif // SWIFT_PACKAGE

#endif // TARGET_OS_IOS

NS_ASSUME_NONNULL_BEGIN

Expand Down Expand Up @@ -482,7 +489,14 @@ - (nullable instancetype)initWithAPIKey:(NSString *)APIKey appName:(NSString *)a
}
UIApplication *application = [applicationClass sharedApplication];

[GULAppDelegateSwizzler proxyOriginalDelegateIncludingAPNSMethods];
// Use Multicast App Delegate when available but fallback to App Delegate Swizzler for backward compatibility.
id<GULMulticastAppDelegateProtocol> multicastDelegate =
Copy link
Member

@paulb777 paulb777 Jul 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this in iOS specific code?

Copy link
Member

@ryanwilson ryanwilson Jul 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My guess is it's due to phone auth not being available on other platforms, but that was out of ease-of-implementation not any product decisions.

[GULMulticastAppDelegate installedMulticastDelegate];
if (multicastDelegate == nil) {
FIRLogInfo(kFIRLoggerAuth, @"I-AUT000018", @"Multicast App delegate was not detected. Make sure app events forwarding is configured manually for you app. <Add link to docs>");
Copy link
Member

@paulb777 paulb777 Jul 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: long line

[GULAppDelegateSwizzler proxyOriginalDelegateIncludingAPNSMethods];
}

[GULSceneDelegateSwizzler proxyOriginalSceneDelegate];
#endif // TARGET_OS_IOS

Expand Down Expand Up @@ -540,7 +554,11 @@ - (nullable instancetype)initWithAPIKey:(NSString *)APIKey appName:(NSString *)a
initWithApplication:application
appCredentialManager:strongSelf->_appCredentialManager];

[GULAppDelegateSwizzler registerAppDelegateInterceptor:strongSelf];
if (multicastDelegate != nil) {
[multicastDelegate addInterceptor:strongSelf];
} else {
[GULAppDelegateSwizzler registerAppDelegateInterceptor:strongSelf];
}
#if ((TARGET_OS_IOS || TARGET_OS_TV) && (__IPHONE_OS_VERSION_MAX_ALLOWED >= 130000))
if (@available(iOS 13, tvos 13, *)) {
[GULSceneDelegateSwizzler registerSceneDelegateInterceptor:strongSelf];
Expand Down
3 changes: 2 additions & 1 deletion Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ let package = Package(
.package(
name: "GoogleUtilities",
url: "https://github.com/google/GoogleUtilities.git",
"7.4.0" ..< "8.0.0"
.branch("mm/app-delegate-sample")
Copy link
Member

@paulb777 paulb777 Jul 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not merge to master.

),
.package(
name: "GTMSessionFetcher",
Expand Down Expand Up @@ -372,6 +372,7 @@ let package = Package(
dependencies: [
"FirebaseCore",
.product(name: "GULAppDelegateSwizzler", package: "GoogleUtilities"),
.product(name: "GULMulticastAppDelegate", package: "GoogleUtilities"),
.product(name: "GULEnvironment", package: "GoogleUtilities"),
.product(name: "GTMSessionFetcherCore", package: "GTMSessionFetcher"),
],
Expand Down