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

Jonathan Hall
  • 75,165
  • 16
  • 143
  • 189
Marilee
  • 1,598
  • 4
  • 22
  • 52

1 Answers1

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.

Barmar
  • 741,623
  • 53
  • 500
  • 612
  • 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! – Marilee Jan 12 '18 at 09:08