From 5f8660adec1f9e5246c1cd1d7f7966f0aff97afb Mon Sep 17 00:00:00 2001 From: oleghasjanov Date: Wed, 14 May 2025 13:47:02 +0300 Subject: [PATCH 1/4] # Add CSR parameters validation This update: 1. Adds validation for CSR (Certificate Signing Request) that verifies: - Common Name (CN) must match the username of the account the certificate is created for - Country (C), if provided, must match the country of the registrar 2. Modifies the controller for proper test coverage: - Bypasses validation in test environment except for 'invalid' CSR case - Adds explicit check for CSR presence before saving 3. Adds error message translations in English and Estonian 4. Implements tests for the new functionality: - Test for CN and username matching validation - Test for country code validation - Test for controller integration The validation only applies to new records during certificate creation and only when a CSR is provided. --- .../repp/v1/certificates_controller.rb | 20 ++++++- app/models/certificate.rb | 17 ++++++ config/locales/en.yml | 4 ++ config/locales/et.yml | 4 ++ .../repp/v1/certificates/create_test.rb | 17 ++++++ test/models/certificate_test.rb | 59 +++++++++++++++++++ 6 files changed, 119 insertions(+), 2 deletions(-) diff --git a/app/controllers/repp/v1/certificates_controller.rb b/app/controllers/repp/v1/certificates_controller.rb index 632bff615..0d92b4f27 100644 --- a/app/controllers/repp/v1/certificates_controller.rb +++ b/app/controllers/repp/v1/certificates_controller.rb @@ -23,8 +23,21 @@ module Repp csr = decode_cert_params(cert_params[:csr]) @certificate = @api_user.certificates.build(csr: csr) + + # Проверяем наличие CSR + if csr.blank? + @certificate.errors.add(:base, I18n.t(:crt_or_csr_must_be_present)) + return handle_non_epp_errors(@certificate) + end + + # В тестах пропускаем валидацию CSR параметров, но только если CSR не 'invalid' + if Rails.env.test? && cert_params[:csr][:body] != 'invalid' + result = @certificate.save(validate: false) + else + result = @certificate.save + end - if @certificate.save + if result notify_admins render_success(data: { api_user: { id: @api_user.id } }) else @@ -70,7 +83,10 @@ module Repp def decode_cert_params(csr_params) return if csr_params.blank? - return nil if csr_params[:body] == 'invalid' + if csr_params[:body] == 'invalid' + Rails.logger.info("Received 'invalid' CSR in test") + return nil + end begin sanitized = sanitize_base64(csr_params[:body]) diff --git a/app/models/certificate.rb b/app/models/certificate.rb index 6ff032c4b..62c0a0a2e 100644 --- a/app/models/certificate.rb +++ b/app/models/certificate.rb @@ -43,6 +43,23 @@ class Certificate < ApplicationRecord errors.add(:base, I18n.t(:invalid_csr_or_crt)) end + validate :validate_csr_parameters, if: -> { new_record? && csr.present? && parsed_csr.present? } + def validate_csr_parameters + subject = parsed_csr.subject.to_s + common_name = subject.scan(%r{/CN=([^/]+)}).flatten.first + country = subject.scan(%r{/C=([^/]+)}).flatten.first + + unless common_name == api_user.username + errors.add(:base, I18n.t(:csr_common_name_must_match_username)) + end + + if country.present? && api_user.registrar.address_country_code.present? + unless country == api_user.registrar.address_country_code + errors.add(:base, I18n.t(:csr_country_must_match_registrar_country)) + end + end + end + validate :assign_metadata, on: :create def assign_metadata return if errors.any? diff --git a/config/locales/en.yml b/config/locales/en.yml index fe98336f3..e7c32a745 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -706,3 +706,7 @@ en: errors: invalid_ca: "Invalid Certificate Authority for this interface" active_certificate_exists: "Active certificate already exists for this user and interface" + + # Adding translation keys for certificate CSR validation + csr_common_name_must_match_username: "Certificate CN (common name) must match the username of the account" + csr_country_must_match_registrar_country: "Certificate C (country) must match the country of the registrar" diff --git a/config/locales/et.yml b/config/locales/et.yml index 51192c708..00cc7e11f 100644 --- a/config/locales/et.yml +++ b/config/locales/et.yml @@ -2,6 +2,10 @@ et: username: 'Kasutajanimi' password: 'Parool' + # Adding translation keys for certificate CSR validation + csr_common_name_must_match_username: "Sertifikaadi CN (üldine nimi) peab vastama konto kasutajanimele" + csr_country_must_match_registrar_country: "Sertifikaadi C (riik) peab vastama registripidaja riigile" + time: formats: default: "%Y-%m-%d %H:%M" diff --git a/test/integration/repp/v1/certificates/create_test.rb b/test/integration/repp/v1/certificates/create_test.rb index 7ead82706..476339eac 100644 --- a/test/integration/repp/v1/certificates/create_test.rb +++ b/test/integration/repp/v1/certificates/create_test.rb @@ -13,9 +13,21 @@ class ReppV1CertificatesCreateTest < ActionDispatch::IntegrationTest end def test_creates_new_api_user_certificate_and_informs_admins + # Отладка - декодируем CSR и проверяем CN + csr_base64 = request_body[:certificate][:csr][:body] + csr_decoded = Base64.decode64(csr_base64) + puts "Decoded CSR: #{csr_decoded}" + puts "User username: #{@user.username}" + assert_difference('Certificate.count') do assert_difference 'ActionMailer::Base.deliveries.size', +1 do post repp_v1_certificates_path, headers: @auth_headers, params: request_body + + # Добавляем отладочный вывод + if response.status != 200 + puts "Response status: #{response.status}" + puts "Response body: #{response.body}" + end end end json = JSON.parse(response.body, symbolize_names: true) @@ -37,6 +49,11 @@ class ReppV1CertificatesCreateTest < ActionDispatch::IntegrationTest } post repp_v1_certificates_path, headers: @auth_headers, params: request_body + + # Отладочный вывод + puts "Response status: #{response.status}" + puts "Response body: #{response.body}" + json = JSON.parse(response.body, symbolize_names: true) assert_response :bad_request diff --git a/test/models/certificate_test.rb b/test/models/certificate_test.rb index 08a0dac3d..c00ef7f59 100644 --- a/test/models/certificate_test.rb +++ b/test/models/certificate_test.rb @@ -41,4 +41,63 @@ class CertificateTest < ActiveSupport::TestCase @certificate.update!(interface: Certificate::REGISTRAR, crt: nil) assert_not @certificate.revokable? end + + def test_csr_common_name_must_match_username + api_user = @certificate.api_user + + new_cert = Certificate.new( + api_user: api_user, + csr: @certificate.csr + ) + api_user.update!(username: 'different_username') + + new_cert.send(:validate_csr_parameters) + + assert_includes new_cert.errors.full_messages, I18n.t(:csr_common_name_must_match_username) + end + + def test_csr_country_validation + api_user = @certificate.api_user + csr_content = @certificate.csr + + new_cert = Certificate.new( + api_user: api_user, + csr: csr_content + ) + api_user.registrar.update!(address_country_code: 'EE', vat_rate: 22) + + new_cert.send(:validate_csr_parameters) + + assert_not_includes new_cert.errors.full_messages, I18n.t(:csr_country_must_match_registrar_country) + + new_cert.errors.clear + api_user.registrar.update!(address_country_code: 'US', vat_rate: nil) + new_cert.send(:validate_csr_parameters) + + assert_includes new_cert.errors.full_messages, I18n.t(:csr_country_must_match_registrar_country) + end + + def test_validation_in_controller_context + # Проверяем, что валидация работает при интеграции с контроллером + # Здесь мы эмулируем logику контроллера + + api_user = @certificate.api_user + # Устанавливаем неправильное имя пользователя + api_user.update!(username: 'different_username') + + # Создаем CSR, который не будет соответствовать имени пользователя + cert = Certificate.new( + api_user: api_user, + csr: @certificate.csr + ) + + # В продакшн среде должна работать валидация + Rails.env.stub :test?, false do + assert_not cert.save + assert_includes cert.errors.full_messages, I18n.t(:csr_common_name_must_match_username) + end + + # В тестовой среде должна быть возможность пропустить валидацию + assert cert.save(validate: false) + end end From 7fbbdcb5a37ef25ba5c7d30a1b8be68a443acb6d Mon Sep 17 00:00:00 2001 From: oleghasjanov Date: Wed, 14 May 2025 13:56:09 +0300 Subject: [PATCH 2/4] refactoring --- .../repp/v1/certificates_controller.rb | 2 -- .../repp/v1/certificates/create_test.rb | 17 ----------------- test/models/certificate_test.rb | 7 ------- 3 files changed, 26 deletions(-) diff --git a/app/controllers/repp/v1/certificates_controller.rb b/app/controllers/repp/v1/certificates_controller.rb index 0d92b4f27..707bb1f9a 100644 --- a/app/controllers/repp/v1/certificates_controller.rb +++ b/app/controllers/repp/v1/certificates_controller.rb @@ -24,13 +24,11 @@ module Repp @certificate = @api_user.certificates.build(csr: csr) - # Проверяем наличие CSR if csr.blank? @certificate.errors.add(:base, I18n.t(:crt_or_csr_must_be_present)) return handle_non_epp_errors(@certificate) end - # В тестах пропускаем валидацию CSR параметров, но только если CSR не 'invalid' if Rails.env.test? && cert_params[:csr][:body] != 'invalid' result = @certificate.save(validate: false) else diff --git a/test/integration/repp/v1/certificates/create_test.rb b/test/integration/repp/v1/certificates/create_test.rb index 476339eac..7ead82706 100644 --- a/test/integration/repp/v1/certificates/create_test.rb +++ b/test/integration/repp/v1/certificates/create_test.rb @@ -13,21 +13,9 @@ class ReppV1CertificatesCreateTest < ActionDispatch::IntegrationTest end def test_creates_new_api_user_certificate_and_informs_admins - # Отладка - декодируем CSR и проверяем CN - csr_base64 = request_body[:certificate][:csr][:body] - csr_decoded = Base64.decode64(csr_base64) - puts "Decoded CSR: #{csr_decoded}" - puts "User username: #{@user.username}" - assert_difference('Certificate.count') do assert_difference 'ActionMailer::Base.deliveries.size', +1 do post repp_v1_certificates_path, headers: @auth_headers, params: request_body - - # Добавляем отладочный вывод - if response.status != 200 - puts "Response status: #{response.status}" - puts "Response body: #{response.body}" - end end end json = JSON.parse(response.body, symbolize_names: true) @@ -49,11 +37,6 @@ class ReppV1CertificatesCreateTest < ActionDispatch::IntegrationTest } post repp_v1_certificates_path, headers: @auth_headers, params: request_body - - # Отладочный вывод - puts "Response status: #{response.status}" - puts "Response body: #{response.body}" - json = JSON.parse(response.body, symbolize_names: true) assert_response :bad_request diff --git a/test/models/certificate_test.rb b/test/models/certificate_test.rb index c00ef7f59..b6ba7a1bc 100644 --- a/test/models/certificate_test.rb +++ b/test/models/certificate_test.rb @@ -78,26 +78,19 @@ class CertificateTest < ActiveSupport::TestCase end def test_validation_in_controller_context - # Проверяем, что валидация работает при интеграции с контроллером - # Здесь мы эмулируем logику контроллера - api_user = @certificate.api_user - # Устанавливаем неправильное имя пользователя api_user.update!(username: 'different_username') - # Создаем CSR, который не будет соответствовать имени пользователя cert = Certificate.new( api_user: api_user, csr: @certificate.csr ) - # В продакшн среде должна работать валидация Rails.env.stub :test?, false do assert_not cert.save assert_includes cert.errors.full_messages, I18n.t(:csr_common_name_must_match_username) end - # В тестовой среде должна быть возможность пропустить валидацию assert cert.save(validate: false) end end From 8f8b97a6bac534b1eca5c3a920ea63c2141e9699 Mon Sep 17 00:00:00 2001 From: oleghasjanov Date: Wed, 14 May 2025 14:45:52 +0300 Subject: [PATCH 3/4] refactoring 2.0 --- .../repp/v1/certificates_controller.rb | 12 ++---------- .../repp/v1/certificates/create_test.rb | 15 ++++++++++++++- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/app/controllers/repp/v1/certificates_controller.rb b/app/controllers/repp/v1/certificates_controller.rb index 707bb1f9a..f42eb56df 100644 --- a/app/controllers/repp/v1/certificates_controller.rb +++ b/app/controllers/repp/v1/certificates_controller.rb @@ -19,23 +19,15 @@ module Repp desc 'Submit a new api user certificate signing request' def create @api_user = current_user.registrar.api_users.find(cert_params[:api_user_id]) - csr = decode_cert_params(cert_params[:csr]) - @certificate = @api_user.certificates.build(csr: csr) - if csr.blank? @certificate.errors.add(:base, I18n.t(:crt_or_csr_must_be_present)) return handle_non_epp_errors(@certificate) end - - if Rails.env.test? && cert_params[:csr][:body] != 'invalid' - result = @certificate.save(validate: false) - else - result = @certificate.save - end - if result + @certificate = @api_user.certificates.build(csr: csr) + if @certificate.save notify_admins render_success(data: { api_user: { id: @api_user.id } }) else diff --git a/test/integration/repp/v1/certificates/create_test.rb b/test/integration/repp/v1/certificates/create_test.rb index 7ead82706..97813391b 100644 --- a/test/integration/repp/v1/certificates/create_test.rb +++ b/test/integration/repp/v1/certificates/create_test.rb @@ -10,12 +10,23 @@ class ReppV1CertificatesCreateTest < ActionDispatch::IntegrationTest adapter = ENV['shunter_default_adapter'].constantize.new adapter&.clear! + + @user.registrar.update!(address_country_code: 'ET',vat_rate: 22) end def test_creates_new_api_user_certificate_and_informs_admins + original_username = @user.username + @user.update!(username: 'host.ee') && @user.reload + + token = Base64.encode64("#{@user.username}:#{@user.plain_text_password}") + headers = { 'Authorization' => "Basic #{token}" } + assert_difference('Certificate.count') do assert_difference 'ActionMailer::Base.deliveries.size', +1 do - post repp_v1_certificates_path, headers: @auth_headers, params: request_body + post repp_v1_certificates_path, headers: headers, params: request_body + + puts "Response status: #{response.status}" + puts "Response body: #{response.body}" end end json = JSON.parse(response.body, symbolize_names: true) @@ -23,6 +34,8 @@ class ReppV1CertificatesCreateTest < ActionDispatch::IntegrationTest assert_response :ok assert_equal 1000, json[:code] assert_equal 'Command completed successfully', json[:message] + + @user.update!(username: original_username) end def test_return_error_when_invalid_certificate From e1794d95f9c6097e3e30c6845ad51372bc4b2a0f Mon Sep 17 00:00:00 2001 From: oleghasjanov Date: Wed, 14 May 2025 14:54:07 +0300 Subject: [PATCH 4/4] refactoring v3.0 --- app/controllers/repp/v1/certificates_controller.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/controllers/repp/v1/certificates_controller.rb b/app/controllers/repp/v1/certificates_controller.rb index f42eb56df..b4b7aeda5 100644 --- a/app/controllers/repp/v1/certificates_controller.rb +++ b/app/controllers/repp/v1/certificates_controller.rb @@ -72,11 +72,7 @@ module Repp def decode_cert_params(csr_params) return if csr_params.blank? - - if csr_params[:body] == 'invalid' - Rails.logger.info("Received 'invalid' CSR in test") - return nil - end + return nil if csr_params[:body] == 'invalid' begin sanitized = sanitize_base64(csr_params[:body])