-
Notifications
You must be signed in to change notification settings - Fork 102
Feature: Linking X/Twitter Account in the Profile and Logging in via X oauth #123
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
base: master
Are you sure you want to change the base?
Conversation
…ion during X redirect
…d apply proper cleanup to twitter_url and twitter_username rows when disconnecting the X account
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @dvassallo! Here is my self review. Please let me know if any changes or further explanation required. Thanks!
| gem "omniauth" | ||
| gem "omniauth-twitter2" | ||
| gem "omniauth-rails_csrf_protection" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These three gems are recommended on X documentation page here.
omniauth-twitter2 is dependent on omniauth.
omniauth-rails_csrf_protection adds CSRF protection to omniauth.
|
|
||
| <% unless @user.twitter_connected_at.present? %> | ||
| <form id="x-oauth-form" action="/auth/twitter2" method="post" style="display: none;"> | ||
| <input type="hidden" name="authenticity_token" value="<%= form_authenticity_token %>"> | ||
| </form> | ||
| <% end %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the form where it initiates the X account linking by calling the auth/twitter2 endpoint.
| class AddTwitterOAuthToUsers < ActiveRecord::Migration[7.2] | ||
| def change | ||
| add_column :users, :twitter_uid, :string | ||
| add_column :users, :twitter_oauth_token, :text | ||
| add_column :users, :twitter_oauth_refresh_token, :text | ||
| add_column :users, :twitter_screen_name, :string | ||
| add_column :users, :twitter_profile_image, :string | ||
| add_column :users, :twitter_connected_at, :datetime | ||
|
|
||
| add_index :users, :twitter_uid, unique: true | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adds OAuth db fields to User table. This will be used for refreshing the session and logging in during X OAuth flow.
| Rails.application.config.middleware.use OmniAuth::Builder do | ||
| provider :twitter2, | ||
| ENV["X_CLIENT_ID"], | ||
| ENV["X_CLIENT_SECRET"], | ||
| scope: "tweet.read users.read offline.access" | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Configures OmniAuth middleware with X client ID/secret and required scopes.
| get "/auth/failure", to: "users/omniauth_callbacks#failure" | ||
| delete "/auth/twitter/disconnect", to: "users/omniauth_callbacks#disconnect" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two endpoints handle auth failures and account disconnection.
|
|
||
| validates_presence_of :email_address, if: :person? | ||
| normalizes :email_address, with: ->(email_address) { email_address.downcase } | ||
| validates :twitter_uid, uniqueness: true, allow_nil: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevents one X account from being linked to multiple users.
| def twitter_connected? | ||
| twitter_uid.present? && twitter_connected_at.present? | ||
| end | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Helper to check if account is linked. This is only used in the test to make sure that the X account is linked properly.
| def mock_auth_hash(uid: "123456789", nickname: "testuser") | ||
| OmniAuth::AuthHash.new({ | ||
| "provider" => "twitter2", | ||
| "uid" => uid, | ||
| "info" => { | ||
| "nickname" => nickname, | ||
| "image" => "https://pbs.twimg.com/profile_images/test.jpg" | ||
| }, | ||
| "credentials" => { | ||
| "token" => "test_token", | ||
| "refresh_token" => "test_refresh" | ||
| } | ||
| }) | ||
| end | ||
|
|
||
| test "twitter callback for login signs in user with linked X account" do | ||
| user = users(:david) | ||
| user.update!(twitter_uid: "123456789", twitter_oauth_token: "token") | ||
|
|
||
| OmniAuth.config.mock_auth[:twitter2] = mock_auth_hash | ||
|
|
||
| post "/auth/twitter2/login" | ||
| assert_response :success | ||
|
|
||
| get "/auth/twitter2/callback" | ||
|
|
||
| assert_redirected_to root_path | ||
| assert_equal "Signed in with X successfully!", flash[:notice] | ||
| end | ||
|
|
||
| test "twitter callback for login rejects user without linked X account" do | ||
| OmniAuth.config.mock_auth[:twitter2] = mock_auth_hash(uid: "nonexistent") | ||
|
|
||
| post "/auth/twitter2/login" | ||
| get "/auth/twitter2/callback" | ||
|
|
||
| assert_redirected_to new_session_path | ||
| assert_equal "No account found with this X account. Please sign in with email first and connect your X account.", flash[:alert] | ||
| end | ||
|
|
||
| test "twitter callback for connect redirects when not signed in" do | ||
| OmniAuth.config.mock_auth[:twitter2] = mock_auth_hash | ||
|
|
||
| get "/auth/twitter2/callback" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests login flow (success and failure cases)
|
|
||
| test "disconnect clears X account data" do | ||
| sign_in :david | ||
| user = users(:david) | ||
| user.update!( | ||
| twitter_uid: "123456789", | ||
| twitter_oauth_token: "token", | ||
| twitter_oauth_refresh_token: "refresh", | ||
| twitter_screen_name: "testuser", | ||
| twitter_profile_image: "https://example.com/image.jpg", | ||
| twitter_connected_at: Time.current | ||
| ) | ||
|
|
||
| delete "/auth/twitter/disconnect" | ||
|
|
||
| assert_redirected_to user_profile_path("me") | ||
| assert_equal "X account disconnected successfully!", flash[:notice] | ||
|
|
||
| user.reload | ||
| assert_nil user.twitter_uid | ||
| assert_nil user.twitter_oauth_token | ||
| assert_nil user.twitter_screen_name | ||
| assert_nil user.twitter_connected_at | ||
| end | ||
|
|
||
| test "disconnect requires authentication" do | ||
| delete "/auth/twitter/disconnect" | ||
|
|
||
| assert_redirected_to new_session_path | ||
| end | ||
|
|
||
| test "initiate_login renders auto-submit form" do | ||
| post "/auth/twitter2/login" | ||
|
|
||
| assert_response :success | ||
| assert_includes response.body, 'action="/auth/twitter2"' | ||
| end | ||
|
|
||
| test "failure redirects with error message" do | ||
| get "/auth/failure", params: { message: "access_denied" } | ||
|
|
||
| assert_redirected_to user_profile_path("me") | ||
| assert_equal "X authentication failed: access_denied", flash[:alert] | ||
| end | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests disconnect functionality.
| test "user twitter_uid must be unique" do | ||
| users(:david).update!(twitter_uid: "123456789") | ||
|
|
||
| user = users(:jason) | ||
| user.twitter_uid = "123456789" | ||
|
|
||
| assert_not user.valid? | ||
| assert_includes user.errors[:twitter_uid], "has already been taken" | ||
| end | ||
|
|
||
| test "twitter_connected? returns true when uid and connected_at present" do | ||
| user = users(:david) | ||
| user.update!(twitter_uid: "123", twitter_connected_at: Time.current) | ||
|
|
||
| assert user.twitter_connected? | ||
| end | ||
|
|
||
| test "twitter_connected? returns false when uid missing" do | ||
| user = users(:david) | ||
| user.update!(twitter_uid: nil, twitter_connected_at: Time.current) | ||
|
|
||
| assert_not user.twitter_connected? | ||
| end | ||
|
|
||
| test "twitter_connected? returns false when connected_at missing" do | ||
| user = users(:david) | ||
| user.update!(twitter_uid: "123", twitter_connected_at: nil) | ||
|
|
||
| assert_not user.twitter_connected? | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test proper X account connection and account uniqueness.
Hello all, this PR closes issue #20.
Users can:
Changes:
omniauth,omniauth-twitter2, andomniauth-rails_csrf_protectiongemstwitter_uid,twitter_oauth_token,twitter_oauth_refresh_token, etc.)twitter_uidTesting:
AI Disclosure:
Test Execution result before code changes:

Test Execution result after code changes plus the new tests added for this feature:

Demo:
https://github.com/user-attachments/assets/4264f624-f82a-454f-8a0c-2c03a64a62e3