GitHub Login Implementation #107
Conversation
app/models/user.rb
Outdated
| end | ||
| def from_omniauth(access_token) | ||
| data = access_token.info | ||
| user = User.where(email: data['email']).first |
There was a problem hiding this comment.
@IMPATIENT89 What would happen if I change my email on Github?
The next time I login to WhyLoveRuby would I be able to Edit my older posts?
There was a problem hiding this comment.
Okay so I have just to add a update option for the email changing.
There was a problem hiding this comment.
@IMPATIENT89 I think we should save the Github User Id instead of their email(same approach is followed in the find_for_twitter_oauth method just above this). So regardless of the User changing their emails on their Github account, we will always map to the correct User on our system.
We could use the uid and provider columns for this, if we know for sure we are going to retire Twitter and use Github instead (post in Slack) - @rtdp
There was a problem hiding this comment.
@IMPATIENT89 we can't reuse the uid, and provider columns.
As a quick measure, we can simply add github_ prefixed columns to the User table like github_uid to resolve this.
Another approach would be to refactor our existing setup, to allow an User to have multiple modes of logging in.
Perhaps a model named ExternalIdentity (belongs_to :user) with columns :uid, :provider at the minimum and any other columns as necessary.
So Twitter and Github logins will each have a separate row on the ExternalIdentity table.
Adding a Sign in with Google in future would be just adding a row to this table instead of requiring a DB migration.
Let me know what you think of this and which approach you would like to follow.
| @@ -0,0 +1,5 @@ | |||
| class AddGithubFieldToUsers < ActiveRecord::Migration[6.1] | |||
| def change | |||
| add_column :users, :git_username, :string | |||
There was a problem hiding this comment.
Git username is different from Github handle. E.g. I could use GitLab as my git provider. So I think we should rename this to column to reflect github.
There was a problem hiding this comment.
i added git_username but it is reflection your github handle. In my case git_username is IMPATIENT89
There was a problem hiding this comment.
Yeah. I suggest we rename the column name to github_handle in that case @IMPATIENT89
app/models/user.rb
Outdated
| email: data['email'], | ||
| git_username: access_token.extra.raw_info.login, | ||
| image: access_token.extra.raw_info.avatar_url, | ||
| password: Devise.friendly_token[0, 20] |
There was a problem hiding this comment.
No, it's just for setup authorization
There was a problem hiding this comment.
Can you remove the line setting the password attribute and try if that breaks anything? If it doesn't we should just remove the line. Let us know if you find there is a need to keep that line.
| class ChangeGitUsernameToGithubHandle < ActiveRecord::Migration[6.1] | ||
| def change | ||
| rename_column :users, :git_username, :github_handle | ||
| end |
There was a problem hiding this comment.
since we haven't deployed the Database migration to the Production server, we should modify the original add_column migration to have the new name.
the rename_column migration can be removed.
On your local make sure to
- first rollback the 2 migrations
- delete the second (rename_column) migration
- edit the first (add_column) migration with the new column name
- run db:migrate
Implemented GitHub Login using OmniAuth.
Updated devise gem and omniauth gem