zend-expressive-session proposed changes

Segment.php

<?php

namespace Zend\Expressive\Session;

/**
 * Class representing a session segment (essentially, a "namespace" within the session).
 *
 * Other proposed methods/functionality:
 *
 * - setFlash(string $name, string $message) :void - on a namespace instance, to set a flash value
 * - getFlash(string $name) : ?string - on a namespace instance, to retrieve a flash value
 * - keepFlash() : void - on a namespace instance, persist flash values for another request
 * - clearFlash() : void - on a namespace instance, to remove any existing flash values
 *
 * The above would require some coordination by the Session class (for purposes
 * of managing which items are current, and which need to persist to next
 * request).
 */
class Segment
{
    use SessionDataTrait;

    private $data;
    private $id;

    public function __construct(string $id, array $data)
    {
        $this->id = $id;
        $this->data = $data;
    }

    /**
     * Retrieve all data, for purposes of persistence.
     */
    public function toArray() : array
    {
        return $this->data;
    }

    /**
     * @param mixed $default Default value to return if value does not exist.
     * @return mixed
     */
    public function get(string $name, $default = null)
    {
        return $this->data[$name] ?? $default;
    }

    public function set(string $name, $value): void
    {
        $this->data[$name] = $value;
    }

    public function unset(string $name): void
    {
        unset($this->data[$name]);
    }

    public function clear() : void
    {
        $this->data = [];
    }
}

Session.php

<?php

namespace Zend\Expressive\Session;

use RuntimeException;

class Session
{
    use SessionDataTrait;

    private $data;
    private $id;
    private $segments = [];

    public function __construct(string $id, array $data)
    {
        $this->id = $id;
        $this->data = $data;
    }

    /**
     * Retrieve all data, including segments, as a nested set of arrays, for
     * purposes of persistence.
     */
    public function toArray() : array
    {
        $data = $this->data;
        foreach ($this->segments as $key => $segment) {
            $segmentData = $segment->toArray();
            if (empty($segmentData)) {
                unset($this->data[$key]);
                continue;
            }
            $data[$key] = $segmentData;
        }
        return $data;
    }

    /**
     * @param mixed $default Default value to return if $name does not exist.
     * @throws RuntimeException if $name refers to a known session segment.
     */
    public function get(string $name, $default = null)
    {
        if (isset($this->segments[$name])) {
            throw new RuntimeException(sprintf(
                'Retrieving session data "%s" via get(); however, this data refers to a session segment; aborting',
                $name
            ));
        }
        return $this->data[$name] ?? $default;
    }

    /**
     * @param mixed $value
     * @throws RuntimeException if $name refers to a known session segment.
     */
    public function set(string $name, $value) : void
    {
        if (isset($this->segments[$name])) {
            throw new RuntimeException(sprintf(
                'Attempting to set session data "%s"; however, this data refers to a session segment; aborting',
                $name
            ));
        }
        $this->data[$name] = $value;
    }

    /**
     * @throws RuntimeException if $name refers to a known session segment.
     */
    public function unset(string $name) : void
    {
        if (isset($this->segments[$name])) {
            throw new RuntimeException(sprintf(
                'Attempting to unset session data "%s"; however, this data refers to a session segment. '
                . 'Use clear() on the segment instead',
                $name
            ));
        }
        unset($this->data[$name]);
    }

    public function segment(string $name) : Segment
    {
        if (isset($this->segments[$name])) {
            return $this->segments[$name];
        }

        if (array_key_exists($name, $this->data)
            && ! is_array($this->data[$name])
        ) {
            throw new RuntimeException(sprintf(
                'Cannot retrieve session segment "%s"; data exists, but as a "%s" instead of an array',
                $name,
                is_object($this->data['name']) ? get_class($this->data[$name]) : gettype($this->data['name'])
            ));
        }

        $this->segments[$name] = new Segment($this->data[$name]);
        return $this->segments[$name];
    }

    public function regenerateId(): void
    {
        $this->id = static::generateToken();
    }
}

SessionDataTrait.php

<?php

namespace Zend\Expressive\Session;

trait SessionDataTrait
{
    public static function generateToken() : string
    {
        return bin2hex(random_bytes(16));
    }

    public function getId(): string
    {
        return $this->id;
    }

    public function generateCsrfToken(string $keyName = '__csrf') : string
    {
        $this->data[$keyName] = static::generateToken();
        return $this->data[$keyName];
    }

    public function validateCsrfToken(string $token, string $csrfKey = '__csrf') : bool
    {
        if (! isset($this->data[$csrfKey])) {
            return false;
        }

        return $token === $this->data[$csrfKey];
    }
}

SessionMiddleware.php

<?php

namespace Zend\Expressive\Session;

use Interop\Http\ServerMiddleware\DelegateInterface;
use Interop\Http\ServerMiddleware\MiddlewareInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Message\ResponseInterface;

class SessionMiddleware implements MiddlewareInterface
{
    public function process(ServerRequestInterface $request, DelegateInterface $delegate) : ResponseInterface
    {
        $cookies = $request->getCookieParams();
        $id = $cookies[session_name()] ?? Session::generateToken();

        $this->startSession($id);

        $session = new Session($id, $_SESSION);

        $response = $delegate->process($request->withAttribute(Session::class, $session));

        if ($id !== $session->getId()) {
            $this->startSession($session->getId());
        }

        $_SESSION = $session->toArray();
        session_write_close();

        if (empty($_SESSION)) {
            return $response;
        }

        return $response->withHeader(
            'Set-Cookie',
            sprintf(
                '%s=%s; path=%s',
                session_name(),
                $session->getId(),
                ini_get('session.cookie_path')
            )
        );
    }

    private function startSession(string $id) : void
    {
        session_id($id);
        session_start([
            'use_cookies'      => false,
            'use_only_cookies' => true,
        ]);
    }
}

In oder to use Session::regenerateId() with ext/session we should call session_id() again in the middleware, after a new session_start(). Here the Session class updated:

namespace Zend\Expressive\Session;
use Interop\Http\ServerMiddleware\DelegateInterface;
use Interop\Http\ServerMiddleware\MiddlewareInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Message\ResponseInterface;
class SessionMiddleware implements MiddlewareInterface
{
    public function process(ServerRequestInterface $request, DelegateInterface $delegate) : ResponseInterface
    {
        $cookies = $request->getCookieParams();
        $id = $cookies[session_name()] ?? Session::generateToken();
        $this->startPhpSession($id);
        $session = new Session($id, $_SESSION);
        $response = $delegate->process($request->withAttribute(Session::class, $session));
        if ($id !== $session->getId()) {
            $this->startPhpSession($session->getId());
        }
        $_SESSION = $session->toArray();
        session_write_close();
        if (empty($_SESSION)) {
            return $response;
        }
        return $response->withHeader(
            'Set-Cookie',
            sprintf(
                '%s=%s; path=%s',
                session_name(),
                $session->getId(),
                ini_get('session.cookie_path')
            )
        );
    }
    protected function startPhpSession(string $id): void
    {
        session_id($id);
        session_start([
            'use_cookies'      => false,
            'use_only_cookies' => true,
        ]);
    }
}

Thanks, @ezimuel! Incorporated those changes into the gist now!

Am I going mad? It doesn't look like Session::toArray() actually returns the segment data. Maybe I'm missing something :|

I'm also wondering if the csrf stuff really belongs here. No harm in it, but it seems outside the session's concern.

@kynx — Thanks for the catch; I'd forgotten to assign the segment data to the returned data. I've fixed that now. (This is a rough draft; I want to settle on an API before we write tests.)

The CSRF stuff is quite useful to have here. It could be implemented using flash messages, potentially, but the point remains: they are inherently session-related (as you store a token in the session, and later compare a submitted token against it), and having a mechanism built-in is tremendously useful. I've used the similar capability that Aura.Session provides, and found the solution far simpler to manage than other solutions such as those found in zend-form and zend-validator. Considering that token generation is a matter of chaining two PHP native functions, a separate library for CSRF generation and validation feels like overkill — particularly considering that session ID generation can use the same algorithm.

I've spoken to @Ocramius, and one piece he suggested is lazy session capabilities. Essentially, the idea is that any work necessary in order to retrieve and/or deserialize the session data should be put off until session data is actually accessed. This is due to the fact that these two operations are the most expensive, and if we can delay them until first-use, we'll have less impact on the application and user experience (the user being the person using the website in this case).

He's drafted an approach in psr-7-session/storageless. The workflow requires an interface for the session class, with implementations performing the necessary work for manipulating session data. To delay session initialization, the method LazySession::fromContainerBuildingCallback() accepts a callable that performs the session initialization and creates the underlying SessionInterface implementation appropriate to the session strategy being used. In practice, that method is called by the session middleware, which creates a callable that performs that work. The session middleware also performs work before returning a response to determine if session data should be persisted, and, if so, it serializes it and persists it. Finally, the session middleware determines how to provide the response with information regarding the session (e.g., setting a session cookie).

We'll likely incorporate those ideas into this draft in the coming days, as the approach allows registering the session middleware to run on any request, without incurring the overhead of starting a session (and the operations of fetching it from persistence and/or deserialization).

@weierophinney I don't store my CSRF tokens in session for reasons outlined over here, so I'm loath to see a hard dependency on this in any future CSRF validator.

@kynx As you also noted in that thread, sessions satisfy the majority of users when it comes to CSRF. We wouldn't necessarily make this library a hard dependency for a CSRF validator, but we could ship an adapter that depends on this for one of the possible implementations.

@weierophinney thank you. That's what I wanted to hear :) So I won't cry if you ignore what follows.

What I liked about @ezimuel's original proposal was the simplicity. PHP sessions are just a way of bunging an array in temporary storage server side. They don't pre-suppose Segments or such like. Those things are undoubtedly useful, but do we need them for the "make sessions work with PSR-7" that was his original purpose? All those if (isset($this->segments[$name])) { bits are pretty smelly. They've tripped you up once already. Can't we move those up a level, so they don't pollute the core concept?

Can't we move those up a level, so they don't pollute the core concept?

The problem with moving them out is ensuring the object representing the main session data — in this case, Session — contains them when it comes time to persist data to whatever storage engine you use. Having them as objects composed in the Session class resolves that cleanly, as the segments then become references within it.

All those if (isset($this->segments[$name])) { bits are pretty smelly. They've tripped you up once already.

Once tests are in place, they no longer become an issue. As noted, this is a rough draft to examine and play with the API. I'm providing full code so that we can see the various interactions between objects. In this particular case, we can likely move those if/throw blocks into an assertion method to cut down on duplication — something along the lines of $this->assertNotSegment($name);.

While I want to honor the simplicity of Enrico's original design, we also have to keep an eye on future needs. We've already had folks wondering about how to implement flash messages for Expressive. The same is true for CSRF (I've had a need for them on several sites already!). Namespacing/segmenting sessions is also a common practice, though optional. However, the way segments interact with session data requires knowledge of segments on the part of the main session data. As such, I've proposed support for it here, so that we can make the code future-proof.

In an iteration I'm working on currently, I've moved the concept of flash messages and CSRF tokens to segments. This means that if all you want to do is work with session data, and have no need for those other features, you can ignore the segment() method and the associated class entirely.

Thanks for the lengthy reply. Yeah, I can see your point - if segments aren't there at the lowest level something somewhere along the line is likely to clobber them. Hope I wasn't too rude ;)

@kynx No worries! Just wanted to ensure we're all on the same page.

Full implementation (untested!) is here: https://github.com/weierophinney/zend-expressive-session

Contains:

• Adapter-based persistence approach
• Lazy-initialization of sessions
• Basic session implementation, with segment support
• Flash messages
• CSRF tokens and validation

@kynx — I've had quite a number of discussions with @Ocramius at this point, and decided to do the following:

• Remove "identifier" awareness in the SessionInterface. regenerate() produces a new instance, where isRegenerated() indicates that this happened. (The LazySession does not return a new instance, but does reset its internal proxy to the regenerated instance the original proxy produced.) This makes identifiers an implementation detail of persistence adapters.
• Remove all segment support. Ideally, this would be done by having separate sessions — which can be done with JWT, or by using a cache server for sessions instead of ext-session. Segments really only were needed for ext-session, and there are ways around it.
• Move flash messages to a sub-component, with their own interface and middleware, and consume the session.
• Likewise with CSRF.

I find that the base implementation is far simpler and more robust, while allowing for the flexibility of adding on features like flash messages and CSRF validations easily as add-ons consuming the component.

@weierophinney fantastic! I much prefer this approach. Having regenerate() return a new instance is a particularly good touch. I’ll have a proper play with the code as soon as I can. Thanks so much for all the work on this.