Thread: Login Improvement

Page 1 of 2 12 LastLast
Results 1 to 10 of 18
  1. #1 Login Improvement 
    Theory Wins?
    Greyfield's Avatar
    Join Date
    Nov 2008
    Age
    32
    Posts
    1,585
    Thanks given
    61
    Thanks received
    265
    Rep Power
    310
    I'm just looking for some advice on improving a login script. I literally just started PHP/HTML/CSS two days ago, so please do not down anything. Just looking for suggestions on improvements.
    Code:
    <?php include("core/config.php");
    	session_start();
    	
    	$submit = $_POST['submit'];
    	$username = $_POST['username'];
    	$password = $_POST['password'];
    
    	if ($submit) {
    		if($username && $password) {
    			$query = mysql_query("SELECT password FROM admin WHERE username='$username'");
    			if(mysql_num_rows($query) == 1) {
    				$row = mysql_fetch_assoc($query);
    				$dbpassword = $row['password'];
    				if($password == $dbpassword) {
    					echo 'Success!';
    				} else {
    					echo 'Failure!';
    				}
    			} else {
    					echo 'Username or password does not exist!';
    			}
    		}
    	}
    	
    ?>
    <!DOCTYPE html>
    <html>
    	<form action='index.php' method='POST'>
    		<p>
    			<input type="text" name="username" placeholder="Username or email"/>
    		</p>
    		<p>
    			<input type="password" name="password" placeholder="Password"/>
    		</p>
    		<p>
    			<input type="submit" name="submit" value="Login"/>
    		</p>
    	</form>
    </html>
    Reply With Quote  
     

  2. #2  
    ye mofxkr

    Join Date
    Oct 2012
    Posts
    133
    Thanks given
    19
    Thanks received
    9
    Rep Power
    0
    No! STOP NOW.
    mysql_*

    NO. LEARN PDO.

    Also if you put that up, I'd have to SQL inject it. Sorry.
    Reply With Quote  
     

  3. #3  
    Theory Wins?
    Greyfield's Avatar
    Join Date
    Nov 2008
    Age
    32
    Posts
    1,585
    Thanks given
    61
    Thanks received
    265
    Rep Power
    310
    Code:
    	function sanitize($string) {
    		$string = strip_tags($string);
    		$string = mysql_real_escape_string($string);
    		return $string;
    	}
    Reply With Quote  
     

  4. #4  
    Registered Member
    Sieu's Avatar
    Join Date
    Dec 2011
    Age
    30
    Posts
    1,167
    Thanks given
    186
    Thanks received
    131
    Rep Power
    160
    I'm pretty sure this just compares the length of it, it won't actually compare the contents. Use string compare. strcmp
    Code:
    if($password == $dbpassword) {
    config.php
    Code:
    <?php
    	//Enter your database connection details here.
    	$host = 'localhost'; //HOST NAME.
    	$db_name = ''; //Database Name
    	$db_username = ''; //Database Username
    	$db_password = ''; //Database Password
    	
    	
    	//DONT DO ANYTHING BELOW UNLESS YOU KNOW WHAT YOU'RE DOING.
    	try
    	{
    		$pdo = new PDO('mysql:host='. $host .';dbname='.$db_name, $db_username, $db_password);
    	} 
    	catch (PDOException $e)
    	{
    		exit('Error Connecting To DataBase');
    	}	
    ?>
    login.php
    Code:
    <?php
    	session_start();
    	include_once('config.php');
    	if(isset($_SESSION['logged_in']))
    	{
    		header('Location: member.php');
    		exit();
    	}
    	else
    	{
    		$submit = $_POST['submit'];
    		$username = $_POST['username'];
    		$password = $_POST['password'];
    		if(isset($submit)){
    			if(isset($username) && isset($password))
    			{
    				$query = $pdo->prepare('SELECT password FROM admin WHERE username = ?');
    				$query->bindValue(1, $username);
    				$query->execute();
    				$user = $query->fetch();
    				$passwordToCheck = $user['password'];
    				if(strcasecmp($passwordToCheck, $password) == 0)
    				{
    					$_SESSION['logged_in'] = true;
    					$_header('Location: member.php');
    					exit();
    				}
    				else
    				{
    					$error = 'Your username or password was invalid.';
    				}
    			}
    			else
    			{
    				$error = 'Please fill in all the fields';
    			}
    		}
    		
    	}
    ?>
    <!DOCTYPE html>
    <html>
    	<?php if(isset($error)){ echo $error; } ?>
    	<form action='login.php' method='POST'>
    		<p>
    			<input type="text" name="username" value="<?php if(isset($username)){ echo $username; } ?>" placeholder="Username or email"/>
    		</p>
    		<p>
    			<input type="password" name="password" placeholder="Password"/>
    		</p>
    		<p>
    			<input type="submit" name="submit" value="Login"/>
    		</p>
    	</form>
    </html>

    member.php
    Code:
    <?php
    	session_start();
    	include_once('config.php');
    	if(!isset($_SESSION['logged_in']))
    	{
    		header('Location: login.php');
    		exit();
    	}
    	else
    	{
    		echo 'you're logged in.';
    	}
    ?>

    ps. not tested.
    Last edited by Sieu; 07-29-2013 at 08:23 PM. Reason: editteed, forgot execute lol
    Reply With Quote  
     

  5. #5  
    Theory Wins?
    Greyfield's Avatar
    Join Date
    Nov 2008
    Age
    32
    Posts
    1,585
    Thanks given
    61
    Thanks received
    265
    Rep Power
    310
    What's the difference of doing include(".php") and include_once(".php") ?
    Reply With Quote  
     

  6. #6  
    Registered Member
    Sieu's Avatar
    Join Date
    Dec 2011
    Age
    30
    Posts
    1,167
    Thanks given
    186
    Thanks received
    131
    Rep Power
    160
    Code:
    include_once
    means you only need it once. there's no need for
    Code:
    include
    since it's only being used once. however, it's up to you as the designer/coder though.
    Reply With Quote  
     

  7. #7  
    ye mofxkr

    Join Date
    Oct 2012
    Posts
    133
    Thanks given
    19
    Thanks received
    9
    Rep Power
    0
    Quote Originally Posted by Dillusion' View Post
    Code:
    	function sanitize($string) {
    		$string = strip_tags($string);
    		$string = mysql_real_escape_string($string);
    		return $string;
    	}
    mmk

    <<<INPUT
    ' onmouseover='alert(/XSS!/);
    INPUT;
    Reply With Quote  
     

  8. #8  
    Registered Member
    Sieu's Avatar
    Join Date
    Dec 2011
    Age
    30
    Posts
    1,167
    Thanks given
    186
    Thanks received
    131
    Rep Power
    160
    I'll write some sample queries so you can get started with pdo

    an insert query for register or so
    Code:
    	$query = $pdo->prepare("INSERT INTO admin (username, email, password) VALUES (?, ?, ?)"); 
    	$query->bindValue(1, $username); 
    	$query->bindValue(2, $email);
    	$query->bindValue(3, $password);
    	$query->execute();
    a select query..
    Code:
    	$query = $pdo->prepare("SELECT * FROM admin WHERE username = ?")
    	$query->bindValue(1, $username);
    	$query->execute();
            $user = $query->fetch(); //or $query->fetchAll(); 
    
    
            //you could also use rowCount(); to check if the query returned something at all or if what you're looking for exists in the database.
    	$row = $query->rowCount();
    	
    	if($row > 0)
    	{
    		//function here... 
                    echo 'account(s) exist?!?';
    	}
    basically, when you bindValue, it goes in the order of (?,?,?) in the query so that would translate to (1,2,3) as their places when you go to do the bindValue.

    I've color coated it so you could see which variables go with which if you're not used using pdo. If you have questions about it, you can ask or google
    Last edited by Sieu; 07-29-2013 at 08:46 PM. Reason: rowCount and some edits.
    Reply With Quote  
     

  9. Thankful user:


  10. #9  
    Registered Member

    Join Date
    Jan 2013
    Posts
    485
    Thanks given
    58
    Thanks received
    132
    Rep Power
    248
    Quote Originally Posted by MMMM View Post
    I'll write some sample queries so you can get started with pdo

    an insert query for register or so
    Code:
    	$query = $pdo->prepare("INSERT INTO admin (username, email, password) VALUES (?, ?, ?)"); 
    	$query->bindValue(1, $username); 
    	$query->bindValue(2, $email);
    	$query->bindValue(3, $password);
    	$query->execute();
    a select query..
    Code:
    	$query = $pdo->prepare("SELECT * FROM admin WHERE username = ?")
    	$query->bindValue(1, $username);
    	$query->execute();
            $user = $query->fetch(); //or $query->fetchAll(); 
    
    
            //you could also use rowCount(); to check if the query returned something at all or if what you're looking for exists in the database.
    	$row = $query->rowCount();
    	
    	if($row > 0)
    	{
    		//function here... 
                    echo 'account(s) exist?!?';
    	}
    basically, when you bindValue, it goes in the order of (?,?,?) in the query so that would translate to (1,2,3) as their places when you go to do the bindValue.

    I've color coated it so you could see which variables go with which if you're not used using pdo. If you have questions about it, you can ask or google
    Once you have stated PDO, Ill give him a few advices with it aswell.

    This script has a easy Session Hijacking method, considering you aren't validating any session IDs or do anything with it, simple as just setting the session name.
    Please do something with that, I will show a little example for it.

    A good solution would be regenerate session id after every action like login, logout and more.
    Use tokens to validate the login, my simple method would be if session id != to the sent hidden token (input type ="hidden" value="<?php echo session_id(); ?>" (...))
    But obviously it is not enough, please read this article, would definitely explain it better: PHP Session Security - Stack Overflow

    Also, please do not use MySQL_*, it is not going to function in the upcoming PHP version and is very outdated, and not object oriented, which will be hard for you to program with in the future, you aren't going to stick programming on-the-fly for ever right?

    - Hash your passwords to increase security.

    @MMMM

    Basically the "?" binding method was originally released for dynamic query methods, I think it is ghetto to bind too any parameters just for one query, instead you can use the name binding (:name) and passing it as a array to execute().
    No need to set a new variable for the method rowCount() as it already does it's job and returning the number of the counter rows.
    Other than that, all is OK.

    This is a basic login example I created, haven't tested but that's the logic I've used for it, this is also showing the PDO usage & sessions usage:

    Index.php:

    Code:
    	session_start();
    	
    	if (isset($_POST['submit'])
    	{
    		new Login();
    	}
    	
    	if (isset(Login::$errors))
    	{
    		echo Login::$errors;
    	}
    and the login class:

    Code:
    	class Login
    	{
    		public static $errors = "";
    		
    		function __construct()
    		{
    			// Start processing.
    			return $this->instance();
    		}
    		
    		private function instance()
    		{
    			// Checking if inptus are filled up.
    			if (isset($_POST['username']) && isset($_POST['password']))
    			{
    				// Checking if the received session ID equals to current session ID
    				if ($_POST['auth'] != session_id())
    				{
    					return false //Or other error..
    				}
    				
    				// Checking if username exists.
    				$query = $this->pdo->prepare("SELECT * FROM users WHERE username = :username");
    				$query->execute(array(":username" => $_POST['username']));
    				
    				// If yes..
    				if ($query->rowCount())
    				{
    					// Hash password
    					$password = Hasher::hashPassword($_POST['password']);
    					
    					// Checking if password is correct
    					$query = $this->pdo->prepare("SELECT * FROM users WHERE username = :username AND password = :password");
    					$query->execute(array(":username" => $_POST['username'], ":password" => $password));
    					
    					// If yes, login
    					if ($query->rowCount())
    					{
    						$this->login();
    					}
    				}
    			}
    			else
    			{
    				// Error!
    				self::$errors = "Empty";
    			}
    		}
    		
    		private function login()
    		{
    			// Regenerate Session ID
    			regenerate_session_id();
    			
    			// Add a new session.
    			$_SESSION['user'] = $_POST['username'];
    		}
    	}
    Also, don't use my example as your code, as I am not sure about the way I've done the checking, which was just an example, your checking should be more proper than this, like fetching the results and validating it PHP-Sided.
    But this is a very basic example for the PDO statements and a minimal session security.

    I hope you get it pretty much, and if you want to add a Remember me button, that's way more complex as you have to properly hash the username, date and more and save it as a cookie and then do the checking, when I first started PHP, I have done a very big mistake and just inserted the username into the cookies.

    And anyone could simply edit the cookie, and login to any of the accounts.


    Anyhow, I hope you understood it all, don't forget to check the link.
    Reply With Quote  
     

  11. #10  
    Registered Member
    Sieu's Avatar
    Join Date
    Dec 2011
    Age
    30
    Posts
    1,167
    Thanks given
    186
    Thanks received
    131
    Rep Power
    160
    I thought bindValue is easier for a beginner to read and get the hang of, because it's just bindValue(1, $value1); bindValue(2, $value2); etc, yea I know of that, but I just use this one lol. Does the same thing anyways

    I put in the rowCount if he wanted to do something with it, doesn't have to do with the code at all. Since he's just learning I was just throwing in stuff he could play with since he was using
    Code:
    mysql_num_rows($query)
    Reply With Quote  
     

Page 1 of 2 12 LastLast

Thread Information
Users Browsing this Thread

There are currently 1 users browsing this thread. (0 members and 1 guests)


User Tag List

Similar Threads

  1. Client login Screen. need improvements etc
    By Robin Spud in forum Graphics
    Replies: 17
    Last Post: 09-09-2010, 08:52 PM
  2. Replies: 11
    Last Post: 08-16-2009, 10:05 PM
  3. Replies: 142
    Last Post: 10-31-2007, 05:32 PM
  4. [TuT]Stopping multi logins through user ips
    By Wolf in forum Tutorials
    Replies: 21
    Last Post: 09-06-2007, 03:20 AM
  5. Improved GraphicsHandler
    By Diablo1123 in forum Tutorials
    Replies: 6
    Last Post: 06-11-2007, 01:08 PM
Posting Permissions
  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •