search

symfony / security-acl

Symfony Security ACL Component
https://symfony.com/components/Security
MIT License
363 stars 48 forks source link

Fix symfony 5.3 security incompatiblity #106

Closed acrobat closed 2 years ago

acrobat commented 2 years ago

Follow up of #100

The AuthenticationTrustResolverInterface::isAuthenticated method was only added in symfony 5.4 but the AuthenticatedVoter::PUBLIC_ACCESS constant was already available in symfony 5.3, this causes an error when using security-acl 3.3.0 with symfony 5.3.x

https://github.com/symfony/security-acl/blob/04d6fadd671d72ff322e20840e510030753e008a/Domain/SecurityIdentityRetrievalStrategy.php#L81-L83

The first commit actually refactors the test case because even when executing the tests on symfony 5.3 they still pass as we mocked that class/method. I've also changed some things in the github ci workflow because I wasn't able to get symfony 5.3 dependencies with theramsey/composer-install packages (lowest = install, highest = update but this would cause dev packages in this setup), so I'm not sure if this is the desired setup. Let me know if I need the change anything.

The second commit actually fixes the problem (the first commit shows a failing test on symfony 5.3 first)

derrabus commented 2 years ago

Do you really need this fix? That's an incredibly complex test setup for an EOL Symfony branch. 😕

acrobat commented 2 years ago

Hmm yes, I thought 5.3 was still supported but you are right as this was also my feeling that the workflow got more complex. Maybe revert the workflow change but keep the test changes and code fix so the library is compatible? Or bump the version constraint for 3.3.x to `symfony/security-core: "^4.4|^5.4|^6.0"?

derrabus commented 2 years ago

I'd be in favor of only merging the second commit and tag a release, if @wouterj agrees. For 3.4 we should make the bump you've suggested.

acrobat commented 2 years ago

@derrabus That works for me! I've updated the PR to only include the last commit with the fix. The other can still be viewed here -> https://github.com/acrobat/security-acl/commit/50f1abe64b848680d41ba048e136d0c1a06a53df

derrabus commented 2 years ago

Thank you @acrobat.

derrabus commented 2 years ago

https://github.com/symfony/security-acl/releases/tag/v3.3.1