Skip to content

Commit

Permalink
use one_login rather than onelogin
Browse files Browse the repository at this point in the history
  • Loading branch information
CatalinVoineag committed Dec 17, 2024
1 parent 35a44c9 commit 422b0c4
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 52 deletions.
16 changes: 8 additions & 8 deletions app/controllers/one_login_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ 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)

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
Expand All @@ -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

Expand All @@ -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

Expand All @@ -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
Expand All @@ -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|
Expand Down
2 changes: 1 addition & 1 deletion app/views/layouts/_header.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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)],
Expand Down
4 changes: 2 additions & 2 deletions config/initializers/omniauth.rb
Original file line number Diff line number Diff line change
@@ -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']
Expand Down Expand Up @@ -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
6 changes: 3 additions & 3 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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,
Expand All @@ -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,
}
Expand Down
58 changes: 29 additions & 29 deletions spec/requests/one_login_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
let(:omniauth_hash) do
OmniAuth::AuthHash.new(
{
provider: 'onelogin',
provider: 'one_login',
uid: '123',
info: {
email: '[email protected]',
Expand All @@ -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
Expand All @@ -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: '[email protected]')
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',
Expand Down Expand Up @@ -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: '[email protected]')

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|
Expand All @@ -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: '[email protected]')
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|
Expand All @@ -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: '[email protected]')
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 " \
Expand All @@ -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,
Expand All @@ -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
2 changes: 1 addition & 1 deletion spec/support/test_helpers/one_login_helper.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module OneLoginHelper
def user_exists_in_onelogin(email_address: '[email protected]', uid: 'UID')
def user_exists_in_one_login(email_address: '[email protected]', uid: 'UID')
OmniAuth.config.mock_auth[:onelogin] = OmniAuth::AuthHash.new(
{
provider: 'onelogin',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 422b0c4

Please sign in to comment.