Skip to content

Fix Symfony 7 compatibility #121

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Oct 18, 2024
Merged

Fix Symfony 7 compatibility #121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8647e14
Fix symfony 7 compatibility
core23 Feb 5, 2024
1247ccb
Fix tests
jordisala1991 Sep 6, 2024
c4c7ca0
suppress psalm since it is not fixable
jordisala1991 Sep 6, 2024
29379e6
fix cs issues
jordisala1991 Sep 6, 2024
17f2255
Actualizar AclVoter.php
jordisala1991 Oct 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .php-cs-fixer.dist.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
'@Symfony' => true,
'@Symfony:risky' => true,
'protected_to_private' => false,
'phpdoc_to_comment' => ['ignored_tags' => ['psalm-suppress']],
])
->setRiskyAllowed(true)
->setFinder(
Expand Down
55 changes: 46 additions & 9 deletions Tests/Dbal/AclProviderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@

namespace Symfony\Component\Security\Acl\Tests\Dbal;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\DriverManager;
use Doctrine\DBAL\Schema\DefaultSchemaManagerFactory;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Security\Acl\Dbal\AclProvider;
use Symfony\Component\Security\Acl\Dbal\Schema;
Expand Down Expand Up @@ -146,10 +148,22 @@ public function testFindAcl()

protected function setUp(): void
{
$this->connection = DriverManager::getConnection([
'driver' => 'pdo_sqlite',
'memory' => true,
]);
$configuration = new Configuration();

/**
* @psalm-suppress RedundantCondition Since we are compatibles with DBAL 2 and 3, we need to check if the method exists
*/
if (method_exists($configuration, 'setSchemaManagerFactory')) {
$configuration->setSchemaManagerFactory(new DefaultSchemaManagerFactory());
}

$this->connection = DriverManager::getConnection(
[
'driver' => 'pdo_sqlite',
'memory' => true,
],
$configuration
);

// import the schema
$schema = new Schema($this->getOptions());
Expand All @@ -160,27 +174,50 @@ protected function setUp(): void
// populate the schema with some test data
$insertClassStmt = $this->connection->prepare('INSERT INTO acl_classes (id, class_type) VALUES (?, ?)');
foreach ($this->getClassData() as $data) {
$insertClassStmt->executeStatement($data);
$insertClassStmt->bindValue(1, $data[0]);
$insertClassStmt->bindValue(2, $data[1]);
$insertClassStmt->executeStatement();
}

$insertSidStmt = $this->connection->prepare('INSERT INTO acl_security_identities (id, identifier, username) VALUES (?, ?, ?)');
foreach ($this->getSidData() as $data) {
$insertSidStmt->executeStatement($data);
$insertSidStmt->bindValue(1, $data[0]);
$insertSidStmt->bindValue(2, $data[1]);
$insertSidStmt->bindValue(3, $data[2]);
$insertSidStmt->executeStatement();
}

$insertOidStmt = $this->connection->prepare('INSERT INTO acl_object_identities (id, class_id, object_identifier, parent_object_identity_id, entries_inheriting) VALUES (?, ?, ?, ?, ?)');
foreach ($this->getOidData() as $data) {
$insertOidStmt->executeStatement($data);
$insertOidStmt->bindValue(1, $data[0]);
$insertOidStmt->bindValue(2, $data[1]);
$insertOidStmt->bindValue(3, $data[2]);
$insertOidStmt->bindValue(4, $data[3]);
$insertOidStmt->bindValue(5, $data[4]);
$insertOidStmt->executeStatement();
}

$insertEntryStmt = $this->connection->prepare('INSERT INTO acl_entries (id, class_id, object_identity_id, field_name, ace_order, security_identity_id, mask, granting, granting_strategy, audit_success, audit_failure) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)');
foreach ($this->getEntryData() as $data) {
$insertEntryStmt->executeStatement($data);
$insertEntryStmt->bindValue(1, $data[0]);
$insertEntryStmt->bindValue(2, $data[1]);
$insertEntryStmt->bindValue(3, $data[2]);
$insertEntryStmt->bindValue(4, $data[3]);
$insertEntryStmt->bindValue(5, $data[4]);
$insertEntryStmt->bindValue(6, $data[5]);
$insertEntryStmt->bindValue(7, $data[6]);
$insertEntryStmt->bindValue(8, $data[7]);
$insertEntryStmt->bindValue(9, $data[8]);
$insertEntryStmt->bindValue(10, $data[9]);
$insertEntryStmt->bindValue(11, $data[10]);
$insertEntryStmt->executeStatement();
}

$insertOidAncestorStmt = $this->connection->prepare('INSERT INTO acl_object_identity_ancestors (object_identity_id, ancestor_id) VALUES (?, ?)');
foreach ($this->getOidAncestorData() as $data) {
$insertOidAncestorStmt->executeStatement($data);
$insertOidAncestorStmt->bindValue(1, $data[0]);
$insertOidAncestorStmt->bindValue(2, $data[1]);
$insertOidAncestorStmt->executeStatement();
}
}

Expand Down
23 changes: 19 additions & 4 deletions Tests/Dbal/MutableAclProviderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@

namespace Symfony\Component\Security\Acl\Tests\Dbal;

use Doctrine\DBAL\Configuration;
use Doctrine\DBAL\Connection;
use Doctrine\DBAL\DriverManager;
use Doctrine\DBAL\Schema\DefaultSchemaManagerFactory;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Security\Acl\Dbal\AclProvider;
use Symfony\Component\Security\Acl\Dbal\MutableAclProvider;
Expand Down Expand Up @@ -518,10 +520,23 @@ protected function callMethod($object, $method, array $args)

protected function setUp(): void
{
$this->connection = DriverManager::getConnection([
'driver' => 'pdo_sqlite',
'memory' => true,
]);
$configuration = new Configuration();

/**
* @psalm-suppress RedundantCondition Since we are compatibles with DBAL 2 and 3, we need to check if the method exists
*/
if (method_exists($configuration, 'setSchemaManagerFactory')) {
$configuration->setSchemaManagerFactory(new DefaultSchemaManagerFactory());
}

$this->connection = DriverManager::getConnection(
[
'driver' => 'pdo_sqlite',
'memory' => true,
],
$configuration
);
$this->connection->setNestTransactionsWithSavepoints(true);

// import the schema
$schema = new Schema($this->getOptions());
Expand Down
2 changes: 1 addition & 1 deletion Tests/Domain/SecurityIdentityRetrievalStrategyTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ public function getRoles(): array
return [];
}

public function eraseCredentials()
public function eraseCredentials(): void
{
}

Expand Down
28 changes: 27 additions & 1 deletion Voter/AclVoter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,39 @@
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;

if (class_exists(\Symfony\Component\Security\Core\Security::class)) {
/**
* @internal
*/
trait AclVoterTrait
{
public function vote(TokenInterface $token, $subject, array $attributes)
{
return $this->doVote($token, $subject, $attributes);
}
}
} else {
/**
* @internal
*/
trait AclVoterTrait
{
public function vote(TokenInterface $token, mixed $subject, array $attributes): int
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the way to support multiple versions without such trait is actually to add the return type without adding the parameter type, thanks to variance rules.

however, note that adding the return type requires releasing this as a major version of the package, as this class is not final or internal.

Copy link
Contributor Author

@jordisala1991 jordisala1991 Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, since this major version was previously tagged as it supports sf 7, we should fix it here, and in the next major remove all the unnecessary code and leave the voter clean again. wdyt? I see it more like bug fixing rather than adding new things.

{
return $this->doVote($token, $subject, $attributes);
}
}
}

/**
* This voter can be used as a base class for implementing your own permissions.
*
* @author Johannes M. Schmitt <[email protected]>
*/
class AclVoter implements VoterInterface
{
use AclVoterTrait;

private $aclProvider;
private $permissionMap;
private $objectIdentityRetrievalStrategy;
Expand All @@ -51,7 +77,7 @@ public function supportsAttribute($attribute)
return \is_string($attribute) && $this->permissionMap->contains($attribute);
}

public function vote(TokenInterface $token, $subject, array $attributes)
private function doVote(TokenInterface $token, $subject, array $attributes): int
{
foreach ($attributes as $attribute) {
if (!$this->supportsAttribute($attribute)) {
Expand Down
Loading