Jump to content

Commenting Script


larsson719

Recommended Posts

Hello all. I'm fairly new to PHP and MySQL; I've been experimenting with the two using a very simple commenting script and a very simple registration/login script. I have fiddled around with the scripts and I can get them to work separately. This is good, since I have no prior experience with either languages but I'm hoping to be able to make the two work together as a whole.To help you understand the problem I'll use a simple XHTML/CSS-driven website as an example. The website contains some articles the webmaster would like to add a comments feature to. In order to add a comment the user has to fill out a form with the following fields: Name, E-Mail(optional), and the Comment Box. After some time the webmaster realizes there is too much spam and wants to make the comments feature available only to authenticated users.Now we get to the point where I am stuck. I have found a simple, yet effective, user registration system which can be found here. I have even fiddled around with the script and taken off some unnecessary features(yes it still functions properly). My only problem now is to somehow make the two systems work together as a whole. What I am hoping to achieve by doing this is:

  • Hiding the comment form from unregistered users
  • Replace the form with text instructing the user to register
  • Registered members will be to see the comment and comment

Although I don't have the PHP knowledge to be able to achieve this I do have one idea that may prove to be helpful to whomever chooses to help me. Perhaps a possible solution to hiding the comment form from an unregistered user is to make the comment script check the client's cookies/session(whichever one tells systems the user is authenticated) and to query the registration database to see if the user/pass combination has a match.There is something that may hinder this method and that is that I am not sure if you can connect to two databases at the same time. The comments script has its own database to connect to aside from the registration system. If that is not a problem, then afterward a simple if-statement can decide whether to display the comment form, or to display text instructing the user to register.Sorry for all of the text, just wanted to give as much information as possible in one post in order to help you understand the problem better. At this point you may be wondering about the link to the comment script. Unfortunately the website I got it from is no longer functioning. I do have my copy which I have tweaked a lot but the basic functions and every other important part remain intact. If the information I have provided is not enough just let me know and I will gladly post snippets of the script or if you'd like me to upload it somewhere and link you.

Link to comment
Share on other sites

There is something that may hinder this method and that is that I am not sure if you can connect to two databases at the same time. The comments script has its own database to connect to aside from the registration system.
I'd suggest putting the comment script tables in the registration database, unless there's some reason that can't be done. The rest of it should work as you proposed- have the login/registration script check to see if the user is logged in and display the comment form only if they are.
Link to comment
Share on other sites

The normal way is:

  • logging in sets session or cookie variables
  • check for the presence of these and only display comments and the button to go to the 'add comment' form if they are set
  • on the add comment form page, check for the variables - if they aren't set, redirect the user back whence they came / have them register - this also prevents people navigating straight to the comment form, which will be useless as it will lack paramaters identifying what is being commented on

I don't think it's necessary to check the validity of the cookie/session values every time - their presence is enough. Well, there are probably ways of manipulating cookies, but session variables are stored on server, so they are pretty secure - if they're there, you set them.You can use more than one database, though more likely is that you have multiple tables in the one database, which is better because you're less likely to confuse them and use the wrong connection information.

Link to comment
Share on other sites

I don't think it's necessary to check the validity of the cookie/session values every time - their presence is enough. Well, there are probably ways of manipulating cookies, but session variables are stored on server, so they are pretty secure - if they're there, you set them.
There are lots of ways to steal cookies and hijack or steal sessions. I'd opt for checking that the login is valid every time, but then I'm more paranoid than most people. I think relying on just the presence of the variables or values is a classic way for a malicious user to be able to spoof their way in. For example:
if($admin){   // allow access to admin functions}else{   // don't allow access to admin functions}

This code assumes that the $admin var can only originate from a cookie, session, or input form that's legitimate, i.e. that a malicious user has no control of. However, that's a fatal misconception. With register_globals enabled(), injecting bogus into the $admin variable is as easy as calling the script like so: script.php?admin=1And there are other ways as well that don't rely on register_globals().

Link to comment
Share on other sites

That's a good point, I guess. In my scripts, I check for the presence of certain variables, but they are always used to fetch DB information - if the value isn't valid, the user will get an error message and nothing malicious will happen. For admin privileges I do check the value.

Link to comment
Share on other sites

Okay, thanks for all the help guys. I'm going to try this out by combing the two databases together in one. Are there any guides covering similar systems like this? I've done my fair share of google searches but I'm just not sure how to word it properly. At this point all I would like to know is how to check the validity of the user in PHP.

Link to comment
Share on other sites

To check the validity of the user, presuming you are accessing your MySQL table using the mysqli commands (I've always used them, and not the older mysql family), you need to do something like:

if(isset($_COOKIE["username"]) || isset($_SESSION["username"])) { //or whatever else your cookie/session variable might be called$username = isset($_COOKIE["username"]) ? $_COOKIE["username"] : $_SESSION["username"];$check_sql = "SELECT * FROM table WHERE username = '$username'";$check_res = mysqli_query($connect, $check_sql) or die("MySQL error: ".mysqli_error($connect));if(mysqli_num_rows($check_res) == 0) { //user isn't recognized - redirect or something, and terminate script} else  if(mysqli_num_rows == 1){ //user is recognized}}

Link to comment
Share on other sites

If you're talking about combining the tables in to a single database, just copy the tables from one db to the other, but first check to make sure that none of them have the same name. If one or more of them do, you'll need to rename the tables so they have different names (and then change the code that references those tables to use the new names).

Okay, thanks for all the help guys. I'm going to try this out by combing the two databases together in one. Are there any guides covering similar systems like this? I've done my fair share of google searches but I'm just not sure how to word it properly. At this point all I would like to know is how to check the validity of the user in PHP.
Link to comment
Share on other sites

Okay so I decided to create a sample webpage to test this out on. I also chose to try for a simpler login/registration script to try and get a better understanding of how PHP works in conjunction with mySQL (or SQL). So I setup the sample website using a PHP-SWITCH script I found a while back. I've used this script a lot since it helps me manage the pages easier:

<?php if (! isset($_GET['page'])) {						include('home.php');					} else {    						$page = $_GET['page'];  						switch($page) {							case 'register_form':								include('includes/user_system/register_form.php');								break;  						}					}					?>

On the registration page the form links to my "register.php" which checks the validity of the form and to check for any blank fields and so on. "register.php" is supposed to refresh the page and add a reason to what the user did wrong when submitting the form.On my "register_form.php" page, which holds the actual form. This field is hidden until the user makes a mistake.

<?php if (isset($reg_error)) { ?><span class="red"><?php echo $reg_error; ?></span>, please try again.<?php } ?>

My "register.php" checks the form for all the errors. Here's the bit of code that will refresh the page with the reason for the error:

// Check if any of the fields are missing if (empty($_POST['username']) || empty($_POST['password']) || empty($_POST['confirmpass'])) { // Reshow the form with an error $reg_error = 'One or more fields missing'; include 'register_form.php';

Now after I submit the form without any fields filled out I get the error code, but it refreshes to the actual "register_form.php". The problem with this is that because of my PHP-SWITCH script (helps me manage the site a lot easier) I don't have any formatting on that page. So when it refreshed I just get a blank html page without any formatting or anything, since it refreshes to the actual "register_form.php". The URL to my "register_form.php" would be: "index.php?page=register_form". Now I have tried several different things such as changing it to:

include 'index.php?page=register_form'

And also changing it to:

header(location : 'index.php?page=register_form')

Unfortunately all this does is refresh the page without the reason for the error. I'm sure this can be easily solved by just adding a Javascript Validator but I'd like to know if it is possible to refresh the page with the error using either "include" or "header()" or something else without having to remove the PHP-SWITCH script.

Link to comment
Share on other sites

When you are sending the location header in the event of an error, add an extra parameter to the URL describing the error. For example:

header(location : 'index.php?page=register_form&reg_error=whatever_the_error_is')

Then just change the if condition surrounding your error text span to:

<?php if (isset($_GET["reg_error"])) { ?><span class="red"><?php echo $_GET["reg_error"]; ?></span>, please try again.<?php } ?>

Note that this will echo whatever the value of the reg_error parameter is, which isn't a security concern but people can play with it and enter any value they like.

Link to comment
Share on other sites

I updated the if-statement on the page holding the form. I went onto the page that processes the form and added the header but I get the following:

Parse error: syntax error, unexpected ':' in L:\Utilities\www\xampplite\htdocs\_sessions\register.php  on line 14

I don't understand how the syntax is wrong. I looked it up on the PHP website's manual and that is how it is written out. Could the error be cause by something else on that page? I remember reading on the manual that a header statement needed to be the very first thing on a page.. could this possible be the cause for the error?

<?php	// Include init file	include 'connect_db.php';	if (!isset($_POST['submit'])) {		// Show the form		include 'index.php?page=register_form';		exit;	}	else {		// Check if any of the fields are missing		if (empty($_POST['username']) || empty($_POST['password']) || empty($_POST['confirmpass'])) {			// Reshow the form with an error			$reg_error = 'One or more fields missing';			header(Location : 'index.php?page=register_form&reg_error=empty');			exit;		}		// Check if the passwords match		if ($_POST['password'] != $_POST['confirmpass']) {			// Reshow the form with an error			$reg_error = 'Your passwords do not match';			include '?page=register_form';			exit;		}		// Everything is ok, register		user_register ($_POST['username'], $_POST['password']);		echo 'Thank you for registering on our site, <a href="?id=home">Click Here</a> to go back.';	}?>

I'm not exactly sure what I should do with the beginning include. Should I change that to a header as well? Don't worry about the rest of the code after the second if-statement. I will eventually change it when I'm able get this past this problem.

Link to comment
Share on other sites

:) its always the simple mistakes i overlook.. sorry about that! Thanks it works now. Redirects back to the page with the form and displays the if-statement.
<?php if (isset($_GET["reg_error"])) { ?>								<span class="red"><?php echo $_GET["reg_error"]; ?></span>, please try again.							<?php } ?>

Instead of typing out:

header("Location:../../index.php?page=register_form&reg_error=error_here");

I used the following:

 if (empty($_POST['username']) || empty($_POST['password']) || empty($_POST['confirmpass'])) {			// Reshow the form with an error			$reg_error = 'One or more fields missing';			header(Location : 'index.php?page=register_form&reg_error=$reg_error');			exit;		}

It works fine now.. but are there any issues with this? In the address bar it displays as:http://localhost/_sessions/index.php?page=register_form&reg_error=One or more fields missing.Is this a bad thing? I've always thought that it is bad to have spaces in your URLs. Is there anyway I can automatically convert:http://localhost/_sessions/index.php?page=register_form&reg_error=One or more fields missingto:http://localhost/_sessions/index.php?page=register_form&reg_error=One%20or%20more%20fields%20missingor something like that? If not, as long as it isn't such a bad thing then I'm okay with it. No worries. Thanks for all your help guys. If I run into anymore problems I'll let you know!EDIT

You can use urlencode or rawurlencode to make a string safe for a URL. In terms of security, the only issues there are if someone links to your site and uses that variable to inject some Javascript code on the page which would make the user think that they are giving information to your site when the could be giving it to someone else. The Javascript code could also embed malicious things on the page. This attack requires that someone clicks a malicious link on another site or email though.
That's exactly what I meant.. making a string safe for a URL. Thanks a lot I will look into this!
Link to comment
Share on other sites

You can use urlencode or rawurlencode to make a string safe for a URL. In terms of security, the only issues there are if someone links to your site and uses that variable to inject some Javascript code on the page which would make the user think that they are giving information to your site when the could be giving it to someone else. The Javascript code could also embed malicious things on the page. This attack requires that someone clicks a malicious link on another site or email though.

Link to comment
Share on other sites

Archived

This topic is now archived and is closed to further replies.

×
×
  • Create New...