core.users.getWithQuery issue

frederic7 · February 21, 2018, 2:45pm

I’m having an issue with the core.users.getWithQuery command.

Whenever I try to search for a user uniquely using his username, I get an empty result:

local users, err, code = core.users.getWithQuery("somescope",{username="fred"})

but when using another table key, i get the expected result:

local users, err, code = core.users.getWithQuery("somescope",{username="fred",active=true})

using the limit=1 doesn’t work as an extra key though.

Could this be fixed so only the username is enough to get a specific user?

Develephant · February 21, 2018, 4:28pm

Hi,

This has been fixed in the upcoming update. For now the active key is needed.

-dev

frederic7 · February 22, 2018, 11:45am

Great! Thanks.

Would it be possible to add the password key?

It would be useful to check user’s credential without having to login necessarily

Develephant · February 22, 2018, 12:38pm

Hi,

Can you explain your use case a bit more? Generally only the user should know their password for security purposes.

-dev

frederic7 · February 22, 2018, 4:44pm

In my case, even the user doesn’t know his password.

1.He only registers with his email.

2.The app then generates a random key and stores it on the phone/device and registers the user on the server using the email as username and the key as password.

The user is then sent a confirmation email to make sure the email he provided is a real one.
By clicking on the confirmation link found in the email he confirms his registration. Only then, the user can start using the app.

The problem is that someone can use somebody’s else email to register (thus creating a password only his device is aware of).

If the real email owner then tries to register (with another auto-generated password), the system should detect that, credentials not being the same, another user is trying to register with an already registered email(not confirmed though).
It will then propose the user to reset the password.
If the user accepts, he is sent a reset link.
By clicking this link, the system knows that the new user is the real owner and resets the password with the one generated by his device.

The same problem could happen if a user tries to register on 2 devices (he will be warned that he can only use one device at a time).

Having the possibility to get a specific user with getWithQuery by providing the username and password as keys would allow to verify the credentials needed in step 5. getWithQuery doesn’t need to return the hashed password (that would be a security hole).

By explaining how my system is supposed to work, I’ve foreseen another problem: The real user won’t be able to reset the password as he doesn’t know the registered password, neither does he know the user_id. I think, I may have to reconsider the way I want to register users.

Develephant · February 22, 2018, 5:19pm

Hi,

Thanks for sharing your use case.

I’m not sure if some of these suggestions might help, but starting in 2.6.0 (which should be released today or tomorrow) you can now login by email, which means that now only one unique email address + username can exist in a scope, even if it is pending confirmation. The password reset in in there also.

I will add the password parameter to the getWithQuery, but it will require a hashed password to search against. I’ll be exposing the internal method that is already used for this.

You may need to create another database + table to manage some of your device state and possibly use the getAndMerge method. Storing a device id of some type might be helpful (unless you are already doing this) https://docs.coronalabs.com/api/library/system/getInfo.html (see deviceId section).

Keeping password state per device is going to be your biggest hurdle, since the password is stored on device.

-dev

frederic7 · February 22, 2018, 6:54pm

That is great news. I’m eager to work with the new version.

The unique email is a nice feature but will not help me as i’m already using the email as username(which is already unique).

The password reset could solve my “reset case scenario” but I will first to see how it works. Will it be a server or client command?

Comparing hashed password is not an issue. This will surely solve my step 5 problem.

I haven’t created another database/table but I use the “extra” row to store the confirmation status of a registered user. I can’t use the coronium confirmation system due to the fact that 1. this is only available through the client user registration and 2. i’m only using the core api for app/server communications. For now, I also store the auto-generated password in the “extra” row to temporarily solve step 5 issue but it’s a dirty hack and will change it asap.

Following this, I have some suggestions:

1. Adding the confirmation mail step on the server side registration as an option would be great in my case. The webmin doesn’t show the right data the way I do it now.

By design, only the developer who installed coronium can modify the server lua scripts. Then, why is the server core commands so strict about security? What i mean is: Why must a developer fetch a user_id with a login command before he can update the user’s values? Doesn’t this developer have access to the database? He could change any values there. I understand the need to restrict direct access to the coronium database/tables throught the core mysql commands: the developer could break the internals of the whole system with some mistyped queries. What I would wish for, is a way to update users data through the server core without having to know their credentials (something I could do manually by accessing the db). A setWithQuery working the same way as getWithQuery would be great (the developer is the only one responsible with what he will do with this function).
On the other hand the client core lua scripts can be hacked and a one could use it to modify one’s own data. In my case, I don’t use any client core methods except the api to communicate with the server. The only exposed methods of my server app are the ones used by the client scripts, and I’m making sure a hacker cannot do much with these. What I can’t control though, is the fact that other methods(the ones provided by the client core) are exposed. To protect the server from misuse of these methods, a configuration file on the server side would be useful. This configuration file would allow clients to use or not use their methods like so:

allowedClientMethods = {application=“apiProjectName”, methods = {analytics = true, api=true,data=false,email=false,files=false, …}}

Suggestion 2 would solve my step 8 issue.

Develephant · February 22, 2018, 8:48pm

Hi,

You offer some good suggestions. In general, the “big” idea with Coronium Core was not having developers needing to mess with the server-side very much, so the emphasis was more client-side oriented. But now that the Webmin has matured I do have a change in my thinking. Before most actions required heavy command line use and SFTP. Not so much anymore with the Webmin.

v2.6.0 introduces Scope Permissions, so that should solve any worries for #3. I think it makes sense now to add the confirmation functionality on the server-side since thats currently the only place the password reset is available.

I do agree about the developer having easier access to working with the Users database. And will think that through a bit. I like the setWithQuery idea. The reason it is the way it is now is that there is only one set of modules for the client and server. The client just makes a call to the server modules, there really isn’t any point of separation between the two, so its mostly by convenience (mine :)) that things are like that.

I am pretty much ready to release 2.6.0 today (testing is done), but let me take a day to see how easy it would be to implement some of these additions. Most of them should be pretty simple to add.

Thanks for sharing your feedback, its very helpful.

-dev

frederic7 · February 23, 2018, 5:55am

Scope permissions will be indeed a great add on.

After some more tests I did notice something I should have seen before (it’s even mentioned in the documentation :wacko: ):

The getWithQuery methods also returns the user_id. With this user_id, I can then use the update method to update the password.

Not as straight forward as a setWithQuery would be but it solves my step 8 issue.

The only problem is that the update method expects an unhashed password; this means that i will have to keep a copy of this plain password in another database until the reset link is clicked. Not the safest way to do things but it will have to do for now.

Maybe the setWithQuery method could take the hashed password instead of the plain one as does the update method. A new helper to hash the plain password (you wouldn’t have to expose the internal method that is used for that) would then be needed.

Develephant · February 23, 2018, 11:30am

Hi,

Funny you stumbled upon this. As I was thinking through it last night, I realized a setWithQuery would just be a getWithQuery followed up with an update. One of the issues with a setWithQuery method is that there is the possibility of returning multiple records, which will cause issues with the update command. So I don’t feel that a setWithQuery is the right solution right now.

Interestingly, the update command already has a third undocumented parameter that signifies whether a password is already hashed or not when updating the password. It takes a boolean value, the signature looks like so:

core.users.update(user\_id, data, is\_pw\_hashed)

Using that parameter will allow you to use a previously hashed password to update with. My biggest concern is developers double hashing passwords, which will render them useless, so I try to handle all of the password hashing internally.

Thinking through your use case more the sendConfirmationLink may be helpful, but the sendPasswordResetLink might not, as it simply redirects the user to a form to reset their password. It sounds like you want to handle how the password is set (I could be wrong).

In that case you will need to create your own logic using the Pages module. It’s actually not that difficult, and if you study the confirmation pages you should be able to understand how to do this. Those pages can be found here:

/home/coronium/pages/_coronium/registration/confirmation.lua

/home/coronium/pages/_coronium/registration/confirmation.tpl

I can also help walk you through this if needed. Your basically going to set up another database+table to hold a code that you can retrieve when the user hits your Pages endpoint and do your password logic at that point. If you wanted to keep it all internal to the app, you could possibly use the stored password as a key to change the password with (not fully sure on your implementation).

I’ve exposed the sendConfirmationLink method on the server-side, so I’m going to be prepping the v2.6.0 for release today. I think the other needs can be handled with whats available.

Feel free to ask any questions if you need some ideas on logic.

-dev

frederic7 · February 23, 2018, 12:20pm

That’s all perfect! Being able to update the user with an already hashed password is exactly what i was missing.

And you are right about the setWithQuery method: the possibility of returning multiple records is a real risk and I don’t see why a developper would want to change multiple basic users infos at once. getWithQuery followed by updateis enough.

You are also right about the fact I want to keep password generation internal and I already thought about the logic (it seems that again we were on the same page):

Someone registers with an already registered email
The app warns the user and propose to reset the password by clicking a ‘reset’ button.
As the button is clicked the newly generated password is sent to the server and stored on a database table along with a unique key and the related email.
The server sends an email with the reset link (which refers the unique key) to the email address.
As the user clicks on the link which directs to a Page module where the server compares the key to the one existing in the database and updates the user password with the stored one.
Done! The new device can now log in.

Thanks for your quick feedbacks. I will change my code to use the sendConfirmationLink method instead of my custom made solution as soon as v2.6 is released.

Thanks again.

Develephant · February 21, 2018, 4:28pm

Hi,

This has been fixed in the upcoming update. For now the active key is needed.

-dev

frederic7 · February 22, 2018, 11:45am

Great! Thanks.

Would it be possible to add the password key?

It would be useful to check user’s credential without having to login necessarily

Develephant · February 22, 2018, 12:38pm

Hi,

Can you explain your use case a bit more? Generally only the user should know their password for security purposes.

-dev

frederic7 · February 22, 2018, 4:44pm

In my case, even the user doesn’t know his password.

1.He only registers with his email.

2.The app then generates a random key and stores it on the phone/device and registers the user on the server using the email as username and the key as password.

The user is then sent a confirmation email to make sure the email he provided is a real one.
By clicking on the confirmation link found in the email he confirms his registration. Only then, the user can start using the app.

The problem is that someone can use somebody’s else email to register (thus creating a password only his device is aware of).

If the real email owner then tries to register (with another auto-generated password), the system should detect that, credentials not being the same, another user is trying to register with an already registered email(not confirmed though).
It will then propose the user to reset the password.
If the user accepts, he is sent a reset link.
By clicking this link, the system knows that the new user is the real owner and resets the password with the one generated by his device.

The same problem could happen if a user tries to register on 2 devices (he will be warned that he can only use one device at a time).

Having the possibility to get a specific user with getWithQuery by providing the username and password as keys would allow to verify the credentials needed in step 5. getWithQuery doesn’t need to return the hashed password (that would be a security hole).

By explaining how my system is supposed to work, I’ve foreseen another problem: The real user won’t be able to reset the password as he doesn’t know the registered password, neither does he know the user_id. I think, I may have to reconsider the way I want to register users.

Develephant · February 22, 2018, 5:19pm

Hi,

Thanks for sharing your use case.

I’m not sure if some of these suggestions might help, but starting in 2.6.0 (which should be released today or tomorrow) you can now login by email, which means that now only one unique email address + username can exist in a scope, even if it is pending confirmation. The password reset in in there also.

I will add the password parameter to the getWithQuery, but it will require a hashed password to search against. I’ll be exposing the internal method that is already used for this.

You may need to create another database + table to manage some of your device state and possibly use the getAndMerge method. Storing a device id of some type might be helpful (unless you are already doing this) https://docs.coronalabs.com/api/library/system/getInfo.html (see deviceId section).

Keeping password state per device is going to be your biggest hurdle, since the password is stored on device.

-dev

frederic7 · February 22, 2018, 6:54pm

That is great news. I’m eager to work with the new version.

The unique email is a nice feature but will not help me as i’m already using the email as username(which is already unique).

The password reset could solve my “reset case scenario” but I will first to see how it works. Will it be a server or client command?

Comparing hashed password is not an issue. This will surely solve my step 5 problem.

I haven’t created another database/table but I use the “extra” row to store the confirmation status of a registered user. I can’t use the coronium confirmation system due to the fact that 1. this is only available through the client user registration and 2. i’m only using the core api for app/server communications. For now, I also store the auto-generated password in the “extra” row to temporarily solve step 5 issue but it’s a dirty hack and will change it asap.

Following this, I have some suggestions:

1. Adding the confirmation mail step on the server side registration as an option would be great in my case. The webmin doesn’t show the right data the way I do it now.

By design, only the developer who installed coronium can modify the server lua scripts. Then, why is the server core commands so strict about security? What i mean is: Why must a developer fetch a user_id with a login command before he can update the user’s values? Doesn’t this developer have access to the database? He could change any values there. I understand the need to restrict direct access to the coronium database/tables throught the core mysql commands: the developer could break the internals of the whole system with some mistyped queries. What I would wish for, is a way to update users data through the server core without having to know their credentials (something I could do manually by accessing the db). A setWithQuery working the same way as getWithQuery would be great (the developer is the only one responsible with what he will do with this function).
On the other hand the client core lua scripts can be hacked and a one could use it to modify one’s own data. In my case, I don’t use any client core methods except the api to communicate with the server. The only exposed methods of my server app are the ones used by the client scripts, and I’m making sure a hacker cannot do much with these. What I can’t control though, is the fact that other methods(the ones provided by the client core) are exposed. To protect the server from misuse of these methods, a configuration file on the server side would be useful. This configuration file would allow clients to use or not use their methods like so:

allowedClientMethods = {application=“apiProjectName”, methods = {analytics = true, api=true,data=false,email=false,files=false, …}}

Suggestion 2 would solve my step 8 issue.

Develephant · February 22, 2018, 8:48pm

Hi,

You offer some good suggestions. In general, the “big” idea with Coronium Core was not having developers needing to mess with the server-side very much, so the emphasis was more client-side oriented. But now that the Webmin has matured I do have a change in my thinking. Before most actions required heavy command line use and SFTP. Not so much anymore with the Webmin.

v2.6.0 introduces Scope Permissions, so that should solve any worries for #3. I think it makes sense now to add the confirmation functionality on the server-side since thats currently the only place the password reset is available.

I do agree about the developer having easier access to working with the Users database. And will think that through a bit. I like the setWithQuery idea. The reason it is the way it is now is that there is only one set of modules for the client and server. The client just makes a call to the server modules, there really isn’t any point of separation between the two, so its mostly by convenience (mine :)) that things are like that.

I am pretty much ready to release 2.6.0 today (testing is done), but let me take a day to see how easy it would be to implement some of these additions. Most of them should be pretty simple to add.

Thanks for sharing your feedback, its very helpful.

-dev

frederic7 · February 23, 2018, 5:55am

Scope permissions will be indeed a great add on.

After some more tests I did notice something I should have seen before (it’s even mentioned in the documentation :wacko: ):

The getWithQuery methods also returns the user_id. With this user_id, I can then use the update method to update the password.

Not as straight forward as a setWithQuery would be but it solves my step 8 issue.

The only problem is that the update method expects an unhashed password; this means that i will have to keep a copy of this plain password in another database until the reset link is clicked. Not the safest way to do things but it will have to do for now.

Maybe the setWithQuery method could take the hashed password instead of the plain one as does the update method. A new helper to hash the plain password (you wouldn’t have to expose the internal method that is used for that) would then be needed.

Develephant · February 23, 2018, 11:30am

Hi,

Funny you stumbled upon this. As I was thinking through it last night, I realized a setWithQuery would just be a getWithQuery followed up with an update. One of the issues with a setWithQuery method is that there is the possibility of returning multiple records, which will cause issues with the update command. So I don’t feel that a setWithQuery is the right solution right now.

Interestingly, the update command already has a third undocumented parameter that signifies whether a password is already hashed or not when updating the password. It takes a boolean value, the signature looks like so:

core.users.update(user\_id, data, is\_pw\_hashed)