3

Use Case:

Some areas of a website is restricted for member only, if user is logged in display page else redirect to login / registration form.

My Logic

I created a method inside the Users() class which simply check if a $_SESSION['userID'] is set.

The logic is if the $_SESSION[] is not set, then the user is not logged in since the $_SESSION['userID'] variable gets set on login. I came up with the following...

Method

public static function isLogged(){
    if(!isset($_SESSION['userID'])){
        header(':Location: login.php');
        die();
    }
}

Considering the above, I simply add the isLogged() method to the start of the page, and if the $_SESSION('userID') super global is not set the user gets redirected.

My Problems / Questions

The problem with the above is I cant out my mind to rest since:

  1. The method seems inefficient...i.e there is a better (more efficient way of doing this)?
  2. Im unsure whether there are any work arounds that unauthorized users can perform to view the page
  3. I'm perhaps missing some critical info inside the isLogged() method?

Additional INFO

Login Method

public function login_user($newEmail, $newPword)
{
    global $db;
    $this->email = $newEmail;
    $this->password = $newPword;
    $sql = "SELECT * FROM bru_users WHERE email = :email AND password = :pword";
    $stmnt = $db->prepare($sql);
    $stmnt->bindValue(':email', $newEmail);
    $stmnt->bindValue(':pword', $newPword);
    $stmnt->execute();
    $is_registered = $stmnt->rowCount();
    if ($is_registered > 0) {
        $users = $stmnt->fetchAll();
        foreach ($users as $user) {
            $userInfo[] = array('userID' => $user['userID'], 'email' => $user['email'], 'userName' =>$user['userName'], 'firstname' => $user['firstname'],
                'lastname' => $user['lastname'], 'reg_date' => $user['registration_date']);
        }
        if(isset($userInfo)){
            foreach ($userInfo as $start) {
               $_SESSION['userID'] = $start['userID'];
               $_SESSION['email'] = $start['email'];
               $_SESSION['userName'] =$start['userName'];
               $_SESSION['firstname'] = $start['firstname'];
               $_SESSION['lastname'] = $start['lastname'];
            }
        }
    } else {
        return false;
    }
    return true;
}

Would greatly appreciate it if a more experienced coder can have a quick scan through this. Many Thanks

7
  • You shouldn't have : before Location in the header() call. Commented Jan 12, 2018 at 9:00
  • Why does that method seem inefficient? That's about as minimal as it gets. Commented Jan 12, 2018 at 9:02
  • @Barmar appologies typo Commented Jan 12, 2018 at 9:04
  • header(':Location: login.php'); NEED TO BE header('Location: login.php'); Commented Jan 12, 2018 at 9:04
  • Not sure if you need select * from and why are you doing foreach ($users as $user) - is it possible to return more than one user with a matching email and password combo? Commented Jan 12, 2018 at 9:04

1 Answer 1

4
  1. The method seems inefficient...i.e there is a better (more efficient way of doing this)?

No, session variables are reasonably efficient. This is the normal way of doing it.

  1. Im unsure whether there are any work arounds that unauthorized users can perform to view the page

Only if they can hijack the session of an authorized user. But then pretty much all bets are off as far as user authentication. Hijacking a session would require them to get the other user's cookies. If you encrypt your connections with SSL, you're as safe as you can get.

  1. I'm perhaps missing some critical info inside the isLogged() method?

Nope, that's all there is to it.

Sign up to request clarification or add additional context in comments.

1 Comment

Thank you so much for the encouragement @Barmar. Yeah I was worried about $_SESSION hijacking but guess that's a whole new topic then...Many thanks Pal!

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.