Update LdapAuthServiceProvider.php

[frankmichel]

Hi @stevebauman,

With regards to our discussion in https://github.com/DirectoryTree/LdapRecord-Laravel/discussions/341 I have come up with this idea.

This PR will allow users to use the following Ldap Auth config:

'driver' => 'eloquent',
'model' => App\Models\User::class,
'ldap' => [
    'model' => LdapRecord\Models\ActiveDirectory\User::class,
    'sync_passwords' => true,
    'sync_attributes' => [...]
], // ldap config here

With this config, Nova will simply use this as the regular Eloquent provider ignoring the 'ldap' config.

However, the app itself will recognize the 'ldap' config and swap the config in order to register the correct providers.

This approach is also backwards compatible.

The only thing that probably wouldn't work is utilizing this config setting in an app that needs both Ldap as well as Eloquent auth in the same request. However, I think that is very unlikely.

This fixes the issues with Nova and is a totally optional config setting. If the 'ldap' key is not set, a regular Eloquent Provider will be returned and everything will be as expected.

I am looking forward to your input and thoughts on this.

[stevebauman]

Hey @frankmichel, appreciate the PR!

I'm really hesitant on this -- as we're hijacking the typical eloquent auth driver registration built into Laravel. I'd really rather not do this and potentially cause issues for apps that could be overriding it already, or if the framework makes an update to it that we don't properly replicate. I'd consider this a major breaking change due to this override, and as such, would require a new major release (v4.0). I'd much prefer another way, if possible.

Unfortunately any requests for support for this super simple change on Nova's end have fallen on deaf ears, as I've tweeted to David Hemphill (no response), and have created an issue in Nova which was closed:

https://github.com/laravel/nova-issues/issues/4081 https://github.com/laravel/nova-issues/discussions/4082

Maybe I'll get a response here, who knows:

https://twitter.com/ste_bau/status/1768303402843345280

[frankmichel]

Hi @stevebauman,

I understand that you are hesitant and you don't want to introduce a breaking change.

However, at this moment the library does break functionality with other tools, such as Nova, because of the way it handles auth providers.

Ideally, Nova would provide the functionality you have requested. But since they don't want to do that, I think it would be great if your library would provide - a totally optional - way of enabling the auth provider swapping as I have come up with in this PR.

In my recent commit I have added a second condition that needs to be met (together with the "ldap" key instead of the "database" key in the auth config). With this added config setting in the ldap configuration file, users can actively choose to use this - hopefully temporary - workaround. You don't even need to document it (or you do as a temporary solution).

But this PR will enable users of your library to use it alongside Nova. It also does not affect current users, nor requires any major version bump since it is totally optional and only enabled by configuration.

I hope you can consider this change. As as sponsor of your library we would really appreciate if this issue can be addressed, even only temporary.

[stevebauman]

Hey @frankmichel, appreciate the detailed replies here, and the sponsorship ❤️

I don't think this is really making this library compatible with Nova though, as it's just returning the standard eloquent auth driver with this switch, which means you still won't be able to login with LDAP accounts (unless you have password sync enabled and the user has logged in once before of course).

If that's what you're intending, you can already setup two auth guards. The default one being for your application that uses the ldap auth driver, and another for Nova to use that uses the standard eloquent driver. Laravel Nova allows you to configure a guard to use in its configuration file:

// config/nova.php

return [
    // ...

    'guard' => 'nova',
];

// config/auth.php

return [
    // ...

    'defaults' => [
        'guard' => 'web',
        'passwords' => 'users',
    ],

    'guards' => [
        'web' => [ // <-- Default web guard for your application
            'driver' => 'session',
            'provider' => 'ldap',
        ],

        'nova' => [ // <-- Nova guard for Laravel Nova
            'driver' => 'session',
            'provider' => 'users',
        ],
    ],

    'providers' => [
        // ...

        'users' => [
            'driver' => 'eloquent',
            'model' => App\Models\User::class,
        ],

        'ldap' => [
            'driver' => 'ldap',
            'model' => LdapRecord\Models\ActiveDirectory\User::class,
            'rules' => [],
            'scopes' => [],
            'database' => [
                'model' => App\Models\User::class,
                'sync_passwords' => false,
                'sync_attributes' => [
                    'name' => 'cn',
                    'email' => 'mail',
                ],
            ],
        ],
    ],
];

Let me know if this isn't what you're intending and we can go from there 👍

[frankmichel]

@stevebauman Thanks for your feedback on this. Providing another guard for Nova would work. However this means the users have to re-authenticate when accessing Nova. Or we would have to implement some logic to automate that process.

Anyway, we have come up with a partly workaround by extending Laravel Nova's ActionEvent class as well as extending the Actionable trait to use that extended ActionEvent class. It's not ideal, since we will have to make sure this all still works after Nova upgrades and it doesn't solve the entire functionality. So hopefully your proposed configuration change will make it into Nova anytime soon.