Apply omniauth defaults when no arguments are given

This ensures that if a provider has default arguments, and
none are specified in the gitlab configuration, then we still
apply the defaults.

Changelog: fixed

app/controllers/oauth/authorizations_controller.rb

@@ -12,10 +12,7 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController
   # Overridden from Doorkeeper::AuthorizationsController to
   # include the call to session.delete
   def new
-    logger.info("#{self.class.name}#new: pre_auth_params['scope'] = #{pre_auth_params['scope'].inspect}")
-
     if pre_auth.authorizable?
-      logger.info("#{self.class.name}#new: pre_auth.scopes = #{pre_auth.scopes.to_a.inspect}")
       if skip_authorization? || matching_token?
         auth = authorization.authorize
         parsed_redirect_uri = URI.parse(auth.redirect_uri)
@@ -46,15 +43,9 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController
     auth_type = params.delete('gl_auth_type')
     return unless auth_type == 'login'
 
-    logger.info("#{self.class.name}: BEFORE application has read_user: #{application_has_read_user_scope?}")
-    logger.info("#{self.class.name}: BEFORE scope = #{params['scope'].inspect}")
-
     ensure_read_user_scope!
 
     params['scope'] = Gitlab::Auth::READ_USER_SCOPE.to_s if application_has_read_user_scope?
-
-    logger.info("#{self.class.name}: AFTER application has read_user: #{application_has_read_user_scope?}")
-    logger.info("#{self.class.name}: AFTER scope = #{params['scope'].inspect}")
   end
 
   # Configure the application to support read_user scope, if it already

lib/gitlab/omniauth_initializer.rb

@@ -3,6 +3,7 @@
 module Gitlab
   class OmniauthInitializer
     OAUTH2_TIMEOUT_SECONDS = 10
+    ConfigurationError = Class.new(StandardError)
 
     def initialize(devise_config)
       @devise_config = devise_config
@@ -75,16 +76,29 @@ module Gitlab
         provider_arguments << provider[argument] if provider[argument]
       end
 
-      case provider['args']
+      arguments = provider.fetch('args', {})
+      defaults = provider_defaults(provider)
+
+      case arguments
       when Array
-        # An Array from the configuration will be expanded.
-        provider_arguments.concat provider['args']
+        # An Array from the configuration will be expanded
+        provider_arguments.concat arguments
+        provider_arguments << defaults unless defaults.empty?
       when Hash
-        defaults = provider_defaults(provider)
-        hash_arguments = provider['args'].deep_symbolize_keys.deep_merge(defaults)
+        hash_arguments = arguments.deep_symbolize_keys.deep_merge(defaults)
+        normalized = normalize_hash_arguments(hash_arguments)
 
         # A Hash from the configuration will be passed as is.
-        provider_arguments << normalize_hash_arguments(hash_arguments)
+        provider_arguments << normalized unless normalized.empty?
+      else
+        # this will prevent the application from starting in development mode.
+        # we still set defaults, and let the application start in prod.
+        Gitlab::ErrorTracking.track_and_raise_for_dev_exception(
+          ConfigurationError.new("Arguments were provided for #{provider['name']}, but not as an array or a hash"),
+          provider_name: provider['name'],
+          arguments_type: arguments.class.name
+        )
+        provider_arguments << defaults unless defaults.empty?
       end
 
       provider_arguments

spec/lib/gitlab/omniauth_initializer_spec.rb

@@ -105,9 +105,48 @@ RSpec.describe Gitlab::OmniauthInitializer do
     it 'configures defaults for gitlab' do
       conf = {
         'name' => 'gitlab',
-        "args" => {}
+        "args" => { 'client_options' => { 'site' => generate(:url) } }
       }
 
+      expect(devise_config).to receive(:omniauth).with(
+        :gitlab,
+        client_options: { site: conf.dig('args', 'client_options', 'site') },
+        authorize_params: { gl_auth_type: 'login' }
+      )
+
+      subject.execute([conf])
+    end
+
+    it 'configures defaults for gitlab, when arguments are not provided' do
+      conf = { 'name' => 'gitlab' }
+
+      expect(devise_config).to receive(:omniauth).with(
+        :gitlab,
+        authorize_params: { gl_auth_type: 'login' }
+      )
+
+      subject.execute([conf])
+    end
+
+    it 'configures defaults for gitlab, when array arguments are provided' do
+      conf = { 'name' => 'gitlab', 'args' => ['a'] }
+
+      expect(devise_config).to receive(:omniauth).with(
+        :gitlab,
+        'a',
+        authorize_params: { gl_auth_type: 'login' }
+      )
+
+      subject.execute([conf])
+    end
+
+    it 'tracks a configuration error if the arguments are neither a hash nor an array' do
+      conf = { 'name' => 'gitlab', 'args' => 17 }
+
+      expect(::Gitlab::ErrorTracking)
+        .to receive(:track_and_raise_for_dev_exception)
+        .with(described_class::ConfigurationError, provider_name: 'gitlab', arguments_type: 'Integer')
+
       expect(devise_config).to receive(:omniauth).with(
         :gitlab,
         authorize_params: { gl_auth_type: 'login' }