plugins
plugins copied to clipboard
[google_sign_in] Add doc for iOS auth with SERVER_CLIENT_ID
Add documentation for iOS auth with SERVER_CLIENT_ID.
After a long time struggling with a non-working iOS configuration but Android working well, I figured out that firebase was not adding the SERVER_CLIENT_ID field in the Google-info-service.plist file genereated. This can be really long to troubleshoot for someone like me who don't know really much about GoogleAuth with flutter plugin.
Pre-launch Checklist
- [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
- [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
- [x] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/plugins repo does use
dart format
.) - [x] I signed the CLA.
- [x] 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.
- [x] I updated
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes. - [x] I updated
CHANGELOG.md
to add a description of the change, following repository CHANGELOG style. - [ ] I updated/added relevant documentation (doc comments with
///
). - [x] I added new tests to check the change I am making, or this PR is test-exempt.
- [x] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel on Discord.
@Milvintsiss Thanks for sending this PR. ~Although this documentation is good to have, we probably also~ ~want to update the code to make the clientId programmatically configurable from a dart API. I think this~ ~proposal here is valid.~
We actually have already implemented a way to dynamically set up the clientId, which should be petty easily discoverable. :https://github.com/flutter/plugins/blob/main/packages/google_sign_in/google_sign_in/lib/google_sign_in.dart#L186
If we are adding the plist approach for clientId in README, then we should add the programmatic approach too so users can choose the way they want to do it
As per documentation this is only for web. Currently there is no way to programmatically configure this.
EDIT: I looked a bit to the native code using the programmatically configured clientId
, in fact iOS and Android use it, so the documentation is outdated, but it has nothing to do with serverClientId
. If a clientId
is set, it will replace the CLIENT_ID
of the google_services
file, not the SERVER_CLIENT_ID
.
serverClientId
has currently no way to be programmatically configured, but I can implement it if needed. Maybe this deserves another PR?
serverClientId
has currently no way to be programmatically configured, but I can implement it if needed. Maybe this deserves another PR?
I see, I got confused with these 2 IDs. Yes I don't see why it should not be programmatically configurable.
And we should update the documentation about saying the clientId
only supports web as well.
I see, I got confused with these 2 IDs. Yes I don't see why it should not be programmatically configurable. And we should update the documentation about saying the
clientId
only supports web as well.
While trying to implement a programmatically configurable serverClientId
for the plugin I found some strange behaviors:
It seems that on Android no clientId
is needed, but only a serverClientId
(so the current clientId
is used as serverClientId
).
This is the prototype of android signIn method:
public GoogleSignInOptions.Builder requestIdToken (String serverClientId)
and how we are using it here. I tried using the clientId
need by iOS to logIn and it doesn't work but work great with the serverClientId
.
Therefor on iOS both clientId
and serverClientId
are needed.
This really need clarification, the current clientId
should be named serverClientId
and be used as it for iOS (currently, you get a crash on iOS if you specify your clientId
in the current clientId
parameter).
This would be a breaking change but the current behavior is wrong and can easily mislead.
Hope this is readable, and please notify me if I'm misunderstanding something.
@cyanglaz Can you do the primary review here since you've been looking at this the most?
@Milvintsiss There is a PR trying to add the serverClientId https://github.com/flutter/plugins/pull/5250. It is currently being worked on. Maybe we can cooperate the new changes into this doc change after the above PR lands?
@cyanglaz Yes it would be great :+1:
@cyanglaz @Milvintsiss looks like #5250 has landed; what are the next steps on this PR?
@Hixie I think we can close this one, the author has already added documentation in the README.
Maybe we can add a note about configuring the SERVER_CLIENT_ID
in the GoogleService-Info.plist
, but as the recommended way to configure the server client ID is to configure it programmatically we probably don't need to document this?
Maybe we can add a note about configuring the SERVER_CLIENT_ID in the GoogleService-Info.plist
Yes, we should also add a document for adding SERVER_CLIENT_ID
in GoogleService-Info.plist
. Would you like to rebase main to your PR and work on it? :)
Hey @cyanglaz can I have a review on this? :)
@cyanglaz Ping on this review from triage.
Why is this pr still open? Changes regarding the doc configuring SERVER_CLIENT_ID in GoogleService-Info.plist is not yet merged. Stumbled upon this ISSUE today and found that setting serverClientId in dart code alone does not work at all. Finally got it working by adding SERVER_CLIENT_ID in GoogleService-Info.plist.