diff --git a/src/AuthorizationMiddleware.php b/src/AuthorizationMiddleware.php index d27fdca..c01efb1 100644 --- a/src/AuthorizationMiddleware.php +++ b/src/AuthorizationMiddleware.php @@ -14,6 +14,7 @@ use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; +use Psr\Log\LoggerInterface; use function is_callable; @@ -41,10 +42,12 @@ class AuthorizationMiddleware implements MiddlewareInterface /** @var ResponseFactoryInterface */ protected $responseFactory; + protected ?LoggerInterface $logger; + /** * @param (callable():ResponseInterface)|ResponseFactoryInterface $responseFactory */ - public function __construct(AuthorizationServer $server, $responseFactory) + public function __construct(AuthorizationServer $server, $responseFactory, ?LoggerInterface $logger = null) { $this->server = $server; if (is_callable($responseFactory)) { @@ -54,6 +57,7 @@ public function __construct(AuthorizationServer $server, $responseFactory) } $this->responseFactory = $responseFactory; + $this->logger = $logger; } /** @@ -75,8 +79,9 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface // for example when the client id is invalid return $exception->generateHttpResponse($response); } catch (BaseException $exception) { + $this->logger?->error('Authorization request error', ['exception' => $exception]); $response = $this->responseFactory->createResponse(); - return (new OAuthServerException($exception->getMessage(), 0, 'unknown_error', 500)) + return (new OAuthServerException('An internal error occurred', 0, 'unknown_error', 500)) ->generateHttpResponse($response); } } diff --git a/src/AuthorizationMiddlewareFactory.php b/src/AuthorizationMiddlewareFactory.php index f2c5881..d72bc87 100644 --- a/src/AuthorizationMiddlewareFactory.php +++ b/src/AuthorizationMiddlewareFactory.php @@ -6,6 +6,9 @@ use League\OAuth2\Server\AuthorizationServer; use Psr\Container\ContainerInterface; +use Psr\Log\LoggerInterface; + +use function assert; final class AuthorizationMiddlewareFactory { @@ -13,9 +16,16 @@ final class AuthorizationMiddlewareFactory public function __invoke(ContainerInterface $container): AuthorizationMiddleware { + $logger = $container->has(LoggerInterface::class) + ? $container->get(LoggerInterface::class) + : null; + + assert($logger === null || $logger instanceof LoggerInterface); + return new AuthorizationMiddleware( $container->get(AuthorizationServer::class), - $this->detectResponseFactory($container) + $this->detectResponseFactory($container), + $logger ); } } diff --git a/test/AuthorizationMiddlewareFactoryTest.php b/test/AuthorizationMiddlewareFactoryTest.php index e4d32df..f126ef0 100644 --- a/test/AuthorizationMiddlewareFactoryTest.php +++ b/test/AuthorizationMiddlewareFactoryTest.php @@ -12,6 +12,7 @@ use Psr\Container\ContainerInterface; use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\ResponseInterface; +use Psr\Log\LoggerInterface; use stdClass; use TypeError; @@ -24,14 +25,19 @@ final class AuthorizationMiddlewareFactoryTest extends TestCase private MockObject&ResponseInterface $response; + private MockObject&LoggerInterface $logger; + protected function setUp(): void { $this->container = $this->createMock(ContainerInterface::class); $this->authServer = $this->createMock(AuthorizationServer::class); $this->response = $this->createMock(ResponseInterface::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->container->method('has') - ->with(ResponseFactoryInterface::class) - ->willReturn(false); + ->willReturnMap([ + [ResponseFactoryInterface::class, false], + [LoggerInterface::class, true], + ]); } public function testConstructor(): void @@ -42,11 +48,12 @@ public function testConstructor(): void public function testRaisesTypeErrorForInvalidAuthorizationServer(): void { - $this->container->expects(self::exactly(2)) + $this->container->expects(self::exactly(3)) ->method('get') ->willReturnMap([ [AuthorizationServer::class, new stdClass()], [ResponseInterface::class, static fn () => null], + [LoggerInterface::class, $this->logger], ]); $factory = new AuthorizationMiddlewareFactory(); @@ -57,11 +64,12 @@ public function testRaisesTypeErrorForInvalidAuthorizationServer(): void public function testFactoryRaisesTypeErrorForNonCallableResponseFactory(): void { - $this->container->expects(self::exactly(2)) + $this->container->expects(self::exactly(3)) ->method('get') ->willReturnMap([ [AuthorizationServer::class, $this->authServer], [ResponseInterface::class, new stdClass()], + [LoggerInterface::class, $this->logger], ]); $factory = new AuthorizationMiddlewareFactory(); @@ -72,11 +80,12 @@ public function testFactoryRaisesTypeErrorForNonCallableResponseFactory(): void public function testFactoryRaisesTypeErrorWhenResponseServiceProvidesResponseInstance(): void { - $this->container->expects(self::exactly(2)) + $this->container->expects(self::exactly(3)) ->method('get') ->willReturnMap([ [AuthorizationServer::class, $this->authServer], [ResponseInterface::class, $this->response], + [LoggerInterface::class, $this->logger], ]); $factory = new AuthorizationMiddlewareFactory(); @@ -87,11 +96,12 @@ public function testFactoryRaisesTypeErrorWhenResponseServiceProvidesResponseIns public function testFactoryReturnsInstanceWhenAppropriateDependenciesArePresentInContainer(): void { - $this->container->expects(self::exactly(2)) + $this->container->expects(self::exactly(3)) ->method('get') ->willReturnMap([ [AuthorizationServer::class, $this->authServer], [ResponseInterface::class, fn (): ResponseInterface => $this->response], + [LoggerInterface::class, $this->logger], ]); $factory = new AuthorizationMiddlewareFactory(); diff --git a/test/AuthorizationMiddlewareTest.php b/test/AuthorizationMiddlewareTest.php index 566831d..2017ddc 100644 --- a/test/AuthorizationMiddlewareTest.php +++ b/test/AuthorizationMiddlewareTest.php @@ -15,6 +15,7 @@ use Psr\Http\Message\StreamInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; +use Psr\Log\LoggerInterface; use RuntimeException; final class AuthorizationMiddlewareTest extends TestCase @@ -131,7 +132,7 @@ public function testAuthorizationRequestRaisingUnknownExceptionGeneratesResponse $body = $this->createMock(StreamInterface::class); $body->expects(self::once()) ->method('write') - ->with(self::stringContains('oauth2 server error')); + ->with(self::stringContains('An internal error occurred')); $this->response ->expects(self::once()) @@ -150,7 +151,7 @@ public function testAuthorizationRequestRaisingUnknownExceptionGeneratesResponse [500, '', $this->response], ]); - $exception = new RuntimeException('oauth2 server error'); + $exception = new RuntimeException('An internal error occurred'); $this->authServer->expects(self::once()) ->method('validateAuthorizationRequest') @@ -169,4 +170,41 @@ public function testAuthorizationRequestRaisingUnknownExceptionGeneratesResponse self::assertSame($this->response, $response); } + + public function testAuthorizationRequestRaisingUnknownExceptionLogsError(): void + { + $exception = new RuntimeException('An internal error occurred'); + + $this->authServer->expects(self::once()) + ->method('validateAuthorizationRequest') + ->with($this->serverRequest) + ->willThrowException($exception); + + $logger = $this->createMock(LoggerInterface::class); + $logger->expects(self::once()) + ->method('error') + ->with( + 'Authorization request error', + ['exception' => $exception] + ); + + $body = $this->createMock(StreamInterface::class); + $body->method('write'); + $this->response->method('getBody')->willReturn($body); + $this->response->method('withHeader')->willReturnSelf(); + $this->response->method('withStatus')->willReturnSelf(); + + $middleware = new AuthorizationMiddleware( + $this->authServer, + $this->responseFactory, + $logger + ); + + $response = $middleware->process( + $this->serverRequest, + $this->handler + ); + + self::assertSame($this->response, $response); + } }