Keta Pan: Profillink richtig gemacht?

Hi!

Ich tüftle derzeit ein bisschen herum und übe mich in PHP. Nun versuche ich die $_GET Variable zu verstehen.

Ich habe eine Index-Seite, auf der, wenn man eingeloggt ist, steht: "Eingeloggt als {Username}(mit Link zur Profilseite).

Nun hab ich aber das Problem, dass ich den Profillink mithilfe von fetch aus der Datenbank hole, was aber erst direkt nach der if(passwort==passwort==datenbankpasswort)-Schleife passiert.

Das heißt, wenn ich bereits eingeloggt bin und diese Schleife nie ausgeführt wird steht da einfach nur:

http://seite.de/profile.php?id= (anstelle von z.B. id=1).

Hier der Code:

<!DOCTYPE HTML>

<?php

session_start();

$submyt = $_POST['subm1t'];

if(isset($submyt)) {

  $uzernm = strip_tags($_POST['usern4me']);

  include __DIR__ . 'res/functions.php';

  $fetch = $connect->prepare("SELECT * FROM usertablex WHERE usern=:uzernm");

  $fetch->execute(array(":uzernm"=>$uzernm));
  $row = $fetch->fetch(PDO::FETCH_ASSOC);
 
  $pwentered = md5(strip_tags($_POST['p4ssword']));
  $pwindbase = $row['passw'];
  $prfid = $row['id'];

  if($pwentered==$pwindbase) {

    $ipfetch = $_SERVER['REMOTE_ADDR'];
    $updateip = $connect->prepare("UPDATE usertablex SET ipaddr=:ipaddr WHERE usern=:uzernm");
    $updateip->execute(array(":uzernm"=>$uzernm, ":ipaddr"=>$ipfetch));
    $_SESSION['uname'] = $uzernm;
  } else {
    echo 'FEHLSCHLAG!';
  }
}
?>

<html>
      <head>
            <title>PHP Übungsseite</title>
            <link rel="stylesheet" type="text/css" href="css/main.css"/>
            <meta name="viewport" content="width=device-width,initial-scale=1.0,user-scalable=no">
      </head>

      <body>
        <div id="main-outer">
        <?php

            if(!$_SESSION['username']) {

        ?> 
            <h1>Herzlich Willkommen! !</h1>
            <hr>
            <div id="logonform">
            <form name="logon" action="index.php" method="POST">
                
                <h2>Bitte loggen Sie sich ein!</h2>
                <ul>
                  <li><label for="usern4me">Username:</label><br>
                  <input type="text" name="usern4me" maxlength="16"></li>
                  <li><label for="p4ssword">Password:</label><br>
                  <input type="password" name="p4ssword" maxlength="16"></li>
                  <li><input type="submit" name="subm1t" value="Log in!"></li>
                </ul>
            </form>
            <br>
            </div>
  
            <?php

            } else {
            
            ?>

          <div id="content">
            <!-- LOGOUT BAR -->

            <div id="memBar">
              <div id="logoutBar">
                <p class="logouttext">
            <?php
              echo 'Eingeloggt als <a href="http://www.seite.de/profile.php?proid='.$id.'">' .$_SESSION['username'].'</a> ! <a href="logout.php">Ausloggen?</a>';
              //var_dump($prfid);
            ?>
                </p>
              </div>
            </div>
          </div>
          
            <!-- END OF LOGOUT BAR -->

            <?php

            }


          ?>
        </div>
      </body>
</html> 
  1. Hallo Keta,

    ich habe deinen PHP Code etwas eingerückt, um ihn lesbarer zu machen.

    Auf Anhieb entdecke ich eine Menge Codefehler - aber vielleicht sind die ja auch beim Übertragen in den Post entstanden. So wie es da steht kann es selbst direkt nach Login nicht funktionieren.

    • Du weist $prfid zu, baust aber $id in den Link ein. $id ist undefiniert.
    • Du weist $_SESSION['uname'] zu, verwendest nachher aber $_SESSION['username']
    • Du verwendest SELECT *. Das tut man in Programmen nicht. Man gibt explizit jede Spalte an, die man haben will und schreibt nicht mehr hin, als man braucht. Allein schon der impliziten Dokumentation wegen. Es wird zwar oft und gerne falsch gemacht, das macht es aber nicht richtiger. SELECT * ist für interaktive Systeme wie phpmyadmin da.
    • Dein Code behandelt zwei Situationen: "Abgemeldet" und "Anmeldung submitted". Die dritte Situation ist "Weiter angemeldet" und das behandelst Du nicht. Du erkennst es daran, dass $_SESSION['username'] bereits gesetzt ist. Du KÖNNTEST auch noch prüfen, ob die Request-Methode (in $_SERVER['REQUEST_METHOD']) GET oder POST ist. Du solltest übrigens beim Login auch noch die userid in der Session speichern, dann brauchst Du nicht für jeden Aufruf in usertablex nachschauen, welche ID dieser User hat.
    • Dein Code behandelt eine Situation NICHT: Unbekannter Username eingegeben. In diesem Fall liefert execute dir FALSE, bzw. $row ist null. Hier musst Du aus der Verarbeitung aussteigen, es führt zu Fehlermeldungen und undefiniertem Verhalten, wenn Du einfach weitermachst.

    Kein Fehler, aber aus meiner Sicht kein guter Stil:

    • Warum gibst Du den Logout-Link als echo aus? Eigentlich müssen nur $id und $_SESSION['username'] eingebaut werden, die kannst Du mit <?= ... ?> am gewünschten Ort einsetzen.
    • Du verwendest 1337speek in deinen Feld- und Variablennamen. Das ist uncool. Nenne sie richtig - es gibt tatsächlich Komponenten, die mit den Standardnamen besser funktionieren (z.B. Assistenzsysteme für Menschen mit Behinderung). Und wenn Du irgendwann mal deinen Code nach Zugriffen auf username durchsuchst, wirst Du Dich ärgern, uzern nicht gefunden zu haben.

    Optimierungstipps:

    • Die Variable submytbrauchstDunicht,dukannstisset(submyt brauchst Du nicht, du kannst isset(_POST['subm1t']) abfragen. Hier merkt man übrigens klar den Nachteil von 1337 Variablen: Du musst Hirnschmalz investieren, um jederzeit im Kopf zu haben, wo in der Submit - subm1t - submyt Kette Du bist. Hirnschmalz ist beim Programmieren ein knappes Gut, spar ihn Dir für wichtige Dinge auf.
    • <div id='outer-main'> - wozu brauchst Du das? Es ist das einzige Element vom body. Kannst Du die CSS Eigenschaften von #outer-main nicht direkt auf body anwenden?
    • <div id='content'> könnte ein <main> Element sein
    • Das <hr> unter dem <h1>Herzlich Willkommen</h1> könnte möglicherweise durch padding und border-bottom am h1 Element ersetzt werden.
    • <div id='logonform'> könnte vermutlich weg. Die id kann ans form selbst wandern, und der <br> hinter dem </form> vermutlich durch eine padding oder margin Eigenschaft ersetzt werden.
    • Das gerade angesprochene <hr> und <br> sind "Schmuck-Markup". Man legt damit das Layout fest. So etwas gehört aber ins CSS.

    Diese Tipps sind möglicherweise unpassend, weil Du Rahmenbedingungen hast, die hier nicht erkennbar sind und die deine Implementierung nötig machen. In dem Fall ignoriere sie einfach.

    Rolf

    --
    sumpsi - posui - clusi
    1. Hi Rolf, danke erstmal für deine Antwort. Das mit dem $id liegt daran, dass ich wohl vergessen hab, einige Stellen beim posten umzuändern. Dachte es wäre wohl schlau nicht alle Variablen so zu zeigen, wie sie in meinem Quelltext vorkommen.

      Ich bin dir sehr dankbar für deine Vorschläge/Tipps/Hinweise!

      Ich beschäftige mich mit HTML/Javascript/PHP mittlerweile seit 6 Jahren und ich denke ich sollte langsam einsehen, dass ich mir wohl eher ein anderes Hobby suchen sollte, da ich anscheinend noch immer nix gebacken krieg.

      Ich versuche zu lernen, doch es will nicht so ganz werden.

      Ich komm auf solche Dinge wie <section> oder <main> noch immer nicht klar, was vermutlich daran liegt, dass ich mir das mithilfe von YT-Videos beigebracht habe, in denen alles mittels <div> gegliedert wird.

      Würde es echt nett finden, wenn du mir nen Link zu nem Tutorial diesbezüglich geben könntest.

      Danke schonmal,

      KP

      1. Hallo Keta,

        ich wär da auch nicht in aller Pracht drauf gekommen, aber die Teilnahme an diesem Forum hier ist ziemlich erhellend. Manchmal guckt man direkt in die Sonne. Was ich oben geschrieben habe, ist eher eine kleine Funzel.

        Das hier kennst Du? Semantische Tags wie main/section/aside findest Du unter Grundsätzliches, Semantik Rolf

        --
        sumpsi - posui - clusi
  2. Hallo Keta,

    ein paar Anmerkungen zu deinem Code:

      $uzernm = strip_tags($_POST['usern4me']);
    

    strip_tags brauchst du hier nicht bzw. es wandelt u.U. den Nutzernamen um. Im Folgenden $fetch->execute kümmert sich das execute um das passende Escaping in den SQL-Kontext. Außerdem reichen einfache Quotes für Strings, solange du keine Variablen expandieren möchtest.

      $fetch = $connect->prepare("SELECT * FROM usertablex WHERE usern=:uzernm");
    
      $fetch->execute(array(":uzernm"=>$uzernm));
      $row = $fetch->fetch(PDO::FETCH_ASSOC);
     
      $pwentered = md5(strip_tags($_POST['p4ssword']));
    

    MD5 ist kein sicherer Passwort-Hashing-Algorithmus! Nimm besser password_hash.

    if($pwentered==$pwindbase) {
        // …
    } else {
        echo 'FEHLSCHLAG!';
    }
    

    Da du die IP-Adresse speicherst, kannst du an dieser Stelle auch eine Log der Anmeldeversuche erstellen und bei zu vielen fehlgeschlagen Versuchen eine temporäre Sperre einrichten.

    <html>
          <head>
                <title>PHP Übungsseite</title>
    

    Oha, ein Umlaut ohne Angabe des charsets.

                <meta name="viewport" content="width=device-width,initial-scale=1.0,user-scalable=no">
    

    Hat es einen Grund, warum du das Scrollen unterbindest?

    if(!$_SESSION['username']) {
    

    Ich weiß jetzt nicht aus dem Kopf, ob das einen Fehler erzeugt, wenn der Key username nicht existiert. Das Prüfen mit array_key_exists oder isset funktioniert auf jeden Fall fehlerfrei.

    echo 'Eingeloggt als <a href="http://www.seite.de/profile.php?proid='.$id.'">' .$_SESSION['username'].'</a> ! <a href="logout.php">Ausloggen?</a>';
    

    An dieser Stelle wird der Kontextwechsel nach HTML nicht berücksichtigt.

    Viele Grüße
    Robert