sensiolabs
@Security annotation User parameter is overridden by authenticated user
If I have the following controller action setup
/**
* @Securty("is_granted('USER_MANAGE', user)")
*/
public function indexAction(User $user)
{
}
The authenticated user will be passed into the voter instead of the user parameter. This can easily be resolved by renaming the parameter. This may be expected behaviour, but at the minimum, should the documentation be updated to better reflect that would be the intended outcome?
The doc must be updated for this to include a warning. We cannot change this behavior as it would be a BC break.
BC break with 4.0 you mean? Two possibilities: 4.0 has been released a couple of days ago, we release 4.0.1 and be done with it. Or if we want to be very strict, we release 5.0.
Well, it would be a BC break with 3.x too. And this would be one where we cannot warn about it easily. It would require checking the expression to know whether it uses the user variable (no need to warn otherwise) and then offer a way to opt-in for the new behavior (continuous migration path) on a per-annotation basis.
and this would not remove the fact that some variable names are reserved and won't ever be set based on request attributes. Among variables defined in https://github.com/sensiolabs/SensioFrameworkExtraBundle/blob/74f1f17e1e5c32ca43de92611e9097a8d0e0acf2/EventListener/SecurityListener.php#L89-L98, the only one we can unreserve is user, as this one was provided only for user convenience to access the current user easily (rather than doing token.getUser()). All others must keep their known values as functions exposed in the expression language are relying on them.
this is the issue when the list of variables passed to the expression is a mix between predefined variables and custom ones (the request attributes), as we can then have conflicts.
IsGranted does not suffer from that, as it does not rely on a user provided expression.