Security must send 403 instead 404

[llins]

I did some overrride to send 403 for a security issue instead 404.

If you think is good...

public class JAASRoles extends HttpCondition {

    private final Collection<String> roles;

    /**
     * Create a new {@link JAASRoles} condition specifying the roles of which
     * the current user must be a member for evaluation to return
     * <code>true</code>.
     */
    public static JAASRoles hasntRoles(String... roles) {
        return new JAASRoles(roles);
    }

    private JAASRoles(String[] roles) {
        this.roles = Arrays.asList(roles);
    }

    @Override
    public boolean evaluateHttp(HttpServletRewrite event, EvaluationContext context) {
        HttpServletRewrite rewrite = event;

        boolean hasAllRoles = true;

        // check if user has all required roles
        for (String role : roles) {
            if (!rewrite.getRequest().isUserInRole(role)) {
                hasAllRoles = false;
            }
        }

        return !hasAllRoles;
    }

}

@Inherited
@Documented
@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
public @interface RolesRequired {

    /**
     * The roles required for the rule to match.
     */
    String[] value();

    /**
     * Security rule priority.
     * Default 99 - before annotations priority  
     */
    int priority() default 99;

}

public class RolesRequiredHandler implements AnnotationHandler<RolesRequired> {

    private Logger logger = LoggerFactory.getLogger(getClass());

    @Override
    public Class<RolesRequired> handles() {
        return RolesRequired.class;
    }

        /**
     * Needs high priority than {@link JoinHandler}
     */
    @Override
    public int priority() {
        return HandlerWeights.WEIGHT_TYPE_STRUCTURAL -1;
    }

    @Override
    public void process(ClassContext context, RolesRequired annotation, HandlerChain chain) {

        Join join = context.getJavaClass().getAnnotation(Join.class);

        if(join != null){
            context.setBaseRule(org.ocpsoft.rewrite.servlet.config.rule.Join.path(join.path()).to(join.to()).withChaining());
            context.getRuleBuilder().withPriority(annotation.priority());
            context.getRuleBuilder().when(JAASRoles.hasntRoles(annotation.value()));
            context.getRuleBuilder().perform(SendStatus.error(403));
        }else{
            logger.warn(String.format("SECURITY VULNERABILITY: The class %s must have org.ocpsoft.rewrite.annotation.Join annotation"));
        }

        chain.proceed();
    }
}

[chkal]

Hmmm. Basically I agree with you. Rewrite should create a 403 instead of a 404 in this case.

However, I'm not sure your code will work correctly in all cases. Especially because you are creating another join in this handler. This should only be done in the JoinHandler. Your handler is basically combining the functionality of the RolesRequiredHandler and the JoinHandler.

I'll keep this issue open. I was planing to refactor some parts of the annotation handling code in the near future. I will consider this use case when I do so. I hope that it will be easier to implement after this refactoring.

[llins]

Actually, i did a minor fix.

My RolesRequiredHandler needs to have a high priority than the JoinHandler, because with the same priority sometimes the @RequestAction was not executing for the @Join.

My RolesRequiredHandler and the JoinHandler will process and so far we did not found any problem.