diff --git a/app/controllers/one_login_controller.rb b/app/controllers/one_login_controller.rb index cb6e382ccce..8b7276512b0 100644 --- a/app/controllers/one_login_controller.rb +++ b/app/controllers/one_login_controller.rb @@ -3,7 +3,7 @@ class OneLoginController < ApplicationController def callback auth = request.env['omniauth.auth'] - session[:onelogin_id_token] = auth&.credentials&.id_token + session[:one_login_id_token] = auth&.credentials&.id_token candidate = OneLoginUser.authenticate(auth) sign_in_candidate(candidate) @@ -11,7 +11,7 @@ def callback redirect_to candidate_interface_interstitial_path rescue OneLoginUser::Error => e session[:one_login_error] = e.message - redirect_to auth_onelogin_sign_out_path + redirect_to auth_one_login_sign_out_path end def bypass_callback @@ -31,7 +31,7 @@ def bypass_callback end def sign_out - id_token = session[:onelogin_id_token] + id_token = session[:one_login_id_token] one_login_error = session[:one_login_error] reset_session @@ -40,7 +40,7 @@ def sign_out redirect_to candidate_interface_create_account_or_sign_in_path else # Go back to one login to sign out the user on their end as well - redirect_to logout_onelogin(id_token), allow_other_host: true + redirect_to logout_one_login(id_token), allow_other_host: true end end @@ -55,9 +55,9 @@ def sign_out_complete def failure session[:one_login_error] = "One login failure with #{params[:message]} " \ - "for onelogin_id_token: #{session[:onelogin_id_token]}" + "for one_login_id_token: #{session[:one_login_id_token]}" - redirect_to auth_onelogin_sign_out_path + redirect_to auth_one_login_sign_out_path end private @@ -73,9 +73,9 @@ def sign_in_candidate(candidate) candidate.update!(last_signed_in_at: Time.zone.now) end - def logout_onelogin(id_token_hint) + def logout_one_login(id_token_hint) params = { - post_logout_redirect_uri: URI(auth_onelogin_sign_out_complete_url), + post_logout_redirect_uri: URI(auth_one_login_sign_out_complete_url), id_token_hint:, } URI.parse("#{ENV['GOVUK_ONE_LOGIN_ISSUER_URL']}logout").tap do |uri| diff --git a/app/views/layouts/_header.html.erb b/app/views/layouts/_header.html.erb index dbbaee47aad..f26511df8a8 100644 --- a/app/views/layouts/_header.html.erb +++ b/app/views/layouts/_header.html.erb @@ -7,7 +7,7 @@ )) %> <%= render PhaseBannerComponent.new(no_border: current_candidate.present?) %> <% if current_candidate %> - <% sign_out_path = FeatureFlag.active?(:one_login_candidate_sign_in) ? auth_onelogin_sign_out_path : candidate_interface_sign_out_path %> + <% sign_out_path = FeatureFlag.active?(:one_login_candidate_sign_in) ? auth_one_login_sign_out_path : candidate_interface_sign_out_path %> <%= render(PrimaryNavigationComponent.new( items: NavigationItems.candidate_primary_navigation(current_candidate:, current_controller: controller), items_right: [NavigationItems::NavigationItem.new('Sign out', sign_out_path)], diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 14d1dceada8..f0f9083da59 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -1,6 +1,6 @@ OmniAuth.config.logger = Rails.logger require 'omniauth/strategies/one_login_developer' -require 'omniauth/onelogin_setup' +require 'omniauth/one_login_setup' dfe_sign_in_identifier = ENV['DFE_SIGN_IN_CLIENT_ID'] dfe_sign_in_secret = ENV['DFE_SIGN_IN_SECRET'] @@ -60,6 +60,6 @@ def self.bypass? end else Rails.application.config.middleware.use OmniAuth::Builder do |builder| - OneloginSetup.configure(builder) + OneLoginSetup.configure(builder) end end diff --git a/config/routes.rb b/config/routes.rb index f85b6aeaf2f..8c7c77194f0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -23,10 +23,10 @@ get '/auth/developer/callback' => 'dfe_sign_in#bypass_callback' get '/auth/dfe/sign-out' => 'dfe_sign_in#redirect_after_dsi_signout' - get '/auth/onelogin/callback', to: 'one_login#callback' + get '/auth/one-login/callback', to: 'one_login#callback' get '/auth/one-login-developer/callback' => 'one_login#bypass_callback' - get '/auth/onelogin/sign-out', to: 'one_login#sign_out' - get '/auth/onelogin/sign-out-complete', to: 'one_login#sign_out_complete' + get '/auth/one-login/sign-out', to: 'one_login#sign_out' + get '/auth/one-login/sign-out-complete', to: 'one_login#sign_out_complete' get '/auth/failure', to: 'one_login#failure' direct :find do diff --git a/lib/omniauth/onelogin_setup.rb b/lib/omniauth/one_login_setup.rb similarity index 82% rename from lib/omniauth/onelogin_setup.rb rename to lib/omniauth/one_login_setup.rb index ff3048bb1ed..7c195cdb3a6 100644 --- a/lib/omniauth/onelogin_setup.rb +++ b/lib/omniauth/one_login_setup.rb @@ -1,9 +1,9 @@ -module OneloginSetup +module OneLoginSetup class OmniAuth::Strategies::GovukOneLogin < OmniAuth::Strategies::OpenIDConnect; end def self.configure(builder) client_id = ENV.fetch('GOVUK_ONE_LOGIN_CLIENT_ID', '') - onelogin_issuer_uri = URI(ENV.fetch('GOVUK_ONE_LOGIN_ISSUER_URL', '')) + one_login_issuer_uri = URI(ENV.fetch('GOVUK_ONE_LOGIN_ISSUER_URL', '')) private_key_pem = ENV.fetch('GOVUK_ONE_LOGIN_PRIVATE_KEY', '') application_url = HostingEnvironment.application_url @@ -17,11 +17,11 @@ def self.configure(builder) builder.provider :govuk_one_login, name: :onelogin, allow_authorize_params: %i[session_id], - callback_path: '/auth/onelogin/callback', + callback_path: '/auth/one-login/callback', discovery: true, - issuer: onelogin_issuer_uri.to_s, + issuer: one_login_issuer_uri.to_s, path_prefix: '/auth', - post_logout_redirect_uri: "#{application_url}/auth/onelogin/sign-out-complete", + post_logout_redirect_uri: "#{application_url}/auth/one-login/sign-out-complete", response_type: :code, scope: %w[email openid], client_auth_method: :jwt_bearer, @@ -30,10 +30,10 @@ def self.configure(builder) end_session_endpoint: '/oauth2/logout', token_endpoint: '/oauth2/token', userinfo_endpoint: '/oauth2/userinfo', - host: onelogin_issuer_uri.host, + host: one_login_issuer_uri.host, identifier: client_id, port: 443, - redirect_uri: "#{application_url}/auth/onelogin/callback", + redirect_uri: "#{application_url}/auth/one-login/callback", scheme: 'https', private_key: private_key, } diff --git a/spec/requests/one_login_controller_spec.rb b/spec/requests/one_login_controller_spec.rb index 9bf0fc2f5dd..44d1df2d616 100644 --- a/spec/requests/one_login_controller_spec.rb +++ b/spec/requests/one_login_controller_spec.rb @@ -10,7 +10,7 @@ let(:omniauth_hash) do OmniAuth::AuthHash.new( { - provider: 'onelogin', + provider: 'one_login', uid: '123', info: { email: 'test@email.com', @@ -22,12 +22,12 @@ ) end - describe 'GET /auth/onelogin/callback' do + describe 'GET /auth/one-login/callback' do it 'redirects to candidate_interface_interstitial_path' do candidate = create(:candidate) create(:one_login_auth, candidate:, token: '123') - get auth_onelogin_callback_path + get auth_one_login_callback_path expect(response).to redirect_to(candidate_interface_interstitial_path) end @@ -36,20 +36,20 @@ let(:omniauth_hash) { nil } it 'returns unprocessable_entity' do - get auth_onelogin_callback_path + get auth_one_login_callback_path expect(response).to have_http_status(:unprocessable_entity) end end - context 'when candidate has a different onelogin token than the one returned by onelogin' do - it 'redirects to auth_onelogin_sign_out_path' do + context 'when candidate has a different one login token than the one returned by one login' do + it 'redirects to auth_one_login_sign_out_path' do candidate = create(:candidate, email_address: 'test@email.com') create(:one_login_auth, candidate:, token: '456') - get auth_onelogin_callback_path + get auth_one_login_callback_path - expect(response).to redirect_to(auth_onelogin_sign_out_path) + expect(response).to redirect_to(auth_one_login_sign_out_path) expect(session[:one_login_error]).to eq( "Candidate #{candidate.id} has a different one login token than the " \ 'user trying to login. Token used to auth 123', @@ -112,15 +112,15 @@ end end - describe 'GET /auth/onelogin/sign_out' do + describe 'GET /auth/one-login/sign_out' do it 'redirects to one_login logout url' do create(:candidate, email_address: 'test@email.com') - get auth_onelogin_callback_path # set the session variables - get auth_onelogin_sign_out_path + get auth_one_login_callback_path # set the session variables + get auth_one_login_sign_out_path params = { - post_logout_redirect_uri: URI(auth_onelogin_sign_out_complete_url), + post_logout_redirect_uri: URI(auth_one_login_sign_out_complete_url), id_token_hint: 'id_token', } one_login_logout_url = URI.parse("#{ENV['GOVUK_ONE_LOGIN_ISSUER_URL']}logout").tap do |uri| @@ -130,22 +130,22 @@ expect(response).to redirect_to(one_login_logout_url) end - context 'when candidate has a different onelogin token than the one returned by onelogin' do + context 'when candidate has a different one login token than the one returned by one login' do it 'redirects to one_login logout url and persists the session error message' do candidate = create(:candidate, email_address: 'test@email.com') create(:one_login_auth, candidate:, token: '456') - get auth_onelogin_callback_path # set the session variables - get auth_onelogin_sign_out_path + get auth_one_login_callback_path # set the session variables + get auth_one_login_sign_out_path - expect(session[:onelogin_id_token]).to be_nil + expect(session[:one_login_id_token]).to be_nil expect(session[:one_login_error]).to eq( "Candidate #{candidate.id} has a different one login token than the " \ 'user trying to login. Token used to auth 123', ) params = { - post_logout_redirect_uri: URI(auth_onelogin_sign_out_complete_url), + post_logout_redirect_uri: URI(auth_one_login_sign_out_complete_url), id_token_hint: 'id_token', } one_login_url = URI.parse("#{ENV['GOVUK_ONE_LOGIN_ISSUER_URL']}logout").tap do |uri| @@ -160,21 +160,21 @@ it 'redirects to sign_in page' do allow(OneLogin).to receive(:bypass?).and_return(true) - get auth_onelogin_sign_out_path + get auth_one_login_sign_out_path expect(response).to redirect_to candidate_interface_create_account_or_sign_in_path end end end - describe 'GET /auth/onelogin/sign_out_complete' do - context 'when candidate has a different onelogin token than the one returned by onelogin' do - it 'redirects to logout_onelogin_path and persists the session error message' do + describe 'GET /auth/one-login/sign_out_complete' do + context 'when candidate has a different one login token than the one returned by one login' do + it 'redirects to logout_one_login_path and persists the session error message' do candidate = create(:candidate, email_address: 'test@email.com') create(:one_login_auth, candidate:, token: '456') allow(Sentry).to receive(:capture_message) - get auth_onelogin_callback_path # set the session variables - get auth_onelogin_sign_out_complete_path + get auth_one_login_callback_path # set the session variables + get auth_one_login_sign_out_complete_path expect(Sentry).to have_received(:capture_message).with( "Candidate #{candidate.id} has a different one login token than the " \ @@ -186,8 +186,8 @@ end context 'candidate has no errors' do - it 'redirects to logout_onelogin_path and persists the session error message' do - get auth_onelogin_sign_out_complete_path + it 'redirects to logout_one_login_path and persists the session error message' do + get auth_one_login_sign_out_complete_path expect(response).to redirect_to( candidate_interface_create_account_or_sign_in_path, @@ -196,15 +196,15 @@ end end - describe 'GET /auth/onelogin/failure' do + describe 'GET /auth/one-login/failure' do it 'redirects to auth_failure_path with one login error' do - get auth_onelogin_callback_path # set the session variables + get auth_one_login_callback_path # set the session variables get auth_failure_path(params: { message: 'error_message' }) expect(session[:one_login_error]).to eq( - 'One login failure with error_message for onelogin_id_token: id_token', + 'One login failure with error_message for one_login_id_token: id_token', ) - expect(response).to redirect_to(auth_onelogin_sign_out_path) + expect(response).to redirect_to(auth_one_login_sign_out_path) end end end diff --git a/spec/support/test_helpers/one_login_helper.rb b/spec/support/test_helpers/one_login_helper.rb index 9eb796f40a2..3ff2393c738 100644 --- a/spec/support/test_helpers/one_login_helper.rb +++ b/spec/support/test_helpers/one_login_helper.rb @@ -1,5 +1,5 @@ module OneLoginHelper - def user_exists_in_onelogin(email_address: 'test@email.com', uid: 'UID') + def user_exists_in_one_login(email_address: 'test@email.com', uid: 'UID') OmniAuth.config.mock_auth[:onelogin] = OmniAuth::AuthHash.new( { provider: 'onelogin', diff --git a/spec/system/candidate_interface/one_login_signup/candidate_signs_in_spec.rb b/spec/system/candidate_interface/one_login_signup/candidate_signs_in_spec.rb index 74cf8e4c8eb..fab5354321e 100644 --- a/spec/system/candidate_interface/one_login_signup/candidate_signs_in_spec.rb +++ b/spec/system/candidate_interface/one_login_signup/candidate_signs_in_spec.rb @@ -48,7 +48,7 @@ def given_i_have_a_candidate_record end def given_i_have_one_login_account(email_address) - user_exists_in_onelogin(email_address:) + user_exists_in_one_login(email_address:) end def given_i_already_have_a_different_one_login_token(candidate)