-1

I am new to PHP, so please be gentle...

I have created a PHP login page in Dreamweaver, and have set the failed login to return to the same page, but would like to display a message to say "username or password are incorrect".

The data is stored in a MySQL database Here is the code that has been generated by Dreamweaver:

if (!isset($_SESSION)) {
  session_start();
}

$loginFormAction = $_SERVER['PHP_SELF'];
if (isset($_GET['accesscheck'])) {
  $_SESSION['PrevUrl'] = $_GET['accesscheck'];
}

if (isset($_POST['email'])) {
  $loginUsername=$_POST['email'];
  $password=$_POST['password'];
  $MM_fldUserAuthorization = "";
  $MM_redirectLoginSuccess = "bac123_update_details.php";
  $MM_redirectLoginFailed = "bac123.php";
  $MM_redirecttoReferrer = true;
  mysql_select_db($database_connRespond, $connRespond);

  $LoginRS__query=sprintf("SELECT email, password FROM customer WHERE email=%s AND password=%s",
    GetSQLValueString($loginUsername, "text"), GetSQLValueString($password, "text")); 

  $LoginRS = mysql_query($LoginRS__query, $connRespond) or die(mysql_error());
  $loginFoundUser = mysql_num_rows($LoginRS);
  if ($loginFoundUser) {
     $loginStrGroup = "";
    if (PHP_VERSION >= 5.1) {session_regenerate_id(true);} else {session_regenerate_id();}
    //declare two session variables and assign them
    $_SESSION['MM_Username'] = $loginUsername;
    $_SESSION['MM_UserGroup'] = $loginStrGroup;       

    if (isset($_SESSION['PrevUrl']) && true) {
      $MM_redirectLoginSuccess = $_SESSION['PrevUrl'];  
    }
    header("Location: " . $MM_redirectLoginSuccess );
  }
  else {
    header("Location: ". $MM_redirectLoginFailed );
  }
}
  • try to use session? – j.Doe Jul 01 '16 at 13:55
  • please dont use mysql as its have been removed in the latest php version. use pdo or mysqli instead. it helps you prevent sql injection too. you can read more here [sql injection](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) and please hash your password. – j.Doe Jul 01 '16 at 13:59

2 Answers2

1

in case of a failed login you are redirecting this way:

else {
  header("Location: ". $MM_redirectLoginFailed );
}

which variable is declared here:

$MM_redirectLoginFailed = "bac123.php";

so you should check the file "back123.php" and change it according to your needs.

I also suggest to exit after any header call:

header("Location: ". $MM_redirectLoginFailed );
exit();
Massimo
  • 21
  • 3
  • Good call to use the `exit`, after the header. not enough people do this. – Martin Jul 01 '16 at 14:14
  • @Martin As you're already talking about as it's a good thing, could you explain, why? (As I don't exit them [also used this only twice]) – pguetschow Jul 01 '16 at 14:17
  • @TechTreeDev It is because the PHP does not stop running the page script once it reaches a header, so if you have a header that is a location redirect and the same page is still executing code, which further down (where the coder has assumed the page will not reach if following a redirection header) can change data values or can even run into another header location redirect, which will of course overwrite the original (intended) location redirect. So the `header` / `exit` method ensures that this overspill does not happen. – Martin Jul 01 '16 at 14:20
1

There are several ways you can do this.

1) You can use a URL variable such as

login.php?message=1

And the login.php script can check if($_GET['message'] == 1){...} and then output a message corresponding to the code number you give it, such as 1 = failed attempt, 2 = logged out ok, etc.

2) You can use a $_SESSION veriable if you are experienced with sessions, this is easy, if not, please take some time to learn and understand sessions, they're extremely useful in PHP.

On your code page you can set a message such as:

$_SESSION['msg'] = "Your username or password are incorrect";
header("Location: login.php");
exit;

And similar to point 1, you can then set an IF statement such as :

if(!empty($_SESSION['msg'])){
   /***
output the session message to the browser. 
   ***/
   print $_SESSION['msg'];
   unset($_SESSION['msg']); //clear the message once it's been output.
}

Some important notes:

  • Passwords should be stored in the database as HASHED values using something like password_hash. This is extremely important.
  • MySQL is DEPRECATED. Use MySQLi or PDO instead. This is extremely important too.
  • Also please start to use Prepared Statements for your SQL queries.
  • After each header("location:...); call you really, really should stop PHP code execution with an exit; or die(); statement (As exampled above).

Edit:

Reasons why exit should be employed after a header("Location: ..."); redirect.

It is because the PHP does not stop running the page script simply because it reaches a header command, even if it does identify the command as a Redirect (Location:), the script will not cease and so if you have a header that is a location redirect and you assume that the page will stop at this point and redirect itself, this is wrong.

The page will still execute code, which further down can change data values / Database values or can even run into additional headers with additional location redirects, which will overwrite the original (intended) location redirect. Redirects will also be cancelled if there is any output (such as if there is HTML output below the header.

Example:

//processing a user login...
if(failed == true){
    header("Location:loginagain.php");
}
header("Location:welcome.php");

The above code examples the issue; that no matter even if the login fails and the IF statement returns TRUE, the user will still be able to reach the welcome.php page.

So the header / exit coupling ensures that this type of overspill does not happen.

Martin
  • 22,212
  • 11
  • 70
  • 132
  • Thanks Martin for a very comprehensive reply - it sounds like I need to do some reading into sessions to get this working. Thanks too for all of the additional recommendations... – Simon Metson Jul 01 '16 at 14:36
  • The [PHP Manual](http://php.net/manual/en/index.php) is your friend :-) – Martin Jul 01 '16 at 14:37
  • Thanks again for your help. I have followed your advice on all of the suggestions above. I am having a little trouble with the password hashing though. I have tried some different methods that I have found, but get an 'undefined variable' error when I do it. I have included the following variables: $encryptedpass = 'password2'; $sha1pass = sha1($encryptedpass); Then I have changed the post script to: GetSQLValueString($_POST[$sha1pass], "text")); – Simon Metson Jul 05 '16 at 15:49
  • @SimonMetson You should be using [`password_hash`](http://php.net/manual/en/function.password-hash.php) for password_hashing. It will allow you to choose your hashing algorithm and there are many Q&A about it on SO. – Martin Jul 05 '16 at 15:57