benanamen
-
Posts
2,134 -
Joined
-
Last visited
-
Days Won
42
Posts posted by benanamen
-
-
Curl test case same EXACT result. (Of course need to change the if submit line to if ($_SERVER['REQUEST_METHOD'] == 'POST'))
<?php $curl = curl_init(); curl_setopt_array($curl, array( CURLOPT_RETURNTRANSFER => 1, CURLOPT_URL => 'http://localhost/help.php', CURLOPT_USERAGENT => 'cURL Request', CURLOPT_POST => 1, CURLOPT_POSTFIELDS => array( 'fname' => 'Joe', 'email' => 'user@example.com' ) )); echo $resp = curl_exec($curl); curl_close($curl); ?> -
In this particular case, I deleted the lname field and changed the error check to
if ($_POST['lname'] != '') { $lastname = $_POST['lname']; } else { $errors[] = "<p>Sorry, you must give your Last Name!</p>"; }What I get (with error checking on) is undefined index (ONLY) which is exactly to be expected. Firebug not even needed although I did look there. While in development mode the problem isn't going to get past you without you knowing it. In this case, if you leave the isset, you are never going to know that you are either missing the field you expect, or you are doing an error check you dont want. It will just silently be ignored. So in this instance I see it as bad to have the isset.
missing fields can also be caused by template bugs
In development, you are going to know about this and be able to fix it before going to production. If your dev environment matches the production environment, which ultimately, it should, its not going to unknowingly make it to production.
I suppose in a group development with different people doing things it would serve as a safety net. It definitely isn't going to hurt anything to do it that way.
Everything I have ever done I was the only programmer so I have never experienced problems from multiple coders. Only once I was coder and another guy did the design .
-
My bad, I meant WHERE username and the use case being a login and only one possible username match. user_id would have been the indexed auto increment which is not the column the WHERE would be checking.
-
Using the provided form, under what possible conditions would the form fields not be set on POST (other than unchecked checkboxes)? Is there a test case that would prove that out? I get that it is not that much more code, but I am always wanting to know what will fail and under what conditions.
-
I personally find the allotted time to be able to edit your post too short. Numerous times I have re-read my post and noticed some typo or something that needed changed and can no longer edit my post. Just how long is the current allowed time and can someone increase it another minute or two?
-
* Actually, OP has a problem in his code I used as an example. He says if firstname is not empty, set $firstname,
Then he says echo '<p>Sorry, you must give your First Name!</p>';} Which is not right since first name was was not empty.
-
I think you misunderstood what I said because of the way I worded it. I should have said
"As far as the form being submitted, all the fields are going to be isset already when the form is submitted."
I didn't mean remove the error checking, just the isset, still check for empty. On submit, all the form fields other then check boxes will be isset so there is no point checking for it.
It was also in specific response to the code from QuickOldCar. OP had it right.
OP Example:
- if (!empty($_POST['fname'])) {
- $firstname = $_POST['fname'];
- $firstnameerror = NULL;
- echo '<p>Sorry, you must give your First Name!</p>';}
Here is what I meant in code:
<?php if ($_SERVER['REQUEST_METHOD'] == 'POST') { // Not this. It will always be isset when using provided form if (isset($_POST['field'])) { echo 'Field Set<br>'; } // Check for empty instead if (empty($_POST['field'])) { echo 'Field Empty'; } } ?> <form action="<?= $_SERVER['SCRIPT_NAME'] ?>" method="post"> <input name="field"> <input name="submit" type="submit" value="Submit"> </form> -
You dont want to do this: if (isset($_POST['submit'])) {
It will completely fail under certain conditions. The correct and error proof way is
if ($_SERVER['REQUEST_METHOD'] == 'POST')
You also dont need to do all those trims. You can trim the whole post array all at once then get rid of all those issets and check for !empty. As far as the form being submitted, all the fields are going to be isset already. You can cut down the code by at least 50%.
-
Facebook now uses HACK for coding. Not sure of the DB.
-
A few times I have seen in the case of SELECT username WHERE user_id=? there is LIMIT 1.
My immediate thought was, why add the LIMIT 1 since there should only be one result anyways until I saw an explanation somewhere else which said it will stop MySQL from continuing searching the DB for more matches. A light bulb went off because this made sense to me.
My question is, is that really the case? And if so, are we talking enough of a performance gain for it to even matter? (Lets consider using something as big as Facebook).
If that is the best practice, I am surprised I almost never see it.
-
Before calling the current code complete, I am thinking the the DB connection code needs to be in at the least a function or a class passing the parameters to it. What would be best practice as far as the connection code. As is, if it were all in its own file complete, you have parameters that could get edited right next to code that shouldn't ever be touched. Additionally, if in a function or class, multiple connections to different DBs could be created if the need was ever there. Also, if in a class or function, should the options be hard coded or passed?
What is the best OOP practice for the DB connection code? I realize there is a ton of code out there, but with my lack of experience with OOP, I wouldn't know good from bad at this point.
-
The if/else is quite clear to me. The && is not. How would I see that the && translates to the same thing as the if since the results are exactly the same with or without it? I am also wondering if the if/else was better to be used for more clarity to those who may read the code.
We could also use a Ternary instead of the other two options as well right? (My Preference, although not as readable as the if/else IMO)
return $passwordHash ? password_verify($password, $passwordHash) : false;
-
Why are you returning $passwordHash? The result is the exact same with or without it.
return $passwordHash && password_verify($password, $passwordHash);// result: bool true or bool false.
return password_verify($password, $passwordHash); // Same exact result: bool true or bool false.
-
I was aware that it didn't return anything on false. I tried to update my post to ask you about that but the site already locked me out of editing the post.
The previous code comment really made sense to me when you put the task to a question, "Is the username and password valid?" which is a yes or no answer, so return Boolean. It got me to re think the process.
-
Thanks for your input @Jaques1. As of now I have about 12 hours of serious OOP experience against 25+ years of procedural experience. As brief as it was, the code comment was extremely helpful. I cut out at least 50% of the previous code and really got down to a single use class as well as transferring the workload to the class.
Here is my revised code. By the way, would the logout function be part of this? What are your thoughts on this revision?
<?php
// ----------------------------------------------------------------------------// Database Connection// ----------------------------------------------------------------------------$dbhost = 'localhost';$dbname = 'meetmarket_development';$dbuser = 'root';$dbpass = '';$charset = 'utf8';$dsn = "mysql:host=$dbhost;dbname=$dbname;charset=$charset";$opt = [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, PDO::ATTR_EMULATE_PREPARES => false, ];$pdo = new PDO($dsn, $dbuser, $dbpass, $opt);// ----------------------------------------------------------------------------// Class AuthenticateUser// ----------------------------------------------------------------------------class AuthenticateUser{public function __construct($pdo){$this->pdo = $pdo;}/*** Checks whether a username/password combination is valid** @param string $username the username to be checked* @param string $password the corresponding password** @return boolean the result of the check*/public function check($username, $password){$sql = "SELECT username, password FROM users WHERE username = ?";$stmt = $this->pdo->prepare($sql);$stmt->execute(array($username));$row = $stmt->fetch(PDO::FETCH_ASSOC);if ($stmt->rowCount()){if (password_verify($password, $row['password'])){return $this;}}} // End function} // End Class// ----------------------------------------------------------------------------//// ----------------------------------------------------------------------------/*CURRENT PROGRAM FLOW1. Compare user submitted username & password to DB username & password.2. If username and password valid return true*/$username = 'myusername';$password = 'pass';$user = new AuthenticateUser($pdo);echo $login_status = ($user->check($username, $password)) ? 'Valid Login' : 'Invalid Login';?> -
As I get more into OOP it appears I should know and understand unit testing and write tests. Since everything I do is DB driven, nearly all an apps functionality requires a database.
From what I read, if you are actually connecting to a DB you are not really doing unit testing, so my question is, how do you do a unit test on a method that gets its data from a DB?
Here is the current class I am experimenting with. As I understand it, a unit test is for ONE method. How would I do a unit test for example on the select_user method?
class User { /** * @var PDO The connection to the database */ protected $pdo; protected $row; protected $error; protected $password_hash; /** * Construct. * @param PDO $pdo The database connection */ public function __construct($pdo) { $this->pdo = $pdo; } /** * @param $request: $_POST or $_GET * @param $columns: Columns to SELECT */ public function select_user($request, $columns) { $sql = "SELECT "; foreach ($columns AS $field) { $sql .= $field . ', '; } $sql = rtrim($sql, ", "); $sql .= ' FROM users WHERE username = ?'; $stmt = $this->pdo->prepare($sql); $stmt->execute(array( $request )); $row = $stmt->fetch(PDO::FETCH_ASSOC); // No match, Do stuff. if (!$stmt->rowCount()) { $this->error['username'] = 'Bad User Name'; } else { $this->username = $row['username']; $this->password_hash = $row['password']; } } // End function select_user public function verify_password($password) { if (isset($this->password_hash)) { if (password_verify($password, $this->password_hash)) { echo 'Valid User'; // Set $_SESSION['logged_in']; } else { $this->error['password'] = 'Bad Password'; } } } public function check_errors() { if (count($this->error) > 0) { echo 'Errors<br>'; echo $error = implode("<br >\n", $this->error) . "\n"; die; } } // End function check_errors } // End Class User -
Does anyone know how facebook or banks do their logged in from new/unknown device routine? Have already googled. Have not found anything definitive yet.
In testing, it is not by cookies. I deleted all my cookies and all facebook did was have me re-login. (Notification Settings are set to notify me of new device.)
-
Current goals accomplished. Anyone see changes that should be made or problems with current code? OOP is making my brain hurt. Done for the day.
Not down on what should be public or private yet. From what I do understand I can make everything private. My understanding is that private/public has to do with being able to modify stuff outside the class. Protected? Not even going there at the moment.
LATEST CODE:
<?php //---------------------------------------------------------------------------- // Database Connection //---------------------------------------------------------------------------- $dbhost = 'localhost'; $dbname = 'meetmarket_development'; $dbuser = 'root'; $dbpass = ''; $charset = 'utf8'; $dsn = "mysql:host=$dbhost;dbname=$dbname;charset=$charset"; $opt = [ PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, PDO::ATTR_EMULATE_PREPARES => false, ]; $pdo = new PDO($dsn, $dbuser, $dbpass, $opt); //---------------------------------------------------------------------------- // Class User //---------------------------------------------------------------------------- class User { /** * @var PDO The connection to the database */ protected $pdo; protected $row; protected $error; protected $password_hash; /** * Construct. * @param PDO $pdo The database connection */ public function __construct($pdo) { $this->pdo = $pdo; } /** * @param $request: $_POST or $_GET * @param $columns: Columns to SELECT */ public function select_user($request, $columns) { $sql = "SELECT "; foreach ($columns AS $field) { $sql .= $field . ', '; } $sql = rtrim($sql, ", "); $sql .= ' FROM users WHERE username = ?'; $stmt = $this->pdo->prepare($sql); $stmt->execute(array( $request )); $row = $stmt->fetch(PDO::FETCH_ASSOC); // No match, Do stuff. if (!$stmt->rowCount()) { $this->error['username'] = 'Bad User Name'; } else { $this->username = $row['username']; $this->password_hash = $row['password']; } } // End function select_user public function verify_password($password) { if (isset($this->password_hash)) { if (password_verify($password, $this->password_hash)) { echo 'Valid User'; // Set $_SESSION['logged_in']; } else { $this->error['password'] = 'Bad Password'; } } } public function check_errors() { if (count($this->error) > 0) { echo 'Errors<br>'; echo $error = implode("<br >\n", $this->error) . "\n"; die; } } // End function check_errors } // End Class User //---------------------------------------------------------------------------- // //---------------------------------------------------------------------------- /* CURRENT PROGRAM FLOW 1. Compare user submitted username to DB username. If match goto 2. else die with error 2. Username is valid, compare user submitted password to DB hashed password. If match, valid user, else die with error. */ $request = $_POST['username'] = 'myusername'; $password = 'pass'; $user = new User($pdo); $columns = array( 'username', 'password' ); $user->select_user($request, $columns); $user->verify_password($password); $user->check_errors(); ?> -
Ok, figured it out. Not sure if it is correct though.
<?php //---------------------------------------------------------------------------- // Database Connection //---------------------------------------------------------------------------- $dbhost = 'localhost'; $dbname = 'meetmarket_development'; $dbuser = 'root'; $dbpass = ''; $charset = 'utf8'; $dsn = "mysql:host=$dbhost;dbname=$dbname;charset=$charset"; $opt = [ PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, PDO::ATTR_EMULATE_PREPARES => false, ]; $pdo = new PDO($dsn, $dbuser, $dbpass, $opt); //---------------------------------------------------------------------------- // Class User //---------------------------------------------------------------------------- class User { /** * @var PDO The connection to the database */ protected $pdo; protected $row; /** * Construct. * @param PDO $pdo The database connection */ public function __construct($pdo) { $this->pdo = $pdo; } /** * @param $request: $_POST or $_GET * @param $columns: Columns to SELECT */ public function select_user($request, $columns) { $sql = "SELECT "; foreach ($columns AS $field) { $sql .= $field . ', '; } $sql = rtrim($sql, ", "); $sql .= ' FROM users WHERE username = ?'; $stmt = $this->pdo->prepare($sql); $stmt->execute(array( $request )); $row = $stmt->fetch(PDO::FETCH_ASSOC); $this -> username = $row['username']; $this -> hash = $row['password']; //return $row; } public function verify_password($password) { echo $password;// User supplied password makes it to here echo $this->hash;// GOT DATA! if (password_verify($password, $this->hash)) { echo 'Valid User';// Set $_SESSION['logged_in']; } else { echo 'Bad'; } } } // End Class User //---------------------------------------------------------------------------- // //---------------------------------------------------------------------------- $request = $_POST['username'] = 'myusername'; $password = 'pass'; $user = new User($pdo); $columns = array( 'username', 'password' ); $user->select_user($request, $columns); $user->verify_password($password); ?> -
Right now I am just trying to make something work to grasp OOP. Right now the name of anything really doesn't matter.
Here is my immediate goal:
Create a Loosely coupled DB connected class - DONE
Compare user supplied password to DB password hash
Set a logged in session for a valid user.
Where I currently am:
I can get the username and password
Where I am now stuck:
comparing user supplied password to password hash from the result of select_user. I need to pass the hashed password from select_user to function verify_password. As of the moment I dont know how to pass results between methods.
Update Code:
<?php //---------------------------------------------------------------------------- // Database Connection //---------------------------------------------------------------------------- $dbhost = 'localhost'; $dbname = 'meetmarket_development'; $dbuser = 'root'; $dbpass = ''; $charset = 'utf8'; $dsn = "mysql:host=$dbhost;dbname=$dbname;charset=$charset"; $opt = [ PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, PDO::ATTR_EMULATE_PREPARES => false, ]; $pdo = new PDO($dsn, $dbuser, $dbpass, $opt); //---------------------------------------------------------------------------- // Class User //---------------------------------------------------------------------------- class User { /** * @var PDO The connection to the database */ protected $pdo; protected $row; /** * Construct. * @param PDO $pdo The database connection */ public function __construct($pdo) { $this->pdo = $pdo; } /** * @param $request: $_POST or $_GET * @param $columns: Columns to SELECT */ public function select_user($request, $columns) { $sql = "SELECT "; foreach ($columns AS $field) { $sql .= $field . ', '; } $sql = rtrim($sql, ", "); $sql .= ' FROM users WHERE username = ?'; $stmt = $this->pdo->prepare($sql); $stmt->execute(array( $request )); $row = $stmt->fetch(PDO::FETCH_ASSOC); return $row; } public function verify_password($password) { echo $password;// User supplied password makes it to here echo $this->row['password'];// No Data if (password_verify($password, $this->row['password'])) { echo 'Valid User';// Set $_SESSION['logged_in']; } else { echo 'Bad'; } } } // End Class User //---------------------------------------------------------------------------- // //---------------------------------------------------------------------------- $request = $_POST['username'] = 'myusername'; $password = 'pass'; $user = new User($pdo); $columns = array( 'username', 'password' ); $user->select_user($request, $columns); $user->verify_password($password); ?> -
Not sure what all you said means, yet. The purpose of the particular function select_user would be used as part of a user logging in or for a particular user profile and is only part of the functionality that will be in the class.
ie;
Select user, password
Compare password hash
Login Valid User
I figure what this class should initially be able to do is login a user, do password reset/change, possibly register/add new user and list all users, deactivate users (for the admin)
-
Starting to get a grasp. Dependency Injection was the unknown I was looking for. Have yet to see if I would have need for a Container/IOC. What I was seeking is a real separation of concerns. I was able to accomplish it quite well in procedural with templates.
Here is what I have so far. Any comments or improvements before I move on?
<?php $dbhost = 'localhost'; $dbname = 'meetmarket_development'; $dbuser = 'root'; $dbpass = ''; $charset = 'utf8'; $dsn = "mysql:host=$dbhost;dbname=$dbname;charset=$charset"; $opt = [ PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, PDO::ATTR_EMULATE_PREPARES => false, ]; $pdo = new PDO($dsn, $dbuser, $dbpass, $opt); class User { /** * @var PDO The connection to the database */ protected $pdo; /** * Construct. * @param PDO $pdo The database connection */ public function __construct($pdo) { $this->pdo = $pdo; } /** * @param $request: $_POST or $_GET * @param $columns: Columns to SELECT */ public function select_user($request, $columns) { $sql = "SELECT "; foreach ($columns AS $field){ $sql .= $field.', '; } $sql = rtrim($sql, ", "); $sql .=' FROM users WHERE username = ?'; $stmt = $this->pdo->prepare($sql); $stmt->execute(array($request)); $row = $stmt->fetch(PDO::FETCH_ASSOC); echo "<pre>"; print_r($row); echo "</pre>"; } } $request = $_POST['username'] = 'myusername'; $user = new User($pdo); $columns = array('username', 'password'); $user->select_user($request, $columns); ?>RESULT
Array ( [username] => myusername [password] => $2y$10$72JVve7WJCctEuB1SWA81OBItahuCuh9bWF/vI5NWTA1siFU9f8U6 ) -
Natasha, lets start at the beginning, your DB design. Post an SQL dump of your DB. Your trying to fix problems from a problem which is a problem.
-
1
-
-
main table is being updated with 4 other tables using different column combinations.
This smells of a bad DB design which I believe is really your biggest problem that needs to be fixed FIRST. Post your DB schema. You also dont seem to have any concern that you are using obsolete mysql code.
Sticky form help; PHP code is showing in HTML form
in PHP Coding Help
Posted · Edited by benanamen
I used quickoldcar's code example instead of what I actually proposed in #9. Nevertheless, the resulting error is exactly the same. If you look at my code in #9 it does use empty as you say.
What I proposed in #9:
// Check for empty instead
if (empty($_POST['field']))
{
echo 'Field Empty';
}
Using the isset makes it impossible to spot the issue that the expected field is not even in the form at all since it silently hides it. The only way you would ever know is by reading the code and seeing the form field doesn't exist. There will be no error in the logs, no error on the screen with error reporting on. No nothing except all the missing data you thought you were getting.
I would call anything that generates a system error a bug even if it is just a notice. It means something is wrong and needs to be fixed. I would say the following is pretty meaningful as to what and where the problem is: Notice: Undefined index: lname in E:\xampp\htdocs\help.php on line 32. Whether you see it in the logs or on the screen it is quite clear. You are also not going to want to abort the script on an undefined index. The worst that is happening using your example of removing the field is your not getting that piece of data. Using isset, you will never know until you look for the data and it is not there. With what I proposed, you are at the least getting the error logged which should be being checked regularly anyways. As you said, there is going to be bugs that will make it to production. Usually found because users do crazy stuff you would never think they would do.
I am pretty sure that is not what I am saying. My comments are specifically relating to whether a site supplied form field will always be isset when POST is submitted or not and if not, under what code provable conditions will it not be. I also tested in Curl and it makes no difference. Besides a form or curl, what other way is someone going to POST your expected form data?
I am not saying there is not a case where what I proposed will be a problem, but I am saying I dont know of it and would like to know what it is.
Same type of thing when we had our conversation about if ($_SERVER['REQUEST_METHOD'] == 'POST') VS if ($_POST). You were able to show me specific cases to support $_SERVER['REQUEST_METHOD'] == 'POST being the correct way and how it could fail otherwise. I want to know what I know, not just because someone says it and if I repeat it to someone else, to be able to back it up in code.
Using your example of removing the form field and, as actual code testing shows, how are you going to know that the error check for missing lname is looking for a form field that doesn't even exist if the problem is completely hidden with the isset?