Revert "Merge pull request #12202 from opf/fix/bump_rack-oauth2"

This reverts commit f110631265, reversing
changes made to 4985ba1c77.

Gemfile.lock

@@ -707,11 +707,10 @@ GEM
       rack (>= 2.0.0)
     rack-mini-profiler (3.0.0)
       rack (>= 1.2.0)
-    rack-oauth2 (2.2.0)
+    rack-oauth2 (1.21.3)
       activesupport
       attr_required
-      faraday (~> 2.0)
-      faraday-follow_redirects
+      httpclient
       json-jwt (>= 1.11.0)
       rack (>= 2.1.0)
     rack-protection (3.0.5)

app/services/oauth_clients/connection_manager.rb

@@ -198,23 +198,20 @@ module OAuthClients
     # Calls client.access_token!
     # Convert the various exceptions into user-friendly error strings.
     def request_new_token(options = {})
-      rack_access_token = rack_oauth_client(options).access_token!(:body)
+      rack_access_token = rack_oauth_client(options)
+                            .access_token!(:body) # Rack::OAuth2::AccessToken
 
       ServiceResult.success(result: rack_access_token)
-    rescue Rack::OAuth2::Client::Error => e
+    rescue Rack::OAuth2::Client::Error => e # Handle Rack::OAuth2 specific errors
       service_result_with_error(i18n_rack_oauth2_error_message(e), e.message)
-    rescue Faraday::TimeoutError,
-           Faraday::ConnectionFailed,
-           Faraday::ParsingError,
-           Faraday::SSLError => e
+    rescue Timeout::Error, EOFError, Net::HTTPBadResponse, Net::HTTPHeaderSyntaxError, Net::ProtocolError,
+           Errno::EINVAL, Errno::ENETUNREACH, Errno::ECONNRESET, Errno::ECONNREFUSED, JSON::ParserError => e
       service_result_with_error(
-        "#{I18n.t('oauth_client.errors.oauth_returned_http_error')}: #{e.class}: #{e.message.to_html}",
-        e.message
+        "#{I18n.t('oauth_client.errors.oauth_returned_http_error')}: #{e.class}: #{e.message.to_html}"
       )
     rescue StandardError => e
       service_result_with_error(
-        "#{I18n.t('oauth_client.errors.oauth_returned_standard_error')}: #{e.class}: #{e.message.to_html}",
-        e.message
+        "#{I18n.t('oauth_client.errors.oauth_returned_standard_error')}: #{e.class}: #{e.message.to_html}"
       )
     end
 

modules/openid_connect/lib/open_project/openid_connect/engine.rb

@@ -28,6 +28,12 @@ module OpenProject::OpenIDConnect
     class_inflection_override('openid_connect' => 'OpenIDConnect')
 
     register_auth_providers do
+      # Use OpenSSL default certificate store instead of HTTPClient's.
+      # It's outdated and it's unclear how it's managed.
+      OpenIDConnect.http_config do |config|
+        config.ssl_config.set_default_paths
+      end
+
       OmniAuth::OpenIDConnect::Providers.configure custom_options: %i[
         display_name? icon? sso? issuer?
         check_session_iframe? end_session_endpoint?

spec/services/oauth_clients/connection_manager_spec.rb

@@ -142,7 +142,7 @@ describe OAuthClients::ConnectionManager, type: :model do
   # The callback endpoint calls `code_to_token(code)` with the code
   # received and exchanges the code for a bearer+refresh token
   # using a HTTP request.
-  describe '#code_to_token', webmock: true do
+  describe '#code_to_token' do
     let(:code) { "7kRGJ...jG3KZ" }
 
     subject { instance.code_to_token(code) }
@@ -158,19 +158,19 @@ describe OAuthClients::ConnectionManager, type: :model do
           user_id: "admin"
         }.to_json
         stub_request(:any, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
-          .to_return(status: 200, body: response_body, headers: { "content-type" => "application/json; charset=utf-8" })
+          .to_return(status: 200, body: response_body)
       end
 
-      it 'returns a valid ClientToken object' do
+      it 'returns a valid ClientToken object', webmock: true do
         expect(subject.success).to be_truthy
         expect(subject.result).to be_a OAuthClientToken
       end
     end
 
-    context 'with known error' do
+    context 'with known error', webmock: true do
       before do
         stub_request(:post, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
-          .to_return(status: 400, body: { error: error_message }.to_json, headers: { "content-type" => "application/json; charset=utf-8" })
+          .to_return(status: 400, body: { error: error_message }.to_json)
       end
 
       shared_examples 'OAuth2 error response' do
@@ -195,10 +195,10 @@ describe OAuthClients::ConnectionManager, type: :model do
       end
     end
 
-    context 'with known reply invalid_grant' do
+    context 'with known reply invalid_grant', webmock: true do
       before do
         stub_request(:post, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
-          .to_return(status: 400, body: { error: "invalid_grant" }.to_json, headers: { "content-type" => "application/json; charset=utf-8" })
+          .to_return(status: 400, body: { error: "invalid_grant" }.to_json)
       end
 
       it 'returns a specific error message' do
@@ -209,10 +209,10 @@ describe OAuthClients::ConnectionManager, type: :model do
       end
     end
 
-    context 'with unknown reply' do
+    context 'with unknown reply', webmock: true do
       before do
         stub_request(:post, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
-          .to_return(status: 400, body: { error: "invalid_requesttt" }.to_json, headers: { "content-type" => "application/json; charset=utf-8" })
+          .to_return(status: 400, body: { error: "invalid_requesttt" }.to_json)
       end
 
       it 'returns an unspecific error message' do
@@ -223,7 +223,7 @@ describe OAuthClients::ConnectionManager, type: :model do
       end
     end
 
-    context 'with reply including JSON syntax error' do
+    context 'with reply including JSON syntax error', webmock: true do
       before do
         stub_request(:post, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
           .to_return(
@@ -235,13 +235,13 @@ describe OAuthClients::ConnectionManager, type: :model do
 
       it 'returns an unspecific error message' do
         expect(subject.success).to be_falsey
-        expect(subject.result).to eq "unexpected token at 'some: very, invalid> <json}'"
+        expect(subject.result).to eq 'Unknown :: some: very, invalid> <json}'
         expect(subject.errors[:base].count).to be(1)
-        expect(subject.errors[:base].first).to include I18n.t('oauth_client.errors.oauth_returned_http_error')
+        expect(subject.errors[:base].first).to include I18n.t('oauth_client.errors.oauth_returned_error')
       end
     end
 
-    context 'with 500 reply without body' do
+    context 'with 500 reply without body', webmock: true do
       before do
         stub_request(:post, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
           .to_return(status: 500)
@@ -255,42 +255,29 @@ describe OAuthClients::ConnectionManager, type: :model do
       end
     end
 
-    context 'when something is wrong with connection' do
+    context 'with bad HTTP response', webmock: true do
       before do
-        stub_request(:post, File.join(host, '/index.php/apps/oauth2/api/v1/token')).to_raise(Faraday::ConnectionFailed)
+        stub_request(:post, File.join(host, '/index.php/apps/oauth2/api/v1/token')).to_raise(Net::HTTPBadResponse)
       end
 
       it 'returns an unspecific error message' do
         expect(subject.success).to be_falsey
-        expect(subject.result).to eq("Exception from WebMock")
+        expect(subject.result).to be_nil
         expect(subject.errors[:base].count).to be(1)
         expect(subject.errors[:base].first).to include I18n.t('oauth_client.errors.oauth_returned_http_error')
       end
     end
 
-    context 'when something is wrong with SSL' do
-      before do
-        stub_request(:post, File.join(host, '/index.php/apps/oauth2/api/v1/token')).to_raise(Faraday::SSLError)
-      end
-
-      it 'returns an unspecific error message' do
-        expect(subject.success).to be_falsey
-        expect(subject.result).to eq("Exception from WebMock")
-        expect(subject.errors[:base].count).to be(1)
-        expect(subject.errors[:base].first).to include I18n.t('oauth_client.errors.oauth_returned_http_error')
-      end
-    end
-
-    context 'with timeout returns internal error' do
+    context 'with timeout returns internal error', webmock: true do
       before do
         stub_request(:post, File.join(host, '/index.php/apps/oauth2/api/v1/token')).to_timeout
       end
 
       it 'returns an unspecific error message' do
         expect(subject.success).to be_falsey
-        expect(subject.result).to eq("execution expired")
+        expect(subject.result).to be_nil
         expect(subject.errors[:base].count).to be(1)
-        expect(subject.errors[:base].first).to include I18n.t('oauth_client.errors.oauth_returned_http_error')
+        expect(subject.errors[:base].first).to include I18n.t('oauth_client.errors.oauth_returned_standard_error')
       end
     end
   end
@@ -325,7 +312,7 @@ describe OAuthClients::ConnectionManager, type: :model do
               user_id: "admin"
             }.to_json
             stub_request(:any, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
-              .to_return(status: 200, body: response_body, headers: { "content-type" => "application/json; charset=utf-8" })
+              .to_return(status: 200, body: response_body)
           end
 
           it 'returns a valid ClientToken object', webmock: true do
@@ -348,7 +335,7 @@ describe OAuthClients::ConnectionManager, type: :model do
               user_id: "admin"
             }.to_json
             stub_request(:any, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
-              .to_return(status: 200, body: response_body, headers: { "content-type" => "application/json; charset=utf-8" })
+              .to_return(status: 200, body: response_body)
           end
 
           it 'returns dependent error from model validation', webmock: true do
@@ -362,7 +349,7 @@ describe OAuthClients::ConnectionManager, type: :model do
         context 'with server error from OAuth2 provider' do
           before do
             stub_request(:any, File.join(host, '/index.php/apps/oauth2/api/v1/token'))
-              .to_return(status: 400, body: { error: "invalid_request" }.to_json, headers: { "content-type" => "application/json; charset=utf-8" })
+              .to_return(status: 400, body: { error: "invalid_request" }.to_json)
           end
 
           it 'returns a server error', webmock: true do
@@ -381,7 +368,7 @@ describe OAuthClients::ConnectionManager, type: :model do
 
           it 'returns a valid ClientToken object', webmock: true do
             expect(subject.success).to be_falsey
-            expect(subject.result).to eq("execution expired")
+            expect(subject.result).to be_nil
             expect(subject.errors.size).to be(1)
           end
         end
@@ -409,8 +396,8 @@ describe OAuthClients::ConnectionManager, type: :model do
             response_body2[:access_token] = "differ...RYvRH"
             request_url = File.join(host, '/index.php/apps/oauth2/api/v1/token')
             stub_request(:any, request_url).to_return(
-              { status: 200, body: response_body1.to_json, headers: { "content-type" => "application/json; charset=utf-8" } },
-              { status: 200, body: response_body2.to_json, headers: { "content-type" => "application/json; charset=utf-8" } }
+              { status: 200, body: response_body1.to_json },
+              { status: 200, body: response_body2.to_json }
             )
 
             result1 = nil
@@ -446,8 +433,8 @@ describe OAuthClients::ConnectionManager, type: :model do
             response_body2[:access_token] = "differ...RYvRH"
             request_url = File.join(host, '/index.php/apps/oauth2/api/v1/token')
             stub_request(:any, request_url)
-              .to_return(status: 200, body: response_body1.to_json, headers: { "content-type" => "application/json; charset=utf-8" }).then
-              .to_return(status: 200, body: response_body2.to_json, headers: { "content-type" => "application/json; charset=utf-8" })
+              .to_return(status: 200, body: response_body1.to_json).then
+              .to_return(status: 200, body: response_body2.to_json)
 
             result1 = nil
             result2 = nil