Skip to content

Commit

Permalink
Merge pull request #35368 from M0rgan01/fix/35306
Browse files Browse the repository at this point in the history
Improve TokenAuthenticator
  • Loading branch information
jolelievre committed Feb 23, 2024
2 parents b2dbfde + fcf7d12 commit 0999e3e
Show file tree
Hide file tree
Showing 27 changed files with 476 additions and 149 deletions.
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

0 comments on commit 0999e3e

Please sign in to comment.