views:

70

answers:

3

I was looking around for more "correct" login/logout portions of code, and found this one: http://snipplr.com/view/1079/auth/

I got two questions though that stop me from using it.
1: How would I instantiate the class, and use it in my script?(I know PHP but am just befuddled for some reason)
2: there's the following lines:

global $db;
$db->query("sql here...");

How on earth does that make a database object? I think maybe I should create an object like $db = mysql_connect(...) outside the script, and global is calling it from outside the class?

If I know how to call this class, others will seem like a breeze, this is really helpful to me!

+1  A: 

global $db just allows the global database pointer $db to be accessed from inside the class method. You need to create the database by running mysql_connect and mysql_select_db. The entire class is in class Auth, so instantiate an Auth object before doing anything else (well, after connecting to the database, of course).

You can then call the methods inside your Auth object. For example, if a login form submits to login-post.php, you can have this code inside:

$m = new Auth();
if ($m -> login($_POST['u'], $_POST['p'])) {
    header('Location: /');
    exit;
} else {
    header('Location: login?wrong=1');
    exit;
}
Delan Azabani
A: 

Looks like the code you posted is just assuming you have a variable defined in your global scope called $db that looks like a wrapper class to the native PHP MySQL resource. So yes, you should create $db outside in the global scope (i.e. not inside a class), but it's expecting a custom class.

I did a little searching, and it looks like he's using THIS class: http://snipplr.com/view/27966/class-db/

It asks you to create an ini file called "crunksauce.ini" (I kid you not) in the same directory as the executing script, and contains configuration variables. The config file should like this:

database = <your database name>
host = <your db host>
user = <your db user>
pass = <your db password>

After creating the config file, you can create the db object like this:

$db = new Db();
Mike Sherov
Awesome, I think I'll place that ini out of public reach, can't imagine someone using this class and failing to remember that, hehe
John D.
+4  A: 

That code is dangerous, you should not use it in it's current state.

  • It blindly trusts $_COOKIE and creates SQL queries using string concatenation, without properly escaping input. Malicious users can easily perform SQL injection on any application using that class.
  • The presence of the var keyword, the presence of a constructor using the same name as the class and the lack of access control keywords in front of the methods tells me that the code was written for PHP4, not PHP5. It will still run, though

Both of these are fixable problems. First, let's address your questions.

  1. The code written by @Delan Azabani is a good example of how to use the class.
  2. The code is confusing. At first, it looks like the code is using a real database object instead of the old, low-level functions provided by the mysql extension. But then it goes and calls functions from the mysql extension! It's probably someone's custom-written wrapper. The code isn't actually creating a database object, it's simply referencing the value of $db in the global scope, as you suspect.

You've mentioned that you're using the mysql extension. I urge you to reconsider and use PDO or mysqli instead. Not only will this class work with either (though with some changes to mitigate the glaring security hole), but your own code will be better off using the more modern, safer techniques used in PDO and mysqli.

Let's fix the login method, using PDO.

public function login($username, $password) {
    global $db;
    $sth = $db->prepare("SELECT user_id FROM users WHERE username = ? AND password = ?");
    $sth->execute(array($username, $password));
    if($sth->rowCount() == 1) {
        $this->user_id = $sth->fetchColumn();
        ...

Let's go over what changed. First, we added query placeholders, those are the question marks. The prepare method returns a statement handle, which you're already probably familiar with. Next, we tell the prepared statement to run itself using the execute method, passing an array of variables. The variables will be automatically escaped for you and then inserted into the query in place of the placeholders.

After the query has run, we use rowCount to pull back the number of matching rows. We want exactly one. Because we're pulling back the first column of the first row, we then use fetchColumn to grab just that bit of data.

The rest of the login method is just fine.

The check method needs similar fixing up.

public function check($username, $password) {
    global $db;
    $sth = $db->prepare("SELECT user_id, password FROM users WHERE username = ?");
    $sth->execute(array($username));
    if($sth->rowCount() == 1) {
        list($db_user_id, $db_password) = $sth->fetch(PDO::FETCH_NUM);
        if(md5($db_password . $this->salt) == $password) {
            $this->user_id = $db_user_id;
            ...

Once again, we prepare a query with placeholders, then execute with the actual variables. Again we want only one row, but this time we want everything in the row. We'll use PDO::FETCH_NUM to tell fetch that we want a numerically indexed array, then we'll take the two resulting entries in the array and stuff them in $db_user_id and $db_password, using them where the old code called mysql_result. PDO doesn't let you pick and choose different columns from the same row using fetchColumn.

If you wanted to use the mysqli extension instead, there's a bit more work to be done. I hope I've convinced you to use PDO instead, though!

The rest of the class is, eh, adequate. Make sure to adjust the $domain class variable at the top to match your actual domain name, it's used in the cookies.

Charles
`+1` for addressing the security concerns, as well as suggesting the use of the newer `mysqli` extension.
Delan Azabani