[Feature] (optional) support for memberof in emulator

[cheesegrits]

Would you be open to a PR which adds optional support for memberof in the Directory Emulator?

I was finding that I just can't do feature or unit testing of my app without it. In fact my internal app LDAP handling logic breaks, because things like $group->members()->count() won't work without memberof. So I started poking around in EmulatesQueries and EmulatesModelQueries, and added code to automatically add/remove memberof whenever member is modified on an object.

It feels a little hacky, but the more I think about the code I'm adding, the more I realize that it's basically what an LDAP server does with the memberof overlay. When a 'member' attribute gets added to anything, find the DN for the new member, add a memberof back to the "anything". When a member gets removed from a thing, find the DN being removed, and remove the memberof for the "thing". It doesn't need any knowledge of the Laravel model side of things. Just internal housekeeping to sync up 'memberof' with 'member' in the emulation models on add/remove/modify/delete/insert.

So for example in EmulatesModelQueries::add() ...

public function add(string $dn, array $attributes): bool
    {
        if (! $model = $this->find($dn)) {
            return false;
        }

        $members = [];

        foreach ($attributes as $name => $values) {
            $model->{$name} = array_merge($model->{$name} ?? [], Arr::wrap($values));

            // if feature is enabled and it's a 'member' attribute, populate our $members for 'memberof' handling
            if (config('ldap.emulate_memberof') && $name === 'member') {
                $members = Arr::wrap($values);
            }
        }

        $model->save();

        // for any $members that were added, find the member's model by DN, and add a 'memberof' for the $dn
        foreach ($members as $member) {
            $memberModel = $this->findEloquentModelByDn($member);

            if ($memberModel) {
                $this->applyBatchModificationToModel($memberModel, [
                    BatchModification::KEY_ATTRIB  => 'memberof',
                    BatchModification::KEY_MODTYPE => LDAP_MODIFY_BATCH_ADD,
                    BatchModification::KEY_VALUES  => [$dn],
                ]);
            }
        }

        return true;
    }

I've similarly modified remove(), and insert() and delete() in EmulatesQueries .

And ... it works! My feature and unit testing now works end to end, with tests like this now working end to end, with the user groups() and group members() relations working both ways.

it('adds a new agency user to ldap and adds them to a joinable channel and the corresponding LDAP group', function () {
    Bus::fake();

    $agency  = Agency::factory()->activeCotak()->create();
    expect($agency)->not->toBeNull();

    $channel = Channel::factory()->visibility()->joinable()->active()->agency($agency)->create();
    expect($channel)->not->toBeNull();

    // Channel creation events should have created an LDAP group with a single dummy user
    $ldapChannel = $channel->getCotakGroup();
    expect($ldapChannel)->not->toBeNull();
    expect($ldapChannel->members()->count())->toBe(1);
    expect($ldapChannel->members()->first()->getName())->toBe(config('coldap.users.dummy_ldap_user'));

    $user = User::factory()->agencyUser($agency)->cotak()->active()->ldapPassword()->create();
    expect($user)->not->toBeNull();

    // When attaching a channel, User model pivot events should handle syncing the user to the corresponding LDAP group
    $user->channels()->attach($channel);

    $ldapUser = $user->getCotakUser();
    expect($ldapUser)->not->toBeNull()
        ->and($ldapChannel->members()->count())->toBe(2)
        ->and($ldapUser->groups()->exists($ldapChannel))->toBeTrue()
        ->and($ldapChannel->members()->exists($ldapUser))->toBeTrue();
});

I need to do a little more work, but I think I could PR a working memberof emulation fairly soon.

As I literally can't really test my app without this (without standing up an actual LDAP server as part of the test environment), I'm kinda hoping you'd consider a PR.

[stevebauman]

Hey @cheesegrits, thanks for the detailed issue, I appreciate it.

Unfortunately this will only work when testing with model relationships that utilize the member and memberof attribute. Some LDAP distributions don't utilize the memberof. For example, some use the uniqueMember, memberUid, and gidNumber attributes. If we can, I'd really prefer to find a cross-distro solution for attaching/detaching relationships in Directory Emulator that works without any additional configuration.

[stevebauman]

I think I may have a solution for this that involves implementing relationship based events that would be cross-distro compatible, such as Attaching/Attached, Detaching/Detached:

namespace LdapRecord\Models\Events;

use LdapRecord\Models\Model;
use LdapRecord\Models\Relations\Relation;

class Detached
{
    public function __construct(
        public Relation $relation,
        public Model $model, 
        public string $attribute,
        public string $value
    ) {
    }
}

// In the Directory Emulator (psuedo-code)...

Container::getInstance()->getDispatcher()->listen(Detached::class, function (Detached $event) {
    // Perform inverse deletion...
});

I'll see if this strategy works and get back to you 👍

[cheesegrits]

Cool! Yes, I like that approach much better. Although you'd need to come up with solutions for things like deleting the parent record, renaming the parent record and (most annoyingly) directly modifying attributes.

Meanwhile, I have a fully working set of patches for member/memberof for (as far as I can tell) all possible ways the 'member' attribute might get changed, which allows me to test my app while you work on this. Lmk if there's anything I can do to help or test.

[cheesegrits]

BTW, the reason I put it behind a feature flag was twofold ...

a) people who have already written tests which involve faking out the memberof might find them failing if the underlying code suddenly started doing it for them. So with a flag, their existing tests will be unaffected, until they enable the flag.

b) memberof is a "feature flag" in OpenLDAP. It's not a built in part of the server, it's an optional overlay. So setting a feature flag for testing is the analog of whether you have enabled the overlay or not on your production LDAP server.

[cheesegrits]

Unfortunately this will only work when testing with model relationships that utilize the member and memberof attribute. Some LDAP distributions don't utilize the memberof. For example, some use the uniqueMember, memberUid, and gidNumber attributes. If we can, I'd really prefer to find a cross-distro solution for attaching/detaching relationships in Directory Emulator that works without any additional configuration.

How would you know which attributes you need to maintain, without some form of configuration? Or would that be an attribute on the DirectoryEmulator which we could set when setting it up, like ...

        $connection = DirectoryEmulator::setup(connection: 'default', membership: 'memberof');

BTW, I've hammered on my hacky implementation pretty hard the last week, writing some comprehensive tests for that TAK application, and it works well. So I'm keen to get something official implemented.

I've looked at the LdapRecord event dispatcher, and your examples above, and think I could probably convert what I've got to using events, at least as a starting point for a PR. I'm just unclear on the question of how to know which strategy the inverse member relationship would use.

As usual, thanks for your time, it is much appreciated.