Browse Source

Fixes #4548 - Allow logout for SAML accounts without set idp_slo_service_url.

Tobias Schäfer 1 year ago
parent
commit
41e87c61e4

+ 6 - 2
app/controllers/sessions_controller.rb

@@ -54,8 +54,12 @@ class SessionsController < ApplicationController
       ENV['FAKE_SELENIUM_LOGIN_PENDING'] = nil # rubocop:disable Rails/EnvironmentVariableAccess
     end
 
-    if session['saml_uid'] || session['saml_session_index']
-      return saml_destroy
+    if (session['saml_uid'] || session['saml_session_index']) && SamlDatabase.setup.fetch('idp_slo_service_url', nil)
+      begin
+        return saml_destroy
+      rescue => e
+        Rails.logger.error "SAML SLO failed: #{e.message}"
+      end
     end
 
     reset_session

+ 8 - 2
app/graphql/gql/mutations/logout.rb

@@ -19,7 +19,13 @@ module Gql::Mutations
 
     def resolve(...)
       # Special handling for SAML logout (we need to redirect to the ISP).
-      return saml_destroy if saml_session?
+      if saml_session?
+        begin
+          return saml_destroy
+        rescue => e
+          Rails.logger.error "SAML SLO failed: #{e.message}"
+        end
+      end
 
       context[:controller].reset_session
       context[:current_user] = nil
@@ -33,7 +39,7 @@ module Gql::Mutations
     end
 
     def saml_session?
-      session['saml_uid'] || session['saml_session_index']
+      (session['saml_uid'] || session['saml_session_index']) && SamlDatabase.setup.fetch('idp_slo_service_url', nil)
     end
 
     def saml_logout_url

+ 44 - 2
spec/system/saml_spec.rb

@@ -35,7 +35,7 @@ RSpec.describe 'SAML Authentication', authenticated_as: false, integration: true
     saml_initialized = true
   end
 
-  def set_saml_config(name_identifier_format: nil, uid_attribute: nil)
+  def set_saml_config(name_identifier_format: nil, uid_attribute: nil, idp_slo_service_url: true)
     # Setup Zammad SAML authentication.
     response = UserAgent.get(saml_realm_zammad_descriptor)
     raise 'No Zammad realm descriptor found' if !response.success?
@@ -47,10 +47,12 @@ RSpec.describe 'SAML Authentication', authenticated_as: false, integration: true
       {
         display_name:           'SAML',
         idp_sso_target_url:     "#{saml_base_url}/realms/zammad/protocol/saml",
-        idp_slo_service_url:    "#{saml_base_url}/realms/zammad/protocol/saml",
         idp_cert:               "-----BEGIN CERTIFICATE-----\n#{match[:cert]}\n-----END CERTIFICATE-----",
         name_identifier_format: name_identifier_format || 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress',
       }
+    if idp_slo_service_url
+      auth_saml_credentials[:idp_slo_service_url] = "#{saml_base_url}/realms/zammad/protocol/saml"
+    end
     auth_saml_credentials[:uid_attribute] = uid_attribute if uid_attribute
     Setting.set('auth_saml_credentials', auth_saml_credentials)
     Setting.set('auth_saml', true)
@@ -174,6 +176,24 @@ RSpec.describe 'SAML Authentication', authenticated_as: false, integration: true
     end
   end
 
+  describe 'SAML logout without IDP SLO service URL' do
+    before do
+      set_saml_config(idp_slo_service_url: false)
+    end
+
+    it 'is successful' do
+      login_saml
+
+      user = User.find_by(email: 'john.doe@saml.example.com')
+      expect(user.login).to eq('john.doe@saml.example.com')
+
+      logout_saml
+
+      visit saml_realm_zammad_accounts
+      expect(page).to have_css('#landingSignOutButton')
+    end
+  end
+
   describe 'Mobile View', app: :mobile do
     context 'when login is tested' do
       before do
@@ -227,5 +247,27 @@ RSpec.describe 'SAML Authentication', authenticated_as: false, integration: true
         expect(page).to have_css('#landingSignOutLink')
       end
     end
+
+    context 'when logout is tested without IDP SLO service URL' do
+      before do
+        set_saml_config(idp_slo_service_url: false)
+      end
+
+      it 'is successful' do
+        login_saml(app: 'mobile')
+
+        visit '/account', app: :mobile
+        click_button('Sign out')
+
+        wait.until do
+          expect(page).to have_button('Sign in')
+        end
+
+        visit saml_realm_zammad_accounts
+        find('#landingMobileKebabButton').click
+        expect(page).to have_css('#landingSignOutLink')
+      end
+    end
+
   end
 end