Skip to content

Commit

Permalink
Ensure role is passed to DsiUser.create_or_update_from_dsi
Browse files Browse the repository at this point in the history
A bug arising from a refactor of OmniAuthCallbacksController has meant that the authorisation role isn't passed and therefore no DsiUserSession is created.
  • Loading branch information
steventux committed Oct 31, 2023
1 parent f3a12cb commit b670911
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ class CheckRecords::OmniauthCallbacksController < ApplicationController
protect_from_forgery except: :dfe_bypass
before_action :add_auth_attributes_to_session, only: :dfe

attr_reader :role

def dfe
unless CheckRecords::DfESignIn.bypass?
role = check_user_access_to_service
check_user_access_to_service

return redirect_to check_records_not_authorised_path unless role
end
Expand All @@ -29,14 +31,14 @@ def add_auth_attributes_to_session
end

def check_user_access_to_service
DfESignInApi::GetUserAccessToService.new(
@role = DfESignInApi::GetUserAccessToService.new(
org_id: auth.extra.raw_info.organisation.id,
user_id: auth.uid,
).call
end

def create_or_update_dsi_user
@dsi_user = DsiUser.create_or_update_from_dsi(auth)
@dsi_user = DsiUser.create_or_update_from_dsi(auth, role:)
session[:dsi_user_id] = @dsi_user.id
session[:dsi_user_session_expiry] = 2.hours.from_now.to_i
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/dsi_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class DsiUser < ApplicationRecord
has_many :search_logs
has_many :dsi_user_sessions, dependent: :destroy

def self.create_or_update_from_dsi(dsi_payload, role = nil)
def self.create_or_update_from_dsi(dsi_payload, role: nil)
dsi_user = find_or_initialize_by(email: dsi_payload.info.fetch(:email))

dsi_user.update!(
Expand Down
2 changes: 1 addition & 1 deletion spec/models/dsi_user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
context "when the user has a role" do
it "creates a session record" do
role = { "id" => "123", "code" => "TestRole_code" }
described_class.create_or_update_from_dsi(dsi_payload, role)
described_class.create_or_update_from_dsi(dsi_payload, role:)

dsi_user_session = DsiUser.first.dsi_user_sessions.first
expect(dsi_user_session).to be_present
Expand Down
1 change: 1 addition & 0 deletions spec/system/check_records/user_signs_in_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@
def then_i_am_signed_in
within("header") { expect(page).to have_content "Sign out" }
expect(DsiUser.count).to eq 1
expect(DsiUserSession.count).to eq 1
end
end

0 comments on commit b670911

Please sign in to comment.