Uitgebreide login en registratie met database
Door P Singh, 8 jaar geleden, 39.668x bekeken
Zat een door deze forum heen te kijken, en ik zag niet zo'n script.
Dit is niet letterlijk kopiëren en plakken. Je moet alles echt in je eigen gegevens zetten. Om duidelijke te maken waar, heb ik het in comments gezet.
Het is geschreven in PDO, geen Mysql/Mysqli dus.
(Een upload fout, het begint bij pdo.php)
Gesponsorde koppelingen
PHP script bestanden
Er zijn 1 reacties op 'Uitgebreide login en registratie met database'
Gesponsorde koppelingen
tl;dr niet veilig en kan op sommige plaatsen wel wat verbeterd worden in termen van organisatie van code, logica en functionaliteit.
loginPDO.php
- "DATABASE CONNECTIE", waarna je vervolgens een sessie start
- $connect = new PDO(...), zou je $connect niet beter $db kunnen noemen ofzo?
- er ontbreekt een charset-parameter in de DSN :( (beschikbaar vanaf PHP 5.3.6)
- uhh, het password staat onversleuteld opgeslagen in je database?
- waar is je tabeldefinitie, is op zijn minst de kolom username case-sensitive, want anders resulteert een controle als $count > 0 rare resultaten op? wat als er een gebruiker Henk en een gebruiker HENK is, en zij beiden hetzelfde wachtwoord hebben (welkom123 ofzo :p), wie wordt er dan ingelogd?
- waarom je voorkeur voor header('Refresh: ...') en niet header('Location: ...')?
- executie van het script gaat gewoon verder na het instellen van headers, deze transporteren je niet direct automagisch naar een andere locatie maar pas aan het einde van het script; meestal is het de bedoeling dat verdere uitvoer van code direct wordt gestaakt na een header(...) aanroep, zet daartoe ALTIJD een exit; statement na een header()-aanroep
- doe je iets met $message?
- dit script is een include, maar kan rechtstreeks aangeroepen worden? kan mogelijk misbruikt worden voor bruteforce aanvallen?
- op regel 51 roep je session_start() opnieuw aan, waarom?
- op regel 52 schrijf je sessie-data weg, maar op regel 19 en regel 40 produceer je al output, wat mogelijk kan resulteren in "headers already sent" fouten, maar mogelijk kwam je die niet tegen omdat je al output buffering aan had staan?
- combinatie van Engels (username, password) en Nederlands (Gebruikers) in naamgeving is nagenoeg altijd onhandig...
pdo.php
- kun je dit script niet beter "register.php" noemen?
- wederom een header() zonder een exit, het hele script wordt dus afgedraaid waarna je meteen geredirect wordt
- hier maak je opnieuw een connectie met hardcoded parameters, waarom?
- waarvoor wordt ID gebruikt? en waarom wordt ID verder niet gebruikt :); als het is om iemand te identificeren, hiervoor zou je toch ook de username al kunnen gebruiken, en iemand zijn werkelijke naam (voornaam, tussenvoegsel(s), achternaam) apart kunnen opslaan; je zou dus onderscheid kunnen maken tussen ACCOUNT-informatie en PROFIEL-informatie
- er is geen controle of je opnieuw een record toevoegt met eenzelfde username, meestal moet er iets uniek zijn en doorgaans is dat (onder andere) de username; ook is het handig om het e-mailadres uniek te laten zijn, immers, als je op den duur een lost password functionaliteit hebt (die overigens ook ontbreekt), en je hebt meerdere usernames geregistreerd onder één e-mailadres, welke accountgegevens verzend je dan en/of voor welk account tref je voorbereidend werk voor een password-reset? En zelfs al heb je een UNIQUE constraint op een case-sensitive username-veld, waarschijnlijk wil je om praktische redenen een case-INsensitive check uitvoeren, want je wilt niet twee gebruikers met nagenoeg dezelfde username (Henk en HENK ofzo); hier mag best wat meer denkwerk ingestoken worden
- ah dus dit is eigenlijk een login- of registreer-pagina zoals je wel vaker ziet, maar hier begeef je je toch een beetje in een rare spagaat met -in wezen- twee if-statements die ofwel de verwerking van een registratie ofwel de verwerking van een inlogpoging afhandelen, waarbij die include aan het eind toch een beetje als een verdwaald stuk code overkomt, en deze zou eigenlijk ge-require-d moeten worden (if anything) want de code kan echt niet verder zonder deze include en is daarmee dus verplicht (required)
- er zit geen limiet op het registreren van leden en er is ook geen activatiestap-via-bevestiging-via-e-mail? dit werkt vervuiling van je systeem in de hand en dat lijkt mij niet wenselijk
- mijn voorkeur zou uitgaan naar het scheiden van acties en andere handelingen:
* één include voor het maken van een database-connectie
* één script voor het tonen van de registreer/login pagina (deze doet dus niets meer dan het weergeven van formulieren)
* één script voor het verwerken van een registratie (bij voorkeur met voorzieningen voor bevestiging en activatie)
* één script voor het verwerken van een inlogpoging (mogelijk uitgebreid met een limiet aan het aantal pogingen of wat dan ook)
veilig.php
- wederom een header() zonder een exit
- ironisch genoeg is veilig.php alles behalve veilig, je propt immers $username RECHTSTREEKS IN JE QUERYSTRING, dus als ik mij registreer met
dan kan ik een rij van alle users uitdraaien, en met een UNION en wat CONCATs kan ik de andere informatie (onversleutelde wachtwoorden enzo) er ook wel uitvissen... GG
- overigens is het een GROVE MISCONCEPTIE om te denken dat DATA "veilig" is op het moment dat deze in de database zit, deze is nog steeds even gevaarlijk als toen deze aangeleverd werd uit een externe bron, het is nog steeds data van een externe bron, of deze nu uit de database komt of niet, deze dien je NOOIT te vertrouwen
- enne SELECT * <--> $row[1] - je bent maar geinteresseerd in één kolom, en je gaat er ook nog eens vanuit dat die zich op een specifieke plaats bevindt (index 1), niet echt handig noch verstandig; indien de samenstelling of de volgorde van de kolommen van de Gebruikers tabel ooit nog een mocht veranderen kan dit mogelijk deze code breken
logout.php
- je doet er ook verstandig aan $_SESSION eerst leeg te maken en het sessie-cookie ongeldig te maken, dit zorgt er mede/beter voor dat de sessie daadwerkelijk onbruikbaar wordt
PS: waarom username in je sessie in plaats van een (auto increment) user-id (niet ID, maar een echt auto-increment veld waarmee je INTERN uniek een gebruiker kunt identificeren), zo zorg je er tevens voor dat iemand totaal geen invloed kan uitoeferen op de data in je sessie en had je je SQL injectie probleem misschien ook niet gehad (maar zou nog steeds opgelost moeten worden uiteraard)
loginPDO.php
- "DATABASE CONNECTIE", waarna je vervolgens een sessie start
- $connect = new PDO(...), zou je $connect niet beter $db kunnen noemen ofzo?
- er ontbreekt een charset-parameter in de DSN :( (beschikbaar vanaf PHP 5.3.6)
- uhh, het password staat onversleuteld opgeslagen in je database?
- waar is je tabeldefinitie, is op zijn minst de kolom username case-sensitive, want anders resulteert een controle als $count > 0 rare resultaten op? wat als er een gebruiker Henk en een gebruiker HENK is, en zij beiden hetzelfde wachtwoord hebben (welkom123 ofzo :p), wie wordt er dan ingelogd?
- waarom je voorkeur voor header('Refresh: ...') en niet header('Location: ...')?
- executie van het script gaat gewoon verder na het instellen van headers, deze transporteren je niet direct automagisch naar een andere locatie maar pas aan het einde van het script; meestal is het de bedoeling dat verdere uitvoer van code direct wordt gestaakt na een header(...) aanroep, zet daartoe ALTIJD een exit; statement na een header()-aanroep
- doe je iets met $message?
- dit script is een include, maar kan rechtstreeks aangeroepen worden? kan mogelijk misbruikt worden voor bruteforce aanvallen?
- op regel 51 roep je session_start() opnieuw aan, waarom?
- op regel 52 schrijf je sessie-data weg, maar op regel 19 en regel 40 produceer je al output, wat mogelijk kan resulteren in "headers already sent" fouten, maar mogelijk kwam je die niet tegen omdat je al output buffering aan had staan?
- combinatie van Engels (username, password) en Nederlands (Gebruikers) in naamgeving is nagenoeg altijd onhandig...
pdo.php
- kun je dit script niet beter "register.php" noemen?
- wederom een header() zonder een exit, het hele script wordt dus afgedraaid waarna je meteen geredirect wordt
- hier maak je opnieuw een connectie met hardcoded parameters, waarom?
- waarvoor wordt ID gebruikt? en waarom wordt ID verder niet gebruikt :); als het is om iemand te identificeren, hiervoor zou je toch ook de username al kunnen gebruiken, en iemand zijn werkelijke naam (voornaam, tussenvoegsel(s), achternaam) apart kunnen opslaan; je zou dus onderscheid kunnen maken tussen ACCOUNT-informatie en PROFIEL-informatie
- er is geen controle of je opnieuw een record toevoegt met eenzelfde username, meestal moet er iets uniek zijn en doorgaans is dat (onder andere) de username; ook is het handig om het e-mailadres uniek te laten zijn, immers, als je op den duur een lost password functionaliteit hebt (die overigens ook ontbreekt), en je hebt meerdere usernames geregistreerd onder één e-mailadres, welke accountgegevens verzend je dan en/of voor welk account tref je voorbereidend werk voor een password-reset? En zelfs al heb je een UNIQUE constraint op een case-sensitive username-veld, waarschijnlijk wil je om praktische redenen een case-INsensitive check uitvoeren, want je wilt niet twee gebruikers met nagenoeg dezelfde username (Henk en HENK ofzo); hier mag best wat meer denkwerk ingestoken worden
- ah dus dit is eigenlijk een login- of registreer-pagina zoals je wel vaker ziet, maar hier begeef je je toch een beetje in een rare spagaat met -in wezen- twee if-statements die ofwel de verwerking van een registratie ofwel de verwerking van een inlogpoging afhandelen, waarbij die include aan het eind toch een beetje als een verdwaald stuk code overkomt, en deze zou eigenlijk ge-require-d moeten worden (if anything) want de code kan echt niet verder zonder deze include en is daarmee dus verplicht (required)
- er zit geen limiet op het registreren van leden en er is ook geen activatiestap-via-bevestiging-via-e-mail? dit werkt vervuiling van je systeem in de hand en dat lijkt mij niet wenselijk
- mijn voorkeur zou uitgaan naar het scheiden van acties en andere handelingen:
* één include voor het maken van een database-connectie
* één script voor het tonen van de registreer/login pagina (deze doet dus niets meer dan het weergeven van formulieren)
* één script voor het verwerken van een registratie (bij voorkeur met voorzieningen voor bevestiging en activatie)
* één script voor het verwerken van een inlogpoging (mogelijk uitgebreid met een limiet aan het aantal pogingen of wat dan ook)
veilig.php
- wederom een header() zonder een exit
- ironisch genoeg is veilig.php alles behalve veilig, je propt immers $username RECHTSTREEKS IN JE QUERYSTRING, dus als ik mij registreer met
dan kan ik een rij van alle users uitdraaien, en met een UNION en wat CONCATs kan ik de andere informatie (onversleutelde wachtwoorden enzo) er ook wel uitvissen... GG
- overigens is het een GROVE MISCONCEPTIE om te denken dat DATA "veilig" is op het moment dat deze in de database zit, deze is nog steeds even gevaarlijk als toen deze aangeleverd werd uit een externe bron, het is nog steeds data van een externe bron, of deze nu uit de database komt of niet, deze dien je NOOIT te vertrouwen
- enne SELECT * <--> $row[1] - je bent maar geinteresseerd in één kolom, en je gaat er ook nog eens vanuit dat die zich op een specifieke plaats bevindt (index 1), niet echt handig noch verstandig; indien de samenstelling of de volgorde van de kolommen van de Gebruikers tabel ooit nog een mocht veranderen kan dit mogelijk deze code breken
logout.php
- je doet er ook verstandig aan $_SESSION eerst leeg te maken en het sessie-cookie ongeldig te maken, dit zorgt er mede/beter voor dat de sessie daadwerkelijk onbruikbaar wordt
PS: waarom username in je sessie in plaats van een (auto increment) user-id (niet ID, maar een echt auto-increment veld waarmee je INTERN uniek een gebruiker kunt identificeren), zo zorg je er tevens voor dat iemand totaal geen invloed kan uitoeferen op de data in je sessie en had je je SQL injectie probleem misschien ook niet gehad (maar zou nog steeds opgelost moeten worden uiteraard)
Om te reageren heb je een account nodig en je moet ingelogd zijn.
PHP hulp
0 seconden vanaf nu