Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include Google profile information #276

Closed
wants to merge 2 commits into from
Closed

Include Google profile information #276

wants to merge 2 commits into from

Conversation

jdnichollsc
Copy link
Contributor

@jdnichollsc jdnichollsc commented Aug 17, 2016

It is necessary to get user data such as displayName and photoURL from Google

It is necessary to get user data such as Save and photoURL
@jdnichollsc jdnichollsc changed the title Include profile information Include Google profile information Aug 17, 2016
Fixed empty space using '+' character to add scopes
@nraboy
Copy link
Owner

nraboy commented Aug 17, 2016

A few things:

  1. Can you link to the documentation that mentions this tag? Would like to keep it as part of the project history.
  2. Does this require a particular scope? I don't mean require to be used, but I mean require to use oauth at all when it is included.
  3. I don't accept pull requests on the master branch. It is typically behind the development branch. Instead you would need to clone the repository, checkout the development branch, apply your changes, then create a PR.

https://github.com/nraboy/ng-cordova-oauth#contribution-rules

I appreciate the contributions though :-)

@jdnichollsc
Copy link
Contributor Author

I can't find this tag in the documentation, only debugging the firebase authentication like the following post https://groups.google.com/forum/#!topic/firebase-talk/35lFiZzzUII and testing with my Android device with GapDebug

@nraboy
Copy link
Owner

nraboy commented Aug 18, 2016

And you're sure it does not force extra permissions?

Make add your changes to the development branch and make a new PR and I'll merge it.

Best,

@matheusrocha89
Copy link
Contributor

I think this could be an option to not force all users of the lib to include that on their authentication request, what do you think @nraboy and @jdnichollsc ?

@jdnichollsc
Copy link
Contributor Author

@nraboy @matheusrocha89 Sorry for the delay, check the pull request #304

Best regards, Nicholls

@nraboy
Copy link
Owner

nraboy commented Nov 10, 2016

I am open to either as long as it doesn't force extra permissions. Would you like it to default or would you like it to be an option?

cc @matheusrocha89 @jdnichollsc

@jdnichollsc
Copy link
Contributor Author

Default option is good for me 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants