Merge pull request #1814 from internetee/add-guard-clause-to-mass-nameserver-change

Bulk NS change: Verify CSV integrity

app/controllers/registrar/admin_contacts_controller.rb

@@ -7,7 +7,10 @@ class Registrar
       authorize! :manage, :repp
       uri = BASE_URL
       request = form_request(uri)
-      response = do_request(request, uri)
+
+      action = Actions::DoRequest.new(request, uri)
+      response = action.call
+
       start_notice = t('.replaced')
 
       process_response(response: response,

app/controllers/registrar/bulk_change_controller.rb

@@ -62,48 +62,6 @@ class Registrar
       notices
     end
 
-    def do_request(request, uri)
-      response = if Rails.env.test?
-                   do_test_request(request, uri)
-                 elsif Rails.env.development?
-                   do_dev_request(request, uri)
-                 else
-                   do_live_request(request, uri)
-                 end
-      response
-    end
-
-    def do_live_request(request, uri)
-      client_cert = File.read(ENV['cert_path'])
-      client_key = File.read(ENV['key_path'])
-      Net::HTTP.start(uri.hostname, uri.port,
-                      use_ssl: (uri.scheme == 'https'),
-                      cert: OpenSSL::X509::Certificate.new(client_cert),
-                      key: OpenSSL::PKey::RSA.new(client_key)) do |http|
-        http.request(request)
-      end
-    end
-
-    def do_dev_request(request, uri)
-      client_cert = File.read(ENV['cert_path'])
-      client_key = File.read(ENV['key_path'])
-      Net::HTTP.start(uri.hostname, uri.port,
-                      use_ssl: (uri.scheme == 'https'),
-                      verify_mode: OpenSSL::SSL::VERIFY_NONE,
-                      cert: OpenSSL::X509::Certificate.new(client_cert),
-                      key: OpenSSL::PKey::RSA.new(client_key)) do |http|
-        http.request(request)
-      end
-    end
-
-    def do_test_request(request, uri)
-      Net::HTTP.start(uri.hostname, uri.port,
-                      use_ssl: (uri.scheme == 'https'),
-                      verify_mode: OpenSSL::SSL::VERIFY_NONE) do |http|
-        http.request(request)
-      end
-    end
-
     def ready_to_renew?
       domain_ids_for_bulk_renew.present? && params[:renew].present?
     end

app/controllers/registrar/domain_transfers_controller.rb

@@ -24,8 +24,8 @@ class Registrar
         request.basic_auth(current_registrar_user.username,
                            current_registrar_user.plain_text_password)
 
-
-        response = do_request(request, uri)
+        action = Actions::DoRequest.new(request, uri)
+        response = action.call
 
         parsed_response = JSON.parse(response.body, symbolize_names: true)
 

app/controllers/registrar/nameservers_controller.rb

@@ -6,19 +6,19 @@ class Registrar
       ipv4 = params[:ipv4].split("\r\n")
       ipv6 = params[:ipv6].split("\r\n")
 
+      uri = URI.parse("#{ENV['repp_url']}registrar/nameservers")
+
       domains = domain_list_from_csv
 
-      uri = URI.parse("#{ENV['repp_url']}registrar/nameservers")
-      request = Net::HTTP::Put.new(uri, 'Content-Type' => 'application/json')
-      request.body = { data: { type: 'nameserver', id: params[:old_hostname],
-                               domains: domains,
-                               attributes: { hostname: params[:new_hostname],
-                                             ipv4: ipv4,
-                                             ipv6: ipv6 } } }.to_json
-      request.basic_auth(current_registrar_user.username,
-                         current_registrar_user.plain_text_password)
+      return csv_list_empty_guard if domains == []
 
-      response = do_request(request, uri)
+      options = {
+        uri: uri,
+        ipv4: ipv4,
+        ipv6: ipv6,
+      }
+      action = Actions::BulkNameserversChange.new(params, domains, current_registrar_user, options)
+      response = action.call
 
       parsed_response = JSON.parse(response.body, symbolize_names: true)
 
@@ -42,12 +42,25 @@ class Registrar
       notices.join(', ')
     end
 
+    def csv_list_empty_guard
+      notice = 'CSV scoped domain list seems empty. Make sure that domains are added and ' \
+      '"domain_name" header is present.'
+      redirect_to(registrar_domains_url, flash: { notice: notice })
+    end
+
     def domain_list_from_csv
-      return [] if params[:puny_file].blank?
+      return if params[:puny_file].blank?
 
       domains = []
-      CSV.read(params[:puny_file].path, headers: true).each { |b| domains << b['domain_name'] }
-      domains
+      csv = CSV.read(params[:puny_file].path, headers: true)
+
+      return [] if csv['domain_name'].blank?
+
+      csv.map { |b| domains << b['domain_name'] }
+
+      domains.compact
+    rescue CSV::MalformedCSVError
+      []
     end
   end
 end

app/controllers/registrar/tech_contacts_controller.rb

@@ -8,7 +8,10 @@ class Registrar
 
       uri = BASE_URL
       request = form_request(uri)
-      response = do_request(request, uri)
+
+      action = Actions::DoRequest.new(request, uri)
+      response = action.call
+
       start_notice = t('.replaced')
 
       process_response(response: response,

app/interactions/actions/bulk_nameservers_change.rb

@@ -0,0 +1,26 @@
+module Actions
+  class BulkNameserversChange
+    def initialize(params, domains, current_registrar_user, options = {})
+      @params = params
+      @domains = domains
+      @current_registrar_user = current_registrar_user
+      @ipv4 = options.fetch(:ipv4)
+      @ipv6 = options.fetch(:ipv6)
+      @uri = options.fetch(:uri)
+    end
+
+    def call
+      request = Net::HTTP::Put.new(@uri, 'Content-Type' => 'application/json')
+      request.body = { data: { type: 'nameserver', id: @params[:old_hostname],
+                               domains: @domains || [],
+                               attributes: { hostname: @params[:new_hostname],
+                                             ipv4: @ipv4,
+                                             ipv6: @ipv6 } } }.to_json
+      request.basic_auth(@current_registrar_user.username,
+                         @current_registrar_user.plain_text_password)
+
+      action = Actions::DoRequest.new(request, @uri)
+      action.call
+    end
+  end
+end

app/interactions/actions/do_request.rb

@@ -0,0 +1,52 @@
+module Actions
+  class DoRequest
+    def initialize(request, uri)
+      @request = request
+      @uri = uri
+    end
+
+    def call
+      response = if Rails.env.test?
+                   do_test_request(@request, @uri)
+                 elsif Rails.env.development?
+                   do_dev_request(@request, @uri)
+                 else
+                   do_live_request(@request, @uri)
+                 end
+      response
+    rescue StandardError, OpenURI::HTTPError => e
+      Rails.logger.debug e.message
+    end
+
+    def do_live_request(request, uri)
+      client_cert = File.read(ENV['cert_path'])
+      client_key = File.read(ENV['key_path'])
+      Net::HTTP.start(uri.hostname, uri.port,
+                      use_ssl: (uri.scheme == 'https'),
+                      cert: OpenSSL::X509::Certificate.new(client_cert),
+                      key: OpenSSL::PKey::RSA.new(client_key)) do |http|
+        http.request(request)
+      end
+    end
+
+    def do_dev_request(request, uri)
+      client_cert = File.read(ENV['cert_path'])
+      client_key = File.read(ENV['key_path'])
+      Net::HTTP.start(uri.hostname, uri.port,
+                      use_ssl: (uri.scheme == 'https'),
+                      verify_mode: OpenSSL::SSL::VERIFY_NONE,
+                      cert: OpenSSL::X509::Certificate.new(client_cert),
+                      key: OpenSSL::PKey::RSA.new(client_key)) do |http|
+        http.request(request)
+      end
+    end
+
+    def do_test_request(request, uri)
+      Net::HTTP.start(uri.hostname, uri.port,
+                      use_ssl: (uri.scheme == 'https'),
+                      verify_mode: OpenSSL::SSL::VERIFY_NONE) do |http|
+        http.request(request)
+      end
+    end
+  end
+end

app/views/registrar/bulk_change/_nameserver_form.html.erb

@@ -50,7 +50,7 @@
         </div>
         <div class="col-md-4">
             <%= file_field_tag :puny_file, required: false, accept: 'text/csv' %>
-            <span class="help-block">CSV format, must have domain_name field. List of domains that nameserver change should be scoped to.</span>
+            <span class="help-block">CSV format, must have domain_name header. List of domains that nameserver change should be scoped to.</span>
         </div>
     </div>
 

config/application.yml.sample

@@ -70,7 +70,7 @@ epp_port: '700'
 cert_path: '/home/registry/registry/shared/ca/certs/webclient.cert.pem'
 key_path: '/home/registry/registry/shared/ca/private/webclient.key.pem'
 epp_hostname: 'registry.gitlab.eu'
-repp_url: 'https://repp.gitlab.eu/repp/v1/'
+repp_url: 'http://epp:3000/repp/v1/'
 
 # Estonian Company Register
 company_register_username:

test/fixtures/files/invalid_domains_for_ns_replacement.csv

@@ -0,0 +1 @@
+shop.test

test/interactions/do_request_test.rb

@@ -0,0 +1,42 @@
+require 'test_helper'
+
+class DoRequestTest < ActiveSupport::TestCase
+
+  setup do
+    WebMock.disable_net_connect!
+
+    @uri = URI.parse("#{ENV['repp_url']}registrar/nameservers")
+    @request = Net::HTTP::Put.new(@uri, 'Content-Type' => 'application/json')
+    @nameserver = nameservers(:shop_ns1)
+    @domain = domains(:shop)
+    @user = users(:api_bestnames)
+
+    @request.body = { data: { type: 'nameserver', id: @nameserver.hostname,
+                             domains: ["shop.test"],
+                             attributes: { hostname: 'new-ns.bestnames.test',
+                                           ipv4: '192.0.2.55',
+                                           ipv6: '2001:db8::55' } } }.to_json
+    @request.basic_auth(@user.username, @user.plain_text_password)
+  end
+
+  def test_request_occurs 
+    stub_request(:put, "http://epp:3000/repp/v1/registrar/nameservers").
+    with(
+      body: "{\"data\":{\"type\":\"nameserver\",\"id\":\"ns1.bestnames.test\",\"domains\":[\"shop.test\"],\"attributes\":{\"hostname\":\"new-ns.bestnames.test\",\"ipv4\":\"192.0.2.55\",\"ipv6\":\"2001:db8::55\"}}}",
+      headers: {
+      'Accept'=>'*/*',
+      'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3',
+      'Authorization'=>'Basic dGVzdF9iZXN0bmFtZXM6dGVzdHRlc3Q=',
+      'Content-Type'=>'application/json',
+      'Host'=>'epp:3000',
+      'User-Agent'=>'Ruby'
+      }).
+    to_return(status: 200, body: ["shop.test"], headers: {})
+
+    action = Actions::DoRequest.new(@request, @uri)
+    response = action.call
+
+    assert_equal response.body, ["shop.test"]
+    assert_equal response.code, "200"
+  end
+end

test/system/registrar_area/bulk_change/nameserver_test.rb

@@ -94,4 +94,25 @@ class RegistrarAreaNameserverBulkChangeTest < ApplicationSystemTestCase
     assert_text 'Nameserver have been successfully replaced'
     assert_text 'Affected domains: shop.test'
   end
+
+  def test_replaces_nameservers_with_invalid_domains_list
+    nameserver = nameservers(:shop_ns1)
+
+    visit registrar_domains_url
+    click_link 'Bulk change'
+    click_link 'Nameserver'
+
+    fill_in 'Old hostname', with: nameserver.hostname
+    fill_in 'New hostname', with: 'new-ns.bestnames.test'
+    fill_in 'ipv4', with: "192.0.2.55\n192.0.2.56"
+    fill_in 'ipv6', with: "2001:db8::55\n2001:db8::56"
+    attach_file :puny_file, Rails.root.join('test', 'fixtures', 'files', 'invalid_domains_for_ns_replacement.csv').to_s
+
+    assert_no_changes -> { nameserver.hostname } do
+      click_on 'Replace nameserver'
+    end
+
+    assert_current_path registrar_domains_path    
+    assert_text 'CSV scoped domain list seems empty. Make sure that domains are added and "domain_name" header is present.'
+  end
 end