[google_sign_in_ios] Upgrade GoogleSignIn iOS SDK to 7.0 #5240

vashworth · 2023-10-26T20:50:05Z

See https://developers.google.com/identity/sign-in/ios/quick-migration-guide and https://developers.google.com/identity/sign-in/ios/release for info about changes made to the SDK.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

vashworth · 2023-10-26T20:52:20Z

packages/google_sign_in/google_sign_in_ios/ios/Classes/FLTGoogleSignInPlugin.m

-    *error = [FlutterError errorWithCode:@"missing-config"
-                                 message:@"GoogleService-Info.plist file not found and clientId "
-                                         @"was not provided programmatically."
-                                 details:nil];


The Google Sign In SDK now gets the configuration settings within the SDK itself from the Info.plist and that's now the preferred way to do it.

Should we remove the GoogleService-Info.plist from the example then? We generally want the example to show the preferred approach.

Also, we should put an updated version of https://pub.dev/packages/google_sign_in#ios-integration into this package's README, and then update the main package README to point to that for iOS integration details. That's the pattern we are increasingly using for non-trivial setup docs in federated plugins (due to issues like this, where the setup steps actually depend on the version of the sub-package you are using).

FYI I moved GoogleService-Info.plist from the Runner target to the RunnerTests target, so we can still use it for the example tests.

vashworth · 2023-10-26T20:56:42Z

packages/google_sign_in/google_sign_in_ios/ios/Classes/FLTGoogleSignInPlugin.m

-    [self didSignInForUser:user withCompletion:completion error:error];
+  [self.signIn restorePreviousSignInWithCompletion:^(GIDGoogleUser *_Nullable user,
+                                                     NSError *_Nullable error) {
+    [self didSignInForUser:user serverAuthCode:nil withCompletion:completion error:error];


You may notice here I set serverAuthCode to nil. This is something I was unsure about. In previous version, the serverAuthCode was a property of the user. However, in the new version, it's now a property of GIDSignInResult, which you get when you sign in but not restore. I did confirm that in the old version on restore, the serverAuthCode was included.

However, I think the new way where it doesn't include the serverAuthCode is more inline with how OAuth works. Usually that code is a temporary one-time use code and then after that tokens are use to validate (see diagram in https://developers.google.com/identity/sign-in/web/server-side-flow).

vashworth · 2023-10-26T21:02:12Z

packages/google_sign_in/google_sign_in_ios/ios/Classes/FLTGoogleSignInPlugin.m

+                              [FlutterError errorWithCode:@"request_scopes"
+                                                  message:@"There is an operation on a previous "
+                                                          @"user. Try signing in again."
+                                                  details:nil];


FYI, they removed kGIDSignInErrorCodeNoCurrentUser as an error, I'm assuming because the method is now part of the GIDGoogleUser class, so there has to be a user. They also added a new error kGIDSignInErrorCodeMismatchWithCurrentUser, which I believe happens when GIDSignIn.currentUser doesn't match the user that makes this request. This shouldn't ever happen since we get the user from self.signIn.currentUser, but I added a new error just in case.

packages/google_sign_in/google_sign_in_ios/ios/Classes/FLTGoogleSignInPlugin.h

stuartmorgan · 2023-11-01T16:49:42Z

The addScopes: method has been moved to GIDGoogleUser. Instead of requesting additional authorization scopes from GIDSignIn, you should now request them from GIDGoogleUser after authentication has completed.

Does this mean we are now stuck with the two-stage UI flow described here?

vashworth · 2023-11-01T17:11:33Z

The addScopes: method has been moved to GIDGoogleUser. Instead of requesting additional authorization scopes from GIDSignIn, you should now request them from GIDGoogleUser after authentication has completed.

Does this mean we are now stuck with the two-stage UI flow described here?

I don't believe so, you can also request the scopes on initial sign in

packages/packages/google_sign_in/google_sign_in_ios/ios/Classes/FLTGoogleSignInPlugin.m

Lines 147 to 149 in 815fd8f

    
           [self.signIn signInWithPresentingViewController:[self topViewController] 
        
                                                      hint:nil 
        
                                          additionalScopes:self.requestedScopes.allObjects

I think that was fixed in 6.2 (flutter/plugins#5708)

stuartmorgan · 2023-11-01T17:34:28Z

I don't believe so, you can also request the scopes on initial sign in

😌

I think that was fixed in 6.2 (flutter/plugins#5708)

Right, I was afraid from that migration description that they had un-reversed that decision in 7 and were re-committing to separating authn and authz.

packages/google_sign_in/google_sign_in_ios/example/ios/RunnerTests/GoogleSignInTests.m

packages/google_sign_in/google_sign_in_ios/ios/Classes/FLTGoogleSignInPlugin.h

stuartmorgan · 2023-11-01T17:45:07Z

packages/google_sign_in/google_sign_in_ios/ios/Classes/FLTGoogleSignInPlugin.m

-    *error = [FlutterError errorWithCode:@"missing-config"
-                                 message:@"GoogleService-Info.plist file not found and clientId "
-                                         @"was not provided programmatically."
-                                 details:nil];


Should we remove the GoogleService-Info.plist from the example then? We generally want the example to show the preferred approach.

stuartmorgan · 2023-11-01T17:47:11Z

packages/google_sign_in/google_sign_in_ios/ios/Classes/FLTGoogleSignInPlugin.m

-    *error = [FlutterError errorWithCode:@"missing-config"
-                                 message:@"GoogleService-Info.plist file not found and clientId "
-                                         @"was not provided programmatically."
-                                 details:nil];


Also, we should put an updated version of https://pub.dev/packages/google_sign_in#ios-integration into this package's README, and then update the main package README to point to that for iOS integration details. That's the pattern we are increasingly using for non-trivial setup docs in federated plugins (due to issues like this, where the setup steps actually depend on the version of the sub-package you are using).

stuartmorgan · 2023-11-01T17:58:19Z

packages/google_sign_in/google_sign_in_ios/ios/Classes/FLTGoogleSignInPlugin.m

@@ -281,7 +298,7 @@ - (void)didSignInForUser:(GIDGoogleUser *)user
                                          email:user.profile.email
                                         userId:user.userID
                                       photoUrl:[photoUrl absoluteString]
-                                 serverAuthCode:user.serverAuthCode],
+                                 serverAuthCode:serverAuthCode],


Have you audited the new API against the Dart types to see if we want any more changes here? The current situation is that we have optional types on various return objects (GoogleSignInUserData, GoogleSignInTokenData) and we populate what we can, which has varied from platform to platform and version to version. Notably, the release notes mention a new idToken property on GIDGoogleUser, and we have that on the Dart side, so it's very likely we should update the Pigeon FSIUserData definition to include it, and then pass it back here and use it to populate that class on the Dart side here.

idToken was the only one from what I could tell.

packages/google_sign_in/google_sign_in_ios/ios/Classes/FLTGoogleSignInPlugin.m

…lue for dart type, fix scopes not being set, add more tests

stuartmorgan

LGTM with nits. Thanks for doing this update!

stuartmorgan · 2023-11-06T19:01:16Z

packages/google_sign_in/google_sign_in_ios/ios/Classes/FLTGoogleSignInPlugin.h

@@ -6,5 +6,8 @@

 #import "messages.g.h"

+#import <GoogleSignIn/GoogleSignIn.h>


Do we need this?

stuartmorgan · 2023-11-06T19:04:33Z

packages/google_sign_in/google_sign_in_ios/ios/Classes/FLTGoogleSignInPlugin.m

+
+                                           [self didSignInForUser:user
+                                                   serverAuthCode:serverAuthCode
+                                                   withCompletion:completion


nit: the with should go on the first additional argument, so didSignInForUser:withServerAuthCode:completion:error:.

* main: (59 commits) [pointer_interceptor] Move pointer_interceptor to sub folder (flutter#5291) [webview_flutter] Update iOS to Pigeon 13 (flutter#5271) [repo] Adjust error message layout for repo checks (flutter#5241) [in_app_pur]: Bump org.json:json from 20230618 to 20231013 in /packages/in_app_purchase/in_app_purchase/example/android/app (flutter#5153) Roll Flutter from f5a9835 to 5a6a322 (15 revisions) (flutter#5345) [google_sign_in_ios] Upgrade GoogleSignIn iOS SDK to 7.0 (flutter#5240) Roll Flutter from 29b2516 to f5a9835 (101 revisions) (flutter#5341) [pigeon]: Bump org.jetbrains.kotlin:kotlin-gradle-plugin from 1.9.10 to 1.9.20 in /packages/pigeon/platform_tests/test_plugin/android (flutter#5334) [flutter_markdown] Strip leading whitespace from list items (flutter#5294) [two_dimensional_scrollables] Add borderRadius support to TableSpanDecoration (flutter#5184) [go_router] Adds an ability to add a custom codec for serializing/des… (flutter#5288) [go_router] Fixes crashes when dynamically updates routing tables wit… (flutter#5242) [camerax] Fix `_getResolutionSelectorFromPreset` NPE (flutter#5287) [go_router]Fixes the problem what pathParameters is null in redirect when the Router is recreated. (flutter#5258) [google_sign_in] Fix Obj-C formatting bug (flutter#5310) Fix mounted checks (flutter#5305) [flutter_image] [flutter_markdown] Do not use dynamic in an == operator override (flutter#5298) [google_sign_in] Enable FedCM for web. Use token expiration. (flutter#5225) [video_player_web] Listen to loadedmetadata event from video element. (flutter#5289) [local_auth] Update iOS to Pigeon 13 (flutter#5269) ... # Conflicts: # packages/webview_flutter/webview_flutter_wkwebview/ios/Classes/FWFWebViewHostApi.m

…tter/packages#5240)

flutter/packages@be18d28...94c7623 2023-11-07 louisehsu@google.com [pointer_interceptor] Move pointer_interceptor to sub folder (flutter/packages#5291) 2023-11-07 stuartmorgan@google.com [webview_flutter] Update iOS to Pigeon 13 (flutter/packages#5271) 2023-11-07 katelovett@google.com [repo] Adjust error message layout for repo checks (flutter/packages#5241) 2023-11-07 49699333+dependabot[bot]@users.noreply.github.com [in_app_pur]: Bump org.json:json from 20230618 to 20231013 in /packages/in_app_purchase/in_app_purchase/example/android/app (flutter/packages#5153) 2023-11-07 engine-flutter-autoroll@skia.org Roll Flutter from f5a9835 to 5a6a322 (15 revisions) (flutter/packages#5345) 2023-11-07 15619084+vashworth@users.noreply.github.com [google_sign_in_ios] Upgrade GoogleSignIn iOS SDK to 7.0 (flutter/packages#5240) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Fixes flutter/flutter#137140. See https://developers.google.com/identity/sign-in/ios/quick-migration-guide and https://developers.google.com/identity/sign-in/ios/release for info about changes made to the SDK.

vashworth added 2 commits October 26, 2023 15:40

upgrade Google Sign In to 7.0 for iOS

067f3fe

update changelog and version

bb30b60

github-actions bot added p: google_sign_in platform-ios labels Oct 26, 2023

vashworth commented Oct 26, 2023

View reviewed changes

vashworth marked this pull request as ready for review October 26, 2023 21:07

vashworth requested a review from stuartmorgan October 26, 2023 21:07

remove comment and make match

815fd8f

vashworth commented Oct 27, 2023

View reviewed changes

packages/google_sign_in/google_sign_in_ios/ios/Classes/FLTGoogleSignInPlugin.h Outdated Show resolved Hide resolved

stuartmorgan requested changes Nov 1, 2023

View reviewed changes

vashworth added 5 commits November 3, 2023 15:09

Update README, update example app to use recommended settings, add va…

177f2f6

…lue for dart type, fix scopes not being set, add more tests

update README again

57a2b31

fix format

798fc19

add changelog, update false secrets, use code-excerpt

04fe8a7

fix more repo checks

f66c4cb

vashworth requested a review from stuartmorgan November 6, 2023 16:38

stuartmorgan approved these changes Nov 6, 2023

View reviewed changes

fix nits

f366a3e

vashworth added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 7, 2023

auto-submit bot merged commit 72c947c into flutter:main Nov 7, 2023
79 checks passed

engine-flutter-autoroll mentioned this pull request Nov 8, 2023

Roll Packages from be18d282c5e7 to 94c762380828 (6 revisions) flutter/flutter#138086

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 8, 2023

72c947c [google_sign_in_ios] Upgrade GoogleSignIn iOS SDK to 7.0 (flu…

8d5b3a7

…tter/packages#5240)

reidbaker mentioned this pull request Jan 12, 2024

Dialog Debug #5881

Closed

reidbaker mentioned this pull request Jan 19, 2024

[in_app_purchase] Add play country code api #5941

Merged

12 tasks

reidbaker mentioned this pull request Feb 19, 2024

[in_app_purchase_android] Add UserChoiceBilling mode. #6162

Merged

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[google_sign_in_ios] Upgrade GoogleSignIn iOS SDK to 7.0 #5240

[google_sign_in_ios] Upgrade GoogleSignIn iOS SDK to 7.0 #5240

vashworth commented Oct 26, 2023 •

edited

Loading

vashworth Oct 26, 2023

stuartmorgan Nov 1, 2023

stuartmorgan Nov 1, 2023

vashworth Nov 3, 2023

vashworth Oct 26, 2023 •

edited

Loading

vashworth Oct 26, 2023 •

edited

Loading

stuartmorgan commented Nov 1, 2023 •

edited

Loading

vashworth commented Nov 1, 2023

stuartmorgan commented Nov 1, 2023

stuartmorgan Nov 1, 2023

stuartmorgan Nov 1, 2023

stuartmorgan Nov 1, 2023

vashworth Nov 3, 2023

stuartmorgan left a comment

stuartmorgan Nov 6, 2023

stuartmorgan Nov 6, 2023

		@@ -6,5 +6,8 @@

		#import "messages.g.h"

		#import <GoogleSignIn/GoogleSignIn.h>

[google_sign_in_ios] Upgrade GoogleSignIn iOS SDK to 7.0 #5240

[google_sign_in_ios] Upgrade GoogleSignIn iOS SDK to 7.0 #5240

Conversation

vashworth commented Oct 26, 2023 • edited Loading

Pre-launch Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vashworth Oct 26, 2023 • edited Loading

Choose a reason for hiding this comment

vashworth Oct 26, 2023 • edited Loading

Choose a reason for hiding this comment

stuartmorgan commented Nov 1, 2023 • edited Loading

vashworth commented Nov 1, 2023

stuartmorgan commented Nov 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stuartmorgan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vashworth commented Oct 26, 2023 •

edited

Loading

vashworth Oct 26, 2023 •

edited

Loading

vashworth Oct 26, 2023 •

edited

Loading

stuartmorgan commented Nov 1, 2023 •

edited

Loading