0

Thanks to my last question im updating my site to PDO, i figured id start on my front pages and work my way deeper, and ive hit my first hurdle and fallen over, my login script.

login-exec.php EDITED

session_start();

include_once ('connect.php');   

$Email = isset($_POST['Email']) ? $_POST['Email'] : "Email Never Sent";
$Password = isset($_POST['Password']) ? $_POST['Password'] : "Password Never Sent";

$stmt = $db->prepare("SELECT * FROM members WHERE Email = :Email AND Password = :Password");
$stmt->bindParam(":Email"   , $Email    );
$stmt->bindParam(":Password", $Password);
$stmt->execute();
$member = $stmt->fetch(PDO::FETCH_ASSOC);
if ($member)    
    { 
            $_SESSION['SESS_MEMBER_ID'] = $member['Member_ID'];
            $_SESSION['SESS_POST_AS'] = $member['Post_As'];
            $_SESSION['SESS_AUTH'] = $member['auth'];
            session_write_close();
            header('location: index.php');
            exit(); 
    } else  {
        header("location: ?p=login-failed");
        exit();
    }

connect.php

$db = new PDO('mysql:host=localhost;dbname=test;charset=utf8', 'root', '');
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$db->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);

EDIT: Now i get sent to the login-failed page, so does my problem now lie in what this page received from the form?

I know my $password is plain text, i was using md5 before and once i get this working ill implement some better protection

5 Answers5

0

Put your connection attempt into a try/catch and then you will see the error

try {
    $dbh = new PDO('mysql:host=localhost;dbname=test;charset=utf8', 'root', '');
} catch (PDOException $e) {
    echo 'Connection failed: ' . $e->getMessage();
}
RiggsFolly
  • 93,638
  • 21
  • 103
  • 149
  • @Gordon Well no he doesn't actually. He is just saying `correctly` in my opinion that they should be used in conjunction with the normal `php error reporting`. In other words you should have `error-reporting` turned on during development, A condition that it is quite obvious many developers dont do. – RiggsFolly Jul 08 '13 at 12:59
0

Maybe the problem is if ($count > 1) and should be

if ($count > 0)

or

if ($count == 1) 

if sql returns 1 row which I assume is what you want.

mirkobrankovic
  • 2,389
  • 1
  • 21
  • 24
0

in addition to setting up a proper error reporting, also change

$count = $stmt->rowCount();

if ($count > 1) 

to

$member = $stmt->fetch();

if ($member) 

to make the code consistent

Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
-1

Try this:

if ($count > 0) 

or

if ($count >= 1) 
{
    session_start();    
    $_SESSION['SESS_MEMBER_ID'] = $member['Member_ID'];
    $_SESSION['SESS_POST_AS'] = $member['Post_As'];
    $_SESSION['SESS_AUTH'] = $member['auth'];
    session_write_close();
    header('location: index.php');
    exit(); 
}
Nono
  • 6,986
  • 4
  • 39
  • 39
-1

This will work, for you to check if a result was found or not:

$stmt->execute();
$result = $stmt->fetch(PDO::FETCH_ASSOC);
if($result)
{
    //...
}
else
{
    //...
}

Also note that you always have to put

session_start();  

at the top of the page, otherwise it won't work.

$member doesn't really make sense here, where do you init it? $member should act as a row but in this case, $result would do the job for you.

I recommend taking a look at phpass for password protection.

You should also make logic around the POST, you can never be sure that anything is really posted unless you check. The Ternary Operator can be handy here:

$Password = isset($_POST['Password']) ? $_POST['Password'] : "Password Never Sent";

You should not use try catch around the whole page if it's only to catch a failed pdo connection, that should be a part of your connect file.

I'd avoid using "?p=login-failed", since it's easy to manipulate if you don't have any valid validations ongoing.

I personally don't like the bindParam approach, it makes more sense to me to execute an array of parameters onto the statement:

$stmt->execute(array(":Email"=>$Email, ":Password"=>$Password));

But your approach shouldn't be causing an error in this case.

Note that you don't need exit(); after header();

I hope that my comments help.

Jonast92
  • 4,964
  • 1
  • 18
  • 32
  • trying this i get the login-failed page, ive updated my main post – user2558771 Jul 08 '13 at 13:22
  • Alright. I'll see if I can figure something out, it'd be nice if you could up-vote, otherwise I'll be forced to delete my answer because of the previous down-vote. Also: Have you checked the actual values of $Email and $Password? You should use an if else statement to check if the values are posted (with the approach that I showed you and you're using, you just have to check the actual values). – Jonast92 Jul 08 '13 at 13:56
  • i dont have enough rep to upvote but ive accepted your answer as its works now with your answer after i edited my forms – user2558771 Jul 08 '13 at 14:28