trouble with admin login scripts

[gary3333]

I wonder if someone can help. I am trying to update my scripts from PHP5 TO PHP7. As I know php7 has done away with connect to mysql. I have been on w3schools and I have tried to do my code the way they give the example but still not working.

I have got an admin login script which goes like this

admin_login.php

[embed=425,349]<?php
session_start();
if (isset($_SESSION[“manager”])) {
header(“location: index.php”);
exit();
}
?>

<?php // Parse the log in form if the user has filled it out and pressed "Log In" if (isset($_POST["username"]) && isset($_POST["password"])) { $manager = preg_replace('#[^A-Za-z0-9]#i', '', $_POST["username"]); // filter everything but numbers and letters $password = preg_replace('#[^A-Za-z0-9]#i', '', $_POST["password"]); // filter everything but numbers and letters // Connect to the MySQL database $servername = "localhost"; $username = "username"; $password = "password"; $dbname = "myDB"; // Create connection $conn = new mysqli($servername, $username, $password, $dbname); // Check connection if ($conn->connect_error) { die("Connection failed: " . $conn->connect_error); } $sql = mysqli_query("SELECT id FROM admin WHERE username='$manager' AND password='$password' LIMIT 1"); // query the person // ------- MAKE SURE PERSON EXISTS IN DATABASE --------- $existCount = mysqli_num_rows($sql); // count the row nums if ($existCount == 1) { // evaluate the count while($row = mysqli_fetch_array($sql)){ $id = $row["id"]; } $_SESSION["id"] = $id; $_SESSION["manager"] = $manager; $_SESSION["password"] = $password; header("location: index.php"); exit(); } else { echo 'That information is incorrect, try again Click Here'; exit(); } } ?>[/embed]

index.php

[embed=425,349]<?php

session_start();
if (!isset($_SESSION[“manager”])) {
header(“location: admin_login.php”);
exit();
}
// Be sure to check that this manager SESSION value is in fact in the database
$managerID = preg_replace(’#[^0-9]#i’, ‘’, $_SESSION[“id”]); // filter everything but numbers and letters
$manager = preg_replace(’#[^A-Za-z0-9]#i’, ‘’, $_SESSION[“manager”]); // filter everything but numbers and letters
$password = preg_replace(’#[^A-Za-z0-9]#i’, ‘’, $_SESSION[“password”]); // filter everything but numbers and letters
// Run mySQL query to be sure that this person is an admin and that their password session var equals the database information
// Connect to the MySQL database
$servername = “localhost”;
$username = “username”;
$password = “password”;
$dbname = “myDB”;
// Create connection
$conn = new mysqli($servername, $username, $password, $dbname);
// Check connection
if ($conn->connect_error) {
die("Connection failed: " . $conn->connect_error);
}
$sql = mysqli_query(“SELECT * FROM admin WHERE id=’$managerID’ AND username=’$manager’ AND password=’$password’ LIMIT 1”); // query the person
// ------- MAKE SURE PERSON EXISTS IN DATABASE ---------
$existCount = mysqli_num_rows($sql); // count the row nums
if ($existCount == 0) { // evaluate the count
echo “Your login session data is not on record in the database.”;
exit();
}
?>[/embed]

I was wondering if someone could tell me what i am doing wrong. The code is suppose to go through the admin_login.php and then after checking the login details give me a connection to index.php. but it is giving me this That information is incorrect, try again Click Here.

many thanks, Gary

[phdr]

First of all, this is not posted in a correct forum section for asking coding questions.

Next, the reason your code isn’t working is because you have used the $password variable for both the submitted form field data and the database connection password and when you are attempting to use it to authenticate the visitor, it isn’t the submitted form field data any longer.

Lastly, this code is doing a bunch of things that are unnecessary or poorly implemented -

1. Don’t Repeat Yourself (DRY.) The database connection credentials should be stored in a file that you require into any code that needs them. You should also use variable names for them that are unique and are named having something to do with their purpose, such as $db_host, $db_user, $db_password, $db_name.

2. Your login should be for users in general. An admin/manager is just a user with specific permissions. All your user data should be stored in one table. The login logic should just authenticate/identify who a user is. The only thing you should store in a session variable is the user id. On each page request, you should take that user id from the session variable and query to get the permissions for the logged in user. The user permission information would not be stored in session variable(s), so that they can be edited without requiring the user to log out and log back in for them to take effect. You would use the permission information to determine what the current user can do and see on any page request. The session variable needs to be uniquely named to indicate the purpose of the data in it, such as $_SESSSION[‘user_id’]. The current name, $_SESSION[‘id’], isn’t very descriptive and could mean anything, making it harder for anyone reading your code to be able to tell what it is doing.

3. All database connection/query statements need error handling and you should NOT echo the raw error information onto the web page. The easiest way of having error handling and prevent echoing the raw error information (when on a public server) is to use exceptions. All you need to do is enable exceptions for the php database extension you are using and let php catch the exceptions. Php will use it’s error_reporting, display_errors, and log_errors settings to control what happens with the actual error information. Once you do this, you can eliminate the error handling logic you have now in your code, Keep It Simple (KISS.)

4. Do NOT filter/modify data when validating it. This changes the meaning of the data and actually opens security holes if you are not applying the same filter/modification every place you input the data (I know of a php help forum that got it’s user database copied because of inconstant filtering on email addresses that allowed a hacker to take over an admin account.) The only thing you should do is trim() data, so that you can detect if it is all white-space characters. For logging a user in, all you care about is that the submitted data isn’t empty and if it matches a user row in your database table. Your existing filtering on the password means that punctuation and special (printing) characters cannot be used, resulting in weaker passwords that will be more easily bruit-force found.

5. Do NOT put data values directly into the sql query statement. You should be using prepared queries, with place-holders in the sql query statement for data values, then supply the data when you execute the query. Unfortunately, the php mysqli extension is not very well implemented and you should switch to use the php PDO extension. In addition to be better implemented and simpler, the PDO extension will let you re-use the same php statements with other database types (the actual sql syntax may be different), so that you can more easily write code for other database types without leaning a different set of php statements for each different database type.

6. Passwords MUST be hashed when stored. See php’s password_hash() and password_verify() functions. When logging in a user, you would retrieve the hashed value and use password_verify() to test if the submitted password matches the hashed value.

7. If an query is expected to match at most one row, don’t use a loop to fetch the data. Just directly execute one fetch statement to get the data.

8. If your form and form processing code are on the same page, you wouldn’t have to output a link for the user to click on when the form processing code doesn’t match a user. Just re-display the form. This will also let you re-populate the form fields with the submitted values so that the user doesn’t need to re-enter everything.

9. On your index page, if $_SESSION[‘user_id’] (see the naming recommendation above) is set, you know there is a logged in user and what the id of the user is. You would just query for the user permissions, as described above in this reply. There’s no good reason to re-check the user’s login credentials and in fact by storing the password in a session variable, you are opening up a security hole, should you leave some var_dump/print_r debugging logic in your code.

[gary3333]

thank you so much for your reply phdr and giving me such a detail list to look at and study in my code. i will go through what you have said bit by bit and study my code. your great thank you so much. i might be back if i get any more trouble after taking everything you have said into account.