Login systeem
Wat denken jullie van dit login systeem,
veilig of niet?
Quote:
<div class="succes">Je bent ingelogd! Je wordt doorgelinkt..</div>
<meta http-equiv="refresh" content="1;URL='index.php'" />
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
<?php
if(isset($_POST['submit'])){
$username = $_POST['username'];
$password = sha1($_POST['password']);
if(empty($username) == true){
echo "<div class='error'>Je moet een gebruikersnaam invullen.</div>";
}elseif(empty($password) == true){
echo "<div class='error'>Je moet een wachtwoord invullen.</div>";
}else{
$stmt = $con->prepare("SELECT * FROM users WHERE username = :username AND password = :password");
$stmt->bindParam(':username', $username, PDO::PARAM_STR);
$stmt->bindParam(':password', $password, PDO::PARAM_STR);
$stmt->execute();
if($stmt->rowCount() > 0){
$row = $stmt->fetch(PDO::FETCH_ASSOC);
if($row['banned'] == "1"){
echo "<div class='error'>Je account is geblokkeerd tot ".$row['banned_date'].". Reden:<br />".$row['banned_reason']."</div>";
}else{
$stmt2 = $con->prepare("UPDATE users SET session_id = '".session_id()."' WHERE id = '".$row['id']."'");
$stmt2->execute();
$_SESSION['login'] = $row['session_id'];
?>
if(isset($_POST['submit'])){
$username = $_POST['username'];
$password = sha1($_POST['password']);
if(empty($username) == true){
echo "<div class='error'>Je moet een gebruikersnaam invullen.</div>";
}elseif(empty($password) == true){
echo "<div class='error'>Je moet een wachtwoord invullen.</div>";
}else{
$stmt = $con->prepare("SELECT * FROM users WHERE username = :username AND password = :password");
$stmt->bindParam(':username', $username, PDO::PARAM_STR);
$stmt->bindParam(':password', $password, PDO::PARAM_STR);
$stmt->execute();
if($stmt->rowCount() > 0){
$row = $stmt->fetch(PDO::FETCH_ASSOC);
if($row['banned'] == "1"){
echo "<div class='error'>Je account is geblokkeerd tot ".$row['banned_date'].". Reden:<br />".$row['banned_reason']."</div>";
}else{
$stmt2 = $con->prepare("UPDATE users SET session_id = '".session_id()."' WHERE id = '".$row['id']."'");
$stmt2->execute();
$_SESSION['login'] = $row['session_id'];
?>
<div class="succes">Je bent ingelogd! Je wordt doorgelinkt..</div>
<meta http-equiv="refresh" content="1;URL='index.php'" />
Als je iets ziet dat onveilig is, wil je dan a.u.b. het voor me verbeteren/me vertellen wat er fout is?
Thnx.
wat je nog zou kunnen doen met het inlog formulier is dat je er nog een hidden field bij maakt waar je een token in zet. dit token bewaar je tevens in de database met de huidige tijd/datum erbij.
als iemand probeert in te loggen dan kijk je of het token dat als POST variabele meegestuurd wordt niet ouder is dan bijv. 10 minuten en of het geldig is.
Ongeveer hetzelfde zou ik ook met de $_SESSION['login'] doen. Geef dat ding een willekeurig token en sla hem tevens op in de database, vergelijk hem iedere keer als een pagina wordt opgevraagd.
Gewijzigd op 22/07/2013 18:26:57 door Frank Nietbelangrijk
Je maakt twee variabelen aan, zonder te controleren of de $_POST waarden wel bestaan. Los daarvan; het aanmaken is natuurlijk niet nodig. Je hebt immers de $_POST waarden.
sha1 is volgens mij niet (meer) het meest veilig. Kijk naar bcrypt.
Zou de controle op regel 8 (het wachtwoord) niet laten afhangen van de uitkomst van regel 6.
Geef iemand bij het onjuist invullen direct daarvan zo veel mogelijk informatie. Als iemand dus naam en wachtwoord niet invult, toon dat dan beide.
Zou geen * gebruiken in de select-query, maar het veld/de velden benoemen die je wilt opvragen.
Zou in php ' gebruiken en in HTML " (dus omgekeerd met wat jij hebt).
Ik zou bij het succesvol inloggen werken met een header ipv refresh.
Frank Nietbelangrijk op 22/07/2013 18:26:09:
Volgens mij is dit redelijk veilig.
wat je nog zou kunnen doen met het inlog formulier is dat je er nog een hidden field bij maakt waar je een token in zet. dit token bewaar je tevens in de database met de huidige tijd/datum erbij.
als iemand probeert in te loggen dan kijk je of het token dat als POST variabele meegestuurd wordt niet ouder is dan bijv. 10 minuten en of het geldig is.
Ongeveer hetzelfde zou ik ook met de $_SESSION['login'] doen. Geef dat ding een willekeurig token en sla hem tevens op in de database, vergelijk hem iedere keer als een pagina wordt opgevraagd.
wat je nog zou kunnen doen met het inlog formulier is dat je er nog een hidden field bij maakt waar je een token in zet. dit token bewaar je tevens in de database met de huidige tijd/datum erbij.
als iemand probeert in te loggen dan kijk je of het token dat als POST variabele meegestuurd wordt niet ouder is dan bijv. 10 minuten en of het geldig is.
Ongeveer hetzelfde zou ik ook met de $_SESSION['login'] doen. Geef dat ding een willekeurig token en sla hem tevens op in de database, vergelijk hem iedere keer als een pagina wordt opgevraagd.
Bedankt voor je reactie.
De session_id() wordt ook in de database opgeslagen.
p.s.: zou je een voorbeeld willen geven van wat je nu precies bedoelt?
Obelix en Idefix op 22/07/2013 18:27:47:
Als iemand dus naam en wachtwoord niet invult, toon dat dan beide.
Zou ik niet doen.
Gewoon iets als: Uw inloggegevens zijn niet correct ingevuld.
Obelix en Idefix op 22/07/2013 18:27:47:
Controleren of een formulier verzonden is (regel 2):
Je maakt twee variabelen aan, zonder te controleren of de $_POST waarden wel bestaan. Los daarvan; het aanmaken is natuurlijk niet nodig. Je hebt immers de $_POST waarden.
sha1 is volgens mij niet (meer) het meest veilig. Kijk naar bcrypt.
Zou de controle op regel 8 (het wachtwoord) niet laten afhangen van de uitkomst van regel 6.
Geef iemand bij het onjuist invullen direct daarvan zo veel mogelijk informatie. Als iemand dus naam en wachtwoord niet invult, toon dat dan beide.
Zou geen * gebruiken in de select-query, maar het veld/de velden benoemen die je wilt opvragen.
Zou in php ' gebruiken en in HTML " (dus omgekeerd met wat jij hebt).
Ik zou bij het succesvol inloggen werken met een header ipv refresh.
Je maakt twee variabelen aan, zonder te controleren of de $_POST waarden wel bestaan. Los daarvan; het aanmaken is natuurlijk niet nodig. Je hebt immers de $_POST waarden.
sha1 is volgens mij niet (meer) het meest veilig. Kijk naar bcrypt.
Zou de controle op regel 8 (het wachtwoord) niet laten afhangen van de uitkomst van regel 6.
Geef iemand bij het onjuist invullen direct daarvan zo veel mogelijk informatie. Als iemand dus naam en wachtwoord niet invult, toon dat dan beide.
Zou geen * gebruiken in de select-query, maar het veld/de velden benoemen die je wilt opvragen.
Zou in php ' gebruiken en in HTML " (dus omgekeerd met wat jij hebt).
Ik zou bij het succesvol inloggen werken met een header ipv refresh.
Ook bedankt voor de reactie, vooral met SHA1 enz.
Voor de rest met de header en de informatie van het onjuist invullen, daar ga ik later verder op in. Het gaat me nu meer om de veiligheid.
Ben ik in zoverre met je eens, dat je niet moet aangeven of het wachtwoord of de naam onjuist is. Waar ik op doel is dat TS controleert op lege velden (regel 6/8).
Ik zou dat niet met een ifelse doen, maar in 1x met OR of (desnoods) met 2x een else.
Gewijzigd op 22/07/2013 19:12:01 door Obelix Idefix
Quote:
<div class="succes">Je bent ingelogd! Je wordt doorgelinkt..</div>
<meta http-equiv="refresh" content="1;URL='index.php'" />
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
<?php
if($_SERVER['REQUEST_METHOD'] == "POST"){
$username = $_POST['username'];
$password = $_POST['password'];
if(empty($username) == true || empty($password) == true){
echo '<div class="error">Je moet een gebruikersnaam invullen.</div>';
}else{
$stmt = $con->prepare('SELECT id,session_id,username,password,banned_date,banned_reason,banned FROM users WHERE username = :username AND password = ENCRYPT(:password, Password)');
$stmt->bindParam(':username', $username, PDO::PARAM_STR);
$stmt->bindParam(':password', $password, PDO::PARAM_STR);
$stmt->execute();
if($stmt->rowCount() > 0){
$row = $stmt->fetch(PDO::FETCH_ASSOC);
if($row['banned'] == '1'){
echo '<div class="error">Je account is geblokkeerd tot '.$row['banned_date'].'. Reden:<br />'.$row['banned_reason'].'</div>';
}else{
$stmt2 = $con->prepare('UPDATE users SET session_id = "'.session_id().'" WHERE id = "'.$row['id'].'"');
$stmt2->execute();
$_SESSION['login'] = $row['session_id'];
?>
if($_SERVER['REQUEST_METHOD'] == "POST"){
$username = $_POST['username'];
$password = $_POST['password'];
if(empty($username) == true || empty($password) == true){
echo '<div class="error">Je moet een gebruikersnaam invullen.</div>';
}else{
$stmt = $con->prepare('SELECT id,session_id,username,password,banned_date,banned_reason,banned FROM users WHERE username = :username AND password = ENCRYPT(:password, Password)');
$stmt->bindParam(':username', $username, PDO::PARAM_STR);
$stmt->bindParam(':password', $password, PDO::PARAM_STR);
$stmt->execute();
if($stmt->rowCount() > 0){
$row = $stmt->fetch(PDO::FETCH_ASSOC);
if($row['banned'] == '1'){
echo '<div class="error">Je account is geblokkeerd tot '.$row['banned_date'].'. Reden:<br />'.$row['banned_reason'].'</div>';
}else{
$stmt2 = $con->prepare('UPDATE users SET session_id = "'.session_id().'" WHERE id = "'.$row['id'].'"');
$stmt2->execute();
$_SESSION['login'] = $row['session_id'];
?>
<div class="succes">Je bent ingelogd! Je wordt doorgelinkt..</div>
<meta http-equiv="refresh" content="1;URL='index.php'" />