1

I have defined this class aspect that is working fine for the services (don't allow access unless the user has the role MANAGER, but there is no restricton for the controller (?)

@Aspect
public class DeviceAspect extends ServiceSupport {


    @Pointcut("execution(* fo.belecam.services.client.ManageLicenseService.*(..))")
    public void manage() {
    }

    @Pointcut("execution(* fo.belecam.services.client.AwardService.*(..))")
    public void award() {
    }

    @Pointcut("execution(* fo.belecam.services.client.DeviceService.*(..))")
    public void handleDeviceServiceMethod() {
    }

    @Pointcut("execution(* fo.belecam.controller.manage.ImportController.*(..))")
    public void handleImportController() {
    }


    @Before("fo.belecam.services.aop.DeviceAspect.handleImportController() || fo.belecam.services.aop.DeviceAspect.handleDeviceServiceMethod() || fo.belecam.services.aop.DeviceAspect.manage() || fo.belecam.services.aop.DeviceAspect.award()")
    @After ("fo.belecam.services.aop.DeviceAspect.handleImportController() || fo.belecam.services.aop.DeviceAspect.handleDeviceServiceMethod() || fo.belecam.services.aop.DeviceAspect.manage() || fo.belecam.services.aop.DeviceAspect.award()")
    public void before(JoinPoint _jp) {
        User user = getUser();
        if(user == null || user.getUserRole() != UserRole.MANAGER) {
            throw new NoSufficientRoleException(user == null ? null : user.getUserRole(), UserRole.MANAGER);
        }
    }

}

and the ImportController:

@SuppressWarnings("deprecation")
public class ImportController extends AbstractFormController {


    private String view;

    private String successView;

    @Autowired
    protected UserService userService;

    @Autowired
    private ManageDeviceService manageDeviceService;

    public String getView() {
        return view;
    }

    public void setView(String view) {
        this.view = view;
    }

    public String getSuccessView() {
        return successView;
    }

    public void setSuccessView(String successView) {
        this.successView = successView;
    }


    @Override
    public ModelAndView processFormSubmission(final HttpServletRequest request,
            HttpServletResponse response, Object command, BindException errors)
            throws Exception {

        final ModelAndView mav = new ModelAndView(getView());

        FileUploadCommand file = (FileUploadCommand)command;

        MultipartFile multipartFile = file.getFile();

        if(multipartFile!=null && multipartFile.getSize()>0) {
            Workbook workbook = Workbook.getWorkbook(multipartFile.getInputStream());
            DataCollector dataCollector = new XLSDataCollector(workbook, true);
            final List<Application> applications = manageDeviceService.loadApplications (dataCollector.getDataCollection());
            List<ApplicationImporterError> importationErrors = manageDeviceService.validateApplications(applications);
            savedApplications.add(manageDeviceService.getApplicationById(application.getId(), true));


        }
        return mav;
    }

    @Override
    public ModelAndView showForm(HttpServletRequest request, HttpServletResponse arg1, BindException errors)
            throws Exception {

        return new ModelAndView(getView());
    }



    }

    /**
     * @param applications
     * @param competentBody
     * @return
     * @throws Exception
     */
    private List<Application> saveApplications(List<Application> applications,User user) throws Exception {

        return manageDeviceService.saveImportedApplications (applications, user);
    }

    /**
     * @param session
     * @return
     */
    public User getUser(HttpSession session) {

        User user = (User) session.getAttribute(Const.SESSION_USER);
        if (user == null) {
            user = new User();
        }
        return user;
    }
}

When I logged as UserRole.MANAGER the method before(JoinPoint _jp) is invoked, otherwise is not

I see, the method before(JoinPoint _jp) is not invoked when I just paste the URL in the browser .... http://127.0.0.1:7001/devices/manage/import.do

Nuñito Calzada
  • 4,394
  • 47
  • 174
  • 301
  • 1
    You have tagged the question with "spring-security", so why don't you use Spring security for that? – a better oliver Oct 02 '15 at 08:49
  • Just a word of advice - I would consider obscuring your company's package names in any sample code you provide. – Tom Bunting Oct 05 '15 at 08:20
  • 1
    Not an answer but just another suggestion - as your named pointcuts are in the same class as your `@Before` method, you don't need to fully qualify them in the pointcut expression - `@Before("handleImportController() || handleDeviceServiceMethod() || etc)` should work and is more concise / readable. – Tom Bunting Oct 05 '15 at 08:58
  • 1
    have you tried debugging to see if the `@Before` method is actually being invoked, or whether the `user` object is coming back null from `getEcasUser()` for some reason - in either case, you won't know as there's no action taken unless the user is not in the `MANAGER` role. – Tom Bunting Oct 06 '15 at 07:57
  • Could you post the code of your ImportController? – Ruben Oct 06 '15 at 08:07
  • I see you edited the question to make the controller methods `public` - did that have any impact? – Tom Bunting Oct 07 '15 at 16:19
  • no, unfortunately same result :-( – Nuñito Calzada Oct 07 '15 at 20:51

3 Answers3

1

A limitation of Spring's dynamic-proxy based AOP is that it can only advise public join points - from the docs:

http://docs.spring.io/spring/docs/current/spring-framework-reference/html/aop.html

Due to the proxy-based nature of Spring’s AOP framework, protected methods are by definition not intercepted, neither for JDK proxies (where this isn’t applicable) nor for CGLIB proxies (where this is technically possible but not recommendable for AOP purposes). As a consequence, any given pointcut will be matched against public methods only! If your interception needs include protected/private methods or even constructors, consider the use of Spring-driven native AspectJ weaving instead of Spring’s proxy-based AOP framework. This constitutes a different mode of AOP usage with different characteristics, so be sure to make yourself familiar with weaving first before making a decision.

A number of your controller methods are protected, so although your controller is registered as a Spring bean (presumably) - the non-public methods will not be advised.

Tom Bunting
  • 1,845
  • 16
  • 24
1

I had the same problem where advice for Repository was working, but advice for Controller was not. Finally I found a solution. In short, you need to make sure your AOP definition is loaded in Servlet context, not a different context.

In my case, my Spring AOP definition is defined in tools-config.xml. After moving it from here

<context-param>
    <param-name>contextConfigLocation</param-name>
    <param-value>classpath:spring/tools-config.xml</param-value>
</context-param>

<listener>
    <listener-class>org.springframework.web.context.ContextLoaderListener</listener-class>
</listener>

to here,

<servlet>
    <servlet-name>petclinic</servlet-name>
    <servlet-class>org.springframework.web.servlet.DispatcherServlet</servlet-class>
    <init-param>
        <param-name>contextConfigLocation</param-name>
        <param-value>classpath:spring/mvc-core-config.xml, classpath:spring/tools-config.xml</param-value>
    </init-param>
    <load-on-startup>1</load-on-startup>
</servlet>

the advice for Controller is working.

xli
  • 2,420
  • 2
  • 20
  • 27
0

The problem may be in that the controller object itself calls the methods you're trying to intercept (they were protected after all originally), a direct call from the abstract controller to one of its overriden methods. The way Spring AOP works is it creates a proxy around your object that intercepts the methods, but that only works if that method is called from outside via a Spring-injected dependency. That is not the case here - since the method is called from within its own object, the call bypasses any proxies that wrap the object for the "outside world".

Jiri Tousek
  • 12,211
  • 5
  • 29
  • 43