procedura di autenticazione e sessioni

[tommasosacramon]

Gentili, sto implementando una procedura di autenticazione (login) che utilizzi le sessioni per verificarne lo stato sulle pagine di un'area privata.
Mi sembra in regola e funzionante ma mi piacerebbe un parere da altri occhi critici che ne evidenziano magari punti di debolezza.
Vi posto il codice della pagina login.php quasi per intero:

<?	
session_start();
if (!(isset($_SESSION["autenticato"])))
{
    if ( (isset($_POST['userhash'])) and (isset($_POST['passwordhash'])) )
    {		
        if ((ereg("^[a-zA-Z0-9]+$", $userhash)) || (ereg("^[a-zA-Z0-9]+$", $passwordhash)))
        {			
            $query="select * from autenticazione where userhash='".$userhash."' and   passwordhash='".$passwordhash."'";			
            ...
           $numrows=(@mysql_num_rows($result));				        
           if ( (isset($numrows)) && ($numrows>0) )
          {				
              session_regenerate_id();
              $_SESSION["autenticato"]="ok";				
              header('location:login_autenticato.php');
           }
          else//dati di autenticazione non corretti 
               header('location:login.php');	
         }//non passa controllo caratteri
        else			
           header('location:login.php');						
      }//se non è stato postato nulla ripropone la form

}//gia autenticato
else
{
    echo "Gia autenticato, vai a login_autenticato";
    die;
}
?>
...
<form action="login.php" name="registrazione" method="post">
User

<input type="text" name="user" size="20" maxlength="20"/>
			
Password

<input type="password" name="password" size="20" maxlength="20"/>
	
<input type="button" value="Accedi" onclick="login();" />      
</form>
...

Le pagine del tipo login_autenticato.php:

<? session_start();	
	if (!isset($_SESSION['autenticato']))
	{
		header('location:login.php');		
		exit;		
	}
?>

E naturalmente la pagina di logout.php

<?php 
session_start();
session_unset();
session_destroy();
?>

Ringrazio chi può darmi qualche spunto per eventuali miglioramenti.
Ciao a tutti

[mr.click]

lo script è corretto...poi il termine miglioramenti è molto soggettivo...la programmazione è un modo di concepire "programmi" molto artigianale..io al posto tuo per ex avrei scritto molte righe di codice in meno

[tommasosacramon]

Ciao, sì hai ragione è molto soggettivo ed in effetti è proprio quello che intendevo.
Miglioramenti soggettivi, in modo da avere un altro punto di vista rispetto al mio che magari tende ad essere sempre lo stesso. Ritengo molto importante cercare di guardare le cose anche da altre prospettive. Grazie cmq x la risposta

[mxdesign]

Il tuo codice è buono, mi pare pero di intendere che non usi password criptate, il che è sconsigliato.

Potresti salvarle in db usando sha1. In pratica il campo pass lo setti come char(40) e salvi i campi usando la funzione php sha1 (o aggiorni le vecchie pass con la funzione mysql sha1), poi vai a sostituire questa tua query:

$query="select * from autenticazione where userhash='".$userhash."' and   passwordhash='".$passwordhash."'"; 

con

$query="select * from autenticazione where userhash='".$userhash."' and   passwordhash='".sha1($passwordhash)."'"; 

[tommasosacramon]

Ciao,
l'ho omesso per non ammorbare ma ho utilizzato nomi dei campi userhash e passwordhash per intendere che i dati sul db sono criptati. Utilizzo un doppio md5.
Non ho utilizzato md5(md5($passwordhash)) nella query perchè il codice si riferisce al caso in cui non sia possibile utilizzare https. In questo caso utilizzo un javascript che cripta la password (2 volte) prima di fare submit.
Nel caso in cui posso utilizzare https invio la password e nella query utilizzo il confronto come detto da te:

$query="select * from autenticazione where userhash='".md5(md5($usernonhashato))."' and   passwordhash='".md5(md5($passwordnonhashato))."'";

Ciao grazie per la risposta