From ea4e930cbb958c6a543999f5ea19c52f1319efce Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Thu, 11 Jun 2020 16:54:51 +0200 Subject: [PATCH] Store sessions in database (#1736) --- UPGRADING.md | 1 + config/packages/framework.yaml | 7 ++- config/services.yaml | 5 ++ .../RedirectToLocaleSubscriber.php | 3 - src/Migrations/Version20200524142042.php | 41 ++++++++++++++ src/Plugin/PluginManager.php | 2 - src/Security/AclDecisionManager.php | 5 +- src/Security/SessionHandler.php | 27 +++++++++ tests/Command/CreateReleaseCommandTest.php | 39 +++++++++++++ tests/Command/ImportCustomerCommandTest.php | 51 +++++++++++++++++ tests/Command/ImportTimesheetCommandTest.php | 53 ++++++++++++++++++ tests/Command/KimaiImporterCommandTest.php | 47 ++++++++++++++++ tests/Command/ReloadCommandTest.php | 39 +++++++++++++ tests/Command/ResetCommandTest.php | 39 +++++++++++++ tests/Controller/PluginControllerTest.php | 2 +- .../RedirectToLocaleSubscriberTest.php | 49 ++++++++++++++++ .../Fixtures/{ => TestPlugin}/TestPlugin.php | 2 +- .../Fixtures/{ => TestPlugin}/composer.json | 0 .../Fixtures/TestPlugin2/TestPlugin2.php | 31 ++++++++++ tests/Plugin/PluginManagerTest.php | 13 ++--- tests/Security/AclDecisionManagerTest.php | 45 +++++++++++++++ tests/Security/SessionHandlerTest.php | 26 +++++++++ var/data/kimai_test.sqlite | Bin 770048 -> 770048 bytes 23 files changed, 507 insertions(+), 20 deletions(-) create mode 100644 src/Migrations/Version20200524142042.php create mode 100644 src/Security/SessionHandler.php create mode 100644 tests/Command/CreateReleaseCommandTest.php create mode 100644 tests/Command/ImportCustomerCommandTest.php create mode 100644 tests/Command/ImportTimesheetCommandTest.php create mode 100644 tests/Command/KimaiImporterCommandTest.php create mode 100644 tests/Command/ReloadCommandTest.php create mode 100644 tests/Command/ResetCommandTest.php create mode 100644 tests/EventSubscriber/RedirectToLocaleSubscriberTest.php rename tests/Plugin/Fixtures/{ => TestPlugin}/TestPlugin.php (91%) rename tests/Plugin/Fixtures/{ => TestPlugin}/composer.json (100%) create mode 100644 tests/Plugin/Fixtures/TestPlugin2/TestPlugin2.php create mode 100644 tests/Security/AclDecisionManagerTest.php create mode 100644 tests/Security/SessionHandlerTest.php diff --git a/UPGRADING.md b/UPGRADING.md index 3aed41c4..2dcb6dec 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -11,6 +11,7 @@ Perform EACH version specific task between your version and the new one, otherwi ## [1.10](https://github.com/kevinpapst/kimai2/releases/tag/1.10) - Invoice renderer `CSV` was removed +- Sessions are now stored in the database (all users have to re-login after upgrade) ### Developer diff --git a/config/packages/framework.yaml b/config/packages/framework.yaml index 29c33e2b..ab8be95d 100644 --- a/config/packages/framework.yaml +++ b/config/packages/framework.yaml @@ -6,9 +6,12 @@ framework: # Enables session support. Note that the session will ONLY be started if you read or write from it. # Remove or comment this section to explicitly disable session support. + #session: + # handler_id: session.handler.native_file + # save_path: "%kernel.project_dir%/var/sessions/%kernel.environment%" + session: - handler_id: session.handler.native_file - save_path: "%kernel.project_dir%/var/sessions/%kernel.environment%" + handler_id: App\Security\SessionHandler #esi: ~ #fragments: ~ diff --git a/config/services.yaml b/config/services.yaml index 2b26359b..6a65951a 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -104,6 +104,11 @@ services: tags: - { name: doctrine.event_listener, event: postConnect } + # store and retrieve sessions in and from database + App\Security\SessionHandler: + arguments: + - !service { class: PDO, factory: ['@database_connection', 'getWrappedConnection'] } + # ================================================================================ # FORMS # ================================================================================ diff --git a/src/EventSubscriber/RedirectToLocaleSubscriber.php b/src/EventSubscriber/RedirectToLocaleSubscriber.php index 9c81f7cd..b8bf1289 100644 --- a/src/EventSubscriber/RedirectToLocaleSubscriber.php +++ b/src/EventSubscriber/RedirectToLocaleSubscriber.php @@ -54,9 +54,6 @@ class RedirectToLocaleSubscriber implements EventSubscriberInterface $this->urlGenerator = $urlGenerator; $this->locales = explode('|', trim($locales)); - if (empty($this->locales)) { - throw new \UnexpectedValueException('The list of supported locales must not be empty.'); - } $this->defaultLocale = $defaultLocale ?: $this->locales[0]; if (!\in_array($this->defaultLocale, $this->locales)) { diff --git a/src/Migrations/Version20200524142042.php b/src/Migrations/Version20200524142042.php new file mode 100644 index 00000000..8e49cc00 --- /dev/null +++ b/src/Migrations/Version20200524142042.php @@ -0,0 +1,41 @@ +createTable('kimai2_sessions'); + $sessions->addColumn('id', 'string', ['length' => 128, 'notnull' => true]); + $sessions->addColumn('data', 'blob', ['length' => 65535, 'notnull' => true]); + $sessions->addColumn('time', 'integer', ['unsigned' => true, 'notnull' => true]); + $sessions->addColumn('lifetime', 'integer', ['unsigned' => true, 'notnull' => true]); + $sessions->setPrimaryKey(['id']); + } + + public function down(Schema $schema): void + { + $schema->dropTable('kimai2_sessions'); + } +} diff --git a/src/Plugin/PluginManager.php b/src/Plugin/PluginManager.php index db3ce184..5df38062 100644 --- a/src/Plugin/PluginManager.php +++ b/src/Plugin/PluginManager.php @@ -20,7 +20,6 @@ class PluginManager /** * @param PluginInterface[] $plugins - * @throws \Exception */ public function __construct(iterable $plugins) { @@ -31,7 +30,6 @@ class PluginManager /** * @param PluginInterface $plugin - * @throws \Exception */ public function addPlugin(PluginInterface $plugin) { diff --git a/src/Security/AclDecisionManager.php b/src/Security/AclDecisionManager.php index 355ca33f..7ef01a4b 100644 --- a/src/Security/AclDecisionManager.php +++ b/src/Security/AclDecisionManager.php @@ -17,11 +17,8 @@ class AclDecisionManager /** * @var AccessDecisionManagerInterface */ - protected $decisionManager; + private $decisionManager; - /** - * @param AccessDecisionManagerInterface $decisionManager - */ public function __construct(AccessDecisionManagerInterface $decisionManager) { $this->decisionManager = $decisionManager; diff --git a/src/Security/SessionHandler.php b/src/Security/SessionHandler.php new file mode 100644 index 00000000..35928c22 --- /dev/null +++ b/src/Security/SessionHandler.php @@ -0,0 +1,27 @@ + 'kimai2_sessions', + 'db_id_col' => 'id', + 'db_data_col' => 'data', + 'db_lifetime_col' => 'lifetime', + 'db_time_col' => 'time', + 'lock_mode' => PdoSessionHandler::LOCK_ADVISORY, + ]); + } +} diff --git a/tests/Command/CreateReleaseCommandTest.php b/tests/Command/CreateReleaseCommandTest.php new file mode 100644 index 00000000..8c14af8e --- /dev/null +++ b/tests/Command/CreateReleaseCommandTest.php @@ -0,0 +1,39 @@ +application = new Application($kernel); + $this->application->add(new CreateReleaseCommand(realpath(__DIR__ . '/../../'))); + } + + public function testCommandName() + { + $command = $this->application->find('kimai:create-release'); + self::assertInstanceOf(CreateReleaseCommand::class, $command); + } +} diff --git a/tests/Command/ImportCustomerCommandTest.php b/tests/Command/ImportCustomerCommandTest.php new file mode 100644 index 00000000..1fca83e3 --- /dev/null +++ b/tests/Command/ImportCustomerCommandTest.php @@ -0,0 +1,51 @@ +application = new Application($kernel); + + $customers = $this->createMock(CustomerRepository::class); + $projects = $this->createMock(ProjectRepository::class); + $teams = $this->createMock(TeamRepository::class); + $users = $this->createMock(UserRepository::class); + $configuration = $this->createMock(FormConfiguration::class); + + $this->application->add(new ImportCustomerCommand($customers, $projects, $teams, $users, $configuration)); + } + + public function testCommandName() + { + $command = $this->application->find('kimai:import:customer'); + self::assertInstanceOf(ImportCustomerCommand::class, $command); + } +} diff --git a/tests/Command/ImportTimesheetCommandTest.php b/tests/Command/ImportTimesheetCommandTest.php new file mode 100644 index 00000000..dc79f26a --- /dev/null +++ b/tests/Command/ImportTimesheetCommandTest.php @@ -0,0 +1,53 @@ +application = new Application($kernel); + + $customers = $this->createMock(CustomerRepository::class); + $projects = $this->createMock(ProjectRepository::class); + $activities = $this->createMock(ActivityRepository::class); + $users = $this->createMock(UserRepository::class); + $timesheets = $this->createMock(TimesheetRepository::class); + $configuration = $this->createMock(FormConfiguration::class); + + $this->application->add(new ImportTimesheetCommand($customers, $projects, $activities, $users, $timesheets, $configuration)); + } + + public function testCommandName() + { + $command = $this->application->find('kimai:import:timesheet'); + self::assertInstanceOf(ImportTimesheetCommand::class, $command); + } +} diff --git a/tests/Command/KimaiImporterCommandTest.php b/tests/Command/KimaiImporterCommandTest.php new file mode 100644 index 00000000..ebe51083 --- /dev/null +++ b/tests/Command/KimaiImporterCommandTest.php @@ -0,0 +1,47 @@ +application = new Application($kernel); + + $encoder = $this->createMock(UserPasswordEncoderInterface::class); + $registry = $this->createMock(ManagerRegistry::class); + $validator = $this->createMock(ValidatorInterface::class); + + $this->application->add(new KimaiImporterCommand($encoder, $registry, $validator)); + } + + public function testCommandName() + { + $command = $this->application->find('kimai:import-v1'); + self::assertInstanceOf(KimaiImporterCommand::class, $command); + } +} diff --git a/tests/Command/ReloadCommandTest.php b/tests/Command/ReloadCommandTest.php new file mode 100644 index 00000000..2e4f42f8 --- /dev/null +++ b/tests/Command/ReloadCommandTest.php @@ -0,0 +1,39 @@ +application = new Application($kernel); + $this->application->add(new ReloadCommand()); + } + + public function testCommandName() + { + $command = $this->application->find('kimai:reload'); + self::assertInstanceOf(ReloadCommand::class, $command); + } +} diff --git a/tests/Command/ResetCommandTest.php b/tests/Command/ResetCommandTest.php new file mode 100644 index 00000000..6e6655b8 --- /dev/null +++ b/tests/Command/ResetCommandTest.php @@ -0,0 +1,39 @@ +application = new Application($kernel); + $this->application->add(new ResetCommand()); + } + + public function testCommandName() + { + $command = $this->application->find('kimai:reset-dev'); + self::assertInstanceOf(ResetCommand::class, $command); + } +} diff --git a/tests/Controller/PluginControllerTest.php b/tests/Controller/PluginControllerTest.php index cd95e5d3..4644d315 100644 --- a/tests/Controller/PluginControllerTest.php +++ b/tests/Controller/PluginControllerTest.php @@ -11,7 +11,7 @@ namespace App\Tests\Controller; use App\Entity\User; use App\Plugin\PluginManager; -use App\Tests\Plugin\Fixtures\TestPlugin; +use App\Tests\Plugin\Fixtures\TestPlugin\TestPlugin; /** * @group integration diff --git a/tests/EventSubscriber/RedirectToLocaleSubscriberTest.php b/tests/EventSubscriber/RedirectToLocaleSubscriberTest.php new file mode 100644 index 00000000..da6e8a6a --- /dev/null +++ b/tests/EventSubscriber/RedirectToLocaleSubscriberTest.php @@ -0,0 +1,49 @@ +createMock(UrlGeneratorInterface::class); + $sut = new RedirectToLocaleSubscriber($urlGenerator, 'de|en', 'en'); + + self::assertEquals([KernelEvents::REQUEST => ['onKernelRequest']], RedirectToLocaleSubscriber::getSubscribedEvents()); + + $request = $this->createMock(Request::class); + $request->expects($this->once())->method('getPathInfo')->willReturn('/de'); + + $event = $this->createMock(RequestEvent::class); + $event->expects($this->once())->method('getRequest')->willReturn($request); + $event->expects($this->never())->method('setResponse'); + + $sut->onKernelRequest($event); + } + + public function testConstructWithUnknownDefaultLocale() + { + $this->expectException(\UnexpectedValueException::class); + $this->expectExceptionMessage('The default locale ("en") must be one of "de|it".'); + + $urlGenerator = $this->createMock(UrlGeneratorInterface::class); + $sut = new RedirectToLocaleSubscriber($urlGenerator, 'de|it', 'en'); + } +} diff --git a/tests/Plugin/Fixtures/TestPlugin.php b/tests/Plugin/Fixtures/TestPlugin/TestPlugin.php similarity index 91% rename from tests/Plugin/Fixtures/TestPlugin.php rename to tests/Plugin/Fixtures/TestPlugin/TestPlugin.php index bd13b279..ede822ac 100644 --- a/tests/Plugin/Fixtures/TestPlugin.php +++ b/tests/Plugin/Fixtures/TestPlugin/TestPlugin.php @@ -7,7 +7,7 @@ * file that was distributed with this source code. */ -namespace App\Tests\Plugin\Fixtures; +namespace App\Tests\Plugin\Fixtures\TestPlugin; use App\Plugin\PluginInterface; diff --git a/tests/Plugin/Fixtures/composer.json b/tests/Plugin/Fixtures/TestPlugin/composer.json similarity index 100% rename from tests/Plugin/Fixtures/composer.json rename to tests/Plugin/Fixtures/TestPlugin/composer.json diff --git a/tests/Plugin/Fixtures/TestPlugin2/TestPlugin2.php b/tests/Plugin/Fixtures/TestPlugin2/TestPlugin2.php new file mode 100644 index 00000000..babfbc14 --- /dev/null +++ b/tests/Plugin/Fixtures/TestPlugin2/TestPlugin2.php @@ -0,0 +1,31 @@ +getMockBuilder(PluginInterface::class) - ->onlyMethods(['getName', 'getPath']) - ->getMock(); - - $plugin->method('getName')->willReturn('foo'); - $plugin->method('getPath')->willReturn('bar'); + $plugin = $this->createMock(PluginInterface::class); + $plugin->expects($this->any())->method('getName')->willReturn('foo'); + $plugin->expects($this->any())->method('getPath')->willReturn('bar'); $sut->addPlugin(new TestPlugin()); $sut->addPlugin($plugin); + $sut->addPlugin(new TestPlugin2()); $sut->addPlugin(new TestPlugin()); // make sure a plugin with the same name is not added twice, the first one wins! diff --git a/tests/Security/AclDecisionManagerTest.php b/tests/Security/AclDecisionManagerTest.php new file mode 100644 index 00000000..012bb016 --- /dev/null +++ b/tests/Security/AclDecisionManagerTest.php @@ -0,0 +1,45 @@ +createMock(AccessDecisionManagerInterface::class); + $manager->expects($this->once())->method('decide')->willReturn(true); + + $token = $this->createMock(TokenInterface::class); + + $sut = new AclDecisionManager($manager); + $result = $sut->isFullyAuthenticated($token); + self::assertTrue($result); + } + + public function testIsNotFullyAuthenticated() + { + $manager = $this->createMock(AccessDecisionManagerInterface::class); + $manager->expects($this->once())->method('decide')->willReturn(false); + + $token = $this->createMock(TokenInterface::class); + + $sut = new AclDecisionManager($manager); + $result = $sut->isFullyAuthenticated($token); + self::assertFalse($result); + } +} diff --git a/tests/Security/SessionHandlerTest.php b/tests/Security/SessionHandlerTest.php new file mode 100644 index 00000000..7055f665 --- /dev/null +++ b/tests/Security/SessionHandlerTest.php @@ -0,0 +1,26 @@ +isSessionExpired()); + } +} diff --git a/var/data/kimai_test.sqlite b/var/data/kimai_test.sqlite index f1c8fb918d8a7b24c898091a92ef9ec4f2ff9e2d..3babc73bc108ed7ce274ebe5c26fc48874f991b8 100644 GIT binary patch delta 451 zcmZo@&~IqapCB!`kb!|=4-hK>aX145NMk%57!k%^&+k%5U3h^K2{s%vDTU}R`zY;I+2xIN99aUKh^DzDRY zb_bwtUMHaL4?My`%$kg-npL-l`Z8`h#27Z6LxAZ%Yd6ppUF~lKn1GlWh*`G35nz>w zXBK48na*Cos$inS@SB05(Yw)(U0hq6vB|e2F)1fCJ2N*i(t-U zo>8P8Caa*4nW7Np8075X7^GonWTC0x=O3cr7wY4qqmYtVlBnS1Us^IAt z;_B`iq!8*C?CI|3>VhPnlbM!ENKHVH=VTcU9u7_0Mte5#K<(+bG@0b3U@nR`M00W4 kcI!gcljoMRG%zZ(vm~$qF*^`*05K;Jb8Tly;QkX203r{61^@s6 delta 5637 zcmY*dd3+S*8J?LvCU=5dazr+WT*<=B>`wMbb^{HHf?Ogbkf2F0yW!_S5+ul-loG5I z*-vUmZPBO*MfOhZ{>C7733buy(H(uoSvK_{fIuxf6~9kf1dA%ufhANx7SY`MtEgHiq_yf^^@wGdcF>3h&1k?1#qDrBROy;x0mF`) zW;|@pTHt)T+#jsjFgW_&&CxGcu;Bv3)2eIg4jiZq8lj*W2^i715sQb6ER%_USH?|S z={v(gl3Pb!clz`v1!*=+ID{8tP z{KxP2Z#&$#m_57m!{#^6S*~f5*c{(?O@K zS|6R9(Rc4_d5Mk`E0JC_qqct(Zks4?zyv?wg_UiJ03dEv+TU}UuhPq_uReXzO)m{L zQ>+*mDmb=W_m_)zhJ%((l?BC2q6Y%uCd-P4Lh*2To>SK94<Jq!(bR zR_H!&1q{3r{IIi3a4dv684G$Gbz}aN#P1g2H!95vCB$+-xRHZjR1%=X1VkmL9JmsJ zZq*Yh{h3CQYDE-K_oY0D;q>rXCB=&d-(e&Q!G~|qgnT8Gq~I6SgSHXOa#4TGK2|T zj}(QfhVZH;R)qgHYHrW8G|O=#(Jm7Z5RMQvmPu92Ho-Hv5uG?)^t@TYTP@9WwxO`y7C|41}X#X33CyZp>PvTJPZX> z>2?Oam0OmWy@9iIac1M$ep?F&JdCbK=oTatEuL+Hmcwc<^azGce`xz>^^bkop4lKH zp~2KC4xm}kpf*hq>+F<;XAlz<#%=`$7?#*FWZk?y-&ChF>m>$MRQ(1R2puy8lg#3N z6T@5+N`jI`3prmVpnh*ebR{7~`PT7lj6^Xmn;J36c zbA$AwR`)CK3O}e{NPaB%A(|Fr*A>RF)$wa0SZU(v<~Qa)x3DF%MnWt>HYN^)Vp&or z_|CyYfeGsZ=8|n+`ZZ0*WcyRd=e)YQOqc70M|~U+NJUm(STs~z6PI(-K4%@LdCj~V z>DQmmtah{ESDzDZKs8YmEIFkAStckFqyyW+o_fyFWY1qJuDs>guyp2n=?IM`vHD=H zK(LTc(jP%mX(Cxfy^9J(Sr%1MN_W<0R!K)N>QyOvd``m=OjMhKfevyLDn+v*r0mIv zrro-W_Jx_9nNI1*T7dN61hxf>76y9U7Gpt}(8owBN@cA5&N=$BKeBA|w_TT8C2Ap7 z*$K)`O7Y#rl|`=?ttuK(c%bmYf|CWS3-a>!<|pz_hU?>#o#k9((=6%c+NaXwL(4=43I*90+TQ>KY=>MAXbZpzLKrZ?3z$MN<&Jx~OeeZuz4gxV4_?{Iss)DD ztoyKW&;gUYpmxC~K~g_pqNxRmu&#TEt8~q_w~HGmo_T5+s}hl5T4?GC96)(dENmmt zQX6ubWdbayXIe^cLio{h`#odFj;(26ap}h#-Mfk(Knu`_f|f?a=8i7nCKD}$aO~^n ziHWoItI7A@fB%kl788DGt04$K9B>@lKg9ya#d?SEV1lS%9vbGRGeX1R+dlEZV^_JR3)FiLLHw<@blhEOHj23(wP{8w_D@fESn* z)QGN0&?U0eQD5+vCVLb%Xi&-LDOM>1A!Qqj=XnKi6@nsT#_`SHH6fd%+w&5lDD59m z8y;H0Y=MERkXQLL$DkR9bs3QWMs7viM7u6LGnCtpi?wJ+A(znv;j|z^iM=6TQZ;Db;;*9q?su(AZ_YPI6#v& zXks*=S+Ekyd+c|)-koL!FsR>fGhEEyNo0a3;hnhmP_J-hBO$;z zg=WT-+x7~K%&OnE<08|@rb<81EZS`02N)G2Mq0E64VdVnhd|(6WLyO&dQHn$4ZZ_Q z-&w|{2n;lwM=cTuLVQG_Q;<@53E?K>2R&$;%6(tRhC7VwRQsMf7L<Ya~5{iAs$t*2pD% zTqINdxP=_ppJwGQhF3kGV~}{D@zLUF53y@Xli2!<1q%hnrbl>f+t_4@LD&Ax!Vk0< zHUTIYt+?4H!h|AEKg{nk$&UE^=W6ewVV9UgTN4sd>;^NFLVuMiK~1kggjL@d^% z*TvLoe)IamebeWs*+e%MKfj_1AvEa|21XRWUkVfMi7IIbGdo{m)FGHx#Q~hbenpom z21$8=v0B#=80NsI&0=K$p^$h=koW@w!09kdGaha`>c^ z+*CE3_V+ecCjDpy+2vk{#;v5rrQv+mgwIX9`@_Z+EkcBTP(KY6d8UO0BnEv+Ddum5 zYSCvQTwZWZbcGBia0cs=5!<+r?~2=p9-~hfZETF|hDVjtnGaiV9mM963=!id)CC`` ztjNM?+>J!XlRIf(r`UP&9cMUtsX7`Gcg64{vbCw>*FW2MrK zT0|mM9al=Yjr^m