5

I've started to improve my OOP skills, by solving some rather easier issues, when building a website. So it started with a login system, i have followed a tutorial on youtube, that helped me made a Login class, but as it went on, it raised many doubts (btw. The code is 100 lines so i'll pass on pasting it).

So in this Login class, there are verification methods etc. but it comes to a point where there's session verification, that with given before parameteres in construct, it can't be used (atleast in this class scope):

    $this->_username = ($this->_login)? $this->filter($_POST['username']) : $_SESSION['username'];
    $this->_password = ($this->_login)? $this->filter($_POST['password']) : '';
    $this->_passmd5 = ($this->_login)? md5($this->_password) : $_SESSION['password'];

So in that case i cannot use the verifySession() method, when there are no session variables set (to distinct what logged user should see on main page for example).

So my question is - is that design correct, and how should i build rest of the login system: loggedIn verification on every page and logging out - should each one of them be in seperate class (and what about methods, that repeat in particular class, should i always inherit them). I realise that there are different approaches to OOP, but is there any particular, that i should follow as a beginner (that will help me understand oop to the best of it).

Malyo
  • 1,900
  • 6
  • 28
  • 51
  • I don't quite understand what your asking here (can you maybe clarify that?), but in general: it is usually a good idea to follow the original concept and model (real-world) things with your classes. A "Login" is not a thing, but a Session and a User are. – Niko Jun 27 '12 at 22:10
  • I'm looking for a ,,pattern'' to build an object oriented login system (loging in, logging out, veryfing if user is logged on every page), and i'm not sure if all of these things should be inside single class – Malyo Jun 27 '12 at 22:17

3 Answers3

4

Im not sure you really want a "login" class. You might do somehting like this

class User
{
   private $username;
   private $password;

   public function __construct($username)
   {
     //load this user object here
   }

   private function hashPassword($password)
   {
      ///Dont do this has the hash, but im just keeping it simple
      return md5($password . 'a}{!@#' . $this->username);

   }

   public function authenticate ($password)
   {
      return $this->hashPassword($password) == $this->password;
   }

}

login.php

$user = new User($_POST['username']);
if($user->authenticate($_POST['password']))
{
 //do session initilization here (can be a class, or whatever)
 Session::createUserSession($user)
}
else
 echo 'bad login';

logout.php

Session::destroyUserSession();

This design is probably not the best way, but may give you an idea.

Kris
  • 5,923
  • 2
  • 26
  • 44
  • Ye that's why i'm struggling, there are many ways to solve this, but i can't decide which way to go. – Malyo Jun 28 '12 at 11:08
3

"i have followed a tutorial on youtube" is the first problem. Just the three lines of code you pasted reveals that the video you watched was created by an amateur PHP developer.

"So my question is - is that design correct, and how should i build rest of the login system"

Since only three lines were posted, I don't know if the design is correct or not. I'd wager that isn't though.

A good rule of thumb for when to use an object is when the code is clearly advanced enough to require more than two functions, will be used in multiple locations in the application, and there will be significant potential for naming conflicts if the code isn't encapsulated in an object or namespace. This doesn't meet that criteria - a login and registration screen are one page each (at most) and therefore that code will be used one time. There is no reason to encapsulate that logic. Niko mentioned Session and User classes, which are common patterns and those are okay to encapsulate.

As to building a login system, they get complicated fast and are almost a specialization these days. I recommend reading the following SO wiki article:

The definitive guide to form-based website authentication

If you want a prepackaged system that follows the above wiki article, you can look at:

http://barebonescms.com/documentation/sso/

The SSO client doesn't present classes to the application - just a set of functions such as SSO_LoggedIn(), SSO_Login(), SSO_Logout(), etc. Doing OOP just for the sake of doing OOP when something simpler will suffice is the wrong way to write software. Sometimes functions are better. Sometimes inlining the code is better. It really depends and it takes years of experience to have the insight as to what will be the best approach.

Community
  • 1
  • 1
CubicleSoft
  • 1,614
  • 14
  • 17
  • thanks that's really helpful, and sorry for the question being not precise enough, it would require lots of code to make it precise enough. – Malyo Jun 28 '12 at 21:12
0

$_SESSION['password']

It should never be necessary to store a password (plain text?) in a session. You should check if a user is allowed to login and if he does and gives the correct password, you store no more in the session than "is logged in". For convenience you could store a username of user-id as well.

But a password is not needed after you compared it to the one in the database.

Ivo P
  • 1,682
  • 1
  • 4
  • 14