Leaderboard
Popular Content
Showing content with the highest reputation on 09/05/2022 in all areas
-
I think it is time to begin a new topic since you are moving from point to point constantly. This latest query looks nothing like the ones we have helped you with all day and is totally wrong. Again - you don't have to prepare if not using any parameters in the query you only need a group by when you are doing arithmetic functions in the query And the DESC is part of the order by clause, not the limit clause.1 point
-
mysqli_query uses an SQL query (string) for its second argument. You are passing $stmt which is a prepared statement object. There is nothing in that query that needs preparing so just use mysqli_query with the query string.1 point
-
I don't use mysqli so I can't answer that. You will have to read the php manual to see how a prepared statement is used. https://www.php.net/manual/en/mysqli-stmt.prepare.php Read the example for 'Procedural style' BTW - since you are not using any user-supplied arguments in your query, you really don't need to 'prepare' the query.1 point
-
This is a good resource for screen resolution (among other things).1 point
-
Dangling can refer to perhaps a thread hanging off of a new shirt or a strand of hair hanging on your forehead.1 point
-
Your code can be simplified in many ways. See the comments in the refactored code below. <?php function setRememberMeToken($pdo, $user_id) { //$length wasn't a great name and is an unnecessary variable. $token = bin2hex(random_bytes('25')); $expirationDate = time() + (86400 * 7); // <-- 7 days later (make sure your comments are accurate) setcookie("token", $token, $expirationDate, "/"); $_COOKIE["token"] = $token; //$_COOKIE['remember'] is unnecessary, just get rid of it //--deleted //You calculated your expiration timestamp above already, no need to do it again. $to = date('Y-m-d', $expirationDate); //Assuming token_id is an auto increment column, you can just omit it from the insert. $sql = "INSERT INTO `user_token` (`user_id`, `expires`, `tokenHash`) VALUES (?, ?, ?);"; $stmt= $pdo->prepare($sql); $stmt->execute([$user_id, $to, sha1($token)]); } function getRememberMeCheck($pdo) { //I find spacing out your queries makes them easier to read and understand. $stmt = $pdo->prepare(" SELECT users.name, users.user_id FROM user_token, users WHERE tokenHash = ? AND expires > NOW() AND users.user_id = user_token.user_id "); $stmt->execute([sha1($_COOKIE["token"])]); $db_query = $stmt->fetch(); //Your token and expiration date are validated as part of the query //All you need to do is check if you got a result or not. if (!$db_query){ //If you didn't get a result, either the token is invalid or it has expired. //header("location: login.php"); return false; } //Otherwise, if you did get a result, the token is valid. $_SESSION["loggedin"] = true; $_SESSION["username"] = $db_query['name']; $_SESSION['the_usr_id'] = $db_query['user_id']; return true; } //This method seems to just be a copy of the method above, why does it exist? //The only difference is $_SESSION["loggedin"] = true; which you could just do above. //function setSessionVarables($pdo) { //... //} //--deleted function isRemembered() { //Instead of a separate remember cookie, just check if the token cookie exists. //if ($whatever){ return true; } else { return false} can be simplified to just return $whatever return isset($_COOKIE['token']); }1 point
-
Every class you create that extends your dbconnect class will be creating it's own separate connection to the database. If you're using several of these classes in a page load then that would mean several separate connections. What you should do is have those classes accept an instance of your dbconnect class as a parameter to their constructor. class secondClass { private $db; public function __construct(dbconnect $db){ $this->db=$db; } } $db=new dbconnect(); $sc = new secondClass($db);1 point
-
Flexbox and/or grid should handle everything you want. I would start with using flexbox for all of the layout pieces you have already described, so certainly you want a container div for those so that you can use display: flex. At that point it's just a matter of applying the appropriate parent/container properties or child div properties you desire. This article might help if you need a refresher: https://css-tricks.com/snippets/css/a-guide-to-flexbox/1 point
-
The point of storing the expiration date is so that you can check it when you lookup the token later and reject expired tokens. You cannot rely on the cookie being deleted by the browser at the requested time, so you must check for yourself if the token is expired or not. $stmt = $pdo->prepare(" SELECT users.name, users.user_id, tokenHash FROM user_token, users WHERE tokenHash = ? AND expires > NOW() AND users.user_id = user_token.user_id "); $stmt->execute([sha1($_COOKIE["token"])]); $db_query = $stmt->fetch();1 point
-
So it looks like you are using your own tokens for login. That is not a good idea. You should be relying on the session instead. If you expect to have a cluster of app servers, you might need to change session storage so that it uses your database, or redis/memcached instead of files, or you can use a load balancer that supports the configuration of a "sticky" session, but otherwise, you don't want to be creating and managing your own session/authentication token for anything other than the remember me feature. Sessions already, when properly configured, utilize a cookie. You also want to make liberal use of session_regenerate_id. From what you posted, this has DRY/ logic issues: if (isset($_POST['remember-me'])){ $_SESSION["loggedin"] = true; $_SESSION["username"] = $username; $_SESSION['the_usr_id'] = $user['user_id']; echo "<hr>"; echo setRememberMeToken($pdo, $user['user_id']); echo "<hr>"; header("location: dashboard.php"); } if (!isset($_POST['remember-me'])) { $_SESSION["loggedin"] = true; $_SESSION["username"] = $username; $_SESSION['the_usr_id'] = $user['user_id']; echo "<hr>"; echo "<hr>"; header("location: dashboard.php"); } Again, I don't have your code, but you have a repetition of code, and worse yet that code is actually not relevant to the variable you are checking (!isset($_POST['remember-me']). What you want to do is avoid blocks like this, which are also mutually exclusive. So again, assuming that this is part of a block where you already established successfull login previously, you can rewrite your code like this, to separate the remember-me check so that it only does what is relevant to the existence of that flag. I will also point out that if the user logs in without the remember me checkbox, you should remove the remember me token, but your code doesn't do that. Something like this would be better, and also fix your current hole. // Assumes authentication succeeded above this code $_SESSION["loggedin"] = true; $_SESSION["username"] = $username; $_SESSION['the_usr_id'] = $user['user_id']; // Not sure why you need this? if (isset($_POST['remember-me'])) { setRememberMeToken($pdo, $user['user_id']); } else if (!$_SESSION['rememberMeLogin']) { // Was not a "remember me" authentication AND remember me was unchecked. So should remove the remember me token & Delete the cookie. unsetRememberMeToken($pdo, $user['user_id']); } header("location: dashboard.php"); exit; Another thing I would suggest is not to use md5, but used sha1 instead. md5 has a smaller collision space. I would also add something to the random input to the hash (md5,sha1). So perhaps add something like username + random bytes. People can generate a large table from random byte input (a rainbow table) and look for matches. By doing so they might figure out you are just using random byte combinations of a certain size. It's not a major security hole, but one you can protect against by not just hashing random characters of the exact same size from a routine. This is similar to the idea of using a "salt".1 point
-
do not store any user information in cookies. anyone can set cookies to any value and can impersonate a user. to do what you are asking, generate a unique token, store the token in a cookie and store it in a row in a 'remember me' database table, along with the user's id and things like when the remember me was set and when you want it to expire if not regenerated. if you receive a cookie containing a token, query to get the user's id and the expire datetime to determine if the token is valid. if it is, set the normal session user_id variable to indicate who the logged in user is. you should only store the user id in a session variable, then query on each page request to get any other user information, such as the username, permissions,... this will insure that an change/edit in this user information will take effect on the very next page request.1 point
-
If you want multi-device support, you need to move your cookie_token field out of your users table and into another table so that you can have multiple active tokens. For example create table user_token ( UserId int not null, TokenHash varchar(255) not null, Expires datetime, primary key (UserId, TokenHash) ); Whenever you generate a new token, insert a row into that table. When you need to validate the token, look for it in the table and make sure it's not expired.0 points
This leaderboard is set to New York/GMT-05:00