diff --git a/app/controllers/unknown_form_submitted_controller.rb b/app/controllers/unknown_form_submitted_controller.rb new file mode 100644 index 000000000..d087f1542 --- /dev/null +++ b/app/controllers/unknown_form_submitted_controller.rb @@ -0,0 +1,4 @@ +class UnknownFormSubmittedController < ApplicationController + # This action is only visited when we've lost the user's session after they've been logged out from One Login + def show; end +end diff --git a/app/controllers/users/omniauth_controller.rb b/app/controllers/users/omniauth_controller.rb index b5df68411..37b4ebd45 100644 --- a/app/controllers/users/omniauth_controller.rb +++ b/app/controllers/users/omniauth_controller.rb @@ -1,22 +1,30 @@ module Users class OmniauthController < ApplicationController - class OmniAuthFailure < StandardError; end + class FailureError < StandardError; end + + rescue_from AuthService::DataMissingError, FailureError do |exception| + CurrentRequestLoggingAttributes.rescued_exception = [exception.class.name, exception.message] + Sentry.capture_exception(exception) + + link_url = copy_of_answers_path(**auth_service.form_path_params) + render "errors/auth_error", locals: { link_url: }, status: :bad_request + rescue Store::ReturnFromOneLoginStore::MissingReturnParamsError + render "errors/return_from_one_login_session_missing", status: :bad_request + end def callback auth_hash = request.env["omniauth.auth"] auth_service.store_auth_details(auth_hash) - path_params = auth_service.form_path_params - - redirect_to check_your_answers_path(**path_params) - rescue Store::ReturnFromOneLoginStore::MissingReturnParamsError - Rails.logger.warn("Missing return params in session for One Login callback") - redirect_to error_404_path + redirect_to check_your_answers_path(**auth_service.form_path_params) + rescue Store::ReturnFromOneLoginStore::MissingReturnParamsError => e + CurrentRequestLoggingAttributes.rescued_exception = [e.class.name, e.message] + render "errors/return_from_one_login_session_missing", status: :bad_request end def failure error = request.env["omniauth.error"] - raise OmniAuthFailure, error + raise FailureError, error end def logged_out @@ -24,6 +32,9 @@ def logged_out path_params = auth_service.form_path_params redirect_to form_submitted_path(**path_params) + rescue Store::ReturnFromOneLoginStore::MissingReturnParamsError => e + CurrentRequestLoggingAttributes.rescued_exception = [e.class.name, e.message] + redirect_to :unknown_form_submitted end end end diff --git a/app/lib/store/return_from_one_login_store.rb b/app/lib/store/return_from_one_login_store.rb index ef7345eb5..4fe219b32 100644 --- a/app/lib/store/return_from_one_login_store.rb +++ b/app/lib/store/return_from_one_login_store.rb @@ -1,6 +1,10 @@ module Store class ReturnFromOneLoginStore - class MissingReturnParamsError < StandardError; end + class MissingReturnParamsError < StandardError + def initialize(message = "Return from One Login parameters are missing from the session") + super(message) + end + end RETURN_FROM_ONE_LOGIN_KEY = "return_from_one_login".freeze LAST_FORM_ID_KEY = "last_form_id".freeze diff --git a/app/views/errors/auth_error.html.erb b/app/views/errors/auth_error.html.erb new file mode 100644 index 000000000..4dc6faa8d --- /dev/null +++ b/app/views/errors/auth_error.html.erb @@ -0,0 +1,9 @@ +<% set_page_title(t('errors.auth_error.title'), t("gov_uk_forms")) %> + +
+
+

<%= t('errors.auth_error.title') %>

+ + <%= t("errors.auth_error.body_html", link_url:) %> +
+
diff --git a/app/views/errors/return_from_one_login_session_missing.html.erb b/app/views/errors/return_from_one_login_session_missing.html.erb new file mode 100644 index 000000000..28004a767 --- /dev/null +++ b/app/views/errors/return_from_one_login_session_missing.html.erb @@ -0,0 +1,9 @@ +<% set_page_title(t('errors.return_from_one_login_session_missing.title'), t("gov_uk_forms")) %> + +
+
+

<%= t('errors.return_from_one_login_session_missing.title') %>

+ + <%= t("errors.return_from_one_login_session_missing.body_html") %> +
+
diff --git a/app/views/unknown_form_submitted/show.html.erb b/app/views/unknown_form_submitted/show.html.erb new file mode 100644 index 000000000..6c430862d --- /dev/null +++ b/app/views/unknown_form_submitted/show.html.erb @@ -0,0 +1,9 @@ +<% set_page_title(t('.title'), t("gov_uk_forms")) %> + +
+
+ <%= govuk_panel(title_text: t('.title'), classes: "govuk-!-margin-bottom-7") %> + +

<%= t('.email_sent') %>

+
+
diff --git a/config/locales/cy.yml b/config/locales/cy.yml index b2afbd2d5..a24534233 100644 --- a/config/locales/cy.yml +++ b/config/locales/cy.yml @@ -296,6 +296,12 @@ cy: user-research: User research error_summary_title: Mae problem errors: + auth_error: + body_html: | +

We could not confirm your email address using GOV.UK One Login.

+

You’ll need to go back to the form and submit your answers.

+

Back to form

+ title: Sorry, something has gone wrong goto_page_routing_error: cannot_have_goto_page_before_routing_page: body_html: | @@ -329,6 +335,11 @@ cy: body_2: Os hoffech chi gyflwyno’r ffurflen eto, mae angen ichi link_text: fynd yn ôl i’r dechrau title: Er lles eich diogelwch, rydyn ni wedi dileu’ch atebion + return_from_one_login_session_missing: + body_html: | +

We could not confirm your email address using GOV.UK One Login.

+

You’ll need to go back to the form and submit your answers - try clicking the ‘Back’ button in your browser.

+ title: Sorry, something has gone wrong submission_error: body: | Roedd yna broblem gyda’r gwasanaeth ac rydyn ni wedi methu cyflwyno’ch atebion. @@ -659,3 +670,7 @@ cy: link_text: "%{link} (agor mewn tab newydd)" online: Ar-lein phone: Ffôn + unknown_form_submitted: + show: + email_sent: We’ve sent you a confirmation email with a copy of your answers. + title: Your form has been submitted diff --git a/config/locales/en.yml b/config/locales/en.yml index a3517b3a3..672042f31 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -296,6 +296,12 @@ en: user-research: User research error_summary_title: There is a problem errors: + auth_error: + body_html: | +

We could not confirm your email address using GOV.UK One Login.

+

You’ll need to go back to the form and submit your answers.

+

Back to form

+ title: Sorry, something has gone wrong goto_page_routing_error: cannot_have_goto_page_before_routing_page: body_html: | @@ -329,6 +335,11 @@ en: body_2: If you want to submit the form again, you need to link_text: go back to the start title: For your security, we deleted your answers + return_from_one_login_session_missing: + body_html: | +

We could not confirm your email address using GOV.UK One Login.

+

You’ll need to go back to the form and submit your answers - try clicking the ‘Back’ button in your browser.

+ title: Sorry, something has gone wrong submission_error: body: | There was a problem with the service and we have been unable to submit your answers. @@ -659,3 +670,7 @@ en: link_text: "%{link} (opens in new tab)" online: Online phone: Phone + unknown_form_submitted: + show: + email_sent: We’ve sent you a confirmation email with a copy of your answers. + title: Your form has been submitted diff --git a/config/routes.rb b/config/routes.rb index da9ea490a..5e3311b98 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -110,6 +110,8 @@ end end + get "/submitted" => "unknown_form_submitted#show", as: :unknown_form_submitted + get "/maintenance" => "errors#maintenance", as: :maintenance_page get "/404", to: "errors#not_found", as: :error_404, via: :all get "/500", to: "errors#internal_server_error", as: :error_500, via: :all diff --git a/spec/requests/unknown_form_submitted_controller_spec.rb b/spec/requests/unknown_form_submitted_controller_spec.rb new file mode 100644 index 000000000..645ed4e36 --- /dev/null +++ b/spec/requests/unknown_form_submitted_controller_spec.rb @@ -0,0 +1,12 @@ +require "rails_helper" + +RSpec.describe UnknownFormSubmittedController, type: :request do + describe "GET #show" do + it "renders the show template" do + get unknown_form_submitted_path + + expect(response).to have_http_status(:ok) + expect(response).to render_template(:show) + end + end +end diff --git a/spec/requests/users/omniauth_controller_spec.rb b/spec/requests/users/omniauth_controller_spec.rb index a52b3b12b..808307d5d 100644 --- a/spec/requests/users/omniauth_controller_spec.rb +++ b/spec/requests/users/omniauth_controller_spec.rb @@ -13,8 +13,15 @@ "last_locale" => locale, } end + let(:store) { {}.with_indifferent_access } - describe "GET #callback" do + before do + allow(AuthService).to receive(:new).and_wrap_original do |original_method, *_args| + original_method.call(store) + end + end + + describe "GET #callback", :capture_logging do let(:store) do { "return_from_one_login" => return_from_one_login_session, @@ -39,17 +46,12 @@ before do OmniAuth.config.test_mode = true OmniAuth.config.mock_auth[:default] = auth_hash + allow(Sentry).to receive(:capture_exception) - allow(AuthService).to receive(:new).and_wrap_original do |original_method, *_args| - original_method.call(store) - end + get omniauth_callback_path end context "when the auth details are present on the request and the user has a valid session" do - before do - get omniauth_callback_path - end - it "redirects to the check your answers page" do expect(response).to redirect_to(check_your_answers_path(form_id:, form_slug:, mode:, locale:)) end @@ -66,36 +68,95 @@ context "when data is missing on the auth details on the request" do let(:auth_hash) { {} } - it "raises an OmniAuthLoggedInDataMissingError" do - expect { get omniauth_callback_path }.to raise_error(AuthService::DataMissingError) + it "renders the auth error page" do + expect(response).to have_http_status(400) + expect(response).to render_template("errors/auth_error") + expect(response.body).to include("href=\"#{copy_of_answers_path(mode:, form_id:, form_slug:, locale:)}\"") + end + + it "logs the error" do + expect(log_lines.last["rescued_exception"]).to eq(["AuthService::DataMissingError", "Auth hash is missing on request"]) + end + + it "sends the error to Sentry" do + expect(Sentry).to have_received(:capture_exception).with( + an_instance_of(AuthService::DataMissingError), + ) end end context "when the return from one login params are not present on the session" do let(:store) { {} } - it "redirects to the 404 error page" do - get omniauth_callback_path - expect(response).to redirect_to(error_404_path) + it "renders the return from one login session missing page" do + expect(response).to have_http_status(400) + expect(response).to render_template("errors/return_from_one_login_session_missing") + end + + it "logs the error" do + expect(log_lines.last["rescued_exception"]).to eq(["Store::ReturnFromOneLoginStore::MissingReturnParamsError", "Return from One Login parameters are missing from the session"]) end end end - describe "GET #failure" do + describe "GET #failure", :capture_logging do let(:error_message) { "an error message" } - it "raises a OmniAuthFailure error" do - expect { get omniauth_failure_path, env: { "omniauth.error" => error_message } }.to raise_error(Users::OmniauthController::OmniAuthFailure, error_message) + before do + allow(Sentry).to receive(:capture_exception) + get omniauth_failure_path, env: { "omniauth.error" => error_message } + end + + context "when the return from one login params are present on the session" do + let(:store) do + { + "return_from_one_login" => return_from_one_login_session, + }.with_indifferent_access + end + + it "renders the auth error page" do + expect(response).to have_http_status(400) + expect(response).to render_template("errors/auth_error") + expect(response.body).to include("href=\"#{copy_of_answers_path(mode:, form_id:, form_slug:, locale:)}\"") + end + + it "logs the error" do + expect(log_lines.last["rescued_exception"]).to eq(["Users::OmniauthController::FailureError", error_message]) + end + + it "sends the error to Sentry" do + expect(Sentry).to have_received(:capture_exception).with( + an_instance_of(Users::OmniauthController::FailureError), + ) + end + end + + context "when the return from one login params are not present on the session" do + it "renders the return from one login session missing page" do + expect(response).to have_http_status(400) + expect(response).to render_template("errors/return_from_one_login_session_missing") + end + + it "logs the error" do + expect(log_lines.last["rescued_exception"]).to eq(["Users::OmniauthController::FailureError", error_message]) + end + + it "sends the error to Sentry" do + expect(Sentry).to have_received(:capture_exception).with( + an_instance_of(Users::OmniauthController::FailureError), + ) + end end end - describe "GET #logged_out" do + describe "GET #logged_out", :capture_logging do let(:token) { Faker::Alphanumeric.alphanumeric } before do allow(AuthService).to receive(:new).and_wrap_original do |original_method, *_args| original_method.call(store) end + get omniauth_logged_out_path end context "when the return from one login params are set on the session" do @@ -106,10 +167,6 @@ }.with_indifferent_access end - before do - get omniauth_logged_out_path - end - it "clears the auth details on the session" do expect(store).not_to have_key("auth") end @@ -122,8 +179,12 @@ context "when the return from one login params are not set on the session" do let(:store) { {} } - it "raises a Store::ReturnFromOneLoginStore::MissingReturnParamsError error" do - expect { get omniauth_logged_out_path }.to raise_error Store::ReturnFromOneLoginStore::MissingReturnParamsError + it "redirects to the unknown form submitted page" do + expect(response).to redirect_to(:unknown_form_submitted) + end + + it "logs the error" do + expect(log_lines.last["rescued_exception"]).to eq(["Store::ReturnFromOneLoginStore::MissingReturnParamsError", "Return from One Login parameters are missing from the session"]) end end end