Spring Security
  1. Spring Security
  2. SEC-1945

Encapsulation of Pre/PostAuthorize expressions in custom annotations and combining them on methods

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 3.1.0
    • Fix Version/s: None
    • Component/s: Core
    • Labels:
      None

      Description

      I would like to raise an idea for further discussion. When using PreAuthorize / PostAuthorize annotations on methods in a larger scale there is a lot of redundancy and repetition of some expression parts. This makes code less readable and might introduce bugs just because developer looses perspective on the code. I described my idea in more detail in article on my blog and would be happy if it would inspire you somehow to extend current features of Spring Security. If you wouldn't consider it to be a good idea, please close this issue immediately.

      http://blog.novoj.net/2012/03/27/combining-custom-annotations-for-securing-methods-with-spring-security/

      Thank you for your time.

      Might be connected in certain aspects with these issues:
      https://jira.springsource.org/browse/SEC-1629
      https://jira.springsource.org/browse/SEC-1796

        Activity

        Hide
        Rob Winch added a comment -

        You can easily do something similar by using static methods in your Spring Expressions. This is made even easier by the new support for custom expressions that was done as a part of SEC-1887. With this in mind, I do not think this proposal is that appealing as there are other ways of centralizing the logic that are more consistent with the existing functionality.

        Show
        Rob Winch added a comment - You can easily do something similar by using static methods in your Spring Expressions. This is made even easier by the new support for custom expressions that was done as a part of SEC-1887 . With this in mind, I do not think this proposal is that appealing as there are other ways of centralizing the logic that are more consistent with the existing functionality.
        Hide
        Jan Novotný added a comment -

        Can you please attach some examples? Static methods in SpEL won't make rules more readable - I think it may even make it worse. For example:

        T(com.my.package.and.class.Name).preconditionFullfilled and T(com.my.package.and.class.Name).anotherPreconditionFullfilled

        Regarding SEC-1887 - if I understand it well it requires programmatic extension to the SpEL that allows using custom objects (and thus may shorten the expression above) in expressions. This would make it better for sure - but you still have no means of structured search that annotations have. Introducing annotations is also way easier than making EL extension IMHO.

        But I see my idea is made from my point of view and different people may have different views on the same thing. Nevertheless I'm already using this proposed extension in our projects (so far it looks fine) and there is no high need to have this implemented in core Security library - it may only make some things better, nothing more.

        Show
        Jan Novotný added a comment - Can you please attach some examples? Static methods in SpEL won't make rules more readable - I think it may even make it worse. For example: T(com.my.package.and.class.Name).preconditionFullfilled and T(com.my.package.and.class.Name).anotherPreconditionFullfilled Regarding SEC-1887 - if I understand it well it requires programmatic extension to the SpEL that allows using custom objects (and thus may shorten the expression above) in expressions. This would make it better for sure - but you still have no means of structured search that annotations have. Introducing annotations is also way easier than making EL extension IMHO. But I see my idea is made from my point of view and different people may have different views on the same thing. Nevertheless I'm already using this proposed extension in our projects (so far it looks fine) and there is no high need to have this implemented in core Security library - it may only make some things better, nothing more.
        Hide
        Petro Semeniuk added a comment -

        Hi there, I came around Jan's blog post and suggested implementation makes security configuration more readable in our project as well. Security rules are quite hairy, it's not that easy to unit test SPEL expression and autocomplete doesn't work with them . It would be really nice to encapsulate them in annotation and combine later

        I wonder if in addition to proposed syntax
        @RulesRelation(BooleanOperation.AND)
        @IsAuthenticatedFully
        @AllowedForOrganizationOwner

        It's possible to have something semantically equivalent to:
        @PreAuthorizeAny(

        {@AdminRole, @BillingRole, ...}

        ) //logical OR

        @PreAuthorizeAll(

        {@SupportRole, @BillingRole, ...}

        ) //logical AND

        Show
        Petro Semeniuk added a comment - Hi there, I came around Jan's blog post and suggested implementation makes security configuration more readable in our project as well. Security rules are quite hairy, it's not that easy to unit test SPEL expression and autocomplete doesn't work with them . It would be really nice to encapsulate them in annotation and combine later I wonder if in addition to proposed syntax @RulesRelation(BooleanOperation.AND) @IsAuthenticatedFully @AllowedForOrganizationOwner It's possible to have something semantically equivalent to: @PreAuthorizeAny( {@AdminRole, @BillingRole, ...} ) //logical OR @PreAuthorizeAll( {@SupportRole, @BillingRole, ...} ) //logical AND
        Hide
        Jan Novotný added a comment -

        @PetroSemeniuk - I tried to find out how it would be possible to compose custom annotations as you suggested too in time of writing the article. Problem is in Java annotation system itself - you cannot define this:

        public @interface SecurityRules

        { Annotation[] value(); }

        Because Annotation class is invalid for usage in annotations. Moreover annotation cannot implement any interface to aggregate custom annotations into the type safe pack such as:

        public @interface SecurityRules

        { SecurityAnnotation[] value(); }

        So as far as I know, you cannot group unknown annotations inside another annotation. It's a pitty.

        Show
        Jan Novotný added a comment - @PetroSemeniuk - I tried to find out how it would be possible to compose custom annotations as you suggested too in time of writing the article. Problem is in Java annotation system itself - you cannot define this: public @interface SecurityRules { Annotation[] value(); } Because Annotation class is invalid for usage in annotations. Moreover annotation cannot implement any interface to aggregate custom annotations into the type safe pack such as: public @interface SecurityRules { SecurityAnnotation[] value(); } So as far as I know, you cannot group unknown annotations inside another annotation. It's a pitty.

          People

          • Assignee:
            Unassigned
            Reporter:
            Jan Novotný
          • Votes:
            6 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated: