Login help!!

[Sabres88]

I’ve made a register and a log in page and they are both connected to the same database. Register page works, but when i enter in Username and Password for log in page I get an error saying “Sorry, your account was not validated”.

Here is my login page:

[php]<?php
session_start();

//include the database connection
include(‘dbc.inc.php’);

//check to see if the connection was successful
if(!$db)
{
echo “

Sorry we cannot process your request at this time. Please try again later.

\n”;
echo “<a href = “index.php”>Home
\n”;
exit;
}

//check for form submission
if($_SERVER[‘REQUEST_METHOD’] == ‘POST’)
{

//minimal form validation
if(isset($_POST['userid'], $_POST['password'])) 
{

	//declare variables
	$tablename='users';
	$userid = trim($_POST['userid']);
	$password = $_POST['password'];
	$access = 'U';
	

	//check if userid & password is in the database
	$query =  "SELECT userid, access, password from $tablename WHERE userid = '$userid' AND password = SHA1('$password')"; 
	//echo "$query";
	$result = mysqli_query($db, $query);

	$row = mysqli_fetch_row($result);


	if(!$row) 
	{
		echo "<h3>Sorry, your account was not validated.</h3><br />\n";
		echo "<a href = \"index.php\">Home</a><br />\n";
		exit;
	}
	else 
	{
		
		//Create a session for the logged in user
		$_SESSION['userid'] = $row[0];
		//Set a session variable for access
		$_SESSION['user_access'] = $row[1];
		
	}


}//end if minimal form validation

}//end if check for form submission

?>[/php]

[awl19]

I made some corrections and commented out your old code so you could see the differences.

[php]<?php
session_start();
//include the database connection
include(‘dbc.inc.php’);
//check to see if the connection was successful
if (!$db){
echo “

Sorry we cannot process your request at this time. Please try again later.

\n”;
echo “<a href = “index.php”>Home
\n”;
exit;
}
//check for form submission
if($_SERVER[‘REQUEST_METHOD’] == ‘POST’) {
//minimal form validation
//if(isset($_POST[‘userid’], $_POST[‘password’])) {
if (isset($_POST[‘userid’]) && ($_POST[‘password’])) {
//declare variables
$tablename=‘users’;
$userid = trim($_POST[‘userid’]);
//$password = $_POST[‘password’];
$passwordP = $_POST[‘password’];
$password = SHA1($password);
$access = ‘U’;

    //check if userid & password is in the database
    //$query =  "SELECT userid, access, password from $tablename WHERE userid = '$userid' AND password = SHA1('$password')";
    $query =  "SELECT userid, access, password from $tablename WHERE userid = $userid AND password = $password";
    //echo "$query";
    $result = mysqli_query($db, $query);

    $row = mysqli_fetch_row($result);


    if(!$row) {
        echo "<h3>Sorry, your account was not validated.</h3><br />\n";
        echo "<a href = \"index.php\">Home</a><br />\n";
        exit;
    }
    else {
        //Create a session for the logged in user
        //$_SESSION['userid'] = $row[0];
        $_SESSION['userid'] = $row['userid'];
        //Set a session variable for access
        //$_SESSION['user_access'] = $row[1];
        $_SESSION['user_access'] = $row['access'];
    }
}//end if minimal form validation

}//end if check for form submission
?>[/php]

What I really noticed was you put quotes around your variables inside the query. You don’t want to do that. Nor should you be wrapping your variables in anything in a query (SHA1, for example). You do that outside of the query and variable you put in should be the “final product.”

Let me know if that works.

[benanamen]

The changes are not going to work. The code will fail. Allow me to explain…

You have set the posted password to $passwordP = $_POST[‘password’];

$passwordP does not exist anywhere else.

You have set $password = SHA1($password);

$password is not created anywhere. This will cause an error.

The query as written is expecting $password. Since it does not exist, the query will fail.

$query = “SELECT userid, access, password from $tablename WHERE userid = $userid AND password = $password”;

This brings up something I was going to mention in our other posts to each other. Using $password and $passwordP as you do is not a good idea. It is way to similar and as we see here, can easily be overlooked. By creating unnecessary variables you are just creating more opportunities for mistakes.

I realize in this case it was the OP that did it. but it is the same thing you were promoting in other posts as the way to do it.

You commented out
$_SESSION[‘userid’] = $row[0];
AND
$_SESSION[‘user_access’] = $row[1];

There is nothing wrong with that, but it is more readable the way you did it and how I would also do it. One reason it is better to name the column names is if you were to add more or less columns to your query. You would have to go re-number everything. With the column names you dont have to change anything.

To the OP: You have $access = ‘U’; $access is not used anywhere in your code. This wont break anything but has no purpose in what you posted. You are using Mysqli but are not using parameterized query’s.

The code is totally vulnerable to SQL Injection.

[awl19]

You have set $password = SHA1($password);

Lol, yah, just a spelling mistake. It was supposed to be $password = SHA1($passwordP); … linking to the password posted by user…

[benanamen]

Typos happen which is why you need a testing environment setup to test your code before you post it.

A couple of basic fundamental programming concepts you may or may not know of.

KISS
Keep It Simple Stupid

DRY
Don’t Repeat Yourself