5

I'm working with PHP and mysqli, what the program is doing is that it is asking for a reset code and email address if the email add and reset code are found in the database it sets the password,this part of the function is working,

I need help with this part: what I need to do is tell the user if the password was set or not so if the update was successful or not.

What I'm working on:

$uinsert =  "UPDATE member SET password = '$password' WHERE emailadd = '$emailadd' AND resetCode = '$resetcode'";

    $update = mysqli_query($mysqli, $uinsert) or die(mysqli_error($mysqli));

     if(mysqli_affected_rows($update) == 1 ){ //ifnum
        header("location: ../index.php"); // Redirecting To Other Page
    }
    else{
        echo "<script> alert('Incorrect code, try again!');</script>";
    }

Note: $mysqli is my connection string

Coffee coder
  • 162
  • 1
  • 1
  • 9
  • 1
    And what exactly is your question or problem you are faced with? – JustBaron Feb 19 '17 at 13:45
  • 1
    Your code is vulnerable to [SQL-Injections](https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php). Please start using Prepared, Parameterized Queries. – Charlotte Dunois Feb 19 '17 at 13:45
  • Unrelated to the actual question, but, out of curiosity, may I ask where the variables `$password`, `$emailadd` and `$resetcode` come from? To the question: So instead of a redirection, you want a feedback for success? Then instead of doing a redirect, do something like you do with the javascript below. – Maximilian Gerhardt Feb 19 '17 at 13:45
  • @MaximilianGerhardt I have more code in the file, didn't paste it all in but its assigned. – Coffee coder Feb 19 '17 at 13:47
  • @CharlotteDunois I have mysqli_real_escape_string around password – Coffee coder Feb 19 '17 at 13:47
  • @justbaron How to check if update was successful or not – Coffee coder Feb 19 '17 at 13:48
  • RTM http://php.net/manual/en/mysqli.affected-rows.php – Funk Forty Niner Feb 19 '17 at 13:52
  • `if(mysqli_affected_rows($mysqli) >0 )` or no comparison at all. – Funk Forty Niner Feb 19 '17 at 13:54
  • @Fred-ii- Thank you so much that works! – Coffee coder Feb 19 '17 at 13:58
  • *"I have mysqli_real_escape_string around password"* - we don't know where that's located and how it's used, the variables and if they contain value or not. You were also using `hash_hmac` in your other question http://stackoverflow.com/q/41430401/1415724 so we don't know if you are now or not. You also should not be manipulating the password in any way then working with hashes. – Funk Forty Niner Feb 19 '17 at 13:59
  • you're welcome. I posted my answer below @Coffeecoder – Funk Forty Niner Feb 19 '17 at 14:00
  • @Fred-ii- Noted :) Thanks for your time – Coffee coder Feb 19 '17 at 14:01
  • @Coffeecoder You're welcome. Btw, if you're still using `hash_hmac`, you may want to move over to `password_hash()`, it's much safer nowadays ;-) – Funk Forty Niner Feb 19 '17 at 14:03
  • Move over to Prepared, Parameterized Statements. Escaping does not provide 100% protection. – Charlotte Dunois Feb 19 '17 at 14:05
  • @Coffeecoder I made a few edits and you may want to reload my answer. Especially the part about "valid passwords", if and when you decide to move to using `password_hash()`. – Funk Forty Niner Feb 19 '17 at 14:16
  • 1
    @CharlotteDunois the code as is given here does not have any vulnerability. You should not make any assumptions as to where the content of the variables come from, especially when it has nothing to do with the question. – JG Estiot Jul 03 '19 at 04:52
  • On my case Im using mariadb and php PDO, so on php I just check after update statement execute: $stmt->execute(); $count = $stmt->rowCount(); The $count variable should have the number of rows affected after update was executed, depending the case, if your are updating just one row, compare $count with 1. if($count != 1) {do something } else {show error} hope this help. – Ricardo Rivera Nieves Dec 06 '20 at 02:33

4 Answers4

8

"@Fred-ii- Thank you so much that works! – Coffee coder 58 secs ago"

Use if(mysqli_affected_rows($mysqli) >0 ) or no comparison at all.

Sidenote: ==1 is only comparing for 1, as opposed to >0 which you may be trying to update more than one row. However and on the rare occasion, >0 is required where this has also happened to me before; that is the reason of my answer.

affected_rows() uses the connection, not the one for the query.

Plus, if you're storing plain text passwords, use password_hash() since it's much safer:

Sidenote: If you do decide to move over to that function, make sure that you do not manipulate the password at all. Hashing/verifying it takes care of that and you may be doing more harm than good in doing so and limiting passwords.

I.e.: A valid password of test'123 would be interpreted as test\'123 and rendering FALSE when using real_escape_string() for example.

Or you may still be using hash_hmac as per your other question Comparing/check if correct Password from mysqli database [hash_hmac]

and a prepared statement:

It is also best to add exit; after header. Otherwise, your code may want to continue to execute.

header("location: ../index.php");
exit;
Community
  • 1
  • 1
Funk Forty Niner
  • 74,450
  • 15
  • 68
  • 141
  • 2
    @Coffee coder You are updating multiple passwords at a time? Are you sure that is what you want? I think this smells like a false-positive. How many rows are you actually updating? Ensuring there is only 1 password update seems logical (without all the necessary information, mind you). – mickmackusa Feb 19 '17 at 14:03
  • 1
    @mickmackusa I'm with you on that. – Funk Forty Niner Feb 19 '17 at 14:05
  • @mickmackusa to a certain point, as per [a comment](http://stackoverflow.com/questions/42327938/check-if-update-query-was-successful-php-mysqli#comment71809118_42327977) I left under your answer, which I am wondering why you're questioning it now. – Funk Forty Niner Feb 19 '17 at 14:22
  • @mickmackusa a single password – Coffee coder Feb 19 '17 at 14:22
  • @Coffeecoder then this is proof you have an error in your process. Your sql was failing on ==1, and passes on >0, which means you are update 2 or more rows and this is definitely not good. – mickmackusa Feb 19 '17 at 14:26
  • guys; I'm sick of this bickering. Open up a chatroom and take it there. @mickmackusa if you feel so strongly that my answer is incorrect, downvote it and move on. – Funk Forty Niner Feb 19 '17 at 14:29
  • @mickmackusa No, I was testing soo many times that I didn't realise that I updated the password then tried to update the same password since it was the same it didn't make a difference in db thus it didn't show as updated... so both you and Fred answers are correct.. – Coffee coder Feb 19 '17 at 14:30
  • So are you saying you accidentally duplicated a member row and the UPDATE was >1 because of it? Then the final fix is tidying up the table. I am not downvoting you -Fred-ii- because you are not wrong. – mickmackusa Feb 19 '17 at 14:32
  • @mickmackusa no that's not the case, no where near. it has been solved and I need to get back to working on another function, so bye! Have a great day, thanks for your time and sorry if I wasted yours! – Coffee coder Feb 19 '17 at 14:34
  • @Coffeecoder How can we both be correct? ok, well unaccept my answer then and accept the other guy's, because I don't know where to throw myself here anymore and tired of chatting really. It's Sunday morning, I'm trying to enjoy it by relaxing. All this I'm right/he's right is not making feel relaxed at all. You guys sort it out. – Funk Forty Niner Feb 19 '17 at 14:34
  • Fred your answer and his answer are both correct, end of story – Coffee coder Feb 19 '17 at 14:35
  • That's actually my question, mysqli_affect_rows() is definitely a good solution, although it returns 0 if the content of the field hasn't changed when you do an update. I'm looking for an alternate solution to save me having to select first and compare (I'm just being lazy). – AdheneManx Nov 28 '18 at 08:34
3
if (mysqli_affected_rows($mysqli) == 1 ) {

Because mysqli_affected_rows() does not use the query $update as its parameter, it uses the connection variable: $mysqli

mickmackusa
  • 43,625
  • 12
  • 83
  • 136
  • Thanks, I tried this but it didn't work – Coffee coder Feb 19 '17 at 13:50
  • Can you be more descriptive with your feedback? There is no error message? Is it successful in your phpmyadmin? How many rows are affected? Zero? Two? – mickmackusa Feb 19 '17 at 13:53
  • Thanks for the help I solved it by applying what @Fred-ii- said – Coffee coder Feb 19 '17 at 13:59
  • @Coffee coder Fred-ii-'s answer is almost certainly giving you a false positive. It's like checking if someone is my grandmother by simply asking if they are older than me. Do not leave your code/db this way. You have a serious flaw in either your db table, your sql, or both. – mickmackusa Feb 19 '17 at 14:13
  • @mickmackusa kind of true and false. I had to use `>0` myself on a few occasions where it didn't necessarily have anything to do with more than one row, but a single row, believe it or not. – Funk Forty Niner Feb 19 '17 at 14:18
  • It should not be the case if you have a member table set up to only store uniquely identified users. Every member should have a unique id which is used for these purposes; or else if we UPDATE based on a non-unique email address, then someone else can update my password by first copying my email to their account then updating password. – mickmackusa Feb 19 '17 at 14:23
  • @mickmackusa will it make you happy if the OP accepts instead? For crying outloud. – Funk Forty Niner Feb 19 '17 at 14:25
  • @mickmackusa in order for them to update password they need to add in the random code which is sent to them via email so no what you are saying won't be possible – Coffee coder Feb 19 '17 at 14:26
  • @mickmackusa I'll tell you what; I'll ask the OP to unaccept then I'll find a duplicate for it and close it, how about that? – Funk Forty Niner Feb 19 '17 at 14:26
  • @Fred-ii- accepts what ? – Coffee coder Feb 19 '17 at 14:26
  • @Coffeecoder for you to unaccept my answer and to accept his. I feel like they're eating sour grapes here and I won't have it. To think I even encouraged him on a few of his other answers. – Funk Forty Niner Feb 19 '17 at 14:27
  • I am not attacking you -Fred-ii-. We still need to solve the true problem here. I don't want to drop it until Coffeecoder has it ironed out. – mickmackusa Feb 19 '17 at 14:29
3

Change the parameter of mysqli_affected_rows(), the parameters must be the mysql connection

mysqli_affected_rows($update)

to

mysqli_affected_rows($mysqli)

Please see this reference https://www.w3schools.com/php/func_mysqli_affected_rows.asp

2

pass your mysqli connection object ($connection) to mysqli_affected_rows(connection_object) to check affected rows.

connection_object is like - $con=mysqli_connect("localhost","bd_user","db_password","your_db_name");

So , code will be

if(mysqli_affected_rows($con)== 1 ){ 
   header("location: ../index.php"); 
}
AGM Tazim
  • 2,213
  • 3
  • 16
  • 25