From f01dcb43996614b59b6f297f33625d0efa09dad3 Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Tue, 11 Jun 2024 11:30:36 +0200
Subject: [PATCH] Remove SAML information when SLO is not supported

---
 app/controllers/users/sessions_controller.rb  |  5 +-
 .../users/sessions_controller_spec.rb         | 67 +++++++++++++++++++
 2 files changed, 71 insertions(+), 1 deletion(-)
 create mode 100644 spec/controllers/users/sessions_controller_spec.rb

diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb
index 687c3952e..c2ea5268d 100644
--- a/app/controllers/users/sessions_controller.rb
+++ b/app/controllers/users/sessions_controller.rb
@@ -52,7 +52,10 @@ def after_sign_out_path_for(_)
         return spslo_path_for(provider) if strategy_class.default_options.idp_slo_service_url
       end
 
-      # If SLO is not supported, we delegate the call to the parent
+      # If SLO is not supported, we first delete all information from the current session
+      # This is mainly done to remove the SAML information we stored before
+      session.clear
+      # Then, we delegate the call to the parent
       super
     end
 
diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb
new file mode 100644
index 000000000..6d4bdbda3
--- /dev/null
+++ b/spec/controllers/users/sessions_controller_spec.rb
@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Users::SessionsController do
+  render_views
+
+  describe '#destroy' do
+    context 'with SAML login' do
+      let(:omniauth_provider) { User.omniauth_providers.first.to_s }
+      let(:saml_uid) { 'saml_uid' }
+      let(:saml_session_index) { 'saml_session_index' }
+
+      before do
+        OmniAuth.config.test_mode = true
+        request.env['devise.mapping'] = Devise.mappings[:user]
+
+        # Add a new provider to the omniauth config and reload routes
+        Devise.omniauth :sso_callback, strategy_class: OmniAuth::Strategies::AbstractSaml
+        Rails.application.reload_routes!
+
+        session[:saml_uid] = saml_uid
+        session[:saml_session_index] = saml_session_index
+        session[:omniauth_provider] = omniauth_provider
+      end
+
+      after do
+        # Remove the provider from the omniauth config and reload routes
+        Devise.class_variable_set(:@@omniauth_configs, {}) # rubocop:disable Style/ClassVars
+        Rails.application.reload_routes!
+      end
+
+      context 'when SLO is supported' do
+        before do
+          options = OmniAuth::Strategy::Options.new(idp_slo_service_url: 'https://idp.example.com/slo')
+          allow(OmniAuth::Strategies::AbstractSaml).to receive(:default_options).and_return(options)
+        end
+
+        it 'redirects to the IdP SLO path' do
+          delete :destroy
+          expect(response).to redirect_to(controller.send(:spslo_path_for, omniauth_provider))
+        end
+
+        it 'preserves the SAML information in the session' do
+          delete :destroy
+          expect(session[:saml_uid]).to eq(saml_uid)
+          expect(session[:saml_session_index]).to eq(saml_session_index)
+          expect(session[:omniauth_provider]).to eq(omniauth_provider)
+        end
+      end
+
+      context 'when SLO is not supported' do
+        it 'redirects to the root path' do
+          delete :destroy
+          expect(response).to redirect_to(root_path)
+        end
+
+        it 'clears the session' do
+          delete :destroy
+          expect(session[:saml_uid]).to be_nil
+          expect(session[:saml_session_index]).to be_nil
+          expect(session[:omniauth_provider]).to be_nil
+        end
+      end
+    end
+  end
+end