fuel / auth

Fuel PHP Framework - Fuel v1.x Authentication package
http://fuelphp.com/docs/packages/auth/intro.html
76 stars 57 forks source link

[Bug] Bad column list on create_user() #101

Closed rickmacgillis closed 9 years ago

rickmacgillis commented 9 years ago

On login/ormauth.php, create_user() tries to select a list of columns through Orm to check if a user exists. When Orm parses the list, the column names are set to numbers. (The array keys in the indexed table_columns list in the config.)

My guess is this issue is due to the other merge on Orm that never got reverted.

WanWizard commented 9 years ago

Which "other merge" are you refering too?

WanWizard commented 9 years ago

I think that select should not be there at all. Say you have decided that you don't want 'email' in your selected results. This would cause unwanted behavior, since that column needs to be checked.

I will remove that select, it is useless in that query.

WanWizard commented 9 years ago

There are more queries that are incorrect, I'll go through them all.

rickmacgillis commented 9 years ago

The merge I meant was https://github.com/fuel/orm/pull/353

Going through and fixing the queries is helpful, but it seems like a bandaid over the underlying problem. I'm not sure why Orm takes the array_keys for the indexed array and makes them the column list.

WanWizard commented 9 years ago

Ok, I get what you mean now. I can reproduce it, I'll have a look.

WanWizard commented 9 years ago

It is not related at all to that merge, it already goes wrong in Query::select(), which does not support an indexed array at all. And I can't remember it ever did, but I have to go through the commit history to be sure.

rickmacgillis commented 9 years ago

Odd. Then I guess that the bad queries are the issue. I'm not sure why it worked before in that case.

WanWizard commented 9 years ago

Maybe an earlier commit that broke it? I know there were quite a few changes trying to get DB::expr() to work as a select() argument...