php - PDO Login Script Always Re-Directing To Header Page -

<?php     include "config.php";     class users extends config {         public function login($username, $password) {             try {                 $this->db->begintransaction();                 $stmt = $this->db->prepare("select `username`, `password` `users` `username` = ? , `password` = ? limit 1");                 $stmt->execute(array($username, $password));                 if($stmt->rowcount == 1) {                     while($row = $stmt->fetch(pdo::fetch_assoc)) {                         header("location: index.php?p=loggedin");                         exit();                     }                 } else {                     header("location: index.php?p=false");                     exit();                 }                 $this->db->commit();             } catch(pdoexception $ex) {                 $this->db->rollback();                  echo $ex->getmessage();             }         }     }     $users = new users(); ?> 

as can see above, code has 2 header() methods, 1 re-directs logged in page while other re-directs error page.
believe @ best of knowledge, script should re-direct logged in page per input correct, behaves in unexpected way.
re-directs error page instead of logged in page. please take , tell me reason why doesn't work? thanks! also, if see flaws in code, feel free throw in criticism wish improve self. lot.

i'm going post little more useful posts above. there several best practices can employed here make life easier when coding; after you've wrapped head around them of course.

first , foremost, looks attempt @ authentication part of application. if user passes in correct username , password, want display correct page. have no idea on scope of application i'm not going suggest massive changes.

autoloading

it looks you're manually including files. php has spl_autoload_register which, if you're following psr-0 (and should take @ that) means can map directory structure class heirachy 1:1 , have classes automatically resolved when ask them. when new object, or (in case) extends config, if config class in top level of heirachy automatically found , used. involve learning namespaces.

once you've understood how namespaces work , how autoloading can save development time (and mean significant time in long run), can move onto dependency management tool composer. composer script run generates autoloader you, need @ beginning of application require_once __dir__ . "/vendor/autoload.php". seriously, take @ autoloading tutorials - once use them, never go , use them in every single application ever write in future.

you'll never need type include more once - which, btw, throws warning if file you're trying include isn't found. looks file needs config file, should use require_once instead in case - meaning code fatal if file isn't found (which seems want).

oop

so what's wrong class users extends config. done @ least having naming convention (caps class names). basically, extends implies is-a relationship. don't extend gain functionality - that's code smell. saying here user config. not case, , if hand code other developer in future - "wait, wtf". in nicer way though.

your aim is: to have configuration variables in class. so, pass them class instead. can't tell what's in config class, should take @ dependency injection pattern; here's tutorial. basically, pass config object in via di (via constructor), decouples code can test each object individually on it's own merits, mocking out other objects instead. testable code. dependency injection. google it.

so, don't extend config. pass in configuration instead:

class users {     /**      * @var config      */     protected $config;      /**      * @constructor      *      * @param config $config class configuration, duh      */     public function __construct(config $config)     {         $this->config = $config;     }      /** snip **/ } 

also take @ phpdoc syntax write commented code other developers can understand , parse ease.

transactions

you're using transactions, don't need use select. why not? because transactions allow either commit or rollback - when select, you're not changing data. there's no point in performing transaction. remove those, , try , retrieve data via select.

your header code

each class, each object, should have 1 reason change. 1 task , that's it. if have code reads database, compares strings , saves file, should have 3 classes @ least: 1 reads persistence, 1 performs comparison , 1 saves file. called single responsibility principle can read , google.

with in mind, individual class should not responsible sending headers. return either true or false , have class using 1 instead. not responsibility of class - should dealing authentication.

if picked framework, either in controller or view depending on how close stick mvc pattern - not possible in it's strictest sense web applications (i put there anal people out there) - should separating concerns , @ least having srp adhered throughout codebase.

security

i no means security expert, don't pass both username , password database. if hashing passwords, should be, retrieve user database via username only, then compute whether or not password matches hash within php code. way not sending password database, , helps (albeit little) against sql injection. php 5.5 contains useful password_* functions in regard, you're not using php 5.5, check out password_compat library.

what sql injection? there's more info bobby tables, , useful image below.

conclusion

these few pointers head in right direction toward better code can pass other developers understand , can proud of it. take @ framework symfony don't have worry authentication, autoloading , things - it's done you. composer must. need start debugging code line-by-line, var_dump() var_dump() until see isn't expect. debugging 101, it'll , more.