Login so korrekt?

[tomahawk]

Hallo, ich versuche mich gerade an einem Login Skript. Das funktioniert auch (bis auf die logout.php) und ich wollte jetzt einmal wissen, was verbessert werden kann oder ich falsch gemacht habe:

login.php

<form role="form" method="post" class="form-signin">
    <h2 class="form-signin-heading">Login</h2>
    <div class="form-group">
        <label for="username" class="sr-only">Username</label>
        <input class="form-control" type="text" id="username" name="username" placeholder="Username" required autofocus>
    </div>
    <div class="form-group">
        <label for="password"class="sr-only">Password</label>
        <input class="form-control" type="password" id="password" name="password" placeholder="Password" required>
    </div>
    <div class="form-group">
        <button class="btn btn-lg btn-primary btn-block" name="login"><span class="glyphicon glyphicon glyphicon-log-in"></span></button>
    </div>
</form>

<?php
session_start();

include('../include/config.php');
include('include/header.php');

if (isset($_POST['login'])) {
    try {
        $stmt = $dbh->prepare("SELECT username, password FROM users WHERE username = :username AND password = :password");
        $stmt->bindParam(':username', htmlspecialchars($_POST['username']));
        $stmt->bindParam(':password', htmlspecialchars(md5($_POST['password'])));
        $stmt->execute();

        $user_id = $stmt->fetchColumn();
        if ($user_id == true) {
            $_SESSION['user_id'] = $user_id;
            header("Location: index.php");
        }
    } catch (PDOException $e) {
        $e->getMessage();
    }
}


if (isset($_SESSION['user_id'])) {
    header("Location: index.php");
}

include('include/footer.php');

index.php

<?php
session_start();

include '../include/config.php';
include 'include/header.php';

if (!isset($_SESSION['user_id'])) {
    header("Location: login.php");
}
?>

<h2>Herzlich Willkommen <a class="btn btn-info" href="logout.php" name="logout"><span class="glyphicon glyphicon glyphicon-log-out"></span></a></p></h2>



<?php
include 'include/footer.php';

Auf versteckten Seiten würde ich immer

if (!isset($_SESSION['user_id'])) {
    header("Location: login.php");
} 

einbauen. Oder kann man hier mit funktionen arbeiten?

[hausl]

Vorweg grundsätziches:

- error reporting aufdrehen

error_reporting(-1);
ini_set('display_errors', 1); 

- EVA Prinzip verinnerlichen und anwenden. http://php-de.github.io/jumpto/eva-prinzip/
Die Ausgabe von HTML gehört zum letzten Schritt ("Ausgabe") Sämtliche Verarbeitungslogik sollte/muss davor kommen, speziell header relevante Dinge wie zB Session etc.. Sonst gibt das "Headers already sent..." Fehler on mass. Du mischt derzeit Ausgabe und Verarbeitung gröbstens.

- zu md5 noch (ist "veraltet"): http://php.net/manual/de/faq.passwords.php

- zu SELECT * http://php-de.github.io/jumpto/code-smells/#select-

LG

[rkr]

Man erstellt schon länger keine einzelnen PHP-Dateien mehr, die jeweils direkt angesprungen werden können. Heute läuft alles über Bootstrap Dateien.

Heisst, egal wie die URL aussieht, jeder Request landet immer bei der index.php. Die macht dann alles, was du bei jedem Script machen müsstest. Bei dir ist das noch überschaubar: Session starten und Logintest (ausser für die Zielseite braucht man keinen Login. Die index.php ruft dann auf Basis der eigentlichen Anfrage die zuständige php-datei auf.

Schau dir dazu mal silex, torophp, slim oder fastroute an.

[tomahawk]

Danke euch beiden.
@ rkr: Das ist doch das was ich dann aber mit einem Framework mache oder?
Ich hätte mich halt zuerst so an PHP versucht, da ich mit Frameworks (CodeIgniter, CakePHP, Yii) bisher nicht so klar gekommen bin.
Bisher habe ich einfach blind alles abgetippt und hatte dann einen Blog oder ähnliches. Das hat aber mit lernen recht wenig zu tun gehabt

[der_robert]

Ersetze md5 gegen SHAX ... md5 gilt schon seit längerem nicht mehr als sicher.

[hausl]

Ersetze md5 gegen SHAX ... md5 gilt schon seit längerem nicht mehr als sicher.

"Vorsicht": http://php.net/manual/de/faq.passwor...words.fasthash

Hashalgorithmen wie MD5, SHA1 und SHA256 sind auf Geschwindigkeit und Effizienz optimiert. Mit modernen Techniken und leistungsstarker Hardware ist es aber trivial geworden diese Hashalgorithmen mit "brute force" Attacken anzugreifen.

LG

[tomahawk]

Kann man das ansonsten so lassen?

[hausl]

Was heißt "ansonsten"? In Bezug auf was? Wie sieht die aktuelle Verison aus? Welche Tipps/Dinge von oben hast du schon umgesetzt?

[tomahawk]

md5 habe ich noch nicht geändert, da lese ich mich gerade ein. Ich probieren mich ebenfalls gerade am composer damit ich in Zukunft einfacher Frameworks installieren kann. Das EVA Prinzip versuche ich umzusetzen. Die aktuelle Version ist oben. Ich editiere immer den ersten Beitrag.

[hausl]

Ich probieren mich ebenfalls gerade am composer

http://php-de.github.io/jumpto/composer/

Falls du noch nicht kennst...

Die aktuelle Version ist oben. Ich editiere immer den ersten Beitrag.

Da ist immer noch viel HTML vor des session_start() das geht auf jeden Fall noch besser...

Und an der Stelle ist das htmlsepcialchars auch falsch:

$stmt->bindParam(':username', htmlspecialchars($_POST['username']));
$stmt->bindParam(':password', htmlspecialchars(md5($_POST['password']))); 

Schau dir dazu mal das an.. bei htmlspecialchars() geht es im Grunde um den Kontext der Ausgabe.

http://php-de.github.io/jumpto/kontextwechsel/

LG

[tomahawk]

Danke schon mal. ich habe silex mit dem composer installieren können und werde mich da mal einlesen. Mal schauen ob ich das damit so hinbekommen, dass ich meinen Code "wegwerfen" kann

[tomahawk]

Gerade angefangen zu lesen, da blicke ich noch überhaupt nicht durch. Ein Hello World habe ich mehr, mehr nicht. Verstehe nich wo cih da meine Abfragen einbaue etc.