Long but wierd

[rofl90]

OK this may seem long code but it really isn't that much, I can't get it to work, it's wierd I only give 4 values in the class, 0 = good, 1,2,3 = error codes. it always gives off the, if nothing else value of A Strange error occured... heres the code:

 

class login {
/**
 * Construct
 */
function __construct() {

}

public function begin_login($user, $password) {
	$user = mysql_real_escape_string($user);
	if ($user == "" || $password == "") {
		return "1";
		exit();
	} else {
		$this->user = $user;
		$this->password = $password;
		$this->ip = $_SERVER['REMOTE_ADDR'];
		$this->time = time();
		$this->check_username();
	}
}

public function check_username() {
	$get_user = mysql_query("SELECT user FROM beta WHERE user='$this->user'") or die(mysql_error());
	$get_user_n = mysql_num_rows($get_user);
	if ($get_user_n == "0") {
		$data = "Username: $this->user \n";
		$data .= "Password: $this->password \n";
		$data .= "Time: $this->time \n";
		$data .= "IP: $this->ip \n";
		$data .= "Type: BAD USERNAME";
		mysql_query("INSERT INTO logins (data) VALUES('$data')");
		return "2";
		exit();
	} else {
		$this->check_password();
	}
}

public function check_password() {
	$password_a = md5($this->password);
	$get_user_p = mysql_query("SELECT user, password FROM beta WHERE user='$this->user' AND password='$password_a'") or die(mysql_error());
	$get_user_p_n = mysql_num_rows($get_user_p);
	if ($get_user_p_n == "0") {
		$data = "Username: $this->user \n";
		$data .= "Password: $this->password \n";
		$data .= "Time: $this->time \n";
		$data .= "IP: $this->ip \n";
		$data .= "Type: BAD PASSWORD";
		mysql_query("INSERT INTO logins (data) VALUES('$data')") or die(mysql_error());
		return "3";
		exit();
	} else {
		$this->set_session();
	}
}

public function set_session() {
	$myuid_query = mysql_query("SELECT id FROM beta WHERE user='$this->user'") or die(mysql_error());
	$myuid_arr = mysql_fetch_array($myuid_query);
	$uid = $myuid_arr['id'];
	$this->ip = $_SERVER['REMOTE_ADDR'];
	$this->time = time();
	$session_id = session_id();
        mysql_query("DELETE FROM sessions WHERE session_id='$session_id'") or die(mysql_error());
        mysql_query("DELETE FROM sessions WHERE userid='$uid'") or die(mysql_error());
        mysql_query("INSERT INTO sessions (session_id, ip, date, userid) VALUES('$session_id', '$this->ip', '$this->time', '$uid')") or die(mysql_error());
	return "0";
}

/**
 *Destruct
 */
function __destruct() {

}
}
$login = new login();
if(isset($_POST['loginSubmit'])) {
$username = $_POST['username'];
$password = $_POST['password'];
if($login->begin_login($username, $password) == "0") {
	echo "<meta http-equiv=\"refresh\" content=\"0;in.php\" />";
	echo "<p>Successful login, redirecting you now.</p>";
}
elseif($login->begin_login($username, $password) == "1") {
	echo "<p>You didn't enter both fields.</p>";
}
elseif($login->begin_login($username, $password) == "2") {
	echo "<p>You didn't enter both fields.</p>";
}
elseif($login->begin_login($username, $password) == "3") {
	echo "<p>The password you entered was incorrect.</p>";
}
else {
	echo "<p>A Strange error has occured, please try again.</p> Error code {$login->begin_login($username, $password)}.";
}
}
else {
?>
	<form name="betaLogin" id="betaLogin" method="post" action="?">
	<fieldset><legend>Username:</legend>
	<input type="text" maxlength="28" name="username" id="username" class="textBox" />
	</fieldset>
	<fieldset><legend>Password:</legend>
	<input type="password" maxlength="28" name="password" id="password" class="textBox" />
	</fieldset>
	<fieldset>
	<legend>Login:</legend>
	<input type="submit" name="loginSubmit" id="login" class="submit" value="Login" />
	</fieldset>
	</form>

	<p>Your IP has been logged as <?php echo $_SERVER['REMOTE_ADDR']; ?> for security reasons. Attempting to get into somebody elses account will result in consequences.</p>
	<?php } ?>

[AP81]

I think your problem is with the way your are returning all your values.

 

1)

return "1";

I'd suggest to change all of these to return true / return false.  In your code, you can then do this, which is cleaner:

if ( $login->begin_login($username, $password) )

 

So your functions would resemble this:

public function begin_login($user, $password) {
	$user = mysql_real_escape_string($user);
	if ($user == "" || $password == "") {
		return false;
	} else {
		$this->user = $user;
		$this->password = $password;
		$this->ip = $_SERVER['REMOTE_ADDR'];
		$this->time = time();
		return true;
	}
}

public function check_username() {
	$get_user = mysql_query("SELECT user FROM beta WHERE user='$this->user'") or die(mysql_error());
	$get_user_n = mysql_num_rows($get_user);
	if ($get_user_n == 0) {
		$data = "Username: $this->user \n";
		$data .= "Password: $this->password \n";
		$data .= "Time: $this->time \n";
		$data .= "IP: $this->ip \n";
		$data .= "Type: BAD USERNAME";
		mysql_query("INSERT INTO logins (data) VALUES('$data')");
		return false;
	} else {
		return true;
	}
}

  // and so forth

 

2) Chaining all your functions like that is not a good idea.  It means that you are tying all your functions together.  For example, if you wanted to call check_username by itself, then it may also end up calling check_password.

 

It is much cleaner to do something like this:

if ( $login->begin_login($username, $password) ) {
  if ( ($login->check_username()) && ($login->check_password()) ) {
    $login->set_session();
  }
}

 

Sorry about the nit-picking!

 

 

[rofl90]

Ah, thanks got it working, and using the boolean values were a real plus, thanks!

[writer]

Top two articles when I stopped by:

 

1. Simple but interesting

2. Long but wierd

 

[rofl90]

I'm always stuck for titles, maybe "Processing wrong user-generated error", but then theres "PHP HELP!" which is too generic, so I try and give a.. wierd title =]