Merge lp:~nurlan0000/psiphon/sprint3-610190 into lp:psiphon

[Nurlan Turdaliev]

[lp:~nurlan0000/psiphon/sprint3-610190]

122. By Nurlan <ubuntu@ubuntu-desktop> on 2010-09-02

Fixed minor bug

[Adam Kruger (adam-kruger)]

Code review comments:

- In cleanup_sessions_cron.php, we see the following line:
$query2 = "update user set session = NULL where session is not NULL and last_activity < (unix_timestamp() - {$config["stay_logged_in"]} ) AND stay_logged_in = 1";

It appears that $config["stay_logged_in"] does not exist and so this sql statement will fail, also causing subsequent sql statements to not run.

This is a severe and obvious error that should have been caught in testing. We did not perform any further review after seeing this error.
Please complete internal QA before resubmitting.

Another pre-existing coding error was identified which may help to identify further related errors:
sql_error_cli($query, $config["sql"]);
Note that $query does not exist, but instead there are numbered variables $query1, $query2, etc. This code should be restructured.

review: Needs Fixing (group review)

[lp:~nurlan0000/psiphon/sprint3-610190]

123. By Nurlan Turdaliev <ubuntu@ubuntu-desktop> on 2010-10-21

Fixed cronjob bug

124. By Nurlan Turdaliev <ubuntu@ubuntu-desktop> on 2010-10-21

Fixed session cleanup cronjob

[Nurlan Turdaliev (nurlan0000)]

review: Needs Resubmitting

[e.fryntov (e-fryntov)]

please remove the var_dump call from the cronjob.

review: Needs Fixing

[lp:~nurlan0000/psiphon/sprint3-610190]

125. By Nurlan Turdaliev <ubuntu@ubuntu-desktop> on 2010-11-01

remove var_dump

[Adam Kruger (adam-kruger)]

- Please create a new permission in config.php (and use user_class_has_permission) for testing whether the Stay Logged In feature is available to a user, rather than repeating the test that the user is a Power User or a Normal User inline and in multiple files.

- Please rename cookie_persistence_period to something more specific to this functionality, such as stay_logged_in_ttl, and move this next to session_ttl

- Changing the value in the Profile page does not work because the cookie is not set when the Profile is updated. We have decided that this Profile option is undesirable and should be removed from the Profile page.

review: Needs Fixing (group review)

[Nurlan Turdaliev (nurlan0000)]

review: Needs Resubmitting

[Adam Kruger (adam-kruger)]

I think the query in profile.php looks broken now:

- $query .= "xml_html = :xml_html ";
+ $query .= "xml_html = :xml_html, ";

This file should be reverted to the trunk version since there are no meaningful changes in this file.

review: Needs Fixing

[Nurlan Turdaliev (nurlan0000)]

review: Needs Resubmitting

[lp:~nurlan0000/psiphon/sprint3-610190]

126. By Nurlan Turdaliev <ubuntu@ubuntu-desktop> on 2010-11-15

Removed comma in sql query

127. By Nurlan Turdaliev <ubuntu@ubuntu-desktop> on 2010-11-15

Sql query fixed

[Adam P (adam+)]

review: Approve (code read)

=== modified file 'trunk/cronjobs/cleanup_sessions_cron.php'
--- trunk/cronjobs/cleanup_sessions_cron.php	2009-11-23 23:23:36 +0000
+++ trunk/cronjobs/cleanup_sessions_cron.php	2010-11-15 11:09:48 +0000
@@ -45,6 +45,8 @@
     exit("{$argv[0]} is already running, pid {$pid}\n");
 }
 
+
+
 if ($f = @fopen($config["cleanup_sessions_pid_file"], "w"))
 {
     $pid = getmypid();
@@ -56,30 +58,34 @@
     echo "Can't create {$config["cleanup_sessions_pid_file"]}, keep running anyway.\n";
 }
 
+
 // Cleanup all expired sessions
 //
 // For anonymous users (grp = 4), delete the user and cookie.  For regular users,
 // clear the session and delete the expired cookies (same as logout.php).
 // also deletes orphaned cookie($query5)
 // Don't need to parameterize the $config value.
-$query1 = "update user set session = NULL where session is not NULL and last_activity < (unix_timestamp() - {$config["session_ttl"]})";
+$queries[] = "update user set session = NULL where session is not NULL and last_activity < (unix_timestamp() - {$config["session_ttl"]}) AND stay_logged_in = 0";
+$queries[] = "update user set session = NULL where session is not NULL and last_activity < (unix_timestamp() - {$config["stay_logged_in_ttl"]} ) AND stay_logged_in = 1";
+
 // NEW: older logic only deleted session and expired cookies ("(expire = -1 or expire < ".(date("U") - date("Z")).")") but now
 // we clear all cookies when a user logs out so we're not hanging onto any 3rd party site credentials
 // TODO: deleting from MySQL isn't wiping from disk, so the cookies table should be in-memory only (page locked)
-$query2 = "delete from cookie using cookie inner join user where cookie.user = user.id and user.session is NULL";
-$query3 = "delete from user where session is NULL and grp = '4'";
-$query4 = "delete cookie from cookie left join user on cookie.user = user.id where user.id is NULL";
+$queries[] = "delete from cookie using cookie inner join user where cookie.user = user.id and user.session is NULL";
+$queries[] = "delete from user where session is NULL and grp = '4'";
+$queries[] = "delete cookie from cookie left join user on cookie.user = user.id where user.id is NULL";
 
-if (!db_query_execute($config, $query1, convert_null_array(array()), true) ||
-    !db_query_execute($config, $query2, convert_null_array(array()), true) ||
-    !db_query_execute($config, $query3, convert_null_array(array()), true) ||
-    !db_query_execute($config, $query4, convert_null_array(array()), true))
+foreach ($queries as $query)
 {
-    @unlink($config["cleanup_sessions_pid_file"]);
-    sql_error_cli($query, $config["sql"]);
+    if (!db_query_execute($config, $query, convert_null_array(array()), true))
+    {
+        @unlink($config["cleanup_sessions_pid_file"]);
+        sql_error_cli($query, $config["sql"]);
+    }
 }
 
+
 @unlink($config["cleanup_sessions_pid_file"]);
 
 exit(0);
-?>
+?>
\ No newline at end of file

=== added file 'trunk/sql/upgrade-2.6.sql'
--- trunk/sql/upgrade-2.6.sql	1970-01-01 00:00:00 +0000
+++ trunk/sql/upgrade-2.6.sql	2010-11-15 11:09:48 +0000
@@ -0,0 +1,1 @@
+ALTER TABLE `user` ADD COLUMN `stay_logged_in` INTEGER  NOT NULL DEFAULT 0;
\ No newline at end of file

=== modified file 'trunk/tools/psiphon_install.sh'
--- trunk/tools/psiphon_install.sh	2010-06-01 17:50:42 +0000
+++ trunk/tools/psiphon_install.sh	2010-11-15 11:09:48 +0000
@@ -271,7 +271,7 @@
 echo "Enter current password for mysql root (enter for none)"
 
 cat sql/create_db.sql sql/grant.new.sql sql/structure.2.3.sql sql/init.sql \
-sql/upgrade-2.4.sql sql/upgrade-2.5.sql sql/jsf.sql sql/youtube.sql | mysql -uroot -p
+sql/upgrade-2.4.sql sql/upgrade-2.5.sql sql/upgrade-2.6.sql sql/jsf.sql sql/youtube.sql | mysql -uroot -p
 
 rm sql/grant.new.sql
 

=== modified file 'trunk/www/check_user.php'
--- trunk/www/check_user.php	2010-09-29 20:00:51 +0000
+++ trunk/www/check_user.php	2010-11-15 11:09:48 +0000
@@ -57,6 +57,8 @@
         break;
     }
 
+    $stay_logged_in = @$_POST['stay_logged_in'];
+
     // - Find user with matching username (password check done after, with salt)
     // - Login delay: if login_delay is not 0, don't allow login
     //   if not passed the delay time.  The delay time is set
@@ -89,7 +91,7 @@
         // (Is there a timing attack here, if == exits early?  Attacker that knows salt could
         // select login_pass in such a way as to probe the 1st, then 2nd, etc. byte of the
         // hashed password.  But failed login delay makes this intractable.)
-        
+
         $user_found = true;
         if ((strlen($login_record['pass']) == 40 && $login_record['pass'] == sha1($_POST['login_pass'].$login_record['pass_salt'])) ||
             (strlen($login_record['pass']) == 32 && $login_record['pass'] == md5($_POST['login_pass'])))
@@ -140,7 +142,7 @@
         {
             // The user has no special permissions and so must log in from one of
             // the proxies associated with his user record.
-           
+
             $login_query = "SELECT TRUE ".
                            "FROM userproxy JOIN proxy ON userproxy.proxyid = proxy.id ".
                            "WHERE userproxy.userid = :userid ".
@@ -166,7 +168,7 @@
         // Old code used MD5 and didn't salt passwords; pass_salt database field will be NULL
         // in this case --> generate a salt and store a new hash since we have the plaintext
         // password at this time.
-    
+
         if (!$login_record['pass_salt'] || strlen($login_record['pass_salt']) == 0)
         {
             $pass_salt = bin2hex(secure_rand());
@@ -204,6 +206,9 @@
         $login_region = lookup_region($config);
         update_login_stats($config, $login_region, $login_record['proxy'], $login_record['grp']);
 
+
+        $can_stay_logged_in = user_has_permission($config, $login_record, 'stay_logged_in');
+
         // - Set user session (cookie), last login time, last login region
         // - Reset failed login data to no delay
         // - Set user's current proxy to the current proxy.
@@ -216,6 +221,7 @@
             "  last_reg = :last_reg, ".
             "  login_delay = 0, ".
             "  consecutive_failed_logins = 0, ".
+            '  stay_logged_in = '.($can_stay_logged_in && $stay_logged_in ? 1 : 0).', '.
             "  current_proxy = :proxy_id ".
             "WHERE user.id = :user_id";
 
@@ -229,6 +235,7 @@
                 ":user_id" => $login_record['id'],
                 ":proxy_id" => $login_record['proxy'])));
 
+
         if ($lang_selected)
         {
             $update_language_query =
@@ -243,7 +250,15 @@
         }
 
         $_COOKIE[p] = $session_id;
-        header("Set-Cookie: p={$_COOKIE[p]}; path=/", false);
+
+        // Set cookie
+        $cookie_str = "Set-Cookie: p={$_COOKIE[p]}; path=/;";
+        if ($can_stay_logged_in && $stay_logged_in)
+        {
+            $cookie_str .= " expires=". gmdate('D, d-M-Y H:i:s', time() + $config['stay_logged_in_ttl']);
+        }
+
+        header($cookie_str, false);
 
         // TODO: need clarification on what this code is for...
         $login_query = "DELETE FROM cookie WHERE user = :user_id AND (expire = -1 OR expire < ".(date("U") + date("Z")).")";
@@ -278,7 +293,7 @@
         // cases, we don't include the CSRF token in GET requests since a GET
         // is idempotent and doesn't leak information.
 
-        if (!$csrf_token && 
+        if (!$csrf_token &&
             ((count($_POST) > 0 && $_POST['csrf_token'] != $record_user['csrf_token']) ||
              ($do_get_csrf && count($_GET) > 0 && $_GET['csrf_token'] != $record_user['csrf_token'])))
         {
@@ -371,6 +386,10 @@
                     <td><label for="pass"><font class="fourteenblue"><?=msg("password")?></font></label></td>
                     <td><input type="password" name="login_pass" size="30" id="pass"></td>
                 </tr>
+                <tr>
+                    <td><label for="stay_logged"><font class="fourteenblue"><?=msg("stay_logged_in")?></font></label></td>
+                    <td><input type="checkbox" name="stay_logged_in" id="stay_logged" value="1"></td>
+                </tr>
                 <tr><td></td></tr>
                 <tr>
                     <td></td>
@@ -417,7 +436,7 @@
    header("Content-type: text/html; charset=".charset());
 ?>
 <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
-<html>    
+<html>
     <head>
         <title><?=msg("usage_terms_title")?></title>
         <?=echo_css()?>
@@ -473,4 +492,4 @@
 
 // Proceed with the screens that include check_user.php...
 
-?>
+?>
\ No newline at end of file

=== modified file 'trunk/www/config.php'
--- trunk/www/config.php	2010-06-28 20:44:20 +0000
+++ trunk/www/config.php	2010-11-15 11:09:48 +0000
@@ -40,7 +40,7 @@
 }
 
 $config["ret"] .= getenv("HTTP_HOST").
-                  substr(getenv("SCRIPT_NAME"), 0, 
+                  substr(getenv("SCRIPT_NAME"), 0,
                          strrpos(getenv("SCRIPT_NAME"), "/"));
 
 // invitation is valid for 5 days, then it's deleted if not used
@@ -53,6 +53,8 @@
 $config["email_change_period"] = 60*60;
 
 $config["session_ttl"] = 60*60; // session expires after 1 idle hour
+// Period to stay logged in
+$config['stay_logged_in_ttl'] = 2 * 7 * 24 * 60 * 60; // 2 weeks
 
 $config[min_pass_length] = 6;
 $config[max_login_delay] = 30; // 30 seconds
@@ -99,6 +101,7 @@
         'view_propagator_info' => 1<<19,
         'multi_proxy'       => 1<20,
         'custom_user_agents' => 1<<21,
+        'stay_logged_in'    => 1<<23,
     );
 
 // This defines the order in which the user groups/classes should be displayed. Generally, least-to-highest privilege.
@@ -140,7 +143,8 @@
        | $config['perm']['send_ticket']
        | $config['perm']['bookmarks']
        | $config['perm']['logout']
-       | $config['perm']['can_grant_grp_3'],
+       | $config['perm']['can_grant_grp_3']
+       | $config['perm']['stay_logged_in'],
 
     // User
     // NOTE: '3' is the grp default in the invitation table. So if the 3 changes here, it'll have to change there.
@@ -150,7 +154,8 @@
        | $config['perm']['send_ticket']
        | $config['perm']['bookmarks']
        | $config['perm']['logout']
-       | $config['perm']['can_grant_grp_3'],
+       | $config['perm']['can_grant_grp_3']
+       | $config['perm']['stay_logged_in'],
 
     // Anonymous User
 
@@ -247,4 +252,7 @@
 
 $config["geoip_type"]= "country";
 $config["geoip_data_file"] = "/usr/share/GeoIP/GeoIP.dat";
-?>
+
+
+
+?>
\ No newline at end of file

=== modified file 'trunk/www/includes/lang.php'
--- trunk/www/includes/lang.php	2010-10-14 16:26:29 +0000
+++ trunk/www/includes/lang.php	2010-11-15 11:09:48 +0000
@@ -4934,6 +4934,19 @@
         'be' => "Войти, используя другую учетную запись",
     ),
 
+    'stay_logged_in' => array(
+        'en' => 'Stay logged in',
+        'fa' => 'Stay logged in',
+        'ru' => 'Stay logged in',
+        'uz' => 'Stay logged in',
+        'tk' => 'Stay logged in',
+        'cn' => 'Stay logged in',
+        'ar' => 'Stay logged in',
+        'vt' => 'Stay logged in',
+        'fr' => 'Stay logged in',
+        'es' => 'Stay logged in',
+    ),
+
     'custom_user_agents' => array(
         'en' => "User Agents",
         'fa' => "کاربران",

=== modified file 'trunk/www/profile.php'
--- trunk/www/profile.php	2010-07-16 17:53:05 +0000
+++ trunk/www/profile.php	2010-11-15 11:09:48 +0000
@@ -103,12 +103,12 @@
     // This is unfortunate but acceptable (at this time).
 
     if (db_check_if_value_exists(
-            $config,
-            "user",
-            "uname",
-            $_POST['uname'],
-            "id != :id",
-            array(":id" => $record_user['id'])))
+        $config,
+        "user",
+        "uname",
+        $_POST['uname'],
+        "id != :id",
+        array(":id" => $record_user['id'])))
     {
         $processing_errors[] = msg("username_exists_in_database");
         $curs = "profile.uname";
@@ -155,12 +155,12 @@
             // This is unfortunate but acceptable (at this time).
 
             if (db_check_if_value_exists(
-                    $config,
-                    "user",
-                    "email",
-                    $_POST['email'],
-                    "id != :id",
-                    array(":id" => $record_user['id'])))
+                $config,
+                "user",
+                "email",
+                $_POST['email'],
+                "id != :id",
+                array(":id" => $record_user['id'])))
             {
                 $processing_errors[] = msg("email_exists_in_database");
                 $curs = "profile.email";
@@ -172,7 +172,7 @@
             // check if enough time has passed since user last caused an
             // email to be sent out
             if ($record_user[email_candidate_date] &&
-                $record_user[email_candidate_date] + $config["email_change_period"] > time())
+                    $record_user[email_candidate_date] + $config["email_change_period"] > time())
             {
                 $processing_errors[] = msg("must_wait_to_change_email");
                 $curs="profile.email";
@@ -247,6 +247,7 @@
 
 include($_SERVER[DOCUMENT_ROOT]."/header.php");
 
+
 ?>
 <br>
 <div style="width:50em;"><?=sprintf(msg("privacy_info_html"), make_proxied_url($config["psiphon_web_resource_page"]))?></div>
@@ -305,7 +306,6 @@
 <th align="right"><?=msg("xml_html")?></th>
 <td nowrap><input type="checkbox" name="xml_html" value="1"<?=$record_user["xml_html"]?" checked":""?>></td>
 </tr>
-
 <tr>
 <td>&nbsp;</td>
 <td><input type="submit" value="<?=msg("update")?>">
@@ -314,4 +314,4 @@
 
 </form>
 
-<? include($_SERVER[DOCUMENT_ROOT]."/footer.php"); ?>
+<? include($_SERVER[DOCUMENT_ROOT]."/footer.php"); ?>
\ No newline at end of file