hi all,

i have this password changer which enables users to change their password (I bet u wouldnt have guessed that from the title...)


heres the script...

PHP Code:

<?php
$user= ( isset($_GET['user']) ) ? $_GET['user'] : $_SESSION['user']['user'];
$sql="SELECT * FROM users WHERE user='$user' ";
$result=mysql_query($sql) or die ('SQL error :'.mysql_error());
$row=mysql_fetch_array($result);
if ($_POST['submit'])
{
$password   =(isset($_POST['password']))   ? mysql_real_escape_string($_POST['password'],$link_db)      : '';
$password1  =(isset($_POST['password1']))  ? mysql_real_escape_string($_POST['password1'],$link_db)      : '';
$password2  = $_POST['password2'];
if ($_POST['current_pword'] != $row['password'])
{
$error_msg = 'Please enter your current password correctly';
}
else {} 
             if (empty($password))  $error_msg='You must fill the password field';
       elseif (empty($password2)) $error_msg='You must retype the password';
             elseif ($password!=$password2) $error_msg='The passwords are not the same';
else { 
$result = mysql_query("UPDATE users SET password='".sha1($password)."'  WHERE user='{$_SESSION['user']['user']}' ", $link_db) 
or die ('Error updateing. Conatct admin. SQL ERROR :'.mysql_error());
$error_msg = 'Password Changed';
}
}      
else {$error_msg = 'Please fill in the fields.';} // if post else..
?>
<div style="border: dashed 1px; text-align: center; margin: 5px">
<span style="color: red"><? echo "$error_msg"; ?></span>
<form method="post" action="<?php echo $_SERVER['php_self']; ?>">
<table>
<tr><td>Your <b>Current password:</b></td><td> <input type="password" name="current_pword" /></td></tr>
<tr><td>Choose a <b>New Password:</b></td><td><input type="password" name="password" /></td></tr>
<tr><td>Confirm your <b>New Password:</b></td><td><input type="password" name="password2" /></td></tr>
</table>
<input type="submit" name="submit" accesskey="s" title="(ALT + S)" value="Save" />
</form>
</div>

The problem is, the current password isnt working...

u can type anything in the field and then type the new password into the two other fields and it appears to change it...

whats wrong and why isnt it first checking if current password = password form db?

and then continureing if it is the same and showing the error if it isnt.


before someone says it the db connect and that are in the page this is included into.. u can see this at http://calm.dansgalaxy.co.uk/usercp.php?id=pword_change (you have to login u can login with user: test pass: test)

i have just noticed it appears to have blanked all the other info in the table for admin (which was the user i was logged in as when testing this...)



Thanks.
Dan

Hi dansgallerie,

maybe you should clean up your code.
I have made some notes

<?php
$user= ( isset($_GET['user']) ) ? $_GET['user'] : $_SESSION['user']['user'];
$sql="SELECT * FROM users WHERE user='$user' ";
$result=mysql_query($sql) or die ('SQL error :'.mysql_error());
$row=mysql_fetch_array($result);

if ($_POST['submit']){
  $password   =(isset($_POST['password']))   ? mysql_real_escape_string($_POST['password'],$link_db)      : '';
  $password1  =(isset($_POST['password1']))  ? mysql_real_escape_string($_POST['password1'],$link_db)      : '';
  $password2  = $_POST['password2'];

    if ($_POST['current_pword'] != $row['password']){
      $error_msg = 'Please enter your current password correctly';
    }
    
    else {} // no else needed
             
    if (empty($password))  
      $error_msg='You must fill the password field';
    
    elseif (empty($password2)) 
      $error_msg='You must retype the password';
    
    elseif ($password!=$password2) 
       $error_msg='The passwords are not the same';

// Here is a '}' missing, otherwise the following else has no use
       
else { 
  $result = mysql_query("UPDATE users SET password='".sha1($password)."'  WHERE user='{$_SESSION['user']['user']}' ", $link_db) 
     or die ('Error updateing. Conatct admin. SQL ERROR :'.mysql_error());
  $error_msg = 'Password Changed';
}

} // this one is the one which is missing above

// Whats these else for?
else {
  $error_msg = 'Please fill in the fields.';
} 
// if post else..
?>

<div style="border: dashed 1px; text-align: center; margin: 5px">
<span style="color: red"><? echo "$error_msg"; ?></span>
<form method="post" action="<?php echo $_SERVER['php_self']; ?>">
<table>
<tr><td>Your <b>Current password:</b></td><td> <input type="password" name="current_pword" /></td></tr>
<tr><td>Choose a <b>New Password:</b></td><td><input type="password" name="password" /></td></tr>
<tr><td>Confirm your <b>New Password:</b></td><td><input type="password" name="password2" /></td></tr>
</table>
<input type="submit" name="submit" accesskey="s" title="(ALT + S)" value="Save" />
</form>
</div>

sadly on my pc isn't php running in the moment so
you have to look for yourself

greetings akratellio

Yeah, if I had to have a guess, I'd say it's that hanging empty else clause. I'm assuming you wanted to wrap all of that other code up in that empty else. I think that's your problem.

akratellio is right, though. Your code is disgusting to look at... it's no wonder you can't see these problems. If you format it properly, a lot of mistakes are a lot easier to pick up on.

I was able to change the password for user test to "wowwow", then I tried to re-login and all was warking ok.

i know u can change the password its the curent password validation that isnt working. :S

Dan

i think i fixed all the elses...

PHP Code:

<?php
$user= ( isset($_GET['user']) ) ? $_GET['user'] : $_SESSION['user']['user'];
$sql="SELECT * FROM users WHERE user='$user' ";
$result=mysql_query($sql) or die ('SQL error :'.mysql_error());
$row=mysql_fetch_array($result);
if ($_POST['submit'])
{
$password   =(isset($_POST['password']))   ? mysql_real_escape_string($_POST['password'],$link_db)      : '';
$password1  =(isset($_POST['password1']))  ? mysql_real_escape_string($_POST['password1'],$link_db)      : '';
$password2  = $_POST['password2'];
if ($_POST['current_pword'] != $row['password'])
{
$error_msg = 'Please enter your current password correctly';
}
else {} 
             if (empty($password))  {$error_msg='You must fill the password field';}
       elseif (empty($password2)) {$error_msg='You must retype the password';}
             elseif ($password!=$password2) {$error_msg='The passwords are not the same';}
else { 
$result = mysql_query("UPDATE users SET password='".sha1($password)."'  WHERE user='{$_SESSION['user']['user']}' ", $link_db) 
or die ('Error updateing. Conatct admin. SQL ERROR :'.mysql_error());
$error_msg = 'Password Changed';
}
}      
else {$error_msg = 'Please fill in the fields.';} // if post else..
?>
<div style="border: dashed 1px; text-align: center; margin: 5px">
<span style="color: red"><? echo "$error_msg"; ?></span>
<form method="post" action="<?php echo $_SERVER['php_self']; ?>">
<table>
<tr><td>Your <b>Current password:</b></td><td> <input type="password" name="current_pword" /></td></tr>
<tr><td>Choose a <b>New Password:</b></td><td><input type="password" name="password" /></td></tr>
<tr><td>Confirm your <b>New Password:</b></td><td><input type="password" name="password2" /></td></tr>
</table>
<input type="submit" name="submit" accesskey="s" title="(ALT + S)" value="Save" />
</form>
</div>

anytthing else wrong which is causeing this not to validate the current password before it changes it?...

Thanks,
dan

Your script never actually checks to see if the given password is the current password. An array is returned as $row which I assume, should contain the original password however, unless I missed something while looking over the code, no check is done to see if the one the user inputs is equal to the one returned from the DB.

PHP Code:

if ($_POST['current_pword'] != $row['password'])
{
$error_msg = 'Please enter your current password correctly';
}
else {} 


This should...

i think i fixed all the elses...

I don't think that is any different to the first one!

Dan... do you not see anything wrong with that chunk of code you just pasted? That empty else is the problem and it's what we've been trying to tell you all along.

That's it. I'm done. No offence mate but every single thread you start involves code that is malformed and shows no signs of you learning anything from your previous threads. Then, you go on and seem to ignore every post that is trying to get you right and instead argue with others. No more for me, mate...

I thought someone might catch this, but let me chime in:

You're comparing the password with

if ($_POST['current_pword'] != $row['password'])

But, the value of $row['password'] is NOT the password, it's the sha1 hash of the password:

$result = mysql_query("UPDATE users SET password='".sha1($password)."'  WHERE user='{$_SESSION['user']['user']}' ", $link_db)  or die ('Error updateing. Conatct admin. SQL ERROR :'.mysql_error());

So, compare the hash of the post to the current password:

if (sha1($_POST['current_pword']) != $row['password'])

ok.

i did a hell of alot of testing before posting this here, when i looked at my script i can honestly say i fixed the errors with the else its nested ifs (i think thats what they called.) so wheres the problem with elses?

And the differance is i closed all the else tags which were open...

ok im going to post a heavily commented one. then hopefully someone can advise me whats wrong with it or if it is to do with the elses how it is...

PHP Code:

<?php
$user= ( isset($_GET['user']) ) ? $_GET['user'] : $_SESSION['user']['user'];
$sql="SELECT * FROM users WHERE user='$user' ";
$result=mysql_query($sql) or die ('SQL error :'.mysql_error());
$row=mysql_fetch_array($result);
if ($_POST['submit']) // IF the form is submitted do the followin [calling this IF 1]
{ // If 1 open
$password   =(isset($_POST['password']))   ? mysql_real_escape_string($_POST['password'],$link_db)      : '';
$password1  =(isset($_POST['password1']))  ? mysql_real_escape_string($_POST['password1'],$link_db)      : '';
$password2  = $_POST['password2'];
 
 
if ($_POST['current_pword'] != $row['password']) // This is a if/else structure inside if 1 [calling this if 2]
{ // If 2 Open
$error_msg = 'Please enter your current password correctly';
} // If 2 Close
else {} // If 2 else last clost for the if/esle structure of if 2
             if (empty($password))  // this checks new passwords... [Calling this if 3]
    {$error_msg='You must fill the password field';} //open/close if 3
       elseif (empty($password2)) 
    {$error_msg='You must retype the password';}
             elseif ($password!=$password2) 
    {$error_msg='The passwords are not the same';} 
             else { 
$result = mysql_query("UPDATE users SET password='".sha1($password)."'  WHERE user='{$_SESSION['user']['user']}' ", $link_db) 
or die ('Error updateing. Conatct admin. SQL ERROR :'.mysql_error());
$error_msg = 'Password Changed';
                   }// last close for the if/else structure for if 3
 
} // close for if 1
 
else {$error_msg = 'Please fill in the fields.';} // if post else.. This is the last closing tag for If/else Structure 1
?>
<div style="border: dashed 1px; text-align: center; margin: 5px">
<span style="color: red"><? echo "$error_msg"; ?></span>
<form method="post" action="<?php echo $_SERVER['php_self']; ?>">
<table>
<tr><td>Your <b>Current password:</b></td><td> <input type="password" name="current_pword" /></td></tr>
<tr><td>Choose a <b>New Password:</b></td><td><input type="password" name="password" /></td></tr>
<tr><td>Confirm your <b>New Password:</b></td><td><input type="password" name="password2" /></td></tr>
</table>
<input type="submit" name="submit" accesskey="s" title="(ALT + S)" value="Save" />
</form>
</div>

This is the same code wise as what i last posted, it has just been commented on and shifted alil to make it easier to read..

i was always lead to believe that it was gd practice to always finish if statments with a else {} even if they empty and wouldnt the else {} be interpreted to be with If 2 not if 1?
because if 1 is only closed before the very last else {} in the code...


to jeremeny Miller:
i know im saving it encrypted as sha1 i was under the impression when it gets retrieved form the db its unencrypted?...

even if this is so though, it should make the script keep failing with the 'Please enter your current password correctly' message, instead the script seems to ignore whether the current password = the current or not.

i dont mean to appear like im ignoreing you often its becuase i test it it dont work for whatever reason and i might then test what someone else has said to me.

OK 2 things:

1) You don't know what a hash is if you think there's anything out there which can undo it, so research hashes.

2) What exactly do you think it will say if the conditional here is true?

if ($_POST['current_pword'] != $row['password']) // This is a if/else structure inside if 1 [calling this if 2]
{ // If 2 Open
$error_msg = 'Please enter your current password correctly';
}

Personally, I think it will give the error message "Please enter your current password correctly".

Good luck!

Hmmm... In checking to see what PHP.net says on sha1, I found http://us2.php.net/manual/en/function.sha1.php#70474 , which was new to me, so thanks! I learned something too.

sorry i am stil like Waaa!? to quite alot of the slightly more advanced php.

I'm just kind of browsing through, but where the password is stored is it encrypted, because if the password is encrypted you would have to convert the input to the encryption the validate. Also check your permissions.

issue fixed

wasnt the extra else...