Skip to content

Commit

Permalink
IBX-8482: Fixed lack of JWT stateless calls recognition (#112)
Browse files Browse the repository at this point in the history
* [REMOVE TMP DEPENDENCY] IBX-8482: Fixed lack of JWT stateless calls recognition

* removed dev dependency
  • Loading branch information
konradoboza authored Jul 2, 2024
1 parent a8e980c commit ddbe28c
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 34 deletions.
2 changes: 2 additions & 0 deletions src/bundle/Resources/config/security.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ services:
arguments:
$headerName: '%ibexa.rest.authorization_header_name%'

Ibexa\Rest\Security\JWTTokenCreationRESTRequestMatcher: ~

Ibexa\Rest\Server\Security\CsrfTokenManager:
arguments:
- '@?security.csrf.token_generator'
Expand Down
6 changes: 2 additions & 4 deletions src/lib/Security/AuthorizationHeaderRESTRequestMatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public function __construct(
int $port = null
) {
parent::__construct($path, $host, $methods, $ips, $attributes, $schemes, $port);

$this->headerName = $headerName;
}

Expand All @@ -43,10 +44,7 @@ public function matches(Request $request): bool
return false;
}

if (
$request->attributes->get('_route') === 'ibexa.rest.create_token'
|| !empty($request->headers->get($this->headerName ?? 'Authorization'))
) {
if (!empty($request->headers->get($this->headerName ?? 'Authorization'))) {
return parent::matches($request);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@

namespace Ibexa\Rest\Security\EventListener\JWT;

use Ibexa\Contracts\Core\Repository\PermissionResolver;
use Ibexa\Core\MVC\Symfony\Security\UserInterface as IbexaUser;
use Ibexa\Rest\Server\Exceptions\BadResponseException;
use Lexik\Bundle\JWTAuthenticationBundle\Event\AuthenticationSuccessEvent;
use Lexik\Bundle\JWTAuthenticationBundle\Events;
Expand All @@ -19,7 +17,6 @@
final readonly class AuthenticationSuccessSubscriber implements EventSubscriberInterface
{
public function __construct(
private PermissionResolver $permissionResolver,
private RequestStack $requestStack,
) {
}
Expand All @@ -42,11 +39,6 @@ public function onAuthenticationSuccess(AuthenticationSuccessEvent $event): void
return;
}

$user = $event->getUser();
if ($user instanceof IbexaUser) {
$this->permissionResolver->setCurrentUserReference($user->getAPIUser());
}

$this->normalizeResponseToRest($event);
}

Expand Down
33 changes: 33 additions & 0 deletions src/lib/Security/JWTTokenCreationRESTRequestMatcher.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

/**
* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace Ibexa\Rest\Security;

use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestMatcher;

/**
* @internal
*
* This class is mandatory for JWT REST calls recognition. It's used within security.firewalls.ibexa_jwt_rest.request_matcher configuration key.
*/
final class JWTTokenCreationRESTRequestMatcher extends RequestMatcher
{
public function matches(Request $request): bool
{
if ($request->attributes->get('is_rest_request', false) !== true) {
return false;
}

if ($request->attributes->get('_route') === 'ibexa.rest.create_token') {
return parent::matches($request);
}

return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,6 @@ public function testMatchesRestRequestsWithCustomHeader(): void
self::assertTrue($matcher->matches($request));
}

public function testMatchesRestJwtCreationEndpoint(): void
{
$matcher = new AuthorizationHeaderRESTRequestMatcher();

$request = $this->createRequest([
'is_rest_request' => true,
'_route' => 'ibexa.rest.create_token',
]);

self::assertTrue($matcher->matches($request));
}

/**
* @param array<string, mixed> $attributes
* @param array<string, array<string>|string> $server
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?php

/**
* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace Ibexa\Tests\Contracts\Rest\Security;

use Ibexa\Rest\Security\JWTTokenCreationRESTRequestMatcher;
use PHPUnit\Framework\TestCase;
use Symfony\Component\HttpFoundation\Request;

final class JWTTokenCreationRESTRequestMatcherTest extends TestCase
{
public function testDoesNotMatchNonRestRequests(): void
{
$matcher = new JWTTokenCreationRESTRequestMatcher();

self::assertFalse($matcher->matches(new Request()));
}

public function testDoesNotMatchRestRequestsWithoutHeader(): void
{
$matcher = new JWTTokenCreationRESTRequestMatcher();

$request = $this->createRequest([
'is_rest_request' => true,
]);

self::assertFalse($matcher->matches($request));
}

public function testMatchesRestJwtCreationEndpoint(): void
{
$matcher = new JWTTokenCreationRESTRequestMatcher();

$request = $this->createRequest([
'is_rest_request' => true,
'_route' => 'ibexa.rest.create_token',
]);

self::assertTrue($matcher->matches($request));
}

/**
* @param array<string, mixed> $attributes
* @param array<string, array<string>|string> $server
*/
private function createRequest(array $attributes = [], array $server = []): Request
{
return new Request([], [], $attributes, [], [], $server);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

namespace Ibexa\Tests\Rest\Security\EventListener\JWT;

use Ibexa\Contracts\Core\Repository\PermissionResolver;
use Ibexa\Core\MVC\Symfony\Security\User;
use Ibexa\Core\Repository\Values\User\User as ApiUser;
use Ibexa\Rest\Security\EventListener\JWT\AuthenticationSuccessSubscriber;
Expand All @@ -27,7 +26,6 @@ final class AuthenticationSuccessSubscriberTest extends TestCase
public function testGetSubscribedEvents(): void
{
$subscriber = new AuthenticationSuccessSubscriber(
$this->createMock(PermissionResolver::class),
$this->getRequestStackMock()
);

Expand All @@ -46,15 +44,9 @@ public function testOnAuthenticationSuccess(
UserInterface $user,
bool $isPermissionResolverInvoked
): void {
$permissionResolver = $this->createMock(PermissionResolver::class);
$permissionResolver
->expects($isPermissionResolverInvoked === true ? self::once() : self::never())
->method('setCurrentUserReference');

$event = new AuthenticationSuccessEvent(['token' => 'foo_token'], $user, new Response());

$subscriber = new AuthenticationSuccessSubscriber(
$permissionResolver,
$this->getRequestStackMock()
);

Expand Down Expand Up @@ -91,7 +83,6 @@ public function dataProviderForTestOnAuthenticationSuccess(): iterable
public function testResponseIsMissingJwtToken(): void
{
$subscriber = new AuthenticationSuccessSubscriber(
$this->createMock(PermissionResolver::class),
$this->getRequestStackMock()
);

Expand All @@ -113,7 +104,6 @@ public function testResponseIsMissingJwtToken(): void
public function testSkippingResponseNormalizingForNonRestRequest(): void
{
$subscriber = new AuthenticationSuccessSubscriber(
$this->createMock(PermissionResolver::class),
$this->getRequestStackMock(false)
);

Expand Down

0 comments on commit ddbe28c

Please sign in to comment.