Allow login for old Twitter users with same email (#1067)

Allow login for old Twitter users with same email

Author: josepegea

app/models/authorization.rb

@@ -14,9 +14,35 @@ def self.handle_authorization(existing_user, auth)
     if authorization.present?
       authorization.user.update_from_auth! auth
     else
-      user = existing_user || User.create_from_hash!(auth)
+      user = existing_user || fallback_user_for_authorization(auth) || User.create_from_hash!(auth)
       authorization = user.authorizations.create! provider:, uid:
     end
     authorization
   end
+
+  def self.fallback_user_for_authorization(auth)
+    return unless user_fallback_in_place?
+
+    email = auth.dig('info', 'email')
+    return if email.blank?
+
+    users_with_email = User.where(email:)
+    return unless users_with_email.count == 1
+
+    user = users_with_email.first
+    return unless user.authorizations.count == 1 && user.authorizations.first.provider == 'twitter'
+
+    user.update_from_auth!(auth)
+  end
+
+  DEFAULT_TWITTER_USER_FALLBACK_DEADLINE = Date.new(2025, 1, 1)
+
+  def self.user_fallback_in_place?
+    deadline = begin
+      Date.parse(ENV['TWITTER_USER_FALLBACK_DEADLINE'])
+    rescue Date::Error, TypeError
+      DEFAULT_TWITTER_USER_FALLBACK_DEADLINE
+    end
+    deadline.future?
+  end
 end

readme.md

@@ -169,6 +169,36 @@ On the admin site we need to:
 - deploy to Heroku
 - add admin privileges to someone for the new RUG
 
+## Users Login
+
+In order to register for events, users need to log in first.
+
+The app uses OAuth for that. At this point, it accepts Twitter [^1], GitHub and Google as valid OAuth providers.
+
+When a user is not currently logged in, selecting one of the login providers creates an account for him/her, attaching this provider as a valid auth mechanism.
+
+When a user is already logged in, selecting another provider will add that auth mechanism to the existing user.
+
+[^1]: As of 2024 Twitter/X has deprecated API v1.1 and Twitter login is not working anymore
+
+### Recovering login for existing users who only had Twitter auth
+
+As a consequence of X deprecating API v1.1, Twitter authentication is not working anymore and registered users that only had defined Twitter as their auth method are left out in the cold.
+
+In order to provide an easy way for these users to access their existing account using another auth provider, the app includes additional logic:
+
+1. If a new session successfully authenticates using a provider that includes email as part of their info.
+2. If a user (and only one user) already exists with that same email
+3. If that user was using Twitter (and only Twitter) as OAuth provider
+
+If all of these conditions are met, then the new provider is added to the existing user and the login proceeds with that.
+
+If the existing user had other providers already registered, then this logic doesn't kick in, as they can still use one of the others to log in.
+
+As this behaviour could lead to potential account takeovers (if another user had access to an OAuth account with the same email of the original user) there's a deadline after which it will stop working.
+
+The default deadline is the end of year 2024. If needed it can be ajusted by setting the environment variable `TWITTER_USER_FALLBACK_DEADLINE` to a valid date address string (like '2025-06-30').
+
 ## Website
 
 ![OnRuby Website](https://cl.ly/image/3U0S3b0T0F0x/Screen%20Shot%202014-01-07%20at%2014.16.44.png)

spec/models/authorization_spec.rb

@@ -8,4 +8,126 @@
       end.to change(User, :count).by(1)
     end.to change(Authorization, :count).by(1)
   end
+
+  describe 'special treatment for stale Twitter users' do
+    let(:existing_twitter_auth) { Authorization.handle_authorization(nil, TWITTER_AUTH_HASH) }
+    let(:existing_twitter_user) { existing_twitter_auth.user }
+    let(:new_auth_hash) { GITHUB_AUTH_HASH }
+    let(:date_of_test) { Authorization::DEFAULT_TWITTER_USER_FALLBACK_DEADLINE - 1 }
+
+    around do |example|
+      travel_to(date_of_test) do
+        example.run
+      end
+    end
+
+    shared_examples 'failing with duplicate nickname' do
+      it 'raises duplication error' do
+        expect do
+          Authorization.handle_authorization(nil, new_auth_hash)
+        end.to raise_error(User::DuplicateNickname)
+      end
+    end
+
+    shared_examples 'updating the existing user' do
+      it 'adds the auth to the existing Twitter account', :aggregate_failures do
+        gh_auth = Authorization.handle_authorization(nil, new_auth_hash)
+        expect(gh_auth.user).to eq(existing_twitter_user)
+        expect(existing_twitter_user.reload.github).to eq(new_auth_hash.dig('info', 'nickname'))
+      end
+    end
+
+    before do
+      existing_twitter_user
+      create(:authorization, provider: 'twitter', user: create(:user, email: nil))
+    end
+
+    context 'when emails do not match' do
+      context 'when nicknames are the same' do
+        it_behaves_like 'failing with duplicate nickname'
+      end
+
+      context 'when nicknames are different' do
+        before { existing_twitter_user.update!(nickname: existing_twitter_user.nickname * 2) }
+
+        it 'creates a new user', :aggregate_failures do
+          gh_user = Authorization.handle_authorization(nil, new_auth_hash).user
+          expect(gh_user).to be_persisted
+          expect(gh_user).not_to eq(existing_twitter_user)
+        end
+      end
+    end
+
+    context 'when emails match' do
+      before { existing_twitter_user.update!(email: new_auth_hash.dig('info', 'email')) }
+
+      context 'when email is unique and user has only Twiter auth' do
+        around do |example|
+          value_before = ENV['TWITTER_USER_FALLBACK_DEADLINE']
+          ENV['TWITTER_USER_FALLBACK_DEADLINE'] = deadline_value
+          example.run
+          ENV['TWITTER_USER_FALLBACK_DEADLINE'] = value_before
+        end
+
+        shared_examples 'updating the user before the deadline, failing afterwards' do
+          context 'before the deadline' do
+            let(:date_of_test) { effective_deadline - 1 }
+
+            it_behaves_like 'updating the existing user'
+          end
+
+          context 'with a valid value and after that deadline' do
+            let(:date_of_test) { effective_deadline + 1 }
+
+            it_behaves_like 'failing with duplicate nickname'
+          end
+        end
+
+        context 'with no explicit deadline' do
+          let(:deadline_value) { nil }
+          let(:effective_deadline) { Authorization::DEFAULT_TWITTER_USER_FALLBACK_DEADLINE }
+
+          it_behaves_like 'updating the user before the deadline, failing afterwards'
+        end
+
+        context 'with a valid explicit deadline' do
+          let(:deadline_value) { '2026-01-01' }
+          let(:effective_deadline) { Date.parse('2026-01-01') }
+
+          it_behaves_like 'updating the user before the deadline, failing afterwards'
+        end
+
+        context 'with an invalid explicit deadline' do
+          let(:deadline_value) { 'bad date!' }
+          let(:effective_deadline) { Authorization::DEFAULT_TWITTER_USER_FALLBACK_DEADLINE }
+
+          it_behaves_like 'updating the user before the deadline, failing afterwards'
+        end
+      end
+
+      context 'when several users have the same email' do
+        before { create(:user, email: existing_twitter_user.email) }
+
+        it_behaves_like 'failing with duplicate nickname'
+      end
+
+      context 'when the user has other authorizations' do
+        before { create(:authorization, user: existing_twitter_user) }
+
+        it_behaves_like 'failing with duplicate nickname'
+      end
+
+      context 'when the user has a single auth but it is not Twitter' do
+        before { existing_twitter_user.authorizations = [create(:authorization)] }
+
+        it_behaves_like 'failing with duplicate nickname'
+      end
+    end
+
+    context 'when new auth has no email' do
+      let(:new_auth_hash) { GITHUB_AUTH_HASH.deep_merge('info' => { 'email' => nil }) }
+
+      it_behaves_like 'failing with duplicate nickname'
+    end
+  end
 end

spec/spec_helper.rb

@@ -12,6 +12,7 @@
   config.include KaminariHelper
   config.include GeocoderHelper
   config.include FactoryBot::Syntax::Methods
+  config.include ActiveSupport::Testing::TimeHelpers
 
   # config.raise_errors_for_deprecations!
   config.infer_spec_type_from_file_location!