[google_sign_in_web] Migrate to the GIS SDK. #6921

ditman · 2023-01-07T04:44:45Z

This PR migrates google_sign_in_web to the Google Identity Services SDK, in a somewhat "backwards-compatible" way.

There are big differences between the old and new authentication/authorization SDKs, so even though this is API compatible with the old client, there are subtle differences in the underlying behavior that prompted a major version update:

signInSilently will trigger the One Tap user experience.
- If the user consents to the One Tap UI, they will be authenticated but not authorized (a call to requestScopes or signIn is required to authorize the user).
  - Since the previous plugin assumes that user authentication and authorization are the same, the signInSilently method has currently been made to always return null (even on successful authentication) to prevent apps from triggering requests without being authorized.
    - This behavior should change in the future!
signIn will trigger an oauth2 popup.
- If the user has NOT authenticated via signInSilently:
  - The plugin will attempt to request the user profile data from the People API and "fill the gap" (oauth2 endpoints no longer return the user authentication info) after a successful authorization.
    - User profiles retrieved in this way do NOT contain a proper idToken.
    - (The plugin will add 2 more scopes to the ones requested by the user to be able to do this).
- If the user has authenticated via signInSilently:
  - The plugin will skip the request to the PeopleAPI completely and instead use the CredentialResponse received from the One Tap flow.
The rest of the methods should work very similarly to how they work now.

Version bump

This is a major reimplementation of the plugin, and there are some subtle differences in behavior (see above), so this probably warrants a new major version.

Tests

Rewritten tests.
- Tests for the GisSdkClient could be added by abstracting the GIS bits a little bit more.
An unmodified Google Sign-In example app has been deployed here:
- https://dit-gis-test.web.app

Issues

Fixes [google_sign_in_web] Migrate from Google Sign-In JavaScript Platform Library to Google Identity Services flutter#88084

Pre-launch Checklist

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

ditman

Some quick self-review.

packages/google_sign_in/google_sign_in_web/CHANGELOG.md

packages/google_sign_in/google_sign_in_web/example/integration_test/utils_test.dart

packages/google_sign_in/google_sign_in_web/lib/src/gis_client.dart

ditman · 2023-01-18T03:10:29Z

packages/google_sign_in/google_sign_in_web/pubspec.yaml

@@ -3,7 +3,7 @@ description: Flutter plugin for Google Sign-In, a secure authentication system
  for signing in with a Google account on Android, iOS and Web.
 repository: https://github.com/flutter/plugins/tree/main/packages/google_sign_in/google_sign_in_web
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+google_sign_in%22
-version: 0.10.2+1
+version: 0.11.0


Major version bump.

packages/google_sign_in/google_sign_in_web/pubspec.yaml

ditman · 2023-01-18T03:23:33Z

I think the legacy_version_analyze is failing because pub cannot resolve a valid version of package:google_identity_services_web (which requires Dart 2.17+) ?

mdebbar

Great tests!

The code looks good to me, and I trust that you are using the GIS API correctly :)

packages/google_sign_in/google_sign_in_web/example/integration_test/src/create_jwt.dart

packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart

stuartmorgan

My review is pretty high level since I don't know the details of the SDK here, but overall it looks good!

packages/google_sign_in/google_sign_in_web/README.md

packages/google_sign_in/google_sign_in_web/example/integration_test/src/create_jwt.dart

packages/google_sign_in/google_sign_in_web/example/integration_test/src/jsify_as.dart

packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart

packages/google_sign_in/google_sign_in_web/lib/src/gis_client.dart

packages/google_sign_in/google_sign_in_web/lib/src/people.dart

packages/google_sign_in/google_sign_in_web/pubspec.yaml

ditman · 2023-02-14T03:02:02Z

+   jwt_decoder: ^2.0.1
Same question here; is there a reasonable way to not add this dependency?

Hm... Maybe? I'll give this another shot tomorrow, it seems all that needs to be done is to base-64 decode (and JSON.parse) the middle bit of the token? :/ https://stackoverflow.com/questions/52017389/how-to-get-the-claims-from-a-jwt-in-my-flutter-application

ditman · 2023-02-14T23:25:21Z

Rebasing to pick up the latest .ci.yaml changes from main.

ditman · 2023-02-14T23:26:43Z

I think I've addressed all your comments @stuartmorgan, PTAL!

stuartmorgan

LGTM with one nit. Thanks for paring down the dependencies!

packages/google_sign_in/google_sign_in_web/lib/src/utils.dart

…ns#6921)

* cd09d9d31 [ci] Update iOS simulator (flutter/plugins#7131) * 016c3b7f1 Roll Flutter from df41e58 to 22e17bb (28 revisions) (flutter/plugins#7186) * 7160f55e8 [ios_platform_images] Update minimum version to iOS 11 (flutter/plugins#6874) * ea048a249 [in_app_purchase] Update minimum Flutter version to 3.3 and iOS 11 (flutter/plugins#6873) * 530442456 [google_sign_in_web] Migrate to the GIS SDK. (flutter/plugins#6921) * 9a3a77e6c [image_picker] Fix images changing to incorrect orientation (flutter/plugins#7187) * 8f3419be5 Roll Flutter from 22e17bb to 298d8c7 (20 revisions) (flutter/plugins#7190)

sbatezat · 2023-02-21T17:53:07Z

@ditman "User profiles retrieved in this way do NOT contain a proper idToken."

Ok, I can see it on the migration guide, on this PR and almost everywhere. You are not populating idToken on signIn. But I was using this plugin only in order to retrieve that idToken, how should I achieve that now? All documentations written said idToken is null, but no one said how to retrieve it from now. I don't need user information, I just need an idtoken to send to my backend...

Could you please explain how should I migrate?
Cheers

ditman · 2023-02-22T00:11:24Z

Could you please explain how should I migrate?

@sbatezat make sure that you use signInSilently on your app, so that UX flow is able to retrieve an idToken.

The lack of idToken coming from the authorization flow has been reported as an issue to the GSI Team, as mentioned here, and if they change their minds and start returning the token, we'll make it available to users.

sbatezat · 2023-02-22T08:09:38Z

Could you please explain how should I migrate?

@sbatezat make sure that you use signInSilently on your app, so that UX flow is able to retrieve an idToken.

The lack of idToken coming from the authorization flow has been reported as an issue to the GSI Team, as mentioned here, and if they change their minds and start returning the token, we'll make it available to users.

I've tried signInSilently, it returns null (as explained on the documentation) and show nothing to the end user. After signinSilently, I'm calling again signIn, and there is still no IdToken.

If there is no IdToken, what should be the process to validate user server side?

ditman · 2023-02-22T19:54:35Z

I've tried signInSilently, it returns null (as explained on the documentation) and show nothing to the end user. After signinSilently, I'm calling again signIn, and there is still no IdToken.

@sbatezat The user must be signed in to their browser for signInSilently to do its thing, and if the OneTap UX worked successfully, you'll have the idToken after signIn. OneTap UX docs.

If there is no IdToken, what should be the process to validate user server side?

You can't validate users without an idToken server-side, so I guess that for the server-side validation of users, a larger change to your app is required until the Oauth popup starts returning idTokens.

Look at the documentation, you may want to use the "Button" flow?

github-actions bot added p: google_sign_in platform-web labels Jan 7, 2023

ditman mentioned this pull request Jan 7, 2023

[google_sign_in_web] Migrate from Google Sign-In JavaScript Platform Library to Google Identity Services flutter/flutter#88084

Closed

ditman force-pushed the google-sign-in-web-using-gis branch from f69101f to 3055319 Compare January 11, 2023 01:47

ditman force-pushed the google-sign-in-web-using-gis branch from ebeeafa to 2e6339b Compare January 18, 2023 02:58

ditman commented Jan 18, 2023

View reviewed changes

ditman marked this pull request as ready for review January 18, 2023 03:16

ditman force-pushed the google-sign-in-web-using-gis branch from 0f708c8 to 6649796 Compare January 28, 2023 02:02

mdebbar approved these changes Feb 2, 2023

View reviewed changes

ditman force-pushed the google-sign-in-web-using-gis branch from ac078b0 to efad291 Compare February 3, 2023 22:42

ditman requested review from stuartmorgan and peixinli February 3, 2023 22:43

stuartmorgan reviewed Feb 13, 2023

View reviewed changes

ditman force-pushed the google-sign-in-web-using-gis branch from d6fd4d8 to 95f3d9e Compare February 14, 2023 02:57

ditman added 13 commits February 14, 2023 23:25

[google_sign_in_web] Migrate to GIS SDK.

bb67b77

include_granted_scopes in requestScopes call.

095feaf

Remove the old JS-interop layer.

60ffbb3

Introduce a mockable GisSdkClient for tests.

66cebf3

Split the people utils.

1e88f00

Delete tests for the old code.

7f3b281

Add some tests for the new code.

c77e6d3

More utils_test.dart

bbe2349

Make jsifyAs reusable.

77fb227

Ignore the tester in utils_test.dart

552d7b3

Make Clients overridable, and some renaming.

060a11d

Test people.dart

17e73fa

Make autoDetectedClientId more testable.

55ff6ac

ditman added 5 commits February 14, 2023 23:25

Rename to Vincent Adultman

0a70e21

_isJsSdkLoaded -> _jsSdkLoadedFuture

34704e6

Remove idToken comment.

908beac

Link issue to split mocking better.

7087d49

Remove dependency in package:jwt_decoder

382ed3d

ditman force-pushed the google-sign-in-web-using-gis branch from 7fb3abf to 382ed3d Compare February 14, 2023 23:26

ditman requested a review from stuartmorgan February 14, 2023 23:26

stuartmorgan approved these changes Feb 16, 2023

View reviewed changes

packages/google_sign_in/google_sign_in_web/lib/src/utils.dart Outdated Show resolved Hide resolved

Remove unneeded cast call.

7d91c22

ditman self-assigned this Feb 16, 2023

ditman added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 16, 2023

auto-submit bot merged commit 5304424 into flutter:main Feb 17, 2023

ditman deleted the google-sign-in-web-using-gis branch February 17, 2023 04:18

engine-flutter-autoroll mentioned this pull request Feb 17, 2023

Roll Plugins from d699b4a91381 to 5304424565f9 (5 revisions) flutter/flutter#120978

Closed

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

530442456 [google_sign_in_web] Migrate to the GIS SDK. (flutter/plugi…

adb55c6

…ns#6921)

engine-flutter-autoroll mentioned this pull request Feb 17, 2023

Roll Plugins from d699b4a91381 to 9a3a77e6c32b (6 revisions) flutter/flutter#120982

Closed

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

530442456 [google_sign_in_web] Migrate to the GIS SDK. (flutter/plugi…

943d52a

…ns#6921)

engine-flutter-autoroll mentioned this pull request Feb 17, 2023

Roll Plugins from d699b4a91381 to 8f3419be5e0e (7 revisions) flutter/flutter#120983

Closed

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

530442456 [google_sign_in_web] Migrate to the GIS SDK. (flutter/plugi…

6759b20

…ns#6921)

engine-flutter-autoroll mentioned this pull request Feb 17, 2023

Roll Plugins from d699b4a91381 to 8f3419be5e0e (7 revisions) flutter/flutter#120993

Merged

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

530442456 [google_sign_in_web] Migrate to the GIS SDK. (flutter/plugi…

52b14c0

…ns#6921)

nehalvpatel mentioned this pull request Feb 22, 2023

Build dashboard sign in silently is failing flutter/flutter#121275

Closed

huycozy mentioned this pull request Mar 7, 2023

package google_sign_in_web needs updated example flutter/flutter#121930

Closed

huycozy mentioned this pull request Jun 8, 2023

Implement google one tap sign on flutter web flutter/flutter#128486

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[google_sign_in_web] Migrate to the GIS SDK. #6921

[google_sign_in_web] Migrate to the GIS SDK. #6921

ditman commented Jan 7, 2023 •

edited

Loading

ditman left a comment

ditman Jan 18, 2023

ditman commented Jan 18, 2023

mdebbar left a comment

stuartmorgan left a comment

ditman commented Feb 14, 2023

ditman commented Feb 14, 2023

ditman commented Feb 14, 2023

stuartmorgan left a comment

sbatezat commented Feb 21, 2023 •

edited

Loading

ditman commented Feb 22, 2023 •

edited

Loading

sbatezat commented Feb 22, 2023 •

edited

Loading

ditman commented Feb 22, 2023 •

edited

Loading

[google_sign_in_web] Migrate to the GIS SDK. #6921

[google_sign_in_web] Migrate to the GIS SDK. #6921

Conversation

ditman commented Jan 7, 2023 • edited Loading

Version bump

Tests

Issues

Pre-launch Checklist

ditman left a comment

Choose a reason for hiding this comment

ditman Jan 18, 2023

Choose a reason for hiding this comment

ditman commented Jan 18, 2023

mdebbar left a comment

Choose a reason for hiding this comment

stuartmorgan left a comment

Choose a reason for hiding this comment

ditman commented Feb 14, 2023

ditman commented Feb 14, 2023

ditman commented Feb 14, 2023

stuartmorgan left a comment

Choose a reason for hiding this comment

sbatezat commented Feb 21, 2023 • edited Loading

ditman commented Feb 22, 2023 • edited Loading

sbatezat commented Feb 22, 2023 • edited Loading

ditman commented Feb 22, 2023 • edited Loading

ditman commented Jan 7, 2023 •

edited

Loading

sbatezat commented Feb 21, 2023 •

edited

Loading

ditman commented Feb 22, 2023 •

edited

Loading

sbatezat commented Feb 22, 2023 •

edited

Loading

ditman commented Feb 22, 2023 •

edited

Loading