PHP login script werkt niet helemaal correct

[RareAMV]

Ok, ik ben bezig gewoon om de tijd te doden om een php/mysql scriptje te maken waarmee je bv kunt inloggen(even niet letten op beveiliging gevallen hier, daarvoor doe ik het nu niet), nu wil ik dat als je de juiste gegevens opgeeft je doorgaat naar een pagina met daarop: u bent ingelogd(suc6log.html), en dat werkt ook.
maar als ik verkeerde gegevens intyp komt hij niet verder dan het openen van het php bestand(browser laat een lege pagina zien met url van de php bestand(die staat in het url gedeelte van de browser)), terwijl hij eigenlijk een andere html pagina moet openen waarop je opnieuw kunt inloggen(nsuc6log.html).
Ik ben aardig nieuw in programmeren en php is mij eerste poging tot, dus ik zou alle hulp kunnen gebruiken die er is.

Al vast bedankt^^.

hier is de code:

PHP:

<?php
$username = $_POST["username"];
$password = $_POST["password"];
$accounts = mysql_connect("localhost" , "root" , "********") or die(mysql_error());

mysql_select_db("accounts" , $accounts);

$sql = "SELECT * FROM users WHERE U_NAME = '" . $username . "' AND PASSWORD = '" . $password . "';";

$result = mysql_query($sql, $accounts);

if($result == FALSE)
{
echo 'your query is not working';
}
else if($result !== FALSE)
{

while($row = mysql_fetch_array($result))
{

$u_name = $row['U_NAME'];
$p_word = $row['PASSWORD'];

 if($u_name == $username AND $p_word == $password)
 {
 header("location: suc6log.html");
 exit();
 }
 else
 {
 header("location: nsuc6log.html");
 exit();
 };

};


};
?>

[Oyster]

Probeer dit eens.

http://php.net/manual/en/function.header.php

Note:

HTTP/1.1 requires an absolute URI as argument to » Location: including the scheme, hostname and absolute path, but some clients accept relative URIs. You can usually use $_SERVER['HTTP_HOST'], $_SERVER['PHP_SELF'] and dirname() to make an absolute URI from a relative one yourself:

E: Whoops, niet goed gelezen.

[ Voor 4% gewijzigd door Oyster op 29-09-2013 16:51 ]

[Sendy]

Je echo't een foutmelding, en daarna probeer je nog de http-header te schrijven. Dat kan niet.

[Voutloos]

Reden lege pagina's: Je controleert username en password al dmv de where clause, dus als die niet kloppen krijg je geen rows terug.

Aangezien dit sowieso je huidige problem is , mag ik ook nog overige tips geven ondanks je disclaimer :
- Hoe je variabelen in queries stopt kan écht niet. Mocht dat in je huidige tutorial niet behandeld worden, zoek het dan aub wel uit voordat je code ooit ergens publiekelijk toegankelijk zet of verspreid.
- Error handling met 'or die()' moet je nooit doen, zelfs niet in development. De wereld zou een betere plek zijn als alle kutvoorbeelden met deze constructie van het web verdwenen.
- Gebruik mysqli_* functies, mysql_* is al jaren afgeraden. Zelfde punt als hierboven over mysql_* voorbeelden. Nog beter maar wellicht een tutorialtje verder: parametrized queries / PDO.

[RareAMV]

@Voutloos bedankt voor de reactie^^, ik zal eens zoeken naar een betere tutorial waar die tips worden behandeld, en wat stom dat ik er niet aan had gedacht dat er natuurlijk niks zou uitkomen, nu ik dat weet werkt het wel.

[Cartman!]

RareAMV schreef op zondag 29 september 2013 @ 16:24:
(even niet letten op beveiliging gevallen hier, daarvoor doe ik het nu niet)

Het maakt niet uit wat of voor wie je iets maakt maar je moet altijd aan beveilging denken. Als je denkt dat je het soms kunt overslaan ga je nooit best practises toepassen en blijf je achter lopen.

[Ramon]

Sowieso even je inspringing regelen, hier snapt toch niemand iets van?

[TJHeuvel]

Er zijn structureel wat dingen mis met je code. Het probleem is als volgt, allereerst haal je alle records op met het ingevoerde gebruikersnaam en wachtwoord, hierna controleer je elke rij die je terug krijgt. Wanneer de gegevens kloppen is dit er dus 1, en zal je while loop 1 maal uitgevoerd worden. Het probleem is; wanneer de gegevens niet kloppen wordt de while helemaal niet gedaan, daarom voert de code niet uit.

Als we even jouw code aanhouden zal dit een correcter scenario opleveren:

PHP:

<?php 
mysql_connect("localhost" , "root" , "********") or die(mysql_error());
mysql_select_db("accounts"); 

$username = $_POST["username"]; 
$password = $_POST["password"]; 

$sql = "SELECT * 
        FROM users 
        WHERE 
            U_NAME = '" . $username . "' AND 
            PASSWORD = '" . $password . "';"; 

$result = mysql_query($sql, $accounts); 

//Pak de eerste rij. We weten zeker dat dit de juiste gebruiker is, in de SQL zoeken we naar de gebruikersnaam/wachtwoord. We hoeven niet weer in PHP dit te controleren.
$row = mysql_fetch_assoc($result);

//De manual zegt dat er false wordt gereturned als er geen rijen zijn, en anders de rij zelf. (http://www.php.net/manual/en/function.mysql-fetch-assoc.php)
if($row) {
    die(header('location: suc6log.html'));
} else {
    die(header('location: nsuc6log.html'));
} 
?>

Wanneer je wat meer ervaring op doet met de constructies die PHP bied zal je dit gelijk opvallen, totdat je daar komt helpt het om te debuggen. Wat we hier praktisch mee bedoelen is dat je bv een aantal echo statements neerzet om te kijken of je er uberhaupt wel in een bepaald stuk code komt.

Zoals je zelf al goed aangeeft, dit is niet veilig, en kan je niet gebruiken in een live omgeving. Enkel moet je toch ergens beginnen!

Een aantal kleine puntjes van verbetering zijn:

• Je hoeft niet de handle van je database connectie op te slaan en mee te geven als argument. Wanneer je er maar 1 hebt begrijpt PHP dat je deze wilt gebruiken, en meestal is 1 genoeg.
• Een check zoals deze is niet echt logisch:
  
  PHP:
  

  if($result == false) {
   echo "Doe A";
  else if($result !== false) {
   echo "Doe B";
  }

  
  De hele else-if kan weg, en vervangen worden door een simpele else. Momenteel staat dubbelop, als hij niet bij "Doe A" komt weet je zeker dat hij niet false is.
• Na een curly bracket ( } ) hoeft geen puntcomma.
• Misschien is het handiger om het verwerken van dit formulier in 1 bestand te zetten, ipv teveel onnodige kleine bestanden aan te maken. Hetvolgende werkt ook gewoon:
  
  
  PHP:
  

  <?php
  
  if(isset($_POST['username'])) {
     //Verwerk hier het formulier!
     $username = $_POST['username'];
  } else {
  //We verlaten hier PHP even, en willen gewoon HTML schrijven.
  ?>
  <form method='POST'>
     <input type='text' name='username' />
     <input type='submit' value='Inloggen' />
  </form>
  <?php 
  }
  
  ?>

• Wanneer je je wat comfortabeler voelt bij het programmeren, zoek even wat tutorials op over veiligheid.

Cartman! schreef op maandag 30 september 2013 @ 21:36:
[...]

Het maakt niet uit wat of voor wie je iets maakt maar je moet altijd aan beveilging denken. Als je denkt dat je het soms kunt overslaan ga je nooit best practises toepassen en blijf je achter lopen.

Ik ben het hier absoluut niet eens, je moet toch ergens beginnen met leren. Jij vind dat je, nog voordat je de taal enigsinds beheerst, al je bezig moet houden met veiligheid? Daarnaast, in hoeverre kan je iemand bijbrengen wat er fout is als 'ie geen idee heeft waar hij of zij mee bezig is.

Het feit dat 'ie er zelf bewust van is dat het niet veilig is, maar eerst de rest wilt beheersen zegt al heel wat!

[ Voor 3% gewijzigd door TJHeuvel op 01-10-2013 08:46 ]

[DeepFreeze.NL]

Voutloos schreef op zondag 29 september 2013 @ 16:41:

- Error handling met 'or die()' moet je nooit doen, zelfs niet in development. De wereld zou een betere plek zijn als alle kutvoorbeelden met deze constructie van het web verdwenen.

Waarom vind je dit een slechte constructie? Ik doe het namelijk ook altijd zo en ik zie niet zo 1-2-3 in waarom dit zo negatief is..

[F.West98]

or die is prima imo, zolang je maar wel een melding geeft als "Kon niet verbinden met Database".
Lege pagina is inderdaad slecht.

Of niet die() maar verwijzen naar een functie / header naar een mooie DB kapot-pagina.

[CH4OS]

Kijk ook even naar zaken als SQL-injection en het gebruik van de user root in MySQL.

[ Voor 59% gewijzigd door CH4OS op 30-09-2013 23:27 ]

[8088]

DeepFreeze.NL schreef op maandag 30 september 2013 @ 23:18:
Waarom vind je dit een slechte constructie? Ik doe het namelijk ook altijd zo en ik zie niet zo 1-2-3 in waarom dit zo negatief is..

Om maar een voorbeeld te noemen: in bovenstaand geval wordt er ondanks de opgetreden fout een status code 200 verstuurd.

[crisp]

CptChaos schreef op maandag 30 september 2013 @ 23:27:
Kijk ook even naar zaken als SQL-injection en het gebruik van de user root in MySQL.

Zeker aangezien de wachtwoorden hier plain-text in de database staan