[PHP]MYSQL Login

basi1 · 19. Januar 2013

Hi,
da wir in der Schule zurzeit ein Projekt am laufen haben brauche ich mal eure Hilfe unzwar schreibe ich zurzeit ein Loginscript und würde gerne wissen was man verbessern kann und ob es so überhaupt funktioniert , die andere eigentliche Frage ist dann noch die ich weiß nicht wieso mir das Script keine Meldung gibt wenn der Benutzer garnicht in der Datenbank ist !

PHP

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
"http://www.w3.org/TR/html4/loose.dtd">
<?php
$db = @new mysqli("localhost", "root", "" , "Webshop");
if (mysqli_connect_errno())
{
die('Datenbankfehler: '.mysqli_connect_error());
}
else
{
?>
Status:<font color="#00FF00">Online</font>
<?php
}
?>
<html>
<head>
<h1><center><div id="top">Webshop v.1</div></center></h1>
<link rel="stylesheet" type="text/css" href="css/style.css">
</head>
<body>
<form method="POST" action="login.php">
<div id="midun">
Username:
<br>
<input type="text" name="username" value="" size="25">
<br>
</div>
<div id="midpw">
Passwort:
<br>
<input type="text" name="passwort" value="" size="25">
<br>
</div>
<div id="midbt">
<input type="submit" name="submit" value="Submit">
<input type="reset" name="reset" value="Reset">
<br>
<a href="register.php">Registrieren</a>
</div>
<?php
if(isset($_POST['username']) && isset($_POST['passwort']))
{
$username = mysql_real_escape_string($_POST['username']);
$passwort = md5(mysql_real_escape_string($_POST['passwort']));
$query = "SELECT login, passwd FROM kunden WHERE login LIKE '$username' LIMIT 1";
$result = $db->query($query);
if (!$result)
{
die('Datenbankfehler: '.$db->error);
};
while ($row = $result->fetch_assoc())
{
if ($row['passwd'] == $passwort && $row['login'] == $username)
{
?>
<center>
<?php
echo "Erfolgreich eingeloggt!";
?>
</center>
<?php
}
else if(empty($row))
{
?>
<center>
<?php
die('Der Benutzer exestiert nicht');
?>
</center>
<?php
}
else
{
?>
<center>
<?php
die('Einloggen Fehlgeschlagen!');
?>
</center>
<?php
}
}
}
?>
</form>
</body>
</html>

Alles anzeigen

Edit: Sessions habe ich nochnicht drinne kommt aber noch !

Coban · 19. Januar 2013

Kleiner Tip, verwende anstatt der IF-Anweisung or die. Is viel cooler und übersichtlicher. Und wenn du ein echo "<center>"; echo"</center>"; machst, musst du php nicht immer schliessen und aufmachen.

basi1 · 19. Januar 2013

Konkretes Beispiel ?

florian0 · 19. Januar 2013

Ich zähl mal alles auf, was mir noch so auffällt ...

- Wenn du vom Passwort eh den MD5-Hash berechnest, brauchst du das nicht durch escape-string quetschen. (Solange du den MD5-Hash per PHP berechnest. Wenn du das per SQL regelst, musst es es escapen).
- LIKE ist für einen direkten Vergleich ungeeignet. LIKE ist für Suchvorgänge da. Verwende lieber WHERE login = '$username'

Logischerweise sollte ein Username nur ein einziges mal vorkommen. Das Result des Querys sollte deshalb auch nur eine einzige Zeile sein (oder keine, wenn der Username nicht existiert)
Daher macht hier eine while-Schleife prinzipell keinen Sinn. Außerdem brauchst du den Usernamen des Results nicht nochmal prüfen (außer du vertraust dem SQL-Server nicht).

Besser (ungetestet) :

PHP

$result = $db->query($query);
if(!$result)
{
// Query fehlgeschlagen
}
else
{
$row = $result->fetch_assoc()
if ($row != NULL)
{
if ($row['passwd'] == $passwort)
{
// Login Ok
}
else
{
// Passwort falsch
}
}
else
{
// Username existiert nicht
}
}

Alles anzeigen

Gruß
florian0

basi1 · 20. Januar 2013

Danke für die Top antwort ! Hat mir sehr geholfen wenn jemand noch weitere Verbesserungsvorschläge hat nur raus damit

Akeshihiro · 20. Januar 2013

Coban schrieb:

Kleiner Tip, verwende anstatt der IF-Anweisung or die. Is viel cooler und übersichtlicher.

Das bitte gleich vergessen, das ist nämlich Schwachsinn. die() und exit() sollte man nur dann benutzen, wenn es irgendwie Sinn ergibt, aber das ist so gut wie nie der Fall und so schon gar nicht. Man bricht damit nämlich die gesamte restliche Verarbeitung des Skriptes ab. Der Output ergibt keinen Sinn, eventuell müssen Ressourcen freigegeben werden, Aufräumarbeiten, etc. Mit solchen blödsinnigen Aktionen hat man dann so tolle Darstellungsprobleme, wie auf vielen Seiten und man sieht sowas auch immer wieder in Tutorials.

Fehler sollte man abfangen und sauber abarbeiten, Fehlermeldungen generieren und in einen Output-Buffer werfen und erst am Ende alles ausgeben lassen. So hat man eine saubere Homepage, selbst im Fehlerfall, der User ist nicht verwirrt, es sieht professioneller aus und der Code ist vor allem auch viel besser strukturiert als mit diesen sprunghaften Anweisungen.

Tonicum · 20. Januar 2013

Eine Verbesserung auf die schnelle - Passwörter MD5 verschlüsselt? sonst verschlüssel sie einfach mit md5()

basi1 · 20. Januar 2013

Tonicum ich weiß ehrlich gesagt nicht was du meinst ist doch md5 verschlüsselt ...

FragSalat · 20. Januar 2013

Erstmal nen Lob das du an das escapen der Strings mit mysql_real_escape_string gedacht hast

Dann kann ich nur sagend as ich dir empfehlen kann die PHP logik oben zu machen und unten nur alles auszugeben.
Sprich DB Connection erstellen, Variablen vorbereiten, Query abfeuern, Validieren und eine user Variable erstellen.
Im HTML püfste dann den erwarteten Wert und gibst das entsprechende html aus.
Logik im HTML ist total unsauber und hässlich.

Zudem benötigst du kein while ($row = $result->fetch_assoc()).
Da du nur einen User überprüfen willst bzw nur ein User zulässig ist reicht $user = $result->fetch_assoc()

Wie schon gesagt wurde kannst du das PW matching bei einem einfachen MD5 hash auch in der Query vornehmen.
SELECT * FROM user WHERE username = $username AND password = MD5($password)

Die Zeile ist auch überflüssig da du ja ohne ein übereinstimmenden eintrag in der Datenbank eh kein ergebnis zurück bekommst.
if ($row['passwd'] == $passwort && $row['login'] == $username)

Ich hoffe das hat geholfen.
Gruß Sick^

Teilen