Eleganteste Lösung für Login-Verwaltung?

[DoubleJ2k]

Ich wollte mal ein bisschen mit Klassen arbeiten. Erstes Ziel: Ein Login-System

Hier erstmal meine "user"-Klasse (gekürzt):

class user
{
  function user(mysql $db_con,$uid = 0, $md5pw = '')
  {
    $this->db = $db_con;
    if ($uid > 0 && !empty($md5pw))
    {
      $this->db->sql_query("SELECT * FROM {$this->db->prefix}users
 WHERE user_id='$uid' AND passwd='$md5pw' LIMIT 1");
      if ($this->db->sql_num_rows() != 0)
      {
        $this->login_ok = true;
        $this->init();
      }
    }
  }

  function login($loginname, $passwd)
  {
    $this->db->sql_query("SELECT * FROM {$this->db->prefix}users 
WHERE login_name = '$loginname' AND passwd='".md5($passwd)."' LIMIT 1");
    if ($this->db->sql_num_rows() !=0)
    {
      $this->login_ok = true;
      $this->init();
      return true;
    }
    else
    {
      return false;
    }
  }  

  private function init()
  {
    /* profil, rechte etc. laden */
  } 
}

?> 

Hier der Code, wie er am Anfang von jedem Script ausgeführt wird (gekürzt):

if (!empty($_SESSION['user_id']) && !empty($_SESSION['md5_pass']))
{
  if (isset($_GET['user_logout']))
  { // logout
  }
  else
  {
    $user = new user($db, $_SESSION['user_id'], $_SESSION['md5_pass']);
  }
}
else
{
  $user = new user($db);
  if (!empty($_POST['login_user']) && !empty($_POST['login_pass']))
  {
    if ($user->login($_POST['login_user'], $_POST['login_pass']))
    {
      $_SESSION['user_id'] = $user->data['user_id'];
      $_SESSION['md5_pass'] = $user->data['passwd'];
      $m = new sysmsg('Login erfolgreich.');
    }
    else
    {
      $m = new sysmsg('Ungültiger Benutzername/falsches Passwort.');
    }
    $m->redirect(eregi_replace('user_logout([^&]*)','',$_SERVER['HTTP_REFERER']));
    $m->out();
  }
} 

Der Konstruktor der Klasse 'user' bekommt also nur Benutzerdaten, wenn bereits eine Session besteht. Beim Login-Vorgang wird dem Konstruktor nur die Datenbank-Klasse übergeben. Der eigentliche Vorgang wird dann über die Methode login() ausgeführt.

Das ich in dem momentan Zustand unmöglich eine Funktion für beide Aufgaben nutzen kann, ist klar - einmal wird der Login-Name sowie das unverschlüsselte Passwort übergeben, ansonsten die Benutzer-ID und das "verschlüsselte" Passwort.

Wäre es nun elegant, diese gewisse Redundanz so zu beseitigen, indem ich eine mySQL-Query für beide Fälle habe, d.h.

...WHERE (login_name='$user' AND passwd=md5($pw)) OR (user_id='$user' AND passwd='$pw')

So könnte ich dann den Konstruktor für beides verwenden. Ob der Login erfolgreich war kann ich ja dann mit $user->login ok überprüfen.

Was meint ihr dazu?

[Shurakai]

Wieso benutzt du nicht immer das unverschlüsselte PW und checkst es dann mit einer Query.. oder immer das verschlüsselte....

Außerdem würde ich mir ne Methode schreiben damit du nicht immer den ganzen Kram an den Anfang eines Scriptes schreiben musst... sondern immer eine Methode aufrufen und fertig.... Ist doch unnötige Redundanz...

[DoubleJ2k]

Original geschrieben von Shurakai
Wieso benutzt du nicht immer das unverschlüsselte PW und checkst es dann mit einer Query.. oder immer das verschlüsselte....

Wäre vielleicht eine Lösung, aber das Problem mit login-name vs. user-id bleibt. Ändert also grundsätzlich nichts...

Original geschrieben von Shurakai
Außerdem würde ich mir ne Methode schreiben damit du nicht immer den ganzen Kram an den Anfang eines Scriptes schreiben musst... sondern immer eine Methode aufrufen und fertig.... Ist doch unnötige Redundanz...

Naja so dumm bin ich ja nicht Der Code eben ist in einer Datei, die am Anfang jeder Seite mittels

<?php require 'include/prepend.inc.php'; ?>

eingebunden wird.

Ich frage mich halt nur:

Soll ich eine Methode schreiben, die funktioniert, egal ob sie user-id und md5 pw bekommt oder login-name und normales pw - oder soll ich es bei zwei verschiedenen Methoden belassen?

Soll ich den Konstruktor überhaupt dafür benutzen, oder soll ich ihn nur dazu benutzen, die mysql-Klasse zu übernehmen?

Momentan sehe ich es so eigentlich am sinnvollsten:
- Dem Konstruktor wird nur die mysql-Klasse übergeben
- Eine Methode/Funktion, die universell funktioniert

Wäre das eine gute Lösung?

[Shurakai]

Ich hatte damals in meiner SessionManagement-Class ne Methode gebaut der ich dann übergebe was ich suche. Z.B. Userid und dann wurde im switch nen case aufgerufen mit userid. dann wird natürlich auch gecheckt ob die values nen integer sind usw...

das passwort habe ich grundsätzlich als md5-hash übergeben

[prego]

Was spricht dagegen username und id in der loginmethode zu prüfen und nur einen md5() zurück zu geben. Diesen md5 speichert man mit Zeit in einer DB und fragt einfach mit einer Methode am Anfang jedes Scripts ab, ob die "session" noch gültig ist - sprich ob der mitgereiche md5 noch gültig ist.


PS:
Ich würde mich als User ziemlich unwohl auf einer Seite fühlen wo mein Passwort Ständig in der Session mit rumgeschleppt wird - und das im klartext.

[DoubleJ2k]

Original geschrieben von prego
Was spricht dagegen username und id in der loginmethode zu prüfen und nur einen md5() zurück zu geben. Diesen md5 speichert man mit Zeit in einer DB und fragt einfach mit einer Methode am Anfang jedes Scripts ab, ob die "session" noch gültig ist - sprich ob der mitgereiche md5 noch gültig ist.

Was soll das für ein md5-Wert sein? Du meinst einfach einen zufällig berechneten, oder? Aber warum soll ich die Gültigkeitsprüfung selbst machen, wenn das die PHP-Sessionverwaltung schon übernimmt...

Original geschrieben von prego
Ich würde mich als User ziemlich unwohl auf einer Seite fühlen wo mein Passwort Ständig in der Session mit rumgeschleppt wird - und das im klartext.

Im Klartext ja sowieso nicht... aber irgendwas muss ich ja in der Session speichern um den Login zu verifizieren. Und wenn ein "böser Hacker" jemandem die session-id "klaut" hat er ja so oder so Zugriff, egal was ich speicher...

[prego]

Sorry, das die Sessionverwaltung ja schon eine Sid hat, ist klar.

Speicher in der Tabelle die IP und den Browser des Users zusammen mit der SessionID und mach am Anfang jeder Seite den Abgleich. Somit kann er nur noch auf der Seite verweilen solange er den gleichen Browser benutzt (Sowieso wegen Cookie) und die gleiche IP hat.
Nachteil daran, nach einer Neueinwahl ist die Session nicht mehr gültig - aber vielleicht ist das ja beabsichtig.

Aber selbst wenn du das md5-PW in der Session oder in einem Cookie speicherst, ist der "böse Hacker" damit immernoch drin.

Die Frage ist eben, wie wahrscheinlich es ist, das jemand ein Cookie entwendet.

Sessions werden afaik serverseitig gespeichert, also kann da keiner ran. PHP nutzt zur Identifikation der Session auf nur die SID in einem Cookie.
Somit kannst du das ganze auch nur noch etwas sicherer machen.

//index.php
session_start();

include('user.class.php');
$user = new user();

if(!$user->is_valid()){
  //user rauswefen, oder ohne login weiterarbeiten
}else{
  //user ist eingeleogt
}



//user.class.php

class user {

  function is_valid(){
    $sql = "SELECT * FROM session_table WHERE sid='".$_SESSION['SID']."' 
    AND datum > DATE_SUB(now(), INTERVAL 10 MINUTE) 
    AND user_ip = '".$_SERVER['REMOTE_ADDR']."' 
    AND user_agent='".$_SERVER["HTTP_USER_AGENT"]."'";
    
    if(mysql_query($sql)){
      if(mysql_affected_rows() == 1){
        return true;        
      }else{
        //jeden Eintrag in der SessionTabelle mit der entsprechenden SID löschen
      }
    }else{
      //fehler in Query
    }
    return false;
  }

} 

Vergiss nicht, das hier ist nur ein gedanklicher Ansatz von mir - es muss also nicht zwingend die beste Lösung sein-

[Spliffster]

Mit dem speichern der IP kann man sich aber auch selbst ein Bein stellen, da man damit z.B. AOL-User aussperren würde. Zudem werden Leute die weniger legal im Service drin sind, nicht unbedingt mit ihrer eigenen IP surfen

[prego]

Da hast du recht - hatte ich ja auch schon erwähnt.
Ich denk mit Session zugehörig zum User sollte alles klar gehn - evtl sogar in der Usertabelle selbst speichern.

Mir ist auf die Schnelle nichts bekannt wie ich dir nen Cookie klauen könnte.

Und die Frage die natürlich offen ist: Wie machen es die Anderen?

[DoubleJ2k]

Die Session in der Datenbank zu speichern hört sich ja nicht schlecht an, aber:

Wenn ich die sid in der user-Tabelle speichere, kann sich jeder Benutzer nur von einem Rechner anmelden. Meldet er sich an einem zweiten an, wird die sid in der Datenbank überschrieben und am 1. Rechner wird er "rausgeworfen".

Nehme ich eine extra "session"-Tabelle, würde es dieses Problem nicht mehr geben, jedoch hätte ich ein anderes, viel größeres Problem: Mit der Zeit sammelt sich ja in der Tabelle auch Müll an. Nun, dazu könnte man einen garbage collector schreiben, der z.B. mit 1% Wahrscheinlichkeit ausgeführt wird. Was ist aber, wenn PHP eine sid vergibt, die schon in der Tabelle steht (auch wenn sie längst nicht mehr benutzt wird, aber vom gc eben noch nicht gelöscht wurde)?
Natürlich, die Chance ist extrem klein. Aber ich will das Script nicht schreiben und dabei schon das Wissen haben, das etwas irgendwann nicht funktionieren könnte.

Wenn dann müsste ich eine komplette, von PHP unabhängige, "Session-Engine" schreiben...

Die zusätzlichen Sicherheitsprüfungen könnte ich ja auch so noch einbauen, indem ich UserAent/IP einfach in $_SESSION speichere.

PHP macht ja alles für mich, und der Benutzer bekommt in seinem Cookie ja auch nur die sid übertragen. Die eigentlichen Session-Daten liegen ja in /tmp (standard-einstellung).

Also momentan sehe ich den Sinn nicht, die sid irgendwie in der Datenbank zu speichern. Aber ich lasse mich gerne belehren (aber wenn, dann will ich es auch verstehen ).

p.s.: Ich gehe natürlich immer davon aus, das ein Angreifer keinen Zugriff auf den Server hat. Hätte er ihn, bräuchte er auch keine sids mehr auszuspionieren