Look over Login Function?

[carsonk]

I apologize if this topic is in the wrong forum. I created a login class, but I don't believe it is as efficient as it could be. So, if a couple people could tell me if what I did was best or if there is a more efficient way to do it.

 

I'm just including the login class, but if you need any more info, just ask and I'll see if I can get it for you.

 

class login {
var $db;

var $username;
var $u_id;

var $input_password;
var $md5_password;

var $remember = FALSE;
var $rem_days = 0;

var $banned = FALSE;
var $ban_time = 0;
var $ban_reason;

public function __construct($db, $username, $password, $remember, $rem_days) {
	$this->db = $db;
	$this->username = $username;
	$this->password = $password;
	$this->remember = $remember;
	$this->rem_days = $rem_days;
}

//..................
//PRIVATE FUNCTIONS
//..................

private function get_uid() {
	$query = "SELECT * FROM users WHERE username = '".$this->username."'";
	$result = $this->db->query($query);
	$row = $result->fetch_array(MYSQLI_ASSOC);
	$id = $row["id"];

	$this->u_id = $id;
	return $id;
}

private function convert_pass() {
	$this->md5_password = md5($this->input_password);

	return $this->md5_passport;
}

//if the function returns 1, user is banned
private function check_ban() {
	$query = "SELECT * from bans WHERE user_id = ".$this->convert_pass()." AND end_time < ".time()." ORDER BY 'end_time' DESC";
	$result = $this->$db->query($query);

	if ($result->num_rows >= 1) {
		$this->banned = TRUE;

		$row = $result->fetch_array(MYSQLI_ASSOC);

		$this->banned = TRUE;
		$this->ban_time = $row["end_time"];
		$this->ban_reason = $row["reason"];

		return TRUE;
	} else {
		return FALSE;
	}
}


//..................
//PUBLIC FUNCTIONS
//..................


public function check_login() {
	$this->convert_pass();
	$this->get_uid();

	$query = "SELECT * FROM users WHERE id = '".$this->u_id."' AND password = '".$this->md5_password."'";
	$result =  $this->db->query($u_query);

	if ($result->num_rows == "1") {
		if (!$this->check_ban()) {
			return TRUE;
		} else {
			echo "BANNED!";
			return FALSE;
		}
	} else if($result->num_rows > 1) {
		echo "Uh-oh! It looks like there are two accounts with the same username! Please send an email to an admin immediately to correct this problem.";
		return FALSE;
	} else {
		return FALSE;
	}
}

public function set_session() {
	session_start();
	$_SESSION["username"] = $this->username;
	$_SESSION["u_id"] = $this->u_id;
	$_SESSION["unique"] = md5($this->md5_password);

	if($remember) {
		setcookie("remember", "", time()+(3600*24)*($this->rem_days));
	}
}

public function fail_reason() {

}


//NOTE: WHEN LOGGING OUT, COOKIES MUST BE DELETED BEFORE ANY OUTPUT IS MADE (so, place before header.php or anything with an echo function)
public function logout() {
	session_destroy;

	if($_COOKIE["rem"]) {
		setcookie("remember", "", time()-3600);
	}
}
}

[WolfRage]

  The only replies that I have is that you have to make sure that you are filtering those inputs especially because they are being directly entered into your database. Which could create a easy situation in which some one could SQL inject your database.

  Also I think you should do a session destroy on your logout in order to destroy the session cookie that is associated with the user; other wise it will appear as if the user is still logged even though they are not susposed to be.

  Good luck.

[Hussam]

Looks good but I think this should be divided to many class and inherit from each other, if it was me, I will divide it into four classes.

 

Database class

has connection, query, fetch, check query, ..etc

 

Database_object class

has the functions to build an object from records came from the database, not just user, it could be post, comment, cart, category, page, ..etc.

 

User class

has the functionality of getting users, ...etc depending on the first two classes and providing the table name to make it dynamic.

also handles some functions of logging in and out and stuff, may be depending a little bit on the session.

 

Session class

has all kinds of checks on sessions and so on.

 

Good luck!