Request for comments - Into making a generic service

[drupol]

Hello,

I have been researching methods for creating sessions without utilizing the $_SESSION superglobal variable and came across this package. I really like the concept of storing session data in a Cookie 🍪, in a JWT.

I am conducting this research for an authentication package that requires retaining information from requests and responses. Although I could use the traditional $_SESSION method, I am aiming to adhere to best practices and would like to explore alternatives. This is what led me to this project.

Until now, I was not very familiar with PSR-15 middlewares, but after spending some time testing and experimenting with them, I really liked the way it is being used. I developed a small proof-of-concept application using the Slim framework and created several custom middlewares to meet our requirements, particularly for authentication. The results have been flawless.

Now, I am considering how to integrate such a package into a library. The library in question deals with PSR requests and responses but does not utilize middlewares. To avoid reinventing the wheel, I propose creating a service called Psr7StoragelessManager that can operate with or without middleware. The proposed interface is as follows:

interface Psr7StoragelessManager
{
    public function getFromResponse(ResponseInterface $response, string $name): array;

    public function withResponse(ResponseInterface $response, string $name, array $data = []): ResponseInterface;

    public function getFromRequest(RequestInterface $request, string $name): array;

    public function withRequest(RequestInterface $request, string $name, array $data = []): RequestInterface;
}

This generic interface could then be incorporated into your original library by using a SessionInterface decorator on top, but also in other places where storing data securely in a cookie is needed.

The key differences between the logic implemented in SessionMiddleware and this approach involve:

1. The absence of cookie-refreshing capabilities. Data retrieval succeeds if completed within the specified timeframe; however, an exception is thrown if the data becomes outdated.
2. No request attribute is set, as it is not needed in this context. As a result, RequestInterface is utilized instead of ServerRequestInterface.
3. Users have the freedom to create multiple cookies without being restricted to just one.

I have successfully tested this proof of concept locally, it's working. I'm now looking for feedback:

1. What are your thoughts on this idea?
2. If this is a good idea, would it be preferable to include this interface within this project (through a PR) or create a separate project for it?

I appreciate your input and look forward to hearing your opinions.

Thanks.

[Ocramius]

My endorsement would be to simplify further:

interface SessionStorage
{
    public function get(ServerRequestInterface $request): SessionInterface;
    public function store(ServerRequestInterface $request, SessionInterface $session): ServerRequestInterface;
}

Usage would then be similar to a repository:

class MyHandler implements RequestHandler
{
    public function __construct(private readonly SessionStorage $storage) {}
    public function handle(ServerRequestInterface $request): ResponseInterface
    {
        // ...
        $session = $this->storage->get($request);
        $userId = $session>get('user');
        // ... do something with user
        $session>set('user', $someValue);
        $request = $this->storage->store($session);
        // ...
    }
}

Note: the only abstraction provided by SessionStorage is removing the dependency on the attribute name.

The absence of cookie-refreshing capabilities. Data retrieval succeeds if completed within the specified timeframe; however, an exception is thrown if the data becomes outdated.

Careful: throwing an exception is not really wished here. The point is rejecting invalid data, not leading to a DoS attack, or an enumeration attack, due to exceptions being raised.

As a result, RequestInterface is utilized instead of ServerRequestInterface.

Not sure what value this brings, tbh: most request handlers and middleware operate with server requests anyway. PSR-18 operates with RequestInterface, but that's a different story.

Users have the freedom to create multiple cookies without being restricted to just one.

This is already possible by configuring the middleware multiple times

[drupol]

Thank you.

I've been testing the interface, but I couldn't get it to work "as-is" because the cookie should be stored within the Response, just like it is done in SessionMiddleware. This is also the reason why the previous proposal was able to save in Requests and Responses (updated to work with SessionInterface here). It's possible that I'm misunderstanding something.

Instead, I made the following modifications:

interface SessionStorage
{
    public function get(ServerRequestInterface $request): SessionInterface;
    public function store(ResponseInterface $response, SessionInterface $session): ResponseInterface;
}

In response(no pun intended!) to your feedback, I also removed the exceptions throwing.

Please find the concrete implementation here and let me know your thoughts.

[Ocramius]

What I'm missing is the use-case though.

Going back to OP:

The library in question deals with PSR requests and responses but does not utilize middlewares.

What is the issue here? 🤔

[drupol]

What I'm missing is the use-case though.

The authentication library need to save in the session some fingerprints that are checked before and after authentication. That's basically a customization of the CAS protocol.

What is the issue here? 🤔

No issue in there, but using a middleware (ex: SessionMiddleware) in there seems to be quite a challenge.

[Ocramius]

Hmm, it sounds like you want to operate "with a session", rather than "with a middleware".

What you want is this part, extracted:

storageless/src/Storageless/Http/SessionMiddleware.php

Lines 128 to 137 in a789d99

	$token = $this->parseToken($request);
	$sessionContainer = LazySession::fromContainerBuildingCallback(function () use ($token): SessionInterface {
	return $this->extractSessionContainer($token);
	});
	return $this->appendToken(
	$sessionContainer,
	$handler->handle($request->withAttribute($this->sessionAttribute, $sessionContainer)),
	$token,
	);

The interface could be:

interface WithSession {
    /**
     * @template T  
     * @param callable(SessionInterface $session): T $callback
     * @return T
     */
    public function handle(Request $request, Response $response, callback $callback): mixed;
}

The implementation would require extracting some logic, but nothing too fancy.

Usage in your code, for example a non-PSR-15 controller:

class SomeController extends SomeFrameworkThing {
    public function myAction(): SomeResponse {
        $this->withSession->handle($this->psr7Request, $this->response, function (SessionInterface $session): void {
            $session->get('foo');
            $session->set('bar', 'baz');
        });
        return $this->response;
    } 
}

As for why that would be hard to integrate: it's possible to build the above abstraction with an anonymous class in pre-existing logic, so I suggest attempting the PSR-15 way, before introducing more complexity in upstream libraries.

You may also want to look at @Slamdunk's #564 proposal, where he's actually trying to introduce middleware logic to check session validity/fingerprint.

[drupol]

Marco,

I've implemented and tested the new interface proposal, and it works. One aspect that really puzzled me initially was the requirement of a Request to construct a new Response. I hadn't realized the importance of this connection.

I've shared a new proof of concept service here, which incorporates the fingerprint logic from #564 (eagerly awaiting that PR to be merged!).

The main difference between your suggestion and mine is that the handle method returns a Response instead of mixed. After evaluating both methods (yours and mine), I determined that, since SessionInterface is stateful, there's no need for the provided callback to return anything. WDYT?

Also, do you believe that such a class would fit well within this project?

Thank you.

[drupol]

Hello,

I've been working the whole day on this, trying to devise the most efficient approach for creating a service abstraction. I have developed this solution (also visible in a PR against my fork for the moment), which I believe addresses the initial requirement effectively.

In essence, the newly implemented service adheres to the SessionStorage interface as follows:

interface SessionStorage
{
    public function get(RequestInterface $request): SessionInterface;

    /**
     * @param Closure(SessionInterface): SessionInterface $callback
     */
    public function handle(RequestInterface $request, ResponseInterface $response, Closure $callback): ResponseInterface;
}

I introduced the get method to enable the retrieval of the current session (for instance, displaying its content in the debug area).

Additionally, I've included a potential implementation of SessionMiddleware within the gist, assuming we decide to incorporate such a service into this project.

I'd greatly appreciate your feedback on my proposed solution.

Thank you.

[drupol]

Hello,

I have updated the local PR based on the outcomes of our discussion. Since my last comment, I've updated the interface, you can find the details in the PR.

A new service, StoragelessSession, has been introduced and is now incorporated within SessionMiddleware.

This service allows for seamless manipulation of a SessionInterface . Since this service is not a middleware, it becomes the user's responsibility to save the session back into the Response.

All tests have passed successfully, with the only required changes being made to the class constructors.

Link to the PR: #572

Let me know what you htink.

Thank you.