php-runtime/runtime

Session Handling with RoadRunnerSymfonyNyholm

alexander-schranz opened this issue · 2 comments

Currently there is no session handling implemented in the Runner.php. Symfony itself does never write a session cookie the the Response object so the session cookie is never set.

Expected Behaviour

First I want to demonstrate the expected behaviour, my research here is based on https://sulu.rocks/admin which currently runs on a normal apache server and the symfony demo using dev php webserver.

1. Visiting the Login site

As we see the login page, and have a look at our Cookies. There are 0 cookies set. So if a route is behind a firewall but the route is anonymous available no session cookies are written by symfony.

State Value
Request Cookie
Response Set-Cookie

2. Log into the Application with a false user

If we login to the application but no user is set also no session is started.

State Value
Request Cookie
Response Set-Cookie

3. Log into the Application with a correct user

Log into an application with a correct user not having a session.

State Value
Request Cookie
Response Set-Cookie my_session_id

4. Navigating inside the application

Aslong as we are now navigating the session_id should stay the same and never again a Set-Cookie should be called aslong as the session does not expire.

State Value
Request Cookie my_session_id
Response Set-Cookie

5. Logout from the application

If we are logout of the application the session cookie value be set to deleted and automatically be expired so its also removed from the browser cookies.

State Value
Request Cookie my_session_id
Response Set-Cookie deleted, expired 1970

6. Visiting with an expired session cookie

If we have an expired session cookie in our browser php will again add a Set-Cookie header that the browsers remove it.

State Value
Request Cookie my_session_id
Response Set-Cookie deleted, expired 1970

The Problems

Problem A: Session is not written to the Response Object (solved)

Currently the Session is not written to the Response Object.

Solution A1:

I currently did workaround this by doing this in my modified Runner.php by adding something like this:

                // ...
                $sessionName = $this->sessionOptions['name'] ?? \session_name();
                
                // ...
                    $sessionId = \session_id();

                    // ...
                        $sfResponse->headers->setCookie(
                            Cookie::create(
                                $sessionName,
                                $sessionId,
                                $expires,
                                $this->sessionOptions['cookie_path'] ?? '/',
                                $this->sessionOptions['cookie_domain'] ?? null,
                                $this->sessionOptions['cookie_secure'] ?? null,
                                $this->sessionOptions['cookie_httponly'] ?? true,
                                false,
                                $this->sessionOptions['cookie_samesite'] ?? Cookie::SAMESITE_LAX,
                            )
                        );

Solution A2: Symfony adds an option that the AbstractSessionListener will write this cookie when the session was saved here: https://github.com/symfony/symfony/blob/bbb0a694fbbd5a527b2236636271a47122b5d47c/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php#L110

Problem B: Cleanup session data at end of the request (solved)

As php is never shutdown we have the problem that we need cleanup all session based php states ourselve

Solution B1: I currently did this in the Runner.php by adding a finally block to the run method

finally {
    if (session_status() === PHP_SESSION_ACTIVE) {
         // abort the session if nobody did write it
         session_abort();
    }

    $_SESSION = []; // reset session array
    session_id(''); // else the next request would have the same session_id which would be a security problem
}

Solution B2: Something which maybe should be tackled with in PHP itself to add a function which would reset this. And so also avoid Problem C.

Problem C: SessionId is not read from new Request object (solved)

The session need also be read from the request object. As session_id will return empty string and php will not read it from the $_COOKIE variable.

Solution C1: In the Runner.php I did do this with:

$sessionName = $this->sessionOptions['name'] ?? \session_name();
$requestSessionId = $sfRequest->cookies->get($sessionName, '');
\session_id($requestSessionId);

Problem D: Use Symfony framework bundle options (solved)

As the runner is Symfony specific it should also use the Symfony Framework cookie configuration when it is set.

Solution D1: That the symfony configuration for the cookie is used, I booted once the kernel in the constructor to read the sessionOptions from it and use them to set the session cookie in the run method as seen above.

    /**
     * @var array<string, mixed>
     */
    private $sessionOptions;

    public function __construct(Kernel $kernel, ?HttpFoundationFactoryInterface $httpFoundationFactory = null, ?HttpMessageFactoryInterface $httpMessageFactory = null)
    {
        $this->kernel = $kernel;
        // ...
        
        $kernel->boot();
        $container = $kernel->getContainer();
        /** @var array<string, mixed> $sessionOptions */
        $this->sessionOptions = $container->getParameter('session.storage.options');
        $kernel->shutdown();
    }

Problem E: Write session only when really started or session id changed (unsolved)

Currently inside the Runner.php we don't know if a session is started or not this is because symfony does when calling session->save() inside the AbstractSessionListener, reseting the session state in its NativeSessionStorage. There would only be 1 hint that a session was started here which is the closed = true flag, sadly this variable is not exposed and so we never now in the Runner.php if a session was really started or not.

Solution E1: Symfony writes the session cookie at https://github.com/symfony/symfony/blob/bbb0a694fbbd5a527b2236636271a47122b5d47c/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php#L110 when a specific option is enabled.

Problem F: Unset Session Cookie when destroyed or expired (unsolved)

Currently we know never when a session is getting destroyed or should be removed so Set-Cookie with deleted and expire 1970 is never called.

I currently have yet a solution for this, see Point 5. and 6. above when this happens

Could you confirm that this issue can be closed?

With the changes in Symfony 5.4 all cases seems to be fixed now 🎉