Database page critique requested

Ryodox

This is a set of pages I made to keep track of membership of a Cyber Nations alliance. I'd like some critique on it; especially security-wise.

membership.php:

<?php
	include('template.html');
	include('mysql_connect.php');

print "<H2><SPAN STYLE=\"color: darkred;\">&deg;</SPAN> (Alliance removed) Membership List</H2>\n";

if ($_COOKIE['auth'] == 2)
	print "<P>Logged in with administrator permissions. (<A HREF=\"login.php?action=logout\">Log out</A>)</P>\n";
elseif ($_COOKIE['auth'] == 1)
	print "<P>Logged in with viewing permissions. (<A HREF=\"login.php?action=logout\">Log out</A>)</P>\n";

if (($_COOKIE['auth'] >= 1))
{
	$result = mysql_query("SELECT * FROM `membership` ORDER BY `forum_handle` ASC");

	if ($result)
	{
		if (mysql_num_rows($result) != 0)
		{
			$members_total = mysql_num_rows($result);
			$members_inactive = mysql_fetch_row(mysql_query("SELECT COUNT(`status`) FROM `membership` WHERE `status` = 'inactive'"));
			$members_inactive_percent = 100 - round($members_inactive[0] / $members_total * 100);

			print "<SCRIPT TYPE=\"text/javascript\">\n"
			."function del_confirm(member)\n"
			."{\n"
			."\tvar r = confirm('Delete member '+member+'?');\n"
			."\tif (r == true) self.location = 'membership_edit.php?action=delete&member='+member+'';\n"
			."}\n"
			."</SCRIPT>\n"
			."\n"
			."<HR color=\"#BDBDBD\" />\n"
			."<P>Total members: <B>".$members_total."</B>\n"
			."<BR />Inactive members<A HREF=\"#note_0\">*</A>: <B>".$members_inactive[0]."</B> (".$members_inactive_percent."% activity rating)</P>\n";
			if ($_COOKIE['auth'] == 2) print "<P><BUTTON onClick=\"window.location='membership_edit.php?action=add'\">Add a member</BUTTON></P>\n";
			print "\n"
			."<TABLE BORDER=\"0\" CELLPADDING=\"4\" CELLSPACING=\"1\" WIDTH=\"100%\">\n"
			."\t<TR>\n"
			."\t\t<TH WIDTH=\"15%\">Forum handle</TD>\n"
			."\t\t<TH WIDTH=\"5%\">Rank</TD>\n"
			."\t\t<TH WIDTH=\"15%\">Title</TD>\n"
			."\t\t<TH WIDTH=\"25%\">Nation (Ruler)</TD>\n"
			."\t\t<TH WIDTH=\"5%\">Team colour</TD>\n"
			."\t\t<TH COLSPAN=\"2\" WIDTH=\"10%\">Resources</TD>\n"
			."\t\t<TH WIDTH=\"5%\">Past alliances</TD>\n"
			."\t\t<TH WIDTH=\"10%\">Referrer</TD>\n"
			."\t\t<TH WIDTH=\"5%\">Edit / Delete</TD>\n"
			."\t</TR>\n";

			while ($row = mysql_fetch_array($result))
			{
				print "\t<TR>\n";
				print "\t\t<TD ALIGN=\"left\" WIDTH=\"15%\"><A ";
				if (($row['status'] == "inactive") || ($row['status'] == "awl")) print "CLASS=\"inactive\" ";
				print "HREF=\"http://forumurl.com/index.php?showuser=".$row['forum_id']."\" TARGET=\"_new\" TITLE=\"View forum profile\">".$row['forum_handle']."</A>";
				if ($row['status'] == "awl") print "*";
				print "</TD>\n"
				."\t\t<TD WIDTH=\"5%\">";
				if ($row['rank'] == "Triumvir") print "<SPAN STYLE=\"color: #990000; font-weight: bold;\">".$row['rank']."</SPAN>";
				elseif ($row['rank'] == "Officer") print "<SPAN STYLE=\"font-weight: bold;\">".$row['rank']."</SPAN>";
				else print $row['rank'];
				print "</TD>\n"
				."\t\t<TD WIDTH=\"15%\">".$row['title']."</TD>\n"
				."\t\t<TD WIDTH=\"25%\"><A HREF=\"http://www.cybernations.net/nation_drill_display.asp?Nation_ID=".$row['nation_id']."\" TARGET=\"_new\" TITLE=\"View nation\">".$row['nation_name']."</A> (<A HREF=\"http://www.cybernations.net/compose2.asp?Nation_ID=".$row['nation_id']."\" TARGET=\"_new\" TITLE=\"Send message\">".$row['nation_ruler']."</A>)</TD>\n"
				."\t\t<TD ALIGN=\"center\" WIDTH=\"5%\"><B><IMG ALT=\"".$row['nation_colour']."\" SRC=\"http://www.cybernations.net/images/teams/team_".$row['nation_colour'].".GIF\" /></TD>\n"
				."\t\t<TD ALIGN=\"center\" WIDTH=\"5%\"><IMG ALT=\"".$row['nation_resource1']."\" SRC=\"http://www.cybernations.net/images/resources/".$row['nation_resource1'].".GIF\" /></TD>\n"
				."\t\t<TD ALIGN=\"center\" WIDTH=\"5%\"><IMG ALT=\"".$row['nation_resource2']."\" SRC=\"http://www.cybernations.net/images/resources/".$row['nation_resource2'].".GIF\" /></TD>\n"
				."\t\t<TD WIDTH=\"5%\">".$row['pastalliances']."</TD>\n"
				."\t\t<TD WIDTH=\"10%\">".$row['referrer']."</TD>\n"
				."\t\t<TD ALIGN=\"center\" WIDTH=\"10%\"><BUTTON onClick=\"window.location='membership_edit.php?action=edit&id=".$row['forum_id']."'\""; if ($_COOKIE['auth'] == 1) print " DISABLED"; print ">Edit</BUTTON>&nbsp;&nbsp;<BUTTON onClick=\"del_confirm('".$row['forum_handle']."')\""; if ($_COOKIE['auth'] == 1) print " DISABLED"; print ">X</BUTTON></TD>\n"
				."\t</TR>\n";
			}
			print "</table>\n";
			mysql_free_result($result);
		}
	}
	print "<P><B>Notes</B>\n"
	."<BR /><A NAME=\"note_0\"><SUP>0</SUP> An inactive member is defined as one whose most recent activity on the forums is two weeks ago or older.  These members are not necessarily inactive in Cyber Nations.</A>\n"
	."<BR /><A NAME=\"note_1\"><SUP>1</SUP> A nation marked with an asterisk (*) is considered absent with leave.</A></P>\n";
	if ($_COOKIE['auth'] == 2) print "<P><BUTTON onClick=\"window.location='membership_edit.php?action=add'\">Add a member</BUTTON></P>\n";
}
else
{
?>

<FORM ACTION="login.php" METHOD="post">
<TABLE ALIGN="center" BORDER="0" CELLPADDING="4" CELLSPACING="1" WIDTH="50%">
	<TR>
		<TH ALIGN="center" COLSPAN="2" WIDTH="100%">Authorize</TD>
	</TR>
	<TR>
		<TD WIDTH="50%">Authorization key:</TD>
		<TD WIDTH="50%"><INPUT NAME="auth" TYPE="password" /></TD>
	</TR>
	<TR>
		<TD ALIGN="center" COLSPAN="2" WIDTH="100%"><INPUT TYPE="submit" VALUE="Enter" /></TD>
	</TR>
</TABLE>
</FORM>

<?
	}

include("template-end.html");
?>

<?
	include('keys.php'); // File containing auth keys for log-in

if (($_POST['auth'] == addslashes($authkey_1)) || ($_POST['auth'] == addslashes($authkey_2)))
{
	$_POST['auth'] == addslashes($authkey_2) ? setcookie("auth", 2, time()+3600, "/") : setcookie("auth", 1, time()+3600, "/");
	print "<SCRIPT TYPE=\"text/javascript\">alert('Successfully authorized.');\n"
	."self.location = 'membership.php';</SCRIPT>";
}
elseif ($_GET['action'] == "logout")
{
	setcookie("auth", "", time()-3600, "/");
	print "<SCRIPT TYPE=\"text/javascript\">alert('Logged out.');\n"
	."self.location = 'membership.php';</SCRIPT>";
}
else
	print "<SCRIPT TYPE=\"text/javascript\">alert('Incorrect password.');\n"
	."self.location = 'membership.php';</SCRIPT>";
?>

Membership_edit.php is a bit long, but it does require the auth cookie to be set to a level of 2. I'll post this file as well upon request.

Piranha

I have a few things.

Don't use cookies for user type. Cookies is stored on the users computer, and can easily be changed. That way it is easy for anyone to get admin priviledges. Use sessions instead, they are stored on the server.
Don't use SELECT . Instead only select what you want, it saves resources.
For debugging purposes always set a variable and then run the query, like this:
```
$sql = "SELECT something FROM somewhere";
$result = mysql_query($sql);
```
It is good that you try to be safe from sql injection. However you should not use addslashes since it is not foolproof. It is better to use mysql_real_escape_string if you run mysql. It also seems like you have magic quotes turned on, turn that off.
What is authkeys? Please don't tell me that it is static passwords that is stored in a file, and that everyone should use one of those passwords. It may be possible to read that file from a browser as well, but it is not sure. It is much better to use a combination of username and password, the easiest way is probably to set up a database table with 3 columns, username, password and authorization.
Don't use capitals in your html tags. Some years ago it should be capitals, but for the code to be compatible with XHTML it should be small letters.
```
// This is ok
<a href="test.php">
<a href="Test.php">
// This is not ok
<A href="test.php">
<a HREF="test.php">
```

Weedpacket

if ($row['rank'] == "Triumvir") print "<SPAN STYLE=\"color: #990000; font-weight: bold;\">".$row['rank']."</SPAN>";
elseif ($row['rank'] == "Officer") print "<SPAN STYLE=\"font-weight: bold;\">".$row['rank']."</SPAN>";
else print $row['rank'];

If you had classes that matched the rank you could reduce this to a single line:

echo "<span class=\"$row[rank]\">$row[rank]</span>";

And this code would not change should you later decide to mark up other ranks in some way; you'd just add the new style to the stylesheet under the appropriate class. (The class name need not be exactly the same as the rank; prefixes and/or suffixes could be used as well: "table$row['rank']" would produce, among other things, "tableOfficer" as a class name.)

While we're on the subject of using styles, they could be used to specify things like cell spacing, padding, borders, and widths, as well.

You may also find it easier to edit the HTML in membership.php if you don't have it inside multiple print statements. First, string literals are allowed to span multiple lines; and second, heredoc syntax<<<EOF
where the string is delimited by a custom sequence of characters identified when the string begins by the "<<<" sequence, and continues until the
EOF
sequence appears again makes it unnecessary to \escape either ' or" characters. (Of course, if the string is just being output and doesn't involve any additional PHP, then escaping out of PHP entirely for the duration is a third option.)

As Piranha already noted, using cookies to store authorisations means that you can't trust the authorisations you receive in the cookies. That said,

$_POST['auth'] == addslashes($authkey_2) ? setcookie("auth", 2, time()+3600, "/") : setcookie("auth", 1, time()+3600, "/");

could be shortened a bit to

setcookie("auth", ($_POST['auth'] == addslashes($authkey_2) ? 2 : 1), time()+3600, "/");

And why do the $authkey variables all have addslashes around them? If they came from a file you wrote, why not use addslashes() when you're writing the file so that you don't need to keep doing at again when reading it? Assuming you even need addslashes for some reason - which at present is unexplained.

I also note

if (mysql_num_rows($result) != 0)
            {
                $members_total = mysql_num_rows($result);

Which would be slightly more efficient as

$members_total = mysql_num_rows($result);
if($members_total!=0)
{

Ryodox

Thank you all very much for this, I will take note and make necessary changes/corrections.