Werken met levels op website (gewone gebruiker of admin)

[Robinho96]

Beste tweakers,

Ik heb een probleempje met mijn website:

Ik zou er voor moeten kunnen zorgen dat ik alleen in het admingedeelte kan geraken (ik heb dan het nummer 1 en de gewone gebruiker heeft het nummer 0.

Ik heb dit als code voor de levels, maar het werkt niet. ik heb voor te testen twee accounts aangemaakt op mijn website (een met level 1 en een ander met level 0):

PHP:

$user = mysql_query("SELECT * FROM `profielen` WHERE
`Gebruikersnaam`='".$_SESSION['valid_user']." ' " );
$rij = mysql_fetch_array($user);

function getUserlevel() {
    if($rij['level'] == '1') { echo "<a href='?action=Admin'> Admin </a> <br>"; }
else($rij['level'] == '0') 
{ echo " "; }

}

[Razr]

Moet:

code:

if($user['level'] == '1')

Niet de $rij variable zijn?

[MisterEagle]

Kan je je code blok aanpassen naar PHP highlights (code=php)

verder raad ik je aan om nooit zomaar $_SESSION[''] in je query te gebruiken, zonder deze inhoud te controleren of te escapen. Dit is SQL injection gevoelig.

En het klopt wat @Razr zegt.

[ Voor 15% gewijzigd door MisterEagle op 08-05-2014 14:17 ]

[Acid_Burn]

Nu onderdruk je die link. Maar wat als die gebruiker dan zelf ?action=Admin in zijn URL invoert? Houd je daar wel rekening mee?

[Robinho96]

@Razr: sorry ik heb dit fout overgenomen van mijn code. Zal het aanpassen!

@MisterEagle: Dit stond ook in het voorbeeld dat op het internet uitgelegd staat.

@Acid_Burn: Ik zal mijn action veranderen in iets anders waar niemand aan denkt.

[Razr]

Dus het werkt nu nog steeds niet? Krijg je een foutmelding? Roep je de functie uberhaupt ergens aan, zo ja waar?

[Robinho96]

nee ik krijg bij beide accounts geen adminlinkje meer te zien.

[ Voor 5% gewijzigd door Robinho96 op 08-05-2014 14:26 ]

[.SnifraM]

Robin Verbeek schreef op donderdag 08 mei 2014 @ 14:20:
@Razr: sorry ik heb dit fout overgenomen van mijn code. Zal het aanpassen!

@MisterEagle: Dit stond ook in het voorbeeld dat op het internet uitgelegd staat.

@Acid_Burn: Ik zal mijn action veranderen in iets anders waar niemand aan denkt.

De action veranderen naar iets wat anderen niet kunnen bedenken is niet slim. Je kunt veel beter op de betreffende pagina nogmaals controleren of diegene die de pagina bezoekt met ?action=Admin wel echt admin is (bijv via een database entry).

[ocwil]

Ik gok dat dit de fout is

PHP:

 if($rij['level'] == '1') { echo "<a href='?action=Admin'> Admin </a> <br>"; }
else($rij['level'] == '0') 
{ echo " "; }

elseif

[ Voor 7% gewijzigd door ocwil op 08-05-2014 14:31 ]

[MisterEagle]

zet na

PHP:

$rij = mysql_fetch_array($user);

eens:

PHP:

print_r($rij);

Wat krijg je dan als output?

en als je

PHP:

$rij = mysql_fetch_array($user);

vervangt door

PHP:

$rij = mysql_fetch_assoc($user);

werkt het dan wel?

[SinergyX]

Moet je niet ergens ook die functie aanroepen?

[RobIII]

Waar hoort mijn topic?
WEB >> PRG

[andydewit]

Ik denk dat je hier een aardig eind mee komt:

PHP:

<?php 
$user = mysql_query("SELECT * FROM profielen WHERE Gebruikersnaam = '".mysql_real_escape_string($_SESSION['valid_user'])." ' " ); 
$rij = mysql_fetch_assoc($user); 

function userIsAdmin() { 
    if($rij['level'] == '1') {
        return true;
    } 
    else($rij['level'] == '0') {
        return false;
    }
} 

if (userIsAdmin()) { 
    echo "<a href='?action=Admin' title='Admin'>Admin</a><br />"; 
} else { 
    echo "Geen admin"; 
} 
?>

[ Voor 9% gewijzigd door andydewit op 08-05-2014 14:45 ]

[kwaakvaak_v2]

code:

<?php 
function userIsAdmin() {  
    if($rij['level'] == '1') { 
        return true; 
    }  
    else($rij['level'] == '0') { 
        return false; 
    }

Ohh je bedoelt

code:

<?php
function userIsAdmin() {  
    return ($rij['level'] === '1');
}

Want als het niet expliciet de string '1' is het dus geen admin en is die andere check overbodig.

[InZane]

Robin Verbeek schreef op donderdag 08 mei 2014 @ 14:20:
@MisterEagle: Dit stond ook in het voorbeeld dat op het internet uitgelegd staat.

Dus alle voorbeelden die op internet staan zijn automatisch goed?

@Acid_Burn: Ik zal mijn action veranderen in iets anders waar niemand aan denkt.

Dat is natuurlijk geen oplossing. Beter maak je het écht veilig!

[Woy]

MisterEagle schreef op donderdag 08 mei 2014 @ 14:15:
verder raad ik je aan om nooit zomaar $_SESSION[''] in je query te gebruiken, zonder deze inhoud te controleren of te escapen. Dit is SQL injection gevoelig.

Hoewel het inderdaad niet handig is om op deze manier query's te bouwen, heb je als applicatie natuurlijk wel zelf de controle over wat er in de SESSION staat, en dus zou ik niet meteen zeggen dat dit gevoelig is voor SQL Injection.

Verder is het natuurlijk belangrijk dat de TS zich realiseert dat de gegevens die van de client komen nooit te vertrouwen zijn.

Robin Verbeek schreef op donderdag 08 mei 2014 @ 14:20:
@Acid_Burn: Ik zal mijn action veranderen in iets anders waar niemand aan denkt.

Dit is niet voldoende, je moet deze gegevens gewoon niet van de client laten komen. Je gebruiker logt in mijn zijn credentials die hij alleen weet. Daarna zul jij gewoon op de server moeten controleren of deze gebruiker rechten heeft, en niet afgaan op wat de gebruiker je voorschotelt. ( Het is overigens prima om de link wel/niet te laten zien aan de hand van de user gegevens, maar je moet ook daadwerkelijk zorgen dat alleen de gebruiker die het daadwerkelijk mag deze uit kan laten voeren ).

PS:
Ik weet overigens niet precies wat je aan het maken bent, en iedereen moet het natuurlijk een keer leren. Maar ik mag hopen dat je niet met privacy gevoelige gegevens bezig bent, want dan kan je het misschien beter over laten aan mensen die beter weten waar ze mee bezig zijn.

[ Voor 16% gewijzigd door Woy op 08-05-2014 15:42 ]

[BikkelZ]

Laten we ophouden over het detail dat hij een $_SESSION var gebruikt in een query en beginnen met het grootste probleem dat er geen PDO gebruikt wordt met prepared statements. Het grootste probleem is dan al ondervangen. Eigenlijk mag er gewoon helemaal geen code meer geschreven worden die geen gebruik maakt van minimaal PDO met prepared statements (of beter).

Met name gericht aan mensen die verbeterde code terug geven. Alles wat je in de verbeterde code laat zitten is namelijk goede code in de optiek van degene die gecorrigeerd wordt.

PHP:

//$user = mysql_query("SELECT * FROM profielen WHERE Gebruikersnaam = '".mysql_real_escape_string($_SESSION['valid_user'])." ' " ); 
//$rij = mysql_fetch_assoc($user);

$data['username'] = $_SESSION['valid_user']; // <-- ik zou alle session reads en writes centraal houden in één omvattend object
$sth = $this->db->prepare("SELECT * FROM profielen WHERE Gebruikersnaam = :username");  
$sth->execute($data);
$result = $sth->fetch();

Ik zal je de rant over Nederlandstalige variabelenamen maar besparen voor vandaag

[ Voor 4% gewijzigd door BikkelZ op 08-05-2014 15:52 ]

[jbdeiman]

andydewit schreef op donderdag 08 mei 2014 @ 14:43:
Ik denk dat je hier een aardig eind mee komt:

PHP:

<?php 
$user = mysql_query("SELECT * FROM profielen WHERE Gebruikersnaam = '".mysql_real_escape_string($_SESSION['valid_user'])." ' " ); 
$rij = mysql_fetch_assoc($user); 

function userIsAdmin() { 
    if($rij['level'] == '1') {
        return true;
    } 
    else($rij['level'] == '0') {
        return false;
    }
} 

if (userIsAdmin()) { 
    echo "<a href='?action=Admin' title='Admin'>Admin</a><br />"; 
} else { 
    echo "Geen admin"; 
} 
?>

En hoe komt die in de functie "userIsAdmin()" dan aan de $rij variabele, als die daarbuiten wordt gedefinieerd, niet global is en ook niet wordt meegegeven aan de functie?

[Robinho96]

Ja dat werkt bij mij ook niet hoor!

[Creepy]

Robin Verbeek schreef op donderdag 08 mei 2014 @ 17:13:
Ja dat werkt bij mij ook niet hoor!

En wat verwacht je nu dan van ons? Als je en klaar werkende oplossing verwacht, dan is je verwachting verkeerd

Heb je uberhaupt al gecontroleerd of de functie wordt aangeroepen en wat daar dan precies in gebeurd? Dan was je waarschijnlijk al opgevallen wat er dan precies mis gaat.

[ Voor 24% gewijzigd door Creepy op 08-05-2014 17:20 ]

[Robinho96]

Ja ik had zoiets in mijn gedachten en ik dacht dat dit wel zou werken.

[Daniel.]

Zoals al wordt aangegeven is het handig om PDO te gebruiken. Mysql is deprecated en wordt zwaar afgeraden om te gebruiken. Je kan je er nog wel in verdiepen en het zal zeker wel werken maar het is verstandiger en beter om PDO of mysqli te gebruiken.
Daarnaast kan je van ons hulp verwachten maar we kunnen geen complete code voor je gaan schrijven.

[EtHeO18]

Hoi Robin,

De variabele $rij is niet beschikbaar binnen je functie userIsAdmin. Dit kun je oplossen door bijvoorbeeld global $rij; bovenin de functie te zetten. Maar een veel schonere oplossing, is de specifieke user meegeven aan de functie die checkt of het een admin betreft.

PHP:

function userIsAdmin($username){
    ....
}

Verder raad ik je aan om eens naar mysqli te kijken. Dit kun je redelijk drop-in je mysql-gebruik laten vervangen. Waarschijnlijk is het enige wat je hoeft te doen, overal waar mysql_ staat, mysqli_ neer te zetten. Hiermee kun je dan gelijk eens kijken hoe je prepared statements toe moet passen. Dat zou genoeg leesvoer moeten zijn, voor nu even