Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve TokenAuthenticator #35368

Merged
merged 10 commits into from
Feb 23, 2024
6 changes: 5 additions & 1 deletion app/config/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,13 @@ api_platform:
enable_swagger: true
enable_swagger_ui: true
formats:
json: [ 'application/json' ]
json: [ 'application/json', 'application/merge-patch+json' ]
# Allow this format for other API endpoint than native endpoint (by default we will use json)
jsonld: [ 'application/ld+json' ]
patch_formats:
json: [ 'application/merge-patch+json' ]
error_formats:
json: [ 'application/json' ]
docs_formats:
# This is used to allow using the Swagger UI in HTML presentation
html: [ 'text/html' ]
Expand Down
6 changes: 6 additions & 0 deletions app/config/config_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,9 @@ monolog:
VERBOSITY_VERY_VERBOSE: NOTICE
VERBOSITY_DEBUG: DEBUG
channels: [ "doctrine" ]

api_platform:
mapping:
paths:
- '%kernel.project_dir%/src/PrestaShopBundle/ApiPlatform/Resources'
- '%kernel.project_dir%/tests/Resources/ApiPlatform/Resources'
4 changes: 0 additions & 4 deletions app/config/security.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ security:
admin:
id: prestashop.security.admin.provider

oauth2:
id: PrestaShopBundle\Security\OAuth2\Provider\ApiClientProvider

firewalls:
# disables authentication for assets and the profiler, adapt it according to your needs
dev:
Expand All @@ -36,7 +33,6 @@ security:
api:
pattern: '^(%api_base_path%)(?!/docs)'
stateless: true
provider: 'oauth2'
custom_authenticators:
- PrestaShop\PrestaShop\Core\Security\OAuth2\TokenAuthenticator
request_matcher: PrestaShop\PrestaShop\Core\Security\OAuth2\ApiRequestMatcher
Expand Down
10 changes: 8 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
"prestashop/gsitemap": "^4",
"prestashop/pagesnotfound": "^2",
"prestashop/productcomments": "^6.0",
"prestashop/ps_apiresources": "dev-dev",
"prestashop/ps_apiresources": "dev-fix/protected-tests",
"prestashop/ps_banner": "^2",
"prestashop/ps_bestsellers": "^1.0",
"prestashop/ps_brandlist": "^1.0",
Expand Down Expand Up @@ -250,13 +250,19 @@
"FakeModule\\": "tests/Unit/Resources/api_platform/fake_module_resources/fake_module/src"
}
},
"repositories": {
"ps_apiresources": {
"type": "vcs",
"url": "https://github.com/M0rgan01/ps_apiresources.git"
}
},
"minimum-stability": "dev",
"prefer-stable": true,
"scripts": {
"api-module-tests": [
"@composer create-test-db",
"@composer dump-autoload --dev --working-dir=modules/ps_apiresources",
"@php -d date.timezone=UTC ./vendor/phpunit/phpunit/phpunit -c modules/ps_apiresources/tests/Integration/phpunit.xml"
"@php -d date.timezone=UTC ./vendor/phpunit/phpunit/phpunit -c modules/ps_apiresources/tests/Integration/phpunit-ci.xml"
],
"create-release": "@php tools/build/CreateRelease.php",
"create-test-db": [
Expand Down
40 changes: 30 additions & 10 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 16 additions & 16 deletions src/Core/Security/OAuth2/TokenAuthenticator.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
use Symfony\Contracts\Translation\TranslatorInterface;

/**
* This class is responsible for authenticating api calls using the Authorization header
Expand All @@ -49,6 +50,7 @@ class TokenAuthenticator extends AbstractAuthenticator
public function __construct(
private readonly AuthorisationServerInterface $authorizationServer,
private readonly HttpMessageFactoryInterface $httpMessageFactory,
private readonly TranslatorInterface $translator,
) {
}

Expand All @@ -59,21 +61,14 @@ public function start(Request $request, AuthenticationException $authException =

public function supports(Request $request): bool
{
$authorization = $request->headers->get('Authorization') ?? null;
if (null === $authorization) {
return false;
}
if (!str_starts_with(strtolower($authorization), 'bearer ')) {
return false;
}

// Every request to the API should be handled by this Authenticator
// A filter is already present for requests linked to the API in security.yml
return true;
}

public function onAuthenticationFailure(Request $request, AuthenticationException $exception): ?Response
{
return $this->returnWWWAuthenticateResponse();
return $this->returnWWWAuthenticateResponse($this->translator->trans($exception->getMessageKey(), $exception->getMessageData()));
}

public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): ?Response
Expand All @@ -82,25 +77,30 @@ public function onAuthenticationSuccess(Request $request, TokenInterface $token,
return null;
}

private function returnWWWAuthenticateResponse(): Response
private function returnWWWAuthenticateResponse(?string $content = null): Response
{
return new Response(null, Response::HTTP_UNAUTHORIZED, ['WWW-Authenticate' => 'Bearer']);
return new Response($content, Response::HTTP_UNAUTHORIZED, ['WWW-Authenticate' => 'Bearer']);
}

public function authenticate(Request $request): Passport
{
$authorization = $request->headers->get('Authorization');
$authorization = $request->headers->get('Authorization') ?? null;
if (null === $authorization) {
throw new CustomUserMessageAuthenticationException('No API token provided');
throw new CustomUserMessageAuthenticationException('No Authorization header provided');
}
if (!str_starts_with($authorization, 'Bearer ')) {
throw new CustomUserMessageAuthenticationException('Bearer token missing');
}

$credentials = $this->httpMessageFactory->createRequest($request);
$userIdentifier = $this->authorizationServer->getUser($credentials);
$user = $this->authorizationServer->getUser($credentials);

if (null === $userIdentifier) {
if (null === $user) {
throw new CustomUserMessageAuthenticationException('Invalid credentials');
}

return new SelfValidatingPassport(new UserBadge($userIdentifier->getUserIdentifier()));
// Returns passport purely based on JWT token here, we set a specific loader that returns the
// user object directly since it was already resolved by our authorization server
return new SelfValidatingPassport(new UserBadge($user->getUserIdentifier(), fn () => $user));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,9 @@ public function __construct(
unset($passedArguments['CQRSQueryMapping']);
unset($passedArguments['ApiResourceMapping']);

// Unless especially specified we only handle JSON format by default
$passedArguments['formats'] = $formats ?? ['json'];

parent::__construct(...$passedArguments);
}

Expand Down

This file was deleted.

3 changes: 3 additions & 0 deletions src/PrestaShopBundle/EventListener/AccessDeniedListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@
*/
class AccessDeniedListener
{
use ExternalApiTrait;

public function __construct(
private readonly RouterInterface $router,
private readonly TranslatorInterface $translator,
Expand All @@ -59,6 +61,7 @@ public function onKernelException(ExceptionEvent $event)
{
if (!$event->isMainRequest()
|| !$event->getThrowable() instanceof AccessDeniedException
|| $this->isResourceApiRequest($event->getRequest())
) {
return;
}
Expand Down