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
This commit is contained in:
Ryan Lue 2019-11-07 14:21:46 +08:00 committed by Thorsten Eckel
parent ea3522c7a2
commit 86a90d287c
4 changed files with 42 additions and 4 deletions

View file

@ -86,6 +86,7 @@ RSpec/FilePath:
- 'spec/db/migrate/issue_2368_add_indices_to_histories_and_tickets_spec.rb' - '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_2541_fix_notification_email_without_body_spec.rb'
- 'spec/db/migrate/issue_2608_missing_trigger_permission_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' - 'spec/lib/import/base_factory_spec.rb'
# Offense count: 60 # Offense count: 60

View file

@ -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

View file

@ -56,16 +56,15 @@ class ExternalCredential::Twitter
access_token_secret: access_token.secret, access_token_secret: access_token.secret,
) )
client_user = client.who_am_i client_user = client.who_am_i
client_user_id = client_user.id
# check if account already exists # check if account already exists
Channel.where(area: 'Twitter::Account').each do |channel| Channel.where(area: 'Twitter::Account').each do |channel|
next if !channel.options next if !channel.options
next if !channel.options['user'] next if !channel.options['user']
next if !channel.options['user']['id'] 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']['screen_name'] = client_user.screen_name
channel.options['user']['name'] = client_user.name channel.options['user']['name'] = client_user.name
@ -90,7 +89,7 @@ class ExternalCredential::Twitter
options: { options: {
adapter: 'twitter', adapter: 'twitter',
user: { user: {
id: client_user_id, id: client_user.id.to_s,
screen_name: client_user.screen_name, screen_name: client_user.screen_name,
name: client_user.name, name: client_user.name,
}, },

View file

@ -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 channels 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