Skip to content

Fixed #17414 - client-side TLS certificate didn't work in Google LDAP#17857

Merged
snipe merged 2 commits intogrokability:developfrom
uberbrady:fix_client_tls_ldap
Sep 15, 2025
Merged

Fixed #17414 - client-side TLS certificate didn't work in Google LDAP#17857
snipe merged 2 commits intogrokability:developfrom
uberbrady:fix_client_tls_ldap

Conversation

@uberbrady
Copy link
Copy Markdown
Member

@uberbrady uberbrady commented Sep 12, 2025

The way we had been setting the 'use client-side TLS certificates' option in LDAP was kinda working, but wasn't actually performing the actions in the right order. That worked OK in some versions of PHP - very likely due to some of the other ldap_set_option() bugs that had existed in older PHP versions. But, now that those bugs are fixed, this incorrect way we had been setting the option seems to have stopped working in later PHP versions.

This change moves the ldap_set_option() statements before the ldap_connect() statement, which looks like what they actually want you to do.

If someone is running the older, buggy versions of the PHP LDAP code, I suspect this will still work correctly - as we do the whole 'setting LDAP options before connect' thing already in the TLS-certificate-validity-ignoring code.

I'm going to see if I can get some tests going to make sure this works before we get it merged over to 'master'.

Fixes #17414

@uberbrady
Copy link
Copy Markdown
Member Author

I ran this on a test instance and it did seem to work. Probably want to still wait for another confirmation or two before we merge it though.

@viclou
Copy link
Copy Markdown
Member

viclou commented Sep 13, 2025

[FD-50814]

@snipe snipe merged commit dcc5388 into grokability:develop Sep 15, 2025
7 checks passed
@snipe snipe added this to the September 2025 Sprint milestone Sep 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants