Mocking Symfony Ldap::create for unit tests

Question

Recently I have been working on an LDAP authentication provider for MediaWiki. In my mind, I have been trying to tackle this issue for a number of days now and cannot come up with a solution.

Context

The way I have developed this plugin is to allow configuration of a number of servers to which we will connect. If we cannot connect to one server, we will try the next... And so on until all are exhausted.

To facilitate this, I have a function in my class that loops over the servers attempting a connection until one succeeds:

private function connect( LdapAuthenticationRequest $req ) {
    $dn = $this->config->get( 'BindDN' )[$req->domain];
    $pass = $this->config->get( 'BindPass' )[$req->domain];
    $servers = $this->config->get( 'Servers' )[$req->domain];
    $encryption = $this->config->get( 'EncryptionType' )[$req->domain];
    if ( false === $dn ) {
        $msgkey = 'ldapauth-attempt-bind-search';
        $bind_with = [ null, null ];
    } else {
        $msgkey = 'ldapauth-attempt-bind-dn-search';
        $bind_with = [ $dn, $pass ];
    }
    $message = new Message( $msgkey, [
        'dn' => "{$dn}@{$req->domain}",
    ] );
    $this->logger->info( $message->text() );
    foreach ( $servers as $server ) {
        if ( false === $server ) {
            continue;
        }
        $ldap = Ldap::create( 'ext_ldap', [
            'host' => $server,
            'encryption' => $encryption
        ] );
        // Attempt bind - on failure, throw an exception
        try {
            call_user_func_array( [ $ldap, 'bind' ], $bind_with );
            $this->server = $server;
            $this->encryption = $encryption;
            // log successful bind
            $msgkey = 'ldapauth-bind-success';
            $message = wfMessage( $msgkey )->text();
            $this->logger->info( $message );
            return $ldap;
        } catch ( SymException $e ) {
            if ( false === $dn ) {
                $msgkey = 'ldapauth-no-bind-search';
            } else {
                $msgkey = 'ldapauth-no-bind-dn-search';
            }
            $message = new Message( $msgkey, [
                'dn' => "{$dn}@{$req->domain}",
            ] );
            $message = $message->text();
            $this->logger->info( $message );
            $this->logger->debug( $e->getMessage() );
        }
    }

I have been trying to come up with a better way to do this, one that would permit me to better unit-test this class, but thus far I am drawing blanks.

A big part of the reason that I am stuck on this issue is that Symfony's LDAP adapter is essentially hard-coupled into my code, as the call to connect is a static call into Symfony's codebase. i.e. I cannot pass in a connector instance of some description that would then attempt the connection. Do I simply wrap Ldap::create with my own connection wrapper, perhaps?

Answer 1

Since you are using Symfony, I guess your best bet would be to inject LDap object using the framework's dependency injection. However I am not an expert in Symfony. So as a simple hack, I would do it like this :

  private function connect($req)
    {
        $dn = $this->config->get('BindDN')[$req->domain];
        $pass = $this->config->get('BindPass')[$req->domain];
        $servers = $this->config->get('Servers')[$req->domain];
        $encryption = $this->config->get('EncryptionType')[$req->domain];
        if (false === $dn) {
            $msgkey = 'ldapauth-attempt-bind-search';
            $bind_with = [null, null];
        } else {
            $msgkey = 'ldapauth-attempt-bind-dn-search';
            $bind_with = [$dn, $pass];
        }
        $message = new Message($msgkey, [
            'dn' => "{$dn}@{$req->domain}",
        ]);
        $this->logger->info($message->text());
        foreach ($servers as $server) {
            if (false === $server) {
                continue;
            }
            $ldap = $this->createLDAPObject($server, $encryption);
            // Attempt bind - on failure, throw an exception
            try {
                call_user_func_array([$ldap, 'bind'], $bind_with);
                $this->server = $server;
                $this->encryption = $encryption;
                // log successful bind
                $msgkey = 'ldapauth-bind-success';
                $message = wfMessage($msgkey)->text();
                $this->logger->info($message);
                return $ldap;
            } catch (SymException $e) {
                if (false === $dn) {
                    $msgkey = 'ldapauth-no-bind-search';
                } else {
                    $msgkey = 'ldapauth-no-bind-dn-search';
                }
                $message = new Message($msgkey, [
                    'dn' => "{$dn}@{$req->domain}",
                ]);
                $message = $message->text();
                $this->logger->info($message);
                $this->logger->debug($e->getMessage());
            }
        }
    }

    /**
     * @param $server
     * @param $encryption
     * @return mixed
     */
    public function createLDAPObject($server, $encryption)
    {
        return Ldap::create('ext_ldap', [
            'host' => $server,
            'encryption' => $encryption
        ]);
    }

Then, you can mock the member method createLDAPObject instead of mocking the static method Ldap::create, which should be easier.

However, I would recommend that you refactor your code, so that it is more readable and testable.

1- First of all, call_user_func_array() is not really test-friendly, and I think your requirements here are not too dynamic so you can replace that line with $ldap->bind($bind_with[0],$bind_with[1]);

2- Your connect method is too large to be tested. Please read about Code Smells - Long Methods

3- The method can be refactored into smaller version by decoupling the presentation from the logic. Like for example you are getting the Message object to get the text from the $msgkey just to log it, which is not helping in code readability and test-ability.

These are my first thoughts on the thing :)

Happy coding & testing :)