Browse Source

Maintenance: Improved authentication helper for developers.

Dominik Klein 3 years ago
parent
commit
614724aa62

+ 12 - 0
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

+ 5 - 0
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

+ 7 - 0
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.

+ 0 - 1
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?

+ 30 - 0
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

+ 28 - 0
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