Feedback (graag wel serieus)

Overzicht Reageren

Sponsored by: Vacatures door Monsterboard

Robert  dat ben ik

Robert dat ben ik

22/03/2011 15:16:06
Quote Anchor link
Goede middag programmeurs

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
 
PHP hulp

PHP hulp

02/12/2021 20:09:42
 
Wouter J

Wouter J

22/03/2011 16:17:45
Quote Anchor link
[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:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
<?php
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.
 
Chris -

Chris -

22/03/2011 16:40:58
Quote Anchor link
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...
 
Robert  dat ben ik

Robert dat ben ik

22/03/2011 16:51:28
Quote Anchor link
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:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
<?php
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...




@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.
 
Wouter J

Wouter J

22/03/2011 16:58:13
Quote Anchor link
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.
 
Pim -

Pim -

22/03/2011 17:33:05
Quote Anchor link
Je User klasse doet te veel.
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.
 
Robert  dat ben ik

Robert dat ben ik

22/03/2011 17:46:36
Quote Anchor link
Wouter J op 22/03/2011 16:58:13:
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)
PHP script in nieuw venster Selecteer het PHP script
1
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> ";
}

?>


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
 



Overzicht Reageren

 
 

Om de gebruiksvriendelijkheid van onze website en diensten te optimaliseren maken wij gebruik van cookies. Deze cookies gebruiken wij voor functionaliteiten, analytische gegevens en marketing doeleinden. U vindt meer informatie in onze privacy statement.