Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[google_sign_in] adds option to re-authenticate on google silent sign in #4251

Merged

Conversation

ThetaSinner
Copy link
Contributor

Adds an optional paramter to the signInSilently method which allows the application using the plugin to decide whether re-authentication should be attempted.

Fixes #88556

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Note that unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • All existing and new tests are passing.

@stuartmorgan
Copy link
Contributor

Thanks for the submission!

@ditman Are you familiar with this portion of the API in order to review?

@ditman
Copy link
Member

ditman commented Aug 27, 2021

@ditman Are you familiar with this portion of the API in order to review?

@stuartmorgan not with the native implementations. I'm almost sure that this would do nothing special on the web, since signInSilently there only makes sense right after the initialization of the plugin, where the currently "recognized" user may be returned (see here).

@ThetaSinner have you verified this change has the desired behavior in the other platforms? I can't find any information about reauthentication in web (I think it's handled automatically by the JS SDK) or iOS.

(Overall, this doesn't seem dangerous to merge, I'm wondering if more needs to be done across all platforms to achieve the same functionality)

@ThetaSinner
Copy link
Contributor Author

Hi @ditman, I haven't tested iOS because I don't have a MacBook or iOS device (or any emulation set up) but I'm happy to give web a try to see how it behaves. I don't have access to my development environment today or tomorrow but I will take a look Sunday/Monday

@ditman
Copy link
Member

ditman commented Aug 27, 2021

Thanks @ThetaSinner! As I said, in web I'm not sure that calling signInSilently multiple times will cause the same effect as in android, but that's a gap that can be filled later. We just need to confirm it, so we can document the difference across platforms.

@ThetaSinner
Copy link
Contributor Author

Looks like you're right about the web behaviour, the existing login is used to build the response to signInSilently.

The underlying JavaScript library does not discuss ID tokens (that I have seen!). The signIn method it provides is interactive.

It doesn't look like working quietly with ID tokens on web will work without a change to the google-api-javascript-client library. I don't need it so I'm happy with documented differences. Would you like me to find somewhere for that to go and add it to this PR?

@ditman
Copy link
Member

ditman commented Sep 3, 2021

The web version is going to need a bunch of rewriting soonish, that might change things. In the meantime, let's do this. I think the web version re-authenticates automatically, so as long as one always requests the current identity from the JS code, the token will always be valid (at least, that's my operating assumption :P)

Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

LGTM! Let me find somebody from the iOS team to verify that this won't hurt there!

Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

LGTM on iOS

@ditman
Copy link
Member

ditman commented Sep 3, 2021

Thanks for the super-late review @cyanglaz !!

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 3, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 3, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 3, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 4, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 4, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
fotiDim pushed a commit to fotiDim/plugins that referenced this pull request Sep 13, 2021
amantoux pushed a commit to amantoux/plugins that referenced this pull request Sep 27, 2021
KyleFin pushed a commit to KyleFin/plugins that referenced this pull request Dec 21, 2021
@Bharathh-Raj
Copy link

Hey @ThetaSinner, I wanted fresh idToken whenever i sign in. So I used this signInSilently method with reAuthenticate param as true. But I keep on getting the same idToken even after using this.

@ThetaSinner
Copy link
Contributor Author

Hi @Bharathh-Raj,

This is the code I use

await _googleSignIn.signInSilently(
        reAuthenticate: true, suppressErrors: false);

I believe the API will not give you a new ID token unless your current ID token is about to expire (less than 5 minutes remaining?) OR your ID token has expired and your REFRESH token is still valid.

You can check this by grabbing your own ID token and decoding it locally.

If this isn't the answer to your problem then feel free to tag me in a new issue on your project and I'll see if I can help out

@ThetaSinner ThetaSinner deleted the google_signin_reauthenticate branch May 26, 2022 09:46
@Bharathh-Raj
Copy link

So the OAuth API itself won't give us new idtoken unless it is about to/did expire? Is there any way to force the api to respond with new id token whenever I request? I'm using a service which have just 60 seconds window time to validate.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes p: google_sign_in waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

google_sign_in: Not possible to re-authenticate with existing credentials
6 participants