SEC-1945: Encapsulation of Pre/PostAuthorize expressions in custom annotations and combining them on methods #2171

Open
spring-issuemaster opened this Issue Mar 27, 2012 · 4 comments

Projects

None yet

1 participant

@spring-issuemaster

Jan Novotný (Migrated from SEC-1945) said:

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

@spring-issuemaster

Rob Winch said:

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.

@spring-issuemaster

Jan Novotný said:

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.

@spring-issuemaster

Petro Semeniuk said:

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

@spring-issuemaster

Jan Novotný said:

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment