diff --git a/app/controllers/registrant/sessions_controller.rb b/app/controllers/registrant/sessions_controller.rb index f1d39898e..b18a802e6 100644 --- a/app/controllers/registrant/sessions_controller.rb +++ b/app/controllers/registrant/sessions_controller.rb @@ -1,26 +1,6 @@ class Registrant::SessionsController < Devise::SessionsController layout 'registrant/application' - def new; end - - def id - client_certificate_subject = request.env['SSL_CLIENT_S_DN'] - client_certificate_issuer = request.env['SSL_CLIENT_I_DN_O'] - - if Rails.env.development? - client_certificate_subject = 'test' - client_certificate_issuer = RegistrantUser::ACCEPTED_ISSUER - end - - @user = RegistrantUser.find_or_create_by_idc_data(client_certificate_subject, client_certificate_issuer) - if @user - sign_in_and_redirect(:registrant_user, @user, event: :authentication) - else - flash[:alert] = t('login_failed_check_id_card') - redirect_to new_registrant_user_session_url - end - end - def login_mid @user = User.new end diff --git a/app/controllers/registrar/sessions_controller.rb b/app/controllers/registrar/sessions_controller.rb index 991d5cbed..e8873c584 100644 --- a/app/controllers/registrar/sessions_controller.rb +++ b/app/controllers/registrar/sessions_controller.rb @@ -48,15 +48,22 @@ class Registrar end end - def id - @user = ApiUser.find_by_idc_data_and_allowed(request.env['SSL_CLIENT_S_DN'], request.ip) + def id_card + self.resource = warden.authenticate!(auth_options) - if @user - sign_in_and_redirect(:registrar_user, @user, event: :authentication) - else - flash[:alert] = t('no_such_user') - redirect_to new_registrar_user_session_url + restricted_ip = Authorization::RestrictedIP.new(request.ip) + ip_allowed = restricted_ip.can_access_registrar_area?(resource.registrar) + + unless ip_allowed + render text: t('registrar.authorization.ip_not_allowed', ip: request.ip) + warden.logout(:registrar_user) + return end + + set_flash_message!(:notice, :signed_in) + sign_in(resource_name, resource) + yield resource if block_given? + respond_with resource, location: after_sign_in_path_for(resource) end def login_mid diff --git a/app/models/api_user.rb b/app/models/api_user.rb index 023d5812c..d08f17380 100644 --- a/app/models/api_user.rb +++ b/app/models/api_user.rb @@ -2,7 +2,8 @@ require 'open3' class ApiUser < User include EppErrors - devise :database_authenticatable, :trackable, :timeoutable, authentication_keys: [:username] + devise :database_authenticatable, :trackable, :timeoutable, :id_card_authenticatable, + authentication_keys: [:username] def epp_code_map { @@ -47,19 +48,9 @@ class ApiUser < User end class << self - def find_by_idc_data_and_allowed(idc_data, ip) - return false if idc_data.blank? - identity_code = idc_data.scan(/serialNumber=(\d+)/).flatten.first - - return false if ip.blank? - possible_users = where(identity_code: identity_code) - possible_users.each do |selected_user| - if selected_user.registrar.white_ips.registrar_area.include_ip?(ip) - return selected_user - end - end + def find_by_id_card(id_card) + find_by(identity_code: id_card.personal_code) end - end def registrar_typeahead diff --git a/app/models/id_card.rb b/app/models/id_card.rb new file mode 100644 index 000000000..0e3c11bb3 --- /dev/null +++ b/app/models/id_card.rb @@ -0,0 +1,6 @@ +class IdCard + attr_accessor :first_name + attr_accessor :last_name + attr_accessor :personal_code + attr_accessor :country_code +end \ No newline at end of file diff --git a/app/models/registrant_user.rb b/app/models/registrant_user.rb index 1c4533bd3..1e787b8b3 100644 --- a/app/models/registrant_user.rb +++ b/app/models/registrant_user.rb @@ -1,8 +1,7 @@ class RegistrantUser < User - ACCEPTED_ISSUER = 'AS Sertifitseerimiskeskus'.freeze attr_accessor :idc_data - devise :database_authenticatable, :trackable, :timeoutable + devise :trackable, :timeoutable, :id_card_authenticatable def ability @ability ||= Ability.new(self) @@ -56,30 +55,6 @@ class RegistrantUser < User end class << self - def find_or_create_by_idc_data(idc_data, issuer_organization) - return false if idc_data.blank? - return false if issuer_organization != ACCEPTED_ISSUER - - idc_data.force_encoding('UTF-8') - user_data = {} - - # handling here new and old mode - if idc_data.starts_with?('/') - user_data[:ident] = idc_data.scan(/serialNumber=(\d+)/).flatten.first - user_data[:country_code] = idc_data.scan(/^\/C=(.{2})/).flatten.first - user_data[:first_name] = idc_data.scan(%r{/GN=(.+)/serialNumber}).flatten.first - user_data[:last_name] = idc_data.scan(%r{/SN=(.+)/GN}).flatten.first - else - parse_str = ',' + idc_data - user_data[:ident] = parse_str.scan(/,serialNumber=(\d+)/).flatten.first - user_data[:country_code] = parse_str.scan(/,C=(.{2})/).flatten.first - user_data[:first_name] = parse_str.scan(/,GN=([^,]+)/).flatten.first - user_data[:last_name] = parse_str.scan(/,SN=([^,]+)/).flatten.first - end - - find_or_create_by_user_data(user_data) - end - def find_or_create_by_api_data(user_data = {}) return false unless user_data[:ident] return false unless user_data[:first_name] @@ -98,6 +73,16 @@ class RegistrantUser < User find_or_create_by_user_data(user_data) end + def find_by_id_card(id_card) + registrant_ident = "#{id_card.country_code}-#{id_card.personal_code}" + username = [id_card.first_name, id_card.last_name].join("\s") + + user = find_or_initialize_by(registrant_ident: registrant_ident) + user.username = username + user.save! + user + end + private def find_or_create_by_user_data(user_data = {}) diff --git a/app/views/registrant/sessions/new.html.erb b/app/views/registrant/sessions/new.html.erb index d711601b3..a3203e83a 100644 --- a/app/views/registrant/sessions/new.html.erb +++ b/app/views/registrant/sessions/new.html.erb @@ -11,7 +11,7 @@ <%= link_to '/registrant/login/mid' do %> <%= image_tag 'mid.gif' %> <% end %> - <%= link_to '/registrant/id', method: :post do %> + <%= link_to registrant_id_card_sign_in_path, method: :post do %> <%= image_tag 'id_card.gif' %> <% end %> diff --git a/app/views/registrar/sessions/new.html.erb b/app/views/registrar/sessions/new.html.erb index 701c41c9e..8056b07c8 100644 --- a/app/views/registrar/sessions/new.html.erb +++ b/app/views/registrar/sessions/new.html.erb @@ -23,7 +23,7 @@ <%= image_tag 'mid.gif' %> <% end %> - <%= link_to '/registrar/id', method: :post do %> + <%= link_to registrar_id_card_sign_in_path, method: :post do %> <%= image_tag 'id_card.gif' %> <% end %> diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 56791c45d..10f937f40 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -280,4 +280,10 @@ Devise.setup do |config| # When using OmniAuth, Devise cannot automatically set OmniAuth path, # so you need to do it manually. For the users scope, it would be: # config.omniauth_path_prefix = '/my_engine/users/auth' + + require 'devise/models/id_card_authenticatable' + require 'devise/strategies/id_card_authenticatable' + + routes = [nil, :new, :destroy] + config.add_module :id_card_authenticatable, strategy: true, route: { session: routes } end diff --git a/config/locales/en.yml b/config/locales/en.yml index 75d2d8850..75e65f602 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -585,7 +585,6 @@ en: pending_delete_expired_notification_subject: "Domeeni %{name} kustutamise taotlus on tühistatud / %{name} deletion cancelled" delete_confirmation_subject: "Domeeni %{name} kustutatud / %{name} deleted" whois: WHOIS - login_failed_check_id_card: 'Log in failed, check ID card' not_valid_domain_verification_title: Domain verification not available not_valid_domain_verification_body: This could mean your verification has been expired or done already.

Please contact us if you think something is wrong. upload_crt: 'Upload CRT' diff --git a/config/routes.rb b/config/routes.rb index 4f03ffae8..453ac067a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -46,7 +46,10 @@ Rails.application.routes.draw do get 'login/mid' => 'sessions#login_mid' post 'login/mid' => 'sessions#mid' post 'login/mid_status' => 'sessions#mid_status' - post 'id' => 'sessions#id' + + # /registrar/id path is hardcoded in Apache config for authentication with Estonian ID-card + post 'id' => 'sessions#id_card', as: :id_card_sign_in + post 'mid' => 'sessions#mid' end @@ -137,7 +140,11 @@ Rails.application.routes.draw do post 'login/mid' => 'sessions#mid' post 'login/mid_status' => 'sessions#mid_status' post 'mid' => 'sessions#mid' - post 'id' => 'sessions#id' + + # /registrant/id path is hardcoded in Apache config for authentication with Estonian ID-card + # Client certificate is asked only on login form submission, therefore the path must be different from the one in + # `new_registrant_user_session_path` route, in case some other auth type will be implemented + post 'id' => 'sessions#create', as: :id_card_sign_in end resources :registrars, only: :show diff --git a/lib/devise/models/id_card_authenticatable.rb b/lib/devise/models/id_card_authenticatable.rb new file mode 100644 index 000000000..53bad663f --- /dev/null +++ b/lib/devise/models/id_card_authenticatable.rb @@ -0,0 +1,7 @@ +module Devise + module Models + # Devise fails without this module (and model: false does not help) + module IdCardAuthenticatable + end + end +end \ No newline at end of file diff --git a/lib/devise/strategies/id_card_authenticatable.rb b/lib/devise/strategies/id_card_authenticatable.rb new file mode 100644 index 000000000..ec26bb4d9 --- /dev/null +++ b/lib/devise/strategies/id_card_authenticatable.rb @@ -0,0 +1,49 @@ +module Devise + module Strategies + class IdCardAuthenticatable < Devise::Strategies::Authenticatable + def valid? + env['SSL_CLIENT_S_DN_CN'].present? + end + + def authenticate! + resource = mapping.to + user = resource.find_by_id_card(id_card) + + if user + success!(user) + else + fail + end + end + + private + + def id_card + id_card = IdCard.new + id_card.first_name = first_name + id_card.last_name = last_name + id_card.personal_code = personal_code + id_card.country_code = country_code + id_card + end + + def first_name + env['SSL_CLIENT_S_DN_CN'].split(',').second.force_encoding('utf-8') + end + + def last_name + env['SSL_CLIENT_S_DN_CN'].split(',').first.force_encoding('utf-8') + end + + def personal_code + env['SSL_CLIENT_S_DN_CN'].split(',').last + end + + def country_code + env['SSL_CLIENT_I_DN_C'] + end + end + end +end + +Warden::Strategies.add(:id_card_authenticatable, Devise::Strategies::IdCardAuthenticatable) \ No newline at end of file diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 057a64557..49508c0cf 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -1,6 +1,7 @@ api_bestnames: username: test_bestnames plain_text_password: testtest + identity_code: 1234 type: ApiUser registrar: bestnames active: true diff --git a/test/fixtures/white_ips.yml b/test/fixtures/white_ips.yml new file mode 100644 index 000000000..bc756f06e --- /dev/null +++ b/test/fixtures/white_ips.yml @@ -0,0 +1,6 @@ +one: + registrar: bestnames + ipv4: 127.0.0.1 + interfaces: + - <%= WhiteIp::REGISTRAR %> + - <%= WhiteIp::API %> \ No newline at end of file diff --git a/test/integration/registrant_area/sign_in/id_card_test.rb b/test/integration/registrant_area/sign_in/id_card_test.rb new file mode 100644 index 000000000..68f0d408e --- /dev/null +++ b/test/integration/registrant_area/sign_in/id_card_test.rb @@ -0,0 +1,31 @@ +require 'test_helper' + +class RegistrantAreaIdCardSignInTest < ApplicationIntegrationTest + setup do + allow_business_registry_component_reach_server + end + + def test_succeeds + post_via_redirect registrant_id_card_sign_in_path, nil, + 'SSL_CLIENT_S_DN_CN' => 'DOE,JOHN,1234', + 'SSL_CLIENT_I_DN_C' => 'US' + + assert_response :ok + assert_equal registrant_root_path, path + assert_not_nil controller.current_registrant_user + end + + def test_fails_when_certificate_is_absent + post_via_redirect registrant_id_card_sign_in_path, nil, 'SSL_CLIENT_S_DN_CN' => '' + + assert_response :ok + assert_equal registrant_id_card_sign_in_path, path + assert_nil controller.current_registrant_user + end + + private + + def allow_business_registry_component_reach_server + WebMock.allow_net_connect! + end +end \ No newline at end of file diff --git a/test/integration/registrar_area/sign_in/id_card_test.rb b/test/integration/registrar_area/sign_in/id_card_test.rb new file mode 100644 index 000000000..750712f34 --- /dev/null +++ b/test/integration/registrar_area/sign_in/id_card_test.rb @@ -0,0 +1,61 @@ +require 'test_helper' + +class RegistrarAreaIdCardSignInTest < ApplicationIntegrationTest + setup do + @user = users(:api_bestnames) + end + + def test_signs_in_a_user_when_id_card_owner_is_found + assert_equal '1234', @user.identity_code + + post_via_redirect registrar_id_card_sign_in_path, nil, 'SSL_CLIENT_S_DN_CN' => 'DOE,JOHN,1234' + + assert_response :ok + assert_equal registrar_root_path, path + assert_not_nil controller.current_registrar_user + end + + def test_does_not_sign_in_a_user_when_id_card_owner_is_not_found + post_via_redirect registrar_id_card_sign_in_path, nil, + 'SSL_CLIENT_S_DN_CN' => 'DOE,JOHN,unacceptable-personal-code' + + assert_nil controller.current_registrar_user + assert_equal registrar_id_card_sign_in_path, path + assert_includes response.body, 'Failed to Login' + end + + def test_does_not_sign_in_a_user_when_id_card_owner_is_found_but_ip_is_not_allowed + allow_access_to_sign_in_page + assert_equal '127.0.0.1', white_ips(:one).ipv4 + assert_equal '1234', @user.identity_code + + Setting.registrar_ip_whitelist_enabled = true + + post registrar_id_card_sign_in_path, nil, 'SSL_CLIENT_S_DN_CN' => 'DOE,JOHN,1234', + 'REMOTE_ADDR' => '127.0.0.2' + + assert_equal registrar_id_card_sign_in_path, path + assert_equal 'Access denied from IP 127.0.0.2', response.body + + get registrar_root_path + assert_redirected_to new_registrar_user_session_path + + Setting.registrar_ip_whitelist_enabled = false + end + + def test_does_not_sign_in_a_user_when_certificate_is_absent + post_via_redirect registrar_id_card_sign_in_path, nil, 'SSL_CLIENT_S_DN_CN' => '' + + assert_nil controller.current_registrar_user + assert_equal registrar_id_card_sign_in_path, path + end + + private + + def allow_access_to_sign_in_page + another_registrar_white_ip = white_ips(:one).dup + another_registrar_white_ip.ipv4 = '127.0.0.2' + another_registrar_white_ip.registrar = registrars(:goodnames) + another_registrar_white_ip.save! + end +end \ No newline at end of file diff --git a/test/lib/devise/strategies/id_card_authenticatable_test.rb b/test/lib/devise/strategies/id_card_authenticatable_test.rb new file mode 100644 index 000000000..e194ccaac --- /dev/null +++ b/test/lib/devise/strategies/id_card_authenticatable_test.rb @@ -0,0 +1,13 @@ +require 'test_helper' + +class IdCardAuthenticatableTest < ActiveSupport::TestCase + def test_valid_when_id_card_data_is_present_in_env + strategy = Devise::Strategies::IdCardAuthenticatable.new({ 'SSL_CLIENT_S_DN_CN' => 'some' }) + assert strategy.valid? + end + + def test_not_valid_when_id_card_data_is_absent_in_env + strategy = Devise::Strategies::IdCardAuthenticatable.new({}) + assert_not strategy.valid? + end +end \ No newline at end of file diff --git a/test/models/api_user_test.rb b/test/models/api_user_test.rb new file mode 100644 index 000000000..12f434a49 --- /dev/null +++ b/test/models/api_user_test.rb @@ -0,0 +1,18 @@ +require 'test_helper' + +class ApiUserTest < ActiveSupport::TestCase + setup do + @user = users(:api_bestnames) + end + + def test_finds_user_by_id_card + id_card = IdCard.new + id_card.personal_code = 'one' + + @user.update!(identity_code: 'one') + assert_equal @user, ApiUser.find_by_id_card(id_card) + + @user.update!(identity_code: 'another') + assert_nil ApiUser.find_by_id_card(id_card) + end +end \ No newline at end of file diff --git a/test/models/registrant_user/registrant_user_creation_test.rb b/test/models/registrant_user/registrant_user_creation_test.rb index fc5a32b4c..42fb0e0f6 100644 --- a/test/models/registrant_user/registrant_user_creation_test.rb +++ b/test/models/registrant_user/registrant_user_creation_test.rb @@ -35,22 +35,4 @@ class RegistrantUserCreationTest < ActiveSupport::TestCase user = User.find_by(registrant_ident: 'EE-37710100070') assert_equal('JOHN SMITH', user.username) end - - def test_find_or_create_by_idc_with_legacy_header_creates_a_user - header = '/C=EE/O=ESTEID/OU=authentication/CN=SMITH,JOHN,37710100070/SN=SMITH/GN=JOHN/serialNumber=37710100070' - - RegistrantUser.find_or_create_by_idc_data(header, RegistrantUser::ACCEPTED_ISSUER) - - user = User.find_by(registrant_ident: 'EE-37710100070') - assert_equal('JOHN SMITH', user.username) - end - - def test_find_or_create_by_idc_with_rfc2253_header_creates_a_user - header = 'serialNumber=37710100070,GN=JOHN,SN=SMITH,CN=SMITH\\,JOHN\\,37710100070,OU=authentication,O=ESTEID,C=EE' - - RegistrantUser.find_or_create_by_idc_data(header, RegistrantUser::ACCEPTED_ISSUER) - - user = User.find_by(registrant_ident: 'EE-37710100070') - assert_equal('JOHN SMITH', user.username) - end end diff --git a/test/models/registrant_user_test.rb b/test/models/registrant_user_test.rb index 15a5238f4..78b9ef901 100644 --- a/test/models/registrant_user_test.rb +++ b/test/models/registrant_user_test.rb @@ -30,6 +30,34 @@ class RegistrantUserTest < ActiveSupport::TestCase assert_equal Country.new('US'), user.country end + def test_finding_by_id_card_creates_new_user_upon_first_sign_in + assert_not_equal 'US-5555', @user.registrant_ident + id_card = IdCard.new + id_card.first_name = 'John' + id_card.last_name = 'Doe' + id_card.personal_code = '5555' + id_card.country_code = 'US' + + assert_difference 'RegistrantUser.count' do + RegistrantUser.find_by_id_card(id_card) + end + + user = RegistrantUser.last + assert_equal 'US-5555', user.registrant_ident + assert_equal 'John Doe', user.username + end + + def test_finding_by_id_card_reuses_existing_user_upon_subsequent_id_card_sign_ins + @user.update!(registrant_ident: 'US-5555') + id_card = IdCard.new + id_card.personal_code = '5555' + id_card.country_code = 'US' + + assert_no_difference 'RegistrantUser.count' do + RegistrantUser.find_by_id_card(id_card) + end + end + def test_queries_company_register_for_associated_companies assert_equal 'US-1234', @user.registrant_ident