patternphpMinor
User Authentication/Permissions for PHP MySQL CRUD
Viewed 0 times
permissionsuserphpauthenticationmysqlforcrud
Problem
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
The Form
If this is a new user or an existing user with no permissions set then
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.
Check for Permission
The next steps I haven't completed yet but basically it will pull
Am I heading in the right direction with this or am I over complicating?
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";
for($z=0; $z";
$form .= "";
}
}
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 .= Personnel::set_permissions_form($current_permissions);
$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?
Solution
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.
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.
Double quotes should be used for strings that require PHP processing.
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.
Sanitizing and Validating
You shouldn't handle raw POST data. Always sanitize and validate it. Check out PHP's
Comparisons
I would use absolute equality comparisons when checking for current permissions rather than loose comparison as your
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 . ':';Double quotes should be used for strings that require PHP processing.
$form .= "$table_name:";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 . ": ][]"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\"" : "");Code Snippets
foreach(self::$table_names as $table_name) {
//etc...
}$form .= $table_name . ':<br />';$form .= "$table_name:<br />";$form .= $action . ": <input type=\"checkbox\" name=\"permissions[$table_name][$action ]\"";
//OR
<input type="checkbox" name="permissions[<?php echo $table_name; ?>][<?php echo $action; ?>]"$form .= ($current_permissions[$table_name][$action] === TRUE ? " checked =\"checked\"" : "");Context
StackExchange Code Review Q#12092, answer score: 2
Revisions (0)
No revisions yet.