HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Is this a safe Login?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisloginsafe

Problem

I've been doing a lot of searching on PHP, logins, forms, cookies, sessions, etc. And so, I've tried to gather all the info that I got from all over the place. But, I didn't find a place with all the info, and tried to do the best secure login I could do.

This is what I have. If this ends up being good, others can use this. Otherwise, just ignore it. But I'd like to have opinions.

First, every page makes sure you use HTTPS:

if($_SERVER["HTTPS"] != "on")
{
    header("Location: https://" . $_SERVER["HTTP_HOST"] . $_SERVER["REQUEST_URI"]);
    exit();
}


Send username and pass using POST over the https should be safe enough?

Using XMLHttpRequest to check password on Check.php:

function validate() {

var un = document.form1.myusername.value;
var pw = document.form1.mypassword.value;

  httpObject = getHTTPObject();

  if (httpObject != null)
  {
    var params = "username="+un+"&password="+pw;

    httpObject.open("POST","Check.php", true);
    httpObject.setRequestHeader("Content-type","application/x-www-form-urlencoded");
    httpObject.setRequestHeader("Content-length",params.length);
    httpObject.setRequestHeader("Connection","close");
    httpObject.onreadystatechange = setValid;
    httpObject.send(params);
  }

}


On Check.php, upon confirming password (which was saved on server encrypted), I set up cookie and session creating unique IDs and session secrets:

```
if($u==$username && $p==$password)
{
$valid=1;

//Setting up session and cookie
//if cookie stored secret does not meet session secret, session has been hijacked, the same for id
//session is stored on server while cookie is stored on client

session_start();
$sessionTime = time();
$unique_id = uniqid(sha1($_SERVER['HTTP_USER_AGENT'] . $username), true); //create a unique ID
$sessionSecret = sha1($sessionTime . $unique_id); // Create a unique secret based on time and ID

$_SESSION[$u

Solution

Some comments:

if($_SERVER["HTTPS"] != "on")


See: https://stackoverflow.com/a/2886224/59087

Avoid calling exit() from more than one location.

The following code is repeated many times:

//this guy did not pass by login!
header("location:Login.php");
exit();


You could write some functions to avoid the duplication:

function redirectLogin() {
  redirect( "Login.php" );
}

function redirect( $page ) {
  header( "Location: $page" );
  exit();
}


This removes the duplication and ensures that you have only one place in the code to change if you wanted to rename Login.php, for example, to login.php, which is a bit more user-friendly (on Unix systems).

Code Snippets

if($_SERVER["HTTPS"] != "on")
//this guy did not pass by login!
header("location:Login.php");
exit();
function redirectLogin() {
  redirect( "Login.php" );
}

function redirect( $page ) {
  header( "Location: $page" );
  exit();
}

Context

StackExchange Code Review Q#30962, answer score: 3

Revisions (0)

No revisions yet.