[Ldap] Improve the LDAP component

[csarrazi]

A few issues are remaining, and will likely need to be fixed after release 3.0 (3.1 or later):

Here is the list of issues which were mentioned in #14602:

• Add a way to configure all ldap_set_option entries.
  • Dereferencing
  • Timeout
  • Debug level
  • ...
• Handle multiple connections

I'm separating all these from the current PR (#15994), as I am currently the only person working on the LDAP component on a regular basis, and won't have time to handle these remaining features for Symfony's 2.8 / 3.0 releases.

[cilefen]

@csarrazi Thank you so much for the time you have put into this. In implementing this in 2.8, we have found that authentication is limited to only users whose DNs fit the configured dn_string, for example:

dn_string: "uid={username},ou=people,dc=example,dc=com"

A user with "uid=foo,ou=something,ou=people,dc=example,dc=com" cannot authenticate.

The immediate reason is that after discovering information from ldap in the ->find() method, the DN is discarded when LdapUserProvider::loadUser() creates the User instance.

Because the DN is discarded, later LdapBindAuthenticationProvider::checkAuthentication() recombines the basic username with what is configured in dn_string:

$dn = str_replace('{username}', $username, $this->dnString);

It would seem if User were extended to store the DN, it would not be necessary to set dn_string, and users in various OU's could authenticate.

[csarrazi]

Hum, could you be a bit more precise regarding your configuration. Are you using the Ldap user provider along with the http_basic_ldap authentication provider?

[ChadSikorra]

Also (unless I'm overlooking something), the LDAP component currently uses the ldap_escape function in the client, which is only available in PHP 5.6. But the PHP requirement for the component is 5.3?

[nicolas-grekas]

There is a polyfill dependency that provides a php implementation of ldap_escape in case you're not on php 5.6

[ChadSikorra]

Ah, then I did overlook something hah (: Thanks. That's the same function I've seen in a SO answer. I do notice that it doesn't encode carriage returns properly when escaping DNs. I've mentioned it in the comments on SO where it's posted too, but I don't think I ever got a response.

Huge edge case I guess (who in their right mind puts a carriage return in a DN..), but it's listed in the RFC.

[nicolas-grekas]

Can you please open an issue on the symfony-polyfill? And if you have a patch, PR is welcomed :-)

[csarrazi]

Regarding the carriage return, we should add a test case in both the PHP extension, and in symfony/polyfill (as the test cases are taken from the PHP extension).

[cilefen]

@csarrazi If you want me to open a new issue, say the word. Here is the configuration:

security:
    providers:
        in_memory:
            memory: ~
        app_users:
            ldap:
                service: app.ldap
                base_dn: %ldap.base_dn%
                search_dn: null
                search_password: null
                filter: (uid={username})
                default_roles: ROLE_USER
    firewalls:
        secure:
            provider:  app_users
            stateless: true
            pattern:   ^/secure
            http_basic_ldap:
                service: app.ldap
                dn_string: "uid={username},ou=people,dc=example,dc=com"
        dev:
            pattern: ^/(_(profiler|wdt)|css|images|js)/
            security: false
        main:
            anonymous: ~

[csarrazi]

Should be in a new issue, but this feedback is great regardless! :)

[cilefen]

#16823

[ChadSikorra]

@csarrazi The version of LDAP escape in polyfill actually already does more than the official version of ldap_escape. The one in polyfill handles leading/trailing spaces in a DN (which is correct, per the RFC), whereas the official version of the function does not.

I can submit a PR with new tests and also account for the carriage return. But should that carriage return be handled properly in the polyfill function if the official ldap_escape doesn't handle it properly? Or should the polyfill version be left alone to behave exactly like the PHP version and the additional encoding be handled elsewhere until the PHP version is fixed?

[nicolas-grekas]

The Polyfill is expected to behave exactly the same as the reference php implementation, so if you know some different behavior, please submit a test case for it, and a fix if possible :-)

[ChadSikorra]

Submitted the polyfill fix here: symfony/polyfill#14 .

[Xeyos88]

This is a complete example also with authentication form? Why I can not authenticate.
Before I make a bind anonymous and should then perform the bind for the form but not function.

[csarrazi]

@Xeyos88 The documentation will be updated as soon as the component will be marked as stable, which should be good for 3.1. Until then, as mentioned in the subtree split, the component is still a work in progress.