-
Notifications
You must be signed in to change notification settings - Fork 330
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Attempting to use with LdapRecord #527
Comments
Just ran into this as well -- and I agree on the use of an override flag (whether it be a static variable somewhere, or be added to the I'll see if I can write a PR to patch this. |
To elaborate further for @JosephSilber, the LdapRecord auth provider is registered like so: 'ldap' => [
'driver' => 'ldap',
'model' => LdapRecord\Models\ActiveDirectory\User::class,
'database' => [
'model' => App\User::class,
'sync_passwords' => false,
'sync_attributes' => [
'name' => 'cn',
'email' => 'mail',
],
],
], So Bouncer is calling |
+1 to this, it's caused a halt in one of my company's projects |
This provides the ability to successfully customize the user model, as the database table is only resolved when needed via the `table` method. This pattern is used in the Laravel core, such as `Illuminate\Auth\AuthManager::$customCreators`.
The PR seems to not fully mitigate this issue. It defaults to App\User as the model when it should be App\Models\User as set in the ldapconfig. |
I'm trying to use Bouncer with LdapRecord package, I'm overriding the user model with App\User within the AppServiceProvider in the boot function.
However when Bouncer loads it first loads the LdapRecord user model and I get
"Call to undefined method LdapRecord\Query\Model\ActiveDirectoryBuilder::getTable()"
Which is coming from
Guessing its because its not a eloquent model.
I tried putting the
within the register function so it kicks it off first which works but then the standard one overrides that.
I added an override flag against the setUsersModel
Then the useUserModel is setup like this
I then had the useUserModel within the register function and it works as expected because its not trying to load something it shouldn't.
I'm not sure this is the best way to do this which is why I'm asking for any other options / feedback before I create a PR. The changes above wouldn't have any effect on previous cases as it still would override if required in the boot for other models, its just in this case it doesn't work because the LdapRecord model doesn't contain getTable.
Another possible option would be to have a config file that the model is read from instead of looking it up against the default guard which still allow an override if that config is not found (so it doesn't break all current implemented things)
The text was updated successfully, but these errors were encountered: