Cześć wszystkim

Napisałem logowanie do serwisu i wszystko działa poprawnie lecz proszę o wasze sugestię na temat bezpieczeństwa skryptu. Proszę o wyrozumiałość Pozdrawiam!

Daję kod:

[PHP] pobierz, plaintext 
<?php
session_start();
?>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
 
	<form method="post" action="login.php">
		 <input type="text" name="email"/>
   		 <input type="password" name="password"/>
   		 <input type="submit" name="submit"/>
	</form>
<?php
 try
   {
	  //conect
      $pdo = new PDO('mysql:host=localhost;dbname=user', 'user', '***');
      $pdo -> setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
 
  if(isset($_POST['submit'])) {
 
	if(filter_var($_POST['email'], FILTER_VALIDATE_EMAIL))
	{
 
    $password = sha1($_POST['password'].'sad8%$9sdk');
 
	$stmt = $pdo -> prepare('SELECT `email`, `password`,`id` FROM `users` WHERE `email` = :email AND `password` = :password'); 
 
                        $stmt -> bindValue(':email', $_POST['email'], PDO::PARAM_STR);
			$stmt -> bindValue(':password', $password, PDO::PARAM_STR);
                        $stmt -> execute(); 
 
						$count = $stmt ->rowCount();
 
						if($count > 0) {
 
							 while($row = $stmt -> fetch())
      						{
								 session_regenerate_id (true);
 
								 $_SESSION['status'] = true;
          						         $_SESSION['id'] = $row['id'];
								 $_SESSION['HTTP_USER_AGENT'] = sha1($_SERVER['HTTP_USER_AGENT'].'dsd#$%^');
 
								 header('Location: home.php');
      						}
 
						} else {
 
						echo "Logowanie zakończone niepowodzeniem!";
 
						}
 
	} else {
 
	echo 'Adres e-mail niepoprawny';
 
	}
 
 }
  }
   catch(PDOException $e)
   {
      echo 'Error: ' . $e->getMessage();
   }
?>
[PHP] pobierz, plaintext 

Sprawdzaj jeszcze długość podanych danych i będzie mniej więcej ok.

A nie lepiej zrobić połączenie dopiero po if(isset) ?

jeśli chodzi o optymalizację to tak - ale na początek nie ma co przesadzać ;]

Tylko co tam na początku robi ten HTML? Przecież poniżej wysyłasz nagłówki.

Chcę umieścić sprawdzenie poprawności hasła

- jedną małą literę
- jedną dużą literę
- jedną cyfrę
- jeden znak specjalny
- min 8 znaków
- max 30 znaków

Podaję kod:

[PHP] pobierz, plaintext 
<?php
 
$pass = "Password!1";
 
if (preg_match("/^(?=.*[A-Z])(?=.*[a-z])(?=.*[0-9])(?=.*[!@#$%^&*()_+|-]).{8,30}$/",$pass)) {
 
	//true
 
	} 
 
?>
[PHP] pobierz, plaintext 

Co o tym myślicie?

IMO Wygląda nieźle.

jako, że rozumiem wyrażenia regularne, to mogę ze spokojnym sumieniem napisać, iż znaków można wprowadzić grubo więcej niż 30 a i poprawne hasło z założenia będzie błędne - przykład:
!0aAAAAA
z kolei błędne hasło które na bank przejdzie:
...............A...............a...............0...............!...............
na wyrażeniu całości tak nie zrobisz , i choć całe wyrażenie nie jest najlepsze to koniec choćby nawet znaczy .{8,30} - minimum 8 max 30 dowolnych znaków, z kolei ".*" znaczy to samo co 0 i więcej dowolnego znaku...

ps. nie wierzę, że hasło w podanym kodzie pasuje do wzorca ;D - gdyż nie może
______________________

edit
mój błąd

Napisałeś że haslo ...............A...............a...............0...............!............... przejdzie a to nie prawda pokazuje false

A jednak pasuje. Wystarczy wrzucić regexpa do pliku php i odpalić go w przeglądarce, co serdecznie polecam.
Pasuje także zmiana kolejności liter, cyfr i znaków specjalnych (czego na pierwszy rzut oka nie widziałem).

rzeczywiście mój błąd - aż mi wstyd - nie trzeba wrzucać do pliku, można z linii poleceń sprawdzić...

chyba przegapiłem znak "=" i potraktowałem jako ":", czyli zamiast (?=) myślałem w kategorii (?:)... jest jeszcze jedno przydatne grupowanie (?!) - czyli ma się nie dopasować do tego co w środku...

Wkleję za niedługo poprawiony skrypt

[PHP] pobierz, plaintext 
<?PHP
session_start();
if(isset($_POST['submit'])) {
 
	if (preg_match("/^(?=.*[A-Z])(?=.*[a-z])(?=.*[0-9])(?=.*[!@#$%^&*()_+|-]).{8,30}$/",$_POST['password'])) {
 
		if(preg_match('/^[a-zA-Z\.\-\_]+\@[a-zA-Z0-9\.\-\_]+\.[a-z]{2,4}$/D', $_POST['email'])) {
 
		try
  		 {
	  	  $pdo = new PDO('mysql:host=localhost;dbname=user', 'user', '***');
     	          $pdo -> setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);	
 
		  $password = sha1($_POST['password'].'sad8%$9sdk');
 
		        $stmt = $pdo -> prepare('SELECT `email`, `password`,`id` FROM `users` WHERE `email` = :email AND `password` = :password'); 
 
                        $stmt -> bindValue(':email', $_POST['email'], PDO::PARAM_STR);
			$stmt -> bindValue(':password', $password, PDO::PARAM_STR);
                        $stmt -> execute(); 
 
						$count = $stmt ->rowCount();
 
						if($count > 0) {
 
							while($row = $stmt -> fetch())
      						{
								 session_regenerate_id (true);
 
							        $_SESSION['status'] = true;
          						        $_SESSION['id'] = $row['id'];
							        $_SESSION['HTTP_USER_AGENT'] = sha1($_SERVER['HTTP_USER_AGENT'].'dsd#$%^');
 
							      // last active the user ip 
								$stmt = $pdo -> prepare('UPDATE `users` SET `ip` = :ip WHERE `id` = :id');     
 
                        	                        	$stmt -> bindValue(':ip', $_SERVER['REMOTE_ADDR'], PDO::PARAM_STR);
                        		                        $stmt -> bindValue(':id', $row['id'], PDO::PARAM_STR);
 
                       		                                $stmt -> execute(); 
							     // 
 
								 header('Location: index.php');
      						}
 
						}
		 }
 
	 	 catch(PDOException $e)
  		 {
         echo 'Error: ' . $e->getMessage();
  		 }
			} else { echo "Email error"; }
				} else { echo "Hasło error"; } 
					} else { echo "Formularz error"; }
 
?>
 
[PHP] pobierz, plaintext 

1. Po co w procesie logowania sprawdzasz poprawność formatu emaila / hasła?
2. Po nagłówku Location powinieneś przerwać działanie skryptu i wyświetlić stronę z informacją o wykonaniu przekierowania do innego URL.
3. Zamiast paskudnych, zagnieżdżonych IF-ELSE-ów, możesz napisać:

[PHP] pobierz, plaintext 
if (!...) {
 
}
 
if (!...) {
 
}
 
// tutaj kod, gdy wszystko w porządku
[PHP] pobierz, plaintext 

Poprawiłem to

Wydaje mi się że wszystko jest okej

[PHP] pobierz, plaintext 
<?php 
session_start();
 
		$password = $_POST['password'];
		$email = $_POST['email'];
 
 
 function checkout_password($password)
{
   if(preg_match("/^(?=.*[A-Z])(?=.*[a-z])(?=.*[0-9])(?=.*[!@#$%^&*()_+|-]).{8,30}$/",$password))
      return true;
   else
      return false;
}	
 
 function checkout_email($email)
{
   if(preg_match('/^[a-zA-Z\.\-\_]+\@[a-zA-Z0-9\.\-\_]+\.[a-z]{2,4}$/D', $email))
      return true;
   else
      return false;
}
 
 
if (checkout_email($email) and checkout_password($password) and isset($_POST['submit']))
   {
    	try
  		 {  
 
	 	   		  $pdo = new PDO('mysql:host=localhost;dbname=user', 'user', 'password');
     	                          $pdo -> setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);	
 
		 			 $passwordhash = sha1($password.'sad8%$9sdk');
 
		      		                $stmt = $pdo -> prepare('SELECT `email`, `password`,`id` FROM `users` WHERE `email` = :email AND `password` = :password'); 
                                                $stmt -> bindValue(':email', $email, PDO::PARAM_STR);
						$stmt -> bindValue(':password', $passwordhash, PDO::PARAM_STR);
                                                $stmt -> execute(); 
 
						$count = $stmt ->rowCount();
 
						if($count > 0) 
						{
 
							while($row = $stmt -> fetch())
      						 {
								 session_regenerate_id (true);
 
							        $_SESSION['status'] = true;
          						        $_SESSION['id'] = $row['id'];     
							        $_SESSION['HTTP_USER_AGENT'] = sha1($_SERVER['HTTP_USER_AGENT'].'dsd#$%^');	    
 
 
								 header('Location: index.php');
								 exit;
      						 }
 
						}	
 
	 	 }
 
	 	 catch(PDOException $e)
  		 {
                 echo 'Error: ' . $e->getMessage();
  		 }
 
   } else {
 
	   header('Location: login.html');
	   exit;
 
   		  }
 
 
 
?>
[PHP] pobierz, plaintext 

Po co pętla while? Nie sądzę by było więcej userów o tym samym emailu i haśle.
Podczas pisania polecam pracować z wyświetlaniem błędów typu E_NOTICE.

Czyli zamiast:

[PHP] pobierz, plaintext 
while($row = $stmt -> fetch())
[PHP] pobierz, plaintext 

Użyć:

[PHP] pobierz, plaintext 
$row = $stmt -> fetch();
[PHP] pobierz, plaintext 

?

Tak

Podobno : $stmt -> closeCursor();
http://pl.wikibooks.org/wiki/PHP/Biblioteka_PDO