fix: push users disabled by the LDAP sync to SCIM (#1834)

backend/internal/ldapsync/service_test.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
+	"time"
 
 	"github.com/go-ldap/ldap/v3"
 	"github.com/stretchr/testify/assert"
@@ -14,6 +15,7 @@ import (
 	"github.com/pocket-id/pocket-id/backend/internal/appconfig"
 	"github.com/pocket-id/pocket-id/backend/internal/apperror"
 	"github.com/pocket-id/pocket-id/backend/internal/model"
+	datatype "github.com/pocket-id/pocket-id/backend/internal/model/types"
 	"github.com/pocket-id/pocket-id/backend/internal/service"
 	"github.com/pocket-id/pocket-id/backend/internal/storage"
 	testutils "github.com/pocket-id/pocket-id/backend/internal/utils/testing"
@@ -359,6 +361,46 @@ func TestLdapServiceSyncAllHandlesDuplicateLDAPIDsInSingleRun(t *testing.T) {
 	assert.ElementsMatch(t, []string{"alice"}, usernames(groups[0].Users))
 }
 
+func TestLdapServiceSyncAllBumpsUpdatedAtOfSoftDeletedUsers(t *testing.T) {
+	service, db := newTestLdapService(t, newFakeLDAPClient(ldapSearchResult(), ldapSearchResult()))
+
+	lastSync := datatype.DateTime(time.Now().Add(-time.Hour))
+	goneLdapID := "u-gone"
+	disabledLdapID := "u-disabled"
+
+	require.NoError(t, db.Create(&model.User{
+		Username:  "gone",
+		Email:     new("gone@example.com"),
+		LdapID:    &goneLdapID,
+		UpdatedAt: &lastSync,
+	}).Error)
+
+	require.NoError(t, db.Create(&model.User{
+		Username:  "disabled",
+		Email:     new("disabled@example.com"),
+		LdapID:    &disabledLdapID,
+		Disabled:  true,
+		UpdatedAt: &lastSync,
+	}).Error)
+
+	err := service.SyncAll(t.Context(), defaultTestLDAPAppConfig())
+	require.NoError(t, err)
+
+	// The SCIM sync only pushes users whose UpdatedAt is newer than the remote copy, so disabling a user must bump it
+	var gone model.User
+	require.NoError(t, db.First(&gone, "ldap_id = ?", goneLdapID).Error)
+	assert.True(t, gone.Disabled)
+	require.NotNil(t, gone.UpdatedAt)
+	assert.True(t, gone.UpdatedAt.ToTime().After(lastSync.ToTime()))
+
+	// A user that an earlier sync already disabled is unchanged and keeps its timestamp
+	var disabled model.User
+	require.NoError(t, db.First(&disabled, "ldap_id = ?", disabledLdapID).Error)
+	assert.True(t, disabled.Disabled)
+	require.NotNil(t, disabled.UpdatedAt)
+	assert.Equal(t, lastSync.ToTime().Unix(), disabled.UpdatedAt.ToTime().Unix())
+}
+
 func TestLdapServiceSyncAllSetsAdminFromGroupMembership(t *testing.T) {
 	tests := []struct {
 		name        string

backend/internal/service/user_service.go

@@ -723,11 +723,15 @@ func (s *UserService) ResetProfilePicture(ctx context.Context, userID string) er
 // DisableUserInternal disables a user within an existing transaction
 // It's exported for the LDAP sync, which soft-deletes users that are no longer in the directory
 func (s *UserService) DisableUserInternal(ctx context.Context, tx *gorm.DB, userID string) error {
+	// Bump UpdatedAt so the SCIM sync pushes the disabled state, but only when the user was still enabled
 	err := tx.
 		WithContext(ctx).
 		Model(&model.User{}).
-		Where("id = ?", userID).
-		Update("disabled", true).
+		Where("id = ? AND disabled = ?", userID, false).
+		Updates(map[string]any{
+			"disabled":   true,
+			"updated_at": datatype.DateTime(time.Now()),
+		}).
 		Error
 
 	if err != nil {