header("Location: ../logga.php?workinglogin");
exit();
}i don’t see you selecting the value of Verf from the database anywhere. Oh it may be because you are hiding what data you really want to receive for this proces behind select *. And i don’t see why you are doing the fetch process again after you already did mysqli_fetch_assoc. Also $sql is only a string, you should get at least an error for using it as an object. Mixing up procedual and OOP MySQLI is confusion and leads to additional errors. For a clearer solution you could use a switch statement like
switch(true)
case !$loscheck:
// redirect
break;
case !password_verify(...):
// redirect password false message
break;
default:
// redirect anywhere
1 Like
Thanks so much for the help! , so do I replace my If ($Loscheck) statements with the switch? But how do I check if the account has been verified the $verf == 1?
Thanks in advance!
Don’t replace simple if/else logic with a complicated switch/case statement. You are only testing if a single value is true or false.
As to your code, I’m not sure where you have been shown this style of programming, but it’s overly complicated. You have double the amount of logic you need. See the following link for a list a programming practices that will in most cases simplify the amount of implementation code - Blog Log in help
The three things that will help the most are -
- Put the form and the form processing code on the same page. This will eliminate all the redirects and the logic you probably have to produce meaningful error messages from the value passed in the url. This will also make the user experience (UX) better since you can re-populate the form fields with the values when there is an error so that the user doesn’t need to keep re-entering the values over and over.
- Use exceptions for database statement errors and in most cases let php catch and handle the error, where it will use its error related settings to control what happens with the actual error information (database statement errors will get displayed/logged the same as php errors.) This will eliminate all the conditional logic for handling the connection, prepare, and execute (which you don’t have anyway) statement errors.
- Switch to the much simpler php PDO extension. The PDO extension requires less statements to accomplish a prepared query.
By simplifying the implementation details in your code, it will make it easier to see where you would add the logic needed to test the ‘verified’ value.
The following is an example login script that makes use of the programming practices that were listed in the linked to reply -
<?php
session_start();
// detect if the current visitor is already logged in
if(isset($_SESSION['user_id']))
{
header('location:index.php'); // go somewhere else
die;
}
require 'pdo_connection.php'; // put database connection code in an external .php file and require it when needed
// note: the connection code should - set the error mode to exceptions, set emulated prepared queries to false, and set the default fetch mode to assoc
$errors = []; // an array to hold errors
$post = []; // an array to hold a trimmed working copy of the form data
// post method form processing
if($_SERVER['REQUEST_METHOD'] == 'POST')
{
// trim the submitted data
$post = array_map('trim',$_POST); // if any of the form fields are arrays, use a recursive trim call-back function here instead of php's trim function
// validate the submitted data
if($post['username'] == '')
{
$errors['username'] = "Username is empty.";
}
if($post['password'] == '')
{
$errors['password'] = "Password is empty.";
}
// if no errors, use the submitted data
if(empty($errors))
{
$sql = "SELECT id, password from users WHERE username = ?";
$stmt = $pdo->prepare($sql);
$stmt->execute([
$post['username']
]);
if(!$row = $stmt->fetch())
{
// username not found
$errors['login'] = "Invalid Username/Password.";
} else {
// username found, verify password hash
if(!password_verify($post['password'],$row['password']))
{
// password doesn't match
$errors['login'] = "Invalid Username/Password.";
} else {
// password matches
$_SESSION['user_id'] = $row['id'];
}
}
}
// if no errors, success
if(empty($errors))
{
// redirect to the exact same url of this page to cause a get request - PRG Post, Redirect, Get.
header("Refresh:0");
die;
}
}
// at the point of (re)displaying the form, use the data in $errors to display any error messages and in $post to repopulate the form fields (you may not desire to populate password fields)
// any 'dynamic' values should have htmlentities() applied when they are being output on a web page to help prevent cross site scripting
?>
output a complete and valid html document starting here...
<?php
// display any errors
if(!empty($errors))
{
echo implode('<br>',$errors);
}
// output the form
?>
<form method='post'>
Username: <input type='text' name='username' value='<?php echo htmlentities($post['username'] ?? '',ENT_QUOTES); ?>'><br>
Password: <input type='text' name='password' value='<?php echo htmlentities($post['password'] ?? '',ENT_QUOTES); ?>'><br>
<input type='submit'>
</form>prg is nice but there is not one single way to implement it. I find that it is dangerous to redisplay post data when an error has occurred. escaping is a defense but it is awful because it could remove legitimate characters. Imagine a user named Ja’Net now sees her name as Ja"Net. oops! we fubarred a legitimate name just because we want to show the user their name (which they all ready know.) I find it best to not show data that isn’t the cause of an error. There is a fine line between usability and treating people like idiots. You know your name and don’t have to see it if no errors exist with this name.
I am less kind to a user and i don’t really care. if you can’t type your name, then i don’t want your business in the first place. hit the bricks! really. i will not tolerate stupidity and i will not sink to a low level of entitlement. I disagree with making everything easier for people. I never show post data back to a user. In german “das ist nich mein problem”, so try again or get lost. I don’t complain about it. I don’t need to see what i type. I hate it when websites show me my data. I usually say buzz off and go somewhere else.
i handle errors with a session variable (bla bla someone will complain about misuse of a session variable like we suddenly have to program software without using memory, lol) set it, unset it and noone knows any better.
thus, here is my method of prg using your code:
try {
$conn = mysqli_connect($server, $user, $pass, $dbname);
} catch (PDOException $e) {
error_log($e->getMessage());
$conn = NULL;
$_SESSION['Login_errors']['Database_connection'] = 1;
}
//return to login page where the following code exists under the login
<?php if (isset($_SESSION['Login_errors']['Database_connection'])) { ?><div>Cannot Connect Error Message</div><?php unset($_SESSION['Login_errors']['Database_connection']); } ?>
//slick how we set and unset, eh? use it or lose it, i always sayin most languages, a switch is faster than an if else branch and you don’t have to be a master of php to know this info. I’ve been reading programming books and tinkering with code since 1997. PHP is horrible with arrays and branching, so a switch is not so fast and it could slow down a website tremendously if it contains more than five cases. But in no way is a switch complicated in itself (bad programmers that create php makes switches complicated). A switch is a binary implementation based upon electrical design (on or off, 0 or 1, easy as a light switch). switch (light) case 1: echo we have light or the pc is turned on"; is easy as pie and no different than the electrical gate (switch) built into the motherboard of your internet device.
Thank you so much for explaining in detail and takeing your time to write it, means a lot .
Best regards!
Thank you for responding and giveing examples
Best ragards