Merge pull request #10701 from opf/fix/42485/disable-user-status-sync

Provide a way to disable user status syncing in LDAP user sync

app/services/ldap/base_service.rb

@@ -18,15 +18,13 @@ def perform
       raise NotImplementedError
     end
 
-    # rubocop:disable Metrics/AbcSize
+    protected
+
     def synchronize_user(user, ldap_con)
       Rails.logger.debug { "[LDAP user sync] Synchronizing user #{user.login}." }
 
       update_attributes = user_attributes(user.login, ldap_con)
-      if update_attributes.nil? && user.persisted?
-        Rails.logger.info { "Could not find user #{user.login} in #{ldap.name}. Locking the user." }
-        user.update_column(:status, Principal.statuses[:locked])
-      end
+      lock_user!(user) if update_attributes.nil? && user.persisted?
       return unless update_attributes
 
       if user.new_record?
@@ -35,7 +33,6 @@ def synchronize_user(user, ldap_con)
         try_to_update(user, update_attributes)
       end
     end
-    # rubocop:enable Metrics/AbcSize
 
     # Try to create the user from attributes
     def try_to_update(user, attrs)
@@ -44,8 +41,7 @@ def try_to_update(user, attrs)
         .call(attrs)
 
       if call.success?
-        # Ensure the user is activated
-        call.result.update_column(:status, Principal.statuses[:active])
+        activate_user!(user)
         Rails.logger.info { "[LDAP user sync] User '#{call.result.login}' updated." }
       else
         Rails.logger.error { "[LDAP user sync] User '#{user.login}' could not be updated: #{call.message}" }
@@ -64,6 +60,32 @@ def try_to_create(attrs)
       end
     end
 
+    ##
+    # Locks the given user if this is what the sync service should do.
+    def lock_user!(user)
+      if OpenProject::Configuration.ldap_users_sync_status?
+        Rails.logger.info { "Could not find user #{user.login} in #{ldap.name}. Locking the user." }
+        user.update_column(:status, Principal.statuses[:locked])
+      else
+        Rails.logger.info do
+          "Could not find user #{user.login} in #{ldap.name}. Ignoring due to ldap_users_sync_status being unset"
+        end
+      end
+    end
+
+    ##
+    # Activates the given user if this is what the sync service should do.
+    def activate_user!(user)
+      if OpenProject::Configuration.ldap_users_sync_status?
+        Rails.logger.info { "Activating #{user.login} due to it being synced from LDAP #{ldap.name}." }
+        user.update_column(:status, Principal.statuses[:active])
+      else
+        Rails.logger.info do
+          "Would activate #{user.login} through #{ldap.name} but ignoring due to ldap_users_sync_status being unset."
+        end
+      end
+    end
+
     ##
     # Get the user attributes of a single matching LDAP entry.
     #

config/constants/settings/definitions.rb

@@ -526,6 +526,12 @@
       value: false,
       writable: false
 
+  # Update users' status through the synchronization job
+  add :ldap_users_sync_status,
+      format: :boolean,
+      value: true,
+      writable: false
+
   add :ldap_tls_options,
       value: {},
       writable: false

docs/installation-and-operations/configuration/environment/README.md

@@ -135,5 +135,7 @@ OPENPROJECT_SECURITY__BADGE__URL (default="https://releases.openproject.com/v1/c
 OPENPROJECT_MIGRATION__CHECK__ON__EXCEPTIONS (default=true)
 OPENPROJECT_SHOW__PENDING__MIGRATIONS__WARNING (default=true)
 OPENPROJECT_SHOW__WARNING__BARS (default=true) 
-OPENPROJECT_SHOW__STORAGE__INFORMATION (default=true)    
+OPENPROJECT_SHOW__STORAGE__INFORMATION (default=true)
+OPENPROJECT_LDAP__USERS__SYNC__STATUS (default=true)
+OPENPROJECT_LDAP__USERS__DISABLE__SYNC__JOB(default=false)
 ```

docs/system-admin-guide/authentication/ldap-authentication/README.md

@@ -137,3 +137,25 @@ With the [OpenProject Enterprise Edition](https://www.openproject.org/enterprise
 OpenProject supports multiple LDAP connections to source users from. The user's authentication source is remembered the first time it is created (but can be switched in the administration backend). This ensures that the correct connection / LDAP source will be used for the user.
 
 Duplicates in the unique attributes (login, email) are not allowed and a second user with the same attributes will not be able to login. Please ensure that amongst all LDAP connections, a unique attribute is used that does not result in conflicting logins.
+
+
+
+## LDAP user synchronization
+
+By default, OpenProject will synchronize user account details (name, e-mail, login) and their account status from the LDAP through a background worker job every 24 hours. 
+
+The user will be ensured to be active if it can be found in LDAP. Likewise, if the user cannot be found in the LDAP, its associated OpenProject account will be locked.
+
+### **Disabling status synchronization**
+
+If you wish to synchronize account data from the LDAP, but not synchronize the status to the associated OpenProject account, you can do so with the following configuration variable:
+
+- `ldap_users_sync_status: false` 
+- (or the ENV variable `OPENPROJECT_LDAP__USERS__SYNC__STATUS=false`) 
+
+### Disabling the synchronization job
+
+If for any reason, you do not wish to perform the synchronization at all, you can also remove the synchronization job from being run at all with the following variable:
+
+- `ldap_users_disable_sync_job: true` 
+- (or the ENV variable `OPENPROJECT_LDAP__USERS__DISABLE__SYNC__JOB=true`) 

spec/services/ldap/synchronize_users_service_integration_spec.rb

@@ -72,6 +72,20 @@
       expect(user_aa729).to be_active
     end
 
+    context 'when requesting not to sync users status',
+            with_config: { ldap_users_sync_status: false } do
+      it 'does not reactivate the account if it is locked' do
+        user_aa729.lock!
+
+        expect(user_aa729.reload).to be_locked
+
+        subject
+
+        expect(user_aa729.reload).to be_locked
+        expect(user_aa729).not_to be_active
+      end
+    end
+
     context 'when requesting only a subset of users' do
       let!(:user_cc414) { create :user, login: 'cc414', auth_source: auth_source }
 
@@ -110,6 +124,17 @@
 
       expect(user_foo.reload).to be_locked
     end
+
+    context 'when requesting not to sync users status',
+            with_config: { ldap_users_sync_status: false } do
+      it 'does not lock that user' do
+        expect(user_foo).to be_active
+
+        subject
+
+        expect(user_foo.reload).to be_active
+      end
+    end
   end
 
   context 'with a user that is in another LDAP' do