0

Is there a better way to do this?


Can I start a session inside a class? and check if isset in the class??

class users {
    function login($username, $password) {
        $stmt = $db->prepare("SELECT * FROM users WHERE username=? AND password =?");
        $result = $stmt->execute(array($username, $hashedPassword));
        // count the returned rows, if you have 1 then good.
        if($result->num_rows > 0) {
            $row = $result->fetch_assoc();
            // get the data you need and store it in the session.
            $_SESSION['username'] = $row['username'];
            return TRUE;
        } else { return FALSE; }
    }
    function getForm() {
        return "<form method='post'>username <input type=text name=username>\n";
        return "password <input type=password name=password></form>;
    }
} 

My objective is to do less codes here:

$user = new users();
echo $user->getLoginForm()
if(isset($_POST['username'])) {
    if($users->login($_POST['username'], crypt($_POST['password'], $salt) == TRUE) {
        //logged in
        session_start();
        //other stuff
    } else { echo "error"; }
}

I'm very new to Object-Oriented programming Thank you all in advance!

anon ymous
  • 39
  • 2
  • 10

2 Answers2

2

Much better approach is to hash password already on the client side, so that through network does not travel password unencoded..

Also if you could add a special token to be encoded with the passowrd, it should increase the security.. (The hash does not travel through network always the same)

Also adding the token do the db should increase the security, so that hashes of same passwords are not the same in the db..

Also note that md5 has been already hacked, so you should use at least sha_256..

The answer to your question is, yes you can start session within the function of the class.. But if you do not init it when the script starts, you do not have access to your session variables..

But what you should think about more is to use some standard libraries for authentication so that it is secure..

0

Some things we need to sort out in your current code before we look at whats next.

Note: this is just my opinion. There are many many different ways of doing what you are trying to do. This is only one way which may or may not fit into the rest of your project. Also, I've not tested any of this, so there maybe bugs as Im rusty on flat mysql stuff as Ive been in frameworks for too long.

  1. MD5 is a bad way of encrypting. Read this to give you some guidelines.
  2. This is just my opinion, but I would search for a valid user who has the username AND hashed password from the database. At the moment, if they are the right user, you'd have to query again to get the user data. Where as if you do as suggested, you'd already have everything.
  3. You can start a session from anywhere, but I would start it at the top of each of your 'main' files to make sure its available everywhere.

Current

class users {
    function login($username, $password) {
        $stmt = $db->prepare("SELECT password FROM users WHERE username=?");
        $stmt->execute(array($username));
        $hash = $stmt->fetch();
        if(md5($password) == $hash) {
            return TRUE;
        } else { return FALSE; }
    }
    function getForm() {
        return "<form method='post'>username <input type=text name=username>\n";
        return "password <input type=password name=password></form>;
    }
} 

Suggested

<?php session_start();  // start your session from your 'main' file. ?>

class users {
    // make sure you specify scope on your methods
    public function login($username, $password) {
        $hashedPassword = crypt($password, $someSalt); // TODO: do whatever you choose here.

        $stmt = $db->prepare("SELECT * FROM users WHERE username=? AND password =?");

        $result = $stmt->execute(array($username, $hashedPassword));

        // count the returned rows, if you have 1 then good.
        if($result->num_rows > 0) {
            $row = $result->fetch_assoc();
            // get the data you need and store it in the session.
            $_SESSION['username'] = $row['username'];

            return TRUE;
        } else { return FALSE; }
    }

    // you can only return once from a method. So return the whole string.
    public function getForm() {
        return "<form method='post'>username <input type='text' name='username'>\n password <input type='password' name='password'></form>;
    }
} 

Possibly helpful resources

  1. http://php.net/manual/en/function.password-hash.php
  2. http://php.net/manual/en/mysqli-result.fetch-assoc.php
  3. http://code.tutsplus.com/tutorials/securely-handling-users-login-credentials--cms-20369
Community
  • 1
  • 1
DevDonkey
  • 4,835
  • 2
  • 27
  • 41
  • okay thanks, I'll fix it. But my question wasn't about the security of the script – anon ymous Jan 21 '16 at 20:19
  • youll need to sort out how you save your passwords into your database so they match whatever method you choose for decryption. And Ive not tested this code, but hopefully itll work ok. – DevDonkey Jan 22 '16 at 10:06