Adldap2 / Adldap2-Laravel

LDAP Authentication & Management for Laravel
MIT License
911 stars 184 forks source link

Use different attributes for model retrieval #174

Open Crinsane opened 8 years ago

Crinsane commented 8 years ago

I'm trying to use this package and it looks really nice. Now I'm facing this problem:

In our situation, I want to login using the samaccountname, so I've changed the username_attribute to ['username' => 'samaccountname']. This makes it possible to login, but now when the Eloquent model must be retrieved, I run into a problem. The model is not a default Laravel User model, but a custom Employee model. Which doesn't have a username/password, but only an id (EmpId)

The EmpId of each employee is stored in the active directory, so I would like to use that attribute to retreive the Eloquent model.

Problem is that the package uses the same config value for both the authentication as the model retrieval.

So instead of a query like 'SELECT * FROM Employee WHERE EmpId = ''I end up withSELECT * FROM Employee WHERE username = ''`

I've not yet found a way to fix this.

stevebauman commented 8 years ago

Hi @Crinsane, good question.

This is actually configurable. Change username to empid:

// config/adldap_auth.php

'username_attribute' => ['empid' => 'samaccountname'],

Though changing this, you'll also need to change the input name of the element in your form to empid.

Adldap utlizes this variable to query for the authenticated user model once authenticated.

Try it and let me know if it works for you, thanks!

Crinsane commented 8 years ago

Hi @stevebauman

Thanks for the answer, but I think that's not exactly the solution to my problem.

The problem is right here I think:

// Get the model key.
$attributes = $this->getUsernameAttribute();

// Get the model key.
$key = key($attributes);

// Get the username from the AD model.
$username = $user->{$attributes[$key]};

// Make sure we retrieve the first username
// result if it's an array.
$username = (is_array($username) ? Arr::get($username, 0) : $username);

// Try to retrieve the model from the model key and AD username.
$model = $this->createModel()->newQuery()->where([$key => $username])->first();

For the actual authentication I indeed want to use the username (samaccountname) and the password. So I have to set the config to:

'username_attribute' => ['username' => 'samaccountname'],

But now when the model is retrieved from the database, I want to use the value in another AD attribute to get the user. So to get that to work, the config should be:

'username_attribute' => ['EmpId' => 'title'],

So that the line $model = $this->createModel()->newQuery()->where([$key => $username])->first(); does not generate a query:

SELECT * FROM Employee WHERE username = '<AD-value-from-samaccountname-attribute>'

but

SELECT * FROM Employee WHERE EmpId= '<AD-value-from-title-attribute>'

When I would try your fix, I'm pretty sure I would end up with a query:

SELECT * FROM Employee WHERE EmpId = '<AD-value-from-samaccountname-attribute>'

Which still doesn't give me the correct query.

So in short, the problem is that both the authentication and the modal retrieval code uses the same config value (username_attribute), where I would need two different config values, one for the authentication and one for retrieving the model.

Hope that makes sense?

stevebauman commented 8 years ago

Hi @Crinsane.

I don't think I'm understanding your situation correctly.

You mention the query:

SELECT * FROM Employee WHERE username = '<AD-value-from-samaccountname-attribute>'

But what do you mean AD value **from** samaccountname attribute? A users samaccountname is a singlular attribute, not multi-valued.

You also mention:

But now when the model is retrieved from the database, I want to use the value in another AD attribute to get the user. So to get that to work, the config should be:

'username_attribute' => ['EmpId' => 'title'],

The key in the username_attribute array has nothing to do with Adldap (which you probably already know). It is used as the search column to perform the local user query.

Are you perhaps able to elaborate on your issue?

Thanks!

Crinsane commented 8 years ago

Hi @stevebauman

I wasn't really clear indeed. Let me try to break it down :) The situation is like this. We have a Employee table, in which the data of employees is saved obviously, And the primary key of this table is EmpId.

This table doesn't have username and/or password columns, It's just a list of employees (it's part of our companies ERP system, no way for me to change it)

Next we have an Active Directory, which also includes all employees, the important attributes in the Active directory are:

samaccountname -> Employee username
password -> Employee password
title -> Holds the EmpId

Now I want to build a login form and authenticate against the Active Directory. The default of your package is to use the mail field from the Active Directory and compare that against the email field from the form post. To change that to use the samaccountname I have to change the config file and change:

'username_attribute' => ['email' => 'mail'],

to:

'username_attribute' => ['username' => 'samaccountname'],

And that works, everything ok. I can disable syncing the username/password attributes (because the Employee table doesn't have any), so no problem there.

But, after the authentication, your package tries to take the Active Directory record, and use that to fetch the "User" model (in our case, an Employee model).

To do this it uses the same configuration value to build up the query to get the "User" model from the database. So the query will become:

SELECT * FROM Employee WHERE username = "<samaccountname>";

But in our case, that will fail, because the way to fetch the employee is using the EmpId column in the database (instead of username) and don't give it the value of the samaccountname, but the title field from the Active Directory. And I can't change that, because the package uses the same configuration value username_attribute for both the authentication on the Active Directory, as for fetching the model from the SQL database.

If you look at the code in the ImportsUsers trait, and specific the getModelFromAdldap method, you see that it does this:

// Get the model key.
$attributes = $this->getUsernameAttribute();

// Get the model key.
$key = key($attributes);

// Get the username from the AD model.
$username = $user->{$attributes[$key]};

// Make sure we retrieve the first username
// result if it's an array.
$username = (is_array($username) ? array_get($username, 0) : $username);

// Try to retrieve the model from the model key and AD username.
$model = $this->createModel()->newQuery()->where([$key => $username])->first();

It uses the getUsernameAttribute() to get the $key and $username used for building the SQL query. So currently it uses $key = 'username' and $username = '<samaccountname-value>' which come from the configuration as you can see here:

protected function getUsernameAttribute()
{
    return config('adldap_auth.username_attribute', ['username' => $this->getSchema()->accountName()]);
}

If this would use another (new) configuration value, something like adldap_auth.model_key or something, which would specify the key and value to use in the WHERE clause, that would fix my problem.

Hope you understand now?

stevebauman commented 8 years ago

Hi @Crinsane, thanks for the detailed explanation! Loud and clear now.

I'll be adding this configuration option in. Thanks for the feedback!