From a0105eb40b490e7b4697def543b0c8663babac82 Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Sat, 10 Feb 2018 12:04:07 +1030 Subject: [PATCH] Use Illuminate Session component instead of Symfony Symfony's component relies on PHP's native session functionality, which is not ideal. It automatically sets its own cookie headers, resulting in this issue: https://github.com/flarum/core/issues/1084#issuecomment-364569953 The Illuminate component is more powerful and has a simpler API for extension with other drivers and such, and fits in nicely with other components we use (the majority of which are from Illuminate). --- framework/core/composer.json | 1 + .../Controller/AbstractOAuth2Controller.php | 2 +- .../src/Forum/Controller/LogOutController.php | 2 +- .../Controller/ResetPasswordController.php | 2 +- framework/core/src/Foundation/Application.php | 2 + framework/core/src/Foundation/Site.php | 11 ++++ framework/core/src/Frontend/FrontendView.php | 2 +- .../Middleware/AuthenticateWithSession.php | 4 +- .../Http/Middleware/RememberFromCookie.php | 19 +++++- .../core/src/Http/Middleware/StartSession.php | 61 +++++++++++++------ .../core/src/Http/SessionAuthenticator.php | 15 ++--- framework/core/src/User/User.php | 8 +-- 12 files changed, 91 insertions(+), 38 deletions(-) diff --git a/framework/core/composer.json b/framework/core/composer.json index cc9151b2d..27c91e627 100644 --- a/framework/core/composer.json +++ b/framework/core/composer.json @@ -35,6 +35,7 @@ "illuminate/filesystem": "5.5.*", "illuminate/hashing": "5.5.*", "illuminate/mail": "5.5.*", + "illuminate/session": "5.5.*", "illuminate/support": "5.5.*", "illuminate/validation": "5.5.*", "illuminate/view": "5.5.*", diff --git a/framework/core/src/Forum/Controller/AbstractOAuth2Controller.php b/framework/core/src/Forum/Controller/AbstractOAuth2Controller.php index 52f8c1e17..ed0946dda 100644 --- a/framework/core/src/Forum/Controller/AbstractOAuth2Controller.php +++ b/framework/core/src/Forum/Controller/AbstractOAuth2Controller.php @@ -62,7 +62,7 @@ abstract class AbstractOAuth2Controller implements ControllerInterface if (! $code) { $authUrl = $this->provider->getAuthorizationUrl($this->getAuthorizationUrlOptions()); - $session->set('oauth2state', $this->provider->getState()); + $session->put('oauth2state', $this->provider->getState()); return new RedirectResponse($authUrl.'&display=popup'); } elseif (! $state || $state !== $session->get('oauth2state')) { diff --git a/framework/core/src/Forum/Controller/LogOutController.php b/framework/core/src/Forum/Controller/LogOutController.php index c819fec7d..86bb45480 100644 --- a/framework/core/src/Forum/Controller/LogOutController.php +++ b/framework/core/src/Forum/Controller/LogOutController.php @@ -102,7 +102,7 @@ class LogOutController implements ControllerInterface // If a valid CSRF token hasn't been provided, show a view which will // allow the user to press a button to complete the log out process. - $csrfToken = $session->get('csrf_token'); + $csrfToken = $session->token(); if (array_get($request->getQueryParams(), 'token') !== $csrfToken) { $return = array_get($request->getQueryParams(), 'return'); diff --git a/framework/core/src/Forum/Controller/ResetPasswordController.php b/framework/core/src/Forum/Controller/ResetPasswordController.php index 3752ead21..a95dd07cc 100644 --- a/framework/core/src/Forum/Controller/ResetPasswordController.php +++ b/framework/core/src/Forum/Controller/ResetPasswordController.php @@ -50,6 +50,6 @@ class ResetPasswordController extends AbstractHtmlController return $this->view->make('flarum.forum::reset-password') ->with('passwordToken', $token->id) - ->with('csrfToken', $request->getAttribute('session')->get('csrf_token')); + ->with('csrfToken', $request->getAttribute('session')->token()); } } diff --git a/framework/core/src/Foundation/Application.php b/framework/core/src/Foundation/Application.php index 2753291b5..1c8ba7718 100644 --- a/framework/core/src/Foundation/Application.php +++ b/framework/core/src/Foundation/Application.php @@ -718,6 +718,8 @@ class Application extends Container implements ApplicationContract 'filesystem.cloud' => [\Illuminate\Contracts\Filesystem\Cloud::class], 'hash' => [\Illuminate\Contracts\Hashing\Hasher::class], 'mailer' => [\Illuminate\Mail\Mailer::class, \Illuminate\Contracts\Mail\Mailer::class, \Illuminate\Contracts\Mail\MailQueue::class], + 'session' => [\Illuminate\Session\SessionManager::class], + 'session.driver' => [\Illuminate\Contracts\Session\Session::class], 'validator' => [\Illuminate\Validation\Factory::class, \Illuminate\Contracts\Validation\Factory::class], 'view' => [\Illuminate\View\Factory::class, \Illuminate\Contracts\View\Factory::class], ]; diff --git a/framework/core/src/Foundation/Site.php b/framework/core/src/Foundation/Site.php index a86f7d6e1..f1978f217 100644 --- a/framework/core/src/Foundation/Site.php +++ b/framework/core/src/Foundation/Site.php @@ -33,6 +33,7 @@ use Illuminate\Config\Repository as ConfigRepository; use Illuminate\Filesystem\FilesystemServiceProvider; use Illuminate\Hashing\HashServiceProvider; use Illuminate\Mail\MailServiceProvider; +use Illuminate\Session\SessionServiceProvider; use Illuminate\Validation\ValidationServiceProvider; use Illuminate\View\ViewServiceProvider; use Monolog\Formatter\LineFormatter; @@ -169,6 +170,7 @@ class Site $app->register(FilesystemServiceProvider::class); $app->register(HashServiceProvider::class); $app->register(MailServiceProvider::class); + $app->register(SessionServiceProvider::class); $app->register(ViewServiceProvider::class); $app->register(ValidationServiceProvider::class); @@ -236,6 +238,15 @@ class Site 'root' => $app->publicPath().'/assets/avatars' ] ] + ], + 'session' => [ + 'driver' => 'file', + 'lifetime' => 120, + 'expire_on_close' => false, + 'encrypt' => false, + 'files' => $app->storagePath().'/sessions', + 'lottery' => [2, 100], + 'cookie' => 'session' ] ]); } diff --git a/framework/core/src/Frontend/FrontendView.php b/framework/core/src/Frontend/FrontendView.php index 45d36d12c..0063384d3 100644 --- a/framework/core/src/Frontend/FrontendView.php +++ b/framework/core/src/Frontend/FrontendView.php @@ -464,7 +464,7 @@ class FrontendView return [ 'userId' => $actor->id, - 'csrfToken' => $session->get('csrf_token') + 'csrfToken' => $session->token() ]; } diff --git a/framework/core/src/Http/Middleware/AuthenticateWithSession.php b/framework/core/src/Http/Middleware/AuthenticateWithSession.php index 6c6729f16..4c780925b 100644 --- a/framework/core/src/Http/Middleware/AuthenticateWithSession.php +++ b/framework/core/src/Http/Middleware/AuthenticateWithSession.php @@ -13,10 +13,10 @@ namespace Flarum\Http\Middleware; use Flarum\User\Guest; use Flarum\User\User; +use Illuminate\Contracts\Session\Session; use Interop\Http\ServerMiddleware\DelegateInterface; use Interop\Http\ServerMiddleware\MiddlewareInterface; use Psr\Http\Message\ServerRequestInterface as Request; -use Symfony\Component\HttpFoundation\Session\SessionInterface; class AuthenticateWithSession implements MiddlewareInterface { @@ -33,7 +33,7 @@ class AuthenticateWithSession implements MiddlewareInterface return $delegate->process($request); } - private function getActor(SessionInterface $session) + private function getActor(Session $session) { $actor = User::find($session->get('user_id')) ?: new Guest; diff --git a/framework/core/src/Http/Middleware/RememberFromCookie.php b/framework/core/src/Http/Middleware/RememberFromCookie.php index 96cf1e289..335c43198 100644 --- a/framework/core/src/Http/Middleware/RememberFromCookie.php +++ b/framework/core/src/Http/Middleware/RememberFromCookie.php @@ -12,15 +12,29 @@ namespace Flarum\Http\Middleware; use Flarum\Http\AccessToken; +use Flarum\Http\CookieFactory; use Interop\Http\ServerMiddleware\DelegateInterface; use Interop\Http\ServerMiddleware\MiddlewareInterface; use Psr\Http\Message\ServerRequestInterface as Request; class RememberFromCookie implements MiddlewareInterface { + /** + * @var CookieFactory + */ + protected $cookie; + + /** + * @param CookieFactory $cookie + */ + public function __construct(CookieFactory $cookie) + { + $this->cookie = $cookie; + } + public function process(Request $request, DelegateInterface $delegate) { - $id = array_get($request->getCookieParams(), 'flarum_remember'); + $id = array_get($request->getCookieParams(), $this->cookie->getName('remember')); if ($id) { $token = AccessToken::find($id); @@ -28,8 +42,9 @@ class RememberFromCookie implements MiddlewareInterface if ($token) { $token->touch(); + /** @var \Illuminate\Contracts\Session\Session $session */ $session = $request->getAttribute('session'); - $session->set('user_id', $token->user_id); + $session->put('user_id', $token->user_id); } } diff --git a/framework/core/src/Http/Middleware/StartSession.php b/framework/core/src/Http/Middleware/StartSession.php index 239fed9c1..701a30732 100644 --- a/framework/core/src/Http/Middleware/StartSession.php +++ b/framework/core/src/Http/Middleware/StartSession.php @@ -13,17 +13,19 @@ namespace Flarum\Http\Middleware; use Dflydev\FigCookies\FigResponseCookies; use Flarum\Http\CookieFactory; -use Illuminate\Support\Str; +use Illuminate\Contracts\Session\Session; +use Illuminate\Session\SessionManager; use Interop\Http\ServerMiddleware\DelegateInterface; use Interop\Http\ServerMiddleware\MiddlewareInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; -use Symfony\Component\HttpFoundation\Session\Session; -use Symfony\Component\HttpFoundation\Session\SessionInterface; class StartSession implements MiddlewareInterface { - const COOKIE_NAME = 'session'; + /** + * @var SessionManager + */ + protected $manager; /** * @var CookieFactory @@ -31,54 +33,75 @@ class StartSession implements MiddlewareInterface protected $cookie; /** + * @param SessionManager $manager * @param CookieFactory $cookie */ - public function __construct(CookieFactory $cookie) + public function __construct(SessionManager $manager, CookieFactory $cookie) { + $this->manager = $manager; $this->cookie = $cookie; } public function process(Request $request, DelegateInterface $delegate) { - $session = $this->startSession(); + $request = $request->withAttribute('session', + $session = $this->startSession($request) + ); - $request = $request->withAttribute('session', $session); + $this->collectGarbage($session); $response = $delegate->process($request); + $session->save(); + $response = $this->withCsrfTokenHeader($response, $session); return $this->withSessionCookie($response, $session); } - private function startSession() + private function startSession(Request $request) { - $session = new Session; + $session = $this->manager->driver(); - $session->setName($this->cookie->getName(self::COOKIE_NAME)); + $id = array_get($request->getCookieParams(), $this->cookie->getName($session->getName())); + + $session->setId($id); $session->start(); - if (! $session->has('csrf_token')) { - $session->set('csrf_token', Str::random(40)); - } - return $session; } - private function withCsrfTokenHeader(Response $response, SessionInterface $session) + private function collectGarbage(Session $session) { - if ($session->has('csrf_token')) { - $response = $response->withHeader('X-CSRF-Token', $session->get('csrf_token')); + $config = $this->manager->getSessionConfig(); + + if ($this->configHitsLottery($config)) { + $session->getHandler()->gc($this->getSessionLifetimeInSeconds()); } + } + + private function configHitsLottery(array $config) + { + return random_int(1, $config['lottery'][1]) <= $config['lottery'][0]; + } + + private function withCsrfTokenHeader(Response $response, Session $session) + { + $response = $response->withHeader('X-CSRF-Token', $session->token()); return $response; } - private function withSessionCookie(Response $response, SessionInterface $session) + private function withSessionCookie(Response $response, Session $session) { return FigResponseCookies::set( $response, - $this->cookie->make(self::COOKIE_NAME, $session->getId()) + $this->cookie->make($session->getName(), $session->getId(), $this->getSessionLifetimeInSeconds()) ); } + + private function getSessionLifetimeInSeconds() + { + return ($this->manager->getSessionConfig()['lifetime'] ?? null) * 60; + } } diff --git a/framework/core/src/Http/SessionAuthenticator.php b/framework/core/src/Http/SessionAuthenticator.php index 8ccb06524..8e23027a0 100644 --- a/framework/core/src/Http/SessionAuthenticator.php +++ b/framework/core/src/Http/SessionAuthenticator.php @@ -11,25 +11,26 @@ namespace Flarum\Http; -use Symfony\Component\HttpFoundation\Session\SessionInterface; +use Illuminate\Contracts\Session\Session; class SessionAuthenticator { /** - * @param SessionInterface $session + * @param Session $session * @param int $userId */ - public function logIn(SessionInterface $session, $userId) + public function logIn(Session $session, $userId) { - $session->migrate(); - $session->set('user_id', $userId); + $session->regenerate(true); + $session->put('user_id', $userId); } /** - * @param SessionInterface $session + * @param Session $session */ - public function logOut(SessionInterface $session) + public function logOut(Session $session) { $session->invalidate(); + $session->regenerateToken(); } } diff --git a/framework/core/src/User/User.php b/framework/core/src/User/User.php index 3a695aa78..fa30ec01a 100644 --- a/framework/core/src/User/User.php +++ b/framework/core/src/User/User.php @@ -34,7 +34,7 @@ use Flarum\User\Event\PasswordChanged; use Flarum\User\Event\Registered; use Flarum\User\Event\Renamed; use Illuminate\Contracts\Hashing\Hasher; -use Symfony\Component\HttpFoundation\Session\SessionInterface; +use Illuminate\Contracts\Session\Session; /** * @property int $id @@ -698,7 +698,7 @@ class User extends AbstractModel } /** - * @return SessionInterface + * @return Session */ public function getSession() { @@ -706,9 +706,9 @@ class User extends AbstractModel } /** - * @param SessionInterface $session + * @param Session $session */ - public function setSession(SessionInterface $session) + public function setSession(Session $session) { $this->session = $session; }