Open drupol opened 1 year ago
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 ofServerRequestInterface
.
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
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.
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? :thinking:
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? :thinking:
No issue in there, but using a middleware (ex: SessionMiddleware
) in there seems to be quite a challenge.
Hmm, it sounds like you want to operate "with a session", rather than "with a middleware".
What you want is this part, extracted:
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.
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.
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.
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.
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 :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: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:request attribute
is set, as it is not needed in this context. As a result,RequestInterface
is utilized instead ofServerRequestInterface
.I have successfully tested this proof of concept locally, it's working. I'm now looking for feedback:
I appreciate your input and look forward to hearing your opinions.
Thanks.