fix: search terms for binary fields will be converted if possible

[jvillafanez]

Related to https://github.com/owncloud/enterprise/issues/6032

The app will take care of converting binary attributes when searching for them.

For example, if the user_ldap app has been configured the "user search attributes" with "objectguid" (among other attributes), searching by "36E271F3" might return results, assuming the converted value we have from binary contains those characters.
Note that the converted value will also be stored as search term for the account in the "oc_account_terms" table.

[jvillafanez]

Failing tests are known: #814

[jvillafanez]

I don't think I can add tests for the changes in the Access class easily. Need to change attributes in the connection class, which are accessed using magic methods. It doesn't seem possible to mock those.

[sonarqubecloud]

Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

2 New issues
0 Security Hotspots
17.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[pako81]

merge this?

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
17.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[dj4oC]

Adds binary-attribute conversion (via ConverterHub) into the LDAP search-filter builders and UserEntry::getSearchTerms(), so binary attributes like objectguid can be searched by their string form.

Findings

Security/correctness — needs human verification before merge (LDAP attribute mapping is a never-auto-fix zone):

• In both getAdvancedFilterPartForSearch() and getFilterPartForSearch(), the converted value is escaped a second time: $this->prepareSearchTerm($converterHub->str2filter($attr, $word)). If str2filter() already returns an LDAP-filter-safe encoded form (e.g. a \hh\hh\hh-style hex escape for binary GUIDs, which is the standard way to filter on binary attributes per RFC 4515), then running that through a generic prepareSearchTerm() escaper again risks double-escaping the backslashes and corrupting the filter — likely breaking the search silently (filter would just never match) rather than being exploitable, but it should be confirmed against a real LDAP directory/mock, not just the unit test in this diff, which only exercises bin2str() on the read path (UserEntryTest::testGetSearchTermsWithConversion), not the filter-building path with the double-escape.
• No test covers getAdvancedFilterPartForSearch()/getFilterPartForSearch() with a converter-backed attribute — the one new test only covers getSearchTerms().

Stability: the catch (ConverterException $e) fallback paths look correct (fall back to the unconverted, still-escaped term rather than failing the whole search).

Verdict

Commenting, not blocking outright — but per policy, LDAP attribute-conversion/mapping changes need a maintainer to confirm the filter round-trips correctly against a real directory (or a filter-level test) before merge, given the double-escape risk above.

🤖 Automated review by Claude Code (security · stability · performance · coverage)

---

Generated by Claude Code