From ff4ef818fbcd4d4c3bca15d782c90557a3b27147 Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Tue, 10 Dec 2019 17:24:53 +0100 Subject: [PATCH] Implement SSO with Azure AD [SCI-4142] --- .rubocop.yml | 18 +++---- Gemfile | 1 + Gemfile.lock | 4 ++ app/assets/stylesheets/themes/scinote.scss | 10 ++++ .../users/omniauth_callbacks_controller.rb | 49 +++++++++++++++++++ app/helpers/addons_helper.rb | 2 +- app/models/user.rb | 14 ++++++ app/services/api/azure_jwt.rb | 4 +- .../shared/_azure_sign_in_links.html.erb | 7 +++ app/views/users/shared/_links.html.erb | 4 ++ config/initializers/azure_ad.rb | 13 +++++ config/initializers/extends.rb | 2 +- config/initializers/omniauth.rb | 37 ++++++++++++++ config/locales/en.yml | 6 +++ .../custom_azure_active_directory.rb | 46 +++++++++++++++++ 15 files changed, 204 insertions(+), 13 deletions(-) create mode 100644 app/views/users/shared/_azure_sign_in_links.html.erb create mode 100644 config/initializers/omniauth.rb create mode 100644 lib/omniauth/strategies/custom_azure_active_directory.rb diff --git a/.rubocop.yml b/.rubocop.yml index ac9797608..1edaa3623 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -14,12 +14,12 @@ AllCops: Layout/AccessModifierIndentation: EnforcedStyle: indent -Layout/AlignHash: +Layout/HashAlignment: EnforcedHashRocketStyle: key EnforcedColonStyle: key EnforcedLastArgumentHashStyle: ignore_implicit -Layout/AlignParameters: +Layout/ParameterAlignment: EnforcedStyle: with_first_parameter Style/AndOr: @@ -83,7 +83,7 @@ Naming/FileName: Enabled: false Exclude: [] -Layout/IndentFirstParameter: +Layout/FirstParameterIndentation: EnforcedStyle: consistent Style/For: @@ -111,10 +111,10 @@ Layout/IndentationConsistency: Layout/IndentationWidth: Width: 2 -Layout/IndentFirstArrayElement: +Layout/FirstArrayElementIndentation: EnforcedStyle: special_inside_parentheses -Layout/IndentFirstHashElement: +Layout/FirstHashElementIndentation: EnforcedStyle: special_inside_parentheses Style/Next: @@ -197,9 +197,9 @@ Naming/PredicateName: - is_ - has_ - have_ - NamePrefixBlacklist: + ForbiddenPrefixes: - is_ - NameWhitelist: + AllowedMethods: - is_a? Exclude: - spec/**/* @@ -282,7 +282,7 @@ Style/TernaryParentheses: EnforcedStyle: require_no_parentheses AllowSafeAssignment: true -Layout/TrailingBlankLines: +Layout/TrailingEmptyLines: EnforcedStyle: final_newline Style/TrailingCommaInArguments: @@ -411,7 +411,7 @@ Lint/UnusedMethodArgument: Lint/EachWithObjectArgument: Enabled: true -Lint/HandleExceptions: +Lint/SuppressedException: Enabled: false Lint/LiteralAsCondition: diff --git a/Gemfile b/Gemfile index 1bd21c3f7..ccb50763a 100644 --- a/Gemfile +++ b/Gemfile @@ -23,6 +23,7 @@ gem 'yomu' # Gems for OAuth2 subsystem gem 'doorkeeper', '>= 4.6' gem 'omniauth' +gem 'omniauth-azure-activedirectory' gem 'omniauth-linkedin-oauth2' # TODO: remove this when omniauth gem resolves CVE issues diff --git a/Gemfile.lock b/Gemfile.lock index c6bed7429..edef64cc1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -364,6 +364,9 @@ GEM omniauth (1.9.0) hashie (>= 3.4.6, < 3.7.0) rack (>= 1.6.2, < 3) + omniauth-azure-activedirectory (1.0.0) + jwt (~> 1.5) + omniauth (~> 1.1) omniauth-linkedin-oauth2 (1.0.0) omniauth-oauth2 omniauth-oauth2 (1.6.0) @@ -648,6 +651,7 @@ DEPENDENCIES newrelic_rpm nokogiri (~> 1.10.3) omniauth + omniauth-azure-activedirectory omniauth-linkedin-oauth2 omniauth-rails_csrf_protection (~> 0.1) overcommit diff --git a/app/assets/stylesheets/themes/scinote.scss b/app/assets/stylesheets/themes/scinote.scss index aa1faf9c4..7be1f04a1 100644 --- a/app/assets/stylesheets/themes/scinote.scss +++ b/app/assets/stylesheets/themes/scinote.scss @@ -380,6 +380,16 @@ a[data-toggle="tooltip"] { width: 100%; } +.azure-sign-in-actions { + margin-bottom: 10px; + margin-top: 10px; + + .btn-azure-ad { + background-color: $office-ms-word; + color: $color-white; + } +} + .navbar-secondary { -webkit-transition: all 0.5s ease; -moz-transition: all 0.5s ease; diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 79c75d232..13fcbd415 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Users class OmniauthCallbacksController < Devise::OmniauthCallbacksController include UsersGenerator @@ -13,6 +15,53 @@ module Users # def twitter # end + def customazureactivedirectory + auth = request.env['omniauth.auth'] + provider_id = auth.dig(:extra, :raw_info, :id_token_claims, :aud) + provider_conf = Rails.configuration.x.azure_ad_apps[provider_id] + raise StandardError, 'No matching Azure AD provider config found' if provider_conf.empty? + + auth.provider = provider_conf[:provider] + + return redirect_to connected_accounts_path if current_user + + 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.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 + end + rescue StandardError => e + Rails.logger.error e.message + Rails.logger.error e.backtrace.join("\n") + error_message = I18n.t('devise.azure.errors.failed_to_save') if e.is_a?(ActiveRecord::RecordInvalid) + error_message ||= I18n.t('devise.azure.errors.generic') + redirect_to after_omniauth_failure_path_for(resource_name) + ensure + if error_message + set_flash_message(:alert, :failure, kind: I18n.t('devise.azure.provider_name'), reason: error_message) + else + set_flash_message(:notice, :success, kind: I18n.t('devise.azure.provider_name')) + end + end + def linkedin auth_hash = request.env['omniauth.auth'] diff --git a/app/helpers/addons_helper.rb b/app/helpers/addons_helper.rb index 150af4976..9a5622ff0 100644 --- a/app/helpers/addons_helper.rb +++ b/app/helpers/addons_helper.rb @@ -3,6 +3,6 @@ module AddonsHelper Rails::Engine .subclasses .select { |c| c.name.start_with?('Scinote') } - .map(&:parent) + .map(&:module_parent) end end diff --git a/app/models/user.rb b/app/models/user.rb index ce0cd785d..c00accce1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -297,6 +297,20 @@ 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/app/services/api/azure_jwt.rb b/app/services/api/azure_jwt.rb index f6804d2ce..e2076f043 100644 --- a/app/services/api/azure_jwt.rb +++ b/app/services/api/azure_jwt.rb @@ -42,7 +42,7 @@ module Api end # Decode token payload and verify it's signature. - payload, = JWT.decode( + payload, header = JWT.decode( token, OpenSSL::PKey::RSA.new(fetch_rsa_key(k_id, app_id)), true, @@ -54,7 +54,7 @@ module Api iss: app_config[:iss], nbf_leeway: LEEWAY ) - HashWithIndifferentAccess.new(payload) + [HashWithIndifferentAccess.new(payload), HashWithIndifferentAccess.new(header)] end end end diff --git a/app/views/users/shared/_azure_sign_in_links.html.erb b/app/views/users/shared/_azure_sign_in_links.html.erb new file mode 100644 index 000000000..fb5c5fab8 --- /dev/null +++ b/app/views/users/shared/_azure_sign_in_links.html.erb @@ -0,0 +1,7 @@ +<% Rails.configuration.x.azure_ad_apps.select { |uid, config| config[:enable_sign_in] }.each do |uid, config| %> +
+ <%= form_tag user_customazureactivedirectory_omniauth_authorize_path(provider: config[:provider]), method: :post do %> + <%= submit_tag config[:sign_in_label], class: 'btn btn-azure-ad' %> + <% end %> +
+<% end %> diff --git a/app/views/users/shared/_links.html.erb b/app/views/users/shared/_links.html.erb index 91f2fec81..5770842f7 100644 --- a/app/views/users/shared/_links.html.erb +++ b/app/views/users/shared/_links.html.erb @@ -37,4 +37,8 @@ <% end -%> <% end -%> <% end -%> + +
+ <%= render partial: "users/shared/azure_sign_in_links", locals: { resource_name: resource_name } %> +
diff --git a/config/initializers/azure_ad.rb b/config/initializers/azure_ad.rb index 78cfc9b23..12807fc10 100644 --- a/config/initializers/azure_ad.rb +++ b/config/initializers/azure_ad.rb @@ -2,6 +2,8 @@ Rails.application.configure do vars = ENV.select { |name, _| name =~ /^[[:alnum:]]*_AZURE_AD_APP_ID/ } + config.x.azure_ad_apps = HashWithIndifferentAccess.new if vars.present? + vars.each do |name, value| app_name = name.sub('_AZURE_AD_APP_ID', '') config.x.azure_ad_apps[value] = {} @@ -20,5 +22,16 @@ Rails.application.configure do raise StandardError, "No PROVIDER_NAME for #{app_name} Azure app" unless provider config.x.azure_ad_apps[value][:provider] = provider + + config.x.azure_ad_apps[value][:enable_sign_in] = ENV["#{app_name}_AZURE_AD_ENABLE_SIGN_IN"] == 'true' + + next unless config.x.azure_ad_apps[value][:enable_sign_in] + + config.x.azure_ad_apps[value][:sign_in_label] = ENV["#{app_name}_AZURE_AD_SIGN_IN_LABEL"] || 'Sign in with Azure AD' + config.x.azure_ad_apps[value][:auto_link_on_sign_in] = ENV["#{app_name}_AZURE_AD_AUTO_LINK_ON_SIGN_IN"] == 'true' + + if ENV["#{app_name}_AZURE_AD_SIGN_IN_POLICY"] + config.x.azure_ad_apps[value][:sign_in_policy] = ENV["#{app_name}_AZURE_AD_SIGN_IN_POLICY"] + end end end diff --git a/config/initializers/extends.rb b/config/initializers/extends.rb index 98010eeaa..ee9b9f3de 100644 --- a/config/initializers/extends.rb +++ b/config/initializers/extends.rb @@ -73,7 +73,7 @@ class Extends 'RepositoryListValue' => 'list', 'RepositoryAssetValue' => 'file' } - OMNIAUTH_PROVIDERS = [:linkedin] + OMNIAUTH_PROVIDERS = [:linkedin, :customazureactivedirectory] INITIAL_USER_OPTIONS = {} diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb new file mode 100644 index 000000000..34f6512da --- /dev/null +++ b/config/initializers/omniauth.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'omniauth/strategies/custom_azure_active_directory' + +SETUP_PROC = lambda do |env| + providers = Rails.configuration.x.azure_ad_apps.select { |_, v| v[:enable_sign_in] == true } + raise StandardError, 'No Azure AD config available for sign in' if providers.empty? + + req = Rack::Request.new(env) + + if providers.size > 1 + if req.params['id_token'].present? # Callback phase + unverified_jwt_payload, = JWT.decode(req.params['id_token'], nil, false) + raise StandardError, 'No Azure AD config available for sign in' if providers[unverified_jwt_payload['aud']].blank? + + provider_id = unverified_jwt_payload['aud'] + else # Authorization phase + raise ActionController::ParameterMissing, 'Provider name is missing' if req.params['provider'].empty? + + provider_id = providers.select { |_, v| v[:provider] == req.params['provider'] }.keys.first + raise StandardError, 'No Azure AD config available for sign in' if provider_id.blank? + end + end + + provider_id ||= providers.keys.first + provider_conf = providers[provider_id] + + env['omniauth.strategy'].options[:client_id] = provider_id + env['omniauth.strategy'].options[:openid_config_url] = provider_conf[:conf_url] + env['omniauth.strategy'].options[:sign_in_policy] = provider_conf[:sign_in_policy] +end + +Rails.application.config.middleware.use OmniAuth::Builder do + provider OmniAuth::Strategies::CustomAzureActiveDirectory, setup: SETUP_PROC +end + +OmniAuth.config.logger = Rails.logger diff --git a/config/locales/en.yml b/config/locales/en.yml index 3bbed6ecf..1b17d6343 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -51,6 +51,12 @@ en: complete_sign_up: "You have to complete the sign up process" email_already_taken: "SciNote account with email %{email} alreday exists" failed_to_save: "Failed to create new user" + azure: + provider_name: "Azure Active Directory" + errors: + generic: "Failed to sign in user" + no_local_user_map: "No local user record found" + failed_to_save: "Failed to create new user" doorkeeper: authorizations: diff --git a/lib/omniauth/strategies/custom_azure_active_directory.rb b/lib/omniauth/strategies/custom_azure_active_directory.rb new file mode 100644 index 000000000..eec52f6f3 --- /dev/null +++ b/lib/omniauth/strategies/custom_azure_active_directory.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module OmniAuth + module Strategies + class CustomAzureActiveDirectory < AzureActiveDirectory + include OmniAuth::Strategy + + option :openid_config_url + option :sign_in_policy + + # Azure doesn't allow query params in callback URL + def callback_url + full_host + script_name + callback_path + end + + def openid_config_url + options[:openid_config_url] + end + + def authorize_endpoint_url + uri = URI(openid_config['authorization_endpoint']) + params = { + client_id: client_id, + redirect_uri: callback_url, + response_mode: response_mode, + response_type: response_type, + nonce: new_nonce, + scope: 'openid' + } + params[:p] = options[:sign_in_policy] if options[:sign_in_policy].present? + + uri.query = URI.encode_www_form(params) + uri.to_s + end + + def validate_and_parse_id_token(id_token) + jwt_claims, jwt_header = Api::AzureJwt.decode(id_token) + return jwt_claims, jwt_header if jwt_claims['nonce'] == read_nonce + + raise JWT::DecodeError, 'Returned nonce did not match.' + end + end + end +end + +OmniAuth.config.add_camelization 'custom_azure_activedirectory', 'CustomAzureActiveDirectory'