From 86a90d287c2b27ec74be312e80fefea8d3f85653 Mon Sep 17 00:00:00 2001
From: Ryan Lue <rl@zammad.com>
Date: Thu, 7 Nov 2019 14:21:46 +0800
Subject: [PATCH] Fix issue #2460 - corrupted IDs on Twitter channels break
 Direct Message functionality

In some cases, editing a Twitter account via the admin panel
would cause new DMs to stop coming in. Why?

All Twitter usernames correspond to a numeric user ID
(see https://tweeterid.com/).
Zammad stores this ID on Twitter channels
(under `channel.options['user']['id']`)
and uses it to monitor incoming DMs.

Zammad was storing these user IDs to the DB as integers,
then passing those values to the frontend (JS) for account editing.
Some user IDs are longer than JavaScript's `Number.MAX_SAFE_INTEGER`,
and so got truncated on their way back to the update endpoint
(e.g., 984608854112718800 instead of 984608854112718848).

This commit enforces string storage of Twitter user IDs on new channels,
and includes a migration to verify and stringify IDs on existing ones.

Zammad Community: https://community.zammad.org/t/1799/34
GitHub: https://github.com/zammad/zammad/issues/2460
---
 .rubocop_todo.rspec.yml                       |  1 +
 ...49_issue_2460_fix_corrupted_twitter_ids.rb | 19 +++++++++++++++++++
 lib/external_credential/twitter.rb            |  7 +++----
 ...sue_2460_fix_corrupted_twitter_ids_spec.rb | 19 +++++++++++++++++++
 4 files changed, 42 insertions(+), 4 deletions(-)
 create mode 100644 db/migrate/20191107050249_issue_2460_fix_corrupted_twitter_ids.rb
 create mode 100644 spec/db/migrate/issue_2460_fix_corrupted_twitter_ids_spec.rb

diff --git a/.rubocop_todo.rspec.yml b/.rubocop_todo.rspec.yml
index f472b0eb5..7a38bdc4e 100644
--- a/.rubocop_todo.rspec.yml
+++ b/.rubocop_todo.rspec.yml
@@ -86,6 +86,7 @@ RSpec/FilePath:
     - 'spec/db/migrate/issue_2368_add_indices_to_histories_and_tickets_spec.rb'
     - 'spec/db/migrate/issue_2541_fix_notification_email_without_body_spec.rb'
     - 'spec/db/migrate/issue_2608_missing_trigger_permission_spec.rb'
+    - 'spec/db/migrate/issue_2460_fix_corrupted_twitter_ids_spec.rb'
     - 'spec/lib/import/base_factory_spec.rb'
 
 # Offense count: 60
diff --git a/db/migrate/20191107050249_issue_2460_fix_corrupted_twitter_ids.rb b/db/migrate/20191107050249_issue_2460_fix_corrupted_twitter_ids.rb
new file mode 100644
index 000000000..e903d4919
--- /dev/null
+++ b/db/migrate/20191107050249_issue_2460_fix_corrupted_twitter_ids.rb
@@ -0,0 +1,19 @@
+class Issue2460FixCorruptedTwitterIds < ActiveRecord::Migration[5.2]
+  def up
+    return if !Setting.find_by(name: 'system_init_done')
+
+    Channel.where(area: 'Twitter::Account').each do |channel|
+
+      client = Twitter::REST::Client.new do |config|
+        config.consumer_key        = channel.options['auth']['consumer_key']
+        config.consumer_secret     = channel.options['auth']['consumer_secret']
+        config.access_token        = channel.options['auth']['oauth_token']
+        config.access_token_secret = channel.options['auth']['oauth_token_secret']
+      end
+
+      channel.options['user']['id'] = client.user.id.to_s
+
+      channel.save!
+    end
+  end
+end
diff --git a/lib/external_credential/twitter.rb b/lib/external_credential/twitter.rb
index 5bc60dd8b..a2db7d7dd 100644
--- a/lib/external_credential/twitter.rb
+++ b/lib/external_credential/twitter.rb
@@ -56,16 +56,15 @@ class ExternalCredential::Twitter
       access_token_secret: access_token.secret,
     )
     client_user = client.who_am_i
-    client_user_id = client_user.id
 
     # check if account already exists
     Channel.where(area: 'Twitter::Account').each do |channel|
       next if !channel.options
       next if !channel.options['user']
       next if !channel.options['user']['id']
-      next if channel.options['user']['id'] != client_user_id && channel.options['user']['screen_name'] != client_user.screen_name
+      next if channel.options['user']['id'].to_s != client_user.id.to_s && channel.options['user']['screen_name'] != client_user.screen_name
 
-      channel.options['user']['id'] = client_user_id
+      channel.options['user']['id'] = client_user.id.to_s
       channel.options['user']['screen_name'] = client_user.screen_name
       channel.options['user']['name'] = client_user.name
 
@@ -90,7 +89,7 @@ class ExternalCredential::Twitter
       options:       {
         adapter: 'twitter',
         user:    {
-          id:          client_user_id,
+          id:          client_user.id.to_s,
           screen_name: client_user.screen_name,
           name:        client_user.name,
         },
diff --git a/spec/db/migrate/issue_2460_fix_corrupted_twitter_ids_spec.rb b/spec/db/migrate/issue_2460_fix_corrupted_twitter_ids_spec.rb
new file mode 100644
index 000000000..ca67353b5
--- /dev/null
+++ b/spec/db/migrate/issue_2460_fix_corrupted_twitter_ids_spec.rb
@@ -0,0 +1,19 @@
+require 'rails_helper'
+
+RSpec.describe Issue2460FixCorruptedTwitterIds, type: :db_migration do
+  before { allow(Twitter::REST::Client).to receive(:new).and_return(client) }
+
+  let(:client) { double('Twitter::REST::Client', user: twitter_api_user) }
+  let(:twitter_api_user) { double('Twitter::User', id: twitter_api_user_id) }
+  let(:twitter_api_user_id) { 1234567890 } # rubocop:disable Style/NumericLiterals
+
+  context 'with existing, corrupted Twitter channel' do
+    let!(:twitter_channel) { create(:twitter_channel) }
+
+    it 'updates the channel’s stored user ID (as string)' do
+      expect { migrate }
+        .to change { twitter_channel.reload.options[:user][:id] }
+        .to(twitter_api_user_id.to_s)
+    end
+  end
+end