[GitHub] Set JH user's email with non-public email if needed and granted scope to do so

[satra]

This allows emails to be read and set in the state dictionary when scope is authorized.

closes #438

Related references

• https://docs.github.com/en/developers/apps/building-oauth-apps/scopes-for-oauth-apps#available-scopes
• https://docs.github.com/en/rest/guides/basics-of-authentication#checking-granted-scopes
• https://docs.github.com/en/rest/reference/users#list-email-addresses-for-the-authenticated-user
• https://docs.github.com/en/developers/apps/building-oauth-apps/authorizing-oauth-apps#response
  The scope field in the JSON response when acquiring the access token is not always present it seems.

[welcome]

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.

You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

[consideRatio]

Thank you @satra for working on this! ❤️ 🎉

Overall I think this make sense! There are some technical details I'd hope to get implemented in a robust way directly in this PR as I believe this PR is going to lead the way for additional PRs and become a pattern to make further use of the /user endpoint that returns more details about the user.

Suggested changes

• Include user alongisde user:email in the check to decide if collecting additional information from the user endpoint is relevant.
• I'd like the comment on line 194-195 to reflect the difference between the single public email, and the plural private emails
• I'd like to know this is at least manually tested against a user with multiple private emails registered, and that we understand the return value in those circumstances. Based on the code it seems like we get a primary email, then I think the action point I ask for is to clarify that while user:email provides info about all emails, we only use the primary email.
• Update PR description to include mentioning of https://docs.github.com/en/developers/apps/building-oauth-apps/scopes-for-oauth-apps#available-scopes as a reference about this

Open questions

self.scope is checked, but I think this is what the user has requested, not what was granted. Is there a point where we update self.scope (the server's spawner object's scope field) or where we keep track of the granted scopes vs the requested scopes?

There is no point for us to make a request if the granted scope lacks permissions, so can we acquire the granted scopes from the response when we get the token to call the /user endpoint perhaps?

https://docs.github.com/en/developers/apps/building-oauth-apps/scopes-for-oauth-apps#requested-scopes-and-granted-scopes

[satra]

@consideRatio - changing the logic to first check if public email is set, and relying on HTTP error rather than scope to set the email. i could not figure out a way to determine what scopes were granted, and it seemed simpler to rely on the endpoint.

[satra]

nevermind, found how to get scopes. so the current state should cover your comments.

[consideRatio]

This looks great! Thanks for your thorough work @satra! This will be included in 14.1.0 release coming this week I hope.

[welcome]

Congrats on your first merged pull request in this project! 🎉

Thank you for contributing, we are very proud of you! ❤️