diff --git a/Gemfile.lock b/Gemfile.lock index 1612c252a7f..28cc41a9370 100644 --- a/Gemfile.lock +++ b/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) diff --git a/app/services/oauth_clients/connection_manager.rb b/app/services/oauth_clients/connection_manager.rb index e83171c7586..91c5df084d1 100644 --- a/app/services/oauth_clients/connection_manager.rb +++ b/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 diff --git a/modules/openid_connect/lib/open_project/openid_connect/engine.rb b/modules/openid_connect/lib/open_project/openid_connect/engine.rb index 50874fb18c4..9d390e7568c 100644 --- a/modules/openid_connect/lib/open_project/openid_connect/engine.rb +++ b/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? diff --git a/spec/services/oauth_clients/connection_manager_spec.rb b/spec/services/oauth_clients/connection_manager_spec.rb index 8e665d7c79e..0a7a10ccf36 100644 --- a/spec/services/oauth_clients/connection_manager_spec.rb +++ b/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> "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