WebBase

Als ik de code vlug bekijk zou je in grote lijnen de aantekeningen bij dit loginscript kunnen bekijken / volgen.

Zoals FangorN al opmaakt is dit onveilig geschreven en zitten er nog veel slordigheden in je code. Kortom nog veel te verbeteren.

p.s. Dit is niet echt OOP, je maakt gebruik van classes etc. alleen hebben je classes niet zijn eigen verantwoordelijkheid. Neem bijvoorbeeld "Users" hier zit het inloggen in verwerkt maar ook het hashen van wachtwoorden. In dit geval zou je bijvoorbeeld een apparte "package" moeten maken voor het hashen van wachtwoorden.

Meh, in een poging om niet meerdere keren allemaal dezelfde dingen op te schrijven is het in dit geval een beetje gebackfired. Daarbij zijn niet alle opmerkingen hier ook van toepassing (vandaar "in grote lijnen").

Het loginscript waar ik naar verwees maakt gebruik van cookies. De user class hier... goochelt met sessievars, ik kan in eerste oogopslag niet echt overzien wat er gebeurt. De __construct methode van een user object lijkt mij iig te lang / te complex.

Ik zie geen documentatie voor initieel gebruik, of een SQL-bestand voor de creatie van tabellen (mogelijk heb ik deze over het hoofd gezien?). En ook geen notities over eventuele minimale PHP/MySQL versies i.v.m. compatibiliteit.

Voor de rest is hier *in grote lijnen* hetzelfde aan de hand als bij het script waar ik naar verwees:
- geen expliciete character encoding (bijvoorbeeld in je database connectie, het escapen van output is nog niet geimplementeerd, mogelijk regelt twig dit?)
- geen "one point of entry" wat voor een beter overzicht / betere security kan zorgen
- het ontbreken van een soortement van autoloader
- het gebruik van "global" in classes (is een beetje "not done" in OOP)
- geen opdeling van "acties" in logische eenheden of "acties" in verkeerde class

En dan iets wat ik keer op keer zie (en niet alleen hier):

<?php
if ($someSecurityCheck === false) {
    // security check failed, user should not be here
    header('Location: elsewhere.php');
}
// SOME IMPORTANT OR SENSITIVE CODE
// ...
?>

headers worden pas verstuurd vlak voordat er output wordt geproduceerd. Stel dat een niet-geauthoriseerd persoon deze pagina bezoekt, dit is wat er achtereenvolgens gebeurt:

1. check levert false op, we stappen het if-statement in
2. er wordt een (HTTP) header geset
3. *** // SOME IMPORTANT OR SENSITIVE CODE wordt uitgevoerd ***
4. einde script, headers + output worden verstuurd en je wordt effectief geredirect

De misvatting is nog altijd dat header('Location: ...') je direct transporteert naar een andere locatie, en dat klopt nog altijd niet. Als je dit zo wilt laten werken, zorg dan dat je altijd een exit; statement laat volgen op deze header() call. Of mogelijk beter: maak een redirect() functie of methode waarin je deze twee statements (header + exit) samenneemt, zodat je dit nooit vergeet.

(EDIT: ik durf te wedden dat sommige lezers nu naarstig op zoek gaan naar dit soort passages LOL)

Daarnaast (en dit voert wellicht wat verder dan de scope van dit project) zit er (nog) geen fancy URL's en/of een soort van rechtenmanagement (Access Control List of equivalent) in.

Mogelijk is dat op dit moment nog niet begroot in je project, maar iets om wellicht wel rekening mee te houden in de toekomst, oftewel, is je ontwerp ook zodanig dat deze nog verder uitgebouwd kan worden?