Textpattern CMS support forum
You are not logged in. Register | Login | Help
- Topics: Active | Unanswered
Re: How to compare passwords in TXP?
Another version of the plugin.
I would be very grateful for opinions.
Tried to make comments that would be easier to read.
Sorry for my English.
Offline
Re: How to compare passwords in TXP?
Nice to see new up-coming plugin authors :)
skrishi wrote:
I would be very grateful for opinions.
As Ruud said earlier, return ends the execution of the current function. You do not need (messy) nested if/else structure when you are already using return.
Also as Ruud said, you probably should limit the request using sleep() to prevent brute attacks.
Currently you are using simple DB time updating (just lockless inserts, row counts and updates) to prevent over-activity; just don’t freak out when there are two Johns and three Eves in the table.
mysql_query()
Textpattern offers safe_query() function.
As far the installer itself goes, You probably shouldn’t load installer on every request. You can use life-cycle callbacks to run installer, or hook it once on the backend. You may get support requests from users about notices popping up if you keep it like that. CREATE operation shouldn’t be thrown loosely.
CREATE TABLE IF NOT EXISTS gvv_user_edit
You shoud use safe_pfx() to prefix the table name. TXP has an option to use prefixed tables, and if prefix is used, the queries won’t be looking for prefix-less table.
name TEXT
txp_users table doesn’t store that long user names ;) Just varchar(64) might be better. Also the users have an ID (integer) which might be better than the name. Or you could store the times in the txp_users table.
If you choose to use your own table, then using unique/primary wouldn’t hurt. You don’t want duplicate rows there.
COLLATE utf8_latvian_ci
Not all users are from latvia. Just set the default character set, and let MySQL set the collations based on it, or by the parent defaults. Refer MySQL manual.
time int
datetime instead of int? Int stores integers up to set length (2147483647 for signed), while datetime stores any date. Not that you will reach it anytime soon :p
empty($_POST['stage_pass'])
You could use ps(). If you don’t want to use ps(), at least change the empty() to isset(). It’s not just empty, it’s not even set.
if (8 <= strlen($new_pass)) //if the password longer than 8 characters
strlen() isn’t unicode compatible. If you want to count the length in characters (not in bytes) use multibyte safe functions (mb_strlen).
mail()
TXP offers txpMail() function, which does take server configuration and encoding into account (which you didn’t).
<div id="accaunt_name"><label>Repeat E-mail:</div>
Label tags are unclosed. As Ruud said, you should allow customization, let the user to change the markup with a their form. Not to mention that your form is completely static, it doesn’t remember the values if error occurs. Would be great if it was error aware and remembered the input.
if (!preg_match("([0-9a-z]([-_.]?[0-9a-z])*[0-9a-z]([-.]?[0-9a-z])*\\.[a-wyz][a-z](fo|g|l|m|mes|o|op|pa|ro|seum|t|u|v|z)?)”, $email))@
Should generate false positives. Doesn’t comply with RFCs. You don’t need to check the email if you are matching it to existing users. If you really need to validate email, you can use filter_var() which is the best simple regex method.
mysql_query("DROP TABLE gvv_user_edit");
echo gvv_gTxt('db_del_ok');
return ($ok=2);
return ($ok=1);
The echos and returns don’t actually know if it’s ok.
$lang = array([..])
You can use TextPacks (and/or $textarray). That way the user doesn’t need to modify the source code if he wants to change the strings.
Last edited by Gocom (2011-03-26 00:14:08)
Offline
Re: How to compare passwords in TXP?
Gocom wrote:
Nice to see new up-coming plugin authors :)
=)
As Ruud said earlier, return ends the execution of the current function. You do not need (messy) nested if/else structure when you are already using return.
This is a fatal error?
mysql_query("DROP TABLE gvv_user_edit");
echo gvv_gTxt('db_del_ok');
return ($ok=2);
return ($ok=1);
The echos and returns don’t actually know if it’s ok.
Something here that something not understand?
You can use TextPacks (and/or $textarray). That way the user doesn’t need to modify the source code if he wants to change the strings.
You can an example of plug?
ps: Thanks for the tip, I have almost everything straightened.
Last edited by skrishi (2011-03-26 19:15:41)
Offline
Re: How to compare passwords in TXP?
mysql_query("DROP TABLE gvv_user_edit");
echo gvv_gTxt('db_del_ok');The assumption here is that the mysql_query never fails. If it does, then the ‘db_del_ok’ message is incorrect. You should check if the result of the mysql_query command to see if it was succesful and then echo whether it was.
return ($ok=2);
return ($ok=1);Same problem here. You have such return lines just below calling the safe_update function, but you’re not checking if the safe_update function actually succeeded in updating anything.
Also, I assume you’re writing “$ok=2” to improve readability somehow, but that’s something (to me at least) that’s a bit unusual. I’m more used to seeing something like this, which does the exact same thing. I’d pick a different variable name anyway, because if $ok can be 1, 2 or other values… which of these is okay? ($status or $error or $return_code would be more meaningful, IMHO):
return 2;
return 1;This is a fatal error?
No. Gocom and I just find it easier to read code if written in the way we suggest. Code that is simpler and can easily be read, is easier to understand, maintain and debug.
if (!preg_match("([0-9a-z]([-_.]?[0-9a-z])*[0-9a-z]([-.]?[0-9a-z])*\\.[a-wyz][a-z](fo|g|l|m|mes|o|op|pa|ro|seum|t|u|v|z)?)”, $email))TXP has a built-in function is_valid_email which you can use.
Offline
Re: How to compare passwords in TXP?
Here , I made the changes. If you are comfortable, look again.
It remains to make a form that the user would be able to change them without interfering with the code.
I’ll do this later.
Syntax: I’m learning php 3 weeks. Hard for me so far to cut. I’m starting to get confused in the code. And so easy for me to find errors and change if new ideas.
Can I make a section of language as a zem_contact_lang?
On another, I do not know how.
Although it can be an example and I’ll try to do the same.
Unfortunately Russian documentation is very little, and my English is very bad.
I take http://translate.google.ru/ that would understand you. = (
There are no errors security? I was able to remove them?
ruud
TXP has a built-in function is_valid_email which you can use.
I’ll try it later.
But not sure. For example, from ps() I had to give up, I could not get it to work.
If you have any comments, I’ll be glad to listen.
Thank you.
Offline
Re: How to compare passwords in TXP?
if (!preg_match("([0-9a-z]([-_.]?[0-9a-z])*[0-9a-z]([-.]?[0-9a-z])*\\.[a-wyz][a-z](fo|g|l|m|mes|o|op|pa|ro|seum|t|u|v|z)?)”, $email))can be replaced with:
if (!is_valid_email($email))Compare what you wrote:
function gvv_edit_pass($form_edit_pass)
{
if (!isset($_POST['stage_pass']))
{
return gvv_form_edit_pass(); //if stage_pass NULL, then representing the form
}
else
{ //else extract
extract(gpsa(array('new_pass', 'cnew_pass', 'oldpass', 'name')));
if (!empty($new_pass) and !empty($cnew_pass) and !empty($oldpass)) //if field is not NULL
{
if (8 <= mb_strlen($new_pass)) //if the password longer than 8 characters
{
if ($new_pass != $cnew_pass) // check whether the passwords match
{
return gvv_gTxt('pass_er');
}
else
{
$a=gvv_query_db ($name); //Look, if there were attempts to change your password or email
if ($a == 1)
{
return gvv_gTxt('time_out');
}
else
{ //Consider, if the password is entered correctly
if(safe_count(safe_pfx('txp_users'), "(pass=password('".doSlash($oldpass)."') or pass=password(lower('".doSlash($oldpass)."'))) and name='".doSlash($name)."'") == 1)
{
$rs = safe_update(safe_pfx('txp_users'), "pass = password(lower('".doSlash($new_pass)."'))", "name = '".doSlash($name)."'");
safe_delete(safe_pfx('gvv_user_edit'), "name = '".doSlash($name)."'");
if($rs)
{ //if so then send a message
$to_address = safe_field('email',safe_pfx('txp_users')," name = '".doSlash($name)."'");
$site_name = site_name();
$site_url = site_url();
$subject = "$site_name:".gvv_gTxt('mail_subject_new_pass');
$body = "\n".gvv_gTxt('mail_body_pass').": $new_pass \n$site_url";
txpMail($to_address, $subject, $body);
return gvv_gTxt('pass_ok');
}
else
{
return gvv_gTxt('unknow_err');
}
}
else
{
return gvv_gTxt('old_pass_error');
}
}
}
}
else
{
return gvv_gTxt('smoll_pass');
}
}
else
{
return gvv_gTxt('password_all_back');
}
}
}To this, which (unless I made a mistake) does the same thing):
function gvv_edit_pass($form_edit_pass)
{
if (!isset($_POST['stage_pass']))
{
return gvv_form_edit_pass(); //if stage_pass NULL, then representing the form
}
extract(gpsa(array('new_pass', 'cnew_pass', 'oldpass', 'name')));
if (empty($new_pass) or empty($cnew_pass) or empty($oldpass)) //if field is not NULL
{
return gvv_gTxt('password_all_back');
}
if (8 <= mb_strlen($new_pass)) //if the password longer than 8 characters
{
return gvv_gTxt('smoll_pass');
}
if ($new_pass != $cnew_pass) // check whether the passwords match
{
return gvv_gTxt('pass_er');
}
// Look, if there were attempts to change your password or email
if (gvv_query_db($name) == 1)
{
return gvv_gTxt('time_out');
}
//Consider, if the password is entered correctly
if(safe_count(safe_pfx('txp_users'), "(pass=password('".doSlash($oldpass)."') or pass=password(lower('".doSlash($oldpass)."'))) and name='".doSlash($name)."'") != 1)
{
return gvv_gTxt('old_pass_error')
}
safe_delete(safe_pfx('gvv_user_edit'), "name = '".doSlash($name)."'");
$rs = safe_update(safe_pfx('txp_users'), "pass = password(lower('".doSlash($new_pass)."'))", "name = '".doSlash($name)."'");
if($rs)
{ //if so then send a message
$to_address = safe_field('email',safe_pfx('txp_users')," name = '".doSlash($name)."'");
$site_name = site_name();
$site_url = site_url();
$subject = "$site_name:".gvv_gTxt('mail_subject_new_pass');
$body = "\n".gvv_gTxt('mail_body_pass').": $new_pass \n$site_url";
txpMail($to_address, $subject, $body);
return gvv_gTxt('pass_ok');
}
else
{
return gvv_gTxt('unknow_err');
}
}For me, it’s easier to understand the code that way, because the cause and effect of the if-constructs are close to each other. It avoids getting confused by the many levels of indenting (I use tabs instead of spaces, btw.).
Offline
Re: How to compare passwords in TXP?
I looked at another function as well, trying to understand what it does:
function gvv_query_db ($name)
{ //Check whether user in the table
if(safe_count(safe_pfx('gvv_user_edit'), "name = '".doSlash($name)."'") == 1)
{
$repet = safe_field('repet', safe_pfx('gvv_user_edit'), "name = '".doSlash($name)."'");
//check how many once tried to remember the password
if ($repet < 5)
{ //If less than 5 once, then add 1 and return to "allowed to try"
$repet = $repet + 1;
safe_update(safe_pfx('gvv_user_edit'), "repet = '$repet'", "name = '".doSlash($name)."'");
return ;
}
else
{ //consider whether the time recorded
$time = safe_field('time',safe_pfx('gvv_user_edit')," name = '".doSlash($name)."'");
if(!empty($time)) //time is not?
{
if($time > time()) //time more than now
{
$a=1;
return $a; // relax and remember your password
}
else
{ //otherwise you can make an attempt initially
$repet = 1;
safe_update(safe_pfx('gvv_user_edit'), "repet = '$repet', time = ''", "name = '".doSlash($name)."'");
return ;
}
}
else
{ //if there was no time, then write to the table time of 10 minutes
$time = time() + 600;
safe_update(safe_pfx('gvv_user_edit'), "time = '$time'", "name = '".doSlash($name)."'");
$a=1;
return $a;
}
}
}
else
{ //If the user does not
$repet = 1;
safe_insert(safe_pfx('gvv_user_edit'), "repet = '$repet', name = '".doSlash($name)."'");
return ;
}
}I rewrote that to this, which should do the same thing, difference being that I use TRUE/FALSE instead of 1/undefined as return values. If you combine that with a different function name, like ‘ggv_query_db_allowed’, you can call the function like this and it’s easy to understand: if (ggv_query_db_allowed()) { do something }:
// returns TRUE if you are allowed another try or FALSE if you tried too many times in a short time.
function gvv_query_db ($name)
{
//check how many once tried to remember the password
$repet = safe_field('repet', safe_pfx('gvv_user_edit'), "name = '".doSlash($name)."'");
// user didn't exist yet?
if($repet === FALSE)
{
safe_insert(safe_pfx('gvv_user_edit'), "repet = 1, name = '".doSlash($name)."'");
return TRUE;
}
//If less than 5 once, then add 1 and return to "allowed to try"
if ($repet++ < 5)
{
safe_update(safe_pfx('gvv_user_edit'), "repet = $repet", "name = '".doSlash($name)."'");
return TRUE;
}
//consider whether the time recorded
$time = safe_field('time',safe_pfx('gvv_user_edit')," name = '".doSlash($name)."'");
//if there was no time, then write to the table time of 10 minutes
if (empty($time))
{
$time = time() + 600;
safe_update(safe_pfx('gvv_user_edit'), "time = '$time'", "name = '".doSlash($name)."'");
return FALSE;
}
if($time > time()) //time more than now
{
return FALSE; // relax and remember your password
}
else
{ //otherwise you can make an attempt initially
safe_update(safe_pfx('gvv_user_edit'), "repet = 1, time = ''", "name = '".doSlash($name)."'");
return TRUE;
}
}If I understand correctly, this means: you’re allowed to try if you’ve never tried before or tried less than 5 times. But on the 5th attempt, you’re checking if $time exists. If it does not, then you set the time to 10 minutes in the future (and that attempt fails) and any other attempts that happen during those 10 minutes fail as well. Once the 10 minutes have passed, you’re once again granted 5 new attempts. So basically this allows to to quickly try 5 times, but over a longer period you can only try at most once per 2 minutes on average. Nice.
Syntax: I’m learning php 3 weeks.
Is PHP your first programming language or did you learn writing programs in other languages before?
Offline
Re: How to compare passwords in TXP?
Thanks, now I understand.
I will try to follow it.
If I understand correctly, this means: you’re allowed to try if you’ve never tried before or tried less than 5 times. But on the 5th attempt, you’re checking if $time exists. If it does not, then you set the time to 10 minutes in the future (and that attempt fails) and any other attempts that happen during those 10 minutes fail as well. Once the 10 minutes have passed, you’re once again granted 5 new attempts. So basically this allows to to quickly try 5 times, but over a longer period you can only try at most once per 2 minutes on average. Nice.
Yes, probably a good thing.
I will try to implement it now.
Is PHP your first programming language or did you learn writing programs in other languages before?
Yes, 15 years ago, VB, and the basis C, but did not write anything on the C.
So, I can say no. =)
Perhaps that is why so many questions.
In connection with the release of 4.4.
As it is now worth checking your password?
And will the source code posted on PHPXref ? There is organized a very good search functions and variables.
Last edited by skrishi (2011-03-27 18:03:35)
Offline
Re: How to compare passwords in TXP?
skrishi wrote:
And will the source code posted on PHPXref ? There is organized a very good search functions and variables.
PHPXref is the old site, the new location is PHPCrossRef.
We Love TXP . TXP Themes . TXP Tags . TXP Planet . TXP Make
Offline
Re: How to compare passwords in TXP?
hcgtv wrote:
PHPXref is the old site, the new location is PHPCrossRef.
Thank you, now would be convenient to seek the necessary.
Offline
Re: How to compare passwords in TXP?
skrishi wrote:
In connection with the release of 4.4.
As it is now worth checking your password?
I don’t understand the question.
Offline
Re: How to compare passwords in TXP?
ruud wrote:
I don’t understand the question.
Can I use txp_validate() to check the password instead of (safe_count(safe_pfx('txp_users'), "(pass=password('".doSlash($oldpass)."')) or pass=password(lower('".doSlash($oldpass)."')) and name='".doSlash($name)."'") == 1)?
It will be right for the public side?
Offline