From 1103c7757bc98605283bdb39ec4f503d980d9188 Mon Sep 17 00:00:00 2001 From: Sylvain Date: Wed, 10 Nov 2021 13:03:12 +0100 Subject: [PATCH 1/2] added debug logs --- .../users/omniauth_callbacks_controller.rb | 22 ++++++++++++++++++- app/models/o_auth2_provider.rb | 4 ++-- app/models/user.rb | 12 +++++++++- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index c7d86a5f3..e56b2c3a1 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -2,15 +2,19 @@ class Users::OmniauthCallbacksController < Devise::OmniauthCallbacksController active_provider = AuthProvider.active define_method active_provider.strategy_name do + logger.debug "[Users::OmniauthCallbacksController##{active_provider.strategy_name}] initiated" if request.env['omniauth.params'].blank? + logger.debug 'the user has not provided any authentication token' @user = User.from_omniauth(request.env['omniauth.auth']) # Here we create the new user or update the existing one with values retrieved from the SSO. if @user.id.nil? # => new user (ie. not updating existing) + logger.debug 'trying to create a new user' # If the username is mapped, we just check its uniqueness as it would break the postgresql # unique constraint otherwise. If the name is not unique, another unique is generated if active_provider.sso_fields.include?('user.username') + logger.debug 'the username was already in use, generating a new one' @user.username = generate_unique_username(@user.username) end # If the email is mapped, we check its uniqueness. If the email is already in use, we mark it as duplicate with an @@ -18,17 +22,21 @@ class Users::OmniauthCallbacksController < Devise::OmniauthCallbacksController # - if it is the same user, his email will be filled from the SSO when he merge his accounts # - if it is not the same user, this will prevent the raise of PG::UniqueViolation if active_provider.sso_fields.include?('user.email') && email_exists?(@user.email) + logger.debug 'the email was already in use, marking it as duplicate' old_mail = @user.email @user.email = "<#{old_mail}>#{Devise.friendly_token}-duplicate" flash[:alert] = t('omniauth.email_already_linked_to_another_account_please_input_your_authentication_code', OLD_MAIL: old_mail) end else # => update of an existing user + logger.debug "an existing user was found (id=#{@user.id})" if username_exists?(@user.username, @user.id) + logger.debug 'the username was already in use, alerting user' flash[:alert] = t('omniauth.your_username_is_already_linked_to_another_account_unable_to_update_it', USERNAME: @user.username) @user.username = User.find(@user.id).username end if email_exists?(@user.email, @user.id) + logger.debug 'the email was already in use, alerting user' flash[:alert] = t('omniauth.your_email_address_is_already_linked_to_another_account_unable_to_update_it', EMAIL: @user.email) @user.email = User.find(@user.id).email end @@ -36,20 +44,32 @@ class Users::OmniauthCallbacksController < Devise::OmniauthCallbacksController # We BYPASS THE VALIDATION because, in case of a new user, we want to save him anyway, we'll ask him later to complete his profile (on first login). # In case of an existing user, we trust the SSO validation as we want the SSO to have authority on users management and policy. - raise @user.errors.full_messages.join(', ') unless @user.save(validate: false) + logger.debug 'saving the user' + unless @user.save(validate: false) + logger.error "unable to save the user, an error occurred : #{@user.errors.full_messages.join(', ')}" + end + logger.debug 'signing-in the user and redirecting' sign_in_and_redirect @user, event: :authentication # this will throw if @user is not activated else + logger.debug 'the user has provided an authentication token' @user = User.find_by(auth_token: request.env['omniauth.params']['auth_token']) # Here the user already exists in the database and request to be linked with the SSO # so let's update its sso attributes and log him on + logger.debug "found user id=#{@user.id}" begin + logger.debug 'linking with the omniauth provider' @user.link_with_omniauth_provider(request.env['omniauth.auth']) + logger.debug 'signing-in the user and redirecting' sign_in_and_redirect @user, event: :authentication rescue DuplicateIndexError + logger.error 'user already linked' redirect_to root_url, alert: t('omniauth.this_account_is_already_linked_to_an_user_of_the_platform', NAME: active_provider.name) + rescue StandardError => e + logger.unknown "an expected error occurred: #{e}" + raise e end end diff --git a/app/models/o_auth2_provider.rb b/app/models/o_auth2_provider.rb index 99b6edd86..75f847d7d 100644 --- a/app/models/o_auth2_provider.rb +++ b/app/models/o_auth2_provider.rb @@ -8,13 +8,13 @@ class OAuth2Provider < ApplicationRecord accepts_nested_attributes_for :o_auth2_mappings, allow_destroy: true def domain - URI(base_url).scheme+'://'+URI(base_url).host + URI(base_url).scheme + '://' + URI(base_url).host end def protected_fields fields = [] o_auth2_mappings.each do |mapping| - fields.push(mapping.local_model+'.'+mapping.local_field) + fields.push(mapping.local_model + '.' + mapping.local_field) end fields end diff --git a/app/models/user.rb b/app/models/user.rb index f4ad820ec..5db0dc4d5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -201,16 +201,20 @@ class User < ApplicationRecord end def self.from_omniauth(auth) + logger.debug "[User::from_omniauth] initiated with parameter #{auth}" active_provider = AuthProvider.active raise SecurityError, 'The identity provider does not match the activated one' if active_provider.strategy_name != auth.provider where(provider: auth.provider, uid: auth.uid).first_or_create.tap do |user| # execute this regardless of whether record exists or not (-> User#tap) # this will init or update the user thanks to the information retrieved from the SSO + logger.debug user.id.nil? ? 'no user found, creating a new one' : "found user id=#{user.id}" user.profile ||= Profile.new auth.info.mapping.each do |key, value| + logger.debug "mapping info #{key} with value=#{value}" user.set_data_from_sso_mapping(key, value) end + logger.debug "generating a new password" user.password = Devise.friendly_token[0, 20] end end @@ -292,6 +296,7 @@ class User < ApplicationRecord ## Merge the provided User's SSO details into the current user and drop the provided user to ensure the unity ## @param sso_user {User} the provided user will be DELETED after the merge was successful def merge_from_sso(sso_user) + logger.debug "[User::merge_from_sso] initiated with parameter #{sso_user}" # update the attributes to link the account to the sso account self.provider = sso_user.provider self.uid = sso_user.uid @@ -303,13 +308,16 @@ class User < ApplicationRecord # check that the email duplication was resolved if sso_user.email.end_with? '-duplicate' email_addr = sso_user.email.match(/^<([^>]+)>.{20}-duplicate$/)[1] + logger.error 'duplicate email was not resolved' raise(DuplicateIndexError, email_addr) unless email_addr == email end # update the user's profile to set the data managed by the SSO auth_provider = AuthProvider.from_strategy_name(sso_user.provider) + logger.debug "found auth_provider=#{auth_provider.name}" auth_provider.sso_fields.each do |field| value = sso_user.get_data_from_sso_mapping(field) + logger.debug "mapping sso field #{field} with value=#{value}" # we do not merge the email field if its end with the special value '-duplicate' as this means # that the user is currently merging with the account that have the same email than the sso set_data_from_sso_mapping(field, value) unless field == 'user.email' && value.end_with?('-duplicate') @@ -318,9 +326,11 @@ class User < ApplicationRecord # run the account transfer in an SQL transaction to ensure data integrity User.transaction do # remove the temporary account + logger.debug 'removing the temporary user' sso_user.destroy # finally, save the new details - save! + logger.debug 'saving the updated user' + logger.error "unable to save the user, an error occurred : #{errors.full_messages.join(', ')}" unless save! end end From e3fa75dd6e8d213d6424e0bdca1ace8e4ca5bdf3 Mon Sep 17 00:00:00 2001 From: Sylvain Date: Wed, 10 Nov 2021 17:02:37 +0100 Subject: [PATCH 2/2] [bug] unable to create a plan --- test/integration/plans/create_plan_test.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 test/integration/plans/create_plan_test.rb diff --git a/test/integration/plans/create_plan_test.rb b/test/integration/plans/create_plan_test.rb new file mode 100644 index 000000000..147b16114 --- /dev/null +++ b/test/integration/plans/create_plan_test.rb @@ -0,0 +1,15 @@ +require 'minitest/autorun' + +class CreatePlanTest < Minitest::Test + def setup + # Do nothing + end + + def teardown + # Do nothing + end + + def test + skip 'Not implemented' + end +end