Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions app/controllers/concerns/saml_idp_logout_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,19 @@ def handle_valid_sp_logout_request
sign_out if user_signed_in?
end

def handle_valid_sp_remote_logout_request(user_id)
def handle_valid_sp_remote_logout_request(user_id:, issuer:)
# Remotely delete the user's current session
session_id = ServiceProviderIdentity.
find_by(user_id: user_id, service_provider: saml_request.issuer).
rails_session_id
where(user_id: user_id, service_provider: issuer).pick(:rails_session_id)

OutOfBandSessionAccessor.new(session_id).destroy if session_id
if session_id
OutOfBandSessionAccessor.new(session_id).destroy
user = User.find_by(id: user_id)
analytics.remote_logout_completed(
service_provider: issuer,
user_id: user&.uuid,
)
end
sign_out if user_signed_in?

# rubocop:disable Rails/RenderInline
Expand Down Expand Up @@ -63,9 +69,9 @@ def track_logout_event
)
end

def track_remote_logout_event
def track_remote_logout_event(issuer)
analytics.remote_logout_initiated(
service_provider: saml_request&.issuer,
service_provider: issuer,
saml_request_valid: valid_saml_request?,
)
end
Expand Down
5 changes: 3 additions & 2 deletions app/controllers/saml_idp_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,17 @@ def remotelogout
return head(:bad_request) if raw_saml_request.nil?

decode_request(raw_saml_request)
issuer = saml_request&.issuer

track_remote_logout_event
track_remote_logout_event(issuer)

return head(:bad_request) unless valid_saml_request?

user_id = find_user_from_session_index

return head(:bad_request) unless user_id.present?

handle_valid_sp_remote_logout_request(user_id)
handle_valid_sp_remote_logout_request(user_id: user_id, issuer: issuer)
end

def external_saml_request?
Expand Down
18 changes: 17 additions & 1 deletion app/services/analytics_events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2439,7 +2439,7 @@ def remembered_device_used_for_authentication
track_event('Remembered device used for authentication')
end

# User initiated remote logout
# Service provider initiated remote logout
# @param [String] service_provider
# @param [Boolean] saml_request_valid
def remote_logout_initiated(
Expand All @@ -2455,6 +2455,22 @@ def remote_logout_initiated(
)
end

# Service provider completed remote logout
# @param [String] service_provider
# @param [String] user_id
def remote_logout_completed(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this worth adding a test for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thank you! Added in 8258d62

service_provider:,
user_id:,
**extra
)
track_event(
'Remote Logout completed',
service_provider: service_provider,
user_id: user_id,
**extra,
)
end

# A response timed out
# @param [String] backtrace
# @param [String] exception_message
Expand Down
10 changes: 10 additions & 0 deletions spec/features/saml/saml_logout_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,9 @@
let(:agency) { sp.agency }

it "terminates the user's session remotely" do
fake_analytics = FakeAnalytics.new
allow_any_instance_of(ApplicationController).to receive(:analytics).and_return(fake_analytics)

# set up SP identity and agency identity
user = sign_in_live_with_2fa
visit_saml_authn_request_url
Expand All @@ -167,6 +170,13 @@

expect(OutOfBandSessionAccessor.new(identity.rails_session_id).exists?).to eq false

expect(fake_analytics.events['Remote Logout initiated']).to eq(
[{ service_provider: sp.issuer, saml_request_valid: true }],
)
expect(fake_analytics.events['Remote Logout completed']).to eq(
[{ service_provider: sp.issuer, user_id: user.uuid }],
)

# should be logged out...
visit account_path
expect(page).to have_current_path(root_path)
Expand Down