0

I want that once the person enters their information for log in it brings them to home.php. my current code is this:

if (isset($_POST["user_login"]) && isset($_POST["password_login"])) {
    $user_login = preg_replace('#[^A-Za-z0-9]#i', '', $_POST["user_login"]);
    $password_login = preg_replace('#[^A-Za-z0-9]#i', '', $_POST["password_login"]);
    $md5password_login = md5($password_login);
    $sql = mysql_query("SELECT id FROM users WHERE username='$user_login' AND password='$md5password_login_md5' AND closed='no' LIMIT 1");
    $userCount = mysql_num_rows($sql);
    if ($userCount == 1) {
        while($row = mysql_fetch_array($sql)){ 
             $id = $row["id"];
        }
        $_SESSION["id"] = $id;
        $_SESSION["user_login"] = $user_login;
        $_SESSION["password_login"] = $password_login;
        header("location: home.php");
        exit();
    } else {
        echo 'That Log In information is incorrect, please try again';
        exit();
   }
}
?>
Scott Arciszewski
  • 33,610
  • 16
  • 89
  • 206
Kasimir Schulz
  • 1
  • Did u write session_start() at the start of the php code?? – h0lyalg0rithm Aug 03 '15 at 18:59
  • That's what I was going to ask. But OP's always leave out important details. What's actually the problem here? What isn't working? Does it not redirect? – Evadecaptcha Aug 03 '15 at 19:00
  • 1
    If you can, you should [stop using `mysql_*` functions](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php). They are no longer maintained and are [officially deprecated](https://wiki.php.net/rfc/mysql_deprecation). Learn about [prepared](http://en.wikipedia.org/wiki/Prepared_statement) [statements](http://php.net/manual/en/pdo.prepared-statements.php) instead, and consider using PDO, [it's really not hard](http://jayblanchard.net/demystifying_php_pdo.html). – Jay Blanchard Aug 03 '15 at 19:04
  • 1
    You really shouldn't use MD5 on password hashes and you really should use PHP's [built-in functions](http://jayblanchard.net/proper_password_hashing_with_PHP.html) to handle password security. – Jay Blanchard Aug 03 '15 at 19:04
  • 1
    [Your script is at risk for SQL Injection Attacks.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) – Jay Blanchard Aug 03 '15 at 19:04
  • 1
    Why are you using a `while` loop when the query can only return one row? – Barmar Aug 03 '15 at 19:07
  • Enable error reporting, and check whether you're getting the warning "Headers already sent". If you are, see [how to fix headers already sent error in php](http://stackoverflow.com/questions/8028957/how-to-fix-headers-already-sent-error-in-php) – Barmar Aug 03 '15 at 19:09
  • very interesting part of code is that, setting session id, login and password without verifying that they exist in database or not and there you problem is that your are setting `$_SESSION["id"] = $id;` outside the loop – Shehary Aug 03 '15 at 19:13
  • You used "$md5password_login_md5'" in your sql statement but your variable name is "$md5password_login" – hellcode Aug 03 '15 at 19:51

1 Answers1

1

I gather this is (hopefully) a testing script, or a way to learn. That's cool, but there's multiple things that I'd suggest you look into:

  1. You're using mysql_* functions. These are now deprecated. It's suggested to use mysqli or PDO
  2. You're using md5 for password storage. Please use password_hash and password_verify instead
  3. Using regexes to validate data isn't a bad idea, and credit for using them. However, you might want to give the regex a quantifier so it matches more than just one character. Just put a + after your [^A-Za-z0-9] blocks
  4. The main reason your header might not be working is that it is case sensitive, and should be a full URL. See the notes on the PHP manual page for header()

Have a look at PHP The Right Way. You're starting out well, but without guidance you're going down paths that'll be problematic later.

NeoThermic
  • 308
  • 2
  • 7