From 1ca3c875a6fa81ce6042abd3c25fec0e1496048b Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Tue, 14 Apr 2020 12:39:47 +0200 Subject: [PATCH 1/6] Add option for storing Azure app configs in settings using JSON format [SCI-4544] --- app/models/application_settings.rb | 4 ++++ config/initializers/azure_ad.rb | 35 ++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 app/models/application_settings.rb diff --git a/app/models/application_settings.rb b/app/models/application_settings.rb new file mode 100644 index 000000000..db93032dd --- /dev/null +++ b/app/models/application_settings.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +class ApplicationSettings < Settings +end diff --git a/config/initializers/azure_ad.rb b/config/initializers/azure_ad.rb index 12807fc10..4df6a3854 100644 --- a/config/initializers/azure_ad.rb +++ b/config/initializers/azure_ad.rb @@ -34,4 +34,39 @@ Rails.application.configure do config.x.azure_ad_apps[value][:sign_in_policy] = ENV["#{app_name}_AZURE_AD_SIGN_IN_POLICY"] end end + + # Checking additional configurations in ApplicationSettings JSON. Key and values should be strings there. + begin + if ApplicationSettings.instance.values['azure_ad_apps']&.is_a?(Array) + config.x.azure_ad_apps ||= HashWithIndifferentAccess.new + settings = ApplicationSettings.instance + + settings.values['azure_ad_apps'].each do |azure_ad_app| + app_config = {} + app_id = azure_ad_app['app_id'] + Rails.logger.error('No app_id present for the entry in Azure app settings') && next unless app_id + + app_config[:iss] = azure_ad_app['iss'] + Rails.logger.error("No iss for #{app_id} Azure app") && next unless app_config[:iss] + + app_config[:conf_url] = azure_ad_app['conf_url'] + Rails.logger.error("No conf_url for #{app_id} Azure app") && next unless app_config[:conf_url] + + app_config[:provider] = azure_ad_app['provider_name'] + Rails.logger.error("No provider_name for #{app_id} Azure app") && next unless app_config[:provider] + + app_config[:enable_sign_in] = azure_ad_app['enable_sign_in'] == 'true' + + if app_config[:enable_sign_in] + app_config[:sign_in_label] = azure_ad_app['sign_in_label'] || 'Sign in with Azure AD' + app_config[:auto_link_on_sign_in] = azure_ad_app['auto_link_on_sign_in'] == 'true' + app_config[:sign_in_policy] = azure_ad_app['sign_in_policy'] if azure_ad_app['sign_in_policy'] + end + + config.x.azure_ad_apps[app_id] = app_config + end + end + rescue ActiveRecord::ActiveRecordError + Rails.logger.info('Not connected to database, skipping additional Azure AD configuration') + end end From 34f1d289f0a9af840cd9de18abfb558406f085a4 Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Thu, 16 Apr 2020 15:27:30 +0200 Subject: [PATCH 2/6] Also ignore PG connection exception in extra Azure initialization code [SCI-4544] --- config/initializers/azure_ad.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/azure_ad.rb b/config/initializers/azure_ad.rb index 4df6a3854..389e53fa4 100644 --- a/config/initializers/azure_ad.rb +++ b/config/initializers/azure_ad.rb @@ -66,7 +66,7 @@ Rails.application.configure do config.x.azure_ad_apps[app_id] = app_config end end - rescue ActiveRecord::ActiveRecordError + rescue ActiveRecord::ActiveRecordError, PG::ConnectionBad Rails.logger.info('Not connected to database, skipping additional Azure AD configuration') end end From def3a8f71a95d19219d05fb185ed44b11cf66353 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 16 Apr 2020 14:15:41 +0000 Subject: [PATCH 3/6] Bump https-proxy-agent from 2.2.1 to 2.2.4 Bumps [https-proxy-agent](https://github.com/TooTallNate/node-https-proxy-agent) from 2.2.1 to 2.2.4. - [Release notes](https://github.com/TooTallNate/node-https-proxy-agent/releases) - [Commits](https://github.com/TooTallNate/node-https-proxy-agent/compare/2.2.1...2.2.4) Signed-off-by: dependabot[bot] --- yarn.lock | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/yarn.lock b/yarn.lock index b922a1f2d..3970a6b1b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1136,7 +1136,7 @@ adjust-sourcemap-loader@^1.1.0: object-path "^0.9.2" regex-parser "^2.2.9" -agent-base@4, agent-base@^4.1.0: +agent-base@4, agent-base@^4.3.0: version "4.3.0" resolved "https://registry.yarnpkg.com/agent-base/-/agent-base-4.3.0.tgz#8165f01c436009bccad0b1d122f05ed770efc6ee" integrity sha512-salcGninV0nPrwpGNn4VTXBb1SOuXQBiqbrNXoeizJsHrsL6ERFM2Ne3JUSBWRE6aeNJI2ROP/WEEIDUiDe3cg== @@ -4659,11 +4659,11 @@ https-browserify@^1.0.0: integrity sha1-7AbBDgo0wPL68Zn3/X/Hj//QPHM= https-proxy-agent@^2.2.0: - version "2.2.1" - resolved "https://registry.yarnpkg.com/https-proxy-agent/-/https-proxy-agent-2.2.1.tgz#51552970fa04d723e04c56d04178c3f92592bbc0" - integrity sha512-HPCTS1LW51bcyMYbxUIOO4HEOlQ1/1qRaFWcyxvwaqUS9TY88aoEuHUY33kuAh1YhVVaDQhLZsnPd+XNARWZlQ== + version "2.2.4" + resolved "https://registry.yarnpkg.com/https-proxy-agent/-/https-proxy-agent-2.2.4.tgz#4ee7a737abd92678a293d9b34a1af4d0d08c787b" + integrity sha512-OmvfoQ53WLjtA9HeYP9RNrWMJzzAz1JGaSFr1nijg0PVR1JaD/xbJq1mdEIIlxGpXp9eSe/O2LgU9DJmTPd0Eg== dependencies: - agent-base "^4.1.0" + agent-base "^4.3.0" debug "^3.1.0" iconv-lite@0.4.24, iconv-lite@^0.4.17, iconv-lite@^0.4.24, iconv-lite@^0.4.4, iconv-lite@~0.4.13: From 943285d4a0469c4dc4988a5399960328f108a53b Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Wed, 22 Apr 2020 16:47:06 +0200 Subject: [PATCH 4/6] Fix Azure AD user creation on sign in [SCI-4544] --- .../users/omniauth_callbacks_controller.rb | 47 ++++++++++++------- app/models/user.rb | 14 ------ config/locales/en.yml | 1 + 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 13fcbd415..b779d6118 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -28,25 +28,40 @@ module Users email = auth.info.email email ||= auth.dig(:extra, :raw_info, :id_token_claims, :emails)&.first user = User.from_omniauth(auth) - if user - # User found in database so just sign in him - sign_in_and_redirect(user) - elsif email.present? - user = User.find_by(email: email) - if user.blank? - # Create new user and identity - User.create_from_omniauth!(auth) - sign_in_and_redirect(user) - elsif provider_conf[:auto_link_on_sign_in] - # Link to existing local account + # User found in database so just signing in + return sign_in_and_redirect(user) if user.present? + + if email.blank? + # No email in the token so can not link or create user + error_message = I18n.t('devise.azure.errors.no_email') + return redirect_to after_omniauth_failure_path_for(resource_name) + end + + user = User.find_by(email: email) + + if user.blank? + # Create new user and identity + full_name = "#{auth.info.first_name} #{auth.info.last_name}" + user = User.new(full_name: full_name, + initials: generate_initials(full_name), + email: email, + password: generate_user_password) + User.transaction do + user.save! user.user_identities.create!(provider: auth.provider, uid: auth.uid) - sign_in_and_redirect(user) - else - # Cannot do anything with it, so just return an error - error_message = I18n.t('devise.azure.errors.no_local_user_map') - redirect_to after_omniauth_failure_path_for(resource_name) + user.update!(confirmed_at: user.created_at) end + + sign_in_and_redirect(user) + elsif provider_conf[:auto_link_on_sign_in] + # Link to existing local account + user.user_identities.create!(provider: auth.provider, uid: auth.uid) + sign_in_and_redirect(user) + else + # Cannot do anything with it, so just return an error + error_message = I18n.t('devise.azure.errors.no_local_user_map') + redirect_to after_omniauth_failure_path_for(resource_name) end rescue StandardError => e Rails.logger.error e.message diff --git a/app/models/user.rb b/app/models/user.rb index 9a8d5fcbf..ddeb1e5fc 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -354,20 +354,6 @@ class User < ApplicationRecord .take end - def self.create_from_omniauth!(auth) - full_name = "#{auth.info.first_name} #{auth.info.last_name}" - user = User.new(full_name: full_name, - initials: generate_initials(full_name), - email: email, - password: generate_user_password) - User.transaction do - user.save! - user.user_identities.create!(provider: auth.provider, uid: auth.uid) - user.update!(confirmed_at: user.created_at) - end - user - end - # Search all active users for username & email. Can # also specify which team to ignore. def self.search( diff --git a/config/locales/en.yml b/config/locales/en.yml index 7781bb3f4..8450d776c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -60,6 +60,7 @@ en: errors: generic: "Failed to sign in user" no_local_user_map: "No local user record found" + no_email: "Email is missing in auth token" failed_to_save: "Failed to create new user" doorkeeper: From d5e3ac27698b6b9e5353b29d9fb173f8c2ec2539 Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Wed, 22 Apr 2020 17:26:06 +0200 Subject: [PATCH 5/6] Add missing scopes for Azure AD OpenID Connect [SCI-4544] --- lib/omniauth/strategies/custom_azure_active_directory.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/omniauth/strategies/custom_azure_active_directory.rb b/lib/omniauth/strategies/custom_azure_active_directory.rb index eec52f6f3..d2c29f240 100644 --- a/lib/omniauth/strategies/custom_azure_active_directory.rb +++ b/lib/omniauth/strategies/custom_azure_active_directory.rb @@ -25,7 +25,7 @@ module OmniAuth response_mode: response_mode, response_type: response_type, nonce: new_nonce, - scope: 'openid' + scope: 'openid profile email' } params[:p] = options[:sign_in_policy] if options[:sign_in_policy].present? From 20bb0c51033df6ad74d0a7afbc81ae28215d4947 Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Tue, 5 May 2020 16:29:52 +0200 Subject: [PATCH 6/6] Bump version to 1.18.8 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index d6f3a382b..1a31d398c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.18.7 +1.18.8