From 09a3f616d43b18afed441ca47d9f8463a0064fe2 Mon Sep 17 00:00:00 2001 From: Karol Date: Tue, 28 Jan 2025 23:01:21 +0100 Subject: [PATCH 1/3] 1680: Improve error when exporting through an account_link --- app/services/exercise_service/push_external.rb | 6 +++++- config/locales/de/exercise.yml | 1 + config/locales/en/exercise.yml | 1 + spec/services/exercise_service/push_external_spec.rb | 7 +++++++ 4 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/services/exercise_service/push_external.rb b/app/services/exercise_service/push_external.rb index d8ed09fbf..89fb59878 100644 --- a/app/services/exercise_service/push_external.rb +++ b/app/services/exercise_service/push_external.rb @@ -18,7 +18,11 @@ def execute request.body = body end - response.success? ? nil : response.body + if response.success? + nil + else + response.status == 401 ? I18n.t('exercises.export_codeharbor.not_authorized') : response.body + end rescue StandardError => e e.message end diff --git a/config/locales/de/exercise.yml b/config/locales/de/exercise.yml index 97b144bfb..d7dd93b49 100644 --- a/config/locales/de/exercise.yml +++ b/config/locales/de/exercise.yml @@ -102,6 +102,7 @@ de: error: Es ist ein Fehler bei der Kommunikation mit CodeHarbor aufgetreten. export_failed: 'Export ist fehlgeschlagen.
ID: %{id}
Title: %{title}

Error: %{error}' label: Zu CodeHarbor exportieren + not_authorized: Die Autorisierung mit CodeHarbor konnte nicht hergestellt werden. Ist der API-Schlüssel korrekt? successfully_exported: 'Aufgabe wurde erfolgreich exportiert.
ID: %{id}
Title: %{title}' external_users: statistics: diff --git a/config/locales/en/exercise.yml b/config/locales/en/exercise.yml index 3f9e04b15..29f8ccce4 100644 --- a/config/locales/en/exercise.yml +++ b/config/locales/en/exercise.yml @@ -102,6 +102,7 @@ en: error: An error occurred while contacting CodeHarbor export_failed: 'Export has failed.
ID: %{id}
Title: %{title}

Error: %{error}' label: Export to CodeHarbor + not_authorized: Authorization with could not be established with CodeHarbor. Is the API Key correct? successfully_exported: 'Exercise has been successfully exported.
ID: %{id}
Title: %{title}' external_users: statistics: diff --git a/spec/services/exercise_service/push_external_spec.rb b/spec/services/exercise_service/push_external_spec.rb index 24e428b50..98b21812d 100644 --- a/spec/services/exercise_service/push_external_spec.rb +++ b/spec/services/exercise_service/push_external_spec.rb @@ -53,6 +53,13 @@ it { is_expected.to be response } end + + context 'when response status is 401' do + let(:status) { 401 } + let(:response) { I18n.t('exercises.export_codeharbor.not_authorized') } + + it { is_expected.to eql response } + end end context 'when an error occurs' do From 6aa73e1d301d0bc2f4ca56313efcc31d71961b5c Mon Sep 17 00:00:00 2001 From: Karol Date: Tue, 4 Feb 2025 21:57:54 +0100 Subject: [PATCH 2/3] 1680: add sanitation of error-message from codeharbor --- .../exercise_service/push_external.rb | 19 ++++++---- config/locales/de/exercise.yml | 1 + config/locales/en/exercise.yml | 1 + .../exercise_service/push_external_spec.rb | 36 ++++++++++++++++--- 4 files changed, 46 insertions(+), 11 deletions(-) diff --git a/app/services/exercise_service/push_external.rb b/app/services/exercise_service/push_external.rb index 89fb59878..a40283974 100644 --- a/app/services/exercise_service/push_external.rb +++ b/app/services/exercise_service/push_external.rb @@ -17,15 +17,22 @@ def execute request.headers['Authorization'] = "Bearer #{@codeharbor_link.api_key}" request.body = body end + return nil if response.success? + return I18n.t('exercises.export_codeharbor.not_authorized') if response.status == 401 - if response.success? - nil - else - response.status == 401 ? I18n.t('exercises.export_codeharbor.not_authorized') : response.body - end + handle_error(message: response.body) + rescue Faraday::ServerError => e + handle_error(error: e, message: I18n.t('exercises.export_codeharbor.server_error')) rescue StandardError => e - e.message + handle_error(error: e) end end + + private + + def handle_error(message: nil, error: nil) + Sentry.capture_exception(error) if error.present? + ERB::Util.html_escape(message || error.to_s) + end end end diff --git a/config/locales/de/exercise.yml b/config/locales/de/exercise.yml index d7dd93b49..7e9070ce4 100644 --- a/config/locales/de/exercise.yml +++ b/config/locales/de/exercise.yml @@ -103,6 +103,7 @@ de: export_failed: 'Export ist fehlgeschlagen.
ID: %{id}
Title: %{title}

Error: %{error}' label: Zu CodeHarbor exportieren not_authorized: Die Autorisierung mit CodeHarbor konnte nicht hergestellt werden. Ist der API-Schlüssel korrekt? + server_error: Verbindung zu CodeHarbor fehlgeschlagen. Gegenseite nicht erreichbar. successfully_exported: 'Aufgabe wurde erfolgreich exportiert.
ID: %{id}
Title: %{title}' external_users: statistics: diff --git a/config/locales/en/exercise.yml b/config/locales/en/exercise.yml index 29f8ccce4..704acc4c9 100644 --- a/config/locales/en/exercise.yml +++ b/config/locales/en/exercise.yml @@ -103,6 +103,7 @@ en: export_failed: 'Export has failed.
ID: %{id}
Title: %{title}

Error: %{error}' label: Export to CodeHarbor not_authorized: Authorization with could not be established with CodeHarbor. Is the API Key correct? + server_error: Connection to CodeHarbor failed. Remote host unreachable. successfully_exported: 'Exercise has been successfully exported.
ID: %{id}
Title: %{title}' external_users: statistics: diff --git a/spec/services/exercise_service/push_external_spec.rb b/spec/services/exercise_service/push_external_spec.rb index 98b21812d..3356b3e7c 100644 --- a/spec/services/exercise_service/push_external_spec.rb +++ b/spec/services/exercise_service/push_external_spec.rb @@ -26,7 +26,11 @@ let(:status) { 200 } let(:response) { '' } - before { stub_request(:post, codeharbor_link.push_url).to_return(status:, body: response) } + before do + # Un-memoize the connection to force a reconnection for each example + described_class.instance_variable_set(:@connection, nil) + stub_request(:post, codeharbor_link.push_url).to_return(status:, body: response) + end it 'calls the correct url' do expect(push_external).to have_requested(:post, codeharbor_link.push_url) @@ -49,9 +53,33 @@ context 'when response status is 500' do let(:status) { 500 } - let(:response) { 'an error occured' } + let(:response) { 'an error occurred' } + + it { is_expected.to eql response } + + context 'when response contains problematic characters' do + let(:response) { 'an occurred' } + + it { is_expected.to eql 'an <error> occurred' } + end + + context 'when faraday throws an error' do + let(:connection) { instance_double(Faraday::Connection) } + let(:error) { Faraday::ServerError } + + before do + allow(Faraday).to receive(:new).and_return(connection) + allow(connection).to receive(:post).and_raise(error) + end + + it { is_expected.to eql I18n.t('exercises.export_codeharbor.server_error') } + + context 'when another error occurs' do + let(:error) { 'another error' } - it { is_expected.to be response } + it { is_expected.to eql 'another error' } + end + end end context 'when response status is 401' do @@ -64,8 +92,6 @@ context 'when an error occurs' do before do - # Un-memoize the connection to force a reconnection - described_class.instance_variable_set(:@connection, nil) allow(Faraday).to receive(:new).and_raise(StandardError) end From fc7802a659cbbb574fb21120c135344c3f29aedf Mon Sep 17 00:00:00 2001 From: karol Date: Tue, 27 May 2025 22:22:19 +0200 Subject: [PATCH 3/3] 1680: add generic error message to avoid showing sensitive information to the user --- app/services/exercise_service/push_external.rb | 6 +++--- config/locales/de/exercise.yml | 1 + config/locales/en/exercise.yml | 1 + spec/services/exercise_service/push_external_spec.rb | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/app/services/exercise_service/push_external.rb b/app/services/exercise_service/push_external.rb index a40283974..61f48a218 100644 --- a/app/services/exercise_service/push_external.rb +++ b/app/services/exercise_service/push_external.rb @@ -24,15 +24,15 @@ def execute rescue Faraday::ServerError => e handle_error(error: e, message: I18n.t('exercises.export_codeharbor.server_error')) rescue StandardError => e - handle_error(error: e) + handle_error(error: e, message: I18n.t('exercises.export_codeharbor.generic_error')) end end private - def handle_error(message: nil, error: nil) + def handle_error(message:, error: nil) Sentry.capture_exception(error) if error.present? - ERB::Util.html_escape(message || error.to_s) + ERB::Util.html_escape(message) end end end diff --git a/config/locales/de/exercise.yml b/config/locales/de/exercise.yml index 7e9070ce4..5e7f06eb0 100644 --- a/config/locales/de/exercise.yml +++ b/config/locales/de/exercise.yml @@ -101,6 +101,7 @@ de: dialogtitle: Zu CodeHarbor exportieren error: Es ist ein Fehler bei der Kommunikation mit CodeHarbor aufgetreten. export_failed: 'Export ist fehlgeschlagen.
ID: %{id}
Title: %{title}

Error: %{error}' + generic_error: Ein unbekannter Fehler ist beim exportieren der Aufgabe aufgetreten. label: Zu CodeHarbor exportieren not_authorized: Die Autorisierung mit CodeHarbor konnte nicht hergestellt werden. Ist der API-Schlüssel korrekt? server_error: Verbindung zu CodeHarbor fehlgeschlagen. Gegenseite nicht erreichbar. diff --git a/config/locales/en/exercise.yml b/config/locales/en/exercise.yml index 704acc4c9..149529fad 100644 --- a/config/locales/en/exercise.yml +++ b/config/locales/en/exercise.yml @@ -101,6 +101,7 @@ en: dialogtitle: Export to CodeHarbor error: An error occurred while contacting CodeHarbor export_failed: 'Export has failed.
ID: %{id}
Title: %{title}

Error: %{error}' + generic_error: An unknown error has occurred while exporting the exercise. label: Export to CodeHarbor not_authorized: Authorization with could not be established with CodeHarbor. Is the API Key correct? server_error: Connection to CodeHarbor failed. Remote host unreachable. diff --git a/spec/services/exercise_service/push_external_spec.rb b/spec/services/exercise_service/push_external_spec.rb index 3356b3e7c..33071023a 100644 --- a/spec/services/exercise_service/push_external_spec.rb +++ b/spec/services/exercise_service/push_external_spec.rb @@ -77,7 +77,7 @@ context 'when another error occurs' do let(:error) { 'another error' } - it { is_expected.to eql 'another error' } + it { is_expected.to eql I18n.t('exercises.export_codeharbor.generic_error') } end end end