Merge branch '64407-vfazio-quirk-omniauth-strategies-openidconnect' into 'master'

Convert client_auth_method to a Symbol for quirked OmniAuth providers

Closes #64407

See merge request gitlab-org/gitlab-ce!30683

4 files changed, 53 insertions, 3 deletions

diff --git a/changelogs/unreleased/64407-vfazio-quirk-omniauth-strategies-openidconnect.yml b/changelogs/unreleased/64407-vfazio-quirk-omniauth-strategies-openidconnect.yml
new file mode 100644
index 00000000000..c2e300863fb
--- /dev/null
+++ b/changelogs/unreleased/64407-vfazio-quirk-omniauth-strategies-openidconnect.yml
@@ -0,0 +1,5 @@
+---
+title: Allow client authentication method to be configured for OpenID Connect
+merge_request: 30683
+author: Vincent Fazio
+type: fixed
diff --git a/doc/administration/auth/oidc.md b/doc/administration/auth/oidc.md
index 454da8c2866..758501629af 100644
--- a/doc/administration/auth/oidc.md
+++ b/doc/administration/auth/oidc.md
@@ -81,6 +81,13 @@ The OpenID Connect will provide you with a client details and secret for you to
     - `<your_oidc_url>` (optional) is the URL that points to the OpenID Connect provider. For example, `https://example.com/auth/realms/your-realm`.
       If this value is not provided, the URL is constructed from the `client_options` in the following format: `<client_options.scheme>://<client_options.host>:<client_options.port>`.
     - If `discovery` is set to `true`, the OpenID Connect provider will try to auto discover the client options using `<your_oidc_url>/.well-known/openid-configuration`. Defaults to `false`.
+    - `client_auth_method` (optional) specifies the method used for authenticating the client with the OpenID Connect provider.
+      - Supported values are:
+        - `basic` - HTTP Basic Authentication
+        - `jwt_bearer` - JWT based authentication (private key and client secret signing)
+        - `mtls` - Mutual TLS or X.509 certificate validation
+        - Any other value will POST the client id and secret in the request body
+      - If not specified, defaults to `basic`.
     - `<uid_field>` (optional) is the field name from the `user_info` details that will be used as `uid` value. For example, `preferred_username`.
       If this value is not provided or the field with the configured value is missing from the `user_info` details, the `uid` will use the `sub` field.
     - `client_options` are the OpenID Connect client-specific options. Specifically:
@@ -155,9 +162,9 @@ If you're having trouble, here are some tips:
    `https://accounts.google.com/.well-known/openid-configuration`.
 
 1. The OpenID Connect client uses HTTP Basic Authentication to send the
-   OAuth2 access token. For example, if you are seeing 401 errors upon
-   retrieving the `userinfo` endpoint, you may want to check your OpenID
-   Web server configuration. For example, for
+   OAuth2 access token if `client_auth_method` is not defined or if set to `basic`.
+   If you are seeing 401 errors upon retrieving the `userinfo` endpoint, you may
+   want to check your OpenID Web server configuration. For example, for
    [oauth2-server-php](https://github.com/bshaffer/oauth2-server-php), you
    may need to [add a configuration parameter to
    Apache](https://github.com/bshaffer/oauth2-server-php/issues/926#issuecomment-387502778).
diff --git a/lib/gitlab/omniauth_initializer.rb b/lib/gitlab/omniauth_initializer.rb
index 2a2083ebae0..ad1377a0892 100644
--- a/lib/gitlab/omniauth_initializer.rb
+++ b/lib/gitlab/omniauth_initializer.rb
@@ -52,6 +52,16 @@ module Gitlab
         args[:strategy_class] = args[:strategy_class].constantize
       end
 
+      # Providers that are known to depend on rack-oauth2, like those using
+      # Omniauth::Strategies::OpenIDConnect, need to be quirked so the
+      # client_auth_method argument value is passed as a symbol.
+      if (args[:strategy_class] == OmniAuth::Strategies::OpenIDConnect ||
+        args[:name] == 'openid_connect') &&
+        args[:client_auth_method].is_a?(String)
+
+        args[:client_auth_method] = args[:client_auth_method].to_sym
+      end
+
       args
     end
 
diff --git a/spec/lib/gitlab/omniauth_initializer_spec.rb b/spec/lib/gitlab/omniauth_initializer_spec.rb
index f9c0daf1ef1..ef5c93e5c6b 100644
--- a/spec/lib/gitlab/omniauth_initializer_spec.rb
+++ b/spec/lib/gitlab/omniauth_initializer_spec.rb
@@ -83,5 +83,33 @@ describe Gitlab::OmniauthInitializer do
 
       subject.execute([cas3_config])
     end
+
+    it 'converts client_auth_method to a Symbol for openid_connect' do
+      openid_connect_config = {
+        'name' => 'openid_connect',
+        'args' => { name: 'openid_connect', client_auth_method: 'basic' }
+      }
+
+      expect(devise_config).to receive(:omniauth).with(
+        :openid_connect,
+        { name: 'openid_connect', client_auth_method: :basic }
+      )
+
+      subject.execute([openid_connect_config])
+    end
+
+    it 'converts client_auth_method to a Symbol for strategy_class OpenIDConnect' do
+      openid_connect_config = {
+        'name' => 'openid_connect',
+        'args' => { strategy_class: OmniAuth::Strategies::OpenIDConnect, client_auth_method: 'jwt_bearer' }
+      }
+
+      expect(devise_config).to receive(:omniauth).with(
+        :openid_connect,
+        { strategy_class: OmniAuth::Strategies::OpenIDConnect, client_auth_method: :jwt_bearer }
+      )
+
+      subject.execute([openid_connect_config])
+    end
   end
 end