From 614724aa6242619dd3bb48fcc4faf5cca894f55e Mon Sep 17 00:00:00 2001
From: Dominik Klein <dk@zammad.com>
Date: Wed, 27 Oct 2021 20:10:17 +0200
Subject: [PATCH] Maintenance: Improved authentication helper for developers.

---
 ...maintenance_remove_active_ldap_sessions.rb | 12 ++++++++
 lib/auth/backend/base.rb                      |  5 ++++
 lib/auth/backend/developer.rb                 |  7 +++++
 lib/auth/backend/internal.rb                  |  1 -
 ...enance_remove_active_ldap_sessions_spec.rb | 30 +++++++++++++++++++
 spec/lib/auth_spec.rb                         | 28 +++++++++++++++++
 6 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 db/migrate/20211028072158_maintenance_remove_active_ldap_sessions.rb
 create mode 100644 spec/db/migrate/maintenance_remove_active_ldap_sessions_spec.rb

diff --git a/db/migrate/20211028072158_maintenance_remove_active_ldap_sessions.rb b/db/migrate/20211028072158_maintenance_remove_active_ldap_sessions.rb
new file mode 100644
index 000000000..b88b561d9
--- /dev/null
+++ b/db/migrate/20211028072158_maintenance_remove_active_ldap_sessions.rb
@@ -0,0 +1,12 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+class MaintenanceRemoveActiveLdapSessions < ActiveRecord::Migration[6.0]
+  def change
+    return if !Setting.exists?(name: 'system_init_done')
+
+    # Only relevant for when ldap integration is used.
+    return if !Setting.get('ldap_integration')
+
+    ActiveRecord::SessionStore::Session.destroy_all
+  end
+end
diff --git a/lib/auth/backend/base.rb b/lib/auth/backend/base.rb
index ff3c584c1..347dfa75f 100644
--- a/lib/auth/backend/base.rb
+++ b/lib/auth/backend/base.rb
@@ -21,6 +21,7 @@ class Auth
       end
 
       def valid?
+        return false if password.blank? && password_required?
         return false if !perform?
 
         authenticated?
@@ -28,6 +29,10 @@ class Auth
 
       private
 
+      def password_required?
+        true
+      end
+
       def perform?
         raise NotImplementedError
       end
diff --git a/lib/auth/backend/developer.rb b/lib/auth/backend/developer.rb
index 4c63b5a83..4f6637fad 100644
--- a/lib/auth/backend/developer.rb
+++ b/lib/auth/backend/developer.rb
@@ -20,6 +20,13 @@ class Auth
         false
       end
 
+      # No password required for developer mode and test environment.
+      #
+      # @returns [Boolean] false
+      def password_required?
+        false
+      end
+
       # Overwrites the default behaviour to check for a allowed environment.
       #
       # @returns [Boolean] true if the environment is development or test.
diff --git a/lib/auth/backend/internal.rb b/lib/auth/backend/internal.rb
index 459c45c13..e398a1be0 100644
--- a/lib/auth/backend/internal.rb
+++ b/lib/auth/backend/internal.rb
@@ -21,7 +21,6 @@ class Auth
       #
       # @returns [Boolean] true if a internal password for the user is present.
       def perform?
-        return false if password.blank?
         return false if !user.verified && user.source == 'signup'
 
         user.password.present?
diff --git a/spec/db/migrate/maintenance_remove_active_ldap_sessions_spec.rb b/spec/db/migrate/maintenance_remove_active_ldap_sessions_spec.rb
new file mode 100644
index 000000000..0d4a81bff
--- /dev/null
+++ b/spec/db/migrate/maintenance_remove_active_ldap_sessions_spec.rb
@@ -0,0 +1,30 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe MaintenanceRemoveActiveLdapSessions, type: :db_migration do
+  before do
+    5.times do
+      ActiveRecord::SessionStore::Session.create(
+        session_id: SecureRandom.hex(16),
+        data:       SecureRandom.base64(10)
+      )
+    end
+  end
+
+  context 'without ldap integration' do
+    before { Setting.set('ldap_integration', false) }
+
+    it 'does not delete existing sessions' do
+      expect { migrate }.not_to change(ActiveRecord::SessionStore::Session, :count)
+    end
+  end
+
+  context 'with ldap integration' do
+    before { Setting.set('ldap_integration', true) }
+
+    it 'deletes all existing sessions' do
+      expect { migrate }.to change(ActiveRecord::SessionStore::Session, :count).to(0)
+    end
+  end
+end
diff --git a/spec/lib/auth_spec.rb b/spec/lib/auth_spec.rb
index 7bedbcefb..a95c26acd 100644
--- a/spec/lib/auth_spec.rb
+++ b/spec/lib/auth_spec.rb
@@ -168,6 +168,30 @@ RSpec.describe Auth do
         allow(Ldap::User).to receive(:new).with(any_args).and_return(ldap_user)
       end
 
+      shared_examples 'check empty password' do
+        before do
+          # Remove adapter from auth developer setting, to avoid execution for this test case, because of special empty
+          # password handling in adapter.
+          Setting.set('auth_developer', {})
+        end
+
+        context 'with empty password string' do
+          let(:password) { '' }
+
+          it 'returns false' do
+            expect(instance.valid?).to be false
+          end
+        end
+
+        context 'when password is nil' do
+          let(:password) { nil }
+
+          it 'returns false' do
+            expect(instance.valid?).to be false
+          end
+        end
+      end
+
       context 'with a ldap user without internal password' do
         let(:user) { create(:user, source: 'Ldap') }
         let(:password) { password_ldap }
@@ -197,6 +221,8 @@ RSpec.describe Auth do
             expect { instance.valid? }.not_to change { user.reload.login_failed }
           end
         end
+
+        include_examples 'check empty password'
       end
 
       context 'with a ldap user which also has a internal password' do
@@ -238,6 +264,8 @@ RSpec.describe Auth do
             expect(instance.valid?).to be true
           end
         end
+
+        include_examples 'check empty password'
       end
     end
   end