From 66f341dc8542f429e75366b6b4ad48bae90ff3d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Tue, 22 Dec 2020 14:03:52 +0200 Subject: [PATCH 1/9] Fix REPP bulk renew SSL CA vericication --- app/models/repp_api.rb | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/app/models/repp_api.rb b/app/models/repp_api.rb index 92e7a22c7..a752a15af 100644 --- a/app/models/repp_api.rb +++ b/app/models/repp_api.rb @@ -10,13 +10,24 @@ class ReppApi def self.request(request, uri, registrar:) request.basic_auth(registrar.username, registrar.plain_text_password) if registrar - http = Net::HTTP.start(uri.hostname, uri.port, use_ssl: (uri.scheme == 'https')) - unless Rails.env.test? - http.cert = OpenSSL::X509::Certificate.new(File.read(ENV['cert_path'])) - http.key = OpenSSL::PKey::RSA.new(File.read(ENV['key_path'])) - end - http.verify_mode = OpenSSL::SSL::VERIFY_NONE if Rails.env.development? || Rails.env.test? + client_cert = Rails.env.test? ? nil : File.read(ENV['cert_path']) + client_key = Rails.env.test? ? nil : File.read(ENV['key_path']) + params = ReppApi.compose_ca_auth_params(uri, client_cert, client_key) - http.request(request) + Net::HTTP.start(uri.hostname, uri.port, params) do |http| + http.request(request) + end + end + + def self.compose_ca_auth_params(uri, client_cert, client_key) + params = { use_ssl: (uri.scheme == 'https') } + params[:verify_mode] = OpenSSL::SSL::VERIFY_NONE if Rails.env.test? || Rails.env.development? + + unless Rails.env.test? + params[:cert] = OpenSSL::X509::Certificate.new(client_cert) + params[:key] = OpenSSL::PKey::RSA.new(client_key) + end + + params end end From 1d3c70ae344eeda9513875157186ec38918553ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20V=C3=B5hmar?= Date: Tue, 22 Dec 2020 14:32:53 +0200 Subject: [PATCH 2/9] Update CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 59e867d1c..fa814db62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +22.12.2020 +* SSL CA verification fix for Bulk renew [#1778](https://github.com/internetee/registry/pull/1778) + 21.12.2020 * Bulk renew for REPP and registrar [#1763](https://github.com/internetee/registry/issues/1763) From 8f25175cec848f43bd7b7f1b7f6c3b34dfa39dd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Tue, 22 Dec 2020 16:08:15 +0200 Subject: [PATCH 3/9] Fix bulk nameserver notification handling --- app/controllers/registrar/nameservers_controller.rb | 7 ++++--- app/views/registrar/bulk_change/_api_errors.html.erb | 4 ++++ .../domain_transfers/form/_api_errors.html.erb | 4 ++++ .../registrar_area/bulk_change/nameserver_test.rb | 12 +++++++----- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/app/controllers/registrar/nameservers_controller.rb b/app/controllers/registrar/nameservers_controller.rb index c8c88c8ca..2a22476be 100644 --- a/app/controllers/registrar/nameservers_controller.rb +++ b/app/controllers/registrar/nameservers_controller.rb @@ -49,12 +49,13 @@ class Registrar if response.code == '200' notices = [t('.replaced')] - notices << "#{t('.affected_domains')}: #{parsed_response[:affected_domains].join(', ')}" + notices << "#{t('.affected_domains')}: " \ + "#{parsed_response[:data][:affected_domains].join(', ')}" - flash[:notice] = notices + flash[:notice] = notices.join(', ') redirect_to registrar_domains_url else - @api_errors = parsed_response[:errors] + @api_errors = parsed_response[:message] render file: 'registrar/bulk_change/new', locals: { active_tab: :nameserver } end end diff --git a/app/views/registrar/bulk_change/_api_errors.html.erb b/app/views/registrar/bulk_change/_api_errors.html.erb index 56bf8c404..8d8862959 100644 --- a/app/views/registrar/bulk_change/_api_errors.html.erb +++ b/app/views/registrar/bulk_change/_api_errors.html.erb @@ -1,9 +1,13 @@ <% if @api_errors %>
    + <% if @api_errors.is_a?(String) %> +
  • <%= @api_errors %>
    • + <% else %> <% @api_errors.each do |error| %>
  • <%= error[:title] %>
    • <% end %> + <% end %>
<% end %> diff --git a/app/views/registrar/domain_transfers/form/_api_errors.html.erb b/app/views/registrar/domain_transfers/form/_api_errors.html.erb index 56bf8c404..8d8862959 100644 --- a/app/views/registrar/domain_transfers/form/_api_errors.html.erb +++ b/app/views/registrar/domain_transfers/form/_api_errors.html.erb @@ -1,9 +1,13 @@ <% if @api_errors %>
    + <% if @api_errors.is_a?(String) %> +
  • <%= @api_errors %>
    • + <% else %> <% @api_errors.each do |error| %>
  • <%= error[:title] %>
    • <% end %> + <% end %>
<% end %> diff --git a/test/system/registrar_area/bulk_change/nameserver_test.rb b/test/system/registrar_area/bulk_change/nameserver_test.rb index 3d4b4dc70..48806df7a 100644 --- a/test/system/registrar_area/bulk_change/nameserver_test.rb +++ b/test/system/registrar_area/bulk_change/nameserver_test.rb @@ -15,10 +15,12 @@ class RegistrarAreaNameserverBulkChangeTest < ApplicationSystemTestCase request_stub = stub_request(:put, /registrar\/nameservers/).with(body: request_body, headers: { 'Content-type' => Mime[:json] }, basic_auth: ['test_goodnames', 'testtest']) - .to_return(body: { data: [{ - type: 'nameserver', - id: 'new-ns.bestnames.test'}], - affected_domains: ["airport.test", "shop.test"]}.to_json, status: 200) + .to_return(body: { data: { + type: 'nameserver', + id: 'new-ns.bestnames.test', + affected_domains: ["airport.test", "shop.test"] + } + }.to_json, status: 200) visit registrar_domains_url click_link 'Bulk change' @@ -38,7 +40,7 @@ class RegistrarAreaNameserverBulkChangeTest < ApplicationSystemTestCase def test_fails_gracefully stub_request(:put, /registrar\/nameservers/).to_return(status: 400, - body: { errors: [{ title: 'epic fail' }] }.to_json, + body: { message: 'epic fail' }.to_json, headers: { 'Content-type' => Mime[:json] }) visit registrar_domains_url From 903044263a6dbaa67ada805850a6ccabd1b0aa8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Tue, 22 Dec 2020 17:58:18 +0200 Subject: [PATCH 4/9] Fix bulk tech contact change notifications --- app/controllers/registrar/tech_contacts_controller.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/registrar/tech_contacts_controller.rb b/app/controllers/registrar/tech_contacts_controller.rb index 1d459ef0f..ba15c5516 100644 --- a/app/controllers/registrar/tech_contacts_controller.rb +++ b/app/controllers/registrar/tech_contacts_controller.rb @@ -43,16 +43,16 @@ class Registrar if response.code == '200' notices = [t('.replaced')] - notices << "#{t('.affected_domains')}: #{parsed_response[:affected_domains].join(', ')}" + notices << "#{t('.affected_domains')}: #{parsed_response[:data][:affected_domains].join(', ')}" - if parsed_response[:skipped_domains] - notices << "#{t('.skipped_domains')}: #{parsed_response[:skipped_domains].join(', ')}" + if parsed_response[:data][:skipped_domains] + notices << "#{t('.skipped_domains')}: #{parsed_response[:data][:skipped_domains].join(', ')}" end flash[:notice] = notices redirect_to registrar_domains_url else - @error = parsed_response[:error] + @error = parsed_response[:message] render file: 'registrar/bulk_change/new', locals: { active_tab: :technical_contact } end end From 8fffe6e73661c840817a6493778af4b7afb97a28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Tue, 22 Dec 2020 19:05:06 +0200 Subject: [PATCH 5/9] Fix tech contact / bulk transfer notifications --- .../registrar/domain_transfers_controller.rb | 6 ++++-- .../registrar/tech_contacts_controller.rb | 4 ++-- app/models/tech_domain_contact.rb | 17 +++++++++-------- .../bulk_change/_tech_contact_form.html.erb | 2 +- .../locales/registrar/domain_transfers.en.yml | 2 +- 5 files changed, 17 insertions(+), 14 deletions(-) diff --git a/app/controllers/registrar/domain_transfers_controller.rb b/app/controllers/registrar/domain_transfers_controller.rb index ca08c73cf..03874adb5 100644 --- a/app/controllers/registrar/domain_transfers_controller.rb +++ b/app/controllers/registrar/domain_transfers_controller.rb @@ -55,10 +55,12 @@ class Registrar parsed_response = JSON.parse(response.body, symbolize_names: true) if response.code == '200' - flash[:notice] = t '.transferred', count: parsed_response[:data].size + failed = parsed_response[:data][:failed].each(&:domain_name).join(', ') + flash[:notice] = t('.transferred', success: parsed_response[:data][:success].size, + failed: failed) redirect_to registrar_domains_url else - @api_errors = parsed_response[:errors] + @api_errors = parsed_response[:message] render file: 'registrar/bulk_change/new', locals: { active_tab: :bulk_transfer } end else diff --git a/app/controllers/registrar/tech_contacts_controller.rb b/app/controllers/registrar/tech_contacts_controller.rb index ba15c5516..057d01857 100644 --- a/app/controllers/registrar/tech_contacts_controller.rb +++ b/app/controllers/registrar/tech_contacts_controller.rb @@ -49,10 +49,10 @@ class Registrar notices << "#{t('.skipped_domains')}: #{parsed_response[:data][:skipped_domains].join(', ')}" end - flash[:notice] = notices + flash[:notice] = notices.join(', ') redirect_to registrar_domains_url else - @error = parsed_response[:message] + @error = response.code == '404' ? 'Contact(s) not found' : parsed_response[:message] render file: 'registrar/bulk_change/new', locals: { active_tab: :technical_contact } end end diff --git a/app/models/tech_domain_contact.rb b/app/models/tech_domain_contact.rb index 04f36c4e4..92799061c 100644 --- a/app/models/tech_domain_contact.rb +++ b/app/models/tech_domain_contact.rb @@ -5,19 +5,20 @@ class TechDomainContact < DomainContact skipped_domains = [] tech_contacts = where(contact: current_contact) - transaction do - tech_contacts.each do |tech_contact| - if tech_contact.domain.discarded? - skipped_domains << tech_contact.domain.name - next - end - + tech_contacts.each do |tech_contact| + if tech_contact.domain.discarded? + skipped_domains << tech_contact.domain.name + next + end + begin tech_contact.contact = new_contact tech_contact.save! affected_domains << tech_contact.domain.name + rescue ActiveRecord::RecordNotUnique + skipped_domains << tech_contact.domain.name end end - return affected_domains.sort, skipped_domains.sort + [affected_domains.sort, skipped_domains.sort] end end diff --git a/app/views/registrar/bulk_change/_tech_contact_form.html.erb b/app/views/registrar/bulk_change/_tech_contact_form.html.erb index dc0693599..2848e3634 100644 --- a/app/views/registrar/bulk_change/_tech_contact_form.html.erb +++ b/app/views/registrar/bulk_change/_tech_contact_form.html.erb @@ -1,7 +1,7 @@ <%= form_tag registrar_tech_contacts_path, method: :patch, class: 'form-horizontal' do %> <% if @error %>
- <%= @error[:message] %> + <%= @error %>
<% end %> diff --git a/config/locales/registrar/domain_transfers.en.yml b/config/locales/registrar/domain_transfers.en.yml index c146a5fd1..fd39e54f7 100644 --- a/config/locales/registrar/domain_transfers.en.yml +++ b/config/locales/registrar/domain_transfers.en.yml @@ -6,7 +6,7 @@ en: create: header: Domain transfer - transferred: "%{count} domains have been successfully transferred" + transferred: "%{count} domains have been successfully transferred. Failed domains: %{failed}" form: submit_btn: Transfer From 7c2e43197b2321ef2c763327a46db503121d3b73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Tue, 22 Dec 2020 19:56:51 +0200 Subject: [PATCH 6/9] Fix bulk action notifications --- app/controllers/registrar/domain_transfers_controller.rb | 2 +- app/views/registrar/bulk_change/_api_errors.html.erb | 4 ++++ .../registrar/domain_transfers/form/_api_errors.html.erb | 4 ++++ .../registrar_area/bulk_change/bulk_transfer_test.rb | 8 ++++---- .../registrar_area/bulk_change/tech_contact_test.rb | 6 +++--- 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/app/controllers/registrar/domain_transfers_controller.rb b/app/controllers/registrar/domain_transfers_controller.rb index 03874adb5..584a50d33 100644 --- a/app/controllers/registrar/domain_transfers_controller.rb +++ b/app/controllers/registrar/domain_transfers_controller.rb @@ -56,7 +56,7 @@ class Registrar if response.code == '200' failed = parsed_response[:data][:failed].each(&:domain_name).join(', ') - flash[:notice] = t('.transferred', success: parsed_response[:data][:success].size, + flash[:notice] = t('.transferred', count: parsed_response[:data][:success].size, failed: failed) redirect_to registrar_domains_url else diff --git a/app/views/registrar/bulk_change/_api_errors.html.erb b/app/views/registrar/bulk_change/_api_errors.html.erb index 56bf8c404..8d8862959 100644 --- a/app/views/registrar/bulk_change/_api_errors.html.erb +++ b/app/views/registrar/bulk_change/_api_errors.html.erb @@ -1,9 +1,13 @@ <% if @api_errors %>
    + <% if @api_errors.is_a?(String) %> +
  • <%= @api_errors %>
    • + <% else %> <% @api_errors.each do |error| %>
  • <%= error[:title] %>
    • <% end %> + <% end %>
<% end %> diff --git a/app/views/registrar/domain_transfers/form/_api_errors.html.erb b/app/views/registrar/domain_transfers/form/_api_errors.html.erb index 56bf8c404..8d8862959 100644 --- a/app/views/registrar/domain_transfers/form/_api_errors.html.erb +++ b/app/views/registrar/domain_transfers/form/_api_errors.html.erb @@ -1,9 +1,13 @@ <% if @api_errors %>
    + <% if @api_errors.is_a?(String) %> +
  • <%= @api_errors %>
    • + <% else %> <% @api_errors.each do |error| %>
  • <%= error[:title] %>
    • <% end %> + <% end %>
<% end %> diff --git a/test/system/registrar_area/bulk_change/bulk_transfer_test.rb b/test/system/registrar_area/bulk_change/bulk_transfer_test.rb index a531ff4dc..820b1cf96 100644 --- a/test/system/registrar_area/bulk_change/bulk_transfer_test.rb +++ b/test/system/registrar_area/bulk_change/bulk_transfer_test.rb @@ -11,9 +11,9 @@ class RegistrarAreaBulkTransferTest < ApplicationSystemTestCase request_stub = stub_request(:post, /domains\/transfer/).with(body: request_body, headers: headers, basic_auth: ['test_goodnames', 'testtest']) - .to_return(body: { data: [{ - type: 'domain_transfer' - }] }.to_json, status: 200) + .to_return(body: { data: { success: [{ type: 'domain_transfer', domain_name: 'shop.test' }], + failed: [] + } }.to_json, status: 200) visit registrar_domains_url click_link 'Bulk change' @@ -27,7 +27,7 @@ class RegistrarAreaBulkTransferTest < ApplicationSystemTestCase end def test_fail_gracefully - body = { errors: [{ title: 'epic fail' }] }.to_json + body = { message: 'epic fail' }.to_json headers = { 'Content-type' => Mime[:json] } stub_request(:post, /domains\/transfer/).to_return(status: 400, body: body, headers: headers) diff --git a/test/system/registrar_area/bulk_change/tech_contact_test.rb b/test/system/registrar_area/bulk_change/tech_contact_test.rb index c678e8f34..e08457f60 100644 --- a/test/system/registrar_area/bulk_change/tech_contact_test.rb +++ b/test/system/registrar_area/bulk_change/tech_contact_test.rb @@ -9,8 +9,8 @@ class RegistrarAreaTechContactBulkChangeTest < ApplicationSystemTestCase request_stub = stub_request(:patch, /domains\/contacts/) .with(body: { current_contact_id: 'william-001', new_contact_id: 'john-001' }, basic_auth: ['test_bestnames', 'testtest']) - .to_return(body: { affected_domains: %w[foo.test bar.test], - skipped_domains: %w[baz.test qux.test] }.to_json, + .to_return(body: { data: { affected_domains: %w[foo.test bar.test], + skipped_domains: %w[baz.test qux.test] } }.to_json, status: 200) visit registrar_domains_url @@ -30,7 +30,7 @@ class RegistrarAreaTechContactBulkChangeTest < ApplicationSystemTestCase def test_fails_gracefully stub_request(:patch, /domains\/contacts/) .to_return(status: 400, - body: { error: { message: 'epic fail' } }.to_json, + body: { message: 'epic fail' }.to_json, headers: { 'Content-type' => Mime[:json] }) visit registrar_domains_url From 490467b5d922b99d18dca68b50d4aa479dbf229b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Tue, 22 Dec 2020 20:15:01 +0200 Subject: [PATCH 7/9] REPP: check webclient IPs to bypass registrar IP whitelist --- app/controllers/repp/v1/base_controller.rb | 24 +++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/app/controllers/repp/v1/base_controller.rb b/app/controllers/repp/v1/base_controller.rb index ca9ef6fb5..76735a686 100644 --- a/app/controllers/repp/v1/base_controller.rb +++ b/app/controllers/repp/v1/base_controller.rb @@ -3,6 +3,7 @@ module Repp class BaseController < ActionController::API rescue_from ActiveRecord::RecordNotFound, with: :not_found_error before_action :authenticate_user + before_action :validate_webclient_ca before_action :check_ip_restriction attr_reader :current_user @@ -93,15 +94,32 @@ module Repp end def check_ip_restriction - allowed = @current_user.registrar.api_ip_white?(request.ip) - - return if allowed + return if webclient_request? + return if @current_user.registrar.api_ip_white?(request.ip) @response = { code: 2202, message: I18n.t('registrar.authorization.ip_not_allowed', ip: request.ip) } render(json: @response, status: :unauthorized) end + def webclient_request? + return if Rails.env.test? + ENV['webclient_ips'].split(',').map(&:strip).include?(request.ip) + end + + def validate_webclient_ca + return unless webclient_request? + + request_name = request.env['HTTP_SSL_CLIENT_S_DN_CN'] + webclient_cn = ENV['webclient_cert_common_name'] || 'webclient' + return if request_name == webclient_cn + + @response = { code: 2202, + message: I18n.t('registrar.authorization.ip_not_allowed', ip: request.ip) } + + render(json: @response, status: :unauthorized) + end + def not_found_error @response = { code: 2303, message: 'Object does not exist' } render(json: @response, status: :not_found) From db4f57308b83ffbe77cc9ce69f1867e783319077 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Wed, 23 Dec 2020 11:30:29 +0200 Subject: [PATCH 8/9] Fix some CC issues --- app/controllers/registrar/tech_contacts_controller.rb | 6 ++++-- app/controllers/repp/v1/base_controller.rb | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/controllers/registrar/tech_contacts_controller.rb b/app/controllers/registrar/tech_contacts_controller.rb index 057d01857..001651250 100644 --- a/app/controllers/registrar/tech_contacts_controller.rb +++ b/app/controllers/registrar/tech_contacts_controller.rb @@ -43,10 +43,12 @@ class Registrar if response.code == '200' notices = [t('.replaced')] - notices << "#{t('.affected_domains')}: #{parsed_response[:data][:affected_domains].join(', ')}" + notices << "#{t('.affected_domains')}: " \ + "#{parsed_response[:data][:affected_domains].join(', ')}" if parsed_response[:data][:skipped_domains] - notices << "#{t('.skipped_domains')}: #{parsed_response[:data][:skipped_domains].join(', ')}" + notices << "#{t('.skipped_domains')}: " \ + "#{parsed_response[:data][:skipped_domains].join(', ')}" end flash[:notice] = notices.join(', ') diff --git a/app/controllers/repp/v1/base_controller.rb b/app/controllers/repp/v1/base_controller.rb index 76735a686..2814ce2da 100644 --- a/app/controllers/repp/v1/base_controller.rb +++ b/app/controllers/repp/v1/base_controller.rb @@ -104,6 +104,7 @@ module Repp def webclient_request? return if Rails.env.test? + ENV['webclient_ips'].split(',').map(&:strip).include?(request.ip) end From bc69e05c91141d80417ce7db6071bbc2390c555e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20V=C3=B5hmar?= Date: Wed, 23 Dec 2020 12:38:29 +0200 Subject: [PATCH 9/9] Update CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa814db62..e9135d83f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +23.12.2020 +* fix for REPP logging and registrar portal communication [#1782](https://github.com/internetee/registry/pull/1782) + 22.12.2020 * SSL CA verification fix for Bulk renew [#1778](https://github.com/internetee/registry/pull/1778)