De PHPhulp kroeg
Feedback (graag wel serieus)
Ik ben nog maar net een weekje bezig met OOP en PDO
en wil graag wat feedback van jullie hebben of ik op de juiste weg ben.
Commentaar zo als:
-Beveiligingen
-Structuur van de code
zijn erg welkom
Ik heb mijn script online in werking staan.
graag als het kan ook sql inject proberen om het beter te kunnen beveilingen.
De sessie id die word mee gegeven in de getName($id) zal uiteindelijk nog 2 variables mee krijgen.
dus hij controlleerd nu op het moment alleen de user id's wat uiteindelijk meer zal worden
maar om dit verder uit te bouwen wil ik dus eerst weten of ik zo goed bezig bent.
Voorbeeld + Code
Al vast heel erg bedankt
m.v.g Rob
Gewijzigd op 22/03/2011 15:16:40 door Robert dat ben ik
- Foutafhandeling moet je niet doen met een echo van de fout, maar met het returnen van false. Tegelijkertijd maak je een nieuwe exception aan en die kan je dan in de producele code opvangen door middel van een try catch systeem. Zie exceptions
- Ik zou niet bij getName weer opnieuw id en username uit de database halen en de sessions aanmaken. Dit laat je over aan de login methode. De getName method hoeft eigenlijk alleen maar de private var username te returnen. Deze kun je bijv. laten opslaan in login methode.
- mysql_real_escape_string is niet de juiste voor pdo, zie ook eerste puntje bij producele code.
[PRODUCELE CODE]
- Wat betekend:
Code (php)
1
2
3
2
3
<?php
if( !preg_match( '/[a-z0-9_]{0,}$/i', $password ) ) { $error++; } //password sql inject protect
?>
if( !preg_match( '/[a-z0-9_]{0,}$/i', $password ) ) { $error++; } //password sql inject protect
?>
Want doordat je gebruikt maakt van prepared statements kan er geen SQL injection gedaan worden. En als je SQL injection tegen wilt gaan moet je werken met mysql_real_escape_string of voor PDO pdo.quote.
- De header functie in index.php gaat een foutmelding opleveren, omdat je deze uitvoert terwijl er al output naar de browser is geweest.
[HTML CODE]
- Je hebt geen doctype, head en html tag. Daardoor gaat hij niet goed werken.
En een echo in een class? Nee, dat heeft weinig nut. Je geeft het terug als een resultaat (array bijvoorbeeld) en die roep je vervolgens aan...
Wouter J op 22/03/2011 16:17:45:
[USER CLASS]
- Foutafhandeling moet je niet doen met een echo van de fout, maar met het returnen van false. Tegelijkertijd maak je een nieuwe exception aan en die kan je dan in de producele code opvangen door middel van een try catch systeem. Zie exceptions
- Ik zou niet bij getName weer opnieuw id en username uit de database halen en de sessions aanmaken. Dit laat je over aan de login methode. De getName method hoeft eigenlijk alleen maar de private var username te returnen. Deze kun je bijv. laten opslaan in login methode.
- mysql_real_escape_string is niet de juiste voor pdo, zie ook eerste puntje bij producele code.
[PRODUCELE CODE]
- Wat betekend:
Want doordat je gebruikt maakt van prepared statements kan er geen SQL injection gedaan worden. En als je SQL injection tegen wilt gaan moet je werken met mysql_real_escape_string of voor PDO pdo.quote.
- De header functie in index.php gaat een foutmelding opleveren, omdat je deze uitvoert terwijl er al output naar de browser is geweest.
[HTML CODE]
- Je hebt geen doctype, head en html tag. Daardoor gaat hij niet goed werken.
- Foutafhandeling moet je niet doen met een echo van de fout, maar met het returnen van false. Tegelijkertijd maak je een nieuwe exception aan en die kan je dan in de producele code opvangen door middel van een try catch systeem. Zie exceptions
- Ik zou niet bij getName weer opnieuw id en username uit de database halen en de sessions aanmaken. Dit laat je over aan de login methode. De getName method hoeft eigenlijk alleen maar de private var username te returnen. Deze kun je bijv. laten opslaan in login methode.
- mysql_real_escape_string is niet de juiste voor pdo, zie ook eerste puntje bij producele code.
[PRODUCELE CODE]
- Wat betekend:
Code (php)
1
2
3
2
3
<?php
if( !preg_match( '/[a-z0-9_]{0,}$/i', $password ) ) { $error++; } //password sql inject protect
?>
if( !preg_match( '/[a-z0-9_]{0,}$/i', $password ) ) { $error++; } //password sql inject protect
?>
Want doordat je gebruikt maakt van prepared statements kan er geen SQL injection gedaan worden. En als je SQL injection tegen wilt gaan moet je werken met mysql_real_escape_string of voor PDO pdo.quote.
- De header functie in index.php gaat een foutmelding opleveren, omdat je deze uitvoert terwijl er al output naar de browser is geweest.
[HTML CODE]
- Je hebt geen doctype, head en html tag. Daardoor gaat hij niet goed werken.
Bedankt voor je informatie Wouter
hier kan ik iets mee om het te verbeteren met error handelingen.
maar dit:
- De header functie in index.php gaat een foutmelding opleveren, omdat je deze uitvoert terwijl er al output naar de browser is geweest.
wat bedoel je hier mee?
Toevoeging op 22/03/2011 16:52:30:
Chris Horeweg op 22/03/2011 16:40:58:
Een wachtwoord alleen bepaalde tekens toe laten, is het slechtste wat je kan doen. Een wachtwoord zet je gecodeerd (MD*, SHA*) in de database en die bestaan ten alle tijden uit cijfers en letters. Onzin dus.
En een echo in een class? Nee, dat heeft weinig nut. Je geeft het terug als een resultaat (array bijvoorbeeld) en die roep je vervolgens aan...
En een echo in een class? Nee, dat heeft weinig nut. Je geeft het terug als een resultaat (array bijvoorbeeld) en die roep je vervolgens aan...
@Chris:
Het wachtwoord word met MD5 opgeslagen.
het bepalen van tekens is niet handig dat weet ik maar het is even om te testen local.
MaDHouSe xxxx op 22/03/2011 16:51:28:
wat bedoel je hier mee?
Zie http://www.phphulp.nl/php/forum/topic/header-probleem/77018/ en mijn uitleg in dat topic.
Het is de drager van de gegevens van een gebruiker.
Het regelt het inloggen.
Het regelt het registeren.
Het regelt de authenticatie (het ingelogd zijn).
Het regelt de autorisatie (wat mag je wel en wat niet).
Je communiceert met de sessie.
Dit hoort (zo veel mogelijk) door verschillende objecten te worden afgehandeld.
Wouter J op 22/03/2011 16:58:13:
Zie http://www.phphulp.nl/php/forum/topic/header-probleem/77018/ en mijn uitleg in dat topic.
MaDHouSe xxxx op 22/03/2011 16:51:28:
wat bedoel je hier mee?
Zie http://www.phphulp.nl/php/forum/topic/header-probleem/77018/ en mijn uitleg in dat topic.
het eenigste wat in die global_header.php staat is dit:
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
<?php
ini_set('display_errors', 1);
error_reporting(E_ALL);
session_start();
include_once "include/db_config.php";
include_once "include/class/users.class.php";
$user = new User($db);
if(isset($_SESSION['id']) && isset($_SESSION['name']))
{
echo "<a href='index.php'>Start</a> | <a href='logout.php'>Logout</a> | <a href='code.php'>Bekijk Code</a><br> ";
}
else
{
echo "<a href='index.php'>Start</a> | <a href='register.php'>Register</a> ";
}
?>
ini_set('display_errors', 1);
error_reporting(E_ALL);
session_start();
include_once "include/db_config.php";
include_once "include/class/users.class.php";
$user = new User($db);
if(isset($_SESSION['id']) && isset($_SESSION['name']))
{
echo "<a href='index.php'>Start</a> | <a href='logout.php'>Logout</a> | <a href='code.php'>Bekijk Code</a><br> ";
}
else
{
echo "<a href='index.php'>Start</a> | <a href='register.php'>Register</a> ";
}
?>
het is niet meer dan bestanden te include bij de pagina waar de class gebruikt word.
De headers zijn
HTTP/1.1 200 OK
Date: Tue, 22 Mar 2011 20:33:55 GMT
Server: Apache/2.2.3 (CentOS)
X-Powered-By: PHP/5.1.6
Set-Cookie: PHPSESSID=q5o38276ueovs1180dith1fc45; path=/
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-store, no-cache, must-revalidate, post-check=0, pre-check=0
Pragma: no-cache
Content-Length: 1046
Connection: close
Content-Type: text/html; charset=UTF-8
dit is toch goed?
Gewijzigd op 22/03/2011 20:36:17 door Robert dat ben ik