GitHub profile.email: nil
nelsonic opened this issue · 19 comments
I've just discovered that it's possible to have a GitHub profile without an email address ... 🤦
email is the only required field in a person. So the whole thing explodes.
going to need to add a guard clause and figure out if this is viable for us.
we might need to just disable GitHub Auth in our MVP and stick to Google or Email+Password. 💭
we will need to have a way of requesting the person's email address if their GitHub profile does not have one ... perhaps we can reuse the Email registration form. 🤔
Something to consider after MVP is launched. 👍
I can re-create this with my own account, just make email "Not visible":
If the GitHub profile does not have an Email key:value we need to redirect to an email form.
This is not strictly relevant to elixir-auth-github, it's more of "consumer" problem.
Different but related error:
Going to need to address this pretty soon as we want to use GitHub Auth to validate accounts. ⏳
Thinking we should update our Model (Schema) in Auth to allow for an Alias when an Email Address is unavailable.
As noted in the following issue: dwyl/smart-home-auth-server#14 (comment)
I don't think using the Github username instead of the email is a good idea as the username could be used to access data of an other person
From https://github.community/t/oauth-and-private-email-address/13550
So we might still be able to access a private email with Oauth using the right scope, https://docs.github.com/en/developers/apps/scopes-for-oauth-apps#requested-scopes-and-granted-scopes
However it seems that auth is already using scopes:
https://github.com/dwyl/auth/blob/master/lib/auth_web/controllers/auth_controller.ex#L60
indeed, we are requesting the email address already in the auth app:
oauth_github_url = ElixirAuthGithub.login_url(%{scopes: ["user:email"], state: state})If there is anything else we can do from a configuration perspective, please share / try it. 👍
I've added some logs to see if we could access the person's email when it is defined as private in the Github settings but it is indeed returned as nil even when using the user:email scope.
At this stage I think the best solution is to redirect to an error page on the auth application which explain how to make an email public via Github. The person has still the possibility to authenticate with Gmail or email/password otherwise
@nelsonic our preference would be to redirect to a page requiring the person to "Add Email Address" which will add it to person.email and send them a verification mail.
My thought for having just an error page is to keep the authentication simple.
With an email field page we will need to make sure that the email is updated if the person add a public email address on Github.
@nelsonic yeah, we already have code to make the update if they update their email address.
https://github.com/dwyl/auth/blob/fafc5e059d65de420b4d99bf1d15ac45a3bcda12/lib/auth/person.ex#L104-L115
Basically each time the person logs in using GitHub (or Google) account we update all their data to the latest. 👍
However if we capture their email manually using a form and they then login with their GitHub account without email, we may need to continue asking them for their email address or we need a check in our upsert_person/1 function that we are not overwriting the data with nil. 💭 (but this is fairly easy to add).
Another issue when we ask the user to define the email manually (when the email is private on Github) is at each authentication we don't have a way to know if the person as already enter the email manually and the page asking for the email will always be displayed. This could also create user duplicates. We could check if a user exist in the database by looking for the username but as mentioned above #46 (comment) the username is not really unique
@SimonLab this isn't very complicated logic, we just need to write it out precisely,
preferably with a diagram so that anyone else can understand+maintain it.
GitHub always returns the following profile info:
profile: %{
public_gists: 32,
name: "Nelson",
type: "User",
updated_at: "2020-08-23T22:30:55Z",
events_url: "https://api.github.com/users/nelsonic/events{/privacy}",
twitter_username: nil,
site_admin: false,
bio: "blah blah...",
organizations_url: "https://api.github.com/users/nelsonic/orgs",
repos_url: "https://api.github.com/users/nelsonic/repos",
avatar_url: "https://avatars3.githubusercontent.com/u/194400?v=4",
created_at: "2010-02-02T08:44:49Z",
login: "nelsonic",
email: nil,
following_url: "https://api.github.com/users/nelsonic/following{/other_user}",
url: "https://api.github.com/users/nelsonic",
subscriptions_url: "https://api.github.com/users/nelsonic/subscriptions",
public_repos: 298,
gists_url: "https://api.github.com/users/nelsonic/gists{/gist_id}",
company: "@dwyl",
location: "London",
blog: "https://dwyl.com",
starred_url: "https://api.github.com/users/nelsonic/starred{/owner}{/repo}",
access_token: "redacted",
received_events_url: "https://api.github.com/users/nelsonic/received_events",
id: 194400,
followers_url: "https://api.github.com/users/nelsonic/followers",
html_url: "https://github.com/nelsonic",
followers: 2942
}The important bit is that email: nil ... 🚫
We have the concept of a username in auth:
https://github.com/dwyl/auth/blob/fafc5e059d65de420b4d99bf1d15ac45a3bcda12/lib/auth/person.ex#L18
So we can create a Person.changeset_github/2 specific to the GitHub "User" where the profile.login is used as their person.username. And then once we have created the person (without an email address), we can make a GET request to /user/emails using the user's profile.access_token:
GET https://api.github.com/user/emails[
{
"email": "nelson@gmail.com",
"primary": true,
"verified": true
}
]See: https://stackoverflow.com/questions/35373995/github-user-email-is-null-despite-useremail-scope
This should avoid any additional UI or data entry from the GitHub User.
Please try it. 👍
Thanks! 🌻
https://hex.pm/packages/elixir_auth_github/1.4.0 published. Thanks @SimonLab 👍
Works on localhost:
Busy updating the demo app now: https://github.com/dwyl/elixir-auth-github-demo ⏳ 👨💻
Fixed. Closing. ✅