[PHP] Problema con conferma registrazione

[Fede 88]

Ho creato la parte della mail di conferma per la registrazione al mio sito internet, che funziona nel seguente modo:

Dopo aver controllato tutti i dati ed averli inseriti nel database creo un numero ad 8 cifre (random) e lo salvo in un campo [codice] nella tupla dell'account appena creato. A questo punto invio una mail con il link alla pagina "attivazione.php" con accodato il suddetto codice tramite metodo GET.
L'utete cliccando sul link attiva l'account.

Inizialmente sembrava funzionare alla perfezione, però provando a sostituire il codice ricevuto via mail con altri codici scritti, diciamo a caso, mi sono accorto che veniva visualizzato comunque account attivato.

Questo è il codice:

<?php

include("connessioneDB.php");

$code = $_GET['code'];

if (!$code) { echo "ERRORE: Codice di attivazione non fornito!"; }
else
{
// Seleziono tutte le tuple che hanno codice uguale al codice ricevuto tramite
// metodo GET e campo conf=1, ovvero che sono già stati attivati
$check = mysql_query("SELECT * FROM utenti WHERE codice='$code' AND conf='1'");
// Seleziono username e password degli utenti con il suddetto codice
$username = mysql_query("SELECT username FROM utenti WHERE codice='$code'");
$password = mysql_query("SELECT password FROM utenti WHERE codice='$code'");
// creo una sessione con i selezionati dal DB
session_start ();
$_SESSION['username'] = $username;
$_SESSION['pass'] = $password;


if(mysql_num_rows($check)==1) { echo "Il tuo account è già stato attivato!"; }
else
{
// Attivo l'account settando il campo conf a 1
$activate = mysql_query("UPDATE utenti SET conf='1' WHERE codice='$code'");
echo "Il tuo account è stato attivato!";
}

}



Qualcuno sa dirmi il perchè?

[kurts]

dunque, prima di tutto un consiglio:
scrivendo
$code = $_GET['code'];
tu dai per scontato che la variabile "code" esista ed abbia un valore assegnato, sarebbe meglio una cosa del genere:

if(IsSet($_GET['code'])){
$code = $_GET['code'];
}
else{
//qui ti gestisci il caso in cui non ci sia un valore
}


per quanto riguarda la query dovrebbe essere questa: (occhio alle virgolette ed al punto per la concatenazione)

$check = mysql_query("SELECT * FROM utenti WHERE codice='".$code."' AND conf='1'");


se non ho capito male il campo conf è un boolean che vale 1 se l'account è attivo, a questo punto mi chiedo, non sarebbe meglio eseguire la query tralasciando per il momento il campo conf per controllarlo in seguito?
così facendo eviti di eseguire le gue query successive, che puoi comunque eseguire in una volta sola.

per quanto riguarda la password io ti consigliere di evitare di memorizzarla, la controlli una sola volta, al momento del login, magari confrontando gli hash MD5 così eviti di trasmetterla in chiaro.
inoltre ora tu non controlli che io conosca effettivamente il nome utente e la password relative al codice immesso, li vai semplicemente a recuperare da DB.

in conclusione io scriverei uno script del genere:

<?php
include("connessioneDB.php");

if(!IsSet($_GET['code'])){
echo "ERRORE: Codice di attivazione non fornito!"; }
}
else{
$code = $_GET['code'];
$check = mysql_query("SELECT * FROM utenti WHERE codice='".$code."'");
$curruser=mysql_fetch_array($check);
if($curruser['conf']=='1'){
echo "Il tuo account è già stato attivato!";
}
else{
$activate = mysql_query("UPDATE utenti SET conf='1' WHERE codice='".$code."'");
echo "Il tuo account è stato attivato!";
}
}
?>

ho dato per scontato che il codice di attivazione sia generato in modo da non averne mai 2 uguali tra loro

ho buttato giu il codice al momento, quindi attendo tue notizie sull'eventuale presenza di errori.

---------

gia che ci sono colgo l'occasione per presentarmi:
ciao a tutti, mi chiamo giacomo, ho 23 anni, vivo in provincia di varese, sono studente in scienze e tecnologie dell'informazione presso l'università degli studi dell'insubria di como dove sto per laurearmi.

complimenti per il forum ed anche a tutti gli utenti che lo popolano, continuate così.

[Luca 88]

ciao, innanzitutto benvenuto! io stò facendo il sito con il mio amico che ha postato il problema....

ho provato a fare come consigliato da te... e funziona come funzionava il nostro... cioè lo attiva ma se nella barra metti un qualsiasi numero ti dice: "Il tuo account è stato attivato!"

ora... magari il problema non è in quel file .php... ma nella registrazione... ti posto il codice del file registra.php inerente all'attivazione:


$codice = rand(11111111,99999999);
$to = $email;
$subject = "Attiva il tuo account";
$headers = "From: [email protected]";
$body = "Ciao $username, \n\nTi sei registrato e devi fare l'attivazione. Clikka il link sotto per attivare il tuo account\n\nhttp://127.0.0.1/cartellasito/attivazione.php?codice=$codice\n\nGrazie!";

//controllo il funzionamento della mail
if (!mail($to,$subject, $body, $headers)) {echo "non funziona la mail";}
else {
// Salvo i dati del nuovo utente nel database
mysql_query("INSERT INTO utenti (email, username, password, nascita , sesso, codice, data_reg) VALUES ('$email', '$username', '".md5($pass)."', DATE('$anno/$mese/$giorno') , '$sesso', '$codice', NOW())") or die (mysql_error());
echo "Registrazione avvenuta con successo! attiva l'accaunt con la mail che ti arriva a ($email)";


ovviamente nelle righe precedenti c'erano dei controlli per vedere delle cose... e se tutto è ok allora ti allora ti appare questa cosa: "Registrazione avvenuta con successo! attiva l'accaunt con la mail che ti arriva a ($email)";

va tutto perfettamente... ma come detto prima con qualsiasi numero si metta ti dice account attivato...

[Fede 88]

Grazie per la risposta, ho provato e funziona, l'unico problema è che sostituendo il codice con altre cifre casuali, visualizza la stringa: "il tuo account è stato attivato", perchè se non trova nessun account corrispondente a quel codice ovviamente il campo con sarà diverso da 1.

Quindi penso che per risolvere basti aggiungere un controllo per vedere se la select ha ottenuto risultati.

[Fede 88]

Sistemato in questo modo:


<?php

include("connessioneDB.php");

// Controllo che la variabile "codice" ricevuta tramite GET abbia un valore
if(!IsSet($_GET['codice'])){ echo "ERRORE: Codice di attivazione non fornito!";}
else
{
$codice = $_GET['codice'];
$check = mysql_query("SELECT * FROM utenti WHERE codice='".$codice."'");
$appo = mysql_num_rows($check);
if ($appo != 1) { echo "Non corrisponde nessun account al codice di attivazione usato"; }
else
{
$curruser=mysql_fetch_array($check);
if($curruser['conf']=='1') { echo "Il tuo account è già stato attivato!"; }
else
{
$activate = mysql_query("UPDATE utenti SET conf='1' WHERE codice='".$codice."'");
echo "Il tuo account è stato attivato!";
}
}

}

?>

[kurts]

se non trova nessun account corrispondente a quel codice ovviamente il campo con sarà diverso da 1.

gia gia, hai ragione, me ne ero proprio dimenticato!
ora funziona tutto come si deve?
mi permetto di darvi un altro consiglio:
voi usate la funzione "rand()" per generare un numero compreso tra 11111111 e 99999999, ora, il mio consiglio è, se non avete assolutamente bisogno di avere un numero intero come codice di attivazione, di usare qualcosa come

$code=time().$username;
MD5($code);


perchè?
-rand() potrebbe generare codici di attivazioni uguali per più utenti;
-col timestamp siamo abbastanza sicuri che 2 utenti non ricevano lo stesso codice
-per sicurezza ci concateniamo l'username che è unico
-alla fine criptiamo il tutto in MD5 così da non visualizzare l'username

chiaramente sono solo dei consigli che vi do senza sapere quali sono le esatte specifiche del vostro progetto quindi vedete voi se a accettarli o no.

P.S.: grazie per il benvenuto! :D

[Fede 88]

Quindi in pratica alla fine accodiamo una stringa di 32 caratteri alfanumerici, giusto?

Mi sembra una buona idea.

Poi ho anche aggiunto un altra modifica, quando l'account è attivato il campo codice viene svuotato.

[kurts]

Quindi in pratica alla fine accodiamo una stringa di 32 caratteri alfanumerici, giusto?

coi 32 caratteri ti riferisci all'hash md5? si, lo puoi usare al posto della funzione rand() ma non sei obbligato, potresti benissimo usare anche solamente questo:
MD5($username)
io ho aggiunto anche il time() così da aumentare il grado di "casualità" del codice.
chiaramente tu puoi implementare la tua funzione personale di generazione del codice.

Poi ho anche aggiunto un altra modifica, quando l'account è attivato il campo codice viene svuotato.
questa è una buona idea, anche se generalmente si preferisce non lasciare mai dei campi vuoti all'interno di un DB se non è proprio necessario.
un'altra miglioria che potresti apportare è aggiungere il LIMIT=1 così da far fermare la query al primo risultato trovato evitanto sprechi di tempo e soprattutto di traffico

[Luca 88]

grazie kurts... non è facile trovare persone che ti aiutino in questa maniera...

comunque... non centra con questa discussone... ma noi dobbiamo ancora rendere "sicura" la nostra registrazione... inserendo dei controlli che impeniscano di "impallarci" il sito... tu sai qualcosa riguardo a questa cosa? quali sono i controlli principali da mettere?

grazie ancora!

[kurts]

forse io non sono la persona più indicata per questo genere di argomento comunque qualche dritta te la posso sempre dare.

-non trasmettere mai dati sensibili in chiaro, ad esempio invece di lavorare sulle pwd usa i loro hash md5.
-quando tratti dei dati presi da campi di input filtrali sempre, in questo modo eviti che vengano visualizzati dei messaggi di warning (innocui ma fastidiosi) in caso di campi vuoti e soprattutto ti proteggi da eventuali tentativi di code injection (decisamente fastidiosi). lo stesso vale per l'upload di file, filtra sempre le estensioni.
-se ti aspetti un dato numerico allora potresti effettuare un cast del tipo in modo da forzarlo ad assumere comunque un valore numerico (stessa cosa per gli altri tipi di dato).
-potresti aggiungere un captcha al form di registrazione oltre che all'ativazione via mail.

che altro?
ora come ora non mi viene in mente niente, comunque ti consiglio di leggerti qualche guida in materia e magari qualche video tutorial che, secondo me, aiutano sempre a capire quello che si sta studiando