Uploaded image for project: 'Sakai'
  1. Sakai
  2. SAK-39552

Improve SecurityAdvisor API to prevent missuse

    Details

    • Type: Feature Request
    • Status: CLOSED
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.x
    • Component/s: Kernel
    • Labels:
    • Previous Issue Keys:
      KNL-542

      Description

      Currently it is quite easy to abuse the security advisor API and end up leaving your advisor on the stack or removing too many advisors (other peoples).

      The currently recommended pattern is:

      try {
      securityService.pushAdvisor(new SecurityAdvisor(....));
      } finally {
      securityService.popAdvisor();
      }

      the problem is that if you get an exception when adding your advisor or in your try block before you've added it you may remove someone elses advisor.

      I think securityService.popAdvisor(SecurityAdvisor) should be used instead so you hold a reference to the advisor locally:

      SecurityAdvisor advisor = new SecurityAdvisor(){};
      try {
      securityService.pushAdvisor(advisor);
      } finally {
      securityService.popAdvisor(advisor);
      }

      and if the advisor you're trying to pop doesn't match the one on the top of the stack it gets logged and not removed.

      And finally a better pattern is:

      securityService.runWithAdvisor( new SecurityAdvisor(){}, new Runnable() {} );

      this is a misuse of Runnable (should probably create our own interface), but you get the idea. This way runWithAdvisor() gets to handle all the push/pop in a try finally.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

                People

                • Assignee:
                  dhorwitz David Horwitz
                  Reporter:
                  buckett Matthew Buckett
                • Votes:
                  0 Vote for this issue
                  Watchers:
                  1 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:

                    Git Source Code