diff --git a/app/models/twitter_user_info.rb b/app/models/twitter_user_info.rb deleted file mode 100644 index 07302bc7771..00000000000 --- a/app/models/twitter_user_info.rb +++ /dev/null @@ -1,21 +0,0 @@ -class TwitterUserInfo < ActiveRecord::Base - belongs_to :user -end - -# == Schema Information -# -# Table name: twitter_user_infos -# -# id :integer not null, primary key -# user_id :integer not null -# screen_name :string not null -# twitter_user_id :bigint(8) not null -# created_at :datetime not null -# updated_at :datetime not null -# email :string(1000) -# -# Indexes -# -# index_twitter_user_infos_on_twitter_user_id (twitter_user_id) UNIQUE -# index_twitter_user_infos_on_user_id (user_id) UNIQUE -# diff --git a/app/models/user.rb b/app/models/user.rb index 464740d4b47..dea7a5f2916 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -66,7 +66,6 @@ class User < ActiveRecord::Base has_one :user_option, dependent: :destroy has_one :user_avatar, dependent: :destroy has_many :user_associated_accounts, dependent: :destroy - has_one :twitter_user_info, dependent: :destroy has_one :github_user_info, dependent: :destroy has_one :google_user_info, dependent: :destroy has_many :oauth2_user_infos, dependent: :destroy diff --git a/app/services/user_anonymizer.rb b/app/services/user_anonymizer.rb index a1f227b0f2e..d87c7e6da7b 100644 --- a/app/services/user_anonymizer.rb +++ b/app/services/user_anonymizer.rb @@ -53,7 +53,6 @@ class UserAnonymizer end @user.user_avatar.try(:destroy) - @user.twitter_user_info.try(:destroy) @user.google_user_info.try(:destroy) @user.github_user_info.try(:destroy) @user.single_sign_on_record.try(:destroy) diff --git a/db/migrate/20181207141900_migrate_twitter_user_info.rb b/db/migrate/20181207141900_migrate_twitter_user_info.rb new file mode 100644 index 00000000000..f47798fe303 --- /dev/null +++ b/db/migrate/20181207141900_migrate_twitter_user_info.rb @@ -0,0 +1,27 @@ +class MigrateTwitterUserInfo < ActiveRecord::Migration[5.2] + def up + execute <<~SQL + INSERT INTO user_associated_accounts ( + provider_name, + provider_uid, + user_id, + info, + last_used, + created_at, + updated_at + ) SELECT + 'twitter', + twitter_user_id, + user_id, + json_build_object('email', email, 'nickname', screen_name), + updated_at, + created_at, + updated_at + FROM twitter_user_infos + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/auth/twitter_authenticator.rb b/lib/auth/twitter_authenticator.rb index be591a02357..06b4ee14b12 100644 --- a/lib/auth/twitter_authenticator.rb +++ b/lib/auth/twitter_authenticator.rb @@ -1,5 +1,4 @@ -class Auth::TwitterAuthenticator < Auth::Authenticator - +class Auth::TwitterAuthenticator < Auth::ManagedAuthenticator def name "twitter" end @@ -8,94 +7,10 @@ class Auth::TwitterAuthenticator < Auth::Authenticator SiteSetting.enable_twitter_logins end - def description_for_user(user) - info = TwitterUserInfo.find_by(user_id: user.id) - info&.email || info&.screen_name || "" - end - - def can_revoke? - true - end - - def revoke(user, skip_remote: false) - info = TwitterUserInfo.find_by(user_id: user.id) - raise Discourse::NotFound if info.nil? - - # We get a token from twitter upon login but do not need it, and do not store it. - # Therefore we do not have any way to revoke the token automatically on twitter's end - - info.destroy! - true - end - - def can_connect_existing_user? - true - end - def after_authenticate(auth_token, existing_account: nil) - result = Auth::Result.new - - data = auth_token[:info] - - result.email = data["email"] - result.email_valid = result.email.present? - result.username = data["nickname"] - result.name = data["name"] - twitter_user_id = auth_token["uid"] - - result.extra_data = { - twitter_email: result.email, - twitter_user_id: twitter_user_id, - twitter_screen_name: result.username, - twitter_image: data["image"], - twitter_description: data["description"], - twitter_location: data["location"] - } - - user_info = TwitterUserInfo.find_by(twitter_user_id: twitter_user_id) - - if existing_account && (user_info.nil? || existing_account.id != user_info.user_id) - user_info.destroy! if user_info - result.user = existing_account - user_info = TwitterUserInfo.create!( - user_id: result.user.id, - screen_name: result.username, - twitter_user_id: twitter_user_id, - email: result.email - ) - else - result.user = user_info&.user - end - - if (!result.user) && result.email_valid && (result.user = User.find_by_email(result.email)) - TwitterUserInfo.create!( - user_id: result.user.id, - screen_name: result.username, - twitter_user_id: twitter_user_id, - email: result.email - ) - end - - retrieve_avatar(result.user, result.extra_data) - retrieve_profile(result.user, result.extra_data) - - result - end - - def after_create_account(user, auth) - extra_data = auth[:extra_data] - - TwitterUserInfo.create( - user_id: user.id, - screen_name: extra_data[:twitter_screen_name], - twitter_user_id: extra_data[:twitter_user_id], - email: extra_data[:email] - ) - - retrieve_avatar(user, extra_data) - retrieve_profile(user, extra_data) - - true + # Twitter sends a huge amount of data which we don't need, so ignore it + auth_token[:extra] = {} + super end def register_middleware(omniauth) @@ -106,31 +21,4 @@ class Auth::TwitterAuthenticator < Auth::Authenticator strategy.options[:consumer_secret] = SiteSetting.twitter_consumer_secret } end - - protected - - def retrieve_avatar(user, data) - return unless user - return if user.user_avatar.try(:custom_upload_id).present? - - if (avatar_url = data[:twitter_image]).present? - url = avatar_url.sub("_normal", "") - Jobs.enqueue(:download_avatar_from_url, url: url, user_id: user.id, override_gravatar: false) - end - end - - def retrieve_profile(user, data) - return unless user - - bio = data[:twitter_description] - location = data[:twitter_location] - - if bio || location - profile = user.user_profile - profile.bio_raw = bio unless profile.bio_raw.present? - profile.location = location unless profile.location.present? - profile.save - end - end - end diff --git a/script/bulk_import/discourse_merger.rb b/script/bulk_import/discourse_merger.rb index bfaddc14f4a..1f6d6c9f51e 100644 --- a/script/bulk_import/discourse_merger.rb +++ b/script/bulk_import/discourse_merger.rb @@ -152,7 +152,7 @@ class BulkImport::DiscourseMerger < BulkImport::Base end [UserAssociatedAccount, GithubUserInfo, GoogleUserInfo, InstagramUserInfo, Oauth2UserInfo, - SingleSignOnRecord, TwitterUserInfo, EmailChangeRequest + SingleSignOnRecord, EmailChangeRequest ].each do |c| copy_model(c, skip_if_merged: true, is_a_user_model: true) end @@ -653,11 +653,6 @@ class BulkImport::DiscourseMerger < BulkImport::Base r end - def process_twitter_user_info(r) - return nil if TwitterUserInfo.where(twitter_user_id: r['twitter_user_id']).exists? - r - end - def process_user_badge(user_badge) user_badge['granted_by_id'] = user_id_from_imported_id(user_badge['granted_by_id']) if user_badge['granted_by_id'] user_badge['notification_id'] = notification_id_from_imported_id(user_badge['notification_id']) if user_badge['notification_id'] diff --git a/spec/components/auth/twitter_authenticator_spec.rb b/spec/components/auth/twitter_authenticator_spec.rb index f6b620dfdf3..208050cacf8 100644 --- a/spec/components/auth/twitter_authenticator_spec.rb +++ b/spec/components/auth/twitter_authenticator_spec.rb @@ -9,20 +9,21 @@ describe Auth::TwitterAuthenticator do auth_token = { info: { - "email" => user.email, - "username" => "test", - "name" => "test", - "nickname" => "minion", + email: user.email, + username: "test", + name: "test", + nickname: "minion", }, - "uid" => "123" + uid: "123", + provider: "twitter" } result = auth.after_authenticate(auth_token) expect(result.user.id).to eq(user.id) - info = TwitterUserInfo.find_by(user_id: user.id) - expect(info.email).to eq(user.email) + info = UserAssociatedAccount.find_by(provider_name: "twitter", user_id: user.id) + expect(info.info["email"]).to eq(user.email) end it 'can connect to a different existing user account' do @@ -30,23 +31,24 @@ describe Auth::TwitterAuthenticator do user1 = Fabricate(:user) user2 = Fabricate(:user) - TwitterUserInfo.create!(user_id: user1.id, twitter_user_id: 100, screen_name: "boris") + UserAssociatedAccount.create!(provider_name: "twitter", user_id: user1.id, provider_uid: 100) hash = { info: { - "email" => user1.email, - "username" => "test", - "name" => "test", - "nickname" => "minion", + email: user1.email, + username: "test", + name: "test", + nickname: "minion", }, - "uid" => "100" + uid: "100", + provider: "twitter" } result = authenticator.after_authenticate(hash, existing_account: user2) expect(result.user.id).to eq(user2.id) - expect(TwitterUserInfo.exists?(user_id: user1.id)).to eq(false) - expect(TwitterUserInfo.exists?(user_id: user2.id)).to eq(true) + expect(UserAssociatedAccount.exists?(provider_name: "twitter", user_id: user1.id)).to eq(false) + expect(UserAssociatedAccount.exists?(provider_name: "twitter", user_id: user2.id)).to eq(true) end context 'revoke' do @@ -58,7 +60,7 @@ describe Auth::TwitterAuthenticator do end it 'revokes correctly' do - TwitterUserInfo.create!(user_id: user.id, twitter_user_id: 100, screen_name: "boris") + UserAssociatedAccount.create!(provider_name: "twitter", user_id: user.id, provider_uid: 100) expect(authenticator.can_revoke?).to eq(true) expect(authenticator.revoke(user)).to eq(true) expect(authenticator.description_for_user(user)).to eq("") diff --git a/spec/models/twitter_user_info_spec.rb b/spec/models/twitter_user_info_spec.rb deleted file mode 100644 index 6c664068bad..00000000000 --- a/spec/models/twitter_user_info_spec.rb +++ /dev/null @@ -1,10 +0,0 @@ -require 'rails_helper' - -describe TwitterUserInfo do - it "does not overflow" do - id = 22019458041 - info = TwitterUserInfo.create!(user_id: -1, screen_name: 'sam', twitter_user_id: id) - info.reload - expect(info.twitter_user_id).to eq(id) - end -end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c4739dfeae0..0c1a35d6de2 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -418,7 +418,7 @@ describe User do user = Fabricate(:user) expect(user.associated_accounts).to eq([]) - TwitterUserInfo.create(user_id: user.id, screen_name: "sam", twitter_user_id: 1) + UserAssociatedAccount.create(user_id: user.id, provider_name: "twitter", provider_uid: "1", info: { nickname: "sam" }) UserAssociatedAccount.create(user_id: user.id, provider_name: "facebook", provider_uid: "1234", info: { email: "test@example.com" }) GoogleUserInfo.create(user_id: user.id, email: "sam@sam.com", google_user_id: 1) GithubUserInfo.create(user_id: user.id, screen_name: "sam", github_user_id: 1) diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 137649c3d8c..67521471593 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -835,7 +835,7 @@ describe UsersController do } expect(response.status).to eq(200) - expect(TwitterUserInfo.count).to eq(1) + expect(UserAssociatedAccount.where(provider_name: "twitter").count).to eq(1) end it "returns an error when email has been changed from the validated email address" do diff --git a/spec/services/user_anonymizer_spec.rb b/spec/services/user_anonymizer_spec.rb index 5b068393840..440a88f6a9c 100644 --- a/spec/services/user_anonymizer_spec.rb +++ b/spec/services/user_anonymizer_spec.rb @@ -176,7 +176,6 @@ describe UserAnonymizer do end it "removes external auth assocations" do - user.twitter_user_info = TwitterUserInfo.create(user_id: user.id, screen_name: "example", twitter_user_id: "examplel123123") user.google_user_info = GoogleUserInfo.create(user_id: user.id, google_user_id: "google@gmail.com") user.github_user_info = GithubUserInfo.create(user_id: user.id, screen_name: "example", github_user_id: "examplel123123") user.user_associated_accounts = [UserAssociatedAccount.create(user_id: user.id, provider_uid: "example", provider_name: "facebook")] @@ -186,7 +185,6 @@ describe UserAnonymizer do UserOpenId.create(user_id: user.id, email: user.email, url: "http://example.com/openid", active: true) make_anonymous user.reload - expect(user.twitter_user_info).to eq(nil) expect(user.google_user_info).to eq(nil) expect(user.github_user_info).to eq(nil) expect(user.user_associated_accounts).to be_empty diff --git a/spec/services/user_merger_spec.rb b/spec/services/user_merger_spec.rb index 19a5c9742e0..e7d67928127 100644 --- a/spec/services/user_merger_spec.rb +++ b/spec/services/user_merger_spec.rb @@ -970,7 +970,6 @@ describe UserMerger do InstagramUserInfo.create(user_id: source_user.id, screen_name: "example", instagram_user_id: "examplel123123") Oauth2UserInfo.create(user_id: source_user.id, uid: "example", provider: "example") SingleSignOnRecord.create(user_id: source_user.id, external_id: "example", last_payload: "looks good") - TwitterUserInfo.create(user_id: source_user.id, screen_name: "example", twitter_user_id: "examplel123123") UserOpenId.create(user_id: source_user.id, email: source_user.email, url: "http://example.com/openid", active: true) merge_users! @@ -981,7 +980,6 @@ describe UserMerger do expect(InstagramUserInfo.where(user_id: source_user.id).count).to eq(0) expect(Oauth2UserInfo.where(user_id: source_user.id).count).to eq(0) expect(SingleSignOnRecord.where(user_id: source_user.id).count).to eq(0) - expect(TwitterUserInfo.where(user_id: source_user.id).count).to eq(0) expect(UserOpenId.where(user_id: source_user.id).count).to eq(0) end