-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Process add'l data on registration #911
base: main
Are you sure you want to change the base?
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? |
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 |
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? |
𝙸 𝚞𝚜𝚎𝚍 𝚝𝚑𝚎 𝚖𝚘𝚍𝚒𝚏𝚒𝚎𝚍 𝚌𝚘𝚍𝚎 𝚎𝚟𝚎𝚗 𝚝𝚑𝚘𝚞𝚐𝚑 𝚒𝚝'𝚜 𝚗𝚘𝚝 𝚢𝚎𝚝 𝚖𝚎𝚛𝚐𝚎𝚍 𝚝𝚘 𝚖𝚊𝚒𝚗. 𝙰𝚗𝚢𝚠𝚊𝚢, 𝚒𝚏 𝚢𝚘𝚞 𝚍𝚘 𝚝𝚘𝚘, 𝚢𝚘𝚞 𝚗𝚎𝚎𝚍 𝚝𝚘 𝚔𝚎𝚎𝚙 𝚝𝚑𝚊𝚝 𝚒𝚗 𝚖𝚒𝚗𝚍 𝚒𝚗 𝚌𝚊𝚜𝚎 𝚢𝚘𝚞 𝚜𝚘𝚖𝚎𝚑𝚘𝚠 𝚞𝚙𝚍𝚊𝚝𝚎 𝚢𝚘𝚞𝚛 𝚊𝚙𝚙 𝚊𝚗𝚍 𝚝𝚑𝚎𝚗 𝚝𝚑𝚎 𝚖𝚘𝚍𝚒𝚏𝚒𝚎𝚍 𝚌𝚘𝚍𝚎 𝚐𝚎𝚝𝚜 𝚘𝚟𝚎𝚛𝚠𝚛𝚒𝚝𝚎𝚗. 𝙰𝚗𝚘𝚝𝚑𝚎𝚛 𝚠𝚊𝚢 𝚠𝚒𝚝𝚑𝚘𝚞𝚝 𝚞𝚜𝚒𝚗𝚐 𝚝𝚑𝚎 𝚖𝚘𝚍𝚒𝚏𝚒𝚎𝚍 𝚌𝚘𝚍𝚎 𝚒𝚜 𝚝𝚘 𝚜𝚝𝚘𝚛𝚎 𝚝𝚑𝚎 𝚘𝚝𝚑𝚎𝚛 𝚞𝚜𝚎𝚛 𝚍𝚎𝚝𝚊𝚒𝚕𝚜 𝚒𝚗 𝚊𝚗𝚘𝚝𝚑𝚎𝚛 𝚝𝚊𝚋𝚕𝚎 𝚊𝚗𝚍 𝚖𝚊𝚔𝚎 𝚒𝚝 𝚊 𝚖𝚞𝚕𝚝𝚒-𝚜𝚝𝚎𝚙 𝚙𝚛𝚘𝚌𝚎𝚜𝚜;
𝙰𝚜 𝚏𝚘𝚛 𝚋𝚕𝚘𝚌𝚔𝚎𝚍 𝚛𝚎𝚚𝚞𝚎𝚜𝚝𝚜, 𝙸 𝚜𝚞𝚙𝚙𝚘𝚜𝚎𝚍 𝚒𝚝'𝚜 𝚌𝚊𝚞𝚜𝚎𝚍 𝚋𝚢 𝚝𝚑𝚎 𝚖𝚒𝚜𝚜𝚒𝚗𝚐 𝚑𝚎𝚊𝚍𝚎𝚛 (𝚡-𝚊𝚙𝚒-𝚔𝚎𝚢). 𝙰𝚜 𝚊 𝚠𝚘𝚛𝚔𝚊𝚛𝚘𝚞𝚗𝚍, 𝚢𝚘𝚞 𝚌𝚊𝚗 𝚜𝚎𝚝𝚞𝚙 𝚝𝚠𝚘 𝚟𝚎𝚛𝚜𝚒𝚘𝚗𝚜 𝚘𝚏 𝚝𝚑𝚎 𝚊𝚙𝚒.𝚙𝚑𝚙, 𝚘𝚗𝚎 𝚌𝚘𝚗𝚏𝚒𝚐𝚞𝚛𝚎𝚍 𝚠𝚒𝚝𝚑 𝚍𝚋𝙰𝚞𝚝𝚑 - 𝚝𝚑𝚒𝚜 𝚠𝚒𝚕𝚕 𝚑𝚊𝚗𝚍𝚕𝚎 𝚞𝚜𝚞𝚊𝚕 𝚛𝚎𝚐𝚒𝚜𝚝𝚛𝚊𝚝𝚒𝚘𝚗 𝚊𝚗𝚍 𝚝𝚑𝚎 𝚘𝚝𝚑𝚎𝚛 𝚒𝚜 𝚏𝚘𝚛 𝚊𝚙𝚒𝙺𝚎𝚢𝙳𝚋𝙰𝚞𝚝𝚑 𝚏𝚘𝚛 𝚞𝚜𝚎 𝚋𝚢 𝚢𝚘𝚞𝚛 𝚏𝚛𝚘𝚗𝚝𝚎𝚗𝚍 |
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.
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.
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.
@@ -71,6 +71,15 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface | |||
$usernameColumnName = $this->getProperty('usernameColumn', 'username'); | |||
$usernameColumn = $table->getColumn($usernameColumnName); | |||
$passwordColumnName = $this->getProperty('passwordColumn', 'password'); | |||
$usernamePattern = $this->getProperty('usernamePattern','/^[A-Za-z0-9]+$/'); // specify regex pattern for username, defaults to alphanumeric characters |
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'
@@ -71,6 +71,15 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface | |||
$usernameColumnName = $this->getProperty('usernameColumn', 'username'); | |||
$usernameColumn = $table->getColumn($usernameColumnName); | |||
$passwordColumnName = $this->getProperty('passwordColumn', 'password'); | |||
$usernamePattern = $this->getProperty('usernamePattern','/^[A-Za-z0-9]+$/'); // specify regex pattern for username, defaults to alphanumeric characters | |||
$usernameMinLength = (int)$this->getProperty('usernameMinLength',5); | |||
$usernameMaxLength = (int)$this->getProperty('usernameMaxLength',30); |
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.
}else if($key === $passwordColumnName){ | ||
$data[$passwordColumnName] = password_hash($password, PASSWORD_DEFAULT); | ||
}else{ | ||
$data[$key] = filter_var($value, FILTER_VALIDATE_EMAIL) ? $value : filter_var($value,FILTER_SANITIZE_ENCODED); |
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.