Add Irker service configuration options

Closes #1713
Closes #1714
Closes gitlab-com/support-forum#139

3 files changed, 49 insertions, 67 deletions

diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb
index dc18bbd8d5b..1e435be8275 100644
--- a/app/controllers/projects/services_controller.rb
+++ b/app/controllers/projects/services_controller.rb
@@ -7,7 +7,8 @@ class Projects::ServicesController < Projects::ApplicationController
                     :colorize_messages, :channels,
                     :push_events, :issues_events, :merge_requests_events, :tag_push_events,
                     :note_events, :send_from_committer_email, :disable_diffs, :external_wiki_url,
-                    :notify, :color]
+                    :notify, :color,
+                    :server_host, :server_port, :default_irc_uri]
   # Authorize
   before_action :authorize_admin_project!
   before_action :service, only: [:edit, :update, :test]
diff --git a/app/models/project_services/irker_service.rb b/app/models/project_services/irker_service.rb
index 89f312e8c98..a4d0914dbe7 100644
--- a/app/models/project_services/irker_service.rb
+++ b/app/models/project_services/irker_service.rb
@@ -21,26 +21,11 @@
 require 'uri'
 
 class IrkerService < Service
+  prop_accessor :server_host, :server_port, :default_irc_uri
   prop_accessor :colorize_messages, :recipients, :channels
   validates :recipients, presence: true, if: :activated?
-  validate :check_recipients_count, if: :activated?
 
   before_validation :get_channels
-  after_initialize :initialize_settings
-
-  # Writer for RSpec tests
-  attr_writer :settings
-
-  def initialize_settings
-    # See the documentation (doc/project_services/irker.md) for possible values
-    # here
-    @settings ||= {
-      server_ip: 'localhost',
-      server_port: 6659,
-      max_channels: 3,
-      default_irc_uri: nil
-    }
-  end
 
   def title
     'Irker (IRC gateway)'
@@ -51,20 +36,6 @@ class IrkerService < Service
     'gateway.'
   end
 
-  def help
-    msg = 'Recipients have to be specified with a full URI: '\
-    'irc[s]://irc.network.net[:port]/#channel. Special cases: if you want '\
-    'the channel to be a nickname instead, append ",isnick" to the channel '\
-    'name; if the channel is protected by a secret password, append '\
-    '"?key=secretpassword" to the URI.'
-
-    unless @settings[:default_irc].nil?
-      msg += ' Note that a default IRC URI is provided by this service\'s '\
-      "administrator: #{default_irc}. You can thus just give a channel name."
-    end
-    msg
-  end
-
   def to_param
     'irker'
   end
@@ -77,30 +48,45 @@ class IrkerService < Service
     return unless supported_events.include?(data[:object_kind])
 
     IrkerWorker.perform_async(project_id, channels,
-                              colorize_messages, data, @settings)
+                              colorize_messages, data, settings)
+  end
+
+  def settings
+    { server_host: server_host.present? ? server_host : 'localhost',
+      server_port: server_port.present? ? server_port : 6659
+    }
   end
 
   def fields
     [
+      { type: 'text', name: 'server_host', placeholder: 'localhost',
+        help: 'Irker daemon hostname (defaults to localhost)' },
+      { type: 'text', name: 'server_port', placeholder: 6659,
+        help: 'Irker daemon port (defaults to 6659)' },
+      { type: 'text', name: 'default_irc_uri',
+        help: 'A default IRC URI to prepend before each recipient (optional)',
+        placeholder: 'irc://irc.network.net:6697/' },
       { type: 'textarea', name: 'recipients',
-        placeholder: 'Recipients/channels separated by whitespaces' },
+        placeholder: 'Recipients/channels separated by whitespaces',
+        help: 'Recipients have to be specified with a full URI: '\
+        'irc[s]://irc.network.net[:port]/#channel. Special cases: if '\
+        'you want the channel to be a nickname instead, append ",isnick" to ' \
+        'the channel name; if the channel is protected by a secret password, ' \
+        ' append "?key=secretpassword" to the URI. Note that if you specify a ' \
+        ' default IRC URI to prepend before each recipient, you can just give ' \
+        ' a channel name.'  },
       { type: 'checkbox', name: 'colorize_messages' },
     ]
   end
 
-  private
-
-  def check_recipients_count
-    return true if recipients.nil? || recipients.empty?
-
-    if recipients.split(/\s+/).count > max_chans
-      errors.add(:recipients, "are limited to #{max_chans}")
-    end
+  def help
+    ' NOTE: Irker does NOT have built-in authentication, which makes it' \
+    ' vulnerable to spamming IRC channels if it is hosted outside of a ' \
+    ' firewall. Please make sure you run the daemon within a secured network ' \
+    ' to prevent abuse. For more details, read: http://www.catb.org/~esr/irker/security.html.'
   end
 
-  def max_chans
-    @settings[:max_channels]
-  end
+  private
 
   def get_channels
     return true unless :activated?
@@ -114,40 +100,35 @@ class IrkerService < Service
 
   def map_recipients
     self.channels = recipients.split(/\s+/).map do |recipient|
-      format_channel default_irc_uri, recipient
+      format_channel(recipient)
     end
     channels.reject! &:nil?
   end
 
-  def default_irc_uri
-    default_irc = @settings[:default_irc_uri]
-    if !(default_irc.nil? || default_irc[-1] == '/')
-      default_irc += '/'
-    end
-    default_irc
-  end
-
-  def format_channel(default_irc, recipient)
-    cnt = 0
-    url = nil
+  def format_channel(recipient)
+    uri = nil
 
     # Try to parse the chan as a full URI
     begin
-      uri = URI.parse(recipient)
-      raise URI::InvalidURIError if uri.scheme.nil? && cnt == 0
+      uri = consider_uri(URI.parse(recipient))
     rescue URI::InvalidURIError
-      unless default_irc.nil?
-        cnt += 1
-        recipient = "#{default_irc}#{recipient}"
-        retry if cnt == 1
+    end
+
+    unless uri.present? and default_irc_uri.nil?
+      begin
+        new_recipient = URI.join(default_irc_uri, '/', recipient).to_s
+        uri = consider_uri(URI.parse(new_recipient))
+      rescue
+        Rails.logger.error("Unable to create a valid URL from #{default_irc_uri} and #{recipient}")
       end
-    else
-      url = consider_uri uri
     end
-    url
+
+    uri
   end
 
   def consider_uri(uri)
+    return nil if uri.scheme.nil?
+
     # Authorize both irc://domain.com/#chan and irc://domain.com/chan
     if uri.is_a?(URI) && uri.scheme[/^ircs?\z/] && !uri.path.nil?
       # Do not authorize irc://domain.com/
diff --git a/app/workers/irker_worker.rb b/app/workers/irker_worker.rb
index 84a54656df2..2d44d8d4dc6 100644
--- a/app/workers/irker_worker.rb
+++ b/app/workers/irker_worker.rb
@@ -19,7 +19,7 @@ class IrkerWorker
       branch = "\x0305#{branch}\x0f"
     end
 
-    # Firsts messages are for branch creation/deletion
+    # First messages are for branch creation/deletion
     send_branch_updates push_data, project, repo_name, committer, branch
 
     # Next messages are for commits
@@ -34,7 +34,7 @@ class IrkerWorker
   def init_perform(set, chans, colors)
     @colors = colors
     @channels = chans
-    start_connection set['server_ip'], set['server_port']
+    start_connection set['server_host'], set['server_port']
   end
 
   def start_connection(irker_server, irker_port)