From 41a244171b1256addf29afe0df2467b5d634dae0 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Wed, 29 Apr 2026 15:23:34 +0100 Subject: [PATCH 1/3] Render error page if there is an unexpected error authorising This page has a link to go back to the copy of your answers page so the user can try again if they wish. --- app/controllers/users/omniauth_controller.rb | 16 +++-- app/views/errors/auth_error.html.erb | 9 +++ config/locales/cy.yml | 6 ++ config/locales/en.yml | 6 ++ .../users/omniauth_controller_spec.rb | 64 +++++++++++++++---- 5 files changed, 82 insertions(+), 19 deletions(-) create mode 100644 app/views/errors/auth_error.html.erb diff --git a/app/controllers/users/omniauth_controller.rb b/app/controllers/users/omniauth_controller.rb index b5df68411..4eb25e135 100644 --- a/app/controllers/users/omniauth_controller.rb +++ b/app/controllers/users/omniauth_controller.rb @@ -1,14 +1,20 @@ 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 + 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) + redirect_to check_your_answers_path(**auth_service.form_path_params) rescue Store::ReturnFromOneLoginStore::MissingReturnParamsError Rails.logger.warn("Missing return params in session for One Login callback") redirect_to error_404_path @@ -16,7 +22,7 @@ def callback def failure error = request.env["omniauth.error"] - raise OmniAuthFailure, error + raise FailureError, error end def logged_out 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/config/locales/cy.yml b/config/locales/cy.yml index b2afbd2d5..2ce4cb7f4 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: | diff --git a/config/locales/en.yml b/config/locales/en.yml index a3517b3a3..21635e483 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: | diff --git a/spec/requests/users/omniauth_controller_spec.rb b/spec/requests/users/omniauth_controller_spec.rb index a52b3b12b..3c1ce5248 100644 --- a/spec/requests/users/omniauth_controller_spec.rb +++ b/spec/requests/users/omniauth_controller_spec.rb @@ -14,7 +14,13 @@ } end - 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 +45,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,8 +67,20 @@ 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 @@ -75,17 +88,40 @@ let(:store) { {} } it "redirects to the 404 error page" do - get omniauth_callback_path expect(response).to redirect_to(error_404_path) end end end - describe "GET #failure" do + describe "GET #failure", :capture_logging do let(:error_message) { "an error message" } + let(:store) do + { + "return_from_one_login" => return_from_one_login_session, + }.with_indifferent_access + end - 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 + 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 end From f513dd4faaa120e76e10887eae5f2af0f363b0d3 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Wed, 29 Apr 2026 16:23:38 +0100 Subject: [PATCH 2/3] Render error page when we don't know which form to return the user to Render a dedicated error page when we are returned from One Login and we don't know which form to return the user to because their session is missing. --- app/controllers/users/omniauth_controller.rb | 8 ++-- app/lib/store/return_from_one_login_store.rb | 6 ++- ...rn_from_one_login_session_missing.html.erb | 9 +++++ config/locales/cy.yml | 5 +++ config/locales/en.yml | 5 +++ .../users/omniauth_controller_spec.rb | 38 +++++++++++++++---- 6 files changed, 60 insertions(+), 11 deletions(-) create mode 100644 app/views/errors/return_from_one_login_session_missing.html.erb diff --git a/app/controllers/users/omniauth_controller.rb b/app/controllers/users/omniauth_controller.rb index 4eb25e135..7d8175ebb 100644 --- a/app/controllers/users/omniauth_controller.rb +++ b/app/controllers/users/omniauth_controller.rb @@ -8,6 +8,8 @@ class FailureError < StandardError; end 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 @@ -15,9 +17,9 @@ def callback auth_service.store_auth_details(auth_hash) redirect_to check_your_answers_path(**auth_service.form_path_params) - rescue Store::ReturnFromOneLoginStore::MissingReturnParamsError - Rails.logger.warn("Missing return params in session for One Login callback") - redirect_to error_404_path + 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 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/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/config/locales/cy.yml b/config/locales/cy.yml index 2ce4cb7f4..80eb3d637 100644 --- a/config/locales/cy.yml +++ b/config/locales/cy.yml @@ -335,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. diff --git a/config/locales/en.yml b/config/locales/en.yml index 21635e483..d548b3f8a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -335,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. diff --git a/spec/requests/users/omniauth_controller_spec.rb b/spec/requests/users/omniauth_controller_spec.rb index 3c1ce5248..63bdca928 100644 --- a/spec/requests/users/omniauth_controller_spec.rb +++ b/spec/requests/users/omniauth_controller_spec.rb @@ -13,6 +13,7 @@ "last_locale" => locale, } end + let(:store) { {}.with_indifferent_access } before do allow(AuthService).to receive(:new).and_wrap_original do |original_method, *_args| @@ -87,19 +88,19 @@ 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 - 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", :capture_logging do let(:error_message) { "an error message" } - let(:store) do - { - "return_from_one_login" => return_from_one_login_session, - }.with_indifferent_access - end before do allow(Sentry).to receive(:capture_exception) @@ -107,6 +108,12 @@ 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") @@ -123,6 +130,23 @@ ) 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 From 602e2f768fc0c7961af95929f1ccffbb1b633d16 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Wed, 29 Apr 2026 16:49:11 +0100 Subject: [PATCH 3/3] Handle the users session being missing after logout We log the user out from One Login if they've opted to receive a copy of their answers when they submit the form. This involves redirecting them to One Login, and then they are returned to a URL on our service which is not form specific. We store details of the form they submitted in the session so we know where to return them to. If somehow the user's session is lost when they're returned to us from One Login, we still want to tell the user that their form has been submitted, even if we don't know which form. Render a generic page and log an error in this case. --- .../unknown_form_submitted_controller.rb | 4 ++++ app/controllers/users/omniauth_controller.rb | 3 +++ app/views/unknown_form_submitted/show.html.erb | 9 +++++++++ config/locales/cy.yml | 4 ++++ config/locales/en.yml | 4 ++++ config/routes.rb | 2 ++ .../unknown_form_submitted_controller_spec.rb | 12 ++++++++++++ spec/requests/users/omniauth_controller_spec.rb | 15 ++++++++------- 8 files changed, 46 insertions(+), 7 deletions(-) create mode 100644 app/controllers/unknown_form_submitted_controller.rb create mode 100644 app/views/unknown_form_submitted/show.html.erb create mode 100644 spec/requests/unknown_form_submitted_controller_spec.rb 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 7d8175ebb..37b4ebd45 100644 --- a/app/controllers/users/omniauth_controller.rb +++ b/app/controllers/users/omniauth_controller.rb @@ -32,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/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 80eb3d637..a24534233 100644 --- a/config/locales/cy.yml +++ b/config/locales/cy.yml @@ -670,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 d548b3f8a..672042f31 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -670,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 63bdca928..808307d5d 100644 --- a/spec/requests/users/omniauth_controller_spec.rb +++ b/spec/requests/users/omniauth_controller_spec.rb @@ -149,13 +149,14 @@ 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 @@ -166,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 @@ -182,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