From 04f0ef9a93de02f4742e0ee38b843e18c1896934 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Thu, 1 Oct 2020 17:01:54 +0300 Subject: [PATCH 01/10] Expand TARA auth flow to registrant portal --- app/controllers/registrant/tara_controller.rb | 33 +++++++++++++++++++ app/models/registrant_user.rb | 22 +++++-------- app/views/registrant/sessions/new.html.erb | 9 ++--- config/application.yml.sample | 4 +++ config/initializers/omniauth.rb | 31 +++++++++++++++++ config/locales/registrant/sessions.en.yml | 10 ++---- config/routes.rb | 3 ++ 7 files changed, 85 insertions(+), 27 deletions(-) create mode 100644 app/controllers/registrant/tara_controller.rb diff --git a/app/controllers/registrant/tara_controller.rb b/app/controllers/registrant/tara_controller.rb new file mode 100644 index 0000000000..a09e4665b9 --- /dev/null +++ b/app/controllers/registrant/tara_controller.rb @@ -0,0 +1,33 @@ +class Registrant + class TaraController < ApplicationController + skip_authorization_check + + # rubocop:disable Style/AndOr + def callback + session[:omniauth_hash] = user_hash + @registrant_user = RegistrantUser.find_or_create_by_omniauth_data(user_hash) + + if @registrant_user + flash[:notice] = t(:signed_in_successfully) + sign_in_and_redirect(:registrant_user, @registrant_user) + else + show_error and return + end + end + # rubocop:enable Style/AndOr + + def cancel + redirect_to root_path, notice: t(:sign_in_cancelled) + end + + def show_error + redirect_to new_registrant_user_session_url, alert: t(:no_such_user) + end + + private + + def user_hash + request.env['omniauth.auth'] + end + end +end diff --git a/app/models/registrant_user.rb b/app/models/registrant_user.rb index 7bd84d5dd2..06a2b668ad 100644 --- a/app/models/registrant_user.rb +++ b/app/models/registrant_user.rb @@ -66,21 +66,17 @@ def find_or_create_by_api_data(user_data = {}) find_or_create_by_user_data(user_data) end - def find_or_create_by_mid_data(response) - user_data = { first_name: response.user_givenname, last_name: response.user_surname, - ident: response.user_id_code, country_code: response.user_country } + def find_or_create_by_omniauth_data(omniauth_hash) + uid = omniauth_hash['uid'] + identity_code = uid.slice(2..-1) + country_code = uid.slice(0..1) + first_name = omniauth_hash.dig('info', 'first_name') + last_name = omniauth_hash.dig('info', 'last_name') - 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_data = { first_name: first_name, last_name: last_name, + ident: identity_code, country_code: country_code } - user = find_or_initialize_by(registrant_ident: registrant_ident) - user.username = username - user.save! - user + find_or_create_by_user_data(user_data) end private diff --git a/app/views/registrant/sessions/new.html.erb b/app/views/registrant/sessions/new.html.erb index a3203e83a6..9f7af3254b 100644 --- a/app/views/registrant/sessions/new.html.erb +++ b/app/views/registrant/sessions/new.html.erb @@ -8,11 +8,6 @@ <%= t '.hint' %>
- <%= link_to '/registrant/login/mid' do %> - <%= image_tag 'mid.gif' %> - <% end %> - <%= link_to registrant_id_card_sign_in_path, method: :post do %> - <%= image_tag 'id_card.gif' %> - <% end %> + <%= link_to t(:sign_in), "/auth/rant_tara", method: :post, class: 'btn btn-lg btn-primary btn-block' %> - \ No newline at end of file + diff --git a/config/application.yml.sample b/config/application.yml.sample index cbe32e5db3..44803120b0 100644 --- a/config/application.yml.sample +++ b/config/application.yml.sample @@ -163,6 +163,10 @@ tara_secret: 'secret' tara_redirect_uri: 'redirect_url' tara_keys: "{\"kty\":\"RSA\",\"kid\":\"de6cc4\",\"n\":\"jWwAjT_03ypme9ZWeSe7c-jY26NO50Wo5I1LBnPW2JLc0dPMj8v7y4ehiRpClYNTaSWcLd4DJmlKXDXXudEUWwXa7TtjBFJfzlZ-1u0tDvJ-H9zv9MzO7UhUFytztUEMTrtStdhGbzkzdEZZCgFYeo2i33eXxzIR1nGvI05d9Y-e_LHnNE2ZKTa89BC7ZiCXq5nfAaCgQna_knh4kFAX-KgiPRAtsiDHcAWKcBY3qUVcb-5XAX8p668MlGLukzsh5tFkQCbJVyNtmlbIHdbGvVHPb8C0H3oLYciv1Fjy_tS1lO7OT_cb3GVp6Ql-CG0uED_8pkpVtfsGRviub4_ElQ\",\"e\":\"AQAB\"}" +tara_rant_identifier: 'identifier' +tara_rant_secret: 'secret' +tara_rant_redirect_uri: 'redirect_uri' + # Since the keys for staging are absent from the repo, we need to supply them separate for testing. test: payments_seb_bank_certificate: 'test/fixtures/files/seb_bank_cert.pem' diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index ef53503840..ec030399e4 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -16,6 +16,10 @@ secret = ENV['tara_secret'] redirect_uri = ENV['tara_redirect_uri'] +registrant_identifier = ENV['tara_rant_identifier'] +registrant_secret = ENV['tara_rant_secret'] +registrant_redirect_uri = ENV['tara_rant_redirect_uri'] + Rails.application.config.middleware.use OmniAuth::Builder do provider "tara", { callback_path: '/registrar/open_id/callback', @@ -43,4 +47,31 @@ redirect_uri: redirect_uri, }, } + + provider "tara", { + callback_path: '/registrant/open_id/callback', + name: 'rant_tara', + scope: ['openid'], + state: Proc.new{ SecureRandom.hex(10) }, + client_signing_alg: :RS256, + client_jwk_signing_key: signing_keys, + send_scope_to_token_endpoint: false, + send_nonce: true, + issuer: issuer, + + client_options: { + scheme: 'https', + host: host, + + authorization_endpoint: '/oidc/authorize', + token_endpoint: '/oidc/token', + userinfo_endpoint: nil, # Not implemented + jwks_uri: '/oidc/jwks', + + # Registry + identifier: registrant_identifier, + secret: registrant_secret, + redirect_uri: registrant_redirect_uri, + }, + } end diff --git a/config/locales/registrant/sessions.en.yml b/config/locales/registrant/sessions.en.yml index 3032382c18..7d4c16da94 100644 --- a/config/locales/registrant/sessions.en.yml +++ b/config/locales/registrant/sessions.en.yml @@ -2,11 +2,7 @@ en: registrant: sessions: new: - header: Log in + header: Sign in with identity document hint: >- - Access currently available only to Estonian citizens and e-residents with Estonian ID-card - or Mobile-ID. - - login_mid: - header: Log in with mobile-id - submit_btn: Login \ No newline at end of file + Sign in using Estonian (incl. e-residents) ID card, mobile ID, + Bank link or other EU citizen's electronic ID supported by EIDAS. diff --git a/config/routes.rb b/config/routes.rb index 41f857bc8e..b8de8557e3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -176,6 +176,9 @@ # 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 + match '/open_id/callback', via: %i[get post], to: 'tara#callback', as: :tara_registrant_callback + match '/open_id/cancel', via: %i[get post delete], to: 'tara#cancel', + as: :tara_registrant_cancel end resources :registrars, only: :show From 11ee1f9f1ed871aae52a09274b53eaeb451cf041 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Thu, 1 Oct 2020 17:07:33 +0300 Subject: [PATCH 02/10] Remove ID card logic from codebase --- app/models/api_user.rb | 6 --- app/models/id_card.rb | 6 --- app/models/registrant_user.rb | 4 +- config/initializers/devise.rb | 4 -- config/routes.rb | 4 -- lib/devise/models/id_card_authenticatable.rb | 7 --- .../strategies/id_card_authenticatable.rb | 49 ------------------- .../registrant_area/sign_in/id_card_test.rb | 31 ------------ .../id_card_authenticatable_test.rb | 13 ----- test/models/api_user_test.rb | 11 ----- test/models/registrant_user_test.rb | 30 +----------- 11 files changed, 3 insertions(+), 162 deletions(-) delete mode 100644 app/models/id_card.rb delete mode 100644 lib/devise/models/id_card_authenticatable.rb delete mode 100644 lib/devise/strategies/id_card_authenticatable.rb delete mode 100644 test/integration/registrant_area/sign_in/id_card_test.rb delete mode 100644 test/lib/devise/strategies/id_card_authenticatable_test.rb diff --git a/app/models/api_user.rb b/app/models/api_user.rb index 8159137a3c..61dd123877 100644 --- a/app/models/api_user.rb +++ b/app/models/api_user.rb @@ -47,12 +47,6 @@ def set_defaults self.active = true unless saved_change_to_active? end - class << self - def find_by_id_card(id_card) - find_by(identity_code: id_card.personal_code) - end - end - def to_s username end diff --git a/app/models/id_card.rb b/app/models/id_card.rb deleted file mode 100644 index 0e3c11bb3b..0000000000 --- a/app/models/id_card.rb +++ /dev/null @@ -1,6 +0,0 @@ -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 06a2b668ad..c0addb5cde 100644 --- a/app/models/registrant_user.rb +++ b/app/models/registrant_user.rb @@ -1,7 +1,7 @@ class RegistrantUser < User attr_accessor :idc_data - devise :trackable, :timeoutable, :id_card_authenticatable + devise :trackable, :timeoutable def ability @ability ||= Ability.new(self) @@ -74,7 +74,7 @@ def find_or_create_by_omniauth_data(omniauth_hash) last_name = omniauth_hash.dig('info', 'last_name') user_data = { first_name: first_name, last_name: last_name, - ident: identity_code, country_code: country_code } + ident: identity_code, country_code: country_code } find_or_create_by_user_data(user_data) end diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index dee2824a41..6631a02391 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -281,9 +281,5 @@ # 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/routes.rb b/config/routes.rb index b8de8557e3..5d6b3d907c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -172,10 +172,6 @@ post 'login/mid_status' => 'sessions#mid_status' post 'mid' => 'sessions#mid' - # /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 match '/open_id/callback', via: %i[get post], to: 'tara#callback', as: :tara_registrant_callback match '/open_id/cancel', via: %i[get post delete], to: 'tara#cancel', as: :tara_registrant_cancel diff --git a/lib/devise/models/id_card_authenticatable.rb b/lib/devise/models/id_card_authenticatable.rb deleted file mode 100644 index 53bad663fd..0000000000 --- a/lib/devise/models/id_card_authenticatable.rb +++ /dev/null @@ -1,7 +0,0 @@ -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 deleted file mode 100644 index 6c3a7ac92c..0000000000 --- a/lib/devise/strategies/id_card_authenticatable.rb +++ /dev/null @@ -1,49 +0,0 @@ -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) diff --git a/test/integration/registrant_area/sign_in/id_card_test.rb b/test/integration/registrant_area/sign_in/id_card_test.rb deleted file mode 100644 index fe6c8a7efd..0000000000 --- a/test/integration/registrant_area/sign_in/id_card_test.rb +++ /dev/null @@ -1,31 +0,0 @@ -require 'test_helper' - -class RegistrantAreaIdCardSignInTest < ApplicationIntegrationTest - setup do - allow_business_registry_component_reach_server - end - - def test_succeeds - post registrant_id_card_sign_in_path, headers: { 'SSL_CLIENT_S_DN_CN' => 'DOE,JOHN,1234', - 'SSL_CLIENT_I_DN_C' => 'US' } - follow_redirect! - - 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 registrant_id_card_sign_in_path, headers: { '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/lib/devise/strategies/id_card_authenticatable_test.rb b/test/lib/devise/strategies/id_card_authenticatable_test.rb deleted file mode 100644 index e194ccaac0..0000000000 --- a/test/lib/devise/strategies/id_card_authenticatable_test.rb +++ /dev/null @@ -1,13 +0,0 @@ -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 index ecbff5cbbe..525e6c2646 100644 --- a/test/models/api_user_test.rb +++ b/test/models/api_user_test.rb @@ -52,17 +52,6 @@ def test_active_by_default assert ApiUser.new.active? 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 - def test_verifies_pki_status certificate = certificates(:api) diff --git a/test/models/registrant_user_test.rb b/test/models/registrant_user_test.rb index 78b9ef9014..c61f095f29 100644 --- a/test/models/registrant_user_test.rb +++ b/test/models/registrant_user_test.rb @@ -30,34 +30,6 @@ def test_returns_country 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 @@ -92,4 +64,4 @@ def test_returns_administered_domains assert_equal %w(shop airport), @user.administered_domains end end -end \ No newline at end of file +end From f83e532fb1e5888aef0ef4d621af03551afa556d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Thu, 1 Oct 2020 18:37:04 +0300 Subject: [PATCH 03/10] Make Registrant/Registrar use same TARA controller --- app/controllers/registrant/tara_controller.rb | 33 --------------- app/controllers/registrar/tara_controller.rb | 33 --------------- app/controllers/sso/tara_controller.rb | 40 +++++++++++++++++++ config/initializers/devise.rb | 2 - config/routes.rb | 32 +++++++-------- 5 files changed, 56 insertions(+), 84 deletions(-) delete mode 100644 app/controllers/registrant/tara_controller.rb delete mode 100644 app/controllers/registrar/tara_controller.rb create mode 100644 app/controllers/sso/tara_controller.rb diff --git a/app/controllers/registrant/tara_controller.rb b/app/controllers/registrant/tara_controller.rb deleted file mode 100644 index a09e4665b9..0000000000 --- a/app/controllers/registrant/tara_controller.rb +++ /dev/null @@ -1,33 +0,0 @@ -class Registrant - class TaraController < ApplicationController - skip_authorization_check - - # rubocop:disable Style/AndOr - def callback - session[:omniauth_hash] = user_hash - @registrant_user = RegistrantUser.find_or_create_by_omniauth_data(user_hash) - - if @registrant_user - flash[:notice] = t(:signed_in_successfully) - sign_in_and_redirect(:registrant_user, @registrant_user) - else - show_error and return - end - end - # rubocop:enable Style/AndOr - - def cancel - redirect_to root_path, notice: t(:sign_in_cancelled) - end - - def show_error - redirect_to new_registrant_user_session_url, alert: t(:no_such_user) - end - - private - - def user_hash - request.env['omniauth.auth'] - end - end -end diff --git a/app/controllers/registrar/tara_controller.rb b/app/controllers/registrar/tara_controller.rb deleted file mode 100644 index e02aa52a5f..0000000000 --- a/app/controllers/registrar/tara_controller.rb +++ /dev/null @@ -1,33 +0,0 @@ -class Registrar - class TaraController < ApplicationController - skip_authorization_check - - # rubocop:disable Style/AndOr - def callback - session[:omniauth_hash] = user_hash - @api_user = ApiUser.from_omniauth(user_hash) - - if @api_user - flash[:notice] = t(:signed_in_successfully) - sign_in_and_redirect(:registrar_user, @api_user) - else - show_error and return - end - end - # rubocop:enable Style/AndOr - - def cancel - redirect_to root_path, notice: t(:sign_in_cancelled) - end - - def show_error - redirect_to new_registrar_user_session_url, alert: t(:no_such_user) - end - - private - - def user_hash - request.env['omniauth.auth'] - end - end -end diff --git a/app/controllers/sso/tara_controller.rb b/app/controllers/sso/tara_controller.rb new file mode 100644 index 0000000000..1571f45b3f --- /dev/null +++ b/app/controllers/sso/tara_controller.rb @@ -0,0 +1,40 @@ +module Sso + class TaraController < ApplicationController + skip_authorization_check + + def registrant_callback + user = RegistrantUser.find_or_create_by_omniauth_data(user_hash) + callback(user, registrar: false) + end + + def registrar_callback + user = ApiUser.from_omniauth(user_hash) + callback(user, registrar: true) + end + + # rubocop:disable Style/AndOr + def callback(user, registrar: true) + session[:omniauth_hash] = user_hash + (show error and return) unless user + + flash[:notice] = t(:signed_in_successfully) + sign_in_and_redirect(registrar ? :registrar_user : :registrant_user, user) + end + # rubocop:enable Style/AndOr + + def cancel + redirect_to root_path, notice: t(:sign_in_cancelled) + end + + def show_error(registrar: true) + path = registrar ? new_registrar_user_session_url : new_registrant_user_session_url + redirect_to path, alert: t(:no_such_user) + end + + private + + def user_hash + request.env['omniauth.auth'] + end + end +end diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 6631a02391..eb04657963 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -280,6 +280,4 @@ # 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' - - routes = [nil, :new, :destroy] end diff --git a/config/routes.rb b/config/routes.rb index 5d6b3d907c..7df315f882 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -76,12 +76,6 @@ devise_for :users, path: '', class_name: 'ApiUser', skip: %i[sessions] - devise_scope :registrar_user do - match '/open_id/callback', via: %i[get post], to: 'tara#callback', as: :tara_callback - match '/open_id/cancel', via: %i[get post delete], to: 'tara#cancel', - as: :tara_cancel - end - resources :invoices, except: %i[new create edit update destroy] do resource :delivery, controller: 'invoices/delivery', only: %i[new create] @@ -158,6 +152,22 @@ post 'sessions', to: 'registrar/sessions#create', as: :registrar_user_session delete 'sign_out', to: 'registrar/sessions#destroy', as: :destroy_registrar_user_session + + # TARA + match '/open_id/callback', via: %i[get post], to: 'sso/tara#registrar_callback' + match '/open_id/cancel', via: %i[get post delete], to: 'sso/tara#cancel' + end + end + + scope :registrant do + devise_scope :registrant_user do + get 'sign_in', to: 'registrant/sessions#new', as: :new_registrant_user_session + post 'sessions', to: 'registrant/sessions#create', as: :registrant_user_session + delete 'sign_out', to: 'registrant/sessions#destroy', as: :destroy_registrant_user_session + + # TARA + match '/open_id/callback', via: %i[get post], to: 'sso/tara#registrant_callback' + match '/open_id/cancel', via: %i[get post delete], to: 'sso/tara#cancel' end end @@ -166,16 +176,6 @@ # POST /registrant/sign_in is not used devise_for :users, path: '', class_name: 'RegistrantUser' - devise_scope :registrant_user do - get 'login/mid' => 'sessions#login_mid' - post 'login/mid' => 'sessions#mid' - post 'login/mid_status' => 'sessions#mid_status' - post 'mid' => 'sessions#mid' - - match '/open_id/callback', via: %i[get post], to: 'tara#callback', as: :tara_registrant_callback - match '/open_id/cancel', via: %i[get post delete], to: 'tara#cancel', - as: :tara_registrant_cancel - end resources :registrars, only: :show resources :domains, only: %i[index show] do From 3162d8cc57beb78cf890495756e27ae807ab86b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Fri, 2 Oct 2020 09:54:14 +0300 Subject: [PATCH 04/10] Remove traces of Mobile-ID direct auth --- .../registrant/sessions_controller.rb | 75 ------------------- app/views/registrant/sessions/login_mid.haml | 40 ---------- app/views/registrar/sessions/login_mid.haml | 40 ---------- .../registrant_user_creation_test.rb | 9 --- 4 files changed, 164 deletions(-) delete mode 100644 app/views/registrant/sessions/login_mid.haml delete mode 100644 app/views/registrar/sessions/login_mid.haml diff --git a/app/controllers/registrant/sessions_controller.rb b/app/controllers/registrant/sessions_controller.rb index b18a802e60..73b6d52da8 100644 --- a/app/controllers/registrant/sessions_controller.rb +++ b/app/controllers/registrant/sessions_controller.rb @@ -1,81 +1,6 @@ class Registrant::SessionsController < Devise::SessionsController layout 'registrant/application' - def login_mid - @user = User.new - end - - def mid - phone = params[:user][:phone] - endpoint = "#{ENV['sk_digi_doc_service_endpoint']}" - client = Digidoc::Client.new(endpoint) - client.logger = Rails.application.config.logger unless Rails.env.test? - - # country_codes = {'+372' => 'EST'} - response = client.authenticate( - phone: "+372#{phone}", - message_to_display: 'Authenticating', - service_name: ENV['sk_digi_doc_service_name'] || 'Testing' - ) - - if response.faultcode - render json: { message: response.detail.message }, status: :unauthorized - return - end - - @user = RegistrantUser.find_or_create_by_mid_data(response) - - if @user.persisted? - session[:user_country] = response.user_country - session[:user_id_code] = response.user_id_code - session[:mid_session_code] = client.session_code - - render json: { - message: t(:confirmation_sms_was_sent_to_your_phone_verification_code_is, { code: response.challenge_id }) - }, status: :ok - else - render json: { message: t(:no_such_user) }, status: :unauthorized - end - end - - def mid_status - endpoint = "#{ENV['sk_digi_doc_service_endpoint']}" - client = Digidoc::Client.new(endpoint) - client.logger = Rails.application.config.logger unless Rails.env.test? - client.session_code = session[:mid_session_code] - auth_status = client.authentication_status - - case auth_status.status - when 'OUTSTANDING_TRANSACTION' - render json: { message: t(:check_your_phone_for_confirmation_code) }, status: :ok - when 'USER_AUTHENTICATED' - @user = RegistrantUser.find_by(registrant_ident: "#{session[:user_country]}-#{session[:user_id_code]}") - - sign_in(:registrant_user, @user) - flash[:notice] = t(:welcome) - flash.keep(:notice) - render js: "window.location = '#{registrant_root_path}'" - when 'NOT_VALID' - render json: { message: t(:user_signature_is_invalid) }, status: :bad_request - when 'EXPIRED_TRANSACTION' - render json: { message: t(:session_timeout) }, status: :bad_request - when 'USER_CANCEL' - render json: { message: t(:user_cancelled) }, status: :bad_request - when 'MID_NOT_READY' - render json: { message: t(:mid_not_ready) }, status: :bad_request - when 'PHONE_ABSENT' - render json: { message: t(:phone_absent) }, status: :bad_request - when 'SENDING_ERROR' - render json: { message: t(:sending_error) }, status: :bad_request - when 'SIM_ERROR' - render json: { message: t(:sim_error) }, status: :bad_request - when 'INTERNAL_ERROR' - render json: { message: t(:internal_error) }, status: :bad_request - else - render json: { message: t(:internal_error) }, status: :bad_request - end - end - private def after_sign_in_path_for(_resource_or_scope) diff --git a/app/views/registrant/sessions/login_mid.haml b/app/views/registrant/sessions/login_mid.haml deleted file mode 100644 index 318e190331..0000000000 --- a/app/views/registrant/sessions/login_mid.haml +++ /dev/null @@ -1,40 +0,0 @@ -.row - .form-signin.col-md-4.center-block.text-center - %h2.form-signin-heading.text-center= t '.header' - %hr - = form_for @user, url: registrant_mid_path, html: {class: 'form-signin'} do |f| - = f.text_field :phone, class: 'form-control', - placeholder: t(:phone_no), autocomplete: 'off', required: true - %button.btn.btn-lg.btn-primary.btn-block.js-login{:type => 'submit'}= t '.submit_btn' - - - if ['development', 'alpha'].include?(Rails.env) - %div.text-center - 00007, 60000007, 00000766 - -:coffee - load_listener = -> - $('.js-login').attr('disabled', false) - - status_interval = null - mid_status = () -> - status_interval = setInterval((-> - $.post('/registrant/login/mid_status').fail((data) -> - clearInterval(status_interval) - flash_alert(data.responseJSON.message) - $('.js-login').attr('disabled', false) - ) - ), 1000) - - $('.js-login').on 'click', (e) -> - e.preventDefault(); - $(this).attr('disabled', true) - - $.post($('form').attr('action'), $('form').serialize()).done((data) -> - if data.message - flash_notice(data.message) - mid_status() - ).fail((data) -> - flash_alert(data.responseJSON.message) - $('.js-login').attr('disabled', false) - ) - window.addEventListener 'load', load_listener diff --git a/app/views/registrar/sessions/login_mid.haml b/app/views/registrar/sessions/login_mid.haml deleted file mode 100644 index 7ee6042098..0000000000 --- a/app/views/registrar/sessions/login_mid.haml +++ /dev/null @@ -1,40 +0,0 @@ -.row - .form-signin.col-md-4.center-block.text-center - %h2.form-signin-heading.text-center= t '.header' - %hr - = form_for @user, url: registrar_mid_path, html: {class: 'form-signin'} do |f| - = f.text_field :phone, class: 'form-control', - placeholder: t(:phone_no), autocomplete: 'off', required: true - %button.btn.btn-lg.btn-primary.btn-block.js-login{:type => 'submit'}= t '.submit_btn' - - - if ['development', 'alpha'].include?(Rails.env) - %div.text-center - 00007, 60000007, 00000766 - -:coffee - load_listener = -> - $('.js-login').attr('disabled', false) - - status_interval = null - mid_status = () -> - status_interval = setInterval((-> - $.post('/registrar/login/mid_status').fail((data) -> - clearInterval(status_interval) - flash_alert(data.responseJSON.message) - $('.js-login').attr('disabled', false) - ) - ), 1000) - - $('.js-login').on 'click', (e) -> - e.preventDefault(); - $(this).attr('disabled', true) - - $.post($('form').attr('action'), $('form').serialize()).done((data) -> - if data.message - flash_notice(data.message) - mid_status() - ).fail((data) -> - flash_alert(data.responseJSON.message) - $('.js-login').attr('disabled', false) - ) - window.addEventListener 'load', load_listener diff --git a/test/models/registrant_user/registrant_user_creation_test.rb b/test/models/registrant_user/registrant_user_creation_test.rb index 42fb0e0f60..5ed680795e 100644 --- a/test/models/registrant_user/registrant_user_creation_test.rb +++ b/test/models/registrant_user/registrant_user_creation_test.rb @@ -26,13 +26,4 @@ def test_find_or_create_by_api_data_creates_a_user_after_upcasing_input user = User.find_by(registrant_ident: 'EE-37710100070') assert_equal('JOHN SMITH', user.username) end - - def test_find_or_create_by_mid_data_creates_a_user - user_data = OpenStruct.new(user_country: 'EE', user_id_code: '37710100070', - user_givenname: 'JOHN', user_surname: 'SMITH') - - RegistrantUser.find_or_create_by_mid_data(user_data) - user = User.find_by(registrant_ident: 'EE-37710100070') - assert_equal('JOHN SMITH', user.username) - end end From f892302e935323ac45c409995e74e7b9b236dce9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Fri, 2 Oct 2020 09:57:49 +0300 Subject: [PATCH 05/10] Fix error rendering on TARA controller --- app/controllers/sso/tara_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/sso/tara_controller.rb b/app/controllers/sso/tara_controller.rb index 1571f45b3f..37c8ab608c 100644 --- a/app/controllers/sso/tara_controller.rb +++ b/app/controllers/sso/tara_controller.rb @@ -15,7 +15,7 @@ def registrar_callback # rubocop:disable Style/AndOr def callback(user, registrar: true) session[:omniauth_hash] = user_hash - (show error and return) unless user + (show_error(registrar: registrar) and return) unless user flash[:notice] = t(:signed_in_successfully) sign_in_and_redirect(registrar ? :registrar_user : :registrant_user, user) From ed29bef7f1863b84b7a681363e678f7b19a99a0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Fri, 2 Oct 2020 10:21:38 +0300 Subject: [PATCH 06/10] Add test for registrant TARA integration --- .../registrant_user/tara/tara_users_test.rb | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 test/models/registrant_user/tara/tara_users_test.rb diff --git a/test/models/registrant_user/tara/tara_users_test.rb b/test/models/registrant_user/tara/tara_users_test.rb new file mode 100644 index 0000000000..105baac5b3 --- /dev/null +++ b/test/models/registrant_user/tara/tara_users_test.rb @@ -0,0 +1,49 @@ +require 'application_system_test_case' + +class RegistrantAreaTaraUsersTest < ApplicationSystemTestCase + def setup + super + + OmniAuth.config.test_mode = true + @registrant = users(:registrant) + + @existing_user_hash = { + 'provider' => 'rant_tara', + 'uid' => "US1234", + 'info': { 'first_name': 'Registrant', 'last_name': 'User' } + } + + @new_user_hash = { + 'provider' => 'rant_tara', + 'uid' => 'EE51007050604', + 'info': { 'first_name': 'New Registrant', 'last_name': 'User'} + } + end + + def teardown + super + + OmniAuth.config.test_mode = false + OmniAuth.config.mock_auth['rant_tara'] = nil + end + + def test_existing_user_gets_signed_in + OmniAuth.config.mock_auth[:rant_tara] = OmniAuth::AuthHash.new(@existing_user_hash) + + visit new_registrant_user_session_path + click_link('Sign in') + + assert_text('Signed in successfully') + end + + def test_new_user_is_created_and_signed_in + OmniAuth.config.mock_auth[:tara] = OmniAuth::AuthHash.new(@new_user_hash) + + visit new_registrant_user_session_path + click_link('Sign in') + + assert_text('Signed in successfully') + assert 'New Registrant User', RegistrantUser.last.username + assert 'EE-51007050604', RegistrantUser.last.registrant_ident + end +end From b6b036f37f108537cb60c5f29fb8fc6423c0e218 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Fri, 2 Oct 2020 10:52:45 +0300 Subject: [PATCH 07/10] Move tara test to system cases --- .../registrant_area}/tara/tara_users_test.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) rename test/{models/registrant_user => system/registrant_area}/tara/tara_users_test.rb (70%) diff --git a/test/models/registrant_user/tara/tara_users_test.rb b/test/system/registrant_area/tara/tara_users_test.rb similarity index 70% rename from test/models/registrant_user/tara/tara_users_test.rb rename to test/system/registrant_area/tara/tara_users_test.rb index 105baac5b3..5020616d41 100644 --- a/test/models/registrant_user/tara/tara_users_test.rb +++ b/test/system/registrant_area/tara/tara_users_test.rb @@ -37,13 +37,15 @@ def test_existing_user_gets_signed_in end def test_new_user_is_created_and_signed_in - OmniAuth.config.mock_auth[:tara] = OmniAuth::AuthHash.new(@new_user_hash) + OmniAuth.config.mock_auth[:rant_tara] = OmniAuth::AuthHash.new(@new_user_hash) - visit new_registrant_user_session_path - click_link('Sign in') + assert_difference 'RegistrantUser.count' do + visit new_registrant_user_session_path + click_link('Sign in') - assert_text('Signed in successfully') - assert 'New Registrant User', RegistrantUser.last.username - assert 'EE-51007050604', RegistrantUser.last.registrant_ident + assert_equal 'New Registrant User', RegistrantUser.last.username + assert_equal 'EE-51007050604', RegistrantUser.last.registrant_ident + assert_text('Signed in successfully') + end end end From a154b0599ff2c7b9228321c0f6d58a9afd7c0c41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Thu, 15 Oct 2020 11:32:52 +0300 Subject: [PATCH 08/10] Registrant: Generate TARA state via gem --- config/initializers/omniauth.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index ec030399e4..e3e0d644bc 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -52,7 +52,6 @@ callback_path: '/registrant/open_id/callback', name: 'rant_tara', scope: ['openid'], - state: Proc.new{ SecureRandom.hex(10) }, client_signing_alg: :RS256, client_jwk_signing_key: signing_keys, send_scope_to_token_endpoint: false, From 0357cfc29ece156c17a0a2ea176e1e0d53c0a4f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Thu, 15 Oct 2020 12:54:38 +0300 Subject: [PATCH 09/10] Provide logging for tara state handler --- Gemfile | 2 +- Gemfile.lock | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index 6ba54e8716..e346eb3ea7 100644 --- a/Gemfile +++ b/Gemfile @@ -56,7 +56,7 @@ gem 'digidoc_client', # TARA gem 'omniauth' gem 'omniauth-rails_csrf_protection' -gem 'omniauth-tara', github: 'internetee/omniauth-tara' +gem 'omniauth-tara', github: 'internetee/omniauth-tara', branch: 'extended-logging' gem 'epp', github: 'internetee/epp', branch: :master diff --git a/Gemfile.lock b/Gemfile.lock index 1a45ed8267..59922042bf 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -54,7 +54,8 @@ GIT GIT remote: https://github.com/internetee/omniauth-tara.git - revision: cec845ec3794532144c4976104a07e206d759aa6 + revision: 75c369dd86a5fe1a2ee4c2a6246252d79431071c + branch: extended-logging specs: omniauth-tara (0.3.0) addressable (~> 2.5) From a8a985b9d3a9099bb527d42eeff67067e19017b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Thu, 15 Oct 2020 16:05:36 +0300 Subject: [PATCH 10/10] Go back to master branch of omniauth-tara --- Gemfile | 2 +- Gemfile.lock | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index e346eb3ea7..6ba54e8716 100644 --- a/Gemfile +++ b/Gemfile @@ -56,7 +56,7 @@ gem 'digidoc_client', # TARA gem 'omniauth' gem 'omniauth-rails_csrf_protection' -gem 'omniauth-tara', github: 'internetee/omniauth-tara', branch: 'extended-logging' +gem 'omniauth-tara', github: 'internetee/omniauth-tara' gem 'epp', github: 'internetee/epp', branch: :master diff --git a/Gemfile.lock b/Gemfile.lock index 3896823737..4ebedc95cd 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -54,8 +54,7 @@ GIT GIT remote: https://github.com/internetee/omniauth-tara.git - revision: 35c93a072e844b7a79a7d4a81b1de1ed9426dcd8 - branch: extended-logging + revision: cec845ec3794532144c4976104a07e206d759aa6 specs: omniauth-tara (0.3.0) addressable (~> 2.5)