rector-symfony icon indicating copy to clipboard operation
rector-symfony copied to clipboard

[Symfony62] Replace Sensio Security

Open wkania opened this issue 3 years ago • 5 comments

sensio-framework-extra-bundle is abandoned. Some attributes have been implemented in Symfony 6.2 and Rector can already refactor code for them. But, Security attribute will not be implemented in Symfony.
From what I know about Rector we can't currently make change from:

use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security;

class PostController extends Controller
{
    #[Security("is_granted('ROLE_ADMIN') and is_granted('ROLE_FRIENDLY_USER')")]
    public function index()
    {
    }
}

to:

use Symfony\Component\ExpressionLanguage\Expression;
use Symfony\Component\Security\Http\Attribute\IsGranted;

class PostController extends Controller
{
    #[IsGranted(new Expression("is_granted('ROLE_ADMIN') and is_granted('ROLE_FRIENDLY_USER')"))]
    public function index()
    {
    }
}

This require a new rule, something custom like this?

wkania avatar Apr 18 '23 14:04 wkania

Yes, this will need a custom rule, to handle the expressions.

TomasVotruba avatar Apr 18 '23 14:04 TomasVotruba

It will be more complex :( When there is a second optional argument:

#[Security("is_granted('ROLE_FRIENDLY_USER', author'])")]
public function showAction(User $author): Response
#[IsGranted(attribute: new Expression("is_granted('ROLE_FRIENDLY_USER', subject['author'])"), subject: ['author'])]
public function showAction(User $author): Response

wkania avatar Apr 19 '23 09:04 wkania

Well its good to have these 2 cases in testfixure

johanadivare avatar Apr 26 '23 12:04 johanadivare

@wkania Why not replace

#[Security("is_granted('ROLE_FRIENDLY_USER', author'])")]
public function showAction(User $author): Response

with

#[IsGranted('ROLE_FRIENDLY_USER', 'author')]
public function showAction(User $author): Response

pepeh avatar Oct 24 '23 11:10 pepeh

@pepeh Yes, you are right. My example was to simple. In my opinion, these are the valid cases and probably many more:

-------------------
#[Security("is_granted('ROLE_ADMIN')")]
#[IsGranted('ROLE_ADMIN')]

-------------------
#[Security("is_granted('ASSET_EDIT', asset)")]
#[IsGranted('ASSET_EDIT', 'asset')]

-------------------
#[Security("is_granted('ORGANIZATION_VIEW', team.getOrganization())")]
#[IsGranted(attribute: new Expression("is_granted('ORGANIZATION_VIEW', subject['team'].getOrganization())"), subject: ['team'])]
// or
#[IsGranted('ORGANIZATION_VIEW', subject: new Expression('args["team"].getOrganization()'))]

// This will throw exception
#[IsGranted('ORGANIZATION_VIEW', 'team.getOrganization')]
Error: Could not find the subject "team.getOrganization" for the #[IsGranted] attribute. Try adding a "$team.getOrganization" argument to your controller method.

-------------------
#[Security("is_granted('ROLE_ADMIN') or type == 'new'")]
#[IsGranted(attribute: new Expression("is_granted('ROLE_ADMIN') or subject['type'] == 'new'"), subject: ['type'])]

-------------------
#[Security("is_granted('ROLE_ADMIN') or asset.getOrganization() == user.getOrganization()")]
#[IsGranted(attribute: new Expression("is_granted('ROLE_ADMIN') or subject['asset'].getOrganization() == user.getOrganization()"), subject: ['asset'])]
// user is one of the special variables

Rector could skip everything with "dot", "or" and "and". Those cases are probably to complex to be refactored automatically or with easy set of conditions.

wkania avatar Nov 10 '23 12:11 wkania