diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 8d62dc8eb..f5b1a55e4 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_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 9ead6c982..021f25693 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_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 rescue FormSubmissionService::ConfirmationEmailToAddressError setup_check_your_answers email_confirmation_input.errors.add(:confirmation_email_address, :invalid_email) diff --git a/app/controllers/forms/continue_to_one_login_controller.rb b/app/controllers/forms/continue_to_one_login_controller.rb index ae10ad8b7..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 - Store::ReturnFromOneLoginStore.new(session).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 0f9ff4eaa..b5df68411 100644 --- a/app/controllers/users/omniauth_controller.rb +++ b/app/controllers/users/omniauth_controller.rb @@ -1,31 +1,29 @@ module Users class OmniauthController < ApplicationController - 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 - email = request.env.dig("omniauth.auth", "info", "email") - if email.blank? - raise OmniAuthLoggedInDataMissingError, "Email is missing in OmniAuth auth hash" - end - - 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 + auth_hash = request.env["omniauth.auth"] + 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 + 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_service.clear_auth_session + + path_params = auth_service.form_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 new file mode 100644 index 000000000..a1c156acb --- /dev/null +++ b/app/lib/store/auth_store.rb @@ -0,0 +1,28 @@ +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 clear + @store.delete(AUTH_KEY) + end + + def logged_in? + get_token.present? + 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 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 new file mode 100644 index 000000000..3d1d2eb70 --- /dev/null +++ b/app/services/auth_service.rb @@ -0,0 +1,56 @@ +class AuthService + 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 + + 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/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/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 8a8ce079b..868bb7bac 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(AuthService).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/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 ad0ceaabe..a52b3b12b 100644 --- a/spec/requests/users/omniauth_controller_spec.rb +++ b/spec/requests/users/omniauth_controller_spec.rb @@ -1,23 +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(:id_token) { Faker::Alphanumeric.alphanumeric } let(:auth_hash) do { provider: :govuk_one_login, @@ -26,7 +31,7 @@ email:, }, credentials: { - id_token: "id_token", + id_token:, }, }.with_indifferent_access end @@ -35,12 +40,9 @@ 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| + allow(AuthService).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 end context "when the auth details are present on the request and the user has a valid session" do @@ -55,21 +57,17 @@ 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 - end - - context "when the auth details are not present on the request" do - let(:auth_hash) { {} } - it "raises an OmniAuthLoggedInDataMissingError" do - expect { get omniauth_callback_path }.to raise_error(Users::OmniauthController::OmniAuthLoggedInDataMissingError) + it "stores the token on the session" do + expect(store["auth"]["token"]).to eq id_token end end - context "when the email on the auth hash is blank" do - let(:email) { "" } + context "when data is missing on the auth details on the request" do + let(:auth_hash) { {} } - it "raises a OmniAuthLoggedInDataMissingError" do - expect { get omniauth_callback_path }.to raise_error(Users::OmniauthController::OmniAuthLoggedInDataMissingError) + it "raises an OmniAuthLoggedInDataMissingError" do + expect { get omniauth_callback_path }.to raise_error(AuthService::DataMissingError) end end @@ -90,4 +88,43 @@ 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(AuthService).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 diff --git a/spec/services/auth_service_spec.rb b/spec/services/auth_service_spec.rb new file mode 100644 index 000000000..d351a9ba9 --- /dev/null +++ b/spec/services/auth_service_spec.rb @@ -0,0 +1,117 @@ +require "rails_helper" + +RSpec.describe AuthService do + 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 "#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_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_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