Skip to content

Commit

Permalink
feat(symfony): Link security (#5290)
Browse files Browse the repository at this point in the history
* [Link] Start Link Security

* feat(provider): Auto Resolve Get Operation and Parameters

* chore(CS): fix CS

* feat(tests): Add DenyAccessListener tests

* feat(tests): Add link security behat tests

* fix(test): fix mongodb document configuration

* fix(readlistner): fix error 500 on not existing entity

* feat(linksecurity): expand functionality to cover all combinations of to and from property and add optional object name

* feat(linksecurity): add more tests

* chore: fix cs

* chore: phpstan fix

* fix: Move logic to refactored, now used, classes

* fix: refactor unit tests

* fix: backport for legacy event system as well

* Revert "fix: backport for legacy event system as well"

This reverts commit 16f14c8.

* refactor: Refactor ReadProvider.php and AccessCheckerProvider.php to extract link security into their own providers

* mark providers final, disable feature by default
  • Loading branch information
KDederichs authored Jan 19, 2024
1 parent 498cabb commit b488366
Show file tree
Hide file tree
Showing 16 changed files with 605 additions and 2 deletions.
2 changes: 1 addition & 1 deletion behat.yml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ legacy:
- 'Behat\MinkExtension\Context\MinkContext'
- 'behatch:context:rest'
filters:
tags: '~@postgres&&~@mongodb&&~@elasticsearch'
tags: '~@postgres&&~@mongodb&&~@elasticsearch&&~@link_security'
extensions:
'FriendsOfBehat\SymfonyExtension':
bootstrap: 'tests/Fixtures/app/bootstrap.php'
Expand Down
82 changes: 82 additions & 0 deletions features/authorization/deny.feature
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,88 @@ Feature: Authorization checking
And the response should contain "ownerOnlyProperty"
And the JSON node "ownerOnlyProperty" should be equal to the string "updated"

@link_security
Scenario: An non existing entity should return Not found
When I add "Accept" header equal to "application/ld+json"
And I add "Content-Type" header equal to "application/ld+json"
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
And I send a "GET" request to "/secured_dummies/40000/to_from"
Then the response status code should be 404

@link_security
Scenario: An user can get related linked dummies for an secured dummy they own
Given there are 1 SecuredDummy objects owned by dunglas with related dummies
When I add "Accept" header equal to "application/ld+json"
And I add "Content-Type" header equal to "application/ld+json"
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
And I send a "GET" request to "/secured_dummies/4/to_from"
Then the response status code should be 200
And the response should contain "securedDummy"
And the JSON node "hydra:member[0].id" should be equal to 1

@link_security
Scenario: I define a custom name of the security object
When I add "Accept" header equal to "application/ld+json"
And I add "Content-Type" header equal to "application/ld+json"
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
And I send a "GET" request to "/secured_dummies/4/with_name"
Then the response status code should be 200
And the response should contain "securedDummy"
And the JSON node "hydra:member[0].id" should be equal to 1

@link_security
Scenario: I define a from from link
When I add "Accept" header equal to "application/ld+json"
And I add "Content-Type" header equal to "application/ld+json"
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
And I send a "GET" request to "/related_linked_dummies/1/from_from"
Then the response status code should be 200
And the response should contain "id"
And the JSON node "hydra:member[0].id" should be equal to 4

@link_security
Scenario: I define multiple links with security
When I add "Accept" header equal to "application/ld+json"
And I add "Content-Type" header equal to "application/ld+json"
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
And I send a "GET" request to "/secured_dummies/4/related/1"
Then the response status code should be 200
And the response should contain "id"
And the JSON node "hydra:member[0].id" should be equal to 1

@link_security
Scenario: An user can not get related linked dummies for an secured dummy they do not own
Given there are 1 SecuredDummy objects owned by someone with related dummies
When I add "Accept" header equal to "application/ld+json"
And I add "Content-Type" header equal to "application/ld+json"
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
And I send a "GET" request to "/secured_dummies/5/to_from"
Then the response status code should be 403

@link_security
Scenario: I define a custom name of the security object
When I add "Accept" header equal to "application/ld+json"
And I add "Content-Type" header equal to "application/ld+json"
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
And I send a "GET" request to "/secured_dummies/5/with_name"
Then the response status code should be 403

@link_security
Scenario: I define a from from link
When I add "Accept" header equal to "application/ld+json"
And I add "Content-Type" header equal to "application/ld+json"
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
And I send a "GET" request to "/related_linked_dummies/2/from_from"
Then the response status code should be 403

@link_security
Scenario: I define multiple links with security
When I add "Accept" header equal to "application/ld+json"
And I add "Content-Type" header equal to "application/ld+json"
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
And I send a "GET" request to "/secured_dummies/5/related/2"
Then the response status code should be 403

Scenario: A user retrieves a resource with an admin only viewable property
When I add "Accept" header equal to "application/json"
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
Expand Down
53 changes: 52 additions & 1 deletion src/Metadata/Link.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#[\Attribute(\Attribute::TARGET_PROPERTY | \Attribute::TARGET_METHOD | \Attribute::TARGET_PARAMETER)]
final class Link
{
public function __construct(private ?string $parameterName = null, private ?string $fromProperty = null, private ?string $toProperty = null, private ?string $fromClass = null, private ?string $toClass = null, private ?array $identifiers = null, private ?bool $compositeIdentifier = null, private ?string $expandedValue = null)
public function __construct(private ?string $parameterName = null, private ?string $fromProperty = null, private ?string $toProperty = null, private ?string $fromClass = null, private ?string $toClass = null, private ?array $identifiers = null, private ?bool $compositeIdentifier = null, private ?string $expandedValue = null, private ?string $security = null, private ?string $securityMessage = null, private ?string $securityObjectName = null)
{
// For the inverse property shortcut
if ($this->parameterName && class_exists($this->parameterName)) {
Expand Down Expand Up @@ -128,6 +128,45 @@ public function withExpandedValue(string $expandedValue): self
return $self;
}

public function getSecurity(): ?string
{
return $this->security;
}

public function getSecurityMessage(): ?string
{
return $this->securityMessage;
}

public function withSecurity(?string $security): self
{
$self = clone $this;
$self->security = $security;

return $self;
}

public function withSecurityMessage(?string $securityMessage): self
{
$self = clone $this;
$self->securityMessage = $securityMessage;

return $self;
}

public function getSecurityObjectName(): ?string
{
return $this->securityObjectName;
}

public function withSecurityObjectName(?string $securityObjectName): self
{
$self = clone $this;
$self->securityObjectName = $securityObjectName;

return $self;
}

public function withLink(self $link): self
{
$self = clone $this;
Expand Down Expand Up @@ -164,6 +203,18 @@ public function withLink(self $link): self
$self->expandedValue = $expandedValue;
}

if (!$self->getSecurity() && ($security = $link->getSecurity())) {
$self->security = $security;
}

if (!$self->getSecurityMessage() && ($securityMessage = $link->getSecurityMessage())) {
$self->securityMessage = $securityMessage;
}

if (!$self->getSecurityObjectName() && ($securityObjectName = $link->getSecurityObjectName())) {
$self->securityObjectName = $securityObjectName;
}

return $self;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ public function load(array $configs, ContainerBuilder $container): void
$this->registerSecurityConfiguration($container, $config, $loader);
$this->registerMakerConfiguration($container, $config, $loader);
$this->registerArgumentResolverConfiguration($loader);
$this->registerLinkSecurityConfiguration($loader, $config);

$container->registerForAutoconfiguration(FilterInterface::class)
->addTag('api_platform.filter');
Expand Down Expand Up @@ -892,4 +893,11 @@ private function registerInflectorConfiguration(array $config): void
Inflector::keepLegacyInflector(false);
}
}

private function registerLinkSecurityConfiguration(XmlFileLoader $loader, array $config): void
{
if ($config['enable_link_security']) {
$loader->load('link_security.xml');
}
}
}
1 change: 1 addition & 0 deletions src/Symfony/Bundle/DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ public function getConfigTreeBuilder(): TreeBuilder
->booleanNode('enable_docs')->defaultTrue()->info('Enable the docs')->end()
->booleanNode('enable_profiler')->defaultTrue()->info('Enable the data collector and the WebProfilerBundle integration.')->end()
->booleanNode('keep_legacy_inflector')->defaultTrue()->info('Keep doctrine/inflector instead of symfony/string to generate plurals for routes.')->end()
->booleanNode('enable_link_security')->defaultFalse()->info('Enable security for Links (sub resources)')->end()
->arrayNode('collection')
->addDefaultsIfNotSet()
->children()
Expand Down
20 changes: 20 additions & 0 deletions src/Symfony/Bundle/Resources/config/link_security.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?xml version="1.0" ?>

<container xmlns="http://symfony.com/schema/dic/services"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">

<services>

<service id="api_platform.state_provider.read_link" class="ApiPlatform\Symfony\Security\State\LinkedReadProvider" decorates="api_platform.state_provider.read">
<argument type="service" id="api_platform.state_provider.read_link.inner" />
<argument type="service" id="api_platform.state_provider.locator" />
<argument type="service" id="api_platform.metadata.resource.metadata_collection_factory" />
</service>

<service id="api_platform.state_provider.access_checker_linked" class="ApiPlatform\Symfony\Security\State\LinkAccessCheckerProvider" decorates="api_platform.state_provider.read_link">
<argument type="service" id="api_platform.state_provider.access_checker_linked.inner" />
<argument type="service" id="api_platform.security.resource_access_checker" />
</service>
</services>
</container>
80 changes: 80 additions & 0 deletions src/Symfony/Security/State/LinkAccessCheckerProvider.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
<?php

/*
* This file is part of the API Platform project.
*
* (c) Kévin Dunglas <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace ApiPlatform\Symfony\Security\State;

use ApiPlatform\Metadata\HttpOperation;
use ApiPlatform\Metadata\Link;
use ApiPlatform\Metadata\Operation;
use ApiPlatform\State\ProviderInterface;
use ApiPlatform\Symfony\Security\Exception\AccessDeniedException;
use ApiPlatform\Symfony\Security\ResourceAccessCheckerInterface;

/**
* Checks the individual parts of the linked resource for access rights.
*
* @experimental
*/
final class LinkAccessCheckerProvider implements ProviderInterface
{
public function __construct(
private readonly ProviderInterface $decorated,
private readonly ResourceAccessCheckerInterface $resourceAccessChecker
) {
}

public function provide(Operation $operation, array $uriVariables = [], array $context = []): object|array|null
{
$request = ($context['request'] ?? null);

$data = $this->decorated->provide($operation, $uriVariables, $context);

if ($operation instanceof HttpOperation && $operation->getUriVariables()) {
foreach ($operation->getUriVariables() as $uriVariable) {
if (!$uriVariable instanceof Link || !$uriVariable->getSecurity()) {
continue;
}

$targetResource = $uriVariable->getFromClass() ?? $uriVariable->getToClass();

if (!$targetResource) {
continue;
}

$propertyName = $uriVariable->getToProperty() ?? $uriVariable->getFromProperty();
$securityObjectName = $uriVariable->getSecurityObjectName();

if (!$securityObjectName) {
$securityObjectName = $propertyName;
}

if (!$securityObjectName) {
continue;
}

$resourceAccessCheckerContext = [
'object' => $data,
'previous_object' => $request?->attributes->get('previous_data'),
$securityObjectName => $request?->attributes->get($securityObjectName),
'request' => $request,
];

if (!$this->resourceAccessChecker->isGranted($targetResource, $uriVariable->getSecurity(), $resourceAccessCheckerContext)) {
throw new AccessDeniedException($uriVariable->getSecurityMessage() ?? 'Access Denied.');
}
}
}

return $data;
}
}
91 changes: 91 additions & 0 deletions src/Symfony/Security/State/LinkedReadProvider.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
<?php

/*
* This file is part of the API Platform project.
*
* (c) Kévin Dunglas <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace ApiPlatform\Symfony\Security\State;

use ApiPlatform\Exception\InvalidIdentifierException;
use ApiPlatform\Exception\InvalidUriVariableException;
use ApiPlatform\Metadata\HttpOperation;
use ApiPlatform\Metadata\Link;
use ApiPlatform\Metadata\Operation;
use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
use ApiPlatform\State\Exception\ProviderNotFoundException;
use ApiPlatform\State\ProviderInterface;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;

/**
* Checks if the linked resources have security attributes and prepares them for access checking.
*
* @experimental
*/
final class LinkedReadProvider implements ProviderInterface
{
public function __construct(
private readonly ProviderInterface $decorated,
private readonly ProviderInterface $locator,
private readonly ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory
) {
}

public function provide(Operation $operation, array $uriVariables = [], array $context = []): object|array|null
{
$data = $this->decorated->provide($operation, $uriVariables, $context);

if (!$operation instanceof HttpOperation) {
return $data;
}

$request = ($context['request'] ?? null);

if ($operation->getUriVariables()) {
foreach ($operation->getUriVariables() as $key => $uriVariable) {
if (!$uriVariable instanceof Link || !$uriVariable->getSecurity()) {
continue;
}

$relationClass = $uriVariable->getFromClass() ?? $uriVariable->getToClass();

if (!$relationClass) {
continue;
}

$parentOperation = $this->resourceMetadataCollectionFactory
->create($relationClass)
->getOperation($operation->getExtraProperties()['parent_uri_template'] ?? null);
try {
$relation = $this->locator->provide($parentOperation, [$uriVariable->getIdentifiers()[0] => $request?->attributes->all()[$key]], $context);
} catch (ProviderNotFoundException) {
$relation = null;
}

if (!$relation) {
throw new NotFoundHttpException('Relation for link security not found.');
}

try {
$securityObjectName = $uriVariable->getSecurityObjectName();

if (!$securityObjectName) {
$securityObjectName = $uriVariable->getToProperty() ?? $uriVariable->getFromProperty();
}

$request?->attributes->set($securityObjectName, $relation);
} catch (InvalidIdentifierException|InvalidUriVariableException $e) {
throw new NotFoundHttpException('Invalid identifier value or configuration.', $e);
}
}
}

return $data;
}
}
Loading

0 comments on commit b488366

Please sign in to comment.