FuelPHP Forums

nnichols

I am using db based sessions for a set of web service methods. The session is set to auto_initialize and both post_cookie_name and db.cookie_name are set to 'sessionId'. Here is the simple test controller I have been using to test my session config.

<?php

/**
 * Test Controller.
 *
 * @package  app
 * @extends  Controller_Rest
 */
class Controller_Test extends Controller_Rest {

    public function before() {
        parent::before();
        $this->session = \Fuel\Core\Session::instance();
    }

    public function action_test() {
        $this->session->set('user_id', 12345);
        $this->response(array('sessionId' => $this->session->key('session_id')));
    }
}

The first time I call /test/test I receive

{"sessionId":false}

but the new session cookie is correctly sent in the response headers. The second time I request /test/test the cookie is sent with the request headers and I then receive the expected response

{"sessionId":"9dddf3fcb058c181bee70e0238581880"}

Additionally, if I do not set a session variable then the session cookie is not sent. Is this intended functionality? Once this is working I would also like to disable the sending of the session cookie so that the sessionId is only sent as part of the response/request when desired instead of with every request. Is there an easy way to disable the session cookie or will I need to override the Session_Db::create() so that it does not call _set_cookie method? TIA
Nick

Harro

*** moved to General, this is not an installation question *** The session is only created on write, which happens in the shutdown event. So this is by design, but I understand in this case not desirable. Can you create an issue for this at http://github.com/fuel/core/issues so someone can take a look at it?

nnichols

Hi Harro, Thank you for your reply. My apologies for posting in the wrong forum. Not quite sure how I managed that. I will create an issue for it now. With regards to my other question about stopping the cookie being set, is there a setting in the config for doing this or will I need to override the method so that the parent _set_cookie method is not called? Many thanks

Harro

Sorry, I missed the second part of the question. If you use the Session class, the cookie will be set, otherwise you can't maintain the session. Why do you want to stop that?

nnichols

The sessionId is being propagated as part of the response and request bodies so there is no point in sending it in the headers too, as it is just wasted bandwidth. With thousands of connections per second sending the session cookie unnecessarily is a significant overhead. I have created https://github.com/fuel/core/issues/1180 as suggested. Thanks

Harro

You don't want the session cookie being send in the response header? Ever? Sending the session ID back and forth in plain text is ihmo a bad idea from a security point of view, it isn't encrypted for nothing. And server side you can't do much with the session ID, the Session class expects the encrypted payload (which contains more then just the session ID).

nnichols

The client does not support cookies which is why the sessionId needs to be sent in the request/response body. All connections are over SSL/TLS so encryption is not an issue. I think it is going to be easier for me to write my own session driver to get around these issues. Thank you for your thoughts.

Harro

That is an option. Just create a Session_Yourdriver in app/classes/session/yourdriver.php, add it to the app bootstrap, and you can define 'yourdriver' as the session driver to use. If you make sure it extends \Session_Driver and implements the abstract methods, you can continue using the standard session interface.

nnichols

I have modified my before method to call Session_Db::create if there is not session_id.

<?php

/**
 * Test Controller.
 *
 * @package  app
 * @extends  Controller_Rest
 */
class Controller_Test extends Controller_Rest {

    public function before() {
        parent::before();
        $this->session = \Fuel\Core\Session::instance();
        if (!$this->session->key('session_id')) {
            $this->session->write();
        } else {
            $this->session->rotate(false);
        }
    }

    public function action_test() {
        $this->session->set('user_id', 12345);
        $this->response(array('sessionId' => $this->session->key('session_id')));
    }
}

Calling rotate in the else block generates the following error -

ErrorException [ Notice ]: Undefined index: created

I am also planning to use post_cookie_name so that I can post the session_id but this seems to be designed to take the same encrypted content as the cookie. How can I retrieve or generate the string that would be assigned to the cookie so that it can be sent in the response body as part of my serialised JSON? Thanks

Harro

The original issue has been fixed a few hours ago, a new session is now setup when the session class loads. I don't know where that notice comes from, as far as I can see the 'created' index is always created in the keys array. The current session driver adds validation data to the payload of the cookie. Using only the session id, unencrypted, whould make session hijjacking relatively simple. There's a rewrite on the roadmap that moves the validation data server side, which would make it possible to expose the session id, and store only the session id in the cookie. After this rewrite it will be easy to implement a 'do-not-write-the-cookie' setting. Ah, and a p.s.: do not use the \Fuel\Core namespace in your code, it's an internal namespace that should not be used by applications. Every Fuel class is accessable in the global namespace.

nnichols

Thank you. I have updated to your latest version of the Session classes and they work fine. Are you able to think of a simple way I can modify either the before method of my controller or my overridden Session_Db class to use the session_id instead of the current payload? I have added a method to generate the cookie payload which can be returned as a string to be passed in my response body. This works fine but it increases my average request size by just over 700% and seems excessive as the session_id is all that is required.

Harro

Short term the only solution I can think of is to write a custom Session driver. Take the one with the backend you want to use. In case of DB, copy it from core/classes/session/db.php to app/classes/session/db.php. Remove the namespace in your copy, and add it to app/bootstrap.php so the framework knows there's an replacement version. Then modify it to your liking. If you stick to the methods and their functionality, it should be completely transparent for your application.

nnichols

Thank you for all your help and thoughts on this. Below is the diff of my modified Session_Db class with the one that you committed a few days ago. I would really appreciate it if you would have a quick look and let me know if you can see any issues with this approach. I have tested it and all seems to be working correctly.

-namespace Fuel\Core;
 
 
-
 // --------------------------------------------------------------------
 
 class Session_Db extends \Session_Driver
@@ -79,10 +75,10 @@
  public function read($force = false)
  {
   // get the session cookie
-  $cookie = $this->_get_cookie();
+        $session_id = (Input::post($this->config['post_cookie_name'])) ?: false;
 
   // if no session cookie was present, initialize a new session
-  if ($cookie === false or $force)
+  if ($session_id === false or $force)
   {
    $this->data = array();
    $this->keys = array();
@@ -90,31 +86,19 @@
   else
   {
    // read the session record
-   $this->record = \DB::select()->where('session_id', '=', $this->keys['session_id'])->from($this->config['table'])->execute($this->config['database']);
+   $this->record = \DB::select()->where('session_id', '=', $session_id)->from($this->config['table'])->execute($this->config['database']);
 
    // record found?
    if ($this->record->count())
    {
+                $this->_set_keys();
     $payload = $this->_unserialize($this->record->get('payload'));
    }
    else
    {
-    // try to find the session on previous id
-    $this->record = \DB::select()->where('previous_id', '=', $this->keys['session_id'])->from($this->config['table'])->execute($this->config['database']);
-
-    // record found?
-    if ($this->record->count())
-    {
-     // previous id used, correctly set session id so it wont be overwritten with previous id.
-     $this->keys['session_id'] = $this->record->get('session_id');
-     $payload = $this->_unserialize($this->record->get('payload'));
-    }
-    else
-    {
      // cookie present, but session record missing. force creation of a new session
      return $this->read(true);
     }
-   }
 
    if (isset($payload[0])) $this->data = $payload[0];
    if (isset($payload[1])) $this->flash = $payload[1];
@@ -138,9 +122,6 @@
   {
    parent::write();
 
-   // rotate the session id if needed
-   $this->rotate(false);
-
    // create the session record, and add the session payload
    $session = $this->keys;
    $session['payload'] = $this->_serialize(array($this->data, $this->flash));
@@ -158,13 +139,8 @@
    }
 
    // update went well?
-   if ($result !== false)
+   if ($result === false)
    {
-    // then update the cookie
-    $this->_set_cookie();
-   }
-   else
-   {
     logger(\Fuel::L_ERROR, 'Session update failed, session record could not be found. Concurrency issue?');
    }
 
@@ -273,4 +249,24 @@
   return parent::_validate_config($validated);
  }
 
+ /**
+  * assign values to the keys that would otherwise have been
+     * set when decrypting the cookie
+  *
+  * @access private
+  * @return  void
+  */
+    protected function _set_keys()
+    {
+        if ($this->record->count())
+        {
+            $this->keys['session_id'] = $this->record->get('session_id');
+            $this->keys['previous_id'] = $this->record->get('session_id');
+            $this->keys['ip_hash']  = $this->record->get('ip_hash');
+            $this->keys['user_agent'] = $this->record->get('user_agent');
+            $this->keys['created']   = $this->record->get('created');
+            $this->keys['updated']   = $this->time->get_timestamp();
 }
+    }
+
+}

Thank you

Harro

If it works for you, ok. When I start rewriting the Session classes I'll see if I can squeeze in this requirement. I'm going to move all the stuff currently in the cookie to the payload for server-side checking, which means it's possible to have a cookie with only the session id (encrypted or not). Adding a config value to determine the way the session id is exchanged between server and client should then not be too difficult.

FuelPHP Forums

Ask your question about FuelPHP in the appropriate forum, or help others by answering their questions.

Howdy, Stranger!

Categories

In this Discussion