Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.
Popular code excerpts may be moved to "Code Snippets" by the moderators.
Moderator: General Moderators
staar2
Forum Commoner
Posts: 83 Joined: Fri Apr 06, 2007 2:57 am
Post
by staar2 » Sun Oct 19, 2008 6:50 am
I tried to make login system so here it is, try it and help to improve it.
Sql database table
Code: Select all
CREATE TABLE `andmed`.`login` (
`id` INT NOT NULL AUTO_INCREMENT ,
`username` VARCHAR( 62 ) NOT NULL ,
`password` VARCHAR( 33 ) NOT NULL ,
`time` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ,
PRIMARY KEY ( `id` )
) ENGINE = InnoDB
Add at page top, what you want protect.
Code: Select all
require_once('Login.php');
$login = Login::getInstance();
if ($login->isLogged()) {
echo 'You are in';
}
Login.php
Code: Select all
<?php
session_start();
require_once('config.php');
class Login {
private $sql;
private static $instance = null;
protected function __construct() {
$this->sql = new mysqli(Config::HOST, Config::USER, Config::PASS, Config::DB);
if (mysqli_connect_error()) {
exit('Error with db connection');
}
}
public static function getInstance() {
if (self::$instance == null) {
$c = __CLASS__;
self::$instance = new $c;
}
return self::$instance;
}
public function isLogged() {
if (isset($_SESSION['user']) && isset($_SESSION['pass'])) {
$stmt = $this->sql->prepare("SELECT `id` , `username` , `password`
FROM `login`
WHERE (
`username` = ?
AND
`password` = ?)");
$stmt->bind_param('ss', $_SESSION['user'], $_SESSION['pass']);
$stmt->execute();
$stmt->store_result();
return ($stmt->num_rows == 1) ? true : false;
} else {
return false;
}
}
public function login($username, $password) {
if (strlen($username) > 3 && strlen($password) > 3) {
$stmt = $this->sql->prepare("SELECT `id` , `username` , `password`
FROM `login`
WHERE (
`username` = ?
AND
`password` = ?)");
$stmt->bind_param('ss', $username, md5($password + Config::SALT));
$stmt->execute();
$stmt->store_result();
if ($stmt->num_rows == 1) {
$_SESSION['user'] = $username;
$_SESSION['pass'] = md5($password + Config::SALT);
return true;
} else {
return false;
}
} else {
return false;
}
}
public function logout() {
unset($_SESSION['user']);
unset($_SESSION['pass']);
session_destroy();
}
public function __destruct() {
$this->sql->close();
}
}
?>
config.php
Code: Select all
<?php
class Config {
const HOST = 'localhost';
const USER = 'root';
const PASS = '';
const DB = 'andmed';
const SALT = 'ThisWillBeSalt';
}
?>
testlog.php
Code: Select all
<?php
require_once('Login.php');
$login = Login::getInstance();
if (isset($_POST['login'])) {
if ($login->login($_POST['user'], $_POST['pass'])) {
header('Location: mysqli_prep.php');
} else {
echo '<p>Problem with login</p>';
}
}
?>
Login test
<form action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post">
<p>User: <input type="text" name="user" value="" /></p>
<p>Pass: <input type="password" name="pass" value="" /></p>
<p> <input type="submit" name="login" value="Log in" /></p>
</form>
phpserver
Forum Newbie
Posts: 22 Joined: Mon Oct 20, 2008 2:59 am
Location: Eastleigh,Nairobi
Post
by phpserver » Wed Oct 22, 2008 2:43 am
staar2 wrote: I tried to make login system so here it is, try it and help to improve it.
Sql database table
Code: Select all
CREATE TABLE `andmed`.`login` (
`id` INT NOT NULL AUTO_INCREMENT ,
`username` VARCHAR( 62 ) NOT NULL ,
`password` VARCHAR( 33 ) NOT NULL ,
`time` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ,
PRIMARY KEY ( `id` )
) ENGINE = InnoDB
Add at page top, what you want protect.
Code: Select all
require_once('Login.php');
$login = Login::getInstance();
if ($login->isLogged()) {
echo 'You are in';
}
Login.php
Code: Select all
<?php
session_start();
require_once('config.php');
class Login {
private $sql;
private static $instance = null;
protected function __construct() {
$this->sql = new mysqli(Config::HOST, Config::USER, Config::PASS, Config::DB);
if (mysqli_connect_error()) {
exit('Error with db connection');
}
}
public static function getInstance() {
if (self::$instance == null) {
$c = __CLASS__;
self::$instance = new $c;
}
return self::$instance;
}
public function isLogged() {
if (isset($_SESSION['user']) && isset($_SESSION['pass'])) {
$stmt = $this->sql->prepare("SELECT `id` , `username` , `password`
FROM `login`
WHERE (
`username` = ?
AND
`password` = ?)");
$stmt->bind_param('ss', $_SESSION['user'], $_SESSION['pass']);
$stmt->execute();
$stmt->store_result();
return ($stmt->num_rows == 1) ? true : false;
} else {
return false;
}
}
public function login($username, $password) {
if (strlen($username) > 3 && strlen($password) > 3) {
$stmt = $this->sql->prepare("SELECT `id` , `username` , `password`
FROM `login`
WHERE (
`username` = ?
AND
`password` = ?)");
$stmt->bind_param('ss', $username, md5($password + Config::SALT));
$stmt->execute();
$stmt->store_result();
if ($stmt->num_rows == 1) {
$_SESSION['user'] = $username;
$_SESSION['pass'] = md5($password + Config::SALT);
return true;
} else {
return false;
}
} else {
return false;
}
}
public function logout() {
unset($_SESSION['user']);
unset($_SESSION['pass']);
session_destroy();
}
public function __destruct() {
$this->sql->close();
}
}
?>
config.php
Code: Select all
<?php
class Config {
const HOST = 'localhost';
const USER = 'root';
const PASS = '';
const DB = 'andmed';
const SALT = 'ThisWillBeSalt';
}
?>
testlog.php
Code: Select all
<?php
require_once('Login.php');
$login = Login::getInstance();
if (isset($_POST['login'])) {
if ($login->login($_POST['user'], $_POST['pass'])) {
header('Location: mysqli_prep.php');
} else {
echo '<p>Problem with login</p>';
}
}
?>
Login test
<form action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post">
<p>User: <input type="text" name="user" value="" /></p>
<p>Pass: <input type="password" name="pass" value="" /></p>
<p> <input type="submit" name="login" value="Log in" /></p>
</form>
I will attempt to store sessions in a a mysql database and get back to you.
josh
DevNet Master
Posts: 4872 Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida
Post
by josh » Sun Oct 26, 2008 12:45 am
staar2 wrote: lets try and modify
no thanks?
staar2
Forum Commoner
Posts: 83 Joined: Fri Apr 06, 2007 2:57 am
Post
by staar2 » Sun Oct 26, 2008 10:17 am
and why not ?
josh
DevNet Master
Posts: 4872 Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida
Post
by josh » Sun Oct 26, 2008 9:51 pm
I'll do it if you finish my website for me, only about 70,000 lines of code need to be written
All sarcasm aside you didn't say what you wanted to modify. This is a coding critique forum anyways, not the volunteer work forum.
josh
DevNet Master
Posts: 4872 Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida
Post
by josh » Mon Oct 27, 2008 6:17 am
I see you edited your post, suggest ways to help to improve it I will.
granted the code presumably "works", the view logic is intertwined with the business logic, by that I mean its not MVC and that is one way it could be improved. Secondly your persistence layer is intertwined with the model ( the $login object's class ). Ideally your login object would not access the data source directly like that, and would allow different login adapters to be used to login from different sources. That may or may not be overkill for your design but those are 2 ways to improve the design, and where I would start.