Merge branch 'oauth2' into dev

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog Fab-manager
 
+- Support for JSONPath syntax in OAuth2 SSO fields mapping
+- Support for OAuth2 scopes
+- Add debug logs for the SSO authentication process
+- Fix a bug: SSO configuration interface has a misnamed field (Common URL)
+- Fix a bug: unable to bind Profile.birthday and Profile.gender from an SSO
+
 - Ability to cancel a payement schedule from the interface
 - Ability to create slots in the past
 - Ability to disable public account creation

Gemfile

@@ -14,6 +14,7 @@ gem 'webpacker', '~> 5.x'
 gem 'jbuilder', '~> 2.5'
 gem 'jbuilder_cache_multi'
 gem 'json', '>= 2.3.0'
+gem 'jsonpath'
 
 gem 'forgery'
 gem 'responders', '~> 2.0'
@@ -65,7 +66,10 @@ gem 'pg_search'
 # authentication
 gem 'devise', '>= 4.6.0'
 
-gem 'omniauth', '~> 1.9.0'
+
+git 'https://github.com/sleede/omniauth.git', branch: 'v1.9' do
+  gem 'omniauth', '~> 1.9.0'
+end
 gem 'omniauth-oauth2'
 gem 'omniauth-rails_csrf_protection', '~> 0.1'
 

Gemfile.lock

@@ -1,3 +1,12 @@
+GIT
+  remote: https://github.com/sleede/omniauth.git
+  revision: 832d5ac48fcc3b93eba8bf1444145d0c876314cf
+  branch: v1.9
+  specs:
+    omniauth (1.9.1)
+      hashie (>= 3.4.6)
+      rack (>= 1.6.2, < 3)
+
 GEM
   remote: https://rubygems.org/
   specs:
@@ -175,6 +184,8 @@ GEM
     jbuilder_cache_multi (0.1.0)
       jbuilder (>= 1.5.0, < 3)
     json (2.3.1)
+    jsonpath (1.1.0)
+      multi_json
     jwt (2.2.1)
     kaminari (1.2.1)
       activesupport (>= 4.1.0)
@@ -234,9 +245,6 @@ GEM
       multi_xml (~> 0.5)
       rack (>= 1.2, < 3)
     oj (3.10.5)
-    omniauth (1.9.1)
-      hashie (>= 3.4.6)
-      rack (>= 1.6.2, < 3)
     omniauth-oauth2 (1.6.0)
       oauth2 (~> 1.1)
       omniauth (~> 1.9)
@@ -456,6 +464,7 @@ DEPENDENCIES
   jbuilder (~> 2.5)
   jbuilder_cache_multi
   json (>= 2.3.0)
+  jsonpath
   kaminari
   listen (~> 3.0.5)
   message_format
@@ -463,7 +472,7 @@ DEPENDENCIES
   minitest-reporters
   notify_with
   oj
-  omniauth (~> 1.9.0)
+  omniauth (~> 1.9.0)!
   omniauth-oauth2
   omniauth-rails_csrf_protection (~> 0.1)
   openlab_ruby

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,19 +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.
-      @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
 
@@ -58,17 +79,17 @@ class Users::OmniauthCallbacksController < Devise::OmniauthCallbacksController
 
   def username_exists?(username, exclude_id = nil)
     if exclude_id.nil?
-      User.where(username: username).size.positive?
+      User.where('lower(username) = ?', username&.downcase).size.positive?
     else
-      User.where(username: username).where.not(id: exclude_id).size.positive?
+      User.where('lower(username) = ?', username&.downcase).where.not(id: exclude_id).size.positive?
     end
   end
 
   def email_exists?(email, exclude_id = nil)
     if exclude_id.nil?
-      User.where(email: email).size.positive?
+      User.where('lower(email) = ?', email&.downcase).size.positive?
     else
-      User.where(email: email).where.not(id: exclude_id).size.positive?
+      User.where('lower(email) = ?', email&.downcase).where.not(id: exclude_id).size.positive?
     end
   end
 

app/frontend/templates/admin/authentications/_oauth2_mapping.html

@@ -64,10 +64,11 @@
             </td>
             <td ng-class="{'has-error': mappingForm['auth_mapping[api_field]'].$dirty && mappingForm['auth_mapping[api_field]'].$invalid}">
                 <input type="text"
-                       class="form-control"
+                       class="form-control help-cursor"
                        placeholder="field_name"
                        ng-model="newMapping.api_field"
                        name="auth_mapping[api_field]"
+                       title="{{ 'app.shared.oauth2.api_field_help' | translate }}"
                        required/>
             </td>
             <td>

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

app/models/profile.rb

@@ -27,7 +27,8 @@ class Profile < ApplicationRecord
     # we protect some fields as they are designed to be managed by the system and must not be updated externally
     blacklist = %w[id user_id created_at updated_at]
     # model-relationships must be added manually
-    additional = [%w[avatar string], %w[address string], %w[organization_name string], %w[organization_address string]]
+    additional = [%w[avatar string], %w[address string], %w[organization_name string], %w[organization_address string],
+                  %w[gender boolean], %w[birthday date]]
     Profile.columns_hash
            .map { |k, v| [k, v.type.to_s] }
            .delete_if { |col| blacklist.include?(col[0]) }

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
@@ -238,6 +242,10 @@ class User < ApplicationRecord
         invoicing_profile.organization.name
       when 'profile.organization_address'
         invoicing_profile.organization.address.address
+      when 'profile.gender'
+        statistic_profile.gender
+      when 'profile.birthday'
+        statistic_profile.birthday
       else
         profile[parsed[2].to_sym]
       end
@@ -256,15 +264,24 @@ class User < ApplicationRecord
         profile.user_avatar ||= UserAvatar.new
         profile.user_avatar.remote_attachment_url = data
       when 'profile.address'
+        invoicing_profile ||= InvoicingProfile.new
         invoicing_profile.address ||= Address.new
         invoicing_profile.address.address = data
       when 'profile.organization_name'
+        invoicing_profile ||= InvoicingProfile.new
         invoicing_profile.organization ||= Organization.new
         invoicing_profile.organization.name = data
       when 'profile.organization_address'
+        invoicing_profile ||= InvoicingProfile.new
         invoicing_profile.organization ||= Organization.new
         invoicing_profile.organization.address ||= Address.new
         invoicing_profile.organization.address.address = data
+      when 'profile.gender'
+        statistic_profile ||= StatisticProfile.new
+        statistic_profile.gender = data
+      when 'profile.birthday'
+        statistic_profile ||= StatisticProfile.new
+        statistic_profile.birthday = data
       else
         profile[sso_mapping[8..-1].to_sym] = data unless data.nil?
       end
@@ -292,6 +309,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,24 +321,34 @@ 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')
     end
 
     # run the account transfer in an SQL transaction to ensure data integrity
-    User.transaction do
+    begin
-      # remove the temporary account
+      User.transaction do
-      sso_user.destroy
+        # remove the temporary account
-      # finally, save the new details
+        logger.debug 'removing the temporary user'
-      save!
+        sso_user.destroy
+        # finally, save the new details
+        logger.debug 'saving the updated user'
+        save!
+      end
+    rescue ActiveRecord::RecordInvalid => e
+      logger.error "error while merging user #{sso_user.id} into #{id}: #{e.message}"
+      raise e
     end
   end
 

app/views/api/auth_providers/mapping_fields.json.jbuilder

@@ -1,4 +1,4 @@
+# frozen_string_literal: true
 
 json.user User.mapping
-
 json.profile Profile.mapping

config/locales/app.shared.en.yml

@@ -252,7 +252,7 @@ en:
         mappings: "Mappings"
       #edition/creation form of an OAuth2 authentication provider
       oauth2:
-        common_url: "Common URL"
+        common_url: "Server root URL"
         common_url_is_required: "Common URL is required."
         provided_url_is_not_a_valid_url: "Provided URL is not a valid URL."
         authorization_endpoint: "Authorization endpoint"
@@ -274,6 +274,7 @@ en:
         api_endpoint_url: "API endpoint URL"
         api_type: "API type"
         api_fields: "API fields"
+        api_field_help: "JsonPath syntax is supported.\n If many fields are selected, the first one will be used.\n Example: $.data[*].name"
       #machine/training slot modification modal
       confirm_modify_slot_modal:
         change_the_slot: "Change the slot"

doc/sso_with_github.md

@@ -26,9 +26,9 @@ For this guide, we will use [GitHub](https://developer.github.com/v3/oauth/) as
   3. The slug of this name is used in the callback URL provided to the SSO server (eg. /users/auth/oauth2-**github**/callback)
 
 - Fulfill the form with the following parameters:
-  - **Common URL**: `https://github.com/login/oauth/` This is the common part in the URLs of the two following parameters.
+  - **Server root URL**: `https://github.com` This is the domain name of the where the SSO server is located.
-  - **Authorization endpoint**: `authorize` This URL can be found [here](https://developer.github.com/v3/oauth/).
+  - **Authorization endpoint**: `/login/oauth/authorize` This URL can be found [here](https://developer.github.com/v3/oauth/).
-  - **Token Acquisition Endpoint**: `access_token` This URL can be found [here](https://developer.github.com/v3/oauth/).
+  - **Token Acquisition Endpoint**: `/login/oauth/access_token` This URL can be found [here](https://developer.github.com/v3/oauth/).
   - **Profile edition URL**: `https://github.com/settings/profile` This is the URL where you are directed when you click on `Edit profile` in your GitHub dashboard.
   - **Client identifier**: Your Client ID, collected just before.
   - **Client secret**: Your Client Secret, collected just before.

lib/omni_auth/strategies/sso_oauth2_provider.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require 'omniauth-oauth2'
+require 'jsonpath'
 
 module OmniAuth::Strategies
   # Authentication strategy provided trough oAuth 2.0
@@ -24,6 +25,12 @@ module OmniAuth::Strategies
            authorize_url: active_provider.providable.authorization_endpoint,
            token_url: active_provider.providable.token_endpoint
 
+    def authorize_params
+      super.tap do |params|
+        params[:scope] = ENV['OAUTH2_SCOPE']
+      end
+    end
+
     def callback_url
       url = Rails.application.config.action_controller.default_url_options
       "#{url[:protocol]}://#{url[:host]}#{script_name}#{callback_path}"
@@ -46,9 +53,15 @@ module OmniAuth::Strategies
     # retrieve data from various url, querying each only once
     def raw_info
       @raw_info ||= {}
+      puts "[raw_info] @raw_infos = #{@raw_info&.to_json}"
       unless @raw_info.size.positive?
         OmniAuth::Strategies::SsoOauth2Provider.active_provider.providable.o_auth2_mappings.each do |mapping|
+          puts "mapping = #{mapping&.to_json}"
           unless @raw_info.key?(mapping.api_endpoint.to_sym)
+            puts "api_endpoint = #{mapping.api_endpoint.to_sym}"
+            puts "access_token = #{access_token&.to_json}"
+            puts "token get = #{access_token.get(mapping.api_endpoint)}"
+            puts "parsed = #{access_token.get(mapping.api_endpoint).parsed}"
             @raw_info[mapping.api_endpoint.to_sym] = access_token.get(mapping.api_endpoint).parsed
           end
         end
@@ -58,6 +71,7 @@ module OmniAuth::Strategies
 
     def parsed_info
       @parsed_info ||= {}
+      puts "[parsed_info] @parsed_info = #{@parsed_info.to_json}"
       unless @parsed_info.size.positive?
         OmniAuth::Strategies::SsoOauth2Provider.active_provider.providable.o_auth2_mappings.each do |mapping|
 
@@ -88,7 +102,8 @@ module OmniAuth::Strategies
 
           ## NO TRANSFORMATION
           else
-            @parsed_info[local_sym(mapping)] = raw_info[mapping.api_endpoint.to_sym][mapping.api_field]
+            puts "@parsed_info[#{local_sym(mapping)}] found in #{raw_info[mapping.api_endpoint.to_sym]}"
+            @parsed_info[local_sym(mapping)] = ::JsonPath.new(mapping.api_field).on(raw_info[mapping.api_endpoint.to_sym]).first
           end
         end
       end