Making a simple session class more secured

I just wrote the session class below. I humbly request for a review on how to make this even more secured/ how sessions are normally written. The code below works, but I would love to make it even more secure and robust.

id);
    session_destroy();
    $logged_in = FALSE;
    Misc::redirect('index.php');
}

public static function is_logged_in(){
    if(isset($_SESSION['id'])){
        $logged_in = TRUE;
        Misc::redirect('127.0.0.1/users/index.php');
    }
}

public static function login($email, $password){
    global $user;
    global $misc;

    if($misc->check_form()){
        echo $this->error;
    }
    if (!MIsc::check_email_format($email)) {
            echo "Email format is invalid";
          }
          $login_user = $user->authenticate($email, $password);
          if($login_user){
            $_SESSION['id'] = $this->id = $user->user_data['id'];
            $logged_in = TRUE;
            Misc::redirect('127.0.0.1/users/index.php');
          }
}
}

$session = new Session;
?>

You don't post the code that interacts with database, there is the main security concern. I hope you use PDO prepared statements, which will keep you safe from SQL Injections.

As for non-security-related problems, there are quite some of them.

Consider creating multiple functions for form submit processing, for example:

function process_form_submit($_POST)
{
    $input = $this->validate_user_input($_POST);
    if ($this->user->authenticate($input['email'], $input['password'])) {
        $this->log_user_in($this->user->getId());
    }
}

Then your functions

• do one thing per function

• are testable

• are able to throw exceptions properly

As for exceptions, you don't just echo problems, do things like

throw new FormSubmitException('Email format is invalid');

and call process_form_submit like this:

try {
    $session->process_form_submit($_POST);
} catch (Exception $e) {
    echo $e->getMessage(); // or any other error handling
}

Why? I suppose your code is in the middle of the page (since you just echo all the errors), what if you want to move it to the top in order to change page title if login fails. Then your echo will show the error above page design and you will have to look for every single echo in all of classes used in order to fix this. Code like I provided will let you move it up and replace a single echo with $error = and the task will be done painlessly. Not even mentioning test-ability improvement.

Don't use global $var;, use Dependency injection instead in constructor or factory:

$session = new Session($user, $misc);

If you don't do that than your session class keeps depending on them implicitly and it makes you unable to just copy it into some other project. If you try - you will get errors here and there saying it is missing something that is not clear from it's initialization. Would not it be better to write a good class once in such a way that you can then use it in other projects?

As a last hint, start documenting your code with phpDocumentor standards in mind. It is a good habit and the sooner you start doing it, the easier it will be for yourself to work with code you write today in a few years, when you won't remember a thing about how it works.

StackExchange Code Review Q#51510, answer score: 10