-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[google_sign_in] Fix Server Authorization code on android #2792
[google_sign_in] Fix Server Authorization code on android #2792
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
I need this fix to get the serverAuthCode such that I can get a refresh token at the backend side to use google API. Currently, it returns |
@zeeshanhussain I tried cloning your branch and gave it a shot. But it's not working for me.
I can see the exact same error message in some git checks |
@abhishek199-dhn There are some changes in google_sign_in_platform_interface as well so you have to clone that locally as well and that's why it was failing few checks. |
@zeeshanhussain which changes you are referring to? can you please give me the link of the PR? |
@abhishek199-dhn it's in the same pr, this pr has changes in both google_sign_in and google_sign_in_platform_interface. I should probably split it into 2 pr after getting this reviewed. |
Thanks @zeeshanhussain, it woking now. I pointed both the dependencies |
@zeeshanhussain I pulled your branch and pointed |
you can copy all the 3 packages in your project folder which includes [google_sign_in], [google_sign_in_platform_interface] , [google_sign_in_web]. In your project's pubspec.yaml you can add the [google_sign_in] dependency. google_sign_in:
path: ./google_sign_in In google_sign_in/pubspec.yaml you can add both [google_sign_in_web] and [google_sign_in_platform_interface]. google_sign_in_platform_interface:
path: ../google_sign_in_platform_interface/
google_sign_in_web:
path: ../google_sign_in_web in google_sign_in_web/pubspec.yaml you can add [google_sign_in_platform_interface] google_sign_in_platform_interface:
path: ../google_sign_in_platform_interface/ |
@scaraux go to firebase/authentication/sign in methods/google |
Yes I created a new clientId this morning, added it to the whitelist clientIds sections and downloaded the google-services.json. Still no server auth code though :/ |
@scaraux you have to add web client id and web client secret in Web SDK configuration, then use the updated google-services.json. |
I can give it a try, I know how to access my "OAuth 2.0 Client IDs" from the https://console.developers.google.com/. But which clientId should I copy ? |
You should use the one which you want to use in your application. Give it a try and it will definitely work. |
All good now, thank you so much. |
@zeeshanhussain any idea when this will be merged ? |
No idea, it has to be reviewed first. you can tag one of the developers for a faster response. |
I have no idea who I can tag 😟 |
For anyone look into this issue and this PR:
|
Thanks for the submission! 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 per Flutter policy, this will need tests before it can move forward. (You've checked the box indicating that you've added tests, but I'm not seeing them; if you did write tests already please ensure that part of the PR is pushed.) |
@stuartmorgan the previous pr added few tests. 88e85c6 Let me know if you need more tests. |
The tests in that PR are passing, so clearly aren't testing the issue that this PR is intended to address. |
@zeeshanhussain @stuartmorgan Any update on this? I used the workaround to use local copy of this branch but these 3 packages have not been migrated to null safety. Hence, this didnot work for me. Any idea when this branch will be migrated to null safety or when this issue will be fixed? |
Closing as obsoleted by #4180. Thanks again for the submission! |
Thanks for the fix. Somehow I am still getting null serverAuthCode. Is this change already pushed in latest published package for google_sign_in? Also, are you aware of any ongoing or past fix for ios for this scenario? I am integrating google sign in for my app and need both these fixes. Will appreciate any help :) |
@jainkrati I am also facing the issue of null serverAuthCode for ios. For android, it is working fine. @zeeshanhussain @stuartmorgan Please suggest some fixes. |
Please report issues via the issue tracker; PRs are not an issue tracking system. |
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
Description
The serverAuthCode on an android device was coming null. This pull request resolves that issue.
Related Issues
flutter/flutter#57712
flutter/flutter#57741
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.///
).flutter analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?