diff --git a/db/migrate/20190227150413_migrate_instagram_user_info.rb b/db/migrate/20190227150413_migrate_instagram_user_info.rb new file mode 100644 index 00000000000..37797fdc486 --- /dev/null +++ b/db/migrate/20190227150413_migrate_instagram_user_info.rb @@ -0,0 +1,27 @@ +class MigrateInstagramUserInfo < 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 + 'instagram', + instagram_user_id, + user_id, + json_build_object('nickname', screen_name), + updated_at, + created_at, + updated_at + FROM instagram_user_infos + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/auth/instagram_authenticator.rb b/lib/auth/instagram_authenticator.rb index 515616fbbcc..5ecb3a14884 100644 --- a/lib/auth/instagram_authenticator.rb +++ b/lib/auth/instagram_authenticator.rb @@ -1,5 +1,4 @@ -class Auth::InstagramAuthenticator < Auth::Authenticator - +class Auth::InstagramAuthenticator < Auth::ManagedAuthenticator def name "instagram" end @@ -8,67 +7,6 @@ class Auth::InstagramAuthenticator < Auth::Authenticator SiteSetting.enable_instagram_logins end - def description_for_user(user) - info = InstagramUserInfo.find_by(user_id: user.id) - info&.screen_name || "" - end - - def can_revoke? - true - end - - def revoke(user, skip_remote: false) - info = InstagramUserInfo.find_by(user_id: user.id) - raise Discourse::NotFound if info.nil? - # Instagram does not have any way for us to revoke tokens on their 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.username = screen_name = data["nickname"] - result.name = name = data["name"].slice!(0) - instagram_user_id = auth_token["uid"] - - result.extra_data = { - instagram_user_id: instagram_user_id, - instagram_screen_name: screen_name - } - - user_info = InstagramUserInfo.find_by(instagram_user_id: instagram_user_id) - - if existing_account && (user_info.nil? || existing_account.id != user_info.user_id) - user_info.destroy! if user_info - user_info = InstagramUserInfo.create!( - user_id: existing_account.id, - screen_name: screen_name, - instagram_user_id: instagram_user_id - ) - end - - result.user = user_info&.user - - result - end - - def after_create_account(user, auth) - data = auth[:extra_data] - InstagramUserInfo.create( - user_id: user.id, - screen_name: data[:instagram_screen_name], - instagram_user_id: data[:instagram_user_id] - ) - end - def register_middleware(omniauth) omniauth.provider :instagram, setup: lambda { |env| @@ -77,5 +15,4 @@ class Auth::InstagramAuthenticator < Auth::Authenticator strategy.options[:client_secret] = SiteSetting.instagram_consumer_secret } end - end diff --git a/script/bulk_import/discourse_merger.rb b/script/bulk_import/discourse_merger.rb index 1f6d6c9f51e..2ad24990f29 100644 --- a/script/bulk_import/discourse_merger.rb +++ b/script/bulk_import/discourse_merger.rb @@ -151,7 +151,7 @@ class BulkImport::DiscourseMerger < BulkImport::Base copy_model(c, skip_if_merged: true, is_a_user_model: true, skip_processing: true) end - [UserAssociatedAccount, GithubUserInfo, GoogleUserInfo, InstagramUserInfo, Oauth2UserInfo, + [UserAssociatedAccount, GithubUserInfo, GoogleUserInfo, Oauth2UserInfo, SingleSignOnRecord, EmailChangeRequest ].each do |c| copy_model(c, skip_if_merged: true, is_a_user_model: true) @@ -633,11 +633,6 @@ class BulkImport::DiscourseMerger < BulkImport::Base r end - def process_instagram_user_info(r) - return nil if InstagramUserInfo.where(instagram_user_id: r['instagram_user_id']).exists? - r - end - def process_oauth2_user_info(r) return nil if Oauth2UserInfo.where(uid: r['uid'], provider: r['provider']).exists? r diff --git a/spec/components/auth/instagram_authenticator_spec.rb b/spec/components/auth/instagram_authenticator_spec.rb new file mode 100644 index 00000000000..d8a30753bd0 --- /dev/null +++ b/spec/components/auth/instagram_authenticator_spec.rb @@ -0,0 +1,58 @@ +require 'rails_helper' + +describe Auth::InstagramAuthenticator do + + it "takes over account if email is supplied" do + auth = Auth::InstagramAuthenticator.new + + user = Fabricate(:user) + + auth_token = { + info: { email: user.email }, + uid: "123", + provider: "instagram" + } + + result = auth.after_authenticate(auth_token) + + expect(result.user.id).to eq(user.id) + + info = UserAssociatedAccount.find_by(provider_name: "instagram", user_id: user.id) + expect(info.info["email"]).to eq(user.email) + end + + it 'can connect to a different existing user account' do + authenticator = Auth::InstagramAuthenticator.new + user1 = Fabricate(:user) + user2 = Fabricate(:user) + + hash = { + info: { email: user1.email }, + uid: "100", + provider: "instagram" + } + + result = authenticator.after_authenticate(hash, existing_account: user2) + + expect(result.user.id).to eq(user2.id) + expect(UserAssociatedAccount.exists?(provider_name: "instagram", user_id: user1.id)).to eq(false) + expect(UserAssociatedAccount.exists?(provider_name: "instagram", user_id: user2.id)).to eq(true) + end + + context 'revoke' do + let(:user) { Fabricate(:user) } + let(:authenticator) { Auth::InstagramAuthenticator.new } + + it 'raises exception if no entry for user' do + expect { authenticator.revoke(user) }.to raise_error(Discourse::NotFound) + end + + it 'revokes correctly' do + UserAssociatedAccount.create!(provider_name: "instagram", 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("") + end + end + +end diff --git a/spec/jobs/invalidate_inactive_admins_spec.rb b/spec/jobs/invalidate_inactive_admins_spec.rb index 62cacc3dec3..6fd0f309c40 100644 --- a/spec/jobs/invalidate_inactive_admins_spec.rb +++ b/spec/jobs/invalidate_inactive_admins_spec.rb @@ -37,7 +37,6 @@ describe Jobs::InvalidateInactiveAdmins do context 'with social logins' do before do GithubUserInfo.create!(user_id: not_seen_admin.id, screen_name: 'bob', github_user_id: 100) - InstagramUserInfo.create!(user_id: not_seen_admin.id, screen_name: 'bob', instagram_user_id: 'examplel123123') UserOpenId.create!(url: 'https://me.yahoo.com/id/123' , user_id: not_seen_admin.id, email: 'bob@example.com', active: true) GoogleUserInfo.create!(user_id: not_seen_admin.id, google_user_id: 100, email: 'bob@example.com') end @@ -45,7 +44,6 @@ describe Jobs::InvalidateInactiveAdmins do it 'removes the social logins' do subject expect(GithubUserInfo.where(user_id: not_seen_admin.id).exists?).to eq(false) - expect(InstagramUserInfo.where(user_id: not_seen_admin.id).exists?).to eq(false) expect(GoogleUserInfo.where(user_id: not_seen_admin.id).exists?).to eq(false) expect(UserOpenId.where(user_id: not_seen_admin.id).exists?).to eq(false) end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d4e8c35638f..0bbea094a75 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -427,9 +427,9 @@ describe User do 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" }) + UserAssociatedAccount.create(user_id: user.id, provider_name: "instagram", provider_uid: "examplel123123", info: { nickname: "sam" }) 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) - InstagramUserInfo.create(user_id: user.id, screen_name: "sam", instagram_user_id: "examplel123123") user.reload expect(user.associated_accounts.map { |a| a[:name] }).to contain_exactly('twitter', 'facebook', 'google_oauth2', 'github', 'instagram') diff --git a/spec/services/user_anonymizer_spec.rb b/spec/services/user_anonymizer_spec.rb index 6f1e3483be2..f1b396032e9 100644 --- a/spec/services/user_anonymizer_spec.rb +++ b/spec/services/user_anonymizer_spec.rb @@ -195,7 +195,6 @@ describe UserAnonymizer do user.user_associated_accounts = [UserAssociatedAccount.create(user_id: user.id, provider_uid: "example", provider_name: "facebook")] user.single_sign_on_record = SingleSignOnRecord.create(user_id: user.id, external_id: "example", last_payload: "looks good") user.oauth2_user_infos = [Oauth2UserInfo.create(user_id: user.id, uid: "example", provider: "example")] - user.instagram_user_info = InstagramUserInfo.create(user_id: user.id, screen_name: "example", instagram_user_id: "examplel123123") UserOpenId.create(user_id: user.id, email: user.email, url: "http://example.com/openid", active: true) make_anonymous user.reload diff --git a/spec/services/user_merger_spec.rb b/spec/services/user_merger_spec.rb index e53cb9f1bbb..304bb0cdb64 100644 --- a/spec/services/user_merger_spec.rb +++ b/spec/services/user_merger_spec.rb @@ -979,7 +979,6 @@ describe UserMerger do UserAssociatedAccount.create(user_id: source_user.id, provider_name: "facebook", provider_uid: "1234") GithubUserInfo.create(user_id: source_user.id, screen_name: "example", github_user_id: "examplel123123") GoogleUserInfo.create(user_id: source_user.id, google_user_id: "google@gmail.com") - 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") UserOpenId.create(user_id: source_user.id, email: source_user.email, url: "http://example.com/openid", active: true) @@ -989,7 +988,6 @@ describe UserMerger do expect(UserAssociatedAccount.where(user_id: source_user.id).count).to eq(0) expect(GithubUserInfo.where(user_id: source_user.id).count).to eq(0) expect(GoogleUserInfo.where(user_id: source_user.id).count).to eq(0) - 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(UserOpenId.where(user_id: source_user.id).count).to eq(0)