From 4f4fa5ef31f71ee7c41ea35cf391338a93977317 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Mon, 27 Apr 2026 15:43:07 +0100 Subject: [PATCH 1/4] Store auth token on session Store the auth token returned by One Login in the session so we can use it to log the user out when they submit their form. --- app/controllers/application_controller.rb | 4 +++ app/controllers/users/omniauth_controller.rb | 15 ++++++++--- app/lib/store/auth_store.rb | 24 +++++++++++++++++ spec/lib/store/auth_store_spec.rb | 26 +++++++++++++++++++ .../users/omniauth_controller_spec.rb | 10 ++++++- 5 files changed, 74 insertions(+), 5 deletions(-) create mode 100644 app/lib/store/auth_store.rb create mode 100644 spec/lib/store/auth_store_spec.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 8d62dc8eb..9bf4e4e25 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -118,4 +118,8 @@ def default_locale?(locale) false end + + def auth_store + @auth_store ||= Store::AuthStore.new(session) + end end diff --git a/app/controllers/users/omniauth_controller.rb b/app/controllers/users/omniauth_controller.rb index 0f9ff4eaa..c0150676a 100644 --- a/app/controllers/users/omniauth_controller.rb +++ b/app/controllers/users/omniauth_controller.rb @@ -1,6 +1,7 @@ module Users class OmniauthController < ApplicationController class OmniAuthLoggedInDataMissingError < StandardError; end + class OmniAuthFailure < StandardError; end rescue_from Store::ReturnFromOneLoginStore::MissingReturnParamsError do @@ -9,10 +10,16 @@ class OmniAuthFailure < StandardError; end end def callback - email = request.env.dig("omniauth.auth", "info", "email") - if email.blank? - raise OmniAuthLoggedInDataMissingError, "Email is missing in OmniAuth auth hash" - end + auth_hash = request.env["omniauth.auth"] + raise OmniAuthLoggedInDataMissingError, "Auth hash is missing on request" if auth_hash.blank? + + email = auth_hash.dig("info", "email") + raise OmniAuthLoggedInDataMissingError, "Email is missing in OmniAuth auth hash" if email.blank? + + token = auth_hash.dig("credentials", "token") + raise OmniAuthLoggedInDataMissingError, "Token is missing in OmniAuth auth hash" if token.blank? + + auth_store.store_token(token) return_from_one_login_store = Store::ReturnFromOneLoginStore.new(session) form_id = return_from_one_login_store.form_id diff --git a/app/lib/store/auth_store.rb b/app/lib/store/auth_store.rb new file mode 100644 index 000000000..cb4905d6c --- /dev/null +++ b/app/lib/store/auth_store.rb @@ -0,0 +1,24 @@ +module Store + class AuthStore + AUTH_KEY = "auth".freeze + TOKEN_KEY = "token".freeze + + def initialize(store) + @store = store + end + + def store_token(token) + @store[AUTH_KEY] = { + TOKEN_KEY => token, + } + end + + def get_token + @store.dig(AUTH_KEY, TOKEN_KEY) + end + + def logged_in? + get_token.present? + end + end +end diff --git a/spec/lib/store/auth_store_spec.rb b/spec/lib/store/auth_store_spec.rb new file mode 100644 index 000000000..eefeadb77 --- /dev/null +++ b/spec/lib/store/auth_store_spec.rb @@ -0,0 +1,26 @@ +require "rails_helper" + +RSpec.describe Store::AuthStore do + subject(:auth_store) { described_class.new(store) } + + let(:store) { {} } + let(:token) { Faker::Alphanumeric.alphanumeric } + + it "stores and returns the auth token" do + auth_store.store_token(token) + + expect(auth_store.get_token).to eq(token) + end + + describe "#logged_in" do + it "returns true when the auth token is stored" do + auth_store.store_token(token) + + expect(auth_store.logged_in?).to be true + end + + it "returns false when the auth token is not stored" do + expect(auth_store.logged_in?).to be false + end + end +end diff --git a/spec/requests/users/omniauth_controller_spec.rb b/spec/requests/users/omniauth_controller_spec.rb index ad0ceaabe..d6f705593 100644 --- a/spec/requests/users/omniauth_controller_spec.rb +++ b/spec/requests/users/omniauth_controller_spec.rb @@ -18,6 +18,7 @@ end let(:email) { "test@example.com" } + let(:token) { Faker::Alphanumeric.alphanumeric } let(:auth_hash) do { provider: :govuk_one_login, @@ -26,7 +27,7 @@ email:, }, credentials: { - id_token: "id_token", + token: token, }, }.with_indifferent_access end @@ -41,6 +42,9 @@ allow(Store::ConfirmationDetailsStore).to receive(:new).and_wrap_original do |original_method, *args| original_method.call(store, args[1]) end + allow(Store::AuthStore).to receive(:new).and_wrap_original do |original_method, *_args| + original_method.call(store) + end end context "when the auth details are present on the request and the user has a valid session" do @@ -55,6 +59,10 @@ it "stores the user's email address on the session" do expect(store.dig("confirmation_details", form_id.to_s, "copy_of_answers_email_address")).to eq email end + + it "stores the token on the session" do + expect(store["auth"]["token"]).to eq token + end end context "when the auth details are not present on the request" do From d7cba7ac8012c88214493d97c841fb47dd7908fd Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Tue, 28 Apr 2026 13:09:14 +0100 Subject: [PATCH 2/4] Log the user out of One Login when they submit their form If the user has an active session with One Login, redirect the user to the One Login logout URL when they submit their form. We provide a token and a post logout redirect URL to receive the user back after they've been logged out. Store the form path params on the session before redirecting. We will have already stored these before they logged in with One Login, but we set them again in case they were also filling out another form in a different browser tab. --- app/controllers/application_controller.rb | 4 ++ .../forms/check_your_answers_controller.rb | 12 +++- .../forms/continue_to_one_login_controller.rb | 2 +- app/services/auth_service.rb | 22 +++++++ config/initializers/omniauth.rb | 3 + config/routes.rb | 1 + config/settings/test.yml | 3 + .../check_your_answers_controller_spec.rb | 65 +++++++++++++++---- spec/services/auth_service_spec.rb | 30 +++++++++ 9 files changed, 127 insertions(+), 15 deletions(-) create mode 100644 app/services/auth_service.rb create mode 100644 spec/services/auth_service_spec.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 9bf4e4e25..116201a6d 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -122,4 +122,8 @@ def default_locale?(locale) def auth_store @auth_store ||= Store::AuthStore.new(session) end + + def return_from_one_login_store + @return_from_one_login_store ||= Store::ReturnFromOneLoginStore.new(session) + end end diff --git a/app/controllers/forms/check_your_answers_controller.rb b/app/controllers/forms/check_your_answers_controller.rb index 9ead6c982..2618898ed 100644 --- a/app/controllers/forms/check_your_answers_controller.rb +++ b/app/controllers/forms/check_your_answers_controller.rb @@ -43,7 +43,12 @@ def submit_answers current_context.save_submission_details(submission_reference, requested_email_confirmation) - redirect_to :form_submitted + if auth_store.logged_in? + return_from_one_login_store.store_return_params(form: current_context.form, mode: mode, locale: locale_param) + redirect_to logout_request.redirect_uri, allow_other_host: true + else + redirect_to :form_submitted + end rescue FormSubmissionService::ConfirmationEmailToAddressError setup_check_your_answers email_confirmation_input.errors.add(:confirmation_email_address, :invalid_email) @@ -82,5 +87,10 @@ def back_link end end end + + def logout_request + token = auth_store.get_token + AuthService.new.logout_request(token, omniauth_logged_out_url) + end end end diff --git a/app/controllers/forms/continue_to_one_login_controller.rb b/app/controllers/forms/continue_to_one_login_controller.rb index ae10ad8b7..697436331 100644 --- a/app/controllers/forms/continue_to_one_login_controller.rb +++ b/app/controllers/forms/continue_to_one_login_controller.rb @@ -3,7 +3,7 @@ class ContinueToOneLoginController < BaseController before_action :redirect_if_feature_disabled, :redirect_if_form_incomplete def show - Store::ReturnFromOneLoginStore.new(session).store_return_params( + return_from_one_login_store.store_return_params( form: current_context.form, mode: mode, locale: locale_param, diff --git a/app/services/auth_service.rb b/app/services/auth_service.rb new file mode 100644 index 000000000..8269af605 --- /dev/null +++ b/app/services/auth_service.rb @@ -0,0 +1,22 @@ +class AuthService + def logout_request(token, post_logout_redirect_uri) + logout_utility.build_request( + id_token_hint: token, + post_logout_redirect_uri: url_without_params(post_logout_redirect_uri), + ) + end + +private + + def logout_utility + @logout_utility ||= OmniAuth::GovukOneLogin::LogoutUtility.new( + idp_configuration: Rails.application.config.x.one_login.idp_configuration, + ) + end + + def url_without_params(url) + url = URI.parse(url) + url.query = nil + url.to_s + end +end diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 328e85e4a..0012cb9e2 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -28,3 +28,6 @@ # will call `Users::OmniauthController#failure` if there are any errors during the login process on_failure { |env| Users::OmniauthController.action(:failure).call(env) } end + +# Store this globally so we only make a request to the One Login discovery endpoint once as the configuration should not regularly change +Rails.application.config.x.one_login.idp_configuration = OmniAuth::GovukOneLogin::IdpConfiguration.new(idp_base_url: Settings.govuk_one_login.base_url) diff --git a/config/routes.rb b/config/routes.rb index e1b8372c6..da9ea490a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -3,6 +3,7 @@ get "/auth/govuk_one_login/callback", to: "users/omniauth#callback", as: :omniauth_callback get "/auth/failure", to: "users/omniauth#failure", as: :omniauth_failure + get "/auth/logged-out", to: "users/omniauth#logged_out", as: :omniauth_logged_out # Reveal health status on /up that returns 200 if the app boots with no exceptions, otherwise 500. # Can be used by load balancers and uptime monitors to verify that the app is live. diff --git a/config/settings/test.yml b/config/settings/test.yml index ba748c3b8..44317bd88 100644 --- a/config/settings/test.yml +++ b/config/settings/test.yml @@ -17,3 +17,6 @@ submission_status_api: file_upload: poll_scan_status_wait_milliseconds: 50 poll_scan_status_max_attempts: 2 + +govuk_one_login: + base_url: http://example.com/one-login-mock/ diff --git a/spec/requests/forms/check_your_answers_controller_spec.rb b/spec/requests/forms/check_your_answers_controller_spec.rb index 8a8ce079b..2403ad404 100644 --- a/spec/requests/forms/check_your_answers_controller_spec.rb +++ b/spec/requests/forms/check_your_answers_controller_spec.rb @@ -29,24 +29,23 @@ let(:submission_email) { Faker::Internet.email(domain: "example.gov.uk") } - let(:store) do + let(:answers) do { - answers: { - form_id.to_s => { - "1" => { - "date_year" => "2000", - "date_month" => "1", - "date_day" => "1", - }, - "2" => { - "date_year" => "2023", - "date_month" => "6", - "date_day" => "9", - }, + form_id.to_s => { + "1" => { + "date_year" => "2000", + "date_month" => "1", + "date_day" => "1", + }, + "2" => { + "date_year" => "2023", + "date_month" => "6", + "date_day" => "9", }, }, } end + let(:store) { { answers: }.with_indifferent_access } let(:steps_data) do [ @@ -93,6 +92,9 @@ allow(context_spy).to receive(:form_submitted?).and_return(repeat_form_submission) context_spy end + allow(Store::AuthStore).to receive(:new).and_wrap_original do |original_method, *_args| + original_method.call(store) + end allow(ReferenceNumberService).to receive(:generate).and_return(reference) allow(FeatureService).to receive(:enabled?).with("filler_answer_email_enabled").and_return(true) @@ -314,6 +316,43 @@ end end + context "when the user has logged in with One Login" do + let(:token) { Faker::Alphanumeric.alphanumeric } + let(:store) do + { + answers:, + auth: { token: }, + }.with_indifferent_access + end + let(:end_session_endpoint) { "http://example.com/one-login-mock/logout" } + + before do + allow(Store::ReturnFromOneLoginStore).to receive(:new).and_wrap_original do |original_method, *_args| + original_method.call(store) + end + + idp_configuration = instance_double(OmniAuth::GovukOneLogin::IdpConfiguration, end_session_endpoint:) + allow(Rails.application.config.x).to receive(:one_login).and_return(double(idp_configuration:)) + + post form_submit_answers_path(form_id:, form_slug: "form-name", mode:), params: { email_confirmation_input: } + end + + it "saves the path params for returning from One Login on the session" do + expect(store).to have_key "return_from_one_login" + expect(store["return_from_one_login"]).to eq({ + "last_form_id" => form_data.form_id, + "last_form_slug" => form_data.form_slug, + "last_mode" => mode.to_s, + "last_locale" => nil, + }) + end + + it "redirects to the One Login logout page" do + post_logout_redirect_url = CGI.escape("http://www.example.com/auth/logged-out") + expect(response).to redirect_to(/#{end_session_endpoint}\?id_token_hint=#{token}&post_logout_redirect_uri=#{post_logout_redirect_url}/) + end + end + context "when answers have already been submitted" do let(:repeat_form_submission) { true } diff --git a/spec/services/auth_service_spec.rb b/spec/services/auth_service_spec.rb new file mode 100644 index 000000000..6080d3862 --- /dev/null +++ b/spec/services/auth_service_spec.rb @@ -0,0 +1,30 @@ +require "rails_helper" + +RSpec.describe AuthService do + subject(:auth_service) { described_class.new } + + let(:end_session_endpoint) { "http://example.com/one-login-mock/logout" } + + before do + idp_configuration = instance_double(OmniAuth::GovukOneLogin::IdpConfiguration, end_session_endpoint:) + allow(Rails.application.config.x).to receive(:one_login).and_return(double(idp_configuration:)) + end + + describe "#logout_request" do + let(:token) { Faker::Alphanumeric.alphanumeric } + let(:post_logout_redirect_uri) { "https://example.com/some-path?with=params" } + + it "returns a logout request with the expected One Login URI" do + logout_request = auth_service.logout_request(token, post_logout_redirect_uri) + expect(logout_request.redirect_uri).to start_with(end_session_endpoint) + end + + it "strips query params from the post_logout_redirect_uri" do + logout_request = auth_service.logout_request(token, post_logout_redirect_uri) + redirect_uri = URI.parse(logout_request.redirect_uri) + query = Rack::Utils.parse_query(redirect_uri.query) + expect(query).to include("post_logout_redirect_uri" => "https://example.com/some-path", + "id_token_hint" => token) + end + end +end From e9acc65e96b86c341b0be75f61af4da12915e1c6 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Tue, 28 Apr 2026 13:09:33 +0100 Subject: [PATCH 3/4] Handle post logout callback from One Login When One Login redirects the user back to us after logging out, retrieve the form path parameters from the session to redirect to the submitted page. If the user's session cookie has been lost and we're unable to determine the form path parameters, let an exception bubble up and show an internal error page for now. We'll probably update this to show something more user friendly in a future commit. --- app/controllers/users/omniauth_controller.rb | 18 +++-- app/lib/store/auth_store.rb | 4 ++ .../users/omniauth_controller_spec.rb | 72 +++++++++++++++---- 3 files changed, 74 insertions(+), 20 deletions(-) diff --git a/app/controllers/users/omniauth_controller.rb b/app/controllers/users/omniauth_controller.rb index c0150676a..ba4b18651 100644 --- a/app/controllers/users/omniauth_controller.rb +++ b/app/controllers/users/omniauth_controller.rb @@ -4,11 +4,6 @@ class OmniAuthLoggedInDataMissingError < StandardError; end class OmniAuthFailure < StandardError; end - rescue_from Store::ReturnFromOneLoginStore::MissingReturnParamsError do - Rails.logger.warn("Missing return params in session for One Login callback") - redirect_to error_404_path - end - def callback auth_hash = request.env["omniauth.auth"] raise OmniAuthLoggedInDataMissingError, "Auth hash is missing on request" if auth_hash.blank? @@ -16,23 +11,32 @@ def callback email = auth_hash.dig("info", "email") raise OmniAuthLoggedInDataMissingError, "Email is missing in OmniAuth auth hash" if email.blank? - token = auth_hash.dig("credentials", "token") + token = auth_hash.dig("credentials", "id_token") raise OmniAuthLoggedInDataMissingError, "Token is missing in OmniAuth auth hash" if token.blank? auth_store.store_token(token) - return_from_one_login_store = Store::ReturnFromOneLoginStore.new(session) form_id = return_from_one_login_store.form_id path_params = return_from_one_login_store.get_path_params Store::ConfirmationDetailsStore.new(session, form_id).save_copy_of_answers_email_address(email) 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 end def failure error = request.env["omniauth.error"] raise OmniAuthFailure, error end + + def logged_out + auth_store.clear + + path_params = return_from_one_login_store.get_path_params + redirect_to form_submitted_path(**path_params) + end end end diff --git a/app/lib/store/auth_store.rb b/app/lib/store/auth_store.rb index cb4905d6c..a1c156acb 100644 --- a/app/lib/store/auth_store.rb +++ b/app/lib/store/auth_store.rb @@ -17,6 +17,10 @@ def get_token @store.dig(AUTH_KEY, TOKEN_KEY) end + def clear + @store.delete(AUTH_KEY) + end + def logged_in? get_token.present? end diff --git a/spec/requests/users/omniauth_controller_spec.rb b/spec/requests/users/omniauth_controller_spec.rb index d6f705593..fecc459da 100644 --- a/spec/requests/users/omniauth_controller_spec.rb +++ b/spec/requests/users/omniauth_controller_spec.rb @@ -1,24 +1,28 @@ require "rails_helper" RSpec.describe Users::OmniauthController, type: :request do + let(:form_id) { 42 } + let(:form_slug) { "test-form" } + let(:mode) { "preview-draft" } + let(:locale) { "cy" } + let(:return_from_one_login_session) do + { + "last_form_id" => form_id, + "last_form_slug" => form_slug, + "last_mode" => mode, + "last_locale" => locale, + } + end + describe "GET #callback" do - let(:form_id) { 42 } - let(:form_slug) { "test-form" } - let(:mode) { "preview-draft" } - let(:locale) { "cy" } let(:store) do { - "return_from_one_login" => { - "last_form_id" => form_id, - "last_form_slug" => form_slug, - "last_mode" => mode, - "last_locale" => locale, - }, + "return_from_one_login" => return_from_one_login_session, }.with_indifferent_access end let(:email) { "test@example.com" } - let(:token) { Faker::Alphanumeric.alphanumeric } + let(:id_token) { Faker::Alphanumeric.alphanumeric } let(:auth_hash) do { provider: :govuk_one_login, @@ -27,7 +31,7 @@ email:, }, credentials: { - token: token, + id_token:, }, }.with_indifferent_access end @@ -61,7 +65,7 @@ end it "stores the token on the session" do - expect(store["auth"]["token"]).to eq token + expect(store["auth"]["token"]).to eq id_token end end @@ -98,4 +102,46 @@ expect { get omniauth_failure_path, env: { "omniauth.error" => error_message } }.to raise_error(Users::OmniauthController::OmniAuthFailure, error_message) end end + + describe "GET #logged_out" do + let(:token) { Faker::Alphanumeric.alphanumeric } + + before do + allow(Store::ReturnFromOneLoginStore).to receive(:new).and_wrap_original do |original_method, *_args| + original_method.call(store) + end + allow(Store::AuthStore).to receive(:new).and_wrap_original do |original_method, *_args| + original_method.call(store) + end + end + + context "when the return from one login params are set on the session" do + let(:store) do + { + "return_from_one_login" => return_from_one_login_session, + "auth": { "token": token }, + }.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 + + it "redirects to the form submitted page" do + expect(response).to redirect_to(form_submitted_path(form_id:, form_slug:, mode:, locale:)) + end + end + + 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 + end + end + end end From cb1c4a1fe43df9949055481ab10152627b3552ad Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Wed, 29 Apr 2026 14:58:45 +0100 Subject: [PATCH 4/4] Wrap auth logic in AuthService This avoids having to keep track of the multiple store classes we now have in the controllers, and removes some logic from the controllers. Hopefully this makes it a bit easier to see what's involved in the auth process. --- app/controllers/application_controller.rb | 8 +- .../forms/check_your_answers_controller.rb | 11 +- .../forms/continue_to_one_login_controller.rb | 2 +- app/controllers/users/omniauth_controller.rb | 21 +--- app/lib/store/return_from_one_login_store.rb | 2 +- app/services/auth_service.rb | 38 ++++++- .../store/return_from_one_login_store_spec.rb | 4 +- .../check_your_answers_controller_spec.rb | 2 +- .../continue_to_one_login_controller_spec.rb | 2 +- .../users/omniauth_controller_spec.rb | 25 +---- spec/services/auth_service_spec.rb | 103 ++++++++++++++++-- 11 files changed, 150 insertions(+), 68 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 116201a6d..f5b1a55e4 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -119,11 +119,7 @@ def default_locale?(locale) false end - def auth_store - @auth_store ||= Store::AuthStore.new(session) - end - - def return_from_one_login_store - @return_from_one_login_store ||= Store::ReturnFromOneLoginStore.new(session) + def auth_service + @auth_service ||= AuthService.new(session) end end diff --git a/app/controllers/forms/check_your_answers_controller.rb b/app/controllers/forms/check_your_answers_controller.rb index 2618898ed..021f25693 100644 --- a/app/controllers/forms/check_your_answers_controller.rb +++ b/app/controllers/forms/check_your_answers_controller.rb @@ -43,9 +43,9 @@ def submit_answers current_context.save_submission_details(submission_reference, requested_email_confirmation) - if auth_store.logged_in? - return_from_one_login_store.store_return_params(form: current_context.form, mode: mode, locale: locale_param) - redirect_to logout_request.redirect_uri, allow_other_host: true + if auth_service.logged_in? + auth_service.store_return_params(form: current_context.form, mode: mode, locale: locale_param) + redirect_to auth_service.logout_redirect_uri(omniauth_logged_out_url), allow_other_host: true else redirect_to :form_submitted end @@ -87,10 +87,5 @@ def back_link end end end - - def logout_request - token = auth_store.get_token - AuthService.new.logout_request(token, omniauth_logged_out_url) - end end end diff --git a/app/controllers/forms/continue_to_one_login_controller.rb b/app/controllers/forms/continue_to_one_login_controller.rb index 697436331..17a443698 100644 --- a/app/controllers/forms/continue_to_one_login_controller.rb +++ b/app/controllers/forms/continue_to_one_login_controller.rb @@ -3,7 +3,7 @@ class ContinueToOneLoginController < BaseController before_action :redirect_if_feature_disabled, :redirect_if_form_incomplete def show - return_from_one_login_store.store_return_params( + auth_service.store_return_params( form: current_context.form, mode: mode, locale: locale_param, diff --git a/app/controllers/users/omniauth_controller.rb b/app/controllers/users/omniauth_controller.rb index ba4b18651..b5df68411 100644 --- a/app/controllers/users/omniauth_controller.rb +++ b/app/controllers/users/omniauth_controller.rb @@ -1,25 +1,12 @@ module Users class OmniauthController < ApplicationController - class OmniAuthLoggedInDataMissingError < StandardError; end - class OmniAuthFailure < StandardError; end def callback auth_hash = request.env["omniauth.auth"] - raise OmniAuthLoggedInDataMissingError, "Auth hash is missing on request" if auth_hash.blank? - - email = auth_hash.dig("info", "email") - raise OmniAuthLoggedInDataMissingError, "Email is missing in OmniAuth auth hash" if email.blank? - - token = auth_hash.dig("credentials", "id_token") - raise OmniAuthLoggedInDataMissingError, "Token is missing in OmniAuth auth hash" if token.blank? - - auth_store.store_token(token) - - form_id = return_from_one_login_store.form_id - path_params = return_from_one_login_store.get_path_params + auth_service.store_auth_details(auth_hash) - Store::ConfirmationDetailsStore.new(session, form_id).save_copy_of_answers_email_address(email) + path_params = auth_service.form_path_params redirect_to check_your_answers_path(**path_params) rescue Store::ReturnFromOneLoginStore::MissingReturnParamsError @@ -33,9 +20,9 @@ def failure end def logged_out - auth_store.clear + auth_service.clear_auth_session - path_params = return_from_one_login_store.get_path_params + path_params = auth_service.form_path_params redirect_to form_submitted_path(**path_params) 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 681b89adb..ef7345eb5 100644 --- a/app/lib/store/return_from_one_login_store.rb +++ b/app/lib/store/return_from_one_login_store.rb @@ -20,7 +20,7 @@ def store_return_params(form:, mode:, locale:) @store[RETURN_FROM_ONE_LOGIN_KEY][LAST_LOCALE_KEY] = locale end - def get_path_params + def form_path_params raise MissingReturnParamsError if @store[RETURN_FROM_ONE_LOGIN_KEY].blank? { diff --git a/app/services/auth_service.rb b/app/services/auth_service.rb index 8269af605..3d1d2eb70 100644 --- a/app/services/auth_service.rb +++ b/app/services/auth_service.rb @@ -1,9 +1,43 @@ class AuthService - def logout_request(token, post_logout_redirect_uri) - logout_utility.build_request( + class DataMissingError < StandardError; end + + attr_reader :auth_store, :return_from_one_login_store + + def initialize(store) + @store = store + @return_from_one_login_store = Store::ReturnFromOneLoginStore.new(store) + @auth_store = Store::AuthStore.new(store) + end + + delegate :logged_in?, to: :auth_store + delegate :store_return_params, :form_path_params, to: :return_from_one_login_store + + def store_auth_details(auth_hash) + form_id = @return_from_one_login_store.form_id + + raise DataMissingError, "Auth hash is missing on request" if auth_hash.blank? + + email = auth_hash.dig("info", "email") + raise DataMissingError, "Email is missing in OmniAuth auth hash" if email.blank? + + token = auth_hash.dig("credentials", "id_token") + raise DataMissingError, "Token is missing in OmniAuth auth hash" if token.blank? + + @auth_store.store_token(token) + Store::ConfirmationDetailsStore.new(@store, form_id).save_copy_of_answers_email_address(email) + end + + def logout_redirect_uri(post_logout_redirect_uri) + token = @auth_store.get_token + logout_request = logout_utility.build_request( id_token_hint: token, post_logout_redirect_uri: url_without_params(post_logout_redirect_uri), ) + logout_request.redirect_uri + end + + def clear_auth_session + @auth_store.clear end private diff --git a/spec/lib/store/return_from_one_login_store_spec.rb b/spec/lib/store/return_from_one_login_store_spec.rb index dc6f29da0..6c81c20a5 100644 --- a/spec/lib/store/return_from_one_login_store_spec.rb +++ b/spec/lib/store/return_from_one_login_store_spec.rb @@ -12,7 +12,7 @@ it "stores and return the path params" do return_from_one_login_store.store_return_params(form:, mode:, locale:) - expect(return_from_one_login_store.get_path_params).to eq({ + expect(return_from_one_login_store.form_path_params).to eq({ mode: mode.to_s, form_id: form.id, form_slug: form.form_slug, @@ -21,7 +21,7 @@ end it "raises an error if the return params have not been stored" do - expect { return_from_one_login_store.get_path_params }.to raise_error(Store::ReturnFromOneLoginStore::MissingReturnParamsError) + expect { return_from_one_login_store.form_path_params }.to raise_error(Store::ReturnFromOneLoginStore::MissingReturnParamsError) end end diff --git a/spec/requests/forms/check_your_answers_controller_spec.rb b/spec/requests/forms/check_your_answers_controller_spec.rb index 2403ad404..868bb7bac 100644 --- a/spec/requests/forms/check_your_answers_controller_spec.rb +++ b/spec/requests/forms/check_your_answers_controller_spec.rb @@ -327,7 +327,7 @@ let(:end_session_endpoint) { "http://example.com/one-login-mock/logout" } before do - allow(Store::ReturnFromOneLoginStore).to receive(:new).and_wrap_original do |original_method, *_args| + allow(AuthService).to receive(:new).and_wrap_original do |original_method, *_args| original_method.call(store) end diff --git a/spec/requests/forms/continue_to_one_login_controller_spec.rb b/spec/requests/forms/continue_to_one_login_controller_spec.rb index c6f3ce828..8416a8504 100644 --- a/spec/requests/forms/continue_to_one_login_controller_spec.rb +++ b/spec/requests/forms/continue_to_one_login_controller_spec.rb @@ -37,7 +37,7 @@ allow(Flow::Context).to receive(:new).and_wrap_original do |original_method, *args| original_method.call(form: args[0][:form], form_document: args[0][:form_document], store:) end - allow(Store::ReturnFromOneLoginStore).to receive(:new).and_wrap_original do |original_method, *_args| + allow(AuthService).to receive(:new).and_wrap_original do |original_method, *_args| original_method.call(store) end end diff --git a/spec/requests/users/omniauth_controller_spec.rb b/spec/requests/users/omniauth_controller_spec.rb index fecc459da..a52b3b12b 100644 --- a/spec/requests/users/omniauth_controller_spec.rb +++ b/spec/requests/users/omniauth_controller_spec.rb @@ -40,13 +40,7 @@ OmniAuth.config.test_mode = true OmniAuth.config.mock_auth[:default] = auth_hash - allow(Store::ReturnFromOneLoginStore).to receive(:new).and_wrap_original do |original_method, *_args| - original_method.call(store) - end - allow(Store::ConfirmationDetailsStore).to receive(:new).and_wrap_original do |original_method, *args| - original_method.call(store, args[1]) - end - allow(Store::AuthStore).to receive(:new).and_wrap_original do |original_method, *_args| + allow(AuthService).to receive(:new).and_wrap_original do |original_method, *_args| original_method.call(store) end end @@ -69,19 +63,11 @@ end end - context "when the auth details are not present on the request" do + 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(Users::OmniauthController::OmniAuthLoggedInDataMissingError) - end - end - - context "when the email on the auth hash is blank" do - let(:email) { "" } - - it "raises a OmniAuthLoggedInDataMissingError" do - expect { get omniauth_callback_path }.to raise_error(Users::OmniauthController::OmniAuthLoggedInDataMissingError) + expect { get omniauth_callback_path }.to raise_error(AuthService::DataMissingError) end end @@ -107,10 +93,7 @@ let(:token) { Faker::Alphanumeric.alphanumeric } before do - allow(Store::ReturnFromOneLoginStore).to receive(:new).and_wrap_original do |original_method, *_args| - original_method.call(store) - end - allow(Store::AuthStore).to receive(:new).and_wrap_original do |original_method, *_args| + allow(AuthService).to receive(:new).and_wrap_original do |original_method, *_args| original_method.call(store) end end diff --git a/spec/services/auth_service_spec.rb b/spec/services/auth_service_spec.rb index 6080d3862..d351a9ba9 100644 --- a/spec/services/auth_service_spec.rb +++ b/spec/services/auth_service_spec.rb @@ -1,30 +1,117 @@ require "rails_helper" RSpec.describe AuthService do - subject(:auth_service) { described_class.new } + subject(:auth_service) { described_class.new(store) } let(:end_session_endpoint) { "http://example.com/one-login-mock/logout" } + let(:store) { {}.with_indifferent_access } + let(:token) { Faker::Alphanumeric.alphanumeric } before do idp_configuration = instance_double(OmniAuth::GovukOneLogin::IdpConfiguration, end_session_endpoint:) allow(Rails.application.config.x).to receive(:one_login).and_return(double(idp_configuration:)) end - describe "#logout_request" do - let(:token) { Faker::Alphanumeric.alphanumeric } + describe "#store_auth_details" do + let(:email) { "test@example.com" } + let(:id_token) { Faker::Alphanumeric.alphanumeric } + let(:auth_hash) do + { + info: { email: }, + credentials: { id_token: }, + }.with_indifferent_access + end + + context "when the return from one login params are not set on the session" do + it "raises a MissingReturnParamsError" do + expect { auth_service.store_auth_details({}) } + .to raise_error(Store::ReturnFromOneLoginStore::MissingReturnParamsError) + end + end + + context "when the return from one login params are set on the session" do + let(:form_id) { 42 } + let(:store) do + { + "return_from_one_login" => { + "last_form_id" => form_id, + }, + }.with_indifferent_access + end + + context "when the auth hash is empty" do + let(:auth_hash) { {}.with_indifferent_access } + + it "raises a DataMissingError" do + expect { auth_service.store_auth_details(auth_hash) } + .to raise_error(AuthService::DataMissingError, "Auth hash is missing on request") + end + end + + context "when the email is missing from the auth hash" do + let(:auth_hash) { { credentials: { id_token: } }.with_indifferent_access } + + it "raises a DataMissingError" do + expect { auth_service.store_auth_details(auth_hash) } + .to raise_error(AuthService::DataMissingError, "Email is missing in OmniAuth auth hash") + end + end + + context "when the token is missing from the auth hash" do + let(:auth_hash) { { info: { email: } }.with_indifferent_access } + + it "raises a DataMissingError" do + expect { auth_service.store_auth_details(auth_hash) } + .to raise_error(AuthService::DataMissingError, "Token is missing in OmniAuth auth hash") + end + end + + context "when the auth hash is valid" do + it "stores the email on the session" do + auth_service.store_auth_details(auth_hash) + expect(store.dig("confirmation_details", form_id.to_s, "copy_of_answers_email_address")).to eq email + end + + it "stores the token on the session" do + auth_service.store_auth_details(auth_hash) + expect(store["auth"]["token"]).to eq id_token + end + end + end + end + + describe "#logout_redirect_uri" do + let(:store) do + { + auth: { token: }, + }.with_indifferent_access + end let(:post_logout_redirect_uri) { "https://example.com/some-path?with=params" } it "returns a logout request with the expected One Login URI" do - logout_request = auth_service.logout_request(token, post_logout_redirect_uri) - expect(logout_request.redirect_uri).to start_with(end_session_endpoint) + logout_redirect_uri = auth_service.logout_redirect_uri(post_logout_redirect_uri) + expect(logout_redirect_uri).to start_with(end_session_endpoint) end it "strips query params from the post_logout_redirect_uri" do - logout_request = auth_service.logout_request(token, post_logout_redirect_uri) - redirect_uri = URI.parse(logout_request.redirect_uri) - query = Rack::Utils.parse_query(redirect_uri.query) + logout_redirect_uri = auth_service.logout_redirect_uri(post_logout_redirect_uri) + parsed_uri = URI.parse(logout_redirect_uri) + query = Rack::Utils.parse_query(parsed_uri.query) expect(query).to include("post_logout_redirect_uri" => "https://example.com/some-path", "id_token_hint" => token) end end + + describe "#clear_auth_session" do + let(:store) do + { + auth: { token: }, + }.with_indifferent_access + end + + it "clears the session" do + auth_service.clear_auth_session + expect(store["auth"]).to be_nil + end + end end