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

Jump to bottom

Add FirAuthUrlPresenter #222

Merged

protocol86 merged 16 commits into master from Add_FIRAuthURLPresenter

Sep 1, 2017

Contributor

protocol86 commented Aug 30, 2017

Adds FIRAuthURLPresenter which allows Firebase Auth to present a URL using SFSafariViewController. Support for UIWebView will be added in a later diff.

protocol86 added 2 commits

August 30, 2017 13:17


          Adds FIRAuthURLPResenter and FIRAuthUIDelegate

378ab15


          Introduces default UIDelegate

a466672

protocol86 requested a review from XiangtianDai

August 30, 2017 22:04

googlebot added the cla: yes label

paulb777 and others added 3 commits

August 30, 2017 15:06


          Update FirebaseCommunity to 0.1.1

b43d0cd


          removes line break

0f70bf0


          Encode the topic name when subscribing (#220)

e00eaa8

This should address the issue filed at: firebase/quickstart-ios#242, where a topic name containing a `%` character was failing. It turns out that the topic name was never being url-encoded.

XiangtianDai reviewed

View reviewed changes

AuthSamples/Samples.xcodeproj/project.pbxproj

@@ @@ -26,6 +26,7 @@ @@
 C975C71EE10B1304EBBEB2 /* Pods_SwiftSample.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = BC8C39EF1F42A0C750FF5186 /* Pods_SwiftSample.framework */; };
 C3F4E18627674CABE02AE /* Pods_EarlGreyTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = AEE2E563FADF8C3382956B4F /* Pods_EarlGreyTests.framework */; };
 AFFB52FF0FC4668D92F2E4 /* Pods_Sample.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = D5FE06BD9AA795DFBA9EFAAD /* Pods_Sample.framework */; };
+E04ACA41F565EA900788114 /* FIRAuthURLPresenterTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 7E04ACA31F565EA900788114 /* FIRAuthURLPresenterTests.m */; };

Contributor

XiangtianDai Aug 30, 2017

Please also update Example/Firebase.xcodeproj/project.pbxproj. And please be care not to break macOS build/test, because the new class only works on iOS.

Contributor Author

protocol86 Aug 31, 2017

done, thanks.

Example/Auth/Tests/FIRAuthURLPresenterTests.m Outdated

+                    [self expectationWithDescription:@"callbackMatcher callback"];
+                FIRAuthURLCallbackMatcher callbackMatcher = ^BOOL(NSURL *_Nonnull callbackURL) {
+                  XCTAssertEqualObjects(callbackURL, presenterURL);
+                  NSLog(@"callback matcher");

Contributor

XiangtianDai Aug 30, 2017

Please remove debug statements.

Contributor Author

protocol86 Aug 31, 2017

Done

Firebase/Auth/Source/FIRAuthDefaultUIDelegate.m Outdated

+                return self;
+              }
+              id<FIRAuthUIDelegate> FIRAuthUIDelegateWithViewController(UIViewController *viewController) {

Contributor

XiangtianDai Aug 30, 2017

I don't think we need this function. Just to call alloc] initWith... directly for all is internal.

Contributor Author

protocol86 Aug 31, 2017

Done

Firebase/Auth/Source/FIRAuthErrorUtils.m Outdated

+              /** @var kFIRAuthErrorMessageWebContextCancelled
+                  @brief Message for @c FIRAuthErrorCodeWebContextCancelled error code.
+               */
+              static NSString *const kFIRAuthErrorMessageWebContextCancelled = @"The user cancelled the sign-in "

Contributor

XiangtianDai Aug 30, 2017

It is not necessarily sign-in flow. How about "The interaction was cancelled by user."?

Contributor Author

protocol86 Aug 31, 2017

Done.

Firebase/Auth/Source/FIRAuthURLPresenter.h Outdated

+              // SFSafariViewController only exists in iOS 9+ SDKs.
+              #if __IPHONE_OS_VERSION_MAX_ALLOWED >= 90000
+              #define HAS_SAFARI_VIEW_CONTROLLER 1

Contributor

XiangtianDai Aug 30, 2017

We don't need this macro because we don't support Xcode 6.

Firebase/Auth/Source/FIRAuthURLPresenter.m Outdated

+                  _completion(nil, [FIRAuthErrorUtils webContextAlreadyPresentedErrorWithMessage:nil]);
+                  return;
+                }
+                SFSafariViewController *safariViewController = [[SFSafariViewController alloc] initWithURL:URL];

Contributor

XiangtianDai Aug 30, 2017

While it is fine that this won't work on iOS 8-, please make sure tests don't fail on iOS 8-.

Contributor Author

protocol86 Aug 31, 2017

Done, had to update unit tests.

Firebase/Auth/Source/FIRAuthURLPresenter.m Outdated

+              - (void)finishPresentationWithURL:(nullable NSURL *)URL
+                                          error:(nullable NSError *)error {
+                FIRAuthURLPresentationCompletion completion = _completion;
+                  void (^finishBlock)() = ^() {

Contributor

XiangtianDai Aug 30, 2017

-2 indent

Contributor Author

protocol86 Aug 31, 2017

Done.

paulb777 and others added 3 commits

August 30, 2017 17:40


          Merge pull request #223 from firebase/release-4.1.1

Update FirebaseCommunity to 0.1.1


          Changes based on comments

- Added new files to Example/Firebase.xcodeproj/project.pbxproj

- Removed typos.

- Simplified construction of dfault UIDelegate by removing one fuctions call.

- Improved text of “user cancellation error message”.

- Updated unit tests to run on iOS 8-.

- Removed unnecessary macro.


          Adds missing unit tests to the Example project and fixes one test cas…

0ce4297

…e. (#224)

XiangtianDai reviewed

View reviewed changes

Firebase/Auth/Source/FIRAuthURLPresenter.m Outdated

+                  SFSafariViewController *safariViewController = _safariViewController;
+                  _safariViewController = nil;
+                  if (safariViewController) {
+                    [safariViewController dismissViewControllerAnimated:YES completion:finishBlock];

Contributor

XiangtianDai Aug 31, 2017

I have a slight preference for the UIDelegate to dismiss the SFSVC here.

Firebase/Auth/Source/FIRAuthURLPresenter.m Outdated

+                _callbackMatcher = callbackMatcher;
+                _completion = completion;
+                // If a UIDelegate is not provided.
+                if (!UIDelegate) {

Contributor

XiangtianDai Aug 31, 2017

if (!UIDelegate) {
UIDelegate = [FIRAuthDefaultUIDelegate defaultUIDelegate];
}

Also, if we need to call dismiss on UIDelegate later, it needs to be saved as an iVar.

Contributor Author

protocol86 Aug 31, 2017

done

Firebase/Auth/Source/FIRAuthURLPresenter.m Outdated

+              - (void)safariViewControllerDidFinish:(SFSafariViewController *)controller {
+                if (controller == _safariViewController) {
+                  _safariViewController = nil;
+                  [self finishPresentationWithURL:nil

Contributor

XiangtianDai Aug 31, 2017

Please add a TODO to investigate if the callback is called too earlier, i.e., before SFSVC is completely closed, and handle it accordingly.

Contributor Author

protocol86 Aug 31, 2017

Done

Firebase/Auth/Source/FIRAuthDefaultUIDelegate.m

		@@ -0,0 +1,85 @@
		/*

Contributor

XiangtianDai Aug 31, 2017

Please add this file to the macOS exclusion in Auth's podspec so it is not compiled for macOS.

Contributor Author

protocol86 Aug 31, 2017

Done


          Addresses comments

baa139b

XiangtianDai approved these changes

View reviewed changes

protocol86 added 7 commits

August 31, 2017 15:55


          Adds FIRAuthURLPResenter and FIRAuthUIDelegate

852500a


          Introduces default UIDelegate

f950b57


          removes line break

c111134


          Changes based on comments

445512f

- Added new files to Example/Firebase.xcodeproj/project.pbxproj

- Removed typos.

- Simplified construction of dfault UIDelegate by removing one fuctions call.

- Improved text of “user cancellation error message”.

- Updated unit tests to run on iOS 8-.

- Removed unnecessary macro.


          Addresses comments

d53e6eb


          Addresses Comments

f701726


          Merge branch 'Add_FIRAuthURLPresenter' of https://github.com/firebase…

…/firebase-ios-sdk into Add_FIRAuthURLPresenter

# Conflicts:
#	Example/Auth/Tests/FIRGetProjectConfigRequestTests.m
#	Firebase/Auth/Source/FIRAuthURLPresenter.m

Contributor Author

protocol86 commented Aug 31, 2017

@XiangtianDai PTAL after merge.

XiangtianDai approved these changes

View reviewed changes

XiangtianDai added the api: auth label

protocol86 merged commit 36c20d9 into master

XiangtianDai added a commit that referenced this pull request


          Fixes macOS unit test failures introduced in #222 .

9fbb806

XiangtianDai added a commit that referenced this pull request


          Fixes macOS unit test failures introduced in #222 . (#229)

540e21c

protocol86 pushed a commit that referenced this pull request


          Fixes macOS unit test failures introduced in #222 . (#229)

efe5669

paulb777 deleted the Add_FIRAuthURLPresenter branch

November 8, 2017 20:28

minafarid pushed a commit to minafarid/firebase-ios-sdk that referenced this pull request


          Add FirAuthUrlPresenter (firebase#222)

bdb0088

* Adds FIRAuthURLPResenter and FIRAuthUIDelegate

minafarid pushed a commit to minafarid/firebase-ios-sdk that referenced this pull request


          Fixes macOS unit test failures introduced in firebase#222 . (firebase…

15dab4f

…#229)

christibbs added a commit that referenced this pull request


          Merge iam-master into iam-staging (#353)

523c270

* compile and run

* unit tests pass and swift target compiles

* dump build version in activity log

* standardize cross sdk imports in firebase iam code (#211)

standardize cross sdk imports & polish with style script

* using new sample app for testing non-development fiam sdk (#213)

* in-app messaging public interface cleanup (#216)

* slim down public header files

* move testing source files to their correct folders

* compiles

* style script fine-tune

* adding root object creation methods to follow the firebse iOS convension

* IAM url following refactor (#218)

* @available not supported by blaze build yet (#220)

* Clearcut integration for in-app messaging (#222)

* Update iam-master to catch up with latest master (#223)

* Support multiple triggers defined for the same iam message (#224)

* FIAM Clearcut Client Implmentation Improvement (#226)

* fixing action url bug and set auto dismiss to be 12 seconds (#227)

* use official public api dns name (#233)

* Logging firebase analytics events for fiam (#232)

* Fine tuning of fiam's code for loading data from caches (#234)

* fiam ui fine tuning before EAP (#235)

* allow fiam SDK functions to be disabled/enabled dynamically (#247)

* analytics event parameter name tuning (#253)

* migrate off @import for fiam code (#261)

* different modes for fiam sdk at runtime (#262)

* realtime clearcut when in running in simulator (#265)

* Update interface based on the Firebase API review feedback  (#267)

* check existence of fiam SDK resource bundle (#268)

* handling api keys with application restrictions (#269)

* fixing typo and refoctor for unit testing (#270)

* fix color method name clash and layout adjust (#271)

* Yongmao/layout issue and class rename (#273)

* fix long text height calc error

* rename class

* fiam modal view layout tuning (#274)

* test on device for ios client (#275)

* dynamic testing mode handling (#277)

* main header file comment update and run style.sh (#281)

* honor global firebase auto data collection flag in fiam (#283)

* honor global firebase auto data collection flag in fiam

* address comments

* tuning

* address review comments

* tuning for comments

* Rename Core global data collection switch (#287)

* Support displaying a messages defined as recurring (#286)

* continue

* Adjust some inapp messaging dependeces due to the firebase core changes from the upcoming release

* bump up version for in-app messaging

* Fix impression log clearing race condition bug. (#289)

* fix impression handling race condition bug

* podspec and interacting interfaces for new firebaseinappmessagingdisplay pod (#290)

* create inapp messaging display podspec

* Default fiam display UI implementation (#292)

* Default fiam display UI implementation. Created the new pod for FirebaseInAppMessagingDisplay

* Make FirebaseInAppMessaging headless (#293)

* import path tuning

* log test message events (#296)

* respect fetch wait time from server (#297)

* add changelog.md for fiam headless sdk (#300)

* port the fetch wait time change back into github (#304)

* remove resource bundle check for headless sdk

* fix a bug in which the test-on-device message does not follow action url

* Add category on NSString to interlace two strings, use it to obfuscate clearcut host name

* Use FIR prefix to avoid collisions

* Change method name for getting server hostname

* Delete Swift example project, its coverage is already duplicated in the FIRInAppMessagingDisplay UITests

* Miscellaneous open sourcing cleanup tasks

* Convert FIAM to use the Analytics connector (#336)

* Convert FIAM to use Analytics connector along with converting to the FIRLibrary component registration model.
* Update interop headers for missing methods
* Convert existing unit tests

* Fix style for fiam sources (#342)

* Clean up non-idiomatic ObjC method names

* Clean up Swift test target from Podfile

* Use dummy GoogleService-Info.plist and don't expose api keys (#343)

* Update plist to include fake API keys

* Remove changes in non fiam code from iam-master branch (#350)

* Fix a few method and parameter naming nits

* Fix comment typos in several files

* Delete remaining Swift project files

firebase locked and limited conversation to collaborators

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: auth cla: yes