User Authentication/Permissions for PHP MySQL CRUD

Question

I took my first stab at a user authentication function. I invented my own way of doing this. Is this a safe and efficient way of giving a user Read/Edit/Delete permission on tables in a MySQL database?

The Class

class Personnel {
//set tables that will need permissions and actions
protected static $actions = array("read", "edit", "delete");
protected static $table_names = array("species", "users", "news");
//this will generate a form with checkbox for each position in the permissions array
public static function set_permissions_form($current_permissions) {
 for($i=0; $i<count(self::$table_names); $i++) {
 $table_name = self::$table_names[$i];
 $form .= $table_name . ":<br />";
 for($z=0; $z<count(self::$actions); $z++) {
 $action = self::$actions[$z];
 $form .= $action . ": <input type='checkbox' name='permissions[" . $table_name . "][" . $action . "]'";
 if($current_permissions) {
 $form .= ($current_permissions[$table_name][$action] == "1" ? " checked =\"checked\"" : ""); 
 }
 $form .= " value=1 />";
 $form .= "<br />";
 }
 }
 return $form;
}
}

The Form

If this is a new user or an existing user with no permissions set then $current_permissions will be NULL. If a permissions array does exist in the database unserialize it and check any checkbox when name = array key.

$current_permissions = unserialize($personnel->permissions);
$form = "<form action='save_personnel.php' method='post'>";
$form .= Personnel::set_permissions_form($current_permissions);
$form .= "<input type='submit' value='Save Changes' />";
$form .= "</form>";
echo $form;

Save

Once the form is submitted each box checked will be a "1" in the permissions array. This next snippet serializes the array and stores it in the database. I did not include code for $personnel->save() but you know what it does. Create user if does not exist, update user if it does.

$personnel = new Personnel();
$personnel->permissions = serialize($_POST['permissions']);
if($personnel->save()) {
 redirect_to("manage_personnel.php");
}

Check for Permission

The next steps I haven't completed yet but basically it will pull $personnel->permissions from database and unserialize it. Then say it is on a page where user is trying to edit species, $permissions["species"]["read"] == '1' ? you shall pass : you shall not pass;.

Am I heading in the right direction with this or am I over complicating?

Answer 1

Loops

Why are you using a for loop? I would just use a foreach loop, its much more efficient, especially for what you are doing here.

foreach(self::$table_names as $table_name) {
 //etc...
}

Strings

The following are micro-optimizations, but ones that are usually accepted as standard practice.

Single quotes should be used if no PHP processing is to be performed on a string. In other words, there are no PHP variables in the string. This is a micro optimization because it doesn't really add any noticeable overhead to your script. However, doing this you'll notice that you wont have to escape quotes nearly as frequently as you would have to if you started with single quotes. This is especially handy when dealing with HTML or XML attributes, which I'll get into soon.

$form .= $table_name . ':<br />';

Double quotes should be used for strings that require PHP processing.

$form .= "$table_name:<br />";

HTML/XML Attributes

While most browsers accept it, it is still improper syntax to use single quotes on HTML/XML tag attributes. XML will actually produce errors. Either escape the double quotes or escape the string and only go back into PHP for the variables.

$form .= $action . ": <input type=\"checkbox\" name=\"permissions[$table_name][$action ]\"";
//OR
<input type="checkbox" name="permissions[<?php echo $table_name; ?>][<?php echo $action; ?>]"

Sanitizing and Validating

You shouldn't handle raw POST data. Always sanitize and validate it. Check out PHP's filter_input() function. That goes right along with what Shaad said about passing serialized data to your tables too. Its not a good idea, especially if it hasn't been sanitized and validated.

Comparisons

I would use absolute equality comparisons when checking for current permissions rather than loose comparison as your $actions table should only have boolean values. Also the quotes around an integer is unnecessary, unless you are comparing a string with absolute equality.

$form .= ($current_permissions[$table_name][$action] === TRUE ? " checked =\"checked\"" : ""); 

• Jonathan Eckman: Thanks this was very helpful. I am attempting to redo using a table to store permissions instead of an array but am stuck on the logic of getting the selected permissions from the submitted form to the table. The key is that the # of permissions and tables that they control must be changed in one spot.

Answer 2

I think storing the serialized php structure to the database is not a good practice.

You can make a separate table like this

permissions
 user_id 
 table_id
 read - boolean
 update - boolean
 delete - boolean

Three boolean values ​​can be easily encoded into int(1), but I do not think it's worth doing.

and this

editable_tables
id - index
name - real table name or pattern like '*'

when editing in php you can show all the tables from editable_tables. store data in the permissions to only those who have rights.