-
Notifications
You must be signed in to change notification settings - Fork 1k
Process add'l data on registration #911
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
Conversation
Updated registration endpoint for dbAuth to accomodate other posted registration data.
Also added some new dbAuth properties
1. usernameMinLength : specify minimum length of username (5)
2. usernameMaxLength : specify maximum length of username (40)
3. usernamePattern : specify regex pattern for usernames ('/^[A-Za-z0-9]+$/') // defaults to alphanumeric chars only
Formatted section on setting up JWT authentication to make the list more readable
Registration w/ addl data
Added the dbAuth properties for checking minimum and maximum length of username as well as allowed characters.
Is it ready to merge? Did you accidentally close it?
Is it ready to merge? Did you accidentally close it?
It still causes fatal error on duplicate keys exception so I closed it to review.
Basically, since we are processing additional data, it is possible that the extra data are defined to be unique in the users table. A common example would be an email address. I think we can query the users table if an email address is already registered, but if there 2,3 or more unique fields in the table (not common but possible), we'll also need to query 2,3, or more times.
Since we're processing additional data during registration, we need to check if these data were defined in db to be unique. For example, email addresses are usually used just once in an application. We can query the database to check if the new email address is not yet registered, but, in some cases, we may more than 2 or 3 or more unique fields (not common, but possible), hence we would also need to query 2,3 or more times. As a TEMPORARY WORKAROUND, we'll just attempt to register the new user and wait for the db to throw a DUPLICATE KEY EXCEPTION.
@mevdschee, Good day! I've added try{}catch around the createSingle invocation when registring new user so I can suppress the PDOException on duplicate key It's not a very elegant solution but that's all I can think of for now.
I hope you can review and improve on it.
chattago2002
commented
Jan 20, 2023
Are there any news about it? Can this improvement be merged in the main branch?
My problem is that I use "apiKeyDbAuth mode" so after the user sends username and password to "/register", every following request is blocked because of the "user api_key" is missing (I need to send many fields including "api_key"). This improvement can be useful to bypass the problem. Or are there other solutions for my problem?
I used the modified code even though it's not yet merged to main. Anyway, if you do too, you need to keep that in mind in case you somehow update your app and then the modified code gets overwriten.
Another way without using the modified code is to store the other user details in another table and make it a multi-step process;
- Register just the username & password,
- Get the other user fields or info and save to another table, linked to user table by foreign key.
As for blocked requests, I supposed it's caused by the missing header (x-api-key).
I haven't successfully tried using the dbAuth and apiKeyDbAuth together. Maybe @mevdschee can confirm if these two middlewares can be used in the same file.
As a workaround, you can setup two versions of the api.php, one configured with dbAuth - this will handle usual registration and the other is for apiKeyDbAuth for use by your frontend
I haven't successfully tried using the dbAuth and apiKeyDbAuth together. Maybe @mevdschee can confirm if these two middlewares can be used in the same file.
You may get it to work, as I don't know of a reason it shouldn't work. Feel free to try it and open an issue if you get stuck.
Another way without using the modified code is to store the other user details in another table and make it a multi-step process;
If you make the fields nullable and add values to them later in the multi-step process then you don't even need multiple tables.
Are there any news about it? Can this improvement be merged in the main branch?
I'll look into merging this code.
Changes $_SESSION['user']['updatedAt'] - set to time when the session was created/updated dbAuth.refreshSession - number of minutes after which the session data is refreshed when the /me end-point is called
Option to update session data via /me end-point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check the review notes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel this should default to non-white-space printable characters in any language (Chinese characters included).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this be sufficient as the regex pattern? Haven't done much checking with other languages,but basically, we'll be checking if it's alphanumeric and also if it's a printable char in Unicode.
'/^[[:alnum:][:print:]]+$/u'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel 30 is too low for an email address for instance. I would default to 255.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This breaks internationalization (Chinese characters for instance).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure yet how to resolve this. Any issue if we simply just
$data[$key] = htmlspecialchars($value); ?
Here's the updated DbAuthMiddleware.
Changes
$usernamePattern - defaults to /^\p{L}+$/u , visible characters, no punctuation or numbers, unicode mode
$usernameMaxLength - defaults to 255
changed validation of other inputs from filter_validate() to htmlspecialchars()
fixed typos missing and extra $
Added procedure to process additional data sent during registration.
Other changes:
NB. Currently, the code throws an error on duplicate entry on columns that are intended to be unique.