Disable user sync status by default

The newly introduced user status synchronization is enabled by default, and has resulted in a lot of customer complaints due to users reactivating by themselves.

As it seems to be an edge case to activate/deactivate users based on presence in the LDAP, we should make this functionality opt-in.

https://community.openproject.org/wp/43561

config/constants/settings/definitions.rb

@@ -533,7 +533,7 @@ Settings::Definition.define do
   # Update users' status through the synchronization job
   add :ldap_users_sync_status,
       format: :boolean,
-      default: true,
+      default: false,
       writable: false
 
   add :ldap_tls_options,

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

@@ -141,14 +141,14 @@ Duplicates in the unique attributes (login, email) are not allowed and a second
 
 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.
+### **Enabling status synchronization**
 
-### **Disabling status synchronization**
+If you wish to synchronize the account status from the LDAP, you can enable status synchronization using the following configuration:
 
-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: true`
+- (or the ENV variable `OPENPROJECT_LDAP__USERS__SYNC__STATUS=true`)
 
-- `ldap_users_sync_status: false` 
-- (or the ENV variable `OPENPROJECT_LDAP__USERS__SYNC__STATUS=false`) 
+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 the synchronization job
 

spec/services/ldap/synchronize_users_service_integration_spec.rb

@@ -23,56 +23,73 @@ describe Ldap::SynchronizeUsersService do
     let!(:user_aa729) { create :user, login: 'aa729', firstname: 'Foobar', auth_source: }
     let!(:user_bb459) { create :user, login: 'bb459', firstname: 'Bla', auth_source: }
 
-    it 'updates the attributes of those users' do
-      subject
+    context 'when user sync status is enabled',
+            with_config: { ldap_users_sync_status: true } do
 
-      user_aa729.reload
-      user_bb459.reload
+      it 'updates the attributes of those users' do
+        subject
 
-      expect(user_aa729.firstname).to eq 'Alexandra'
-      expect(user_aa729.lastname).to eq 'Adams'
-      expect(user_aa729.mail).to eq 'alexandra@example.org'
+        user_aa729.reload
+        user_bb459.reload
 
-      expect(user_bb459.firstname).to eq 'Belle'
-      expect(user_bb459.lastname).to eq 'Baldwin'
-      expect(user_bb459.mail).to eq 'belle@example.org'
-    end
+        expect(user_aa729.firstname).to eq 'Alexandra'
+        expect(user_aa729.lastname).to eq 'Adams'
+        expect(user_aa729.mail).to eq 'alexandra@example.org'
 
-    it 'updates one user if the other fails' do
-      allow(Users::UpdateService)
-        .to receive(:new)
-        .and_call_original
+        expect(user_bb459.firstname).to eq 'Belle'
+        expect(user_bb459.lastname).to eq 'Baldwin'
+        expect(user_bb459.mail).to eq 'belle@example.org'
+      end
 
-      allow(Users::UpdateService)
-        .to receive(:new)
-        .with(model: user_aa729, user: User.system)
-        .and_raise("Some bad error happening here")
+      it 'updates one user if the other fails' do
+        allow(Users::UpdateService)
+          .to receive(:new)
+                .and_call_original
 
-      subject
+        allow(Users::UpdateService)
+          .to receive(:new)
+                .with(model: user_aa729, user: User.system)
+                .and_raise("Some bad error happening here")
 
-      user_aa729.reload
-      user_bb459.reload
+        subject
 
-      expect(user_aa729.firstname).to eq 'Foobar'
-      expect(user_aa729.lastname).to eq 'Bobbit'
+        user_aa729.reload
+        user_bb459.reload
 
-      expect(user_bb459.firstname).to eq 'Belle'
-      expect(user_bb459.lastname).to eq 'Baldwin'
-      expect(user_bb459.mail).to eq 'belle@example.org'
-    end
+        expect(user_aa729.firstname).to eq 'Foobar'
+        expect(user_aa729.lastname).to eq 'Bobbit'
 
-    it 'reactivates the account if it is locked' do
-      user_aa729.lock!
+        expect(user_bb459.firstname).to eq 'Belle'
+        expect(user_bb459.lastname).to eq 'Baldwin'
+        expect(user_bb459.mail).to eq 'belle@example.org'
+      end
 
-      expect(user_aa729.reload).to be_locked
+      it 'reactivates the account if it is locked' do
+        user_aa729.lock!
 
-      subject
+        expect(user_aa729.reload).to be_locked
 
-      expect(user_aa729.reload).not_to be_locked
-      expect(user_aa729).to be_active
+        subject
+
+        expect(user_aa729.reload).not_to be_locked
+        expect(user_aa729).to be_active
+      end
+
+      context 'with a user that is in another LDAP' do
+        let(:auth_source2) { create :ldap_auth_source, name: 'Another LDAP' }
+        let(:user_foo) { create :user, login: 'login', auth_source: auth_source2 }
+
+        it 'does not touch that user' do
+          expect(user_foo).to be_active
+
+          subject
+
+          expect(user_foo.reload).to be_active
+        end
+      end
     end
 
-    context 'when requesting not to sync users status',
+    context 'when user sync status is disabled',
             with_config: { ldap_users_sync_status: false } do
       it 'does not reactivate the account if it is locked' do
         user_aa729.lock!
@@ -117,15 +134,18 @@ describe Ldap::SynchronizeUsersService do
   context 'with a user that is no longer in LDAP' do
     let(:user_foo) { create :user, login: 'login', auth_source: }
 
-    it 'locks that user' do
-      expect(user_foo).to be_active
+    context 'when user sync status is enabled',
+            with_config: { ldap_users_sync_status: true } do
+      it 'locks that user' do
+        expect(user_foo).to be_active
 
-      subject
+        subject
 
-      expect(user_foo.reload).to be_locked
+        expect(user_foo.reload).to be_locked
+      end
     end
 
-    context 'when requesting not to sync users status',
+    context 'when user sync status is disabled',
             with_config: { ldap_users_sync_status: false } do
       it 'does not lock that user' do
         expect(user_foo).to be_active
@@ -137,16 +157,4 @@ describe Ldap::SynchronizeUsersService do
     end
   end
 
-  context 'with a user that is in another LDAP' do
-    let(:auth_source2) { create :ldap_auth_source, name: 'Another LDAP' }
-    let(:user_foo) { create :user, login: 'login', auth_source: auth_source2 }
-
-    it 'does not touch that user' do
-      expect(user_foo).to be_active
-
-      subject
-
-      expect(user_foo.reload).to be_active
-    end
-  end
 end