[PM-33583] AD User Filter Timeout issue

[JaredScar]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-33583

📔 Objective

Fixes issue in DC where timeout makes it seem like sync succeeds and brings back no results... Passing timeLimit to server allows the server to use it's own query policy for timeout instead.

Before
directory-connector-timeout-issue.webm

After
dc-no-longer-times-out-on-client-side.webm

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This revision replaces the earlier timeLimit: 0 with an explicit 5-minute server-side search limit (LdapSearchTimeLimitSeconds) applied in the single shared LdapDirectoryService.search() helper, so it covers the user, deleted-user (AD tombstone), and group queries consistently. On Active Directory the effective limit is still capped by the server's MaxQueryDuration (120s by default), so the behavior demonstrated in the linked before/after recordings is preserved while avoiding an unbounded wait on servers with looser policies. The new unit spec stubs ldapts and asserts the option is present on every search issued by getEntries() across the users-only, users+groups, and AD deleted-users paths, which locks in the regression risk of a future search call site dropping the option. No security, credential-handling, or data-exposure concerns were identified, and error propagation from getEntries() is unchanged.

Code Review Details

No blocking findings.

Both prior review threads appear addressed by this revision: the unbounded-search question by the explicit 300-second limit, and the request for verification/test coverage by libs/services/directory-services/ldap-directory.service.spec.ts.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (e0f51d3) to head (b079d5c).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@     Coverage Diff      @@
##   main   #1210   +/-   ##
============================
============================

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

❓ QUESTION: With timeLimit: 0 there is no upper bound on how long a search can block — was that acceptable, or would a generous explicit limit be safer?

Details

ldapts.ClientOptions.timeout is not set in bind(), so there is no client-side operation timeout either. The search now relies entirely on the server's query policy:

• Active Directory: MaxQueryDuration defaults to 120s, so bounded.
• OpenLDAP: olcTimeLimit defaults to 3600s, and the rootdn is exempt from limits entirely — a sync bound as rootdn could hang with no feedback to the user or CLI cron job.

If unbounded waits are not desired, either an explicit generous timeLimit (e.g. 300–600 seconds) or a client-level timeout in the ldapts.ClientOptions built in bind() would keep the fix for the AD case while still failing loudly on a truly stuck query.

[BTreston]

This seems ok on the surface, but I have no idea if it actually fixes the issue... We need to be able to guarantee this fixes the issue and provide test coverage for it.

Jared signed off in Brandon's absence