0

I would like to get some clear information or reply how to solve the next issue.

Currently i used MySQL connection, but now i want to move onto MySQLi. I dont want to use PDO, so please do not prefer it.

The new mysqli code is this, but its not working also i think i used a bit too much else, which is not needed.

<?php
include('includes/functions.php');
session_start();   
if(isset($_POST['login'])) {
if(isset($_POST['username'])) {
    if(isset($_POST['password'])) {
        $username = $_POST['username'];
        mysqli_query($query, "SELECT * FROM cm_users WHERE Username = '$username'") or die(mysql_error());
        foreach ($query as $user)
        if(sha3($_POST['password'],256) == $user['Password']) {
            $_SESSION['user'] = $user['Username'];
        if(isset($_POST['g-recaptcha-response'])){
            $captcha=$_POST['g-recaptcha-response'];
            }
        if(!$captcha){
            header("Location: login.php");
            echo "<button class='btn btn-block btn-warning btn-sm'>Please check your login details.</button>";
        exit;
        }
        $response=file_get_contents("https://www.google.com/recaptcha/api/siteverify?secret=******&response=".$captcha."&remoteip=".$_SERVER['REMOTE_ADDR']);
        if($response.success==false)
        {
        echo '<h2>You are spammer ! Get the @$%K out</h2>';
        } else {
        echo '<h2>Thanks for posting comment.</h2>';
        }
            header("Location: redirect.php");
        } else {
            echo "<button class='btn btn-block btn-warning btn-sm'>Please check your login details.</button>";
            include('login.php');
        }
        } else {
            echo "<button class='btn btn-block btn-warning btn-sm'>Please check that you filled out the login form!</button>";
            include('login.php');
        }
}
}
?>

Any idea how to fix the issue to get work?

Marcell
  • 500
  • 1
  • 4
  • 19
  • 1
    you're still mixing APIs using `mysql_error()` and we don't know if your connection is in fact `mysqli_`, and that's unknown. Check for the real errors. We also don't know if your POST arrays contain values or not. – Funk Forty Niner Dec 18 '15 at 15:10
  • What are you errors? What is $query? Please provide some more code – Dacaspex Dec 18 '15 at 15:11
  • then you have a bunch of echos with headers, so you may be outputting before header. error reporting. – Funk Forty Niner Dec 18 '15 at 15:11
  • No error just "please check your login details." Of course i added ini_set('error_reporting', E_ALL|E_STRICT); ini_set('display_errors', 1); the $query is $query = mysqli_connect("127.0.0.1", "root", "xxxx", "cmv1"); – Marcell Dec 18 '15 at 15:11
  • 1
    Edit: ok, so you have it. Original comment: Your system may not be setup to catch/log/display errors/notices. Add error reporting to the top of your file(s) right after your opening PHP tag for example ` – Funk Forty Niner Dec 18 '15 at 15:13
  • 2
    and this is failing you for sure `foreach ($query as $user)` there is nothing assigned to `$query` (for the query), you're checking a `foreach` against your db connection's variable and that alone should have thrown you an error. edit: had you used the right error function. – Funk Forty Niner Dec 18 '15 at 15:15
  • http://cs2d.hu/cm_admin/dologin.php you can see it live, however error reporting already added its not debug anything currently – Marcell Dec 18 '15 at 15:15
  • Look [my comment received an upvote...](http://stackoverflow.com/questions/34358552/login-system-with-mysqli#comment56458824_34358552), there's a reason here ;-) edit: it's up to 2 now. – Funk Forty Niner Dec 18 '15 at 15:16
  • i assigned the db connection to it :( its just included by include('includes/functions.php'); – Marcell Dec 18 '15 at 15:17
  • and there you go ;-) assign a variable to the query other than your db connection. solved. `$other_var = mysqli_query($connection, query)...` and use that variable in your `foreach`. would you like my comments as an answer? ;-) – Funk Forty Niner Dec 18 '15 at 15:17
  • $test = mysqli_query($query, "SELECT * FROM cm_users WHERE Username = '$username'") or die(mysqli_error($query)); foreach ($test as $user) like that?:D – Marcell Dec 18 '15 at 15:19
  • you missed the `$` --- `$test = mysqli_query....` sure. then `foreach($test..)`. – Funk Forty Niner Dec 18 '15 at 15:19
  • yes, edited it now its login, however i still not understand why its takes more than 1 minutes to login :O – Marcell Dec 18 '15 at 15:20
  • That is not due to your code but probably due to your machine – Dacaspex Dec 18 '15 at 15:21
  • that's because of the `foreach` it's going over your entire table of rows/users. that can be cut down to a mere `num_rows()` call and looking if `>0`. No idea why you're using a `foreach` here. – Funk Forty Niner Dec 18 '15 at 15:21
  • well, i dont thin, VPS can simple handle it well. Before it was so fast. Well there is only 2 user in the table, so i dont think it would takes that too much. Maybe SHA3? – Marcell Dec 18 '15 at 15:23
  • 1
    @Fred-ii- he is only going over all the rows where the username is the same as the given username. So he isn't looping through all the users. Or am I missing something here? – Dacaspex Dec 18 '15 at 15:24
  • You are right Fredii :) – Marcell Dec 18 '15 at 15:25
  • @Dacaspex their method is flawed and can be simplified a lot. I wouldn't do a `foreach` for one thing and that alone could be taking a lot resources. A simple check if a row exists, in a while loop with an `if` will suffice, and even then, those are probably not even needed. – Funk Forty Niner Dec 18 '15 at 15:26
  • Should i write a tottally new login system then? – Marcell Dec 18 '15 at 15:27
  • @Marcell I would and using a safer method (you're open to an SQL injection right now). Here, use one of ircmaxell's great answers http://stackoverflow.com/a/29778421/ I suggest it often. Upvote his answer (if you can) and if you use it and worked for you. It's a much simpler method and will work quite fast. – Funk Forty Niner Dec 18 '15 at 15:28
  • Thanks people, really appricate all of your replies :) – Marcell Dec 18 '15 at 15:30
  • You're welcome Marcell. – Funk Forty Niner Dec 18 '15 at 15:30
  • @Marcell so, did the comments help solve your question and what would you like to do with it? If it's left unanswered, then it will be in Stack's unanswered category. If you want to post your own answer, Stack does allow you to do that. The choice is yours if you want and if you wish to delete it. – Funk Forty Niner Dec 18 '15 at 15:36
  • I did, but cannot accept it for 2 days :( – Marcell Dec 18 '15 at 15:40

1 Answers1

1

Making this as a wiki - I have nothing to gain from this, but more for the OP and future visitors to the question.


Pulled from comments and slightly modified:

Firstly, you're still mixing APIs using mysql_error() where it should read as mysqli_error($query) assuming that $query is your connection variable used in your connection codes.

Then this is failing you foreach ($query as $user) because there is nothing assigned to $query (for the query), as you are checking a foreach against your db connection's variable and that alone should have thrown you an error, had you used the right error function.

Being mysqli_error($query) where that function requires a database connection as a parameter.

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

As suggested, use one of ircmaxell's answers and using a better hashing/query function.

Pulled from his answer:

Just use a library. Seriously. They exist for a reason.

Don't do it yourself. If you're creating your own salt, YOU'RE DOING IT WRONG. You should be using a library that handles that for you.

$dbh = new PDO(...);

$username = $_POST["username"];
$email = $_POST["email"];
$password = $_POST["password"];
$hash = password_hash($password, PASSWORD_DEFAULT);

$stmt = $dbh->prepare("insert into users set username=?, email=?, password=?");
$stmt->execute([$username, $email, $hash]);

And on login:

$sql = "SELECT * FROM users WHERE username = ?";
$stmt = $dbh->prepare($sql);
$result = $stmt->execute([$_POST['username']]);
$users = $result->fetchAll();
if (isset($users[0]) {
    if (password_verify($_POST['password'], $users[0]->password) {
        // valid login
    } else {
        // invalid password
    }
} else {
    // invalid username
}
Community
  • 1
  • 1
Funk Forty Niner
  • 74,450
  • 15
  • 68
  • 141