4 Replies - 1585 Views - Last Post: 24 May 2014 - 11:44 AM Rate Topic: -----

#1 Lieoften  Icon User is offline

  • D.I.C Head

Reputation: 17
  • View blog
  • Posts: 244
  • Joined: 06-January 10

Spent 2 hours trying to figure out why everyone was logged in as ADMI

Posted 21 April 2014 - 08:54 PM

I was rewritting a bunch of code for my website--namely the password encryption and login system... When my beta testers got back on they were all surprised to see that they were logged in as admin...

I took the site down and spent six hours trying to locate 1 line of code that was causing the issue.

	function login($username, $password)
	{
		$password = $this->passwordEncryption($password);
		$sql = "SELECT count(*), uid, homeX, homeY, first_time FROM users WHERE username = :username AND password = :password";
		$que = $this->db->prepare($sql);
		$que->bindParam('username', $username);
		$que->bindParam('password', $password );
		try{ 
			$que->execute();
			while($row = $que->fetch(PDO::FETCH_BOTH))
			{
				if($row[0] == 0)
				{
					header('location:index.php?error=nope');
				}
				else
				{
					$uid = $row[0];
					$x = $row[1];
					$y = $row[2];
					$home = "index.php?X={$x}&Y={$y}";
					$_SESSION['uid'] = $uid;
					$_SESSION['index'] = $home;
					return $home;
				}
			}
		}catch(PDOException $e) { echo $e->getMessage();}

		
	}


Is This A Good Question/Topic? 0
  • +

Replies To: Spent 2 hours trying to figure out why everyone was logged in as ADMI

#2 Atli  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 3717
  • View blog
  • Posts: 5,981
  • Joined: 08-June 10

Re: Spent 2 hours trying to figure out why everyone was logged in as ADMI

Posted 21 April 2014 - 09:21 PM

This is sort of a perfect example of why I always prefer to use associative arrays or objects, instead of numeric result arrays. The positions of the fields don't matter when you use those. That alone would have saved you from this.

Although, in this case I'd also point out that the COUNT call in the SELECT is quite unnecessary. If the username and password fail to match, and you exclude the COUNT, then the result set will be empty and the PDO fetch call will return false. - Also, since you are only ever expecting one row, the WHILE loop is pointless.

This would be my suggested alternative:
function login($username, $password)
{
    $password = $this->passwordEncryption($password);
    $sql = "SELECT uid, homeX, homeY
            FROM users 
            WHERE username = :username AND password = :password";
    $que = $this->db->prepare($sql);
    $que->bindParam('username', $username);
    $que->bindParam('password', $password );
    try
    { 
        $que->execute();
        $row = $que->fetch(PDO::FETCH_OBJ);
        if($row !== false)
        {
            $home = "index.php?X={$row->homeX}&Y={$row->homeY}";
            $_SESSION['uid'] = $row->uid;
            $_SESSION['index'] = $home;
            return $home;
        }
        else
        {
            header('location:index.php?error=nope');
        }
    }
    catch(PDOException $e) 
    {
        // You need proper handling of this exception!
        echo $e->getMessage();
    }
}


Was This Post Helpful? 0
  • +
  • -

#3 Lieoften  Icon User is offline

  • D.I.C Head

Reputation: 17
  • View blog
  • Posts: 244
  • Joined: 06-January 10

Re: Spent 2 hours trying to figure out why everyone was logged in as ADMI

Posted 21 April 2014 - 09:53 PM

For some reason, I never once thought to use !== false for my code... THat would've made that whole thing a lot easier. as for the while loop--i have a habit of over using them... i need to go to while loop rehab.
Was This Post Helpful? 0
  • +
  • -

#4 freelance php programmer  Icon User is offline

  • New D.I.C Head

Reputation: 3
  • View blog
  • Posts: 14
  • Joined: 14-May 14

Re: Spent 2 hours trying to figure out why everyone was logged in as ADMI

Posted 15 May 2014 - 01:16 AM

Your effort is appreciable. Really liked this.
Was This Post Helpful? 0
  • +
  • -

#5 IceTheNet  Icon User is offline

  • New D.I.C Head

Reputation: -5
  • View blog
  • Posts: 18
  • Joined: 23-May 14

Re: Spent 2 hours trying to figure out why everyone was logged in as ADMI

Posted 24 May 2014 - 11:44 AM

Atli good one you program like me. can't beat the asoc
Was This Post Helpful? 0
  • +
  • -

Page 1 of 1