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

[request] Allow definition of alternative user table column names #1790

Closed
sstringer opened this Issue Jul 1, 2013 · 14 comments

Comments

Projects
None yet
6 participants
@sstringer
Copy link

sstringer commented Jul 1, 2013

I'm in the process of converting a very large, active application over to Laravel 4.

Deep within the Laravel code are hard-coded definitions for the key columns used in the users table, "id," "username," and "password." Because this is an active and stable application I'm converting, it's not practical for me to alter the column names on this live table.

It would be very helpful in this situation if I could edit a config file value to change these column names to match my legacy values.

Thanks,
Steve

@crynobone

This comment has been minimized.

Copy link
Contributor

crynobone commented Jul 1, 2013

The column name is already configurable for such purpose. Have you look at https://github.com/laravel/laravel/blob/master/app/models/User.php

@sstringer

This comment has been minimized.

Copy link

sstringer commented Jul 1, 2013

I have, but there's nothing in there about renaming or remapping column names; only the table name.

@crynobone

This comment has been minimized.

Copy link
Contributor

crynobone commented Jul 1, 2013

There no need to change the column name, Auth only need to know what the value of ID, and password. Which is handled by getAuthIdentifier(), getAuthPassword().

@sstringer

This comment has been minimized.

Copy link

sstringer commented Jul 1, 2013

Unless I'm mistaken, the framework looks for "password" or any column name with that string in it. In my case, the legacy table name uses a strong naming convention and the characters "pw" instead. Therefore, Laravel never finds the password column automatically.

My point is this: it would be extremely helpful to make these column names as explicit as the table name in users.php rather than make them implicit and make their discovery automatic deep within the bowels of Laravel. In my case, it wasn't at all clear why my attempts to get auth working with a legacy schema failed; login simply failed quietly because it couldn't match a password. This is one of those times when Laravel is too clever for its own good.

@crynobone

This comment has been minimized.

Copy link
Contributor

crynobone commented Jul 1, 2013

Unless I'm mistaken, the framework looks for "password" or any column name with that string in it. In my case, the legacy table name uses a strong naming convention and the characters "pw" instead. Therefore, Laravel never finds the password column automatically.

No, read on https://github.com/laravel/framework/blob/master/src/Illuminate/Auth/EloquentUserProvider.php, especially https://github.com/laravel/framework/blob/master/src/Illuminate/Auth/EloquentUserProvider.php#L60 and https://github.com/laravel/framework/blob/master/src/Illuminate/Auth/EloquentUserProvider.php#L73-L78

It does have "password" there, but understand "password" is just a key used by Auth to identify the value of input password. It doesn't use it in the query at all.

@sstringer

This comment has been minimized.

Copy link

sstringer commented Jul 1, 2013

Thanks for the pointers. I guess I'm dense. I'm still not comprehending where the password key turns into a password column name or where I would specify that column name. The key=>value par I'm passing to the Attempt method is the password keyword (not the column name) and the input password.

I guess this is just underscoring my desire to keep it simple and have the option to set column names explicitly in a setting in users.php rather that figure it out through deep code inspection into the Eloquent user service provider.

@Kindari

This comment has been minimized.

Copy link

Kindari commented Jul 1, 2013

Not entirely correct @crynobone - On line 60 there, it loops over all the input variables and adds them as a where() unless 'password' is in the name. Therefore an input of ['username' => 'foobar', 'pw' => 'hunter2', 'active' => 1] would produce where username = 'foobar' and pw = 'hunter2' and active = 1 which the pw column being hashed is obviously going to be an issue.

I believe this is an issue that needs to be addressed, though it only affects a small number of people.

@crynobone

This comment has been minimized.

Copy link
Contributor

crynobone commented Jul 1, 2013


Auth::attempt(['username' => 'foobar', 'password' => 'hunter2', 'active' => 1]);

And for app/models/User.php.

public function getAuthPassword()
{
    return $this->pw;
}

This would work out of the box, for bcrypt encryption.

@heybige

This comment has been minimized.

Copy link

heybige commented Jul 1, 2013

@sstringer I too use "non-standard" table names and fields. I just wrote up a blog post about this - http://www.mindworksdev.com/2013/06/laravel-4-different-usernamepasswd-fields-in/

@sstringer

This comment has been minimized.

Copy link

sstringer commented Jul 1, 2013

@crynobone, this simply doesn't work if your password column name is non-standard. At least, I haven't been able to figure out a way to make it work. Simply changing the getAuthPassword() method to look for the column name results in an error in Illuminate/Auth/GenericUser.php stating that it can't find the index 'password' when passed.

Regardless, I'm not looking for others to debug my code or for this to devolve into an argument. I've already found work-arounds that are inelegant but nonetheless work. I am simply highlighting the need in certain fringe cases to use non-standard column names for user validation, and a desire on my part to have these be as easily, explicitly, and clearly defined as the table name itself in the stock Users.php model to speed implementation.

If the feature request is rejected, that's fine. If there are ways to handle this by changing a line of code or two somewhere I'm missing, then I'd suggest updating the inline comments and the documentation to reflect this ability.

Thanks,
Steve

@crynobone

This comment has been minimized.

Copy link
Contributor

crynobone commented Jul 1, 2013

@sstringer The argument that I provide is to show what Laravel already provide or intended to do with the current implementation. Now if Laravel was unable to provide the feature that mean we need to suggest a feature to it, while if the intended feature does not deliver as expected we need to fix it.

Simply changing the getAuthPassword() method to look for the column name results in an error in Illuminate/Auth/GenericUser.php stating that it can't find the index 'password' when passed.

Now we're getting somewhere. In this scenario I can see that you're using "database" driver instead of "eloquent", "database" driver would use Illuminate\Auth\GenericUser (hard-coded) instead of User class. Using "eloquent" would enable you to do what I and @heybige mention above.

As for "database" driver, @taylorotwell may need to decide whether it should be as flexible as "eloquent".

@sstringer

This comment has been minimized.

Copy link

sstringer commented Jul 1, 2013

Okay, switching the auth setting back from database back to eloquent did allow me to do as you suggested, although I would have had no way of knowing this functionality existed as a newcomer to Laravel and Eloquent.

That said, I think we can all agree the documentation and in-line commenting on the authorization system needs a lot of improvement.

Thanks,
Steve

@taylorotwell

This comment has been minimized.

Copy link
Member

taylorotwell commented Jul 3, 2013

Looks like this is solved. Please send any issues to the laravel/docs repo if you have any suggestions for things that need to be improved there. Thanks.

@jonwhittlestone

This comment has been minimized.

Copy link

jonwhittlestone commented Jul 29, 2013

Hi, I (am being forced to) use a legacy mssql db with column names that are in a mixture of cases:

Would this affect the authing? My login fails silently.

routes.php

$userdata = array(
        'Email' => Input::get('username'),
        'password' => Input::get('password')
    );

In my Eloquent user model:

protected $primaryKey = "UserID";
public function getAuthIdentifier()
{
return $this->Email;
}

public function getAuthPassword()
{
    return $this->Password;
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment