Login script
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
25
26
27
28
29
30
31
32
33
34
35
36
37
38
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
<?php
require_once 'db.php';
$errors = [];
if(!isset($_POST['username'])){
$errors[] = 'Username is required.';
}
$username = htmlspecialchars(trim($_POST['username']));
if(!isset($_POST['password'])){
$errors[] = 'Password is required.';
}
$password = htmlspecialchars(trim($_POST['password']));
$query=$db->prepare("SELECT * FROM `users` WHERE `username` = :username");
$query->execute([':username' => $username]);
if($query->rowCount() == 1){
$result = $query->fetch(PDO::FETCH_ASSOC);
$dbpassword = $result['password'];
$user_id = $result['id'];
if(password_verify($password, $dbpassword))
{
$msg = "Login succesful";
session_start();
$_SESSION['id'] = $user_id;
}
else
{
$msg = "Wrong password";
}
}
?>
require_once 'db.php';
$errors = [];
if(!isset($_POST['username'])){
$errors[] = 'Username is required.';
}
$username = htmlspecialchars(trim($_POST['username']));
if(!isset($_POST['password'])){
$errors[] = 'Password is required.';
}
$password = htmlspecialchars(trim($_POST['password']));
$query=$db->prepare("SELECT * FROM `users` WHERE `username` = :username");
$query->execute([':username' => $username]);
if($query->rowCount() == 1){
$result = $query->fetch(PDO::FETCH_ASSOC);
$dbpassword = $result['password'];
$user_id = $result['id'];
if(password_verify($password, $dbpassword))
{
$msg = "Login succesful";
session_start();
$_SESSION['id'] = $user_id;
}
else
{
$msg = "Wrong password";
}
}
?>
Gewijzigd op 24/09/2019 03:59:47 door Jorn Reed
Sowieso hoor je voor de veiligheid nooit te vermelden of een wachtwoord incorrect is. Op die manier kan een aanvaller eenvoudig zien of een gebruikersnaam al bestaat, zodat die verder kan hameren om een wachtwoord te raden. Geef gewoon aan dat "de gebruikersnaam en/of het wachtwoord fout zijn".
En waarom zou je htmlspecialchars() en trim() op een password en username gooien? Een bewerking als htmlspecialchars() hoort in de output te gebeuren. Verder zou je aan de hand van een reguliere expressie kunnen bepalen waaraan een gebruikersnaam moet voldoen. Logischerwijs zijn dit voornamelijk normale karakters van A tot Z en, cijfers en een hoog of laag streepje.
session_start(); kan je ook gewoon aan het begin van je site plaatsen, dan kan je overal je sessies gebruiken.
Verder zou je session-fixation nog kunnen voorkomen daar een nieuwe SessieID te maken na de inlog met session_regenerate_id().
Gewijzigd op 24/09/2019 08:51:35 door - Ariën -
- hangt van db.php af :p; is dit PDO, een aangepaste variant van MySQLi? Iets anders? Want als die op een of andere manier lek gaat maakt het niet uit wat je verder doet
- het enige wat je nodig hebt uit de users tabel zijn het id en het wachtwoord, dit zijn dus ook de enige kolommen die je op zou moeten halen; wat natuurlijk ook niet echt handig is is dat je kolomnaam "password" een gereserveerd woord is in MySQL (er vanuitgaande dat je een MySQL database gebruikt)
- gebruik je ook HTTPS? Want als je op een onveilig netwerk zit dan halen dit soort (serverside) voorzieningen anders niet zoveel uit
- het aanpassen van user-credentials en/of sessie-gegevens is een state change, je zou dan ook direct na afloop de pagina moeten verversen of van locatie moeten veranderen met behulp van een header('Location: ...') in combinatie met een exit; statement, en zoals @Ariën aangeeft, session_regenerate_id() is dan geen overbodige luxe
- er is geen scheiding tussen validatie en verdere verwerking, het een vloeit gewoon over in het ander; het heeft helemaal geen zin om een query uit te voeren als een of beide velden leeg zijn of ontbreken, dus dan zou ik daar de loginpoging al afbreken eerlijk gezegd; zoals in elk formulier: pas als de validatie succesvol is ga je verder
Wat je ook zou kunnen overwegen:
- loginpogingen loggen, zowel succesvol als niet succesvol
- een limiet aan het aantal inlogpogingen
En natuurlijk is het ook interessant wat je hierna allemaal kunt doen. Zitten er ook rechten/rollen/privileges in het systeem? Het kan zijn dat dat op dit moment nog hard coded is op userniveau. In principe is daar niets mis mee maar op het moment dat je straks meer functionaliteit hebt is dit mogelijk een dingetje wat je verder uit zult moeten breiden.
En dan, hoe refereer je aan de gegevens van een geauthenticeerd persoon? Stop je alles in $_SESSION en raadpleeg je rechtstreeks $_SESSION of bouw je elk request een soort User object ofzo?
En tot slot: simpelweg omdat je alles aan de poort eenmalig controleert wil niet zeggen dat alles daarna ineens veilig is :p. Zelfs als je login veilig is zul je moeten blijven waken voor veiligheid. Als op een andere plek SQL-injectie mogelijk is dan heb je nog steeds een heel groot probleem en maakt het niet uit hoe veilig de loginprocedure is. Iets met kettingen en zwakste schakels.
Gewijzigd op 24/09/2019 16:00:50 door Thomas van den Heuvel
Code (php)
1
2
3
4
2
3
4
if(!isset($_POST['username'])){
$errors[] = 'Username is required.';
}
$username = htmlspecialchars(trim($_POST['username']));
$errors[] = 'Username is required.';
}
$username = htmlspecialchars(trim($_POST['username']));
Eerst controleer je dus of $_POST['username'] wel/niet bestaat, maar vervolgens ga je er toch altijd mee aan de slag. PHP rammelt wel door, maar in je log zul je vermoedelijk "Undefined index" terug vinden ('username' zit niet in de $_POST). Het is altijd fijn als je log helemaal "schoon" is. _Als_ er dan een keer wat in komt te staan weet je ook meteen dat je een probleem(pje) op moet lossen (anders zie je door de bomen het bos niet meer, en zie je nooit of je een fout hebt gemaakt).
Ik zou zelf iets doen ala:
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
2
3
4
5
6
7
8
9
10
11
12
13
14
15
$errors = [];
$username = $password = null;
if(isset($_POST['username'])) $username = $_POST['username'];
else $errors[] = 'Username is required.';
if(isset($_POST['password'])) $password = $_POST['password'];
else $errors[] = 'Password is required.';
if(!$errors){
$query=$db->prepare("SELECT * FROM `users` WHERE `username` = :username");
$query->execute([':username' => $username]);
if($query->rowCount() == 1){
//enz
$username = $password = null;
if(isset($_POST['username'])) $username = $_POST['username'];
else $errors[] = 'Username is required.';
if(isset($_POST['password'])) $password = $_POST['password'];
else $errors[] = 'Password is required.';
if(!$errors){
$query=$db->prepare("SELECT * FROM `users` WHERE `username` = :username");
$query->execute([':username' => $username]);
if($query->rowCount() == 1){
//enz
Wijnand de Ridder op 24/09/2019 11:44:11:
En daarnaast: je controleert met isSet, ik zou dat met empty doen, want een lege waarde is ook gezet.
Het probleem van empty is dat een string '0' ook als leeg gezien wordt.
Beter alternatief:
Hoewel dit topic over het inloggen gaat is het belangrijker dat bij het aanmaken van een gebruiker (registratie) en bij het wijzigen van wachtwoorden er op wordt gevalideerd dat er een wachtwoord van een bepaalde minimale lengte is ingeklopt.
Gewijzigd op 24/09/2019 21:11:06 door Frank Nietbelangrijk