diff --git a/phpunit.dist.xml b/phpunit.dist.xml index 07d5366..7d648fe 100644 --- a/phpunit.dist.xml +++ b/phpunit.dist.xml @@ -4,9 +4,8 @@ diff --git a/tests/Unit/Listener/AcceptListenerTest.php b/tests/Unit/Listener/AcceptListenerTest.php index 7fdfb73..0e06d98 100644 --- a/tests/Unit/Listener/AcceptListenerTest.php +++ b/tests/Unit/Listener/AcceptListenerTest.php @@ -5,6 +5,7 @@ namespace App\Tests\Unit\Listener; use App\Listener\AcceptListener; use App\Service\DomainManager; +use PHPUnit\Framework\MockObject\Rule\InvocationOrder; use PHPUnit\Framework\TestCase; use Psr\Cache\CacheItemInterface; use Psr\Cache\CacheItemPoolInterface; @@ -15,27 +16,34 @@ use Symfony\Component\HttpKernel\HttpKernelInterface; final class AcceptListenerTest extends TestCase { private function createEvent(Request $request): RequestEvent { - $kernel = $this->createMock(HttpKernelInterface::class); + $kernel = $this->createStub(HttpKernelInterface::class); return new RequestEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST); } - private function createMockItem(string $key, mixed $value = null, bool $isHit = true): CacheItemInterface { + private function createMockItem(mixed $value = null): CacheItemInterface { $item = $this->createMock(CacheItemInterface::class); - $item->method('getKey')->willReturn($key); - $item->method('get')->willReturn($value); - $item->method('isHit')->willReturn($isHit); + $item->expects($this->atLeastOnce())->method('get')->willReturn($value); return $item; } + private function createMockLogger(?InvocationOrder $expectation = null): LoggerInterface { + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($expectation ?? $this->atLeastOnce())->method('debug'); + return $logger; + } + + /** ensure accept works when using direct-auth */ public function testOnKernelRequestWithValidCookie(): void { $cache = $this->createMock(CacheItemPoolInterface::class); $domainManager = new DomainManager(false, ''); $listener = new AcceptListener($cache, $domainManager); - $listener->setLogger($this->createMock(LoggerInterface::class)); + $listener->setLogger($this->createMockLogger()); - $cacheItem = $this->createMockItem('cookie_abc123', 'user1', true); - $cache->method('hasItem')->with('cookie_abc123')->willReturn(true); - $cache->method('getItem')->with('cookie_abc123')->willReturn($cacheItem); + $cacheItem = $this->createMockItem('user1'); + $cache->expects($this->atLeastOnce())->method('hasItem') + ->with('cookie_abc123')->willReturn(true); + $cache->expects($this->atLeastOnce())->method('getItem') + ->with('cookie_abc123')->willReturn($cacheItem); $request = Request::create('https://example.com/'); $request->cookies->set('__Host-Http-Preauth', 'abc123'); @@ -50,11 +58,14 @@ final class AcceptListenerTest extends TestCase { self::assertSame('user1', $response->headers->get('Remote-User')); } + /** missing cookie should be quietly ignored */ public function testOnKernelRequestWithNoCookie(): void { $cache = $this->createMock(CacheItemPoolInterface::class); $domainManager = new DomainManager(false, ''); $listener = new AcceptListener($cache, $domainManager); - $listener->setLogger($this->createMock(LoggerInterface::class)); + $listener->setLogger($this->createMockLogger($this->never())); + + $cache->expects($this->never())->method('hasItem'); $request = Request::create('https://example.com/'); $event = $this->createEvent($request); @@ -63,13 +74,15 @@ final class AcceptListenerTest extends TestCase { self::assertNull($event->getResponse()); } + /** invalid cookie should be quietly ignored */ public function testOnKernelRequestWithInvalidCookie(): void { $cache = $this->createMock(CacheItemPoolInterface::class); $domainManager = new DomainManager(false, ''); $listener = new AcceptListener($cache, $domainManager); - $listener->setLogger($this->createMock(LoggerInterface::class)); + $listener->setLogger($this->createMockLogger($this->never())); - $cache->method('hasItem')->with('cookie_badcookie')->willReturn(false); + $cache->expects($this->atLeastOnce())->method('hasItem') + ->with('cookie_badcookie')->willReturn(false); $request = Request::create('https://example.com/'); $request->cookies->set('__Host-Http-Preauth', 'badcookie'); @@ -80,15 +93,37 @@ final class AcceptListenerTest extends TestCase { self::assertNull($event->getResponse()); } + /** ensure we sanitize cookie input */ + public function testOnKernelRequestWithDangerousCookie(): void { + $cache = $this->createMock(CacheItemPoolInterface::class); + $domainManager = new DomainManager(false, ''); + $listener = new AcceptListener($cache, $domainManager); + $listener->setLogger($this->createMockLogger($this->never())); + + $cache->expects($this->atLeastOnce())->method('hasItem') + ->with('cookie_bad_string_value')->willReturn(false); + + $request = Request::create('https://example.com/'); + $request->cookies->set('__Host-Http-Preauth', 'bad%string;value'); + + $event = $this->createEvent($request); + $listener->onKernelRequest($event); + + self::assertNull($event->getResponse()); + } + + /** ensure accept works when using central-auth */ public function testOnKernelRequestWithAuthBaseUsesAuthCookieName(): void { $cache = $this->createMock(CacheItemPoolInterface::class); $domainManager = new DomainManager(true, 'auth.example.com'); $listener = new AcceptListener($cache, $domainManager); - $listener->setLogger($this->createMock(LoggerInterface::class)); + $listener->setLogger($this->createMockLogger()); - $cacheItem = $this->createMockItem('cookie_xyz789', 'user2', true); - $cache->method('hasItem')->with('cookie_xyz789')->willReturn(true); - $cache->method('getItem')->with('cookie_xyz789')->willReturn($cacheItem); + $cacheItem = $this->createMockItem('user2'); + $cache->expects($this->atLeastOnce())->method('hasItem') + ->with('cookie_xyz789')->willReturn(true); + $cache->expects($this->atLeastOnce())->method('getItem') + ->with('cookie_xyz789')->willReturn($cacheItem); $request = Request::create('https://example.com/'); $request->cookies->set('__Http-Domain-Preauth', 'xyz789'); @@ -101,11 +136,51 @@ final class AcceptListenerTest extends TestCase { self::assertSame('hi user2', $response->getContent()); } - public function testOnKernelRequestWithEmptyCookieValue(): void { + /** cookie for direct-auth when using auth-subdomain */ + public function testOnKernelRequestWithWrongCookie(): void { + $cache = $this->createMock(CacheItemPoolInterface::class); + $domainManager = new DomainManager(true, 'auth.example.com'); + $listener = new AcceptListener($cache, $domainManager); + $listener->setLogger($this->createMockLogger($this->never())); + + $cache->expects($this->never())->method('hasItem'); + + $request = Request::create('https://example.com/'); + $request->cookies->set('__Host-Http-Preauth', 'abc123'); + + $event = $this->createEvent($request); + $listener->onKernelRequest($event); + + $response = $event->getResponse(); + self::assertNull($response); + } + + /** cookie for auth-subdomain when using direct-auth */ + public function testOnKernelRequestWithWrongCookieAlt(): void { $cache = $this->createMock(CacheItemPoolInterface::class); $domainManager = new DomainManager(false, ''); $listener = new AcceptListener($cache, $domainManager); - $listener->setLogger($this->createMock(LoggerInterface::class)); + $listener->setLogger($this->createMockLogger($this->never())); + + $cache->expects($this->never())->method('hasItem'); + + $request = Request::create('https://example.com/'); + $request->cookies->set('__Http-Domain-Preauth', 'abc123'); + + $event = $this->createEvent($request); + $listener->onKernelRequest($event); + + $response = $event->getResponse(); + self::assertNull($response); + } + + /** empty cookie should be quietly ignored */ + public function testOnKernelRequestWithEmptyCookieValue(): void { + $cache = $this->createMock(CacheItemPoolInterface::class); + $cache->expects($this->never())->method('hasItem'); + $domainManager = new DomainManager(false, ''); + $listener = new AcceptListener($cache, $domainManager); + $listener->setLogger($this->createMockLogger($this->never())); $request = Request::create('https://example.com/'); $request->cookies->set('__Host-Http-Preauth', ''); diff --git a/tests/Unit/Listener/AllowListenerTest.php b/tests/Unit/Listener/AllowListenerTest.php index 340a3cf..a74de7e 100644 --- a/tests/Unit/Listener/AllowListenerTest.php +++ b/tests/Unit/Listener/AllowListenerTest.php @@ -7,6 +7,7 @@ use App\ConfigBag; use App\Listener\AllowListener; use App\Utilities; use OTPHP\TOTP; +use PHPUnit\Framework\MockObject\Rule\InvocationOrder; use PHPUnit\Framework\TestCase; use Psr\Cache\CacheItemInterface; use Psr\Cache\CacheItemPoolInterface; @@ -18,21 +19,25 @@ use Symfony\Component\HttpKernel\HttpKernelInterface; final class AllowListenerTest extends TestCase { private function createEvent(Request $request): RequestEvent { - $kernel = $this->createMock(HttpKernelInterface::class); + $kernel = $this->createStub(HttpKernelInterface::class); return new RequestEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST); } - private function createMockItem(string $key, mixed $value = null, bool $isHit = true): CacheItemInterface { + private function createMockItem(mixed $value = null): CacheItemInterface { $item = $this->createMock(CacheItemInterface::class); - $item->method('getKey')->willReturn($key); - $item->method('get')->willReturn($value); - $item->method('isHit')->willReturn($isHit); + $item->expects($this->atLeastOnce())->method('get')->willReturn($value); return $item; } + private function createMockLogger(?InvocationOrder $expectation = null): LoggerInterface { + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($expectation ?? $this->atLeastOnce())->method('debug'); + return $logger; + } + private function createConfigBag(int $ipTtl = 1800): ConfigBag { - $clock = $this->createMock(ClockInterface::class); - $cache = $this->createMock(CacheItemPoolInterface::class); + $clock = $this->createStub(ClockInterface::class); + $cache = $this->createStub(CacheItemPoolInterface::class); $utilities = new Utilities($clock, $cache); $totp = TOTP::generate($clock); $totp->setLabel('Test'); @@ -50,15 +55,18 @@ final class AllowListenerTest extends TestCase { ); } + /** ip-ttl enabled, valid session for client-ip */ public function testOnKernelRequestWithValidIp(): void { $cache = $this->createMock(CacheItemPoolInterface::class); $config = $this->createConfigBag(1800); $listener = new AllowListener($cache, $config); - $listener->setLogger($this->createMock(LoggerInterface::class)); + $listener->setLogger($this->createMockLogger()); - $cacheItem = $this->createMockItem('ip_192.168.1.1', 'user1', true); - $cache->method('hasItem')->with('ip_192.168.1.1')->willReturn(true); - $cache->method('getItem')->with('ip_192.168.1.1')->willReturn($cacheItem); + $cacheItem = $this->createMockItem('user1'); + $cache->expects($this->atLeastOnce())->method('hasItem') + ->with('ip_192.168.1.1')->willReturn(true); + $cache->expects($this->atLeastOnce())->method('getItem') + ->with('ip_192.168.1.1')->willReturn($cacheItem); $request = Request::create('https://example.com/'); $request->server->set('REMOTE_ADDR', '192.168.1.1'); @@ -73,26 +81,14 @@ final class AllowListenerTest extends TestCase { self::assertSame('user1', $response->headers->get('Remote-User')); } + /** ip-ttl disabled (default), should not even check the cache */ public function testOnKernelRequestWithIpTtlZero(): void { $cache = $this->createMock(CacheItemPoolInterface::class); $config = $this->createConfigBag(0); $listener = new AllowListener($cache, $config); - $listener->setLogger($this->createMock(LoggerInterface::class)); + $listener->setLogger($this->createMockLogger($this->never())); - $request = Request::create('https://example.com/'); - $request->server->set('REMOTE_ADDR', '192.168.1.1'); - - $event = $this->createEvent($request); - $listener->onKernelRequest($event); - - self::assertNull($event->getResponse()); - } - - public function testOnKernelRequestWithIpTtlNull(): void { - $cache = $this->createMock(CacheItemPoolInterface::class); - $config = $this->createConfigBag(0); - $listener = new AllowListener($cache, $config); - $listener->setLogger($this->createMock(LoggerInterface::class)); + $cache->expects($this->never())->method('hasItem'); $request = Request::create('https://example.com/'); $request->server->set('REMOTE_ADDR', '192.168.1.1'); @@ -103,13 +99,15 @@ final class AllowListenerTest extends TestCase { self::assertNull($event->getResponse()); } + /** ip-ttl enabled, but client-ip does not have a session */ public function testOnKernelRequestWithUnknownIp(): void { $cache = $this->createMock(CacheItemPoolInterface::class); $config = $this->createConfigBag(1800); $listener = new AllowListener($cache, $config); - $listener->setLogger($this->createMock(LoggerInterface::class)); + $listener->setLogger($this->createMockLogger($this->never())); - $cache->method('hasItem')->with('ip_192.168.1.1')->willReturn(false); + $cache->expects($this->atLeastOnce())->method('hasItem') + ->with('ip_192.168.1.1')->willReturn(false); $request = Request::create('https://example.com/'); $request->server->set('REMOTE_ADDR', '192.168.1.1'); diff --git a/tests/Unit/Listener/InterceptListenerTest.php b/tests/Unit/Listener/InterceptListenerTest.php index 618e01d..c1b8834 100644 --- a/tests/Unit/Listener/InterceptListenerTest.php +++ b/tests/Unit/Listener/InterceptListenerTest.php @@ -8,6 +8,7 @@ use App\Listener\InterceptListener; use App\Service\DomainManager; use App\Utilities; use OTPHP\TOTP; +use PHPUnit\Framework\MockObject\Rule\InvocationOrder; use PHPUnit\Framework\TestCase; use Psr\Cache\CacheItemInterface; use Psr\Cache\CacheItemPoolInterface; @@ -20,13 +21,19 @@ use Twig\Environment; final class InterceptListenerTest extends TestCase { private function createEvent(Request $request): RequestEvent { - $kernel = $this->createMock(HttpKernelInterface::class); + $kernel = $this->createStub(HttpKernelInterface::class); return new RequestEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST); } + private function createMockLogger(?InvocationOrder $expectation = null): LoggerInterface { + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($expectation ?? $this->atLeastOnce())->method('debug'); + return $logger; + } + private function createConfigBag(): ConfigBag { - $clock = $this->createMock(ClockInterface::class); - $cache = $this->createMock(CacheItemPoolInterface::class); + $clock = $this->createStub(ClockInterface::class); + $cache = $this->createStub(CacheItemPoolInterface::class); $utilities = new Utilities($clock, $cache); $totp = TOTP::generate($clock); $totp->setLabel('Test'); @@ -44,39 +51,38 @@ final class InterceptListenerTest extends TestCase { ); } - private function createTwig(): Environment { + private function createTwig(InvocationOrder $expectation): Environment { $twig = $this->createMock(Environment::class); - $twig->method('render')->with('login.html.twig', self::anything())->willReturn('login'); + $twig->expects($expectation)->method('render') + ->with('login.html.twig', self::anything())->willReturn('login'); return $twig; } private function createListener( ?DomainManager $dm = null, - ?Environment $twig = null, - ?CacheItemPoolInterface $nonceCache = null + bool $cacheUsed = false, ): InterceptListener { + /* redirect should be quiet, login page renders twig and generates nonce */ + $expectation = $cacheUsed ? $this->atLeastOnce() : $this->never(); + $config = $this->createConfigBag(); $domainManager = $dm ?? new DomainManager(false, ''); - $twig = $twig ?? $this->createTwig(); - $listener = new InterceptListener($config, $domainManager, $twig); - $listener->setLogger($this->createMock(LoggerInterface::class)); + $listener = new InterceptListener($config, $domainManager, $this->createTwig($expectation)); + $listener->setLogger($this->createMockLogger($expectation)); - if ($nonceCache) { - $listener->setNonceCache($nonceCache); - } else { - $cache = $this->createMock(CacheItemPoolInterface::class); - $item = $this->createMock(CacheItemInterface::class); - $item->method('isHit')->willReturn(false); - $item->method('set')->willReturnSelf(); - $item->method('expiresAfter')->willReturnSelf(); - $cache->method('getItem')->willReturn($item); - $cache->method('save')->willReturn(true); - $listener->setNonceCache($cache); - } + $cache = $this->createMock(CacheItemPoolInterface::class); + $item = $this->createMock(CacheItemInterface::class); + $item->expects($expectation)->method('isHit')->willReturn(false); + $item->expects($expectation)->method('set')->willReturnSelf(); + $item->expects($expectation)->method('expiresAfter')->willReturnSelf(); + $cache->expects($expectation)->method('getItem')->willReturn($item); + $cache->expects($expectation)->method('save')->willReturn(true); + $listener->setNonceCache($cache); return $listener; } + /** ensure when using central-auth that we get redirected from protected subdomain */ public function testOnKernelRequestRedirectsToAuthSubdomain(): void { $dm = new DomainManager(true, 'auth.example.com'); $listener = $this->createListener($dm); @@ -92,9 +98,10 @@ final class InterceptListenerTest extends TestCase { self::assertStringContainsString('return=', $response->headers->get('Location')); } + /** when using central-auth on auth-subdomain, expect the login page */ public function testOnKernelRequestPresentsLoginPageOnAuthSubdomain(): void { $dm = new DomainManager(true, 'auth.example.com'); - $listener = $this->createListener($dm); + $listener = $this->createListener($dm, true); $request = Request::create('https://auth.example.com/'); $event = $this->createEvent($request); @@ -106,9 +113,24 @@ final class InterceptListenerTest extends TestCase { self::assertSame('login', $response->getContent()); } + /** when using central-auth on wrong base-domain, expect the login page */ + public function testOnKernelRequestPresentsLoginPageOnRougeDomain(): void { + $dm = new DomainManager(true, 'auth.example.com'); + $listener = $this->createListener($dm, true); + + $request = Request::create('https://example.org/'); /* different domain */ + $event = $this->createEvent($request); + $listener->onKernelRequest($event); + + $response = $event->getResponse(); + self::assertNotNull($response); + self::assertSame(401, $response->getStatusCode()); + self::assertSame('login', $response->getContent()); + } + + /** when using direct-login, expect the login page */ public function testOnKernelRequestPresentsLoginPageWithoutAuthSubdomain(): void { - $dm = new DomainManager(false, ''); - $listener = $this->createListener($dm); + $listener = $this->createListener(cacheUsed: true); $request = Request::create('https://example.com/'); $event = $this->createEvent($request); @@ -117,11 +139,12 @@ final class InterceptListenerTest extends TestCase { $response = $event->getResponse(); self::assertNotNull($response); self::assertSame(401, $response->getStatusCode()); + self::assertSame('login', $response->getContent()); } + /** ensure invalid cookies get cleared with direct-login */ public function testOnKernelRequestPrunesInvalidCookie(): void { - $dm = new DomainManager(false, ''); - $listener = $this->createListener($dm); + $listener = $this->createListener(cacheUsed: true); $request = Request::create('https://example.com/'); $request->cookies->set('__Host-Http-Preauth', 'invalid'); @@ -137,9 +160,9 @@ final class InterceptListenerTest extends TestCase { self::assertNull($cookies[0]->getValue()); } + /** only prunes cookies if invalid/expired cookie was sent */ public function testOnKernelRequestDoesNotPruneCookieWhenNotPresent(): void { - $dm = new DomainManager(false, ''); - $listener = $this->createListener($dm); + $listener = $this->createListener(cacheUsed: true); $request = Request::create('https://example.com/'); $event = $this->createEvent($request); @@ -150,9 +173,10 @@ final class InterceptListenerTest extends TestCase { self::assertCount(0, $response->headers->getCookies()); } - public function testOnKernelRequestWithAuthBaseUsesAuthCookieName(): void { + /** ensure invalid cookies get cleared with central-auth */ + public function testOnKernelRequestPruneInvalidAuthBaseCookie(): void { $dm = new DomainManager(true, 'auth.example.com'); - $listener = $this->createListener($dm); + $listener = $this->createListener($dm, true); $request = Request::create('https://auth.example.com/'); $request->cookies->set('__Http-Domain-Preauth', 'invalid'); @@ -165,5 +189,6 @@ final class InterceptListenerTest extends TestCase { $cookies = $response->headers->getCookies(); self::assertCount(1, $cookies); self::assertSame('__Http-Domain-Preauth', $cookies[0]->getName()); + self::assertNull($cookies[0]->getValue()); } }