From 33498bac91dec0f52fa6313472851877e9fa016b Mon Sep 17 00:00:00 2001 From: Rolf Schmidt Date: Thu, 13 Feb 2020 09:27:36 +0100 Subject: [PATCH] Feature: Chat origin whitelisting functionality. --- app/assets/javascripts/app/models/chat.coffee | 3 +- app/models/chat.rb | 22 +++++++-- db/migrate/20120101000010_create_ticket.rb | 1 + .../20200205000001_chat_add_allow_website.rb | 9 ++++ lib/sessions/event/base.rb | 8 ++++ lib/sessions/event/chat_session_init.rb | 10 ++--- lib/sessions/event/chat_status_customer.rb | 45 ++++++++++--------- lib/websocket_server.rb | 10 +---- spec/factories/chat.rb | 9 ++++ spec/models/chat_spec.rb | 18 ++++++++ 10 files changed, 96 insertions(+), 39 deletions(-) create mode 100644 db/migrate/20200205000001_chat_add_allow_website.rb create mode 100644 spec/factories/chat.rb create mode 100644 spec/models/chat_spec.rb diff --git a/app/assets/javascripts/app/models/chat.coffee b/app/assets/javascripts/app/models/chat.coffee index ddd0f7eaa..be68edbf0 100644 --- a/app/assets/javascripts/app/models/chat.coffee +++ b/app/assets/javascripts/app/models/chat.coffee @@ -1,5 +1,5 @@ class App.Chat extends App.Model - @configure 'Chat', 'name', 'active', 'public', 'max_queue', 'block_ip', 'block_country', 'note' + @configure 'Chat', 'name', 'active', 'public', 'max_queue', 'block_ip', 'whitelisted_websites', 'block_country', 'note' @extend Spine.Model.Ajax @url: @apiPath + '/chats' @countries: @@ -250,6 +250,7 @@ class App.Chat extends App.Model { name: 'note', display: 'Note', tag: 'textarea', limit: 250, null: true }, { name: 'max_queue', display: 'Max. clients in waitlist', tag: 'input', default: 2 }, { name: 'block_ip', display: 'Blocked IPs (separated by ;)', tag: 'input', default: '', null: true }, + { name: 'whitelisted_websites', display: 'Allow websites (separated by ;)', tag: 'input', default: '', null: true }, { name: 'block_country', display: 'Blocked countries', tag: 'column_select', multiple: true, null: true, default: '', options: @countries, seperator: ';' }, { name: 'active', display: 'Active', tag: 'active', default: true }, { name: 'created_by_id', display: 'Created by', relation: 'User', readonly: 1 }, diff --git a/app/models/chat.rb b/app/models/chat.rb index 5211773fd..d0ae016eb 100644 --- a/app/models/chat.rb +++ b/app/models/chat.rb @@ -617,6 +617,23 @@ check if ip address is blocked for chat =begin +check if website is allowed for chat + + chat = Chat.find(123) + chat.website_whitelisted?('zammad.org') + +=end + + def website_whitelisted?(website) + return true if whitelisted_websites.blank? + + whitelisted_websites.split(';').any? do |whitelisted_website| + website.downcase.include?(whitelisted_website.downcase.strip) + end + end + +=begin + check if country is blocked for chat chat = Chat.find(123) @@ -633,10 +650,9 @@ check if country is blocked for chat return false if geo_ip['country_code'].blank? countries = block_country.split(';') - countries.each do |local_country| - return true if geo_ip['country_code'] == local_country + countries.any? do |local_country| + geo_ip['country_code'] == local_country end - false end end diff --git a/db/migrate/20120101000010_create_ticket.rb b/db/migrate/20120101000010_create_ticket.rb index 965789fe2..694a4a27d 100644 --- a/db/migrate/20120101000010_create_ticket.rb +++ b/db/migrate/20120101000010_create_ticket.rb @@ -483,6 +483,7 @@ class CreateTicket < ActiveRecord::Migration[4.2] t.boolean :public, null: false, default: false t.string :block_ip, limit: 5000, null: true t.string :block_country, limit: 5000, null: true + t.string :whitelisted_websites, limit: 5000, null: true t.string :preferences, limit: 5000, null: true t.integer :updated_by_id, null: false t.integer :created_by_id, null: false diff --git a/db/migrate/20200205000001_chat_add_allow_website.rb b/db/migrate/20200205000001_chat_add_allow_website.rb new file mode 100644 index 000000000..d75a15ba8 --- /dev/null +++ b/db/migrate/20200205000001_chat_add_allow_website.rb @@ -0,0 +1,9 @@ +class ChatAddAllowWebsite < ActiveRecord::Migration[5.1] + def up + + # return if it's a new setup + return if !Setting.find_by(name: 'system_init_done') + + add_column :chats, :whitelisted_websites, :string, limit: 5000, null: true + end +end diff --git a/lib/sessions/event/base.rb b/lib/sessions/event/base.rb index 94a840ac4..1469be44a 100644 --- a/lib/sessions/event/base.rb +++ b/lib/sessions/event/base.rb @@ -105,6 +105,14 @@ class Sessions::Event::Base user end + def remote_ip + @headers&.fetch('X-Forwarded-For', nil).presence + end + + def origin + @headers&.fetch('Origin', nil).presence + end + def permission_check(key, event) user = current_user return if !user diff --git a/lib/sessions/event/chat_session_init.rb b/lib/sessions/event/chat_session_init.rb index 935d0ebac..817d16409 100644 --- a/lib/sessions/event/chat_session_init.rb +++ b/lib/sessions/event/chat_session_init.rb @@ -24,17 +24,17 @@ return is sent as message back to peer # geo ip lookup geo_ip = nil - if @remote_ip - geo_ip = Service::GeoIp.location(@remote_ip) + if remote_ip + geo_ip = Service::GeoIp.location(remote_ip) end # dns lookup dns_name = nil - if @remote_ip + if remote_ip begin dns = Resolv::DNS.new dns.timeouts = 3 - result = dns.getname @remote_ip + result = dns.getname remote_ip if result dns_name = result.to_s end @@ -51,7 +51,7 @@ return is sent as message back to peer preferences: { url: @payload['data']['url'], participants: [@client_id], - remote_ip: @remote_ip, + remote_ip: remote_ip, geo_ip: geo_ip, dns_name: dns_name, }, diff --git a/lib/sessions/event/chat_status_customer.rb b/lib/sessions/event/chat_status_customer.rb index 5a8bf7a51..cb7c5e3e7 100644 --- a/lib/sessions/event/chat_status_customer.rb +++ b/lib/sessions/event/chat_status_customer.rb @@ -21,8 +21,9 @@ return is sent as message back to peer def run return super if super return if !check_chat_exists - return if !check_chat_block_by_ip - return if !check_chat_block_by_country + return if blocked_ip? + return if blocked_country? + return if blocked_origin? # check if it's a chat sessin reconnect session_id = nil @@ -54,10 +55,28 @@ return is sent as message back to peer } end - def check_chat_block_by_ip - chat = current_chat - return true if !chat.blocked_ip?(@remote_ip) + def blocked_ip? + return false if !current_chat.blocked_ip?(remote_ip) + send_unavailable + true + end + + def blocked_country? + return false if !current_chat.blocked_country?(remote_ip) + + send_unavailable + true + end + + def blocked_origin? + return false if current_chat.website_whitelisted?(origin) + + send_unavailable + true + end + + def send_unavailable error = { event: 'chat_error', data: { @@ -65,21 +84,5 @@ return is sent as message back to peer }, } Sessions.send(@client_id, error) - false end - - def check_chat_block_by_country - chat = current_chat - return true if !chat.blocked_country?(@remote_ip) - - error = { - event: 'chat_error', - data: { - state: 'chat_unavailable', - }, - } - Sessions.send(@client_id, error) - false - end - end diff --git a/lib/websocket_server.rb b/lib/websocket_server.rb index 9d075cb86..98247750c 100644 --- a/lib/websocket_server.rb +++ b/lib/websocket_server.rb @@ -48,7 +48,6 @@ class WebsocketServer def self.onopen(websocket, handshake) headers = handshake.headers - remote_ip = get_remote_ip(headers) client_id = websocket.object_id.to_s log 'notice', 'Client connected.', client_id Sessions.create( client_id, {}, { type: 'websocket' } ) @@ -60,7 +59,6 @@ class WebsocketServer last_ping: Time.now.utc.to_i, error_count: 0, headers: headers, - remote_ip: remote_ip, } end @@ -103,7 +101,7 @@ class WebsocketServer event: data['event'], payload: data, session: @clients[client_id][:session], - remote_ip: @clients[client_id][:remote_ip], + headers: @clients[client_id][:headers], client_id: client_id, clients: @clients, options: @options, @@ -116,12 +114,6 @@ class WebsocketServer end end - def self.get_remote_ip(headers) - return headers['X-Forwarded-For'] if headers && headers['X-Forwarded-For'] - - nil - end - def self.websocket_send(client_id, data) msg = if data.class != Array "[#{data.to_json}]" diff --git a/spec/factories/chat.rb b/spec/factories/chat.rb new file mode 100644 index 000000000..ab60aa98d --- /dev/null +++ b/spec/factories/chat.rb @@ -0,0 +1,9 @@ +FactoryBot.define do + factory :chat do + sequence(:name) { |n| "Chat #{n}" } + max_queue { 5 } + active { true } + created_by_id { 1 } + updated_by_id { 1 } + end +end diff --git a/spec/models/chat_spec.rb b/spec/models/chat_spec.rb new file mode 100644 index 000000000..88196fb8a --- /dev/null +++ b/spec/models/chat_spec.rb @@ -0,0 +1,18 @@ +require 'rails_helper' + +RSpec.describe Chat, type: :model do + + describe 'website whitelisting' do + let(:chat) { create(:chat, whitelisted_websites: 'zammad.org') } + + it 'detects whitelisted website' do + result = chat.website_whitelisted?('https://www.zammad.org') + expect(result).to be true + end + + it 'detects non-whitelisted website' do + result = chat.website_whitelisted?('https://www.zammad.com') + expect(result).to be false + end + end +end