0

My PHP knowledge level: Novice.

Learning resources: Codecademy, TheNewBoston, Wikipedia, PHP Documentation.

What I am trying to achieve: I am trying to learn how to work with object oriented PHP since I have read that is more efficient in the long term than procedural methods. I am trying to redirect users to the login.php page if the entered name and password match with the database and relogin.php upon fail.

The problem: I can't figure out why my conditional is not working as expected. It does work, otherwise I would have had an error message.

What I tried: Worked on the below code on scratch. I did a google search on the problem, tried to fix it by checking if any semicolons or brackets are missing, and even checked if my database connection could be established. It seems there are no errors so it means the conditional is only showing true.

Current error messages: No error messages on screen or in the source code which means the code is right but not working as expected.

If you want me to comment the code please let me know.

My PHP code:

if(isset($_POST['name'])) {
    $name= $_POST['name'];
    $password = $_POST['password'];
    $login = new login($name, $password);
}

class Login {

    public function __construct ($name, $password) {

        if ($this->check($name, $password)){
            header("location:login.php");
        }
        else {
            header("location:relogin.php");
        }
    }   

    public function check ($name, $password) {
        $request = "SELECT `id` FROM `members` WHERE `name` LIKE '$name' AND ´password´ LIKE '$password'";
        $result = mysqli_query($connection, $request);
        return mysqli_num_rows($result) > 0;
    }
} 

        public function check ($name, $password) {
            $request = "SELECT `id` FROM `members` WHERE `name` LIKE '$name' AND ´password´ LIKE '$password'";
            $result = mysqli_query($connection, $request);
            return $result;
        }
    }

**HTML code**

    <form action="Login.php" method="post">
         <input type="text" placeholder="Name" name="name">
         <input type="password" placeholder="Password" name="password">
         <input type="submit" name="button" value="Login">     
    </form>
Asperger
  • 3,064
  • 8
  • 52
  • 100
  • Define "doesn't work". What do you expect this to do, and what does it do instead? Put in some `var_dump()` and `echo` statements to debug where the code does go and where it unexpectedly doesn't. – deceze Aug 18 '15 at 07:55
  • Also, indeed, this won't do anything by itself as is. You're *declaring* a class, you're never *instantiating or executing* any of its code... – deceze Aug 18 '15 at 07:56
  • @deceze it returns true everytime, even if login is wrong – Asperger Aug 18 '15 at 07:56
  • Two major issues here: 1. your code is wide open to sql injections, you should read about the advantages of using "prepared statements" and "parameter binding". 2. you should _never_ store passwords in a database! No! You should store only a salted hash of a password and compare hashes at runtime. – arkascha Aug 18 '15 at 07:57
  • You also need to call `$this->check`, not `check`, and you certainly need to read this: http://stackoverflow.com/q/16959576/476 – deceze Aug 18 '15 at 07:58
  • You need to call $this->check() – Jan Aug 18 '15 at 07:58
  • @tibzon indeed, only on here. Was a bad typo : ). Doesnt affect the conditional though – Asperger Aug 18 '15 at 07:59
  • Query always return some value whether login details are correct or wrong and because of that if condition is getting true everytime. You have to check id which you returning from the query, check whether id id greter than zero or not – Tushar Aug 18 '15 at 07:59
  • @Tushar you are right, how come I didnt think of that? – Asperger Aug 18 '15 at 08:02
  • 2
    Make sure you didnt simplify it to the point of mutilation – Mihai Aug 18 '15 at 08:04

2 Answers2

2

You are storing password twice:

$password = $_POST['name'];
$password = $_POST['password'];

I guess the first one should be $name.

Also to make the class work you need to make an object from it:

if(isset($_POST['password'])) {
    $name= $_POST['name'];
    $password = $_POST['password'];
    $login = new login($name, $password);
}

You will also need to fix the code inside your constructor:

When you call a function inside the same class you need to reference it by using $this->:

check($name, $password)

should become:

$this->check($name, $password)

Also:

your check function isn't returning true or false. You can return something like:

return mysqli_num_rows($result) > 0;
nowhere
  • 1,558
  • 1
  • 12
  • 31
  • Apparently this is a simplified example the connection is there – Mihai Aug 18 '15 at 08:03
  • Yes i've fixed the answer ;) But he's still not returning true or false. – nowhere Aug 18 '15 at 08:05
  • I see that I need to made a new object first. Why does it work without making an object then? For example the conditional inside gives me true even if there is no object – Asperger Aug 18 '15 at 08:06
  • Which makes me think that the class fires even without the object? Maybe the user triggers it as soon as the form button is clicked – Asperger Aug 18 '15 at 08:07
  • There is no relation between the user's click and the execution of a method inside a class (unless you coded something to achieve this, which is not mentioned inside your example). The code inside __construct is executed when you generate an object from the Login class. – nowhere Aug 18 '15 at 08:10
  • This is the only code I created. I do have a connect class which creates a connection to the database but thats all. Weird. – Asperger Aug 18 '15 at 08:12
  • It's really strange indeed. But if you want to start with oop you should totally give a look at constructors and how they work: http://php.net/manual/en/language.oop5.decon.php – nowhere Aug 18 '15 at 08:13
  • I watched the Newboston and phpacademy tutorials on OOP but it seems im just lacking experience on how to use the tools right now – Asperger Aug 18 '15 at 08:16
  • @CosLu now I get redirected directly to relogin.php. It returns false and redirects me even without triggering from the form. – Asperger Aug 18 '15 at 08:21
  • sorry, move the $login = new login($name, $password); part inside your if. I'm fixing my answer now. – nowhere Aug 18 '15 at 08:23
  • Thanks alot for your help. I really know the basics including OOP but I never had experience working with it yet until now. – Asperger Aug 18 '15 at 08:25
  • you're welcome. To return true or false from the check method try this: return mysqli_num_rows($result) > 0; – nowhere Aug 18 '15 at 08:27
  • @CosLu, I will test now and let you know. – Asperger Aug 18 '15 at 10:20
  • @CosLu, updated my code here. It seems only true is fired, or better said the if but never the else when I log in with wrong data. mysqli_num_rows should have done the trick since we checked if its more than 0. If ID doesnt exist then it should have fired the else – Asperger Aug 18 '15 at 10:41
  • can you try "echo mysqli_num_rows($result);" to see what it is returning? is it always 1? – nowhere Aug 18 '15 at 10:43
  • Maybe it depends on what you have in your db but using "LIKE" in your query could give you more results than you would expect. Try to replace it with "=": `name` = '$name' (same thing for password). – nowhere Aug 18 '15 at 10:46
  • It returns 1 for some reason – Asperger Aug 18 '15 at 10:54
  • So the problem is with your query and the data you are submitting.. have you tried to replace "like" as i mentioned? – nowhere Aug 18 '15 at 10:59
  • @CosLu yep, same result unfortunately – Asperger Aug 18 '15 at 11:17
  • I can hardly know why is your query always returning something.. did you try to check which values does $result contain? If you check that you should easily be able to understand why your query is behaving like that. – nowhere Aug 18 '15 at 14:14
0

Try this,

class Login {

 public function __construct ($name, $password) {

    if ($this->check($name, $password) > 0) {
        header("location:login.php");
    }
    else {
        header("location:relogin.php");
    }
}   

public function check ($name, $password) {

    $request = "SELECT `id` FROM `members` WHERE `name` LIKE '$name' AND ´password´ LIKE '$password'";
    $result = mysqli_query($connection, $request);
    return mysqli_num_rows($result);
}

}

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

$name= $_POST['name'];
$password = $_POST['password'];
$login = new Login($name, $password);

}