Multiple Annotations Not Working

[jeremylivingston]

It appears that when using multiple annotations on the same method, only the last one is taken into consideration. See the below example:

    /**
     * @Secure(roles="ROLE_USER")
     * @PreAuthorize("!hasRole('ROLE_EMAIL_VERIFIED')")
     */
    public function verifyAction(Request $request)
    {
        // Action code
    }

This method only takes the PreAuthorize call into account. Users who don't have ROLE_USER are allowed to call this action.

This should be fixed or documented somewhere if it is the expected behavior.

[craue]

I've just noticed this behavior as well using multiple @PreAuthorize:

/**
 * @PreAuthorize("...")
 */
class MyController {

    /**
     * @PreAuthorize("...")
     */
    public function myAction() {
    }

}

/**
 * @PreAuthorize("...")
 * @PreAuthorize("...")
 */
public function myAction() {
}

In both cases, only the last specified annotation is actually active.

This looks like a bug to me, not something that just needs to be documented.

[schmittjoh]

This is expected actually, you can only add one @PreAuthorize annotation.

Maybe you can elaborate your use case a bit because at the moment I don't see a need to allow this as the expression syntax so flexible that you can build arbitrary complex things with just one expression.

[jeremylivingston]

@schmittjoh Do you agree that my example is a bug or something that should be documented? It seems that the use of both @Secure and @PreAuthorize doesn't take both into consideration.

[craue]

@schmittjoh: My first example is analogous to what can be done with @Route:

/**
 * @Route("/admin")
 * @PreAuthorize("isAdmin()")
 */
class MyController {

    /**
     * @Route("/manage-users/", name="...")
     * @PreAuthorize("isAllowedToManageUsers()")
     */
    public function myAction() {
    }

}

This is quite intuitive for routes, and as well would be for auth checks. You could argue that this can be done with the security.access_control config, but I prefer to just use one approach for defining those checks, which makes them easier to maintain.

The second example just looks better to me, i.e. using one rule per line than conjuncting them all in one long line. I agree that this is personal preference, but you can have several @Route annotations on one method as well.

That the example @jeremylivingston gave doesn't work is even worse. One would expect that both (different) annotations are considered. Just ignoring all but the last one makes using them very counter intuitive.

[schmittjoh]

I think we need to mark this as a documentation issue only or/and add an exception when @PreAuthorize is used in combination with other annotations.

On one hand, this behavior exists for too long to change it at this point. On the other hand, I also think that it makes sense as it is at the moment. I do not see how we can augment a security access rule in a general fashion, f.e. when would you use conjunction (&&) vs. disjunction (||)? Personally, I use the class-level annotation when I want to secure an entire class and then overwrite it for specific actions where I want to have something different.

On the bright side, you can easily achieve what you want by using:

@PreAuthorize("isAdmin() && isAllowedToManageUsers()")
@PreAuthorize("hasRole('ROLE_USER') && ! hasRole('ROLE_EMAIL_VERIFIED')")

[craue]

@schmittjoh: Good point about the use case of overriding/invalidating auth checks in hierarchy. But if I'd need different rules anyway, I wouldn't define a general one on controller level, as with the prefix for @Route. Maybe these annotations (@Secure, @PreAuthorize, are there more?) could have a new parameter which defines that higher level rules should be either inherited or overridden.

[noemi-salaun]

Up

All routing annotation like @Symfony\Component\Routing\Annotation\Route or @FOS\RestBundle\Controller\Annotations\Get are ignored when used with @JMS\SecurityExtraBundle\Annotation\Secure

[smedelyan]

@schmittjoh Suppose, I have following meta-annotation:

/**
 * Container for annotations to check access to different stuff before letting user call
 * an actual controller method
 */
@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
public @interface CheckAccess {
    
    @Target(ElementType.METHOD)
    @Retention(RetentionPolicy.RUNTIME)
    @PreAuthorize("@authenticationService.canRead(#someEntityId)")
    @interface CanReadEntity {
    }

    @Target(ElementType.METHOD)
    @Retention(RetentionPolicy.RUNTIME)
    @PreAuthorize("@authenticationService.isDeveloper()")
    @interface OnlyDevelopers {
    }

    /* etc*/
}

The purpose of this annotation is to delegate auth checks to some service with complex logic (say, it queries db to get list of permissions or may compute somethings very dynamically).

Next I'd like to compose these two checks on controller method like:

@CheckAccess.CanReadEntity
@CheckAccess.OnlyDevelopers
public void doSmth(/*...*/) {/*...*/}

How would I achieve that without cumbersome workarounds (and without writing @PreAuthorize(...) in-place which is ugly and not-maintainable)?

P. S. Yet it would still be hard to implement And()-s, Or()-s etc=)

EDIT

Well, I think there is a possibility to achieve what I want:

@PreAuthorize(CHECK_CAN_READ_ENTITY + " && " +
CHECK_ONLY_DEVELOPERS
/*as many checks and as complex logic as you want*/ )
public void doSmth(/*...*/) {/*...*/}

This seems to be simple and powerful enough. I'll leave the comment as is for history.