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

[google_sign_in] Fix serverAuthCode getToken on iOS #2788

Closed

Conversation

yamarkz
Copy link
Contributor

@yamarkz yamarkz commented May 21, 2020

Description

The method of getting serverAuthCode was not correct.
I fixed it and confirmed that my local was able to get it.

here are

try {
  final _googleSignIn = GoogleSignIn(
    scopes: [
      'email',
      'https://www.googleapis.com/auth/contacts.readonly',
    ],
  );
  final googleSignInAccount = await _googleSignIn.signIn();
  final googleSignInAuthentication = await googleSignInAccount.authentication;
  final code = googleSignInAuthentication.serverAuthCode;
  print(code);
} catch (error) {
  print(error);
}

Related Issues

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • 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 signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

@scaraux
Copy link

scaraux commented May 29, 2020

@yamarkz Hey I am pulling from your branch but the app crashes immediatly when google sign in resolves, any idea?

0 CoreFoundation 0x00007fff23e3cf0e __exceptionPreprocess + 350 1 libobjc.A.dylib 0x00007fff50ba89b2 objc_exception_throw + 48 2 CoreFoundation 0x00007fff23ecfa51 _CFThrowFormattedException + 194 3 CoreFoundation 0x00007fff23eda4a6 -[__NSPlaceholderDictionary initWithObjects:forKeys:count:].cold.4 + 38 4 CoreFoundation 0x00007fff23e9b787 -[__NSPlaceholderDictionary initWithObjects:forKeys:count:] + 247 5 CoreFoundation 0x00007fff23e38e11 +[NSDictionary dictionaryWithObjects:forKeys:count:] + 49 6 Runner 0x00000001099fe6e4 __49-[FLTGoogleSignInPlugin handleMethodCall:result:]_block_invoke + 452

@yamarkz
Copy link
Contributor Author

yamarkz commented May 30, 2020

@scaraux
Thank you for comment.
Do you have the SERVER_CLIENT_ID set to GoogleService-Info.plist ?
I'm able to get serverAuthCode in my environment.

@scaraux
Copy link

scaraux commented May 30, 2020

@scaraux
Thank you for comment.
Do you have the SERVER_CLIENT_ID set to GoogleService-Info.plist ?
I'm able to get serverAuthCode in my environment.

If you are talking about passing clientId in the constructor, yes I did.

EDIT: I didn't know about this. I added it to the .plist file. Thank you.
Looking forward for your pull request to be merged.

@abhishek199-dhn
Copy link

abhishek199-dhn commented Jun 3, 2020

I need this fix to get the serverAuthCode such that I can get a refresh token at the backend side to use google API.
By when we can expect this to be merged?

@scaraux
Copy link

scaraux commented Jun 5, 2020

I need this fix to get the serverAuthCode such that I can get a refresh token at the backend side to use google API.
By when we can expect this to be merged?

In the meantime you can always do this:
pubspec.yaml

  google_sign_in:
    git:
      url: [email protected]:yamarkz/plugins.git
      path: packages/google_sign_in/google_sign_in
      ref: fix-server-auth-code-get-token

Don't forget to replace it when it's merged

@JonasABR
Copy link

JonasABR commented Jun 19, 2020

@yamarkz & @scaraux I'm trying to reference your project to test it, cause I also need this token in my project, but everytime after login when i try to access anything from the GoogleSignInAccount returned object, I get a crash in my app.

*** First throw call stack:
(
	0   CoreFoundation                      0x00007fff23e3cf0e __exceptionPreprocess + 350
	1   libobjc.A.dylib                     0x00007fff50ba89b2 objc_exception_throw + 48
	2   CoreFoundation                      0x00007fff23ecfa51 _CFThrowFormattedException + 194
	3   CoreFoundation                      0x00007fff23eda4a6 -[__NSPlaceholderDictionary initWithObjects:forKeys:count:].cold.4 + 38
	4   CoreFoundation                      0x00007fff23e9b787 -[__NSPlaceholderDictionary initWithObjects:forKeys:count:] + 247
	5   CoreFoundation                      0x00007fff23e38e11 +[NSDictionary dictionaryWithObjects:forKeys:count:] + 49
	6   Runner                              0x0000000100e7df94 __49-[FLTGoogleSignInPlugin handleMethodCall:result:]_block_invoke + 452
	7   Runner  <…>
Lost connection to device.

@chakrihacker
Copy link

@JonasABR Add server_client_id to your plist or else it keeps crashing

@@ -112,6 +112,7 @@ - (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result
result(error != nil ? getFlutterError(error) : @{
@"idToken" : authentication.idToken,
@"accessToken" : authentication.accessToken,
@"serverAuthCode" : currentUser.serverAuthCode,

Choose a reason for hiding this comment

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

When app is started with an older Google Sign In state the currentUser.serverAuthCode can be nil and this will cause a crash since nil can't be inserted into NSDictionary

Choose a reason for hiding this comment

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

That's right, I had this behaviour also.

Choose a reason for hiding this comment

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

Hey guys, can you please detail how to reproduce this issue? We've tried a few scenarios with this branch, but none caused a crash. @Dahlgren

Copy link

Choose a reason for hiding this comment

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

It occurs when an existing session with Google Sign In is reused. The serverAuthCode only exists on initial login. If the existing session is flushed (logged out) and a new session is created the serverAuthCode will exist, if applicable.

@assemmarwan
Copy link

assemmarwan commented Jul 28, 2020

@scaraux
Thank you for comment.
Do you have the SERVER_CLIENT_ID set to GoogleService-Info.plist ?
I'm able to get serverAuthCode in my environment.

I don't have this key in the .plist downloaded from Firebase. There are three Client IDs:

  • CLIENT_ID
  • REVERSED_CLIENT_ID
  • ANDROID_CLIENT_ID

Below are all the keys in the .plist

image

Can you please guide me.

EDIT: Finally I found the solution. You'd need to manually add the key SERVER_CLIENT_ID which is the client id of the Web Application. The documentation need to be updated mentioning this.

@JonasABR
Copy link

JonasABR commented Sep 7, 2020

This been open for a long time and still no conclusion? I particulary really need this feature working in iOS & Android. The android branch is also open for a long time. Wanted to avoid having it locally.

@scaraux
Copy link

scaraux commented Sep 10, 2020

This been open for a long time and still no conclusion? I particulary really need this feature working in iOS & Android. The android branch is also open for a long time. Wanted to avoid having it locally.

Same thing here. This is bad. Triage has not been updated in over a month
https://github.com/flutter/plugins/projects/3#card-43160874

I understand this is open source and people have other priorities, let us know if we can do anything.

@scaraux
Copy link

scaraux commented Nov 16, 2020

Guys, we really need this merged. Please...

@scaraux
Copy link

scaraux commented Feb 17, 2021

@stuartmorgan is there anything you can do to get this merged? Been open for NINE months now...

@stuartmorgan
Copy link
Contributor

We apologize for the long delay in triaging this PR. We’re in the process of overhauling our PR triage system to respond much more quickly, as well as working through the backlog.

We’re currently working through a large backlog of PRs, and this will require non-trivial review, so it will take some time before we’re able to review it. As explained in CONTRIBUTING.md, votes for the corresponding issue are the primary way we’re prioritizing non-trivial reviews, so we encourage anyone interested in this PR to vote for the corresponding issue.

Note that based on the discussion above it appears that this PR is not correct as-is (not correctly handling nil values in the ObjC code), and it's also missing tests, so wouldn't be mergeable as-is. Addressing the known issues and adding tests would be important steps toward making this reviewable and landable.

@yamarkz
Copy link
Contributor Author

yamarkz commented Feb 18, 2021

I'm sorry that this PR will not be merged.
I have no desire to merge and use this PR, so I will close it.
If you want to use it as a public package, please create a similar PR. Thanks.

@scaraux @stuartmorgan

@yamarkz yamarkz closed this Feb 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants