PDA

Click to See Complete Forum and Search --> : Authentication Class.FeedBack Please.


PlaGuE
Sep 28th, 2006, 03:54 AM
This is an Authentication Class i am working on.

I'd appreciate any feedback on making improvements to its security.

<?
class AuthClass{

function Exeption($Error = array(), $BREAK = true){
$this->ErrMessage = "";
if($BREAK == true){
$this->ErrMessage = "<br />";
foreach($Error as $Err){
$this->ErrMessage .= "<br /><strong>Portal Error</strong>:".$Err." \n";
}
return trigger_error(exit($this->ErrMessage));
}else{
$this->ErrMessage = "<br />";
foreach($Error as $Err){
$this->ErrMessage .= "<strong>Portal Error</strong>:".$Err." \n";
}
return print("".$this->ErrMessage);
}
}

function Authentication($mRank){
// Authentication Functrion
if(isset($_POST['AuthLogin'])){
if($_POST['userName'] == NULL || $_POST['userName'] == ""){
$AuthErrors[] = "You Need to Input A UserName.";
}
if($_POST['passWord'] == NULL || $_POST['passWord'] == ""){
$AuthErrors[] = "You Need to Input A PassWord.";
}
$DUserName = stripslashes(htmlentities($_POST['userName']));
$DPassWord = stripslashes(htmlentities($_POST['passWord']));
$result = mysql_query("SELECT * FROM users WHERE username='".$DUserName."' AND password = PASSWORD('".$DPassWord."')");
if(mysql_num_rows($result) == 0 ){
$AuthErrors[] = "You Have Specified An Incorrect UserName/Password.";
}
extract($userinfo = mysql_fetch_object($result));
if($_POST['RememberMe']){
setcookie("CuSeR",$userinfo->username, time() + 3600);
setcookie("CuSeR_ID",$userinfo->ID, time() + 3600);
}
$_SESSION['CuSeR'] = $userinfo->username;
$_SESSION['CuSeR_ID'] = $userinfo->ID;
$_SESSION['CuSeR_Rank'] = $userinfo->rank;
$_SESSION['CuSeR_LastLogin'] = $userinfo->lastlogin;

}
if(isset($_COOKIE['CuSeR']) && isset($_COOKIE['CuSeR_ID'])){

$result = mysql_query("SELECT * FROM users WHERE username='".($_COOKIE['CuSeR'])."' OR ID='".$_COOKIE['CuSeR_ID']."'");
extract($userinfo = mysql_fetch_object($result));
$_SESSION['CuSeR'] = $userinfo->username;
$_SESSION['CuSeR_ID'] = $userinfo->ID;
$_SESSION['CuSeR_Rank'] = $userinfo->rank;
$_SESSION['CuSeR_LastLogin'] = $userinfo->lastlogin;

}
//*
$result = mysql_query("SELECT * FROM users WHERE username='".$_SESSION['CuSeR']."'");
extract($userinfo = mysql_fetch_object($result));

//print("My Rank:".$userinfo->rank."<br />");//-Making Sure
//print("Needed Rank:".$mRank."<br />");//------Rank Checking Works

if(!$userinfo->ID){
//return false;
$this->Exeption($AuthErrors,false);
$this->loginForm();
}else{
if(($userinfo->banned == 1) && ($userinfo->verified != 1)){
$this->Exeption(array("Not Only are you not a verified user, but you are also banned."),false);
}
elseif($userinfo->banned == 1){

$this->Exeption(array("You Are Banned"),false);
}
elseif($userinfo->rank >= $mRank){
if($userinfo->verified != 1){
$this->Exeption(array("You Are UnVerified"),false);
}else{
return 1;
}
}else{
$this->Exeption(array("You do not have a high enough rank to view this page.<br />\n"));
}
}


}//End Authentication Function

function loginForm(){
?>
<style type="text/css">
<!--
.style3 {
font-size: 10px;
font-family: Geneva, Arial, Helvetica, sans-serif;
color: #FFFFFF;
}
-->
</style>

<form action="<?=$_SERVER['REQUEST_URI']?>" name="AuthLoginForm" method="post">
<table width="31%" border="0" align="center" cellpadding="0" cellspacing="1" bgcolor="#000000">
<tr>
<td colspan="2" bgcolor="#999999"><div align="center">Login</div></td>
</tr>
<tr>
<td width="8%" bgcolor="#999999">Username</td>
<td width="92%" bgcolor="#999999"><input name="userName" type="text" value="UserName" class="login_text" /></td>
</tr>
<tr>
<td bgcolor="#999999">Password</td>
<td bgcolor="#999999"><input name="passWord" type="password" value="Password" class="login_text" /></td>
</tr>
<tr>
<td colspan="2" valign="middle" bgcolor="#999999"><div align="center">
<input type="submit" name="AuthLogin" value="Submit" />
<span class="style3">Remember Me?</span>
<input type="checkbox" name="RememberMe" value="true" />
</div></td>
</tr>
</table>

</form>
<?
}
}
?>

PlaGuE
Sep 29th, 2006, 02:43 AM
AnyOne?

kows
Sep 29th, 2006, 03:10 AM
I don't work much with classes, but I looked it over and it looked alright.

If anything, I would improve your grammar case more than anything. Like, for example, "You Have Specified An Incorrect UserName/Password." could be changed to "You have specified an incorrect username/password." Not that it needs to be, but oh my god, I'm a grammar nazi and if I was using whatever you made, whenever I read that I would get annoyed. I'm picky.

Aside from that, there seems to be some useless stuff in there that you can remove to make it faster, albeit not much of an increase because your script isn't doing much, but efficiency is efficiency! Here are what I pointed out with a quick skim:
if($_POST['userName'] == NULL || $_POST['userName'] == ""){
$AuthErrors[] = "You Need to Input A UserName.";
}
if($_POST['passWord'] == NULL || $_POST['passWord'] == ""){
$AuthErrors[] = "You Need to Input A PassWord.";
}
###########################
// could be changed to
###########################
if($_POST['userName'] == "" || $_POST['passWord'] == ""){
$AuthErrors[] = "You didn't input a username and/or password.";
}

I suggest this change just because, well, if you use isset() it should be removing any possibility of the string being null, and because I think that it's illogical to give two error messages for that, when it could be confined to just one.

Just small stuff like that. Other than that, I don't see anything really -wrong- or harmful or insecure.

I would point out more pointless things, but something just came up and I have to leave @_@.

PlaGuE
Sep 29th, 2006, 04:43 AM
koo thanks.
I actually was thinking of changing the part about checking the $_POST variables as one statement, but I just kept overlooking it.

Im actually picky with my grammar too. Exept that, sometimes im in a hurry and I capitalize "important" words and/or phrases.

kows
Sep 29th, 2006, 05:39 AM
I'm a complete grammar nazi.. the only thing acceptable is all lowercase, or proper case. I even get bugged by the fact that you don't put spaces after your periods and commas, lol. that's just how I like it, though. hooray for standardization