Problem mit Loginscript (wahrscheinlich MySQL Fehler)

[MarkusH.]

Hallo, ich habe in den letzten Tagen ein Loginscript geschrieben. Doch jetzt habe ich ein problem bei dem ich hoffe das ihr mir weiter helfen könnt. Ich habe schonmal ein Login/Registrierscript geschrieben, doch das hab ich mit PDO gemacht. Dieses hier möchte ich aber gern mit MySQLi lösen.

login.php
mysql.php

MySQL tabelle users:

Es kommt bei der anmeldung immer zu error 3 / verhalten 3

[Robert B.]

Moin @MarkusH.,

vorab: relevanten Code bitte hier posten, das erleichtert es ungemein.

Zu deinem Problem: Ich schreibe mal den Code deiner mysql.php an die Stelle in login.php, an der er inkludiert wird, vielleicht fällt es dann schon auf:

$verhalten = 0;

if(!isset($_SESSION["user"]) && isset($_GET["page"]) && $_GET["page"]=="log"){
    $user = strtolower($_POST["username"]);
    $password = $_POST["password"];

    # Beginn mysql.php
    $host = "localhost";
    $databank = "web";
    $user = "root";
    $password = "";
    $mysql = mysqli_connect($host, $user, $password, $databank);
    if($mysql->errno){
        die ("Fehler: Verbindung zur Datenbank fehlgeschlagen!");
    }
    # Ende mysql.php

    //$passwordstatment = mysqli_query($mysql, "SELECT password FROM users WHERE username = ?");
    
    $passwordstatment = $mysql->query( "SELECT password
            FROM users
            WHERE username = '$user'" );

Wenn dein $user wie beabsichtigt nicht überschrieben würde, hättest du übrigens eine hübsche SQL-Injection in deinem Query.

Viele Grüße
Robert

[MarkusH.]

Ich bin ja blöd 🙈

Danke @Robert B. Es funktioniert!

[Felix Riesterer]

Lieber MarkusH.,

Ich habe schonmal ein Login/Registrierscript geschrieben, doch das hab ich mit PDO gemacht. Dieses hier möchte ich aber gern mit MySQLi lösen.

warum?

login.php

Du machst etwas sehr gefährliches! Dein Code:

$user = strtolower($_POST["username"]);
[...]
$passwordstatment = $mysql->query( "SELECT password
            FROM users
            WHERE username = '$user'" );

Durch Deine Umkopiererei von Userinput in scheinbar harmlose Variablennamen übersiehst Du einen SQL-Injection-Angriffsvektor. Besser sieht man die Gefahr ohne die Umkopiererei:

$passwordstatment = $mysql->query(
  "SELECT password
   FROM users
   WHERE username = '".strtolower($_POST["username"])."'"
);

Die vom Benutzer eingegebenen Daten könnten auch keinen Benutzernamen, sondern SQL-Syntax beinhalten (siehe verlinkten Comic). Daher sollst Du das unbedingt lassen!

Warum liest Du nicht eine Liste der vorhandenen Benutzernamen in ein Array, am besten noch mit dem Passwort-Hash, und prüfst in einer Schleife, ob die Kombi passt? Der Anmeldeprozess darf ruhig eine Weile dauern, dann sind auch Brute-Force-Attacken auf Dein Loginscript weniger effektiv.

Liebe Grüße,

Felix Riesterer.

[MarkusH.]

Ok werd drauf achten!

Danke

[dedlfix]

Tach!

Ich habe schonmal ein Login/Registrierscript geschrieben, doch das hab ich mit PDO gemacht. Dieses hier möchte ich aber gern mit MySQLi lösen.

Bringt keinen Vorteil. Dafür ist das Handling von Prepared Statements weniger schön als bei PDO. Bleib lieber bei PDO. Und nimm Prepared Statements statt Nutzereinaben direkt ins Statement einzufügen, ohne den Kontext zu beachten. Das ist eine SQL-Injection-Lücke, einer der häufigsten und leicht ausnutzbaren Programmierfehler.

Es kommt bei der anmeldung immer zu error 3 / verhalten 3

Debugging kommt nun an die Reihe. Wenn es zu diesem Fehler kommt, dann liegt das daran, dass deine if-Anweisungen entsprechend verzweigen. Du muss nun nachschauen, wo die Werte herkommen, die die ifs verarbeiten und warum sie so sind und nicht so wie du dir das vorstellst. Du baust dir ein Statement zusammen. Lass es dir anzeigen. Kopier es und lass es direkt im phpMyAdmin ausführen, um die Ergebnismenge zu sehen, die dabei rauskommt.

dedlfix.

[Felix Riesterer]

Lieber MarkusH.,

login.php

manchmal ist es übersichtlicher, anstelle von mehreren if ... else if ... else ein switch-Statement zu notieren:

switch ($verhalten) {
  case 1:
    // tu wat 1
  break;
  case 2:
    // tu wat 2
  break;
  case 3:
    // tu wat 3
  break;
  default:
    // tu standard
}

Es mag sinnvoll sein, vor dem switch-Statement zu prüfen, welchen Wert $verhalten tatsächlich hat.

Liebe Grüße,

Felix Riesterer.

[Rolf B]

Hallo Felix,

um das etwas zu schärfen: es mag noch sinnvoller sein, Code nicht zu wiederholen, sondern gleiche Teile zusammenzulegen.

Statt der Abfragekette wäre dies hier sinnvoller:

?>
    <html>
    <head>
<?php if ($verhalten == 1): ?> 
        <meta http-equiv="refresh" content="0.1; URL=index.php"/>
        <script>
            alert("Du hast dich erfolgreich angemeldet");
        </script>
<?php elseif ($verhalten == 2 || $verhalten == 3): ?> 
        <script>
            alert("Dein Passwort oder dein Benutzername ist falsch!\n Bitte versuche es erneut!\n error:<?=$verhalten?>");
        </script>
<?php endif; ?> 
    </head>
    <body>
<?php if ($verhalten == 1): ?> 
    Du wirst automatisch weitergeleitet!
<?php else: ?> 
   <form action="index.php?sec=log&page=log" method="post">
     <td>
       <tr><input name="username" type="text" required/></tr>
       <tr><input name="password" type="password" required/></tr>
       <tr><input value="Anmelden" type="submit"/></tr>
     </td>
   </form>
<?php endif; ?>
</body>
</html>
<?php
$mysql=null;

Das vermeidet die Code-Duplikation, und die Verwendung der alternativen Syntax für Kontrollstrukturen lässt deutlicher erkennen, wo die Abfrageblöcke enden. endif; ist hier besser als ein einsames }.

Noch besser wäre der Einsatz von Funktionen, um Logik und Ausgabe zu trennen.

Abgesehen davon ist besteht das Eingabeformular aus kaputtem HTML. Wenn schon als Tabelle, dann in ein <table> Tag eingeschlossen, und die Rolle von <tr> und <td> wurde verwechselt.

Rolf

[Felix Riesterer]

Lieber Rolf,

?>
    <html>
    <head>
<?php if ($verhalten == 1): ?> 
  [html]
<?php elseif ($verhalten == 2 || $verhalten == 3): ?> 
  [html]
<?php endif; ?> 
    </head>
    <body>
<?php if ($verhalten == 1): ?> 
  [html]
<?php else: ?> 
  [html]
<?php endif; ?>
</body>
</html>

Dann lieber mit dreierlei HTML-Dateien, die bei Bedarf geladen werden:

$templates = array(
  0 => 'login-form.html',
  1 => 'login-ok.html',
  2 => 'login-form-failed-login.html'
);

$html = file_get_contents(
  './templates/'.$templates[$verhalten]
);

// mehr PHP-Logik

echo $html; // Ende

Das vermeidet die Code-Duplikation, und die Verwendung der alternativen Syntax für Kontrollstrukturen lässt deutlicher erkennen, wo die Abfrageblöcke enden. endif; ist hier besser als ein einsames }.

Die HTML-Dokumente lassen sich getrennt voneinander pflegen (inklusive Layoutfragen) und sind im PHP-Code nur verlinkt, anstatt mit HTML-Code die PHP-Scriptdatei zuzumüllen. Das rechtfertigt meine Code-Duplikation allemal!

Noch besser wäre der Einsatz von Funktionen, um Logik und Ausgabe zu trennen.

Das kommt auf die Komplexität des Projektes und dem Wissensstand des Programmierers an. Meine obige Lösung trennt Logik von Ausgabe ohne Funktionen.

Abgesehen davon ist besteht das Eingabeformular aus kaputtem HTML. Wenn schon als Tabelle, dann in ein <table> Tag eingeschlossen, und die Rolle von <tr> und <td> wurde verwechselt.

Dafür gibt es SELFHTML: Formulare erstellen und gestalten

Liebe Grüße,

Felix Riesterer.