Review OO login-script

Overzicht Reageren

Sponsored by: Vacatures door Monsterboard

SilverWolf NL

SilverWolf NL

22/01/2010 18:52:00
Quote Anchor link
Ik ben al een tijdje bezig met een (goed) login script, en dit is wat ik er tot nu toe van gemaakt heb. Het is 'mn eerste OO-project, dus wellicht zitten er nog wat foutjes in het "object-georiënteerd denken", maar dan hoor ik het graag!

Vragen zijn natuurlijk altijd welkom. De voornaamste vraag die ikzelf heb, is of dit een goed (en dus veilig) login script is.


De code:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
<?PHP
//Define (session)settings
session_set_cookie_params(1814400);
session_start();
ini_set('display_errors',1);
error_reporting(E_ALL);

//Tabelstructuur
$databaseSQL="CREATE TABLE IF NOT EXISTS tblUsers (
  User varchar(24) NOT NULL,
  Pass varchar(128) NOT NULL,
  Active int(1) DEFAULT NULL,
  Level int(1) DEFAULT NULL,
  PRIMARY KEY (User),
  UNIQUE KEY Us3r (User)
) ENGINE=MyISAM DEFAULT CHARSET=latin1;

INSERT INTO tblUsers (User, Pass, Active, Level) VALUES
('Administrator', '18e3b68aebdcd361b0ed9c82f9d76ffd7f7c344e5b986030223beb1e51e88329b823c237c43512ba5ede056431d339505608eef747717afd8defe2eda4a69f18', 1, 1);";
//Het wachtwoord is PHPhulp, als je dit wil veranderen moet je even de Hash class gebruiken om een wachtwoord te genereren.

/*
 * Database Class
 * @return object Database;
 * @functions return:
 *   array getUserData($username, $password, mixed $fields);
 *   bool uploadUser(object $UserObject);
 */
class DBO{
    private $MySQLi_HOST="127.0.0.1";
    private $MySQLi_USER="root";
    private $MySQLi_PASS="";
    private $MySQLi_DB="cms_db";
    //-------------------------------------------------------------------------------------------------------------------//
    private $infoArrayFields=array("username"=>"User","password"=>"Pass","active"=>"Active","level"=>"Level");
    private $infoArrayTables=array("users"=>"tblUsers","online"=>"tblOnline");
    private $dbCon;
    public function __construct(){
        $this->dbCon=new MySQLi($this->MySQLi_HOST, $this->MySQLi_USER, $this->MySQLi_PASS, $this->MySQLi_DB);
        if(mysqli_connect_error()){
            Log::Log("Connection error (" . mysqli_connect_errno() . ") " . mysqli_connect_error());
            return false;
        }else{
            return true;
        }
    }
    public function getUserData($username,$password,$data){
        if(!is_array($data)){
            $sql="SELECT " . $data . " FROM tblUsers WHERE User='" . $username . "' AND Pass='" . $password . "'";
            $result=$this->dbCon->query($sql);
            if(($this->dbCon->affected_rows===0)||(mysqli_error($this->dbCon)!==false)){
                return false;
            }else{
                $return=mysqli_fetch_assoc($result);
                return $return[$data];
            }
        }else{
            foreach($data as $userVar){
                $array[]=$this->infoArrayFields[$userVar];
            }w
            $array=implode(", ",$array);
            $sql="SELECT " . $array . " FROM tblUsers WHERE LOWER(User)='" . strtolower($username) . "' AND Pass='" . $password . "'";
            $result=$this->dbCon->query($sql);
            if(($this->dbCon->affected_rows===0) || mysqli_error($this->dbCon)){
                return false;
            }else{
                $databaseData=mysqli_fetch_assoc($result);
                foreach(array_keys($databaseData) as $search){
                    $key=array_search($search, $this->infoArrayFields);
                    $return[$key]=$databaseData[$search];
                }
                return $return;
            }
        }
    }
    public function uploadUser($userData){
        //
        foreach(array_keys($userData) as $key){
            $fields[]=$key;
        }
        foreach($userData as $value){
            if(is_int($value)){
                $values[]=$value;
            }else{
                $values[]="'".$value."'";
            }
        }
        $fields=implode(",",$fields);
        $values=implode(",",$values);
        unset($userData);
        $sql="INSERT INTO tblUsers(" . $fields . ") VALUES(" . $values . ")";
        $result=$this->dbCon->query($sql);
        if(($this->dbCon->affected_rows===0)||(mysqli_error($this->dbCon)!==false)){
               Log::Log("Connection error (" . $this->dbCon->errno . ") " . $this->dbCon->error);
            return false;
        }else{
            return true;
        }
    }
}
/*
 * User Class
 * @return object User;
 * @functions return:
 *   bool setUsername(string $username);
 *   bool setPassword(string $password);
 *   bool setLevel   (int $level      );
 *   bool setActive  (int $active     );
 *   string getUsername();
 *   string getPassword();
 *   int getActive();
 *   int getLevel();
 */
class User{
    private $username;
    private $password;
    private $level;
    private $active;
    
    public function setUsername($name){
        $this->username=$name;
    }
    public function setPassword($pass){
        $this->password=$pass;
    }
    public function setActive($a){
        $this->active=intval($a);
    }
    public function setLevel($l){
        $this->level=intval($l);
    }
    
    public function getUsername(){
        return $this->username;
    }
    public function getPassword(){
        return $this->password;
    }
    public function getActive(){
        return $this->active;
    }
    public function getLevel(){
        return $this->level;
    }
}
/*
 * UserHandler Class
 * @return object UserHandler;
 * @functions return:
 *   object getUserObject();
 *   bool login($username, $password [, $sessionName="userObject" [, array $data_to_get ]);
 *   bool logout([$sessionName="userObject"]);
 *   bool register($username, $password [, int $active [, int $level ]]);
 *   bool check([$sessionName="userObject"])
 */
class UserHandler{
    private $UserObject;
    private $DatabaseObject;
    private $EncryptionObject;
    private $SessionObject;
    public function __construct(){
        $this->DatabaseObject=new DBO;
        $this->SessionObject=new SessionHandler;
    }
    private function getUserData($username,$password,$fields=NULL){
        if(preg_match("/^[a-zA-Z][a-zA-Z0-9_]+[a-zA-Z0-9_]/",$username)===0){
            return false;
        }else{
            $password=Hash::HashString($password);
            if($fields==NULL){
                $fields=array("active","level");
            }
            $data=$this->DatabaseObject->getUserData($username,$password,$fields);
            if($data===false){
                return false;
            }else{
                $this->UserObject=new User;
                $this->UserObject->setUsername($username);
                $this->UserObject->setPassword($password);
                unset($username,$password);
                if($data===false){
                    return false;
                }else{
                    if(!is_array($fields)){
                        $fields="set" . ucfirst(strtolower($fields));
                        $this->UserObject->$fields($data);
                    }else{
                        foreach($data as $var=>$value){
                            $fields="set" . ucfirst(strtolower($var));
                            $this->UserObject->$fields($value);
                        }
                    }
                    unset($data,$var,$value);
                    return true;
                }
            }
        }
    }
    public function getUserObject(){
        return $this->UserObject;
    }
    public function login($username,$password,$sessionName=NULL,$UserVars=NULL){
        if(empty($sessionName)){
            $sessionName="userObject";
        }
        if($this->getUserData($username,$password,$UserVars)){
            unset($username,$password);
            $this->UserObject->setLevel($this->UserObject->getLevel());
            $this->SessionObject->setSession($this->UserObject,$sessionName);
            return true;
        }else{
            unset($username,$password);
            return false;
        }
    }
    public function logout($sessionName=NULL){
        if(empty($sessionName)){
            $sessionName="userObject";
        }
        $this->SessionObject->removeSession($sessionName);
        return true;
    }
    public function register($username,$password,$active=NULL,$level=NULL){
        if((preg_match("/^[a-zA-Z][a-zA-Z0-9_]+[a-zA-Z0-9_]/",$username)===0) || (strlen($username)>24)){
            return false;
        }else{
            $this->UserObject=new User;
            $this->UserObject->setUsername($username);
            $password=Hash::HashString($password);
            $this->UserObject->setPassword($password);
            $this->UserObject->setActive($active);
            if($level==NULL)$level=0;
            if($active=NULL)$active=0;
            $this->UserObject->setLevel($level);
            $data=array("username"=>$username, "password"=>$password, "active"=>$active, "level"=>$level);
            unset($username,$password,$level,$active);
            $succes=$this->DatabaseObject->uploadUser($data);
            if($succes){
                return true;
            }else{
                return false;
            }
        }
    }
    public function check($sessionName=NULL){
        if(empty($sessionName)){
            $sessionName="userObject";
        }
        
        if($this->SessionObject->instanceofClass($sessionName,"User")){
            $this->UserObject=$this->SessionObject->getSession($sessionName);
            return true;
        }else{
            return false;
        }
    }
}
/*
 * Log Class
 * @return object Log;
 */
class Log{
    private static $fileHandler;
    private static $logPath = 'errorLog.txt';

    public static function LogString($error){
        if(!isset(self::$fileHandler))
             self::$fileHandler=fopen(self::$logPath, "a");

        $error="[" . date("d-m-Y H:i:s") . "] -> '" . $error . "'\r\n";
        if(fwrite(self::$fileHandler,$error)){
            return true;
        }else{
            return false;
        }
    }
}
/*
 * Hash Class
 * @return object Hash;
 * @functions return:
 *   bool setKey($key);
 *   bool setInput($input);
 *   string getHash();
 */
class Hash{
    public static $_hash;
    private static $_key;
    private static $_hashMethod;
    public static function HashString($input,$method=NULL,$key=NULL){
        if($key==NULL&&empty(self::$_key))$key=trim(__FILE__,"./\\");
        if($method==NULL&&empty($_hashMethod)){
            switch(true){
                case (in_array("whirlpool",hash_algos())):
                    $method="whirlpool";
                break;
                case (in_array("sha512",hash_algos())):
                    $method="sha512";
                break;
                case (in_array("sha384",hash_algos())):
                    $style="sha384";
                break;
                case (in_array("sha256",hash_algos())):
                    $method="sha256";
                break;
                case (in_array("haval256,5",hash_algos())):
                    $method="haval256,5";
                break;
                default:
                    $method="md5";
                break;
            }
        }
        self::$_key=$key;
        self::$_hashMethod=$method;
        self::$_hash=hash_hmac(self::$_hashMethod,$input,self::$_key);
        return self::$_hash;
    }
    public static function getHash(){
        return self::$_hash;
    }
}

/*
 * SessionHandler Class
 * @return object SessionHandler;
 * @functions return:
 *   bool setSession(mixed $data, $sessionName [, time $time_to_live ]);
 *   bool removeSession($sessionName);
 *   mixed getSession($sessionName);
 *   bool instanceofClass($sessionName, $class);
 */
class SessionHandler{
    public function setSession($data,$sessionName){
        if(is_object($data)){
            $_SESSION[$sessionName]=base64_encode(serialize($data));
        }else{
            $SESSID=md5(time() . $_SERVER['REMOTE_ADDR'] . mt_rand());
            $_SESSION[$SESSID]=$data;
            setcookie($sessionName,$SESSID,0);
        }
        return true;
    }
    public function removeSession($sessionName){
        unset($_SESSION[$sessionName]);
        return true;
    }
    public function getSession($sessionName){
        if( (!empty($_SESSION[$sessionName])) && is_object(unserialize(base64_decode($_SESSION[$sessionName]))) ){
            return unserialize(base64_decode($_SESSION[$sessionName]));
        }elseif(!empty($_COOKIE[$sessionName])){
            $sessionName=$_COOKIE[$sessionName];
            return $_SESSION[$sessionName];
        }else{
            return false;
        }
    }
    public function instanceofClass($sessionName,$class){
        if((!empty($_SESSION[$sessionName]))){
            $temp=$_SESSION[$sessionName];
            $temp=unserialize(base64_decode($temp));
            if($temp instanceof $class){
                return true;
            }else{
                return false;
            }
        }else{
            return false;
        }
    }
}
?>
Gewijzigd op 01/01/1970 01:00:00 door SilverWolf NL
 
PHP hulp

PHP hulp

19/04/2024 17:03:37
 
- Ariën -
Beheerder

- Ariën -

22/01/2010 19:01:00
Quote Anchor link
Us3r varchar(24) NOT NULL,
P4ss varchar(128) NOT NULL,
4ct1v3 int(1) DEFAULT NULL,
L3v31 int(1) DEFAULT NULL,

Vanwaar die belachelijke namen?
 
---- ----

---- ----

22/01/2010 19:03:00
Quote Anchor link
User
Pass
Active
Level

Zo kan het ook gewoon
Maarja dat is zijn eigen keus.. xD
 

22/01/2010 19:04:00
Quote Anchor link
Waarvoor moet je database object weten hoe hij gegevens van een user ophaalt? Dat klopt niet.
(En in principe kan je de mysqli class extenden.)
Gewijzigd op 01/01/1970 01:00:00 door
 
SilverWolf NL

SilverWolf NL

22/01/2010 19:06:00
Quote Anchor link
Aar schreef op 22.01.2010 19:01:
Us3r varchar(24) NOT NULL,
P4ss varchar(128) NOT NULL,
4ct1v3 int(1) DEFAULT NULL,
L3v31 int(1) DEFAULT NULL,

Vanwaar die belachelijke namen?


Omdat je dan niet makkelijk kan raden wat de naam van een kolom is. Het voordeel hiervan is dat met SQL-injections, of iets in die vorm, moeilijker de data veranderd kan worden, omdat de tabelnamen moeilijker te raden zijn. Ik kan ze ook gewoon een standaard naam geven, mocht dat overzichtelijker zijn, maar ik heb het nu zo gedaan.
 

22/01/2010 19:11:00
Quote Anchor link
Edoxile schreef op 22.01.2010 19:06:
Aar schreef op 22.01.2010 19:01:
Us3r varchar(24) NOT NULL,
P4ss varchar(128) NOT NULL,
4ct1v3 int(1) DEFAULT NULL,
L3v31 int(1) DEFAULT NULL,

Vanwaar die belachelijke namen?


Omdat je dan niet makkelijk kan raden wat de naam van een kolom is. Het voordeel hiervan is dat met SQL-injections, of iets in die vorm, moeilijker de data veranderd kan worden, omdat de tabelnamen moeilijker te raden zijn. Ik kan ze ook gewoon een standaard naam geven, mocht dat overzichtelijker zijn, maar ik heb het nu zo gedaan.

Sommige sql injections kan gewoon achterhalen wat de tabel- en kolomnamen zijn. Dus zo'n rare naam heeft geen toegevoegde waarde.
 
Pim -

Pim -

22/01/2010 19:25:00
Quote Anchor link
Niet bij blinde injections, dan heeft het wel voordeel. Hoewel ik hier leesbaarheid toch belangrijker zou vinden. SQL Injections zijn makkelijk af te weren met prepared statements of escapes.
Maar waarom die session class, als je er niks bijzonders mee doet? Je kan er bijvoorbeeld een session hijacking preventie mee inbouwen oid.

EDIT: En nog wat, nu maak je bij elke log een nieuwe file verbinding. Je kan beter die verbinding static opslaan. Ook kan je die hele method beter static maken, het object zelf is toch leeg.
Gewijzigd op 01/01/1970 01:00:00 door Pim -
 
SilverWolf NL

SilverWolf NL

23/01/2010 00:17:00
Quote Anchor link
@Karl en Pim: dat wist ik niet, dan kan ik idd gewoon 'normale' namen doen.

@Pim, ik zal eens kijken naar die static method, dat is een goede tip

@Karl, dat databaseobject die de info van de gebruiker ophaalt valt me, nu je het zegt, ook op. Ik zal kijken wat ik eraan kan doen...
 
SilverWolf NL

SilverWolf NL

24/01/2010 16:46:00
Quote Anchor link
Ik heb geprobeerd het script aan te passen, zodat al het bovenstaande beter is. Zijn er nu nog tips of aanmerkingen? Dit zou mij heel erg helpen, dus b.v.d. ;)

PS: omdat ik niet 2x zo'n lange post wil neerzetten, heb ik de originele post aangepast.
 
Pim -

Pim -

24/01/2010 16:59:00
Quote Anchor link
Ik zou dit van je log class maken:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
<?php
class Log{
    private static $fileHandler;
    private static $logPath = 'errorLog.txt';

    public static function log($error){
        if(!isset(self::$fileHandler))
             self::$fileHandler=fopen(self::$logPath, "a");

        $error="[".date("d-m-Y H:i:s")."] -> '".$error."'\r\n";
        if(fwrite(self::$fileHandler,$error)){
            return true;
        }else{
            return false;
        }
    }
}
?>


En van die hashclass kan je toch ook gewoon hetzelfde doen?
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
<?php
class Hash
{
    private static $_defaultMethod = 'md5';
    private static $_defaultKey = 'asdfla;sduf08';

    public static function setMethod($method);
    public static function getMethod();

    public static function setKey($key);
    public static function getKey();

    public static hash($string, $key = null, $method = null);
}
?>
Gewijzigd op 01/01/1970 01:00:00 door Pim -
 
SilverWolf NL

SilverWolf NL

24/01/2010 21:52:00
Quote Anchor link
@Pim: jah, dankjewel. Ik had me nog niet echt verdiept in static methods en properties, maar dat komt bij deze twee inderdaad van pas. Ik heb het nu ongeveer zo aangepast, als jij had voorgesteld.

Verder, wil ik graag weten (dat was immers ook een vraag) in hoeverre dit veilig is. Ik moet nog iets verzinnen om de sessionid te koppelen aan iets, zodat deze niet gekopieerd kan worden. Nu is het volgens mij nog zo, dat als iemand de sessionid uit het koekje kopieert, dat je dan als iemand anders kan inloggen. Hoe kan ik dit het beste doen? Behalve een IP-adres link, wat door (bijna) iedereen wordt afgeraden, kan ik niets bedenken. Iemand ideeën?
 



Overzicht Reageren

 
 

Om de gebruiksvriendelijkheid van onze website en diensten te optimaliseren maken wij gebruik van cookies. Deze cookies gebruiken wij voor functionaliteiten, analytische gegevens en marketing doeleinden. U vindt meer informatie in onze privacy statement.