-1

I am creating a login form and am having trouble with it not successfully logging in. My form looks like this

<form method="post" action="phpScripts/loginProcess.php">

    <input type="text" name="uname" value="" placeholder="Username">

    <input type="password" name="pass" value="" placeholder="Password">

    <input id="submit" type="submit" value="submit">

</form>

I have already checked and I am connecting to the database, and my uname and pass are both correct. I have double checked plenty of times. The issue is within my query. Here is the loginProcess.php file

<?php

    require_once ('dbconn.php');



    if(isset($_POST['login'])){

        $uname  = mysqli_real_escape_string($_POST['uname']);
        $pass   = mysqli_real_escape_string($_POST['pass']);

        $query  = mysql_query ("SELECT * FROM staff WHERE uname='$uname'");
        $numrows = mysql_num_rows($query);

        if ($numrows !=0){
            die("Success!!");
        }
        else{
            die("That user doesnt exist");
        }

    }
    else{
        echo "Username or Password incorrect";
    }
?>

With the help of Fred-ii- I was able to figure out my mistakes and really quickly fix my login issue! I wanted to supply the code I used to login in case anyone else came across this problem.

Here is the code that I am using to login. I feel as I can make it a little less redundant with the execute and $userSql/$pwSql. Any suggestions to clean this up would be greatly appreciated!

<?php
ini_set('display_errors', 1); error_reporting(E_ALL);


require_once ('dbconn.php');


$uname = $_POST['uname'];
$pass  = $_POST['pass'];

$userSql   = "SELECT * FROM staff WHERE uname=:uname";
$getUser = $conn->prepare($userSql);
$getUser->execute(array(
    ':uname' => $uname
));


if($getUser->rowCount()){    
    $pwSql = "SELECT pass FROM staff WHERE uname =:uname";
    $getPw = $conn->prepare($pwSql);
    $getPw->execute(array(
        ':uname' => $uname
    ));
    $pw = implode($getPw->fetch(PDO::FETCH_ASSOC));

    if(password_verify($pass,$pw)){
        header("Location: ../../staffHome.php");
    }else{
        echo "Oops! You have entered in an incorrect password!";
    }

}else{
    echo "Oops! That user doesn't exist!";
}
?>
Community
  • 1
  • 1
Jeff
  • 62
  • 11
  • Which of the 3 message do you see after submitting the form? – segFault Jan 04 '16 at 00:12
  • you have many errors here. Once you get this going `if(isset($_POST['login'])){...}` you'll be faced with other brick walls. Consult these following links http://php.net/manual/en/function.mysql-error.php and http://php.net/manual/en/function.error-reporting.php and apply that to your code. Because this will NEVER happen `if(isset($_POST['login'])){...}` since and without JS, forms do not hold the name attribute. But you're probably using JS but not showing us. – Funk Forty Niner Jan 04 '16 at 00:13
  • @sebastianForsberg I am getting the 3rd. The "Username or Password incorrect" – Jeff Jan 04 '16 at 00:13
  • Then you need to read this http://stackoverflow.com/questions/17498216/can-i-mix-mysql-apis-in-php – Funk Forty Niner Jan 04 '16 at 00:14
  • @Fred-ii- I am not using any JS. – Jeff Jan 04 '16 at 00:17
  • @CTrujillo I have posted my answer for you below. I believe I have given you enough in there to get you going. – Funk Forty Niner Jan 04 '16 at 00:25
  • @Fred-ii- Thanks again. I was able to figure out a solution by skimming through everything you supplied me with. I am now going to focus on the security portion and check out the password hashing and XSS that you mentioned as well. I provided an edit and showed what I found to work for me. If you can take a look and see if there is anything you can recommend I would appreciate it. I cannot thank you enough, you have been extremely helpful! – Jeff Jan 04 '16 at 02:10
  • @CTrujillo You're welcome. Looks good to me. Remember to use that password hashing I mentioned. Other than that, you're good to go ;-) just don't go LIVE yet if you're using plain text passwords, because hackers can easily "hack" into your db. *Stay safe!* – Funk Forty Niner Jan 04 '16 at 02:12
  • @Fred-ii- Hey sorry to bug, but I have updated the code once more to include hashed passwords. I used the password_hash() to create the hashed password in case that it wasn't clear from what I supplied. I feel like the code could be more "dry" tho? any suggestions on how to clean it up? Am I still vulnerable to any XSS or brute attacks? After double checking your comments and the resources I believe that I am in the clear? – Jeff Jan 04 '16 at 05:34

1 Answers1

4

Here's the scoop.

Your conditional statement if(isset($_POST['login'])){...} will never happen, since forms do not hold the name attribute (unless you were using JS/jQuery). You need to name your submit that. <input name="submit" id="submit" type="submit" value="submit">, as PHP does not rely on "id", it relies on a "name" attribute (or again, if you were using JS/jQuery, but that isn't the case here). Then remove name="login" from <form>.

Then you're not passing the connection to your escape function.

Then you're mixing mysql_ functions that to not intermix with anything other than that, mysql_.

We also don't know which MySQL API you're using to connect with.

So, use the same one from connecting to querying, Use either mysqli_ or PDO and preferably using a prepared statement.

Edit: comment from the OP: "I am using PDO to connect."

  • You must use the same one from connecting to querying. You can't mix mysql_ or mysqli_ with PDO.

Consult:

Add error reporting to the top of your file(s) which will help find errors.

<?php 
error_reporting(E_ALL);
ini_set('display_errors', 1);

// rest of your code

Sidenote: Displaying errors should only be done in staging, and never production.


Passwords

I also noticed that you may be storing passwords in plain text. This is not recommended.

Use one of the following:

Other links:


Your present code is open to SQL injection. Use mysqli_* with prepared statements, or PDO with prepared statements.

Community
  • 1
  • 1
Funk Forty Niner
  • 74,450
  • 15
  • 68
  • 141
  • I am using PDO to connect. Sorry I should have mentioned that. You have given me a lot to read up on, thank you! I will read through that you have supplied me with and update this once I have something to update. – Jeff Jan 04 '16 at 00:27
  • @RamRaider thanks ;-) – Funk Forty Niner Jan 04 '16 at 00:28
  • @CTrujillo ah there you go and I did write that in my answer that you can't mix MySQL APIs. You must use the same one from connecting to querying. You can't mix `mysql_` or `mysqli_` with PDO. and you're welcome. – Funk Forty Niner Jan 04 '16 at 00:28
  • @CTrujillo You can also consult one of my answers http://stackoverflow.com/a/22253579/1415724 where it does contain a PDO method to check if a row exists. I'm sure you will make use of that also and to get it to work for what you want to do. – Funk Forty Niner Jan 04 '16 at 00:33
  • @Fred-ii- Yea I didn't realize that I couldn't mix them. I am under the impression that PDO is a little more beginner friendly? is this wrong? What would the pros/cons be of using mysqli over pdo and vice versa? – Jeff Jan 04 '16 at 00:34
  • @CTrujillo it's somewhat of a preferential method really. Some things in mysqli with prepared statements that can't be done in PDO and vice-versa. But, the important thing here is to use a prepared statement for either one, and using a safe password hashing function. – Funk Forty Niner Jan 04 '16 at 00:35
  • @CTrujillo another thing to remember if you're going to use `password_hash()` or the compat pack (as per the links in my answer and to which I hope you do if you're going live with this), is that your password column will need to be long enough to accomodate the hash. Minimum length should be 60, but PHP.net recommends 255 (varchar). As per the manual http://php.net/manual/en/function.password-hash.php – Funk Forty Niner Jan 04 '16 at 00:39
  • 1
    Always quality answers sir! – Darren Jan 04 '16 at 00:45
  • @Darren Thanks Darren. *Happy New Year* btw ;-) cheers – Funk Forty Niner Jan 04 '16 at 00:46
  • @CTrujillo I believe I have answered the question and the question should be closed, otherwise others may be tempted to post more answers. One of which was posted other than my own, but that is wrong for too many reasons. If you can mark my answer, great. Otherwise, your question will remain in the unanswered category. – Funk Forty Niner Jan 04 '16 at 00:51
  • You are right, and I selected your answer thanks again! If I need to do something else let me know. I am still learning how SO works as well. – Jeff Jan 04 '16 at 00:53
  • @CTrujillo All is good ;-) *Happy New Year* and coding :-) *Cheers* – Funk Forty Niner Jan 04 '16 at 00:54