DirectoryTree / LdapRecord-Laravel

Multi-domain LDAP Authentication & Management for Laravel.
https://ldaprecord.com/docs/laravel/v3
MIT License
492 stars 51 forks source link

[Bug]: Custom connection is not used with certain query methods. #590

Closed DijkeMark closed 10 months ago

DijkeMark commented 10 months ago

Environment:

Describe the bug:

I am using multiple custom classes that extend the ActiveDirectory classes in which I (among other things) set a custom connection. In the 3.0.8 update, I am no longer able to use a custom connection when using the ::find() method on the custom classes.

The ::find() calls will simply return null, which is not true, considering I can retrieve all records using ::all() which returns all rows I expect.

The same goes for using ->refresh() which returns TypeError: LdapRecord\Models\Model::fresh(): Return value must be of type App\Models\Ldap\User|false, null returned

See the following example:

use App\Models\Ldap\User as CustomUser;
use LdapRecord\Models\ActiveDirectory\Group;
use LdapRecord\Models\ActiveDirectory\User;
use Tests\TestCase;

/**
 * @test
 */
public function it_should_use_correct_connection_for_find(): void
{
    // Create new user
    $user = new User([
        'cn' => '678900jd',
        'sn' => 'Jane Doe',
        'userprincipalname' => '678900jd@test.nl',
        'displayname' => 'Jane',
    ]);
    $user->save();

    // Create new CustomUser
    $customUser = new CustomUser([
        'cn' => '012345jd',
        'sn' => 'John Doe',
        'userprincipalname' => '01234jod@test.nl',
        'displayname' => 'John',
    ]);
    $customUser->save();

    // Retrieving all users seems to work
    $this->assertCount(1, User::all());
    $this->assertEquals('cn=678900jd,dc=default,dc=com', User::all()->first()->getDn());

    // Retrieving all custom users seems to work
    $this->assertCount(1, CustomUser::all());
    $this->assertEquals('cn=012345jd,dc=other,dc=com', CustomUser::all()->first()->getDn());

    // Find a user succeeds
    $userFind = User::find('cn=678900jd,dc=default,dc=com');
    $this->assertEquals('cn=678900jd,dc=default,dc=com', $userFind?->getDn());

    // Find a custom user fails
    $customUserFind = CustomUser::find('cn=012345jd,dc=other,dc=com');
    $this->assertEquals('cn=012345jd,dc=other,dc=com', $customUserFind?->getDn());

    // Refresh a user succeeds
    $user->refresh();

    // Refresh a custom user fails
    $customUser->refresh();
}

The custom user class:

use LdapRecord\Models\ActiveDirectory\User as ActiveDirectoryUser;

class User extends ActiveDirectoryUser
{
    protected ?string $connection = 'other';
}

The TestCase class (which is the default TestCase class in the Test directory):

abstract class TestCase extends BaseTestCase
{
    use CreatesApplication;

    protected function setUp(): void
    {
        parent::setUp();

        DirectoryEmulator::setup('default');
        DirectoryEmulator::setup('other');
    }

    protected function tearDown(): void
    {
        DirectoryEmulator::tearDown();

        parent::tearDown();
    }
}

The connections in ldap.php config file

'connections' => [
    'default' => [
        'hosts' => [env('LDAP_HOST', '127.0.0.1')],
        'username' => env('LDAP_USERNAME', 'cn=user,dc=default,dc=com'),
        'password' => env('LDAP_PASSWORD', 'secret'),
        'port' => env('LDAP_PORT', 389),
        'base_dn' => env('LDAP_BASE_DN', 'dc=default,dc=com'),
        'timeout' => env('LDAP_TIMEOUT', 5),
        'use_ssl' => env('LDAP_SSL', false),
        'use_tls' => env('LDAP_TLS', false),
    ],
    'other' => [
        'hosts' => [env('LDAP_HOST', '127.0.0.1')],
        'username' => env('LDAP_USERNAME', 'cn=user,dc=other,dc=com'),
        'password' => env('LDAP_PASSWORD', 'secret'),
        'port' => env('LDAP_PORT', 389),
        'base_dn' => env('LDAP_BASE_DN', 'dc=other,dc=com'),
        'timeout' => env('LDAP_TIMEOUT', 5),
        'use_ssl' => env('LDAP_SSL', false),
        'use_tls' => env('LDAP_TLS', false),
    ],
],
stevebauman commented 10 months ago

Hi @DijkeMark,

To clarify this issue, you're only experiencing this with the Directory Emulator, right?

DijkeMark commented 10 months ago

Unfortunately I have not been able to test this against an actual ActiveDirectory yet.

stevebauman commented 10 months ago

Thanks @DijkeMark I'm able to reproduce this, these new methods are the culprit:

https://github.com/DirectoryTree/LdapRecord-Laravel/commit/d6ddd52dc1f0a22366b7ea7708babf3b1a49a693#diff-0abe304552d3d506a4e5a35c54cb856a4129b0eeae8950ae40bb4eed83d08484R46-R60

Working on a patch. I should have this out by end of weekend.

stevebauman commented 10 months ago

New release is out with a patch for this, thanks again for the report! 🙏

https://github.com/DirectoryTree/LdapRecord-Laravel/releases/tag/v3.0.9

DijkeMark commented 10 months ago

Thanks for the fix, everything seems to be working on my end now aswell. 👍