Whats wrong?

[jwk811]

I am creating a membership and got it so that someone can register and the info goes to a database.. after that they recieve an email and are given a link to click to activate their account. Heres the acivate.php file.
[code]<?
/* Account activation script */

// Get database connection
include 'db.php';

// Create variables from URL.

$userid = $_REQUEST['id'];
$code = $_REQUEST['code'];

$sql = mysql_query("UPDATE users SET activated='1' WHERE userid='$userid' AND password='$code'");

$sql_doublecheck = mysql_query("SELECT * FROM users WHERE userid='$userid' AND password='$code' AND activated='1'");
$doublecheck = mysql_num_rows($sql_doublecheck);

if($doublecheck == 0){
    echo "<strong><font color=red>Your account could not be activated!</font></strong>";
} elseif ($doublecheck > 0) {
    echo "<strong>Your account has been activated!</strong> You may login below!<br />";
    include 'login.php';
}

?>[/code]
For some reason everytime I do it, it will say "Your account could not be activated!". And I can see that in the if else statment its double checking to make sure that the activated cell in the table on my database is set to 1, but why isn't is setting it to 1? That's why I'm getting the error? Would you happen to know why? I'm connecting to the database and everything okay, is there something wrong with this script? Any help would be great!

[alpine]

I've adjusted it a bit, see what you get out of this one:
[code]

<?php

if(!empty($_GET['id'] && !empty($_GET['code'])))
{
include 'db.php';

$userid = $_GET['id'];
settype($id,"integer");
$code = htmlspecialchars($_GET['code']);

$sql_check = mysql_query("SELECT * FROM users WHERE userid='$userid' AND password='$code'") or die(mysql_error());
if(mysql_num_rows($sql_check)==1)
{
$sql_update = mysql_query("UPDATE users SET activated='1' WHERE userid='$userid' AND password='$code'") or die(mysql_error());
if(mysql_affected_rows()==1)
{
echo "<strong>Your account has been activated!</strong> You may login below!<br />";
include 'login.php';
}
else
{
echo "<strong><font color=red>Your account could not be activated at this time!</font></strong>";
}
}
else
{
echo "<strong><font color=red>No account found matching the submitted activation data</font></strong>";
}
}
else
{
echo "<strong><font color=red>Missing nessesary activation data</font></strong>";
}

?>

[/code]

But, i wouldn't recomend you using the userid AND the password so wide open like that, if you want to use the password i would at least encrypt it with md5() or sha1() in the table AND in the activation email etc.

[printf]

Your not testing the query for errors, plus your not stopping anyone from reavtivating that already activated. Plus your not validating the dangerous inputs!

If you want to do it your way then do something like this!

[code]<?

/* Account activation script */

// Get database connection

include ( './db.php' );

// Create variables from URL.

// first check if it's already been activated

$sql = mysql_query ( "SELECT COUNT(*) AS total FROM users WHERE userid = '" . mysql_real_escape_string ( $_REQUEST['id'] ) . "' AND password = '" . mysql_real_escape_string ( $_REQUEST['code'] ) . "' AND activated = 1" ) or die ( 'Query Error: ' . mysql_error );

$found = mysql_ftech_assoc ( $sql );

if ( $found['total'] == 0 )
{
$sql = mysql_query ( "UPDATE users SET activated = 1 WHERE userid = '" . mysql_real_escape_string ( $_REQUEST['id'] ) . "' AND password = '" . mysql_real_escape_string ( $_REQUEST['code'] ) . "'" ) or die ( 'Query Error: ' . mysql_error );

if ( mysql_affected_rows ( $sql ) == 0 )
{
echo "<strong><font color='red'>Your account could not be activated, no user found by that id or password!</font></strong>";
}
else
{
echo "<strong>Your account has been activated!</strong> You may login below!<br />";

    include ( './login.php' );
}
}
else
{
echo "<strong>You have already activated your account!</strong> You may login below!<br />";

    include ( './login.php' );
}

?>[/code]


But I wouldn't do that, I would have (2) tables, one that holds the activated users and the other that holds the users awaiting activation, this way you don't add user to the user table that may never activate. But more importantly you would need only (1) query for activation, instead of this way which needs (2). It all about control logic!

me!

[jwk811]

okay thanks, ill just something like that.. how could i combine the two to make it so it does all that stuff? and what did you mean about the password thing? ill fairly new to php.