Rate my login en register class
Meningen aub
Quote:
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
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
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
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
<?php
//require ('config/config.php');
//session_start();
error_reporting(0);
class auth_form {
public $username;
public $login_user;
public $login_pass;
public $get_user_query;
public $logged_in;
public $login_submit;
public $password;
public $password_again;
public $submit;
public $errors = array();
public $dbc;
public $errMsg = "<div class=\"alert alert-danger\">
<strong>Foutmelding:
</strong>
Alle velden zijn verplicht, wachtwoord moet minimaal 8 tekens zijn.
</div>";
public $succMsg = "<div class=\"alert alert-success\">
<strong>Gebruiker toegevoegd</strong>
</div>";
public $loginSuccMsg = "<div class=\"alert alert-success\">
<strong>U bent ingelogd</strong>
</div>";
public $faultLoginMsg = "<div class=\"alert alert-danger\">
<strong>Foutmelding:
</strong>
Verkeerde inlog-gegevens.
</div>";
public function __construct()
{
$this->dbc = new mysqli("localhost", "root", '', "spotitube");
if ($this->dbc->connect_errno){
die($this->dbc->connect_error);
}
}
//DONT MIND THE WAY OF SECURING, ITS JUST FOR TEST PURPOSE, AND NOT FOR REAL SECURING !
public function register(){
$this->username = htmlspecialchars($_POST['username']);
$this->password = md5(htmlspecialchars($_POST['password']));
$this->password_again = md5(htmlspecialchars($_POST['password2']));
$this->submit = $_POST['submit'];
if (isset($this->submit)){
if ($this->validate_registration() > 0){
echo $this->errMsg;
}
}
}
public function login(){
$this->login_user = htmlspecialchars($_POST['login_username']);
$this->login_pass = md5(htmlspecialchars($_POST['login_password']));
$this->login_submit = $_POST['login_submit'];
$this->get_user_query = "SELECT username, password from users WHERE username = '$this->login_user' AND password = '$this->login_pass'";
$this->validate_login();
}
public function validate_login(){
if (isset($this->login_submit)) {
$result = $this->dbc->query($this->get_user_query);
if ($result->num_rows > 0) {
echo $this->loginSuccMsg;
$_SESSION['username'] = $this->login_user;
//echo $_SESSION['username'];
} else {
echo $this->faultLoginMsg;
}
}
}
public function validate_registration(){
if (strlen($this->password) < 8
|| empty($this->username)
|| empty($this->password)
|| empty($this->password_again)
|| $this->password !== $this->password_again
){
$this->errors[] = $this->errMsg;
return $this->errors;
}else{
echo $this->succMsg;
$query = "INSERT INTO users (username, password) VALUES ('$this->username', '$this->password')";
$this->dbc->query($query);
}
}
}
$auth = new auth_form();
$auth->register();
$auth->login();
?>
//require ('config/config.php');
//session_start();
error_reporting(0);
class auth_form {
public $username;
public $login_user;
public $login_pass;
public $get_user_query;
public $logged_in;
public $login_submit;
public $password;
public $password_again;
public $submit;
public $errors = array();
public $dbc;
public $errMsg = "<div class=\"alert alert-danger\">
<strong>Foutmelding:
</strong>
Alle velden zijn verplicht, wachtwoord moet minimaal 8 tekens zijn.
</div>";
public $succMsg = "<div class=\"alert alert-success\">
<strong>Gebruiker toegevoegd</strong>
</div>";
public $loginSuccMsg = "<div class=\"alert alert-success\">
<strong>U bent ingelogd</strong>
</div>";
public $faultLoginMsg = "<div class=\"alert alert-danger\">
<strong>Foutmelding:
</strong>
Verkeerde inlog-gegevens.
</div>";
public function __construct()
{
$this->dbc = new mysqli("localhost", "root", '', "spotitube");
if ($this->dbc->connect_errno){
die($this->dbc->connect_error);
}
}
//DONT MIND THE WAY OF SECURING, ITS JUST FOR TEST PURPOSE, AND NOT FOR REAL SECURING !
public function register(){
$this->username = htmlspecialchars($_POST['username']);
$this->password = md5(htmlspecialchars($_POST['password']));
$this->password_again = md5(htmlspecialchars($_POST['password2']));
$this->submit = $_POST['submit'];
if (isset($this->submit)){
if ($this->validate_registration() > 0){
echo $this->errMsg;
}
}
}
public function login(){
$this->login_user = htmlspecialchars($_POST['login_username']);
$this->login_pass = md5(htmlspecialchars($_POST['login_password']));
$this->login_submit = $_POST['login_submit'];
$this->get_user_query = "SELECT username, password from users WHERE username = '$this->login_user' AND password = '$this->login_pass'";
$this->validate_login();
}
public function validate_login(){
if (isset($this->login_submit)) {
$result = $this->dbc->query($this->get_user_query);
if ($result->num_rows > 0) {
echo $this->loginSuccMsg;
$_SESSION['username'] = $this->login_user;
//echo $_SESSION['username'];
} else {
echo $this->faultLoginMsg;
}
}
}
public function validate_registration(){
if (strlen($this->password) < 8
|| empty($this->username)
|| empty($this->password)
|| empty($this->password_again)
|| $this->password !== $this->password_again
){
$this->errors[] = $this->errMsg;
return $this->errors;
}else{
echo $this->succMsg;
$query = "INSERT INTO users (username, password) VALUES ('$this->username', '$this->password')";
$this->dbc->query($query);
}
}
}
$auth = new auth_form();
$auth->register();
$auth->login();
?>
Enkele opmerkingen:
- Gebruik zeker GEEN md5() voor beveiliging. Deze werkwijze is zwaar gedateerd. Gebruik password_hash() en password_verify().
- Gebruik geen HTML in je class.
- Waarom sla je een username in de class op? Deze kan je via een method prima uit je database halen samen met je andere velden. Sla als sessie bijv. enkel de ID op.
- Zorg ervoor dat je sessie na het inloggen direct verandert. Dit voorkomt een session-hijacking.
Gewijzigd op 03/06/2018 08:43:04 door - Ariën -
Of Het nu logisch is of niet.
Dit script is nu echt zo'n geval.
Als je met classes werkt, dan is het mooi als je daarin een stukje logica in brengt.
Het mooie van classes is dat je daarin OO concepten kunt verwerken, maar dan moet je ze wel toepassen.
Vaak zie je dan dat men zoiets maakt en bijvoorbeeld er een database in propt, daar doe je het eigenlijk wel goed. Immers een database heeft totaal niets te maken met een login/registreer ding.
Je zou dus dit nog een stapje verder kunnen doen. Want login is een op zich staand ding en registreren ook.
Valideren van input ook. Dus in dit geval zou ik nog eens kijken of je dit niet in meerdere classes kunt maken.
Om een simpel voorbeeld te geven:
Een class voor user input kan je immers ook elders gebruiken. Denk bijvoorbeeld met een contact class, of comment class. Dat is het mooie van in classes met OO werken. Een class doet een ding, maar weet niet wat het moet doen. Daardoor word je herbruikbaarheid vergroot.
Dan in deze class zelf, je maakt alles public, waarom?
Je hebt ook nog iets van private, protected,
Public: wil zeggen je maakt overal in een variabele/functie beschikbaar.
private: wil zeggen je maakt een variabele/functie beschikbaar alleen in zijn eigen class.
protected: wanneer je variabele functie zichtbaar wilt maken in alle klassen die de huidige klasse uitbreiden, inclusief de bovenliggende klasse.
* Ik denk dat het bovenstaande nog wel wat verder uitgekristalliseerd kan worden. Je bent met allerlei user-gerelateerde zaken bezig. Waarom noem je je klasse dan niet "User" in plaats van "auth_form"? Ook kun je dingen splitsen. Het opdelen van de webpagina in verschillende acties, het bouwen en valideren van een formulier et cetera. Er valt iets voor te zeggen om voor elk van die onderdelen klassen en methoden te schrijven.
* Het idee van het creëren van een object van een klasse is mede dat je de "toestand" van een object op die manier kunt bijhouden. Ik weet niet welke toestand je in de bovenstaande code wilt vangen, maar volgens mij is het in deze aanpak niet echt nodig om unberhaupt een object aan te maken?
* Ik kan mij wel voorstellen dat je op een gegeven moment een User object binnen je code wilt kunnen gebruiken, ik neem aan dat je niet de hele tijd rechtstreeks aan $_SESSION wilt refereren (dit zou je ook niet moeten willen). De sessie zou je eigenlijk alleen moeten gebruiken om te propageren WIE iemand is, WAT iemand vervolgens kan/mag doen (door middel van rollen, rechten of een ander systeem) zou elke page-access opnieuw berekend moeten worden (dit zodat er geen privileges van het moment van inloggen blijven "hangen" of niet (direct) worden bijgewerkt op het moment dat deze wijzigen), dus het ligt dan voor de hand om daar een User object voor te gebruiken. Het enige wat eigenlijk hoeft bij te houden in zo'n object is:
- het user id (waarbij je eerst met behulp van de sessie en andere hulpstukken aannemelijk hebt gemaakt dat je nog steeds met een eerder geauthenticeerde gebruiker te maken hebt)
- rollen/rechten.
That's it. Supersimpel. 100% transparant.
* Het bovenstaande is een samenraapsel van dingen die een gebruiker zou moeten kunnen doen:
- registreren
- inloggen
- authenti(fi)ceren (vaststellen of iemand is die hij/zij zegt te zijn, dit is niet echt "valideren" denk ik? je valideert gegevens, niet personen :))
- maar ook uitloggen
Alle toeters en bellen hier omheen, formulieren en de validatie enzo. Is allemaal leuk, maar heeft (nogmaals) niet direct iets te maken met een User (class) en valt dus ook buiten de scope van deze klasse. Een klasse en zijn methoden zouden allemaal eenvoudige en eenduidige acties moeten bevatten (idealiter verricht elke methode maximaal één specfifieke actie). Het zou in principe niet eens uit moeten maken waar de informatie vandaan komt ($_POST of anderszins), deze data zouden gewoon parameters moeten zijn in de methoden... Misschien is dat ook een hele eenvoudige vuistregel: als geen of heel weinig methoden parameters hebben, dan is er iets niet pluis :).
* Vanaf de eerst letter code zou er al nagedacht moeten worden over een transparante en veilige aanpak, vooral als het een dergelijke pilaar van je applicatie vormt. Dit is geen saus die je er na afloop nog eventjes overheen gooit. Dus dit:
Code (php)
1
//DONT MIND THE WAY OF SECURING, ITS JUST FOR TEST PURPOSE, AND NOT FOR REAL SECURING !
lijkt mij geen goede zaak.
* in de constructor maak je een database-connectie aan. Waarom injecteer je niet een extern database-object (stuur je deze niet mee in de constructor)? Klasses kunnen ook objecten uitwisselen.
* Voor het geval je je nog niet hebt verdiept in de wondere wereld van character encoderingen is dit een goed moment. Bij het aanmaken van je database-connectie stel je geen character encoding in :(.
* Waarom escape je alles op voorhand met htmlspecialchars()? Dit is niet nodig, en mogelijk ook foutgevoelig. htmlspecialchars() dient eigenlijk alleen toegepast te worden op externe (user) data voordat deze wordt weergegeven in een HTML-document. Dit biedt ook geen enkele bescherming tegen SQL-injectie. Met enig experimenteren is het waarschijnlijk mogelijk om de login() routine om de tuin te leiden en als een willekeurige gebruiker in te loggen. Dit lijkt mij een definitieve showstopper.
* Persoonlijk zou ik een user id opslaan in de sessie, en niet een username.
* EDIT als je je registreert wordt niet gecontroleerd of de username reeds aanwezig is. Dit zou je wel moeten doen lijkt mij. Voor de goede orde zou dit geheel aan queries samen één ondeelbare actie moeten vormen, hiervoor heb je dus een database-transactie nodig. En hiervoor heb je (bij mijn weten?) nog steeds database-tabellen met de InnoDB-engine nodig. Ik weet niet of jij jouw tabel ook zo hebt gecreëerd (ook met een specifieke character set)?
En zo kan ik nog wel even doorgaan denk ik.
Gewijzigd op 03/06/2018 14:28:35 door Thomas van den Heuvel
Een wachtwoord is als een sleutel met een aantal willekeurige bits. Het wachwoord wordt ingetikt als een string, je moet dus de encoding op orde hebben. Zie deze tutorial. Zo moet je valideren of de encoding van $_POST['password'] juist is.
En je moet je inlezen in hoe je sessies überhaupt veilig maakt. Zie de OWASP Session Management Cheat Sheet.
Ook vanuit PHP zijn er de nodige veiligheidsinstellingen te maken. Zie Sessions and Security
Op logisch vlak zijn er ook nog verbeteringen mogelijk: implementatie van de SessionHandlerInterface, regenereren van de sessie ID, controle over de sessie cookie (bv. same site attribuut).
Gewijzigd op 02/06/2021 12:51:18 door Ad Fundum
Dit topic is gesloten.