#1
  1. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jan 2006
    Posts
    122
    Rep Power
    9

    Small daily checkin script


    I trying to add this to my login form as it has to detect when the user was last logged in and if it was 1 day after the last day they signed in the purpose is to update mysql and add 1 to the specified field. Am i going about this the right way?

    as far as i can see the dates are being calculated correctly but for some reason the field is not being updated.

    is there a better way to do this. pls help

    Code:
    $sql = mysql_query("SELECT * FROM `assignments` WHERE userid='$session_id'");
    		
    		while($row == mysql_fetch_array($sql)){
    		if ($row['last_logged_in'] == date('Y-m-d',strtotime("-1 days",strtotime(date('Y-m-d', time()))))){
    				$incscore = $row['dailycheckin'];
    				$incscore++;
    				$sql = "UPDATE `assignments` SET `dailycheckin` = ".$incscore." WHERE userid=".$row['userid']."";
    				$retval = mysql_query($sql);
    			if(! $retval ){
      				die('Could not update data: ' . mysql_error());
    			}
    		}elseif (date('Y-m-d',strtotime("-1 days",strtotime($row['last_logged_in']))) != $row['last_logged_in']){
    			$sql = "UPDATE `assignments` SET `dailycheckin` = 0  WHERE userid=".$row['userid']."";
    
    			$retval = mysql_query($sql);
    			if(! $retval ){
     			die('Could not update data: ' . mysql_error());
    			}
    	}
    }
  2. #2
  3. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,957
    Rep Power
    1046
    Hi,

    do the counter logic with SQL and not PHP. If you just want to check for any login within the last day, use
    Code:
    SELECT EXISTS (
    	SELECT
    		1
    	FROM
    		`assignments`
    	WHERE
    		`last_logged_in` >= CURDATE() - INTERVAL 1 DAY
    )
    ;
    and then update the counter accordingly.

    There are several other issues:
    • Escape all values you insert into a query string. In this particular case you can probably rely on the values actually being integers, but don't risk it. All values must be escaped using mysql_real_escape_string() and wrapped in quotes.
    • Don't use "SELECT *". It's inefficient, insecure and can lead to strange errors. Specify the concrete columns you want to fetch.
    • Don't output errors messages revealing internal information. This will make attackers happy and irritate legitimate users.


    You should generally consider dropping the obsolete mysql_ functions and switching to one of the modern APIs. These allow prepared statements to prevent SQL injections.
  4. #3
  5. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jan 2006
    Posts
    122
    Rep Power
    9
    ok thx for the input

    Originally Posted by Jacques1
    Hi,

    do the counter logic with SQL and not PHP. If you just want to check for any login within the last day, use
    Code:
    SELECT EXISTS (
    	SELECT
    		1
    	FROM
    		`assignments`
    	WHERE
    		`last_logged_in` >= CURDATE() - INTERVAL 1 DAY
    )
    ;
    and then update the counter accordingly.

    There are several other issues:
    • Escape all values you insert into a query string. In this particular case you can probably rely on the values actually being integers, but don't risk it. All values must be escaped using mysql_real_escape_string() and wrapped in quotes.
    • Don't use "SELECT *". It's inefficient, insecure and can lead to strange errors. Specify the concrete columns you want to fetch.
    • Don't output errors messages revealing internal information. This will make attackers happy and irritate legitimate users.


    You should generally consider dropping the obsolete mysql_ functions and switching to one of the modern APIs. These allow prepared statements to prevent SQL injections.
  6. #4
  7. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jan 2006
    Posts
    122
    Rep Power
    9

    Almost there...


    I am making progress so i took your advice and converted my code to mysqli. i ran into a problem along the process and i dont know where it happend or why its not working everything was fine untill i added the last part of code to finish it up and it doesnt want to work..

    Nothing happens after the "//Check for assignment lvl up and if progress is greater then 7 award lvl point" comment . before i converted the code i also tested the mysql query in phpmyadmin and replaced the vars with numbers and the field was updated. can anyone see what could be giving me this problem? even if i die(mysql_error()); on each query no errors are returned.

    Code:
    <?php
     include_once("/scripts/global.php");
    //Check Last Day is if was yesterday add 1 to assignment progress
     $sql    = "SELECT * FROM `assignments` WHERE `userid` = $session_id ";
     $result = mysqli_query($GLOBALS["___mysqli_ston"],  $sql );
     $row    = mysqli_fetch_assoc( $result );
     if ( $row['userid'] == $session_id )
       {
         $sql2    = "SELECT * FROM `members` WHERE `id` = $session_id ";
         $result2 = mysqli_query($GLOBALS["___mysqli_ston"],  $sql2 );
         $row2    = mysqli_fetch_array( $result2 );
         if ( $row2['last_logged_in'] !== date( 'Y-m-d', time() ) )
           {
    //Failed dailylogin assignment reset counter to 1st day
             $sql     = "UPDATE `members` SET `last_logged_in` = '" . date( 'Y-m-d', time() ) . "' WHERE `last_logged_in` >= CURDATE() - INTERVAL 1 DAY";
             $retval1 = mysqli_query($GLOBALS["___mysqli_ston"],  $sql );
             if ( !$retval1 )
               {
                 $sql     = "UPDATE `assignments` SET `dailycheckin` = 1 WHERE `userid` = $session_id";
                 $retval2 = mysqli_query($GLOBALS["___mysqli_ston"],  $sql );
                 $sql     = "UPDATE `members` SET `last_logged_in` = '" . date( 'Y-m-d', time() ) . "' WHERE `id` = $session_id";
                 $retval3 = mysqli_query($GLOBALS["___mysqli_ston"],  $sql );
               }
             elseif ( $retval1 )
               {
    //Award 1 progrssion point
                 $sql      = "UPDATE `members` SET `last_logged_in` = '" . date( 'Y-m-d', time() ) . "' WHERE `id` = $session_id";
                 $retval4  = mysqli_query($GLOBALS["___mysqli_ston"],  $sql );
                 $incscore = $row['dailycheckin'];
                 $incscore++;
                 $sql           = "UPDATE `assignments` SET `dailycheckin` = $incscore WHERE `userid` = $session_id";
                 $retval5       = mysqli_query($GLOBALS["___mysqli_ston"],  $sql );
                 
    //Check for assignment lvl up and if progress is greater then 7 award lvl point
                 $sql           = "SELECT * FROM `assignments` WHERE `userid` = $session_id ";
                 $result99      = mysqli_query($GLOBALS["___mysqli_ston"],  $sql );
                 $row           = mysqli_fetch_array( $result99 );
                 $progresscheck = $row['dailycheckin'];
                 if ( $progresscheck == 7 )
                   {
                     $sql4          = "SELECT * FROM `assignmentlvl` WHERE `userid` = $session_id ";
                     $result4       = mysqli_query($GLOBALS["___mysqli_ston"],  $sql4 );
                     $row           = mysqli_fetch_array( $result4 );
                     $dailyprogress = $row['dailycheckin'];
                     $dailyprogress++;
                     $sql     = "UPDATE `assignmentlvl` SET `dailycheckin` = $dailyprogress WHERE `userid` = $session_id ";
                     $retval6 = mysqli_query($GLOBALS["___mysqli_ston"],  $sql );
                     $sql     = "UPDATE `assignments` SET `dailycheckin` = 1 WHERE `userid` = $session_id ";
                     $retval7 = mysqli_query($GLOBALS["___mysqli_ston"],  $sql );
                   }
               }
             else
               {
     //Already checked daily checkin status just update the date
                 $sql     = "UPDATE `members` SET `last_logged_in` = '" . date( 'Y-m-d', time() ) . "' WHERE `id` = $session_id";
                 $retval8 = mysqli_query($GLOBALS["___mysqli_ston"],  $sql );
               }
           }
       }
    ?>
    Last edited by IMaVBNoob; November 20th, 2012 at 11:26 PM. Reason: Added more comments to the php script
  8. #5
  9. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,957
    Rep Power
    1046
    Well, the MySQLi library won't help you if you just replace "mysql_" with "mysqli_" and continue to inject raw variables into query string. Don't do that. If you plan to put your script online (which I suppose), this will be a serious security issue. Manipulating your SQL queries might enable attackers to steal/delete/corrupt your database and maybe even access your server. So either always use prepared statements or at least escape every value. Anything below that opens your script to SQL injections.

    Since you currently don't do any escaping at all, I fear there are other security issues in your script (like XSS vulnerabilities). So you should seriously rethink your security concept and check every script for those problems.

    And you still have the "SELECT *".

    As to your script:

    That's a lot of code for a rather simple operation. I think the main problem is that you make a lot of unnecessary queries and re-implement database functionalities in PHP (like incrementing a column, which could simply be done in SQL with `col` = `col` + 1).

    Some things are simply odd. For example, what is the "else" part in line 52 supposed to do? A value is either truthy or falsy. There is no third option.

    And what's they point of the level counter when it's basically just a copy of the daily login counter?

    OK, what do you want to do? As far as I understand, you have the following steps:
    Code:
    - fetch last login date of current user
    - was it yesterday?
    	yes:
    		- increment daily login counter
    		- fetch login counter
    		- has the counter reached 7?
    			yes: increment level counter (??)		
    	no:
    		- reset daily login counter to 1
    - update last login date
    If you implement that in PHP, the code should be pretty straightforward and simple.
  10. #6
  11. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jan 2006
    Posts
    122
    Rep Power
    9
    i did acctually escape my code after i posted that because i seen that it wasnt in there after u mentioned it. And its odd that mysql to mysqli converter would not use the correct syntax to convert mysql to mysqli.. and yes u have the right idea that is exactly what im trying to do. looks like it should work but im going wrong some where..

    the else makes sure that if it is the current day and if the user has already been awarded 1 point and that they wont receive the award again when page that the script is included on is visited a second time and only the date in mysql is updated.

    the elseif awards the point if yesterday was 1 day ago

    the if checks their last login was yesterday and if not reset the date to the current date and the dailycheckin col back to 1 because they failed the assignment...

    that else satement is needed because the login will be awarded if it was 2 days and not 1. and that will be a invalid award and the date will stay as that date and not be updated to the current date.

    as for the cloned table this will act as a lvl and just that...

    lets say the userd logged in for 7 days straight..
    the assignments table col for dailycheckin will equal 7... now where to go from here...
    thats why i thought reset dailycheckin to 1 and lvl up that dailycheckin assignmentlvl to later be displayed as "Daily Check in score: 6 0f 7days x1" x1 being the lvl or how many times they commpleted the assignment.

    Originally Posted by Jacques1
    Well, the MySQLi library won't help you if you just replace "mysql_" with "mysqli_" and continue to inject raw variables into query string. Don't do that. If you plan to put your script online (which I suppose), this will be a serious security issue. Manipulating your SQL queries might enable attackers to steal/delete/corrupt your database and maybe even access your server. So either always use prepared statements or at least escape every value. Anything below that opens your script to SQL injections.

    Since you currently don't do any escaping at all, I fear there are other security issues in your script (like XSS vulnerabilities). So you should seriously rethink your security concept and check every script for those problems.

    And you still have the "SELECT *".

    As to your script:

    That's a lot of code for a rather simple operation. I think the main problem is that you make a lot of unnecessary queries and re-implement database functionalities in PHP (like incrementing a column, which could simply be done in SQL with `col` = `col` + 1).

    Some things are simply odd. For example, what is the "else" part in line 52 supposed to do? A value is either truthy or falsy. There is no third option.

    And what's they point of the level counter when it's basically just a copy of the daily login counter?

    OK, what do you want to do? As far as I understand, you have the following steps:
    Code:
    - fetch last login date of current user
    - was it yesterday?
    	yes:
    		- increment daily login counter
    		- fetch login counter
    		- has the counter reached 7?
    			yes: increment level counter (??)		
    	no:
    		- reset daily login counter to 1
    - update last login date
    If you implement that in PHP, the code should be pretty straightforward and simple.
  12. #7
  13. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jan 2006
    Posts
    122
    Rep Power
    9
    oh forgot to ask. should escape be use on update or insert or for every query even when selecting WHERE col=$var
    ?
  14. #8
  15. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,957
    Rep Power
    1046
    Please don't make full quotes for every reply. The text is right above your posting, so no need to repeat it.



    Originally Posted by IMaVBNoob
    the else makes sure that if it is the current day and if the user has already been awarded 1 point and that they wont receive the award again when page that the script is included on is visited a second time and only the date in mysql is updated.
    Then you've nested the "if" statements wrong. Currently, you have this:
    PHP Code:
    if (!$retval) {
        ...
    } elseif (
    $retval) {
        ...
    } else {
        ...

    And this obviously makes no sense, because it's either $retval or !$retval. The "else" part never gets executed.

    It looks like the "else" part is supposed to be one level higher (that is, after the closing brace) so that it belongs to
    PHP Code:
    if ( $row2['last_logged_in'] !== date'Y-m-d'time() ) ) ... 


    Originally Posted by IMaVBNoob
    as for the cloned table this will act as a lvl and just that...

    lets say the userd logged in for 7 days straight..
    the assignments table col for dailycheckin will equal 7... now where to go from here...
    thats why i thought reset dailycheckin to 1 and lvl up that dailycheckin assignmentlvl to later be displayed as "Daily Check in score: 6 0f 7days x1" x1 being the lvl or how many times they commpleted the assignment.
    Currently, your lvl is either the default value or always 8.

    Have a look at the update logic:
    PHP Code:
    $progresscheck $row['dailycheckin'];
    if ( 
    $progresscheck == )
    {
        
    $dailyprogress $row['dailycheckin'];                                                                            // this is 7
        
    $dailyprogress++;                                                                                                // this is 8
        
    $sql     "UPDATE `assignmentlvl` SET `dailycheckin` = $dailyprogress WHERE `userid` = $session_id ";            // this set assignmentlvl to 8

    So whenever the table is updated, the level is always set to 8. If you want to add 1 to the level, you have to actually do that:

    PHP Code:
    $progresscheck $row['dailycheckin'];
    if ( 
    $progresscheck == )
    {
        
    $sql     "UPDATE `assignmentlvl` SET `dailycheckin` = `dailycheckin` + 1 WHERE `userid` = $session_id ";            // increment dailycheckin

    I don't even see why you used this $dailyprogress stuff? Why not simply count 1 up?



    Originally Posted by IMaVBNoob
    oh forgot to ask. should escape be use on update or insert or for every query even when selecting WHERE col=$var
    ?
    You must escape every variable in every single query. But a better solution is to use prepared statements (as I explained earlier). That's a kind of "query template" with variables for data. You can then safely pass values to the variables without having to worry that they could be used to manipulate the actual query.

    Comments on this post

    • IMaVBNoob agrees
    Last edited by Jacques1; November 21st, 2012 at 10:29 AM.
  16. #9
  17. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jan 2006
    Posts
    122
    Rep Power
    9
    i didnt know i could do that to + to a col i will change that. sorry about the quote i pushed quick reply..
  18. #10
  19. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jan 2006
    Posts
    122
    Rep Power
    9

    aHHH finally the functionality is there


    its workin now thanks so much i was getting fustrated. Now to handle some of the security issues u pointed out.. thanks sooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo much....

    DEV SHED ROCKS!!!!

IMN logo majestic logo threadwatch logo seochat tools logo