delight-im / PHP-Auth

Authentication for PHP. Simple, lightweight and secure.
MIT License
1.1k stars 235 forks source link

Additional columns #10

Closed Takitaf closed 7 years ago

Takitaf commented 7 years ago

Hello,

Is there any way to set additional data in users table during registration? It would be perfect if after sucessful login we'll receive it in Session either. Any chances to do so?

Of course I can insert additional data just after registration and then make wrapper to login method with additional SELECT statement, but maybe there is a way to do so only using PHP-Auth?

ocram commented 7 years ago

Thanks for your feedback!

Is there any way to set additional data in users table during registration? It would be perfect if after sucessful login we'll receive it in Session either. Any chances to do so?

This is definitely something that has been thought about. Right now, there is no built-in solution in the way you expect. Everybody wants other columns to be added, so if we started that way, the users table would continuously grow in size while never actually reaching the point where everybody is satisfied.

Some want this column, some want that column -- adding both for every user of this library makes no sense for a general-purpose, lightweight and re-usable library.

Of course I can insert additional data just after registration and then make wrapper to login method with additional SELECT statement, but maybe there is a way to do so only using PHP-Auth?

Yes, this is (almost) the recommended solution. It's not because we're too lazy to implement what you described before, but because we genuinely think that this is the better solution. Do you disagree? If anybody comes up with a better solution where we can agree that it really works, we'll definitely consider that solution instead.

We don't actually think the current solution is that complicated and the overhead compared with the columns being hard-coded into the library is minimal.

So what you should do is the following:

  1. Add any number of your own database tables where you store your custom user information, e.g. a table named profiles.
  2. Whenever you call Auth#register (which should be in a single place only, of course, as per common best practices), add your own logic afterwards that fills the profiles table (or whatever you have here) with the custom information that you need. Even before the call to Auth#register, you may add a call to ignore_user_abort(true) to ensure that both the library's insertions and your own insertions are really executed in the extremely rare cases where the client disconnects between those steps.
  3. If you rarely need the custom user information, you may just retrieve it as needed. If you need it more frequently, however, you'd probably want to have it in your session data, right? That's what wanted to add in a wrapper around the Auth#login call, it seems. Now that would work perfectly, except for one feature: The automatic login using "remember me" tokens. So if you use the "remember me" feature during login (at least for some users), you can't just wrap the Auth#login call because a login can also happen automatically based on the "remember me" token, if available.

    So what you need if you want to access your custom user information frequently and make use of the "remember me" feature during login is the following:

    Add two functions or methods like the following in order to retrieve your custom user information:

    function getUserInfo(\Delight\Auth\Auth $auth) {
        if (!$auth->isLoggedIn()) {
            return null;
        }
    
        if (!isset($_SESSION['_internal_user_info'])) {
            $_SESSION['_internal_user_info'] = getUserInfoFromMyCustomTables($auth->getUserId());
        }
    
        return $_SESSION['_internal_user_info'];
    }
    
    function getUserInfoFromMyCustomTables($userId) {
        // to be implemented by you ...
    }

    Now you only ever access your custom user information by calling getUserInfo($auth) and working with the result, which will probably be an array or object.

Does that make sense? Once implemented, you'll see that this is really not much effort and the code is short and readable as well. We really don't think that there's any more efficient solution.

Of course, for some popular columns that many people need, e.g. date_of_birth or billing_address, we could hard-code those into the library. But then, at some point, you'll notice that you need yet another column, e.g. hometown, and we'd be back to square one.

Takitaf commented 7 years ago

Whoah, I didn't expect such a long instruction, thanks!

To be honest, eventually I had made code very similar to your suggestions before even saw the response, so I can agree with you without any doubts. And I still doesn't use "remember me" option, so I'll take your tips into account. Thanks, I really appreciate it!

According to the main point of our discussion, for now I got one idea, that MAYBE could help people with the "additional columns" problem. Hard-coding is, of course, terrible idea, I agree, but how about allowing small config during instantiation? Let's say, as an optional parameter of constructor, we could pass an array e.g.

$config = array( "<column_name1>", "<column_name2"> );

Then, in register method also additional parameter with data:

$additionalColumns = array( "<column_name1>" => "<value_for_1_column>", "<column_name2>" => "<value_for_2_column>" );

... and of course we then need a way of storing those values in Session (maybe kind of "getAdditionalColumnValue('columnName')"?).

I don't know the whole code of your library, so can't say if it is hard to make work. What do You think about it?

Of course it's just a theoretical problem for me, as I don't need it for now, but who knows what future brings?

ocram commented 7 years ago

Thanks for your response!

Glad to hear that it works and that you agree with this idea. If you implement it like that, you will be able to make use of the "remember me" feature later (if you need it) by just turning it on as described in the README. There won't be any other required modifications left. So we have full compatibility there.

Hard-coding some additional columns might work. If we could agree on a set of "important" columns that most people will need, we could add them to the library and everybody would be able to use what they need and ignore the other columns.

But, first, that would be a small overhead already for all the columns you don't use. Second, if we agreed on the date of birth as YYYY-MM-DD, for example, somebody will ultimately come up with another requirement for this column, e.g. the reverse format or a timestamp. If we agreed on a postal (ZIP) code consisting of 6 alphanumeric chars, somebody will ultimately find out that they need 8 characters with special chars. So we will never have a solution that fits everybody's needs, and we will fit nobody's need perfectly.

The solution that we agreed on here does that, however.

I actually like your next solution, too. Thought about that as well. But it has two problems:

While additional columns are a feature that almost everybody needs and I'd therefore love to make this as simple as possible, I'm afraid we don't have any better solution at the moment than what we've agreed on earlier. So I'm closing this for now. If anybody comes up with a better solution, we'd love to hear about it!

wonpingonly commented 2 years ago

I know this is an old issue and I completely understand the issue with trying to have columns be all things for all people which is of course not possible.

However, how about implementing the columns as generic fields that can be assigned to whatever purpose the developer wants to use them for. For example, the user table could be extended to have the following columns: user_attribute_01, user_attribute_02, etc. Then the column could be used to store phone numbers, addresses, birthdays, department names, or whatever. They could all be text fields and then rely on the developer to do any type conversion necessary. Or there could be some of each type (user_attribute_text_01, etc.; user_attribute_int_01, etc.; user_attribute_date_01, etc.; and so forth) though that is not as efficient use of columns as having them all be text which could encompass any data type.

Of course there will still be people wanting more but I believe adding generic columns would be a way to provide wanted functionality that addresses many of the issues with adding explicit columns. Not a perfect solution but would provide good bang for the buck IMO.

maietta commented 2 years ago

If this project adopts this strategy, I will fork and maintain my own. I love how PHPAuth is elegant and simplistic yet feature-rich already.

Call me old-fashioned, but I like the traditional "tie tables" strategy for storing extra data. I also make use of serialized data instead of dedicated table columns whenever possible. This affords me the opportunity to encrypt and decrypt that serialized data using their own credentials and since the password is salted and hashed, a database breach is far less concerning than if the serialized data was stored in plain text.

I think adding additional generic, but user-definable columns would muddle what this project seems to be about. I love PHPAuth just the way it is.

nssnl commented 2 years ago

I fully agree @maietta, for the reasons explained by ocram. Simply create an additional table. For example: I created a table named users_additional, with a column id_users that references to the id of table users. The other columns contain data specific to the user account, this way your columns have logical names, correct data type, etc.

ocram commented 2 years ago

@maietta Fully agree, thanks!

@nssnl Thanks for the explanation on how to do it and why that is also better.

@wonpingonly The solution you proposed has been considered, but it provides so little value (untyped, non-descriptive columns) that the question is: Why would you want this over a custom extra database table? In the long-term, you will be happier with a separate table that you design for your own needs.